Skip to content

Adjust ID token handling for proxy mode #7

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Adjust ID token handling for proxy mode #7

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion handlers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,14 @@ func (h *OAuth2Handler) HandleToken(w http.ResponseWriter, r *http.Request) {

// Add ID token if present
if idToken, ok := token.Extra("id_token").(string); ok {
response["id_token"] = idToken
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
}
Comment on lines +589 to +596
Copy link

@coderabbitai coderabbitai bot Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 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:

  1. Discards the original access token from the upstream provider, making it unavailable to clients
  2. Violates OAuth2/OIDC separation of concerns: access_token is for API authorization while id_token is for authentication/identity verification
  3. Breaks standard OAuth2 client expectations that may rely on both tokens being present in their respective fields
  4. 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:

  1. Return both tokens in separate fields and document that clients should use id_token for verification in proxy mode
  2. Add a new custom field (e.g., verifiable_token) to avoid overwriting standard fields
  3. Configure MCP-Trino to accept and verify tokens from the id_token field rather than access_token
  4. 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=go

Length 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 information
  • id_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:

  1. Discards the original access token from the upstream provider (making it unavailable)
  2. Sends an authentication token (id_token) to clients expecting an authorization token
  3. Breaks standard OAuth2 clients that validate token audience or scope claims
  4. 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_token directly
  • 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.

Copy link

@coderabbitai coderabbitai bot Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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.

Suggested change
}
}
🤖 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.

}

// Add scope if present
Expand Down