Skip to content

Commit 22cb973

Browse files
bizybotYogesh Gaikwad
authored andcommitted
[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
1 parent aaec9cd commit 22cb973

File tree

4 files changed

+23
-18
lines changed

4 files changed

+23
-18
lines changed

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/privilege/SystemPrivilege.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ public final class SystemPrivilege extends Privilege {
2323
"indices:admin/mapping/put", // needed for recovery and shrink api
2424
"indices:admin/template/put", // needed for the TemplateUpgradeService
2525
"indices:admin/template/delete", // needed for the TemplateUpgradeService
26-
"indices:admin/seq_no/global_checkpoint_sync*" // needed for global checkpoint syncs
26+
"indices:admin/seq_no/global_checkpoint_sync*", // needed for global checkpoint syncs
27+
"indices:admin/settings/update" // needed for DiskThresholdMonitor.markIndicesReadOnly
2728
), Automatons.patterns("internal:transport/proxy/*"))); // no proxy actions for system user!
2829

2930
private SystemPrivilege() {

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/privilege/PrivilegeTests.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,8 @@ public void testSystem() throws Exception {
126126
assertThat(predicate.test("indices:admin/seq_no/global_checkpoint_sync"), is(true));
127127
assertThat(predicate.test("indices:admin/seq_no/global_checkpoint_sync[p]"), is(true));
128128
assertThat(predicate.test("indices:admin/seq_no/global_checkpoint_sync[r]"), is(true));
129+
assertThat(predicate.test("indices:admin/settings/update"), is(true));
130+
assertThat(predicate.test("indices:admin/settings/foo"), is(false));
129131
}
130132

131133
public void testManageCcrPrivilege() {

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -568,9 +568,12 @@ private ElasticsearchSecurityException denialException(Authentication authentica
568568
}
569569
// check for run as
570570
if (authentication.getUser().isRunAs()) {
571+
logger.debug("action [{}] is unauthorized for user [{}] run as [{}]", action, authUser.principal(),
572+
authentication.getUser().principal());
571573
return authorizationError("action [{}] is unauthorized for user [{}] run as [{}]", action, authUser.principal(),
572574
authentication.getUser().principal());
573575
}
576+
logger.debug("action [{}] is unauthorized for user [{}]", action, authUser.principal());
574577
return authorizationError("action [{}] is unauthorized for user [{}]", action, authUser.principal());
575578
}
576579

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -239,22 +239,23 @@ private void authorize(Authentication authentication, String action, TransportRe
239239
future.actionGet();
240240
}
241241

242-
public void testActionsSystemUserIsAuthorized() {
243-
TransportRequest request = mock(TransportRequest.class);
242+
public void testActionsForSystemUserIsAuthorized() {
243+
final TransportRequest request = mock(TransportRequest.class);
244244

245245
// A failure would throw an exception
246-
Authentication authentication = createAuthentication(SystemUser.INSTANCE);
247-
authorize(authentication, "indices:monitor/whatever", request);
248-
verify(auditTrail).accessGranted(authentication, "indices:monitor/whatever", request,
249-
new String[]{SystemUser.ROLE_NAME});
250-
251-
authentication = createAuthentication(SystemUser.INSTANCE);
252-
authorize(authentication, "internal:whatever", request);
253-
verify(auditTrail).accessGranted(authentication, "internal:whatever", request, new String[]{SystemUser.ROLE_NAME});
246+
final Authentication authentication = createAuthentication(SystemUser.INSTANCE);
247+
final String[] actions = { "indices:monitor/whatever", "internal:whatever", "cluster:monitor/whatever", "cluster:admin/reroute",
248+
"indices:admin/mapping/put", "indices:admin/template/put", "indices:admin/seq_no/global_checkpoint_sync",
249+
"indices:admin/settings/update" };
250+
for (String action : actions) {
251+
authorize(authentication, action, request);
252+
verify(auditTrail).accessGranted(authentication, action, request, new String[] { SystemUser.ROLE_NAME });
253+
}
254+
254255
verifyNoMoreInteractions(auditTrail);
255256
}
256257

257-
public void testIndicesActionsAreNotAuthorized() {
258+
public void testIndicesActionsForSystemUserWhichAreNotAuthorized() {
258259
final TransportRequest request = mock(TransportRequest.class);
259260
final Authentication authentication = createAuthentication(SystemUser.INSTANCE);
260261
assertThrowsAuthorizationException(
@@ -264,25 +265,23 @@ public void testIndicesActionsAreNotAuthorized() {
264265
verifyNoMoreInteractions(auditTrail);
265266
}
266267

267-
public void testClusterAdminActionsAreNotAuthorized() {
268+
public void testClusterAdminActionsForSystemUserWhichAreNotAuthorized() {
268269
final TransportRequest request = mock(TransportRequest.class);
269270
final Authentication authentication = createAuthentication(SystemUser.INSTANCE);
270271
assertThrowsAuthorizationException(
271272
() -> authorize(authentication, "cluster:admin/whatever", request),
272273
"cluster:admin/whatever", SystemUser.INSTANCE.principal());
273-
verify(auditTrail).accessDenied(authentication, "cluster:admin/whatever", request,
274-
new String[]{SystemUser.ROLE_NAME});
274+
verify(auditTrail).accessDenied(authentication, "cluster:admin/whatever", request, new String[] { SystemUser.ROLE_NAME });
275275
verifyNoMoreInteractions(auditTrail);
276276
}
277277

278-
public void testClusterAdminSnapshotStatusActionIsNotAuthorized() {
278+
public void testClusterAdminSnapshotStatusActionForSystemUserWhichIsNotAuthorized() {
279279
final TransportRequest request = mock(TransportRequest.class);
280280
final Authentication authentication = createAuthentication(SystemUser.INSTANCE);
281281
assertThrowsAuthorizationException(
282282
() -> authorize(authentication, "cluster:admin/snapshot/status", request),
283283
"cluster:admin/snapshot/status", SystemUser.INSTANCE.principal());
284-
verify(auditTrail).accessDenied(authentication, "cluster:admin/snapshot/status", request,
285-
new String[]{SystemUser.ROLE_NAME});
284+
verify(auditTrail).accessDenied(authentication, "cluster:admin/snapshot/status", request, new String[] { SystemUser.ROLE_NAME });
286285
verifyNoMoreInteractions(auditTrail);
287286
}
288287

0 commit comments

Comments
 (0)