From 75319f4e949a4f6778d17d95ce93aedf256ed394 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Tue, 19 Oct 2021 19:50:34 +1100 Subject: [PATCH] Preserve request headers in a mixed version cluster (#79412) When rewriting authentication for requests crossing nodes of different versions, we now preserve all request headers except the authentication one which needs to be rewritten. Previously all other request headers were dropped and it caused issue like an operator user not being recognised on the remote node. Other now preserved headers include audit and system index access. This new behaviour is more correct because we would never drop these headers if the nodes are on the same version. Resolves: #79354 --- .../xpack/core/security/SecurityContext.java | 8 ++++++++ .../xpack/security/SecurityContextTests.java | 8 ++++++++ x-pack/qa/rolling-upgrade/build.gradle | 8 +++++++- .../src/test/resources/operator_users.yml | 2 ++ .../mixed_cluster/130_operator_privileges.yml | 17 +++++++++++++++++ 5 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 x-pack/qa/rolling-upgrade/src/test/resources/operator_users.yml create mode 100644 x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/130_operator_privileges.yml diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityContext.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityContext.java index 1bdbb6e6c4c59..3e47b63f3c8ed 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityContext.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityContext.java @@ -21,6 +21,7 @@ import org.elasticsearch.node.Node; import org.elasticsearch.xpack.core.security.authc.Authentication; import org.elasticsearch.xpack.core.security.authc.Authentication.AuthenticationType; +import org.elasticsearch.xpack.core.security.authc.AuthenticationField; import org.elasticsearch.xpack.core.security.authc.support.AuthenticationContextSerializer; import org.elasticsearch.xpack.core.security.authc.support.SecondaryAuthentication; import org.elasticsearch.xpack.core.security.user.User; @@ -157,12 +158,19 @@ public T executeWithAuthentication(Authentication authentication, Function consumer, Version version) { + // Preserve request headers other than authentication + final Map existingRequestHeaders = threadContext.getRequestHeadersOnly(); final StoredContext original = threadContext.newStoredContext(true); final Authentication authentication = getAuthentication(); try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { setAuthentication(new Authentication(authentication.getUser(), authentication.getAuthenticatedBy(), authentication.getLookedUpBy(), version, authentication.getAuthenticationType(), rewriteMetadataForApiKeyRoleDescriptors(version, authentication))); + existingRequestHeaders.forEach((k, v) -> { + if (false == AuthenticationField.AUTHENTICATION_KEY.equals(k)) { + threadContext.putHeader(k, v); + } + }); consumer.accept(original); } } 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 4be9bb184f0e4..d681e86239b65 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 @@ -33,6 +33,7 @@ import static org.elasticsearch.xpack.core.security.authc.Authentication.VERSION_API_KEY_ROLES_AS_BYTES; import static org.elasticsearch.xpack.core.security.authc.AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY; import static org.elasticsearch.xpack.core.security.authc.AuthenticationField.API_KEY_ROLE_DESCRIPTORS_KEY; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; public class SecurityContextTests extends ESTestCase { @@ -119,6 +120,11 @@ public void testExecuteAfterRewritingAuthentication() throws IOException { RealmRef authBy = new RealmRef("ldap", "foo", "node1"); final Authentication original = new Authentication(user, authBy, authBy); original.writeToContext(threadContext); + final Map requestHeaders = Map.of( + AuthenticationField.PRIVILEGE_CATEGORY_KEY, randomAlphaOfLengthBetween(3, 10), + randomAlphaOfLengthBetween(3, 8), randomAlphaOfLengthBetween(3, 8) + ); + threadContext.putHeader(requestHeaders); final AtomicReference contextAtomicReference = new AtomicReference<>(); securityContext.executeAfterRewritingAuthentication(originalCtx -> { @@ -129,6 +135,8 @@ public void testExecuteAfterRewritingAuthentication() throws IOException { assertEquals(VersionUtils.getPreviousVersion(), authentication.getVersion()); assertEquals(original.getAuthenticationType(), securityContext.getAuthentication().getAuthenticationType()); contextAtomicReference.set(originalCtx); + // Other request headers should be preserved + requestHeaders.forEach((k, v) -> assertThat(threadContext.getHeader(k), equalTo(v))); }, VersionUtils.getPreviousVersion()); final Authentication authAfterExecution = securityContext.getAuthentication(); diff --git a/x-pack/qa/rolling-upgrade/build.gradle b/x-pack/qa/rolling-upgrade/build.gradle index 0b47289f89d16..1ae1f7f9e1dcc 100644 --- a/x-pack/qa/rolling-upgrade/build.gradle +++ b/x-pack/qa/rolling-upgrade/build.gradle @@ -66,6 +66,11 @@ BuildParams.bwcVersions.withWireCompatiple { bwcVersion, baseName -> if (bwcVersion.onOrAfter('6.6.0')) { setting 'ccr.auto_follow.wait_for_metadata_timeout', '1s' } + if (bwcVersion.onOrAfter('7.11.0')) { + extraConfigFile 'operator_users.yml', file("${project.projectDir}/src/test/resources/operator_users.yml") + setting 'xpack.security.operator_privileges.enabled', "true" + user username: "non_operator", password: 'x-pack-test-password', role: "superuser" + } user username: "test_user", password: "x-pack-test-password" @@ -131,7 +136,8 @@ BuildParams.bwcVersions.withWireCompatiple { bwcVersion, baseName -> 'mixed_cluster/90_ml_data_frame_analytics_crud/Put an outlier_detection job on the mixed cluster', 'mixed_cluster/110_enrich/Enrich stats query smoke test for mixed cluster', 'mixed_cluster/120_api_key/Test API key authentication will work in a mixed cluster', - 'mixed_cluster/120_api_key/Create API key with metadata in a mixed cluster' + 'mixed_cluster/120_api_key/Create API key with metadata in a mixed cluster', + 'mixed_cluster/130_operator_privileges/Test operator privileges will work in the mixed cluster' ].join(',') } diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/operator_users.yml b/x-pack/qa/rolling-upgrade/src/test/resources/operator_users.yml new file mode 100644 index 0000000000000..074ca6fa4f2c4 --- /dev/null +++ b/x-pack/qa/rolling-upgrade/src/test/resources/operator_users.yml @@ -0,0 +1,2 @@ +operator: + - usernames: ["test_user"] diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/130_operator_privileges.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/130_operator_privileges.yml new file mode 100644 index 0000000000000..eafaecaa67eeb --- /dev/null +++ b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/130_operator_privileges.yml @@ -0,0 +1,17 @@ +--- +"Test operator privileges will work in the mixed cluster": + + - skip: + features: headers + version: " - 7.10.99" + reason: "operator privileges are available since 7.11" + + # The default user ("test_user") is an operator, so this works + - do: + cluster.delete_voting_config_exclusions: { } + + - do: + catch: forbidden + headers: # the non_operator user + Authorization: Basic bm9uX29wZXJhdG9yOngtcGFjay10ZXN0LXBhc3N3b3Jk + cluster.delete_voting_config_exclusions: { }