Skip to content

Commit

Permalink
Remove synthetic role names of API keys as they confuse users (#56005) (
Browse files Browse the repository at this point in the history
#56012)

Synthetic role names of API keys add confusion to users. This happens to API responses as well as audit logs. The PR removes them for clarity.
  • Loading branch information
ywangd committed Apr 30, 2020
1 parent ffdac3c commit a7d01c2
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -510,9 +510,7 @@ private void validateApiKeyExpiration(Map<String, Object> source, ApiKeyCredenti
final Map<String, Object> metadata = (Map<String, Object>) creator.get("metadata");
final Map<String, Object> roleDescriptors = (Map<String, Object>) source.get("role_descriptors");
final Map<String, Object> limitedByRoleDescriptors = (Map<String, Object>) source.get("limited_by_role_descriptors");
final String[] roleNames = (roleDescriptors != null) ? roleDescriptors.keySet().toArray(Strings.EMPTY_ARRAY)
: limitedByRoleDescriptors.keySet().toArray(Strings.EMPTY_ARRAY);
final User apiKeyUser = new User(principal, roleNames, null, null, metadata, true);
final User apiKeyUser = new User(principal, Strings.EMPTY_ARRAY, null, null, metadata, true);
final Map<String, Object> authResultMetadata = new HashMap<>();
authResultMetadata.put(API_KEY_CREATOR_REALM_NAME, creator.get("realm"));
authResultMetadata.put(API_KEY_CREATOR_REALM_TYPE, creator.get("realm_type"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@

import static org.elasticsearch.test.TestMatchers.throwableWithMessage;
import static org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR;
import static org.hamcrest.Matchers.arrayContaining;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.emptyArray;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
Expand Down Expand Up @@ -306,7 +306,7 @@ public void testValidateApiKey() throws Exception {
assertNotNull(result);
assertTrue(result.isAuthenticated());
assertThat(result.getUser().principal(), is("test_user"));
assertThat(result.getUser().roles(), arrayContaining("a role"));
assertThat(result.getUser().roles(), is(emptyArray()));
assertThat(result.getUser().metadata(), is(Collections.emptyMap()));
assertThat(result.getMetadata().get(ApiKeyService.API_KEY_ROLE_DESCRIPTORS_KEY), equalTo(sourceMap.get("role_descriptors")));
assertThat(result.getMetadata().get(ApiKeyService.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY),
Expand All @@ -320,7 +320,7 @@ public void testValidateApiKey() throws Exception {
assertNotNull(result);
assertTrue(result.isAuthenticated());
assertThat(result.getUser().principal(), is("test_user"));
assertThat(result.getUser().roles(), arrayContaining("a role"));
assertThat(result.getUser().roles(), is(emptyArray()));
assertThat(result.getUser().metadata(), is(Collections.emptyMap()));
assertThat(result.getMetadata().get(ApiKeyService.API_KEY_ROLE_DESCRIPTORS_KEY), equalTo(sourceMap.get("role_descriptors")));
assertThat(result.getMetadata().get(ApiKeyService.API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,7 @@ teardown:
security.authenticate: {}

- match: { username: "api_key_user" }
- match: { roles.0: "role-b" }
- match: { roles.1: "role-a" }
- length: { roles: 0 }
- match: { authentication_realm.name: "_es_api_key" }
- match: { authentication_realm.type: "_es_api_key" }

Expand Down

0 comments on commit a7d01c2

Please sign in to comment.