Skip to content

Include role names in access denied errors #69318

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 10, 2021
Merged

Include role names in access denied errors #69318

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6e4a695
Include role names in access denied errors
tvernum Feb 10, 2021
78feea3
Fix ML test
tvernum Feb 22, 2021
28fe0d2
Remove extra space in test assertion
tvernum Feb 23, 2021
f209af2
Merge branch 'master' into feature/access-denied-list-roles
tvernum Mar 10, 2021
32e52e0
Merge branch 'feature/access-denied-list-roles' of github.com:tvernum…
tvernum Mar 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ...a/with-security/src/javaRestTest/java/org/elasticsearch/xpack/security/PermissionsIT.java
View file
Open in desktop
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
11 changes: 8 additions & 3 deletions ...ests/src/javaRestTest/java/org/elasticsearch/xpack/ml/integration/DatafeedJobsRestIT.java
View file
Open in desktop
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
4 changes: 4 additions & 0 deletions ...n/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java
View file
Open in desktop
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
29 changes: 20 additions & 9 deletions ...urity/src/test/java/org/elasticsearch/xpack/security/authz/AuthorizationServiceTests.java
View file
Open in desktop
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
6 changes: 4 additions & 2 deletions x-pack/plugin/src/test/resources/rest-api-spec/test/api_key/11_invalidation.yml
View file
Open in desktop
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