From d7517d0d0e1cc111772b34a507371b580894dd58 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Tue, 19 Oct 2021 12:03:32 +1100 Subject: [PATCH 1/4] wip --- x-pack/qa/rolling-upgrade/build.gradle | 5 +++++ .../src/test/resources/operator_users.yml | 2 ++ .../mixed_cluster/130_operator_privileges.yml | 16 ++++++++++++++++ .../old_cluster/130_operator_privileges.yml | 17 +++++++++++++++++ 4 files changed, 40 insertions(+) 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 create mode 100644 x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/130_operator_privileges.yml diff --git a/x-pack/qa/rolling-upgrade/build.gradle b/x-pack/qa/rolling-upgrade/build.gradle index 0b47289f89d16..306b87a2d76d7 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" 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..dbba6c09124bb --- /dev/null +++ b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/mixed_cluster/130_operator_privileges.yml @@ -0,0 +1,16 @@ +--- +"Test operator pivileges will work in the mixed cluster": + + - skip: + features: headers + version: " - 7.10.99" + reason: "operator privileges are available since 7.11" + + - do: + cluster.delete_voting_config_exclusions: { } + +# - do: +# catch: forbidden +# headers: # the non_operator user +# Authorization: Basic bm9uX29wZXJhdG9yOngtcGFjay10ZXN0LXBhc3N3b3Jk +# cluster.delete_voting_config_exclusions: { } diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/130_operator_privileges.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/130_operator_privileges.yml new file mode 100644 index 0000000000000..c39c61dd69be6 --- /dev/null +++ b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/130_operator_privileges.yml @@ -0,0 +1,17 @@ +--- +"Test operator pivileges will work in the old cluster": + + - skip: + features: headers + version: " - 7.10.99" + reason: "operator privileges are available since 7.11" + + - do: + cluster.delete_voting_config_exclusions: { } +# +# - do: +# catch: forbidden +# headers: # the non_operator user +# Authorization: Basic bm9uX29wZXJhdG9yOngtcGFjay10ZXN0LXBhc3N3b3Jk +# cluster.delete_voting_config_exclusions: {} + From b8fac3a86723515c972f11729614c271e4b35fb9 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Tue, 19 Oct 2021 12:46:47 +1100 Subject: [PATCH 2/4] Preserve request headers in a mixed version cluster 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 | 9 +++++++++ x-pack/qa/rolling-upgrade/build.gradle | 5 +++-- .../mixed_cluster/130_operator_privileges.yml | 10 +++++----- .../old_cluster/130_operator_privileges.yml | 17 ----------------- 5 files changed, 25 insertions(+), 24 deletions(-) delete mode 100644 x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_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..00aba9f426a2a 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 @@ -21,6 +21,7 @@ import org.elasticsearch.xpack.core.security.authc.AuthenticationField; import org.elasticsearch.xpack.core.security.user.SystemUser; import org.elasticsearch.xpack.core.security.user.User; +import org.elasticsearch.xpack.security.audit.AuditUtil; import org.junit.Before; import java.io.EOFException; @@ -33,6 +34,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 +121,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 +136,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 306b87a2d76d7..4589ece357e7b 100644 --- a/x-pack/qa/rolling-upgrade/build.gradle +++ b/x-pack/qa/rolling-upgrade/build.gradle @@ -68,7 +68,7 @@ BuildParams.bwcVersions.withWireCompatiple { bwcVersion, baseName -> } 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" + setting 'xpack.security.operator_privileges.enabled', "true" user username: "non_operator", password: 'x-pack-test-password', role: "superuser" } @@ -136,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 pivileges will work in the mixed cluster' ].join(',') } 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 index dbba6c09124bb..d1f0602c2e5f5 100644 --- 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 @@ -9,8 +9,8 @@ - do: cluster.delete_voting_config_exclusions: { } -# - do: -# catch: forbidden -# headers: # the non_operator user -# Authorization: Basic bm9uX29wZXJhdG9yOngtcGFjay10ZXN0LXBhc3N3b3Jk -# cluster.delete_voting_config_exclusions: { } + - do: + catch: forbidden + headers: # the non_operator user + Authorization: Basic bm9uX29wZXJhdG9yOngtcGFjay10ZXN0LXBhc3N3b3Jk + cluster.delete_voting_config_exclusions: { } diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/130_operator_privileges.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/130_operator_privileges.yml deleted file mode 100644 index c39c61dd69be6..0000000000000 --- a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/old_cluster/130_operator_privileges.yml +++ /dev/null @@ -1,17 +0,0 @@ ---- -"Test operator pivileges will work in the old cluster": - - - skip: - features: headers - version: " - 7.10.99" - reason: "operator privileges are available since 7.11" - - - do: - cluster.delete_voting_config_exclusions: { } -# -# - do: -# catch: forbidden -# headers: # the non_operator user -# Authorization: Basic bm9uX29wZXJhdG9yOngtcGFjay10ZXN0LXBhc3N3b3Jk -# cluster.delete_voting_config_exclusions: {} - From e18a76debd8a9950dd137d4c828b4b492e5b5865 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Tue, 19 Oct 2021 13:39:52 +1100 Subject: [PATCH 3/4] checkstyle --- .../org/elasticsearch/xpack/security/SecurityContextTests.java | 1 - 1 file changed, 1 deletion(-) 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 00aba9f426a2a..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 @@ -21,7 +21,6 @@ import org.elasticsearch.xpack.core.security.authc.AuthenticationField; import org.elasticsearch.xpack.core.security.user.SystemUser; import org.elasticsearch.xpack.core.security.user.User; -import org.elasticsearch.xpack.security.audit.AuditUtil; import org.junit.Before; import java.io.EOFException; From 5cf0bf0d5a4602fe7042503774f7d2d4b82c8793 Mon Sep 17 00:00:00 2001 From: Yang Wang Date: Tue, 19 Oct 2021 14:53:12 +1100 Subject: [PATCH 4/4] Apply suggestions from code review Co-authored-by: Tim Vernum --- x-pack/qa/rolling-upgrade/build.gradle | 2 +- .../test/mixed_cluster/130_operator_privileges.yml | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/x-pack/qa/rolling-upgrade/build.gradle b/x-pack/qa/rolling-upgrade/build.gradle index 4589ece357e7b..1ae1f7f9e1dcc 100644 --- a/x-pack/qa/rolling-upgrade/build.gradle +++ b/x-pack/qa/rolling-upgrade/build.gradle @@ -137,7 +137,7 @@ BuildParams.bwcVersions.withWireCompatiple { bwcVersion, baseName -> '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/130_operator_privileges/Test operator pivileges will work in the 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/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 index d1f0602c2e5f5..eafaecaa67eeb 100644 --- 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 @@ -1,11 +1,12 @@ --- -"Test operator pivileges will work in the mixed cluster": +"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: { }