Skip to content

Commit e7a84b1

Browse files
ywangdywangd
authored
Remove deprecated convenient methods from GetApiKeyRequest (#89360)
A builder class for GetApiKeyRequest is added as part of #89273. As a result, the existing convenient methods got deprecated in favour of the builder. This PR removes the deprecated methods and replaces all usages with the builder.
1 parent f87ce07 commit e7a84b1

File tree

7 files changed

+58
-97
lines changed

7 files changed

+58
-97
lines changed

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/action/apikey/GetApiKeyRequest.java

Lines changed: 0 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -94,76 +94,6 @@ public boolean withLimitedBy() {
9494
return withLimitedBy;
9595
}
9696

97-
/**
98-
* Creates get API key request for given realm name
99-
* @param realmName realm name
100-
* @return {@link GetApiKeyRequest}
101-
*/
102-
@Deprecated
103-
public static GetApiKeyRequest usingRealmName(String realmName) {
104-
return new GetApiKeyRequest(realmName, null, null, null, false, false);
105-
}
106-
107-
/**
108-
* Creates get API key request for given user name
109-
* @param userName user name
110-
* @return {@link GetApiKeyRequest}
111-
*/
112-
@Deprecated
113-
public static GetApiKeyRequest usingUserName(String userName) {
114-
return new GetApiKeyRequest(null, userName, null, null, false, false);
115-
}
116-
117-
/**
118-
* Creates get API key request for given realm and user name
119-
* @param realmName realm name
120-
* @param userName user name
121-
* @return {@link GetApiKeyRequest}
122-
*/
123-
@Deprecated
124-
public static GetApiKeyRequest usingRealmAndUserName(String realmName, String userName) {
125-
return new GetApiKeyRequest(realmName, userName, null, null, false, false);
126-
}
127-
128-
/**
129-
* Creates get API key request for given api key id
130-
* @param apiKeyId api key id
131-
* @param ownedByAuthenticatedUser set {@code true} if the request is only for the API keys owned by current authenticated user else
132-
* {@code false}
133-
* @return {@link GetApiKeyRequest}
134-
*/
135-
@Deprecated
136-
public static GetApiKeyRequest usingApiKeyId(String apiKeyId, boolean ownedByAuthenticatedUser) {
137-
return new GetApiKeyRequest(null, null, apiKeyId, null, ownedByAuthenticatedUser, false);
138-
}
139-
140-
/**
141-
* Creates get api key request for given api key name
142-
* @param apiKeyName api key name
143-
* @param ownedByAuthenticatedUser set {@code true} if the request is only for the API keys owned by current authenticated user else
144-
* {@code false}
145-
* @return {@link GetApiKeyRequest}
146-
*/
147-
public static GetApiKeyRequest usingApiKeyName(String apiKeyName, boolean ownedByAuthenticatedUser) {
148-
return new GetApiKeyRequest(null, null, null, apiKeyName, ownedByAuthenticatedUser, false);
149-
}
150-
151-
/**
152-
* Creates get api key request to retrieve api key information for the api keys owned by the current authenticated user.
153-
*/
154-
@Deprecated
155-
public static GetApiKeyRequest forOwnedApiKeys() {
156-
return new GetApiKeyRequest(null, null, null, null, true, false);
157-
}
158-
159-
/**
160-
* Creates get api key request to retrieve api key information for all api keys if the authenticated user is authorized to do so.
161-
*/
162-
@Deprecated
163-
public static GetApiKeyRequest forAllApiKeys() {
164-
return GetApiKeyRequest.builder().build();
165-
}
166-
16797
@Override
16898
public ActionRequestValidationException validate() {
16999
ActionRequestValidationException validationException = null;

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/apikey/GetApiKeyRequestTests.java

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,22 @@
2929
public class GetApiKeyRequestTests extends ESTestCase {
3030

3131
public void testRequestValidation() {
32-
GetApiKeyRequest request = GetApiKeyRequest.usingApiKeyId(randomAlphaOfLength(5), randomBoolean());
32+
GetApiKeyRequest request = GetApiKeyRequest.builder()
33+
.apiKeyId(randomAlphaOfLength(5))
34+
.ownedByAuthenticatedUser(randomBoolean())
35+
.build();
3336
ActionRequestValidationException ve = request.validate();
3437
assertNull(ve);
35-
request = GetApiKeyRequest.usingApiKeyName(randomAlphaOfLength(5), randomBoolean());
38+
request = GetApiKeyRequest.builder().apiKeyName(randomAlphaOfLength(5)).ownedByAuthenticatedUser(randomBoolean()).build();
3639
ve = request.validate();
3740
assertNull(ve);
38-
request = GetApiKeyRequest.usingRealmName(randomAlphaOfLength(5));
41+
request = GetApiKeyRequest.builder().realmName(randomAlphaOfLength(5)).build();
3942
ve = request.validate();
4043
assertNull(ve);
41-
request = GetApiKeyRequest.usingUserName(randomAlphaOfLength(5));
44+
request = GetApiKeyRequest.builder().userName(randomAlphaOfLength(5)).build();
4245
ve = request.validate();
4346
assertNull(ve);
44-
request = GetApiKeyRequest.usingRealmAndUserName(randomAlphaOfLength(5), randomAlphaOfLength(7));
47+
request = GetApiKeyRequest.builder().realmName(randomAlphaOfLength(5)).userName(randomAlphaOfLength(7)).build();
4548
ve = request.validate();
4649
assertNull(ve);
4750
}
@@ -120,9 +123,8 @@ public void writeTo(StreamOutput out) throws IOException {
120123

121124
public void testSerialization() throws IOException {
122125
final String apiKeyId = randomAlphaOfLength(5);
123-
final boolean ownedByAuthenticatedUser = true;
124-
GetApiKeyRequest getApiKeyRequest = GetApiKeyRequest.usingApiKeyId(apiKeyId, ownedByAuthenticatedUser);
125126
{
127+
final GetApiKeyRequest getApiKeyRequest = GetApiKeyRequest.builder().ownedByAuthenticatedUser(true).apiKeyId(apiKeyId).build();
126128
ByteArrayOutputStream outBuffer = new ByteArrayOutputStream();
127129
OutputStreamStreamOutput out = new OutputStreamStreamOutput(outBuffer);
128130
out.setVersion(randomVersionBetween(random(), Version.V_7_0_0, Version.V_7_3_0));
@@ -137,13 +139,34 @@ public void testSerialization() throws IOException {
137139
assertThat(requestFromInputStream.ownedByAuthenticatedUser(), is(false));
138140
}
139141
{
142+
final GetApiKeyRequest getApiKeyRequest = GetApiKeyRequest.builder()
143+
.apiKeyId(apiKeyId)
144+
.ownedByAuthenticatedUser(true)
145+
.withLimitedBy(randomBoolean())
146+
.build();
147+
ByteArrayOutputStream outBuffer = new ByteArrayOutputStream();
148+
OutputStreamStreamOutput out = new OutputStreamStreamOutput(outBuffer);
149+
out.setVersion(randomVersionBetween(random(), Version.V_7_4_0, Version.V_8_4_0));
150+
getApiKeyRequest.writeTo(out);
151+
152+
InputStreamStreamInput inputStreamStreamInput = new InputStreamStreamInput(new ByteArrayInputStream(outBuffer.toByteArray()));
153+
inputStreamStreamInput.setVersion(randomVersionBetween(random(), Version.V_7_4_0, Version.V_8_4_0));
154+
GetApiKeyRequest requestFromInputStream = new GetApiKeyRequest(inputStreamStreamInput);
155+
156+
assertThat(requestFromInputStream.getApiKeyId(), equalTo(getApiKeyRequest.getApiKeyId()));
157+
assertThat(requestFromInputStream.ownedByAuthenticatedUser(), is(true));
158+
// old version so the default for `withLimitedBy` is false
159+
assertThat(requestFromInputStream.withLimitedBy(), is(false));
160+
}
161+
{
162+
final GetApiKeyRequest getApiKeyRequest = GetApiKeyRequest.builder().apiKeyId(apiKeyId).withLimitedBy(randomBoolean()).build();
140163
ByteArrayOutputStream outBuffer = new ByteArrayOutputStream();
141164
OutputStreamStreamOutput out = new OutputStreamStreamOutput(outBuffer);
142-
out.setVersion(randomVersionBetween(random(), Version.V_7_4_0, Version.CURRENT));
165+
out.setVersion(randomVersionBetween(random(), Version.V_8_5_0, Version.CURRENT));
143166
getApiKeyRequest.writeTo(out);
144167

145168
InputStreamStreamInput inputStreamStreamInput = new InputStreamStreamInput(new ByteArrayInputStream(outBuffer.toByteArray()));
146-
inputStreamStreamInput.setVersion(randomVersionBetween(random(), Version.V_7_4_0, Version.CURRENT));
169+
inputStreamStreamInput.setVersion(randomVersionBetween(random(), Version.V_8_5_0, Version.CURRENT));
147170
GetApiKeyRequest requestFromInputStream = new GetApiKeyRequest(inputStreamStreamInput);
148171

149172
assertThat(requestFromInputStream, equalTo(getApiKeyRequest));

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStoreTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,7 @@ public void testKibanaSystemRole() {
416416
final CreateApiKeyRequest createApiKeyRequest = new CreateApiKeyRequest(randomAlphaOfLength(8), null, null);
417417
assertThat(kibanaRole.cluster().check(CreateApiKeyAction.NAME, createApiKeyRequest, authentication), is(true));
418418
// Can only get and query its own API keys
419-
assertThat(kibanaRole.cluster().check(GetApiKeyAction.NAME, GetApiKeyRequest.forAllApiKeys(), authentication), is(false));
419+
assertThat(kibanaRole.cluster().check(GetApiKeyAction.NAME, GetApiKeyRequest.builder().build(), authentication), is(false));
420420
assertThat(
421421
kibanaRole.cluster().check(GetApiKeyAction.NAME, GetApiKeyRequest.builder().ownedByAuthenticatedUser().build(), authentication),
422422
is(true)

‎x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/ApiKeyIntegTests.java

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -475,7 +475,7 @@ public void testInvalidatedApiKeysDeletedByRemover() throws Exception {
475475
refreshSecurityIndex();
476476

477477
PlainActionFuture<GetApiKeyResponse> getApiKeyResponseListener = new PlainActionFuture<>();
478-
client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.usingRealmName("file"), getApiKeyResponseListener);
478+
client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.builder().realmName("file").build(), getApiKeyResponseListener);
479479
Set<String> expectedKeyIds = Sets.newHashSet(createdApiKeys.get(0).getId(), createdApiKeys.get(1).getId());
480480
boolean apiKeyInvalidatedButNotYetDeletedByExpiredApiKeysRemover = false;
481481
for (ApiKey apiKey : getApiKeyResponseListener.get().getApiKeyInfos()) {
@@ -513,7 +513,7 @@ public void testInvalidatedApiKeysDeletedByRemover() throws Exception {
513513
// Verify that 1st invalidated API key is deleted whereas the next one may be or may not be as it depends on whether update was
514514
// indexed before ExpiredApiKeysRemover ran
515515
getApiKeyResponseListener = new PlainActionFuture<>();
516-
client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.usingRealmName("file"), getApiKeyResponseListener);
516+
client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.builder().realmName("file").build(), getApiKeyResponseListener);
517517
expectedKeyIds = Sets.newHashSet(createdApiKeys.get(1).getId());
518518
apiKeyInvalidatedButNotYetDeletedByExpiredApiKeysRemover = false;
519519
for (ApiKey apiKey : getApiKeyResponseListener.get().getApiKeyInfos()) {
@@ -558,7 +558,7 @@ public void testExpiredApiKeysBehaviorWhenKeysExpired1WeekBeforeAnd1DayBefore()
558558
Instant created = Instant.now();
559559

560560
PlainActionFuture<GetApiKeyResponse> getApiKeyResponseListener = new PlainActionFuture<>();
561-
client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.usingRealmName("file"), getApiKeyResponseListener);
561+
client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.builder().realmName("file").build(), getApiKeyResponseListener);
562562
assertThat(getApiKeyResponseListener.get().getApiKeyInfos().length, is(noOfKeys));
563563

564564
// Expire the 1st key such that it cannot be deleted by the remover
@@ -596,7 +596,7 @@ public void testExpiredApiKeysBehaviorWhenKeysExpired1WeekBeforeAnd1DayBefore()
596596

597597
// Verify get API keys does not return api keys deleted by ExpiredApiKeysRemover
598598
getApiKeyResponseListener = new PlainActionFuture<>();
599-
client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.usingRealmName("file"), getApiKeyResponseListener);
599+
client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.builder().realmName("file").build(), getApiKeyResponseListener);
600600

601601
Set<String> expectedKeyIds = Sets.newHashSet(
602602
createdApiKeys.get(0).getId(),
@@ -1059,7 +1059,7 @@ public void testGetApiKeysOwnedByRunAsUserWillNotWorkWhenAuthUserInfoIsGiven() t
10591059
);
10601060
getClientForRunAsUser().execute(
10611061
GetApiKeyAction.INSTANCE,
1062-
GetApiKeyRequest.usingRealmAndUserName(invalidRealmAndUserPair.v1(), invalidRealmAndUserPair.v2()),
1062+
GetApiKeyRequest.builder().realmName(invalidRealmAndUserPair.v1()).userName(invalidRealmAndUserPair.v2()).build(),
10631063
listener
10641064
);
10651065
final ElasticsearchSecurityException e = expectThrows(ElasticsearchSecurityException.class, listener::actionGet);
@@ -1156,7 +1156,7 @@ public void testGetAllApiKeysFailsForUserWithNoRoleOrRetrieveOwnApiKeyRole() thr
11561156
Collections.singletonMap("Authorization", basicAuthHeaderValue(withUser, TEST_PASSWORD_SECURE_STRING))
11571157
);
11581158
PlainActionFuture<GetApiKeyResponse> listener = new PlainActionFuture<>();
1159-
client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.forAllApiKeys(), listener);
1159+
client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.builder().build(), listener);
11601160
ElasticsearchSecurityException ese = expectThrows(ElasticsearchSecurityException.class, () -> listener.actionGet());
11611161
assertErrorMessage(ese, "cluster:admin/xpack/security/api_key/get", withUser);
11621162
}
@@ -1298,14 +1298,14 @@ public void testApiKeyAuthorizationApiKeyMustBeAbleToRetrieveItsOwnInformationBu
12981298
final PlainActionFuture<GetApiKeyResponse> failureListener = new PlainActionFuture<>();
12991299
client.execute(
13001300
GetApiKeyAction.INSTANCE,
1301-
GetApiKeyRequest.usingApiKeyId(responses.get(1).getId(), randomBoolean()),
1301+
GetApiKeyRequest.builder().apiKeyId(responses.get(1).getId()).ownedByAuthenticatedUser(randomBoolean()).build(),
13021302
failureListener
13031303
);
13041304
ElasticsearchSecurityException ese = expectThrows(ElasticsearchSecurityException.class, () -> failureListener.actionGet());
13051305
assertErrorMessage(ese, "cluster:admin/xpack/security/api_key/get", ES_TEST_ROOT_USER, responses.get(0).getId());
13061306

13071307
final PlainActionFuture<GetApiKeyResponse> failureListener1 = new PlainActionFuture<>();
1308-
client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.forOwnedApiKeys(), failureListener1);
1308+
client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.builder().ownedByAuthenticatedUser().build(), failureListener1);
13091309
ese = expectThrows(ElasticsearchSecurityException.class, () -> failureListener1.actionGet());
13101310
assertErrorMessage(ese, "cluster:admin/xpack/security/api_key/get", ES_TEST_ROOT_USER, responses.get(0).getId());
13111311
}
@@ -1576,7 +1576,7 @@ public void testApiKeyRunAsAnotherUserCanCreateApiKey() {
15761576

15771577
final GetApiKeyResponse getApiKeyResponse = client.execute(
15781578
GetApiKeyAction.INSTANCE,
1579-
GetApiKeyRequest.usingApiKeyId(response2.getId(), true)
1579+
GetApiKeyRequest.builder().apiKeyId(response2.getId()).ownedByAuthenticatedUser(true).build()
15801580
).actionGet();
15811581
assertThat(getApiKeyResponse.getApiKeyInfos(), arrayWithSize(1));
15821582
final ApiKey apiKeyInfo = getApiKeyResponse.getApiKeyInfos()[0];
@@ -2729,7 +2729,9 @@ private void assertApiKeyNotCreated(Client client, String keyName) throws Execut
27292729
new RefreshRequestBuilder(client, RefreshAction.INSTANCE).setIndices(SECURITY_MAIN_ALIAS).execute().get();
27302730
assertEquals(
27312731
0,
2732-
client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.usingApiKeyName(keyName, false)).get().getApiKeyInfos().length
2732+
client.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.builder().apiKeyName(keyName).ownedByAuthenticatedUser(false).build())
2733+
.get()
2734+
.getApiKeyInfos().length
27332735
);
27342736
}
27352737

