Skip to content

Commit 13946cd

Browse files
committed
Ignore app priv failures when resolving superuser
In elastic#81400 we changed `superuser` to no longer have _every_ privilege. Consequently, we also removed the special case code that existed that would ignore all other roles for any user that had superuser role. However, we added some special handling so that failing to resolve those other roles would not block superuser access - when a user has superuser role, any failures in role resolution will be effectively ignored, and the user will be given the superuser role only. However, this failure handling did not account for the loading of application privileges. If application privileges needed to be loaded, but failed, this could prevent resolution of the superuser role. This change extends the failure handling to encompass the full resolution of roles, and fallback to superuser only if other roles or application privileges are unavailable
1 parent af35d04 commit 13946cd

File tree

2 files changed

+82
-25
lines changed

2 files changed

+82
-25
lines changed

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

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -240,22 +240,7 @@ public void buildRoleFromRoleReference(RoleReference roleReference, ActionListen
240240
final Role existing = roleCache.get(roleKey);
241241
if (existing == null) {
242242
final long invalidationCounter = numInvalidation.get();
243-
roleReference.resolve(roleReferenceResolver, ActionListener.wrap(rolesRetrievalResult -> {
244-
if (RolesRetrievalResult.EMPTY == rolesRetrievalResult) {
245-
roleActionListener.onResponse(Role.EMPTY);
246-
} else if (RolesRetrievalResult.SUPERUSER == rolesRetrievalResult) {
247-
roleActionListener.onResponse(superuserRole);
248-
} else {
249-
buildThenMaybeCacheRole(
250-
roleKey,
251-
rolesRetrievalResult.getRoleDescriptors(),
252-
rolesRetrievalResult.getMissingRoles(),
253-
rolesRetrievalResult.isSuccess(),
254-
invalidationCounter,
255-
roleActionListener
256-
);
257-
}
258-
}, e -> {
243+
final Consumer<Exception> failureHandler = e -> {
259244
// Because superuser does not have write access to restricted indices, it is valid to mix superuser with other roles to
260245
// gain addition access. However, if retrieving those roles fails for some reason, then that could leave admins in a
261246
// situation where they are unable to administer their cluster (in order to resolve the problem that is leading to failures
@@ -274,7 +259,23 @@ public void buildRoleFromRoleReference(RoleReference roleReference, ActionListen
274259
} else {
275260
roleActionListener.onFailure(e);
276261
}
277-
}));
262+
};
263+
roleReference.resolve(roleReferenceResolver, ActionListener.wrap(rolesRetrievalResult -> {
264+
if (RolesRetrievalResult.EMPTY == rolesRetrievalResult) {
265+
roleActionListener.onResponse(Role.EMPTY);
266+
} else if (RolesRetrievalResult.SUPERUSER == rolesRetrievalResult) {
267+
roleActionListener.onResponse(superuserRole);
268+
} else {
269+
buildThenMaybeCacheRole(
270+
roleKey,
271+
rolesRetrievalResult.getRoleDescriptors(),
272+
rolesRetrievalResult.getMissingRoles(),
273+
rolesRetrievalResult.isSuccess(),
274+
invalidationCounter,
275+
ActionListener.wrap(roleActionListener::onResponse, failureHandler)
276+
);
277+
}
278+
}, failureHandler));
278279
} else {
279280
roleActionListener.onResponse(existing);
280281
}

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

Lines changed: 64 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@
8888
import org.elasticsearch.xpack.security.authc.service.ServiceAccountService;
8989
import org.elasticsearch.xpack.security.support.CacheInvalidatorRegistry;
9090
import org.elasticsearch.xpack.security.support.SecurityIndexManager;
91+
import org.hamcrest.Matcher;
9192
import org.hamcrest.Matchers;
9293

9394
import java.io.IOException;
@@ -114,6 +115,7 @@
114115
import java.util.function.Predicate;
115116

116117
import static org.elasticsearch.test.ActionListenerUtils.anyActionListener;
118+
import static org.elasticsearch.test.TestMatchers.throwableWithMessage;
117119
import static org.elasticsearch.xpack.core.security.SecurityField.DOCUMENT_LEVEL_SECURITY_FEATURE;
118120
import static org.elasticsearch.xpack.core.security.authc.AuthenticationField.API_KEY_ID_KEY;
119121
import static org.elasticsearch.xpack.core.security.authc.AuthenticationField.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY;
@@ -320,6 +322,55 @@ public void testRolesWhenDlsFlsLicensed() throws IOException {
320322
}
321323

322324
public void testSuperuserIsEffectiveWhenOtherRolesUnavailable() {
325+
final Consumer<ActionListener<RoleRetrievalResult>> rolesHandler = callback -> {
326+
final RuntimeException exception = new RuntimeException("Test(" + getTestName() + ") - native roles unavailable");
327+
if (randomBoolean()) {
328+
callback.onFailure(exception);
329+
} else {
330+
callback.onResponse(RoleRetrievalResult.failure(exception));
331+
}
332+
};
333+
final Consumer<ActionListener<Collection<ApplicationPrivilegeDescriptor>>> privilegesHandler = callback -> callback.onResponse(
334+
Collections.emptyList()
335+
);
336+
337+
final CompositeRolesStore compositeRolesStore = setupRolesStore(rolesHandler, privilegesHandler);
338+
trySuccessfullyLoadSuperuserRole(compositeRolesStore);
339+
tryFailOnNonSuperuserRole(compositeRolesStore, throwableWithMessage(containsString("native roles unavailable")));
340+
}
341+
342+
public void testSuperuserIsEffectiveWhenApplicationPrivilegesAreUnavailable() {
343+
final RoleDescriptor role = new RoleDescriptor(
344+
"_mock_role",
345+
new String[0],
346+
new IndicesPrivileges[0],
347+
new RoleDescriptor.ApplicationResourcePrivileges[] {
348+
RoleDescriptor.ApplicationResourcePrivileges.builder()
349+
.application(randomAlphaOfLengthBetween(5, 12))
350+
.privileges("all")
351+
.resources("*")
352+
.build() },
353+
new ConfigurableClusterPrivilege[0],
354+
new String[0],
355+
Map.of(),
356+
Map.of()
357+
);
358+
final Consumer<ActionListener<RoleRetrievalResult>> rolesHandler = callback -> callback.onResponse(
359+
RoleRetrievalResult.success(Set.of(role))
360+
);
361+
final Consumer<ActionListener<Collection<ApplicationPrivilegeDescriptor>>> privilegesHandler = callback -> callback.onFailure(
362+
new RuntimeException("No privileges for you!")
363+
);
364+
365+
final CompositeRolesStore compositeRolesStore = setupRolesStore(rolesHandler, privilegesHandler);
366+
trySuccessfullyLoadSuperuserRole(compositeRolesStore);
367+
tryFailOnNonSuperuserRole(compositeRolesStore, throwableWithMessage(containsString("No privileges for you!")));
368+
}
369+
370+
private CompositeRolesStore setupRolesStore(
371+
Consumer<ActionListener<RoleRetrievalResult>> rolesHandler,
372+
Consumer<ActionListener<Collection<ApplicationPrivilegeDescriptor>>> privilegesHandler
373+
) {
323374
final FileRolesStore fileRolesStore = mock(FileRolesStore.class);
324375
doCallRealMethod().when(fileRolesStore).accept(anySet(), anyActionListener());
325376
when(fileRolesStore.roleDescriptors(anySet())).thenReturn(Collections.emptySet());
@@ -329,13 +380,7 @@ public void testSuperuserIsEffectiveWhenOtherRolesUnavailable() {
329380
doAnswer((invocationOnMock) -> {
330381
@SuppressWarnings("unchecked")
331382
ActionListener<RoleRetrievalResult> callback = (ActionListener<RoleRetrievalResult>) invocationOnMock.getArguments()[1];
332-
final RuntimeException exception = new RuntimeException("Test(" + getTestName() + ") - native roles unavailable");
333-
if (randomBoolean()) {
334-
callback.onFailure(exception);
335-
} else {
336-
callback.onResponse(RoleRetrievalResult.failure(exception));
337-
338-
}
383+
rolesHandler.accept(callback);
339384
return null;
340385
}).when(nativeRolesStore).getRoleDescriptors(isASet(), anyActionListener());
341386

@@ -345,7 +390,7 @@ public void testSuperuserIsEffectiveWhenOtherRolesUnavailable() {
345390
@SuppressWarnings("unchecked")
346391
ActionListener<Collection<ApplicationPrivilegeDescriptor>> callback = (ActionListener<
347392
Collection<ApplicationPrivilegeDescriptor>>) invocationOnMock.getArguments()[2];
348-
callback.onResponse(Collections.emptyList());
393+
privilegesHandler.accept(callback);
349394
return null;
350395
}).when(nativePrivilegeStore).getPrivileges(anySet(), anySet(), anyActionListener());
351396

@@ -361,7 +406,10 @@ public void testSuperuserIsEffectiveWhenOtherRolesUnavailable() {
361406
null,
362407
null
363408
);
409+
return compositeRolesStore;
410+
}
364411

412+
private void trySuccessfullyLoadSuperuserRole(CompositeRolesStore compositeRolesStore) {
365413
final Set<String> roles = Set.of(randomAlphaOfLengthBetween(1, 6), "superuser", randomAlphaOfLengthBetween(7, 12));
366414
PlainActionFuture<Role> future = new PlainActionFuture<>();
367415
getRoleForRoleNames(compositeRolesStore, roles, future);
@@ -384,6 +432,14 @@ public void testSuperuserIsEffectiveWhenOtherRolesUnavailable() {
384432
assertThat(securityActionPredicate.test(IndexAction.NAME), is(false));
385433
}
386434

435+
private void tryFailOnNonSuperuserRole(CompositeRolesStore compositeRolesStore, Matcher<? super Exception> exceptionMatcher) {
436+
final Set<String> roles = Set.of(randomAlphaOfLengthBetween(1, 6), randomAlphaOfLengthBetween(7, 12));
437+
PlainActionFuture<Role> future = new PlainActionFuture<>();
438+
getRoleForRoleNames(compositeRolesStore, roles, future);
439+
final Exception exception = expectThrows(Exception.class, future::actionGet);
440+
assertThat(exception, exceptionMatcher);
441+
}
442+
387443
public void testNegativeLookupsAreCached() {
388444
final FileRolesStore fileRolesStore = mock(FileRolesStore.class);
389445
doCallRealMethod().when(fileRolesStore).accept(anySet(), anyActionListener());

0 commit comments

Comments
 (0)