Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove the deprecated Authentication#getVersion method #91067

Merged
merged 1 commit into from
Oct 24, 2022
Merged

Remove the deprecated Authentication#getVersion method #91067

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ClientHelper.java
View file
Open in desktop
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
37 changes: 17 additions & 20 deletions ...plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java
View file
Open in desktop
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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional: could make sense to add a note along the lines of "Use getEffectiveSubject().getVersion() to retrieve the version of an authentication instance." to the Javadoc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be helpful to add some more comments in the javadoc for the Authenticaton to explain its current structure and how it should be reasoned about. I'll make it in a separate PR.

*/
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
2 changes: 1 addition & 1 deletion x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Subject.java
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ClientHelperTests.java
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions ...e/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTestHelper.java
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion ...ck/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion ...n/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportInterceptor.java
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion .../plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityContextTests.java
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions ...t/java/org/elasticsearch/xpack/security/action/user/TransportAuthenticateActionTests.java
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion ...ugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenServiceTests.java
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions ...ecurity/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationUtilsTests.java
View file
Open in desktop
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
10 changes: 5 additions & 5 deletions ...a/org/elasticsearch/xpack/security/transport/SecurityServerTransportInterceptorTests.java
View file
Open in desktop
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