panva / openid-client

OAuth 2 / OpenID Connect Client API for JavaScript Runtimes
MIT License
1.83k stars 392 forks source link

support audience as comma separated values #563

Closed koalazak closed 1 year ago

koalazak commented 1 year ago

Hello, I came across an openID implementation that is issuing tokens for multiple audiences but as comma separated values. Decoded token looks like (see 'aud'):

{
  "exp": 1675457424,
  "iat": 1675371024,
  "auth_time": 1675371023,
  "jti": "...",
  "iss": "...",
  "aud": "pos,foo,bar,universal-admin,dayparts,new-portal",
  "sub": "...",
  "typ": "ID",
  "azp": "new-portal",
  "session_state": "...",
  "at_hash": "...",
  "sid": "...",
  "firstName": "New",
  "lastName": "Portal",
  "preferred_username": "...",
  "email": "...",
  "username": "..."
}

So the token validation is failing because the client_id (new-portal) is not equals to the token.aud string in token. Original validateJWT() method is expecting token.aud to be an array if you want to validate for multiples audiences.

With this PR token.aud can be a string with comma separated values of audiences. There is a check to be backward compatible in case someone is using a client_id like literal pos,foo,bar,universal-admin,dayparts,new-portal (maybe someone workaround this problem doing that horrible thing...but who knows...). The check is "if client_id and aud matched, no matter if they have comma in the string, we are good. If they do not match and have a comma then try to split and validate"

what do you think?

regards

panva commented 1 year ago

Hi @koalazak

Workarounds to inexplicable behaviours aren't in the scope of this library.

You should reach out to the vendor for a fix instead.

koalazak commented 1 year ago

hello @panva, thanks for taking a look at the PR. Do you mean that the "aud": "pos,foo,bar,universal-admin,dayparts,new-portal" to define multiples audiences is out of the RFC?

thanks

panva commented 1 year ago

If that string represents a list of audiences yes.

koalazak commented 1 year ago

Yeah, the RFC seems pretty clear about this:

4.1.3 RFC here for future reference:

"aud" (Audience) Claim

   The "aud" (audience) claim identifies the recipients that the JWT is
   intended for.  Each principal intended to process the JWT MUST
   identify itself with a value in the audience claim.  If the principal
   processing the claim does not identify itself with a value in the
   "aud" claim when this claim is present, then the JWT MUST be
   rejected.  In the general case, the "aud" value is an array of case-
   sensitive strings, each containing a StringOrURI value.  In the
   special case when the JWT has one audience, the "aud" value MAY be a
   single case-sensitive string containing a StringOrURI value.  The
   interpretation of audience values is generally application specific.
   Use of this claim is OPTIONAL.

thanks again