Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support to retrieve all API keys if user has privilege #47274

Merged
merged 3 commits into from
Oct 7, 2019
Merged

Add support to retrieve all API keys if user has privilege #47274

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8b2e516
Add support to retrieve all API keys if user has privilege
Sep 30, 2019
bfa49ab
address review comment, update documentation
Oct 7, 2019
c26efe2
Merge branch 'master' into get-all-api-keys
Oct 7, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions client/rest-high-level/src/main/java/org/elasticsearch/client/security/GetApiKeyRequest.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
12 changes: 10 additions & 2 deletions x-pack/docs/en/rest-api/security/get-api-keys.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,9 @@ 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 to do so.
bizybot marked this conversation as resolved.
Show resolved Hide resolved

[[security-api-get-api-key-example]]
==== {api-examples-title}
Expand Down Expand Up @@ -123,6 +124,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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,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 @@ -92,6 +93,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 @@ -103,13 +106,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 @@ -521,6 +526,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 @@ -604,6 +652,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 @@ -652,4 +705,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 + "]"));
}
}