Skip to content

Commit

Permalink
Remove deprecated Authentication#getAuthenticatedBy (#91104)
Browse files Browse the repository at this point in the history
This PR removes the deprecated Authentication#getAuthenticatedBy method
and replaces its usages with #getAuthenticatingSubject#getRealm

Relates: #88494
  • Loading branch information
ywangd authored Oct 25, 2022
1 parent 4a575e7 commit 0ac81ce
Show file tree
Hide file tree
Showing 18 changed files with 82 additions and 83 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public SamlAuthenticateResponse(StreamInput in) throws IOException {

public SamlAuthenticateResponse(Authentication authentication, String tokenString, String refreshToken, TimeValue expiresIn) {
this.principal = authentication.getEffectiveSubject().getUser().principal();
this.realm = authentication.getAuthenticatedBy().getName();
this.realm = authentication.getEffectiveSubject().getRealm().getName();
this.tokenString = tokenString;
this.refreshToken = refreshToken;
this.expiresIn = expiresIn;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,14 +167,6 @@ public boolean isRunAs() {
return authenticatingSubject != effectiveSubject;
}

/**
* Use {@code getAuthenticatingSubject().getRealm()} instead.
*/
@Deprecated
public RealmRef getAuthenticatedBy() {
return authenticatingSubject.getRealm();
}

/**
* The use case for this method is largely trying to tell whether there is a run-as user
* and can be replaced by {@code isRunAs}
Expand Down Expand Up @@ -367,7 +359,7 @@ public boolean isAssignedToDomain() {
}

public boolean isAuthenticatedWithServiceAccount() {
return ServiceAccountSettings.REALM_TYPE.equals(getAuthenticatedBy().getType());
return ServiceAccountSettings.REALM_TYPE.equals(getAuthenticatingSubject().getRealm().getType());
}

/**
Expand Down Expand Up @@ -568,12 +560,12 @@ public void toXContentFragment(XContentBuilder builder) throws IOException {
builder.field(User.Fields.METADATA.getPreferredName(), user.metadata());
builder.field(User.Fields.ENABLED.getPreferredName(), user.enabled());
builder.startObject(User.Fields.AUTHENTICATION_REALM.getPreferredName());
builder.field(User.Fields.REALM_NAME.getPreferredName(), getAuthenticatedBy().getName());
builder.field(User.Fields.REALM_TYPE.getPreferredName(), getAuthenticatedBy().getType());
builder.field(User.Fields.REALM_NAME.getPreferredName(), getAuthenticatingSubject().getRealm().getName());
builder.field(User.Fields.REALM_TYPE.getPreferredName(), getAuthenticatingSubject().getRealm().getType());
// domain name is generally ambiguous, because it can change during the lifetime of the authentication,
// but it is good enough for display purposes (including auditing)
if (getAuthenticatedBy().getDomain() != null) {
builder.field(User.Fields.REALM_DOMAIN.getPreferredName(), getAuthenticatedBy().getDomain().name());
if (getAuthenticatingSubject().getRealm().getDomain() != null) {
builder.field(User.Fields.REALM_DOMAIN.getPreferredName(), getAuthenticatingSubject().getRealm().getDomain().name());
}
builder.endObject();
builder.startObject(User.Fields.LOOKUP_REALM.getPreferredName());
Expand All @@ -584,10 +576,10 @@ public void toXContentFragment(XContentBuilder builder) throws IOException {
builder.field(User.Fields.REALM_DOMAIN.getPreferredName(), getLookedUpBy().getDomain().name());
}
} else {
builder.field(User.Fields.REALM_NAME.getPreferredName(), getAuthenticatedBy().getName());
builder.field(User.Fields.REALM_TYPE.getPreferredName(), getAuthenticatedBy().getType());
if (getAuthenticatedBy().getDomain() != null) {
builder.field(User.Fields.REALM_DOMAIN.getPreferredName(), getAuthenticatedBy().getDomain().name());
builder.field(User.Fields.REALM_NAME.getPreferredName(), getAuthenticatingSubject().getRealm().getName());
builder.field(User.Fields.REALM_TYPE.getPreferredName(), getAuthenticatingSubject().getRealm().getType());
if (getAuthenticatingSubject().getRealm().getDomain() != null) {
builder.field(User.Fields.REALM_DOMAIN.getPreferredName(), getAuthenticatingSubject().getRealm().getDomain().name());
}
}
builder.endObject();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ public void testIsServiceAccount() {
authentication = AuthenticationTestHelper.builder().serviceAccount().build();
} else {
authentication = randomValueOtherThanMany(
authc -> "_service_account".equals(authc.getAuthenticatedBy().getName()),
authc -> "_service_account".equals(authc.getAuthenticatingSubject().getRealm().getName()),
() -> AuthenticationTestHelper.builder().build()
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -745,8 +745,8 @@ public void testOperationsOnReservedUsers() throws Exception {
Collections.singletonMap("Authorization", basicAuthHeaderValue(username, getReservedPassword()))
).execute(AuthenticateAction.INSTANCE, AuthenticateRequest.INSTANCE).get();
assertThat(authenticateResponse.authentication().getEffectiveSubject().getUser().principal(), is(username));
assertThat(authenticateResponse.authentication().getAuthenticatedBy().getName(), equalTo("reserved"));
assertThat(authenticateResponse.authentication().getAuthenticatedBy().getType(), equalTo("reserved"));
assertThat(authenticateResponse.authentication().getAuthenticatingSubject().getRealm().getName(), equalTo("reserved"));
assertThat(authenticateResponse.authentication().getAuthenticatingSubject().getRealm().getType(), equalTo("reserved"));
assertNull(authenticateResponse.authentication().getLookedUpBy());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ protected void doExecute(Task task, OpenIdConnectLogoutRequest request, ActionLi
final String token = request.getToken();
tokenService.getAuthenticationAndMetadata(token, ActionListener.wrap(tuple -> {
final Authentication authentication = tuple.v1();
assert false == authentication.isRunAs() : "oidc realm authentication cannot have run-as";
final Map<String, Object> tokenMetadata = tuple.v2();
validateAuthenticationAndMetadata(authentication, tokenMetadata);
tokenService.invalidateAccessToken(token, ActionListener.wrap(result -> {
Expand All @@ -86,7 +87,7 @@ protected void doExecute(Task task, OpenIdConnectLogoutRequest request, ActionLi

private OpenIdConnectLogoutResponse buildResponse(Authentication authentication, Map<String, Object> tokenMetadata) {
final String idTokenHint = (String) getFromMetadata(tokenMetadata, "id_token_hint");
final Realm realm = this.realms.realm(authentication.getAuthenticatedBy().getName());
final Realm realm = this.realms.realm(authentication.getEffectiveSubject().getRealm().getName());
final JWT idToken;
try {
idToken = JWTParser.parse(idTokenHint);
Expand All @@ -108,11 +109,11 @@ private void validateAuthenticationAndMetadata(Authentication authentication, Ma
throw new ElasticsearchSecurityException("No active user");
}

final Authentication.RealmRef ref = authentication.getAuthenticatedBy();
final Authentication.RealmRef ref = authentication.getEffectiveSubject().getRealm();
if (ref == null || Strings.isNullOrEmpty(ref.getName())) {
throw new ElasticsearchSecurityException("Authentication {} has no authenticating realm", authentication);
}
final Realm realm = this.realms.realm(authentication.getAuthenticatedBy().getName());
final Realm realm = this.realms.realm(authentication.getEffectiveSubject().getRealm().getName());
if (realm == null) {
throw new ElasticsearchSecurityException("Authenticating realm {} does not exist", ref.getName());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ protected void doExecute(Task task, SamlAuthenticateRequest request, ActionListe
return;
}
assert authentication != null : "authentication should never be null at this point";
assert false == authentication.isRunAs() : "saml realm authentication cannot have run-as";
@SuppressWarnings("unchecked")
final Map<String, Object> tokenMeta = (Map<String, Object>) result.getMetadata().get(SamlRealm.CONTEXT_TOKEN_DATA);
tokenService.createOAuth2Tokens(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ protected void doExecute(Task task, SamlLogoutRequest request, ActionListener<Sa
final String token = request.getToken();
tokenService.getAuthenticationAndMetadata(token, ActionListener.wrap(tuple -> {
Authentication authentication = tuple.v1();
assert false == authentication.isRunAs() : "saml realm authentication cannot have run-as";
final Map<String, Object> tokenMetadata = tuple.v2();
SamlLogoutResponse response = buildResponse(authentication, tokenMetadata);
tokenService.invalidateAccessToken(token, ActionListener.wrap(created -> {
Expand Down Expand Up @@ -134,9 +135,9 @@ private String getMetadataString(Map<String, Object> metadata, String key) {
}

private SamlRealm findRealm(Authentication authentication) {
final Authentication.RealmRef ref = authentication.getAuthenticatedBy();
final Authentication.RealmRef ref = authentication.getEffectiveSubject().getRealm();
if (ref == null || Strings.isNullOrEmpty(ref.getName())) {
throw SamlUtils.samlException("Authentication {} has no authenticating realm", authentication);
throw SamlUtils.samlException("Authentication {} has no effective realm", authentication);
}
final Realm realm = realms.realm(ref.getName());
if (realm == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ public void authenticationSuccess(String requestId, Authentication authenticatio
)
) == false) {
// this is redundant information maintained for bwc purposes
final String authnRealm = authentication.getAuthenticatedBy().getName();
final String authnRealm = authentication.getAuthenticatingSubject().getRealm().getName();
new LogEntryBuilder().with(EVENT_TYPE_FIELD_NAME, REST_ORIGIN_FIELD_VALUE)
.with(EVENT_ACTION_FIELD_NAME, "authentication_success")
.with(REALM_FIELD_NAME, authnRealm)
Expand Down Expand Up @@ -1531,10 +1531,10 @@ LogEntryBuilder withRestUriAndMethod(RestRequest request) {

LogEntryBuilder withRunAsSubject(Authentication authentication) {
logEntry.with(PRINCIPAL_FIELD_NAME, authentication.getAuthenticatingSubject().getUser().principal())
.with(PRINCIPAL_REALM_FIELD_NAME, authentication.getAuthenticatedBy().getName())
.with(PRINCIPAL_REALM_FIELD_NAME, authentication.getAuthenticatingSubject().getRealm().getName())
.with(PRINCIPAL_RUN_AS_FIELD_NAME, authentication.getEffectiveSubject().getUser().principal());
if (authentication.getAuthenticatedBy().getDomain() != null) {
logEntry.with(PRINCIPAL_DOMAIN_FIELD_NAME, authentication.getAuthenticatedBy().getDomain().name());
if (authentication.getAuthenticatingSubject().getRealm().getDomain() != null) {
logEntry.with(PRINCIPAL_DOMAIN_FIELD_NAME, authentication.getAuthenticatingSubject().getRealm().getDomain().name());
}
if (authentication.getLookedUpBy() != null) {
logEntry.with(PRINCIPAL_RUN_AS_REALM_FIELD_NAME, authentication.getLookedUpBy().getName());
Expand Down Expand Up @@ -1625,7 +1625,7 @@ LogEntryBuilder withAuthentication(Authentication authentication) {
// No domain information is needed here since API key itself does not work across realms
}
} else {
final Authentication.RealmRef authenticatedBy = authentication.getAuthenticatedBy();
final Authentication.RealmRef authenticatedBy = authentication.getAuthenticatingSubject().getRealm();
if (authentication.isRunAs()) {
final Authentication.RealmRef lookedUpBy = authentication.getLookedUpBy();
logEntry.with(PRINCIPAL_REALM_FIELD_NAME, lookedUpBy.getName())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1513,17 +1513,20 @@ private static Optional<ElasticsearchSecurityException> checkClientCanRefresh(
clientAuthentication.getEffectiveSubject().getUser().principal()
);
return Optional.of(invalidGrantException("tokens must be refreshed by the creating client"));
} else if (clientAuthentication.getAuthenticatedBy().getName().equals(refreshToken.getAssociatedRealm()) == false) {
logger.warn(
"[{}] created the refresh token while authenticated by [{}] but is now authenticated by [{}]",
refreshToken.getAssociatedUser(),
refreshToken.getAssociatedRealm(),
clientAuthentication.getAuthenticatedBy().getName()
);
return Optional.of(invalidGrantException("tokens must be refreshed by the creating client"));
} else {
return Optional.empty();
}
} else if (clientAuthentication.getAuthenticatingSubject()
.getRealm()
.getName()
.equals(refreshToken.getAssociatedRealm()) == false) {
logger.warn(
"[{}] created the refresh token while authenticated by [{}] but is now authenticated by [{}]",
refreshToken.getAssociatedUser(),
refreshToken.getAssociatedRealm(),
clientAuthentication.getAuthenticatingSubject().getRealm().getName()
);
return Optional.of(invalidGrantException("tokens must be refreshed by the creating client"));
} else {
return Optional.empty();
}
}
}

Expand Down Expand Up @@ -1795,9 +1798,9 @@ static BytesReference createTokenDocument(
builder.field("authentication", originatingClientAuth.maybeRewriteForOlderVersion(userToken.getVersion()).encode());
} else {
builder.field("user", originatingClientAuth.getEffectiveSubject().getUser().principal())
.field("realm", originatingClientAuth.getAuthenticatedBy().getName());
if (originatingClientAuth.getAuthenticatedBy().getDomain() != null) {
builder.field("realm_domain", originatingClientAuth.getAuthenticatedBy().getDomain());
.field("realm", originatingClientAuth.getAuthenticatingSubject().getRealm().getName());
if (originatingClientAuth.getAuthenticatingSubject().getRealm().getDomain() != null) {
builder.field("realm_domain", originatingClientAuth.getAuthenticatingSubject().getRealm().getDomain());
}
}
builder.endObject().endObject();
Expand Down Expand Up @@ -2546,7 +2549,7 @@ static final class RefreshTokenStatus {
this.invalidated = invalidated;
// not used, filled-in for consistency's sake
this.associatedUser = associatedAuthentication.getEffectiveSubject().getUser().principal();
this.associatedRealm = associatedAuthentication.getAuthenticatedBy().getName();
this.associatedRealm = associatedAuthentication.getAuthenticatingSubject().getRealm().getName();
this.associatedAuthentication = associatedAuthentication;
this.refreshed = refreshed;
this.refreshInstant = refreshInstant;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,8 @@ private void buildUser(X509AuthenticationToken token, String principal, ActionLi
"pki_delegated_by_user",
token.getDelegateeAuthentication().getEffectiveSubject().getUser().principal(),
"pki_delegated_by_realm",
token.getDelegateeAuthentication().getAuthenticatedBy().getName()
// TODO: this should be the realm of effective subject
token.getDelegateeAuthentication().getAuthenticatingSubject().getRealm().getName()
);
} else {
metadata = Map.of("pki_dn", token.dn());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -866,7 +866,7 @@ private static boolean checkChangePasswordAction(Authentication authentication)
if (isRunAs) {
realmType = authentication.getLookedUpBy().getType();
} else {
realmType = authentication.getAuthenticatedBy().getType();
realmType = authentication.getAuthenticatingSubject().getRealm().getType();
}

assert realmType != null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ public void testExecuteAfterRewritingAuthentication() throws IOException {
securityContext.executeAfterRewritingAuthentication(originalCtx -> {
Authentication authentication = securityContext.getAuthentication();
assertEquals(original.getEffectiveSubject().getUser(), authentication.getEffectiveSubject().getUser());
assertEquals(original.getAuthenticatedBy(), authentication.getAuthenticatedBy());
assertEquals(original.getAuthenticatingSubject().getRealm(), authentication.getAuthenticatingSubject().getRealm());
assertEquals(original.getLookedUpBy(), authentication.getLookedUpBy());
assertEquals(VersionUtils.getPreviousVersion(), authentication.getEffectiveSubject().getVersion());
assertEquals(original.getAuthenticationType(), securityContext.getAuthentication().getAuthenticationType());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ public void onFailure(Exception e) {
if (auth.isRunAs()) {
assertThat(auth.getAuthenticatingSubject().getUser(), sameInstance(authentication.getAuthenticatingSubject().getUser()));
}
assertThat(auth.getAuthenticatedBy(), sameInstance(auth.getAuthenticatedBy()));
assertThat(auth.getAuthenticatingSubject().getRealm(), sameInstance(auth.getAuthenticatingSubject().getRealm()));
assertThat(auth.getLookedUpBy(), sameInstance(auth.getLookedUpBy()));
assertThat(auth.getEffectiveSubject().getVersion(), sameInstance(auth.getEffectiveSubject().getVersion()));
assertThat(auth.getAuthenticationType(), sameInstance(auth.getAuthenticationType()));
Expand Down Expand Up @@ -196,7 +196,7 @@ public void onFailure(Exception e) {
final Authentication auth = responseRef.get().authentication();
final User authUser = auth.getEffectiveSubject().getUser();
assertThat(authUser.roles(), emptyArray());
assertThat(auth.getAuthenticatedBy(), sameInstance(auth.getAuthenticatedBy()));
assertThat(auth.getAuthenticatingSubject().getRealm(), sameInstance(auth.getAuthenticatingSubject().getRealm()));
assertThat(auth.getLookedUpBy(), sameInstance(auth.getLookedUpBy()));
assertThat(auth.getEffectiveSubject().getVersion(), sameInstance(auth.getEffectiveSubject().getVersion()));
assertThat(auth.getAuthenticationType(), sameInstance(auth.getAuthenticationType()));
Expand Down
Loading

0 comments on commit 0ac81ce

Please sign in to comment.