| @@ -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 | |
| @@ -66,8 +84,49 @@ | ||
| 66 | 84 | * Scopes we advertise + honor. `mcp` is the umbrella scope MCP clients request. |
| 67 | 85 | */ |
| 68 | 86 | private const SUPPORTED_SCOPES = [ 'mcp', 'read', 'write' ]; |
| 69 | 87 | |
| 88 | + /** | |
| 89 | + * Throttle window (seconds) for per-client last-used writes — at most one | |
| 90 | + * option write per minute per client, so a busy connector can't turn every | |
| 91 | + * MCP call into a database write. | |
| 92 | + */ | |
| 93 | + private const LAST_USED_THROTTLE = 60; | |
| 94 | + | |
| 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 | + /** | |
| 114 | + * How many registered clients to keep. RFC 7591 registration is open by | |
| 115 | + * necessity — a client must register BEFORE it can hold any credential — | |
| 116 | + * so without a cap anyone on the internet can grow this option without | |
| 117 | + * bound, and every state() read pays for it. Clients holding a live token | |
| 118 | + * are never evicted, so the cap only ever discards abandoned registrations. | |
| 119 | + */ | |
| 120 | + private const MAX_CLIENTS = 50; | |
| 121 | + | |
| 122 | + /** | |
| 123 | + * How long an unused client registration survives (seconds). A client that | |
| 124 | + * registers and never completes the flow is abandoned; real ones exchange | |
| 125 | + * a code within a minute. | |
| 126 | + */ | |
| 127 | + private const CLIENT_TTL = 86400; // 24 hours. | |
| 128 | + | |
| 70 | 129 | // -- URLs ------------------------------------------------------------ |
| 71 | 130 | |
| 72 | 131 | /** |
| 73 | 132 | * The OAuth issuer identifier. Path-based (RFC 8414 §2 allows an issuer |
| @@ -121,8 +180,41 @@ | ||
| 121 | 180 | public static function register_url(): string { |
| 122 | 181 | return rest_url( 'thinkrank/v1/mcp/oauth/register' ); |
| 123 | 182 | } |
| 124 | 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 | + | |
| 125 | 217 | // -- Discovery documents (RFC 8414 / RFC 9728) ----------------------- |
| 126 | 218 | |
| 127 | 219 | /** |
| 128 | 220 | * RFC 9728 protected-resource metadata — tells the client which |
| @@ -183,15 +275,18 @@ | ||
| 183 | 275 | |
| 184 | 276 | $name = isset( $body['client_name'] ) ? sanitize_text_field( (string) $body['client_name'] ) : 'MCP Client'; |
| 185 | 277 | $client_id = 'trk_' . bin2hex( random_bytes( 16 ) ); |
| 186 | 278 | |
| 187 | - $state = self::state(); | |
| 188 | - $state['clients'][ $client_id ] = [ | |
| 189 | - 'redirect_uris' => $redirect_uris, | |
| 190 | - 'name' => $name, | |
| 191 | - 'created' => time(), | |
| 192 | - ]; | |
| 193 | - 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 | + ); | |
| 194 | 289 | |
| 195 | 290 | return [ |
| 196 | 291 | 'client_id' => $client_id, |
| 197 | 292 | 'client_id_issued_at' => time(), |
| @@ -251,8 +346,22 @@ | ||
| 251 | 346 | 'redirectable' => true, |
| 252 | 347 | ] |
| 253 | 348 | ); |
| 254 | 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 | + } | |
| 255 | 364 | |
| 256 | 365 | return [ |
| 257 | 366 | 'client_id' => $client_id, |
| 258 | 367 | 'client_name' => $client['name'], |
| @@ -272,19 +381,29 @@ | ||
| 272 | 381 | * @param int $user_id Approving admin user id. |
| 273 | 382 | * @return string The authorization code. |
| 274 | 383 | */ |
| 275 | 384 | public static function issue_code( array $req, int $user_id ): string { |
| 276 | - $code = bin2hex( random_bytes( 32 ) ); | |
| 277 | - $state = self::state(); | |
| 278 | - $state['codes'][ $code ] = [ | |
| 279 | - 'client_id' => $req['client_id'], | |
| 280 | - 'redirect_uri' => $req['redirect_uri'], | |
| 281 | - 'challenge' => $req['code_challenge'], | |
| 282 | - 'scope' => $req['scope'], | |
| 283 | - 'user_id' => $user_id, | |
| 284 | - 'expires' => time() + self::CODE_TTL, | |
| 285 | - ]; | |
| 286 | - 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 | + | |
| 287 | 406 | return $code; |
| 288 | 407 | } |
| 289 | 408 | |
| 290 | 409 | // -- Token endpoint -------------------------------------------------- |
| @@ -319,18 +438,34 @@ | ||
| 319 | 438 | $client_id = isset( $body['client_id'] ) ? (string) $body['client_id'] : ''; |
| 320 | 439 | $redirect_uri = isset( $body['redirect_uri'] ) ? (string) $body['redirect_uri'] : ''; |
| 321 | 440 | $verifier = isset( $body['code_verifier'] ) ? (string) $body['code_verifier'] : ''; |
| 322 | 441 | |
| 323 | - $state = self::state(); | |
| 324 | - 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 ) { | |
| 325 | 465 | return self::oauth_error( 'invalid_grant', 'Unknown or expired authorization code.' ); |
| 326 | 466 | } |
| 327 | - $entry = $state['codes'][ $code ]; | |
| 328 | 467 | |
| 329 | - // Single-use: remove immediately whether or not verification passes. | |
| 330 | - unset( $state['codes'][ $code ] ); | |
| 331 | - self::save( $state ); | |
| 332 | - | |
| 333 | 468 | if ( $entry['expires'] < time() ) { |
| 334 | 469 | return self::oauth_error( 'invalid_grant', 'Authorization code expired.' ); |
| 335 | 470 | } |
| 336 | 471 | if ( ! hash_equals( (string) $entry['client_id'], $client_id ) ) { |
| @@ -357,24 +492,39 @@ | ||
| 357 | 492 | private static function grant_refresh_token( array $body ) { |
| 358 | 493 | $refresh = isset( $body['refresh_token'] ) ? (string) $body['refresh_token'] : ''; |
| 359 | 494 | $client_id = isset( $body['client_id'] ) ? (string) $body['client_id'] : ''; |
| 360 | 495 | |
| 361 | - $state = self::state(); | |
| 362 | 496 | $rhash = self::hash( $refresh ); |
| 363 | - if ( '' === $refresh || ! isset( $state['refresh'][ $rhash ] ) ) { | |
| 364 | - 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'] ); | |
| 365 | 524 | } |
| 366 | - $entry = $state['refresh'][ $rhash ]; | |
| 367 | - if ( '' !== $client_id && ! hash_equals( (string) $entry['client_id'], $client_id ) ) { | |
| 368 | - return self::oauth_error( 'invalid_grant', 'client_id mismatch.' ); | |
| 369 | - } | |
| 370 | 525 | |
| 371 | - // Rotate: drop old refresh + its access token. | |
| 372 | - unset( $state['refresh'][ $rhash ] ); | |
| 373 | - if ( isset( $entry['access_hash'] ) ) { | |
| 374 | - unset( $state['tokens'][ $entry['access_hash'] ] ); | |
| 375 | - } | |
| 376 | - self::save( $state ); | |
| 526 | + $entry = $claim['entry']; | |
| 377 | 527 | |
| 378 | 528 | return self::mint_tokens( (string) $entry['client_id'], (string) $entry['scope'], (int) $entry['user_id'] ); |
| 379 | 529 | } |
| 380 | 530 | |
| @@ -392,24 +542,26 @@ | ||
| 392 | 542 | $refresh = bin2hex( random_bytes( 32 ) ); |
| 393 | 543 | $ahash = self::hash( $access ); |
| 394 | 544 | $rhash = self::hash( $refresh ); |
| 395 | 545 | |
| 396 | - $state = self::state(); | |
| 397 | - $state['tokens'][ $ahash ] = [ | |
| 398 | - 'client_id' => $client_id, | |
| 399 | - 'scope' => $scope, | |
| 400 | - 'user_id' => $user_id, | |
| 401 | - 'expires' => time() + self::ACCESS_TTL, | |
| 402 | - 'refresh' => $rhash, | |
| 403 | - ]; | |
| 404 | - $state['refresh'][ $rhash ] = [ | |
| 405 | - 'access_hash' => $ahash, | |
| 406 | - 'client_id' => $client_id, | |
| 407 | - 'scope' => $scope, | |
| 408 | - 'user_id' => $user_id, | |
| 409 | - 'expires' => time() + self::REFRESH_TTL, | |
| 410 | - ]; | |
| 411 | - 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 | + ); | |
| 412 | 564 | |
| 413 | 565 | return [ |
| 414 | 566 | 'access_token' => $access, |
| 415 | 567 | 'token_type' => 'Bearer', |
| @@ -441,10 +593,21 @@ | ||
| 441 | 593 | $entry = $state['tokens'][ $hash ]; |
| 442 | 594 | if ( (int) $entry['expires'] < time() ) { |
| 443 | 595 | return null; |
| 444 | 596 | } |
| 597 | + | |
| 598 | + // Record activity against the owning client so the "Connected AI apps" | |
| 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). | |
| 603 | + $client_id = (string) $entry['client_id']; | |
| 604 | + if ( isset( $state['clients'][ $client_id ] ) && is_array( $state['clients'][ $client_id ] ) ) { | |
| 605 | + self::touch_last_used( $client_id, array_keys( $state['clients'] ) ); | |
| 606 | + } | |
| 607 | + | |
| 445 | 608 | return [ |
| 446 | - 'client_id' => (string) $entry['client_id'], | |
| 609 | + 'client_id' => $client_id, | |
| 447 | 610 | 'scope' => (string) $entry['scope'], |
| 448 | 611 | 'user_id' => (int) $entry['user_id'], |
| 449 | 612 | ]; |
| 450 | 613 | } |
| @@ -469,10 +632,116 @@ | ||
| 469 | 632 | * @return void |
| 470 | 633 | */ |
| 471 | 634 | public static function revoke_all(): void { |
| 472 | 635 | delete_option( self::OPTION ); |
| 636 | + delete_option( self::LAST_USED_OPTION ); | |
| 473 | 637 | } |
| 474 | 638 | |
| 639 | + /** | |
| 640 | + * The OAuth clients currently holding a live grant, for the "Connected AI | |
| 641 | + * apps" list. A client counts as connected while it holds an unexpired | |
| 642 | + * refresh token (the durable 30-day grant) or access token; a client that | |
| 643 | + * only registered but never completed consent is excluded. One entry per | |
| 644 | + * client_id, newest connection first. | |
| 645 | + * | |
| 646 | + * @return array<int,array{client_id:string,name:string,scope:string,read_only:bool,user_id:int,connected_at:int,last_used:int}> | |
| 647 | + */ | |
| 648 | + public static function connected_apps(): array { | |
| 649 | + $state = self::state(); | |
| 650 | + $last_used = self::last_used_map(); | |
| 651 | + | |
| 652 | + // Collect the scope + approving user per active client. Refresh tokens | |
| 653 | + // are the durable grant, so prefer them; fall back to access tokens. | |
| 654 | + $active = []; | |
| 655 | + foreach ( [ 'refresh', 'tokens' ] as $bucket ) { | |
| 656 | + foreach ( $state[ $bucket ] as $entry ) { | |
| 657 | + $cid = isset( $entry['client_id'] ) ? (string) $entry['client_id'] : ''; | |
| 658 | + if ( '' === $cid || isset( $active[ $cid ] ) ) { | |
| 659 | + continue; | |
| 660 | + } | |
| 661 | + $active[ $cid ] = [ | |
| 662 | + 'scope' => isset( $entry['scope'] ) ? (string) $entry['scope'] : 'mcp', | |
| 663 | + 'user_id' => isset( $entry['user_id'] ) ? (int) $entry['user_id'] : 0, | |
| 664 | + ]; | |
| 665 | + } | |
| 666 | + } | |
| 667 | + | |
| 668 | + $apps = []; | |
| 669 | + foreach ( $active as $cid => $info ) { | |
| 670 | + $client = isset( $state['clients'][ $cid ] ) && is_array( $state['clients'][ $cid ] ) ? $state['clients'][ $cid ] : []; | |
| 671 | + $apps[] = [ | |
| 672 | + 'client_id' => $cid, | |
| 673 | + 'name' => isset( $client['name'] ) ? (string) $client['name'] : __( 'MCP Client', 'thinkrank' ), | |
| 674 | + 'scope' => $info['scope'], | |
| 675 | + 'read_only' => self::scope_is_read_only( $info['scope'] ), | |
| 676 | + 'user_id' => $info['user_id'], | |
| 677 | + 'connected_at' => isset( $client['created'] ) ? (int) $client['created'] : 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 ), | |
| 683 | + ]; | |
| 684 | + } | |
| 685 | + | |
| 686 | + // Newest connection first. | |
| 687 | + usort( | |
| 688 | + $apps, | |
| 689 | + static function ( array $a, array $b ): int { | |
| 690 | + return $b['connected_at'] <=> $a['connected_at']; | |
| 691 | + } | |
| 692 | + ); | |
| 693 | + | |
| 694 | + return $apps; | |
| 695 | + } | |
| 696 | + | |
| 697 | + /** | |
| 698 | + * Revoke a single OAuth client's ACCESS — drops its access tokens, refresh | |
| 699 | + * tokens, and any pending codes, cutting that one app off immediately while | |
| 700 | + * leaving every other connection intact. It disappears from | |
| 701 | + * connected_apps() (which keys off live tokens), so the UI shows it gone. | |
| 702 | + * | |
| 703 | + * The client's dynamic registration (its client_id + redirect_uris) is | |
| 704 | + * intentionally KEPT: MCP clients such as ChatGPT cache the client_id from | |
| 705 | + * their first registration and reuse it on reconnect, hitting /authorize | |
| 706 | + * with that id rather than registering afresh. If we deleted the | |
| 707 | + * registration, that reconnect would fail with "Unknown client_id". Keeping | |
| 708 | + * it lets the app re-authorize — which still requires fresh admin consent | |
| 709 | + * (and mints brand-new tokens), so revocation loses nothing. | |
| 710 | + * | |
| 711 | + * @param string $client_id The client whose access to revoke. | |
| 712 | + * @return bool True if any live grant was removed. | |
| 713 | + */ | |
| 714 | + public static function revoke_client( string $client_id ): bool { | |
| 715 | + if ( '' === $client_id ) { | |
| 716 | + return false; | |
| 717 | + } | |
| 718 | + $removed = self::mutate( | |
| 719 | + static function ( array &$state ) use ( $client_id ): bool { | |
| 720 | + $found = false; | |
| 721 | + | |
| 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 | + } | |
| 729 | + } | |
| 730 | + | |
| 731 | + return $found; | |
| 732 | + } | |
| 733 | + ); | |
| 734 | + | |
| 735 | + if ( $removed ) { | |
| 736 | + $map = self::last_used_map(); | |
| 737 | + unset( $map[ $client_id ] ); | |
| 738 | + update_option( self::LAST_USED_OPTION, $map, false ); | |
| 739 | + } | |
| 740 | + | |
| 741 | + return $removed; | |
| 742 | + } | |
| 743 | + | |
| 475 | 744 | // -- State + helpers ------------------------------------------------- |
| 476 | 745 | |
| 477 | 746 | /** |
| 478 | 747 | * Load state with defaults, pruning expired codes/tokens/refresh |
| @@ -513,8 +782,11 @@ | ||
| 513 | 782 | |
| 514 | 783 | /** |
| 515 | 784 | * Persist state (autoload off — hot-write, request-scoped option). |
| 516 | 785 | * |
| 786 | + * Private on purpose: every mutation goes through mutate(), so that the | |
| 787 | + * state being written was read inside the same guard. | |
| 788 | + * | |
| 517 | 789 | * @param array<string,mixed> $state State to persist. |
| 518 | 790 | * @return void |
| 519 | 791 | */ |
| 520 | 792 | private static function save( array $state ): void { |
| @@ -521,8 +793,164 @@ | ||
| 521 | 793 | update_option( self::OPTION, $state, false ); |
| 522 | 794 | } |
| 523 | 795 | |
| 524 | 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 ); | |
| 950 | + } | |
| 951 | + | |
| 952 | + /** | |
| 525 | 953 | * Look up a registered client. |
| 526 | 954 | * |
| 527 | 955 | * @param string $client_id Client id. |
| 528 | 956 | * @return array{redirect_uris:string[],name:string,created:int}|null |
| @@ -543,8 +971,67 @@ | ||
| 543 | 971 | ]; |
| 544 | 972 | } |
| 545 | 973 | |
| 546 | 974 | /** |
| 975 | + * Bound the registered-client list. Drops abandoned registrations past | |
| 976 | + * CLIENT_TTL first, then — if still over MAX_CLIENTS — the oldest of what | |
| 977 | + * is left. A client referenced by a live code, access token, or refresh | |
| 978 | + * token is NEVER dropped: evicting one would break a working connection, | |
| 979 | + * so a site legitimately holding more than MAX_CLIENTS live grants keeps | |
| 980 | + * them all and the cap simply stops applying to that remainder. | |
| 981 | + * | |
| 982 | + * @param array<string,array<string,mixed>> $state Full state (clients + grant buckets). | |
| 983 | + * @return array<string,array<string,mixed>> The clients array to store. | |
| 984 | + */ | |
| 985 | + private static function prune_clients( array $state ): array { | |
| 986 | + $clients = $state['clients']; | |
| 987 | + | |
| 988 | + $in_use = []; | |
| 989 | + foreach ( [ 'codes', 'tokens', 'refresh' ] as $bucket ) { | |
| 990 | + foreach ( $state[ $bucket ] as $entry ) { | |
| 991 | + if ( is_array( $entry ) && isset( $entry['client_id'] ) ) { | |
| 992 | + $in_use[ (string) $entry['client_id'] ] = true; | |
| 993 | + } | |
| 994 | + } | |
| 995 | + } | |
| 996 | + | |
| 997 | + $now = time(); | |
| 998 | + foreach ( $clients as $id => $client ) { | |
| 999 | + $created = isset( $client['created'] ) ? (int) $client['created'] : 0; | |
| 1000 | + if ( ! isset( $in_use[ $id ] ) && $created + self::CLIENT_TTL < $now ) { | |
| 1001 | + unset( $clients[ $id ] ); | |
| 1002 | + } | |
| 1003 | + } | |
| 1004 | + | |
| 1005 | + if ( count( $clients ) <= self::MAX_CLIENTS ) { | |
| 1006 | + return $clients; | |
| 1007 | + } | |
| 1008 | + | |
| 1009 | + // Still over the cap — evict the oldest unused registrations. | |
| 1010 | + $evictable = array_filter( | |
| 1011 | + $clients, | |
| 1012 | + static function ( $id ) use ( $in_use ) { | |
| 1013 | + return ! isset( $in_use[ $id ] ); | |
| 1014 | + }, | |
| 1015 | + ARRAY_FILTER_USE_KEY | |
| 1016 | + ); | |
| 1017 | + uasort( | |
| 1018 | + $evictable, | |
| 1019 | + static function ( $a, $b ) { | |
| 1020 | + return ( isset( $a['created'] ) ? (int) $a['created'] : 0 ) <=> ( isset( $b['created'] ) ? (int) $b['created'] : 0 ); | |
| 1021 | + } | |
| 1022 | + ); | |
| 1023 | + foreach ( array_keys( $evictable ) as $id ) { | |
| 1024 | + if ( count( $clients ) <= self::MAX_CLIENTS ) { | |
| 1025 | + break; | |
| 1026 | + } | |
| 1027 | + unset( $clients[ $id ] ); | |
| 1028 | + } | |
| 1029 | + | |
| 1030 | + return $clients; | |
| 1031 | + } | |
| 1032 | + | |
| 1033 | + /** | |
| 547 | 1034 | * SHA-256 hash used to store tokens at rest. |
| 548 | 1035 | * |
| 549 | 1036 | * @param string $value Raw secret. |
| 550 | 1037 | * @return string |
| @@ -559,8 +1046,9 @@ | ||
| 559 | 1046 | * @param string $verifier PKCE code verifier. |
| 560 | 1047 | * @return string |
| 561 | 1048 | */ |
| 562 | 1049 | private static function s256( string $verifier ): string { |
| 1050 | + // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.obfuscation_base64_encode -- base64url of the PKCE challenge, mandated by RFC 7636. | |
| 563 | 1051 | return rtrim( strtr( base64_encode( hash( 'sha256', $verifier, true ) ), '+/', '-_' ), '=' ); |
| 564 | 1052 | } |
| 565 | 1053 | |
| 566 | 1054 | /** |
