Skip to content

Commit

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

The synthetic role names of API key add confusion to users. This
happens to API responses as well as audit logs. This PR removes
them for clarity.

* Fix typo
  • Loading branch information
jkakavas committed Apr 30, 2020
1 parent ce1aaec commit 66f0e17
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 @@ -537,9 +537,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_ROLE_DESCRIPTORS_KEY, roleDescriptors);
authResultMetadata.put(API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY, limitedByRoleDescriptors);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@
import java.util.concurrent.atomic.AtomicInteger;

import static org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore.SUPERUSER_ROLE_DESCRIPTOR;
import static org.hamcrest.Matchers.arrayContaining;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.emptyArray;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasKey;
import static org.hamcrest.Matchers.instanceOf;
Expand Down Expand Up @@ -374,7 +374,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 @@ -387,7 +387,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:
xpack.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 66f0e17

Please sign in to comment.