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

LICENS-45 Add auth logic #16

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 14 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
4 changes: 4 additions & 0 deletions docs/features.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,7 @@ The Uplink library should mark a registered plugin as needing an update when a v
#### Plugin page

On the _Plugins_ page in the WP Dashboard, any plugin that has an update available should display the update similar to how WordPress core does it. If a user click on `Update`, the zip files should be fetched from the Stellar Licensing system and installed if their license key is valid.

#### Auth button

Use `Auth::do_auth_html()` function to display auth button
20 changes: 18 additions & 2 deletions src/Uplink/API/Validation_Response.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use StellarWP\Uplink\Config;
use StellarWP\Uplink\Messages;
use StellarWP\Uplink\Resources\Resource;
use StellarWP\Uplink\Utils\Namespaces;

class Validation_Response {
/**
Expand Down Expand Up @@ -313,6 +314,7 @@ public function handle_api_errors() : stdClass {
'id',
'slug',
'version',
'auth_required',
'homepage',
'download_url',
'upgrade_notice',
Expand Down Expand Up @@ -492,8 +494,22 @@ public function to_wp_format() {
}
}

//Other fields need to be renamed and/or transformed.
$info->download_link = isset( $this->response->download_url ) ? $this->response->download_url . '&pu_get_download=1' : '';
if ( empty( $this->response->auth_required ) || $this->resource->has_valid_auth_token( (array) $this->response->origin ) ) {
$info->download_link = isset($this->response->download_url) ? $this->response->download_url . '&pu_get_download=1' : '';
} else {
$url = $this->response->origin->url;
$query_params = [
'callback_uri' => urlencode( sprintf( '%s/%s', get_site_url(), Namespaces::get_hook_name( 'connect', '%TEXTDOMAIN%' ) ) ),
Copy link
Contributor

Choose a reason for hiding this comment

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

This is hard to understand what's going on. I'm not sure I understand why the Namespaces class is necessary?

'refer' => urlencode( wp_get_referer() ),
];

$url = sprintf( '%s/%s?%s', $url, Namespaces::get_hook_name( 'oauth_connect/login' ) , http_build_query( $query_params ) );
$info->api_upgrade = sprintf(
esc_html__( 'Please <a href="%s">authenticate this plugin</a> to receive updates.', '%TEXTDOMAIN%' ),
$url
);
$info->download_link = '';
}

if ( ! empty( $this->author_homepage ) && ! empty( $this->response->author ) ) {
$info->author = sprintf( '<a href="%s">%s</a>', esc_url( $this->author_homepage ), $this->response->author );
Expand Down
137 changes: 137 additions & 0 deletions src/Uplink/Admin/Actions.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
<?php declare(strict_types=1);

namespace StellarWP\Uplink\Admin;

use StellarWP\Uplink\API;
use StellarWP\Uplink\Config;
use StellarWP\Uplink\Resources\Collection;
use StellarWP\Uplink\Site\Data;
use StellarWP\Uplink\Utils\Namespaces;

class Actions {

const QUERY_VAR = 'stellarwp_action';

/**
* Register handle route for connect/disconnect
*
* @since 1.0.1
Copy link
Contributor

Choose a reason for hiding this comment

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

@borkweb how do we want to handle @since comments if the version hasn't been determined yet? We've used this in other libraries:

/**
 * Some really awesome method.
 * 
 * @since TBD
 * 
 * @return void
 */

*
* @return void
*
* @action init
*/
public function register_route() {
add_rewrite_endpoint( 'stellarwp', EP_ROOT, self::QUERY_VAR );
}

/**
* Handle auth connect and disconnect request
*
* @since 1.0.1
*
* @param \WP $wp
*
* @return void
*
* @action parse_request
*/
public function handle_auth_request( $wp ) {
if ( empty( $wp->query_vars[ self::QUERY_VAR ] ) ) {
return;
}

$args = apply_filters( Namespaces::get_hook_name( 'auth/request_args', '%TEXTDOMAIN%' ) , explode( '/', $wp->query_vars[ self::QUERY_VAR ] ) );

if ( ! empty( $args['disconnect'] ) ) { // @phpstan-ignore-line
$this->handle_disconnect();
}

$this->handle_connect( $args );
}

/**
* Remove auth tokens and redirect back to settings page
*
* @since 1.0.1
*/
public function handle_disconnect() {
$license = $this->get_license_object();

do_action( Namespaces::get_hook_name( 'disconnect/before/redirect', '%TEXTDOMAIN%' ), $license );

delete_option( sprintf( '%s%s_auth_token', Namespaces::get_option_name( 'origin', '%TEXTDOMAIN%' ), $license->origin->slug ?? '' ) );

do_action( Namespaces::get_hook_name( 'disconnect/after/redirect', '%TEXTDOMAIN%' ), $license );

wp_safe_redirect( wp_get_referer() );
Copy link
Member

Choose a reason for hiding this comment

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

We should fire an action before the redirect.

exit();
}

/**
* Save auth token and redirect back to referer URL
*
* @since 1.0.1
*
* @param array $args
*/
public function handle_connect( $args ) {
if ( empty( $args['token'] ) ) {
$url = $this->get_origin_url();
if ( empty( $url ) ) {
return;
}

$query_params = [
'callback_uri' => urlencode( sprintf( '%s/%s', get_site_url(), Namespaces::get_hook_name( 'connect', '%TEXTDOMAIN%' ) ) ),
'refer' => urlencode( wp_get_referer() ),
];
$url = sprintf( '%s/%s?%s', $url, Namespaces::get_hook_name( 'oauth_connect/login' ), http_build_query( $query_params ) );

wp_safe_redirect( $url );
Copy link
Member

Choose a reason for hiding this comment

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

We should fire an action before the redirect.

exit();
}

do_action( Namespaces::get_hook_name( 'disconnect/before/save_auth_token', '%TEXTDOMAIN%' ), $args );

Config::get_container()->get( Data::class )->save_auth_token( $args['token'] );

do_action( Namespaces::get_hook_name( 'disconnect/after/save_auth_token', '%TEXTDOMAIN%' ), $args );

wp_safe_redirect( $args['refer'] );
exit();
}

/**
* Retrieve origin URL from server
*
* @since 1.0.1
*
* @return string
*/
protected function get_origin_url() {
$license = $this->get_license_object();
$api = Config::get_container()->get( API\Client::class );
$origin = $api->post('/origin', [ 'slug' => $license->get_slug() ] );

if ( ! empty( $origin ) ) {
return $origin->url . '/stellarwp_connect';
}

return '';
}

/**
* Retrieve License
*
* @since 1.0.1
*
* @return mixed
*/
protected function get_license_object() {
$collection = Config::get_container()->get( Collection::class );
$plugin = $collection->current();

return $plugin->get_license_object();
}
}
44 changes: 44 additions & 0 deletions src/Uplink/Admin/Auth.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<?php declare(strict_types=1);

