Adjust ID token handling for proxy mode #7
Conversation
In proxy mode, the `id_token` from the upstream identity provider is now mapped to `access_token` in the response. This ensures that the token is verifiable, whereas the upstream access token might be from a different issuer. Without this fix, authentication to MCP-Trino appears to work, but upon making any requests, there are token verification failures.
WalkthroughOAuth2 token response handling is modified to place the ID token value into the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Handler
participant OAuth2
Client->>Handler: Token exchange request
Handler->>OAuth2: Generate ID token
OAuth2-->>Handler: ID token value
alt Proxy Mode
Handler->>Handler: Place ID token in access_token field
else Native Mode
Handler->>Handler: Place ID token in id_token field
end
Handler-->>Client: Token response (restructured)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| if h.config.Mode == "proxy" { | ||
| // In proxy mode, trino-mcp is going to expect to receive id tokens | ||
| // that can be validated, not access tokens which can be opaque or | ||
| // from another issuer (e.g. Microsoft Graph when using Azure). | ||
| response["access_token"] = idToken | ||
| } else { | ||
| response["id_token"] = idToken | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Critical: Overwriting access_token with id_token violates OAuth2/OIDC semantics and discards the original access token.
In proxy mode, this change overwrites the access_token field (originally set on line 577) with the id_token value, which:
- Discards the original access token from the upstream provider, making it unavailable to clients
- Violates OAuth2/OIDC separation of concerns:
access_tokenis for API authorization whileid_tokenis for authentication/identity verification - Breaks standard OAuth2 client expectations that may rely on both tokens being present in their respective fields
- Omits the id_token field entirely from the proxy mode response, which some clients may expect
While this may resolve the immediate MCP-Trino verification issue, it creates a non-standard token response that could break other OAuth2 clients or integrations.
Alternative approaches to consider:
- Return both tokens in separate fields and document that clients should use
id_tokenfor verification in proxy mode - Add a new custom field (e.g.,
verifiable_token) to avoid overwriting standard fields - Configure MCP-Trino to accept and verify tokens from the
id_tokenfield rather thanaccess_token - Document the behavior and validate that all downstream consumers can handle this non-standard response
Please verify the OAuth2/OIDC implications of this approach:
Additionally, run the following script to identify other potential consumers of the token endpoint:
🏁 Script executed:
#!/bin/bash
# Description: Find references to token endpoint and token response handling to assess impact
# Search for token response handling
rg -n -C3 -P '(access_token|id_token).*token\.(AccessToken|Extra)' --type=go
# Search for HandleToken callers or references
rg -n -C3 'HandleToken|/token' --type=goLength of output: 7346
🌐 Web query:
OAuth2 and OIDC spec: Can id_token be used as access_token? What are the semantic differences between access_token and id_token?
💡 Result:
Short answer
- No — the OIDC id_token is not a substitute for an OAuth2 access_token. The specs treat them as different tokens with different audiences and purposes. [1][2]
Key semantic differences
-
Purpose
- access_token: credential the client presents to a resource server to authorize API/resource access (represents scopes/permissions). [2]
- id_token: proof to the client that an end-user was authenticated; carries identity claims (iss, sub, aud, nonce, etc.). It is intended for the client (relying party), not for resource servers. [1]
-
Audience / who consumes it
- access_token: audience = resource server(s). The resource server must understand/validate it. [2]
- id_token: audience = client_id (the relying party). The client validates it and consumes the identity claims. [1]
-
Format and semantics
- access_token: can be opaque or structured (JWT); contains authorization data (scope, expiry) or an identifier for server-side lookup. [2]
- id_token: defined by OIDC as a signed (optionally encrypted) JWT containing authentication claims. Includes at_hash/c_hash when issued with access_token or code. [1]
-
Validation, revocation, and lifecycle
- access_tokens are designed for resource access, have scopes, lifetimes, and may be introspected or revoked by the authorization server. [2]
- id_tokens assert authentication and are not meant for resource authorization; they are not intended to be used for calling APIs and are not a replacement for an access token. [1][2]
Security note
- Using an id_token as an access token breaks the intended audience/validation model and can create security problems (token misuse, wrong audience, inability for resource servers to enforce scopes/introspection). OIDC explicitly provides at_hash to bind an access_token to an id_token when both are issued. [1][2]
Sources
- OpenID Connect Core 1.0 (ID Token semantics, at_hash, audience/intended use). [1]
- RFC 6749 — OAuth 2.0 Authorization Framework (access_token definition, scope, usage). [2]
- RFC 6750 — Bearer Token Usage (how access tokens are used to call resources). [3]
Critical: Code violates OAuth2/OIDC specification by substituting id_token for access_token.
Lines 589-596 in proxy mode overwrite the access_token field (set on line 577) with the id_token value. Per the OAuth2 and OIDC specifications, these tokens serve fundamentally different purposes and audiences:
access_token: credential for API/resource server authorization; contains scope and permission informationid_token: authentication assertion for the client (relying party) containing identity claims
Using an id_token as an access_token breaks the intended audience/validation model and can create security problems. This change:
- Discards the original access token from the upstream provider (making it unavailable)
- Sends an authentication token (id_token) to clients expecting an authorization token
- Breaks standard OAuth2 clients that validate token audience or scope claims
- Violates RFC 6749 (OAuth 2.0) and OpenID Connect Core 1.0
Recommended fixes:
- Return both tokens in their proper fields (accept id_token verification in MCP-Trino instead of changing token semantics)
- Add conditional token field mapping only if MCP-Trino cannot be configured to use
id_tokendirectly - Document and validate that all downstream clients can handle non-standard token responses
🤖 Prompt for AI Agents
In handlers.go around lines 589 to 596, the code currently overwrites the
access_token with the id_token in proxy mode which violates OAuth2/OIDC
semantics; instead preserve the original access_token set earlier and add the
id_token as a separate field (e.g., response["id_token"] = idToken) so both
tokens are returned, and only perform any mapping if an explicit configuration
flag requires replacing the access token; also ensure you document this behavior
and validate downstream clients can accept id_token if you must change
semantics.
| response["access_token"] = idToken | ||
| } else { | ||
| response["id_token"] = idToken | ||
| } |
There was a problem hiding this comment.
Fix indentation inconsistency.
Line 596 has excessive indentation (appears to use more spaces than the surrounding code).
Apply this diff:
- }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } | |
| } |
🤖 Prompt for AI Agents
In handlers.go around line 596, the closing brace has excessive indentation
compared to the surrounding block; reduce the leading spaces so the brace aligns
with the other block braces (match the repository's indentation style—tabs or
the same number of spaces used nearby), save the file, and run the
formatter/linter to ensure consistent indentation across the file.
1 participant
In proxy mode, the
id_tokenfrom the upstream identity provider is now mapped toaccess_tokenin the response. This ensures that the token is verifiable, whereas the upstream access token might be from a different issuer.Without this fix, authentication to MCP-Trino appears to work, but upon making any requests, there are token verification failures.
TBH I'm not 100% confident in this fix (not an expert in oauth or authentication in general)... but I needed it in order to get authentication for Cursor via azure working (in proxy mode). Below is the config I was using (with aggressive redaction 😛):
I'd be happy to learn that I'm just using it incorrectly, but figured this was a good place to start the conversation. Thanks!
Summary by CodeRabbit