| @@ -63,8 +63,14 @@ | ||
| 63 | 63 | */ |
| 64 | 64 | private const WELLKNOWN_QUERY_VAR = 'thinkrank_mcp_wellknown'; |
| 65 | 65 | |
| 66 | 66 | /** |
| 67 | + * Query var carrying the resource path a root-form discovery request asked | |
| 68 | + * about, so the handler can tell whether that resource is ours (#516). | |
| 69 | + */ | |
| 70 | + private const WELLKNOWN_RESOURCE_QUERY_VAR = 'thinkrank_mcp_wellknown_resource'; | |
| 71 | + | |
| 72 | + /** | |
| 67 | 73 | * Query var flagging the browser-facing OAuth authorize page. This is |
| 68 | 74 | * served OUTSIDE the REST API on purpose: a REST route only honors cookie |
| 69 | 75 | * auth when a REST nonce accompanies it, but a browser arriving from |
| 70 | 76 | * wp-login carries the cookie with NO nonce — so is_user_logged_in() |
| @@ -81,8 +87,14 @@ | ||
| 81 | 87 | */ |
| 82 | 88 | public function init(): void { |
| 83 | 89 | add_action( 'rest_api_init', [ $this, 'register_rest' ] ); |
| 84 | 90 | |
| 91 | + // Published /.well-known/ files are served ahead of WordPress, so a | |
| 92 | + // site URL change leaves them advertising the old domain's issuer with | |
| 93 | + // nothing to correct them. Registered unconditionally: a stale | |
| 94 | + // document is harmful whether or not MCP is currently enabled (#486). | |
| 95 | + Mcp_Static_Discovery::init(); | |
| 96 | + | |
| 85 | 97 | // Pretty per-site endpoint: /thinkrank/mcp → MCP JSON-RPC handler. |
| 86 | 98 | add_action( 'init', [ $this, 'add_rewrite' ] ); |
| 87 | 99 | add_filter( 'query_vars', [ $this, 'register_query_var' ] ); |
| 88 | 100 | add_action( 'parse_request', [ $this, 'maybe_handle_pretty_endpoint' ] ); |
| @@ -170,11 +182,20 @@ | ||
| 170 | 182 | ); |
| 171 | 183 | // Root-form fallback for clients that only try the bare well-known |
| 172 | 184 | // URL. Harmless when another plugin also registers this exact regex — |
| 173 | 185 | // last registrant wins, and our clients use the path-suffixed form. |
| 186 | + // | |
| 187 | + // The trailing path is CAPTURED rather than discarded (#516). It names | |
| 188 | + // the resource the client is asking about, and answering for a resource | |
| 189 | + // that is not ours is how this rule broke subdirectory multisite: the | |
| 190 | + // network root belongs to the main site, so a client discovering | |
| 191 | + // /ca/thinkrank/mcp was served the MAIN site's document, with every | |
| 192 | + // endpoint missing the /ca/ prefix. The same rule also answered for | |
| 193 | + // another plugin's resource path on a plain single site. The handler | |
| 194 | + // below compares the capture with our own path and declines the rest. | |
| 174 | 195 | add_rewrite_rule( |
| 175 | - '^\.well-known/oauth-(protected-resource|authorization-server)(?:/.*)?/?$', | |
| 176 | - 'index.php?' . self::WELLKNOWN_QUERY_VAR . '=$matches[1]', | |
| 196 | + '^\.well-known/oauth-(protected-resource|authorization-server)(/.*)?/?$', | |
| 197 | + 'index.php?' . self::WELLKNOWN_QUERY_VAR . '=$matches[1]&' . self::WELLKNOWN_RESOURCE_QUERY_VAR . '=$matches[2]', | |
| 177 | 198 | 'top' |
| 178 | 199 | ); |
| 179 | 200 | // Suffix form: <issuer>/.well-known/... . RFC 8414 specifies the |
| 180 | 201 | // path-INSERT form above, but the older OpenID Connect Discovery |
| @@ -203,8 +224,13 @@ | ||
| 203 | 224 | $expected = [ |
| 204 | 225 | '^thinkrank/mcp/([a-f0-9]{64})/?$', |
| 205 | 226 | '^thinkrank/mcp/?$', |
| 206 | 227 | '^\.well-known/oauth-(protected-resource|authorization-server)/thinkrank/mcp/?$', |
| 228 | + // Listed so an upgrade re-flushes and the pre-#516 rule, which | |
| 229 | + // discarded the resource path, leaves the stored rewrite table. | |
| 230 | + // Without this the old regex keeps matching until someone re-saves | |
| 231 | + // permalinks by hand. | |
| 232 | + '^\.well-known/oauth-(protected-resource|authorization-server)(/.*)?/?$', | |
| 207 | 233 | '^thinkrank/mcp/\.well-known/oauth-(protected-resource|authorization-server)/?$', |
| 208 | 234 | '^thinkrank/mcp/\.well-known/openid-configuration/?$', |
| 209 | 235 | '^thinkrank/authorize/?$', |
| 210 | 236 | ]; |
| @@ -228,13 +254,174 @@ | ||
| 228 | 254 | public function register_query_var( array $vars ): array { |
| 229 | 255 | $vars[] = self::QUERY_VAR; |
| 230 | 256 | $vars[] = self::TOKEN_QUERY_VAR; |
| 231 | 257 | $vars[] = self::WELLKNOWN_QUERY_VAR; |
| 258 | + $vars[] = self::WELLKNOWN_RESOURCE_QUERY_VAR; | |
| 232 | 259 | $vars[] = self::AUTHORIZE_QUERY_VAR; |
| 233 | 260 | return $vars; |
| 234 | 261 | } |
| 235 | 262 | |
| 236 | 263 | /** |
| 264 | + * Resolve a root-form discovery request to the site that owns the resource. | |
| 265 | + * | |
| 266 | + * The path-suffixed and issuer-suffixed rules are already pinned to | |
| 267 | + * `thinkrank/mcp`, so only the broad root-form rule can arrive here naming | |
| 268 | + * something else. Three outcomes: | |
| 269 | + * | |
| 270 | + * - `0` — serve from the current site. That covers the bare form | |
| 271 | + * (`/.well-known/oauth-authorization-server`, the whole reason the | |
| 272 | + * fallback rule exists) and this site's own endpoint path. | |
| 273 | + * - a blog id — a subdirectory multisite request for another site's | |
| 274 | + * resource. On subdirectory multisite everything under the network root | |
| 275 | + * is served by the MAIN site, so a client discovering | |
| 276 | + * `/ca/thinkrank/mcp` lands here; the document has to be built from the | |
| 277 | + * `/ca/` site or every endpoint in it loses the prefix. | |
| 278 | + * - `null` — not ours. Another plugin's resource, an unknown site path, or | |
| 279 | + * a path that merely contains ours. | |
| 280 | + * | |
| 281 | + * @since 2.9.0 | |
| 282 | + * | |
| 283 | + * @param \WP $wp The WP request object. | |
| 284 | + * @return int|null Blog id to serve from, 0 for the current site, null to decline. | |
| 285 | + */ | |
| 286 | + private static function resolve_wellknown_target( $wp ): ?int { | |
| 287 | + $requested = isset( $wp->query_vars[ self::WELLKNOWN_RESOURCE_QUERY_VAR ] ) | |
| 288 | + ? trim( (string) $wp->query_vars[ self::WELLKNOWN_RESOURCE_QUERY_VAR ], '/' ) | |
| 289 | + : ''; | |
| 290 | + | |
| 291 | + $ours = trim( Mcp_Pairing::SITE_ENDPOINT_PATH, '/' ); | |
| 292 | + | |
| 293 | + if ( '' === $requested || $requested === $ours ) { | |
| 294 | + return 0; | |
| 295 | + } | |
| 296 | + | |
| 297 | + if ( ! is_multisite() || ! function_exists( 'get_site_by_path' ) ) { | |
| 298 | + return null; | |
| 299 | + } | |
| 300 | + | |
| 301 | + // Whatever precedes our endpoint path is the candidate site path: | |
| 302 | + // `ca/thinkrank/mcp` -> `/ca/`. Matching the tail as a whole path | |
| 303 | + // segment, not a substring, so `thinkrank/mcp-other` cannot qualify. | |
| 304 | + $candidate = '/' . $requested; | |
| 305 | + $suffix = '/' . $ours; | |
| 306 | + | |
| 307 | + if ( substr( $candidate, - strlen( $suffix ) ) !== $suffix ) { | |
| 308 | + return null; | |
| 309 | + } | |
| 310 | + | |
| 311 | + $site_path = substr( $candidate, 0, - strlen( $ours ) ); | |
| 312 | + $domain = self::request_domain(); | |
| 313 | + | |
| 314 | + if ( '' === $domain || '' === $site_path ) { | |
| 315 | + return null; | |
| 316 | + } | |
| 317 | + | |
| 318 | + $site = get_site_by_path( $domain, $site_path ); | |
| 319 | + | |
| 320 | + if ( ! $site ) { | |
| 321 | + return null; | |
| 322 | + } | |
| 323 | + | |
| 324 | + // get_site_by_path() walks the path segments and falls back to the | |
| 325 | + // network's root site when none match, so an unknown prefix comes back | |
| 326 | + // as the MAIN site rather than as nothing. Taking that at face value | |
| 327 | + // reinstates the exact bug for every path that is not a real subsite: | |
| 328 | + // /nope/thinkrank/mcp would be answered with the main site's document. | |
| 329 | + // Require the match to be the path that was actually asked for. | |
| 330 | + if ( untrailingslashit( (string) $site->path ) !== untrailingslashit( $site_path ) ) { | |
| 331 | + return null; | |
| 332 | + } | |
| 333 | + | |
| 334 | + // get_sites() applies no status filter, so a site the network has taken | |
| 335 | + // out of service resolves like any other. Advertising an authorization | |
| 336 | + // server for one would point a client at an endpoint that cannot serve | |
| 337 | + // it. `public` is deliberately NOT checked: on multisite that flag is | |
| 338 | + // search-engine visibility, not availability, and a site can reasonably | |
| 339 | + // be hidden from search while still running MCP. | |
| 340 | + if ( ! empty( $site->archived ) || ! empty( $site->deleted ) || ! empty( $site->spam ) ) { | |
| 341 | + return null; | |
| 342 | + } | |
| 343 | + | |
| 344 | + return (int) $site->blog_id === get_current_blog_id() ? 0 : (int) $site->blog_id; | |
| 345 | + } | |
| 346 | + | |
| 347 | + /** | |
| 348 | + * Host for a `get_site_by_path()` lookup. | |
| 349 | + * | |
| 350 | + * Mirrors what WordPress itself stores in `wp_blogs`: core strips only the | |
| 351 | + * default ports when it resolves the current site, so a development network | |
| 352 | + * running on a non-default port keeps it, and stripping every port here | |
| 353 | + * would fail to match those rows. | |
| 354 | + * | |
| 355 | + * @since 2.9.0 | |
| 356 | + * | |
| 357 | + * @return string Host, or an empty string when the request carries none. | |
| 358 | + */ | |
| 359 | + private static function request_domain(): string { | |
| 360 | + if ( empty( $_SERVER['HTTP_HOST'] ) ) { | |
| 361 | + return ''; | |
| 362 | + } | |
| 363 | + | |
| 364 | + $host = strtolower( sanitize_text_field( wp_unslash( $_SERVER['HTTP_HOST'] ) ) ); | |
| 365 | + | |
| 366 | + if ( ':80' === substr( $host, -3 ) ) { | |
| 367 | + return substr( $host, 0, -3 ); | |
| 368 | + } | |
| 369 | + | |
| 370 | + if ( ':443' === substr( $host, -4 ) ) { | |
| 371 | + return substr( $host, 0, -4 ); | |
| 372 | + } | |
| 373 | + | |
| 374 | + return $host; | |
| 375 | + } | |
| 376 | + | |
| 377 | + /** | |
| 378 | + * Build a discovery document from the site that owns the resource. | |
| 379 | + * | |
| 380 | + * The switch is what makes the returned endpoints carry the subsite prefix, | |
| 381 | + * since every URL in the document comes from `home_url()` / `rest_url()`. | |
| 382 | + * Settings memoizes per setting name with no notion of which site it read | |
| 383 | + * from; it clears itself on `switch_blog` (see Settings::init), which is | |
| 384 | + * what stops the MCP-enabled check below answering for the previous site. | |
| 385 | + * | |
| 386 | + * Returns null when the owning site has MCP turned off: a site that is not | |
| 387 | + * serving MCP must not advertise an authorization server for it. | |
| 388 | + * | |
| 389 | + * @since 2.9.0 | |
| 390 | + * | |
| 391 | + * @param int $blog_id Blog to build from, 0 for the current site. | |
| 392 | + * @param string $doc Document type from the rewrite. | |
| 393 | + * @return array<string,mixed>|null | |
| 394 | + */ | |
| 395 | + private static function discovery_document_for( int $blog_id, string $doc ): ?array { | |
| 396 | + $switched = false; | |
| 397 | + | |
| 398 | + if ( $blog_id > 0 ) { | |
| 399 | + switch_to_blog( $blog_id ); | |
| 400 | + $switched = true; | |
| 401 | + } | |
| 402 | + | |
| 403 | + $data = null; | |
| 404 | + | |
| 405 | + try { | |
| 406 | + if ( self::is_enabled() ) { | |
| 407 | + $data = 'authorization-server' === $doc | |
| 408 | + ? Mcp_OAuth::authorization_server_metadata() | |
| 409 | + : Mcp_OAuth::protected_resource_metadata(); | |
| 410 | + } | |
| 411 | + } finally { | |
| 412 | + // A throw between the switch and the restore would leave the rest | |
| 413 | + // of the request, including shutdown hooks, running against the | |
| 414 | + // wrong site. Cheap to make impossible. | |
| 415 | + if ( $switched ) { | |
| 416 | + restore_current_blog(); | |
| 417 | + } | |
| 418 | + } | |
| 419 | + | |
| 420 | + return $data; | |
| 421 | + } | |
| 422 | + | |
| 423 | + /** | |
| 237 | 424 | * Serve the MCP endpoint on the pretty path. Runs on parse_request so it |
| 238 | 425 | * fires before the main query, and short-circuits WP entirely. |
| 239 | 426 | * |
| 240 | 427 | * @param \WP $wp The WP request object. |
| @@ -242,16 +429,30 @@ | ||
| 242 | 429 | */ |
| 243 | 430 | public function maybe_handle_pretty_endpoint( $wp ): void { |
| 244 | 431 | // OAuth discovery documents (served at the site root). |
| 245 | 432 | if ( ! empty( $wp->query_vars[ self::WELLKNOWN_QUERY_VAR ] ) ) { |
| 246 | - if ( ! self::is_enabled() ) { | |
| 433 | + // The path after the document type names the RESOURCE being | |
| 434 | + // discovered, and it used to be discarded (#516). Resolve it to | |
| 435 | + // the site that actually owns it, which on subdirectory multisite | |
| 436 | + // is how /ca/thinkrank/mcp stops being answered by the main site | |
| 437 | + // with endpoints that have no /ca/ in them. | |
| 438 | + $target = self::resolve_wellknown_target( $wp ); | |
| 439 | + | |
| 440 | + if ( null === $target ) { | |
| 247 | 441 | status_header( 404 ); |
| 248 | 442 | exit; |
| 249 | 443 | } |
| 250 | - $doc = (string) $wp->query_vars[ self::WELLKNOWN_QUERY_VAR ]; | |
| 251 | - $data = 'authorization-server' === $doc | |
| 252 | - ? Mcp_OAuth::authorization_server_metadata() | |
| 253 | - : Mcp_OAuth::protected_resource_metadata(); | |
| 444 | + | |
| 445 | + $data = self::discovery_document_for( | |
| 446 | + $target, | |
| 447 | + (string) $wp->query_vars[ self::WELLKNOWN_QUERY_VAR ] | |
| 448 | + ); | |
| 449 | + | |
| 450 | + if ( null === $data ) { | |
| 451 | + status_header( 404 ); | |
| 452 | + exit; | |
| 453 | + } | |
| 454 | + | |
| 254 | 455 | status_header( 200 ); |
| 255 | 456 | header( 'Content-Type: application/json; charset=utf-8' ); |
| 256 | 457 | // Discovery metadata is public + cacheable. |
| 257 | 458 | header( 'Cache-Control: public, max-age=3600' ); |
| @@ -414,10 +615,35 @@ | ||
| 414 | 615 | // --- OAuth 2.1 authorization server (the "paste a URL only" path) - |
| 415 | 616 | // Discovery, dynamic client registration, and the token endpoint are |
| 416 | 617 | // all public (permission enforced inside): a client must reach them |
| 417 | 618 | // BEFORE it holds any credential. |
| 619 | + // | |
| 620 | + // The discovery documents are ALSO served here, not only at the | |
| 621 | + // /.well-known/ rewrites: hosts that resolve root /.well-known/ at | |
| 622 | + // their proxy edge (SiteGround) never let those requests reach | |
| 623 | + // WordPress, while /wp-json/ always arrives. The 401 challenge | |
| 624 | + // advertises this route (Mcp_OAuth::resource_metadata_url), so the | |
| 625 | + // flow survives on such hosts. | |
| 418 | 626 | register_rest_route( |
| 419 | 627 | self::NS, |
| 628 | + '/mcp/oauth/protected-resource', | |
| 629 | + [ | |
| 630 | + 'methods' => 'GET', | |
| 631 | + 'callback' => [ $this, 'rest_oauth_discovery_resource' ], | |
| 632 | + 'permission_callback' => '__return_true', | |
| 633 | + ] | |
| 634 | + ); | |
| 635 | + register_rest_route( | |
| 636 | + self::NS, | |
| 637 | + '/mcp/oauth/authorization-server', | |
| 638 | + [ | |
| 639 | + 'methods' => 'GET', | |
| 640 | + 'callback' => [ $this, 'rest_oauth_discovery_server' ], | |
| 641 | + 'permission_callback' => '__return_true', | |
| 642 | + ] | |
| 643 | + ); | |
| 644 | + register_rest_route( | |
| 645 | + self::NS, | |
| 420 | 646 | '/mcp/oauth/register', |
| 421 | 647 | [ |
| 422 | 648 | 'methods' => 'POST', |
| 423 | 649 | 'callback' => [ $this, 'rest_oauth_register' ], |
| @@ -592,8 +818,43 @@ | ||
| 592 | 818 | |
| 593 | 819 | // -- OAuth 2.1 handlers ------------------------------------------------ |
| 594 | 820 | |
| 595 | 821 | /** |
| 822 | + * GET /mcp/oauth/protected-resource — RFC 9728 metadata via REST. | |
| 823 | + * | |
| 824 | + * @return \WP_REST_Response|\WP_Error | |
| 825 | + */ | |
| 826 | + public function rest_oauth_discovery_resource() { | |
| 827 | + return $this->oauth_discovery_response( Mcp_OAuth::protected_resource_metadata() ); | |
| 828 | + } | |
| 829 | + | |
| 830 | + /** | |
| 831 | + * GET /mcp/oauth/authorization-server — RFC 8414 metadata via REST. | |
| 832 | + * | |
| 833 | + * @return \WP_REST_Response|\WP_Error | |
| 834 | + */ | |
| 835 | + public function rest_oauth_discovery_server() { | |
| 836 | + return $this->oauth_discovery_response( Mcp_OAuth::authorization_server_metadata() ); | |
| 837 | + } | |
| 838 | + | |
| 839 | + /** | |
| 840 | + * Shape one discovery document response: public, cacheable, and 404 when | |
| 841 | + * MCP is off — matching the /.well-known/ rewrites exactly, so a client | |
| 842 | + * sees the same truth regardless of which serving path reached it. | |
| 843 | + * | |
| 844 | + * @param array<string,mixed> $document Discovery metadata. | |
| 845 | + * @return \WP_REST_Response|\WP_Error | |
| 846 | + */ | |
| 847 | + private function oauth_discovery_response( array $document ) { | |
| 848 | + if ( ! self::is_enabled() ) { | |
| 849 | + return new \WP_Error( 'thinkrank_mcp_disabled', __( 'MCP is disabled on this site.', 'thinkrank' ), [ 'status' => 404 ] ); | |
| 850 | + } | |
| 851 | + $response = new \WP_REST_Response( $document, 200 ); | |
| 852 | + $response->header( 'Cache-Control', 'public, max-age=3600' ); | |
| 853 | + return $response; | |
| 854 | + } | |
| 855 | + | |
| 856 | + /** | |
| 596 | 857 | * POST /mcp/oauth/register — RFC 7591 dynamic client registration. |
| 597 | 858 | * |
| 598 | 859 | * @param \WP_REST_Request $request JSON body with redirect_uris. |
| 599 | 860 | * @return \WP_REST_Response|\WP_Error |
| @@ -680,14 +941,14 @@ | ||
| 680 | 941 | // phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- compared against a literal after strtoupper(); nothing is stored or echoed. |
| 681 | 942 | $is_post = isset( $_SERVER['REQUEST_METHOD'] ) && 'POST' === strtoupper( (string) wp_unslash( $_SERVER['REQUEST_METHOD'] ) ); |
| 682 | 943 | // Params come from GET on the consent link and POST on the form submit. |
| 683 | 944 | // Nonce is verified below before any POST value is acted on. |
| 684 | - // phpcs:disable WordPress.Security.NonceVerification.Recommended, WordPress.Security.NonceVerification.Missing, WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- each member is sanitize_text_field()ed in the loop below; nothing reads $source directly. | |
| 945 | + // phpcs:disable WordPress.Security.NonceVerification.Recommended, WordPress.Security.NonceVerification.Missing, WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- read verbatim by oauth_param(); see its docblock for why, and where each value is validated or escaped instead. | |
| 685 | 946 | $source = $is_post ? $_POST : $_GET; |
| 686 | 947 | // phpcs:enable WordPress.Security.NonceVerification.Recommended, WordPress.Security.NonceVerification.Missing, WordPress.Security.ValidatedSanitizedInput.InputNotSanitized |
| 687 | 948 | $params = []; |
| 688 | 949 | foreach ( [ 'client_id', 'redirect_uri', 'response_type', 'code_challenge', 'code_challenge_method', 'scope', 'state', 'approve', 'deny', '_thinkrank_oauth_nonce' ] as $k ) { |
| 689 | - $params[ $k ] = isset( $source[ $k ] ) ? sanitize_text_field( wp_unslash( $source[ $k ] ) ) : ''; | |
| 950 | + $params[ $k ] = self::oauth_param( $source, $k ); | |
| 690 | 951 | } |
| 691 | 952 | |
| 692 | 953 | // Validate the OAuth params before touching the session. |
| 693 | 954 | $req = Mcp_OAuth::validate_authorize_request( $params ); |
| @@ -802,10 +1063,10 @@ | ||
| 802 | 1063 | private function emit_consent_screen( array $req ): void { |
| 803 | 1064 | $read_only = Mcp_OAuth::scope_is_read_only( $req['scope'] ); |
| 804 | 1065 | $access_label = $read_only ? __( 'Read-only', 'thinkrank' ) : __( 'Read & write', 'thinkrank' ); |
| 805 | 1066 | $access_desc = $read_only |
| 806 | - ? __( 'Review your SEO across posts and site settings — metadata, schema, sitemaps, robots, social, and SEO scores. No changes are made.', 'thinkrank' ) | |
| 807 | - : __( 'Read and improve your SEO across posts and site settings — metadata, schema, sitemaps, robots, social, indexing, and SEO scores.', 'thinkrank' ); | |
| 1067 | + ? __( 'Review your SEO across posts and site settings metadata, schema, sitemaps, robots, social, and SEO scores. No changes are made.', 'thinkrank' ) | |
| 1068 | + : __( 'Read and improve your SEO across posts and site settings metadata, schema, sitemaps, robots, social, indexing, and SEO scores.', 'thinkrank' ); | |
| 808 | 1069 | $client = '' !== $req['client_name'] ? $req['client_name'] : __( 'An AI assistant', 'thinkrank' ); |
| 809 | 1070 | $action_url = Mcp_OAuth::authorize_url(); |
| 810 | 1071 | $nonce = wp_create_nonce( 'thinkrank_oauth_consent' ); |
| 811 | 1072 | $user = wp_get_current_user(); |
| @@ -919,17 +1180,93 @@ | ||
| 919 | 1180 | |
| 920 | 1181 | // -- Helpers -- |
| 921 | 1182 | |
| 922 | 1183 | /** |
| 1184 | + * Read one /authorize parameter verbatim. | |
| 1185 | + * | |
| 1186 | + * Deliberately NOT sanitize_text_field(). That function exists to make | |
| 1187 | + * untrusted text safe to store and display, and part of what it does is | |
| 1188 | + * strip %XX sequences as an anti-obfuscation measure. Applied to an OAuth | |
| 1189 | + * protocol value it quietly changes the value's meaning. | |
| 1190 | + * | |
| 1191 | + * The concrete failure: registration stores redirect_uris raw from a JSON | |
| 1192 | + * body, but at /authorize the same URI arrives as a query parameter, so | |
| 1193 | + * PHP has already URL-decoded it — and sanitising then removed the percent | |
| 1194 | + * sequences. A client registered with `.../cb?next=%2Fdashboard` was | |
| 1195 | + * compared as `.../cb?next=dashboard`, failed the strict match, and was | |
| 1196 | + * told `invalid_redirect_uri` for sending exactly what it registered | |
| 1197 | + * (#487). `state` has the same problem: it is opaque to us and must | |
| 1198 | + * round-trip byte for byte, or the client aborts its own callback. | |
| 1199 | + * | |
| 1200 | + * Protocol identifiers want validation and rejection, not cleaning. Every | |
| 1201 | + * value read here is constrained somewhere better suited to it: | |
| 1202 | + * - redirect_uri strict in_array() against the client's registered set | |
| 1203 | + * - client_id must resolve to a registered client | |
| 1204 | + * - response_type must equal 'code' | |
| 1205 | + * - code_challenge_method must equal 'S256' | |
| 1206 | + * - code_challenge validated against the RFC 7636 character set | |
| 1207 | + * - scope intersected with SUPPORTED_SCOPES | |
| 1208 | + * - state opaque; escaped at output (esc_attr / rawurlencode) | |
| 1209 | + * - approve/deny tested for emptiness only | |
| 1210 | + * - the nonce passed to wp_verify_nonce() | |
| 1211 | + * | |
| 1212 | + * An array value (`?state[]=x`) reads as absent rather than becoming the | |
| 1213 | + * string "Array". | |
| 1214 | + * | |
| 1215 | + * @since 2.1.0 | |
| 1216 | + * | |
| 1217 | + * @param array<string,mixed> $source $_GET or $_POST. | |
| 1218 | + * @param string $key Parameter name. | |
| 1219 | + * @return string | |
| 1220 | + */ | |
| 1221 | + private static function oauth_param( array $source, string $key ): string { | |
| 1222 | + if ( ! isset( $source[ $key ] ) || ! is_scalar( $source[ $key ] ) ) { | |
| 1223 | + return ''; | |
| 1224 | + } | |
| 1225 | + | |
| 1226 | + return (string) wp_unslash( $source[ $key ] ); | |
| 1227 | + } | |
| 1228 | + | |
| 1229 | + /** | |
| 923 | 1230 | * Read an inbound HTTP header from $_SERVER (for the pretty path). |
| 924 | 1231 | * |
| 1232 | + * Mirrors WP_REST_Server::get_headers(): on Apache with CGI/FastCGI/suPHP the | |
| 1233 | + * Authorization header never lands in HTTP_AUTHORIZATION. WordPress's own | |
| 1234 | + * .htaccess passthrough re-publishes it as REDIRECT_HTTP_AUTHORIZATION, and a | |
| 1235 | + * few Apache module setups populate neither key but do answer getallheaders(). | |
| 1236 | + * The REST route gets this handling from core; the pretty route builds its own | |
| 1237 | + * WP_REST_Request, so it has to do the same here or it 401s on those hosts. | |
| 1238 | + * | |
| 925 | 1239 | * @param string $name Header name. |
| 926 | 1240 | * @return string|null |
| 927 | 1241 | */ |
| 928 | 1242 | private static function server_header( string $name ): ?string { |
| 929 | 1243 | $key = 'HTTP_' . strtoupper( str_replace( '-', '_', $name ) ); |
| 1244 | + | |
| 930 | 1245 | // phpcs:ignore WordPress.Security.ValidatedSanitizedInput -- token compared constant-time downstream; raw header needed verbatim. |
| 931 | - return isset( $_SERVER[ $key ] ) ? wp_unslash( $_SERVER[ $key ] ) : null; | |
| 1246 | + if ( isset( $_SERVER[ $key ] ) && '' !== $_SERVER[ $key ] ) { | |
| 1247 | + // phpcs:ignore WordPress.Security.ValidatedSanitizedInput -- as above. | |
| 1248 | + return wp_unslash( $_SERVER[ $key ] ); | |
| 1249 | + } | |
| 1250 | + | |
| 1251 | + // phpcs:ignore WordPress.Security.ValidatedSanitizedInput -- as above. | |
| 1252 | + if ( isset( $_SERVER[ 'REDIRECT_' . $key ] ) && '' !== $_SERVER[ 'REDIRECT_' . $key ] ) { | |
| 1253 | + // phpcs:ignore WordPress.Security.ValidatedSanitizedInput -- as above. | |
| 1254 | + return wp_unslash( $_SERVER[ 'REDIRECT_' . $key ] ); | |
| 1255 | + } | |
| 1256 | + | |
| 1257 | + if ( function_exists( 'getallheaders' ) ) { | |
| 1258 | + $headers = getallheaders(); | |
| 1259 | + if ( is_array( $headers ) ) { | |
| 1260 | + foreach ( $headers as $header => $value ) { | |
| 1261 | + if ( 0 === strcasecmp( (string) $header, $name ) && '' !== (string) $value ) { | |
| 1262 | + return (string) $value; | |
| 1263 | + } | |
| 1264 | + } | |
| 1265 | + } | |
| 1266 | + } | |
| 1267 | + | |
| 1268 | + return null; | |
| 932 | 1269 | } |
| 933 | 1270 | |
| 934 | 1271 | /** |
| 935 | 1272 | * Emit a WP_REST_Response as a JSON HTTP response and stop. |
| @@ -942,8 +1279,12 @@ | ||
| 942 | 1279 | // MCP Streamable HTTP: advertise the protocol version we speak so a |
| 943 | 1280 | // strict client can pin it. We answer JSON (a spec-permitted response |
| 944 | 1281 | // type); we never open an SSE stream, so no session header is needed. |
| 945 | 1282 | header( 'MCP-Protocol-Version: ' . Mcp_Server::PROTOCOL_VERSION ); |
| 1283 | + // Never cached. The pretty endpoint can carry the pairing token in its | |
| 1284 | + // path, so a shared cache or proxy holding a response keyed on that URL | |
| 1285 | + // would keep an admin-equivalent credential in its store (#396). | |
| 1286 | + header( 'Cache-Control: no-store, private' ); | |
| 946 | 1287 | // Forward any headers the handler set (notably WWW-Authenticate on a |
| 947 | 1288 | // 401, which drives the OAuth discovery flow). |
| 948 | 1289 | foreach ( $response->get_headers() as $name => $value ) { |
| 949 | 1290 | // Re-assert the status on every header: PHP special-cases |
