From 34100b0de69a292cf97afbfaa552ae9938f26919 Mon Sep 17 00:00:00 2001 From: Diede Exterkate <5352634+diedexx@users.noreply.github.com> Date: Wed, 17 Jun 2026 15:41:27 +0200 Subject: [PATCH 01/26] feat(myyoast-client): map 429/404 registration errors to typed exceptions Reintroduces Rate_Limited_Exception and Registration_Not_Found_Exception (both extending Registration_Failed_Exception) and maps the relevant HTTP statuses onto them in the registration client: a 429 from DCR, read, or RFC 7592 update becomes a Rate_Limited_Exception carrying the parsed Retry-After, and a 401/404 on read/update becomes a Registration_Not_Found_Exception after clearing the local registration. This gives the REST layer the retry-after countdown and a distinct "registration gone" path that the user-facing connection card relies on, which the generic Registration_Failed_Exception alone could not provide. Co-Authored-By: Claude Opus 4.8 --- .../exceptions/rate-limited-exception.php | 78 ++++++++++ .../registration-not-found-exception.php | 14 ++ .../registration/client-registration.php | 55 ++++++- .../Rate_Limited_Exception_Test.php | 137 ++++++++++++++++++ .../Registration_Not_Found_Exception_Test.php | 30 ++++ .../Registration/Client_Registration_Test.php | 78 ++++++++++ 6 files changed, 388 insertions(+), 4 deletions(-) create mode 100644 src/myyoast-client/application/exceptions/rate-limited-exception.php create mode 100644 src/myyoast-client/application/exceptions/registration-not-found-exception.php create mode 100644 tests/Unit/MyYoast_Client/Application/Rate_Limited_Exception_Test.php create mode 100644 tests/Unit/MyYoast_Client/Application/Registration_Not_Found_Exception_Test.php diff --git a/src/myyoast-client/application/exceptions/rate-limited-exception.php b/src/myyoast-client/application/exceptions/rate-limited-exception.php new file mode 100644 index 00000000000..da3b6b8114e --- /dev/null +++ b/src/myyoast-client/application/exceptions/rate-limited-exception.php @@ -0,0 +1,78 @@ +retry_after_seconds = $retry_after_seconds; + } + + /** + * Returns the parsed `Retry-After` value in seconds, or null when absent. + * + * @return int|null + */ + public function get_retry_after_seconds(): ?int { + return $this->retry_after_seconds; + } + + /** + * Parses an HTTP `Retry-After` value into seconds-until-retry. + * + * Accepts either the delta-seconds form (`"120"`) or the HTTP-date form + * (`"Wed, 27 May 2026 14:30:00 GMT"`) per RFC 9110 §10.2.3. + * + * phpcs:disable SlevomatCodingStandard.TypeHints.DisallowMixedTypeHint.DisallowedMixedTypeHint -- Raw header value is heterogeneous (string, numeric, array, or null). + * + * @param mixed $retry_after The raw header value (string, numeric, array, or null). + * + * @return int|null Seconds until retry (clamped to >= 0), or null when unparseable. + * + * phpcs:enable SlevomatCodingStandard.TypeHints.DisallowMixedTypeHint.DisallowedMixedTypeHint + */ + public static function parse_retry_after( $retry_after ): ?int { + if ( \is_array( $retry_after ) ) { + $retry_after = \reset( $retry_after ); + } + + if ( $retry_after === null || $retry_after === '' ) { + return null; + } + + if ( \is_numeric( $retry_after ) ) { + return \max( 0, (int) $retry_after ); + } + + $timestamp = \strtotime( (string) $retry_after ); + if ( $timestamp === false ) { + return null; + } + + return \max( 0, ( $timestamp - \time() ) ); + } +} diff --git a/src/myyoast-client/application/exceptions/registration-not-found-exception.php b/src/myyoast-client/application/exceptions/registration-not-found-exception.php new file mode 100644 index 00000000000..cd234f74805 --- /dev/null +++ b/src/myyoast-client/application/exceptions/registration-not-found-exception.php @@ -0,0 +1,14 @@ + The registration metadata. * - * @throws Registration_Failed_Exception If the read fails. + * @throws Registration_Not_Found_Exception If the server reports the registration is gone (HTTP 401/404). + * @throws Rate_Limited_Exception If the server rate-limited the request (HTTP 429). + * @throws Registration_Failed_Exception If the read fails for any other reason. */ public function read_registration(): array { $registered_client = $this->get_registered_client(); @@ -252,12 +257,18 @@ public function read_registration(): array { if ( $result->get_status() === 401 || $result->get_status() === 404 ) { $this->logger->warning( 'Registration is no longer valid (HTTP {status}), clearing local registration.', [ 'status' => $result->get_status() ] ); $this->forget_registration(); - throw new Registration_Failed_Exception( + throw new Registration_Not_Found_Exception( // phpcs:ignore WordPress.Security.EscapeOutput.ExceptionNotEscaped -- Internal exception message. 'Registration is no longer valid (HTTP ' . $result->get_status() . ').', ); } + if ( $result->get_status() === 429 ) { + $this->logger->warning( 'Registration read was rate-limited (HTTP 429).' ); + // phpcs:ignore WordPress.Security.EscapeOutput.ExceptionNotEscaped -- Internal exception message. + throw new Rate_Limited_Exception( 'Registration read was rate-limited (HTTP 429).', $this->get_retry_after_seconds( $result ) ); + } + if ( ! $result->is_successful() ) { $error_message = (string) $result->get_body_value( 'error_description', $result->get_body_value( 'error', '' ) ); throw new Registration_Failed_Exception( @@ -348,7 +359,9 @@ public function rotate_registration_keys(): Registered_Client { * * @return Registered_Client The updated credentials. * - * @throws Registration_Failed_Exception If the update fails. + * @throws Registration_Not_Found_Exception If the server reports the registration is gone (HTTP 401/404). + * @throws Rate_Limited_Exception If the server rate-limited the request (HTTP 429). + * @throws Registration_Failed_Exception If the update fails for any other reason. */ private function update_redirect_uris( array $redirect_uris ): Registered_Client { $registered_client = $this->get_registered_client(); @@ -382,6 +395,21 @@ private function update_redirect_uris( array $redirect_uris ): Registered_Client ], ); + if ( $result->get_status() === 401 || $result->get_status() === 404 ) { + $this->logger->warning( 'Registration is no longer valid on update (HTTP {status}), clearing local registration.', [ 'status' => $result->get_status() ] ); + $this->forget_registration(); + throw new Registration_Not_Found_Exception( + // phpcs:ignore WordPress.Security.EscapeOutput.ExceptionNotEscaped -- Internal exception message. + 'Registration is no longer valid (HTTP ' . $result->get_status() . ').', + ); + } + + if ( $result->get_status() === 429 ) { + $this->logger->warning( 'Registration update was rate-limited (HTTP 429).' ); + // phpcs:ignore WordPress.Security.EscapeOutput.ExceptionNotEscaped -- Internal exception message. + throw new Rate_Limited_Exception( 'Registration update was rate-limited (HTTP 429).', $this->get_retry_after_seconds( $result ) ); + } + if ( ! $result->is_successful() ) { $error_message = (string) $result->get_body_value( 'error_description', $result->get_body_value( 'error', '' ) ); throw new Registration_Failed_Exception( @@ -568,6 +596,18 @@ private function get_option_key(): string { return self::OPTION_KEY_PREFIX . $this->issuer_config->get_issuer_key(); } + /** + * Extracts the `Retry-After` value (in seconds) from a 429 response, if any. + * + * @param HTTP_Response $result The 429 response. + * + * @return int|null Seconds until retry, or null when absent or unparseable. + */ + private function get_retry_after_seconds( $result ): ?int { + $headers = $result->get_headers(); + return Rate_Limited_Exception::parse_retry_after( ( $headers['retry-after'] ?? null ) ); + } + /** * Performs the actual DCR registration request. * @@ -575,7 +615,8 @@ private function get_option_key(): string { * * @return Registered_Client The registration result. * - * @throws Registration_Failed_Exception If registration fails. + * @throws Rate_Limited_Exception If the server rate-limited the request (HTTP 429). + * @throws Registration_Failed_Exception If registration fails for any other reason. */ private function do_register( array $redirect_uris ): Registered_Client { try { @@ -631,6 +672,12 @@ private function do_register( array $redirect_uris ): Registered_Client { throw new Registration_Failed_Exception( 'DCR request failed: ' . $error_message ); } + if ( $result->get_status() === 429 ) { + $this->logger->warning( 'DCR was rate-limited (HTTP 429).' ); + // phpcs:ignore WordPress.Security.EscapeOutput.ExceptionNotEscaped -- Internal exception message. + throw new Rate_Limited_Exception( 'DCR was rate-limited (HTTP 429).', $this->get_retry_after_seconds( $result ) ); + } + if ( $result->get_status() !== 201 ) { $error_message = (string) $result->get_body_value( 'error_description', $result->get_body_value( 'error', '' ) ); throw new Registration_Failed_Exception( diff --git a/tests/Unit/MyYoast_Client/Application/Rate_Limited_Exception_Test.php b/tests/Unit/MyYoast_Client/Application/Rate_Limited_Exception_Test.php new file mode 100644 index 00000000000..5baefec7669 --- /dev/null +++ b/tests/Unit/MyYoast_Client/Application/Rate_Limited_Exception_Test.php @@ -0,0 +1,137 @@ +assertInstanceOf( Registration_Failed_Exception::class, $exception ); + $this->assertSame( 'Slow down.', $exception->getMessage() ); + } + + /** + * Tests that the retry-after value defaults to null when not provided. + * + * @covers ::get_retry_after_seconds + * + * @return void + */ + public function test_retry_after_seconds_defaults_to_null() { + $exception = new Rate_Limited_Exception( 'Slow down.' ); + + $this->assertNull( $exception->get_retry_after_seconds() ); + } + + /** + * Tests that the retry-after value is exposed when provided. + * + * @covers ::__construct + * @covers ::get_retry_after_seconds + * + * @return void + */ + public function test_retry_after_seconds_is_exposed() { + $exception = new Rate_Limited_Exception( 'Slow down.', 120 ); + + $this->assertSame( 120, $exception->get_retry_after_seconds() ); + } + + /** + * Tests parse_retry_after on the delta-seconds form. + * + * @covers ::parse_retry_after + * + * @return void + */ + public function test_parse_retry_after_seconds_form() { + $this->assertSame( 120, Rate_Limited_Exception::parse_retry_after( '120' ) ); + $this->assertSame( 30, Rate_Limited_Exception::parse_retry_after( 30 ) ); + $this->assertSame( 0, Rate_Limited_Exception::parse_retry_after( '0' ) ); + } + + /** + * Tests that a negative seconds value is clamped to zero. + * + * @covers ::parse_retry_after + * + * @return void + */ + public function test_parse_retry_after_clamps_negative_to_zero() { + $this->assertSame( 0, Rate_Limited_Exception::parse_retry_after( '-5' ) ); + } + + /** + * Tests parse_retry_after on the HTTP-date form. + * + * @covers ::parse_retry_after + * + * @return void + */ + public function test_parse_retry_after_http_date_form() { + $future_seconds = 300; + $date = \gmdate( 'D, d M Y H:i:s', ( \time() + $future_seconds ) ) . ' GMT'; + + $parsed = Rate_Limited_Exception::parse_retry_after( $date ); + + // Allow a small jitter since `time()` is called twice (once here, once in the parser). + $this->assertNotNull( $parsed ); + $this->assertGreaterThanOrEqual( ( $future_seconds - 5 ), $parsed ); + $this->assertLessThanOrEqual( ( $future_seconds + 5 ), $parsed ); + } + + /** + * Tests that a past HTTP-date is clamped to zero rather than going negative. + * + * @covers ::parse_retry_after + * + * @return void + */ + public function test_parse_retry_after_past_date_clamps_to_zero() { + $past_date = \gmdate( 'D, d M Y H:i:s', ( \time() - 3600 ) ) . ' GMT'; + + $this->assertSame( 0, Rate_Limited_Exception::parse_retry_after( $past_date ) ); + } + + /** + * Tests that null / empty / garbage Retry-After values return null. + * + * @covers ::parse_retry_after + * + * @return void + */ + public function test_parse_retry_after_returns_null_when_unparseable() { + $this->assertNull( Rate_Limited_Exception::parse_retry_after( null ) ); + $this->assertNull( Rate_Limited_Exception::parse_retry_after( '' ) ); + $this->assertNull( Rate_Limited_Exception::parse_retry_after( 'not a date' ) ); + } + + /** + * Tests that a list-valued header (e.g. from wp_remote_*) is unwrapped. + * + * @covers ::parse_retry_after + * + * @return void + */ + public function test_parse_retry_after_unwraps_list_header() { + $this->assertSame( 60, Rate_Limited_Exception::parse_retry_after( [ '60', '120' ] ) ); + } +} diff --git a/tests/Unit/MyYoast_Client/Application/Registration_Not_Found_Exception_Test.php b/tests/Unit/MyYoast_Client/Application/Registration_Not_Found_Exception_Test.php new file mode 100644 index 00000000000..2743cdf0f83 --- /dev/null +++ b/tests/Unit/MyYoast_Client/Application/Registration_Not_Found_Exception_Test.php @@ -0,0 +1,30 @@ +assertInstanceOf( Registration_Failed_Exception::class, $exception ); + $this->assertSame( 'Gone.', $exception->getMessage() ); + } +} diff --git a/tests/Unit/MyYoast_Client/Infrastructure/Registration/Client_Registration_Test.php b/tests/Unit/MyYoast_Client/Infrastructure/Registration/Client_Registration_Test.php index 294b00aa8b5..8031711712e 100644 --- a/tests/Unit/MyYoast_Client/Infrastructure/Registration/Client_Registration_Test.php +++ b/tests/Unit/MyYoast_Client/Infrastructure/Registration/Client_Registration_Test.php @@ -7,7 +7,9 @@ use Mockery; use Yoast\WP\SEO\Exceptions\Locking\Lock_Timeout_Exception; use Yoast\WP\SEO\Helpers\Lock_Helper; +use Yoast\WP\SEO\MyYoast_Client\Application\Exceptions\Rate_Limited_Exception; use Yoast\WP\SEO\MyYoast_Client\Application\Exceptions\Registration_Failed_Exception; +use Yoast\WP\SEO\MyYoast_Client\Application\Exceptions\Registration_Not_Found_Exception; use Yoast\WP\SEO\MyYoast_Client\Domain\HTTP_Response; use Yoast\WP\SEO\MyYoast_Client\Infrastructure\Crypto\Encryption; use Yoast\WP\SEO\MyYoast_Client\Infrastructure\Crypto\Key_Pair; @@ -759,6 +761,82 @@ public function test_mark_uri_validated_is_noop_when_not_registered() { $this->instance->mark_uri_validated( 'https://example.com/callback' ); } + /** + * Tests that a 401 from the RFC 7592 read clears local state and throws Registration_Not_Found_Exception. + * + * @covers ::read_registration + * + * @return void + */ + public function test_read_registration_throws_not_found_on_401() { + $this->mock_get_client(); + + $this->http_client + ->expects( 'authenticated_request' ) + ->andReturn( new HTTP_Response( 401, [], [ 'error' => 'invalid_token' ] ) ); + + Functions\expect( 'delete_option' )->once()->with( self::OPTION_KEY )->andReturn( true ); + + $this->expectException( Registration_Not_Found_Exception::class ); + $this->instance->read_registration(); + } + + /** + * Tests that a 404 from the RFC 7592 read clears local state and throws Registration_Not_Found_Exception. + * + * @covers ::read_registration + * + * @return void + */ + public function test_read_registration_throws_not_found_on_404() { + $this->mock_get_client(); + + $this->http_client + ->expects( 'authenticated_request' ) + ->andReturn( new HTTP_Response( 404, [], [] ) ); + + Functions\expect( 'delete_option' )->once()->with( self::OPTION_KEY )->andReturn( true ); + + $this->expectException( Registration_Not_Found_Exception::class ); + $this->instance->read_registration(); + } + + /** + * Tests that a 429 from the RFC 7592 read throws Rate_Limited_Exception carrying the Retry-After. + * + * @covers ::read_registration + * @covers ::get_retry_after_seconds + * + * @return void + */ + public function test_read_registration_throws_rate_limited_on_429() { + $this->mock_get_client(); + + $this->http_client + ->expects( 'authenticated_request' ) + ->andReturn( new HTTP_Response( 429, [ 'retry-after' => '120' ], [] ) ); + + try { + $this->instance->read_registration(); + $this->fail( 'Expected Rate_Limited_Exception was not thrown.' ); + } catch ( Rate_Limited_Exception $e ) { + $this->assertSame( 120, $e->get_retry_after_seconds() ); + } + } + + /** + * Tests that the typed registration exceptions are still caught as Registration_Failed_Exception. + * + * @covers \Yoast\WP\SEO\MyYoast_Client\Application\Exceptions\Rate_Limited_Exception + * @covers \Yoast\WP\SEO\MyYoast_Client\Application\Exceptions\Registration_Not_Found_Exception + * + * @return void + */ + public function test_typed_exceptions_extend_registration_failed_exception() { + $this->assertInstanceOf( Registration_Failed_Exception::class, new Rate_Limited_Exception( 'rate limited' ) ); + $this->assertInstanceOf( Registration_Failed_Exception::class, new Registration_Not_Found_Exception( 'gone' ) ); + } + /** * Sets up mocks for get_client() to return a valid Registered_Client. * From 6d44bcefd60e3168886b61a4bae708739aadb29a Mon Sep 17 00:00:00 2001 From: Diede Exterkate <5352634+diedexx@users.noreply.github.com> Date: Wed, 17 Jun 2026 15:54:33 +0200 Subject: [PATCH 02/26] feat(myyoast-client): handle the OAuth authorization-code callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a dedicated OAuth callback endpoint on admin-post.php and registers it as the site's redirect URI through the client's redirect-URI provider filters, replacing the base client's placeholder admin-page default. The endpoint is reachable from any page the flow may have started on; the per-flow return URL sends the user back to where they began. On the returning request it exchanges the authorization code — which the authorization-code handler now also marks the redirect URI validated for — and stashes a one-shot per-user outcome the connection card surfaces. Adds Authorization_Code_Handler::discard_flow_state() to clear a pending flow when the provider returns an error (e.g. the user denied consent). Co-Authored-By: Claude Opus 4.8 --- src/integrations/admin/helpscout-beacon.php | 2 +- src/integrations/admin/integrations-page.php | 26 +- .../authorization-code-handler.php | 18 +- .../wordpress/redirect-uri-provider.php | 7 +- .../integrations-page-script-data.php | 99 +++ .../user-interface/management-route.php | 509 +++++++++++++ .../oauth-callback-integration.php | 237 ++++++ .../user-interface/status-presenter.php | 211 ++++++ .../Integrations_Page_Integration_Test.php | 12 + .../WordPress/Redirect_URI_Provider_Test.php | 2 +- .../Integrations_Page_Script_Data_Test.php | 219 ++++++ .../User_Interface/Management_Route_Test.php | 694 ++++++++++++++++++ .../OAuth_Callback_Integration_Test.php | 461 ++++++++++++ .../User_Interface/Status_Presenter_Test.php | 239 ++++++ 14 files changed, 2724 insertions(+), 12 deletions(-) create mode 100644 src/myyoast-client/user-interface/integrations-page-script-data.php create mode 100644 src/myyoast-client/user-interface/management-route.php create mode 100644 src/myyoast-client/user-interface/oauth-callback-integration.php create mode 100644 src/myyoast-client/user-interface/status-presenter.php create mode 100644 tests/Unit/MyYoast_Client/User_Interface/Integrations_Page_Script_Data_Test.php create mode 100644 tests/Unit/MyYoast_Client/User_Interface/Management_Route_Test.php create mode 100644 tests/Unit/MyYoast_Client/User_Interface/OAuth_Callback_Integration_Test.php create mode 100644 tests/Unit/MyYoast_Client/User_Interface/Status_Presenter_Test.php diff --git a/src/integrations/admin/helpscout-beacon.php b/src/integrations/admin/helpscout-beacon.php index c17c1befa44..a05d5b68ade 100644 --- a/src/integrations/admin/helpscout-beacon.php +++ b/src/integrations/admin/helpscout-beacon.php @@ -98,7 +98,7 @@ class HelpScout_Beacon implements Integration_Interface { 'wpseo_tools', Plans_Page_Integration::PAGE, 'wpseo_workouts', - 'wpseo_integrations', + Integrations_Page::PAGE, ]; /** diff --git a/src/integrations/admin/integrations-page.php b/src/integrations/admin/integrations-page.php index 34f073b236b..8a444931f75 100644 --- a/src/integrations/admin/integrations-page.php +++ b/src/integrations/admin/integrations-page.php @@ -10,6 +10,7 @@ use Yoast\WP\SEO\Dashboard\Infrastructure\Integrations\Site_Kit; use Yoast\WP\SEO\Helpers\Options_Helper; use Yoast\WP\SEO\Integrations\Integration_Interface; +use Yoast\WP\SEO\MyYoast_Client\User_Interface\Integrations_Page_Script_Data as MyYoast_Connection_Script_Data; use Yoast\WP\SEO\Schema\Application\Configuration\Schema_Configuration; /** @@ -17,6 +18,13 @@ */ class Integrations_Page implements Integration_Interface { + /** + * The page identifier of the integrations page. + * + * @var string + */ + public const PAGE = 'wpseo_integrations'; + /** * The admin asset manager. * @@ -66,6 +74,13 @@ class Integrations_Page implements Integration_Interface { */ private $schema_configuration; + /** + * The MyYoast connection script-data provider. + * + * @var MyYoast_Connection_Script_Data + */ + private $myyoast_connection_script_data; + /** * {@inheritDoc} */ @@ -85,6 +100,8 @@ public static function get_conditionals() { * @param Site_Kit_Consent_Management_Endpoint $site_kit_consent_management_endpoint The site kit consent * management endpoint. * @param Schema_Configuration $schema_configuration The schema configuration. + * @param MyYoast_Connection_Script_Data $myyoast_connection_script_data The MyYoast connection + * script-data provider. */ public function __construct( WPSEO_Admin_Asset_Manager $admin_asset_manager, @@ -93,7 +110,8 @@ public function __construct( Jetpack_Conditional $jetpack_conditional, Site_Kit $site_kit_integration_data, Site_Kit_Consent_Management_Endpoint $site_kit_consent_management_endpoint, - Schema_Configuration $schema_configuration + Schema_Configuration $schema_configuration, + MyYoast_Connection_Script_Data $myyoast_connection_script_data ) { $this->admin_asset_manager = $admin_asset_manager; $this->options_helper = $options_helper; @@ -102,6 +120,7 @@ public function __construct( $this->site_kit_integration_data = $site_kit_integration_data; $this->site_kit_consent_management_endpoint = $site_kit_consent_management_endpoint; $this->schema_configuration = $schema_configuration; + $this->myyoast_connection_script_data = $myyoast_connection_script_data; } /** @@ -125,7 +144,7 @@ public function add_submenu_page( $submenu_pages ) { '', \__( 'Integrations', 'wordpress-seo' ), 'wpseo_manage_options', - 'wpseo_integrations', + self::PAGE, [ $this, 'render_target' ], ]; @@ -141,7 +160,7 @@ public function add_submenu_page( $submenu_pages ) { */ public function enqueue_assets() { // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Date is not processed or saved. - if ( ! isset( $_GET['page'] ) || $_GET['page'] !== 'wpseo_integrations' ) { + if ( ! isset( $_GET['page'] ) || $_GET['page'] !== self::PAGE ) { return; } @@ -226,6 +245,7 @@ public function enqueue_assets() { 'site_kit_configuration' => $this->site_kit_integration_data->to_array(), 'site_kit_consent_management_url' => $this->site_kit_consent_management_endpoint->get_url(), 'schema_framework_enabled' => $this->options_helper->get( 'enable_schema', true ) === true && ! $this->schema_configuration->is_schema_disabled_programmatically(), + 'myyoast_connection' => $this->myyoast_connection_script_data->present(), ], ); } diff --git a/src/myyoast-client/application/authorization-code-handler.php b/src/myyoast-client/application/authorization-code-handler.php index 8a3f84a2aac..9a54cfb103f 100644 --- a/src/myyoast-client/application/authorization-code-handler.php +++ b/src/myyoast-client/application/authorization-code-handler.php @@ -224,12 +224,12 @@ public function exchange_code( int $user_id, string $code, string $state ): Toke // Validate state (CSRF protection). if ( ! \hash_equals( $flow_state->get_state(), $state ) ) { $this->logger->warning( 'Authorization code exchange failed: state parameter mismatch for user {user_id} (potential CSRF).', [ 'user_id' => $user_id ] ); - $this->expiring_store->delete_for_user( self::CURRENT_AUTH_FLOW_STATE_KEY, $user_id ); + $this->discard_flow_state( $user_id ); throw new Token_Request_Failed_Exception( 'invalid_request', 'State parameter mismatch.' ); } // Clean up the stored flow state. - $this->expiring_store->delete_for_user( self::CURRENT_AUTH_FLOW_STATE_KEY, $user_id ); + $this->discard_flow_state( $user_id ); $resource_indicator = $flow_state->get_resource_indicator(); $grant = new Authorization_Code_Grant( $code, $flow_state->get_redirect_uri(), $flow_state->get_code_verifier() ); @@ -258,6 +258,20 @@ public function get_return_url( int $user_id ): ?string { } } + /** + * Discards any pending authorization-flow state for a user. + * + * Used when the provider returns an error (e.g. the user denied consent) so a + * stale flow can't be resumed. A no-op when no flow is pending. + * + * @param int $user_id The WordPress user ID. + * + * @return void + */ + public function discard_flow_state( int $user_id ): void { + $this->expiring_store->delete_for_user( self::CURRENT_AUTH_FLOW_STATE_KEY, $user_id ); + } + /** * Validates the nonce claim in the ID token against the stored nonce. * diff --git a/src/myyoast-client/infrastructure/wordpress/redirect-uri-provider.php b/src/myyoast-client/infrastructure/wordpress/redirect-uri-provider.php index e45fb7ae0d3..e1f9a93ef55 100644 --- a/src/myyoast-client/infrastructure/wordpress/redirect-uri-provider.php +++ b/src/myyoast-client/infrastructure/wordpress/redirect-uri-provider.php @@ -3,10 +3,10 @@ // phpcs:disable Yoast.NamingConventions.NamespaceName.TooLong -- Needed in the folder structure. namespace Yoast\WP\SEO\MyYoast_Client\Infrastructure\WordPress; -use Yoast\WP\SEO\General\User_Interface\General_Page_Integration; use Yoast\WP\SEO\MyYoast_Client\Application\Ports\Redirect_URI_Provider_Interface; use Yoast\WP\SEO\MyYoast_Client\Domain\Registered_Client; use Yoast\WP\SEO\MyYoast_Client\Domain\Resource_Indicator; +use Yoast\WP\SEO\MyYoast_Client\User_Interface\OAuth_Callback_Integration; /** * WordPress implementation of the redirect URI provider. @@ -124,10 +124,7 @@ public function get_authorization_redirect_uri( * @return string The canonical redirect URI. */ private function get_canonical_redirect_uri(): string { - return \get_admin_url( - null, - 'admin.php?page=' . General_Page_Integration::PAGE . '&yoast_myyoast_oauth_callback=1', - ); + return OAuth_Callback_Integration::get_callback_url(); } /** diff --git a/src/myyoast-client/user-interface/integrations-page-script-data.php b/src/myyoast-client/user-interface/integrations-page-script-data.php new file mode 100644 index 00000000000..b827dcc293d --- /dev/null +++ b/src/myyoast-client/user-interface/integrations-page-script-data.php @@ -0,0 +1,99 @@ +status_presenter = $status_presenter; + $this->myyoast_connection_conditional = $myyoast_connection_conditional; + } + + /** + * Returns the MyYoast connection payload, or `null` when the feature flag + * is disabled so the Integrations page can omit the key entirely. + * + * The `callbackOutcome` slot is populated (and consumed) when an OAuth + * callback finished for this user since the last time the Integrations page + * was rendered, so the React app can surface a one-shot notification. + * + * @return array{initialStatus: array{is_provisioned: bool, is_registered: bool, registered_at: int|null, registered_at_iso: string|null, redirect_uris: array, redirect_uris_match: bool}, profileUrl: string, callbackOutcome: array{kind: string, key: string}|null}|null + */ + public function present(): ?array { + if ( ! $this->myyoast_connection_conditional->is_met() ) { + return null; + } + + return [ + 'initialStatus' => $this->status_presenter->present(), + 'profileUrl' => \admin_url( 'profile.php' ), + 'callbackOutcome' => $this->consume_callback_outcome(), + ]; + } + + /** + * Reads and deletes the per-user OAuth callback outcome transient. + * + * Consumed-on-read so the notification only fires once. + * + * @return array{kind: string, key: string}|null The outcome, or null when none is pending. + */ + private function consume_callback_outcome(): ?array { + $user_id = \get_current_user_id(); + if ( $user_id <= 0 ) { + return null; + } + + $transient_key = OAuth_Callback_Integration::TRANSIENT_PREFIX . $user_id; + $stored = \get_transient( $transient_key ); + if ( ! \is_array( $stored ) ) { + return null; + } + \delete_transient( $transient_key ); + + $kind = ( $stored['kind'] ?? '' ); + $key = ( $stored['key'] ?? '' ); + if ( ! \is_string( $kind ) || ! \is_string( $key ) || $kind === '' || $key === '' ) { + return null; + } + + return [ + 'kind' => $kind, + 'key' => $key, + ]; + } +} diff --git a/src/myyoast-client/user-interface/management-route.php b/src/myyoast-client/user-interface/management-route.php new file mode 100644 index 00000000000..a0cc9e11df8 --- /dev/null +++ b/src/myyoast-client/user-interface/management-route.php @@ -0,0 +1,509 @@ +myyoast_client = $myyoast_client; + $this->status_presenter = $status_presenter; + $this->issuer_config = $issuer_config; + $this->redirect_uri = $redirect_uri; + $this->client_registration = $client_registration; + $this->logger = new NullLogger(); + } + + /** + * Returns the conditionals on which this route should be registered. + * + * @return array + */ + public static function get_conditionals() { + return [ MyYoast_Connection_Conditional::class ]; + } + + /** + * Registers the routes with WordPress. + * + * @return void + */ + public function register_routes() { + $permission_callback = [ $this, 'can_manage' ]; + + \register_rest_route( + Main::API_V1_NAMESPACE, + self::ROUTE_PREFIX . self::STATUS_ROUTE, + [ + 'methods' => 'GET', + 'callback' => [ $this, 'get_status' ], + 'permission_callback' => $permission_callback, + ], + ); + + \register_rest_route( + Main::API_V1_NAMESPACE, + self::ROUTE_PREFIX . self::VERIFY_ROUTE, + [ + 'methods' => 'POST', + 'callback' => [ $this, 'verify' ], + 'permission_callback' => $permission_callback, + ], + ); + + \register_rest_route( + Main::API_V1_NAMESPACE, + self::ROUTE_PREFIX . self::REGISTER_ROUTE, + [ + 'methods' => 'POST', + 'callback' => [ $this, 'register' ], + 'permission_callback' => $permission_callback, + ], + ); + + \register_rest_route( + Main::API_V1_NAMESPACE, + self::ROUTE_PREFIX . self::REGISTRATION_ROUTE, + [ + [ + 'methods' => 'PUT', + 'callback' => [ $this, 'update_registration' ], + 'permission_callback' => $permission_callback, + ], + [ + 'methods' => 'DELETE', + 'callback' => [ $this, 'deregister' ], + 'permission_callback' => $permission_callback, + ], + ], + ); + + \register_rest_route( + Main::API_V1_NAMESPACE, + self::ROUTE_PREFIX . self::AUTHORIZE_ROUTE, + [ + 'methods' => 'POST', + 'callback' => [ $this, 'authorize' ], + 'permission_callback' => $permission_callback, + 'args' => [ + 'redirect_uri' => [ + 'type' => 'string', + 'required' => true, + ], + ], + ], + ); + } + + /** + * Permission callback for every endpoint. + * + * @return bool + */ + public function can_manage() { + return \current_user_can( 'wpseo_manage_options' ); + } + + /** + * GET /myyoast/status — returns the current status payload. + * + * @return WP_REST_Response + */ + public function get_status() { + return $this->respond_with_status( 200, null ); + } + + /** + * POST /myyoast/verify — verifies the registration with the server. + * + * @return WP_REST_Response + */ + public function verify() { + try { + $this->myyoast_client->verify_registration(); + } catch ( Throwable $e ) { + return $this->handle_exception( $e ); + } + + return $this->respond_with_status( 200, null ); + } + + /** + * POST /myyoast/register — connects the site to MyYoast. + * + * @return WP_REST_Response + */ + public function register() { + $gate = $this->require_provisioned(); + if ( $gate !== null ) { + return $gate; + } + + try { + $this->myyoast_client->ensure_registered( [ $this->redirect_uri->get() ] ); + } catch ( Throwable $e ) { + return $this->handle_exception( $e ); + } + + return $this->respond_with_status( 200, 'connect_success' ); + } + + /** + * PUT /myyoast/registration — updates the connection's redirect URIs. + * + * Used to recover the connection after the site's URL has changed. + * + * @return WP_REST_Response + */ + public function update_registration() { + $gate = $this->require_provisioned(); + if ( $gate !== null ) { + return $gate; + } + + try { + $this->myyoast_client->update_redirect_uris( [ $this->redirect_uri->get() ] ); + } catch ( Throwable $e ) { + return $this->handle_exception( $e ); + } + + return $this->respond_with_status( 200, 'update_success' ); + } + + /** + * POST /myyoast/authorize — starts the authorization-code flow for the + * given redirect URI and returns the URL the browser should be sent to. + * + * Used to verify that a stored redirect URI is reachable and legitimate by + * completing a real auth-code round-trip. The actual marking-as-verified + * happens on the callback side (introduced in #1207). + * + * @param WP_REST_Request $request The REST request. + * + * @return WP_REST_Response + */ + public function authorize( WP_REST_Request $request ): WP_REST_Response { + $gate = $this->require_provisioned(); + if ( $gate !== null ) { + return $gate; + } + + $registered_client = $this->client_registration->get_registered_client(); + if ( $registered_client === null ) { + return $this->error_response( 'registration_gone' ); + } + + $redirect_uri = (string) $request->get_param( 'redirect_uri' ); + if ( ! $this->is_known_redirect_uri( $registered_client->get_metadata(), $redirect_uri ) ) { + return $this->error_response( 'unknown_redirect_uri', null, 400 ); + } + + $user_id = \get_current_user_id(); + if ( $user_id <= 0 ) { + return $this->error_response( 'invalid_user', null, 401 ); + } + + $return_url = \admin_url( 'admin.php?page=' . Integrations_Page::PAGE ); + + try { + $authorize_url = $this->myyoast_client->get_authorization_url( + $user_id, + $redirect_uri, + [ 'openid' ], + null, + $return_url, + ); + } catch ( Authorization_Flow_Exception $e ) { + return $this->error_response( 'registration_failed', $e ); + } catch ( Throwable $e ) { + return $this->handle_exception( $e ); + } + + $body = [ + 'authorize_url' => $authorize_url, + 'status' => $this->status_presenter->present(), + ]; + + return new WP_REST_Response( $body, 200 ); + } + + /** + * DELETE /myyoast/registration — disconnects the site server-side and locally. + * + * @return WP_REST_Response + */ + public function deregister() { + // Disconnect is best-effort on the server but always authoritative + // locally: whatever happens with the remote RFC 7592 DELETE, the site + // ends up disconnected here. An orphaned server-side client is cleaned up + // automatically by MyYoast. deregister() already clears the local + // registration and returns false (rather than throwing) on transport + // failure. + $remote_cleared = false; + try { + $remote_cleared = $this->myyoast_client->deregister(); + } catch ( Throwable $e ) { + $this->logger->warning( + 'Unexpected error during MyYoast deregistration; disconnecting locally anyway: {error}', + [ 'error' => $e->getMessage() ], + ); + } finally { + // Always clear site tokens, even when the remote call threw, so the + // site is never left half-connected. + $this->myyoast_client->clear_all_site_tokens(); + } + + if ( ! $remote_cleared ) { + $this->logger->warning( 'MyYoast server-side deregistration was not confirmed; the site was disconnected locally.' ); + } + + return $this->respond_with_status( 200, 'disconnect_success' ); + } + + /** + * Returns a "not provisioned" response when SS or IAT is empty. + * + * @return WP_REST_Response|null Response when blocked, null otherwise. + */ + private function require_provisioned(): ?WP_REST_Response { + if ( $this->is_provisioned() ) { + return null; + } + + return $this->error_response( 'not_provisioned' ); + } + + /** + * Whether the plugin is provisioned for OAuth (software statement + IAT). + * + * @return bool + */ + private function is_provisioned(): bool { + return ( $this->issuer_config->get_software_statement() !== '' ) + && ( $this->issuer_config->get_initial_access_token() !== '' ); + } + + /** + * Whether the supplied URI is among the registered client's redirect URIs. + * + * @param array> $metadata The registered client's metadata. + * @param string $uri The URI to check. + * + * @return bool + */ + private function is_known_redirect_uri( array $metadata, string $uri ): bool { + if ( $uri === '' ) { + return false; + } + + $stored = ( $metadata['redirect_uris'] ?? [] ); + if ( ! \is_array( $stored ) ) { + return false; + } + + return \in_array( $uri, $stored, true ); + } + + /** + * Maps an exception to a REST error response. + * + * The REST endpoint itself executed correctly — what failed is an upstream + * call to MyYoast or a precondition. We therefore return HTTP 200 with an + * `error_code` in the body that the UI translates into actionable copy. + * Genuine request-validation failures return 4xx separately (see callers). + * + * @param Throwable $exception The exception to handle. + * + * @return WP_REST_Response + */ + private function handle_exception( Throwable $exception ): WP_REST_Response { + if ( $exception instanceof Registration_Not_Found_Exception ) { + return $this->error_response( 'registration_gone', $exception ); + } + + if ( $exception instanceof Rate_Limited_Exception ) { + $retry_after = $exception->get_retry_after_seconds(); + $details = ( $retry_after !== null ) ? [ 'retry_after_seconds' => $retry_after ] : []; + return $this->error_response( 'rate_limited', $exception, 200, $details ); + } + + if ( $exception instanceof Server_Capability_Exception ) { + return $this->error_response( 'server_capability', $exception ); + } + + if ( $exception instanceof Discovery_Failed_Exception ) { + return $this->error_response( 'myyoast_unreachable', $exception ); + } + + if ( $exception instanceof Token_Request_Failed_Exception ) { + $code = ( $exception->get_error_code() === 'invalid_grant' ) ? 'token_request_failed_invalid_grant' : 'token_request_failed'; + return $this->error_response( $code, $exception ); + } + + if ( $exception instanceof Token_Storage_Exception ) { + return $this->error_response( 'token_storage_failed', $exception ); + } + + if ( $exception instanceof Invalid_Resource_Exception ) { + return $this->error_response( 'invalid_resource', $exception ); + } + + if ( $exception instanceof Registration_Failed_Exception ) { + return $this->error_response( 'registration_failed', $exception ); + } + + $this->logger->error( + 'Unexpected exception in MyYoast management route: {message}', + [ 'message' => $exception->getMessage() ], + ); + + return $this->error_response( 'unexpected_error', $exception ); + } + + /** + * Builds a successful response carrying the refreshed status payload. + * + * @param int $status The HTTP status. + * @param string|null $message_key The key in the i18n message map for the success notice, or null when none applies. + * + * @return WP_REST_Response + */ + private function respond_with_status( int $status, ?string $message_key ): WP_REST_Response { + $body = [ + 'status' => $this->status_presenter->present(), + ]; + if ( $message_key !== null ) { + $body['message_key'] = $message_key; + } + + return new WP_REST_Response( $body, $status ); + } + + /** + * Builds an error response. + * + * Defaults to HTTP 200 — the REST endpoint succeeded; the failure is in + * an upstream call or precondition, and the UI keys off `error_code`, + * not the HTTP status. Genuine 4xx (e.g. validation failures) pass an + * explicit status. + * + * @param string $error_code The machine-readable error code (looked up client-side in the i18n map). + * @param Throwable|null $exception Optional exception (logged when present). + * @param int $status The HTTP status. Defaults to 200. + * @param array $details Optional extra fields the UI may use to enrich the error message. + * + * @return WP_REST_Response + */ + private function error_response( string $error_code, ?Throwable $exception = null, int $status = 200, array $details = [] ): WP_REST_Response { + if ( $exception !== null ) { + $this->logger->warning( + 'MyYoast management error ({code}): {message}', + [ + 'code' => $error_code, + 'message' => $exception->getMessage(), + ], + ); + } + + $body = [ + 'error_code' => $error_code, + 'status' => $this->status_presenter->present(), + ]; + if ( $details !== [] ) { + $body['details'] = $details; + } + + return new WP_REST_Response( $body, $status ); + } +} diff --git a/src/myyoast-client/user-interface/oauth-callback-integration.php b/src/myyoast-client/user-interface/oauth-callback-integration.php new file mode 100644 index 00000000000..e8d491cb265 --- /dev/null +++ b/src/myyoast-client/user-interface/oauth-callback-integration.php @@ -0,0 +1,237 @@ +myyoast_client = $myyoast_client; + $this->auth_code_handler = $auth_code_handler; + $this->redirect_helper = $redirect_helper; + $this->logger = new NullLogger(); + } + + /** + * Returns the conditionals on which this integration should be loaded. + * + * @return array + */ + public static function get_conditionals() { + return [ MyYoast_Connection_Conditional::class ]; + } + + /** + * Registers the callback endpoint and points the site's OAuth redirect URI at it. + * + * @return void + */ + public function register_hooks() { + \add_action( 'admin_post_' . self::CALLBACK_ACTION, [ $this, 'handle' ] ); + } + + + /** + * Returns this site's dedicated OAuth callback endpoint URL. + * + * @return string The callback URL. + */ + public static function get_callback_url(): string { + return \get_admin_url( null, 'admin-post.php?action=' . self::CALLBACK_ACTION ); + } + + /** + * Handles the OAuth callback request. + * + * @return void + */ + public function handle(): void { + $user_id = \get_current_user_id(); + $return_url = $this->resolve_return_url( $user_id ); + + // admin_post_* (no _nopriv variant) only fires for logged-in users, so $user_id should + // always be > 0 here. Defensive check in case the hook is dispatched manually. + if ( $user_id <= 0 ) { + $this->redirect_helper->do_safe_redirect( $return_url ); + return; + } + + $error = $this->read_query_arg( 'error' ); + if ( $error !== '' ) { + $this->auth_code_handler->discard_flow_state( $user_id ); + $key = ( $error === 'access_denied' ) ? 'connection_cancelled' : 'unexpected_error'; + $this->set_outcome( $user_id, 'error', $key ); + $this->redirect_helper->do_safe_redirect( $return_url ); + return; + } + + $code = $this->read_query_arg( 'code' ); + $state = $this->read_query_arg( 'state' ); + + if ( $code === '' || $state === '' ) { + // Stale bookmark or someone hitting the callback URL directly. No notification. + $this->redirect_helper->do_safe_redirect( $return_url ); + return; + } + + try { + $this->myyoast_client->exchange_authorization_code( $user_id, $code, $state ); + } catch ( Token_Request_Failed_Exception $e ) { + $is_invalid_grant = ( $e->get_error_code() === 'invalid_grant' ); + $key = ( $is_invalid_grant ) ? 'token_request_failed_invalid_grant' : 'token_request_failed'; + $this->set_outcome( $user_id, 'error', $key ); + $this->redirect_helper->do_safe_redirect( $return_url ); + return; + } catch ( Throwable $e ) { + $this->logger->error( + 'Unexpected error during MyYoast OAuth callback exchange for user {user_id}: {error}', + [ + 'user_id' => $user_id, + 'error' => $e->getMessage(), + ], + ); + $this->set_outcome( $user_id, 'error', 'unexpected_error' ); + $this->redirect_helper->do_safe_redirect( $return_url ); + return; + } + + // The authorization-code handler marks the redirect URI validated as part of + // exchanging the code, so the refreshed status already reflects the verified state. + $this->set_outcome( $user_id, 'success', 'verify_success' ); + $this->redirect_helper->do_safe_redirect( $return_url ); + } + + /** + * Resolves the URL to send the browser back to after the callback runs. + * + * Falls back to the integrations page when no return URL is stored + * (stale bookmark, no pending flow). + * + * @param int $user_id The WordPress user ID. + * + * @return string The return URL. + */ + private function resolve_return_url( int $user_id ): string { + $fallback = \admin_url( 'admin.php?page=' . General_Page_Integration::PAGE ); + + if ( $user_id > 0 ) { + $stored = $this->auth_code_handler->get_return_url( $user_id ); + if ( \is_string( $stored ) && $stored !== '' ) { + // Defense in depth: the stored URL is only ever written as an + // admin_url by the management route, but validate it against the + // site's own host before redirecting so a tampered store entry + // can't become an open redirect. + return \wp_validate_redirect( $stored, $fallback ); + } + } + + return $fallback; + } + + /** + * Reads a query argument, returning an empty string when missing. + * + * @param string $name The query argument name. + * + * @return string The sanitized value. + */ + private function read_query_arg( string $name ): string { + // phpcs:disable WordPress.Security.NonceVerification.Recommended -- CSRF defense is OAuth `state` validated inside exchange_code. + if ( ! isset( $_GET[ $name ] ) || ! \is_string( $_GET[ $name ] ) ) { + return ''; + } + return \sanitize_text_field( \wp_unslash( $_GET[ $name ] ) ); + // phpcs:enable WordPress.Security.NonceVerification.Recommended + } + + /** + * Stores the callback outcome in a short-lived per-user transient. + * + * @param int $user_id The WordPress user ID. + * @param string $kind The outcome kind, either "success" or "error". + * @param string $key The message key the front-end maps to copy. + * + * @return void + */ + private function set_outcome( int $user_id, string $kind, string $key ): void { + if ( $user_id <= 0 ) { + return; + } + \set_transient( + self::TRANSIENT_PREFIX . $user_id, + [ + 'kind' => $kind, + 'key' => $key, + ], + self::TRANSIENT_TTL, + ); + } +} diff --git a/src/myyoast-client/user-interface/status-presenter.php b/src/myyoast-client/user-interface/status-presenter.php new file mode 100644 index 00000000000..9ce6a3ee127 --- /dev/null +++ b/src/myyoast-client/user-interface/status-presenter.php @@ -0,0 +1,211 @@ +client_registration = $client_registration; + $this->issuer_config = $issuer_config; + $this->redirect_uri = $redirect_uri; + } + + /** + * Returns the current status payload. + * + * @return array{is_provisioned: bool, is_registered: bool, registered_at: int|null, registered_at_iso: string|null, redirect_uris: array, redirect_uris_match: bool} + */ + public function present(): array { + $is_provisioned = ( $this->issuer_config->get_software_statement() !== '' ) + && ( $this->issuer_config->get_initial_access_token() !== '' ); + + $registered_client = $this->client_registration->get_registered_client(); + $is_registered = ( $registered_client !== null ); + + $registered_at = null; + $registered_at_iso = null; + $redirect_uris = []; + $redirect_uris_match = true; + + if ( $registered_client !== null ) { + $registered_at = $this->extract_registered_at( $registered_client ); + $registered_at_iso = ( $registered_at !== null ) ? \gmdate( 'c', $registered_at ) : null; + $redirect_uris = $this->extract_redirect_uris( $registered_client ); + $redirect_uris_match = $this->redirect_uris_match( $registered_client ); + } + + return [ + 'is_provisioned' => $is_provisioned, + 'is_registered' => $is_registered, + 'registered_at' => $registered_at, + 'registered_at_iso' => $registered_at_iso, + 'redirect_uris' => $redirect_uris, + 'redirect_uris_match' => $redirect_uris_match, + ]; + } + + /** + * Extracts the registration timestamp (RFC 7591 `client_id_issued_at`). + * + * @param Registered_Client $client The registered client. + * + * @return int|null Unix timestamp, or null if absent or not coercible. + */ + private function extract_registered_at( Registered_Client $client ): ?int { + $metadata = $client->get_metadata(); + if ( ! isset( $metadata['client_id_issued_at'] ) ) { + return null; + } + + $value = $metadata['client_id_issued_at']; + if ( ! \is_numeric( $value ) ) { + return null; + } + + $timestamp = (int) $value; + return ( $timestamp > 0 ) ? $timestamp : null; + } + + /** + * Whether the currently-computed redirect URI is among the stored ones. + * + * Compares the full URI — a mismatch means the site's URL has changed + * since it was connected, and the registration needs updating. + * + * @param Registered_Client $client The registered client. + * + * @return bool + */ + private function redirect_uris_match( Registered_Client $client ): bool { + $metadata = $client->get_metadata(); + $stored = ( $metadata['redirect_uris'] ?? [] ); + if ( ! \is_array( $stored ) ) { + return false; + } + + return \in_array( $this->redirect_uri->get(), $stored, true ); + } + + /** + * Returns the stored redirect URIs annotated with their origin (scheme + + * host + optional port) and their verification state. + * + * Verification state is hardcoded to `false` for now: issue #1207 introduces + * the verified-site concept along with the helpers that decide which URIs + * have completed an authorization code flow. Replace this once #1207 lands + * on trunk. + * + * @param Registered_Client $client The registered client. + * + * @return array + */ + private function extract_redirect_uris( Registered_Client $client ): array { + $metadata = $client->get_metadata(); + $uris = ( $metadata['redirect_uris'] ?? [] ); + if ( ! \is_array( $uris ) ) { + return []; + } + + $result = []; + foreach ( $uris as $uri ) { + if ( ! \is_string( $uri ) || $uri === '' ) { + continue; + } + + $origin = $this->extract_origin( $uri ); + if ( $origin === null ) { + continue; + } + + $result[] = [ + 'uri' => $uri, + 'origin' => $origin, + 'is_verified' => $this->is_uri_verified( $uri ), + ]; + } + + return $result; + } + + /** + * Extracts the origin (scheme + host + optional port) from a URI. + * + * @param string $uri The URI to parse. + * + * @return string|null The origin, or null if the URI couldn't be parsed. + */ + private function extract_origin( string $uri ): ?string { + $parts = \wp_parse_url( $uri ); + if ( ! \is_array( $parts ) || empty( $parts['scheme'] ) || empty( $parts['host'] ) ) { + return null; + } + + $origin = $parts['scheme'] . '://' . $parts['host']; + if ( isset( $parts['port'] ) ) { + $origin .= ':' . $parts['port']; + } + + return $origin; + } + + /** + * Whether the given redirect URI has been verified by completing an + * authorization code flow. + * + * TODO: replace stub once #1207 (Yoast AI requests authenticated via + * MyYoast OAuth tokens) lands on trunk — it introduces the verified-site + * concept and the helper to check it. + * + * @param string $uri The redirect URI to check. + * + * @return bool + */ + private function is_uri_verified( string $uri ): bool { // phpcs:ignore Generic.CodeAnalysis.UnusedFunctionParameter.Found,VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable -- Stub ignores $uri until #1207 adds the verified-site helper. + return false; + } +} diff --git a/tests/Unit/Integrations/Admin/Integrations_Page_Integration_Test.php b/tests/Unit/Integrations/Admin/Integrations_Page_Integration_Test.php index c6179dca092..b03d526c328 100644 --- a/tests/Unit/Integrations/Admin/Integrations_Page_Integration_Test.php +++ b/tests/Unit/Integrations/Admin/Integrations_Page_Integration_Test.php @@ -12,6 +12,7 @@ use Yoast\WP\SEO\Dashboard\Infrastructure\Integrations\Site_Kit; use Yoast\WP\SEO\Helpers\Options_Helper; use Yoast\WP\SEO\Integrations\Admin\Integrations_Page; +use Yoast\WP\SEO\MyYoast_Client\User_Interface\Integrations_Page_Script_Data; use Yoast\WP\SEO\Schema\Application\Configuration\Schema_Configuration; use Yoast\WP\SEO\Tests\Unit\TestCase; @@ -74,6 +75,13 @@ final class Integrations_Page_Integration_Test extends TestCase { */ private $schema_configuration; + /** + * The MyYoast connection script-data provider. + * + * @var Mockery\MockInterface|Integrations_Page_Script_Data + */ + private $myyoast_connection_script_data; + /** * The instance under test. * @@ -98,6 +106,7 @@ protected function set_up() { $this->site_kit_configuration = Mockery::mock( Site_Kit::class ); $this->site_kit_consent_management_endpoint = Mockery::mock( Site_Kit_Consent_Management_Endpoint::class ); $this->schema_configuration = Mockery::mock( Schema_Configuration::class ); + $this->myyoast_connection_script_data = Mockery::mock( Integrations_Page_Script_Data::class ); $this->instance = new Integrations_Page( $this->admin_asset_manager, @@ -107,6 +116,7 @@ protected function set_up() { $this->site_kit_configuration, $this->site_kit_consent_management_endpoint, $this->schema_configuration, + $this->myyoast_connection_script_data, ); } @@ -210,6 +220,7 @@ public function test_enqueue_assets() { $this->site_kit_consent_management_endpoint->expects( 'get_url' ) ->andReturn( 'https://www.example.com/manage-consent' ); $this->schema_configuration->expects( 'is_schema_disabled_programmatically' )->andReturnFalse(); + $this->myyoast_connection_script_data->expects( 'present' )->andReturnNull(); $this->admin_asset_manager->expects( 'localize_script' )->with( 'integrations-page', @@ -243,6 +254,7 @@ public function test_enqueue_assets() { 'site_kit_configuration' => $site_kit_config, 'site_kit_consent_management_url' => 'https://www.example.com/manage-consent', 'schema_framework_enabled' => true, + 'myyoast_connection' => null, ], ); diff --git a/tests/Unit/MyYoast_Client/Infrastructure/WordPress/Redirect_URI_Provider_Test.php b/tests/Unit/MyYoast_Client/Infrastructure/WordPress/Redirect_URI_Provider_Test.php index fa8c4b7d9b1..41f3573a47b 100644 --- a/tests/Unit/MyYoast_Client/Infrastructure/WordPress/Redirect_URI_Provider_Test.php +++ b/tests/Unit/MyYoast_Client/Infrastructure/WordPress/Redirect_URI_Provider_Test.php @@ -21,7 +21,7 @@ final class Redirect_URI_Provider_Test extends TestCase { * * @var string */ - private const CANONICAL = 'https://example.com/wp-admin/admin.php?page=wpseo_dashboard&yoast_myyoast_oauth_callback=1'; + private const CANONICAL = 'https://example.com/wp-admin/admin-post.php?action=yoast_myyoast_oauth_callback'; /** * The test instance. diff --git a/tests/Unit/MyYoast_Client/User_Interface/Integrations_Page_Script_Data_Test.php b/tests/Unit/MyYoast_Client/User_Interface/Integrations_Page_Script_Data_Test.php new file mode 100644 index 00000000000..bac65159049 --- /dev/null +++ b/tests/Unit/MyYoast_Client/User_Interface/Integrations_Page_Script_Data_Test.php @@ -0,0 +1,219 @@ +status_presenter = Mockery::mock( Status_Presenter::class ); + $this->myyoast_connection_conditional = Mockery::mock( MyYoast_Connection_Conditional::class ); + $this->instance = new Integrations_Page_Script_Data( + $this->status_presenter, + $this->myyoast_connection_conditional, + ); + } + + /** + * Tests the payload is returned when the feature flag is enabled and no + * callback outcome transient is pending. + * + * @covers ::__construct + * @covers ::present + * @covers ::consume_callback_outcome + * + * @return void + */ + public function test_present_when_enabled() { + $status = [ + 'is_provisioned' => true, + 'is_registered' => false, + 'registered_at' => null, + 'registered_at_iso' => null, + 'redirect_uris' => [], + ]; + + $this->myyoast_connection_conditional->shouldReceive( 'is_met' )->andReturn( true ); + $this->status_presenter->shouldReceive( 'present' )->andReturn( $status ); + + Monkey\Functions\expect( 'admin_url' ) + ->with( 'profile.php' ) + ->andReturn( 'https://example.com/wp-admin/profile.php' ); + Monkey\Functions\expect( 'get_current_user_id' )->andReturn( 42 ); + Monkey\Functions\expect( 'get_transient' ) + ->with( 'wpseo_myyoast_oauth_outcome_42' ) + ->andReturn( false ); + + $result = $this->instance->present(); + + $this->assertIsArray( $result ); + $this->assertSame( $status, $result['initialStatus'] ); + $this->assertSame( 'https://example.com/wp-admin/profile.php', $result['profileUrl'] ); + $this->assertNull( $result['callbackOutcome'] ); + } + + /** + * Tests the callback outcome transient is read and consumed. + * + * @covers ::present + * @covers ::consume_callback_outcome + * + * @return void + */ + public function test_present_consumes_callback_outcome() { + $status = [ + 'is_provisioned' => true, + 'is_registered' => true, + 'registered_at' => 1_731_369_600, + 'registered_at_iso' => '2024-11-12T00:00:00+00:00', + 'redirect_uris' => [], + ]; + + $this->myyoast_connection_conditional->shouldReceive( 'is_met' )->andReturn( true ); + $this->status_presenter->shouldReceive( 'present' )->andReturn( $status ); + + Monkey\Functions\expect( 'admin_url' ) + ->with( 'profile.php' ) + ->andReturn( 'https://example.com/wp-admin/profile.php' ); + Monkey\Functions\expect( 'get_current_user_id' )->andReturn( 7 ); + Monkey\Functions\expect( 'get_transient' ) + ->with( 'wpseo_myyoast_oauth_outcome_7' ) + ->andReturn( + [ + 'kind' => 'success', + 'key' => 'verify_success', + ], + ); + Monkey\Functions\expect( 'delete_transient' ) + ->once() + ->with( 'wpseo_myyoast_oauth_outcome_7' ); + + $result = $this->instance->present(); + + $this->assertIsArray( $result ); + $this->assertSame( + [ + 'kind' => 'success', + 'key' => 'verify_success', + ], + $result['callbackOutcome'], + ); + } + + /** + * Tests a malformed transient payload is ignored. + * + * @covers ::consume_callback_outcome + * + * @return void + */ + public function test_present_ignores_malformed_callback_outcome() { + $status = [ + 'is_provisioned' => true, + 'is_registered' => false, + 'registered_at' => null, + 'registered_at_iso' => null, + 'redirect_uris' => [], + ]; + + $this->myyoast_connection_conditional->shouldReceive( 'is_met' )->andReturn( true ); + $this->status_presenter->shouldReceive( 'present' )->andReturn( $status ); + + Monkey\Functions\expect( 'admin_url' ) + ->with( 'profile.php' ) + ->andReturn( 'https://example.com/wp-admin/profile.php' ); + Monkey\Functions\expect( 'get_current_user_id' )->andReturn( 9 ); + Monkey\Functions\expect( 'get_transient' ) + ->with( 'wpseo_myyoast_oauth_outcome_9' ) + ->andReturn( [ 'kind' => 'success' ] ); + Monkey\Functions\expect( 'delete_transient' ) + ->once() + ->with( 'wpseo_myyoast_oauth_outcome_9' ); + + $result = $this->instance->present(); + + $this->assertNull( $result['callbackOutcome'] ); + } + + /** + * Tests the callback outcome is not consulted when no user is logged in. + * + * @covers ::consume_callback_outcome + * + * @return void + */ + public function test_present_skips_transient_lookup_without_user() { + $status = [ + 'is_provisioned' => true, + 'is_registered' => false, + 'registered_at' => null, + 'registered_at_iso' => null, + 'redirect_uris' => [], + ]; + + $this->myyoast_connection_conditional->shouldReceive( 'is_met' )->andReturn( true ); + $this->status_presenter->shouldReceive( 'present' )->andReturn( $status ); + + Monkey\Functions\expect( 'admin_url' ) + ->with( 'profile.php' ) + ->andReturn( 'https://example.com/wp-admin/profile.php' ); + Monkey\Functions\expect( 'get_current_user_id' )->andReturn( 0 ); + + $result = $this->instance->present(); + + $this->assertNull( $result['callbackOutcome'] ); + } + + /** + * Tests `null` is returned when the feature flag is disabled. + * + * @covers ::present + * + * @return void + */ + public function test_present_when_disabled() { + $this->myyoast_connection_conditional->shouldReceive( 'is_met' )->andReturn( false ); + $this->status_presenter->shouldNotReceive( 'present' ); + + $this->assertNull( $this->instance->present() ); + } +} diff --git a/tests/Unit/MyYoast_Client/User_Interface/Management_Route_Test.php b/tests/Unit/MyYoast_Client/User_Interface/Management_Route_Test.php new file mode 100644 index 00000000000..24ca199d95e --- /dev/null +++ b/tests/Unit/MyYoast_Client/User_Interface/Management_Route_Test.php @@ -0,0 +1,694 @@ +myyoast_client = Mockery::mock( MyYoast_Client::class ); + $this->status_presenter = Mockery::mock( Status_Presenter::class ); + $this->issuer_config = Mockery::mock( Issuer_Config::class ); + $this->redirect_uri = Mockery::mock( OAuth_Redirect_Uri::class ); + $this->client_registration = Mockery::mock( Client_Registration_Interface::class ); + + $this->instance = new Management_Route( + $this->myyoast_client, + $this->status_presenter, + $this->issuer_config, + $this->redirect_uri, + $this->client_registration, + ); + } + + /** + * Marks the config as provisioned (default in most tests). + * + * @return void + */ + private function provision(): void { + $this->issuer_config->shouldReceive( 'get_software_statement' )->andReturn( 'jwt' ); + $this->issuer_config->shouldReceive( 'get_initial_access_token' )->andReturn( 'iat' ); + } + + /** + * Marks the config as unprovisioned. + * + * @return void + */ + private function unprovision(): void { + $this->issuer_config->shouldReceive( 'get_software_statement' )->andReturn( '' ); + $this->issuer_config->shouldReceive( 'get_initial_access_token' )->andReturn( '' ); + } + + /** + * Returns the default status payload used by mocked presenter calls. + * + * @return array + */ + private function status_payload(): array { + return [ + 'is_provisioned' => true, + 'is_registered' => true, + 'registered_at' => 1_731_369_600, + 'registered_at_iso' => '2025-11-12T00:00:00+00:00', + 'redirect_uris' => [ + [ + 'uri' => 'https://example.com/wp-admin/admin.php?page=wpseo_dashboard&yoast_myyoast_oauth_callback=1', + 'origin' => 'https://example.com', + 'is_verified' => false, + ], + ], + 'redirect_uris_match' => true, + ]; + } + + /** + * Tests the conditionals. + * + * @covers ::get_conditionals + * + * @return void + */ + public function test_get_conditionals() { + $this->assertSame( + [ MyYoast_Connection_Conditional::class ], + Management_Route::get_conditionals(), + ); + } + + /** + * Tests that all routes are registered. Five register_rest_route calls + * are made: /status (GET), /verify (POST), /register (POST), + * /registration (PUT + DELETE on the same path), and /authorize (POST). + * + * @covers ::register_routes + * + * @return void + */ + public function test_register_routes() { + Monkey\Functions\expect( 'register_rest_route' )->times( 5 ); + + $this->instance->register_routes(); + } + + /** + * Tests that the permission callback checks `wpseo_manage_options`. + * + * @covers ::can_manage + * + * @return void + */ + public function test_can_manage() { + Monkey\Functions\expect( 'current_user_can' ) + ->with( 'wpseo_manage_options' ) + ->once() + ->andReturn( true ); + + $this->assertTrue( $this->instance->can_manage() ); + } + + /** + * Tests GET /myyoast/status. + * + * @covers ::get_status + * @covers ::respond_with_status + * + * @return void + */ + public function test_get_status_returns_payload() { + $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + + Mockery::mock( 'overload:' . WP_REST_Response::class ); + + $response = $this->instance->get_status(); + + $this->assertInstanceOf( WP_REST_Response::class, $response ); + } + + /** + * Tests that verify dispatches to the facade and returns a success payload. + * + * @covers ::verify + * + * @return void + */ + public function test_verify_success() { + $this->provision(); + $this->myyoast_client->shouldReceive( 'verify_registration' )->once()->andReturn( [] ); + $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + + Mockery::mock( 'overload:' . WP_REST_Response::class ); + + $response = $this->instance->verify(); + + $this->assertInstanceOf( WP_REST_Response::class, $response ); + } + + /** + * Tests that verify hitting Registration_Not_Found_Exception surfaces registration_gone. + * + * @covers ::verify + * @covers ::handle_exception + * + * @return void + */ + public function test_verify_with_registration_gone() { + $this->provision(); + $this->myyoast_client->shouldReceive( 'verify_registration' ) + ->once() + ->andThrow( new Registration_Not_Found_Exception( 'gone' ) ); + $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + + Mockery::mock( 'overload:' . WP_REST_Response::class ); + + $response = $this->instance->verify(); + + $this->assertInstanceOf( WP_REST_Response::class, $response ); + } + + /** + * Tests that a Rate_Limited_Exception carrying a Retry-After value + * surfaces it on the response body under `details.retry_after_seconds`. + * + * @covers ::handle_exception + * @covers ::error_response + * + * @return void + */ + public function test_rate_limited_response_includes_retry_after_details() { + $this->provision(); + $this->myyoast_client->shouldReceive( 'verify_registration' ) + ->once() + ->andThrow( new Rate_Limited_Exception( 'slow down', 240 ) ); + $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + + $captured = null; + $mock = Mockery::mock( 'overload:' . WP_REST_Response::class ); + $mock->shouldReceive( '__construct' )->andReturnUsing( + static function ( $body ) use ( &$captured ): void { + $captured = $body; + }, + ); + + $this->instance->verify(); + + $this->assertIsArray( $captured ); + $this->assertSame( 'rate_limited', $captured['error_code'] ); + $this->assertArrayHasKey( 'details', $captured ); + $this->assertSame( [ 'retry_after_seconds' => 240 ], $captured['details'] ); + } + + /** + * Tests that a Rate_Limited_Exception without a Retry-After value omits + * the `details` field entirely. + * + * @covers ::handle_exception + * @covers ::error_response + * + * @return void + */ + public function test_rate_limited_response_omits_details_when_retry_after_missing() { + $this->provision(); + $this->myyoast_client->shouldReceive( 'verify_registration' ) + ->once() + ->andThrow( new Rate_Limited_Exception( 'slow down' ) ); + $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + + $captured = null; + $mock = Mockery::mock( 'overload:' . WP_REST_Response::class ); + $mock->shouldReceive( '__construct' )->andReturnUsing( + static function ( $body ) use ( &$captured ): void { + $captured = $body; + }, + ); + + $this->instance->verify(); + + $this->assertIsArray( $captured ); + $this->assertSame( 'rate_limited', $captured['error_code'] ); + $this->assertArrayNotHasKey( 'details', $captured ); + } + + /** + * Tests that verify maps each domain exception to a response (smoke test + * — exception-mapping coverage is per-branch but groups into one test). + * + * @covers ::handle_exception + * + * @return void + */ + public function test_handle_exception_branches() { + $this->provision(); + $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + + Mockery::mock( 'overload:' . WP_REST_Response::class ); + + $exceptions = [ + new Rate_Limited_Exception( 'rate' ), + new Server_Capability_Exception( 'cap' ), + new Discovery_Failed_Exception( 'discovery' ), + new Token_Request_Failed_Exception( 'invalid_grant', 'bad', 400 ), + new Token_Request_Failed_Exception( 'invalid_client', 'bad', 400 ), + new Token_Storage_Exception( 'storage' ), + new Registration_Failed_Exception( 'other' ), + new Exception( 'boom' ), + ]; + + foreach ( $exceptions as $exception ) { + $this->myyoast_client->shouldReceive( 'verify_registration' ) + ->once() + ->andThrow( $exception ); + + $response = $this->instance->verify(); + + $this->assertInstanceOf( WP_REST_Response::class, $response ); + } + } + + /** + * Tests POST /myyoast/register delegates to ensure_registered with the current redirect URI. + * + * @covers ::register + * + * @return void + */ + public function test_register_delegates_to_ensure_registered() { + $this->provision(); + + $this->redirect_uri->shouldReceive( 'get' )->andReturn( 'https://example.com/wp-admin/admin.php?page=wpseo_dashboard' ); + $this->myyoast_client->shouldNotReceive( 'deregister' ); + $this->myyoast_client->shouldReceive( 'ensure_registered' ) + ->once() + ->with( [ 'https://example.com/wp-admin/admin.php?page=wpseo_dashboard' ] ); + $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + + Mockery::mock( 'overload:' . WP_REST_Response::class ); + + $response = $this->instance->register(); + + $this->assertInstanceOf( WP_REST_Response::class, $response ); + } + + /** + * Tests PUT /myyoast/registration calls update_redirect_uris with the current URI. + * + * @covers ::update_registration + * + * @return void + */ + public function test_update_registration() { + $this->provision(); + + $this->redirect_uri->shouldReceive( 'get' )->andReturn( 'https://example.com/wp-admin/admin.php?page=wpseo_dashboard' ); + $this->myyoast_client->shouldReceive( 'update_redirect_uris' ) + ->once() + ->with( [ 'https://example.com/wp-admin/admin.php?page=wpseo_dashboard' ] ); + $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + + Mockery::mock( 'overload:' . WP_REST_Response::class ); + + $response = $this->instance->update_registration(); + + $this->assertInstanceOf( WP_REST_Response::class, $response ); + } + + /** + * Tests update_registration surfaces Registration_Not_Found as a normal error response. + * + * @covers ::update_registration + * @covers ::handle_exception + * + * @return void + */ + public function test_update_registration_handles_registration_gone() { + $this->provision(); + + $this->redirect_uri->shouldReceive( 'get' )->andReturn( 'https://example.com/uri' ); + $this->myyoast_client->shouldReceive( 'update_redirect_uris' ) + ->once() + ->andThrow( new Registration_Not_Found_Exception( 'gone' ) ); + $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + + Mockery::mock( 'overload:' . WP_REST_Response::class ); + + $response = $this->instance->update_registration(); + + $this->assertInstanceOf( WP_REST_Response::class, $response ); + } + + /** + * Tests deregister dispatches to the facade and also clears all site tokens. + * + * @covers ::deregister + * + * @return void + */ + public function test_deregister_success() { + $this->provision(); + + $this->myyoast_client->shouldReceive( 'deregister' )->once()->andReturn( true ); + $this->myyoast_client->shouldReceive( 'clear_all_site_tokens' )->once(); + $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + + Mockery::mock( 'overload:' . WP_REST_Response::class ); + + $response = $this->instance->deregister(); + + $this->assertInstanceOf( WP_REST_Response::class, $response ); + } + + /** + * Tests deregister still clears site tokens and reports success when the + * server-side teardown could not be confirmed (transport failure). + * + * @covers ::deregister + * + * @return void + */ + public function test_deregister_succeeds_locally_when_remote_unconfirmed() { + $this->provision(); + + $this->myyoast_client->shouldReceive( 'deregister' )->once()->andReturn( false ); + $this->myyoast_client->shouldReceive( 'clear_all_site_tokens' )->once(); + $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + + Mockery::mock( 'overload:' . WP_REST_Response::class ); + + $response = $this->instance->deregister(); + + $this->assertInstanceOf( WP_REST_Response::class, $response ); + } + + /** + * Tests deregister still clears site tokens when the remote call throws + * unexpectedly, so the site is never left half-connected. + * + * @covers ::deregister + * + * @return void + */ + public function test_deregister_clears_tokens_when_remote_throws() { + $this->provision(); + + $this->myyoast_client->shouldReceive( 'deregister' )->once()->andThrow( new Exception( 'boom' ) ); + $this->myyoast_client->shouldReceive( 'clear_all_site_tokens' )->once(); + $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + + Mockery::mock( 'overload:' . WP_REST_Response::class ); + + $response = $this->instance->deregister(); + + $this->assertInstanceOf( WP_REST_Response::class, $response ); + } + + /** + * Tests the registration-mutating endpoints short-circuit when the plugin is + * not provisioned. + * + * Only register/update/authorize require the software statement and initial + * access token; verify and deregister work without them and are covered by + * their own tests. + * + * @covers ::require_provisioned + * + * @return void + */ + public function test_registration_actions_blocked_when_not_provisioned() { + $this->unprovision(); + $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + + $this->myyoast_client->shouldNotReceive( 'ensure_registered' ); + $this->myyoast_client->shouldNotReceive( 'update_redirect_uris' ); + $this->myyoast_client->shouldNotReceive( 'get_authorization_url' ); + + $request = Mockery::mock( WP_REST_Request::class ); + + Mockery::mock( 'overload:' . WP_REST_Response::class ); + + $this->assertInstanceOf( WP_REST_Response::class, $this->instance->register() ); + $this->assertInstanceOf( WP_REST_Response::class, $this->instance->update_registration() ); + $this->assertInstanceOf( WP_REST_Response::class, $this->instance->authorize( $request ) ); + } + + /** + * Tests verify works regardless of provisioning — it talks to MyYoast with + * the stored registration access token, not the software statement. + * + * @covers ::verify + * + * @return void + */ + public function test_verify_works_when_not_provisioned() { + $this->unprovision(); + $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + $this->myyoast_client->shouldReceive( 'verify_registration' )->once(); + + Mockery::mock( 'overload:' . WP_REST_Response::class ); + + $this->assertInstanceOf( WP_REST_Response::class, $this->instance->verify() ); + } + + /** + * Tests deregister works regardless of provisioning — disconnecting only + * needs the stored registration, not the software statement. + * + * @covers ::deregister + * + * @return void + */ + public function test_deregister_works_when_not_provisioned() { + $this->unprovision(); + $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + $this->myyoast_client->shouldReceive( 'deregister' )->once()->andReturn( true ); + $this->myyoast_client->shouldReceive( 'clear_all_site_tokens' )->once(); + + Mockery::mock( 'overload:' . WP_REST_Response::class ); + + $this->assertInstanceOf( WP_REST_Response::class, $this->instance->deregister() ); + } + + /** + * Tests authorize returns the authorization URL for a known redirect URI. + * + * @covers ::authorize + * @covers ::is_known_redirect_uri + * + * @return void + */ + public function test_authorize_returns_authorization_url() { + $this->provision(); + + $target_uri = 'https://example.com/wp-admin/admin-post.php?action=yoast_myyoast_oauth_callback'; + $return_url = 'https://example.com/wp-admin/admin.php?page=wpseo_integrations'; + $authorize_url = 'https://my.yoast.com/auth?code_challenge=abc'; + + $this->client_registration->shouldReceive( 'get_registered_client' ) + ->andReturn( + new Registered_Client( + 'client-123', + 'rat', + 'https://my.yoast.com/clients/client-123', + [ 'redirect_uris' => [ $target_uri ] ], + ), + ); + + Monkey\Functions\expect( 'get_current_user_id' )->andReturn( 42 ); + + Monkey\Functions\expect( 'admin_url' ) + ->with( 'admin.php?page=wpseo_integrations' ) + ->andReturn( 'https://example.com/wp-admin/admin.php?page=wpseo_integrations' ); + + $this->myyoast_client->shouldReceive( 'get_authorization_url' ) + ->once() + ->with( 42, $target_uri, [ 'openid' ], null, $return_url ) + ->andReturn( $authorize_url ); + + $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + + $request = Mockery::mock( WP_REST_Request::class ); + $request->shouldReceive( 'get_param' )->with( 'redirect_uri' )->andReturn( $target_uri ); + + Mockery::mock( 'overload:' . WP_REST_Response::class ); + + $response = $this->instance->authorize( $request ); + + $this->assertInstanceOf( WP_REST_Response::class, $response ); + } + + /** + * Tests authorize rejects an unknown redirect URI. + * + * @covers ::authorize + * @covers ::is_known_redirect_uri + * + * @return void + */ + public function test_authorize_rejects_unknown_redirect_uri() { + $this->provision(); + + $this->client_registration->shouldReceive( 'get_registered_client' ) + ->andReturn( + new Registered_Client( + 'client-123', + 'rat', + 'https://my.yoast.com/clients/client-123', + [ 'redirect_uris' => [ 'https://example.com/known' ] ], + ), + ); + + $this->myyoast_client->shouldNotReceive( 'get_authorization_url' ); + + $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + + $request = Mockery::mock( WP_REST_Request::class ); + $request->shouldReceive( 'get_param' )->with( 'redirect_uri' )->andReturn( 'https://attacker.example/evil' ); + + Mockery::mock( 'overload:' . WP_REST_Response::class ); + + $this->assertInstanceOf( WP_REST_Response::class, $this->instance->authorize( $request ) ); + } + + /** + * Tests authorize surfaces an error when the site is not registered. + * + * @covers ::authorize + * + * @return void + */ + public function test_authorize_when_not_registered() { + $this->provision(); + + $this->client_registration->shouldReceive( 'get_registered_client' )->andReturn( null ); + $this->myyoast_client->shouldNotReceive( 'get_authorization_url' ); + $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + + $request = Mockery::mock( WP_REST_Request::class ); + + Mockery::mock( 'overload:' . WP_REST_Response::class ); + + $this->assertInstanceOf( WP_REST_Response::class, $this->instance->authorize( $request ) ); + } + + /** + * Tests authorize maps Authorization_Flow_Exception to a registration_failed error. + * + * @covers ::authorize + * + * @return void + */ + public function test_authorize_handles_flow_exception() { + $this->provision(); + + $target_uri = 'https://example.com/known'; + + $this->client_registration->shouldReceive( 'get_registered_client' ) + ->andReturn( + new Registered_Client( + 'client-123', + 'rat', + 'https://my.yoast.com/clients/client-123', + [ 'redirect_uris' => [ $target_uri ] ], + ), + ); + + Monkey\Functions\expect( 'get_current_user_id' )->andReturn( 42 ); + + Monkey\Functions\expect( 'admin_url' ) + ->with( 'admin.php?page=wpseo_integrations' ) + ->andReturn( 'https://example.com/wp-admin/admin.php?page=wpseo_integrations' ); + + $this->myyoast_client->shouldReceive( 'get_authorization_url' ) + ->andThrow( new Authorization_Flow_Exception( 'registration_failed', 'boom' ) ); + + $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + + $request = Mockery::mock( WP_REST_Request::class ); + $request->shouldReceive( 'get_param' )->with( 'redirect_uri' )->andReturn( $target_uri ); + + Mockery::mock( 'overload:' . WP_REST_Response::class ); + + $this->assertInstanceOf( WP_REST_Response::class, $this->instance->authorize( $request ) ); + } +} diff --git a/tests/Unit/MyYoast_Client/User_Interface/OAuth_Callback_Integration_Test.php b/tests/Unit/MyYoast_Client/User_Interface/OAuth_Callback_Integration_Test.php new file mode 100644 index 00000000000..081a33c2818 --- /dev/null +++ b/tests/Unit/MyYoast_Client/User_Interface/OAuth_Callback_Integration_Test.php @@ -0,0 +1,461 @@ +myyoast_client = Mockery::mock( MyYoast_Client::class ); + $this->auth_code_handler = Mockery::mock( Authorization_Code_Handler::class ); + $this->redirect_helper = Mockery::mock( Redirect_Helper::class ); + + $this->instance = new OAuth_Callback_Integration( + $this->myyoast_client, + $this->auth_code_handler, + $this->redirect_helper, + ); + + // resolve_return_url always builds the fallback and validates any stored + // URL against it; stub both so every test path has them available. + Monkey\Functions\stubs( + [ + 'admin_url' => static function ( $path ) { + return 'https://example.com/wp-admin/' . $path; + }, + // Valid URLs pass through; a fallback would only be returned for an + // off-host URL, which is covered explicitly where it matters. + 'wp_validate_redirect' => static function ( $location ) { + return $location; + }, + ], + ); + + $_GET = []; + } + + /** + * Tears down test fixtures. + * + * @return void + */ + protected function tear_down() { + $_GET = []; + parent::tear_down(); + } + + /** + * Tests the conditional list. + * + * @covers ::get_conditionals + * + * @return void + */ + public function test_get_conditionals() { + $this->assertSame( + [ MyYoast_Connection_Conditional::class ], + OAuth_Callback_Integration::get_conditionals(), + ); + } + + /** + * Tests the admin-post hook is registered and the redirect URI is pointed at the callback endpoint. + * + * @covers ::register_hooks + * + * @return void + */ + public function test_register_hooks() { + Monkey\Actions\expectAdded( 'admin_post_yoast_myyoast_oauth_callback' ) + ->once() + ->with( [ $this->instance, 'handle' ] ); + + Monkey\Filters\expectAdded( 'wpseo_myyoast_redirect_uris' ) + ->once() + ->with( [ $this->instance, 'filter_redirect_uris' ] ); + + Monkey\Filters\expectAdded( 'wpseo_myyoast_authorization_redirect_uri' ) + ->once() + ->with( [ $this->instance, 'filter_authorization_redirect_uri' ] ); + + $this->instance->register_hooks(); + } + + /** + * Tests the redirect-URI filters replace the defaults with the dedicated callback endpoint. + * + * @covers ::filter_redirect_uris + * @covers ::filter_authorization_redirect_uri + * @covers ::get_callback_url + * + * @return void + */ + public function test_redirect_uri_filters_use_the_callback_endpoint() { + $callback_url = 'https://example.com/wp-admin/admin-post.php?action=yoast_myyoast_oauth_callback'; + + Monkey\Functions\expect( 'get_admin_url' ) + ->with( null, 'admin-post.php?action=yoast_myyoast_oauth_callback' ) + ->andReturn( $callback_url ); + + $this->assertSame( [ $callback_url ], $this->instance->filter_redirect_uris( [ 'https://default/cb' ] ) ); + $this->assertSame( $callback_url, $this->instance->filter_authorization_redirect_uri( 'https://default/cb' ) ); + } + + /** + * Tests the happy path: code + state exchange succeeds, success transient is set. + * + * @covers ::__construct + * @covers ::handle + * @covers ::resolve_return_url + * @covers ::read_query_arg + * @covers ::set_outcome + * + * @return void + */ + public function test_handle_exchanges_code_on_success() { + $_GET = [ + 'code' => 'abc', + 'state' => 'xyz', + ]; + + $this->expect_user( 42 ); + $this->expect_return_url_lookup( 42, self::RETURN_URL ); + + $this->myyoast_client->shouldReceive( 'exchange_authorization_code' ) + ->once() + ->with( 42, 'abc', 'xyz' ); + + $this->expect_transient_set( + 42, + [ + 'kind' => 'success', + 'key' => 'verify_success', + ], + ); + + $this->expect_redirect( self::RETURN_URL ); + + $this->instance->handle(); + } + + /** + * Tests `error=access_denied` translates to a `connection_cancelled` transient and discards flow state. + * + * @covers ::handle + * + * @return void + */ + public function test_handle_access_denied_sets_cancelled_outcome() { + $_GET = [ 'error' => 'access_denied' ]; + + $this->expect_user( 7 ); + $this->expect_return_url_lookup( 7, self::RETURN_URL ); + + $this->auth_code_handler->shouldReceive( 'discard_flow_state' )->once()->with( 7 ); + + $this->myyoast_client->shouldNotReceive( 'exchange_authorization_code' ); + + $this->expect_transient_set( + 7, + [ + 'kind' => 'error', + 'key' => 'connection_cancelled', + ], + ); + + $this->expect_redirect( self::RETURN_URL ); + + $this->instance->handle(); + } + + /** + * Tests other OAuth provider errors map to a generic unexpected_error outcome. + * + * @covers ::handle + * + * @return void + */ + public function test_handle_generic_oauth_error_maps_to_unexpected() { + $_GET = [ 'error' => 'server_error' ]; + + $this->expect_user( 7 ); + $this->expect_return_url_lookup( 7, self::RETURN_URL ); + + $this->auth_code_handler->shouldReceive( 'discard_flow_state' )->once()->with( 7 ); + + $this->expect_transient_set( + 7, + [ + 'kind' => 'error', + 'key' => 'unexpected_error', + ], + ); + + $this->expect_redirect( self::RETURN_URL ); + + $this->instance->handle(); + } + + /** + * Tests Token_Request_Failed_Exception with `invalid_grant` maps to its dedicated message key. + * + * @covers ::handle + * + * @return void + */ + public function test_handle_invalid_grant_exception_maps_to_dedicated_key() { + $_GET = [ + 'code' => 'abc', + 'state' => 'xyz', + ]; + + $this->expect_user( 11 ); + $this->expect_return_url_lookup( 11, self::RETURN_URL ); + + $this->myyoast_client->shouldReceive( 'exchange_authorization_code' ) + ->once() + ->andThrow( new Token_Request_Failed_Exception( 'invalid_grant', 'expired' ) ); + + $this->expect_transient_set( + 11, + [ + 'kind' => 'error', + 'key' => 'token_request_failed_invalid_grant', + ], + ); + + $this->expect_redirect( self::RETURN_URL ); + + $this->instance->handle(); + } + + /** + * Tests other Token_Request_Failed_Exception codes map to the generic token failure key. + * + * @covers ::handle + * + * @return void + */ + public function test_handle_token_request_failure_maps_to_generic_key() { + $_GET = [ + 'code' => 'abc', + 'state' => 'xyz', + ]; + + $this->expect_user( 11 ); + $this->expect_return_url_lookup( 11, self::RETURN_URL ); + + $this->myyoast_client->shouldReceive( 'exchange_authorization_code' ) + ->once() + ->andThrow( new Token_Request_Failed_Exception( 'invalid_request', 'state mismatch' ) ); + + $this->expect_transient_set( + 11, + [ + 'kind' => 'error', + 'key' => 'token_request_failed', + ], + ); + + $this->expect_redirect( self::RETURN_URL ); + + $this->instance->handle(); + } + + /** + * Tests an unexpected exception sets an unexpected_error outcome. + * + * @covers ::handle + * + * @return void + */ + public function test_handle_unexpected_exception() { + $_GET = [ + 'code' => 'abc', + 'state' => 'xyz', + ]; + + $this->expect_user( 11 ); + $this->expect_return_url_lookup( 11, self::RETURN_URL ); + + $this->myyoast_client->shouldReceive( 'exchange_authorization_code' ) + ->once() + ->andThrow( new Exception( 'boom' ) ); + + $this->expect_transient_set( + 11, + [ + 'kind' => 'error', + 'key' => 'unexpected_error', + ], + ); + + $this->expect_redirect( self::RETURN_URL ); + + $this->instance->handle(); + } + + /** + * Tests visiting the callback URL without code/state/error just redirects to the return URL. + * + * @covers ::handle + * + * @return void + */ + public function test_handle_missing_params_redirects_without_outcome() { + $_GET = []; + + $this->expect_user( 42 ); + $this->expect_return_url_lookup( 42, self::RETURN_URL ); + + $this->myyoast_client->shouldNotReceive( 'exchange_authorization_code' ); + Monkey\Functions\expect( 'set_transient' )->never(); + + $this->expect_redirect( self::RETURN_URL ); + + $this->instance->handle(); + } + + /** + * Tests the handler falls back to the integrations page when no return URL is stored. + * + * @covers ::resolve_return_url + * + * @return void + */ + public function test_handle_falls_back_to_integrations_page_when_no_return_url_stored() { + $_GET = []; + + $this->expect_user( 42 ); + + $this->auth_code_handler->shouldReceive( 'get_return_url' )->once()->with( 42 )->andReturn( null ); + + $this->expect_redirect( self::FALLBACK_URL ); + + $this->instance->handle(); + } + + /** + * Tests anonymous requests are redirected to the fallback without any transient activity. + * + * @covers ::handle + * + * @return void + */ + public function test_handle_anonymous_request_redirects_to_fallback() { + $_GET = [ + 'code' => 'abc', + 'state' => 'xyz', + ]; + + $this->expect_user( 0 ); + + $this->myyoast_client->shouldNotReceive( 'exchange_authorization_code' ); + Monkey\Functions\expect( 'set_transient' )->never(); + + $this->expect_redirect( self::FALLBACK_URL ); + + $this->instance->handle(); + } + + /** + * Configures the current user id stub. + * + * @param int $user_id The user id to return. + * + * @return void + */ + private function expect_user( int $user_id ): void { + Monkey\Functions\expect( 'get_current_user_id' )->andReturn( $user_id ); + } + + /** + * Configures the stored return URL lookup. + * + * @param int $user_id The user id. + * @param string $url The URL to return. + * + * @return void + */ + private function expect_return_url_lookup( int $user_id, string $url ): void { + $this->auth_code_handler->shouldReceive( 'get_return_url' )->once()->with( $user_id )->andReturn( $url ); + } + + /** + * Configures the expected transient write. + * + * @param int $user_id The user id. + * @param array{kind: string, key: string} $value The expected stored value. + * + * @return void + */ + private function expect_transient_set( int $user_id, array $value ): void { + Monkey\Functions\expect( 'set_transient' ) + ->once() + ->with( 'wpseo_myyoast_oauth_outcome_' . $user_id, $value, \MINUTE_IN_SECONDS ); + } + + /** + * Configures the expected redirect. + * + * @param string $url The expected redirect target. + * + * @return void + */ + private function expect_redirect( string $url ): void { + $this->redirect_helper->shouldReceive( 'do_safe_redirect' )->once()->with( $url ); + } +} diff --git a/tests/Unit/MyYoast_Client/User_Interface/Status_Presenter_Test.php b/tests/Unit/MyYoast_Client/User_Interface/Status_Presenter_Test.php new file mode 100644 index 00000000000..f630d7922a5 --- /dev/null +++ b/tests/Unit/MyYoast_Client/User_Interface/Status_Presenter_Test.php @@ -0,0 +1,239 @@ +client_registration = Mockery::mock( Client_Registration_Interface::class ); + $this->issuer_config = Mockery::mock( Issuer_Config::class ); + $this->redirect_uri = Mockery::mock( OAuth_Redirect_Uri::class ); + + $this->instance = new Status_Presenter( + $this->client_registration, + $this->issuer_config, + $this->redirect_uri, + ); + + Monkey\Functions\stubs( + [ + 'wp_parse_url' => static function ( $url ) { + return \parse_url( $url ); + }, + ], + ); + } + + /** + * Tests the not-provisioned, not-registered branch. + * + * @covers ::__construct + * @covers ::present + * + * @return void + */ + public function test_present_when_not_provisioned() { + $this->issuer_config->shouldReceive( 'get_software_statement' )->andReturn( '' ); + $this->issuer_config->shouldReceive( 'get_initial_access_token' )->andReturn( '' ); + $this->client_registration->shouldReceive( 'get_registered_client' )->andReturn( null ); + + $result = $this->instance->present(); + + $this->assertFalse( $result['is_provisioned'] ); + $this->assertFalse( $result['is_registered'] ); + $this->assertNull( $result['registered_at'] ); + $this->assertNull( $result['registered_at_iso'] ); + $this->assertSame( [], $result['redirect_uris'] ); + $this->assertTrue( $result['redirect_uris_match'] ); + } + + /** + * Tests the provisioned but not-yet-registered branch. + * + * @covers ::present + * + * @return void + */ + public function test_present_when_provisioned_but_not_registered() { + $this->issuer_config->shouldReceive( 'get_software_statement' )->andReturn( 'jwt' ); + $this->issuer_config->shouldReceive( 'get_initial_access_token' )->andReturn( 'iat' ); + $this->client_registration->shouldReceive( 'get_registered_client' )->andReturn( null ); + + $result = $this->instance->present(); + + $this->assertTrue( $result['is_provisioned'] ); + $this->assertFalse( $result['is_registered'] ); + $this->assertNull( $result['registered_at'] ); + $this->assertSame( [], $result['redirect_uris'] ); + $this->assertTrue( $result['redirect_uris_match'] ); + } + + /** + * Tests the registered branch where the stored redirect URIs include the + * currently-computed one — redirect_uris_match should be true. + * + * @covers ::present + * @covers ::extract_registered_at + * @covers ::extract_redirect_uris + * @covers ::extract_origin + * @covers ::is_uri_verified + * @covers ::redirect_uris_match + * + * @return void + */ + public function test_present_when_registered_with_matching_uri() { + $this->issuer_config->shouldReceive( 'get_software_statement' )->andReturn( 'jwt' ); + $this->issuer_config->shouldReceive( 'get_initial_access_token' )->andReturn( 'iat' ); + $this->redirect_uri->shouldReceive( 'get' )->andReturn( self::CURRENT_REDIRECT_URI ); + + $staging_uri = 'https://staging.example.com/wp-admin/admin.php?page=wpseo_dashboard'; + $registered_client = new Registered_Client( + 'client-123', + 'rat', + 'https://my.yoast.com/clients/client-123', + [ + 'client_id_issued_at' => 1_731_369_600, + 'redirect_uris' => [ + self::CURRENT_REDIRECT_URI, + $staging_uri, + ], + ], + ); + + $this->client_registration->shouldReceive( 'get_registered_client' )->andReturn( $registered_client ); + + $result = $this->instance->present(); + + $this->assertTrue( $result['is_provisioned'] ); + $this->assertTrue( $result['is_registered'] ); + $this->assertSame( 1_731_369_600, $result['registered_at'] ); + $this->assertIsString( $result['registered_at_iso'] ); + $this->assertSame( + [ + [ + 'uri' => self::CURRENT_REDIRECT_URI, + 'origin' => 'https://example.com', + 'is_verified' => false, + ], + [ + 'uri' => $staging_uri, + 'origin' => 'https://staging.example.com', + 'is_verified' => false, + ], + ], + $result['redirect_uris'], + ); + $this->assertTrue( $result['redirect_uris_match'] ); + } + + /** + * Tests the registered branch where the site URL has drifted (the + * currently-computed redirect URI is no longer in the stored list). + * + * @covers ::present + * @covers ::redirect_uris_match + * + * @return void + */ + public function test_present_when_registered_with_drifted_uri() { + $this->issuer_config->shouldReceive( 'get_software_statement' )->andReturn( 'jwt' ); + $this->issuer_config->shouldReceive( 'get_initial_access_token' )->andReturn( 'iat' ); + $this->redirect_uri->shouldReceive( 'get' )->andReturn( 'https://new-domain.example.com/wp-admin/admin.php?page=wpseo_dashboard&yoast_myyoast_oauth_callback=1' ); + + $registered_client = new Registered_Client( + 'client-123', + 'rat', + 'https://my.yoast.com/clients/client-123', + [ + 'redirect_uris' => [ self::CURRENT_REDIRECT_URI ], + ], + ); + + $this->client_registration->shouldReceive( 'get_registered_client' )->andReturn( $registered_client ); + + $result = $this->instance->present(); + + $this->assertTrue( $result['is_registered'] ); + $this->assertFalse( $result['redirect_uris_match'] ); + } + + /** + * Tests that absent `client_id_issued_at` falls back to null. + * + * @covers ::extract_registered_at + * + * @return void + */ + public function test_present_when_registered_without_issued_at() { + $this->issuer_config->shouldReceive( 'get_software_statement' )->andReturn( 'jwt' ); + $this->issuer_config->shouldReceive( 'get_initial_access_token' )->andReturn( 'iat' ); + $this->redirect_uri->shouldReceive( 'get' )->andReturn( self::CURRENT_REDIRECT_URI ); + + $registered_client = new Registered_Client( + 'client-123', + 'rat', + 'https://my.yoast.com/clients/client-123', + [ + 'redirect_uris' => [ self::CURRENT_REDIRECT_URI ], + ], + ); + + $this->client_registration->shouldReceive( 'get_registered_client' )->andReturn( $registered_client ); + + $result = $this->instance->present(); + + $this->assertNull( $result['registered_at'] ); + $this->assertNull( $result['registered_at_iso'] ); + } +} From c475fbe48a6ca69231c31ff5eae324dbfe452e41 Mon Sep 17 00:00:00 2001 From: Diede Exterkate <5352634+diedexx@users.noreply.github.com> Date: Wed, 17 Jun 2026 15:54:52 +0200 Subject: [PATCH 03/26] feat(integrations-page): rewire the MyYoast REST layer to the new client API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Updates the REST management route and status presenter for the reworked client: registration takes no caller-supplied redirect URIs (the client resolves them itself), so register and the URL-change recovery both call ensure_registered(), and authorize no longer accepts or validates a redirect_uri — the client picks the registered one. The status presenter now reports real per-URI verification state via the registration's is_uri_validated(), and derives redirect_uris_match from the redirect-URI provider instead of a standalone builder. Co-Authored-By: Claude Opus 4.8 --- .../user-interface/management-route.php | 72 +++----------- .../user-interface/status-presenter.php | 69 ++++---------- .../User_Interface/Management_Route_Test.php | 95 +++---------------- .../User_Interface/Status_Presenter_Test.php | 36 ++++--- 4 files changed, 71 insertions(+), 201 deletions(-) diff --git a/src/myyoast-client/user-interface/management-route.php b/src/myyoast-client/user-interface/management-route.php index a0cc9e11df8..35ea18076a0 100644 --- a/src/myyoast-client/user-interface/management-route.php +++ b/src/myyoast-client/user-interface/management-route.php @@ -5,7 +5,6 @@ namespace Yoast\WP\SEO\MyYoast_Client\User_Interface; use Throwable; -use WP_REST_Request; use WP_REST_Response; use Yoast\WP\SEO\Conditionals\MyYoast_Connection_Conditional; use Yoast\WP\SEO\Integrations\Admin\Integrations_Page; @@ -67,13 +66,6 @@ class Management_Route implements Route_Interface, LoggerAwareInterface { */ private $issuer_config; - /** - * The redirect URI builder. - * - * @var OAuth_Redirect_Uri - */ - private $redirect_uri; - /** * The client registration port. * @@ -87,20 +79,17 @@ class Management_Route implements Route_Interface, LoggerAwareInterface { * @param MyYoast_Client $myyoast_client The MyYoast client facade. * @param Status_Presenter $status_presenter The status presenter. * @param Issuer_Config $issuer_config The issuer configuration. - * @param OAuth_Redirect_Uri $redirect_uri The redirect URI builder. * @param Client_Registration_Interface $client_registration The client registration port. */ public function __construct( MyYoast_Client $myyoast_client, Status_Presenter $status_presenter, Issuer_Config $issuer_config, - OAuth_Redirect_Uri $redirect_uri, Client_Registration_Interface $client_registration ) { $this->myyoast_client = $myyoast_client; $this->status_presenter = $status_presenter; $this->issuer_config = $issuer_config; - $this->redirect_uri = $redirect_uri; $this->client_registration = $client_registration; $this->logger = new NullLogger(); } @@ -176,12 +165,6 @@ public function register_routes() { 'methods' => 'POST', 'callback' => [ $this, 'authorize' ], 'permission_callback' => $permission_callback, - 'args' => [ - 'redirect_uri' => [ - 'type' => 'string', - 'required' => true, - ], - ], ], ); } @@ -231,7 +214,7 @@ public function register() { } try { - $this->myyoast_client->ensure_registered( [ $this->redirect_uri->get() ] ); + $this->myyoast_client->ensure_registered(); } catch ( Throwable $e ) { return $this->handle_exception( $e ); } @@ -240,9 +223,11 @@ public function register() { } /** - * PUT /myyoast/registration — updates the connection's redirect URIs. + * PUT /myyoast/registration — re-syncs the connection's redirect URIs. * - * Used to recover the connection after the site's URL has changed. + * Used to recover the connection after the site's URL has changed. The client + * resolves the current redirect URIs itself and updates the registration in + * place (RFC 7592 PUT) when the set differs from what is stored. * * @return WP_REST_Response */ @@ -253,7 +238,7 @@ public function update_registration() { } try { - $this->myyoast_client->update_redirect_uris( [ $this->redirect_uri->get() ] ); + $this->myyoast_client->ensure_registered(); } catch ( Throwable $e ) { return $this->handle_exception( $e ); } @@ -262,33 +247,26 @@ public function update_registration() { } /** - * POST /myyoast/authorize — starts the authorization-code flow for the - * given redirect URI and returns the URL the browser should be sent to. + * POST /myyoast/authorize — starts the authorization-code flow and returns + * the URL the browser should be sent to. * - * Used to verify that a stored redirect URI is reachable and legitimate by - * completing a real auth-code round-trip. The actual marking-as-verified - * happens on the callback side (introduced in #1207). - * - * @param WP_REST_Request $request The REST request. + * Completing the round-trip verifies that the site's redirect URI is + * reachable and that the user is who they claim to be. The client resolves + * the redirect URI itself, and the authorization-code handler marks it + * validated once the returning code is exchanged. * * @return WP_REST_Response */ - public function authorize( WP_REST_Request $request ): WP_REST_Response { + public function authorize(): WP_REST_Response { $gate = $this->require_provisioned(); if ( $gate !== null ) { return $gate; } - $registered_client = $this->client_registration->get_registered_client(); - if ( $registered_client === null ) { + if ( $this->client_registration->get_registered_client() === null ) { return $this->error_response( 'registration_gone' ); } - $redirect_uri = (string) $request->get_param( 'redirect_uri' ); - if ( ! $this->is_known_redirect_uri( $registered_client->get_metadata(), $redirect_uri ) ) { - return $this->error_response( 'unknown_redirect_uri', null, 400 ); - } - $user_id = \get_current_user_id(); if ( $user_id <= 0 ) { return $this->error_response( 'invalid_user', null, 401 ); @@ -299,7 +277,6 @@ public function authorize( WP_REST_Request $request ): WP_REST_Response { try { $authorize_url = $this->myyoast_client->get_authorization_url( $user_id, - $redirect_uri, [ 'openid' ], null, $return_url, @@ -374,27 +351,6 @@ private function is_provisioned(): bool { && ( $this->issuer_config->get_initial_access_token() !== '' ); } - /** - * Whether the supplied URI is among the registered client's redirect URIs. - * - * @param array> $metadata The registered client's metadata. - * @param string $uri The URI to check. - * - * @return bool - */ - private function is_known_redirect_uri( array $metadata, string $uri ): bool { - if ( $uri === '' ) { - return false; - } - - $stored = ( $metadata['redirect_uris'] ?? [] ); - if ( ! \is_array( $stored ) ) { - return false; - } - - return \in_array( $uri, $stored, true ); - } - /** * Maps an exception to a REST error response. * diff --git a/src/myyoast-client/user-interface/status-presenter.php b/src/myyoast-client/user-interface/status-presenter.php index 9ce6a3ee127..0399310c196 100644 --- a/src/myyoast-client/user-interface/status-presenter.php +++ b/src/myyoast-client/user-interface/status-presenter.php @@ -5,11 +5,12 @@ namespace Yoast\WP\SEO\MyYoast_Client\User_Interface; use Yoast\WP\SEO\MyYoast_Client\Application\Ports\Client_Registration_Interface; +use Yoast\WP\SEO\MyYoast_Client\Application\Ports\Redirect_URI_Provider_Interface; use Yoast\WP\SEO\MyYoast_Client\Domain\Registered_Client; use Yoast\WP\SEO\MyYoast_Client\Infrastructure\OIDC\Issuer_Config; /** - * Builds the status payload surfaced on the "MyYoast connection" settings page. + * Builds the status payload surfaced on the MyYoast connection card on the integrations page. * * Intentionally narrow: registration state, registration date, and the * stored redirect URIs with their verification state. Software statement, @@ -32,27 +33,27 @@ class Status_Presenter { private $issuer_config; /** - * The OAuth redirect URI builder. + * The redirect URI provider. * - * @var OAuth_Redirect_Uri + * @var Redirect_URI_Provider_Interface */ - private $redirect_uri; + private $redirect_uri_provider; /** * Status_Presenter constructor. * - * @param Client_Registration_Interface $client_registration The client registration port. - * @param Issuer_Config $issuer_config The issuer configuration. - * @param OAuth_Redirect_Uri $redirect_uri The OAuth redirect URI builder. + * @param Client_Registration_Interface $client_registration The client registration port. + * @param Issuer_Config $issuer_config The issuer configuration. + * @param Redirect_URI_Provider_Interface $redirect_uri_provider The redirect URI provider. */ public function __construct( Client_Registration_Interface $client_registration, Issuer_Config $issuer_config, - OAuth_Redirect_Uri $redirect_uri + Redirect_URI_Provider_Interface $redirect_uri_provider ) { - $this->client_registration = $client_registration; - $this->issuer_config = $issuer_config; - $this->redirect_uri = $redirect_uri; + $this->client_registration = $client_registration; + $this->issuer_config = $issuer_config; + $this->redirect_uri_provider = $redirect_uri_provider; } /** @@ -112,47 +113,33 @@ private function extract_registered_at( Registered_Client $client ): ?int { } /** - * Whether the currently-computed redirect URI is among the stored ones. + * Whether the registration's redirect URIs still match what this site would register today. * - * Compares the full URI — a mismatch means the site's URL has changed - * since it was connected, and the registration needs updating. + * A mismatch means the site's URL has changed since it was connected, and the + * registration needs re-syncing. * * @param Registered_Client $client The registered client. * * @return bool */ private function redirect_uris_match( Registered_Client $client ): bool { - $metadata = $client->get_metadata(); - $stored = ( $metadata['redirect_uris'] ?? [] ); - if ( ! \is_array( $stored ) ) { - return false; - } - - return \in_array( $this->redirect_uri->get(), $stored, true ); + return $client->has_redirect_uris( $this->redirect_uri_provider->get_redirect_uris() ); } /** * Returns the stored redirect URIs annotated with their origin (scheme + * host + optional port) and their verification state. * - * Verification state is hardcoded to `false` for now: issue #1207 introduces - * the verified-site concept along with the helpers that decide which URIs - * have completed an authorization code flow. Replace this once #1207 lands - * on trunk. + * A URI is verified once a user has completed the authorization-code flow for + * it on this site; that state is tracked on the registration. * * @param Registered_Client $client The registered client. * * @return array */ private function extract_redirect_uris( Registered_Client $client ): array { - $metadata = $client->get_metadata(); - $uris = ( $metadata['redirect_uris'] ?? [] ); - if ( ! \is_array( $uris ) ) { - return []; - } - $result = []; - foreach ( $uris as $uri ) { + foreach ( $client->get_redirect_uris() as $uri ) { if ( ! \is_string( $uri ) || $uri === '' ) { continue; } @@ -165,7 +152,7 @@ private function extract_redirect_uris( Registered_Client $client ): array { $result[] = [ 'uri' => $uri, 'origin' => $origin, - 'is_verified' => $this->is_uri_verified( $uri ), + 'is_verified' => $client->is_uri_validated( $uri ), ]; } @@ -192,20 +179,4 @@ private function extract_origin( string $uri ): ?string { return $origin; } - - /** - * Whether the given redirect URI has been verified by completing an - * authorization code flow. - * - * TODO: replace stub once #1207 (Yoast AI requests authenticated via - * MyYoast OAuth tokens) lands on trunk — it introduces the verified-site - * concept and the helper to check it. - * - * @param string $uri The redirect URI to check. - * - * @return bool - */ - private function is_uri_verified( string $uri ): bool { // phpcs:ignore Generic.CodeAnalysis.UnusedFunctionParameter.Found,VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable -- Stub ignores $uri until #1207 adds the verified-site helper. - return false; - } } diff --git a/tests/Unit/MyYoast_Client/User_Interface/Management_Route_Test.php b/tests/Unit/MyYoast_Client/User_Interface/Management_Route_Test.php index 24ca199d95e..888dab038aa 100644 --- a/tests/Unit/MyYoast_Client/User_Interface/Management_Route_Test.php +++ b/tests/Unit/MyYoast_Client/User_Interface/Management_Route_Test.php @@ -5,7 +5,6 @@ use Brain\Monkey; use Exception; use Mockery; -use WP_REST_Request; use WP_REST_Response; use Yoast\WP\SEO\Conditionals\MyYoast_Connection_Conditional; use Yoast\WP\SEO\MyYoast_Client\Application\Exceptions\Authorization_Flow_Exception; @@ -21,7 +20,6 @@ use Yoast\WP\SEO\MyYoast_Client\Domain\Registered_Client; use Yoast\WP\SEO\MyYoast_Client\Infrastructure\OIDC\Issuer_Config; use Yoast\WP\SEO\MyYoast_Client\User_Interface\Management_Route; -use Yoast\WP\SEO\MyYoast_Client\User_Interface\OAuth_Redirect_Uri; use Yoast\WP\SEO\MyYoast_Client\User_Interface\Status_Presenter; use Yoast\WP\SEO\Tests\Unit\TestCase; @@ -58,13 +56,6 @@ final class Management_Route_Test extends TestCase { */ private $issuer_config; - /** - * The OAuth redirect URI builder mock. - * - * @var OAuth_Redirect_Uri|Mockery\MockInterface - */ - private $redirect_uri; - /** * The client registration port mock. * @@ -90,14 +81,12 @@ protected function set_up() { $this->myyoast_client = Mockery::mock( MyYoast_Client::class ); $this->status_presenter = Mockery::mock( Status_Presenter::class ); $this->issuer_config = Mockery::mock( Issuer_Config::class ); - $this->redirect_uri = Mockery::mock( OAuth_Redirect_Uri::class ); $this->client_registration = Mockery::mock( Client_Registration_Interface::class ); $this->instance = new Management_Route( $this->myyoast_client, $this->status_presenter, $this->issuer_config, - $this->redirect_uri, $this->client_registration, ); } @@ -348,7 +337,8 @@ public function test_handle_exception_branches() { } /** - * Tests POST /myyoast/register delegates to ensure_registered with the current redirect URI. + * Tests POST /myyoast/register delegates to ensure_registered, which resolves the + * redirect URIs itself. * * @covers ::register * @@ -357,11 +347,8 @@ public function test_handle_exception_branches() { public function test_register_delegates_to_ensure_registered() { $this->provision(); - $this->redirect_uri->shouldReceive( 'get' )->andReturn( 'https://example.com/wp-admin/admin.php?page=wpseo_dashboard' ); $this->myyoast_client->shouldNotReceive( 'deregister' ); - $this->myyoast_client->shouldReceive( 'ensure_registered' ) - ->once() - ->with( [ 'https://example.com/wp-admin/admin.php?page=wpseo_dashboard' ] ); + $this->myyoast_client->shouldReceive( 'ensure_registered' )->once()->withNoArgs(); $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); Mockery::mock( 'overload:' . WP_REST_Response::class ); @@ -372,7 +359,7 @@ public function test_register_delegates_to_ensure_registered() { } /** - * Tests PUT /myyoast/registration calls update_redirect_uris with the current URI. + * Tests PUT /myyoast/registration re-syncs the registration via ensure_registered. * * @covers ::update_registration * @@ -381,10 +368,7 @@ public function test_register_delegates_to_ensure_registered() { public function test_update_registration() { $this->provision(); - $this->redirect_uri->shouldReceive( 'get' )->andReturn( 'https://example.com/wp-admin/admin.php?page=wpseo_dashboard' ); - $this->myyoast_client->shouldReceive( 'update_redirect_uris' ) - ->once() - ->with( [ 'https://example.com/wp-admin/admin.php?page=wpseo_dashboard' ] ); + $this->myyoast_client->shouldReceive( 'ensure_registered' )->once()->withNoArgs(); $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); Mockery::mock( 'overload:' . WP_REST_Response::class ); @@ -405,8 +389,7 @@ public function test_update_registration() { public function test_update_registration_handles_registration_gone() { $this->provision(); - $this->redirect_uri->shouldReceive( 'get' )->andReturn( 'https://example.com/uri' ); - $this->myyoast_client->shouldReceive( 'update_redirect_uris' ) + $this->myyoast_client->shouldReceive( 'ensure_registered' ) ->once() ->andThrow( new Registration_Not_Found_Exception( 'gone' ) ); $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); @@ -500,16 +483,13 @@ public function test_registration_actions_blocked_when_not_provisioned() { $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); $this->myyoast_client->shouldNotReceive( 'ensure_registered' ); - $this->myyoast_client->shouldNotReceive( 'update_redirect_uris' ); $this->myyoast_client->shouldNotReceive( 'get_authorization_url' ); - $request = Mockery::mock( WP_REST_Request::class ); - Mockery::mock( 'overload:' . WP_REST_Response::class ); $this->assertInstanceOf( WP_REST_Response::class, $this->instance->register() ); $this->assertInstanceOf( WP_REST_Response::class, $this->instance->update_registration() ); - $this->assertInstanceOf( WP_REST_Response::class, $this->instance->authorize( $request ) ); + $this->assertInstanceOf( WP_REST_Response::class, $this->instance->authorize() ); } /** @@ -550,17 +530,15 @@ public function test_deregister_works_when_not_provisioned() { } /** - * Tests authorize returns the authorization URL for a known redirect URI. + * Tests authorize returns the authorization URL when the site is registered. * * @covers ::authorize - * @covers ::is_known_redirect_uri * * @return void */ public function test_authorize_returns_authorization_url() { $this->provision(); - $target_uri = 'https://example.com/wp-admin/admin-post.php?action=yoast_myyoast_oauth_callback'; $return_url = 'https://example.com/wp-admin/admin.php?page=wpseo_integrations'; $authorize_url = 'https://my.yoast.com/auth?code_challenge=abc'; @@ -570,7 +548,7 @@ public function test_authorize_returns_authorization_url() { 'client-123', 'rat', 'https://my.yoast.com/clients/client-123', - [ 'redirect_uris' => [ $target_uri ] ], + [ 'redirect_uris' => [ 'https://example.com/cb' ] ], ), ); @@ -578,58 +556,22 @@ public function test_authorize_returns_authorization_url() { Monkey\Functions\expect( 'admin_url' ) ->with( 'admin.php?page=wpseo_integrations' ) - ->andReturn( 'https://example.com/wp-admin/admin.php?page=wpseo_integrations' ); + ->andReturn( $return_url ); $this->myyoast_client->shouldReceive( 'get_authorization_url' ) ->once() - ->with( 42, $target_uri, [ 'openid' ], null, $return_url ) + ->with( 42, [ 'openid' ], null, $return_url ) ->andReturn( $authorize_url ); $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); - $request = Mockery::mock( WP_REST_Request::class ); - $request->shouldReceive( 'get_param' )->with( 'redirect_uri' )->andReturn( $target_uri ); - Mockery::mock( 'overload:' . WP_REST_Response::class ); - $response = $this->instance->authorize( $request ); + $response = $this->instance->authorize(); $this->assertInstanceOf( WP_REST_Response::class, $response ); } - /** - * Tests authorize rejects an unknown redirect URI. - * - * @covers ::authorize - * @covers ::is_known_redirect_uri - * - * @return void - */ - public function test_authorize_rejects_unknown_redirect_uri() { - $this->provision(); - - $this->client_registration->shouldReceive( 'get_registered_client' ) - ->andReturn( - new Registered_Client( - 'client-123', - 'rat', - 'https://my.yoast.com/clients/client-123', - [ 'redirect_uris' => [ 'https://example.com/known' ] ], - ), - ); - - $this->myyoast_client->shouldNotReceive( 'get_authorization_url' ); - - $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); - - $request = Mockery::mock( WP_REST_Request::class ); - $request->shouldReceive( 'get_param' )->with( 'redirect_uri' )->andReturn( 'https://attacker.example/evil' ); - - Mockery::mock( 'overload:' . WP_REST_Response::class ); - - $this->assertInstanceOf( WP_REST_Response::class, $this->instance->authorize( $request ) ); - } - /** * Tests authorize surfaces an error when the site is not registered. * @@ -644,11 +586,9 @@ public function test_authorize_when_not_registered() { $this->myyoast_client->shouldNotReceive( 'get_authorization_url' ); $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); - $request = Mockery::mock( WP_REST_Request::class ); - Mockery::mock( 'overload:' . WP_REST_Response::class ); - $this->assertInstanceOf( WP_REST_Response::class, $this->instance->authorize( $request ) ); + $this->assertInstanceOf( WP_REST_Response::class, $this->instance->authorize() ); } /** @@ -661,15 +601,13 @@ public function test_authorize_when_not_registered() { public function test_authorize_handles_flow_exception() { $this->provision(); - $target_uri = 'https://example.com/known'; - $this->client_registration->shouldReceive( 'get_registered_client' ) ->andReturn( new Registered_Client( 'client-123', 'rat', 'https://my.yoast.com/clients/client-123', - [ 'redirect_uris' => [ $target_uri ] ], + [ 'redirect_uris' => [ 'https://example.com/cb' ] ], ), ); @@ -684,11 +622,8 @@ public function test_authorize_handles_flow_exception() { $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); - $request = Mockery::mock( WP_REST_Request::class ); - $request->shouldReceive( 'get_param' )->with( 'redirect_uri' )->andReturn( $target_uri ); - Mockery::mock( 'overload:' . WP_REST_Response::class ); - $this->assertInstanceOf( WP_REST_Response::class, $this->instance->authorize( $request ) ); + $this->assertInstanceOf( WP_REST_Response::class, $this->instance->authorize() ); } } diff --git a/tests/Unit/MyYoast_Client/User_Interface/Status_Presenter_Test.php b/tests/Unit/MyYoast_Client/User_Interface/Status_Presenter_Test.php index f630d7922a5..c58e7fb6025 100644 --- a/tests/Unit/MyYoast_Client/User_Interface/Status_Presenter_Test.php +++ b/tests/Unit/MyYoast_Client/User_Interface/Status_Presenter_Test.php @@ -5,9 +5,9 @@ use Brain\Monkey; use Mockery; use Yoast\WP\SEO\MyYoast_Client\Application\Ports\Client_Registration_Interface; +use Yoast\WP\SEO\MyYoast_Client\Application\Ports\Redirect_URI_Provider_Interface; use Yoast\WP\SEO\MyYoast_Client\Domain\Registered_Client; use Yoast\WP\SEO\MyYoast_Client\Infrastructure\OIDC\Issuer_Config; -use Yoast\WP\SEO\MyYoast_Client\User_Interface\OAuth_Redirect_Uri; use Yoast\WP\SEO\MyYoast_Client\User_Interface\Status_Presenter; use Yoast\WP\SEO\Tests\Unit\TestCase; @@ -35,11 +35,11 @@ final class Status_Presenter_Test extends TestCase { private $issuer_config; /** - * The OAuth redirect URI builder mock. + * The redirect URI provider mock. * - * @var OAuth_Redirect_Uri|Mockery\MockInterface + * @var Redirect_URI_Provider_Interface|Mockery\MockInterface */ - private $redirect_uri; + private $redirect_uri_provider; /** * The instance under test. @@ -56,14 +56,14 @@ final class Status_Presenter_Test extends TestCase { protected function set_up() { parent::set_up(); - $this->client_registration = Mockery::mock( Client_Registration_Interface::class ); - $this->issuer_config = Mockery::mock( Issuer_Config::class ); - $this->redirect_uri = Mockery::mock( OAuth_Redirect_Uri::class ); + $this->client_registration = Mockery::mock( Client_Registration_Interface::class ); + $this->issuer_config = Mockery::mock( Issuer_Config::class ); + $this->redirect_uri_provider = Mockery::mock( Redirect_URI_Provider_Interface::class ); $this->instance = new Status_Presenter( $this->client_registration, $this->issuer_config, - $this->redirect_uri, + $this->redirect_uri_provider, ); Monkey\Functions\stubs( @@ -127,7 +127,6 @@ public function test_present_when_provisioned_but_not_registered() { * @covers ::extract_registered_at * @covers ::extract_redirect_uris * @covers ::extract_origin - * @covers ::is_uri_verified * @covers ::redirect_uris_match * * @return void @@ -135,9 +134,13 @@ public function test_present_when_provisioned_but_not_registered() { public function test_present_when_registered_with_matching_uri() { $this->issuer_config->shouldReceive( 'get_software_statement' )->andReturn( 'jwt' ); $this->issuer_config->shouldReceive( 'get_initial_access_token' )->andReturn( 'iat' ); - $this->redirect_uri->shouldReceive( 'get' )->andReturn( self::CURRENT_REDIRECT_URI ); - $staging_uri = 'https://staging.example.com/wp-admin/admin.php?page=wpseo_dashboard'; + $staging_uri = 'https://staging.example.com/wp-admin/admin.php?page=wpseo_dashboard'; + $this->redirect_uri_provider + ->shouldReceive( 'get_redirect_uris' ) + ->andReturn( [ self::CURRENT_REDIRECT_URI, $staging_uri ] ); + + // The current URI has completed an authorization-code flow; the staging one has not. $registered_client = new Registered_Client( 'client-123', 'rat', @@ -149,6 +152,7 @@ public function test_present_when_registered_with_matching_uri() { $staging_uri, ], ], + [ self::CURRENT_REDIRECT_URI ], ); $this->client_registration->shouldReceive( 'get_registered_client' )->andReturn( $registered_client ); @@ -164,7 +168,7 @@ public function test_present_when_registered_with_matching_uri() { [ 'uri' => self::CURRENT_REDIRECT_URI, 'origin' => 'https://example.com', - 'is_verified' => false, + 'is_verified' => true, ], [ 'uri' => $staging_uri, @@ -189,7 +193,9 @@ public function test_present_when_registered_with_matching_uri() { public function test_present_when_registered_with_drifted_uri() { $this->issuer_config->shouldReceive( 'get_software_statement' )->andReturn( 'jwt' ); $this->issuer_config->shouldReceive( 'get_initial_access_token' )->andReturn( 'iat' ); - $this->redirect_uri->shouldReceive( 'get' )->andReturn( 'https://new-domain.example.com/wp-admin/admin.php?page=wpseo_dashboard&yoast_myyoast_oauth_callback=1' ); + $this->redirect_uri_provider + ->shouldReceive( 'get_redirect_uris' ) + ->andReturn( [ 'https://new-domain.example.com/wp-admin/admin.php?page=wpseo_dashboard&yoast_myyoast_oauth_callback=1' ] ); $registered_client = new Registered_Client( 'client-123', @@ -218,7 +224,9 @@ public function test_present_when_registered_with_drifted_uri() { public function test_present_when_registered_without_issued_at() { $this->issuer_config->shouldReceive( 'get_software_statement' )->andReturn( 'jwt' ); $this->issuer_config->shouldReceive( 'get_initial_access_token' )->andReturn( 'iat' ); - $this->redirect_uri->shouldReceive( 'get' )->andReturn( self::CURRENT_REDIRECT_URI ); + $this->redirect_uri_provider + ->shouldReceive( 'get_redirect_uris' ) + ->andReturn( [ self::CURRENT_REDIRECT_URI ] ); $registered_client = new Registered_Client( 'client-123', From d2b9e235598404e3e87c52d3b32287277edd8519 Mon Sep 17 00:00:00 2001 From: Diede Exterkate <5352634+diedexx@users.noreply.github.com> Date: Wed, 17 Jun 2026 15:58:57 +0200 Subject: [PATCH 04/26] feat(integrations-page): add the MyYoast connection card MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the MyYoast connection card to the Recommended integrations section: a standalone wp.data store seeded from the localized status, the card UI (connect, per-state notices for connection-lost and verification-needed, disconnect via a confirm modal), and the entry/recommended-integrations wiring. Connecting continues straight into the authorization-code flow. The verify/authorize action no longer sends a redirect URI — the client resolves the registered one — and heroicons are imported per-icon to satisfy the current lint rule. Co-Authored-By: Claude Opus 4.8 --- packages/js/images/myyoast-logo.svg | 1 + packages/js/src/integrations-page.js | 3 + .../myyoast-disconnect-modal.js | 60 +++ .../integrations-page/myyoast-integration.js | 423 ++++++++++++++++++ .../js/src/integrations-page/myyoast-store.js | 253 +++++++++++ .../recommended-integrations.js | 7 + 6 files changed, 747 insertions(+) create mode 100644 packages/js/images/myyoast-logo.svg create mode 100644 packages/js/src/integrations-page/myyoast-disconnect-modal.js create mode 100644 packages/js/src/integrations-page/myyoast-integration.js create mode 100644 packages/js/src/integrations-page/myyoast-store.js diff --git a/packages/js/images/myyoast-logo.svg b/packages/js/images/myyoast-logo.svg new file mode 100644 index 00000000000..f51bde0393d --- /dev/null +++ b/packages/js/images/myyoast-logo.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/packages/js/src/integrations-page.js b/packages/js/src/integrations-page.js index e5e41620eed..fb73f958a8d 100644 --- a/packages/js/src/integrations-page.js +++ b/packages/js/src/integrations-page.js @@ -3,12 +3,15 @@ import domReady from "@wordpress/dom-ready"; import { Root } from "@yoast/ui-library"; import IntegrationsGrid from "./integrations-page/integrations-grid"; +import { registerMyyoastStore } from "./integrations-page/myyoast-store"; import { registerReactComponent, renderReactRoot } from "./helpers/reactRoot"; window.YoastSEO = window.YoastSEO || {}; window.YoastSEO._registerReactComponent = registerReactComponent; domReady( () => { + registerMyyoastStore(); + const context = { isRtl: Boolean( get( window, "wpseoScriptData.metabox.isRtl", false ) ), }; diff --git a/packages/js/src/integrations-page/myyoast-disconnect-modal.js b/packages/js/src/integrations-page/myyoast-disconnect-modal.js new file mode 100644 index 00000000000..e5dc47884b1 --- /dev/null +++ b/packages/js/src/integrations-page/myyoast-disconnect-modal.js @@ -0,0 +1,60 @@ +import ExclamationIcon from "@heroicons/react/outline/ExclamationIcon"; +import { __ } from "@wordpress/i18n"; +import { Button, Modal, useSvgAria } from "@yoast/ui-library"; +import { noop } from "lodash"; +import PropTypes from "prop-types"; + +/** + * Confirm modal for disconnecting the site from MyYoast. + * + * @param {boolean} isOpen Whether the modal is open. + * @param {function} onClose Cancel handler. + * @param {function} onConfirm Confirm handler. + * @returns {JSX.Element} The modal element. + */ +export const MyyoastConnectionDisconnectModal = ( { + isOpen, + onClose = noop, + onConfirm = noop, +} ) => { + const svgAriaProps = useSvgAria(); + + return ( + + +
+
+ +
+
+ + { __( "Disconnect this site from MyYoast?", "wordpress-seo" ) } + + + { __( "All connected users will be signed out and the site stops working with MyYoast until you connect it again.", "wordpress-seo" ) } + +
+
+
+ + +
+ + + ); +}; + +MyyoastConnectionDisconnectModal.propTypes = { + isOpen: PropTypes.bool.isRequired, + onClose: PropTypes.func, + onConfirm: PropTypes.func, +}; diff --git a/packages/js/src/integrations-page/myyoast-integration.js b/packages/js/src/integrations-page/myyoast-integration.js new file mode 100644 index 00000000000..8a7959c399b --- /dev/null +++ b/packages/js/src/integrations-page/myyoast-integration.js @@ -0,0 +1,423 @@ +/* eslint-disable camelcase */ +import ArrowNarrowRightIcon from "@heroicons/react/outline/ArrowNarrowRightIcon"; +import CheckIcon from "@heroicons/react/solid/CheckIcon"; +import ExclamationCircleIcon from "@heroicons/react/solid/ExclamationCircleIcon"; +import ExclamationIcon from "@heroicons/react/solid/ExclamationIcon"; +import { dispatch, select, useSelect } from "@wordpress/data"; +import { useCallback, useEffect, useId, useState } from "@wordpress/element"; +import { __, _n, sprintf } from "@wordpress/i18n"; +import { Alert, Button, TooltipContainer, TooltipTrigger, TooltipWithContext, useSvgAria, useToggleState } from "@yoast/ui-library"; +import PropTypes from "prop-types"; +import { ReactComponent as MyYoastLogo } from "../../images/myyoast-logo.svg"; +import { MyyoastConnectionDisconnectModal } from "./myyoast-disconnect-modal"; +import { MYYOAST_STORE_NAME } from "./myyoast-store"; +import { Card } from "./tailwind-components/card"; + +// Placeholder until the final MyYoast integrations article URL is provided. +const LEARN_MORE_LINK = "https://yoa.st/myyoast-connection"; + +/** + * Returns the error and success message map keyed by the machine codes the + * backend emits (`error_code` for errors, `message_key` for successes). + * + * Built lazily inside a function so `__()` is called at call time rather than + * at module load — keeps locale switching working. + * + * @returns {Object} The message map. + */ +const buildMessages = () => ( { + not_provisioned: __( "Your server doesn't support the MyYoast connection. Update Yoast SEO to the latest version. If the issue persists after updating, contact support.", "wordpress-seo" ), + registration_gone: __( "MyYoast no longer recognizes this site. Connect this site to MyYoast again to restore the connection.", "wordpress-seo" ), + rate_limited: __( "MyYoast has had a lot of connection attempts from this site or network. Please wait a few minutes and try again.", "wordpress-seo" ), + server_capability: __( "MyYoast doesn't support a feature this version of Yoast SEO needs. Update Yoast SEO to the latest version. If the issue persists, contact support.", "wordpress-seo" ), + myyoast_unreachable: __( "Couldn't reach MyYoast from this server. Check your server's outbound network access, then try again. If MyYoast is having issues, wait a few minutes and retry.", "wordpress-seo" ), + token_request_failed_invalid_grant: __( "MyYoast rejected the credentials stored for this site. Disconnect and connect this site again to restore the connection.", "wordpress-seo" ), + token_request_failed: __( "Something went wrong while talking to MyYoast. Try again in a moment. If the problem keeps happening, update Yoast SEO or contact support.", "wordpress-seo" ), + token_storage_failed: __( "Couldn't save the new credentials on this site. Make sure your WordPress database is writable, then try again.", "wordpress-seo" ), + invalid_resource: __( "Something went wrong. Refresh the page and try again. If the problem keeps happening, contact support.", "wordpress-seo" ), + registration_failed: __( "Couldn't connect this site to MyYoast. Try again in a moment. If the problem keeps happening, update Yoast SEO or contact support.", "wordpress-seo" ), + unknown_redirect_uri: __( "Couldn't verify this site because it's no longer recognized. Refresh the page and try again.", "wordpress-seo" ), + invalid_user: __( "You need to be signed in to verify this site.", "wordpress-seo" ), + connection_cancelled: __( "Connection cancelled. You can try again whenever you're ready.", "wordpress-seo" ), + timeout: __( "Request to MyYoast timed out. Please try again.", "wordpress-seo" ), + unexpected_error: __( "Something went wrong. Try again in a moment. If the problem keeps happening, update Yoast SEO or contact support.", "wordpress-seo" ), + connect_success: __( "This site is now connected to MyYoast.", "wordpress-seo" ), + update_success: __( "Connection updated to match this site's current URL.", "wordpress-seo" ), + disconnect_success: __( "This site is no longer connected to MyYoast.", "wordpress-seo" ), + verify_success: __( "This site is now verified.", "wordpress-seo" ), +} ); + +/** + * Formats the rate-limit message in minutes or hours, with the correct + * singular/plural form. Sub-minute values round up to one minute. + * + * @param {number} seconds The retry-after value in seconds. + * @returns {string} The localised message. + */ +const formatRateLimitedMessage = ( seconds ) => { + const minutes = Math.ceil( seconds / 60 ); + if ( minutes >= 60 ) { + const hours = Math.ceil( seconds / 3600 ); + /* translators: %d is a number of hours. */ + return sprintf( _n( "MyYoast has had a lot of connection attempts from this site or network. Please wait about %d hour and try again.", "MyYoast has had a lot of connection attempts from this site or network. Please wait about %d hours and try again.", hours, "wordpress-seo" ), hours ); + } + /* translators: %d is a number of minutes. */ + return sprintf( _n( "MyYoast has had a lot of connection attempts from this site or network. Please wait about %d minute and try again.", "MyYoast has had a lot of connection attempts from this site or network. Please wait about %d minutes and try again.", minutes, "wordpress-seo" ), minutes ); +}; + +const ACTION_DISPATCHERS = { + verify: "verifyMyyoastConnection", + connect: "connectMyyoastConnection", + update: "updateMyyoastConnection", + disconnect: "disconnectMyyoastConnection", +}; + +/** + * Resolves the user-facing message for a given error code. + * + * @param {string} code The backend error code. + * @param {Object} [details] Extra detail from the backend payload. + * @returns {string} The translated message. + */ +const resolveErrorMessage = ( code, details ) => { + const messages = buildMessages(); + if ( code === "rate_limited" ) { + const seconds = Number( details?.retry_after_seconds ); + if ( Number.isFinite( seconds ) && seconds > 0 ) { + return formatRateLimitedMessage( seconds ); + } + } + return messages[ code ] ?? messages.unexpected_error; +}; + +/** + * Runs a MyYoast management action: dispatches the slice action and, unless + * silent, surfaces the outcome as inline card feedback. + * + * @param {string} actionName The action (verify/connect/update/disconnect). + * @param {Object} [body] The request body. + * @param {Object} [options] Options. + * @param {boolean} [options.silent] When true, suppress feedback. + * @param {function} [options.onFeedback] Receives `{ variant, message }` to show in the card. + * @returns {Promise} The slice action's result. + */ +// eslint-disable-next-line complexity +const runAction = async( actionName, body, options ) => { + // Serialize actions: they all mutate the same server-side registration, so a + // second action started while one is in flight (e.g. the mount-time verify + // overlapping a user click) would race on the shared status. Ignore it. + if ( select( MYYOAST_STORE_NAME ).selectMyyoastConnectionActionInFlight() ) { + return { ok: false, errorCode: "action_in_flight" }; + } + const store = dispatch( MYYOAST_STORE_NAME ); + const result = await store[ ACTION_DISPATCHERS[ actionName ] ]( body ); + + if ( options?.silent ) { + return result; + } + + if ( result.ok && result.messageKey ) { + const message = buildMessages()[ result.messageKey ]; + if ( message ) { + options?.onFeedback?.( { variant: "success", message } ); + } + } else if ( ! result.ok ) { + const message = resolveErrorMessage( result.errorCode, result.details ); + store.setMyyoastActionError( { actionName, errorCode: result.errorCode, message } ); + options?.onFeedback?.( { variant: "error", message } ); + } + + return result; +}; + +/** + * Starts the verify-site flow: asks the backend for an authorization URL and + * navigates the browser there. The backend resolves which registered redirect + * URI to use. Errors surface as inline card feedback. + * + * @param {function} onFeedback Receives `{ variant, message }` to show in the card. + * @returns {Promise} Resolves once the navigation has been kicked off. + */ +const runAuthorize = async( onFeedback ) => { + if ( select( MYYOAST_STORE_NAME ).selectMyyoastConnectionActionInFlight() ) { + return; + } + const store = dispatch( MYYOAST_STORE_NAME ); + const result = await store.authorizeMyyoastSite(); + + if ( result.ok && result.authorizeUrl ) { + window.location.assign( result.authorizeUrl ); + return; + } + + const message = resolveErrorMessage( result.errorCode, result.details ); + onFeedback?.( { variant: "error", message } ); +}; + +/** + * The footer status line of the card. Mirrors the three registered states from + * the design: connected (green check), connection lost (red error icon, URL no + * longer matches), and verification needed (amber warning icon, a connected + * site still needs an authorization-code flow). Not rendered when the site is + * not registered — the footer shows the connect button in that case instead. + * + * @param {Object} props The component props. + * @param {boolean} props.connectionLost Whether the registered URL no longer matches. + * @param {boolean} props.verificationNeeded Whether a connected site still needs verification. + * @returns {JSX.Element} The status line. + */ +const StatusFooter = ( { connectionLost, verificationNeeded } ) => { + const svgAriaProps = useSvgAria(); + const tooltipId = `myyoast-verification-${ useId() }`; + const iconClass = "yst-h-5 yst-w-5 yst-flex-shrink-0"; + + if ( connectionLost ) { + return ( +

+ { __( "Site connection lost", "wordpress-seo" ) } + +

+ ); + } + + if ( verificationNeeded ) { + return ( +

+ { __( "Site connected", "wordpress-seo" ) } + + + + + + { __( "Sign in to MyYoast to finish setting up this connection so everything works as expected.", "wordpress-seo" ) } + + +

+ ); + } + + return ( +

+ { __( "Site connected", "wordpress-seo" ) } + +

+ ); +}; + +StatusFooter.propTypes = { + connectionLost: PropTypes.bool.isRequired, + verificationNeeded: PropTypes.bool.isRequired, +}; + +/** + * The MyYoast connection card on the integrations page. + * + * @returns {JSX.Element} The card element. + */ +// eslint-disable-next-line complexity +export const MyyoastIntegration = () => { + const status = useSelect( s => s( MYYOAST_STORE_NAME ).selectMyyoastConnectionStatus(), [] ); + const actionInFlight = useSelect( s => s( MYYOAST_STORE_NAME ).selectMyyoastConnectionActionInFlight(), [] ); + const pendingCallbackOutcome = useSelect( s => s( MYYOAST_STORE_NAME ).selectMyyoastConnectionPendingCallbackOutcome(), [] ); + const [ feedback, setFeedback ] = useState( null ); + const [ isDisconnectOpen, , , openDisconnect, closeDisconnect ] = useToggleState( false ); + + // Auto-fired verify on mount: confirms with MyYoast that the stored + // registration is still valid. Errors are silent; the response refreshes + // the local status, so Registration_Not_Found clears state and the card + // re-renders as "not connected". + useEffect( () => { + if ( status.isRegistered ) { + runAction( "verify", null, { silent: true } ); + } + }, [] ); + + // Surfaces the one-shot feedback stashed by the OAuth callback handler + // (success or error). The transient is consumed server-side on read, so + // after we've shown it we clear it from the slice to avoid re-firing on + // subsequent renders. + useEffect( () => { + if ( ! pendingCallbackOutcome ) { + return; + } + const { kind, key } = pendingCallbackOutcome; + const message = buildMessages()[ key ]; + if ( message ) { + setFeedback( { variant: kind === "success" ? "success" : "error", message } ); + } + dispatch( MYYOAST_STORE_NAME ).clearMyyoastCallbackOutcome(); + }, [ pendingCallbackOutcome ] ); + + // Connecting only registers the site as an OAuth client; the connection is + // not usable until one user completes an authorization-code grant. So on a + // successful registration we continue straight into that flow, sending the + // user to MyYoast to sign in rather than leaving them on the unverified + // "Verification needed" state. A failed registration just shows its error. + const handleConnect = useCallback( async() => { + // Register silently: on success we redirect to MyYoast immediately, so a + // transient "connected" message would only flash before the page leaves. + // A failure still needs to be shown, so surface only that. + const result = await runAction( "connect", null, { silent: true } ); + if ( ! result.ok ) { + const message = resolveErrorMessage( result.errorCode, result.details ); + setFeedback( { variant: "error", message } ); + return; + } + // Registration succeeded; continue straight into the authorization-code + // flow. The backend resolves which registered redirect URI to use. + await runAuthorize( setFeedback ); + }, [] ); + const handleReconnect = useCallback( () => runAction( "update", null, { onFeedback: setFeedback } ), [] ); + const handleDisconnectConfirm = useCallback( () => { + closeDisconnect(); + runAction( "disconnect", null, { onFeedback: setFeedback } ); + }, [] ); + + const redirectUris = Array.isArray( status.redirectUris ) ? status.redirectUris : []; + // "Connection lost" takes precedence over "verification needed": once the + // registered URL no longer matches, reconnecting is the only fix and any + // unverified-site notice would be premature. + const connectionLost = status.isRegistered && status.redirectUrisMatch === false; + // Show a single verification notice for the first unverified site, even when + // several are connected — verifying them all clears it. Suppressed while the + // connection is lost. + const firstUnverified = connectionLost ? null : redirectUris.find( ( entry ) => ! entry.isVerified ) ?? null; + const verificationNeeded = Boolean( firstUnverified ); + + const handleVerify = useCallback( () => { + if ( firstUnverified ) { + runAuthorize( setFeedback ); + } + }, [ firstUnverified ] ); + + return ( + <> + + + + + +
+

