| @@ -8,12 +8,17 @@ | ||
| 8 | 8 | * authorization-code + PKCE flow themselves. See the security contract and |
| 9 | 9 | * the end-to-end flow notes below. |
| 10 | 10 | * |
| 11 | 11 | * Flow: unauthenticated MCP call -> 401 + WWW-Authenticate (Mcp_Server) -> |
| 12 | - * client fetches /.well-known/oauth-protected-resource + oauth-authorization- | |
| 13 | - * server -> dynamic registration (RFC 7591) -> /authorize (admin consent + | |
| 14 | - * PKCE) -> /token (code + verifier -> access + refresh) -> MCP calls with | |
| 15 | - * `Authorization: Bearer <access>` validated by validate_token(). | |
| 12 | + * client fetches /.well-known/oauth-protected-resource/xspeed/mcp + | |
| 13 | + * /.well-known/oauth-authorization-server/xspeed/mcp -> dynamic registration | |
| 14 | + * (RFC 7591) -> /authorize (admin consent + PKCE) -> /token (code + verifier | |
| 15 | + * -> access + refresh) -> MCP calls with `Authorization: Bearer <access>` | |
| 16 | + * validated by validate_token(). Both canonical identifiers -- issuer and | |
| 17 | + * resource -- are the MCP endpoint URL, which is what puts the documents | |
| 18 | + * under that path rather than at the contested site root (#266). The root | |
| 19 | + * URLs are still answered, with the legacy host-only issuer, on sites where | |
| 20 | + * no other plugin has claimed them -- see legacy_issuer(). | |
| 16 | 21 | * |
| 17 | 22 | * Security contract: |
| 18 | 23 | * - PKCE S256 REQUIRED (OAuth 2.1 public clients); codes are single-use, |
| 19 | 24 | * 60 s TTL, bound to client_id + redirect_uri + challenge. |
| @@ -52,18 +57,116 @@ | ||
| 52 | 57 | |
| 53 | 58 | /** Refresh-token lifetime (seconds) -- 30 days. */ |
| 54 | 59 | private const REFRESH_TTL = 2592000; |
| 55 | 60 | |
| 56 | - /** Scopes we advertise + honor. `mcp` is the umbrella scope MCP clients request. */ | |
| 57 | - private const SUPPORTED_SCOPES = array( 'mcp', 'read', 'write' ); | |
| 61 | + /** | |
| 62 | + * Scopes we advertise + honor. `mcp` is the umbrella scope MCP clients | |
| 63 | + * request (read+write). `configure` is an ADDITIONAL, opt-in scope that a | |
| 64 | + * client must request explicitly to write credential/secret fields — it is | |
| 65 | + * NOT implied by `mcp` or `write`, so credential writes stay off by default | |
| 66 | + * on an ordinary connection. (#116) | |
| 67 | + */ | |
| 68 | + private const SUPPORTED_SCOPES = array( 'mcp', 'read', 'write', 'configure' ); | |
| 58 | 69 | |
| 70 | + /** | |
| 71 | + * Resource bounds for dynamic client registration (RFC 7591). | |
| 72 | + * | |
| 73 | + * The register endpoint is public by design — that is what makes an MCP | |
| 74 | + * client able to connect without an admin minting credentials first. What | |
| 75 | + * was missing is a resource policy: every request appended a client to one | |
| 76 | + * persistent option with no count, size, expiry or rate bound, so repeated | |
| 77 | + * anonymous requests grew `xspeed_mcp_oauth` without limit. Every later | |
| 78 | + * registration and OAuth operation then loaded and reserialized a larger | |
| 79 | + * option, burning storage, memory and CPU until availability degraded. | |
| 80 | + * | |
| 81 | + * Codes, access tokens and refresh tokens were already pruned on expiry; | |
| 82 | + * `clients` was the one collection retained forever. | |
| 83 | + */ | |
| 84 | + private const MAX_CLIENTS = 100; | |
| 85 | + | |
| 86 | + /** Redirect URIs accepted per registration. */ | |
| 87 | + private const MAX_REDIRECT_URIS = 5; | |
| 88 | + | |
| 89 | + /** Longest accepted redirect URI, in bytes. */ | |
| 90 | + private const MAX_REDIRECT_URI_LEN = 2048; | |
| 91 | + | |
| 92 | + /** Longest accepted client_name, in bytes. */ | |
| 93 | + private const MAX_CLIENT_NAME_LEN = 200; | |
| 94 | + | |
| 95 | + /** | |
| 96 | + * How long an UNUSED client survives. A client that never completes a | |
| 97 | + * flow is almost always an abandoned or hostile registration, so it is | |
| 98 | + * collected after this window. A client with a live code, access token or | |
| 99 | + * refresh token is never collected on age — see prune_clients(). | |
| 100 | + */ | |
| 101 | + private const UNUSED_CLIENT_TTL = 86400; | |
| 102 | + | |
| 103 | + /** Registrations allowed per IP inside RATE_WINDOW. */ | |
| 104 | + private const RATE_MAX = 10; | |
| 105 | + | |
| 106 | + /** Window for the registration rate limit, in seconds. Fixed, not sliding. */ | |
| 107 | + private const RATE_WINDOW = 3600; | |
| 108 | + | |
| 109 | + /** | |
| 110 | + * Fixed number of rate-limit counters. | |
| 111 | + * | |
| 112 | + * One transient per IP would let a distributed flood grow the options | |
| 113 | + * TABLE without bound — the same CWE-770 shape this class exists to fix, | |
| 114 | + * moved from the option value to the row count. Hashing the IP into a | |
| 115 | + * fixed bucket space caps that at a constant, whatever the traffic. | |
| 116 | + * (QA F2 on #254) | |
| 117 | + */ | |
| 118 | + private const RATE_BUCKETS = 64; | |
| 119 | + | |
| 120 | + /** | |
| 121 | + * True while a caller is between state() and its own save(). | |
| 122 | + * | |
| 123 | + * state() persists a self-heal prune on read-only paths, but a caller that | |
| 124 | + * is about to save() anyway would then write twice — and the second write | |
| 125 | + * would be against a $state it had already mutated. Callers that mutate | |
| 126 | + * set this so the read-side write stands down. (QA F1 on #254) | |
| 127 | + */ | |
| 128 | + private static $writing = false; | |
| 129 | + | |
| 59 | 130 | // -- URLs ------------------------------------------------------------ |
| 60 | 131 | |
| 61 | - /** Base site URL used as the OAuth issuer (no trailing slash). */ | |
| 132 | + /** | |
| 133 | + * The OAuth issuer identifier -- the MCP endpoint URL, identical to | |
| 134 | + * resource(). | |
| 135 | + * | |
| 136 | + * RFC 8414 §2 allows an issuer to carry a path, and §3.1 then moves its | |
| 137 | + * metadata to /.well-known/oauth-authorization-server/xspeed/mcp, a URL | |
| 138 | + * only this plugin answers. A bare-host issuer put the document at the | |
| 139 | + * site root, which every other MCP-serving plugin on the same site also | |
| 140 | + * wants, and WordPress hands that URL to whichever rewrite rule happens | |
| 141 | + * to sit first in the table. | |
| 142 | + * | |
| 143 | + * Nothing stored carries the issuer -- access and refresh tokens are | |
| 144 | + * opaque random strings, client records hold redirect_uris/name/created | |
| 145 | + * -- so changing it invalidates no grant. A client that re-discovers | |
| 146 | + * simply registers again and asks the admin for consent once more. | |
| 147 | + * (#266) | |
| 148 | + */ | |
| 62 | 149 | public static function issuer(): string { |
| 63 | - return untrailingslashit( home_url() ); | |
| 150 | + return self::resource(); | |
| 64 | 151 | } |
| 65 | 152 | |
| 153 | + /** | |
| 154 | + * The host-only issuer earlier builds used, still served at the bare | |
| 155 | + * /.well-known/oauth-* URLs when no other plugin has claimed them. | |
| 156 | + * | |
| 157 | + * RFC 8414 §3.3 makes a client reject a document whose `issuer` is not | |
| 158 | + * the value it inserted into the URL it fetched, and a client that | |
| 159 | + * fetched the ROOT document inserted nothing -- it derived that URL from | |
| 160 | + * `https://site`. Stamping the path issuer there would be the same RFC | |
| 161 | + * violation this change set out to remove, pointed the other way. So the | |
| 162 | + * two locations carry two identities, each self-consistent, and a client | |
| 163 | + * ends up on whichever one it asked for. (#266) | |
| 164 | + */ | |
| 165 | + public static function legacy_issuer(): string { | |
| 166 | + return untrailingslashit( home_url( '/' ) ); | |
| 167 | + } | |
| 168 | + | |
| 66 | 169 | /** The protected resource identifier -- the MCP endpoint URL. */ |
| 67 | 170 | public static function resource(): string { |
| 68 | 171 | return Mcp_Pairing::site_endpoint(); |
| 69 | 172 | } |
| @@ -74,17 +177,17 @@ | ||
| 74 | 177 | * round-trip — a REST route would see the cookie without a nonce and |
| 75 | 178 | * treat the admin as logged-out, looping back to login. |
| 76 | 179 | */ |
| 77 | 180 | public static function authorize_url(): string { |
| 78 | - return home_url( '/xspeed/authorize' ); | |
| 181 | + return Mcp_Pairing::absolute( home_url( '/xspeed/authorize' ) ); | |
| 79 | 182 | } |
| 80 | 183 | |
| 81 | 184 | public static function token_url(): string { |
| 82 | - return rest_url( 'xspeed/v1/mcp/oauth/token' ); | |
| 185 | + return Mcp_Pairing::absolute( rest_url( 'xspeed/v1/mcp/oauth/token' ) ); | |
| 83 | 186 | } |
| 84 | 187 | |
| 85 | 188 | public static function register_url(): string { |
| 86 | - return rest_url( 'xspeed/v1/mcp/oauth/register' ); | |
| 189 | + return Mcp_Pairing::absolute( rest_url( 'xspeed/v1/mcp/oauth/register' ) ); | |
| 87 | 190 | } |
| 88 | 191 | |
| 89 | 192 | // -- Discovery documents (RFC 8414 / RFC 9728) ----------------------- |
| 90 | 193 | |
| @@ -91,14 +194,20 @@ | ||
| 91 | 194 | /** |
| 92 | 195 | * RFC 9728 protected-resource metadata -- tells the client which |
| 93 | 196 | * authorization server(s) protect the MCP endpoint (this site). |
| 94 | 197 | * |
| 198 | + * @param string|null $issuer The authorization server to name. Defaults | |
| 199 | + * to the canonical path issuer; the root | |
| 200 | + * /.well-known/ location passes the legacy | |
| 201 | + * host-only one, so that the AS document a | |
| 202 | + * client goes on to fetch is the one served | |
| 203 | + * at the URL that issuer derives. | |
| 95 | 204 | * @return array<string,mixed> |
| 96 | 205 | */ |
| 97 | - public static function protected_resource_metadata(): array { | |
| 206 | + public static function protected_resource_metadata( ?string $issuer = null ): array { | |
| 98 | 207 | return array( |
| 99 | 208 | 'resource' => self::resource(), |
| 100 | - 'authorization_servers' => array( self::issuer() ), | |
| 209 | + 'authorization_servers' => array( $issuer ?? self::issuer() ), | |
| 101 | 210 | 'scopes_supported' => self::SUPPORTED_SCOPES, |
| 102 | 211 | 'bearer_methods_supported' => array( 'header' ), |
| 103 | 212 | ); |
| 104 | 213 | } |
| @@ -107,13 +216,18 @@ | ||
| 107 | 216 | * RFC 8414 authorization-server metadata -- the endpoint map + the |
| 108 | 217 | * capabilities we actually implement (auth-code grant, PKCE S256, |
| 109 | 218 | * dynamic registration, refresh tokens). |
| 110 | 219 | * |
| 220 | + * @param string|null $issuer Which identity this copy of the document | |
| 221 | + * speaks for; see protected_resource_metadata(). | |
| 222 | + * Every endpoint URL below is identical either | |
| 223 | + * way, which is why a client that switches | |
| 224 | + * identities keeps its cached endpoints. | |
| 111 | 225 | * @return array<string,mixed> |
| 112 | 226 | */ |
| 113 | - public static function authorization_server_metadata(): array { | |
| 227 | + public static function authorization_server_metadata( ?string $issuer = null ): array { | |
| 114 | 228 | return array( |
| 115 | - 'issuer' => self::issuer(), | |
| 229 | + 'issuer' => $issuer ?? self::issuer(), | |
| 116 | 230 | 'authorization_endpoint' => self::authorize_url(), |
| 117 | 231 | 'token_endpoint' => self::token_url(), |
| 118 | 232 | 'registration_endpoint' => self::register_url(), |
| 119 | 233 | 'scopes_supported' => self::SUPPORTED_SCOPES, |
| @@ -134,12 +248,42 @@ | ||
| 134 | 248 | * @param array<string,mixed> $body Parsed JSON registration request. |
| 135 | 249 | * @return array<string,mixed>|\WP_Error |
| 136 | 250 | */ |
| 137 | 251 | public static function register_client( array $body ) { |
| 138 | - $redirect_uris = isset( $body['redirect_uris'] ) && is_array( $body['redirect_uris'] ) | |
| 139 | - ? array_values( array_filter( array_map( 'strval', $body['redirect_uris'] ), array( self::class, 'is_valid_redirect_uri' ) ) ) | |
| 252 | + // CHECK the rate limit before any work — an over-limit caller must not | |
| 253 | + // be able to make us read, mutate or reserialize the option at all. | |
| 254 | + // The COUNT happens later, only once the payload has proven valid, so | |
| 255 | + // a legitimate but buggy client sending malformed bodies is not locked | |
| 256 | + // out for an hour over registrations that never stored anything. | |
| 257 | + // (QA F3 on #254) | |
| 258 | + if ( ! self::rate_limit_ok( false ) ) { | |
| 259 | + return new \WP_Error( | |
| 260 | + 'too_many_requests', | |
| 261 | + __( 'Too many client registrations. Try again later.', 'xspeed' ), | |
| 262 | + array( 'status' => 429 ) | |
| 263 | + ); | |
| 264 | + } | |
| 265 | + | |
| 266 | + $raw = isset( $body['redirect_uris'] ) && is_array( $body['redirect_uris'] ) | |
| 267 | + ? array_map( 'strval', $body['redirect_uris'] ) | |
| 140 | 268 | : array(); |
| 141 | 269 | |
| 270 | + // Cap the count before validating, so a huge array costs a count() | |
| 271 | + // rather than a full validation pass. | |
| 272 | + if ( count( $raw ) > self::MAX_REDIRECT_URIS ) { | |
| 273 | + return new \WP_Error( | |
| 274 | + 'invalid_redirect_uri', | |
| 275 | + sprintf( | |
| 276 | + /* translators: %d: maximum number of redirect URIs. */ | |
| 277 | + __( 'At most %d redirect_uris are allowed.', 'xspeed' ), | |
| 278 | + self::MAX_REDIRECT_URIS | |
| 279 | + ), | |
| 280 | + array( 'status' => 400 ) | |
| 281 | + ); | |
| 282 | + } | |
| 283 | + | |
| 284 | + $redirect_uris = array_values( array_filter( $raw, array( self::class, 'is_valid_redirect_uri' ) ) ); | |
| 285 | + | |
| 142 | 286 | if ( empty( $redirect_uris ) ) { |
| 143 | 287 | return new \WP_Error( |
| 144 | 288 | 'invalid_redirect_uri', |
| 145 | 289 | __( 'At least one valid redirect_uri is required.', 'xspeed' ), |
| @@ -146,19 +290,64 @@ | ||
| 146 | 290 | array( 'status' => 400 ) |
| 147 | 291 | ); |
| 148 | 292 | } |
| 149 | 293 | |
| 150 | - $name = isset( $body['client_name'] ) ? sanitize_text_field( (string) $body['client_name'] ) : 'MCP Client'; | |
| 294 | + $name = isset( $body['client_name'] ) ? sanitize_text_field( (string) $body['client_name'] ) : 'MCP Client'; | |
| 295 | + if ( strlen( $name ) > self::MAX_CLIENT_NAME_LEN ) { | |
| 296 | + // mb_strcut, NOT substr: the bound is in BYTES (that is what the | |
| 297 | + // storage limit is about), but cutting at byte 200 lands inside a | |
| 298 | + // multi-byte character for any CJK or emoji name — 200 is not a | |
| 299 | + // multiple of 3 — and the result is invalid UTF-8. MySQL then | |
| 300 | + // refuses the whole option write, so the registration was silently | |
| 301 | + // dropped while the endpoint still answered 201 with a client_id | |
| 302 | + // that had never been stored. mb_strcut keeps the byte budget and | |
| 303 | + // never splits a character. (QA on #254) | |
| 304 | + $name = function_exists( 'mb_strcut' ) | |
| 305 | + ? mb_strcut( $name, 0, self::MAX_CLIENT_NAME_LEN, 'UTF-8' ) | |
| 306 | + : substr( $name, 0, self::MAX_CLIENT_NAME_LEN ); | |
| 307 | + } | |
| 308 | + | |
| 309 | + // The payload is valid, so this request counts against the window. | |
| 310 | + // Deliberately AFTER validation (F3) but BEFORE the option read below, | |
| 311 | + // so an over-limit caller still cannot make us touch the option. | |
| 312 | + self::rate_limit_ok( true ); | |
| 313 | + | |
| 151 | 314 | $client_id = 'xsc_' . bin2hex( random_bytes( 16 ) ); |
| 152 | 315 | |
| 153 | - $state = self::state(); | |
| 316 | + // state() has already pruned unused/expired clients on load. | |
| 317 | + $state = self::state_for_write(); | |
| 318 | + | |
| 319 | + // Hard cap. Pruning above already dropped unused and expired | |
| 320 | + // registrations, so hitting this means MAX_CLIENTS clients are | |
| 321 | + // genuinely in use — refuse rather than evict a live one, which would | |
| 322 | + // break a working integration to satisfy an anonymous caller. | |
| 323 | + if ( count( $state['clients'] ) >= self::MAX_CLIENTS ) { | |
| 324 | + return new \WP_Error( | |
| 325 | + 'too_many_clients', | |
| 326 | + __( 'Client registration limit reached.', 'xspeed' ), | |
| 327 | + array( 'status' => 429 ) | |
| 328 | + ); | |
| 329 | + } | |
| 330 | + | |
| 154 | 331 | $state['clients'][ $client_id ] = array( |
| 155 | 332 | 'redirect_uris' => $redirect_uris, |
| 156 | 333 | 'name' => $name, |
| 157 | 334 | 'created' => time(), |
| 158 | 335 | ); |
| 159 | - self::save( $state ); | |
| 160 | 336 | |
| 337 | + // A freshly-minted client_id always changes the option, so a false here | |
| 338 | + // is a genuine write failure and never the "value unchanged" case. Fail | |
| 339 | + // loudly: handing back a 201 and a client_id that was never stored | |
| 340 | + // leaves the caller holding a credential that can never authorize, and | |
| 341 | + // the only symptom is a confusing "Unknown client_id" much later. | |
| 342 | + if ( ! self::save( $state ) ) { | |
| 343 | + return new \WP_Error( | |
| 344 | + 'registration_failed', | |
| 345 | + __( 'The client registration could not be stored.', 'xspeed' ), | |
| 346 | + array( 'status' => 500 ) | |
| 347 | + ); | |
| 348 | + } | |
| 349 | + | |
| 161 | 350 | return array( |
| 162 | 351 | 'client_id' => $client_id, |
| 163 | 352 | 'client_id_issued_at' => time(), |
| 164 | 353 | 'redirect_uris' => $redirect_uris, |
| @@ -226,9 +415,9 @@ | ||
| 226 | 415 | * @return string The authorization code. |
| 227 | 416 | */ |
| 228 | 417 | public static function issue_code( array $req, int $user_id ): string { |
| 229 | 418 | $code = bin2hex( random_bytes( 32 ) ); |
| 230 | - $state = self::state(); | |
| 419 | + $state = self::state_for_write(); | |
| 231 | 420 | $state['codes'][ $code ] = array( |
| 232 | 421 | 'client_id' => $req['client_id'], |
| 233 | 422 | 'redirect_uri' => $req['redirect_uri'], |
| 234 | 423 | 'challenge' => $req['code_challenge'], |
| @@ -273,9 +462,9 @@ | ||
| 273 | 462 | $client_id = isset( $body['client_id'] ) ? (string) $body['client_id'] : ''; |
| 274 | 463 | $redirect_uri = isset( $body['redirect_uri'] ) ? (string) $body['redirect_uri'] : ''; |
| 275 | 464 | $verifier = isset( $body['code_verifier'] ) ? (string) $body['code_verifier'] : ''; |
| 276 | 465 | |
| 277 | - $state = self::state(); | |
| 466 | + $state = self::state_for_write(); | |
| 278 | 467 | if ( '' === $code || ! isset( $state['codes'][ $code ] ) ) { |
| 279 | 468 | return self::oauth_error( 'invalid_grant', 'Unknown or expired authorization code.' ); |
| 280 | 469 | } |
| 281 | 470 | $entry = $state['codes'][ $code ]; |
| @@ -311,9 +500,9 @@ | ||
| 311 | 500 | private static function grant_refresh_token( array $body ) { |
| 312 | 501 | $refresh = isset( $body['refresh_token'] ) ? (string) $body['refresh_token'] : ''; |
| 313 | 502 | $client_id = isset( $body['client_id'] ) ? (string) $body['client_id'] : ''; |
| 314 | 503 | |
| 315 | - $state = self::state(); | |
| 504 | + $state = self::state_for_write(); | |
| 316 | 505 | $rhash = self::hash( $refresh ); |
| 317 | 506 | if ( '' === $refresh || ! isset( $state['refresh'][ $rhash ] ) ) { |
| 318 | 507 | return self::oauth_error( 'invalid_grant', 'Unknown refresh token.' ); |
| 319 | 508 | } |
| @@ -346,9 +535,9 @@ | ||
| 346 | 535 | $refresh = bin2hex( random_bytes( 32 ) ); |
| 347 | 536 | $ahash = self::hash( $access ); |
| 348 | 537 | $rhash = self::hash( $refresh ); |
| 349 | 538 | |
| 350 | - $state = self::state(); | |
| 539 | + $state = self::state_for_write(); | |
| 351 | 540 | $state['tokens'][ $ahash ] = array( |
| 352 | 541 | 'client_id' => $client_id, |
| 353 | 542 | 'scope' => $scope, |
| 354 | 543 | 'user_id' => $user_id, |
| @@ -413,8 +602,19 @@ | ||
| 413 | 602 | $parts = preg_split( '/\s+/', trim( $scope ) ) ?: array(); |
| 414 | 603 | return ! in_array( 'write', $parts, true ) && ! in_array( 'mcp', $parts, true ); |
| 415 | 604 | } |
| 416 | 605 | |
| 606 | + /** | |
| 607 | + * Whether a granted scope string may write credential/secret fields. Unlike | |
| 608 | + * read/write, `configure` is never implied by the `mcp` umbrella — the | |
| 609 | + * client must ask for it by name — so an ordinary read-write connection | |
| 610 | + * cannot rewrite API tokens or passwords. (#116) | |
| 611 | + */ | |
| 612 | + public static function scope_allows_configure( string $scope ): bool { | |
| 613 | + $parts = preg_split( '/\s+/', trim( $scope ) ) ?: array(); | |
| 614 | + return in_array( 'configure', $parts, true ); | |
| 615 | + } | |
| 616 | + | |
| 417 | 617 | /** Revoke every OAuth token + client (used by disconnect). */ |
| 418 | 618 | public static function revoke_all(): void { |
| 419 | 619 | delete_option( self::OPTION ); |
| 420 | 620 | } |
| @@ -454,14 +654,62 @@ | ||
| 454 | 654 | if ( isset( $v['expires'] ) && $v['expires'] < $now ) { |
| 455 | 655 | unset( $state['refresh'][ $k ] ); |
| 456 | 656 | } |
| 457 | 657 | } |
| 658 | + | |
| 659 | + // Clients were the one collection this method never pruned, which is | |
| 660 | + // what let an anonymous caller grow the option without bound. Prune | |
| 661 | + // them here too, AFTER the grant buckets above, so "in use" is | |
| 662 | + // decided against live grants only. | |
| 663 | + $before = count( $state['clients'] ); | |
| 664 | + $state['clients'] = self::prune_clients( $state ); | |
| 665 | + | |
| 666 | + // Persist when pruning ACTUALLY removed something. Pruning in memory | |
| 667 | + // alone left a bloated option on disk until the next write happened to | |
| 668 | + // land — so a site whose flood had stopped kept carrying the weight | |
| 669 | + // indefinitely, which is not the self-heal this was described as. | |
| 670 | + // (QA F1 on #254) | |
| 671 | + // | |
| 672 | + // Guarded on a real reduction, so the common case — nothing to prune — | |
| 673 | + // stays a pure read and adds no write to an OAuth request. $writing | |
| 674 | + // stops a caller that is about to save() anyway from writing twice. | |
| 675 | + if ( ! self::$writing && count( $state['clients'] ) < $before ) { | |
| 676 | + self::save( $state ); | |
| 677 | + } | |
| 678 | + | |
| 458 | 679 | return $state; |
| 459 | 680 | } |
| 460 | 681 | |
| 682 | + /** | |
| 683 | + * Load state for a caller that intends to mutate and save() it. | |
| 684 | + * | |
| 685 | + * Identical to state(), except it suppresses the read-side self-heal | |
| 686 | + * write — the caller's own save() persists the same prune moments later, | |
| 687 | + * so doing it here would write twice per request. (QA F1 on #254) | |
| 688 | + * | |
| 689 | + * @return array<string,array<string,mixed>> | |
| 690 | + */ | |
| 691 | + private static function state_for_write(): array { | |
| 692 | + self::$writing = true; | |
| 693 | + try { | |
| 694 | + return self::state(); | |
| 695 | + } finally { | |
| 696 | + self::$writing = false; | |
| 697 | + } | |
| 698 | + } | |
| 699 | + | |
| 461 | 700 | /** Persist state (autoload off -- this is a hot-write, request-scoped option). */ |
| 462 | - private static function save( array $state ): void { | |
| 463 | - update_option( self::OPTION, $state, false ); | |
| 701 | + private static function save( array $state ): bool { | |
| 702 | + // Report the outcome rather than discarding it. update_option() returns | |
| 703 | + // false when the DB refuses the write — e.g. a value MySQL rejects as | |
| 704 | + // invalid — and swallowing that let register_client() answer 201 with a | |
| 705 | + // client_id it had never stored. A caller that mints a credential must | |
| 706 | + // be able to tell a real write from a silent no-op. (QA on #254) | |
| 707 | + // | |
| 708 | + // Note update_option() also returns false when the value is UNCHANGED, | |
| 709 | + // so this is "did not write", not "failed" — only callers that just | |
| 710 | + // added something to $state may treat false as an error. | |
| 711 | + return (bool) update_option( self::OPTION, $state, false ); | |
| 464 | 712 | } |
| 465 | 713 | |
| 466 | 714 | /** |
| 467 | 715 | * Look up a registered client. |
| @@ -514,10 +762,142 @@ | ||
| 514 | 762 | $uri = trim( $uri ); |
| 515 | 763 | if ( '' === $uri ) { |
| 516 | 764 | return false; |
| 517 | 765 | } |
| 766 | + // Bound the length: without this a single registration could carry | |
| 767 | + // multi-megabyte URIs straight into the stored option. | |
| 768 | + if ( strlen( $uri ) > self::MAX_REDIRECT_URI_LEN ) { | |
| 769 | + return false; | |
| 770 | + } | |
| 518 | 771 | // Allow standard web redirect URIs and native-client custom schemes. |
| 519 | 772 | return (bool) preg_match( '#^[a-zA-Z][a-zA-Z0-9+.\-]*://#', $uri ); |
| 773 | + } | |
| 774 | + | |
| 775 | + /** | |
| 776 | + * Drop registered clients that are neither in use nor recent. | |
| 777 | + * | |
| 778 | + * "In use" means the client still owns a live authorization code, access | |
| 779 | + * token or refresh token — state() has already pruned the expired ones, so | |
| 780 | + * whatever remains is genuinely live. Those are kept whatever their age; a | |
| 781 | + * connected client must never be collected out from under a working | |
| 782 | + * integration. | |
| 783 | + * | |
| 784 | + * Everything else is a registration that never completed a flow. Those are | |
| 785 | + * kept for UNUSED_CLIENT_TTL so a slow but legitimate authorization can | |
| 786 | + * finish, then collected. | |
| 787 | + * | |
| 788 | + * @param array<string,array<string,mixed>> $state Loaded state. | |
| 789 | + * @return array<string,mixed> The surviving clients. | |
| 790 | + */ | |
| 791 | + private static function prune_clients( array $state ): array { | |
| 792 | + $clients = isset( $state['clients'] ) && is_array( $state['clients'] ) ? $state['clients'] : array(); | |
| 793 | + if ( empty( $clients ) ) { | |
| 794 | + return array(); | |
| 795 | + } | |
| 796 | + | |
| 797 | + // Which client ids still hold live grants? | |
| 798 | + $in_use = array(); | |
| 799 | + foreach ( array( 'codes', 'tokens', 'refresh' ) as $bucket ) { | |
| 800 | + if ( empty( $state[ $bucket ] ) || ! is_array( $state[ $bucket ] ) ) { | |
| 801 | + continue; | |
| 802 | + } | |
| 803 | + foreach ( $state[ $bucket ] as $entry ) { | |
| 804 | + if ( is_array( $entry ) && ! empty( $entry['client_id'] ) ) { | |
| 805 | + $in_use[ (string) $entry['client_id'] ] = true; | |
| 806 | + } | |
| 807 | + } | |
| 808 | + } | |
| 809 | + | |
| 810 | + $cutoff = time() - self::UNUSED_CLIENT_TTL; | |
| 811 | + foreach ( $clients as $id => $client ) { | |
| 812 | + if ( isset( $in_use[ (string) $id ] ) ) { | |
| 813 | + continue; | |
| 814 | + } | |
| 815 | + $created = ( is_array( $client ) && isset( $client['created'] ) ) ? (int) $client['created'] : 0; | |
| 816 | + if ( $created < $cutoff ) { | |
| 817 | + unset( $clients[ $id ] ); | |
| 818 | + } | |
| 819 | + } | |
| 820 | + | |
| 821 | + return $clients; | |
| 822 | + } | |
| 823 | + | |
| 824 | + /** | |
| 825 | + * Per-IP sliding-window rate limit for dynamic client registration. | |
| 826 | + * | |
| 827 | + * Deliberately mirrors Mcp_Rate_Limiter's approach (transient counter | |
| 828 | + * keyed on a hashed IP) rather than adding a dependency: that class gates | |
| 829 | + * failed *authentication*, while this gates *creation* of state, and the | |
| 830 | + * two must be tunable apart. | |
| 831 | + * | |
| 832 | + * Fails OPEN when the IP is unavailable — a proxy that hides REMOTE_ADDR | |
| 833 | + * must not lock every client out of registering. | |
| 834 | + */ | |
| 835 | + private static function rate_limit_ok( bool $count = true ): bool { | |
| 836 | + $ip = isset( $_SERVER['REMOTE_ADDR'] ) ? sanitize_text_field( wp_unslash( $_SERVER['REMOTE_ADDR'] ) ) : ''; | |
| 837 | + if ( '' === $ip ) { | |
| 838 | + return true; | |
| 839 | + } | |
| 840 | + | |
| 841 | + /** | |
| 842 | + * Filter the dynamic-client-registration rate limit. | |
| 843 | + * | |
| 844 | + * @param int $max Registrations allowed per IP inside the window. | |
| 845 | + */ | |
| 846 | + $max = (int) apply_filters( 'xspeed_mcp_register_rate_limit', self::RATE_MAX ); | |
| 847 | + if ( $max <= 0 ) { | |
| 848 | + return true; | |
| 849 | + } | |
| 850 | + | |
| 851 | + // Bucket the IP into a FIXED key space instead of one transient per | |
| 852 | + // IP. Per-IP keys meant a distributed flood grew the options TABLE | |
| 853 | + // without bound — the same CWE-770 shape as the bug this class fixes, | |
| 854 | + // just moved from the option value to the row count. RATE_BUCKETS | |
| 855 | + // caps it at a constant: 64 counters, whatever the traffic. | |
| 856 | + // (QA F2 on #254) | |
| 857 | + // | |
| 858 | + // Collisions make the limit stricter for the colliding IPs, never | |
| 859 | + // looser, so the bound still holds. With 64 buckets a handful of | |
| 860 | + // unrelated clients may share a counter; that is the deliberate trade | |
| 861 | + // for a storage ceiling, and RATE_MAX is generous enough to absorb it. | |
| 862 | + $bucket = hexdec( substr( md5( $ip ), 0, 4 ) ) % self::RATE_BUCKETS; | |
| 863 | + $key = 'xspeed_mcp_reg_' . $bucket; | |
| 864 | + | |
| 865 | + $entry = get_transient( $key ); | |
| 866 | + $now = time(); | |
| 867 | + | |
| 868 | + // Window START is stored with the counter, so the window is genuinely | |
| 869 | + // FIXED rather than extending on every hit. set_transient()'s TTL was | |
| 870 | + // previously reset on each accepted request, which quietly turned | |
| 871 | + // "10 per hour" into "10, then locked until an hour after your LAST | |
| 872 | + // attempt". (QA F4 on #254) | |
| 873 | + if ( ! is_array( $entry ) || ! isset( $entry['start'], $entry['count'] ) || ( $now - (int) $entry['start'] ) >= self::RATE_WINDOW ) { | |
| 874 | + $entry = array( | |
| 875 | + 'start' => $now, | |
| 876 | + 'count' => 0, | |
| 877 | + ); | |
| 878 | + } | |
| 879 | + | |
| 880 | + if ( (int) $entry['count'] >= $max ) { | |
| 881 | + return false; | |
| 882 | + } | |
| 883 | + | |
| 884 | + // CHECK-only mode writes nothing. The caller re-invokes with $count | |
| 885 | + // true once the payload has proven valid, so a malformed request | |
| 886 | + // costs the caller nothing — it never consumed a slot and never | |
| 887 | + // touched the database. (QA F3 on #254) | |
| 888 | + if ( ! $count ) { | |
| 889 | + return true; | |
| 890 | + } | |
| 891 | + | |
| 892 | + ++$entry['count']; | |
| 893 | + | |
| 894 | + // TTL covers only the REMAINDER of the current window, so the entry | |
| 895 | + // expires when the window does instead of being pushed forward. | |
| 896 | + $remaining = self::RATE_WINDOW - ( $now - (int) $entry['start'] ); | |
| 897 | + set_transient( $key, $entry, max( 1, $remaining ) ); | |
| 898 | + | |
| 899 | + return true; | |
| 520 | 900 | } |
| 521 | 901 | |
| 522 | 902 | /** |
| 523 | 903 | * Build a WP_Error whose data carries an OAuth 2.0 `error` code so the |
