Skip to content

Commit a598e6b

Browse files
ywangdywangd
authored
Remove the deprecated Authentication#getSourceRealm method (#92222)
This PR removes the deprecated Authentication#getSourceRealm method. Its usage is mostly replaced by #getEffectiveSubject#getRealm except for ApiKeyService#getCreatorRealmName and ApiKeyService#getCreatorRealmType which has a special handling to return authenticatingSubject's realm when run-as lookup fails. This is to maintain BWC since these information is used in audit logs. Therefore, even it is technically incorrect, we should not break it without careful planning. Relates: #88494
1 parent 62fb333 commit a598e6b

File tree

13 files changed

+105
-80
lines changed

13 files changed

+105
-80
lines changed

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java‎

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,22 @@
7070
* Authentication is serialized and travels across the cluster nodes as the sub-requests are handled,
7171
* and can also be cached by long-running jobs that continue to act on behalf of the user, beyond
7272
* the lifetime of the original request.
73+
*
74+
* The authentication consists of two {@link Subject}s
75+
* <ul>
76+
* <li>{@link #authenticatingSubject} performs the authentication, i.e. it provides a credential.</li>
77+
* <li>{@link #effectiveSubject} The subject that {@link #authenticatingSubject} impersonates ({@link #isRunAs()})</li>
78+
* </ul>
79+
* If {@link #isRunAs()} is {@code false}, the two {@link Subject}s will be the same object.
80+
*
81+
* Authentication also has a {@link #type} that indicates which mechanism the {@link #authenticatingSubject}
82+
* uses to perform the authentication.
83+
*
84+
* The Authentication's version is its {@link Subject}'s version, i.e. {@code getEffectiveSubject().getVersion()}.
85+
* It is guaranteed that the versions are identical for the two Subjects. Hence {@code getAuthenticatingSubject().getVersion()}
86+
* will give out the same result. But using {@code getEffectiveSubject()} is more idiomatic since most callers
87+
* of this class should just need to know about the {@link #effectiveSubject}. That is, often times, the caller
88+
* begins with {@code authentication.getEffectiveSubject()} for interrogating an Authentication object.
7389
*/
7490
public final class Authentication implements ToXContentObject {
7591

@@ -167,22 +183,8 @@ public boolean isRunAs() {
167183
return authenticatingSubject != effectiveSubject;
168184
}
169185

170-
/**
171-
* Get the realm where the effective user comes from.
172-
* The effective user is the es-security-runas-user if present or the authenticated user.
173-
*
174-
* Use {@code getEffectiveSubject().getRealm()} instead.
175-
*/
176-
@Deprecated
177-
public RealmRef getSourceRealm() {
178-
// TODO: This code retains the existing behaviour which is slightly wrong because
179-
// when run-as lookup fails, the effectiveSubject will have a null realm. In this
180-
// case, the code returns the authenticatingSubject's realm. This is wrong in theory
181-
// because it is not the intention of this method. In practice, it does not matter
182-
// because failed lookup will be rejected at authZ time. But fixing it causes test
183-
// failures. So leave it for now.
184-
final RealmRef sourceRealm = effectiveSubject.getRealm();
185-
return sourceRealm == null ? authenticatingSubject.getRealm() : sourceRealm;
186+
public boolean isFailedRunAs() {
187+
return isRunAs() && effectiveSubject.getRealm() == null;
186188
}
187189

188190
/**
@@ -228,9 +230,6 @@ public Authentication maybeRewriteForOlderVersion(Version olderVersion) {
228230
);
229231

230232
}
231-
if (isAssignedToDomain() && false == newAuthentication.isAssignedToDomain()) {
232-
logger.info("Rewriting authentication [" + this + "] without domain");
233-
}
234233
return newAuthentication;
235234
}
236235

@@ -262,7 +261,6 @@ public Authentication runAs(User runAs, @Nullable RealmRef lookupRealmRef) {
262261
public Authentication token() {
263262
assert false == isServiceAccount();
264263
final Authentication newTokenAuthentication = new Authentication(effectiveSubject, authenticatingSubject, AuthenticationType.TOKEN);
265-
assert Objects.equals(getDomain(), newTokenAuthentication.getDomain());
266264
return newTokenAuthentication;
267265
}
268266

@@ -325,23 +323,28 @@ public Authentication maybeAddAnonymousRoles(@Nullable AnonymousUser anonymousUs
325323
}
326324
}
327325

326+
// Package private for tests
328327
/**
329328
* Returns {@code true} if the effective user belongs to a realm under a domain.
330-
* See also {@link #getDomain()} and {@link #getSourceRealm()}.
331329
*/
332-
public boolean isAssignedToDomain() {
330+
boolean isAssignedToDomain() {
333331
return getDomain() != null;
334332
}
335333

334+
// Package private for tests
336335
/**
337336
* Returns the {@link RealmDomain} that the effective user belongs to.
338337
* A user belongs to a realm which in turn belongs to a domain.
339338
*
340339
* The same username can be authenticated by different realms (e.g. with different credential types),
341340
* but resources created across realms cannot be accessed unless the realms are also part of the same domain.
342341
*/
343-
public @Nullable RealmDomain getDomain() {
344-
return getSourceRealm().getDomain();
342+
@Nullable
343+
RealmDomain getDomain() {
344+
if (isFailedRunAs()) {
345+
return null;
346+
}
347+
return getEffectiveSubject().getRealm().getDomain();
345348
}
346349

347350
public boolean isAuthenticatedWithServiceAccount() {
@@ -861,6 +864,7 @@ public static Authentication newApiKeyAuthentication(AuthenticationResult<User>
861864

862865
private static RealmRef maybeRewriteRealmRef(Version streamVersion, RealmRef realmRef) {
863866
if (realmRef != null && realmRef.getDomain() != null && streamVersion.before(VERSION_REALM_DOMAINS)) {
867+
logger.info("Rewriting realm [" + realmRef + "] without domain");
864868
// security domain erasure
865869
new RealmRef(realmRef.getName(), realmRef.getType(), realmRef.getNodeName(), null);
866870
}

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,11 +145,11 @@ private static boolean checkIfUserIsOwnerOfApiKeys(
145145
if (false == username.equals(authentication.getEffectiveSubject().getUser().principal())) {
146146
return false;
147147
}
148-
RealmDomain domain = authentication.getSourceRealm().getDomain();
148+
RealmDomain domain = authentication.getEffectiveSubject().getRealm().getDomain();
149149
if (domain != null) {
150150
return domain.realms().stream().anyMatch(realmIdentifier -> realmName.equals(realmIdentifier.getName()));
151151
} else {
152-
return realmName.equals(authentication.getSourceRealm().getName());
152+
return realmName.equals(authentication.getEffectiveSubject().getRealm().getName());
153153
}
154154
}
155155
}

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTestHelper.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,12 @@ public Authentication build() {
377377
return build(ESTestCase.randomBoolean());
378378
}
379379

380-
public Authentication build(boolean runAsIfNotAlready) {
380+
/**
381+
* @param maybeRunAsIfNotAlready If the authentication is *not* run-as and the subject is a realm user, it will be transformed
382+
* into a run-as authentication by moving the realm user to be the run-as user. The authenticating
383+
* subject can be either a realm user or an API key (in general any subject type that can run-as).
384+
*/
385+
public Authentication build(boolean maybeRunAsIfNotAlready) {
381386
if (authenticatingAuthentication != null) {
382387
if (user == null) {
383388
user = randomUser();
@@ -402,7 +407,7 @@ public Authentication build(boolean runAsIfNotAlready) {
402407
realmRef = randomRealmRef(isRealmUnderDomain == null ? ESTestCase.randomBoolean() : isRealmUnderDomain);
403408
}
404409
assert false == SYNTHETIC_REALM_TYPES.contains(realmRef.getType()) : "use dedicate methods for synthetic realms";
405-
if (runAsIfNotAlready) {
410+
if (maybeRunAsIfNotAlready) {
406411
authentication = builder().runAs().user(user).realmRef(realmRef).build();
407412
} else {
408413
authentication = Authentication.newRealmAuthentication(user, realmRef);

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTests.java‎

Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -40,28 +40,14 @@
4040

4141
public class AuthenticationTests extends ESTestCase {
4242

43-
public void testWillGetLookedUpByWhenItExists() {
44-
final RealmRef authenticatedBy = new RealmRef("auth_by", "auth_by_type", "node");
45-
final RealmRef lookedUpBy = new RealmRef("lookup_by", "lookup_by_type", "node");
46-
final Authentication authentication = AuthenticationTestHelper.builder()
47-
.user(new User("not-user"))
48-
.realmRef(authenticatedBy)
49-
.runAs()
50-
.user(new User("user"))
51-
.realmRef(lookedUpBy)
52-
.build();
53-
54-
assertEquals(lookedUpBy, authentication.getSourceRealm());
55-
}
56-
57-
public void testWillGetAuthenticateByWhenLookupIsNull() {
58-
final RealmRef authenticatedBy = new RealmRef("auth_by", "auth_by_type", "node");
59-
final Authentication authentication = AuthenticationTestHelper.builder()
60-
.user(new User("user"))
61-
.realmRef(authenticatedBy)
62-
.build(false);
63-
64-
assertEquals(authenticatedBy, authentication.getSourceRealm());
43+
public void testIsFailedRunAs() {
44+
final Authentication failedAuthentication = randomRealmAuthentication(randomBoolean()).runAs(randomUser(), null);
45+
assertTrue(failedAuthentication.isRunAs());
46+
assertTrue(failedAuthentication.isFailedRunAs());
47+
48+
final Authentication authentication = AuthenticationTestHelper.builder().realm().runAs().build();
49+
assertTrue(authentication.isRunAs());
50+
assertFalse(authentication.isFailedRunAs());
6551
}
6652

6753
public void testCanAccessResourcesOf() {

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1748,7 +1748,11 @@ public static String getCreatorRealmName(final Authentication authentication) {
17481748
} else {
17491749
// TODO we should use the effective subject realm here but need to handle the failed lookup scenario, in which the realm may be
17501750
// `null`. Since this method is used in audit logging, this requires some care.
1751-
return authentication.getSourceRealm().getName();
1751+
if (authentication.isFailedRunAs()) {
1752+
return authentication.getAuthenticatingSubject().getRealm().getName();
1753+
} else {
1754+
return authentication.getEffectiveSubject().getRealm().getName();
1755+
}
17521756
}
17531757
}
17541758

@@ -1791,7 +1795,11 @@ public static String getCreatorRealmType(final Authentication authentication) {
17911795
} else {
17921796
// TODO we should use the effective subject realm here but need to handle the failed lookup scenario, in which the realm may be
17931797
// `null`. Since this method is used in audit logging, this requires some care.
1794-
return authentication.getSourceRealm().getType();
1798+
if (authentication.isFailedRunAs()) {
1799+
return authentication.getAuthenticatingSubject().getRealm().getType();
1800+
} else {
1801+
return authentication.getEffectiveSubject().getRealm().getType();
1802+
}
17951803
}
17961804
}
17971805

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1805,12 +1805,13 @@ static BytesReference createTokenDocument(
18051805
}
18061806
builder.endObject().endObject();
18071807
}
1808+
final Authentication.RealmRef userTokenEffectiveRealm = userToken.getAuthentication().getEffectiveSubject().getRealm();
18081809
builder.startObject("access_token")
18091810
.field("invalidated", false)
18101811
.field("user_token", userToken)
1811-
.field("realm", userToken.getAuthentication().getSourceRealm().getName());
1812-
if (userToken.getAuthentication().getSourceRealm().getDomain() != null) {
1813-
builder.field("realm_domain", userToken.getAuthentication().getSourceRealm().getDomain());
1812+
.field("realm", userTokenEffectiveRealm.getName());
1813+
if (userTokenEffectiveRealm.getDomain() != null) {
1814+
builder.field("realm_domain", userTokenEffectiveRealm.getDomain());
18141815
}
18151816
builder.endObject().endObject();
18161817
return BytesReference.bytes(builder);

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/service/IndexServiceAccountTokenStore.java‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import org.elasticsearch.xpack.core.security.action.service.TokenInfo;
4848
import org.elasticsearch.xpack.core.security.action.service.TokenInfo.TokenSource;
4949
import org.elasticsearch.xpack.core.security.authc.Authentication;
50+
import org.elasticsearch.xpack.core.security.authc.Subject;
5051
import org.elasticsearch.xpack.core.security.authc.support.Hasher;
5152
import org.elasticsearch.xpack.security.authc.service.ServiceAccount.ServiceAccountId;
5253
import org.elasticsearch.xpack.security.authc.service.ServiceAccountToken.ServiceAccountTokenId;
@@ -269,15 +270,16 @@ private XContentBuilder newDocument(Authentication authentication, ServiceAccoun
269270
.field("creation_time", clock.instant().toEpochMilli())
270271
.field("enabled", true);
271272
{
273+
final Subject effectiveSubject = authentication.getEffectiveSubject();
272274
builder.startObject("creator")
273-
.field("principal", authentication.getEffectiveSubject().getUser().principal())
274-
.field("full_name", authentication.getEffectiveSubject().getUser().fullName())
275-
.field("email", authentication.getEffectiveSubject().getUser().email())
276-
.field("metadata", authentication.getEffectiveSubject().getUser().metadata())
277-
.field("realm", authentication.getSourceRealm().getName())
278-
.field("realm_type", authentication.getSourceRealm().getType());
279-
if (authentication.getSourceRealm().getDomain() != null) {
280-
builder.field("realm_domain", authentication.getSourceRealm().getDomain());
275+
.field("principal", effectiveSubject.getUser().principal())
276+
.field("full_name", effectiveSubject.getUser().fullName())
277+
.field("email", effectiveSubject.getUser().email())
278+
.field("metadata", effectiveSubject.getUser().metadata())
279+
.field("realm", effectiveSubject.getRealm().getName())
280+
.field("realm_type", effectiveSubject.getRealm().getType());
281+
if (effectiveSubject.getRealm().getDomain() != null) {
282+
builder.field("realm_domain", effectiveSubject.getRealm().getDomain());
281283
}
282284
builder.endObject();
283285
}

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/operator/FileOperatorUsersStore.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,11 @@ public boolean isOperatorUser(Authentication authentication) {
6868
// If not null, it will be compared exactly as well.
6969
// The special handling for realm name is because there can only be one file or native realm and it does
7070
// not matter what the name is.
71+
final Authentication.RealmRef realm = authentication.getEffectiveSubject().getRealm();
72+
if (realm == null) {
73+
return false;
74+
}
7175
return operatorUsersDescriptor.groups.stream().anyMatch(group -> {
72-
final Authentication.RealmRef realm = authentication.getSourceRealm();
7376
final boolean match = group.usernames.contains(authentication.getEffectiveSubject().getUser().principal())
7477
&& group.authenticationType == authentication.getAuthenticationType()
7578
&& realm.getType().equals(group.realmType)

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/audit/logfile/LoggingAuditTrailTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2019,7 +2019,7 @@ public void testAccessGrantedInternalSystemAction() throws Exception {
20192019
.put(LoggingAuditTrail.EVENT_ACTION_FIELD_NAME, "access_granted")
20202020
.put(LoggingAuditTrail.AUTHENTICATION_TYPE_FIELD_NAME, authentication.getAuthenticationType().toString())
20212021
.put(LoggingAuditTrail.PRINCIPAL_FIELD_NAME, systemUser.principal())
2022-
.put(LoggingAuditTrail.PRINCIPAL_REALM_FIELD_NAME, authentication.getSourceRealm().getName())
2022+
.put(LoggingAuditTrail.PRINCIPAL_REALM_FIELD_NAME, authentication.getEffectiveSubject().getRealm().getName())
20232023
.put(LoggingAuditTrail.ACTION_FIELD_NAME, "internal:_action")
20242024
.put(LoggingAuditTrail.REQUEST_NAME_FIELD_NAME, request.getClass().getSimpleName())
20252025
.put(LoggingAuditTrail.REQUEST_ID_FIELD_NAME, requestId);

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1512,8 +1512,8 @@ public void testGetCreatorRealm() {
15121512

15131513
// Realm
15141514
final Authentication authentication3 = AuthenticationTests.randomRealmAuthentication(randomBoolean());
1515-
assertThat(ApiKeyService.getCreatorRealmName(authentication3), equalTo(authentication3.getSourceRealm().getName()));
1516-
assertThat(ApiKeyService.getCreatorRealmType(authentication3), equalTo(authentication3.getSourceRealm().getType()));
1515+
assertThat(ApiKeyService.getCreatorRealmName(authentication3), equalTo(authentication3.getEffectiveSubject().getRealm().getName()));
1516+
assertThat(ApiKeyService.getCreatorRealmType(authentication3), equalTo(authentication3.getEffectiveSubject().getRealm().getType()));
15171517

15181518
// Realm run-as
15191519
final Authentication authentication4 = authentication3.runAs(AuthenticationTests.randomUser(), lookupRealmRef);
@@ -1526,8 +1526,19 @@ public void testGetCreatorRealm() {
15261526
AuthenticationTests.randomAnonymousAuthentication(),
15271527
AuthenticationTests.randomInternalAuthentication()
15281528
);
1529-
assertThat(ApiKeyService.getCreatorRealmName(authentication5), equalTo(authentication5.getSourceRealm().getName()));
1530-
assertThat(ApiKeyService.getCreatorRealmType(authentication5), equalTo(authentication5.getSourceRealm().getType()));
1529+
assertThat(ApiKeyService.getCreatorRealmName(authentication5), equalTo(authentication5.getEffectiveSubject().getRealm().getName()));
1530+
assertThat(ApiKeyService.getCreatorRealmType(authentication5), equalTo(authentication5.getEffectiveSubject().getRealm().getType()));
1531+
1532+
// Failed run-as returns authenticating subject's realm
1533+
final Authentication authentication6 = authentication3.runAs(AuthenticationTests.randomUser(), null);
1534+
assertThat(
1535+
ApiKeyService.getCreatorRealmName(authentication6),
1536+
equalTo(authentication6.getAuthenticatingSubject().getRealm().getName())
1537+
);
1538+
assertThat(
1539+
ApiKeyService.getCreatorRealmType(authentication6),
1540+
equalTo(authentication6.getAuthenticatingSubject().getRealm().getType())
1541+
);
15311542
}
15321543

15331544
public void testGetOwnersRealmNames() {

0 commit comments

Comments
 (0)