Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verifying multiple audiences #342

Open
oberlage opened this issue Sep 4, 2023 · 6 comments
Open

Verifying multiple audiences #342

oberlage opened this issue Sep 4, 2023 · 6 comments

Comments

@oberlage
Copy link

oberlage commented Sep 4, 2023

Hi there,

I've just updated to v5 and found that the new RegisteredClaims (in registered_claims.go) struct allows for a []string type via the ClaimStrings type. This opens the way for verifying multiple audiences from the token.

I'm very happy with this, as my authentication provider does provide multiple audiences and v4 gave no option to verify these.

However, the new ParserOption named WithAudience(aud string) and accompanying validator still only allows for a single string audience to be verified.

With RFC 7519 specifically mentioning multiple audiences, it does feel like something nice to support.

"(..) In the general case, the "aud" value is an array of case-sensitive strings (..)"

My questions are as follows:

  1. Are there specific reasons to not implement checking of multiple audiences?
  2. Are there any plans to do so?
  3. If not, how do you feel about having an additional ParserOption called WithAudiences(auds []string) for the specific case of multiple audiences. This would not break the existing WithAudience(..) and add functionality.

Thanks in advance!

@oxisto
Copy link
Collaborator

oxisto commented Sep 13, 2023

Just to double-check: Are you sure you want to check against multiple expected audiences?

What WithAudience does, it checks all supplied audience, whether one of them includes the expected one (or is equal to it, in the case of just one audience). Usually your application should define one expected audience (e.g., its own hostname or an application name or a similar semantic) and then check for that particular one. I am not so sure about what the semantics about multiple expected audiences would entail.

If there is a valid use case for it, I suppose we (or rather you ;) as part of a PR) could add the WithAudiences parser option. Although there is probably a discussion then whether all expected audiences must match or any of them.

We tried to do the most basic functionality first in v5 and then see where we could add additional features that make sense.

@chamilto
Copy link

@oxisto Would you still be open to accepting a PR for this feature?

Although there is probably a discussion then whether all expected audiences must match or any of them.

Perhaps the option could be configurable in this sense, e.g.

func WithAudiences(auds []string, matchAll bool) ParserOption {}

@iRbouh
Copy link

iRbouh commented Aug 18, 2024

Having multiple audiences is beneficial when using Google OAuth, especially since we have different apps across web and mobile, each with its client ID (audience). The server endpoint needs to validate against these multiple audiences.

Source: https://developers.google.com/identity/gsi/web/guides/verify-google-id-token

The value of aud in the ID token is equal to one of your app's client IDs. This check is necessary to prevent ID tokens issued to a malicious app being used to access data about the same user on your app's backend server.

@oxisto
Copy link
Collaborator

oxisto commented Aug 18, 2024

Having multiple audiences is beneficial when using Google OAuth, especially since we have different apps across web and mobile, each with its client ID (audience). The server endpoint needs to validate against these multiple audiences.

Source: https://developers.google.com/identity/gsi/web/guides/verify-google-id-token

The value of aud in the ID token is equal to one of your app's client IDs. This check is necessary to prevent ID tokens issued to a malicious app being used to access data about the same user on your app's backend server.

Ok, this could make sense. I would be open for a PR implementing this.

@mfridman
Copy link
Member

Would we extend the existing parameter to be a variaadic, or add another functional option?

@oxisto
Copy link
Collaborator

oxisto commented Aug 18, 2024

Would we extend the existing parameter to be a variaadic, or add another functional option?

I think the best way would be the approach suggested before:
func WithAudiences(auds []string, matchAll bool) ParserOption {}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants