fix: Cognito token verification checks client_id instead of aud#3786
fix: Cognito token verification checks client_id instead of aud#3786
Conversation
marvin-context-protocol
Bot
added
auth
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b892808d60
ℹ️ 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".
| kwargs.pop("audience", None) | ||
| super().__init__(audience=None, **kwargs) |
There was a problem hiding this comment.
Dropping audience in AWSCognitoTokenVerifier.__init__ creates a silent security regression for callers that instantiate this class directly (instead of going through AWSCognitoProvider.get_token_verifier). Before this change, passing audience="client-a" enforced client binding via JWTVerifier; now audience is discarded and, unless expected_client_id is also set, no client binding is enforced at all, so tokens from other app clients can validate as long as issuer/signature/scope checks pass.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6a5db1d56
ℹ️ 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".
| if self._expected_client_id: | ||
| token_client_id = access_token.claims.get("client_id") | ||
| if token_client_id != self._expected_client_id: |
There was a problem hiding this comment.
AWSCognitoTokenVerifier.__init__ still accepts audience: str | list[str] | None, but verify_token now does a direct equality check between token_client_id (string claim) and self._expected_client_id. When callers pass multiple allowed audiences (a list), every valid token is rejected because a string can never equal that list, which is a regression from the base JWTVerifier behavior that supports list membership checks. This affects direct instantiations of AWSCognitoTokenVerifier and any future call path that forwards a list audience.
Useful? React with 👍 / 👎.
1 participant
AWS Cognito access tokens don't include an
audclaim; they useclient_idinstead. #3406 correctly added audience binding to prevent cross-client token reuse, but the implementation validatedaudvia the baseJWTVerifier, which always failed for Cognito tokens since the claim doesn't exist.AWSCognitoTokenVerifiernow intercepts theaudienceparameter, passesNoneto the parent (disabling theaudcheck), and validates theclient_idclaim directly. The provider API is unchanged;get_token_verifierstill passesaudience=self.client_idexactly as before. The security intent (binding verification to the configured app client) is preserved; only the claim being checked changes.Closes #3739