Skip to content

Conversation

Copy link

Copilot AI commented Oct 29, 2025

Fix OAuth client enumeration bug in oidc-auth-lib

Problem Statement

When multiple OAuth providers are configured via AUTH_PROVIDERS, there was an enumeration bug where tokens from one provider could be incorrectly validated when intended for another. This was caused by swapping the order of AUTH_PROVIDERS fixing the issue temporarily (workaround in mcp-fhir-agent PR #68).

Root Cause Analysis

Two critical issues were identified:

  1. Missing issuer/audience validation: After JWT signature verification, the code never validated that the token's issuer/audience matched any configured auth provider. This meant ANY validly-signed token would be accepted regardless of which provider it came from.

  2. Duplicate key ID handling: When multiple providers use the same key ID (kid), only the first provider's key was kept, potentially causing signature verification failures.

Solution Implemented

Modified oidcauthlib/auth/token_reader.py:

  1. Added issuer/audience validation (lines 241-268):

    • After JWT signature verification succeeds, validate that either the token's issuer OR audience matches at least one configured auth provider
    • Supports flexible validation (matches if either issuer or audience aligns)
    • Raises descriptive AuthorizationBearerTokenInvalidException with clear message when validation fails
  2. Added duplicate key ID warning (lines 125-130):

    • Logs warning when duplicate key IDs are detected across providers
    • Helps operators identify potential configuration issues
  3. Fixed exception handling (lines 315-317):

    • Added specific catch for AuthorizationBearerTokenInvalidException to re-raise without wrapping
    • Ensures validation errors are properly propagated with correct error messages
  4. Strip whitespace from AUTH_PROVIDERS (environment_variables.py:45):

    • Strip whitespace from provider names in AUTH_PROVIDERS comma-separated list
    • Handles cases like "client2, client1" or " client2 , client1 "

Changes Made

  • Added issuer/audience validation in verify_token_async()
  • Added warning logging for duplicate JWKS key IDs
  • Fixed exception handling to properly propagate validation errors
  • Strip whitespace from AUTH_PROVIDERS values
  • Created test to verify tokens with wrong issuer/audience are rejected
  • All existing tests pass (11/11)
  • Code review completed - no issues found
  • Security scan completed - no vulnerabilities found

Testing

  • New test: test_token_validation_rejects_wrong_issuer_and_audience - Passes ✅
  • Existing tests: 11/11 passing ✅
  • Code review: No issues ✅
  • CodeQL security scan: No vulnerabilities ✅

Security Summary

No new security vulnerabilities introduced. The fix actually improves security by preventing tokens from one OAuth provider being accepted when intended for another provider.

Original prompt

we have this bug for now we have made the yaml fix by changin order but cna we find the issue and fix it correctly

Alvin, Imran mentioned you might be someone to talk to on the mcp-fhir-agent project. I have a PR up as a temporary workaround for auth (for openai specifically). I think we could remove auth validation altogether since our api gateway will do this, but if we don't, there is an enumeration bug in the support for multiple oauth clients that should be addressed. Happy to pass more details later. Here's the PR for the temp solution to unblock openai https://github.com/icanbwell/mcp-fhir-agent/pull/68 (I swapped the order of the AUTH_PROVIDERS)

it could be the bu in authoidclib


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Fix enumeration bug in multiple OAuth client support Fix token validation to prevent cross-provider enumeration Oct 29, 2025
Copilot AI requested a review from alvinhenrick October 29, 2025 21:47
Copilot finished work on behalf of alvinhenrick October 29, 2025 21:47
@alvinhenrick
Copy link
Collaborator

@copilot you did work on this this is how we seti nin envrioem variable pattserns see how siffix worsk

  • name: AUTH_PROVIDERS
    value: "client2,client1"

first client for testing

Copy link
Author

Copilot AI commented Oct 30, 2025

@copilot you did work on this this is how we seti nin envrioem variable pattserns see how siffix worsk

  • name: AUTH_PROVIDERS...

Updated auth_providers property to strip whitespace from provider names when parsing the comma-separated AUTH_PROVIDERS value. This handles cases like "client2, client1" or " client2 , client1 " correctly, ensuring provider names are properly trimmed before being converted to uppercase for environment variable lookups (AUTH_ISSUER_CLIENT1, etc.).

Commit: 5629e73

Copilot finished work on behalf of alvinhenrick October 30, 2025 17:39
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.

2 participants