Merge pull request #23425 from Yoast/fix/revoking-consent-creates-tokens

Revoke consent by invalidating tokens

Author: diedexx

src/ai-authorization/application/token-manager.php

@@ -113,6 +113,9 @@ public function __construct(
 	/**
 	 * Invalidates the access token.
 	 *
+	 * The locally stored JWTs are always cleared, even when the remote invalidation fails — the
+	 * remote exception still propagates to the caller, but no credentials are left behind.
+	 *
 	 * @param int $user_id The user ID.
 	 *
 	 * @return void
@@ -133,27 +136,26 @@ public function token_invalidate( int $user_id ): void {
 			$access_jwt = '';
 		}
 
-		$request_body    = [
-			'user_id' => (string) $user_id,
-		];
 		$request_headers = [
 			'Authorization' => "Bearer $access_jwt",
 		];
 
 		try {
+			// The endpoint takes no request body; the user is identified by the access token.
 			$this->request_handler->handle(
 				new Request(
 					'/token/invalidate',
-					$request_body,
+					[],
 					$request_headers,
 				),
 			);
 		} catch ( Unauthorized_Exception | Forbidden_Exception $e ) { // phpcs:ignore Generic.CodeAnalysis.EmptyStatement.DetectedCatch -- Reason: Ignored on purpose.
 			// If the credentials in our request were already invalid, our job is done and we continue to remove the tokens client-side.
+		} finally {
+			// Always clear the local tokens, even when the remote invalidation fails with an exception
+			// that propagates: leaving credentials behind would contradict the intent of invalidating.
+			$this->clear_tokens( $user_id );
 		}
-
-		// Delete the stored JWT tokens.
-		$this->clear_tokens( $user_id );
 	}
 
 	/**
@@ -168,6 +170,31 @@ public function clear_tokens( int $user_id ): void {
 		$this->refresh_token_repository->delete_token( $user_id );
 	}
 
+	/**
+	 * Checks whether any JWT (access or refresh) is stored locally for the user.
+	 *
+	 * @param int $user_id The user ID.
+	 *
+	 * @return bool Whether a locally stored JWT exists.
+	 */
+	public function has_local_tokens( int $user_id ): bool {
+		try {
+			$this->access_token_repository->get_token( $user_id );
+
+			return true;
+		} catch ( RuntimeException $e ) { // phpcs:ignore Generic.CodeAnalysis.EmptyStatement.DetectedCatch -- Reason: Ignored on purpose.
+			// No access token; fall through to the refresh token check.
+		}
+
+		try {
+			$this->refresh_token_repository->get_token( $user_id );
+
+			return true;
+		} catch ( RuntimeException $e ) {
+			return false;
+		}
+	}
+
 	/**
 	 * Requests a new set of JWT tokens.
 	 *

src/ai-consent/application/consent-handler.php

@@ -109,11 +109,14 @@ public function grant_consent( int $user_id ) {
 	}
 
 	/**
-	 * Revokes the user's consent on the Yoast AI service and clears the local user meta.
+	 * Revokes the user's consent, both locally (user meta) and remotely (Yoast AI service).
 	 *
 	 * Security-first: the local meta is always cleared before the remote call, so consent is
-	 * revoked locally even if the remote DELETE fails. Any HTTP-layer exception is propagated
-	 * and its management is deferred to the caller.
+	 * revoked locally even if the remote `DELETE /user/consent` fails. Any locally stored JWTs
+	 * are then invalidated regardless of the remote outcome — credentials must not outlive
+	 * consent. The invalidation runs after the DELETE on purpose: authenticating the DELETE may
+	 * mint a fresh JWT, and invalidating afterwards catches that token too. Any HTTP-layer
+	 * exception is propagated and its management is deferred to the caller.
 	 *
 	 * @param int $user_id The user ID.
 	 *
@@ -129,7 +132,7 @@ public function grant_consent( int $user_id ) {
 	 * @throws Too_Many_Requests_Exception     When the AI service responds with 429.
 	 * @throws Unauthorized_Exception          When the AI service responds with 401.
 	 * @throws WP_Request_Exception            When the underlying WordPress HTTP call fails.
-	 * @throws RuntimeException           When the user is not found.
+	 * @throws RuntimeException                When the user is not found.
 	 */
 	public function revoke_consent( int $user_id ) {
 		$user = \get_user_by( 'id', $user_id );
@@ -140,11 +143,19 @@ public function revoke_consent( int $user_id ) {
 		// Local consent is always revoked regardless of remote failures.
 		$this->user_helper->delete_meta( $user_id, '_yoast_wpseo_ai_consent' );
 
-		$jwt = $this->token_manager->get_or_request_access_token( $user );
-
-		$this->request_handler->handle(
-			new Request( '/user/consent', [], [ 'Authorization' => "Bearer $jwt" ], Request::METHOD_DELETE ),
-		);
+		try {
+			$jwt = $this->token_manager->get_or_request_access_token( $user );
+
+			$this->request_handler->handle(
+				new Request( '/user/consent', [], [ 'Authorization' => "Bearer $jwt" ], Request::METHOD_DELETE ),
+			);
+		} finally {
+			// Invalidate the JWTs — including ones minted to authenticate the DELETE above — so
+			// credentials never outlive consent.
+			if ( $this->token_manager->has_local_tokens( $user_id ) ) {
+				$this->token_manager->token_invalidate( $user_id );
+			}
+		}
 	}
 
 	// phpcs:enable Squiz.Commenting.FunctionCommentThrowTag.WrongNumber

src/ai-consent/user-interface/consent-route.php

@@ -77,8 +77,9 @@ public static function get_conditionals() {
 	 */
 	public function __construct( Consent_Handler $consent_handler, Token_Manager $token_manager, Logger $logger ) {
 		$this->consent_handler = $consent_handler;
-		$this->token_manager   = $token_manager;
-		$this->logger          = $logger;
+		// @TODO: Remove the token manager as soon as we don't care about BC, because it's no longer used.
+		$this->token_manager = $token_manager;
+		$this->logger        = $logger;
 	}
 
 	/**
@@ -122,9 +123,7 @@ public function consent( WP_REST_Request $request ): WP_REST_Response {
 				$this->consent_handler->grant_consent( $user_id );
 			}
 			else {
-				// Invalidate the token if the user revoked the consent.
-				$this->token_manager->token_invalidate( $user_id );
-				// Delete the consent at user level.
+				// Revoke the consent locally and remotely (this also invalidates the JWT tokens).
 				$this->consent_handler->revoke_consent( $user_id );
 			}
 		} catch ( Remote_Request_Exception | RuntimeException $e ) {

src/ai-http-request/domain/request.php

@@ -78,10 +78,14 @@ public function get_action_path(): string {
 	/**
 	 * Get the body of the request.
 	 *
-	 * @return array<string> The body of the request.
+	 * Returns null for an empty body: an empty PHP array is ambiguous once JSON-encoded (`[]` vs `{}`),
+	 * so an empty body is omitted from the request entirely rather than sent as an empty array, which
+	 * the AI service rejects.
+	 *
+	 * @return array<string>|null The body of the request, or null when there is no body to send.
 	 */
-	public function get_body(): array {
-		return $this->body;
+	public function get_body(): ?array {
+		return ( $this->body === [] ) ? null : $this->body;
 	}
 
 	/**

src/ai-http-request/infrastructure/api-client-interface.php

@@ -14,10 +14,10 @@ interface API_Client_Interface {
 	/**
 	 * Performs a request to the API.
 	 *
-	 * @param string        $action_path The action path for the request.
-	 * @param array<string> $body        The body of the request.
-	 * @param array<string> $headers     The headers for the request.
-	 * @param string        $http_method The HTTP method for the request. One of `Request::METHOD_*`.
+	 * @param string             $action_path The action path for the request.
+	 * @param array<string>|null $body        The body of the request, or null/empty to send no body.
+	 * @param array<string>      $headers     The headers for the request.
+	 * @param string             $http_method The HTTP method for the request. One of `Request::METHOD_*`.
 	 *
 	 * @return array<int|string|array<string>> The response from the API.
 	 *

src/ai-http-request/infrastructure/api-client.php

@@ -24,10 +24,10 @@ class API_Client implements API_Client_Interface {
 	/**
 	 * Performs a request to the API.
 	 *
-	 * @param string        $action_path The action path for the request.
-	 * @param array<string> $body        The body of the request.
-	 * @param array<string> $headers     The headers for the request.
-	 * @param string        $http_method The HTTP method for the request. One of `Request::METHOD_*`.
+	 * @param string             $action_path The action path for the request.
+	 * @param array<string>|null $body        The body of the request, or null/empty to send no body.
+	 * @param array<string>      $headers     The headers for the request.
+	 * @param string             $http_method The HTTP method for the request. One of `Request::METHOD_*`.
 	 *
 	 * @return array<int|string|array<string>> The response from the API.
 	 *
@@ -41,8 +41,10 @@ public function perform_request( string $action_path, $body, $headers, string $h
 			'headers' => $headers,
 		];
 
-		// Only POST sends a body to the AI API today; GET and DELETE endpoints do not.
-		if ( $http_method === Request::METHOD_POST ) {
+		// Only POST sends a body to the AI API today; GET and DELETE endpoints do not. An empty body is
+		// omitted entirely: an empty array is ambiguous once JSON-encoded (`[]` vs `{}`) and the AI
+		// service rejects it, so a bodyless POST is sent instead.
+		if ( $http_method === Request::METHOD_POST && ! empty( $body ) ) {
 			// phpcs:ignore Yoast.Yoast.JsonEncodeAlternative.Found -- Reason: We don't want the debug/pretty possibility.
 			$arguments['body'] = WPSEO_Utils::format_json_encode( $body );
 		}

src/ai/authentication/application/ai-request-sender.php

@@ -154,10 +154,12 @@ public function grant_consent( WP_User $user ): Response {
 	}
 
 	/**
-	 * Revokes the user's consent on the AI service.
+	 * Revokes the user's consent on the AI service via `DELETE /user/consent`.
 	 *
-	 * The strategy identifies the WP user to the service (the OAuth path injects `user_id` into the
-	 * query string for DELETE requests).
+	 * The strategy identifies the WP user to the service (the OAuth path appends the `user_id`
+	 * query parameter to the DELETE), so no body is built here. Note the legacy Token path may
+	 * provision a fresh JWT to authenticate the DELETE — Consent_Handler::revoke_consent()
+	 * invalidates any locally stored JWTs afterwards, so credentials never outlive consent.
 	 *
 	 * @param WP_User $user The WP user revoking consent.
 	 *

src/ai/authentication/application/oauth-auth-strategy.php

@@ -120,7 +120,7 @@ public function send( Request $request, WP_User $user ): Response {
 		];
 
 		if ( $method === Request::METHOD_POST ) {
-			$body            = \array_merge( $request->get_body(), [ 'user_id' => $user_id ] );
+			$body            = \array_merge( ( $request->get_body() ?? [] ), [ 'user_id' => $user_id ] );
 			$options['body'] = WPSEO_Utils::format_json_encode( $body );
 		}
 		else {

src/ai/authorization/application/token-manager.php

@@ -115,6 +115,9 @@ public function __construct(
 	/**
 	 * Invalidates the access token.
 	 *
+	 * The locally stored JWTs are always cleared, even when the remote invalidation fails — the
+	 * remote exception still propagates to the caller, but no credentials are left behind.
+	 *
 	 * @param int $user_id The user ID.
 	 *
 	 * @return void
@@ -135,26 +138,26 @@ public function token_invalidate( int $user_id ): void {
 			$access_jwt = '';
 		}
 
-		$request_body    = [
-			'user_id' => (string) $user_id,
-		];
 		$request_headers = [
 			'Authorization' => "Bearer $access_jwt",
 		];
 
 		try {
+			// The endpoint takes no request body; the user is identified by the access token.
 			$this->request_handler->handle(
 				new Request(
 					'/token/invalidate',
-					$request_body,
+					[],
 					$request_headers,
 				),
 			);
 		} catch ( Unauthorized_Exception | Forbidden_Exception $e ) { // phpcs:ignore Generic.CodeAnalysis.EmptyStatement.DetectedCatch -- Reason: Ignored on purpose.
 			// If the credentials in our request were already invalid, our job is done and we continue to remove the tokens client-side.
+		} finally {
+			// Always clear the local tokens, even when the remote invalidation fails with an exception
+			// that propagates: leaving credentials behind would contradict the intent of invalidating.
+			$this->clear_tokens( $user_id );
 		}
-
-		$this->clear_tokens( $user_id );
 	}
 
 	/**
@@ -169,6 +172,31 @@ public function clear_tokens( int $user_id ): void {
 		$this->refresh_token_repository->delete_token( $user_id );
 	}
 
+	/**
+	 * Checks whether any JWT (access or refresh) is stored locally for the user.
+	 *
+	 * @param int $user_id The user ID.
+	 *
+	 * @return bool Whether a locally stored JWT exists.
+	 */
+	public function has_local_tokens( int $user_id ): bool {
+		try {
+			$this->access_token_repository->get_token( $user_id );
+
+			return true;
+		} catch ( RuntimeException $e ) { // phpcs:ignore Generic.CodeAnalysis.EmptyStatement.DetectedCatch -- Reason: Ignored on purpose.
+			// No access token; fall through to the refresh token check.
+		}
+
+		try {
+			$this->refresh_token_repository->get_token( $user_id );
+
+			return true;
+		} catch ( RuntimeException $e ) {
+			return false;
+		}
+	}
+
 	/**
 	 * Requests a new set of JWT tokens.
 	 *

src/ai/consent/application/consent-handler.php

@@ -7,6 +7,7 @@
 use RuntimeException;
 use WP_User;
 use Yoast\WP\SEO\AI\Authentication\Application\AI_Request_Sender_Factory;
+use Yoast\WP\SEO\AI\Authorization\Application\Token_Manager;
 use Yoast\WP\SEO\AI\HTTP_Request\Domain\Exceptions\Bad_Request_Exception;
 use Yoast\WP\SEO\AI\HTTP_Request\Domain\Exceptions\Consent_Required_Exception;
 use Yoast\WP\SEO\AI\HTTP_Request\Domain\Exceptions\Forbidden_Exception;
@@ -43,18 +44,28 @@ class Consent_Handler implements Consent_Handler_Interface {
 	 */
 	private $ai_request_sender_factory;
 
+	/**
+	 * The token manager, used to invalidate leftover legacy JWTs when consent is revoked.
+	 *
+	 * @var Token_Manager
+	 */
+	private $token_manager;
+
 	/**
 	 * Class constructor.
 	 *
 	 * @param User_Helper               $user_helper               The user helper.
 	 * @param AI_Request_Sender_Factory $ai_request_sender_factory The AI request sender factory.
+	 * @param Token_Manager             $token_manager             The token manager.
 	 */
 	public function __construct(
 		User_Helper $user_helper,
-		AI_Request_Sender_Factory $ai_request_sender_factory
+		AI_Request_Sender_Factory $ai_request_sender_factory,
+		Token_Manager $token_manager
 	) {
 		$this->user_helper               = $user_helper;
 		$this->ai_request_sender_factory = $ai_request_sender_factory;
+		$this->token_manager             = $token_manager;
 	}
 
 	// phpcs:disable Squiz.Commenting.FunctionCommentThrowTag.WrongNumber -- PHPCS doesn't take into account exceptions thrown in called methods.
@@ -99,8 +110,11 @@ public function grant_consent( int $user_id ) {
 	 * Revokes the user's consent on the Yoast AI service and clears the local user meta.
 	 *
 	 * Security-first: the local meta is always cleared before the remote call, so consent is
-	 * revoked locally even if the remote DELETE fails. Any HTTP-layer exception is propagated
-	 * and its management is deferred to the caller.
+	 * revoked locally even if the remote `DELETE /user/consent` fails. Any locally stored legacy
+	 * JWTs are then invalidated regardless of the remote outcome — credentials must not outlive
+	 * consent. The invalidation runs after the DELETE on purpose: the legacy Token path may mint
+	 * a fresh JWT to authenticate the DELETE, and invalidating afterwards catches that token too.
+	 * Any HTTP-layer exception is propagated and its management is deferred to the caller.
 	 *
 	 * @param int $user_id The user ID.
 	 *
@@ -129,7 +143,16 @@ public function revoke_consent( int $user_id ) {
 		// Local consent is always revoked regardless of remote failures.
 		$this->user_helper->delete_meta( $user_id, '_yoast_wpseo_ai_consent' );
 
-		$this->ai_request_sender_factory->create( $user )->revoke_consent( $user );
+		try {
+			$this->ai_request_sender_factory->create( $user )->revoke_consent( $user );
+		} finally {
+			// Invalidate the legacy JWTs — including ones minted to authenticate the DELETE above —
+			// so credentials never outlive consent. Skipped when no local JWTs exist (the OAuth path
+			// without a leftover pre-OAuth grant).
+			if ( $this->token_manager->has_local_tokens( $user_id ) ) {
+				$this->token_manager->token_invalidate( $user_id );
+			}
+		}
 	}
 
 	// phpcs:enable Squiz.Commenting.FunctionCommentThrowTag.WrongNumber