[Server] Narrow authorization to the resource server role by chr-hertel · Pull Request #532 · modelcontextprotocol/php-sdk · GitHub

Narrows the auth layer to the resource server role, see ADR 0002 - the proxy and client registration couldn't be hardened without becoming an authorization server, and the 2026-07-28 spec doesn't need them since the metadata points clients at the AS directly.

[BC Break] across Mcp\Server\Transport\Http\OAuth & the auth middleware, see CHANGELOG under 0.9.0.

Remove OAuthProxyMiddleware, ClientRegistrationMiddleware, ClientRegistrarInterface - DCR is deprecated in 2026-07-28 and the proxied flow conflicts with RFC 9207 iss validation and CIMD
Replace OAuthRequestMetaMiddleware with RequestContext::getAccessToken() - handlers get the validated token from the request context instead of _meta.oauth, and the body isn't re-encoded anymore
ScopePolicy on AuthorizationMiddleware for per-method/per-tool scopes incl. hierarchy => 403 insufficient_scope
JwtTokenValidator is final, uses firebase's CachedKeySet (refetch on unknown kid), enforces the alg allowlist, optional typ/leeway
ProtectedResourceMetadata requires resource, derives the metadata path & challenge URL from it, https except loopback
WWW-Authenticate exposed via CORS
Both OAuth examples are plain resource servers now

Checked Drupal's mcp_server/mcp_server_oauth, Sulu's SuluMcpBundle, API Platform, Shopware and symfony/mcp-bundle - none of them uses the removed or changed classes.

Not run against a live Keycloak or Entra yet - the examples need a manual check.

cc @Nyholm @CodeWithKyrian @soyuka WDYT?

* Remove OAuthProxyMiddleware, ClientRegistrationMiddleware and their interfaces (ADR 0002) * Replace OAuthRequestMetaMiddleware with RequestContext::getAccessToken() * Add ScopePolicy to AuthorizationMiddleware for 403 insufficient_scope step-up * JwtTokenValidator: final, CachedKeySet with refetch on unknown kid, alg allowlist, token type, leeway * ProtectedResourceMetadata: require resource, derive metadata path and challenge URL from it, enforce https * Expose WWW-Authenticate via CORS

maybe worth a comment ? (for human readers :p)

I like the readability but not a huge fan of the algorithm's complexity, imo a simple foreach loop with an indexed array would lower the complexity behind $required here. (nit)

interesting algorithm and this one is optimized ^^

we already looped onto scopes so we could compute this in the above loop

Nice refactoring/cleanup!

Thanks Chris. The narrowing makes sense to me. I checked SuluMcpBundle and symfony/mcp-bundle against the diff. Neither uses the removed or changed classes.

Needs a fix before merge

An unknown kid returns a 500 on the fromIssuer() path. CachedKeySet::offsetGet throws \OutOfBoundsException. The catch in JwtTokenValidator.php:129 does not cover it. The same happens after a key rotation once the refetch limit is used up. An unreachable JWKS endpoint throws a PSR-18 exception that is not caught either. testRejectsUnknownKeyId uses a plain key array, so this path has no test. Please map these to 401 and add a test with a real CachedKeySet.
Tokens without exp are accepted forever. php-jwt only checks exp when it is present. Please require a numeric exp.
Keys without alg break. fromIssuer() passes $algorithms[0] as defaultAlg. With ['RS256', 'ES256'], EC keys without alg get tagged RS256. The CachedKeySet example in the docs passes no defaultAlg at all. I think Entra JWKS keys have no alg, so the Microsoft example may fail. I have not run it.
The CHANGELOG misses some BC breaks. Protocol::processInput, BaseTransport::handleMessage and StreamableHttpTransport::handlePostRequest gained a parameter. Subclasses that override them with the old signature will fatal.

Smaller points

AccessToken::hasScope() ignores the implies hierarchy. The docs tell handlers to use it. A files:admin token passes the middleware but fails hasScope('files:write').
A non-Bearer scheme gets 400 invalid_request. RFC 6750 §3.1 asks for 401 without an error code, so the client sees no discovery challenge.
ProtectedResourceMetadata drops the query string of the resource. RFC 9728 §3.1 keeps it.
fromIssuer() does discovery at construction time. If the authorization server is down while the container builds, validation fails.
Missing tests: SecureUrl edge cases and 403 on batch requests.

Question on the token seam

RequestContext::getAccessToken() is only filled by the SDK's AuthorizationMiddleware. Both transports read the PSR-7 request attribute AccessToken::class. That is currently a docblock convention. Bundles that authenticate outside the SDK, like Sulu's, need a way to hand over a token. Could you document the attribute as public API? new AccessToken($scopes, $claims) is already public, so that would be enough.

Smoke test

