From c123edde6d051467d8f08457c0cbfc8c98e5aba7 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Tue, 25 Oct 2022 10:52:23 +1100 Subject: [PATCH] Remove the deprecated Authentication#getVersion method (#91067) This PR removes the deprecated Authentication#getVersion method and replaces its usages by #getEffectiveSubject#getVersion. --- .../xpack/core/ClientHelper.java | 2 +- .../core/security/authc/Authentication.java | 37 +++++++++---------- .../xpack/core/security/authc/Subject.java | 2 +- .../xpack/core/ClientHelperTests.java | 4 +- .../authc/AuthenticationTestHelper.java | 4 +- .../xpack/security/authc/TokenService.java | 2 +- .../SecurityServerTransportInterceptor.java | 2 +- .../xpack/security/SecurityContextTests.java | 2 +- .../TransportAuthenticateActionTests.java | 4 +- .../security/authc/TokenServiceTests.java | 2 +- .../authz/AuthorizationUtilsTests.java | 4 +- ...curityServerTransportInterceptorTests.java | 10 ++--- 12 files changed, 36 insertions(+), 39 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ClientHelper.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ClientHelper.java index c194c5973857a..59ff0e2c53cd2 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ClientHelper.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ClientHelper.java @@ -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) { diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java index aefff7ba2afce..3cb92eebec14a 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Authentication.java @@ -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); @@ -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. */ @@ -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 @@ -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())); } @@ -918,7 +915,7 @@ private static Map 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( @@ -931,7 +928,7 @@ private static Map 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( diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Subject.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Subject.java index 6a7a1fedade69..2a8fa800b2ae4 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Subject.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Subject.java @@ -87,7 +87,7 @@ public Map getMetadata() { return metadata; } - Version getVersion() { + public Version getVersion() { return version; } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ClientHelperTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ClientHelperTests.java index bea688e9784de..06fc5842f11ef 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ClientHelperTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ClientHelperTests.java @@ -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())); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTestHelper.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTestHelper.java index 1bbcec5d92ce8..4a4932ebf4baf 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTestHelper.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTestHelper.java @@ -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() { @@ -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; diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java index e2ace1b12ba13..5180587a979a2 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java @@ -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(); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportInterceptor.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportInterceptor.java index 6de25b214f6fd..9ca4c17fe2967 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportInterceptor.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportInterceptor.java @@ -117,7 +117,7 @@ public 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( diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityContextTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityContextTests.java index 2dd6bbd0cbd34..0c16c8106c30e 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityContextTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityContextTests.java @@ -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 diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportAuthenticateActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportAuthenticateActionTests.java index 6a4541f52ae90..96a95fa8c303d 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportAuthenticateActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/user/TransportAuthenticateActionTests.java @@ -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 { @@ -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 { diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenServiceTests.java index 73a97ebe11122..bf4e0146eb37c 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenServiceTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenServiceTests.java @@ -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( diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationUtilsTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationUtilsTests.java index cb4008f266761..8b58592ccf5a0 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationUtilsTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationUtilsTests.java @@ -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())); @@ -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); }; diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportInterceptorTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportInterceptorTests.java index df3cdab44e1d5..283529a4496c7 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportInterceptorTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportInterceptorTests.java @@ -295,8 +295,8 @@ public 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 { @@ -361,8 +361,8 @@ public 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() { @@ -421,7 +421,7 @@ public 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 {