[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
This commit is contained in:
Yogesh Gaikwad 2018-10-04 11:31:51 +10:00 committed by GitHub
parent 6dd716b0c4
commit 81227dc389
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 22 additions and 17 deletions

View File

@ -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() {

View File

@ -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() {

View File

@ -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());
}

View File

@ -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);
}