Validate API credentials using the Parse.ly API validation endpoint (#…

…1897) * Validate API credentials using the Parse.ly API validation endpoint * Add tests * Make the API Secret optional * Change @SInCE versions on the new functions * Addressing phpstan errors * Extracted API query logic to outside `get_items` * Fix e2e tests by allowing to bypass the API validate call when saving API credentials. * Fix e2e tests by allowing to bypass the API validate call when saving API credentials. * Allow empty Site ID and API Secret * Address @acicovic feedback * Add test to check if saving empty API credentials is possible. * Remove unnecessary `validate_site_id` and `validate_api_secret` methods * Apply @acicovic suggestions --------- Co-authored-by: Alex Cicovic <23142906+acicovic@users.noreply.github.com>
Parsely · Oct 11, 2023 · 7c95a67 · 7c95a67
1 parent d15f2d5
commit 7c95a67
Show file tree

Hide file tree

Showing 12 changed files with 401 additions and 248 deletions.
diff --git a/phpstan.neon b/phpstan.neon
@@ -13,7 +13,7 @@ parameters:
     - vendor/php-stubs/wordpress-stubs/wordpress-stubs.php
     - vendor/php-stubs/wordpress-tests-stubs/wordpress-tests-stubs.php
   type_coverage:
-      return_type: 94
+      return_type: 93
       param_type: 74
       property_type: 0 # We can't use property types until PHP 7.4 becomes the plugin's minimum version.
       print_suggestions: false

