Skip to content

Commit

Permalink
Include role names in access denied errors
Browse files Browse the repository at this point in the history
This commit adds a User's list of current role names to access denied
error messages to aid in diagnostics.
This allows an administrator to know whether the correct course of
action is to add another role to the user (e.g. by fixing incorrect
role mappings) or by modifying a role to add more privileges.

Resolves: elastic#42166
  • Loading branch information
tvernum committed Feb 22, 2021
1 parent 1256558 commit 6e4a695
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ public void testCanManageIndexWithNoPermissions() throws Exception {
assertThat(stepInfo.get("type"), equalTo("security_exception"));
assertThat(stepInfo.get("reason"), equalTo("action [indices:monitor/stats] is unauthorized" +
" for user [test_ilm]" +
" with roles [ilm]" +
" on indices [not-ilm]," +
" this action is granted by the index privileges [monitor,manage,all]"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.elasticsearch.cluster.metadata.Metadata;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Setting.Property;
Expand Down Expand Up @@ -630,6 +631,9 @@ private ElasticsearchSecurityException denialException(Authentication authentica
final String apiKeyId = (String) authentication.getMetadata().get(ApiKeyService.API_KEY_ID_KEY);
assert apiKeyId != null : "api key id must be present in the metadata";
userText = "API key id [" + apiKeyId + "] of " + userText;
} else {
// Don't print roles for API keys because they're not meaningful
userText = userText + " with roles [" + Strings.arrayToCommaDelimitedString(authentication.getUser().roles()) + "]";
}

String message = "action [" + action + "] is unauthorized for " + userText;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -677,8 +677,11 @@ public void testUnknownRoleCausesDenial() throws IOException {

ElasticsearchSecurityException securityException = expectThrows(ElasticsearchSecurityException.class,
() -> authorize(authentication, action, request));
assertThat(securityException,
throwableWithMessage(containsString("[" + action + "] is unauthorized for user [test user] on indices [")));
assertThat(securityException, throwableWithMessage(containsString(
"[" + action + "] is unauthorized" +
" for user [test user]" +
" with roles [non-existent-role]" +
" on indices [")));
assertThat(securityException, throwableWithMessage(containsString("this action is granted by the index privileges [read,all]")));

verify(auditTrail).accessDenied(eq(requestId), eq(authentication), eq(action), eq(request), authzInfoRoles(Role.EMPTY.names()));
Expand Down Expand Up @@ -716,8 +719,11 @@ public void testThatRoleWithNoIndicesIsDenied() throws IOException {

ElasticsearchSecurityException securityException = expectThrows(ElasticsearchSecurityException.class,
() -> authorize(authentication, action, request));
assertThat(securityException,
throwableWithMessage(containsString("[" + action + "] is unauthorized for user [test user] on indices [")));
assertThat(securityException, throwableWithMessage(containsString(
"[" + action + "] is unauthorized" +
" for user [test user]" +
" with roles [no_indices]" +
" on indices [")));
assertThat(securityException, throwableWithMessage(containsString("this action is granted by the index privileges [read,all]")));

verify(auditTrail).accessDenied(eq(requestId), eq(authentication), eq(action), eq(request),
Expand Down Expand Up @@ -888,23 +894,28 @@ public void testCreateIndexWithAlias() throws IOException {
}

public void testDenialErrorMessagesForSearchAction() throws IOException {
RoleDescriptor role = new RoleDescriptor("some_indices_" + randomAlphaOfLengthBetween(3, 6), null, new IndicesPrivileges[]{
RoleDescriptor indexRole = new RoleDescriptor("some_indices_" + randomAlphaOfLengthBetween(3, 6), null, new IndicesPrivileges[]{
IndicesPrivileges.builder().indices("all*").privileges("all").build(),
IndicesPrivileges.builder().indices("read*").privileges("read").build(),
IndicesPrivileges.builder().indices("write*").privileges("write").build()
}, null);
User user = new User(randomAlphaOfLengthBetween(6, 8), role.getName());
RoleDescriptor emptyRole = new RoleDescriptor("empty_role_" + randomAlphaOfLengthBetween(1,4), null, null, null);
User user = new User(randomAlphaOfLengthBetween(6, 8), indexRole.getName(), emptyRole.getName());
final Authentication authentication = createAuthentication(user);
roleMap.put(role.getName(), role);
roleMap.put(indexRole.getName(), indexRole);
roleMap.put(emptyRole.getName(), emptyRole);

AuditUtil.getOrGenerateRequestId(threadContext);

TransportRequest request = new SearchRequest("all-1", "read-2", "write-3", "other-4");

ElasticsearchSecurityException securityException = expectThrows(ElasticsearchSecurityException.class,
() -> authorize(authentication, SearchAction.NAME, request));
assertThat(securityException, throwableWithMessage(
containsString("[" + SearchAction.NAME + "] is unauthorized for user [" + user.principal() + "] on indices [")));
assertThat(securityException, throwableWithMessage(containsString(
"[" + SearchAction.NAME + "] is unauthorized" +
" for user [" + user.principal() + "]" +
" with roles [" + indexRole.getName() + "," + emptyRole.getName() + "]" +
" on indices [")));
assertThat(securityException, throwableWithMessage(containsString("write-3")));
assertThat(securityException, throwableWithMessage(containsString("other-4")));
assertThat(securityException, throwableWithMessage(not(containsString("all-1"))));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ teardown:
"username": "api_key_manager"
}
- match: { "error.type": "security_exception" }
- match: { "error.reason": "action [cluster:admin/xpack/security/api_key/invalidate] is unauthorized for user [api_key_user_1], this action is granted by the cluster privileges [manage_api_key,manage_security,all]" }
- match:
"error.reason": "action [cluster:admin/xpack/security/api_key/invalidate] is unauthorized for user [api_key_user_1] with roles [user_role], this action is granted by the cluster privileges [manage_api_key,manage_security,all]"

- do:
headers:
Expand Down Expand Up @@ -189,7 +190,8 @@ teardown:
"realm_name": "default_native"
}
- match: { "error.type": "security_exception" }
- match: { "error.reason": "action [cluster:admin/xpack/security/api_key/invalidate] is unauthorized for user [api_key_user_1], this action is granted by the cluster privileges [manage_api_key,manage_security,all]" }
- match:
"error.reason": "action [cluster:admin/xpack/security/api_key/invalidate] is unauthorized for user [api_key_user_1] with roles [user_role], this action is granted by the cluster privileges [manage_api_key,manage_security,all]"

- do:
headers:
Expand Down

0 comments on commit 6e4a695

Please sign in to comment.