namespace StellarWP\Uplink\Admin;

use StellarWP\Uplink\Config;
use StellarWP\Uplink\Resources\Collection;
use StellarWP\Uplink\Utils\Namespaces;

class Auth {

/**
* Return auth button html
*
* @since 1.0.1
*
* @return mixed
*/
public static function do_auth_html() {
$collection = Config::get_container()->get( Collection::class );
$plugin = $collection->current();
$license = $plugin->get_license_object();

$token = get_option( sprintf( '%s%s_auth_token', Namespaces::get_option_name( 'origin', '%TEXTDOMAIN%' ), $license->origin->slug ?? '' ), '' );
$message = esc_html__( 'Connect to receive updates', '%TEXTDOMAIN%' );
$classes = [ 'button', 'button-primary' ];
Copy link
Contributor

Choose a reason for hiding this comment

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

Defining the structure here looks too prescriptive for the plugins implementing this. Could we provide a new filter to allow them to override the HTML?

$url = Namespaces::get_hook_name( 'connect', '%TEXTDOMAIN%' );

if ( ! empty( $token ) ) {
$message = esc_html__( 'Disconnect', '%TEXTDOMAIN%' );
$classes = [ 'button', 'button-secondary'];
$url = Namespaces::get_hook_name( 'disconnect', '%TEXTDOMAIN%' );
}

$btn_html = sprintf(
'<a href="%s" class="%s">%s</a>',
$url,
implode( ' ', $classes ),
$message
);
Copy link
Member

Choose a reason for hiding this comment

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

Let's do an apply_filters on the resulting HTML here.


return apply_filters( Namespaces::get_hook_name( 'connect/btn/html', '%TEXTDOMAIN%' ), $btn_html, $url, $classes );
}

}
16 changes: 16 additions & 0 deletions src/Uplink/Admin/Provider.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ public function register_hooks() {
add_action( 'admin_enqueue_scripts', [ $this, 'store_admin_notices' ], 10, 1 );
add_action( 'admin_notices', [ $this, 'admin_notices' ], 10, 0 );
add_action( 'load-plugins.php', [ $this, 'remove_default_update_message' ], 50, 0 );
add_action( 'parse_request', [ $this, 'auth_request' ], 10, 1 );
}

