From 81227dc3892de76b2c22781a57865bef6028a959 Mon Sep 17 00:00:00 2001 From: Yogesh Gaikwad <902768+bizybot@users.noreply.github.com> Date: Thu, 4 Oct 2018 11:31:51 +1000 Subject: [PATCH] [Authz] Allow update settings action for system user (#34030) When the cluster.routing.allocation.disk.watermark.flood_stage watermark is breached, DiskThresholdMonitor marks the indices as read-only. This failed when x-pack security was present as system user does not have the privilege for update settings action("indices:admin/settings/update"). This commit adds the required privilege for the system user. Also added missing debug logs when access is denied to help future debugging. An assert statement is added to catch any missed privileges required for system user. Closes #33119 --- .../authz/privilege/SystemPrivilege.java | 3 +- .../authz/privilege/PrivilegeTests.java | 2 ++ .../security/authz/AuthorizationService.java | 3 ++ .../authz/AuthorizationServiceTests.java | 31 +++++++++---------- 4 files changed, 22 insertions(+), 17 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/SystemPrivilege.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/SystemPrivilege.java index f1527429b32..d5a5d04dded 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/SystemPrivilege.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/SystemPrivilege.java @@ -23,7 +23,8 @@ public final class SystemPrivilege extends Privilege { "indices:admin/mapping/put", // needed for recovery and shrink api "indices:admin/template/put", // needed for the TemplateUpgradeService "indices:admin/template/delete", // needed for the TemplateUpgradeService - "indices:admin/seq_no/global_checkpoint_sync*" // needed for global checkpoint syncs + "indices:admin/seq_no/global_checkpoint_sync*", // needed for global checkpoint syncs + "indices:admin/settings/update" // needed for DiskThresholdMonitor.markIndicesReadOnly ), Automatons.patterns("internal:transport/proxy/*"))); // no proxy actions for system user! private SystemPrivilege() { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/PrivilegeTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/PrivilegeTests.java index c4c95211d4c..f2bfd8d2d8e 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/PrivilegeTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/PrivilegeTests.java @@ -126,6 +126,8 @@ public class PrivilegeTests extends ESTestCase { assertThat(predicate.test("indices:admin/seq_no/global_checkpoint_sync"), is(true)); assertThat(predicate.test("indices:admin/seq_no/global_checkpoint_sync[p]"), is(true)); assertThat(predicate.test("indices:admin/seq_no/global_checkpoint_sync[r]"), is(true)); + assertThat(predicate.test("indices:admin/settings/update"), is(true)); + assertThat(predicate.test("indices:admin/settings/foo"), is(false)); } public void testManageCcrPrivilege() { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java index 642bc167f7d..f9fe2b7eaa7 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java @@ -568,9 +568,12 @@ public class AuthorizationService extends AbstractComponent { } // check for run as if (authentication.getUser().isRunAs()) { + logger.debug("action [{}] is unauthorized for user [{}] run as [{}]", action, authUser.principal(), + authentication.getUser().principal()); return authorizationError("action [{}] is unauthorized for user [{}] run as [{}]", action, authUser.principal(), authentication.getUser().principal()); } + logger.debug("action [{}] is unauthorized for user [{}]", action, authUser.principal()); return authorizationError("action [{}] is unauthorized for user [{}]", action, authUser.principal()); } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java index 8ccac83c86f..47cf458e19a 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java @@ -240,22 +240,23 @@ public class AuthorizationServiceTests extends ESTestCase { future.actionGet(); } - public void testActionsSystemUserIsAuthorized() { - TransportRequest request = mock(TransportRequest.class); + public void testActionsForSystemUserIsAuthorized() { + final TransportRequest request = mock(TransportRequest.class); // A failure would throw an exception - Authentication authentication = createAuthentication(SystemUser.INSTANCE); - authorize(authentication, "indices:monitor/whatever", request); - verify(auditTrail).accessGranted(authentication, "indices:monitor/whatever", request, - new String[]{SystemUser.ROLE_NAME}); + final Authentication authentication = createAuthentication(SystemUser.INSTANCE); + final String[] actions = { "indices:monitor/whatever", "internal:whatever", "cluster:monitor/whatever", "cluster:admin/reroute", + "indices:admin/mapping/put", "indices:admin/template/put", "indices:admin/seq_no/global_checkpoint_sync", + "indices:admin/settings/update" }; + for (String action : actions) { + authorize(authentication, action, request); + verify(auditTrail).accessGranted(authentication, action, request, new String[] { SystemUser.ROLE_NAME }); + } - authentication = createAuthentication(SystemUser.INSTANCE); - authorize(authentication, "internal:whatever", request); - verify(auditTrail).accessGranted(authentication, "internal:whatever", request, new String[]{SystemUser.ROLE_NAME}); verifyNoMoreInteractions(auditTrail); } - public void testIndicesActionsAreNotAuthorized() { + public void testIndicesActionsForSystemUserWhichAreNotAuthorized() { final TransportRequest request = mock(TransportRequest.class); final Authentication authentication = createAuthentication(SystemUser.INSTANCE); assertThrowsAuthorizationException( @@ -265,25 +266,23 @@ public class AuthorizationServiceTests extends ESTestCase { verifyNoMoreInteractions(auditTrail); } - public void testClusterAdminActionsAreNotAuthorized() { + public void testClusterAdminActionsForSystemUserWhichAreNotAuthorized() { final TransportRequest request = mock(TransportRequest.class); final Authentication authentication = createAuthentication(SystemUser.INSTANCE); assertThrowsAuthorizationException( () -> authorize(authentication, "cluster:admin/whatever", request), "cluster:admin/whatever", SystemUser.INSTANCE.principal()); - verify(auditTrail).accessDenied(authentication, "cluster:admin/whatever", request, - new String[]{SystemUser.ROLE_NAME}); + verify(auditTrail).accessDenied(authentication, "cluster:admin/whatever", request, new String[] { SystemUser.ROLE_NAME }); verifyNoMoreInteractions(auditTrail); } - public void testClusterAdminSnapshotStatusActionIsNotAuthorized() { + public void testClusterAdminSnapshotStatusActionForSystemUserWhichIsNotAuthorized() { final TransportRequest request = mock(TransportRequest.class); final Authentication authentication = createAuthentication(SystemUser.INSTANCE); assertThrowsAuthorizationException( () -> authorize(authentication, "cluster:admin/snapshot/status", request), "cluster:admin/snapshot/status", SystemUser.INSTANCE.principal()); - verify(auditTrail).accessDenied(authentication, "cluster:admin/snapshot/status", request, - new String[]{SystemUser.ROLE_NAME}); + verify(auditTrail).accessDenied(authentication, "cluster:admin/snapshot/status", request, new String[] { SystemUser.ROLE_NAME }); verifyNoMoreInteractions(auditTrail); }