Skip to content

chore(oauth2): Implement Protected Resource protocol#133

Open
timothedelion wants to merge 7 commits intomainfrom
tdelion/APPAI-524/protected-resource-metadata
Open

chore(oauth2): Implement Protected Resource protocol#133
timothedelion wants to merge 7 commits intomainfrom
tdelion/APPAI-524/protected-resource-metadata

Conversation

@timothedelion
Copy link
Copy Markdown
Member

@timothedelion timothedelion self-assigned this Apr 14, 2026
@linear
Copy link
Copy Markdown

linear Bot commented Apr 14, 2026

@timothedelion timothedelion force-pushed the tdelion/APPAI-524/protected-resource-metadata branch from c5bafa2 to 539003c Compare April 30, 2026 08:47
Claude.ai does not follow the authorization_servers field from RFC
9728 protected resource metadata, so 401 responses now include an
as_metadata parameter pointing to the AS metadata endpoint directly.

Issue: APPAI-524
The thin proxy was overwriting the incoming client_id with the
configured ggshield_oauth on both the /authorize redirect and the
/token form forwarding. That silently dropped any client_id obtained
via RFC 7591 Dynamic Client Registration (e.g. Claude.ai's
auto-registered DCR client), funnelling every MCP user through the
ggshield client.

Fall back to the configured client_id only when the request omits it,
so legacy ggshield CLI behaviour stays the same while DCR clients
reach the GG backend with their registered identity.

Refs: APPAI-524, SI-3215
create_streamable_http_app takes auth as an explicit kwarg and does
not pull it from server.auth, so the StreamableHTTP /mcp route was
wired without RequireAuthMiddleware. Unauthenticated requests slipped
past the HTTP layer and surfaced as a JSON-RPC error envelope deep in
tool execution, with no 401 and no WWW-Authenticate. The OAuth proxy's
discovery routes (well-known metadata, /authorize, /token, /register)
were never registered either.

Refs: APPAI-524, SI-3215
The proxy was hard-coding scopes_supported=["scan"] in both the AS
metadata and the protected-resource metadata. DCR clients (Claude.ai,
Cursor) read this list at registration / authorize time, so they ended
up registered with only "scan" and got 403 on tools whose endpoints
need broader scopes (e.g. /secret_detectors → secrets:read).

Plumb the scopes the MCP server's tools may need (already populated in
GITGUARDIAN_SCOPES by set_{secops,developer}_scopes()) through to the
proxy and override scopes_supported to surface them. The base class
default would alias scopes_supported to required_scopes, which has
the wrong semantics — required_scopes is enforced on every request,
whereas advertised scopes describe the menu DCR clients can pick from.

Refs: APPAI-524, SI-3215
Inlined imports inside functions hide the dependency graph from
readers and make the module's coupling harder to reason about. Move
this one to the top alongside the other gg_api_core imports.
The previous wiring read GITGUARDIAN_SCOPES (populated by
set_{secops,developer}_scopes), which on self-hosted instances is
narrowed to MINIMAL_SCOPES — too narrow for tools like
list_detectors (incidents:read) and most write tools.

Use ALL_SCOPES instead. Per RFC 6749 §3.3, the registered scope is
the upper bound for any future authorize request, so capping it
narrowly at registration would block scopes the user might want to
consent to. The granted set on the issued token still reflects only
what the user actually approves.

Refs: APPAI-524, SI-3215
@timothedelion timothedelion force-pushed the tdelion/APPAI-524/protected-resource-metadata branch from 9ba1e2f to b381f80 Compare May 2, 2026 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant