Skip to content

Commit 38765dd

Browse files
committed
Include role names in access denied errors
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. Backport of: elastic#69318
1 parent 67b040f commit 38765dd

File tree

5 files changed

+37
-14
lines changed

5 files changed

+37
-14
lines changed

x-pack/plugin/ilm/qa/with-security/src/javaRestTest/java/org/elasticsearch/xpack/security/PermissionsIT.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ public void testCanManageIndexWithNoPermissions() throws Exception {
154154
assertThat(stepInfo.get("type"), equalTo("security_exception"));
155155
assertThat(stepInfo.get("reason"), equalTo("action [indices:monitor/stats] is unauthorized" +
156156
" for user [test_ilm]" +
157+
" with roles [ilm]" +
157158
" on indices [not-ilm]," +
158159
" this action is granted by the index privileges [monitor,manage,all]"));
159160
}

x-pack/plugin/ml/qa/native-multi-node-tests/src/javaRestTest/java/org/elasticsearch/xpack/ml/integration/DatafeedJobsRestIT.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -819,7 +819,10 @@ public void testLookbackWithoutPermissions() throws Exception {
819819
new Request("GET", NotificationsIndex.NOTIFICATIONS_INDEX + "/_search?size=1000&q=job_id:" + jobId));
820820
String notificationsResponseAsString = EntityUtils.toString(notificationsResponse.getEntity());
821821
assertThat(notificationsResponseAsString, containsString("\"message\":\"Datafeed is encountering errors extracting data: " +
822-
"action [indices:data/read/search] is unauthorized for user [ml_admin_plus_data] on indices [network-data],"));
822+
"action [indices:data/read/search] is unauthorized" +
823+
" for user [ml_admin_plus_data]" +
824+
" with roles [machine_learning_admin,test_data_access]" +
825+
" on indices [network-data]"));
823826
}
824827

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

974979
public void testLookbackWithSingleBucketAgg() throws Exception {

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.elasticsearch.cluster.metadata.Metadata;
2929
import org.elasticsearch.cluster.service.ClusterService;
3030
import org.elasticsearch.common.Nullable;
31+
import org.elasticsearch.common.Strings;
3132
import org.elasticsearch.common.collect.Tuple;
3233
import org.elasticsearch.common.settings.Setting;
3334
import org.elasticsearch.common.settings.Setting.Property;
@@ -631,6 +632,9 @@ private ElasticsearchSecurityException denialException(Authentication authentica
631632
final String apiKeyId = (String) authentication.getMetadata().get(ApiKeyService.API_KEY_ID_KEY);
632633
assert apiKeyId != null : "api key id must be present in the metadata";
633634
userText = "API key id [" + apiKeyId + "] of " + userText;
635+
} else {
636+
// Don't print roles for API keys because they're not meaningful
637+
userText = userText + " with roles [" + Strings.arrayToCommaDelimitedString(authentication.getUser().roles()) + "]";
634638
}
635639

636640
String message = "action [" + action + "] is unauthorized for " + userText;

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

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -677,8 +677,11 @@ public void testUnknownRoleCausesDenial() throws IOException {
677677

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

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

717720
ElasticsearchSecurityException securityException = expectThrows(ElasticsearchSecurityException.class,
718721
() -> authorize(authentication, action, request));
719-
assertThat(securityException,
720-
throwableWithMessage(containsString("[" + action + "] is unauthorized for user [test user] on indices [")));
722+
assertThat(securityException, throwableWithMessage(containsString(
723+
"[" + action + "] is unauthorized" +
724+
" for user [test user]" +
725+
" with roles [no_indices]" +
726+
" on indices [")));
721727
assertThat(securityException, throwableWithMessage(containsString("this action is granted by the index privileges [read,all]")));
722728

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

890896
public void testDenialErrorMessagesForSearchAction() throws IOException {
891-
RoleDescriptor role = new RoleDescriptor("some_indices_" + randomAlphaOfLengthBetween(3, 6), null, new IndicesPrivileges[]{
897+
RoleDescriptor indexRole = new RoleDescriptor("some_indices_" + randomAlphaOfLengthBetween(3, 6), null, new IndicesPrivileges[]{
892898
IndicesPrivileges.builder().indices("all*").privileges("all").build(),
893899
IndicesPrivileges.builder().indices("read*").privileges("read").build(),
894900
IndicesPrivileges.builder().indices("write*").privileges("write").build()
895901
}, null);
896-
User user = new User(randomAlphaOfLengthBetween(6, 8), role.getName());
902+
RoleDescriptor emptyRole = new RoleDescriptor("empty_role_" + randomAlphaOfLengthBetween(1,4), null, null, null);
903+
User user = new User(randomAlphaOfLengthBetween(6, 8), indexRole.getName(), emptyRole.getName());
897904
final Authentication authentication = createAuthentication(user);
898-
roleMap.put(role.getName(), role);
905+
roleMap.put(indexRole.getName(), indexRole);
906+
roleMap.put(emptyRole.getName(), emptyRole);
899907

900908
AuditUtil.getOrGenerateRequestId(threadContext);
901909

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

904912
ElasticsearchSecurityException securityException = expectThrows(ElasticsearchSecurityException.class,
905913
() -> authorize(authentication, SearchAction.NAME, request));
906-
assertThat(securityException, throwableWithMessage(
907-
containsString("[" + SearchAction.NAME + "] is unauthorized for user [" + user.principal() + "] on indices [")));
914+
assertThat(securityException, throwableWithMessage(containsString(
915+
"[" + SearchAction.NAME + "] is unauthorized" +
916+
" for user [" + user.principal() + "]" +
917+
" with roles [" + indexRole.getName() + "," + emptyRole.getName() + "]" +
918+
" on indices [")));
908919
assertThat(securityException, throwableWithMessage(containsString("write-3")));
909920
assertThat(securityException, throwableWithMessage(containsString("other-4")));
910921
assertThat(securityException, throwableWithMessage(not(containsString("all-1"))));

x-pack/plugin/src/test/resources/rest-api-spec/test/api_key/11_invalidation.yml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,8 @@ teardown:
126126
"username": "api_key_manager"
127127
}
128128
- match: { "error.type": "security_exception" }
129-
- 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]" }
129+
- match:
130+
"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]"
130131

131132
- do:
132133
headers:
@@ -189,7 +190,8 @@ teardown:
189190
"realm_name": "default_native"
190191
}
191192
- match: { "error.type": "security_exception" }
192-
- 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]" }
193+
- match:
194+
"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]"
193195

194196
- do:
195197
headers:

0 commit comments

Comments
 (0)