I'm happy to run a smoke test against the Sulu MCP bundle once this is ready. Ping me when the findings are in.

Keys without alg are tagged with the token's algorithm instead of the first allowed one.

chr-hertel requested review from CodeWithKyrian, Nyholm and soyuka as code owners October 5, 2026 22:07

chr-hertel added Server Issues & PRs related to the Server component breaking change Breaking the Backwards Compatibility Promise auth Issues and PRs related to Authentication / OAuth labels Oct 5, 2026

chr-hertel mentioned this pull request Oct 5, 2026

[Server] Narrow authorization to the resource server role chr-hertel/php-sdk#55

Closed

soyuka reviewed Oct 6, 2026

View reviewed changes

soyuka previously approved these changes Oct 6, 2026

View reviewed changes

chr-hertel added 11 commits October 6, 2026 23:14

[Server] Answer unresolvable token signing keys with 401

d33e8bc

[Server] Reject access tokens without exp

4fa5521

[Server] Discover issuer keys lazily, one key set per algorithm

6f8dba8

Keys without alg are tagged with the token's algorithm instead of the first allowed one.

[Server] Document the access token parameter BC breaks

ee0f742

[Server] Dedupe scopes on insert in ScopePolicy

8e8bb87

[Server] Hand on implied scopes with the access token

4dc4964

[Server] Challenge other authorization schemes without error code

019f5ed

[Server] Explain bearer parsing and header escaping

7016500

[Server] Keep the resource query in the protected resource metadata URL

126edeb

[Server] Cover SecureUrl and forbidden batch requests

7bd1624

[Docs] Document the AccessToken request attribute as integration seam

344d46e

chr-hertel dismissed soyuka’s stale review via 344d46e October 6, 2026 21:23

chr-hertel added 4 commits October 7, 2026 00:42

[Examples] Note that Entra step-up needs the prefixed scope

3b9d784

[Server] Require a numeric exp in access tokens

f539399

[Server] Expand the scope hierarchy once per request

78a3aee

[Server] Share scope validation between policy and metadata

68fee28

chr-hertel added 2 commits October 7, 2026 00:45

[Server] Back off briefly after a failed OIDC discovery

09803b0

[Server] Note the global leeway swap in JwtTokenValidator

930571a

[Server] Narrow authorization to the resource server role by chr-hertel · Pull Request #532 · modelcontextprotocol/php-sdk · GitHub

Repository navigation

[Server] Narrow authorization to the resource server role - #532

[Server] Narrow authorization to the resource server role#532
chr-hertel wants to merge 18 commits into
modelcontextprotocol:mainfrom
chr-hertel:auth-resource-server-only

chr-hertel commented Oct 5, 2026

Uh oh!

soyuka Oct 6, 2026 •
edited
Loading

Uh oh!

Uh oh!

soyuka Oct 6, 2026

Uh oh!

soyuka Oct 6, 2026

Uh oh!

soyuka Oct 6, 2026

Uh oh!

soyuka left a comment

Uh oh!

wachterjohannes commented Oct 6, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

                   private function escapeHeaderValue(string $value): string
                   {
-                      return str_replace(['\\', '"'], ['\\\\', '\\"'], $value);
+                      return str_replace(['\\', '"'], ['\\\\', '\\"'], preg_replace('/[\x00-\x1F\x7F]/', '', $value) ?? '');

+                          }
+                      }
+                      return array_values(array_unique($required));

+                          $effective[$scope] = true;
+                          array_push($pending, ...($this->implies[$scope] ?? []));
+                      }

+                          }
+                      }
+                      return array_values(array_unique($scopes));

[Server] Narrow authorization to the resource server role by chr-hertel · Pull Request #532 · modelcontextprotocol/php-sdk · GitHub

Repository navigation

[Server] Narrow authorization to the resource server role - #532

[Server] Narrow authorization to the resource server role#532chr-hertel wants to merge 18 commits intomodelcontextprotocol:mainmodelcontextprotocol/php-sdk:mainfrom chr-hertel:auth-resource-server-onlychr-hertel/php-sdk:auth-resource-server-onlyCopy head branch name to clipboard

Conversation

chr-hertel commented Oct 5, 2026

Uh oh!

soyuka Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

soyuka Oct 6, 2026

Choose a reason for hiding this comment

Uh oh!

soyuka Oct 6, 2026

Choose a reason for hiding this comment

Uh oh!

soyuka Oct 6, 2026

Choose a reason for hiding this comment

Uh oh!

soyuka left a comment

Choose a reason for hiding this comment

Uh oh!

wachterjohannes commented Oct 6, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[Server] Narrow authorization to the resource server role#532
chr-hertel wants to merge 18 commits into
modelcontextprotocol:mainfrom
chr-hertel:auth-resource-server-only

soyuka Oct 6, 2026 •
edited
Loading