/**
Expand All @@ -62,6 +63,20 @@ public function filter_plugins_api( $result, $action, $args ) {
return $this->container->get( Plugins_Page::class )->inject_info( $result, $action, $args );
}

/**
* Handle auth and disconnect requests to origin
*
* @since 1.0.1
*
* @param \WP $wp
*/
public function auth_request( $wp ) {
if ( ! is_user_logged_in() || ! current_user_can( 'manage_options' ) ) {
return;
}
$this->container->get( Actions::class )->handle_auth_request( $wp );
}

/**
* Filter the plugins transient.
*
Expand Down Expand Up @@ -94,6 +109,7 @@ public function ajax_validate_license() {
* @return void
*/
public function admin_init() {
$this->container->get( Actions::class )->register_route();
$this->container->get( License_Field::class )->register_settings();
}

Expand Down
14 changes: 14 additions & 0 deletions src/Uplink/Resources/Resource.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
use StellarWP\Uplink\Config;
use StellarWP\Uplink\Exceptions;
use StellarWP\Uplink\Site\Data;
use StellarWP\Uplink\Utils\Namespaces;

/**
* The base resource class for StellarWP Uplink plugins and services.
*
Expand Down Expand Up @@ -499,4 +501,16 @@ public function validate_license( $key = null, $do_network_validate = false ) {

return $results;
}

public function has_valid_auth_token( array $origin ) {
$token = get_option( sprintf( '%s%s_auth_token', Namespaces::get_option_name( 'origin', '%TEXTDOMAIN%' ), $origin['slug'] ?? '' ), '' );

if ( empty( $token ) ) {
return false;
}

$token = ! is_array( $token ) ? json_decode( $token, true ) : $token;

return ! empty( $token );
}
}
7 changes: 7 additions & 0 deletions src/Uplink/Site/Data.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use StellarWP\ContainerContract\ContainerInterface;
use StellarWP\Uplink\Config;
use StellarWP\Uplink\Utils\Namespaces;

class Data {
/**
Expand Down Expand Up @@ -518,4 +519,10 @@ public function is_public(): bool {

return (bool) $is_public;
}

public function save_auth_token( string $data ) {
$data = json_decode( base64_decode( $data ), true );
update_option( sprintf( '%s%s_auth_token', Namespaces::get_option_name( 'origin', '%TEXTDOMAIN%' ), $data['origin'] ?? '' ), json_encode( $data ) );
}

}
17 changes: 17 additions & 0 deletions src/Uplink/Utils/Namespaces.php
Copy link
Contributor

@tarecord tarecord Jul 17, 2023

Choose a reason for hiding this comment

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

I don't think this is necessary because it's adding a lot of additional abstraction to hooks (you're having to figure out what the hook is doing by referencing this class).

We've dealt with this a bit in the Telemetry library too where plugins can hook into their own implementation without affecting others.

Instead of adding a prefix to the hook name, we've adjusted to pass the plugin slug as an argument to the hook:

apply_filters( 'your/hook/name', '[plugin-slug]', 'value' );

do_action( 'your/hook/name', '[plugin-slug]' );

Since the plugin slug is passed in the hooks, plugins can validate that the hook is firing for a specific implementation:

add_filter( 'the/hook/name', 'filter_something', 10, 2 );

function filter_something( $plugin_slug, $value ) {
    if ( 'the-events-calendar' !== $plugin_slug) {
        return $value;
    }

    // Do value adjustment here...

    return $value;
}

Once all the hooks are updated to pass in the plugin slug, hook names shouldn't have to change and they'll be simpler to understand in the library 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tarecord maybe I understood the idea for #16 (comment) in wrong way #16 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

You didn't understand wrong, a namespace is perfect when using them for options and endpoints.

I am only referring to the filters/actions you are adding a namespace to.

I did notice that you have Config::get_hook_prefix() available to you. Would that be good to use instead of this class? I imagine brands will want to define their prefix once and have it used everywhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php declare(strict_types=1);

namespace StellarWP\Uplink\Utils;

class Namespaces {

public const DEFAULT_NAME = 'stellarwp';

public static function get_hook_name( string $entity, string $plugin_slug = '' ): string {
return apply_filters( 'stellarwp/namespace/hook_name', sprintf( '%s/%s', $plugin_slug ?: self::DEFAULT_NAME, $entity ), $entity, $plugin_slug );
}

public static function get_option_name( string $entity, string $plugin_slug = '' ): string {
return apply_filters( 'stellarwp/namespace/option_name', sprintf( '%s_%s_', $plugin_slug ?: self::DEFAULT_NAME, $entity ), $entity, $plugin_slug );
}

}
2 changes: 2 additions & 0 deletions src/admin-views/fields/settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* @var bool $show_title Whether to show the title. Default true.
*/

