-
Notifications
You must be signed in to change notification settings - Fork 285
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
[FEATURE] required_audience should support a list #3723
Comments
[Triage] Hi @m4rkw, this sounds like a great idea. If you would like to submit your internal patch as a PR, we would happily review any PRs towards this effort. Thanks. |
@scrawfor99 sure - https://github.com/opensearch-project/security/pull/3765/files this is what we're using internally. It works however I'm not a java developer so have no idea if the code is sensible/efficient/etc. |
@m4rkw I modified your PR a bit to accommodate this change everywhere where a JWT could be verified in this plugin: cwperks#12 Could you please update this issue to describe how multiple audiences could be handled? i.e. Would you setup an authenticator with multiple required audiences and if a token has audiences wholly contained in the list of required audiences it would be verified? JWT Authenticator
Valid token payloads:
What about a token that has some overlapping audiences, but not others:
|
@cwperks I’m not an expert on the oauth2 spec but from what I understand talking to our internal team who run the identity provider, only supporting a single audience claim essentially isn’t fully supporting oauth2. This is (I think) because the two access methods, that is API access to elastic and web access via dashboards, are different oauth2 flows and so necessitate different claims. Eg if your cluster is elkmark you might have a claim for elasticsearch api access of “elkmark-dev”, but the identity provider may mandate a .web extension for web-based flows so the audience claim for the dashboards flow would have to be “elkmark-dev.web”. This is the case in our environment and the identity provider won’t let us use either one of these claims in both scenarios. I’m speculating now but I think this is probably part of the spec because one point of JWTs is to limit exposure in the event of a token being stolen. If the claims are limited in scope then an intercepted token from a dashboards session can’t then be used for direct api access, and vice versa. In my opinion having any valid claim in the token should grant access regardless of there being other invalid claims. |
Looks like this issue is fixed/now supported with the linked PR. I will close it. @m4rkw feel free to re-open or create a new one if I am mistaken. |
Is your feature request related to a problem?
The identity provider we use in our environment doesn't allow us to use the same audience claim for cli-based access vs web access (eg dashboards). Therefore we're unable to use the claim validation in its current state because it only supports a single value. I originally reported the lack of claim validation a couple of years ago when we were still running OpenDistro and was happy to see it had been added, but unfortunately a static value makes it impossible for us to use so we'll have to write a patch for internal use.
What solution would you like?
The required_audience field should support a list of valid claims.
What alternatives have you considered?
Tried to make the stack run with the same single claim but our identity provider doesn't allow for it.
Do you have any additional context?
No
The text was updated successfully, but these errors were encountered: