Skip to content

Commit 9e61a9c

Browse files
bizybotbizybot
authored
Simplify API key service API (#44935)
This commit merely refactors API key service interface for retrieving and invalidating API keys. The service layer need not do any authorization so we do not need multiple interfaces to retrieve or invalidate API keys but one interface to do each operation. Relates #40031
1 parent 185c583 commit 9e61a9c

File tree

3 files changed

+57
-174
lines changed

3 files changed

+57
-174
lines changed

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportGetApiKeyAction.java

+1-10
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import org.elasticsearch.action.ActionListener;
1010
import org.elasticsearch.action.support.ActionFilters;
1111
import org.elasticsearch.action.support.HandledTransportAction;
12-
import org.elasticsearch.common.Strings;
1312
import org.elasticsearch.common.inject.Inject;
1413
import org.elasticsearch.common.io.stream.Writeable;
1514
import org.elasticsearch.tasks.Task;
@@ -32,15 +31,7 @@ public TransportGetApiKeyAction(TransportService transportService, ActionFilters
3231

3332
@Override
3433
protected void doExecute(Task task, GetApiKeyRequest request, ActionListener<GetApiKeyResponse> listener) {
35-
if (Strings.hasText(request.getRealmName()) || Strings.hasText(request.getUserName())) {
36-
apiKeyService.getApiKeysForRealmAndUser(request.getRealmName(), request.getUserName(), listener);
37-
} else if (Strings.hasText(request.getApiKeyId())) {
38-
apiKeyService.getApiKeyForApiKeyId(request.getApiKeyId(), listener);
39-
} else if (Strings.hasText(request.getApiKeyName())) {
40-
apiKeyService.getApiKeyForApiKeyName(request.getApiKeyName(), listener);
41-
} else {
42-
listener.onFailure(new IllegalArgumentException("One of [api key id, api key name, username, realm name] must be specified"));
43-
}
34+
apiKeyService.getApiKeys(request.getRealmName(), request.getUserName(), request.getApiKeyName(), request.getApiKeyId(), listener);
4435
}
4536

4637
}

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/TransportInvalidateApiKeyAction.java

+1-8
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import org.elasticsearch.action.ActionListener;
1010
import org.elasticsearch.action.support.ActionFilters;
1111
import org.elasticsearch.action.support.HandledTransportAction;
12-
import org.elasticsearch.common.Strings;
1312
import org.elasticsearch.common.inject.Inject;
1413
import org.elasticsearch.common.io.stream.Writeable;
1514
import org.elasticsearch.tasks.Task;
@@ -32,13 +31,7 @@ public TransportInvalidateApiKeyAction(TransportService transportService, Action
3231

3332
@Override
3433
protected void doExecute(Task task, InvalidateApiKeyRequest request, ActionListener<InvalidateApiKeyResponse> listener) {
35-
if (Strings.hasText(request.getRealmName()) || Strings.hasText(request.getUserName())) {
36-
apiKeyService.invalidateApiKeysForRealmAndUser(request.getRealmName(), request.getUserName(), listener);
37-
} else if (Strings.hasText(request.getId())) {
38-
apiKeyService.invalidateApiKeyForApiKeyId(request.getId(), listener);
39-
} else {
40-
apiKeyService.invalidateApiKeyForApiKeyName(request.getName(), listener);
41-
}
34+
apiKeyService.invalidateApiKeys(request.getRealmName(), request.getUserName(), request.getName(), request.getId(), listener);
4235
}
4336

4437
}

‎x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java

+55-156
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,6 @@
7171
import org.elasticsearch.xpack.core.security.user.User;
7272
import org.elasticsearch.xpack.security.support.SecurityIndexManager;
7373

74-
import javax.crypto.SecretKeyFactory;
7574
import java.io.Closeable;
7675
import java.io.IOException;
7776
import java.io.UncheckedIOException;
@@ -95,6 +94,8 @@
9594
import java.util.function.Function;
9695
import java.util.stream.Collectors;
9796

97+
import javax.crypto.SecretKeyFactory;
98+
9899
import static org.elasticsearch.index.mapper.MapperService.SINGLE_MAPPING_NAME;
99100
import static org.elasticsearch.search.SearchService.DEFAULT_KEEPALIVE_SETTING;
100101
import static org.elasticsearch.xpack.core.ClientHelper.SECURITY_ORIGIN;
@@ -105,7 +106,7 @@ public class ApiKeyService {
105106

106107
private static final Logger logger = LogManager.getLogger(ApiKeyService.class);
107108
private static final DeprecationLogger deprecationLogger = new DeprecationLogger(logger);
108-
static final String API_KEY_ID_KEY = "_security_api_key_id";
109+
public static final String API_KEY_ID_KEY = "_security_api_key_id";
109110
static final String API_KEY_ROLE_DESCRIPTORS_KEY = "_security_api_key_role_descriptors";
110111
static final String API_KEY_LIMITED_ROLE_DESCRIPTORS_KEY = "_security_api_key_limited_by_role_descriptors";
111112

@@ -639,98 +640,41 @@ public void usedDeprecatedField(String usedName, String replacedWith) {
639640
}
640641

641642
/**
642-
* Invalidate API keys for given realm and user name.
643+
* Invalidate API keys for given realm, user name, API key name and id.
643644
* @param realmName realm name
644-
* @param userName user name
645+
* @param username user name
646+
* @param apiKeyName API key name
647+
* @param apiKeyId API key id
645648
* @param invalidateListener listener for {@link InvalidateApiKeyResponse}
646649
*/
647-
public void invalidateApiKeysForRealmAndUser(String realmName, String userName,
648-
ActionListener<InvalidateApiKeyResponse> invalidateListener) {
650+
public void invalidateApiKeys(String realmName, String username, String apiKeyName, String apiKeyId,
651+
ActionListener<InvalidateApiKeyResponse> invalidateListener) {
649652
ensureEnabled();
650-
if (Strings.hasText(realmName) == false && Strings.hasText(userName) == false) {
651-
logger.trace("No realm name or username provided");
652-
invalidateListener.onFailure(new IllegalArgumentException("realm name or username must be provided"));
653+
if (Strings.hasText(realmName) == false && Strings.hasText(username) == false && Strings.hasText(apiKeyName) == false
654+
&& Strings.hasText(apiKeyId) == false) {
655+
logger.trace("none of the parameters [api key id, api key name, username, realm name] were specified for invalidation");
656+
invalidateListener
657+
.onFailure(new IllegalArgumentException("One of [api key id, api key name, username, realm name] must be specified"));
653658
} else {
654-
findApiKeysForUserAndRealm(userName, realmName, true, false, ActionListener.wrap(apiKeyIds -> {
655-
if (apiKeyIds.isEmpty()) {
656-
logger.warn("No active api keys to invalidate for realm [{}] and username [{}]", realmName, userName);
657-
invalidateListener.onResponse(InvalidateApiKeyResponse.emptyResponse());
658-
} else {
659-
invalidateAllApiKeys(apiKeyIds.stream().map(apiKey -> apiKey.getId()).collect(Collectors.toSet()), invalidateListener);
660-
}
661-
}, invalidateListener::onFailure));
659+
findApiKeysForUserRealmApiKeyIdAndNameCombination(realmName, username, apiKeyName, apiKeyId, true, false,
660+
ActionListener.wrap(apiKeys -> {
661+
if (apiKeys.isEmpty()) {
662+
logger.debug(
663+
"No active api keys to invalidate for realm [{}], username [{}], api key name [{}] and api key id [{}]",
664+
realmName, username, apiKeyName, apiKeyId);
665+
invalidateListener.onResponse(InvalidateApiKeyResponse.emptyResponse());
666+
} else {
667+
invalidateAllApiKeys(apiKeys.stream().map(apiKey -> apiKey.getId()).collect(Collectors.toSet()),
668+
invalidateListener);
669+
}
670+
}, invalidateListener::onFailure));
662671
}
663672
}
664673

665674
private void invalidateAllApiKeys(Collection<String> apiKeyIds, ActionListener<InvalidateApiKeyResponse> invalidateListener) {
666675
indexInvalidation(apiKeyIds, invalidateListener, null);
667676
}
668677

669-
/**
670-
* Invalidate API key for given API key id
671-
* @param apiKeyId API key id
672-
* @param invalidateListener listener for {@link InvalidateApiKeyResponse}
673-
*/
674-
public void invalidateApiKeyForApiKeyId(String apiKeyId, ActionListener<InvalidateApiKeyResponse> invalidateListener) {
675-
ensureEnabled();
676-
if (Strings.hasText(apiKeyId) == false) {
677-
logger.trace("No api key id provided");
678-
invalidateListener.onFailure(new IllegalArgumentException("api key id must be provided"));
679-
} else {
680-
findApiKeysForApiKeyId(apiKeyId, true, false, ActionListener.wrap(apiKeyIds -> {
681-
if (apiKeyIds.isEmpty()) {
682-
logger.warn("No api key to invalidate for api key id [{}]", apiKeyId);
683-
invalidateListener.onResponse(InvalidateApiKeyResponse.emptyResponse());
684-
} else {
685-
invalidateAllApiKeys(apiKeyIds.stream().map(apiKey -> apiKey.getId()).collect(Collectors.toSet()), invalidateListener);
686-
}
687-
}, invalidateListener::onFailure));
688-
}
689-
}
690-
691-
/**
692-
* Invalidate API key for given API key name
693-
* @param apiKeyName API key name
694-
* @param invalidateListener listener for {@link InvalidateApiKeyResponse}
695-
*/
696-
public void invalidateApiKeyForApiKeyName(String apiKeyName, ActionListener<InvalidateApiKeyResponse> invalidateListener) {
697-
ensureEnabled();
698-
if (Strings.hasText(apiKeyName) == false) {
699-
logger.trace("No api key name provided");
700-
invalidateListener.onFailure(new IllegalArgumentException("api key name must be provided"));
701-
} else {
702-
findApiKeyForApiKeyName(apiKeyName, true, false, ActionListener.wrap(apiKeyIds -> {
703-
if (apiKeyIds.isEmpty()) {
704-
logger.warn("No api key to invalidate for api key name [{}]", apiKeyName);
705-
invalidateListener.onResponse(InvalidateApiKeyResponse.emptyResponse());
706-
} else {
707-
invalidateAllApiKeys(apiKeyIds.stream().map(apiKey -> apiKey.getId()).collect(Collectors.toSet()), invalidateListener);
708-
}
709-
}, invalidateListener::onFailure));
710-
}
711-
}
712-
713-
private void findApiKeysForUserAndRealm(String userName, String realmName, boolean filterOutInvalidatedKeys,
714-
boolean filterOutExpiredKeys, ActionListener<Collection<ApiKey>> listener) {
715-
final SecurityIndexManager frozenSecurityIndex = securityIndex.freeze();
716-
if (frozenSecurityIndex.indexExists() == false) {
717-
listener.onResponse(Collections.emptyList());
718-
} else if (frozenSecurityIndex.isAvailable() == false) {
719-
listener.onFailure(frozenSecurityIndex.getUnavailableReason());
720-
} else {
721-
final BoolQueryBuilder boolQuery = QueryBuilders.boolQuery()
722-
.filter(QueryBuilders.termQuery("doc_type", "api_key"));
723-
if (Strings.hasText(userName)) {
724-
boolQuery.filter(QueryBuilders.termQuery("creator.principal", userName));
725-
}
726-
if (Strings.hasText(realmName)) {
727-
boolQuery.filter(QueryBuilders.termQuery("creator.realm", realmName));
728-
}
729-
730-
findApiKeys(boolQuery, filterOutInvalidatedKeys, filterOutExpiredKeys, listener);
731-
}
732-
}
733-
734678
private void findApiKeys(final BoolQueryBuilder boolQuery, boolean filterOutInvalidatedKeys, boolean filterOutExpiredKeys,
735679
ActionListener<Collection<ApiKey>> listener) {
736680
if (filterOutInvalidatedKeys) {
@@ -767,35 +711,28 @@ private void findApiKeys(final BoolQueryBuilder boolQuery, boolean filterOutInva
767711
}
768712
}
769713

770-
private void findApiKeyForApiKeyName(String apiKeyName, boolean filterOutInvalidatedKeys, boolean filterOutExpiredKeys,
771-
ActionListener<Collection<ApiKey>> listener) {
714+
private void findApiKeysForUserRealmApiKeyIdAndNameCombination(String realmName, String userName, String apiKeyName, String apiKeyId,
715+
boolean filterOutInvalidatedKeys, boolean filterOutExpiredKeys,
716+
ActionListener<Collection<ApiKey>> listener) {
772717
final SecurityIndexManager frozenSecurityIndex = securityIndex.freeze();
773718
if (frozenSecurityIndex.indexExists() == false) {
774719
listener.onResponse(Collections.emptyList());
775720
} else if (frozenSecurityIndex.isAvailable() == false) {
776721
listener.onFailure(frozenSecurityIndex.getUnavailableReason());
777722
} else {
778-
final BoolQueryBuilder boolQuery = QueryBuilders.boolQuery()
779-
.filter(QueryBuilders.termQuery("doc_type", "api_key"));
723+
final BoolQueryBuilder boolQuery = QueryBuilders.boolQuery().filter(QueryBuilders.termQuery("doc_type", "api_key"));
724+
if (Strings.hasText(realmName)) {
725+
boolQuery.filter(QueryBuilders.termQuery("creator.realm", realmName));
726+
}
727+
if (Strings.hasText(userName)) {
728+
boolQuery.filter(QueryBuilders.termQuery("creator.principal", userName));
729+
}
780730
if (Strings.hasText(apiKeyName)) {
781731
boolQuery.filter(QueryBuilders.termQuery("name", apiKeyName));
782732
}
783-
784-
findApiKeys(boolQuery, filterOutInvalidatedKeys, filterOutExpiredKeys, listener);
785-
}
786-
}
787-
788-
private void findApiKeysForApiKeyId(String apiKeyId, boolean filterOutInvalidatedKeys, boolean filterOutExpiredKeys,
789-
ActionListener<Collection<ApiKey>> listener) {
790-
final SecurityIndexManager frozenSecurityIndex = securityIndex.freeze();
791-
if (frozenSecurityIndex.indexExists() == false) {
792-
listener.onResponse(Collections.emptyList());
793-
} else if (frozenSecurityIndex.isAvailable() == false) {
794-
listener.onFailure(frozenSecurityIndex.getUnavailableReason());
795-
} else {
796-
final BoolQueryBuilder boolQuery = QueryBuilders.boolQuery()
797-
.filter(QueryBuilders.termQuery("doc_type", "api_key"))
798-
.filter(QueryBuilders.termQuery("_id", apiKeyId));
733+
if (Strings.hasText(apiKeyId)) {
734+
boolQuery.filter(QueryBuilders.termQuery("_id", apiKeyId));
735+
}
799736

800737
findApiKeys(boolQuery, filterOutInvalidatedKeys, filterOutExpiredKeys, listener);
801738
}
@@ -818,9 +755,9 @@ private void indexInvalidation(Collection<String> apiKeyIds, ActionListener<Inva
818755
BulkRequestBuilder bulkRequestBuilder = client.prepareBulk();
819756
for (String apiKeyId : apiKeyIds) {
820757
UpdateRequest request = client
821-
.prepareUpdate(SECURITY_MAIN_ALIAS, SINGLE_MAPPING_NAME, apiKeyId)
822-
.setDoc(Collections.singletonMap("api_key_invalidated", true))
823-
.request();
758+
.prepareUpdate(SECURITY_MAIN_ALIAS, SINGLE_MAPPING_NAME, apiKeyId)
759+
.setDoc(Collections.singletonMap("api_key_invalidated", true))
760+
.request();
824761
bulkRequestBuilder.add(request);
825762
}
826763
bulkRequestBuilder.setRefreshPolicy(RefreshPolicy.WAIT_UNTIL);
@@ -924,64 +861,26 @@ private void maybeStartApiKeyRemover() {
924861
}
925862

926863
/**
927-
* Get API keys for given realm and user name.
864+
* Get API key information for given realm, user, API key name and id combination
928865
* @param realmName realm name
929-
* @param userName user name
930-
* @param listener listener for {@link GetApiKeyResponse}
931-
*/
932-
public void getApiKeysForRealmAndUser(String realmName, String userName, ActionListener<GetApiKeyResponse> listener) {
933-
ensureEnabled();
934-
if (Strings.hasText(realmName) == false && Strings.hasText(userName) == false) {
935-
logger.trace("No realm name or username provided");
936-
listener.onFailure(new IllegalArgumentException("realm name or username must be provided"));
937-
} else {
938-
findApiKeysForUserAndRealm(userName, realmName, false, false, ActionListener.wrap(apiKeyInfos -> {
939-
if (apiKeyInfos.isEmpty()) {
940-
logger.warn("No active api keys found for realm [{}] and username [{}]", realmName, userName);
941-
listener.onResponse(GetApiKeyResponse.emptyResponse());
942-
} else {
943-
listener.onResponse(new GetApiKeyResponse(apiKeyInfos));
944-
}
945-
}, listener::onFailure));
946-
}
947-
}
948-
949-
/**
950-
* Get API key for given API key id
951-
* @param apiKeyId API key id
952-
* @param listener listener for {@link GetApiKeyResponse}
953-
*/
954-
public void getApiKeyForApiKeyId(String apiKeyId, ActionListener<GetApiKeyResponse> listener) {
955-
ensureEnabled();
956-
if (Strings.hasText(apiKeyId) == false) {
957-
logger.trace("No api key id provided");
958-
listener.onFailure(new IllegalArgumentException("api key id must be provided"));
959-
} else {
960-
findApiKeysForApiKeyId(apiKeyId, false, false, ActionListener.wrap(apiKeyInfos -> {
961-
if (apiKeyInfos.isEmpty()) {
962-
logger.warn("No api key found for api key id [{}]", apiKeyId);
963-
listener.onResponse(GetApiKeyResponse.emptyResponse());
964-
} else {
965-
listener.onResponse(new GetApiKeyResponse(apiKeyInfos));
966-
}
967-
}, listener::onFailure));
968-
}
969-
}
970-
971-
/**
972-
* Get API key for given API key name
866+
* @param username user name
973867
* @param apiKeyName API key name
868+
* @param apiKeyId API key id
974869
* @param listener listener for {@link GetApiKeyResponse}
975870
*/
976-
public void getApiKeyForApiKeyName(String apiKeyName, ActionListener<GetApiKeyResponse> listener) {
871+
public void getApiKeys(String realmName, String username, String apiKeyName, String apiKeyId,
872+
ActionListener<GetApiKeyResponse> listener) {
977873
ensureEnabled();
978-
if (Strings.hasText(apiKeyName) == false) {
979-
logger.trace("No api key name provided");
980-
listener.onFailure(new IllegalArgumentException("api key name must be provided"));
874+
if (Strings.hasText(realmName) == false && Strings.hasText(username) == false && Strings.hasText(apiKeyName) == false
875+
&& Strings.hasText(apiKeyId) == false) {
876+
logger.trace("none of the parameters [api key id, api key name, username, realm name] were specified for retrieval");
877+
listener.onFailure(new IllegalArgumentException("One of [api key id, api key name, username, realm name] must be specified"));
981878
} else {
982-
findApiKeyForApiKeyName(apiKeyName, false, false, ActionListener.wrap(apiKeyInfos -> {
879+
findApiKeysForUserRealmApiKeyIdAndNameCombination(realmName, username, apiKeyName, apiKeyId, false, false,
880+
ActionListener.wrap(apiKeyInfos -> {
983881
if (apiKeyInfos.isEmpty()) {
984-
logger.warn("No api key found for api key name [{}]", apiKeyName);
882+
logger.debug("No active api keys found for realm [{}], user [{}], api key name [{}] and api key id [{}]",
883+
realmName, username, apiKeyName, apiKeyId);
985884
listener.onResponse(GetApiKeyResponse.emptyResponse());
986885
} else {
987886
listener.onResponse(new GetApiKeyResponse(apiKeyInfos));

0 commit comments

Comments
 (0)