Skip to content

Commit

Permalink
add tests
Browse files Browse the repository at this point in the history
  • Loading branch information
Yogesh Gaikwad committed Aug 20, 2019
1 parent aa623c3 commit efc2c2b
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 10 deletions.
10 changes: 4 additions & 6 deletions ...rg/elasticsearch/xpack/core/security/authz/privilege/ManageOwnApiKeyClusterPrivilege.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,10 @@
public class ManageOwnApiKeyClusterPrivilege implements NamedClusterPrivilege {
public static final ManageOwnApiKeyClusterPrivilege INSTANCE = new ManageOwnApiKeyClusterPrivilege();
private static final Predicate<String> ACTION_PREDICATE = Automatons.predicate("cluster:admin/xpack/security/api_key/*");
private final String name;
private static final String PRIVILEGE_NAME = "manage_own_api_key";
private final BiPredicate<TransportRequest, Authentication> requestAuthnPredicate;

private ManageOwnApiKeyClusterPrivilege() {
this.name = "manage_own_api_key";
this.requestAuthnPredicate = (request, authentication) -> {
if (request instanceof CreateApiKeyRequest) {
return true;
Expand All @@ -52,8 +51,8 @@ private boolean checkIfUserIsOwnerOfApiKeys(Authentication authentication, Strin
return true;
} else {
/*
* TODO ygaikwad we need to think on how we can propagate appropriate error message to the end user when username, realm name
* is missing. This is similar to the problem of propagating proper error messages in case of access denied.
* TODO bizybot we need to think on how we can propagate appropriate error message to the end user when username, realm name
* is missing. This is similar to the problem of propagating right error messages in case of access denied.
*/
String authenticatedUserPrincipal = authentication.getUser().principal();
String authenticatedUserRealm = authentication.getAuthenticatedBy().getName();
Expand All @@ -65,7 +64,6 @@ private boolean checkIfUserIsOwnerOfApiKeys(Authentication authentication, Strin
}

private boolean isCurrentAuthenticationUsingSameApiKeyIdFromRequest(Authentication authentication, String apiKeyId) {
// TODO ygaikwad replace with constants after merging other change
if (authentication.getAuthenticatedBy().getType().equals("_es_api_key")) {
// API key id from authentication must match the id from request
String authenticatedApiKeyId = (String) authentication.getMetadata().get("_security_api_key_id");
Expand All @@ -78,7 +76,7 @@ private boolean isCurrentAuthenticationUsingSameApiKeyIdFromRequest(Authenticati

@Override
public String name() {
return name;
return PRIVILEGE_NAME;
}

@Override
Expand Down
5 changes: 5 additions & 0 deletions ...urity/src/main/java/org/elasticsearch/xpack/security/action/TransportGetApiKeyAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ protected void doExecute(Task task, GetApiKeyRequest request, ActionListener<Get
if (request.ownedByAuthenticatedUser()) {
assert username == null;
assert realm == null;
if (authentication.getAuthenticatedBy().getType().equals(ApiKeyService.API_KEY_REALM_TYPE)) {
throw new IllegalArgumentException(
"failed to retrieve owned API keys for a user authenticated by API key as they cannot own any API keys");
}

// restrict username and realm to current authenticated user.
username = authentication.getUser().principal();
realm = authentication.getAuthenticatedBy().getName();
Expand Down
4 changes: 4 additions & 0 deletions ...rc/main/java/org/elasticsearch/xpack/security/action/TransportInvalidateApiKeyAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ protected void doExecute(Task task, InvalidateApiKeyRequest request, ActionListe
if (request.ownedByAuthenticatedUser()) {
assert username == null;
assert realm == null;
if (authentication.getAuthenticatedBy().getType().equals(ApiKeyService.API_KEY_REALM_TYPE)) {
throw new IllegalArgumentException(
"failed to invalidate owned API keys for a user authenticated by API key as they cannot own any API keys");
}
// restrict username and realm to current authenticated user.
username = authentication.getUser().principal();
realm = authentication.getAuthenticatedBy().getName();
Expand Down
9 changes: 9 additions & 0 deletions ...n/security/src/main/java/org/elasticsearch/xpack/security/authz/AuthorizationService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
import org.elasticsearch.xpack.security.audit.AuditLevel;
import org.elasticsearch.xpack.security.audit.AuditTrailService;
import org.elasticsearch.xpack.security.audit.AuditUtil;
import org.elasticsearch.xpack.security.authc.ApiKeyService;
import org.elasticsearch.xpack.security.authz.interceptor.RequestInterceptor;
import org.elasticsearch.xpack.security.authz.store.CompositeRolesStore;

Expand Down Expand Up @@ -572,6 +573,14 @@ private ElasticsearchSecurityException denialException(Authentication authentica
return authorizationError("action [{}] is unauthorized for user [{}] run as [{}]", cause, action, authUser.principal(),
authentication.getUser().principal());
}
// check for authentication by API key
if (authentication.getAuthenticatedBy().getType().equals(ApiKeyService.API_KEY_REALM_TYPE)) {
final String apiKeyId = (String) authentication.getMetadata().get(ApiKeyService.API_KEY_ID_KEY);
assert apiKeyId != null : "api key id must be present in the metadata";
logger.debug("action [{}] is unauthorized for API key id [{}] of user [{}]", action, apiKeyId, authUser.principal());
return authorizationError("action [{}] is unauthorized for API key id [{}] of user [{}]", cause, action, apiKeyId,
authUser.principal());
}
logger.debug("action [{}] is unauthorized for user [{}]", action, authUser.principal());
return authorizationError("action [{}] is unauthorized for user [{}]", cause, action, authUser.principal());
}
Expand Down
45 changes: 41 additions & 4 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 @@ -545,9 +545,37 @@ public void testApiKeyAuthorizationApiKeyMustBeAbleToRetrieveItsOwnInformation()

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);
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);
assertErrorMessage(ese, "cluster:admin/xpack/security/api_key/get", SecuritySettingsSource.TEST_SUPERUSER,
responses.get(0).getId());
}

public void testApiKeyWithManageOwnPrivilegeIsAbleToInvalidateItselfButNotAnyOtherKeysCreatedBySameOwner() throws InterruptedException
, ExecutionException {
List<CreateApiKeyResponse> responses = createApiKeys(SecuritySettingsSource.TEST_SUPERUSER, 2, null, "manage_own_api_key");
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<InvalidateApiKeyResponse> listener = new PlainActionFuture<>();

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

client.execute(InvalidateApiKeyAction.INSTANCE, InvalidateApiKeyRequest.usingApiKeyId(responses.get(0).getId(), false),
listener);
InvalidateApiKeyResponse invalidateResponse = listener.get();

assertThat(invalidateResponse.getInvalidatedApiKeys().size(), equalTo(1));
assertThat(invalidateResponse.getInvalidatedApiKeys(), containsInAnyOrder(responses.get(0).getId()));
assertThat(invalidateResponse.getPreviouslyInvalidatedApiKeys().size(), equalTo(0));
assertThat(invalidateResponse.getErrors().size(), equalTo(0));
}

private void verifyGetResponse(int expectedNumberOfApiKeys, List<CreateApiKeyResponse> responses,
Expand Down Expand Up @@ -582,13 +610,17 @@ private void verifyGetResponse(String user, int expectedNumberOfApiKeys, List<Cr
}

private List<CreateApiKeyResponse> createApiKeys(int noOfApiKeys, TimeValue expiration) {
return createApiKeys(SecuritySettingsSource.TEST_SUPERUSER, noOfApiKeys, expiration);
return createApiKeys(SecuritySettingsSource.TEST_SUPERUSER, noOfApiKeys, expiration, "monitor");
}

private List<CreateApiKeyResponse> createApiKeys(String user, int noOfApiKeys, TimeValue expiration) {
return createApiKeys(user, noOfApiKeys, expiration, "monitor");
}

private List<CreateApiKeyResponse> createApiKeys(String user, int noOfApiKeys, TimeValue expiration, String role) {
List<CreateApiKeyResponse> responses = new ArrayList<>();
for (int i = 0; i < noOfApiKeys; i++) {
final RoleDescriptor descriptor = new RoleDescriptor("role", new String[] { "monitor" }, null, null);
final RoleDescriptor descriptor = new RoleDescriptor("role", new String[] { role }, null, null);
Client client = client().filterWithHeader(Collections.singletonMap("Authorization", UsernamePasswordToken
.basicAuthHeaderValue(user, SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING)));
final CreateApiKeyResponse response = new CreateApiKeyRequestBuilder(client)
Expand All @@ -602,6 +634,11 @@ private List<CreateApiKeyResponse> createApiKeys(String user, int noOfApiKeys, T
return responses;
}

private void assertErrorMessage(final ElasticsearchSecurityException ese, String action, String userName, String apiKeyId) {
assertThat(ese.getMessage(),
is("action [" + action + "] is unauthorized for API key id [" + apiKeyId + "] of user [" + userName + "]"));
}

private void assertErrorMessage(final ElasticsearchSecurityException ese, String action, String userName) {
assertThat(ese.getMessage(), is("action [" + action + "] is unauthorized for user [" + userName + "]"));
}
Expand Down

0 comments on commit efc2c2b

Please sign in to comment.