‎x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/authc/apikey/ApiKeySingleNodeTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -244,15 +244,15 @@ public void testGetApiKeyWorksForTheApiKeyItself() {
244244
// Can get its own info
245245
final GetApiKeyResponse getApiKeyResponse = clientKey1.execute(
246246
GetApiKeyAction.INSTANCE,
247-
GetApiKeyRequest.usingApiKeyId(apiKeyId, randomBoolean())
247+
GetApiKeyRequest.builder().apiKeyId(apiKeyId).ownedByAuthenticatedUser(randomBoolean()).build()
248248
).actionGet();
249249
assertThat(getApiKeyResponse.getApiKeyInfos().length, equalTo(1));
250250
assertThat(getApiKeyResponse.getApiKeyInfos()[0].getId(), equalTo(apiKeyId));
251251

252252
// Cannot get any other keys
253253
final ElasticsearchSecurityException e = expectThrows(
254254
ElasticsearchSecurityException.class,
255-
() -> clientKey1.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.forAllApiKeys()).actionGet()
255+
() -> clientKey1.execute(GetApiKeyAction.INSTANCE, GetApiKeyRequest.builder().build()).actionGet()
256256
);
257257
assertThat(e.getMessage(), containsString("unauthorized for API key id [" + apiKeyId + "]"));
258258
}

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/service/ElasticServiceAccountsTests.java

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -156,10 +156,13 @@ public void testElasticFleetServerPrivileges() {
156156
.check(CreateApiKeyAction.NAME, new CreateApiKeyRequest(randomAlphaOfLengthBetween(3, 8), null, null), authentication),
157157
is(true)
158158
);
159-
assertThat(role.cluster().check(GetApiKeyAction.NAME, GetApiKeyRequest.forOwnedApiKeys(), authentication), is(true));
159+
assertThat(
160+
role.cluster().check(GetApiKeyAction.NAME, GetApiKeyRequest.builder().ownedByAuthenticatedUser().build(), authentication),
161+
is(true)
162+
);
160163
assertThat(role.cluster().check(InvalidateApiKeyAction.NAME, InvalidateApiKeyRequest.forOwnedApiKeys(), authentication), is(true));
161164

