From 6d535043f860687b08e2d13f3a2fb9b0dee02cbf Mon Sep 17 00:00:00 2001 From: Karel Minarik Date: Wed, 29 Jun 2016 17:09:48 +0200 Subject: [PATCH 1/4] [SECURITY] Added a `teardown` section to the integration test for roles Currently, the REST tests for security (and possibly others) don't clean up the environment after they have run, eg. they don't delete the users and roles they create. This leads to test failures, because in a subsequent run, a user or role already exists, so eg. a test like `match: { role: { created: true } }` fails. This patch adds a `teardown` section to the test, with `do` actions which are to be executed _after_ the test runs. This patch assumes that REST tests runners for all languages support the `teardown` directive in a xUnimt nomenclature -- similarly to the `setup` directive, which they already support. Original commit: elastic/x-pack-elasticsearch@70d0ff4ee98cdd1e7e17fe6bf832f8b51a802e96 --- .../test/roles/11_idx_arrays.yaml | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/roles/11_idx_arrays.yaml b/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/roles/11_idx_arrays.yaml index aeda2d10749..10909bfb3ab 100644 --- a/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/roles/11_idx_arrays.yaml +++ b/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/roles/11_idx_arrays.yaml @@ -3,9 +3,28 @@ - skip: features: headers - - do: - cluster.health: - wait_for_status: yellow + - setup: + - do: + cluster.health: + wait_for_status: yellow + + - teardown: + - do: + xpack.security.delete_role: + name: "admin_role2" + ignore: 404 + + - do: + xpack.security.delete_user: + username: "joe" + ignore: 404 + + - do: + delete: + index: foo + type: doc + id: 1 + ignore: 404 - do: xpack.security.put_role: @@ -50,7 +69,6 @@ - match: { _index: foo } - match: { _type: doc } - match: { _id: "1"} - - match: { _version: 1} - match: { _source: { foo: bar }} # test that the role works on the cluster level From 67706a9a190defe4abe4d57e82dd5ef100feef41 Mon Sep 17 00:00:00 2001 From: Karel Minarik Date: Thu, 30 Jun 2016 11:22:38 +0200 Subject: [PATCH 2/4] [SECURITY] Changed the setup/teardown YAML structure in the integration test for roles Related: 176fd6a Original commit: elastic/x-pack-elasticsearch@90e210dbc29527c5260f3ededa2f2c59408f997a --- .../test/roles/11_idx_arrays.yaml | 47 ++++++++++--------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/roles/11_idx_arrays.yaml b/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/roles/11_idx_arrays.yaml index 10909bfb3ab..da66d7ffebb 100644 --- a/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/roles/11_idx_arrays.yaml +++ b/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/roles/11_idx_arrays.yaml @@ -1,31 +1,32 @@ +--- +setup: + - do: + cluster.health: + wait_for_status: yellow + +teardown: + - do: + xpack.security.delete_role: + name: "admin_role2" + ignore: 404 + + - do: + xpack.security.delete_user: + username: "joe" + ignore: 404 + + - do: + delete: + index: foo + type: doc + id: 1 + ignore: 404 + --- "Test put role api using as array of index names": - skip: features: headers - - setup: - - do: - cluster.health: - wait_for_status: yellow - - - teardown: - - do: - xpack.security.delete_role: - name: "admin_role2" - ignore: 404 - - - do: - xpack.security.delete_user: - username: "joe" - ignore: 404 - - - do: - delete: - index: foo - type: doc - id: 1 - ignore: 404 - - do: xpack.security.put_role: name: "admin_role2" From d1b945d1f2abe38fab0cc1a00bd7fc1850437337 Mon Sep 17 00:00:00 2001 From: jaymode Date: Thu, 30 Jun 2016 11:14:37 -0400 Subject: [PATCH 3/4] security: remove user/role deletion logic in XPackRestTestCase Original commit: elastic/x-pack-elasticsearch@d6064e520a31465a2d37b5ceb470eace89a8794f --- .../test/license/20_put_license.yaml | 5 + .../test/authenticate/10_basic.yaml | 8 ++ .../test/change_password/10_basic.yaml | 131 +++++++----------- .../rest-api-spec/test/roles/10_basic.yaml | 39 ++++-- .../test/roles/11_idx_arrays.yaml | 5 +- .../rest-api-spec/test/users/10_basic.yaml | 13 +- .../test/users/15_overwrite_user.yaml | 22 ++- .../test/users/16_update_user.yaml | 46 +++--- .../xpack/test/rest/XPackRestTestCase.java | 23 --- 9 files changed, 145 insertions(+), 147 deletions(-) diff --git a/elasticsearch/x-pack/license-plugin/src/test/resources/rest-api-spec/test/license/20_put_license.yaml b/elasticsearch/x-pack/license-plugin/src/test/resources/rest-api-spec/test/license/20_put_license.yaml index 21af326eeeb..2e11ee0e2fa 100644 --- a/elasticsearch/x-pack/license-plugin/src/test/resources/rest-api-spec/test/license/20_put_license.yaml +++ b/elasticsearch/x-pack/license-plugin/src/test/resources/rest-api-spec/test/license/20_put_license.yaml @@ -1,3 +1,8 @@ +--- +teardown: + - do: + license.delete: {} + --- "installing and getting license works": diff --git a/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/authenticate/10_basic.yaml b/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/authenticate/10_basic.yaml index afefac0f844..f9caca85123 100644 --- a/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/authenticate/10_basic.yaml +++ b/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/authenticate/10_basic.yaml @@ -1,3 +1,4 @@ +--- setup: - skip: features: headers @@ -16,6 +17,13 @@ setup: "full_name" : "Authenticate User" } +--- +teardown: + - do: + xpack.security.delete_user: + username: "authenticate_user" + ignore: 404 + --- "Test authenticate api": diff --git a/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/change_password/10_basic.yaml b/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/change_password/10_basic.yaml index 9f83345d7b6..a2cc4430a5c 100644 --- a/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/change_password/10_basic.yaml +++ b/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/change_password/10_basic.yaml @@ -1,23 +1,58 @@ --- -"Test changing users password": +setup: - skip: features: headers - - do: cluster.health: wait_for_status: yellow - - do: xpack.security.put_user: username: "joe" body: > - { - "password": "s3krit", - "roles" : [ "superuser" ] - } - - match: { user: { created: true } } + { + "password": "s3krit", + "roles" : [ "superuser" ] + } + - do: + xpack.security.put_role: + name: "user" + body: > + { + "cluster": ["monitor"], + "indices": [ + { + "names": "*", + "privileges": ["all"] + } + ] + } + - do: + xpack.security.put_user: + username: "unprivileged_user" + body: > + { + "password": "s3krit", + "roles" : [ "user" ] + } -# test that the role actually works +--- +teardown: + - do: + xpack.security.delete_user: + username: "joe" + ignore: 404 + - do: + xpack.security.delete_role: + name: "user" + ignore: 404 + - do: + xpack.security.delete_user: + username: "unprivileged_user" + ignore: 404 + +--- +"Test changing users password": +# validate that the user actually works - do: headers: Authorization: "Basic am9lOnMza3JpdA==" @@ -49,49 +84,17 @@ --- "Test user changing their own password": - - skip: - features: headers - - - do: - cluster.health: - wait_for_status: yellow - - - do: - xpack.security.put_role: - name: "user" - body: > - { - "cluster": ["monitor"], - "indices": [ - { - "names": "*", - "privileges": ["all"] - } - ] - } - - match: { role: { created: true } } - - - do: - xpack.security.put_user: - username: "joe" - body: > - { - "password": "s3krit", - "roles" : [ "user" ] - } - - match: { user: { created: true } } - # test that the role actually works - do: headers: - Authorization: "Basic am9lOnMza3JpdA==" + Authorization: "Basic dW5wcml2aWxlZ2VkX3VzZXI6czNrcml0" cluster.health: {} - match: { timed_out: false } -# change password as the current user. the power_user role only grants the ability to change their own password +# change password as the current user. the user role only grants the ability to change their own password - do: headers: - Authorization: "Basic am9lOnMza3JpdA==" + Authorization: "Basic dW5wcml2aWxlZ2VkX3VzZXI6czNrcml0" xpack.security.change_password: body: > { @@ -102,61 +105,29 @@ - do: catch: request headers: - Authorization: "Basic am9lOnMza3JpdA==" + Authorization: "Basic dW5wcml2aWxlZ2VkX3VzZXI6czNrcml0" cluster.health: {} # login with new credentials - do: headers: - Authorization: "Basic am9lOnMza3JpdDI=" + Authorization: "Basic dW5wcml2aWxlZ2VkX3VzZXI6czNrcml0Mg==" cluster.health: {} - match: { timed_out: false } --- "Test unauthorized user changing anothers password": - - skip: - features: headers - - - do: - cluster.health: - wait_for_status: yellow - - - do: - xpack.security.put_role: - name: "user" - body: > - { - "cluster": ["monitor"], - "indices": [ - { - "names": "*", - "privileges": ["all"] - } - ] - } - - match: { role: { created: true } } - - - do: - xpack.security.put_user: - username: "joe" - body: > - { - "password": "s3krit", - "roles" : [ "user" ] - } - - match: { user: { created: true } } - # test that the role actually works - do: headers: - Authorization: "Basic am9lOnMza3JpdA==" + Authorization: "Basic dW5wcml2aWxlZ2VkX3VzZXI6czNrcml0" cluster.health: {} - match: { timed_out: false } # attempt to change another users password - do: headers: - Authorization: "Basic am9lOnMza3JpdA==" + Authorization: "Basic dW5wcml2aWxlZ2VkX3VzZXI6czNrcml0" catch: forbidden xpack.security.change_password: username: "anotheruser" diff --git a/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/roles/10_basic.yaml b/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/roles/10_basic.yaml index 9e5898103c6..21498c69b0d 100644 --- a/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/roles/10_basic.yaml +++ b/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/roles/10_basic.yaml @@ -1,12 +1,37 @@ --- -"Test put role api": +setup: - skip: features: headers - do: cluster.health: - wait_for_status: yellow + wait_for_status: yellow + - do: + xpack.security.put_user: + username: "joe" + body: > + { + "password": "s3krit", + "roles" : [ "admin_role" ] + } +--- +teardown: + - do: + xpack.security.delete_user: + username: "joe" + ignore: 404 + - do: + xpack.security.delete_role: + name: "admin_role" + ignore: 404 + - do: + xpack.security.delete_role: + name: "backwards_role" + ignore: 404 + +--- +"Test put role api": - do: xpack.security.put_role: name: "admin_role" @@ -37,16 +62,6 @@ } - match: { role: { created: true } } - - do: - xpack.security.put_user: - username: "joe" - body: > - { - "password": "s3krit", - "roles" : [ "admin_role" ] - } - - match: { user: { created: true } } - # test that the role actually works - do: headers: diff --git a/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/roles/11_idx_arrays.yaml b/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/roles/11_idx_arrays.yaml index da66d7ffebb..c9743a005e8 100644 --- a/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/roles/11_idx_arrays.yaml +++ b/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/roles/11_idx_arrays.yaml @@ -1,9 +1,12 @@ --- setup: + - skip: + features: headers - do: cluster.health: wait_for_status: yellow +--- teardown: - do: xpack.security.delete_role: @@ -24,8 +27,6 @@ teardown: --- "Test put role api using as array of index names": - - skip: - features: headers - do: xpack.security.put_role: diff --git a/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/users/10_basic.yaml b/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/users/10_basic.yaml index 309b9d693fd..1e18f51ac13 100644 --- a/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/users/10_basic.yaml +++ b/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/users/10_basic.yaml @@ -1,12 +1,21 @@ --- -"Test put user api": +setup: - skip: features: headers - do: cluster.health: - wait_for_status: yellow + wait_for_status: yellow +--- +teardown: + - do: + xpack.security.delete_user: + username: "joe" + ignore: 404 + +--- +"Test put user api": - do: xpack.security.put_user: username: "joe" diff --git a/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/users/15_overwrite_user.yaml b/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/users/15_overwrite_user.yaml index 16eb52fbfcc..66bcc9d1c5a 100644 --- a/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/users/15_overwrite_user.yaml +++ b/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/users/15_overwrite_user.yaml @@ -1,22 +1,30 @@ --- -"Test overwriting a user": +setup: - skip: features: headers - do: cluster.health: - wait_for_status: yellow + wait_for_status: yellow - do: xpack.security.put_user: username: "joe" body: > - { - "password": "s3krit", - "roles" : [ "superuser" ] - } - - match: { user: { created: true } } + { + "password": "s3krit", + "roles" : [ "superuser" ] + } +--- +teardown: + - do: + xpack.security.delete_user: + username: "joe" + ignore: 404 + +--- +"Test overwriting a user": - do: xpack.security.get_user: username: "joe" diff --git a/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/users/16_update_user.yaml b/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/users/16_update_user.yaml index 71cba78bf22..92c6d3fea1c 100644 --- a/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/users/16_update_user.yaml +++ b/elasticsearch/x-pack/security/src/test/resources/rest-api-spec/test/users/16_update_user.yaml @@ -1,24 +1,5 @@ --- -"Test creating a user without password": - - do: - cluster.health: - wait_for_status: yellow - - - do: - catch: request - xpack.security.put_user: - username: "joe" - body: > - { - "roles" : [ "superuser" ] - } - - match: { error.root_cause.0.reason: 'Validation Failed: 1: password must be specified unless you are updating an existing user;' } - ---- -"Test create user and update without and with password": - - skip: - features: headers - +setup: - do: cluster.health: wait_for_status: yellow @@ -31,7 +12,30 @@ "password": "s3krit", "roles" : [ "superuser" ] } - - match: { user: { created: true } } + +--- +teardown: + - do: + xpack.security.delete_user: + username: "joe" + ignore: 404 + +--- +"Test creating a user without password": + - do: + catch: request + xpack.security.put_user: + username: "no_password_user" + body: > + { + "roles" : [ "superuser" ] + } + - match: { error.root_cause.0.reason: 'Validation Failed: 1: password must be specified unless you are updating an existing user;' } + +--- +"Test create user and update without and with password": + - skip: + features: headers # test that the role actually works - do: diff --git a/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/test/rest/XPackRestTestCase.java b/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/test/rest/XPackRestTestCase.java index a02d6307bd9..ddcd5479716 100644 --- a/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/test/rest/XPackRestTestCase.java +++ b/elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/test/rest/XPackRestTestCase.java @@ -63,29 +63,6 @@ public abstract class XPackRestTestCase extends ESRestTestCase { } } - @After - public void clearUsersAndRoles() throws Exception { - // we cannot delete the .security index from a rest test since we aren't the internal user, lets wipe the data - // TODO remove this once the built-in SUPERUSER role is added that can delete the index and we use the built in admin user here - RestTestResponse response = getAdminExecutionContext().callApi("xpack.security.get_user", emptyMap(), emptyList(), emptyMap()); - @SuppressWarnings("unchecked") - Map users = (Map) response.getBody(); - for (String user: users.keySet()) { - if (ReservedRealm.isReserved(user) == false) { - getAdminExecutionContext().callApi("xpack.security.delete_user", singletonMap("username", user), emptyList(), emptyMap()); - } - } - - response = getAdminExecutionContext().callApi("xpack.security.get_role", emptyMap(), emptyList(), emptyMap()); - @SuppressWarnings("unchecked") - Map roles = (Map) response.getBody(); - for (String role: roles.keySet()) { - if (ReservedRolesStore.isReserved(role) == false) { - getAdminExecutionContext().callApi("xpack.security.delete_role", singletonMap("name", role), emptyList(), emptyMap()); - } - } - } - @Override protected Settings restClientSettings() { return Settings.builder() From 4999f3a38c3b87c909ae9b8f87b8ec30fb61d863 Mon Sep 17 00:00:00 2001 From: jaymode Date: Thu, 30 Jun 2016 11:39:37 -0400 Subject: [PATCH 4/4] test: remove unnecessary teardown section in license test Original commit: elastic/x-pack-elasticsearch@b457404452afbb8216d56fb4f890862631edaf27 --- .../resources/rest-api-spec/test/license/20_put_license.yaml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/elasticsearch/x-pack/license-plugin/src/test/resources/rest-api-spec/test/license/20_put_license.yaml b/elasticsearch/x-pack/license-plugin/src/test/resources/rest-api-spec/test/license/20_put_license.yaml index 2e11ee0e2fa..21af326eeeb 100644 --- a/elasticsearch/x-pack/license-plugin/src/test/resources/rest-api-spec/test/license/20_put_license.yaml +++ b/elasticsearch/x-pack/license-plugin/src/test/resources/rest-api-spec/test/license/20_put_license.yaml @@ -1,8 +1,3 @@ ---- -teardown: - - do: - license.delete: {} - --- "installing and getting license works":