Skip to content

Bind AWS Cognito token verification to configured app client#3406

Merged
jlowin merged 2 commits intomainfrom
codex/fix-aws-cognito-token-validation-issue
Mar 7, 2026
Merged

Bind AWS Cognito token verification to configured app client#3406
jlowin merged 2 commits intomainfrom
codex/fix-aws-cognito-token-validation-issue

Conversation

@jlowin
Copy link
Copy Markdown
Member

@jlowin jlowin commented Mar 6, 2026

Motivation

  • The AWSCognitoProvider created a verifier without a default audience, allowing a valid token issued to a different Cognito app client in the same user pool to pass verification and access protected endpoints.

Description

  • Store the configured Cognito client_id on AWSCognitoProvider and pass it as the default audience to AWSCognitoTokenVerifier (i.e. audience=audience or self.client_id), and add unit tests to assert the default and override behaviors.

Testing

  • Ran uv sync successfully and ran the provider-focused tests with uv run pytest tests/server/auth/providers/test_aws.py, which passed (5 passed).
  • Ran the full test suite with uv run pytest -n auto, which completed but reported unrelated failures (11 failed, 4679 passed, 2 skipped, 14 xfailed, 1 error); the AWS Cognito provider tests are green.
  • Attempted uv run prek run --all-files which failed due to a network error fetching a pre-commit hook mirror (CONNECT tunnel failed, response 403).

Codex Task

🤖 Generated with GPT-5.2-Codex
@marvin-context-protocol marvin-context-protocol Bot added bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. auth Related to authentication (Bearer, JWT, OAuth, WorkOS) for client or server. high-priority labels Mar 6, 2026
@marvin-context-protocol
Copy link
Copy Markdown
Contributor

Test Failure Analysis

Summary: The ty type checker fails on two new tests in tests/server/auth/providers/test_aws.py because get_token_verifier() is typed to return TokenVerifier, which doesn't have an audience attribute — even though the actual returned object (AWSCognitoTokenVerifier) does.

Root Cause: AWSCognitoProvider.get_token_verifier() has return type -> TokenVerifier (inherited from the parent class signature). The audience attribute is defined on JWTVerifier (a subclass of TokenVerifier), not on TokenVerifier itself. When the tests do verifier.audience, ty correctly flags this as an unresolved attribute on the declared type.

Suggested Solution: In tests/server/auth/providers/test_aws.py, add an isinstance assertion before accessing .audience. This narrows the type for the type checker and also adds a meaningful assertion:

# In test_token_verifier_defaults_audience_to_client_id
verifier = provider.get_token_verifier()
assert isinstance(verifier, AWSCognitoTokenVerifier)
assert verifier.audience == "test_client"

# In test_token_verifier_supports_audience_override
verifier = provider.get_token_verifier(audience="custom-audience")
assert isinstance(verifier, AWSCognitoTokenVerifier)
assert verifier.audience == "custom-audience"

Alternatively, update the return type annotation of get_token_verifier in src/fastmcp/server/auth/providers/aws.py to -> AWSCognitoTokenVerifier (covariant return types are valid in Python). Either fix resolves the type error.

Detailed Analysis

Failing check: ty check (static type analysis)

Error log:

error[unresolved-attribute]: Object of type `TokenVerifier` has no attribute `audience`
   --> tests/server/auth/providers/test_aws.py:118:20
    |
116 |             verifier = provider.get_token_verifier()
117 |
118 |             assert verifier.audience == "test_client"
    |                    ^^^^^^^^^^^^^^^^^

error[unresolved-attribute]: Object of type `TokenVerifier` has no attribute `audience`
   --> tests/server/auth/providers/test_aws.py:133:20
    |
131 |             verifier = provider.get_token_verifier(audience="custom-audience")
132 |
133 |             assert verifier.audience == "custom-audience"
    |                    ^^^^^^^^^^^^^^^^^

Type hierarchy:

  • TokenVerifier (base, in src/fastmcp/server/auth/auth.py) — no audience attribute
  • JWTVerifier(TokenVerifier) (in src/fastmcp/server/auth/providers/jwt.py:142) — defines self.audience at line 241
  • AWSCognitoTokenVerifier(JWTVerifier) (in src/fastmcp/server/auth/providers/aws.py:39) — inherits audience

Return type: AWSCognitoProvider.get_token_verifier()TokenVerifier (aws.py:181), overriding parent OIDCProxyProvider.get_token_verifier()TokenVerifier (oidc_proxy.py:456).

Related Files
  • tests/server/auth/providers/test_aws.py:118,133 — new tests accessing .audience on TokenVerifier-typed variable
  • src/fastmcp/server/auth/providers/aws.py:174-192get_token_verifier() with -> TokenVerifier return type
  • src/fastmcp/server/auth/providers/jwt.py:241 — where self.audience is set on JWTVerifier
  • src/fastmcp/server/auth/auth.py:352TokenVerifier base class (no audience attribute)

🤖 Analysis by marvin (Claude Sonnet 4.6)

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a72ec4402d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

return AWSCognitoTokenVerifier(
issuer=str(self.oidc_config.issuer),
audience=audience,
audience=audience or self.client_id,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Avoid forcing aud validation on Cognito access tokens

AWSCognitoProvider.get_token_verifier now defaults audience to self.client_id, but this verifier is used against the upstream access token path (OAuthProxy._get_verification_token returns upstream_token_set.access_token) and JWTVerifier.load_access_token validates audience only against the JWT aud claim. Cognito access tokens are client-bound via the client_id claim (and may not carry a matching aud), so this change can reject otherwise valid Cognito tokens by default and break authentication unless callers explicitly override the verifier behavior.

Useful? React with 👍 / 👎.

@jlowin
Copy link
Copy Markdown
Member Author

jlowin commented Mar 7, 2026

Auto-reviewed and merging on behalf of @jlowin — CI is green (Windows OAuth proxy timeouts are pre-existing flaky tests).

@jlowin jlowin merged commit 1708e53 into main Mar 7, 2026
7 checks passed
@jlowin jlowin deleted the codex/fix-aws-cognito-token-validation-issue branch March 7, 2026 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aardvark auth Related to authentication (Bearer, JWT, OAuth, WorkOS) for client or server. bug Something isn't working. Reports of errors, unexpected behavior, or broken functionality. codex high-priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant