Skip to content

Commit

Permalink
Allow API key to retrieve its own information with no API key privile…
Browse files Browse the repository at this point in the history 
…ge (#45433)

Unless the API key has `manage_api_key` privilege, it cannot get its
own API key information when authenticating using an API key. There can
be a use case wherein we do not wish the user authenticating using an API
key to be able to invalidate or view any other API keys but only view information
about itself. This commit addresses this by allowing the request when
API key id from the `GetApiKeyRequest` matches the API key id present in the
`authentication` metadata.

Relates: #40031
  • Loading branch information
@bizybot
bizybot authored Aug 15, 2019
1 parent 9cf2432 commit 4d1bed0
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 23 deletions.
2 changes: 2 additions & 0 deletions ...k/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ public class ApiKeyService {
private static final Logger logger = LogManager.getLogger(ApiKeyService.class);
private static final DeprecationLogger deprecationLogger = new DeprecationLogger(logger);
public static final String API_KEY_ID_KEY = "_security_api_key_id";
public static final String API_KEY_REALM_NAME = "_es_api_key";
public static final String API_KEY_REALM_TYPE = "_es_api_key";
static final String API_KEY_ROLE_DESCRIPTORS_KEY = "_security_api_key_role_descriptors";
static final String API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY = "_security_api_key_limited_by_role_descriptors";

Expand Down
2 changes: 1 addition & 1 deletion .../security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ private void checkForApiKey() {
apiKeyService.authenticateWithApiKeyIfPresent(threadContext, ActionListener.wrap(authResult -> {
if (authResult.isAuthenticated()) {
final User user = authResult.getUser();
authenticatedBy = new RealmRef("_es_api_key", "_es_api_key", nodeName);
authenticatedBy = new RealmRef(ApiKeyService.API_KEY_REALM_NAME, ApiKeyService.API_KEY_REALM_TYPE, nodeName);
writeAuthToContext(new Authentication(user, authenticatedBy, null, Version.CURRENT,
Authentication.AuthenticationType.API_KEY, authResult.getMetadata()));
} else if (authResult.getStatus() == AuthenticationResult.Status.TERMINATE) {
Expand Down
54 changes: 35 additions & 19 deletions x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/RBACEngine.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
import org.elasticsearch.common.util.set.Sets;
import org.elasticsearch.transport.TransportActionProxy;
import org.elasticsearch.transport.TransportRequest;
import org.elasticsearch.xpack.core.security.action.GetApiKeyAction;
import org.elasticsearch.xpack.core.security.action.GetApiKeyRequest;
import org.elasticsearch.xpack.core.security.action.user.AuthenticateAction;
import org.elasticsearch.xpack.core.security.action.user.ChangePasswordAction;
import org.elasticsearch.xpack.core.security.action.user.GetUserPrivilegesAction;
Expand Down Expand Up @@ -62,6 +64,7 @@
import org.elasticsearch.xpack.core.security.authz.privilege.Privilege;
import org.elasticsearch.xpack.core.security.support.Automatons;
import org.elasticsearch.xpack.core.security.user.User;
import org.elasticsearch.xpack.security.authc.ApiKeyService;
import org.elasticsearch.xpack.security.authc.esnative.ReservedRealm;
import org.elasticsearch.xpack.security.authz.store.CompositeRolesStore;

Expand All @@ -86,7 +89,7 @@
public class RBACEngine implements AuthorizationEngine {

private static final Predicate<String> SAME_USER_PRIVILEGE = Automatons.predicate(
ChangePasswordAction.NAME, AuthenticateAction.NAME, HasPrivilegesAction.NAME, GetUserPrivilegesAction.NAME);
ChangePasswordAction.NAME, AuthenticateAction.NAME, HasPrivilegesAction.NAME, GetUserPrivilegesAction.NAME, GetApiKeyAction.NAME);
private static final String INDEX_SUB_REQUEST_PRIMARY = IndexAction.NAME + "[p]";
private static final String INDEX_SUB_REQUEST_REPLICA = IndexAction.NAME + "[r]";
private static final String DELETE_SUB_REQUEST_PRIMARY = DeleteAction.NAME + "[p]";
Expand Down Expand Up @@ -154,26 +157,39 @@ public void authorizeClusterAction(RequestInfo requestInfo, AuthorizationInfo au
boolean checkSameUserPermissions(String action, TransportRequest request, Authentication authentication) {
final boolean actionAllowed = SAME_USER_PRIVILEGE.test(action);
if (actionAllowed) {
if (request instanceof UserRequest == false) {
assert false : "right now only a user request should be allowed";
return false;
}
UserRequest userRequest = (UserRequest) request;
String[] usernames = userRequest.usernames();
if (usernames == null || usernames.length != 1 || usernames[0] == null) {
assert false : "this role should only be used for actions to apply to a single user";
if (request instanceof UserRequest) {
UserRequest userRequest = (UserRequest) request;
String[] usernames = userRequest.usernames();
if (usernames == null || usernames.length != 1 || usernames[0] == null) {
assert false : "this role should only be used for actions to apply to a single user";
return false;
}
final String username = usernames[0];
final boolean sameUsername = authentication.getUser().principal().equals(username);
if (sameUsername && ChangePasswordAction.NAME.equals(action)) {
return checkChangePasswordAction(authentication);
}

assert AuthenticateAction.NAME.equals(action) || HasPrivilegesAction.NAME.equals(action)
|| GetUserPrivilegesAction.NAME.equals(action) || sameUsername == false
: "Action '" + action + "' should not be possible when sameUsername=" + sameUsername;
return sameUsername;
} else if (request instanceof GetApiKeyRequest) {
GetApiKeyRequest getApiKeyRequest = (GetApiKeyRequest) request;
if (authentication.getAuthenticatedBy().getType().equals(ApiKeyService.API_KEY_REALM_TYPE)) {
assert authentication.getLookedUpBy() == null : "runAs not supported for api key authentication";
// if authenticated by API key then the request must also contain same API key id
String authenticatedApiKeyId = (String) authentication.getMetadata().get(ApiKeyService.API_KEY_ID_KEY);
if (Strings.hasText(getApiKeyRequest.getApiKeyId())) {
return getApiKeyRequest.getApiKeyId().equals(authenticatedApiKeyId);
} else {
return false;
}
}
} else {
assert false : "right now only a user request or get api key request should be allowed";
return false;
}
final String username = usernames[0];
final boolean sameUsername = authentication.getUser().principal().equals(username);
if (sameUsername && ChangePasswordAction.NAME.equals(action)) {
return checkChangePasswordAction(authentication);
}

assert AuthenticateAction.NAME.equals(action) || HasPrivilegesAction.NAME.equals(action)
|| GetUserPrivilegesAction.NAME.equals(action) || sameUsername == false
: "Action '" + action + "' should not be possible when sameUsername=" + sameUsername;
return sameUsername;
}
return false;
}
Expand Down
27 changes: 24 additions & 3 deletions ...lugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import java.util.Base64;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
Expand Down Expand Up @@ -532,9 +533,25 @@ public void testInvalidateApiKeysOwnedByCurrentAuthenticatedUser() throws Interr
verifyInvalidateResponse(noOfApiKeysForUserWithManageApiKeyRole, userWithManageApiKeyRoleApiKeys, invalidateResponse);
}

private void verifyGetResponse(int expectedNumberOfApiKeys, List<CreateApiKeyResponse> responses, GetApiKeyResponse response,
Set<String> validApiKeyIds,
List<String> invalidatedApiKeyIds) {
public void testApiKeyAuthorizationApiKeyMustBeAbleToRetrieveItsOwnInformation() throws InterruptedException, ExecutionException {
List<CreateApiKeyResponse> responses = createApiKeys(2, null);
final String base64ApiKeyKeyValue = Base64.getEncoder().encodeToString(
(responses.get(0).getId() + ":" + responses.get(0).getKey().toString()).getBytes(StandardCharsets.UTF_8));
Client client = client().filterWithHeader(Map.of("Authorization", "ApiKey " + base64ApiKeyKeyValue));
PlainActionFuture<GetApiKeyResponse> listener = new PlainActionFuture<>();
client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.usingApiKeyId(responses.get(0).getId(), false), listener);
GetApiKeyResponse response = listener.get();
verifyGetResponse(1, responses, response, Collections.singleton(responses.get(0).getId()), null);

final PlainActionFuture<GetApiKeyResponse> failureListener = new PlainActionFuture<>();
// for any other API key id, it must deny access
client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.usingApiKeyId(responses.get(1).getId(), false), failureListener);
ElasticsearchSecurityException ese = expectThrows(ElasticsearchSecurityException.class, () -> failureListener.actionGet());
assertErrorMessage(ese, "cluster:admin/xpack/security/api_key/get", SecuritySettingsSource.TEST_SUPERUSER);
}

private void verifyGetResponse(int expectedNumberOfApiKeys, List<CreateApiKeyResponse> responses,
GetApiKeyResponse response, Set<String> validApiKeyIds, List<String> invalidatedApiKeyIds) {
verifyGetResponse(SecuritySettingsSource.TEST_SUPERUSER, expectedNumberOfApiKeys, responses, response, validApiKeyIds,
invalidatedApiKeyIds);
}
Expand Down Expand Up @@ -584,4 +601,8 @@ private List<CreateApiKeyResponse> createApiKeys(String user, int noOfApiKeys, T
assertThat(responses.size(), is(noOfApiKeys));
return responses;
}

private void assertErrorMessage(final ElasticsearchSecurityException ese, String action, String userName) {
assertThat(ese.getMessage(), is("action [" + action + "] is unauthorized for user [" + userName + "]"));
}
}
51 changes: 51 additions & 0 deletions ...plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
import org.elasticsearch.license.GetLicenseAction;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.transport.TransportRequest;
import org.elasticsearch.xpack.core.security.action.GetApiKeyAction;
import org.elasticsearch.xpack.core.security.action.GetApiKeyRequest;
import org.elasticsearch.xpack.core.security.action.user.AuthenticateAction;
import org.elasticsearch.xpack.core.security.action.user.AuthenticateRequest;
import org.elasticsearch.xpack.core.security.action.user.AuthenticateRequestBuilder;
Expand Down Expand Up @@ -51,6 +53,7 @@
import org.elasticsearch.xpack.core.security.authz.privilege.Privilege;
import org.elasticsearch.xpack.core.security.index.RestrictedIndicesNames;
import org.elasticsearch.xpack.core.security.user.User;
import org.elasticsearch.xpack.security.authc.ApiKeyService;
import org.elasticsearch.xpack.security.authc.esnative.ReservedRealm;
import org.elasticsearch.xpack.security.authz.RBACEngine.RBACAuthorizationInfo;
import org.elasticsearch.xpack.security.authz.store.CompositeRolesStore;
Expand All @@ -63,6 +66,7 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;

import static java.util.Collections.emptyMap;
Expand Down Expand Up @@ -232,6 +236,53 @@ public void testSameUserPermissionDoesNotAllowChangePasswordForLookedUpByOtherRe
verifyNoMoreInteractions(authentication, lookedUpBy, authenticatedBy);
}

public void testSameUserPermissionAllowsSelfApiKeyInfoRetrievalWhenAuthenticatedByApiKey() {
final User user = new User("joe");
final String apiKeyId = randomAlphaOfLengthBetween(4, 7);
final TransportRequest request = GetApiKeyRequest.usingApiKeyId(apiKeyId, false);
final Authentication authentication = mock(Authentication.class);
final Authentication.RealmRef authenticatedBy = mock(Authentication.RealmRef.class);
when(authentication.getUser()).thenReturn(user);
when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy);
when(authenticatedBy.getType()).thenReturn(ApiKeyService.API_KEY_REALM_TYPE);
when(authentication.getMetadata()).thenReturn(Map.of(ApiKeyService.API_KEY_ID_KEY, apiKeyId));

assertTrue(engine.checkSameUserPermissions(GetApiKeyAction.NAME, request, authentication));
}

public void testSameUserPermissionDeniesApiKeyInfoRetrievalWhenAuthenticatedByADifferentApiKey() {
final User user = new User("joe");
final String apiKeyId = randomAlphaOfLengthBetween(4, 7);
final TransportRequest request = GetApiKeyRequest.usingApiKeyId(apiKeyId, false);
final Authentication authentication = mock(Authentication.class);
final Authentication.RealmRef authenticatedBy = mock(Authentication.RealmRef.class);
when(authentication.getUser()).thenReturn(user);
when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy);
when(authenticatedBy.getType()).thenReturn(ApiKeyService.API_KEY_REALM_TYPE);
when(authentication.getMetadata()).thenReturn(Map.of(ApiKeyService.API_KEY_ID_KEY, randomAlphaOfLengthBetween(4, 7)));

assertFalse(engine.checkSameUserPermissions(GetApiKeyAction.NAME, request, authentication));
}

public void testSameUserPermissionDeniesApiKeyInfoRetrievalWhenLookedupByIsPresent() {
final User user = new User("joe");
final String apiKeyId = randomAlphaOfLengthBetween(4, 7);
final TransportRequest request = GetApiKeyRequest.usingApiKeyId(apiKeyId, false);
final Authentication authentication = mock(Authentication.class);
final Authentication.RealmRef authenticatedBy = mock(Authentication.RealmRef.class);
final Authentication.RealmRef lookedupBy = mock(Authentication.RealmRef.class);
when(authentication.getUser()).thenReturn(user);
when(authentication.getAuthenticatedBy()).thenReturn(authenticatedBy);
when(authentication.getLookedUpBy()).thenReturn(lookedupBy);
when(authenticatedBy.getType()).thenReturn(ApiKeyService.API_KEY_REALM_TYPE);
when(authentication.getMetadata()).thenReturn(Map.of(ApiKeyService.API_KEY_ID_KEY, randomAlphaOfLengthBetween(4, 7)));

final AssertionError assertionError = expectThrows(AssertionError.class, () -> engine.checkSameUserPermissions(GetApiKeyAction.NAME,
request, authentication));
assertNotNull(assertionError);
assertThat(assertionError.getLocalizedMessage(), is("runAs not supported for api key authentication"));
}

/**
* This tests that action names in the request are considered "matched" by the relevant named privilege
* (in this case that {@link DeleteAction} and {@link IndexAction} are satisfied by {@link IndexPrivilege#WRITE}).
Expand Down

0 comments on commit 4d1bed0

Please sign in to comment.