Skip to content

Commit

Permalink
Remove deprecated Authentication#getLookedUpBy
Browse files Browse the repository at this point in the history
This PR removes the deprecated Authentication#getLookedUpBy method.
Its usages are replaced by either isRunAs or
getEffectiveSubject#getRealm or their combinations depending on the
actual context.

Relates: elastic#88494
  • Loading branch information
ywangd committed Oct 26, 2022
1 parent 0ac81ce commit 18986bc
Show file tree
Hide file tree
Showing 12 changed files with 45 additions and 60 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -167,20 +167,6 @@ public boolean isRunAs() {
return authenticatingSubject != effectiveSubject;
}

/**
* 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}
*/
@Deprecated
public RealmRef getLookedUpBy() {
if (isRunAs()) {
return effectiveSubject.getRealm();
} else {
// retain the behaviour of returning null for lookup realm for if the authentication is not run-as
return null;
}
}

/**
* Get the realm where the effective user comes from.
* The effective user is the es-security-runas-user if present or the authenticated user.
Expand Down Expand Up @@ -472,9 +458,7 @@ public void writeTo(StreamOutput out) throws IOException {
AuthenticationSerializationHelper.writeUserTo(user, out);
}
authenticatingSubject.getRealm().writeTo(out);
final RealmRef lookedUpBy = getLookedUpBy();
// See detailed comment on the same assertion in the Constructor with StreamInput
assert isRunAs() || lookedUpBy == null : "Authentication has no inner-user, but looked-up-by is [" + lookedUpBy + "]";
final RealmRef lookedUpBy = isRunAs() ? effectiveSubject.getRealm() : null;

if (lookedUpBy != null) {
out.writeBoolean(true);
Expand Down Expand Up @@ -569,11 +553,12 @@ public void toXContentFragment(XContentBuilder builder) throws IOException {
}
builder.endObject();
builder.startObject(User.Fields.LOOKUP_REALM.getPreferredName());
if (getLookedUpBy() != null) {
builder.field(User.Fields.REALM_NAME.getPreferredName(), getLookedUpBy().getName());
builder.field(User.Fields.REALM_TYPE.getPreferredName(), getLookedUpBy().getType());
if (getLookedUpBy().getDomain() != null) {
builder.field(User.Fields.REALM_DOMAIN.getPreferredName(), getLookedUpBy().getDomain().name());
final RealmRef lookedUpBy = isRunAs() ? getEffectiveSubject().getRealm() : null;
if (lookedUpBy != null) {
builder.field(User.Fields.REALM_NAME.getPreferredName(), lookedUpBy.getName());
builder.field(User.Fields.REALM_TYPE.getPreferredName(), lookedUpBy.getType());
if (lookedUpBy.getDomain() != null) {
builder.field(User.Fields.REALM_DOMAIN.getPreferredName(), lookedUpBy.getDomain().name());
}
} else {
builder.field(User.Fields.REALM_NAME.getPreferredName(), getAuthenticatingSubject().getRealm().getName());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -747,7 +747,7 @@ public void testOperationsOnReservedUsers() throws Exception {
assertThat(authenticateResponse.authentication().getEffectiveSubject().getUser().principal(), is(username));
assertThat(authenticateResponse.authentication().getAuthenticatingSubject().getRealm().getName(), equalTo("reserved"));
assertThat(authenticateResponse.authentication().getAuthenticatingSubject().getRealm().getType(), equalTo("reserved"));
assertNull(authenticateResponse.authentication().getLookedUpBy());
assertFalse(authenticateResponse.authentication().isRunAs());
}

public void testOperationsOnReservedRoles() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1536,10 +1536,11 @@ LogEntryBuilder withRunAsSubject(Authentication authentication) {
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());
if (authentication.getLookedUpBy().getDomain() != null) {
logEntry.with(PRINCIPAL_RUN_AS_DOMAIN_FIELD_NAME, authentication.getLookedUpBy().getDomain().name());
final Authentication.RealmRef lookedUpBy = authentication.isRunAs() ? authentication.getEffectiveSubject().getRealm() : null;
if (lookedUpBy != null) {
logEntry.with(PRINCIPAL_RUN_AS_REALM_FIELD_NAME, lookedUpBy.getName());
if (lookedUpBy.getDomain() != null) {
logEntry.with(PRINCIPAL_RUN_AS_DOMAIN_FIELD_NAME, lookedUpBy.getDomain().name());
}
}
return this;
Expand Down Expand Up @@ -1627,7 +1628,7 @@ LogEntryBuilder withAuthentication(Authentication authentication) {
} else {
final Authentication.RealmRef authenticatedBy = authentication.getAuthenticatingSubject().getRealm();
if (authentication.isRunAs()) {
final Authentication.RealmRef lookedUpBy = authentication.getLookedUpBy();
final Authentication.RealmRef lookedUpBy = authentication.getEffectiveSubject().getRealm();
logEntry.with(PRINCIPAL_REALM_FIELD_NAME, lookedUpBy.getName())
.with(PRINCIPAL_RUN_BY_FIELD_NAME, authentication.getAuthenticatingSubject().getUser().principal())
// API key can run-as, when that happens, the following field will be _es_api_key,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ private static void consumeNullUser(
* names of users that exist using a timing attack
*/
public void lookupRunAsUser(Context context, Authentication authentication, ActionListener<Tuple<User, Realm>> listener) {
assert authentication.getLookedUpBy() == null : "authentication already has a lookup realm";
assert false == authentication.isRunAs() : "authentication already has run-as";
final String runAsUsername = context.getThreadContext().getHeader(AuthenticationServiceField.RUN_AS_USER_HEADER);
if (runAsUsername != null && runAsUsername.isEmpty() == false) {
logger.trace(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,8 @@ private void authorizeRunAs(
final ActionListener<AuthorizationResult> listener
) {
final Authentication authentication = requestInfo.getAuthentication();
if (authentication.getLookedUpBy() == null) {
assert authentication.isRunAs() : "authentication does not have a run-as";
if (authentication.getEffectiveSubject().getRealm() == null) {
// this user did not really exist
// TODO(jaymode) find a better way to indicate lookup failed for a user and we need to fail authz
listener.onResponse(AuthorizationResult.deny());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -864,7 +864,7 @@ private static boolean checkChangePasswordAction(Authentication authentication)
final boolean isRunAs = authentication.isRunAs();
final String realmType;
if (isRunAs) {
realmType = authentication.getLookedUpBy().getType();
realmType = authentication.getEffectiveSubject().getRealm().getType();
} else {
realmType = authentication.getAuthenticatingSubject().getRealm().getType();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,8 @@ public void testExecuteAfterRewritingAuthentication() throws IOException {
Authentication authentication = securityContext.getAuthentication();
assertEquals(original.getEffectiveSubject().getUser(), authentication.getEffectiveSubject().getUser());
assertEquals(original.getAuthenticatingSubject().getRealm(), authentication.getAuthenticatingSubject().getRealm());
assertEquals(original.getLookedUpBy(), authentication.getLookedUpBy());
assertEquals(original.isRunAs(), authentication.isRunAs());
assertEquals(original.getEffectiveSubject().getRealm(), authentication.getEffectiveSubject().getRealm());
assertEquals(VersionUtils.getPreviousVersion(), authentication.getEffectiveSubject().getVersion());
assertEquals(original.getAuthenticationType(), securityContext.getAuthentication().getAuthenticationType());
contextAtomicReference.set(originalCtx);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ public void onFailure(Exception e) {

assertThat(responseRef.get(), notNullValue());
if (anonymousUser.enabled() && false == authentication.isApiKey()) {
// Roles of anonymousUser are added to non api key authentication
final Authentication auth = responseRef.get().authentication();
final User userInResponse = auth.getEffectiveSubject().getUser();
assertThat(
Expand All @@ -152,11 +153,14 @@ public void onFailure(Exception e) {
if (auth.isRunAs()) {
assertThat(auth.getAuthenticatingSubject().getUser(), sameInstance(authentication.getAuthenticatingSubject().getUser()));
}
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()));
assertThat(auth.getAuthenticatingSubject().getMetadata(), sameInstance(auth.getAuthenticatingSubject().getMetadata()));
assertThat(auth.getAuthenticatingSubject().getRealm(), sameInstance(authentication.getAuthenticatingSubject().getRealm()));
assertThat(auth.getEffectiveSubject().getRealm(), sameInstance(authentication.getEffectiveSubject().getRealm()));
assertThat(auth.getEffectiveSubject().getVersion(), sameInstance(authentication.getEffectiveSubject().getVersion()));
assertThat(auth.getAuthenticationType(), sameInstance(authentication.getAuthenticationType()));
assertThat(
auth.getAuthenticatingSubject().getMetadata(),
sameInstance(authentication.getAuthenticatingSubject().getMetadata())
);
} else {
assertThat(responseRef.get().authentication(), sameInstance(authentication));
}
Expand All @@ -174,6 +178,8 @@ public void testShouldNotAddAnonymousRolesForApiKeyOrServiceAccount() {
authentication = AuthenticationTestHelper.builder().serviceAccount().build();
}
final User user = authentication.getEffectiveSubject().getUser();
// API key or service account have no named roles
assertThat(user.roles(), emptyArray());

TransportAuthenticateAction action = prepareAction(anonymousUser, user, authentication);

Expand All @@ -192,18 +198,8 @@ public void onFailure(Exception e) {
});

assertThat(responseRef.get(), notNullValue());
if (anonymousUser.enabled()) {
final Authentication auth = responseRef.get().authentication();
final User authUser = auth.getEffectiveSubject().getUser();
assertThat(authUser.roles(), emptyArray());
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()));
assertThat(auth.getAuthenticatingSubject().getMetadata(), sameInstance(auth.getAuthenticatingSubject().getMetadata()));
} else {
assertThat(responseRef.get().authentication(), sameInstance(authentication));
}
// Anonymous roles should not be added which means there is no change to the authentication at all
assertThat(responseRef.get().authentication(), sameInstance(authentication));
assertThat(throwableRef.get(), nullValue());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2895,7 +2895,7 @@ private static void authentication(Authentication authentication, MapBuilder<Str
} else {
final RealmRef authenticatedBy = authentication.getAuthenticatingSubject().getRealm();
if (authentication.isRunAs()) {
final RealmRef lookedUpBy = authentication.getLookedUpBy();
final RealmRef lookedUpBy = authentication.getEffectiveSubject().getRealm();
checkedFields.put(LoggingAuditTrail.PRINCIPAL_REALM_FIELD_NAME, lookedUpBy.getName())
.put(LoggingAuditTrail.PRINCIPAL_RUN_BY_FIELD_NAME, authentication.getAuthenticatingSubject().getUser().principal())
.put(LoggingAuditTrail.PRINCIPAL_RUN_BY_REALM_FIELD_NAME, authenticatedBy.getName());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ public void testAuthenticateBothSupportSecondSucceeds() throws Exception {
}
assertThat(result, notNullValue());
assertThat(result.getEffectiveSubject().getUser(), is(user));
assertThat(result.getLookedUpBy(), is(nullValue()));
assertThat(result.isRunAs(), is(false));
assertThat(result.getAuthenticatingSubject().getRealm(), is(notNullValue())); // TODO implement equals
assertThat(result.getAuthenticationType(), is(AuthenticationType.REALM));
assertThreadContextContainsAuthentication(result);
Expand Down Expand Up @@ -520,7 +520,7 @@ public void testAuthenticateSmartRealmOrdering() {
}
assertThat(result, notNullValue());
assertThat(result.getEffectiveSubject().getUser(), is(user));
assertThat(result.getLookedUpBy(), is(nullValue()));
assertThat(result.isRunAs(), is(false));
assertThat(result.getAuthenticatingSubject().getRealm(), is(notNullValue())); // TODO implement equals
assertThat(result.getAuthenticatingSubject().getRealm().getName(), is(SECOND_REALM_NAME));
assertThat(result.getAuthenticatingSubject().getRealm().getType(), is(SECOND_REALM_TYPE));
Expand All @@ -541,7 +541,7 @@ public void testAuthenticateSmartRealmOrdering() {
assertThat(expectAuditRequestId(threadContext), is(reqId.get()));
assertThat(result, notNullValue());
assertThat(result.getEffectiveSubject().getUser(), is(user));
assertThat(result.getLookedUpBy(), is(nullValue()));
assertThat(result.isRunAs(), is(false));
assertThat(result.getAuthenticatingSubject().getRealm(), is(notNullValue())); // TODO implement equals
assertThat(result.getAuthenticatingSubject().getRealm().getName(), is(SECOND_REALM_NAME));
assertThat(result.getAuthenticatingSubject().getRealm().getType(), is(SECOND_REALM_TYPE));
Expand Down Expand Up @@ -575,7 +575,7 @@ public void testAuthenticateSmartRealmOrdering() {
assertThat(expectAuditRequestId(threadContext), is(reqId.get()));
assertThat(result, notNullValue());
assertThat(result.getEffectiveSubject().getUser(), is(user));
assertThat(result.getLookedUpBy(), is(nullValue()));
assertThat(result.isRunAs(), is(false));
assertThat(result.getAuthenticatingSubject().getRealm(), is(notNullValue()));
assertThat(result.getAuthenticatingSubject().getRealm().getName(), is(FIRST_REALM_NAME));
assertThat(result.getAuthenticatingSubject().getRealm().getType(), is(FIRST_REALM_TYPE));
Expand Down Expand Up @@ -663,7 +663,7 @@ public void testAuthenticateSmartRealmOrderingDisabled() {
}
assertThat(result, notNullValue());
assertThat(result.getEffectiveSubject().getUser(), is(user));
assertThat(result.getLookedUpBy(), is(nullValue()));
assertThat(result.isRunAs(), is(false));
assertThat(result.getAuthenticatingSubject().getRealm().getName(), is(SECOND_REALM_NAME)); // TODO implement equals
assertThat(result.getAuthenticatingSubject().getRealm().getDomain(), is(secondDomain));
assertThreadContextContainsAuthentication(result);
Expand All @@ -679,7 +679,7 @@ public void testAuthenticateSmartRealmOrderingDisabled() {
assertThat(expectAuditRequestId(threadContext), is(reqId.get()));
assertThat(result, notNullValue());
assertThat(result.getEffectiveSubject().getUser(), is(user));
assertThat(result.getLookedUpBy(), is(nullValue()));
assertThat(result.isRunAs(), is(false));
assertThat(result.getAuthenticatingSubject().getRealm().getName(), is(SECOND_REALM_NAME)); // TODO implement equals
assertThat(result.getAuthenticatingSubject().getRealm().getDomain(), is(secondDomain));
assertThreadContextContainsAuthentication(result);
Expand Down Expand Up @@ -1888,7 +1888,7 @@ public void testAuthenticateWithToken() throws Exception {
service.authenticate("_action", transportRequest, true, ActionListener.wrap(result -> {
assertThat(result, notNullValue());
assertThat(result.getEffectiveSubject().getUser(), is(user));
assertThat(result.getLookedUpBy(), is(nullValue()));
assertThat(result.isRunAs(), is(false));
assertThat(result.getAuthenticatingSubject().getRealm(), is(notNullValue()));
assertThat(result.getAuthenticatingSubject().getRealm().getName(), is("realm")); // TODO implement equals
assertThat(result.getAuthenticationType(), is(AuthenticationType.TOKEN));
Expand Down Expand Up @@ -1931,7 +1931,7 @@ public void testInvalidToken() throws Exception {
service.authenticate("_action", transportRequest, true, ActionListener.wrap(result -> {
assertThat(result, notNullValue());
assertThat(result.getEffectiveSubject().getUser(), is(user));
assertThat(result.getLookedUpBy(), is(nullValue()));
assertThat(result.isRunAs(), is(false));
assertThat(result.getAuthenticatingSubject().getRealm(), is(notNullValue()));
assertThreadContextContainsAuthentication(result);
assertEquals(expected, result);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1049,7 +1049,8 @@ private void mockGetTokenAsyncForDecryptedToken(String accessToken) {
public static void assertAuthentication(Authentication result, Authentication expected) {
assertEquals(expected.getEffectiveSubject().getUser(), result.getEffectiveSubject().getUser());
assertEquals(expected.getAuthenticatingSubject().getRealm(), result.getAuthenticatingSubject().getRealm());
assertEquals(expected.getLookedUpBy(), result.getLookedUpBy());
assertEquals(expected.isRunAs(), result.isRunAs());
assertEquals(expected.getEffectiveSubject().getRealm(), result.getEffectiveSubject().getRealm());
assertEquals(expected.getAuthenticatingSubject().getMetadata(), result.getAuthenticatingSubject().getMetadata());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ public void testWillSetRunAsRealmForNonApiKeyAuth() throws Exception {

Map<String, Object> result = ingestDocument.getFieldValue("_field", Map.class);
assertThat(result, aMapWithSize(3));
final Authentication.RealmRef lookedUpRealmRef = auth.getLookedUpBy();
final Authentication.RealmRef lookedUpRealmRef = auth.getEffectiveSubject().getRealm();
assertThat(((Map<String, String>) result.get("realm")).get("name"), equalTo(lookedUpRealmRef.getName()));
assertThat(((Map<String, String>) result.get("realm")).get("type"), equalTo(lookedUpRealmRef.getType()));
}
Expand Down

0 comments on commit 18986bc

Please sign in to comment.