Skip to content

fix: support protected resource enterprise credentials#4389

Merged
joeyorlando merged 15 commits intomainfrom
fix/protected-resource-enterprise-auth
May 6, 2026
Merged

fix: support protected resource enterprise credentials#4389
joeyorlando merged 15 commits intomainfrom
fix/protected-resource-enterprise-auth

Conversation

@joeyorlando
Copy link
Copy Markdown
Contributor

@joeyorlando joeyorlando commented May 5, 2026

Summary

  • support protected-resource credential exchange for enterprise-managed MCP transports
  • synthesize read-resource tools for resource-only MCP servers during discovery
  • preserve enterprise-managed credential mode when assigning installed catalog tools
  • keep chat tool discovery available when session-derived external gateway auth fails
  • validate gateway tokens through the configured secret manager and allow email-shaped JWT subjects when the email claim is omitted
  • centralize OAuth token, grant, and client assertion constants shared by backend and frontend identity-provider flows
  • type enterprise subject-token form options against generated API types while using shared runtime constants
  • clarify Okta SP-initiated SSO in the supported features docs table

Archestra Contributor

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 5, 2026

Claude finished @joeyorlando's task in 7m 24s —— View job


PR Review: fix: support protected resource enterprise credentials

PR #4389 · 44 files · +1440 / -197


Summary

This PR adds a two-step enterprise credential exchange flow for OAuth Protected Resources (RFC 8414). When an agent's tool is configured with resourceType: "oauth_protected_resource" and requestedCredentialType: "id_jag", the broker:

  1. Exchanges the user's IdP token for an ID-JAG at the configured identity provider
  2. Discovers the protected resource's authorization server via /.well-known/oauth-protected-resource metadata
  3. Exchanges the ID-JAG at the authorization server's token endpoint for an access token

The PR also introduces synthesized read-resource tools for resource-only MCP servers, a fallback chain from external IdP tokens to internal gateway tokens, and centralizes OAuth URN constants into @shared.


Findings

Code Quality & Architecture

Well-structured flow decomposition. The broker cleanly separates the two-step exchange (exchangeSessionAssertionForIdJagexchangeIdJagAtProtectedResource) from the existing single-step path. The shouldExchangeIdJagAtProtectedResource guard makes the branching explicit and testable.

Good use of shared constants. Moving OAUTH_TOKEN_TYPE, OAUTH_GRANT_TYPE, and OAUTH_CLIENT_ASSERTION_TYPE into @shared eliminates scattered inline URN strings across frontend and backend. This is the right pattern for protocol-level constants.

Synthesized read-resource tools are a pragmatic addition. Catching JSON-RPC method-not-found errors from tools/list and falling back to resources/list handles resource-only MCP servers without requiring server-side changes.

Potential Issues

  1. discoverProtectedResourceTokenEndpoint trusts the first authorization server unconditionally (broker.ts:199)
    The function picks authorizationServers[0] without validating the URL origin. If a malicious or misconfigured protected resource metadata document points to an attacker-controlled authorization server, the broker will POST the ID-JAG assertion there. Consider validating that the authorization server URL shares the same origin or base domain as the resource identifier, or at minimum logging a warning when they diverge.

  2. No caching for protected resource metadata discovery (broker.ts:191-218)
    Every credential exchange performs two sequential HTTP fetches (resource metadata + auth server metadata) before the actual token exchange. For high-frequency tool calls, this triples the outbound request count. The cacheTtlSeconds field exists on EnterpriseManagedCredentialConfig but doesn't appear to be used for metadata caching. Consider caching the discovered token endpoint keyed by resourceIdentifier.

  3. private_key_jwt not supported for protected resource exchange (broker.ts:280-283)
    buildProtectedResourceTokenHeaders throws for private_key_jwt authentication. This is fine as a starting point, but worth noting in documentation or the error message that this is a known limitation — some enterprise IdPs (especially Entra ID certificate-based auth) may require it.

  4. fallbackMode and cacheTtlSeconds defined but not enforced (types/enterprise-managed-credentials.ts)
    These fields exist on EnterpriseManagedCredentialConfig but I don't see enforcement in the broker or exchange logic. If they're planned for a follow-up, consider adding a TODO comment or removing them from the type to avoid confusion.

  5. Entra OBO hardcodes credentialType: "bearer_token" (entra-obo-strategy.ts)
    The Entra OBO exchange strategy always returns credentialType: "bearer_token" regardless of what the token response actually contains. This works for the common case but could be misleading if the response includes a different token_type. Minor concern — just worth being aware of.

  6. Email-from-subject fallback uses a simple regex (mcp-gateway.utils.ts)
    getEmailFromSubject uses /^[^\s@]+@[^\s@]+\.[^\s@]+$/ to detect email-shaped sub claims. This is appropriately loose for a best-effort fallback after token validation has already succeeded — not a security concern, just noting it accepts some technically invalid email formats.

Security

  • Prototype pollution guards are properly implemented in both broker.ts:getValueAtPath and mcp-server.ts:extractInstallDiscoveryCredentialValue. Both block __proto__, constructor, and prototype path segments.
  • AbortSignal.timeout(10_000) on all outbound fetches prevents hanging on unresponsive IdPs/auth servers.
  • Client secrets handled appropriatelyENTERPRISE_MANAGED_CLIENT_SECRET_OVERRIDE_SECRET_KEY routes through the secrets manager rather than storing in the database directly.

Test Coverage

Comprehensive test coverage across all new features:

  • broker.test.ts — session IdP token exchange, external IdP passthrough, ID-JAG protected resource exchange
  • session-token.test.ts — subject token preference resolution, access token refresh, expiry handling
  • mcp-client.test.ts — synthesized read-resource tools, method-not-found handling
  • chat-mcp-client.test.ts — fallback from external IdP to internal gateway
  • mcp-gateway.utils.test.ts — email-shaped subject fallback
  • rfc8693-token-exchange.test.ts — protected resource exchange flow

Tests use real PGlite database and test fixtures per project conventions.

Documentation

Documentation updates in platform-enterprise-managed-auth.md and platform-mcp.md accurately describe the new protected resource flow and synthesized resource tools. The OpenAPI spec includes the new clientSecretOverride field.


Verdict

Approve with suggestions. The core architecture is sound and well-tested. The two items most worth addressing before merge are (1) the lack of origin validation on discovered authorization server URLs and (2) the missing metadata cache for protected resource discovery. The remaining items are minor and could be addressed in follow-ups.

@joeyorlando joeyorlando merged commit 7d2f3b2 into main May 6, 2026
34 checks passed
@joeyorlando joeyorlando deleted the fix/protected-resource-enterprise-auth branch May 6, 2026 18:09
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