-
Notifications
You must be signed in to change notification settings - Fork 25.1k
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
Add run-as support for OAuth2 tokens #86680
Changes from all commits
c1d26ba
df3aa8c
48d6646
749d895
3fb83f7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
pr: 86680 | ||
summary: Add run-as support for OAuth2 tokens | ||
area: Security | ||
type: enhancement | ||
issues: [] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,7 +29,6 @@ | |
import org.elasticsearch.xpack.security.operator.OperatorPrivileges.OperatorPrivilegesService; | ||
|
||
import java.util.List; | ||
import java.util.Locale; | ||
import java.util.Map; | ||
import java.util.function.BiConsumer; | ||
import java.util.function.Consumer; | ||
|
@@ -203,15 +202,8 @@ void maybeLookupRunAsUser(Authenticator.Context context, Authentication authenti | |
return; | ||
} | ||
|
||
// Run-as is supported for authentication with realm or api_key. Run-as for other authentication types is ignored. | ||
// Both realm user and api_key can create tokens. They can also run-as another user and create tokens. | ||
// In both cases, the created token will have a TOKEN authentication type and hence does not support run-as. | ||
if (Authentication.AuthenticationType.REALM != authentication.getAuthenticationType() | ||
&& Authentication.AuthenticationType.API_KEY != authentication.getAuthenticationType()) { | ||
logger.info( | ||
"ignore run-as header since it is currently not supported for authentication type [{}]", | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Log parameter changed from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
finishAuthentication(context, authentication, listener); | ||
return; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.