Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat/603: remove Auth0 related features #310

Merged
merged 6 commits into from
Jul 26, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,10 @@
"scripts": {
"lint": "phpcs",
"lint-fix": "phpcbf"
},
"config": {
"allow-plugins": {
"dealerdirect/phpcodesniffer-composer-installer": true
}
}
}
135 changes: 0 additions & 135 deletions includes/classes/SiteAutomation/Auth.php

This file was deleted.

41 changes: 21 additions & 20 deletions includes/classes/SiteAutomation/Request.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,6 @@
*/
class Request {

/**
* Auth object which manages access_token.
*
* @var Auth $auth
*/
private $auth;

/**
* Site Automation API URL.
*
Expand Down Expand Up @@ -52,11 +45,8 @@ class Request {
/**
* Class constructor.
*
* @param Auth $auth Authentication object.
*/
public function __construct( $auth ) {
$this->auth = $auth;

public function __construct() {
add_action(
'sophi_retry_get_curated_data',
[ $this, 'do_cron' ],
Expand Down Expand Up @@ -213,16 +203,11 @@ private function set_status( $data ) {
* @return mixed WP_Error on failure or body request on success.
*/
private function request( $timeout ) {
$access_token = $this->auth->get_access_token();

if ( is_wp_error( $access_token ) ) {
return $access_token;
}

$args = [
'headers' => [
'Content-Type' => 'application/json',
'Authorization' => 'Bearer ' . $access_token,
'Cache-Control' => 'no-cache',
],
];

Expand Down Expand Up @@ -309,13 +294,29 @@ private function process( $response, $bypass_cache, $post_id ) {
*
* @return string
*/
private function set_api_url( $page, $widget ) {
private function set_api_url( $page = '', $widget = '' ) {
$site_automation_url = get_sophi_settings( 'site_automation_url' );
$site_automation_url = untrailingslashit( $site_automation_url );
$host = get_sophi_settings( 'host' );
$tenant_id = get_sophi_settings( 'tenant_id' );

$url = sprintf(
'%1$s/%2$s/hosts/%3$s/pages/%4$s',
$site_automation_url,
$tenant_id,
$host,
$page
);

$host = wp_parse_url( get_home_url(), PHP_URL_HOST );
if ( ! empty ( $widget ) ) {
$url = sprintf(
'%1$s/widgets/%2$s',
$url,
$widget
);
}

return sprintf( '%1$s/curatedHosts/%2$s/curator?page=%3$s&widget=%4$s', $site_automation_url, $host, $page, $widget );
return $url . '.json';
}

/**
Expand Down
3 changes: 1 addition & 2 deletions includes/classes/SiteAutomation/Services.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ class Services {
* Register services needs for Site Automation.
*/
public function register() {
$this->auth = new Auth();
$this->request = new Request( $this->auth );
$this->request = new Request();
$this->integration = new Integration( $this->request );
}

Expand Down
51 changes: 16 additions & 35 deletions includes/functions/settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

namespace SophiWP\Settings;

use SophiWP\SiteAutomation\Auth;
use function SophiWP\Utils\get_domain;
use function SophiWP\Utils\is_configured;

Expand Down Expand Up @@ -156,24 +155,24 @@ function fields_setup() {
);

add_settings_field(
'client_id',
__( 'Client ID', 'sophi-wp' ),
'host',
__( 'Host', 'sophi-wp' ),
__NAMESPACE__ . '\render_input',
SETTINGS_GROUP,
'sophi_api',
[
'label_for' => 'client_id',
'label_for' => 'host',
]
);

add_settings_field(
'client_secret',
__( 'Client Secret', 'sophi-wp' ),
'tenant_id',
__( 'Tenant ID', 'sophi-wp' ),
__NAMESPACE__ . '\render_input',
SETTINGS_GROUP,
'sophi_api',
[
'label_for' => 'client_secret',
'label_for' => 'tenant_id',
]
);

Expand Down Expand Up @@ -232,8 +231,8 @@ function get_default_settings( $key = '' ) {
'environment' => $default_environment,
'collector_url' => 'collector.sophi.io',
'tracker_client_id' => get_domain(),
'client_id' => '',
'client_secret' => '',
'host' => '',
'tenant_id' => '',
'site_automation_url' => '',
'query_integration' => 1,
];
Expand All @@ -259,32 +258,6 @@ function sanitize_settings( $settings ) {
$settings['query_integration'] = 0;
}

$prev_settings = get_option( SETTINGS_GROUP );

// Delete the option that holds the access token when the environment is changed
if ( ! empty( $prev_settings['environment'] ) && ! empty( $settings['environment'] ) && $prev_settings['environment'] !== $settings['environment'] ) {
delete_option( 'sophi_site_automation_access_token' );
}

if ( ! empty( $settings['client_id'] && ! empty( $settings['client_secret'] ) ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we may still want some validation that Host/Tenant ID are entered when someone tries to save the form.

$auth = new Auth();
$auth->set_environment( $settings['environment'] );
$response = $auth->request_access_token( $settings['client_id'], $settings['client_secret'] );
if ( is_wp_error( $response ) ) {
add_settings_error(
SETTINGS_GROUP,
SETTINGS_GROUP,
$response->get_error_message()
);
}
} else {
add_settings_error(
SETTINGS_GROUP,
SETTINGS_GROUP,
__( 'Both Client ID and Client Secret are required for Site Automation integration.', 'sophi-wp' )
);
}

if ( empty( $settings['site_automation_url']) ) {
add_settings_error(
SETTINGS_GROUP,
Expand Down Expand Up @@ -312,6 +285,14 @@ function sanitize_settings( $settings ) {
$settings['collector_url'] = $url;
}

if ( empty( $settings['host'] ) || empty( $settings['tenant_id'] ) ) {
add_settings_error(
SETTINGS_GROUP,
SETTINGS_GROUP,
__( 'Both Host and Tenant ID are required for Site Automation integration.', 'sophi-wp' )
);
}

return $settings;
}

Expand Down
43 changes: 43 additions & 0 deletions sophi.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,49 @@ function() {
}
);

/**
* Method to cleanup settings before setting the new version.
*
* @param string $plugin Full path to the plugin's main file.
*/
function sophi_remove_stale_data( $plugin ) {
/** Return if the plugin is not Sophi. */
if ( 'sophi.php' !== basename( $plugin ) ) {
return;
}

$version_key = 'sophi_version';
$current_version = get_transient( $version_key );

if ( false === $current_version ) {
$current_version = get_option( $version_key, false );

if ( false !== $current_version ) {
set_transient( $version_key, $current_version );
}
}

if ( SOPHI_WP_VERSION === $current_version ) {
return;
}

if ( false === $current_version || version_compare( $current_version, SOPHI_WP_VERSION, '<' ) ) {

/** Cleanup logic before setting the new version of the plugin. */
delete_option( 'sophi_site_automation_access_token' );

$sophi_settings = get_option( 'sophi_settings' );
unset( $sophi_settings['client_id'] );
unset( $sophi_settings['client_secret'] );

update_option( 'sophi_settings', $sophi_settings );
update_option( $version_key, SOPHI_WP_VERSION, true );
set_transient( $version_key, SOPHI_WP_VERSION );
}
}

add_action( 'plugin_loaded', 'sophi_remove_stale_data' );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit worried about the new option in terms of site performance. Is there another way to clean the data without adding a new option?

I think we can remove irrelevant options when updating the Sophi setting. Do you think it's sufficient to clean the old settings?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dinhtungdu I've replaced this logic, can you check?


/**
* Sophi HTTPS notice.
*/
Expand Down