| @@ -16,10 +16,11 @@ | ||
| 16 | 16 | * Security contract: |
| 17 | 17 | * - PKCE S256 REQUIRED (OAuth 2.1 public clients); codes are single-use, |
| 18 | 18 | * 60 s TTL, bound to client_id + redirect_uri + challenge. |
| 19 | 19 | * - /authorize gates on manage_options — only an admin can grant access. |
| 20 | - * - Access/refresh tokens stored only as SHA-256 hashes; the raw value | |
| 21 | - * exists solely in the /token response. Constant-time comparison. | |
| 20 | + * - Authorization codes and access/refresh tokens stored only as SHA-256 | |
| 21 | + * hashes; the raw value exists solely in the response that hands it out. | |
| 22 | + * Constant-time comparison. | |
| 22 | 23 | * - Tokens carry the read/write scope model; a read-only grant refuses |
| 23 | 24 | * every write tool, exactly like a read-only pairing token. |
| 24 | 25 | * |
| 25 | 26 | * State lives in the `thinkrank_mcp_oauth` option (clients, codes, tokens, |
| @@ -47,8 +48,25 @@ | ||
| 47 | 48 | */ |
| 48 | 49 | public const OPTION = 'thinkrank_mcp_oauth'; |
| 49 | 50 | |
| 50 | 51 | /** |
| 52 | + * Per-client "last used" stamps, kept OUT of self::OPTION. | |
| 53 | + * | |
| 54 | + * Every authenticated MCP call used to stamp this inside the credential | |
| 55 | + * option, which meant ordinary tool traffic did a read-modify-write of the | |
| 56 | + * whole client/code/token/refresh store. A tool call overlapping a token | |
| 57 | + * refresh could write back its stale snapshot and erase a token the server | |
| 58 | + * had just minted — the client then holds an access token the server has | |
| 59 | + * no record of, and every later call 401s (#485). | |
| 60 | + * | |
| 61 | + * A cosmetic timestamp has no business sharing a store with credentials, | |
| 62 | + * so it lives in its own option. Losing a race here costs one stamp. | |
| 63 | + * | |
| 64 | + * @since 2.1.0 | |
| 65 | + */ | |
| 66 | + public const LAST_USED_OPTION = 'thinkrank_mcp_oauth_last_used'; | |
| 67 | + | |
| 68 | + /** | |
| 51 | 69 | * Authorization-code lifetime (seconds). Deliberately short. |
| 52 | 70 | */ |
| 53 | 71 | private const CODE_TTL = 60; |
| 54 | 72 | |
| @@ -74,8 +92,26 @@ | ||
| 74 | 92 | */ |
| 75 | 93 | private const LAST_USED_THROTTLE = 60; |
| 76 | 94 | |
| 77 | 95 | /** |
| 96 | + * Seconds to wait for the advisory lock before giving up and proceeding | |
| 97 | + * unguarded. Short: these are user-facing OAuth endpoints, and waiting is | |
| 98 | + * worse than the small race we are narrowing. | |
| 99 | + * | |
| 100 | + * @since 2.1.0 | |
| 101 | + */ | |
| 102 | + private const LOCK_TIMEOUT = 3; | |
| 103 | + | |
| 104 | + /** | |
| 105 | + * Nesting depth of mutate() on this request, so a mutation that calls | |
| 106 | + * another (grant -> mint) releases the lock once, at the outermost exit. | |
| 107 | + * | |
| 108 | + * @since 2.1.0 | |
| 109 | + * @var int | |
| 110 | + */ | |
| 111 | + private static int $lock_depth = 0; | |
| 112 | + | |
| 113 | + /** | |
| 78 | 114 | * How many registered clients to keep. RFC 7591 registration is open by |
| 79 | 115 | * necessity — a client must register BEFORE it can hold any credential — |
| 80 | 116 | * so without a cap anyone on the internet can grow this option without |
| 81 | 117 | * bound, and every state() read pays for it. Clients holding a live token |
| @@ -144,8 +180,41 @@ | ||
| 144 | 180 | public static function register_url(): string { |
| 145 | 181 | return rest_url( 'thinkrank/v1/mcp/oauth/register' ); |
| 146 | 182 | } |
| 147 | 183 | |
| 184 | + /** | |
| 185 | + * The protected-resource metadata URL the 401 challenge advertises. | |
| 186 | + * | |
| 187 | + * REST-served, NOT the RFC 9728 path-insert form. The path-insert URL | |
| 188 | + * lives under the site root's /.well-known/ directory, and some hosts | |
| 189 | + * (SiteGround shared hosting confirmed, see #374) resolve that directory | |
| 190 | + * at their Nginx edge as physical files — the request 404s before | |
| 191 | + * WordPress runs, and the connecting client reports "server does not | |
| 192 | + * implement OAuth" on its very first fetch. The challenge parameter is an | |
| 193 | + * explicit pointer (that is what it exists for), so pointing it at a | |
| 194 | + * /wp-json/ URL is spec-clean and reaches WordPress on every host and | |
| 195 | + * permalink structure. The well-known variants stay served for clients | |
| 196 | + * that ignore the pointer and derive the URL themselves. | |
| 197 | + * | |
| 198 | + * @return string | |
| 199 | + */ | |
| 200 | + public static function resource_metadata_url(): string { | |
| 201 | + /** | |
| 202 | + * Filter the resource_metadata URL advertised in the WWW-Authenticate | |
| 203 | + * challenge, for hosts where neither the REST route nor the | |
| 204 | + * /.well-known/ forms are reachable and the metadata must be served | |
| 205 | + * from somewhere custom (a CDN, a static file, another domain). | |
| 206 | + * | |
| 207 | + * @since 1.32.0 | |
| 208 | + * | |
| 209 | + * @param string $url The advertised protected-resource metadata URL. | |
| 210 | + */ | |
| 211 | + return apply_filters( | |
| 212 | + 'thinkrank_mcp_resource_metadata_url', | |
| 213 | + rest_url( 'thinkrank/v1/mcp/oauth/protected-resource' ) | |
| 214 | + ); | |
| 215 | + } | |
| 216 | + | |
| 148 | 217 | // -- Discovery documents (RFC 8414 / RFC 9728) ----------------------- |
| 149 | 218 | |
| 150 | 219 | /** |
| 151 | 220 | * RFC 9728 protected-resource metadata — tells the client which |
| @@ -206,16 +275,18 @@ | ||
| 206 | 275 | |
| 207 | 276 | $name = isset( $body['client_name'] ) ? sanitize_text_field( (string) $body['client_name'] ) : 'MCP Client'; |
| 208 | 277 | $client_id = 'trk_' . bin2hex( random_bytes( 16 ) ); |
| 209 | 278 | |
| 210 | - $state = self::state(); | |
| 211 | - $state['clients'][ $client_id ] = [ | |
| 212 | - 'redirect_uris' => $redirect_uris, | |
| 213 | - 'name' => $name, | |
| 214 | - 'created' => time(), | |
| 215 | - ]; | |
| 216 | - $state['clients'] = self::prune_clients( $state ); | |
| 217 | - self::save( $state ); | |
| 279 | + self::mutate( | |
| 280 | + static function ( array &$state ) use ( $client_id, $redirect_uris, $name ): void { | |
| 281 | + $state['clients'][ $client_id ] = [ | |
| 282 | + 'redirect_uris' => $redirect_uris, | |
| 283 | + 'name' => $name, | |
| 284 | + 'created' => time(), | |
| 285 | + ]; | |
| 286 | + $state['clients'] = self::prune_clients( $state ); | |
| 287 | + } | |
| 288 | + ); | |
| 218 | 289 | |
| 219 | 290 | return [ |
| 220 | 291 | 'client_id' => $client_id, |
| 221 | 292 | 'client_id_issued_at' => time(), |
| @@ -275,8 +346,22 @@ | ||
| 275 | 346 | 'redirectable' => true, |
| 276 | 347 | ] |
| 277 | 348 | ); |
| 278 | 349 | } |
| 350 | + // The challenge reaches us verbatim now (#487), so it is checked | |
| 351 | + // against its own character set rather than cleaned as display text. | |
| 352 | + // RFC 7636 unreserved base64url; an S256 challenge is 43 characters, | |
| 353 | + // the wider bound leaves room for a client that pads. | |
| 354 | + if ( ! preg_match( '/^[A-Za-z0-9\-._~]{43,128}$/', $challenge ) ) { | |
| 355 | + return new \WP_Error( | |
| 356 | + 'invalid_request', | |
| 357 | + __( 'code_challenge is not a valid S256 challenge.', 'thinkrank' ), | |
| 358 | + [ | |
| 359 | + 'status' => 400, | |
| 360 | + 'redirectable' => true, | |
| 361 | + ] | |
| 362 | + ); | |
| 363 | + } | |
| 279 | 364 | |
| 280 | 365 | return [ |
| 281 | 366 | 'client_id' => $client_id, |
| 282 | 367 | 'client_name' => $client['name'], |
| @@ -296,19 +381,29 @@ | ||
| 296 | 381 | * @param int $user_id Approving admin user id. |
| 297 | 382 | * @return string The authorization code. |
| 298 | 383 | */ |
| 299 | 384 | public static function issue_code( array $req, int $user_id ): string { |
| 300 | - $code = bin2hex( random_bytes( 32 ) ); | |
| 301 | - $state = self::state(); | |
| 302 | - $state['codes'][ $code ] = [ | |
| 303 | - 'client_id' => $req['client_id'], | |
| 304 | - 'redirect_uri' => $req['redirect_uri'], | |
| 305 | - 'challenge' => $req['code_challenge'], | |
| 306 | - 'scope' => $req['scope'], | |
| 307 | - 'user_id' => $user_id, | |
| 308 | - 'expires' => time() + self::CODE_TTL, | |
| 309 | - ]; | |
| 310 | - self::save( $state ); | |
| 385 | + $code = bin2hex( random_bytes( 32 ) ); | |
| 386 | + | |
| 387 | + // Keyed by hash, like access and refresh tokens. The authorization | |
| 388 | + // code is a bearer credential too, and this file's own contract says | |
| 389 | + // the raw value exists solely in the response that hands it out — the | |
| 390 | + // code was the one exception (#488). The exposure is small (60 s TTL, | |
| 391 | + // single use, bound to client_id + redirect_uri + PKCE) but #396 made | |
| 392 | + // exactly that argument about the pairing token and still hashed it. | |
| 393 | + self::mutate( | |
| 394 | + static function ( array &$state ) use ( $code, $req, $user_id ): void { | |
| 395 | + $state['codes'][ self::hash( $code ) ] = [ | |
| 396 | + 'client_id' => $req['client_id'], | |
| 397 | + 'redirect_uri' => $req['redirect_uri'], | |
| 398 | + 'challenge' => $req['code_challenge'], | |
| 399 | + 'scope' => $req['scope'], | |
| 400 | + 'user_id' => $user_id, | |
| 401 | + 'expires' => time() + self::CODE_TTL, | |
| 402 | + ]; | |
| 403 | + } | |
| 404 | + ); | |
| 405 | + | |
| 311 | 406 | return $code; |
| 312 | 407 | } |
| 313 | 408 | |
| 314 | 409 | // -- Token endpoint -------------------------------------------------- |
| @@ -343,18 +438,34 @@ | ||
| 343 | 438 | $client_id = isset( $body['client_id'] ) ? (string) $body['client_id'] : ''; |
| 344 | 439 | $redirect_uri = isset( $body['redirect_uri'] ) ? (string) $body['redirect_uri'] : ''; |
| 345 | 440 | $verifier = isset( $body['code_verifier'] ) ? (string) $body['code_verifier'] : ''; |
| 346 | 441 | |
| 347 | - $state = self::state(); | |
| 348 | - if ( '' === $code || ! isset( $state['codes'][ $code ] ) ) { | |
| 442 | + // Claim the code and remove it in one guarded read-modify-write. | |
| 443 | + // Single-use has to mean single-use: looking it up, saving the removal, | |
| 444 | + // and letting a concurrent writer restore its pre-removal snapshot put | |
| 445 | + // a spent code back in the store (#485). Looked up by hash, because | |
| 446 | + // that is how issue_code() stores it (#488). | |
| 447 | + $entry = self::mutate( | |
| 448 | + static function ( array &$state ) use ( $code ) { | |
| 449 | + $chash = self::hash( $code ); | |
| 450 | + | |
| 451 | + if ( '' === $code || ! isset( $state['codes'][ $chash ] ) ) { | |
| 452 | + return null; | |
| 453 | + } | |
| 454 | + | |
| 455 | + $claimed = $state['codes'][ $chash ]; | |
| 456 | + | |
| 457 | + // Removed whether or not verification below passes. | |
| 458 | + unset( $state['codes'][ $chash ] ); | |
| 459 | + | |
| 460 | + return $claimed; | |
| 461 | + } | |
| 462 | + ); | |
| 463 | + | |
| 464 | + if ( null === $entry ) { | |
| 349 | 465 | return self::oauth_error( 'invalid_grant', 'Unknown or expired authorization code.' ); |
| 350 | 466 | } |
| 351 | - $entry = $state['codes'][ $code ]; | |
| 352 | 467 | |
| 353 | - // Single-use: remove immediately whether or not verification passes. | |
| 354 | - unset( $state['codes'][ $code ] ); | |
| 355 | - self::save( $state ); | |
| 356 | - | |
| 357 | 468 | if ( $entry['expires'] < time() ) { |
| 358 | 469 | return self::oauth_error( 'invalid_grant', 'Authorization code expired.' ); |
| 359 | 470 | } |
| 360 | 471 | if ( ! hash_equals( (string) $entry['client_id'], $client_id ) ) { |
| @@ -381,24 +492,39 @@ | ||
| 381 | 492 | private static function grant_refresh_token( array $body ) { |
| 382 | 493 | $refresh = isset( $body['refresh_token'] ) ? (string) $body['refresh_token'] : ''; |
| 383 | 494 | $client_id = isset( $body['client_id'] ) ? (string) $body['client_id'] : ''; |
| 384 | 495 | |
| 385 | - $state = self::state(); | |
| 386 | 496 | $rhash = self::hash( $refresh ); |
| 387 | - if ( '' === $refresh || ! isset( $state['refresh'][ $rhash ] ) ) { | |
| 388 | - return self::oauth_error( 'invalid_grant', 'Unknown refresh token.' ); | |
| 497 | + | |
| 498 | + // Look up and rotate under one guard. A mismatched client_id must not | |
| 499 | + // consume the token, so the check happens inside the mutation. | |
| 500 | + $claim = self::mutate( | |
| 501 | + static function ( array &$state ) use ( $refresh, $rhash, $client_id ): array { | |
| 502 | + if ( '' === $refresh || ! isset( $state['refresh'][ $rhash ] ) ) { | |
| 503 | + return [ 'error' => 'Unknown refresh token.' ]; | |
| 504 | + } | |
| 505 | + | |
| 506 | + $entry = $state['refresh'][ $rhash ]; | |
| 507 | + | |
| 508 | + if ( '' !== $client_id && ! hash_equals( (string) $entry['client_id'], $client_id ) ) { | |
| 509 | + return [ 'error' => 'client_id mismatch.' ]; | |
| 510 | + } | |
| 511 | + | |
| 512 | + // Rotate: drop old refresh + its access token. | |
| 513 | + unset( $state['refresh'][ $rhash ] ); | |
| 514 | + if ( isset( $entry['access_hash'] ) ) { | |
| 515 | + unset( $state['tokens'][ $entry['access_hash'] ] ); | |
| 516 | + } | |
| 517 | + | |
| 518 | + return [ 'entry' => $entry ]; | |
| 519 | + } | |
| 520 | + ); | |
| 521 | + | |
| 522 | + if ( isset( $claim['error'] ) ) { | |
| 523 | + return self::oauth_error( 'invalid_grant', (string) $claim['error'] ); | |
| 389 | 524 | } |
| 390 | - $entry = $state['refresh'][ $rhash ]; | |
| 391 | - if ( '' !== $client_id && ! hash_equals( (string) $entry['client_id'], $client_id ) ) { | |
| 392 | - return self::oauth_error( 'invalid_grant', 'client_id mismatch.' ); | |
| 393 | - } | |
| 394 | 525 | |
| 395 | - // Rotate: drop old refresh + its access token. | |
| 396 | - unset( $state['refresh'][ $rhash ] ); | |
| 397 | - if ( isset( $entry['access_hash'] ) ) { | |
| 398 | - unset( $state['tokens'][ $entry['access_hash'] ] ); | |
| 399 | - } | |
| 400 | - self::save( $state ); | |
| 526 | + $entry = $claim['entry']; | |
| 401 | 527 | |
| 402 | 528 | return self::mint_tokens( (string) $entry['client_id'], (string) $entry['scope'], (int) $entry['user_id'] ); |
| 403 | 529 | } |
| 404 | 530 | |
| @@ -416,24 +542,26 @@ | ||
| 416 | 542 | $refresh = bin2hex( random_bytes( 32 ) ); |
| 417 | 543 | $ahash = self::hash( $access ); |
| 418 | 544 | $rhash = self::hash( $refresh ); |
| 419 | 545 | |
| 420 | - $state = self::state(); | |
| 421 | - $state['tokens'][ $ahash ] = [ | |
| 422 | - 'client_id' => $client_id, | |
| 423 | - 'scope' => $scope, | |
| 424 | - 'user_id' => $user_id, | |
| 425 | - 'expires' => time() + self::ACCESS_TTL, | |
| 426 | - 'refresh' => $rhash, | |
| 427 | - ]; | |
| 428 | - $state['refresh'][ $rhash ] = [ | |
| 429 | - 'access_hash' => $ahash, | |
| 430 | - 'client_id' => $client_id, | |
| 431 | - 'scope' => $scope, | |
| 432 | - 'user_id' => $user_id, | |
| 433 | - 'expires' => time() + self::REFRESH_TTL, | |
| 434 | - ]; | |
| 435 | - self::save( $state ); | |
| 546 | + self::mutate( | |
| 547 | + static function ( array &$state ) use ( $ahash, $rhash, $client_id, $scope, $user_id ): void { | |
| 548 | + $state['tokens'][ $ahash ] = [ | |
| 549 | + 'client_id' => $client_id, | |
| 550 | + 'scope' => $scope, | |
| 551 | + 'user_id' => $user_id, | |
| 552 | + 'expires' => time() + self::ACCESS_TTL, | |
| 553 | + 'refresh' => $rhash, | |
| 554 | + ]; | |
| 555 | + $state['refresh'][ $rhash ] = [ | |
| 556 | + 'access_hash' => $ahash, | |
| 557 | + 'client_id' => $client_id, | |
| 558 | + 'scope' => $scope, | |
| 559 | + 'user_id' => $user_id, | |
| 560 | + 'expires' => time() + self::REFRESH_TTL, | |
| 561 | + ]; | |
| 562 | + } | |
| 563 | + ); | |
| 436 | 564 | |
| 437 | 565 | return [ |
| 438 | 566 | 'access_token' => $access, |
| 439 | 567 | 'token_type' => 'Bearer', |
| @@ -467,18 +595,15 @@ | ||
| 467 | 595 | return null; |
| 468 | 596 | } |
| 469 | 597 | |
| 470 | 598 | // Record activity against the owning client so the "Connected AI apps" |
| 471 | - // list can show a last-used date. Throttled + stored on the client | |
| 472 | - // record so it survives access-token rotation. | |
| 599 | + // list can show a last-used date. Throttled, and written to its own | |
| 600 | + // option: this runs on every authenticated MCP call, and writing it | |
| 601 | + // back into the credential store meant ordinary tool traffic could | |
| 602 | + // erase a token minted by an overlapping refresh (#485). | |
| 473 | 603 | $client_id = (string) $entry['client_id']; |
| 474 | - $now = time(); | |
| 475 | 604 | if ( isset( $state['clients'][ $client_id ] ) && is_array( $state['clients'][ $client_id ] ) ) { |
| 476 | - $last = isset( $state['clients'][ $client_id ]['last_used'] ) ? (int) $state['clients'][ $client_id ]['last_used'] : 0; | |
| 477 | - if ( $now - $last >= self::LAST_USED_THROTTLE ) { | |
| 478 | - $state['clients'][ $client_id ]['last_used'] = $now; | |
| 479 | - self::save( $state ); | |
| 480 | - } | |
| 605 | + self::touch_last_used( $client_id, array_keys( $state['clients'] ) ); | |
| 481 | 606 | } |
| 482 | 607 | |
| 483 | 608 | return [ |
| 484 | 609 | 'client_id' => $client_id, |
| @@ -507,8 +632,9 @@ | ||
| 507 | 632 | * @return void |
| 508 | 633 | */ |
| 509 | 634 | public static function revoke_all(): void { |
| 510 | 635 | delete_option( self::OPTION ); |
| 636 | + delete_option( self::LAST_USED_OPTION ); | |
| 511 | 637 | } |
| 512 | 638 | |
| 513 | 639 | /** |
| 514 | 640 | * The OAuth clients currently holding a live grant, for the "Connected AI |
| @@ -519,9 +645,10 @@ | ||
| 519 | 645 | * |
| 520 | 646 | * @return array<int,array{client_id:string,name:string,scope:string,read_only:bool,user_id:int,connected_at:int,last_used:int}> |
| 521 | 647 | */ |
| 522 | 648 | public static function connected_apps(): array { |
| 523 | - $state = self::state(); | |
| 649 | + $state = self::state(); | |
| 650 | + $last_used = self::last_used_map(); | |
| 524 | 651 | |
| 525 | 652 | // Collect the scope + approving user per active client. Refresh tokens |
| 526 | 653 | // are the durable grant, so prefer them; fall back to access tokens. |
| 527 | 654 | $active = []; |
| @@ -547,9 +674,13 @@ | ||
| 547 | 674 | 'scope' => $info['scope'], |
| 548 | 675 | 'read_only' => self::scope_is_read_only( $info['scope'] ), |
| 549 | 676 | 'user_id' => $info['user_id'], |
| 550 | 677 | 'connected_at' => isset( $client['created'] ) ? (int) $client['created'] : 0, |
| 551 | - 'last_used' => isset( $client['last_used'] ) ? (int) $client['last_used'] : 0, | |
| 678 | + // Legacy fallback: stamps written before #485 still sit on the | |
| 679 | + // client record, so an existing install keeps its dates. | |
| 680 | + 'last_used' => isset( $last_used[ $cid ] ) | |
| 681 | + ? (int) $last_used[ $cid ] | |
| 682 | + : ( isset( $client['last_used'] ) ? (int) $client['last_used'] : 0 ), | |
| 552 | 683 | ]; |
| 553 | 684 | } |
| 554 | 685 | |
| 555 | 686 | // Newest connection first. |
| @@ -583,23 +714,31 @@ | ||
| 583 | 714 | public static function revoke_client( string $client_id ): bool { |
| 584 | 715 | if ( '' === $client_id ) { |
| 585 | 716 | return false; |
| 586 | 717 | } |
| 587 | - $state = self::state(); | |
| 588 | - $removed = false; | |
| 718 | + $removed = self::mutate( | |
| 719 | + static function ( array &$state ) use ( $client_id ): bool { | |
| 720 | + $found = false; | |
| 589 | 721 | |
| 590 | - foreach ( [ 'tokens', 'refresh', 'codes' ] as $bucket ) { | |
| 591 | - foreach ( $state[ $bucket ] as $key => $entry ) { | |
| 592 | - if ( isset( $entry['client_id'] ) && (string) $entry['client_id'] === $client_id ) { | |
| 593 | - unset( $state[ $bucket ][ $key ] ); | |
| 594 | - $removed = true; | |
| 722 | + foreach ( [ 'tokens', 'refresh', 'codes' ] as $bucket ) { | |
| 723 | + foreach ( $state[ $bucket ] as $key => $entry ) { | |
| 724 | + if ( isset( $entry['client_id'] ) && (string) $entry['client_id'] === $client_id ) { | |
| 725 | + unset( $state[ $bucket ][ $key ] ); | |
| 726 | + $found = true; | |
| 727 | + } | |
| 728 | + } | |
| 595 | 729 | } |
| 730 | + | |
| 731 | + return $found; | |
| 596 | 732 | } |
| 597 | - } | |
| 733 | + ); | |
| 598 | 734 | |
| 599 | 735 | if ( $removed ) { |
| 600 | - self::save( $state ); | |
| 736 | + $map = self::last_used_map(); | |
| 737 | + unset( $map[ $client_id ] ); | |
| 738 | + update_option( self::LAST_USED_OPTION, $map, false ); | |
| 601 | 739 | } |
| 740 | + | |
| 602 | 741 | return $removed; |
| 603 | 742 | } |
| 604 | 743 | |
| 605 | 744 | // -- State + helpers ------------------------------------------------- |
| @@ -643,13 +782,172 @@ | ||
| 643 | 782 | |
| 644 | 783 | /** |
| 645 | 784 | * Persist state (autoload off — hot-write, request-scoped option). |
| 646 | 785 | * |
| 786 | + * Private on purpose: every mutation goes through mutate(), so that the | |
| 787 | + * state being written was read inside the same guard. | |
| 788 | + * | |
| 647 | 789 | * @param array<string,mixed> $state State to persist. |
| 648 | 790 | * @return void |
| 649 | 791 | */ |
| 650 | 792 | private static function save( array $state ): void { |
| 651 | 793 | update_option( self::OPTION, $state, false ); |
| 794 | + } | |
| 795 | + | |
| 796 | + /** | |
| 797 | + * Read-modify-write the OAuth state under a guard, re-reading inside it. | |
| 798 | + * | |
| 799 | + * Clients, codes, access tokens and refresh tokens share one option, and | |
| 800 | + * every mutation used to read a snapshot at the top of the request and | |
| 801 | + * write the whole thing back later. Two overlapping requests therefore had | |
| 802 | + * one silently erase the other's work — the damaging order being a tool | |
| 803 | + * call writing back a pre-refresh snapshot over a token pair that had just | |
| 804 | + * been minted, leaving the client holding an access token the server has no | |
| 805 | + * record of (#485). | |
| 806 | + * | |
| 807 | + * The mutator receives the state by reference and may return a value, which | |
| 808 | + * is handed back to the caller — so a caller can claim-and-remove (a | |
| 809 | + * single-use code, a rotating refresh token) without the lookup and the | |
| 810 | + * removal being separate writes. | |
| 811 | + * | |
| 812 | + * @param callable $mutator function ( array &$state ): mixed | |
| 813 | + * @return mixed Whatever the mutator returned. | |
| 814 | + */ | |
| 815 | + private static function mutate( callable $mutator ) { | |
| 816 | + $locked = self::lock(); | |
| 817 | + | |
| 818 | + try { | |
| 819 | + $state = self::state(); | |
| 820 | + $result = $mutator( $state ); | |
| 821 | + self::save( $state ); | |
| 822 | + } finally { | |
| 823 | + if ( $locked ) { | |
| 824 | + self::unlock(); | |
| 825 | + } | |
| 826 | + } | |
| 827 | + | |
| 828 | + return $result; | |
| 829 | + } | |
| 830 | + | |
| 831 | + /** | |
| 832 | + * Take the cross-request advisory lock guarding self::OPTION. | |
| 833 | + * | |
| 834 | + * MySQL GET_LOCK is what WordPress gives us that actually holds ACROSS | |
| 835 | + * processes — wp_cache_add() is per-request without a persistent object | |
| 836 | + * cache, which is exactly the configuration this bug bites hardest on. | |
| 837 | + * The name is namespaced by database + table prefix because GET_LOCK names | |
| 838 | + * are server-wide and shared MySQL hosts are the common case. | |
| 839 | + * | |
| 840 | + * Best-effort by design: a host where the lock cannot be taken (SQLite | |
| 841 | + * drop-in, a proxy that does not support session locks, contention past | |
| 842 | + * the timeout) proceeds unguarded, which is exactly today's behaviour | |
| 843 | + * rather than a new failure. | |
| 844 | + * | |
| 845 | + * @return bool Whether the lock is held. | |
| 846 | + */ | |
| 847 | + private static function lock(): bool { | |
| 848 | + global $wpdb; | |
| 849 | + | |
| 850 | + // Already inside a guarded mutation on this request (grant -> mint). | |
| 851 | + // MySQL's lock is re-entrant per session; the depth counter is what | |
| 852 | + // keeps the release paired with the outermost acquire. | |
| 853 | + if ( self::$lock_depth > 0 ) { | |
| 854 | + ++self::$lock_depth; | |
| 855 | + return true; | |
| 856 | + } | |
| 857 | + | |
| 858 | + if ( ! isset( $wpdb ) || ! is_object( $wpdb ) ) { | |
| 859 | + return false; | |
| 860 | + } | |
| 861 | + | |
| 862 | + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- advisory lock, not cacheable data. | |
| 863 | + $got = $wpdb->get_var( $wpdb->prepare( 'SELECT GET_LOCK(%s, %d)', self::lock_name(), self::LOCK_TIMEOUT ) ); | |
| 864 | + | |
| 865 | + if ( '1' !== (string) $got ) { | |
| 866 | + return false; | |
| 867 | + } | |
| 868 | + | |
| 869 | + self::$lock_depth = 1; | |
| 870 | + | |
| 871 | + return true; | |
| 872 | + } | |
| 873 | + | |
| 874 | + /** | |
| 875 | + * Release the advisory lock taken by lock(). Only the outermost mutation | |
| 876 | + * actually releases it. | |
| 877 | + * | |
| 878 | + * @return void | |
| 879 | + */ | |
| 880 | + private static function unlock(): void { | |
| 881 | + global $wpdb; | |
| 882 | + | |
| 883 | + if ( self::$lock_depth <= 0 ) { | |
| 884 | + return; | |
| 885 | + } | |
| 886 | + | |
| 887 | + --self::$lock_depth; | |
| 888 | + | |
| 889 | + if ( self::$lock_depth > 0 || ! isset( $wpdb ) || ! is_object( $wpdb ) ) { | |
| 890 | + return; | |
| 891 | + } | |
| 892 | + | |
| 893 | + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- advisory lock, not cacheable data. | |
| 894 | + $wpdb->get_var( $wpdb->prepare( 'SELECT RELEASE_LOCK(%s)', self::lock_name() ) ); | |
| 895 | + } | |
| 896 | + | |
| 897 | + /** | |
| 898 | + * Lock name, inside MySQL's 64-character limit and unique per install. | |
| 899 | + * | |
| 900 | + * @return string | |
| 901 | + */ | |
| 902 | + private static function lock_name(): string { | |
| 903 | + global $wpdb; | |
| 904 | + | |
| 905 | + $prefix = isset( $wpdb ) && is_object( $wpdb ) ? (string) $wpdb->prefix : ''; | |
| 906 | + | |
| 907 | + return 'trk_mcp_oauth_' . md5( ( defined( 'DB_NAME' ) ? (string) DB_NAME : '' ) . '|' . $prefix ); | |
| 908 | + } | |
| 909 | + | |
| 910 | + /** | |
| 911 | + * Per-client last-used stamps, client_id => unix timestamp. | |
| 912 | + * | |
| 913 | + * @return array<string,int> | |
| 914 | + */ | |
| 915 | + private static function last_used_map(): array { | |
| 916 | + $stored = get_option( self::LAST_USED_OPTION, [] ); | |
| 917 | + | |
| 918 | + return is_array( $stored ) ? $stored : []; | |
| 919 | + } | |
| 920 | + | |
| 921 | + /** | |
| 922 | + * Stamp a client as having just been used, at most once per throttle | |
| 923 | + * window. Writes its own option, never the credential store. | |
| 924 | + * | |
| 925 | + * @param string $client_id Client to stamp. | |
| 926 | + * @param string[] $known_clients Client ids that still exist, so the map | |
| 927 | + * cannot outgrow the store it describes. | |
| 928 | + * @return void | |
| 929 | + */ | |
| 930 | + private static function touch_last_used( string $client_id, array $known_clients ): void { | |
| 931 | + $map = self::last_used_map(); | |
| 932 | + $now = time(); | |
| 933 | + $last = isset( $map[ $client_id ] ) ? (int) $map[ $client_id ] : 0; | |
| 934 | + | |
| 935 | + if ( $now - $last < self::LAST_USED_THROTTLE ) { | |
| 936 | + return; | |
| 937 | + } | |
| 938 | + | |
| 939 | + $map[ $client_id ] = $now; | |
| 940 | + | |
| 941 | + // Drop stamps for clients that are gone (revoked, pruned, expired). | |
| 942 | + $known = array_flip( $known_clients ); | |
| 943 | + foreach ( array_keys( $map ) as $id ) { | |
| 944 | + if ( ! isset( $known[ $id ] ) ) { | |
| 945 | + unset( $map[ $id ] ); | |
| 946 | + } | |
| 947 | + } | |
| 948 | + | |
| 949 | + update_option( self::LAST_USED_OPTION, $map, false ); | |
| 652 | 950 | } |
| 653 | 951 | |
| 654 | 952 | /** |
| 655 | 953 | * Look up a registered client. |
