Skip to content

Commit

Permalink
Code quality improvements (#226)
Browse files Browse the repository at this point in the history 
* Code improves and PHP 8.0 compat

* Fix dispatch

* Remove get_class_caller

* More type checks

* More code quality changes

* Improve error handling

* Fix lint

* Fix another PHP error

* Revert some of the changes

* Fix error

* Change type

* Ensure that callback is registered

* Ensure that no cache control headers are sent when using oauth1

* Improve error message

* Fix WP_Error code

* Code improvements

* Fallback to get_called_class

* Also delete request tokens

* Translate string in WP CLI comment

* Update version number
  • Loading branch information
@jonnynews
jonnynews authored Nov 24, 2023
1 parent 78d8055 commit e23038c
Show file tree
Hide file tree
Showing 10 changed files with 245 additions and 99 deletions.
7 changes: 4 additions & 3 deletions admin.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
function rest_oauth1_profile_section( $user ) {
global $wpdb;

$results = $wpdb->get_col( "SELECT option_value FROM {$wpdb->options} WHERE option_name LIKE 'oauth1_access_%'", 0 );
$results = $wpdb->get_col( "SELECT option_value FROM $wpdb->options WHERE option_name LIKE 'oauth1_access_%'" );
$approved = array();
foreach ( $results as $result ) {
$row = unserialize( $result );
Expand All @@ -31,8 +31,6 @@ function rest_oauth1_profile_section( $user ) {
}
}

$authenticator = new WP_REST_OAuth1(); // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable

?>
<table class="form-table">
<tbody>
Expand All @@ -51,6 +49,9 @@ function rest_oauth1_profile_section( $user ) {
<?php foreach ( $approved as $row ) : ?>
<?php
$application = get_post( $row['consumer'] );
if ( ! $application ) {
continue;
}
?>
<tr>
<td><?php echo esc_html( $application->post_title ); ?></td>
Expand Down
37 changes: 25 additions & 12 deletions lib/class-wp-rest-client.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,20 @@
* REST Client.
*/
abstract class WP_REST_Client {

/**
* Post object.
*
* @var WP_Post
*/
protected $post;

/**
* Get the client type.
*
* Must be overridden in subclass.
*
* @return string
* @return string | WP_Error
*/
protected static function get_type() {
return new WP_Error( 'rest_client_missing_type', __( 'Overridden class must implement get_type', 'rest_oauth1' ) );
Expand Down Expand Up @@ -126,7 +134,8 @@ public static function get( $id ) {
return new WP_Error( 'rest_oauth1_invalid_id', __( 'Client ID is not valid.', 'rest_oauth1' ), array( 'status' => 404 ) );
}

$class = function_exists( 'get_called_class' ) ? get_called_class() : self::get_called_class();
$class = get_called_class();

return new $class( $post );
}

Expand All @@ -137,8 +146,7 @@ public static function get( $id ) {
* @return WP_Post|WP_Error
*/
public static function get_by_key( $key ) {
$class = function_exists( 'get_called_class' ) ? get_called_class() : self::get_called_class();
$type = call_user_func( array( $class, 'get_type' ) );
$type = call_user_func( array( get_called_class(), 'get_type' ) );

$query = new WP_Query();
$consumers = $query->query(
Expand All @@ -159,8 +167,7 @@ public static function get_by_key( $key ) {
);

if ( empty( $consumers ) || empty( $consumers[0] ) ) {
$code = is_user_logged_in() ? 403 : 401;
return new WP_Error( 'json_consumer_notfound', __( 'Consumer Key is invalid', 'rest_oauth1' ), array( 'status' => $code ) );
return new WP_Error( 'json_consumer_notfound', __( 'Consumer Key is invalid', 'rest_oauth1' ), array( 'status' => 401 ) );
}

return $consumers[0];
Expand All @@ -170,11 +177,11 @@ public static function get_by_key( $key ) {
* Create a new client.
*
* @param array $params { .
* @type string $name Client name
* @type string $description Client description
* @type array $meta Metadata for the client (map of key => value)
* @type string $name Client name
* @type string $description Client description
* @type array $meta Metadata for the client (map of key => value)
* }
* @return WP_Post|WP_Error
* @return WP_REST_Client|WP_Error
*/
public static function create( $params ) {
$default = array(
Expand All @@ -194,12 +201,12 @@ public static function create( $params ) {
return $id;
}

$class = function_exists( 'get_called_class' ) ? get_called_class() : self::get_called_class();
$meta = $params['meta'];
$class = get_called_class();
$meta['type'] = call_user_func( array( $class, 'get_type' ) );

// Allow types to add their own meta too.
$meta = $class::add_extra_meta( $meta, $params );
$meta = call_user_func( array( $class, 'add_extra_meta' ), $meta, $params );

/**
* Add extra meta to the consumer on creation.
Expand Down Expand Up @@ -235,9 +242,15 @@ protected static function add_extra_meta( $meta, $params ) { //phpcs:ignore Vari
/**
* Shim for get_called_class() for PHP 5.2
*
* @deprecated 0.4.0
* @return string Class name.
*/
protected static function get_called_class() {
_deprecated_function( __METHOD__, '0.4.0', 'get_called_class()' );
if ( function_exists( 'get_called_class' ) ) {
return get_called_class();
}

// PHP 5.2 only.
$backtrace = debug_backtrace();
// [0] WP_REST_Client::get_called_class()
Expand Down
66 changes: 34 additions & 32 deletions lib/class-wp-rest-oauth1-admin.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ public static function register() {
*/
include_once __DIR__ . '/class-wp-rest-oauth1-listtable.php';

$class = get_class();

$hook = add_users_page(
// Page title.
__( 'Registered OAuth Applications', 'rest_oauth1' ),
Expand All @@ -34,10 +36,10 @@ public static function register() {
// Menu slug.
self::BASE_SLUG,
// Callback.
array( get_class(), 'dispatch' )
array( $class, 'dispatch' )
);

add_action( 'load-' . $hook, array( get_class(), 'load' ) );
add_action( 'load-' . $hook, array( $class, 'load' ) );
}

/**
Expand Down Expand Up @@ -68,37 +70,31 @@ public static function load() {
switch ( self::current_action() ) {
case 'add':
case 'edit':
return self::render_edit_page();

self::render_edit_page();
break;
case 'delete':
return self::handle_delete();

self::handle_delete();
break;
case 'regenerate':
return self::handle_regenerate();

self::handle_regenerate();
break;
default:
global $wp_list_table;

$wp_list_table = new WP_REST_OAuth1_ListTable();

$wp_list_table->prepare_items();

return;
}
}

/**
* Render callback.
*/
public static function dispatch() {
switch ( self::current_action() ) {
case 'add':
case 'edit':
case 'delete':
break;
default:
self::render();
if ( in_array( self::current_action(), array( 'add', 'edit', 'delete' ), true ) ) {
return;
}

self::render();
}

/**
Expand Down Expand Up @@ -163,19 +159,18 @@ protected static function validate_parameters( $params ) {
$valid['description'] = wp_filter_post_kses( $params['description'] );

if ( empty( $params['callback'] ) ) {
return new WP_Error( 'rest_oauth1_missing_description', __( 'Consumer callback is required and must be a valid URL.', 'rest_oauth1' ) );
}
if ( ! empty( $params['callback'] ) ) {
$valid['callback'] = $params['callback'];
return new WP_Error( 'rest_oauth1_missing_callback', __( 'Consumer callback is required and must be a valid URL.', 'rest_oauth1' ) );
}

$valid['callback'] = $params['callback'];

return $valid;
}

/**
* Handle submission of the add page
* @param WP_User $consumer Consumer user.
* @param WP_REST_Client $consumer Consumer user.
*
* @return array|null List of errors. Issues a redirect and exits on success.
*/
Expand All @@ -197,8 +192,6 @@ protected static function handle_edit_submit( $consumer ) {
}

if ( empty( $consumer ) ) {
new WP_REST_OAuth1();

// Create the consumer.
$data = array(
'name' => $params['name'],
Expand Down Expand Up @@ -248,8 +241,9 @@ public static function render_edit_page() {
}

// Are we editing?
$consumer = null;
$form_action = self::get_url( 'action=add' );
$consumer = null;
$regenerate_action = '';
$form_action = self::get_url( 'action=add' );
if ( ! empty( $_REQUEST['id'] ) ) {
$id = absint( $_REQUEST['id'] );
$consumer = WP_REST_OAuth1_Client::get( $id );
Expand Down Expand Up @@ -430,13 +424,15 @@ public static function handle_delete() {
$client = WP_REST_OAuth1_Client::get( $id );
if ( is_wp_error( $client ) ) {
wp_die( $client );
return;
}

if ( ! $client->delete() ) {
$message = 'Invalid consumer ID';
wp_die( $message );
return;
$code = is_user_logged_in() ? 403 : 401;
wp_die(
'<h1>' . __( 'An error has occurred.', 'rest_oauth1' ) . '</h1>' .
'<p>' . __( 'Invalid consumer ID', 'rest_oauth1' ) . '</p>',
$code
);
}

wp_safe_redirect( self::get_url( 'deleted=1' ) );
Expand Down Expand Up @@ -464,7 +460,13 @@ public static function handle_regenerate() {
}

$client = WP_REST_OAuth1_Client::get( $id );
$client->regenerate_secret();
if ( is_wp_error( $client ) ) {
wp_die( $client );
}
$result = $client->regenerate_secret();
if ( is_wp_error( $result ) ) {
wp_die( $result );
}

wp_safe_redirect(
self::get_url(
Expand Down
69 changes: 64 additions & 5 deletions lib/class-wp-rest-oauth1-cli.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,77 @@
class WP_REST_OAuth1_CLI extends WP_CLI_Command {

/**
* Creates a new OAuth1 Client.
*
* ## OPTIONS
*
* [--name=<name>]
* : Consumer name
*
* [--description=<description>]
* : Consumer description
*
* [--callback=<callback>]
* : Consumer callback
*/
public function add( $_, $args ) {
$consumer = WP_REST_OAuth1_Client::create( $args );
WP_CLI::line( sprintf( 'ID: %d', $consumer->ID ) );
WP_CLI::line( sprintf( 'Key: %s', $consumer->key ) );
WP_CLI::line( sprintf( 'Secret: %s', $consumer->secret ) );
public function add( $args, $assoc_args ) {
$consumer = WP_REST_OAuth1_Client::create( $assoc_args );
if ( is_wp_error( $consumer ) ) {
WP_CLI::error( $consumer );
}

WP_CLI::line(
sprintf(
/* translators: %d: client ID **/
__( 'ID: %d', 'rest_oauth1' ),
$consumer->ID
)
);
WP_CLI::line(
sprintf(
/* translators: %d: client key **/
__( 'Key: %s', 'rest_oauth1' ),
$consumer->key
)
);
WP_CLI::line(
sprintf(
/* translators: %d: client secret **/
__( 'Secret: %s', 'rest_oauth1' ),
$consumer->secret
)
);
}

/**
* Delete a new OAuth1 Client.
*
* ## OPTIONS
*
* <id>
* : Database ID for the client.
*/
public function delete( $args ) {
$consumer = WP_REST_OAuth1_Client::get( $args[0] );
if ( is_wp_error( $consumer ) ) {
WP_CLI::error( $consumer );
}
if ( ! $consumer->delete() ) {
WP_CLI::error(
sprintf(
/* translators: %d: client ID **/
__( 'Unable to delete client with ID: %d', 'rest_oauth1' ),
$consumer->ID
)
);
}

WP_CLI::success(
sprintf(
/* translators: %d: client ID **/
__( 'Client deleted with ID: %d', 'rest_oauth1' ),
$consumer->ID
)
);
}
}
26 changes: 26 additions & 0 deletions lib/class-wp-rest-oauth1-client.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,32 @@ protected static function get_type() {
return 'oauth1';
}

/**
* Delete a client.
*
* @since 0.4.0
*
* @return bool True if delete, false otherwise.
*/
public function delete() {
global $wpdb;
$results = $wpdb->get_results( "SELECT * FROM $wpdb->options WHERE option_name LIKE 'oauth1_access_%' OR option_name LIKE 'oauth1_request_%'", ARRAY_A );
$delete_option = array();
foreach ( $results as $result ) {
$row = unserialize( $result['option_value'] );
if ( $this->post->ID === $row['consumer'] ) {
$delete_option[] = $result['option_name'];
}
}

if ( (bool) wp_delete_post( $this->post->ID, true ) ) {
array_map( 'delete_option', $delete_option );
return true;
}

return false;
}

/**
* Add extra meta to a post.
*
Expand Down
Loading

0 comments on commit e23038c

Please sign in to comment.