[Feature Request] Document how to use the generic RSA verifier with Cognito JWTs

[kenblair]

Describe the bug Documentation describes using JwtRsaVerifier with Cognito like so:

const verifier = JwtRsaVerifier.create([
  {
    issuer: "https://cognito-idp.eu-west-1.amazonaws.com/<user_pool_id>",
    audience: "<client_id>",
  },
  {
    issuer: "https://example.com/my/other/idp",
    audience: "myaudience",
  },
]);

It appears this will fail every single time because Cognito tokens have no aud. If we want to use Cognito with another IDP we have to either skip the audience check and introduce a vulnerability or implement it ourselves as a custom check in every Cognito verifier.

Versions Which version of aws-jwt-verify are you using?

2.0.0

Which version of Node.js are you using? (Should be at least 14)

14.18.2

To Reproduce

1. Generate a Cognito access token from a user pool.
2. Observe the JWT has no aud claim.
3. Feed it to a JwtRsaVerifier with the client_id as audience and it fails due to missing aud.

[kenblair]

I would have expected the JwtRsaVerifier, when configured with a correct client_id, to verify the "audience" by checking the client_id for Cognito based issuers.

[ottokruse]

Hi @kenblair

Great feedback.

The JwtRsaVerifier was intended to be 'generic', doesn't do any Cognito specific things (by design).

We need to think about this and form an opinion. We'll get back to you. Your feedback to us is clear, thank you.

Note btw this only concerns Cognito Access tokens, as they have a client_id instead of aud. ID-tokens are "normal" and would work with the generic aud check. Building the custom JWT check for access tokens (to verify client_id instead of aud) is straightforward but I understand your point: it's code you should not have to write.

[kenblair]

The JwtRsaVerifier was intended to be 'generic', doesn't do any Cognito specific things (by design).

We need to think about this and form an opinion. We'll get back to you. Your feedback to us is clear, thank you.

I understand, thank you. If you decide not to change the behavior I suggest adding a note to that area of the README.

Note btw this only concerns Cognito Access tokens, as they have a client_id instead of aud. ID-tokens are "normal" and would work with the generic aud check. Building the custom JWT check for access tokens (to verify client_id instead of aud) is straightforward but I understand your point: it's code you should not have to write.

I didn't know that about Cognito tokens, thank you.

Obviously payload.client_id !== '<client_id>' is an easy check to write but it is awkward to fit in. There is already a customJwtCheck that is passed in to verify based on the event object injected into the handler function. So that will override any customJwtCheck function I might choose to include when the verifier is created. So I'd have to disable audience checking, then add a check that only runs when the issuer is Cognito.

Or I have to stop using a single verifier object and build something like a map of issuer (k) to verifier (v) so that I can do a lookup and call verify on the correct instance. I'm not sure if that would cause new problems with the JWKS cache though.

[ottokruse]

an easy check to write but it is awkward to fit in. There is already a customJwtCheck that is passed in to verify based on the event object injected into the handler function. So that will override any customJwtCheck function I might choose to include when the verifier is created.

Can you maybe share your code so we can fully understand what you want to do?

[kenblair]

I cannot due to contractual obligations. Here's different code in a similar overall structure. Please feel free to suggest improvements. Thank you.

const verifier = JwtRsaVerifier.create(getVerifierConfigurations());

export const handler = async (event) => {
    verifier.verify(jwt, {
        customJwtCheck: ({ payload }) => {
            const { routeKey } = event.requestContext;
            if (isAdminRoute(routeKey) && !hasAdmin(payload['cognito:groups'])) {
                throw new Error('Unauthorized');
            }
        }
    });
}

[ottokruse]

Thanks.

So this is a Lambda behind API Gateway? How many routes are being a handled in this lambda, just one?

And do you have other (different) custom checks at verifier level?

[kenblair]

Yes it is behind a gateway. There's something like 4-5 routes right now but that will change. There are not different checks at the verifier level. For the moment we're not checking audience because after review it is not necessary in our current setup.

[ottokruse]

We could simply export this function, so that it can be imported by users and called when using the generic verifier for Cognito: https://github.com/awslabs/aws-jwt-verify/blob/68b017ff83ffec0810961f8d4a756e46d1f13674/src/cognito-verifier.ts#L149

What do you think @leelalagudu @elorzafe ?

The code example in README.md would then become:

import { JwtRsaVerifier } from "aws-jwt-verify";
import { validateCognitoJwtFields } from "aws-jwt-verify/cognito-verifier";

const verifier = JwtRsaVerifier.create([
  {
    issuer: "https://cognito-idp.eu-west-1.amazonaws.com/<user_pool_id>",
    audience: null, // this is checked instead, by the Cognito specific checks, that we'll invoke as customJwtCheck below:
    customJwtCheck: ({ payload }) =>
      validateCognitoJwtFields(payload, {
        groups: ["admin", "others"], // optional, provide a group name, or array of group names
        tokenUse: "access", // set to "id" or "access" (or null if both are fine) 
        clientId: "<client_id>", // provide the client id, or an array of client ids (or null if you do not want to check client id)
      }),
  },
  {
    issuer: "https://example.com/my/other/idp",
    audience: "myaudience",
  },
]);

[leelalagudu]

While this would mean the lib would slightly drift away from being a generic JWT verifier, since we already have an extended verifier built specifically for Cognito as part of lib, I agree that exporting validateCognitoJwtFields along with doc examples for validating Cognito ID and access tokens would be a good way forward

[ottokruse]

Done!