use StellarWP\Uplink\Admin\Auth;
use StellarWP\Uplink\Admin\License_Field;
use StellarWP\Uplink\Config;

Expand All @@ -14,6 +15,7 @@
}

$field = Config::get_container()->get( License_Field::class );
$auth = Config::get_container()->get( Auth::class );
$group = $field->get_group_name( sanitize_title( $plugin->get_slug() ) );

?>
Expand Down
51 changes: 51 additions & 0 deletions tests/wpunit/Resources/ResourceTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?php declare( strict_types=1 );

namespace StellarWP\Uplink\Tests\Resources;

use StellarWP\Uplink\API\Validation_Response;
use StellarWP\Uplink\Register;
use StellarWP\Uplink\Uplink;

class ResourceTest extends \StellarWP\Uplink\Tests\UplinkTestCase {

public $resource;

public function setUp() {
parent::setUp();

$root = dirname( __DIR__, 3 );
$this->resource = Register::plugin(
'sample',
'Lib Sample',
$root . '/plugin.php',
Uplink::class,
'1.0.10',
Uplink::class
);
}

/**
* @test
*/
public function it_should_check_auth_token_valid() {
MlKilderkin marked this conversation as resolved.
Show resolved Hide resolved
$result = $this->resource->has_valid_auth_token( [ 'slug' => 'sample' ] );
$this->assertFalse( $result );

update_option( sprintf( 'stellarwp_origin_%s_auth_token', 'sample' ), [
'token' => '11111',
'origin' => '',
] );
add_filter( 'stellarwp/namespace/option_name', function( $name, $entity, $slug ) {
return 'stellarwp_origin_';
}, 10, 3);

update_option( sprintf( 'stellarwp_origin_%s_auth_token', 'sample' ), [
'token' => '11111',
'origin' => '',
] );

$result = $this->resource->has_valid_auth_token( [ 'slug' => 'sample' ] );
$this->assertTrue( $result );
}

}
Loading