Handle cross-cluster authentication in authorization info report #95203
Conversation
This PR adds support for reporting authorization information for cross-cluster authentication.
ywangd added >non-issue :Security/Security 
| Pinging @elastic/es-security (Team:Security) |
| builder.startObject("cross_cluster_access"); | ||
| { | ||
| addApiKeyInfo(builder, subject); | ||
| builder.startObject("remote_authorization"); |
There was a problem hiding this comment.
This field is named remote_authorization to be both different from and consistent with the outer authorization field. I am open to suggestions.
There was a problem hiding this comment.
Ya I think remote_authorization works. Only alternative I can think of is authorization_from_remote but I don't have a preference either way.
| } | ||
| case SERVICE_ACCOUNT -> builder.field("service_account", authenticationSubject.getUser().principal()); | ||
| case SERVICE_ACCOUNT -> builder.field("service_account", subject.getUser().principal()); | ||
| case CROSS_CLUSTER_ACCESS -> { |
There was a problem hiding this comment.
Based on the callers of this method, a cross-cluster authentication should never reach here. Therefore it can be argued to not handle it at all, e.g. just return without doing anything. However since the method itself is not prescribed to be used only by existing callers, it feels more appropriate to handle the case for self-consistency.
| AuthenticationTestHelper.builder().build() | ||
| ); | ||
| assert false == crossClusterAccessSubjectInfo.getAuthentication().isCrossClusterAccess(); | ||
| final Authentication authentication = AuthenticationTestHelper.builder() |
There was a problem hiding this comment.
Nit: I think we can just use builder.crossCluster().build() instead, i.e.:
final Authentication authentication = AuthenticationTestHelper.builder().crossClusterAccess().build(); final var apiKeyName = (String) authentication.getAuthenticatingSubject().getMetadata().get(API_KEY_NAME_KEY); final var innerAuthentication = (Authentication) authentication.getAuthenticatingSubject() .getMetadata() .get(CROSS_CLUSTER_ACCESS_AUTHENTICATION_KEY);There was a problem hiding this comment.
Thanks I forgot we have this convenient method.
| builder.startObject("cross_cluster_access"); | ||
| { | ||
| addApiKeyInfo(builder, subject); | ||
| builder.startObject("remote_authorization"); |
There was a problem hiding this comment.
Ya I think remote_authorization works. Only alternative I can think of is authorization_from_remote but I don't have a preference either way.
| } | ||
| case SERVICE_ACCOUNT -> builder.field("service_account", authenticationSubject.getUser().principal()); | ||
| case SERVICE_ACCOUNT -> builder.field("service_account", subject.getUser().principal()); | ||
| case CROSS_CLUSTER_ACCESS -> { |
ywangd added the auto-merge-without-approval 3 participants
This PR adds support for reporting authorization information for cross-cluster authentication.
Note: Labelled as
>non-issuebecause the cross-cluster-access is still behind feature flag.