Add run-as support for OAuth2 tokens #86680
Conversation
Authentication with an OAuth2 token can now perform run-as with changes in this PR. This is in addition to the existing run-as support for realm and API key authentication. NB there are additional constraints on whether an OAuth2 token is indeed qualified for run-as: 1. The token cannot itself cannot already be a run-as 2. The token cannot be created by anonymous or internal users Service accounts cannot create tokens which meant run-as is not supported for service accounts (none of existing service accounts requires it anyway).
|
Pinging @elastic/es-security (Team:Security) |
|
Hi @ywangd, I've created a changelog YAML for you. |
|
Can you clarify this in the description?
|
| // anonymous user? | ||
| if (getUser().equals(anonymousUser)) { | ||
| assert ANONYMOUS_REALM_TYPE.equals(getAuthenticatingSubject().getRealm().getType()) | ||
| && ANONYMOUS_REALM_NAME.equals(getAuthenticatingSubject().getRealm().getName()); |
There was a problem hiding this comment.
Should this be || based on your comment that a custom realm can use __anonymous for its name and type?
There was a problem hiding this comment.
The intention is to make sure we only treat ES's own anonymous user as the true anonymous user. So the user's realm must match both of the name and type of ES anonymous realm.
| authentication.getAuthenticationType().name().toLowerCase(Locale.ROOT) | ||
| ); | ||
| if (false == authentication.supportsRunAs(anonymousUser)) { | ||
| logger.info("ignore run-as header since it is currently not supported for authentication [{}]", authentication); |
There was a problem hiding this comment.
Log parameter changed from authentication.getAuthenticationType().name().toLowerCase(Locale.ROOT) to authentication. Is that design intent?
There was a problem hiding this comment.
With the changes here, determine whether an authentication supports run-as is more involved and not just based on the authenticationType. So I changed it to log the whole authentication object, which will be easier for us to diagnose if the logging message ever comes to us in a SDH.
Technically we can log multiple different message but more precise about exactly which part of authentication preventing run-as. But I am not sure if it's worth it. Let me know if you feel strongly about it.
|
I left a couple of drive by comments. |
|
Are there any OAuth2 tests that use Access Tokens formatted as JWTs? |
Not intentionally. But practically the |
|
@ywangd, as part of #86411, I'm clarifying that:
This is true until your PR is merged. As part of your PR, I think that we should remove that sentence and then provide examples in the |
Makse sense. I'll get to it when your PR is merged. Thanks for the reminder! |
...ck/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java
Outdated
Show resolved
Hide resolved
…security/authc/Authentication.java Co-authored-by: Tim Vernum <tim@adjective.org>
|
@lockewritesdocs This PR is now ready to be merged. Just need a round of CI to pass. In this case, would it make more sense for you to update your PR on the docs? I am happy to help out with providing technical details if needed. Thanks! |
|
@ywangd, go ahead and merge your PR and I'll remove the note about OAauth2. Please let me know if you have examples that we can use for |
Thanks. I'll message you for examples in a separate channel. |
Authentication with an OAuth2 token can now perform run-as with changes
in this PR. This is in addition to the existing run-as support for realm
and API key authentication.
NB there are additional constraints on whether an OAuth2 token is
indeed qualified for run-as:
client_credentialsgrant)Service accounts cannot create tokens which meant run-as is not
supported for service accounts (none of existing service accounts
requires it anyway).
Relates: #84336