Skip to content

Commit

Permalink
Add support to retrieve all API keys if user has privilege (#47274)
Browse files Browse the repository at this point in the history 
This commit adds support to retrieve all API keys if the authenticated
user is authorized to do so.
This removes the restriction of specifying one of the
parameters (like id, name, username and/or realm name)
when the `owner` is set to `false`.

Closes #46887
  • Loading branch information
@bizybot
bizybot authored Oct 7, 2019
1 parent b958467 commit 01f9177
Show file tree
Hide file tree
Showing 9 changed files with 117 additions and 32 deletions.
15 changes: 11 additions & 4 deletions client/rest-high-level/src/main/java/org/elasticsearch/client/security/GetApiKeyRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@ public final class GetApiKeyRequest implements Validatable, ToXContentObject {
private final String name;
private final boolean ownedByAuthenticatedUser;

private GetApiKeyRequest() {
this(null, null, null, null, false);
}

// pkg scope for testing
GetApiKeyRequest(@Nullable String realmName, @Nullable String userName, @Nullable String apiKeyId,
@Nullable String apiKeyName, boolean ownedByAuthenticatedUser) {
if (Strings.hasText(realmName) == false && Strings.hasText(userName) == false && Strings.hasText(apiKeyId) == false
&& Strings.hasText(apiKeyName) == false && ownedByAuthenticatedUser == false) {
throwValidationError("One of [api key id, api key name, username, realm name] must be specified if [owner] flag is false");
}
if (Strings.hasText(apiKeyId) || Strings.hasText(apiKeyName)) {
if (Strings.hasText(realmName) || Strings.hasText(userName)) {
throwValidationError(
Expand Down Expand Up @@ -147,6 +147,13 @@ public static GetApiKeyRequest forOwnedApiKeys() {
return new GetApiKeyRequest(null, null, null, null, true);
}

/**
* Creates get api key request to retrieve api key information for all api keys if the authenticated user is authorized to do so.
*/
public static GetApiKeyRequest forAllApiKeys() {
return new GetApiKeyRequest();
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
return builder;
Expand Down
12 changes: 12 additions & 0 deletions ...h-level/src/test/java/org/elasticsearch/client/documentation/SecurityDocumentationIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1985,6 +1985,18 @@ public void testGetApiKey() throws Exception {
verifyApiKey(getApiKeyResponse.getApiKeyInfos().get(0), expectedApiKeyInfo);
}

{
// tag::get-all-api-keys-request
GetApiKeyRequest getApiKeyRequest = GetApiKeyRequest.forAllApiKeys();
// end::get-all-api-keys-request

GetApiKeyResponse getApiKeyResponse = client.security().getApiKey(getApiKeyRequest, RequestOptions.DEFAULT);

assertThat(getApiKeyResponse.getApiKeyInfos(), is(notNullValue()));
assertThat(getApiKeyResponse.getApiKeyInfos().size(), is(1));
verifyApiKey(getApiKeyResponse.getApiKeyInfos().get(0), expectedApiKeyInfo);
}

{
// tag::get-user-realm-api-keys-request
GetApiKeyRequest getApiKeyRequest = GetApiKeyRequest.usingRealmAndUserName("default_file", "test_user");
Expand Down
2 changes: 0 additions & 2 deletions ...est-high-level/src/test/java/org/elasticsearch/client/security/GetApiKeyRequestTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,13 @@ public void testRequestValidation() {

public void testRequestValidationFailureScenarios() throws IOException {
String[][] inputs = new String[][] {
{ randomNullOrEmptyString(), randomNullOrEmptyString(), randomNullOrEmptyString(), randomNullOrEmptyString(), "false" },
{ randomNullOrEmptyString(), "user", "api-kid", "api-kname", "false" },
{ "realm", randomNullOrEmptyString(), "api-kid", "api-kname", "false" },
{ "realm", "user", "api-kid", randomNullOrEmptyString(), "false" },
{ randomNullOrEmptyString(), randomNullOrEmptyString(), "api-kid", "api-kname", "false" },
{ "realm", randomNullOrEmptyString(), randomNullOrEmptyString(), randomNullOrEmptyString(), "true"},
{ randomNullOrEmptyString(), "user", randomNullOrEmptyString(), randomNullOrEmptyString(), "true"} };
String[] expectedErrorMessages = new String[] {
"One of [api key id, api key name, username, realm name] must be specified if [owner] flag is false",
"username or realm name must not be specified when the api key id or api key name is specified",
"username or realm name must not be specified when the api key id or api key name is specified",
"username or realm name must not be specified when the api key id or api key name is specified",
Expand Down
8 changes: 8 additions & 0 deletions docs/java-rest/high-level/security/get-api-key.asciidoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ The +{request}+ supports retrieving API key information for

. A specific key or all API keys owned by the current authenticated user

. All API keys if the user is authorized to do so

===== Retrieve a specific API key by its id
["source","java",subs="attributes,callouts,macros"]
--------------------------------------------------
Expand Down Expand Up @@ -59,6 +61,12 @@ include-tagged::{doc-tests-file}[get-user-realm-api-keys-request]
include-tagged::{doc-tests-file}[get-api-keys-owned-by-authenticated-user-request]
--------------------------------------------------

===== Retrieve all API keys if the user is authorized to do so
["source","java",subs="attributes,callouts,macros"]
--------------------------------------------------
include-tagged::{doc-tests-file}[get-all-api-keys-request]
--------------------------------------------------

include::../execution.asciidoc[]

[id="{upid}-{api}-response"]
Expand Down
13 changes: 11 additions & 2 deletions x-pack/docs/en/rest-api/security/get-api-keys.asciidoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,10 @@ by the currently authenticated user. Defaults to false.
The 'realm_name' or 'username' parameters cannot be specified when this
parameter is set to 'true' as they are assumed to be the currently authenticated ones.

NOTE: At least one of "id", "name", "username" and "realm_name" must be specified
if "owner" is "false" (default).
NOTE: When none of the parameters "id", "name", "username" and "realm_name"
are specified, and the "owner" is set to false then it will retrieve all API
keys if the user is authorized. If the user is not authorized to retrieve other user's
API keys, then an error will be returned.

[[security-api-get-api-key-example]]
==== {api-examples-title}
Expand Down Expand Up @@ -123,6 +125,13 @@ GET /_security/api_key?owner=true
--------------------------------------------------
// TEST[continued]

The following example retrieves all API keys if the user is authorized to do so:
[source,console]
--------------------------------------------------
GET /_security/api_key
--------------------------------------------------
// TEST[continued]

Following creates an API key

[source,console]
Expand Down
12 changes: 7 additions & 5 deletions ...gin/core/src/main/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,14 +133,16 @@ public static GetApiKeyRequest forOwnedApiKeys() {
return new GetApiKeyRequest(null, null, null, null, true);
}

/**
* Creates get api key request to retrieve api key information for all api keys if the authenticated user is authorized to do so.
*/
public static GetApiKeyRequest forAllApiKeys() {
return new GetApiKeyRequest();
}

@Override
public ActionRequestValidationException validate() {
ActionRequestValidationException validationException = null;
if (Strings.hasText(realmName) == false && Strings.hasText(userName) == false && Strings.hasText(apiKeyId) == false
&& Strings.hasText(apiKeyName) == false && ownedByAuthenticatedUser == false) {
validationException = addValidationError("One of [api key id, api key name, username, realm name] must be specified if " +
"[owner] flag is false", validationException);
}
if (Strings.hasText(apiKeyId) || Strings.hasText(apiKeyName)) {
if (Strings.hasText(realmName) || Strings.hasText(userName)) {
validationException = addValidationError(
Expand Down
3 changes: 0 additions & 3 deletions ...ore/src/test/java/org/elasticsearch/xpack/core/security/action/GetApiKeyRequestTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,6 @@ public void writeTo(StreamOutput out) throws IOException {
}

String[][] inputs = new String[][]{
{randomNullOrEmptyString(), randomNullOrEmptyString(), randomNullOrEmptyString(),
randomNullOrEmptyString(), "false"},
{randomNullOrEmptyString(), "user", "api-kid", "api-kname", "false"},
{"realm", randomNullOrEmptyString(), "api-kid", "api-kname", "false"},
{"realm", "user", "api-kid", randomNullOrEmptyString(), "false"},
Expand All @@ -86,7 +84,6 @@ public void writeTo(StreamOutput out) throws IOException {
{randomNullOrEmptyString(), "user", randomNullOrEmptyString(), randomNullOrEmptyString(), "true"}
};
String[][] expectedErrorMessages = new String[][]{
{"One of [api key id, api key name, username, realm name] must be specified if [owner] flag is false"},
{"username or realm name must not be specified when the api key id or api key name is specified",
"only one of [api key id, api key name] can be specified"},
{"username or realm name must not be specified when the api key id or api key name is specified",
Expand Down
26 changes: 10 additions & 16 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 @@ -881,22 +881,16 @@ private void maybeStartApiKeyRemover() {
public void getApiKeys(String realmName, String username, String apiKeyName, String apiKeyId,
ActionListener<GetApiKeyResponse> listener) {
ensureEnabled();
if (Strings.hasText(realmName) == false && Strings.hasText(username) == false && Strings.hasText(apiKeyName) == false
&& Strings.hasText(apiKeyId) == false) {
logger.trace("none of the parameters [api key id, api key name, username, realm name] were specified for retrieval");
listener.onFailure(new IllegalArgumentException("One of [api key id, api key name, username, realm name] must be specified"));
} else {
findApiKeysForUserRealmApiKeyIdAndNameCombination(realmName, username, apiKeyName, apiKeyId, false, false,
ActionListener.wrap(apiKeyInfos -> {
if (apiKeyInfos.isEmpty()) {
logger.debug("No active api keys found for realm [{}], user [{}], api key name [{}] and api key id [{}]",
realmName, username, apiKeyName, apiKeyId);
listener.onResponse(GetApiKeyResponse.emptyResponse());
} else {
listener.onResponse(new GetApiKeyResponse(apiKeyInfos));
}
}, listener::onFailure));
}
findApiKeysForUserRealmApiKeyIdAndNameCombination(realmName, username, apiKeyName, apiKeyId, false, false,
ActionListener.wrap(apiKeyInfos -> {
if (apiKeyInfos.isEmpty()) {
logger.debug("No active api keys found for realm [{}], user [{}], api key name [{}] and api key id [{}]",
realmName, username, apiKeyName, apiKeyId);
listener.onResponse(GetApiKeyResponse.emptyResponse());
} else {
listener.onResponse(new GetApiKeyResponse(apiKeyInfos));
}
}, listener::onFailure));
}

/**
Expand Down
58 changes: 58 additions & 0 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 @@ -53,6 +53,7 @@
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static org.elasticsearch.index.mapper.MapperService.SINGLE_MAPPING_NAME;
import static org.elasticsearch.xpack.core.security.index.RestrictedIndicesNames.SECURITY_MAIN_ALIAS;
Expand Down Expand Up @@ -93,6 +94,8 @@ public void wipeSecurityIndex() throws Exception {
@Override
public String configRoles() {
return super.configRoles() + "\n" +
"no_api_key_role:\n" +
" cluster: [\"manage_token\"]\n" +
"manage_api_key_role:\n" +
" cluster: [\"manage_api_key\"]\n" +
"manage_own_api_key_role:\n" +
Expand All @@ -104,13 +107,15 @@ public String configUsers() {
final String usersPasswdHashed = new String(
getFastStoredHashAlgoForTests().hash(SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING));
return super.configUsers() +
"user_with_no_api_key_role:" + usersPasswdHashed + "\n" +
"user_with_manage_api_key_role:" + usersPasswdHashed + "\n" +
"user_with_manage_own_api_key_role:" + usersPasswdHashed + "\n";
}

@Override
public String configUsersRoles() {
return super.configUsersRoles() +
"no_api_key_role:user_with_no_api_key_role\n" +
"manage_api_key_role:user_with_manage_api_key_role\n" +
"manage_own_api_key_role:user_with_manage_own_api_key_role\n";
}
Expand Down Expand Up @@ -549,6 +554,49 @@ public void testGetApiKeysOwnedByCurrentAuthenticatedUser() throws InterruptedEx
response, userWithManageApiKeyRoleApiKeys.stream().map(o -> o.getId()).collect(Collectors.toSet()), null);
}

public void testGetAllApiKeys() throws InterruptedException, ExecutionException {
int noOfSuperuserApiKeys = randomIntBetween(3, 5);
int noOfApiKeysForUserWithManageApiKeyRole = randomIntBetween(3, 5);
int noOfApiKeysForUserWithManageOwnApiKeyRole = randomIntBetween(3,7);
List<CreateApiKeyResponse> defaultUserCreatedKeys = createApiKeys(noOfSuperuserApiKeys, null);
List<CreateApiKeyResponse> userWithManageApiKeyRoleApiKeys = createApiKeys("user_with_manage_api_key_role",
noOfApiKeysForUserWithManageApiKeyRole, null, "monitor");
List<CreateApiKeyResponse> userWithManageOwnApiKeyRoleApiKeys = createApiKeys("user_with_manage_own_api_key_role",
noOfApiKeysForUserWithManageOwnApiKeyRole, null, "monitor");

final Client client = client().filterWithHeader(Collections.singletonMap("Authorization", UsernamePasswordToken
.basicAuthHeaderValue("user_with_manage_api_key_role", SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING)));
PlainActionFuture<GetApiKeyResponse> listener = new PlainActionFuture<>();
client.execute(GetApiKeyAction.INSTANCE, new GetApiKeyRequest(), listener);
GetApiKeyResponse response = listener.get();
int totalApiKeys = noOfSuperuserApiKeys + noOfApiKeysForUserWithManageApiKeyRole + noOfApiKeysForUserWithManageOwnApiKeyRole;
List<CreateApiKeyResponse> allApiKeys = new ArrayList<>();
Stream.of(defaultUserCreatedKeys, userWithManageApiKeyRoleApiKeys, userWithManageOwnApiKeyRoleApiKeys).forEach(
allApiKeys::addAll);
verifyGetResponse(new String[]{SecuritySettingsSource.TEST_SUPERUSER, "user_with_manage_api_key_role",
"user_with_manage_own_api_key_role"}, totalApiKeys, allApiKeys, response,
allApiKeys.stream().map(o -> o.getId()).collect(Collectors.toSet()), null);
}

public void testGetAllApiKeysFailsForUserWithNoRoleOrRetrieveOwnApiKeyRole() throws InterruptedException, ExecutionException {
int noOfSuperuserApiKeys = randomIntBetween(3, 5);
int noOfApiKeysForUserWithManageApiKeyRole = randomIntBetween(3, 5);
int noOfApiKeysForUserWithManageOwnApiKeyRole = randomIntBetween(3,7);
List<CreateApiKeyResponse> defaultUserCreatedKeys = createApiKeys(noOfSuperuserApiKeys, null);
List<CreateApiKeyResponse> userWithManageApiKeyRoleApiKeys = createApiKeys("user_with_manage_api_key_role",
noOfApiKeysForUserWithManageApiKeyRole, null, "monitor");
List<CreateApiKeyResponse> userWithManageOwnApiKeyRoleApiKeys = createApiKeys("user_with_manage_own_api_key_role",
noOfApiKeysForUserWithManageOwnApiKeyRole, null, "monitor");

final String withUser = randomFrom("user_with_manage_own_api_key_role", "user_with_no_api_key_role");
final Client client = client().filterWithHeader(Collections.singletonMap("Authorization", UsernamePasswordToken
.basicAuthHeaderValue(withUser, SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING)));
PlainActionFuture<GetApiKeyResponse> listener = new PlainActionFuture<>();
client.execute(GetApiKeyAction.INSTANCE, new GetApiKeyRequest(), listener);
ElasticsearchSecurityException ese = expectThrows(ElasticsearchSecurityException.class, () -> listener.actionGet());
assertErrorMessage(ese, "cluster:admin/xpack/security/api_key/get", withUser);
}

public void testInvalidateApiKeysOwnedByCurrentAuthenticatedUser() throws InterruptedException, ExecutionException {
int noOfSuperuserApiKeys = randomIntBetween(3, 5);
int noOfApiKeysForUserWithManageApiKeyRole = randomIntBetween(3, 5);
Expand Down Expand Up @@ -632,6 +680,11 @@ private void verifyGetResponse(int expectedNumberOfApiKeys, List<CreateApiKeyRes

private void verifyGetResponse(String user, int expectedNumberOfApiKeys, List<CreateApiKeyResponse> responses,
GetApiKeyResponse response, Set<String> validApiKeyIds, List<String> invalidatedApiKeyIds) {
verifyGetResponse(new String[]{user}, expectedNumberOfApiKeys, responses, response, validApiKeyIds, invalidatedApiKeyIds);
}

private void verifyGetResponse(String[] user, int expectedNumberOfApiKeys, List<CreateApiKeyResponse> responses,
GetApiKeyResponse response, Set<String> validApiKeyIds, List<String> invalidatedApiKeyIds) {
assertThat(response.getApiKeyInfos().length, equalTo(expectedNumberOfApiKeys));
List<String> expectedIds = responses.stream().filter(o -> validApiKeyIds.contains(o.getId())).map(o -> o.getId())
.collect(Collectors.toList());
Expand Down Expand Up @@ -680,4 +733,9 @@ private void assertErrorMessage(final ElasticsearchSecurityException ese, String
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 + "]"));
}
}

0 comments on commit 01f9177

Please sign in to comment.