feat: allow array of strings as scope

[dbryar]

Issue #, if available:

Description of changes: Allows the scope claim to be made up of an array of strings, as there are some older IDPs still in use that do not correctly use space delimited strings.

This PR still ensures the scope is an array of strings, and handles both string or array-of-string scopes

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[elorzafe]

@dbryar

Thanks for your PR, we are a little hesitant on adding this because is not part of the spec. But we want to know more about your use case?

Which IDPs you are using that have this behavior?

Can you use customJwtCheck option? https://github.com/awslabs/aws-jwt-verify#custom-jwt-and-jwk-checks

[dbryar]

Thanks for the review. Doing some digging and the IDP is an old Microsoft Authentication Service implemented prior to the release of .NET Core Identity. It is obsolete, I agree, but it is still in production use (for now). Prior to standardization, a scope claim could be an array of strings according to my limited research.

We are implementing custom authorizer in API Gateway using a Lambda Function that accepts multiple tokens from multiple IDPs based on partnerships established with other vendors that wish to leverage our service with their own white labelling. Thus, we have to accept their (non-standard) tokens while we await those IDPs to be brought up to date.

[dbryar]

At present, the token fails header checks due to

if (typeof payload.scope !== "string") {
   throw new JwtParseError("JWT payload scope claim is not a string");
}

So I have modified that check to first check for an Array, and then for strings if true

[ottokruse]

Given it is non-standard, it feels most fair at this point to solve it using a customJwtCheck, and not add support in the library:

import { JwtRsaVerifier } from "aws-jwt-verify";
import { assertStringArraysOverlap } from "aws-jwt-verify/assert";
import { JwtInvalidScopeError } from "aws-jwt-verify/error";

const verifier = JwtRsaVerifier.create({
  issuer: "changeme",
  audience: "changeme",
  jwksUri: "changeme",
  customJwtCheck: ({ payload }) => {
    assertStringArraysOverlap(
      "Scope",
      Array.isArray(payload.scope) ? payload.scope : payload.scope.split(" "),
      ["the", "list", "of", "scopes", "you", "allow"],
      JwtInvalidScopeError
    );
  },
});

Does that make sense?

[ottokruse]

Belay that, would still run into the "JWT payload scope claim is not a string" error, that check is done before customJwtChecks.

[ottokruse]

FYI we are discussing this internally, please bear with us.

[hakanson]

tl;dr - we discussed and are not inclined to make this change

Our discussion on this focused around the topics of standards compliance, minimizing the ability to use library incorrectly, and backwards compatibility.

• The string array is non-standard, but as mentioned was used by some legacy systems.
• We thought about ways (parameters, configuration, etc) to opt in to this behavior, but how do we decide which claims can be non-standard, and in which ways? If this was generic solution and used incorrectly, it could break overall validation.
• Lastly, if downstream consumers of the JWT are expecting the payload to be standard (a string vs a string array), it becomes an incompatible change. Also, our library would not want to alter the JWT payload to convert the data format (e.g change the string array into a space separated string).

Therefore, there are too many concerns to adopt this change.

[ottokruse]

Thanks @dbryar for your effort anyway! (We might revisit if this issue turns out to be widespread)