Skip to content

Commit

Permalink
Remove the deprecated Authentication#getVersion method (#91067)
Browse files Browse the repository at this point in the history
This PR removes the deprecated Authentication#getVersion method and
replaces its usages by #getEffectiveSubject#getVersion.
  • Loading branch information
ywangd authored Oct 24, 2022
1 parent 6e1e807 commit c123edd
Show file tree
Hide file tree
Showing 12 changed files with 36 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ private static String maybeRewriteSingleAuthenticationHeaderForVersion(
) {
try {
final Authentication authentication = authenticationReader.apply(authenticationHeaderKey);
if (authentication != null && authentication.getVersion().after(minNodeVersion)) {
if (authentication != null && authentication.getEffectiveSubject().getVersion().after(minNodeVersion)) {
return authentication.maybeRewriteForOlderVersion(minNodeVersion).encode();
}
} catch (IOException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,14 @@
import static org.elasticsearch.xpack.core.security.authc.AuthenticationField.FALLBACK_REALM_TYPE;
import static org.elasticsearch.xpack.core.security.authc.RealmDomain.REALM_DOMAIN_PARSER;

/**
* The Authentication class encapsulates identity information created after successful authentication
* and is the starting point of subsequent authorization.
*
* Authentication is serialized and travels across the cluster nodes as the sub-requests are handled,
* and can also be cached by long-running jobs that continue to act on behalf of the user, beyond
* the lifetime of the original request.
*/
public final class Authentication implements ToXContentObject {

private static final Logger logger = LogManager.getLogger(Authentication.class);
Expand Down Expand Up @@ -207,21 +215,6 @@ public RealmRef getSourceRealm() {
return sourceRealm == null ? authenticatingSubject.getRealm() : sourceRealm;
}

/**
* Returns the authentication version.
* Nodes can only interpret authentications from current or older versions as the node's.
*
* Authentication is serialized and travels across the cluster nodes as the sub-requests are handled,
* and can also be cached by long-running jobs that continue to act on behalf of the user, beyond
* the lifetime of the original request.
*
* Use {@code getEffectiveSubject().getVersion()} instead.
*/
@Deprecated
public Version getVersion() {
return effectiveSubject.getVersion();
}

/**
* Use {@code getAuthenticatingSubject().getMetadata()} instead.
*/
Expand Down Expand Up @@ -294,7 +287,11 @@ public Authentication runAs(User runAs, @Nullable RealmRef lookupRealmRef) {
assert false == hasSyntheticRealmNameOrType(lookupRealmRef) : "should not use synthetic realm name/type for lookup realms";

Objects.requireNonNull(runAs);
return new Authentication(new Subject(runAs, lookupRealmRef, getVersion(), Map.of()), authenticatingSubject, type);
return new Authentication(
new Subject(runAs, lookupRealmRef, getEffectiveSubject().getVersion(), Map.of()),
authenticatingSubject,
type
);
}

/** Returns a new {@code Authentication} for tokens created by the current {@code Authentication}, which is used when
Expand Down Expand Up @@ -478,8 +475,8 @@ public void writeToContext(ThreadContext ctx) throws IOException, IllegalArgumen

public String encode() throws IOException {
BytesStreamOutput output = new BytesStreamOutput();
output.setVersion(getVersion());
Version.writeVersion(getVersion(), output);
output.setVersion(getEffectiveSubject().getVersion());
Version.writeVersion(getEffectiveSubject().getVersion(), output);
writeTo(output);
return Base64.getEncoder().encodeToString(BytesReference.toBytes(output.bytes()));
}
Expand Down Expand Up @@ -918,7 +915,7 @@ private static Map<String, Object> maybeRewriteMetadataForApiKeyRoleDescriptors(
: "metadata must contain role descriptor for API key authentication";
assert metadata.containsKey(AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY)
: "metadata must contain limited role descriptor for API key authentication";
if (authentication.getVersion().onOrAfter(VERSION_API_KEY_ROLES_AS_BYTES)
if (authentication.getEffectiveSubject().getVersion().onOrAfter(VERSION_API_KEY_ROLES_AS_BYTES)
&& streamVersion.before(VERSION_API_KEY_ROLES_AS_BYTES)) {
metadata = new HashMap<>(metadata);
metadata.put(
Expand All @@ -931,7 +928,7 @@ private static Map<String, Object> maybeRewriteMetadataForApiKeyRoleDescriptors(
(BytesReference) metadata.get(AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY)
)
);
} else if (authentication.getVersion().before(VERSION_API_KEY_ROLES_AS_BYTES)
} else if (authentication.getEffectiveSubject().getVersion().before(VERSION_API_KEY_ROLES_AS_BYTES)
&& streamVersion.onOrAfter(VERSION_API_KEY_ROLES_AS_BYTES)) {
metadata = new HashMap<>(metadata);
metadata.put(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public Map<String, Object> getMetadata() {
return metadata;
}

Version getVersion() {
public Version getVersion() {
return version;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -454,14 +454,14 @@ public void testGetPersistableSafeSecurityHeaders() throws IOException {
final Authentication rewrittenAuth = AuthenticationContextSerializer.decode(
headers2.get(AuthenticationField.AUTHENTICATION_KEY)
);
assertThat(rewrittenAuth.getVersion(), equalTo(previousVersion));
assertThat(rewrittenAuth.getEffectiveSubject().getVersion(), equalTo(previousVersion));
assertThat(rewrittenAuth.getUser(), equalTo(authentication.getUser()));
}
if (hasSecondaryAuthHeader) {
final Authentication rewrittenSecondaryAuth = AuthenticationContextSerializer.decode(
headers2.get(SecondaryAuthentication.THREAD_CTX_KEY)
);
assertThat(rewrittenSecondaryAuth.getVersion(), equalTo(previousVersion));
assertThat(rewrittenSecondaryAuth.getEffectiveSubject().getVersion(), equalTo(previousVersion));
assertThat(rewrittenSecondaryAuth.getUser(), equalTo(authentication.getUser()));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ private AuthenticationTestBuilder() {}
private AuthenticationTestBuilder(Authentication authentication) {
assert false == authentication.isRunAs() : "authenticating authentication cannot itself be run-as";
this.authenticatingAuthentication = authentication;
this.version = authentication.getVersion();
this.version = authentication.getEffectiveSubject().getVersion();
}

public AuthenticationTestBuilder realm() {
Expand Down Expand Up @@ -502,7 +502,7 @@ public Authentication build(boolean runAsIfNotAlready) {
if (version == null) {
version = Version.CURRENT;
}
if (version.before(authentication.getVersion())) {
if (version.before(authentication.getEffectiveSubject().getVersion())) {
return authentication.maybeRewriteForOlderVersion(version);
} else {
return authentication;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2542,7 +2542,7 @@ static final class RefreshTokenStatus {
String iv,
String salt
) {
assert associatedAuthentication.getVersion().onOrAfter(VERSION_CLIENT_AUTH_FOR_REFRESH);
assert associatedAuthentication.getEffectiveSubject().getVersion().onOrAfter(VERSION_CLIENT_AUTH_FOR_REFRESH);
this.invalidated = invalidated;
// not used, filled-in for consistency's sake
this.associatedUser = associatedAuthentication.getUser().principal();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public <T extends TransportResponse> void sendRequest(
)
);
} else if (securityContext.getAuthentication() != null
&& securityContext.getAuthentication().getVersion().equals(minVersion) == false) {
&& securityContext.getAuthentication().getEffectiveSubject().getVersion().equals(minVersion) == false) {
// re-write the authentication since we want the authentication version to match the version of the connection
securityContext.executeAfterRewritingAuthentication(
original -> sendWithUser(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ public void testExecuteAfterRewritingAuthentication() throws IOException {
assertEquals(original.getUser(), authentication.getUser());
assertEquals(original.getAuthenticatedBy(), authentication.getAuthenticatedBy());
assertEquals(original.getLookedUpBy(), authentication.getLookedUpBy());
assertEquals(VersionUtils.getPreviousVersion(), authentication.getVersion());
assertEquals(VersionUtils.getPreviousVersion(), authentication.getEffectiveSubject().getVersion());
assertEquals(original.getAuthenticationType(), securityContext.getAuthentication().getAuthenticationType());
contextAtomicReference.set(originalCtx);
// Other request headers should be preserved
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ public void onFailure(Exception e) {
}
assertThat(auth.getAuthenticatedBy(), sameInstance(auth.getAuthenticatedBy()));
assertThat(auth.getLookedUpBy(), sameInstance(auth.getLookedUpBy()));
assertThat(auth.getVersion(), sameInstance(auth.getVersion()));
assertThat(auth.getEffectiveSubject().getVersion(), sameInstance(auth.getEffectiveSubject().getVersion()));
assertThat(auth.getAuthenticationType(), sameInstance(auth.getAuthenticationType()));
assertThat(auth.getMetadata(), sameInstance(auth.getMetadata()));
} else {
Expand Down Expand Up @@ -198,7 +198,7 @@ public void onFailure(Exception e) {
assertThat(authUser.roles(), emptyArray());
assertThat(auth.getAuthenticatedBy(), sameInstance(auth.getAuthenticatedBy()));
assertThat(auth.getLookedUpBy(), sameInstance(auth.getLookedUpBy()));
assertThat(auth.getVersion(), sameInstance(auth.getVersion()));
assertThat(auth.getEffectiveSubject().getVersion(), sameInstance(auth.getEffectiveSubject().getVersion()));
assertThat(auth.getAuthenticationType(), sameInstance(auth.getAuthenticationType()));
assertThat(auth.getMetadata(), sameInstance(auth.getMetadata()));
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ private RefreshTokenStatus newRefreshTokenStatus(
String iv,
String salt
) {
if (authentication.getVersion().onOrAfter(VERSION_CLIENT_AUTH_FOR_REFRESH)) {
if (authentication.getEffectiveSubject().getVersion().onOrAfter(VERSION_CLIENT_AUTH_FOR_REFRESH)) {
return new RefreshTokenStatus(invalidated, authentication, refreshed, refreshInstant, supersedingTokens, iv, salt);
} else {
return new RefreshTokenStatus(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ private void assertSwitchBasedOnOriginAndExecute(String origin, User user, Versi
assertNull(threadContext.getHeader(headerName));
final Authentication authentication = securityContext.getAuthentication();
assertEquals(user, authentication.getUser());
assertEquals(version, authentication.getVersion());
assertEquals(version, authentication.getEffectiveSubject().getVersion());
latch.countDown();
}, e -> fail(e.getMessage()));

Expand All @@ -160,7 +160,7 @@ private void assertSwitchBasedOnOriginAndExecute(String origin, User user, Versi
assertNull(threadContext.getHeader(headerName));
final Authentication authentication = securityContext.getAuthentication();
assertEquals(user, authentication.getUser());
assertEquals(version, authentication.getVersion());
assertEquals(version, authentication.getEffectiveSubject().getVersion());
latch.countDown();
listener.onResponse(null);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,8 +295,8 @@ public <T extends TransportResponse> void sendRequest(
assertTrue(calledWrappedSender.get());
assertEquals(authentication.getUser(), sendingUser.get());
assertEquals(authentication.getUser(), securityContext.getUser());
assertEquals(Version.CURRENT, authRef.get().getVersion());
assertEquals(Version.CURRENT, authentication.getVersion());
assertEquals(Version.CURRENT, authRef.get().getEffectiveSubject().getVersion());
assertEquals(Version.CURRENT, authentication.getEffectiveSubject().getVersion());
}

public void testSendToOlderVersionSetsCorrectVersion() throws Exception {
Expand Down Expand Up @@ -361,8 +361,8 @@ public <T extends TransportResponse> void sendRequest(
assertTrue(calledWrappedSender.get());
assertEquals(authentication.getUser(), sendingUser.get());
assertEquals(authentication.getUser(), securityContext.getUser());
assertEquals(connectionVersion, authRef.get().getVersion());
assertEquals(Version.CURRENT, authentication.getVersion());
assertEquals(connectionVersion, authRef.get().getEffectiveSubject().getVersion());
assertEquals(Version.CURRENT, authentication.getEffectiveSubject().getVersion());
}

public void testSetUserBasedOnActionOrigin() {
Expand Down Expand Up @@ -421,7 +421,7 @@ public <T extends TransportResponse> void sendRequest(
final Authentication authentication = authenticationRef.get();
assertThat(authentication, notNullValue());
assertThat(authentication.getUser(), equalTo(originToUserMap.get(origin)));
assertThat(authentication.getVersion(), equalTo(connectionVersion));
assertThat(authentication.getEffectiveSubject().getVersion(), equalTo(connectionVersion));
}

public void testContextRestoreResponseHandler() throws Exception {
Expand Down

0 comments on commit c123edd

Please sign in to comment.