162-
assertThat(role.cluster().check(GetApiKeyAction.NAME, randomFrom(GetApiKeyRequest.forAllApiKeys()), authentication), is(false));
165+
assertThat(role.cluster().check(GetApiKeyAction.NAME, randomFrom(GetApiKeyRequest.builder().build()), authentication), is(false));
163166
assertThat(
164167
role.cluster()
165168
.check(
@@ -320,7 +323,10 @@ public void testElasticEnterpriseSearchServerAccount() {
320323
.check(CreateApiKeyAction.NAME, new CreateApiKeyRequest(randomAlphaOfLengthBetween(3, 8), null, null), authentication),
321324
is(true)
322325
);
323-
assertThat(role.cluster().check(GetApiKeyAction.NAME, GetApiKeyRequest.forOwnedApiKeys(), authentication), is(true));
326+
assertThat(
327+
role.cluster().check(GetApiKeyAction.NAME, GetApiKeyRequest.builder().ownedByAuthenticatedUser().build(), authentication),
328+
is(true)
329+
);
324330
assertThat(role.cluster().check(InvalidateApiKeyAction.NAME, InvalidateApiKeyRequest.forOwnedApiKeys(), authentication), is(true));
325331

326332
assertThat(role.cluster().check(PutUserAction.NAME, request, authentication), is(true));

‎x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/RBACEngineTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -329,15 +329,15 @@ public void testSameUserPermissionDeniesSelfApiKeyInfoRetrievalWithLimitedByWhen
329329
public void testSameUserPermissionDeniesApiKeyInfoRetrievalWhenAuthenticatedByADifferentApiKey() {
330330
final User user = new User("joe");
331331
final String apiKeyId = randomAlphaOfLengthBetween(4, 7);
332-
final TransportRequest request = GetApiKeyRequest.usingApiKeyId(apiKeyId, false);
332+
final TransportRequest request = GetApiKeyRequest.builder().apiKeyId(apiKeyId).ownedByAuthenticatedUser(false).build();
333333
final Authentication authentication = AuthenticationTests.randomApiKeyAuthentication(user, randomAlphaOfLength(8));
334334
assertFalse(RBACEngine.checkSameUserPermissions(GetApiKeyAction.NAME, request, authentication));
335335
}
336336

337337
public void testSameUserPermissionDeniesApiKeyInfoRetrievalWhenLookedupByIsPresent() {
338338
final User user = new User("joe");
339339
final String apiKeyId = randomAlphaOfLengthBetween(4, 7);
340-
final TransportRequest request = GetApiKeyRequest.usingApiKeyId(apiKeyId, false);
340+
final TransportRequest request = GetApiKeyRequest.builder().apiKeyId(apiKeyId).ownedByAuthenticatedUser(false).build();
341341
final Authentication authentication = AuthenticationTests.randomApiKeyAuthentication(new User("not-joe"), apiKeyId)
342342
.runAs(user, new Authentication.RealmRef("name", "type", randomAlphaOfLengthBetween(3, 8)));
343343
assertFalse(RBACEngine.checkSameUserPermissions(GetApiKeyAction.NAME, request, authentication));

0 commit comments

Comments
 (0)