Skip to content

Commit

Permalink
Include role names in access denied errors (#69318)
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: #42166
  • Loading branch information
tvernum authored Mar 10, 2021
1 parent c3f11f2 commit d5d8cf9
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,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 @@ -819,7 +819,10 @@ public void testLookbackWithoutPermissions() throws Exception {
new Request("GET", NotificationsIndex.NOTIFICATIONS_INDEX + "/_search?size=1000&q=job_id:" + jobId));
String notificationsResponseAsString = EntityUtils.toString(notificationsResponse.getEntity());
assertThat(notificationsResponseAsString, containsString("\"message\":\"Datafeed is encountering errors extracting data: " +
"action [indices:data/read/search] is unauthorized for user [ml_admin_plus_data] on indices [network-data]"));
"action [indices:data/read/search] is unauthorized" +
" for user [ml_admin_plus_data]" +
" with roles [machine_learning_admin,test_data_access]" +
" on indices [network-data]"));
}

public void testLookbackWithPipelineBucketAgg() throws Exception {
Expand Down Expand Up @@ -967,8 +970,10 @@ public void testLookbackWithoutPermissionsAndRollup() throws Exception {
new Request("GET", NotificationsIndex.NOTIFICATIONS_INDEX + "/_search?size=1000&q=job_id:" + jobId));
String notificationsResponseAsString = EntityUtils.toString(notificationsResponse.getEntity());
assertThat(notificationsResponseAsString, containsString("\"message\":\"Datafeed is encountering errors extracting data: " +
"action [indices:data/read/xpack/rollup/search] is unauthorized for user [ml_admin_plus_data] " +
"on indices [airline-data-aggs-rollup]"));
"action [indices:data/read/xpack/rollup/search] is unauthorized" +
" for user [ml_admin_plus_data]" +
" with roles [machine_learning_admin,test_data_access]" +
" on indices [airline-data-aggs-rollup]"));
}

public void testLookbackWithSingleBucketAgg() throws Exception {
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 @@ -676,8 +676,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 @@ -715,8 +718,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 @@ -887,23 +893,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 d5d8cf9

Please sign in to comment.