+ { "MyYoast" } +

+

+ { __( "Connect your site to MyYoast to enable AI features, manage your Yoast products, and simplify setup.", "wordpress-seo" ) } +

+ + + { __( "Learn more", "wordpress-seo" ) } + + + + { feedback && ( + { feedback.message } + ) } + + { ! status.isProvisioned && ( + + { __( "MyYoast connection is not configured on this build of Yoast SEO. Site features that depend on it are unavailable.", "wordpress-seo" ) } + + ) } + + { status.isProvisioned && status.isRegistered && ( +
+
+
+ { __( "Site connection", "wordpress-seo" ) } +
+ { redirectUris.length > 0 && ( +
    + { redirectUris.map( ( entry ) => ( +
  • { entry.origin }
    • + ) ) } +
+ ) } +
+ + + { connectionLost && ( + +
+

{ __( "Connection lost", "wordpress-seo" ) }

+

{ __( "Domain connection lost, try to reconnect.", "wordpress-seo" ) }

+ +
+ + ) } + + { verificationNeeded && ( + +
+

{ __( "Verification needed", "wordpress-seo" ) }

+

{ __( "Sign in to MyYoast to finish setting up this connection.", "wordpress-seo" ) }

+ +
+ + ) } +
+ ) } +
+ + { status.isProvisioned && ( + + { status.isRegistered ? ( + + ) : ( + + ) } + + ) } + + + + + ); +}; diff --git a/packages/js/src/integrations-page/myyoast-store.js b/packages/js/src/integrations-page/myyoast-store.js new file mode 100644 index 00000000000..0f1d10e24c7 --- /dev/null +++ b/packages/js/src/integrations-page/myyoast-store.js @@ -0,0 +1,253 @@ +import { createSlice } from "@reduxjs/toolkit"; +import apiFetch from "@wordpress/api-fetch"; +import { combineReducers, registerStore } from "@wordpress/data"; +import { get } from "lodash"; + +export const MYYOAST_CONNECTION_NAME = "myyoastConnection"; +export const MYYOAST_STORE_NAME = "yoast-seo/myyoast-connection"; + +const REQUEST_TIMEOUT_MS = 30000; + +const ENDPOINTS = { + verify: { actionType: "verifyMyyoastConnection", method: "POST", path: "/yoast/v1/myyoast/verify" }, + connect: { actionType: "connectMyyoastConnection", method: "POST", path: "/yoast/v1/myyoast/register" }, + update: { actionType: "updateMyyoastConnection", method: "PUT", path: "/yoast/v1/myyoast/registration" }, + disconnect: { actionType: "disconnectMyyoastConnection", method: "DELETE", path: "/yoast/v1/myyoast/registration" }, + authorize: { actionType: "authorizeMyyoastSite", method: "POST", path: "/yoast/v1/myyoast/authorize" }, +}; + +/** + * @returns {Object} The initial myyoastConnection state. + */ +export const createInitialMyyoastConnectionState = () => ( { + status: null, + profileUrl: "", + actionInFlight: null, + actionError: null, + pendingCallbackOutcome: null, +} ); + +const DEFAULT_STATUS = { + isProvisioned: false, + isRegistered: false, + registeredAt: null, + registeredAtIso: null, + redirectUris: [], + redirectUrisMatch: true, +}; + +/** + * Transforms a snake_case status payload from the backend into the camelCase + * shape used inside the React app. + * + * @param {Object} payload The backend payload. + * @returns {Object} The camelCase status. + */ +export const transformStatus = ( payload ) => { + if ( ! payload ) { + return DEFAULT_STATUS; + } + const redirectUris = Array.isArray( payload.redirect_uris ) + ? payload.redirect_uris.map( ( entry ) => ( { + uri: entry?.uri ?? "", + origin: entry?.origin ?? "", + isVerified: Boolean( entry?.is_verified ), + } ) ) + : []; + return { + isProvisioned: Boolean( payload.is_provisioned ), + isRegistered: Boolean( payload.is_registered ), + registeredAt: payload.registered_at ?? null, + registeredAtIso: payload.registered_at_iso ?? null, + redirectUris, + redirectUrisMatch: payload.redirect_uris_match !== false, + }; +}; + +const slice = createSlice( { + name: MYYOAST_CONNECTION_NAME, + initialState: createInitialMyyoastConnectionState(), + reducers: { + setMyyoastStatus: ( state, { payload } ) => { + if ( payload ) { + state.status = payload; + } + }, + startMyyoastAction: ( state, { payload } ) => { + // Guard against starting a second action while one is already running: + // every action mutates the same registration, so they must serialize. + if ( state.actionInFlight ) { + return; + } + state.actionInFlight = payload; + state.actionError = null; + }, + setMyyoastActionError: ( state, { payload } ) => { + state.actionError = payload; + }, + finishMyyoastAction: state => { + state.actionInFlight = null; + }, + clearMyyoastCallbackOutcome: state => { + state.pendingCallbackOutcome = null; + }, + }, +} ); + +const { setMyyoastStatus, startMyyoastAction, setMyyoastActionError, finishMyyoastAction } = slice.actions; + +/** + * Builds a generator action that performs a MyYoast management request through + * the matching control, mirrors the response into the slice, and returns a + * result object the caller can use to drive UI notifications. + * + * @param {string} name The action name (verify/connect/update/disconnect). + * @returns {GeneratorFunction} The generator action. + */ +// eslint-disable-next-line complexity +const createMyyoastAction = ( name ) => function* ( body ) { + yield startMyyoastAction( name ); + try { + const payload = yield{ type: ENDPOINTS[ name ].actionType, payload: body }; + if ( payload?.status ) { + yield setMyyoastStatus( transformStatus( payload.status ) ); + } + // The backend signals failure with `error_code` in the body — HTTP status + // stays 200 for upstream/precondition failures. + if ( payload?.error_code ) { + yield setMyyoastActionError( { actionName: name, errorCode: payload.error_code, message: "" } ); + return { ok: false, errorCode: payload.error_code, details: payload.details }; + } + return { ok: true, messageKey: payload?.message_key }; + } catch ( error ) { + const errorCode = error?.name === "AbortError" ? "timeout" : "unexpected_error"; + yield setMyyoastActionError( { actionName: name, errorCode, message: "" } ); + return { ok: false, errorCode }; + } finally { + yield finishMyyoastAction(); + } +}; + +const verifyMyyoastConnection = createMyyoastAction( "verify" ); +const connectMyyoastConnection = createMyyoastAction( "connect" ); +const updateMyyoastConnection = createMyyoastAction( "update" ); +const disconnectMyyoastConnection = createMyyoastAction( "disconnect" ); + +/** + * Starts the authorization-code flow for the site's registration. + * + * The backend resolves which registered redirect URI to use, so no URI is + * sent. On success it returns an `authorize_url` the browser should be + * navigated to; the caller decides how to do that. On failure the slice's + * actionError is set, mirroring the other actions. + * + * @returns {GeneratorFunction} The generator action. + */ +// eslint-disable-next-line complexity +const authorizeMyyoastSite = function* () { + yield startMyyoastAction( "authorize" ); + try { + const payload = yield{ type: ENDPOINTS.authorize.actionType, payload: {} }; + if ( payload?.status ) { + yield setMyyoastStatus( transformStatus( payload.status ) ); + } + if ( payload?.error_code ) { + yield setMyyoastActionError( { actionName: "authorize", errorCode: payload.error_code, message: "" } ); + return { ok: false, errorCode: payload.error_code, details: payload.details }; + } + if ( ! payload?.authorize_url ) { + yield setMyyoastActionError( { actionName: "authorize", errorCode: "unexpected_error", message: "" } ); + return { ok: false, errorCode: "unexpected_error" }; + } + return { ok: true, authorizeUrl: payload.authorize_url }; + } catch ( error ) { + const errorCode = error?.name === "AbortError" ? "timeout" : "unexpected_error"; + yield setMyyoastActionError( { actionName: "authorize", errorCode, message: "" } ); + return { ok: false, errorCode }; + } finally { + yield finishMyyoastAction(); + } +}; + +/** + * Calls a MyYoast endpoint via apiFetch with a client-side timeout. + * + * @param {Object} endpoint The endpoint config. + * @param {Object} body The request body. + * @returns {Promise} The parsed response payload. + */ +const callEndpoint = async( endpoint, body ) => { + const controller = new AbortController(); + const timeoutId = setTimeout( () => controller.abort(), REQUEST_TIMEOUT_MS ); + try { + return await apiFetch( { + method: endpoint.method, + path: endpoint.path, + data: body, + signal: controller.signal, + } ); + } finally { + clearTimeout( timeoutId ); + } +}; + +export const myyoastConnectionActions = { + ...slice.actions, + verifyMyyoastConnection, + connectMyyoastConnection, + updateMyyoastConnection, + disconnectMyyoastConnection, + authorizeMyyoastSite, +}; + +export const myyoastConnectionControls = { + [ ENDPOINTS.verify.actionType ]: ( { payload } ) => callEndpoint( ENDPOINTS.verify, payload ), + [ ENDPOINTS.connect.actionType ]: ( { payload } ) => callEndpoint( ENDPOINTS.connect, payload ), + [ ENDPOINTS.update.actionType ]: ( { payload } ) => callEndpoint( ENDPOINTS.update, payload ), + [ ENDPOINTS.disconnect.actionType ]: ( { payload } ) => callEndpoint( ENDPOINTS.disconnect, payload ), + [ ENDPOINTS.authorize.actionType ]: ( { payload } ) => callEndpoint( ENDPOINTS.authorize, payload ), +}; + +export const myyoastConnectionSelectors = { + selectMyyoastConnectionStatus: state => get( state, "myyoastConnection.status", DEFAULT_STATUS ) ?? DEFAULT_STATUS, + selectMyyoastConnectionProfileUrl: state => get( state, "myyoastConnection.profileUrl", "" ), + selectMyyoastConnectionActionInFlight: state => get( state, "myyoastConnection.actionInFlight", null ), + selectMyyoastConnectionActionError: state => get( state, "myyoastConnection.actionError", null ), + selectMyyoastConnectionPendingCallbackOutcome: state => get( state, "myyoastConnection.pendingCallbackOutcome", null ), + selectHasMyyoastConnection: state => get( state, "myyoastConnection.status", null ) !== null, +}; + +/** + * Registers the standalone MyYoast connection store, seeded from the + * `wpseoIntegrationsData.myyoast_connection` payload the integrations page + * localizes. No-ops when the payload is absent (feature flag disabled) — + * the card is not rendered in that case either. + * + * The slice state is nested under the `myyoastConnection` key so the + * selectors keep the same shape they had inside the settings store. + * + * @returns {void} + */ +export const registerMyyoastStore = () => { + const data = get( window, "wpseoIntegrationsData.myyoast_connection", null ); + if ( ! data ) { + return; + } + + registerStore( MYYOAST_STORE_NAME, { + reducer: combineReducers( { [ MYYOAST_CONNECTION_NAME ]: slice.reducer } ), + actions: myyoastConnectionActions, + selectors: myyoastConnectionSelectors, + controls: myyoastConnectionControls, + initialState: { + [ MYYOAST_CONNECTION_NAME ]: { + ...createInitialMyyoastConnectionState(), + status: transformStatus( data.initialStatus ), + profileUrl: data.profileUrl || "", + pendingCallbackOutcome: data.callbackOutcome || null, + }, + }, + } ); +}; + +export default slice.reducer; diff --git a/packages/js/src/integrations-page/recommended-integrations.js b/packages/js/src/integrations-page/recommended-integrations.js index 00da7184317..ad459b2dc0b 100644 --- a/packages/js/src/integrations-page/recommended-integrations.js +++ b/packages/js/src/integrations-page/recommended-integrations.js @@ -4,6 +4,7 @@ import { safeCreateInterpolateElement } from "../helpers/i18n"; import { ReactComponent as SemrushLogo } from "../../images/semrush-logo.svg"; import { ReactComponent as WincherLogo } from "../../images/wincher-logo.svg"; import { getInitialState, getIsMultisiteAvailable, getIsNetworkControlEnabled, updateIntegrationState } from "./helper"; +import { MyyoastIntegration } from "./myyoast-integration"; import { SiteKitIntegration } from "./site-kit-integration"; import { ToggleableIntegration } from "./toggleable-integration"; @@ -78,6 +79,12 @@ const RecommendedIntegrations = [ } ), ]; +// The payload is only localized when the MyYoast connection feature flag is on. +const isMyyoastConnectionAvailable = get( window, "wpseoIntegrationsData.myyoast_connection", null ) !== null; +if ( isMyyoastConnectionAvailable ) { + RecommendedIntegrations.unshift( ); +} + const isSiteKitFeatureEnabled = get( window, "wpseoIntegrationsData.site_kit_configuration.isFeatureEnabled", false ); if ( isSiteKitFeatureEnabled ) { RecommendedIntegrations.push( Date: Fri, 19 Jun 2026 13:51:12 +0200 Subject: [PATCH 05/26] refactor(myyoast-client): rename registration verify to refresh-status The remote registration liveness check was named "verify", which collided with the cryptographic Jwt_Signer::verify and, more confusingly, with the site-verification product feature in the integrations UI (isVerified, verificationNeeded, handleVerify). Renaming to refresh_registration_status / refresh-status disambiguates the two and better reflects that the call reconciles local state with the server, self-healing by forgetting the registration on 401/404 rather than performing a pure check. Renames the application facade method, the REST route (POST /myyoast/refresh-status) and its callback, the WP-CLI subcommand (wp yoast auth refresh-status), the JS store endpoint/action, and the corresponding unit tests. The crypto verify and all site-verification identifiers are left untouched. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../integrations-page/myyoast-integration.js | 12 ++--- .../js/src/integrations-page/myyoast-store.js | 10 ++-- .../application/myyoast-client.php | 8 +++- .../user-interface/auth-command.php | 16 ++++--- .../user-interface/management-route.php | 20 ++++---- .../User_Interface/Management_Route_Test.php | 48 +++++++++---------- 6 files changed, 60 insertions(+), 54 deletions(-) diff --git a/packages/js/src/integrations-page/myyoast-integration.js b/packages/js/src/integrations-page/myyoast-integration.js index 8a7959c399b..bfd80a1278f 100644 --- a/packages/js/src/integrations-page/myyoast-integration.js +++ b/packages/js/src/integrations-page/myyoast-integration.js @@ -66,7 +66,7 @@ const formatRateLimitedMessage = ( seconds ) => { }; const ACTION_DISPATCHERS = { - verify: "verifyMyyoastConnection", + refreshStatus: "refreshMyyoastConnectionStatus", connect: "connectMyyoastConnection", update: "updateMyyoastConnection", disconnect: "disconnectMyyoastConnection", @@ -94,7 +94,7 @@ const resolveErrorMessage = ( code, details ) => { * Runs a MyYoast management action: dispatches the slice action and, unless * silent, surfaces the outcome as inline card feedback. * - * @param {string} actionName The action (verify/connect/update/disconnect). + * @param {string} actionName The action (refreshStatus/connect/update/disconnect). * @param {Object} [body] The request body. * @param {Object} [options] Options. * @param {boolean} [options.silent] When true, suppress feedback. @@ -104,8 +104,8 @@ const resolveErrorMessage = ( code, details ) => { // eslint-disable-next-line complexity const runAction = async( actionName, body, options ) => { // Serialize actions: they all mutate the same server-side registration, so a - // second action started while one is in flight (e.g. the mount-time verify - // overlapping a user click) would race on the shared status. Ignore it. + // second action started while one is in flight (e.g. the mount-time status + // refresh overlapping a user click) would race on the shared status. Ignore it. if ( select( MYYOAST_STORE_NAME ).selectMyyoastConnectionActionInFlight() ) { return { ok: false, errorCode: "action_in_flight" }; } @@ -222,13 +222,13 @@ export const MyyoastIntegration = () => { const [ feedback, setFeedback ] = useState( null ); const [ isDisconnectOpen, , , openDisconnect, closeDisconnect ] = useToggleState( false ); - // Auto-fired verify on mount: confirms with MyYoast that the stored + // Auto-fired status refresh on mount: confirms with MyYoast that the stored // registration is still valid. Errors are silent; the response refreshes // the local status, so Registration_Not_Found clears state and the card // re-renders as "not connected". useEffect( () => { if ( status.isRegistered ) { - runAction( "verify", null, { silent: true } ); + runAction( "refreshStatus", null, { silent: true } ); } }, [] ); diff --git a/packages/js/src/integrations-page/myyoast-store.js b/packages/js/src/integrations-page/myyoast-store.js index 0f1d10e24c7..af5a39ecd4f 100644 --- a/packages/js/src/integrations-page/myyoast-store.js +++ b/packages/js/src/integrations-page/myyoast-store.js @@ -9,7 +9,7 @@ export const MYYOAST_STORE_NAME = "yoast-seo/myyoast-connection"; const REQUEST_TIMEOUT_MS = 30000; const ENDPOINTS = { - verify: { actionType: "verifyMyyoastConnection", method: "POST", path: "/yoast/v1/myyoast/verify" }, + refreshStatus: { actionType: "refreshMyyoastConnectionStatus", method: "POST", path: "/yoast/v1/myyoast/refresh-status" }, connect: { actionType: "connectMyyoastConnection", method: "POST", path: "/yoast/v1/myyoast/register" }, update: { actionType: "updateMyyoastConnection", method: "PUT", path: "/yoast/v1/myyoast/registration" }, disconnect: { actionType: "disconnectMyyoastConnection", method: "DELETE", path: "/yoast/v1/myyoast/registration" }, @@ -101,7 +101,7 @@ const { setMyyoastStatus, startMyyoastAction, setMyyoastActionError, finishMyyoa * the matching control, mirrors the response into the slice, and returns a * result object the caller can use to drive UI notifications. * - * @param {string} name The action name (verify/connect/update/disconnect). + * @param {string} name The action name (refreshStatus/connect/update/disconnect). * @returns {GeneratorFunction} The generator action. */ // eslint-disable-next-line complexity @@ -128,7 +128,7 @@ const createMyyoastAction = ( name ) => function* ( body ) { } }; -const verifyMyyoastConnection = createMyyoastAction( "verify" ); +const refreshMyyoastConnectionStatus = createMyyoastAction( "refreshStatus" ); const connectMyyoastConnection = createMyyoastAction( "connect" ); const updateMyyoastConnection = createMyyoastAction( "update" ); const disconnectMyyoastConnection = createMyyoastAction( "disconnect" ); @@ -193,7 +193,7 @@ const callEndpoint = async( endpoint, body ) => { export const myyoastConnectionActions = { ...slice.actions, - verifyMyyoastConnection, + refreshMyyoastConnectionStatus, connectMyyoastConnection, updateMyyoastConnection, disconnectMyyoastConnection, @@ -201,7 +201,7 @@ export const myyoastConnectionActions = { }; export const myyoastConnectionControls = { - [ ENDPOINTS.verify.actionType ]: ( { payload } ) => callEndpoint( ENDPOINTS.verify, payload ), + [ ENDPOINTS.refreshStatus.actionType ]: ( { payload } ) => callEndpoint( ENDPOINTS.refreshStatus, payload ), [ ENDPOINTS.connect.actionType ]: ( { payload } ) => callEndpoint( ENDPOINTS.connect, payload ), [ ENDPOINTS.update.actionType ]: ( { payload } ) => callEndpoint( ENDPOINTS.update, payload ), [ ENDPOINTS.disconnect.actionType ]: ( { payload } ) => callEndpoint( ENDPOINTS.disconnect, payload ), diff --git a/src/myyoast-client/application/myyoast-client.php b/src/myyoast-client/application/myyoast-client.php index b1df9a1c6d8..241c25d5fd4 100644 --- a/src/myyoast-client/application/myyoast-client.php +++ b/src/myyoast-client/application/myyoast-client.php @@ -182,13 +182,17 @@ public function is_registered(): bool { } /** - * Reads the current client registration from the server. + * Refreshes the local registration status against the server. + * + * Reads the current client registration (RFC 7592 GET) to confirm it is + * still live; the underlying read self-heals by forgetting the local + * registration when the server reports it gone. * * @return array The registration metadata. * * @throws Registration_Failed_Exception If the read fails. */ - public function verify_registration(): array { + public function refresh_registration_status(): array { return $this->client_registration->read_registration(); } diff --git a/src/myyoast-client/user-interface/auth-command.php b/src/myyoast-client/user-interface/auth-command.php index 2ad361b1f33..672b4ce391f 100644 --- a/src/myyoast-client/user-interface/auth-command.php +++ b/src/myyoast-client/user-interface/auth-command.php @@ -266,7 +266,7 @@ public function register( $args = null, $assoc_args = null ): void { } /** - * Verifies the client registration with the server. + * Refreshes the client registration status against the server. * * Reads the current registration from the authorization server to * confirm it is still valid and shows the registration metadata. @@ -284,8 +284,10 @@ public function register( $args = null, $assoc_args = null ): void { * * ## EXAMPLES * - * wp yoast auth verify - * wp yoast auth verify --format=json + * wp yoast auth refresh-status + * wp yoast auth refresh-status --format=json + * + * @subcommand refresh-status * * @when after_wp_load * @@ -294,17 +296,17 @@ public function register( $args = null, $assoc_args = null ): void { * * @return void * - * @throws ExitException When verification fails. + * @throws ExitException When the status refresh fails. */ - public function verify( $args = null, $assoc_args = null ): void { + public function refresh_status( $args = null, $assoc_args = null ): void { if ( ! $this->myyoast_client->is_registered() ) { WP_CLI::error( 'Not registered. Run "wp yoast auth register" first.' ); } try { - $metadata = $this->myyoast_client->verify_registration(); + $metadata = $this->myyoast_client->refresh_registration_status(); } catch ( Exception $e ) { - WP_CLI::error( 'Verification failed: ' . $e->getMessage() ); + WP_CLI::error( 'Status refresh failed: ' . $e->getMessage() ); return; } diff --git a/src/myyoast-client/user-interface/management-route.php b/src/myyoast-client/user-interface/management-route.php index 35ea18076a0..a34594479a7 100644 --- a/src/myyoast-client/user-interface/management-route.php +++ b/src/myyoast-client/user-interface/management-route.php @@ -39,11 +39,11 @@ class Management_Route implements Route_Interface, LoggerAwareInterface { public const ROUTE_PREFIX = '/myyoast'; - public const STATUS_ROUTE = '/status'; - public const VERIFY_ROUTE = '/verify'; - public const REGISTER_ROUTE = '/register'; - public const REGISTRATION_ROUTE = '/registration'; - public const AUTHORIZE_ROUTE = '/authorize'; + public const STATUS_ROUTE = '/status'; + public const REFRESH_STATUS_ROUTE = '/refresh-status'; + public const REGISTER_ROUTE = '/register'; + public const REGISTRATION_ROUTE = '/registration'; + public const AUTHORIZE_ROUTE = '/authorize'; /** * The MyYoast client facade. @@ -123,10 +123,10 @@ public function register_routes() { \register_rest_route( Main::API_V1_NAMESPACE, - self::ROUTE_PREFIX . self::VERIFY_ROUTE, + self::ROUTE_PREFIX . self::REFRESH_STATUS_ROUTE, [ 'methods' => 'POST', - 'callback' => [ $this, 'verify' ], + 'callback' => [ $this, 'refresh_status' ], 'permission_callback' => $permission_callback, ], ); @@ -188,13 +188,13 @@ public function get_status() { } /** - * POST /myyoast/verify — verifies the registration with the server. + * POST /myyoast/refresh-status — refreshes the registration status against the server. * * @return WP_REST_Response */ - public function verify() { + public function refresh_status() { try { - $this->myyoast_client->verify_registration(); + $this->myyoast_client->refresh_registration_status(); } catch ( Throwable $e ) { return $this->handle_exception( $e ); } diff --git a/tests/Unit/MyYoast_Client/User_Interface/Management_Route_Test.php b/tests/Unit/MyYoast_Client/User_Interface/Management_Route_Test.php index 888dab038aa..e0914ac1d3f 100644 --- a/tests/Unit/MyYoast_Client/User_Interface/Management_Route_Test.php +++ b/tests/Unit/MyYoast_Client/User_Interface/Management_Route_Test.php @@ -149,7 +149,7 @@ public function test_get_conditionals() { /** * Tests that all routes are registered. Five register_rest_route calls - * are made: /status (GET), /verify (POST), /register (POST), + * are made: /status (GET), /refresh-status (POST), /register (POST), * /registration (PUT + DELETE on the same path), and /authorize (POST). * * @covers ::register_routes @@ -197,42 +197,42 @@ public function test_get_status_returns_payload() { } /** - * Tests that verify dispatches to the facade and returns a success payload. + * Tests that refresh_status dispatches to the facade and returns a success payload. * - * @covers ::verify + * @covers ::refresh_status * * @return void */ - public function test_verify_success() { + public function test_refresh_status_success() { $this->provision(); - $this->myyoast_client->shouldReceive( 'verify_registration' )->once()->andReturn( [] ); + $this->myyoast_client->shouldReceive( 'refresh_registration_status' )->once()->andReturn( [] ); $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); Mockery::mock( 'overload:' . WP_REST_Response::class ); - $response = $this->instance->verify(); + $response = $this->instance->refresh_status(); $this->assertInstanceOf( WP_REST_Response::class, $response ); } /** - * Tests that verify hitting Registration_Not_Found_Exception surfaces registration_gone. + * Tests that refresh_status hitting Registration_Not_Found_Exception surfaces registration_gone. * - * @covers ::verify + * @covers ::refresh_status * @covers ::handle_exception * * @return void */ - public function test_verify_with_registration_gone() { + public function test_refresh_status_with_registration_gone() { $this->provision(); - $this->myyoast_client->shouldReceive( 'verify_registration' ) + $this->myyoast_client->shouldReceive( 'refresh_registration_status' ) ->once() ->andThrow( new Registration_Not_Found_Exception( 'gone' ) ); $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); Mockery::mock( 'overload:' . WP_REST_Response::class ); - $response = $this->instance->verify(); + $response = $this->instance->refresh_status(); $this->assertInstanceOf( WP_REST_Response::class, $response ); } @@ -248,7 +248,7 @@ public function test_verify_with_registration_gone() { */ public function test_rate_limited_response_includes_retry_after_details() { $this->provision(); - $this->myyoast_client->shouldReceive( 'verify_registration' ) + $this->myyoast_client->shouldReceive( 'refresh_registration_status' ) ->once() ->andThrow( new Rate_Limited_Exception( 'slow down', 240 ) ); $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); @@ -261,7 +261,7 @@ static function ( $body ) use ( &$captured ): void { }, ); - $this->instance->verify(); + $this->instance->refresh_status(); $this->assertIsArray( $captured ); $this->assertSame( 'rate_limited', $captured['error_code'] ); @@ -280,7 +280,7 @@ static function ( $body ) use ( &$captured ): void { */ public function test_rate_limited_response_omits_details_when_retry_after_missing() { $this->provision(); - $this->myyoast_client->shouldReceive( 'verify_registration' ) + $this->myyoast_client->shouldReceive( 'refresh_registration_status' ) ->once() ->andThrow( new Rate_Limited_Exception( 'slow down' ) ); $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); @@ -293,7 +293,7 @@ static function ( $body ) use ( &$captured ): void { }, ); - $this->instance->verify(); + $this->instance->refresh_status(); $this->assertIsArray( $captured ); $this->assertSame( 'rate_limited', $captured['error_code'] ); @@ -301,7 +301,7 @@ static function ( $body ) use ( &$captured ): void { } /** - * Tests that verify maps each domain exception to a response (smoke test + * Tests that refresh_status maps each domain exception to a response (smoke test * — exception-mapping coverage is per-branch but groups into one test). * * @covers ::handle_exception @@ -326,11 +326,11 @@ public function test_handle_exception_branches() { ]; foreach ( $exceptions as $exception ) { - $this->myyoast_client->shouldReceive( 'verify_registration' ) + $this->myyoast_client->shouldReceive( 'refresh_registration_status' ) ->once() ->andThrow( $exception ); - $response = $this->instance->verify(); + $response = $this->instance->refresh_status(); $this->assertInstanceOf( WP_REST_Response::class, $response ); } @@ -471,7 +471,7 @@ public function test_deregister_clears_tokens_when_remote_throws() { * not provisioned. * * Only register/update/authorize require the software statement and initial - * access token; verify and deregister work without them and are covered by + * access token; refresh-status and deregister work without them and are covered by * their own tests. * * @covers ::require_provisioned @@ -493,21 +493,21 @@ public function test_registration_actions_blocked_when_not_provisioned() { } /** - * Tests verify works regardless of provisioning — it talks to MyYoast with + * Tests refresh_status works regardless of provisioning — it talks to MyYoast with * the stored registration access token, not the software statement. * - * @covers ::verify + * @covers ::refresh_status * * @return void */ - public function test_verify_works_when_not_provisioned() { + public function test_refresh_status_works_when_not_provisioned() { $this->unprovision(); $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); - $this->myyoast_client->shouldReceive( 'verify_registration' )->once(); + $this->myyoast_client->shouldReceive( 'refresh_registration_status' )->once(); Mockery::mock( 'overload:' . WP_REST_Response::class ); - $this->assertInstanceOf( WP_REST_Response::class, $this->instance->verify() ); + $this->assertInstanceOf( WP_REST_Response::class, $this->instance->refresh_status() ); } /** From 7f16647ad30ce12bc50eddd5680819f05a273820 Mon Sep 17 00:00:00 2001 From: Diede Exterkate <5352634+diedexx@users.noreply.github.com> Date: Fri, 19 Jun 2026 17:03:40 +0200 Subject: [PATCH 06/26] refactor(myyoast-client): extract OAuth callback orchestration into the App layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the authorization-code callback use-case out of the UI integration and into a new App-layer OAuth_Callback_Handler plus a Callback_Outcome value object. The handler does the transport-agnostic work — discard pending flow state on a provider error, exchange the code, persist the outcome for one-shot surfacing, and report it in neutral OAuth terms — so any consumer (admin-post endpoint, REST route, WP-CLI) can drive it and translate the outcome onto its own surface. OAuth_Callback_Integration now only extracts the request parameters and delegates; Integrations_Page_Script_Data consumes the stored outcome through the handler and maps it to the front-end message keys. This keeps the onion dependency rule intact: the orchestration lives in App, the WordPress-specific adapters stay in User_Interface. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../application/callback-outcome.php | 192 ++++++++++++ .../application/oauth-callback-handler.php | 174 +++++++++++ .../integrations-page-script-data.php | 73 +++-- .../oauth-callback-integration.php | 109 ++----- .../Application/Callback_Outcome_Test.php | 133 ++++++++ .../OAuth_Callback_Handler_Test.php | 293 ++++++++++++++++++ .../Integrations_Page_Script_Data_Test.php | 81 +++-- .../OAuth_Callback_Integration_Test.php | 238 +++----------- 8 files changed, 961 insertions(+), 332 deletions(-) create mode 100644 src/myyoast-client/application/callback-outcome.php create mode 100644 src/myyoast-client/application/oauth-callback-handler.php create mode 100644 tests/Unit/MyYoast_Client/Application/Callback_Outcome_Test.php create mode 100644 tests/Unit/MyYoast_Client/Application/OAuth_Callback_Handler_Test.php diff --git a/src/myyoast-client/application/callback-outcome.php b/src/myyoast-client/application/callback-outcome.php new file mode 100644 index 00000000000..2fce1120a34 --- /dev/null +++ b/src/myyoast-client/application/callback-outcome.php @@ -0,0 +1,192 @@ +is_success = $is_success; + $this->is_no_op = $is_no_op; + $this->error_phase = $error_phase; + $this->error_code = $error_code; + } + + /** + * Creates a successful outcome (the code was exchanged and tokens stored). + * + * @return self + */ + public static function success(): self { + return new self( true, false, self::PHASE_NONE, null ); + } + + /** + * Creates a no-op outcome (empty code/state — not a real callback). + * + * @return self + */ + public static function no_op(): self { + return new self( false, true, self::PHASE_NONE, null ); + } + + /** + * Creates an outcome for an error reported by the authorization endpoint + * redirect (the `error` query parameter on the callback). + * + * @param string $oauth_error_code The native OAuth error code. + * + * @return self + */ + public static function provider_error( string $oauth_error_code ): self { + return new self( false, false, self::PHASE_PROVIDER, $oauth_error_code ); + } + + /** + * Creates an outcome for a failure while exchanging the code at the token + * endpoint. + * + * @param string|null $oauth_error_code The native OAuth error code, or null + * when the failure produced no OAuth + * response at all. + * + * @return self + */ + public static function exchange_error( ?string $oauth_error_code ): self { + return new self( false, false, self::PHASE_EXCHANGE, $oauth_error_code ); + } + + /** + * Whether the callback completed successfully. + * + * @return bool + */ + public function is_success(): bool { + return $this->is_success; + } + + /** + * Whether the request carried no actionable callback parameters. + * + * @return bool + */ + public function is_no_op(): bool { + return $this->is_no_op; + } + + /** + * Whether the callback failed. + * + * @return bool + */ + public function is_failure(): bool { + return ! $this->is_success && ! $this->is_no_op; + } + + /** + * Returns the OAuth phase that produced a failure. + * + * @return string A PHASE_* constant. + */ + public function get_error_phase(): string { + return $this->error_phase; + } + + /** + * Returns the native OAuth error code on failure. + * + * @return string|null The OAuth error code, or null when there is none. + */ + public function get_error_code(): ?string { + return $this->error_code; + } + + /** + * Converts the outcome to an associative array for storage. + * + * @return array{is_success: bool, is_no_op: bool, error_phase: string, error_code: string|null} + */ + public function to_array(): array { + return [ + 'is_success' => $this->is_success, + 'is_no_op' => $this->is_no_op, + 'error_phase' => $this->error_phase, + 'error_code' => $this->error_code, + ]; + } + + /** + * Creates a Callback_Outcome from a stored array. + * + * @param array $data The stored array data. + * + * @return self + */ + public static function from_array( array $data ): self { + $error_phase = ( isset( $data['error_phase'] ) && \is_string( $data['error_phase'] ) ) ? $data['error_phase'] : self::PHASE_NONE; + $error_code = ( isset( $data['error_code'] ) && \is_string( $data['error_code'] ) ) ? $data['error_code'] : null; + + return new self( + ! empty( $data['is_success'] ), + ! empty( $data['is_no_op'] ), + $error_phase, + $error_code, + ); + } +} diff --git a/src/myyoast-client/application/oauth-callback-handler.php b/src/myyoast-client/application/oauth-callback-handler.php new file mode 100644 index 00000000000..a55acaad47c --- /dev/null +++ b/src/myyoast-client/application/oauth-callback-handler.php @@ -0,0 +1,174 @@ +myyoast_client = $myyoast_client; + $this->auth_code_handler = $auth_code_handler; + $this->expiring_store = $expiring_store; + $this->logger = new NullLogger(); + } + + /** + * Handles an OAuth authorization-code callback. + * + * The outcome is persisted for the user (except for a no-op, which is not a + * real callback) so a later page load can surface it once, and also returned + * for the caller to act on immediately. + * + * @param int $user_id The WordPress user ID the flow belongs to. + * @param string $code The authorization code from the callback (empty if absent). + * @param string $state The state parameter from the callback (empty if absent). + * @param string $error The provider error code from the callback (empty if none). + * + * @return Callback_Outcome The outcome of the callback. + */ + public function handle( int $user_id, string $code, string $state, string $error ): Callback_Outcome { + $outcome = $this->resolve( $user_id, $code, $state, $error ); + + if ( ! $outcome->is_no_op() && $user_id > 0 ) { + $this->expiring_store->persist_for_user( + self::OUTCOME_KEY, + $outcome->to_array(), + self::OUTCOME_TTL, + $user_id, + ); + } + + return $outcome; + } + + /** + * Reads and consumes the pending callback outcome for a user. + * + * Consumed-on-read so the outcome is surfaced exactly once. + * + * @param int $user_id The WordPress user ID. + * + * @return Callback_Outcome|null The outcome, or null when none is pending. + */ + public function consume_outcome( int $user_id ): ?Callback_Outcome { + if ( $user_id <= 0 ) { + return null; + } + + try { + $stored = $this->expiring_store->get_for_user( self::OUTCOME_KEY, $user_id ); + } catch ( Key_Not_Found_Exception | Corrupted_Value_Exception $e ) { + return null; + } + + $this->expiring_store->delete_for_user( self::OUTCOME_KEY, $user_id ); + + if ( ! \is_array( $stored ) ) { + return null; + } + + return Callback_Outcome::from_array( $stored ); + } + + /** + * Performs the callback orchestration and classifies the result. + * + * @param int $user_id The WordPress user ID the flow belongs to. + * @param string $code The authorization code from the callback (empty if absent). + * @param string $state The state parameter from the callback (empty if absent). + * @param string $error The provider error code from the callback (empty if none). + * + * @return Callback_Outcome The outcome of the callback. + */ + private function resolve( int $user_id, string $code, string $state, string $error ): Callback_Outcome { + if ( $error !== '' ) { + // The provider returned an error: drop the pending flow so it can't be resumed. + $this->auth_code_handler->discard_flow_state( $user_id ); + return Callback_Outcome::provider_error( $error ); + } + + if ( $code === '' || $state === '' ) { + // Stale bookmark or someone hitting the callback URL directly: not a real callback. + return Callback_Outcome::no_op(); + } + + try { + $this->myyoast_client->exchange_authorization_code( $user_id, $code, $state ); + } catch ( Token_Request_Failed_Exception $e ) { + return Callback_Outcome::exchange_error( $e->get_error_code() ); + } catch ( Throwable $e ) { + $this->logger->error( + 'Unexpected error during MyYoast OAuth callback exchange for user {user_id}: {error}', + [ + 'user_id' => $user_id, + 'error' => $e->getMessage(), + ], + ); + // No OAuth response was produced, so there is no native error code to surface. + return Callback_Outcome::exchange_error( null ); + } + + return Callback_Outcome::success(); + } +} diff --git a/src/myyoast-client/user-interface/integrations-page-script-data.php b/src/myyoast-client/user-interface/integrations-page-script-data.php index b827dcc293d..88bcf6dc51f 100644 --- a/src/myyoast-client/user-interface/integrations-page-script-data.php +++ b/src/myyoast-client/user-interface/integrations-page-script-data.php @@ -5,6 +5,8 @@ namespace Yoast\WP\SEO\MyYoast_Client\User_Interface; use Yoast\WP\SEO\Conditionals\MyYoast_Connection_Conditional; +use Yoast\WP\SEO\MyYoast_Client\Application\Callback_Outcome; +use Yoast\WP\SEO\MyYoast_Client\Application\OAuth_Callback_Handler; /** * Builds the MyYoast connection payload exposed to the Integrations page's @@ -29,18 +31,28 @@ class Integrations_Page_Script_Data { */ private $myyoast_connection_conditional; + /** + * The callback handler — reads the pending OAuth callback outcome. + * + * @var OAuth_Callback_Handler + */ + private $callback_handler; + /** * Integrations_Page_Script_Data constructor. * * @param Status_Presenter $status_presenter The status presenter. * @param MyYoast_Connection_Conditional $myyoast_connection_conditional The MyYoast connection feature-flag conditional. + * @param OAuth_Callback_Handler $callback_handler The callback handler. */ public function __construct( Status_Presenter $status_presenter, - MyYoast_Connection_Conditional $myyoast_connection_conditional + MyYoast_Connection_Conditional $myyoast_connection_conditional, + OAuth_Callback_Handler $callback_handler ) { $this->status_presenter = $status_presenter; $this->myyoast_connection_conditional = $myyoast_connection_conditional; + $this->callback_handler = $callback_handler; } /** @@ -66,34 +78,57 @@ public function present(): ?array { } /** - * Reads and deletes the per-user OAuth callback outcome transient. - * - * Consumed-on-read so the notification only fires once. + * Reads and consumes the pending OAuth callback outcome for the current user + * and shapes it for the React app. * * @return array{kind: string, key: string}|null The outcome, or null when none is pending. */ private function consume_callback_outcome(): ?array { - $user_id = \get_current_user_id(); - if ( $user_id <= 0 ) { + $outcome = $this->callback_handler->consume_outcome( \get_current_user_id() ); + if ( $outcome === null ) { return null; } - $transient_key = OAuth_Callback_Integration::TRANSIENT_PREFIX . $user_id; - $stored = \get_transient( $transient_key ); - if ( ! \is_array( $stored ) ) { - return null; - } - \delete_transient( $transient_key ); - - $kind = ( $stored['kind'] ?? '' ); - $key = ( $stored['key'] ?? '' ); - if ( ! \is_string( $kind ) || ! \is_string( $key ) || $kind === '' || $key === '' ) { - return null; + if ( $outcome->is_success() ) { + return [ + 'kind' => 'success', + 'key' => 'verify_success', + ]; } return [ - 'kind' => $kind, - 'key' => $key, + 'kind' => 'error', + 'key' => $this->error_message_key( $outcome ), ]; } + + /** + * Maps a failed callback outcome to the front-end message key. + * + * Translates the neutral, native-OAuth outcome into the message keys the + * integrations-page JS understands (see `buildMessages()` in + * `myyoast-integration.js`). The same missing code means different things per + * OAuth phase: a provider error other than `access_denied` is unexpected, + * while a token-endpoint error other than `invalid_grant` is a generic token + * failure. + * + * @param Callback_Outcome $outcome The failed callback outcome. + * + * @return string The message key the front-end maps to copy. + */ + private function error_message_key( Callback_Outcome $outcome ): string { + if ( $outcome->get_error_phase() === Callback_Outcome::PHASE_PROVIDER ) { + return ( $outcome->get_error_code() === 'access_denied' ) ? 'connection_cancelled' : 'unexpected_error'; + } + + if ( $outcome->get_error_code() === 'invalid_grant' ) { + return 'token_request_failed_invalid_grant'; + } + + if ( $outcome->get_error_code() === null ) { + return 'unexpected_error'; + } + + return 'token_request_failed'; + } } diff --git a/src/myyoast-client/user-interface/oauth-callback-integration.php b/src/myyoast-client/user-interface/oauth-callback-integration.php index e8d491cb265..0c46f22a0b5 100644 --- a/src/myyoast-client/user-interface/oauth-callback-integration.php +++ b/src/myyoast-client/user-interface/oauth-callback-integration.php @@ -4,17 +4,12 @@ namespace Yoast\WP\SEO\MyYoast_Client\User_Interface; -use Throwable; use Yoast\WP\SEO\Conditionals\MyYoast_Connection_Conditional; use Yoast\WP\SEO\General\User_Interface\General_Page_Integration; use Yoast\WP\SEO\Helpers\Redirect_Helper; use Yoast\WP\SEO\Integrations\Integration_Interface; use Yoast\WP\SEO\MyYoast_Client\Application\Authorization_Code_Handler; -use Yoast\WP\SEO\MyYoast_Client\Application\Exceptions\Token_Request_Failed_Exception; -use Yoast\WP\SEO\MyYoast_Client\Application\MyYoast_Client; -use YoastSEO_Vendor\Psr\Log\LoggerAwareInterface; -use YoastSEO_Vendor\Psr\Log\LoggerAwareTrait; -use YoastSEO_Vendor\Psr\Log\NullLogger; +use Yoast\WP\SEO\MyYoast_Client\Application\OAuth_Callback_Handler; /** * Handles the OAuth authorization-code callback redirect. @@ -22,33 +17,27 @@ * Registers a dedicated callback endpoint on `admin-post.php` (reachable for * any logged-in user, regardless of which admin page the flow started from) as * the site's OAuth redirect URI, hooks the matching `admin_post_*` action, - * exchanges the returning code, surfaces a notification via a short-lived - * per-user transient, and redirects the user to the `return_url` they were - * sent off from. + * drives the callback handler (which exchanges the returning code and records + * the outcome for one-shot surfacing), and redirects the user to the + * `return_url` they were sent off from. * * The base client defaults its redirect URI to an admin page; we replace that * with this endpoint through the redirect-URI provider's filters so the * callback never depends on a specific page being loaded. */ -class OAuth_Callback_Integration implements Integration_Interface, LoggerAwareInterface { - - use LoggerAwareTrait; +class OAuth_Callback_Integration implements Integration_Interface { public const CALLBACK_ACTION = 'yoast_myyoast_oauth_callback'; - public const TRANSIENT_PREFIX = 'wpseo_myyoast_oauth_outcome_'; - private const TRANSIENT_TTL = \MINUTE_IN_SECONDS; - /** - * The MyYoast client facade. + * The callback handler — performs the callback-URL-agnostic orchestration. * - * @var MyYoast_Client + * @var OAuth_Callback_Handler */ - private $myyoast_client; + private $callback_handler; /** - * The authorization code handler — used to read the stored return URL and - * to discard pending state when the provider returns an error. + * The authorization code handler — used to read the stored return URL. * * @var Authorization_Code_Handler */ @@ -65,19 +54,18 @@ class OAuth_Callback_Integration implements Integration_Interface, LoggerAwareIn /** * Constructor. * - * @param MyYoast_Client $myyoast_client The MyYoast client facade. + * @param OAuth_Callback_Handler $callback_handler The callback handler. * @param Authorization_Code_Handler $auth_code_handler The authorization code handler. * @param Redirect_Helper $redirect_helper The redirect helper. */ public function __construct( - MyYoast_Client $myyoast_client, + OAuth_Callback_Handler $callback_handler, Authorization_Code_Handler $auth_code_handler, Redirect_Helper $redirect_helper ) { - $this->myyoast_client = $myyoast_client; + $this->callback_handler = $callback_handler; $this->auth_code_handler = $auth_code_handler; $this->redirect_helper = $redirect_helper; - $this->logger = new NullLogger(); } /** @@ -98,7 +86,6 @@ public function register_hooks() { \add_action( 'admin_post_' . self::CALLBACK_ACTION, [ $this, 'handle' ] ); } - /** * Returns this site's dedicated OAuth callback endpoint URL. * @@ -124,48 +111,15 @@ public function handle(): void { return; } - $error = $this->read_query_arg( 'error' ); - if ( $error !== '' ) { - $this->auth_code_handler->discard_flow_state( $user_id ); - $key = ( $error === 'access_denied' ) ? 'connection_cancelled' : 'unexpected_error'; - $this->set_outcome( $user_id, 'error', $key ); - $this->redirect_helper->do_safe_redirect( $return_url ); - return; - } - - $code = $this->read_query_arg( 'code' ); - $state = $this->read_query_arg( 'state' ); - - if ( $code === '' || $state === '' ) { - // Stale bookmark or someone hitting the callback URL directly. No notification. - $this->redirect_helper->do_safe_redirect( $return_url ); - return; - } - - try { - $this->myyoast_client->exchange_authorization_code( $user_id, $code, $state ); - } catch ( Token_Request_Failed_Exception $e ) { - $is_invalid_grant = ( $e->get_error_code() === 'invalid_grant' ); - $key = ( $is_invalid_grant ) ? 'token_request_failed_invalid_grant' : 'token_request_failed'; - $this->set_outcome( $user_id, 'error', $key ); - $this->redirect_helper->do_safe_redirect( $return_url ); - return; - } catch ( Throwable $e ) { - $this->logger->error( - 'Unexpected error during MyYoast OAuth callback exchange for user {user_id}: {error}', - [ - 'user_id' => $user_id, - 'error' => $e->getMessage(), - ], - ); - $this->set_outcome( $user_id, 'error', 'unexpected_error' ); - $this->redirect_helper->do_safe_redirect( $return_url ); - return; - } + // The handler records the outcome for the next page load to surface; this + // endpoint only needs to send the browser back where the flow started. + $this->callback_handler->handle( + $user_id, + $this->read_query_arg( 'code' ), + $this->read_query_arg( 'state' ), + $this->read_query_arg( 'error' ), + ); - // The authorization-code handler marks the redirect URI validated as part of - // exchanging the code, so the refreshed status already reflects the verified state. - $this->set_outcome( $user_id, 'success', 'verify_success' ); $this->redirect_helper->do_safe_redirect( $return_url ); } @@ -211,27 +165,4 @@ private function read_query_arg( string $name ): string { return \sanitize_text_field( \wp_unslash( $_GET[ $name ] ) ); // phpcs:enable WordPress.Security.NonceVerification.Recommended } - - /** - * Stores the callback outcome in a short-lived per-user transient. - * - * @param int $user_id The WordPress user ID. - * @param string $kind The outcome kind, either "success" or "error". - * @param string $key The message key the front-end maps to copy. - * - * @return void - */ - private function set_outcome( int $user_id, string $kind, string $key ): void { - if ( $user_id <= 0 ) { - return; - } - \set_transient( - self::TRANSIENT_PREFIX . $user_id, - [ - 'kind' => $kind, - 'key' => $key, - ], - self::TRANSIENT_TTL, - ); - } } diff --git a/tests/Unit/MyYoast_Client/Application/Callback_Outcome_Test.php b/tests/Unit/MyYoast_Client/Application/Callback_Outcome_Test.php new file mode 100644 index 00000000000..7b9572f6bfa --- /dev/null +++ b/tests/Unit/MyYoast_Client/Application/Callback_Outcome_Test.php @@ -0,0 +1,133 @@ +assertTrue( $outcome->is_success() ); + $this->assertFalse( $outcome->is_no_op() ); + $this->assertFalse( $outcome->is_failure() ); + $this->assertSame( Callback_Outcome::PHASE_NONE, $outcome->get_error_phase() ); + $this->assertNull( $outcome->get_error_code() ); + } + + /** + * Tests the no-op outcome. + * + * @covers ::no_op + * @covers ::is_success + * @covers ::is_no_op + * @covers ::is_failure + * + * @return void + */ + public function test_no_op() { + $outcome = Callback_Outcome::no_op(); + + $this->assertFalse( $outcome->is_success() ); + $this->assertTrue( $outcome->is_no_op() ); + $this->assertFalse( $outcome->is_failure() ); + } + + /** + * Tests the provider-error outcome. + * + * @covers ::provider_error + * @covers ::is_failure + * @covers ::get_error_phase + * @covers ::get_error_code + * + * @return void + */ + public function test_provider_error() { + $outcome = Callback_Outcome::provider_error( 'access_denied' ); + + $this->assertTrue( $outcome->is_failure() ); + $this->assertFalse( $outcome->is_success() ); + $this->assertFalse( $outcome->is_no_op() ); + $this->assertSame( Callback_Outcome::PHASE_PROVIDER, $outcome->get_error_phase() ); + $this->assertSame( 'access_denied', $outcome->get_error_code() ); + } + + /** + * Tests the exchange-error outcome, including the code-less case. + * + * @covers ::exchange_error + * @covers ::is_failure + * @covers ::get_error_phase + * @covers ::get_error_code + * + * @return void + */ + public function test_exchange_error() { + $with_code = Callback_Outcome::exchange_error( 'invalid_grant' ); + $this->assertTrue( $with_code->is_failure() ); + $this->assertSame( Callback_Outcome::PHASE_EXCHANGE, $with_code->get_error_phase() ); + $this->assertSame( 'invalid_grant', $with_code->get_error_code() ); + + $without_code = Callback_Outcome::exchange_error( null ); + $this->assertTrue( $without_code->is_failure() ); + $this->assertSame( Callback_Outcome::PHASE_EXCHANGE, $without_code->get_error_phase() ); + $this->assertNull( $without_code->get_error_code() ); + } + + /** + * Tests an outcome survives a round-trip through array serialization. + * + * @covers ::to_array + * @covers ::from_array + * + * @dataProvider provide_round_trip_outcomes + * + * @param Callback_Outcome $outcome The outcome to round-trip. + * + * @return void + */ + public function test_array_round_trip( Callback_Outcome $outcome ) { + $restored = Callback_Outcome::from_array( $outcome->to_array() ); + + $this->assertSame( $outcome->is_success(), $restored->is_success() ); + $this->assertSame( $outcome->is_no_op(), $restored->is_no_op() ); + $this->assertSame( $outcome->is_failure(), $restored->is_failure() ); + $this->assertSame( $outcome->get_error_phase(), $restored->get_error_phase() ); + $this->assertSame( $outcome->get_error_code(), $restored->get_error_code() ); + } + + /** + * Provides outcomes for the array round-trip test. + * + * @return array + */ + public static function provide_round_trip_outcomes(): array { + return [ + 'success' => [ Callback_Outcome::success() ], + 'no_op' => [ Callback_Outcome::no_op() ], + 'provider error' => [ Callback_Outcome::provider_error( 'access_denied' ) ], + 'exchange error' => [ Callback_Outcome::exchange_error( 'invalid_grant' ) ], + 'code-less exchange' => [ Callback_Outcome::exchange_error( null ) ], + ]; + } +} diff --git a/tests/Unit/MyYoast_Client/Application/OAuth_Callback_Handler_Test.php b/tests/Unit/MyYoast_Client/Application/OAuth_Callback_Handler_Test.php new file mode 100644 index 00000000000..c33f06910e1 --- /dev/null +++ b/tests/Unit/MyYoast_Client/Application/OAuth_Callback_Handler_Test.php @@ -0,0 +1,293 @@ +myyoast_client = Mockery::mock( MyYoast_Client::class ); + $this->auth_code_handler = Mockery::mock( Authorization_Code_Handler::class ); + $this->expiring_store = Mockery::mock( Expiring_Store::class ); + + $this->instance = new OAuth_Callback_Handler( + $this->myyoast_client, + $this->auth_code_handler, + $this->expiring_store, + ); + } + + /** + * Tests a provider `access_denied` error discards the flow state and reports the native code. + * + * @covers ::__construct + * @covers ::handle + * + * @return void + */ + public function test_handle_provider_access_denied() { + $this->auth_code_handler->shouldReceive( 'discard_flow_state' )->once()->with( 7 ); + $this->myyoast_client->shouldNotReceive( 'exchange_authorization_code' ); + $this->expect_persist( 7 ); + + $outcome = $this->instance->handle( 7, '', '', 'access_denied' ); + + $this->assertTrue( $outcome->is_failure() ); + $this->assertSame( Callback_Outcome::PHASE_PROVIDER, $outcome->get_error_phase() ); + $this->assertSame( 'access_denied', $outcome->get_error_code() ); + } + + /** + * Tests another provider error still discards the flow state and passes the native code through. + * + * @covers ::handle + * + * @return void + */ + public function test_handle_provider_other_error() { + $this->auth_code_handler->shouldReceive( 'discard_flow_state' )->once()->with( 7 ); + $this->expect_persist( 7 ); + + $outcome = $this->instance->handle( 7, '', '', 'server_error' ); + + $this->assertTrue( $outcome->is_failure() ); + $this->assertSame( Callback_Outcome::PHASE_PROVIDER, $outcome->get_error_phase() ); + $this->assertSame( 'server_error', $outcome->get_error_code() ); + } + + /** + * Tests a missing code is treated as a no-op without exchanging or discarding. + * + * @covers ::handle + * + * @return void + */ + public function test_handle_missing_code_is_no_op() { + $this->auth_code_handler->shouldNotReceive( 'discard_flow_state' ); + $this->myyoast_client->shouldNotReceive( 'exchange_authorization_code' ); + $this->expiring_store->shouldNotReceive( 'persist_for_user' ); + + $outcome = $this->instance->handle( 42, '', 'xyz', '' ); + + $this->assertTrue( $outcome->is_no_op() ); + $this->assertFalse( $outcome->is_failure() ); + } + + /** + * Tests a missing state is treated as a no-op without exchanging or discarding. + * + * @covers ::handle + * + * @return void + */ + public function test_handle_missing_state_is_no_op() { + $this->auth_code_handler->shouldNotReceive( 'discard_flow_state' ); + $this->myyoast_client->shouldNotReceive( 'exchange_authorization_code' ); + $this->expiring_store->shouldNotReceive( 'persist_for_user' ); + + $outcome = $this->instance->handle( 42, 'abc', '', '' ); + + $this->assertTrue( $outcome->is_no_op() ); + } + + /** + * Tests a successful exchange reports success. + * + * @covers ::handle + * + * @return void + */ + public function test_handle_success() { + $this->myyoast_client->shouldReceive( 'exchange_authorization_code' ) + ->once() + ->with( 42, 'abc', 'xyz' ); + $this->expect_persist( 42 ); + + $outcome = $this->instance->handle( 42, 'abc', 'xyz', '' ); + + $this->assertTrue( $outcome->is_success() ); + $this->assertFalse( $outcome->is_failure() ); + $this->assertNull( $outcome->get_error_code() ); + } + + /** + * Tests an `invalid_grant` token failure passes the native code through as an exchange error. + * + * @covers ::handle + * + * @return void + */ + public function test_handle_invalid_grant_exchange_error() { + $this->myyoast_client->shouldReceive( 'exchange_authorization_code' ) + ->once() + ->andThrow( new Token_Request_Failed_Exception( 'invalid_grant', 'expired' ) ); + $this->expect_persist( 11 ); + + $outcome = $this->instance->handle( 11, 'abc', 'xyz', '' ); + + $this->assertTrue( $outcome->is_failure() ); + $this->assertSame( Callback_Outcome::PHASE_EXCHANGE, $outcome->get_error_phase() ); + $this->assertSame( 'invalid_grant', $outcome->get_error_code() ); + } + + /** + * Tests another token failure passes its native code through as an exchange error. + * + * @covers ::handle + * + * @return void + */ + public function test_handle_other_token_exchange_error() { + $this->myyoast_client->shouldReceive( 'exchange_authorization_code' ) + ->once() + ->andThrow( new Token_Request_Failed_Exception( 'invalid_request', 'state mismatch' ) ); + $this->expect_persist( 11 ); + + $outcome = $this->instance->handle( 11, 'abc', 'xyz', '' ); + + $this->assertTrue( $outcome->is_failure() ); + $this->assertSame( Callback_Outcome::PHASE_EXCHANGE, $outcome->get_error_phase() ); + $this->assertSame( 'invalid_request', $outcome->get_error_code() ); + } + + /** + * Tests an unexpected exception is logged and reported as a code-less exchange error. + * + * @covers ::handle + * + * @return void + */ + public function test_handle_unexpected_exception_is_logged() { + $this->myyoast_client->shouldReceive( 'exchange_authorization_code' ) + ->once() + ->andThrow( new Exception( 'boom' ) ); + + $logger = Mockery::mock( LoggerInterface::class ); + $logger->shouldReceive( 'error' )->once(); + $this->instance->setLogger( $logger ); + $this->expect_persist( 11 ); + + $outcome = $this->instance->handle( 11, 'abc', 'xyz', '' ); + + $this->assertTrue( $outcome->is_failure() ); + $this->assertSame( Callback_Outcome::PHASE_EXCHANGE, $outcome->get_error_phase() ); + $this->assertNull( $outcome->get_error_code() ); + } + + /** + * Tests consuming the outcome reads, deletes, and reconstructs it. + * + * @covers ::consume_outcome + * + * @return void + */ + public function test_consume_outcome_reads_and_deletes() { + $stored = Callback_Outcome::provider_error( 'access_denied' )->to_array(); + + $this->expiring_store->shouldReceive( 'get_for_user' )->once()->with( self::OUTCOME_KEY, 7 )->andReturn( $stored ); + $this->expiring_store->shouldReceive( 'delete_for_user' )->once()->with( self::OUTCOME_KEY, 7 ); + + $outcome = $this->instance->consume_outcome( 7 ); + + $this->assertInstanceOf( Callback_Outcome::class, $outcome ); + $this->assertTrue( $outcome->is_failure() ); + $this->assertSame( Callback_Outcome::PHASE_PROVIDER, $outcome->get_error_phase() ); + $this->assertSame( 'access_denied', $outcome->get_error_code() ); + } + + /** + * Tests consuming returns null and does not delete when nothing is stored. + * + * @covers ::consume_outcome + * + * @return void + */ + public function test_consume_outcome_returns_null_when_absent() { + $this->expiring_store->shouldReceive( 'get_for_user' ) + ->once() + ->with( self::OUTCOME_KEY, 7 ) + ->andThrow( new Key_Not_Found_Exception() ); + $this->expiring_store->shouldNotReceive( 'delete_for_user' ); + + $this->assertNull( $this->instance->consume_outcome( 7 ) ); + } + + /** + * Tests consuming is skipped for an invalid user id. + * + * @covers ::consume_outcome + * + * @return void + */ + public function test_consume_outcome_skips_invalid_user() { + $this->expiring_store->shouldNotReceive( 'get_for_user' ); + + $this->assertNull( $this->instance->consume_outcome( 0 ) ); + } + + /** + * Configures the expectation that the outcome is persisted once for a user. + * + * @param int $user_id The user id the outcome should be stored for. + * + * @return void + */ + private function expect_persist( int $user_id ): void { + $this->expiring_store->shouldReceive( 'persist_for_user' ) + ->once() + ->with( self::OUTCOME_KEY, Mockery::type( 'array' ), \MINUTE_IN_SECONDS, $user_id ); + } +} diff --git a/tests/Unit/MyYoast_Client/User_Interface/Integrations_Page_Script_Data_Test.php b/tests/Unit/MyYoast_Client/User_Interface/Integrations_Page_Script_Data_Test.php index bac65159049..2b9536a535f 100644 --- a/tests/Unit/MyYoast_Client/User_Interface/Integrations_Page_Script_Data_Test.php +++ b/tests/Unit/MyYoast_Client/User_Interface/Integrations_Page_Script_Data_Test.php @@ -5,6 +5,8 @@ use Brain\Monkey; use Mockery; use Yoast\WP\SEO\Conditionals\MyYoast_Connection_Conditional; +use Yoast\WP\SEO\MyYoast_Client\Application\Callback_Outcome; +use Yoast\WP\SEO\MyYoast_Client\Application\OAuth_Callback_Handler; use Yoast\WP\SEO\MyYoast_Client\User_Interface\Integrations_Page_Script_Data; use Yoast\WP\SEO\MyYoast_Client\User_Interface\Status_Presenter; use Yoast\WP\SEO\Tests\Unit\TestCase; @@ -30,6 +32,13 @@ final class Integrations_Page_Script_Data_Test extends TestCase { */ private $myyoast_connection_conditional; + /** + * The callback handler mock. + * + * @var OAuth_Callback_Handler|Mockery\MockInterface + */ + private $callback_handler; + /** * The instance under test. * @@ -47,15 +56,17 @@ protected function set_up() { $this->status_presenter = Mockery::mock( Status_Presenter::class ); $this->myyoast_connection_conditional = Mockery::mock( MyYoast_Connection_Conditional::class ); + $this->callback_handler = Mockery::mock( OAuth_Callback_Handler::class ); $this->instance = new Integrations_Page_Script_Data( $this->status_presenter, $this->myyoast_connection_conditional, + $this->callback_handler, ); } /** * Tests the payload is returned when the feature flag is enabled and no - * callback outcome transient is pending. + * callback outcome is pending. * * @covers ::__construct * @covers ::present @@ -79,9 +90,7 @@ public function test_present_when_enabled() { ->with( 'profile.php' ) ->andReturn( 'https://example.com/wp-admin/profile.php' ); Monkey\Functions\expect( 'get_current_user_id' )->andReturn( 42 ); - Monkey\Functions\expect( 'get_transient' ) - ->with( 'wpseo_myyoast_oauth_outcome_42' ) - ->andReturn( false ); + $this->callback_handler->shouldReceive( 'consume_outcome' )->once()->with( 42 )->andReturn( null ); $result = $this->instance->present(); @@ -92,14 +101,14 @@ public function test_present_when_enabled() { } /** - * Tests the callback outcome transient is read and consumed. + * Tests a successful outcome is shaped into the verify_success notification. * * @covers ::present * @covers ::consume_callback_outcome * * @return void */ - public function test_present_consumes_callback_outcome() { + public function test_present_consumes_success_outcome() { $status = [ 'is_provisioned' => true, 'is_registered' => true, @@ -115,17 +124,7 @@ public function test_present_consumes_callback_outcome() { ->with( 'profile.php' ) ->andReturn( 'https://example.com/wp-admin/profile.php' ); Monkey\Functions\expect( 'get_current_user_id' )->andReturn( 7 ); - Monkey\Functions\expect( 'get_transient' ) - ->with( 'wpseo_myyoast_oauth_outcome_7' ) - ->andReturn( - [ - 'kind' => 'success', - 'key' => 'verify_success', - ], - ); - Monkey\Functions\expect( 'delete_transient' ) - ->once() - ->with( 'wpseo_myyoast_oauth_outcome_7' ); + $this->callback_handler->shouldReceive( 'consume_outcome' )->once()->with( 7 )->andReturn( Callback_Outcome::success() ); $result = $this->instance->present(); @@ -140,13 +139,20 @@ public function test_present_consumes_callback_outcome() { } /** - * Tests a malformed transient payload is ignored. + * Tests an error outcome is translated to its front-end message key. * + * @covers ::present * @covers ::consume_callback_outcome + * @covers ::error_message_key + * + * @dataProvider provide_error_outcomes + * + * @param Callback_Outcome $outcome The stored outcome. + * @param string $expected_key The expected front-end message key. * * @return void */ - public function test_present_ignores_malformed_callback_outcome() { + public function test_present_translates_error_outcome( Callback_Outcome $outcome, string $expected_key ) { $status = [ 'is_provisioned' => true, 'is_registered' => false, @@ -161,27 +167,43 @@ public function test_present_ignores_malformed_callback_outcome() { Monkey\Functions\expect( 'admin_url' ) ->with( 'profile.php' ) ->andReturn( 'https://example.com/wp-admin/profile.php' ); - Monkey\Functions\expect( 'get_current_user_id' )->andReturn( 9 ); - Monkey\Functions\expect( 'get_transient' ) - ->with( 'wpseo_myyoast_oauth_outcome_9' ) - ->andReturn( [ 'kind' => 'success' ] ); - Monkey\Functions\expect( 'delete_transient' ) - ->once() - ->with( 'wpseo_myyoast_oauth_outcome_9' ); + Monkey\Functions\expect( 'get_current_user_id' )->andReturn( 7 ); + $this->callback_handler->shouldReceive( 'consume_outcome' )->once()->with( 7 )->andReturn( $outcome ); $result = $this->instance->present(); - $this->assertNull( $result['callbackOutcome'] ); + $this->assertSame( + [ + 'kind' => 'error', + 'key' => $expected_key, + ], + $result['callbackOutcome'], + ); + } + + /** + * Provides error outcomes and the message keys they map to. + * + * @return array + */ + public static function provide_error_outcomes(): array { + return [ + 'provider access_denied -> cancelled' => [ Callback_Outcome::provider_error( 'access_denied' ), 'connection_cancelled' ], + 'other provider error -> unexpected' => [ Callback_Outcome::provider_error( 'server_error' ), 'unexpected_error' ], + 'invalid_grant -> dedicated key' => [ Callback_Outcome::exchange_error( 'invalid_grant' ), 'token_request_failed_invalid_grant' ], + 'other exchange error -> generic key' => [ Callback_Outcome::exchange_error( 'invalid_request' ), 'token_request_failed' ], + 'code-less exchange error -> unexpected' => [ Callback_Outcome::exchange_error( null ), 'unexpected_error' ], + ]; } /** - * Tests the callback outcome is not consulted when no user is logged in. + * Tests the store still drives consumption with the resolved (zero) user id when no user is logged in. * * @covers ::consume_callback_outcome * * @return void */ - public function test_present_skips_transient_lookup_without_user() { + public function test_present_without_user() { $status = [ 'is_provisioned' => true, 'is_registered' => false, @@ -197,6 +219,7 @@ public function test_present_skips_transient_lookup_without_user() { ->with( 'profile.php' ) ->andReturn( 'https://example.com/wp-admin/profile.php' ); Monkey\Functions\expect( 'get_current_user_id' )->andReturn( 0 ); + $this->callback_handler->shouldReceive( 'consume_outcome' )->once()->with( 0 )->andReturn( null ); $result = $this->instance->present(); diff --git a/tests/Unit/MyYoast_Client/User_Interface/OAuth_Callback_Integration_Test.php b/tests/Unit/MyYoast_Client/User_Interface/OAuth_Callback_Integration_Test.php index 081a33c2818..eabe5ae1a7a 100644 --- a/tests/Unit/MyYoast_Client/User_Interface/OAuth_Callback_Integration_Test.php +++ b/tests/Unit/MyYoast_Client/User_Interface/OAuth_Callback_Integration_Test.php @@ -3,13 +3,12 @@ namespace Yoast\WP\SEO\Tests\Unit\MyYoast_Client\User_Interface; use Brain\Monkey; -use Exception; use Mockery; use Yoast\WP\SEO\Conditionals\MyYoast_Connection_Conditional; use Yoast\WP\SEO\Helpers\Redirect_Helper; use Yoast\WP\SEO\MyYoast_Client\Application\Authorization_Code_Handler; -use Yoast\WP\SEO\MyYoast_Client\Application\Exceptions\Token_Request_Failed_Exception; -use Yoast\WP\SEO\MyYoast_Client\Application\MyYoast_Client; +use Yoast\WP\SEO\MyYoast_Client\Application\Callback_Outcome; +use Yoast\WP\SEO\MyYoast_Client\Application\OAuth_Callback_Handler; use Yoast\WP\SEO\MyYoast_Client\User_Interface\OAuth_Callback_Integration; use Yoast\WP\SEO\Tests\Unit\TestCase; @@ -20,15 +19,17 @@ */ final class OAuth_Callback_Integration_Test extends TestCase { - private const FALLBACK_URL = 'https://example.com/wp-admin/admin.php?page=wpseo_integrations'; - private const RETURN_URL = 'https://example.com/wp-admin/admin.php?page=wpseo_dashboard'; + // The fallback mirrors General_Page_Integration::PAGE; the return URL is a distinct + // stored page so the success path proves a stored URL is honored over the fallback. + private const FALLBACK_URL = 'https://example.com/wp-admin/admin.php?page=wpseo_dashboard'; + private const RETURN_URL = 'https://example.com/wp-admin/admin.php?page=wpseo_integrations'; /** - * The MyYoast client mock. + * The callback handler mock. * - * @var MyYoast_Client|Mockery\MockInterface + * @var OAuth_Callback_Handler|Mockery\MockInterface */ - private $myyoast_client; + private $callback_handler; /** * The authorization code handler mock. @@ -59,12 +60,12 @@ final class OAuth_Callback_Integration_Test extends TestCase { protected function set_up() { parent::set_up(); - $this->myyoast_client = Mockery::mock( MyYoast_Client::class ); + $this->callback_handler = Mockery::mock( OAuth_Callback_Handler::class ); $this->auth_code_handler = Mockery::mock( Authorization_Code_Handler::class ); $this->redirect_helper = Mockery::mock( Redirect_Helper::class ); $this->instance = new OAuth_Callback_Integration( - $this->myyoast_client, + $this->callback_handler, $this->auth_code_handler, $this->redirect_helper, ); @@ -112,7 +113,11 @@ public function test_get_conditionals() { } /** - * Tests the admin-post hook is registered and the redirect URI is pointed at the callback endpoint. + * Tests the admin-post callback hook is registered. + * + * The redirect URI no longer needs filtering here: the redirect-URI provider + * defaults to this endpoint's URL directly, so the integration only wires the + * `admin_post_*` handler. * * @covers ::register_hooks * @@ -123,68 +128,46 @@ public function test_register_hooks() { ->once() ->with( [ $this->instance, 'handle' ] ); - Monkey\Filters\expectAdded( 'wpseo_myyoast_redirect_uris' ) - ->once() - ->with( [ $this->instance, 'filter_redirect_uris' ] ); - - Monkey\Filters\expectAdded( 'wpseo_myyoast_authorization_redirect_uri' ) - ->once() - ->with( [ $this->instance, 'filter_authorization_redirect_uri' ] ); - $this->instance->register_hooks(); } /** - * Tests the redirect-URI filters replace the defaults with the dedicated callback endpoint. + * Tests the callback endpoint URL points at the dedicated admin-post action. * - * @covers ::filter_redirect_uris - * @covers ::filter_authorization_redirect_uri * @covers ::get_callback_url * * @return void */ - public function test_redirect_uri_filters_use_the_callback_endpoint() { + public function test_get_callback_url_points_at_admin_post_action() { $callback_url = 'https://example.com/wp-admin/admin-post.php?action=yoast_myyoast_oauth_callback'; Monkey\Functions\expect( 'get_admin_url' ) ->with( null, 'admin-post.php?action=yoast_myyoast_oauth_callback' ) ->andReturn( $callback_url ); - $this->assertSame( [ $callback_url ], $this->instance->filter_redirect_uris( [ 'https://default/cb' ] ) ); - $this->assertSame( $callback_url, $this->instance->filter_authorization_redirect_uri( 'https://default/cb' ) ); + $this->assertSame( $callback_url, OAuth_Callback_Integration::get_callback_url() ); } /** - * Tests the happy path: code + state exchange succeeds, success transient is set. + * Tests the extracted callback parameters are passed to the handler and the user is redirected back. * * @covers ::__construct * @covers ::handle * @covers ::resolve_return_url * @covers ::read_query_arg - * @covers ::set_outcome * * @return void */ - public function test_handle_exchanges_code_on_success() { + public function test_handle_drives_callback_handler_with_extracted_params() { $_GET = [ 'code' => 'abc', 'state' => 'xyz', + 'error' => '', ]; $this->expect_user( 42 ); $this->expect_return_url_lookup( 42, self::RETURN_URL ); - - $this->myyoast_client->shouldReceive( 'exchange_authorization_code' ) - ->once() - ->with( 42, 'abc', 'xyz' ); - - $this->expect_transient_set( - 42, - [ - 'kind' => 'success', - 'key' => 'verify_success', - ], - ); + $this->expect_callback( 42, 'abc', 'xyz', '', Callback_Outcome::success() ); $this->expect_redirect( self::RETURN_URL ); @@ -192,123 +175,19 @@ public function test_handle_exchanges_code_on_success() { } /** - * Tests `error=access_denied` translates to a `connection_cancelled` transient and discards flow state. + * Tests a provider error is forwarded verbatim to the handler before redirecting. * * @covers ::handle + * @covers ::read_query_arg * * @return void */ - public function test_handle_access_denied_sets_cancelled_outcome() { + public function test_handle_forwards_provider_error() { $_GET = [ 'error' => 'access_denied' ]; $this->expect_user( 7 ); $this->expect_return_url_lookup( 7, self::RETURN_URL ); - - $this->auth_code_handler->shouldReceive( 'discard_flow_state' )->once()->with( 7 ); - - $this->myyoast_client->shouldNotReceive( 'exchange_authorization_code' ); - - $this->expect_transient_set( - 7, - [ - 'kind' => 'error', - 'key' => 'connection_cancelled', - ], - ); - - $this->expect_redirect( self::RETURN_URL ); - - $this->instance->handle(); - } - - /** - * Tests other OAuth provider errors map to a generic unexpected_error outcome. - * - * @covers ::handle - * - * @return void - */ - public function test_handle_generic_oauth_error_maps_to_unexpected() { - $_GET = [ 'error' => 'server_error' ]; - - $this->expect_user( 7 ); - $this->expect_return_url_lookup( 7, self::RETURN_URL ); - - $this->auth_code_handler->shouldReceive( 'discard_flow_state' )->once()->with( 7 ); - - $this->expect_transient_set( - 7, - [ - 'kind' => 'error', - 'key' => 'unexpected_error', - ], - ); - - $this->expect_redirect( self::RETURN_URL ); - - $this->instance->handle(); - } - - /** - * Tests Token_Request_Failed_Exception with `invalid_grant` maps to its dedicated message key. - * - * @covers ::handle - * - * @return void - */ - public function test_handle_invalid_grant_exception_maps_to_dedicated_key() { - $_GET = [ - 'code' => 'abc', - 'state' => 'xyz', - ]; - - $this->expect_user( 11 ); - $this->expect_return_url_lookup( 11, self::RETURN_URL ); - - $this->myyoast_client->shouldReceive( 'exchange_authorization_code' ) - ->once() - ->andThrow( new Token_Request_Failed_Exception( 'invalid_grant', 'expired' ) ); - - $this->expect_transient_set( - 11, - [ - 'kind' => 'error', - 'key' => 'token_request_failed_invalid_grant', - ], - ); - - $this->expect_redirect( self::RETURN_URL ); - - $this->instance->handle(); - } - - /** - * Tests other Token_Request_Failed_Exception codes map to the generic token failure key. - * - * @covers ::handle - * - * @return void - */ - public function test_handle_token_request_failure_maps_to_generic_key() { - $_GET = [ - 'code' => 'abc', - 'state' => 'xyz', - ]; - - $this->expect_user( 11 ); - $this->expect_return_url_lookup( 11, self::RETURN_URL ); - - $this->myyoast_client->shouldReceive( 'exchange_authorization_code' ) - ->once() - ->andThrow( new Token_Request_Failed_Exception( 'invalid_request', 'state mismatch' ) ); - - $this->expect_transient_set( - 11, - [ - 'kind' => 'error', - 'key' => 'token_request_failed', - ], - ); + $this->expect_callback( 7, '', '', 'access_denied', Callback_Outcome::provider_error( 'access_denied' ) ); $this->expect_redirect( self::RETURN_URL ); @@ -316,53 +195,18 @@ public function test_handle_token_request_failure_maps_to_generic_key() { } /** - * Tests an unexpected exception sets an unexpected_error outcome. + * Tests the handler still drives the use-case (and redirects) for a no-op callback. * * @covers ::handle * * @return void */ - public function test_handle_unexpected_exception() { - $_GET = [ - 'code' => 'abc', - 'state' => 'xyz', - ]; - - $this->expect_user( 11 ); - $this->expect_return_url_lookup( 11, self::RETURN_URL ); - - $this->myyoast_client->shouldReceive( 'exchange_authorization_code' ) - ->once() - ->andThrow( new Exception( 'boom' ) ); - - $this->expect_transient_set( - 11, - [ - 'kind' => 'error', - 'key' => 'unexpected_error', - ], - ); - - $this->expect_redirect( self::RETURN_URL ); - - $this->instance->handle(); - } - - /** - * Tests visiting the callback URL without code/state/error just redirects to the return URL. - * - * @covers ::handle - * - * @return void - */ - public function test_handle_missing_params_redirects_without_outcome() { + public function test_handle_no_op_redirects() { $_GET = []; $this->expect_user( 42 ); $this->expect_return_url_lookup( 42, self::RETURN_URL ); - - $this->myyoast_client->shouldNotReceive( 'exchange_authorization_code' ); - Monkey\Functions\expect( 'set_transient' )->never(); + $this->expect_callback( 42, '', '', '', Callback_Outcome::no_op() ); $this->expect_redirect( self::RETURN_URL ); @@ -382,6 +226,7 @@ public function test_handle_falls_back_to_integrations_page_when_no_return_url_s $this->expect_user( 42 ); $this->auth_code_handler->shouldReceive( 'get_return_url' )->once()->with( 42 )->andReturn( null ); + $this->expect_callback( 42, '', '', '', Callback_Outcome::no_op() ); $this->expect_redirect( self::FALLBACK_URL ); @@ -389,7 +234,7 @@ public function test_handle_falls_back_to_integrations_page_when_no_return_url_s } /** - * Tests anonymous requests are redirected to the fallback without any transient activity. + * Tests anonymous requests are redirected to the fallback without driving the handler. * * @covers ::handle * @@ -403,8 +248,7 @@ public function test_handle_anonymous_request_redirects_to_fallback() { $this->expect_user( 0 ); - $this->myyoast_client->shouldNotReceive( 'exchange_authorization_code' ); - Monkey\Functions\expect( 'set_transient' )->never(); + $this->callback_handler->shouldNotReceive( 'handle' ); $this->expect_redirect( self::FALLBACK_URL ); @@ -435,17 +279,21 @@ private function expect_return_url_lookup( int $user_id, string $url ): void { } /** - * Configures the expected transient write. + * Configures the expected delegation to the callback handler. * - * @param int $user_id The user id. - * @param array{kind: string, key: string} $value The expected stored value. + * @param int $user_id The user id. + * @param string $code The expected authorization code argument. + * @param string $state The expected state argument. + * @param string $error The expected provider error argument. + * @param Callback_Outcome $outcome The outcome to return. * * @return void */ - private function expect_transient_set( int $user_id, array $value ): void { - Monkey\Functions\expect( 'set_transient' ) + private function expect_callback( int $user_id, string $code, string $state, string $error, Callback_Outcome $outcome ): void { + $this->callback_handler->shouldReceive( 'handle' ) ->once() - ->with( 'wpseo_myyoast_oauth_outcome_' . $user_id, $value, \MINUTE_IN_SECONDS ); + ->with( $user_id, $code, $state, $error ) + ->andReturn( $outcome ); } /** From 104de1eb37bd3aba53487836e60061180251728a Mon Sep 17 00:00:00 2001 From: Diede Exterkate <5352634+diedexx@users.noreply.github.com> Date: Fri, 19 Jun 2026 17:03:54 +0200 Subject: [PATCH 07/26] feat(myyoast-client): throttle status refreshes and let callers set the OAuth return URL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two additions to the MyYoast management route: Refresh throttle — a successful upstream status refresh now sets a short-lived, issuer-scoped marker that suppresses further upstream reads for an hour, so the integrations page reloading does not hit MyYoast's aggressive RFC 7592 rate limit. Only the fact that we checked is stored, never the response body, so the endpoint's no-store contract holds. Connect, re-sync, and disconnect clear the marker so a deliberate state change is reflected on the next read. A failed or rate-limited attempt does not set the marker, leaving the next retry free. Caller-supplied return URL — POST /myyoast/authorize accepts an optional return_url, since the flow can be started from admin pages other than the integrations page. It is validated against the site's own host and dropped when off-site or invalid (the callback re-validates before redirecting, so this is the first of two open-redirect gates). When absent, null flows through and the callback surfaces a standalone outcome instead of redirecting. Also narrows the over-broad Throwable catches to the exception types each endpoint can actually raise, and renames respond_with_status to respond_with_connection_status for clarity. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../js/src/integrations-page/myyoast-store.js | 16 +- .../user-interface/management-route.php | 133 +++++++++-- .../User_Interface/Management_Route_Test.php | 223 ++++++++++++++++-- 3 files changed, 330 insertions(+), 42 deletions(-) diff --git a/packages/js/src/integrations-page/myyoast-store.js b/packages/js/src/integrations-page/myyoast-store.js index af5a39ecd4f..d19f8d82108 100644 --- a/packages/js/src/integrations-page/myyoast-store.js +++ b/packages/js/src/integrations-page/myyoast-store.js @@ -137,17 +137,23 @@ const disconnectMyyoastConnection = createMyyoastAction( "disconnect" ); * Starts the authorization-code flow for the site's registration. * * The backend resolves which registered redirect URI to use, so no URI is - * sent. On success it returns an `authorize_url` the browser should be - * navigated to; the caller decides how to do that. On failure the slice's - * actionError is set, mirroring the other actions. + * sent. The optional `returnUrl` tells the backend where to send the browser + * once the flow completes — pass the page the flow was started from, since the + * flow can be kicked off from several admin pages. It is validated server-side + * and ignored when off-site or invalid. On success the action returns an + * `authorize_url` the browser should be navigated to; the caller decides how to + * do that. On failure the slice's actionError is set, mirroring the other actions. * + * @param {string} [returnUrl] The URL to return to after the flow completes. * @returns {GeneratorFunction} The generator action. */ // eslint-disable-next-line complexity -const authorizeMyyoastSite = function* () { +const authorizeMyyoastSite = function* ( returnUrl ) { yield startMyyoastAction( "authorize" ); try { - const payload = yield{ type: ENDPOINTS.authorize.actionType, payload: {} }; + // eslint-disable-next-line camelcase -- snake_case matches the REST endpoint's request contract. + const body = returnUrl ? { return_url: returnUrl } : {}; + const payload = yield{ type: ENDPOINTS.authorize.actionType, payload: body }; if ( payload?.status ) { yield setMyyoastStatus( transformStatus( payload.status ) ); } diff --git a/src/myyoast-client/user-interface/management-route.php b/src/myyoast-client/user-interface/management-route.php index a34594479a7..9c127a308b4 100644 --- a/src/myyoast-client/user-interface/management-route.php +++ b/src/myyoast-client/user-interface/management-route.php @@ -5,9 +5,9 @@ namespace Yoast\WP\SEO\MyYoast_Client\User_Interface; use Throwable; +use WP_REST_Request; use WP_REST_Response; use Yoast\WP\SEO\Conditionals\MyYoast_Connection_Conditional; -use Yoast\WP\SEO\Integrations\Admin\Integrations_Page; use Yoast\WP\SEO\Main; use Yoast\WP\SEO\MyYoast_Client\Application\Exceptions\Authorization_Flow_Exception; use Yoast\WP\SEO\MyYoast_Client\Application\Exceptions\Discovery_Failed_Exception; @@ -45,6 +45,25 @@ class Management_Route implements Route_Interface, LoggerAwareInterface { public const REGISTRATION_ROUTE = '/registration'; public const AUTHORIZE_ROUTE = '/authorize'; + /** + * How long a successful upstream status refresh suppresses further upstream + * calls, in seconds. The integrations page auto-refreshes on every load, and + * MyYoast rate-limits the RFC 7592 read aggressively, so we throttle our own + * calls. This caches no response data — only the fact that we checked — so it + * does not conflict with the endpoint's no-store header. + * + * @var int + */ + private const REFRESH_THROTTLE_TTL_IN_SECONDS = \HOUR_IN_SECONDS; + + /** + * Transient key prefix for the refresh throttle marker. Suffixed with the + * issuer key so switching issuers does not carry the marker across. + * + * @var string + */ + private const REFRESH_THROTTLE_TRANSIENT_PREFIX = 'wpseo_myyoast_refresh_throttle_'; + /** * The MyYoast client facade. * @@ -165,6 +184,14 @@ public function register_routes() { 'methods' => 'POST', 'callback' => [ $this, 'authorize' ], 'permission_callback' => $permission_callback, + 'args' => [ + 'return_url' => [ + 'type' => 'string', + 'required' => false, + 'description' => 'URL to send the browser back to once the flow completes. Validated against the site host; an invalid or off-site URL is ignored.', + 'sanitize_callback' => 'esc_url_raw', + ], + ], ], ); } @@ -184,22 +211,34 @@ public function can_manage() { * @return WP_REST_Response */ public function get_status() { - return $this->respond_with_status( 200, null ); + return $this->respond_with_connection_status( 200, null ); } /** * POST /myyoast/refresh-status — refreshes the registration status against the server. * + * Throttled: a successful upstream refresh suppresses further upstream calls + * for an hour. Within that window the call is skipped and the locally-derived + * status is returned unchanged, so a page reload does not hit MyYoast's rate + * limit. The upstream response body is never stored — only the throttle marker. + * * @return WP_REST_Response */ public function refresh_status() { + if ( \get_transient( $this->get_refresh_throttle_key() ) !== false ) { + return $this->respond_with_connection_status( 200, null ); + } + try { $this->myyoast_client->refresh_registration_status(); - } catch ( Throwable $e ) { + } catch ( Registration_Failed_Exception $e ) { return $this->handle_exception( $e ); } - return $this->respond_with_status( 200, null ); + // Mark only on success: a failed or rate-limited attempt must not suppress the next retry. + \set_transient( $this->get_refresh_throttle_key(), 1, self::REFRESH_THROTTLE_TTL_IN_SECONDS ); + + return $this->respond_with_connection_status( 200, null ); } /** @@ -215,11 +254,13 @@ public function register() { try { $this->myyoast_client->ensure_registered(); - } catch ( Throwable $e ) { + } catch ( Registration_Failed_Exception $e ) { return $this->handle_exception( $e ); } - return $this->respond_with_status( 200, 'connect_success' ); + $this->clear_refresh_throttle(); + + return $this->respond_with_connection_status( 200, 'connect_success' ); } /** @@ -243,7 +284,9 @@ public function update_registration() { return $this->handle_exception( $e ); } - return $this->respond_with_status( 200, 'update_success' ); + $this->clear_refresh_throttle(); + + return $this->respond_with_connection_status( 200, 'update_success' ); } /** @@ -255,14 +298,17 @@ public function update_registration() { * the redirect URI itself, and the authorization-code handler marks it * validated once the returning code is exchanged. * + * The optional `return_url` is where the browser is sent once the flow + * completes; the caller supplies it because the flow can be started from + * different admin pages. It is validated against the site's own host, so an + * off-site or tampered value is dropped (and the callback then surfaces a + * standalone outcome rather than redirecting anywhere). + * + * @param WP_REST_Request $request The REST request. + * * @return WP_REST_Response */ - public function authorize(): WP_REST_Response { - $gate = $this->require_provisioned(); - if ( $gate !== null ) { - return $gate; - } - + public function authorize( WP_REST_Request $request ): WP_REST_Response { if ( $this->client_registration->get_registered_client() === null ) { return $this->error_response( 'registration_gone' ); } @@ -272,7 +318,7 @@ public function authorize(): WP_REST_Response { return $this->error_response( 'invalid_user', null, 401 ); } - $return_url = \admin_url( 'admin.php?page=' . Integrations_Page::PAGE ); + $return_url = $this->resolve_return_url( $request->get_param( 'return_url' ) ); try { $authorize_url = $this->myyoast_client->get_authorization_url( @@ -283,7 +329,7 @@ public function authorize(): WP_REST_Response { ); } catch ( Authorization_Flow_Exception $e ) { return $this->error_response( 'registration_failed', $e ); - } catch ( Throwable $e ) { + } catch ( Invalid_Resource_Exception $e ) { return $this->handle_exception( $e ); } @@ -325,7 +371,34 @@ public function deregister() { $this->logger->warning( 'MyYoast server-side deregistration was not confirmed; the site was disconnected locally.' ); } - return $this->respond_with_status( 200, 'disconnect_success' ); + $this->clear_refresh_throttle(); + + return $this->respond_with_connection_status( 200, 'disconnect_success' ); + } + + /** + * Validates a caller-supplied return URL against the site's own host. + * + * The return URL is optional: callers that have nowhere meaningful to send + * the user back to omit it. Anything off-site or otherwise invalid is treated + * as absent rather than rewritten to a default — `wp_validate_redirect()` with + * an empty fallback yields an empty string, which we normalize to null. The + * callback re-validates the stored value before redirecting, so this is the + * first of two gates against an open redirect. + * + * @param string|null $return_url The sanitized `return_url` request parameter (the route's + * args schema coerces it to a string; absent when not sent). + * + * @return string|null The validated same-host URL, or null when none applies. + */ + private function resolve_return_url( ?string $return_url ): ?string { + if ( $return_url === null || $return_url === '' ) { + return null; + } + + $validated = \wp_validate_redirect( $return_url, '' ); + + return ( $validated === '' ) ? null : $validated; } /** @@ -407,6 +480,32 @@ private function handle_exception( Throwable $exception ): WP_REST_Response { return $this->error_response( 'unexpected_error', $exception ); } + /** + * Returns the issuer-scoped transient key for the refresh throttle marker. + * + * @return string The transient key. + */ + private function get_refresh_throttle_key(): string { + return \sprintf( + '%s_%s', + \rtrim( self::REFRESH_THROTTLE_TRANSIENT_PREFIX, '_' ), + $this->issuer_config->get_issuer_key(), + ); + } + + /** + * Clears the refresh throttle marker so the next status read hits the server. + * + * Called after any endpoint that changes the registration (connect, re-sync, + * disconnect): the throttle exists only to spare MyYoast's rate limit on + * unchanged status, so a deliberate state change must invalidate it. + * + * @return void + */ + private function clear_refresh_throttle(): void { + \delete_transient( $this->get_refresh_throttle_key() ); + } + /** * Builds a successful response carrying the refreshed status payload. * @@ -415,7 +514,7 @@ private function handle_exception( Throwable $exception ): WP_REST_Response { * * @return WP_REST_Response */ - private function respond_with_status( int $status, ?string $message_key ): WP_REST_Response { + private function respond_with_connection_status( int $status, ?string $message_key ): WP_REST_Response { $body = [ 'status' => $this->status_presenter->present(), ]; diff --git a/tests/Unit/MyYoast_Client/User_Interface/Management_Route_Test.php b/tests/Unit/MyYoast_Client/User_Interface/Management_Route_Test.php index e0914ac1d3f..29aa2f311fa 100644 --- a/tests/Unit/MyYoast_Client/User_Interface/Management_Route_Test.php +++ b/tests/Unit/MyYoast_Client/User_Interface/Management_Route_Test.php @@ -5,6 +5,7 @@ use Brain\Monkey; use Exception; use Mockery; +use WP_REST_Request; use WP_REST_Response; use Yoast\WP\SEO\Conditionals\MyYoast_Connection_Conditional; use Yoast\WP\SEO\MyYoast_Client\Application\Exceptions\Authorization_Flow_Exception; @@ -89,6 +90,13 @@ protected function set_up() { $this->issuer_config, $this->client_registration, ); + + // Default throttle stubs: the issuer key is always available and the marker + // is absent (cache miss). Writes/deletes are asserted per-test with + // Functions\expect(); a Functions\when() default here would shadow those + // expectations, so it is intentionally omitted. + $this->issuer_config->shouldReceive( 'get_issuer_key' )->andReturn( 'abcd1234' )->byDefault(); + Monkey\Functions\when( 'get_transient' )->justReturn( false ); } /** @@ -111,6 +119,20 @@ private function unprovision(): void { $this->issuer_config->shouldReceive( 'get_initial_access_token' )->andReturn( '' ); } + /** + * Builds a REST request mock whose `return_url` param returns the given value. + * + * @param string|null $return_url The value `get_param( 'return_url' )` should return. + * + * @return WP_REST_Request|Mockery\MockInterface + */ + private function request_with_return_url( ?string $return_url ) { + $request = Mockery::mock( WP_REST_Request::class ); + $request->shouldReceive( 'get_param' )->with( 'return_url' )->andReturn( $return_url ); + + return $request; + } + /** * Returns the default status payload used by mocked presenter calls. * @@ -197,9 +219,11 @@ public function test_get_status_returns_payload() { } /** - * Tests that refresh_status dispatches to the facade and returns a success payload. + * Tests that refresh_status dispatches to the facade, sets the throttle + * marker, and returns a success payload. * * @covers ::refresh_status + * @covers ::get_refresh_throttle_key * * @return void */ @@ -208,6 +232,58 @@ public function test_refresh_status_success() { $this->myyoast_client->shouldReceive( 'refresh_registration_status' )->once()->andReturn( [] ); $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + Monkey\Functions\expect( 'set_transient' ) + ->once() + ->with( 'wpseo_myyoast_refresh_throttle_abcd1234', 1, \HOUR_IN_SECONDS ); + + Mockery::mock( 'overload:' . WP_REST_Response::class ); + + $response = $this->instance->refresh_status(); + + $this->assertInstanceOf( WP_REST_Response::class, $response ); + } + + /** + * Tests that refresh_status skips the upstream call when the throttle marker + * is present, returning the local status without touching the facade. + * + * @covers ::refresh_status + * @covers ::get_refresh_throttle_key + * + * @return void + */ + public function test_refresh_status_skips_when_throttled() { + $this->provision(); + Monkey\Functions\when( 'get_transient' )->justReturn( 1 ); + $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + + $this->myyoast_client->shouldNotReceive( 'refresh_registration_status' ); + Monkey\Functions\expect( 'set_transient' )->never(); + + Mockery::mock( 'overload:' . WP_REST_Response::class ); + + $response = $this->instance->refresh_status(); + + $this->assertInstanceOf( WP_REST_Response::class, $response ); + } + + /** + * Tests that a failed upstream refresh does not set the throttle marker, so + * the next load retries. + * + * @covers ::refresh_status + * + * @return void + */ + public function test_refresh_status_does_not_throttle_on_failure() { + $this->provision(); + $this->myyoast_client->shouldReceive( 'refresh_registration_status' ) + ->once() + ->andThrow( new Rate_Limited_Exception( 'slow down' ) ); + $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + + Monkey\Functions\expect( 'set_transient' )->never(); + Mockery::mock( 'overload:' . WP_REST_Response::class ); $response = $this->instance->refresh_status(); @@ -301,8 +377,12 @@ static function ( $body ) use ( &$captured ): void { } /** - * Tests that refresh_status maps each domain exception to a response (smoke test - * — exception-mapping coverage is per-branch but groups into one test). + * Tests that handle_exception maps each domain exception to a response (smoke + * test — exception-mapping coverage is per-branch but groups into one test). + * + * Driven through update_registration, which catches the full Throwable family; + * refresh_status only catches Registration_Failed_Exception (its declared + * throw type), so it cannot reach the sibling branches exercised here. * * @covers ::handle_exception * @@ -311,6 +391,7 @@ static function ( $body ) use ( &$captured ): void { public function test_handle_exception_branches() { $this->provision(); $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + Monkey\Functions\expect( 'delete_transient' )->never(); Mockery::mock( 'overload:' . WP_REST_Response::class ); @@ -326,11 +407,11 @@ public function test_handle_exception_branches() { ]; foreach ( $exceptions as $exception ) { - $this->myyoast_client->shouldReceive( 'refresh_registration_status' ) + $this->myyoast_client->shouldReceive( 'ensure_registered' ) ->once() ->andThrow( $exception ); - $response = $this->instance->refresh_status(); + $response = $this->instance->update_registration(); $this->assertInstanceOf( WP_REST_Response::class, $response ); } @@ -351,6 +432,10 @@ public function test_register_delegates_to_ensure_registered() { $this->myyoast_client->shouldReceive( 'ensure_registered' )->once()->withNoArgs(); $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + Monkey\Functions\expect( 'delete_transient' ) + ->once() + ->with( 'wpseo_myyoast_refresh_throttle_abcd1234' ); + Mockery::mock( 'overload:' . WP_REST_Response::class ); $response = $this->instance->register(); @@ -371,6 +456,10 @@ public function test_update_registration() { $this->myyoast_client->shouldReceive( 'ensure_registered' )->once()->withNoArgs(); $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + Monkey\Functions\expect( 'delete_transient' ) + ->once() + ->with( 'wpseo_myyoast_refresh_throttle_abcd1234' ); + Mockery::mock( 'overload:' . WP_REST_Response::class ); $response = $this->instance->update_registration(); @@ -415,6 +504,10 @@ public function test_deregister_success() { $this->myyoast_client->shouldReceive( 'clear_all_site_tokens' )->once(); $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + Monkey\Functions\expect( 'delete_transient' ) + ->once() + ->with( 'wpseo_myyoast_refresh_throttle_abcd1234' ); + Mockery::mock( 'overload:' . WP_REST_Response::class ); $response = $this->instance->deregister(); @@ -436,6 +529,7 @@ public function test_deregister_succeeds_locally_when_remote_unconfirmed() { $this->myyoast_client->shouldReceive( 'deregister' )->once()->andReturn( false ); $this->myyoast_client->shouldReceive( 'clear_all_site_tokens' )->once(); $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + Monkey\Functions\expect( 'delete_transient' )->zeroOrMoreTimes(); Mockery::mock( 'overload:' . WP_REST_Response::class ); @@ -458,6 +552,7 @@ public function test_deregister_clears_tokens_when_remote_throws() { $this->myyoast_client->shouldReceive( 'deregister' )->once()->andThrow( new Exception( 'boom' ) ); $this->myyoast_client->shouldReceive( 'clear_all_site_tokens' )->once(); $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + Monkey\Functions\expect( 'delete_transient' )->zeroOrMoreTimes(); Mockery::mock( 'overload:' . WP_REST_Response::class ); @@ -470,9 +565,11 @@ public function test_deregister_clears_tokens_when_remote_throws() { * Tests the registration-mutating endpoints short-circuit when the plugin is * not provisioned. * - * Only register/update/authorize require the software statement and initial - * access token; refresh-status and deregister work without them and are covered by - * their own tests. + * The register/update endpoints gate on provisioning; authorize gates on + * having a registered client (a stricter precondition that also implies + * provisioning), so with no client stubbed it short-circuits before touching + * the facade too. The refresh-status and deregister endpoints work without + * provisioning and are covered by their own tests. * * @covers ::require_provisioned * @@ -480,6 +577,7 @@ public function test_deregister_clears_tokens_when_remote_throws() { */ public function test_registration_actions_blocked_when_not_provisioned() { $this->unprovision(); + $this->client_registration->shouldReceive( 'get_registered_client' )->andReturn( null ); $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); $this->myyoast_client->shouldNotReceive( 'ensure_registered' ); @@ -489,7 +587,7 @@ public function test_registration_actions_blocked_when_not_provisioned() { $this->assertInstanceOf( WP_REST_Response::class, $this->instance->register() ); $this->assertInstanceOf( WP_REST_Response::class, $this->instance->update_registration() ); - $this->assertInstanceOf( WP_REST_Response::class, $this->instance->authorize() ); + $this->assertInstanceOf( WP_REST_Response::class, $this->instance->authorize( $this->request_with_return_url( null ) ) ); } /** @@ -504,6 +602,7 @@ public function test_refresh_status_works_when_not_provisioned() { $this->unprovision(); $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); $this->myyoast_client->shouldReceive( 'refresh_registration_status' )->once(); + Monkey\Functions\expect( 'set_transient' )->zeroOrMoreTimes(); Mockery::mock( 'overload:' . WP_REST_Response::class ); @@ -523,6 +622,7 @@ public function test_deregister_works_when_not_provisioned() { $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); $this->myyoast_client->shouldReceive( 'deregister' )->once()->andReturn( true ); $this->myyoast_client->shouldReceive( 'clear_all_site_tokens' )->once(); + Monkey\Functions\expect( 'delete_transient' )->zeroOrMoreTimes(); Mockery::mock( 'overload:' . WP_REST_Response::class ); @@ -530,13 +630,14 @@ public function test_deregister_works_when_not_provisioned() { } /** - * Tests authorize returns the authorization URL when the site is registered. + * Tests authorize passes a valid same-host return URL through to the facade. * * @covers ::authorize + * @covers ::resolve_return_url * * @return void */ - public function test_authorize_returns_authorization_url() { + public function test_authorize_passes_valid_return_url() { $this->provision(); $return_url = 'https://example.com/wp-admin/admin.php?page=wpseo_integrations'; @@ -554,8 +655,9 @@ public function test_authorize_returns_authorization_url() { Monkey\Functions\expect( 'get_current_user_id' )->andReturn( 42 ); - Monkey\Functions\expect( 'admin_url' ) - ->with( 'admin.php?page=wpseo_integrations' ) + Monkey\Functions\expect( 'wp_validate_redirect' ) + ->once() + ->with( $return_url, '' ) ->andReturn( $return_url ); $this->myyoast_client->shouldReceive( 'get_authorization_url' ) @@ -567,7 +669,92 @@ public function test_authorize_returns_authorization_url() { Mockery::mock( 'overload:' . WP_REST_Response::class ); - $response = $this->instance->authorize(); + $response = $this->instance->authorize( $this->request_with_return_url( $return_url ) ); + + $this->assertInstanceOf( WP_REST_Response::class, $response ); + } + + /** + * Tests authorize passes null to the facade when no return URL is supplied, + * so the callback later surfaces a standalone outcome instead of redirecting. + * + * @covers ::authorize + * @covers ::resolve_return_url + * + * @return void + */ + public function test_authorize_passes_null_when_return_url_absent() { + $this->provision(); + + $this->client_registration->shouldReceive( 'get_registered_client' ) + ->andReturn( + new Registered_Client( + 'client-123', + 'rat', + 'https://my.yoast.com/clients/client-123', + [ 'redirect_uris' => [ 'https://example.com/cb' ] ], + ), + ); + + Monkey\Functions\expect( 'get_current_user_id' )->andReturn( 42 ); + Monkey\Functions\expect( 'wp_validate_redirect' )->never(); + + $this->myyoast_client->shouldReceive( 'get_authorization_url' ) + ->once() + ->with( 42, [ 'openid' ], null, null ) + ->andReturn( 'https://my.yoast.com/auth?code_challenge=abc' ); + + $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + + Mockery::mock( 'overload:' . WP_REST_Response::class ); + + $response = $this->instance->authorize( $this->request_with_return_url( null ) ); + + $this->assertInstanceOf( WP_REST_Response::class, $response ); + } + + /** + * Tests authorize drops an off-site return URL to null rather than forwarding + * it, closing the open-redirect surface at the route. + * + * @covers ::authorize + * @covers ::resolve_return_url + * + * @return void + */ + public function test_authorize_drops_offsite_return_url() { + $this->provision(); + + $evil_url = 'https://evil.example.org/phish'; + + $this->client_registration->shouldReceive( 'get_registered_client' ) + ->andReturn( + new Registered_Client( + 'client-123', + 'rat', + 'https://my.yoast.com/clients/client-123', + [ 'redirect_uris' => [ 'https://example.com/cb' ] ], + ), + ); + + Monkey\Functions\expect( 'get_current_user_id' )->andReturn( 42 ); + + // wp_validate_redirect returns the (empty) fallback for an off-site URL. + Monkey\Functions\expect( 'wp_validate_redirect' ) + ->once() + ->with( $evil_url, '' ) + ->andReturn( '' ); + + $this->myyoast_client->shouldReceive( 'get_authorization_url' ) + ->once() + ->with( 42, [ 'openid' ], null, null ) + ->andReturn( 'https://my.yoast.com/auth?code_challenge=abc' ); + + $this->status_presenter->shouldReceive( 'present' )->andReturn( $this->status_payload() ); + + Mockery::mock( 'overload:' . WP_REST_Response::class ); + + $response = $this->instance->authorize( $this->request_with_return_url( $evil_url ) ); $this->assertInstanceOf( WP_REST_Response::class, $response ); } @@ -588,7 +775,7 @@ public function test_authorize_when_not_registered() { Mockery::mock( 'overload:' . WP_REST_Response::class ); - $this->assertInstanceOf( WP_REST_Response::class, $this->instance->authorize() ); + $this->assertInstanceOf( WP_REST_Response::class, $this->instance->authorize( $this->request_with_return_url( null ) ) ); } /** @@ -613,10 +800,6 @@ public function test_authorize_handles_flow_exception() { Monkey\Functions\expect( 'get_current_user_id' )->andReturn( 42 ); - Monkey\Functions\expect( 'admin_url' ) - ->with( 'admin.php?page=wpseo_integrations' ) - ->andReturn( 'https://example.com/wp-admin/admin.php?page=wpseo_integrations' ); - $this->myyoast_client->shouldReceive( 'get_authorization_url' ) ->andThrow( new Authorization_Flow_Exception( 'registration_failed', 'boom' ) ); @@ -624,6 +807,6 @@ public function test_authorize_handles_flow_exception() { Mockery::mock( 'overload:' . WP_REST_Response::class ); - $this->assertInstanceOf( WP_REST_Response::class, $this->instance->authorize() ); + $this->assertInstanceOf( WP_REST_Response::class, $this->instance->authorize( $this->request_with_return_url( null ) ) ); } } From 064f31d9aafc08ec42169e841f7504e6970815b9 Mon Sep 17 00:00:00 2001 From: Diede Exterkate <5352634+diedexx@users.noreply.github.com> Date: Mon, 22 Jun 2026 16:50:51 +0200 Subject: [PATCH 08/26] refactor(integrations-page): move the MyYoast connection into a feature folder Restructure the MyYoast connection from loose files plus a flat `myyoast-store.js` into a self-contained feature folder, matching the `ai-generator` and `introductions` layout where a feature directory is named for the store it owns. - Split the store into a `store/` directory: a `myyoast-connection` slice module and an `index.js` that builds and registers it. - Register through `createReduxStore` + `register` rather than the deprecated `registerStore`. - Centralize the store name in `constants.js` so the store and its consumers share one address without a circular import. - Add unit tests for the slice, generator actions, controls-driven branches, selectors, and the status transform. - Surface backend message codes through a `messageFor` switch so only the matched string is translated at call time. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/js/src/integrations-page.js | 2 +- .../myyoast-connection/constants.js | 9 + .../myyoast-disconnect-modal.js | 0 .../myyoast-integration.js | 104 +++--- .../myyoast-connection/store/index.js | 70 ++++ .../store/myyoast-connection.js} | 41 +-- .../recommended-integrations.js | 2 +- .../store/myyoast-connection.test.js | 306 ++++++++++++++++++ 8 files changed, 452 insertions(+), 82 deletions(-) create mode 100644 packages/js/src/integrations-page/myyoast-connection/constants.js rename packages/js/src/integrations-page/{ => myyoast-connection}/myyoast-disconnect-modal.js (100%) rename packages/js/src/integrations-page/{ => myyoast-connection}/myyoast-integration.js (77%) create mode 100644 packages/js/src/integrations-page/myyoast-connection/store/index.js rename packages/js/src/integrations-page/{myyoast-store.js => myyoast-connection/store/myyoast-connection.js} (86%) create mode 100644 packages/js/tests/integrations-page/myyoast-connection/store/myyoast-connection.test.js diff --git a/packages/js/src/integrations-page.js b/packages/js/src/integrations-page.js index fb73f958a8d..db07f1e13a2 100644 --- a/packages/js/src/integrations-page.js +++ b/packages/js/src/integrations-page.js @@ -3,7 +3,7 @@ import domReady from "@wordpress/dom-ready"; import { Root } from "@yoast/ui-library"; import IntegrationsGrid from "./integrations-page/integrations-grid"; -import { registerMyyoastStore } from "./integrations-page/myyoast-store"; +import { registerMyyoastStore } from "./integrations-page/myyoast-connection/store"; import { registerReactComponent, renderReactRoot } from "./helpers/reactRoot"; window.YoastSEO = window.YoastSEO || {}; diff --git a/packages/js/src/integrations-page/myyoast-connection/constants.js b/packages/js/src/integrations-page/myyoast-connection/constants.js new file mode 100644 index 00000000000..7c4061c974e --- /dev/null +++ b/packages/js/src/integrations-page/myyoast-connection/constants.js @@ -0,0 +1,9 @@ +/** + * Keep constants centralized to avoid circular dependency problems. + */ + +/** + * The Redux store name of the MyYoast connection. + * @type {string} + */ +export const MYYOAST_STORE_NAME = "yoast-seo/myyoast-connection"; diff --git a/packages/js/src/integrations-page/myyoast-disconnect-modal.js b/packages/js/src/integrations-page/myyoast-connection/myyoast-disconnect-modal.js similarity index 100% rename from packages/js/src/integrations-page/myyoast-disconnect-modal.js rename to packages/js/src/integrations-page/myyoast-connection/myyoast-disconnect-modal.js diff --git a/packages/js/src/integrations-page/myyoast-integration.js b/packages/js/src/integrations-page/myyoast-connection/myyoast-integration.js similarity index 77% rename from packages/js/src/integrations-page/myyoast-integration.js rename to packages/js/src/integrations-page/myyoast-connection/myyoast-integration.js index bfd80a1278f..d6fc3f0c5ef 100644 --- a/packages/js/src/integrations-page/myyoast-integration.js +++ b/packages/js/src/integrations-page/myyoast-connection/myyoast-integration.js @@ -1,4 +1,3 @@ -/* eslint-disable camelcase */ import ArrowNarrowRightIcon from "@heroicons/react/outline/ArrowNarrowRightIcon"; import CheckIcon from "@heroicons/react/solid/CheckIcon"; import ExclamationCircleIcon from "@heroicons/react/solid/ExclamationCircleIcon"; @@ -8,44 +7,72 @@ import { useCallback, useEffect, useId, useState } from "@wordpress/element"; import { __, _n, sprintf } from "@wordpress/i18n"; import { Alert, Button, TooltipContainer, TooltipTrigger, TooltipWithContext, useSvgAria, useToggleState } from "@yoast/ui-library"; import PropTypes from "prop-types"; -import { ReactComponent as MyYoastLogo } from "../../images/myyoast-logo.svg"; +import { ReactComponent as MyYoastLogo } from "../../../images/myyoast-logo.svg"; import { MyyoastConnectionDisconnectModal } from "./myyoast-disconnect-modal"; -import { MYYOAST_STORE_NAME } from "./myyoast-store"; -import { Card } from "./tailwind-components/card"; +import { MYYOAST_STORE_NAME } from "./constants"; +import { Card } from "../tailwind-components/card"; // Placeholder until the final MyYoast integrations article URL is provided. const LEARN_MORE_LINK = "https://yoa.st/myyoast-connection"; /** - * Returns the error and success message map keyed by the machine codes the - * backend emits (`error_code` for errors, `message_key` for successes). + * Resolves the user-facing message for a machine code the backend emits + * (`error_code` for errors, `message_key` for successes). * - * Built lazily inside a function so `__()` is called at call time rather than - * at module load — keeps locale switching working. + * A switch rather than a map so only the matched string is translated, and + * `__()` runs at call time rather than module load — keeping locale switching + * working. Unknown codes fall through to the generic error. * - * @returns {Object} The message map. + * @param {string} code The backend code. + * @returns {string} The translated message. */ -const buildMessages = () => ( { - not_provisioned: __( "Your server doesn't support the MyYoast connection. Update Yoast SEO to the latest version. If the issue persists after updating, contact support.", "wordpress-seo" ), - registration_gone: __( "MyYoast no longer recognizes this site. Connect this site to MyYoast again to restore the connection.", "wordpress-seo" ), - rate_limited: __( "MyYoast has had a lot of connection attempts from this site or network. Please wait a few minutes and try again.", "wordpress-seo" ), - server_capability: __( "MyYoast doesn't support a feature this version of Yoast SEO needs. Update Yoast SEO to the latest version. If the issue persists, contact support.", "wordpress-seo" ), - myyoast_unreachable: __( "Couldn't reach MyYoast from this server. Check your server's outbound network access, then try again. If MyYoast is having issues, wait a few minutes and retry.", "wordpress-seo" ), - token_request_failed_invalid_grant: __( "MyYoast rejected the credentials stored for this site. Disconnect and connect this site again to restore the connection.", "wordpress-seo" ), - token_request_failed: __( "Something went wrong while talking to MyYoast. Try again in a moment. If the problem keeps happening, update Yoast SEO or contact support.", "wordpress-seo" ), - token_storage_failed: __( "Couldn't save the new credentials on this site. Make sure your WordPress database is writable, then try again.", "wordpress-seo" ), - invalid_resource: __( "Something went wrong. Refresh the page and try again. If the problem keeps happening, contact support.", "wordpress-seo" ), - registration_failed: __( "Couldn't connect this site to MyYoast. Try again in a moment. If the problem keeps happening, update Yoast SEO or contact support.", "wordpress-seo" ), - unknown_redirect_uri: __( "Couldn't verify this site because it's no longer recognized. Refresh the page and try again.", "wordpress-seo" ), - invalid_user: __( "You need to be signed in to verify this site.", "wordpress-seo" ), - connection_cancelled: __( "Connection cancelled. You can try again whenever you're ready.", "wordpress-seo" ), - timeout: __( "Request to MyYoast timed out. Please try again.", "wordpress-seo" ), - unexpected_error: __( "Something went wrong. Try again in a moment. If the problem keeps happening, update Yoast SEO or contact support.", "wordpress-seo" ), - connect_success: __( "This site is now connected to MyYoast.", "wordpress-seo" ), - update_success: __( "Connection updated to match this site's current URL.", "wordpress-seo" ), - disconnect_success: __( "This site is no longer connected to MyYoast.", "wordpress-seo" ), - verify_success: __( "This site is now verified.", "wordpress-seo" ), -} ); +// eslint-disable-next-line complexity +const messageFor = ( code ) => { + switch ( code ) { + case "not_provisioned": + return __( "Your server doesn't support the MyYoast connection. Update Yoast SEO to the latest version. If the issue persists after updating, contact support.", "wordpress-seo" ); + case "registration_gone": + return __( "MyYoast no longer recognizes this site. Connect this site to MyYoast again to restore the connection.", "wordpress-seo" ); + case "rate_limited": + return __( "MyYoast has had a lot of connection attempts from this site or network. Please wait a few minutes and try again.", "wordpress-seo" ); + case "server_capability": + return __( "MyYoast doesn't support a feature this version of Yoast SEO needs. Update Yoast SEO to the latest version. If the issue persists, contact support.", "wordpress-seo" ); + case "myyoast_unreachable": + return __( "Couldn't reach MyYoast from this server. Check your server's outbound network access, then try again. If MyYoast is having issues, wait a few minutes and retry.", "wordpress-seo" ); + case "token_request_failed_invalid_grant": + return __( "MyYoast rejected the credentials stored for this site. Disconnect and connect this site again to restore the connection.", "wordpress-seo" ); + case "token_request_failed": + return __( "Something went wrong while talking to MyYoast. Try again in a moment. If the problem keeps happening, update Yoast SEO or contact support.", "wordpress-seo" ); + case "token_storage_failed": + return __( "Couldn't save the new credentials on this site. Make sure your WordPress database is writable, then try again.", "wordpress-seo" ); + case "invalid_resource": + return __( "Something went wrong. Refresh the page and try again. If the problem keeps happening, contact support.", "wordpress-seo" ); + case "registration_failed": + return __( "Couldn't connect this site to MyYoast. Try again in a moment. If the problem keeps happening, update Yoast SEO or contact support.", "wordpress-seo" ); + case "unknown_redirect_uri": + return __( "Couldn't verify this site because it's no longer recognized. Refresh the page and try again.", "wordpress-seo" ); + case "invalid_user": + return __( "You need to be signed in to verify this site.", "wordpress-seo" ); + case "connection_cancelled": + return __( "Connection cancelled. You can try again whenever you're ready.", "wordpress-seo" ); + case "timeout": + return __( "Request to MyYoast timed out. Please try again.", "wordpress-seo" ); + case "connect_success": + return __( "This site is now connected to MyYoast.", "wordpress-seo" ); + case "update_success": + return __( "Connection updated to match this site's current URL.", "wordpress-seo" ); + case "disconnect_success": + return __( "This site is no longer connected to MyYoast.", "wordpress-seo" ); + case "verify_success": + return __( "This site is now verified.", "wordpress-seo" ); + default: + return __( "Something went wrong. Try again in a moment. If the problem keeps happening, update Yoast SEO or contact support.", "wordpress-seo" ); + } +}; + +// Success keys the backend may send. Used to gate success feedback so an +// unrecognized key doesn't fall through to `messageFor`'s generic error string. +const SUCCESS_MESSAGE_KEYS = new Set( [ "connect_success", "update_success", "disconnect_success", "verify_success" ] ); /** * Formats the rate-limit message in minutes or hours, with the correct @@ -80,14 +107,13 @@ const ACTION_DISPATCHERS = { * @returns {string} The translated message. */ const resolveErrorMessage = ( code, details ) => { - const messages = buildMessages(); if ( code === "rate_limited" ) { const seconds = Number( details?.retry_after_seconds ); if ( Number.isFinite( seconds ) && seconds > 0 ) { return formatRateLimitedMessage( seconds ); } } - return messages[ code ] ?? messages.unexpected_error; + return messageFor( code ); }; /** @@ -116,11 +142,8 @@ const runAction = async( actionName, body, options ) => { return result; } - if ( result.ok && result.messageKey ) { - const message = buildMessages()[ result.messageKey ]; - if ( message ) { - options?.onFeedback?.( { variant: "success", message } ); - } + if ( result.ok && SUCCESS_MESSAGE_KEYS.has( result.messageKey ) ) { + options?.onFeedback?.( { variant: "success", message: messageFor( result.messageKey ) } ); } else if ( ! result.ok ) { const message = resolveErrorMessage( result.errorCode, result.details ); store.setMyyoastActionError( { actionName, errorCode: result.errorCode, message } ); @@ -199,7 +222,7 @@ const StatusFooter = ( { connectionLost, verificationNeeded } ) => { return (

{ __( "Site connected", "wordpress-seo" ) } - +

); }; @@ -241,10 +264,7 @@ export const MyyoastIntegration = () => { return; } const { kind, key } = pendingCallbackOutcome; - const message = buildMessages()[ key ]; - if ( message ) { - setFeedback( { variant: kind === "success" ? "success" : "error", message } ); - } + setFeedback( { variant: kind === "success" ? "success" : "error", message: messageFor( key ) } ); dispatch( MYYOAST_STORE_NAME ).clearMyyoastCallbackOutcome(); }, [ pendingCallbackOutcome ] ); diff --git a/packages/js/src/integrations-page/myyoast-connection/store/index.js b/packages/js/src/integrations-page/myyoast-connection/store/index.js new file mode 100644 index 00000000000..f3f1e3396d6 --- /dev/null +++ b/packages/js/src/integrations-page/myyoast-connection/store/index.js @@ -0,0 +1,70 @@ +import { combineReducers, createReduxStore, register } from "@wordpress/data"; +import { get, merge } from "lodash"; +import { MYYOAST_STORE_NAME } from "../constants"; +import { + getInitialMyyoastConnectionState, + MYYOAST_CONNECTION_NAME, + myyoastConnectionActions, + myyoastConnectionControls, + myyoastConnectionReducer, + myyoastConnectionSelectors, + transformStatus, +} from "./myyoast-connection"; + +/** @typedef {import("@wordpress/data/src/types").WPDataStore} WPDataStore */ + +/** + * Builds the MyYoast connection store descriptor. + * + * The slice state is nested under the `myyoastConnection` key so the selectors + * keep the same shape they had inside the settings store. + * + * @param {Object} initialState Initial state, merged over the slice defaults. + * @returns {WPDataStore} The WP data store. + */ +const createStore = ( { initialState } ) => { + return createReduxStore( MYYOAST_STORE_NAME, { + actions: { + ...myyoastConnectionActions, + }, + selectors: { + ...myyoastConnectionSelectors, + }, + controls: { + ...myyoastConnectionControls, + }, + reducer: combineReducers( { + [ MYYOAST_CONNECTION_NAME ]: myyoastConnectionReducer, + } ), + initialState: merge( + {}, + { [ MYYOAST_CONNECTION_NAME ]: getInitialMyyoastConnectionState() }, + initialState + ), + } ); +}; + +/** + * Registers the standalone MyYoast connection store, seeded from the + * `wpseoIntegrationsData.myyoast_connection` payload the integrations page + * localizes. No-ops when the payload is absent (feature flag disabled) — + * the card is not rendered in that case either. + * + * @returns {void} + */ +export const registerMyyoastStore = () => { + const data = get( window, "wpseoIntegrationsData.myyoast_connection", null ); + if ( ! data ) { + return; + } + + register( createStore( { + initialState: { + [ MYYOAST_CONNECTION_NAME ]: { + status: transformStatus( data.initialStatus ), + profileUrl: data.profileUrl || "", + pendingCallbackOutcome: data.callbackOutcome || null, + }, + }, + } ) ); +}; diff --git a/packages/js/src/integrations-page/myyoast-store.js b/packages/js/src/integrations-page/myyoast-connection/store/myyoast-connection.js similarity index 86% rename from packages/js/src/integrations-page/myyoast-store.js rename to packages/js/src/integrations-page/myyoast-connection/store/myyoast-connection.js index d19f8d82108..d05f530ee0a 100644 --- a/packages/js/src/integrations-page/myyoast-store.js +++ b/packages/js/src/integrations-page/myyoast-connection/store/myyoast-connection.js @@ -1,10 +1,8 @@ import { createSlice } from "@reduxjs/toolkit"; import apiFetch from "@wordpress/api-fetch"; -import { combineReducers, registerStore } from "@wordpress/data"; import { get } from "lodash"; export const MYYOAST_CONNECTION_NAME = "myyoastConnection"; -export const MYYOAST_STORE_NAME = "yoast-seo/myyoast-connection"; const REQUEST_TIMEOUT_MS = 30000; @@ -19,7 +17,7 @@ const ENDPOINTS = { /** * @returns {Object} The initial myyoastConnection state. */ -export const createInitialMyyoastConnectionState = () => ( { +export const getInitialMyyoastConnectionState = () => ( { status: null, profileUrl: "", actionInFlight: null, @@ -66,7 +64,7 @@ export const transformStatus = ( payload ) => { const slice = createSlice( { name: MYYOAST_CONNECTION_NAME, - initialState: createInitialMyyoastConnectionState(), + initialState: getInitialMyyoastConnectionState(), reducers: { setMyyoastStatus: ( state, { payload } ) => { if ( payload ) { @@ -223,37 +221,4 @@ export const myyoastConnectionSelectors = { selectHasMyyoastConnection: state => get( state, "myyoastConnection.status", null ) !== null, }; -/** - * Registers the standalone MyYoast connection store, seeded from the - * `wpseoIntegrationsData.myyoast_connection` payload the integrations page - * localizes. No-ops when the payload is absent (feature flag disabled) — - * the card is not rendered in that case either. - * - * The slice state is nested under the `myyoastConnection` key so the - * selectors keep the same shape they had inside the settings store. - * - * @returns {void} - */ -export const registerMyyoastStore = () => { - const data = get( window, "wpseoIntegrationsData.myyoast_connection", null ); - if ( ! data ) { - return; - } - - registerStore( MYYOAST_STORE_NAME, { - reducer: combineReducers( { [ MYYOAST_CONNECTION_NAME ]: slice.reducer } ), - actions: myyoastConnectionActions, - selectors: myyoastConnectionSelectors, - controls: myyoastConnectionControls, - initialState: { - [ MYYOAST_CONNECTION_NAME ]: { - ...createInitialMyyoastConnectionState(), - status: transformStatus( data.initialStatus ), - profileUrl: data.profileUrl || "", - pendingCallbackOutcome: data.callbackOutcome || null, - }, - }, - } ); -}; - -export default slice.reducer; +export const myyoastConnectionReducer = slice.reducer; diff --git a/packages/js/src/integrations-page/recommended-integrations.js b/packages/js/src/integrations-page/recommended-integrations.js index ad459b2dc0b..23bc0c0cd60 100644 --- a/packages/js/src/integrations-page/recommended-integrations.js +++ b/packages/js/src/integrations-page/recommended-integrations.js @@ -4,7 +4,7 @@ import { safeCreateInterpolateElement } from "../helpers/i18n"; import { ReactComponent as SemrushLogo } from "../../images/semrush-logo.svg"; import { ReactComponent as WincherLogo } from "../../images/wincher-logo.svg"; import { getInitialState, getIsMultisiteAvailable, getIsNetworkControlEnabled, updateIntegrationState } from "./helper"; -import { MyyoastIntegration } from "./myyoast-integration"; +import { MyyoastIntegration } from "./myyoast-connection/myyoast-integration"; import { SiteKitIntegration } from "./site-kit-integration"; import { ToggleableIntegration } from "./toggleable-integration"; diff --git a/packages/js/tests/integrations-page/myyoast-connection/store/myyoast-connection.test.js b/packages/js/tests/integrations-page/myyoast-connection/store/myyoast-connection.test.js new file mode 100644 index 00000000000..880836ecf36 --- /dev/null +++ b/packages/js/tests/integrations-page/myyoast-connection/store/myyoast-connection.test.js @@ -0,0 +1,306 @@ +/* eslint-disable camelcase -- snake_case keys model the backend's REST request/response contract. */ +import { describe, expect, it } from "@jest/globals"; +import { + getInitialMyyoastConnectionState, + MYYOAST_CONNECTION_NAME, + myyoastConnectionActions, + myyoastConnectionReducer, + myyoastConnectionSelectors, + transformStatus, +} from "../../../../src/integrations-page/myyoast-connection/store/myyoast-connection"; + +it( "MYYOAST_CONNECTION_NAME is myyoastConnection", () => { + expect( MYYOAST_CONNECTION_NAME ).toBe( "myyoastConnection" ); +} ); + +describe( "transformStatus", () => { + it( "returns the default status when the payload is empty", () => { + expect( transformStatus( null ) ).toEqual( { + isProvisioned: false, + isRegistered: false, + registeredAt: null, + registeredAtIso: null, + redirectUris: [], + redirectUrisMatch: true, + } ); + } ); + + it( "maps the snake_case payload to the camelCase shape", () => { + const status = transformStatus( { + is_provisioned: true, + is_registered: true, + registered_at: 1700000000, + registered_at_iso: "2023-11-14T22:13:20+00:00", + redirect_uris: [ + { uri: "https://example.com/callback", origin: "https://example.com", is_verified: true }, + { uri: "https://example.com/other", origin: "https://example.com", is_verified: false }, + ], + redirect_uris_match: true, + } ); + + expect( status ).toEqual( { + isProvisioned: true, + isRegistered: true, + registeredAt: 1700000000, + registeredAtIso: "2023-11-14T22:13:20+00:00", + redirectUris: [ + { uri: "https://example.com/callback", origin: "https://example.com", isVerified: true }, + { uri: "https://example.com/other", origin: "https://example.com", isVerified: false }, + ], + redirectUrisMatch: true, + } ); + } ); + + it( "defaults each redirect URI field when the entry is malformed", () => { + const status = transformStatus( { redirect_uris: [ {}, null ] } ); + + expect( status.redirectUris ).toEqual( [ + { uri: "", origin: "", isVerified: false }, + { uri: "", origin: "", isVerified: false }, + ] ); + } ); + + it( "treats a non-array redirect_uris as an empty list", () => { + expect( transformStatus( { redirect_uris: "nope" } ).redirectUris ).toEqual( [] ); + } ); + + it( "only reports redirectUrisMatch false when explicitly false", () => { + // A missing or truthy value is treated as a match; only an explicit `false` flips it. + expect( transformStatus( {} ).redirectUrisMatch ).toBe( true ); + expect( transformStatus( { redirect_uris_match: false } ).redirectUrisMatch ).toBe( false ); + } ); +} ); + +describe( "initial state", () => { + it( "is the empty connection state", () => { + expect( myyoastConnectionReducer( undefined, { type: "" } ) ).toEqual( getInitialMyyoastConnectionState() ); + } ); +} ); + +describe( "reducer", () => { + describe( "startMyyoastAction", () => { + it( "records the in-flight action and clears any previous error", () => { + const state = { ...getInitialMyyoastConnectionState(), actionError: { errorCode: "boom" } }; + const next = myyoastConnectionReducer( state, myyoastConnectionActions.startMyyoastAction( "connect" ) ); + + expect( next.actionInFlight ).toBe( "connect" ); + expect( next.actionError ).toBeNull(); + } ); + + it( "ignores a second action while one is already in flight", () => { + // Every action mutates the same registration, so a started action must + // not be overwritten by another that begins before it finishes. + const state = { ...getInitialMyyoastConnectionState(), actionInFlight: "connect" }; + const next = myyoastConnectionReducer( state, myyoastConnectionActions.startMyyoastAction( "disconnect" ) ); + + expect( next.actionInFlight ).toBe( "connect" ); + } ); + } ); + + describe( "finishMyyoastAction", () => { + it( "clears the in-flight action", () => { + const state = { ...getInitialMyyoastConnectionState(), actionInFlight: "connect" }; + const next = myyoastConnectionReducer( state, myyoastConnectionActions.finishMyyoastAction() ); + + expect( next.actionInFlight ).toBeNull(); + } ); + } ); + + describe( "setMyyoastStatus", () => { + it( "stores the status payload", () => { + const status = transformStatus( { is_registered: true } ); + const next = myyoastConnectionReducer( getInitialMyyoastConnectionState(), myyoastConnectionActions.setMyyoastStatus( status ) ); + + expect( next.status ).toEqual( status ); + } ); + + it( "leaves the status untouched for a falsy payload", () => { + const state = { ...getInitialMyyoastConnectionState(), status: { isRegistered: true } }; + const next = myyoastConnectionReducer( state, myyoastConnectionActions.setMyyoastStatus( null ) ); + + expect( next.status ).toEqual( { isRegistered: true } ); + } ); + } ); + + describe( "clearMyyoastCallbackOutcome", () => { + it( "clears the pending callback outcome", () => { + const state = { ...getInitialMyyoastConnectionState(), pendingCallbackOutcome: { kind: "success", key: "connect_success" } }; + const next = myyoastConnectionReducer( state, myyoastConnectionActions.clearMyyoastCallbackOutcome() ); + + expect( next.pendingCallbackOutcome ).toBeNull(); + } ); + } ); +} ); + +describe( "selectors", () => { + const status = transformStatus( { is_registered: true } ); + const state = { + [ MYYOAST_CONNECTION_NAME ]: { + status, + profileUrl: "https://my.yoast.com/profile", + actionInFlight: "connect", + actionError: { errorCode: "boom" }, + pendingCallbackOutcome: { kind: "success", key: "connect_success" }, + }, + }; + + it( "selectMyyoastConnectionStatus returns the stored status", () => { + expect( myyoastConnectionSelectors.selectMyyoastConnectionStatus( state ) ).toEqual( status ); + } ); + + it( "selectMyyoastConnectionStatus falls back to the default status when absent", () => { + expect( myyoastConnectionSelectors.selectMyyoastConnectionStatus( {} ) ).toEqual( transformStatus( null ) ); + } ); + + it( "selectMyyoastConnectionProfileUrl returns the profile URL", () => { + expect( myyoastConnectionSelectors.selectMyyoastConnectionProfileUrl( state ) ).toBe( "https://my.yoast.com/profile" ); + } ); + + it( "selectMyyoastConnectionActionInFlight returns the in-flight action", () => { + expect( myyoastConnectionSelectors.selectMyyoastConnectionActionInFlight( state ) ).toBe( "connect" ); + } ); + + it( "selectMyyoastConnectionActionError returns the action error", () => { + expect( myyoastConnectionSelectors.selectMyyoastConnectionActionError( state ) ).toEqual( { errorCode: "boom" } ); + } ); + + it( "selectMyyoastConnectionPendingCallbackOutcome returns the pending outcome", () => { + expect( myyoastConnectionSelectors.selectMyyoastConnectionPendingCallbackOutcome( state ) ).toEqual( { kind: "success", key: "connect_success" } ); + } ); + + describe( "selectHasMyyoastConnection", () => { + it( "is true when a status is present", () => { + expect( myyoastConnectionSelectors.selectHasMyyoastConnection( state ) ).toBe( true ); + } ); + + it( "is false when the status is null", () => { + expect( myyoastConnectionSelectors.selectHasMyyoastConnection( { [ MYYOAST_CONNECTION_NAME ]: { status: null } } ) ).toBe( false ); + } ); + } ); +} ); + +describe( "management actions", () => { + // The generator yields plain control objects and slice actions, so it can be + // driven by hand: `next( value )` feeds back what a control would have resolved + // to, and the final `return` value is the result the UI layer consumes. + const REQUEST_STATUS = { is_registered: true }; + + describe( "connectMyyoastConnection", () => { + it( "starts the action, dispatches the control, mirrors the status and finishes on success", () => { + const generator = myyoastConnectionActions.connectMyyoastConnection( { foo: "bar" } ); + + expect( generator.next().value ).toEqual( { type: "myyoastConnection/startMyyoastAction", payload: "connect" } ); + + expect( generator.next().value ).toEqual( { type: "connectMyyoastConnection", payload: { foo: "bar" } } ); + + // Feed back the control's resolved payload; the status mirror is yielded next. + expect( generator.next( { status: REQUEST_STATUS, message_key: "connect_success" } ).value ) + .toEqual( { type: "myyoastConnection/setMyyoastStatus", payload: transformStatus( REQUEST_STATUS ) } ); + + expect( generator.next().value ).toEqual( { type: "myyoastConnection/finishMyyoastAction" } ); + + const final = generator.next(); + expect( final.done ).toBe( true ); + expect( final.value ).toEqual( { ok: true, messageKey: "connect_success" } ); + } ); + + it( "returns the error code and details when the body carries an error_code", () => { + const generator = myyoastConnectionActions.connectMyyoastConnection(); + generator.next(); + generator.next(); + + // A precondition/upstream failure arrives as a 200 with an error_code body. + expect( generator.next( { error_code: "rate_limited", details: { retry_after_seconds: 120 } } ).value ) + .toEqual( { type: "myyoastConnection/setMyyoastActionError", payload: { actionName: "connect", errorCode: "rate_limited", message: "" } } ); + + expect( generator.next().value ).toEqual( { type: "myyoastConnection/finishMyyoastAction" } ); + + const final = generator.next(); + expect( final.done ).toBe( true ); + expect( final.value ).toEqual( { ok: false, errorCode: "rate_limited", details: { retry_after_seconds: 120 } } ); + } ); + + it( "maps an aborted request to the timeout error and still finishes", () => { + const generator = myyoastConnectionActions.connectMyyoastConnection(); + generator.next(); + generator.next(); + + const abortError = new Error( "aborted" ); + abortError.name = "AbortError"; + + expect( generator.throw( abortError ).value ) + .toEqual( { type: "myyoastConnection/setMyyoastActionError", payload: { actionName: "connect", errorCode: "timeout", message: "" } } ); + + expect( generator.next().value ).toEqual( { type: "myyoastConnection/finishMyyoastAction" } ); + + const final = generator.next(); + expect( final.done ).toBe( true ); + expect( final.value ).toEqual( { ok: false, errorCode: "timeout" } ); + } ); + + it( "maps any other thrown error to unexpected_error", () => { + const generator = myyoastConnectionActions.connectMyyoastConnection(); + generator.next(); + generator.next(); + + expect( generator.throw( new Error( "network down" ) ).value ) + .toEqual( { type: "myyoastConnection/setMyyoastActionError", payload: { actionName: "connect", errorCode: "unexpected_error", message: "" } } ); + + generator.next(); + expect( generator.next().value ).toEqual( { ok: false, errorCode: "unexpected_error" } ); + } ); + } ); + + describe( "authorizeMyyoastSite", () => { + it( "sends the return_url and returns the authorize URL on success", () => { + const generator = myyoastConnectionActions.authorizeMyyoastSite( "https://example.com/admin" ); + + expect( generator.next().value ).toEqual( { type: "myyoastConnection/startMyyoastAction", payload: "authorize" } ); + + expect( generator.next().value ).toEqual( { type: "authorizeMyyoastSite", payload: { return_url: "https://example.com/admin" } } ); + + expect( generator.next( { authorize_url: "https://my.yoast.com/authorize" } ).value ) + .toEqual( { type: "myyoastConnection/finishMyyoastAction" } ); + + const final = generator.next(); + expect( final.done ).toBe( true ); + expect( final.value ).toEqual( { ok: true, authorizeUrl: "https://my.yoast.com/authorize" } ); + } ); + + it( "omits the body when no return URL is given", () => { + const generator = myyoastConnectionActions.authorizeMyyoastSite(); + generator.next(); + + expect( generator.next().value ).toEqual( { type: "authorizeMyyoastSite", payload: {} } ); + } ); + + it( "fails with unexpected_error when the success response has no authorize_url", () => { + const generator = myyoastConnectionActions.authorizeMyyoastSite(); + generator.next(); + generator.next(); + + expect( generator.next( {} ).value ) + .toEqual( { type: "myyoastConnection/setMyyoastActionError", payload: { actionName: "authorize", errorCode: "unexpected_error", message: "" } } ); + + expect( generator.next().value ).toEqual( { type: "myyoastConnection/finishMyyoastAction" } ); + + const final = generator.next(); + expect( final.done ).toBe( true ); + expect( final.value ).toEqual( { ok: false, errorCode: "unexpected_error" } ); + } ); + + it( "returns the error code and details when the body carries an error_code", () => { + const generator = myyoastConnectionActions.authorizeMyyoastSite(); + generator.next(); + generator.next(); + + expect( generator.next( { error_code: "invalid_user" } ).value ) + .toEqual( { type: "myyoastConnection/setMyyoastActionError", payload: { actionName: "authorize", errorCode: "invalid_user", message: "" } } ); + + generator.next(); + const final = generator.next(); + expect( final.done ).toBe( true ); + expect( final.value ).toEqual( { ok: false, errorCode: "invalid_user", details: undefined } ); + } ); + } ); +} ); From b307876e8219c1b6993805be6abbdf6d242cd43a Mon Sep 17 00:00:00 2001 From: Diede Exterkate <5352634+diedexx@users.noreply.github.com> Date: Mon, 22 Jun 2026 17:19:17 +0200 Subject: [PATCH 09/26] fix(integrations-page): send the OAuth return URL from the MyYoast card The authorize action accepted a return URL, but the card never passed one, so after the OAuth round-trip the backend fell back to its default instead of returning the user to the integrations page. Pass the current page as the return URL from both the connect and verify flows. The backend validates it and ignores it when off-site or invalid. Take the return URL as a named option (`{ returnUrl }`) so the call site is self-documenting and the action can grow more options without a signature change. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../myyoast-connection/myyoast-integration.js | 6 +++++- .../myyoast-connection/store/myyoast-connection.js | 5 +++-- .../myyoast-connection/store/myyoast-connection.test.js | 2 +- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/js/src/integrations-page/myyoast-connection/myyoast-integration.js b/packages/js/src/integrations-page/myyoast-connection/myyoast-integration.js index d6fc3f0c5ef..39e32073e66 100644 --- a/packages/js/src/integrations-page/myyoast-connection/myyoast-integration.js +++ b/packages/js/src/integrations-page/myyoast-connection/myyoast-integration.js @@ -158,6 +158,10 @@ const runAction = async( actionName, body, options ) => { * navigates the browser there. The backend resolves which registered redirect * URI to use. Errors surface as inline card feedback. * + * The current page is passed as the return URL so the OAuth callback sends the + * user back to the integrations page. The backend validates it and ignores it + * when off-site or invalid. + * * @param {function} onFeedback Receives `{ variant, message }` to show in the card. * @returns {Promise} Resolves once the navigation has been kicked off. */ @@ -166,7 +170,7 @@ const runAuthorize = async( onFeedback ) => { return; } const store = dispatch( MYYOAST_STORE_NAME ); - const result = await store.authorizeMyyoastSite(); + const result = await store.authorizeMyyoastSite( { returnUrl: window.location.href } ); if ( result.ok && result.authorizeUrl ) { window.location.assign( result.authorizeUrl ); diff --git a/packages/js/src/integrations-page/myyoast-connection/store/myyoast-connection.js b/packages/js/src/integrations-page/myyoast-connection/store/myyoast-connection.js index d05f530ee0a..0f35a80bbbb 100644 --- a/packages/js/src/integrations-page/myyoast-connection/store/myyoast-connection.js +++ b/packages/js/src/integrations-page/myyoast-connection/store/myyoast-connection.js @@ -142,11 +142,12 @@ const disconnectMyyoastConnection = createMyyoastAction( "disconnect" ); * `authorize_url` the browser should be navigated to; the caller decides how to * do that. On failure the slice's actionError is set, mirroring the other actions. * - * @param {string} [returnUrl] The URL to return to after the flow completes. + * @param {Object} [options] The action options. + * @param {string} [options.returnUrl] The URL to return to after the flow completes. * @returns {GeneratorFunction} The generator action. */ // eslint-disable-next-line complexity -const authorizeMyyoastSite = function* ( returnUrl ) { +const authorizeMyyoastSite = function* ( { returnUrl } = {} ) { yield startMyyoastAction( "authorize" ); try { // eslint-disable-next-line camelcase -- snake_case matches the REST endpoint's request contract. diff --git a/packages/js/tests/integrations-page/myyoast-connection/store/myyoast-connection.test.js b/packages/js/tests/integrations-page/myyoast-connection/store/myyoast-connection.test.js index 880836ecf36..a43c1a7eb77 100644 --- a/packages/js/tests/integrations-page/myyoast-connection/store/myyoast-connection.test.js +++ b/packages/js/tests/integrations-page/myyoast-connection/store/myyoast-connection.test.js @@ -253,7 +253,7 @@ describe( "management actions", () => { describe( "authorizeMyyoastSite", () => { it( "sends the return_url and returns the authorize URL on success", () => { - const generator = myyoastConnectionActions.authorizeMyyoastSite( "https://example.com/admin" ); + const generator = myyoastConnectionActions.authorizeMyyoastSite( { returnUrl: "https://example.com/admin" } ); expect( generator.next().value ).toEqual( { type: "myyoastConnection/startMyyoastAction", payload: "authorize" } ); From 42556d73d7778ff03f639e11f6a7e962dcac2681 Mon Sep 17 00:00:00 2001 From: Diede Exterkate <5352634+diedexx@users.noreply.github.com> Date: Tue, 23 Jun 2026 08:48:00 +0200 Subject: [PATCH 10/26] fix(integrations-page): don't flash the verification notice while connecting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Connecting registers the site as an OAuth client and then redirects to MyYoast to complete the authorization-code grant. Registration leaves the site registered-but-unverified, and `window.location.assign` navigates asynchronously, so the card kept rendering — and briefly showed the "Verification needed" notice — while the MyYoast page loaded. Track the connect flow with a local `isConnecting` flag that suppresses the notice, and keep it set through the redirect tail. It is cleared only when we stay on the page (registration failed, or authorize returned no URL) so the notice and error still surface there. Scoped to the connect auto-flow, so a deliberate verify click still shows its context. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../myyoast-connection/myyoast-integration.js | 31 ++++++++++++++++--- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/packages/js/src/integrations-page/myyoast-connection/myyoast-integration.js b/packages/js/src/integrations-page/myyoast-connection/myyoast-integration.js index 39e32073e66..df2f17eb726 100644 --- a/packages/js/src/integrations-page/myyoast-connection/myyoast-integration.js +++ b/packages/js/src/integrations-page/myyoast-connection/myyoast-integration.js @@ -163,22 +163,26 @@ const runAction = async( actionName, body, options ) => { * when off-site or invalid. * * @param {function} onFeedback Receives `{ variant, message }` to show in the card. - * @returns {Promise} Resolves once the navigation has been kicked off. + * @returns {Promise} True once a redirect has been kicked off (the page + * is navigating away); false when we stay on the page. */ const runAuthorize = async( onFeedback ) => { if ( select( MYYOAST_STORE_NAME ).selectMyyoastConnectionActionInFlight() ) { - return; + return false; } const store = dispatch( MYYOAST_STORE_NAME ); const result = await store.authorizeMyyoastSite( { returnUrl: window.location.href } ); if ( result.ok && result.authorizeUrl ) { window.location.assign( result.authorizeUrl ); - return; + // The navigation is async — the page keeps rendering until MyYoast loads. + // Report that a redirect started so callers don't restore in-page state. + return true; } const message = resolveErrorMessage( result.errorCode, result.details ); onFeedback?.( { variant: "error", message } ); + return false; }; /** @@ -247,6 +251,11 @@ export const MyyoastIntegration = () => { const actionInFlight = useSelect( s => s( MYYOAST_STORE_NAME ).selectMyyoastConnectionActionInFlight(), [] ); const pendingCallbackOutcome = useSelect( s => s( MYYOAST_STORE_NAME ).selectMyyoastConnectionPendingCallbackOutcome(), [] ); const [ feedback, setFeedback ] = useState( null ); + // True only while the connect → authorize auto-flow is running: registration + // succeeds (status becomes registered-but-unverified) before we redirect to + // MyYoast, and we don't want to flash the "Verification needed" notice for + // that gap. Scoped to this flow so a deliberate verify click still shows it. + const [ isConnecting, setIsConnecting ] = useState( false ); const [ isDisconnectOpen, , , openDisconnect, closeDisconnect ] = useToggleState( false ); // Auto-fired status refresh on mount: confirms with MyYoast that the stored @@ -278,6 +287,9 @@ export const MyyoastIntegration = () => { // user to MyYoast to sign in rather than leaving them on the unverified // "Verification needed" state. A failed registration just shows its error. const handleConnect = useCallback( async() => { + // Mark the combined flow so the registered-but-unverified gap between + // registration and the redirect doesn't flash the verification notice. + setIsConnecting( true ); // Register silently: on success we redirect to MyYoast immediately, so a // transient "connected" message would only flash before the page leaves. // A failure still needs to be shown, so surface only that. @@ -285,11 +297,18 @@ export const MyyoastIntegration = () => { if ( ! result.ok ) { const message = resolveErrorMessage( result.errorCode, result.details ); setFeedback( { variant: "error", message } ); + setIsConnecting( false ); return; } // Registration succeeded; continue straight into the authorization-code // flow. The backend resolves which registered redirect URI to use. - await runAuthorize( setFeedback ); + const redirecting = await runAuthorize( setFeedback ); + // Only clear when we're staying on the page: `window.location.assign` is + // async, so the page keeps rendering while MyYoast loads. Clearing now + // would flash the verification notice during that tail. + if ( ! redirecting ) { + setIsConnecting( false ); + } }, [] ); const handleReconnect = useCallback( () => runAction( "update", null, { onFeedback: setFeedback } ), [] ); const handleDisconnectConfirm = useCallback( () => { @@ -306,7 +325,9 @@ export const MyyoastIntegration = () => { // several are connected — verifying them all clears it. Suppressed while the // connection is lost. const firstUnverified = connectionLost ? null : redirectUris.find( ( entry ) => ! entry.isVerified ) ?? null; - const verificationNeeded = Boolean( firstUnverified ); + // Suppressed during the connect → authorize auto-flow: registration leaves the + // site registered-but-unverified for the moment before the redirect fires. + const verificationNeeded = Boolean( firstUnverified ) && ! isConnecting; const handleVerify = useCallback( () => { if ( firstUnverified ) { From 40859bc6650fc197335499f61faf90859601f4f4 Mon Sep 17 00:00:00 2001 From: Diede Exterkate <5352634+diedexx@users.noreply.github.com> Date: Tue, 23 Jun 2026 08:50:08 +0200 Subject: [PATCH 11/26] fix(integrations-page): clarify the MyYoast connection success message The OAuth callback emits one success notice for both first-time setup and a standalone re-verify, so "This site is now verified" read as jargon right after connecting. Describe the end state instead: the connection is active. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../myyoast-connection/myyoast-integration.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/js/src/integrations-page/myyoast-connection/myyoast-integration.js b/packages/js/src/integrations-page/myyoast-connection/myyoast-integration.js index df2f17eb726..3cb2d0ab7ba 100644 --- a/packages/js/src/integrations-page/myyoast-connection/myyoast-integration.js +++ b/packages/js/src/integrations-page/myyoast-connection/myyoast-integration.js @@ -64,7 +64,10 @@ const messageFor = ( code ) => { case "disconnect_success": return __( "This site is no longer connected to MyYoast.", "wordpress-seo" ); case "verify_success": - return __( "This site is now verified.", "wordpress-seo" ); + // Emitted by the OAuth callback for both first-time setup and a + // standalone re-verify, so the copy describes the end state rather + // than the "verify" action. + return __( "Your MyYoast connection is now active.", "wordpress-seo" ); default: return __( "Something went wrong. Try again in a moment. If the problem keeps happening, update Yoast SEO or contact support.", "wordpress-seo" ); } From 5b28a97ac73e2496e1b77c7205eaedfd38d5a11a Mon Sep 17 00:00:00 2001 From: Diede Exterkate <5352634+diedexx@users.noreply.github.com> Date: Tue, 23 Jun 2026 11:04:34 +0200 Subject: [PATCH 12/26] refactor(myyoast-client): simplify the refresh-throttle transient key Drop the trailing underscore from the prefix constant and the redundant rtrim() that stripped it, so the throttle key is built in one step. The resulting transient key is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/myyoast-client/user-interface/management-route.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/myyoast-client/user-interface/management-route.php b/src/myyoast-client/user-interface/management-route.php index 9c127a308b4..b3be151a567 100644 --- a/src/myyoast-client/user-interface/management-route.php +++ b/src/myyoast-client/user-interface/management-route.php @@ -62,7 +62,7 @@ class Management_Route implements Route_Interface, LoggerAwareInterface { * * @var string */ - private const REFRESH_THROTTLE_TRANSIENT_PREFIX = 'wpseo_myyoast_refresh_throttle_'; + private const REFRESH_THROTTLE_TRANSIENT_PREFIX = 'wpseo_myyoast_refresh_throttle'; /** * The MyYoast client facade. @@ -488,7 +488,7 @@ private function handle_exception( Throwable $exception ): WP_REST_Response { private function get_refresh_throttle_key(): string { return \sprintf( '%s_%s', - \rtrim( self::REFRESH_THROTTLE_TRANSIENT_PREFIX, '_' ), + self::REFRESH_THROTTLE_TRANSIENT_PREFIX, $this->issuer_config->get_issuer_key(), ); } From 5c9ba0e68206252b7c3937a0b3515a74cb688e05 Mon Sep 17 00:00:00 2001 From: Diede Exterkate <5352634+diedexx@users.noreply.github.com> Date: Tue, 23 Jun 2026 11:04:45 +0200 Subject: [PATCH 13/26] fix(integrations-page): clarify the MyYoast connection card copy Reword the card heading and description to lead with the offline/firewall benefit, and replace the terse connection-lost message with an actionable one explaining that the site's URL changed. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../myyoast-connection/myyoast-integration.js | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/js/src/integrations-page/myyoast-connection/myyoast-integration.js b/packages/js/src/integrations-page/myyoast-connection/myyoast-integration.js index 3cb2d0ab7ba..431dfef4158 100644 --- a/packages/js/src/integrations-page/myyoast-connection/myyoast-integration.js +++ b/packages/js/src/integrations-page/myyoast-connection/myyoast-integration.js @@ -8,6 +8,7 @@ import { __, _n, sprintf } from "@wordpress/i18n"; import { Alert, Button, TooltipContainer, TooltipTrigger, TooltipWithContext, useSvgAria, useToggleState } from "@yoast/ui-library"; import PropTypes from "prop-types"; import { ReactComponent as MyYoastLogo } from "../../../images/myyoast-logo.svg"; +import { safeCreateInterpolateElement } from "../../helpers/i18n"; import { MyyoastConnectionDisconnectModal } from "./myyoast-disconnect-modal"; import { MYYOAST_STORE_NAME } from "./constants"; import { Card } from "../tailwind-components/card"; @@ -347,10 +348,18 @@ export const MyyoastIntegration = () => {

- { "MyYoast" } + { safeCreateInterpolateElement( + sprintf( + /* translators: 1: bold open tag; 2: bold close tag. */ + __( "Unlock more from Yoast with %1$sMyYoast%2$s", "wordpress-seo" ), + "", + "" + ), + { strong: } + ) }

- { __( "Connect your site to MyYoast to enable AI features, manage your Yoast products, and simplify setup.", "wordpress-seo" ) } + { __( "Connect your site to MyYoast so Yoast AI works even when your site is offline, behind a firewall, or with the REST API disabled.", "wordpress-seo" ) }

{

{ __( "Connection lost", "wordpress-seo" ) }

-

{ __( "Domain connection lost, try to reconnect.", "wordpress-seo" ) }

+

{ __( "Your site's URL changed since the connection with MyYoast was made. Please reconnect.", "wordpress-seo" ) }