diff --git a/src/RemoteAPI/class-remote-api-base.php b/src/RemoteAPI/class-remote-api-base.php
@@ -149,9 +149,9 @@ public function get_api_url( array $query ): string {
 	 * @param array<string, mixed> $query The query arguments to send to the remote API.
 	 * @param bool                 $associative When TRUE, returned objects will be converted into associative arrays.
 	 *
-	 * @return WP_Error|array<string, mixed>
+	 * @return array<string, mixed>|object|WP_Error
 	 */
-	public function get_items( $query, $associative = false ) {
+	public function get_items( array $query, bool $associative = false ) {
 		$full_api_url = $this->get_api_url( $query );
 		/**
 		 * GET request options.

diff --git a/src/RemoteAPI/class-remote-api-cache.php b/src/RemoteAPI/class-remote-api-cache.php
@@ -48,12 +48,13 @@ public function __construct( Remote_API_Base $remote_api, Cache $cache ) {
 	 * Implements caching for the Remote API interface.
 	 *
 	 * @param array<string, mixed> $query The query arguments to send to the remote API.
-	 * @param bool                 $associative Always `false`, just present to make definition compatible with interface.
+	 * @param bool                 $associative Always `false`, just present to make definition compatible
+	 *                             with interface.
 	 *
-	 * @return array<string, mixed>|WP_Error|false The response from the remote API, or false if the
-	 *                                             response is empty.
+	 * @return array<string, mixed>|object|WP_Error The response from the remote API, or false if the
+	 *                                              response is empty.
 	 */
-	public function get_items( $query, $associative = false ) {
+	public function get_items( array $query, bool $associative = false ) {
 		$cache_key = 'parsely_api_' .
 			wp_hash( $this->remote_api->get_endpoint() ) . '_' .
 			wp_hash( (string) wp_json_encode( $query ) );

diff --git a/src/RemoteAPI/class-validate-api.php b/src/RemoteAPI/class-validate-api.php
@@ -0,0 +1,112 @@
+<?php
+/**
+ * Class for Validate API (`/validate`)
+ *
+ * @package Parsely
+ * @since   3.11.0
+ */
+
+declare(strict_types=1);
+
+namespace Parsely\RemoteAPI;
+
+use Parsely\Parsely;
+use WP_Error;
+use function Parsely\Utils\convert_to_associative_array;
+
+/**
+ * Class for credentials validation API (`/validate`).
+ *
+ * @since 3.11.0
+ *
+ * @phpstan-import-type WP_HTTP_Request_Args from Parsely
+ */
+class Validate_API extends Remote_API_Base {
+	protected const ENDPOINT     = '/validate/secret';
+	protected const QUERY_FILTER = 'wp_parsely_validate_secret_endpoint_args';
+
+	/**
+	 * Indicates whether the endpoint is public or protected behind permissions.
+	 *
+	 * @since 3.11.0
+	 *
+	 * @var bool
+	 */
+	protected $is_public_endpoint = false;
+
+	/**
+	 * Gets the URL for the Parse.ly API credentials validation endpoint.
+	 *
+	 * @since 3.11.0
+	 *
+	 * @param array<string, mixed> $query The query arguments to send to the remote API.
+	 * @return string
+	 */
+	public function get_api_url( array $query ): string {
+		$query = array(
+			'apikey' => $query['apikey'],
+			'secret' => $query['secret'],
+		);
+		return add_query_arg( $query, Parsely::PUBLIC_API_BASE_URL . static::ENDPOINT );
+	}
+
+	/**
+	 * Queries the Parse.ly API credentials validation endpoint.
+	 * The API will return a 200 response if the credentials are valid and a 401 response if they are not.
+	 *
+	 * @param array<string, mixed> $query The query arguments to send to the remote API.
+	 *
+	 * @return object|WP_Error The response from the remote API, or a WP_Error object if the response is an error.
+	 */
+	private function api_validate_credentials( array $query ) {
+		/**
+		 * GET request options.
+		 *
+		 * @var WP_HTTP_Request_Args $options
+		 */
+		$options  = $this->get_request_options();
+		$response = wp_safe_remote_get( $this->get_api_url( $query ), $options );
+
+		if ( is_wp_error( $response ) ) {
+			return $response;
+		}
+
+		$body    = wp_remote_retrieve_body( $response );
+		$decoded = json_decode( $body );
+
+		if ( ! is_object( $decoded ) ) {
+			return new WP_Error(
+				400,
+				__(
+					'Unable to decode upstream API response',
+					'wp-parsely'
+				)
+			);
+		}
+
+		if ( ! property_exists( $decoded, 'success' ) || false === $decoded->success ) {
+			return new WP_Error(
+				$decoded->code ?? 400,
+				$decoded->message ?? __( 'Unable to read data from upstream API', 'wp-parsely' )
+			);
+		}
+
+		return $decoded;
+	}
+
+	/**
+	 * Returns the response from the Parse.ly API credentials validation endpoint.
+	 *
+	 * @since 3.11.0
+	 *
+	 * @param array<string, mixed> $query The query arguments to send to the remote API.
+	 * @param bool                 $associative (optional) When TRUE, returned objects will be converted into
+	 *                             associative arrays.
+	 *
+	 * @return array<string, mixed>|object|WP_Error
+	 */
+	public function get_items( array $query, bool $associative = false ) {
+		$api_request = $this->api_validate_credentials( $query );
+		return $associative ? convert_to_associative_array( $api_request ) : $api_request;
+	}
+}
diff --git a/src/RemoteAPI/interface-remote-api.php b/src/RemoteAPI/interface-remote-api.php
@@ -20,11 +20,12 @@ interface Remote_API_Interface {
 	 * Returns the items provided by this interface.
 	 *
 	 * @param array<string, mixed> $query The query arguments to send to the remote API.
-	 * @param bool                 $associative (optional) When TRUE, returned objects will be converted into associative arrays.
+	 * @param bool                 $associative (optional) When TRUE, returned objects will be converted into
+	 *                             associative arrays.
 	 *
-	 * @return WP_Error|array<string, mixed>|false
+	 * @return array<string, mixed>|object|WP_Error
 	 */
-	public function get_items( $query, $associative = false );
+	public function get_items( array $query, bool $associative = false );
 
 	/**
 	 * Checks if the current user is allowed to make the API call.

diff --git a/src/UI/class-settings-page.php b/src/UI/class-settings-page.php
@@ -968,37 +968,32 @@ private function validate_basic_section( $input ) {
 			$input['apikey']     = '';
 			$input['api_secret'] = '';
 		} else {
-			if ( '' === $input['apikey'] ) {
-				add_settings_error(
-					Parsely::OPTIONS_KEY,
-					'apikey',
-					__( 'Please specify the Site ID', 'wp-parsely' )
-				);
-			} else {
-				$site_id = $this->sanitize_site_id( $input['apikey'] );
-				if ( false === Validator::validate_site_id( $site_id ) ) {
-					add_settings_error(
-						Parsely::OPTIONS_KEY,
-						'apikey',
-						__( 'The Site ID was not saved because it is incorrect. It should look like "example.com".', 'wp-parsely' )
-					);
-					$input['apikey'] = $options['apikey'];
-				} else {
-					$input['apikey'] = $site_id;
-				}
+			$site_id    = $this->sanitize_site_id( $input['apikey'] );
+			$api_secret = $this->get_unobfuscated_value( $input['api_secret'], $this->parsely->get_api_secret() );
+
+			$valid_credentials = Validator::validate_api_credentials( $this->parsely, $site_id, $api_secret );
+
+			// When running e2e tests, we need to update the API keys without validating them, as they will be invalid.
+			// phpcs:ignore WordPress.Security.NonceVerification.Missing
+			if ( isset( $_POST['e2e_parsely_skip_api_validate'] ) && 'y' === $_POST['e2e_parsely_skip_api_validate'] ) {
+				$valid_credentials = true;
 			}
 
-			$input['api_secret'] = $this->get_unobfuscated_value( $input['api_secret'], $this->parsely->get_api_secret() );
-			$api_secret_length   = strlen( $input['api_secret'] );
-			if ( $api_secret_length > 0 &&
-					false === Validator::validate_api_secret( $input['api_secret'] ) ) {
+			if ( is_wp_error( $valid_credentials ) && Validator::INVALID_API_CREDENTIALS === $valid_credentials->get_error_code() ) {
 				add_settings_error(
 					Parsely::OPTIONS_KEY,
 					'api_secret',
-					__( 'The API Secret was not saved because it is incorrect. Please contact Parse.ly support!', 'wp-parsely' )
+					__( 'The Site ID and API Secret weren\'t saved as they failed to authenticate with the Parse.ly API. Try again with different credentials or contact Parse.ly support', 'wp-parsely' )
 				);
+				$input['apikey']     = $options['apikey'];
 				$input['api_secret'] = $options['api_secret'];
 			}
+
+			// Since the API secret is obfuscated, we need to make sure that the value
+			// is not changed when the credentials are valid.
+			if ( true === $valid_credentials && $input['api_secret'] !== $api_secret ) {
+				$input['api_secret'] = $api_secret;
+			}
 		}
 
 		if ( ! isset( $input['meta_type'] ) ) {

diff --git a/src/class-validator.php b/src/class-validator.php
@@ -10,49 +10,16 @@
 
 namespace Parsely;
 
+use WP_Error;
+
 /**
  * Contains a variety of validation functions.
  *
  * @since 3.9.0
  */
 class Validator {
-	/**
-	 * Validates the passed Site ID.
-	 *
-	 * Accepts a www prefix and up to 3 periods.
-	 *
-	 * Valid examples: 'test.com', 'www.test.com', 'subdomain.test.com',
-	 * 'www.subdomain.test.com', 'subdomain.subdomain.test.com'.
-	 *
-	 * Invalid examples: 'test', 'test.com/', 'http://test.com', 'https://test.com',
-	 * 'www.subdomain.subdomain.test.com'.
-	 *
-	 * @since 3.3.0
-	 * @since 3.9.0 Moved to Validator class.
-	 *
-	 * @param string $site_id The Site ID to be validated.
-	 * @return bool
-	 */
-	public static function validate_site_id( string $site_id ): bool {
-		$key_format = '/^((\w+)\.)?(([\w-]+)?)(\.[\w-]+){1,2}$/';
 
-		return 1 === preg_match( $key_format, $site_id );
-	}
-
-	/**
-	 * Validates the passed API Secret.
-	 *
-	 * Currently, the API Secret is considered valid if it is longer than 30
-	 * characters.
-	 *
-	 * @since 3.9.0
-	 *
-	 * @param string $api_secret The API Secret to be validated.
-	 * @return bool True if the API Secret is valid, false otherwise.
-	 */
-	public static function validate_api_secret( string $api_secret ): bool {
-		return strlen( $api_secret ) > 30;
-	}
+	public const INVALID_API_CREDENTIALS = 'invalid_api_credentials';
 
 	/**
 	 * Validates the passed Metadata Secret.
@@ -68,4 +35,41 @@ public static function validate_api_secret( string $api_secret ): bool {
 	public static function validate_metadata_secret( string $metadata_secret ): bool {
 		return strlen( $metadata_secret ) === 10;
 	}
+
+	/**
+	 * Validates the passed API Credentials.
+	 *
+	 * @since 3.11.0
+	 *
+	 * @param Parsely $parsely The Parsely instance.
+	 * @param string  $site_id The Site ID to be validated.
+	 * @param string  $api_secret The API Secret to be validated.
+	 * @return true|WP_Error True if the API Credentials are valid, WP_Error otherwise.
+	 */
+	public static function validate_api_credentials( Parsely $parsely, string $site_id, string $api_secret ) {
+
+		// If the API secret is empty, the validation endpoint will always fail. Since it's possible to
+		// use the plugin without an API Secret, and providing only a Site ID (API key), we'll skip the validation and
+		// assume it's valid.
+		if ( '' === $api_secret ) {
+			return true;
+		}
+
+		$query_args = array(
+			'apikey' => $site_id,
+			'secret' => $api_secret,
+		);
+
+		$validate_api = new RemoteAPI\Validate_API( $parsely );
+		$request      = $validate_api->get_items( $query_args );
+
+		if ( is_wp_error( $request ) ) {
+			return new WP_Error(
+				self::INVALID_API_CREDENTIALS,
+				__( 'Invalid API Credentials', 'wp-parsely' )
+			);
+		}
+
+		return true;
+	}
 }