[OAuth] Public resource-server contracts, an extensible scope catalogue, and resource-bound tokens - #2028
Merged
wwidergoldpimcore merged 14 commits intoSep 4, 2026
Conversation
Bundles that expose their own OAuth-protected endpoints need to validate tokens and register their endpoints as protected resources, which @internal forbade. No behaviour change: docblocks only. Co-Authored-By: Claude <noreply@anthropic.com>
Adds an OAuth-Protected Applications page: the two roles, the public contracts, and a blueprint for adding an application, with Pimcore MCP and Data Hub Simple REST as the first two. Also corrects the MCP page, which claimed the authenticator validates the audience; it does not. Co-Authored-By: Claude <noreply@anthropic.com>
register() stored the caller's URI verbatim while the newly public docblock said the registry canonicalises, so the RFC 9728 document could echo a non-canonical resource. Unreadable key material now returns null instead of escaping as a 500. Corrects the blueprint doc, which described a firewall its stated reference implementation does not use. Co-Authored-By: Claude <noreply@anthropic.com>
…ible Implements RFC 8707: a client names the resource it wants a token for, the server validates it, stamps it as `aud` and refuses the token elsewhere. Without this any token opened every protected resource, which only became exploitable once a second one existed. Tokens with no audience stay valid everywhere, so clients that do not ask are unaffected. Scopes are now contributed by tagged providers, so a bundle can ship its own namespace instead of reusing `mcp:*`. Co-Authored-By: Claude <noreply@anthropic.com>
The `aud` claim carried the client's spelling of the resource, so every consumer had to canonicalise before comparing. A registration that omits `scope` no longer falls back to the first registered scope, which depended on bundle order. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ej6YARj6HakERFuERFuDF9
The resource is now mandatory: a control the client can decline is not a control, and an unbound token was accepted by every protected resource of this server. Replaces two mirrored league methods with a resource column on the token record. completeAuthorizationRequest and BearerTokenResponse::generateHttpResponse were duplicated wholesale to smuggle one field through league's payloads; the binding is now written when the code and refresh token are issued and read back by id, so both mirrors are gone and the audience is queryable. Recovery also moves out of validateClient, which was the wrong layer. Co-Authored-By: Claude <noreply@anthropic.com>
The server requires a request to name its resource, but the validator still honoured a token without one at every resource, so a single token that reached either end unbound opened every protected resource of the server. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ej6YARj6HakERFuERFuDF9
No discovery document lists an authorization server's resources, so a refusal was a dead end. It now names them, and says plainly when none are configured, which is otherwise indistinguishable from naming the wrong one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ej6YARj6HakERFuERFuDF9
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Core audience propagation lacks automated regression coverage, and multiple changed comments document the opposite security behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds extensible OAuth scopes, public resource-server contracts, and fail-closed RFC 8707 audience binding.
Changes:
- Introduces tagged scope providers and a shared scope registry.
- Persists resource bindings through authorization-code and refresh flows.
- Enforces token audiences and documents third-party resource-server integration.
Review assessment: The implementation addresses the root cause at the grant, persistence, and validation boundaries. Call sites and DI wiring were updated without identified public API breakage, but core token-exchange paths lack regression coverage and several comments contradict fail-closed behavior.
File summaries
| File | Description |
|---|---|
tests/Unit/OAuth/Token/EmbeddedTokenValidatorTest.php |
Tests audience validation. |
tests/Unit/OAuth/Server/ScopeRepositoryTest.php |
Tests registry-backed scopes. |
tests/Unit/OAuth/Server/Grant/LoopbackAuthCodeGrantTest.php |
Tests resource request validation. |
tests/Unit/OAuth/Server/ClientRegistrarTest.php |
Tests extensible registration scopes. |
tests/Unit/OAuth/Registry/ConfigProtectedResourceRegistryTest.php |
Tests resource canonicalization. |
tests/Unit/OAuth/Controller/AuthorizationServerMetadataControllerTest.php |
Tests advertised scopes. |
src/PimcoreStudioBackendBundle.php |
Registers the scope compiler pass. |
src/OAuth/Token/EmbeddedTokenValidator.php |
Enforces token audiences. |
src/OAuth/Server/RequestType/ResourceAuthorizationRequest.php |
Carries authorization resources. |
src/OAuth/Server/Repository/TokenRecordStoreInterface.php |
Adds resource persistence APIs. |
src/OAuth/Server/Repository/TokenRecordStore.php |
Stores token resources. |
src/OAuth/Server/Repository/ScopeRepository.php |
Uses the scope registry. |
src/OAuth/Server/Repository/RefreshTokenRepository.php |
Persists refresh audiences. |
src/OAuth/Server/Grant/ResourceRefreshTokenGrant.php |
Preserves audiences on refresh. |
src/OAuth/Server/Grant/LoopbackAuthCodeGrant.php |
Validates and propagates resources. |
src/OAuth/Server/Entity/AccessTokenEntity.php |
Emits the aud claim. |
src/OAuth/Server/ClientRegistrar.php |
Validates registered scopes dynamically. |
src/OAuth/Server/AuthorizationServerFactory.php |
Wires resource-aware grants. |
src/OAuth/Scope/McpScopeProvider.php |
Contributes MCP scopes. |
src/OAuth/Registry/ScopeRegistry.php |
Aggregates scope providers. |
src/OAuth/Registry/ConfigProtectedResourceRegistry.php |
Canonicalizes registered resources. |
src/OAuth/Dto/ResolvedAccess.php |
Marks access results public. |
src/OAuth/Dto/ProtectedResourceMetadata.php |
Marks metadata public. |
src/OAuth/Dto/ProtectedResource.php |
Marks resources public. |
src/OAuth/Controller/AuthorizationServerMetadataController.php |
Advertises registry scopes. |
src/OAuth/Contract/TokenValidatorInterface.php |
Opens token validation publicly. |
src/OAuth/Contract/ScopeRegistryInterface.php |
Defines the public catalogue API. |
src/OAuth/Contract/ScopeProviderInterface.php |
Defines scope contributions. |
src/OAuth/Contract/ResourceRegistryInterface.php |
Opens resource registration publicly. |
src/Migrations/Version20260901120000.php |
Adds the resource column. |
src/Entity/OAuth/OAuthTokenRecord.php |
Maps stored resources. |
src/DependencyInjection/CompilerPass/OAuthScopeProviderPass.php |
Validates tagged providers. |
doc/04_Development_Details/README.md |
Links OAuth integration guidance. |
doc/04_Development_Details/08_MCP_Server.md |
Reframes MCP OAuth usage. |
doc/04_Development_Details/07_OAuth_Protected_Applications.md |
Documents resource-server integration. |
doc/02_Installation_and_Configuration/README.md |
Generalizes OAuth introduction. |
doc/02_Installation_and_Configuration/06_OAuth_Server.md |
Documents required resource binding. |
config/oauth.yaml |
Registers scope services. |
Review details
- Files reviewed: 38/38 changed files
- Comments generated: 8
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The audience claim, the authorization-code round trip and the refresh binding had no tests, though each keys off a league-internal payload field. Prose written before the validator failed closed still described the old fail-open behaviour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ej6YARj6HakERFuERFuDF9
…king league returns null for an absent state or code challenge and its setters reject null, so copying them onto the resource-carrying request turned a spec-legal request into an uncaught TypeError and a 500 before consent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ej6YARj6HakERFuERFuDF9
It described the earlier design where both surfaces shared one resource. They are separate resources, and a token minted for either is refused at the other. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ej6YARj6HakERFuERFuDF9
A full pass found claims that never held or stopped holding: dynamic registration does issue client secrets, the resource-registration step contradicted itself on gating and on the URI source, and two statements promised MCP specifics this bundle does not ship. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ej6YARj6HakERFuERFuDF9
This was referenced Sep 3, 2026
RFC 8707: the token named a resource, so it should carry only the scopes that resource can process, and the consent screen should say so. Narrowing happens on the authorization request, since that is what the screen renders; the granted set is then reported in the `scope` response parameter league omits entirely. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ej6YARj6HakERFuERFuDF9
`scopes_supported` reads as advisory in both pages, and the note saying scopes are not enforced now understates what happens at issuance. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ej6YARj6HakERFuERFuDF9
|
wwidergoldpimcore
approved these changes
Sep 4, 2026
wwidergoldpimcore
merged commit Sep 4, 2026
92789bc
into
feature/oauth-1308-basic-integration
20 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Splits the OAuth half out of #2022, so the authorization server can land on a different release than
the MCP server configuration. #2022 will be closed; its MCP-related changes follow in a second PR
once
feature/mcp-1309-server-config-managementis rebased onto this branch.What this does
Opens the resource-server contracts as public API.
ResourceRegistryInterface,TokenValidatorInterface,ProtectedResource,ProtectedResourceMetadataandResolvedAccesslose@internal. Any bundle can now make its endpoints an OAuth resource server; Data Hub Simple REST isthe first consumer (pimcore/data-hub-simple-rest#356).
Makes the scope catalogue extensible. Scopes were a hard-coded
['mcp:read', 'mcp:write']inScopeRepository. A bundle now contributes its own by tagging aScopeProviderInterfaceservice withpimcore_studio_backend.oauth.scope_provider; the registry is what the metadata document advertises.The bundle's own MCP scopes move into
McpScopeProviderunchanged.Binds tokens to a resource (RFC 8707). The authorization request names the resource it wants a
token for, the resource is stamped onto the token as
aud, and it is enforced when the token ispresented, so a token minted for one resource is refused at another. The binding travels with the
authorization code, so the token request does not repeat it and a refresh keeps the original resource.
A
resourcecolumn on the token record carries it.Fails closed at both ends. A request naming no resource, or an unknown one, is refused; a token
carrying no audience is refused everywhere rather than accepted everywhere. Previously only one end
was guarded, so any unbound token that appeared would have opened every protected resource at once.
Says what to do when a request is refused. No discovery document lists an authorization server's
resources, so a refusal was a dead end. It now names the known resources, and distinguishes "this
server has no protected resources configured" from "you named one that does not exist".
Behaviour changes
resourceis required on the authorization request. Clients that do not implement RFC 8707 must addit. MCP clients already send it, and Data Hub Simple REST's Swagger UI adds it for the same reason.
audis no longer accepted. OAuth is unreleased, so nothing is in circulation.scopeyields no scope, rather than inheriting whichever scope happened tobe registered first, which depended on bundle registration order.
Verified
Unit suite green (828 tests), PHPStan clean. Driven end to end against a demo instance together with
pimcore/data-hub-simple-rest#356: dynamic client registration, authorization, consent, token, refresh,
and an MCP
tools/listcall over the resulting token. A token minted for the REST resource is refusedat the MCP endpoint and vice versa.
Not in scope
OAuthAccessTokenAuthenticatoris untouched. The endpoints behind thepimcore_mcpfirewall are ownedby other bundles, and deriving their audience belongs with the MCP server configuration in the
follow-up PR.
🤖 Generated with Claude Code