Skip to content

Commit 28048a5

Browse files
authored
Updatable API keys - noop check (#88346)
This PR adds a noop check for API key updates. If we detect a noop update, i.e., an update that does not result in any changes to the existing doc, we skip the index step and return updated = false in the response. This PR also extends test coverage around various corner cases.
1 parent 67cacde commit 28048a5

File tree

8 files changed

+623
-201
lines changed

8 files changed

+623
-201
lines changed

docs/changelog/88346.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 88346
2+
summary: Updatable API keys - noop check
3+
area: Security
4+
type: enhancement
5+
issues: []

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/RealmDomain.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.elasticsearch.xcontent.ParseField;
1515
import org.elasticsearch.xcontent.ToXContentObject;
1616
import org.elasticsearch.xcontent.XContentBuilder;
17+
import org.elasticsearch.xcontent.XContentParser;
1718

1819
import java.io.IOException;
1920
import java.util.List;
@@ -48,6 +49,10 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
4849
return builder;
4950
}
5051

52+
public static RealmDomain fromXContent(final XContentParser parser) {
53+
return REALM_DOMAIN_PARSER.apply(parser, null);
54+
}
55+
5156
@Override
5257
public String toString() {
5358
return "RealmDomain{" + "name='" + name + '\'' + ", realms=" + realms + '}';

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/AuthenticationTestHelper.java

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,36 @@ public static User randomUser() {
9090
);
9191
}
9292

93+
public static User userWithRandomMetadataAndDetails(final String username, final String... roles) {
94+
return new User(
95+
username,
96+
roles,
97+
ESTestCase.randomFrom(ESTestCase.randomAlphaOfLengthBetween(1, 10), null),
98+
// Not a very realistic email address, but we don't validate this nor rely on correct format, so keeping it simple
99+
ESTestCase.randomFrom(ESTestCase.randomAlphaOfLengthBetween(1, 10), null),
100+
randomUserMetadata(),
101+
true
102+
);
103+
}
104+
105+
public static Map<String, Object> randomUserMetadata() {
106+
return ESTestCase.randomFrom(
107+
Map.of(
108+
"employee_id",
109+
ESTestCase.randomAlphaOfLength(5),
110+
"number",
111+
1,
112+
"numbers",
113+
List.of(1, 3, 5),
114+
"extra",
115+
Map.of("favorite pizza", "hawaii", "age", 42)
116+
),
117+
Map.of(ESTestCase.randomAlphaOfLengthBetween(3, 8), ESTestCase.randomAlphaOfLengthBetween(3, 8)),
118+
Map.of(),
119+
null
120+
);
121+
}
122+
93123
public static RealmDomain randomDomain(boolean includeInternal) {
94124
final Supplier<String> randomRealmTypeSupplier = randomRealmTypeSupplier(includeInternal);
95125
final Set<RealmConfig.RealmIdentifier> domainRealms = new HashSet<>(

x-pack/plugin/security/qa/security-trial/src/javaRestTest/java/org/elasticsearch/xpack/security/apikey/ApiKeyRestIT.java

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ public void testGrantApiKeyWithOnlyManageOwnApiKeyPrivilegeFails() throws IOExce
198198

199199
public void testUpdateApiKey() throws IOException {
200200
final var apiKeyName = "my-api-key-name";
201-
final Map<String, String> apiKeyMetadata = Map.of("not", "returned");
201+
final Map<String, Object> apiKeyMetadata = Map.of("not", "returned");
202202
final Map<String, Object> createApiKeyRequestBody = Map.of("name", apiKeyName, "metadata", apiKeyMetadata);
203203

204204
final Request createApiKeyRequest = new Request("POST", "_security/api_key");
@@ -215,7 +215,7 @@ public void testUpdateApiKey() throws IOException {
215215
assertThat(apiKeyId, not(emptyString()));
216216
assertThat(apiKeyEncoded, not(emptyString()));
217217

218-
doTestUpdateApiKey(apiKeyName, apiKeyId, apiKeyEncoded);
218+
doTestUpdateApiKey(apiKeyName, apiKeyId, apiKeyEncoded, apiKeyMetadata);
219219
}
220220

221221
public void testGrantTargetCanUpdateApiKey() throws IOException {
@@ -240,7 +240,7 @@ public void testGrantTargetCanUpdateApiKey() throws IOException {
240240
assertThat(apiKeyId, not(emptyString()));
241241
assertThat(apiKeyEncoded, not(emptyString()));
242242

243-
doTestUpdateApiKey(apiKeyName, apiKeyId, apiKeyEncoded);
243+
doTestUpdateApiKey(apiKeyName, apiKeyId, apiKeyEncoded, null);
244244
}
245245

246246
public void testGrantorCannotUpdateApiKeyOfGrantTarget() throws IOException {
@@ -283,18 +283,26 @@ private void doTestAuthenticationWithApiKey(final String apiKeyName, final Strin
283283
assertThat(authenticate, hasEntry("api_key", Map.of("id", apiKeyId, "name", apiKeyName)));
284284
}
285285

286-
private void doTestUpdateApiKey(String apiKeyName, String apiKeyId, String apiKeyEncoded) throws IOException {
286+
private void doTestUpdateApiKey(
287+
final String apiKeyName,
288+
final String apiKeyId,
289+
final String apiKeyEncoded,
290+
final Map<String, Object> oldMetadata
291+
) throws IOException {
287292
final var updateApiKeyRequest = new Request("PUT", "_security/api_key/" + apiKeyId);
288-
final Map<String, Object> expectedApiKeyMetadata = Map.of("not", "returned (changed)", "foo", "bar");
289-
final Map<String, Object> updateApiKeyRequestBody = Map.of("metadata", expectedApiKeyMetadata);
293+
final boolean updated = randomBoolean();
294+
final Map<String, Object> expectedApiKeyMetadata = updated ? Map.of("not", "returned (changed)", "foo", "bar") : oldMetadata;
295+
final Map<String, Object> updateApiKeyRequestBody = expectedApiKeyMetadata == null
296+
? Map.of()
297+
: Map.of("metadata", expectedApiKeyMetadata);
290298
updateApiKeyRequest.setJsonEntity(XContentTestUtils.convertToXContent(updateApiKeyRequestBody, XContentType.JSON).utf8ToString());
291299

292300
final Response updateApiKeyResponse = doUpdateUsingRandomAuthMethod(updateApiKeyRequest);
293301

294302
assertOK(updateApiKeyResponse);
295303
final Map<String, Object> updateApiKeyResponseMap = responseAsMap(updateApiKeyResponse);
296-
assertTrue((Boolean) updateApiKeyResponseMap.get("updated"));
297-
expectMetadata(apiKeyId, expectedApiKeyMetadata);
304+
assertEquals(updated, updateApiKeyResponseMap.get("updated"));
305+
expectMetadata(apiKeyId, expectedApiKeyMetadata == null ? Map.of() : expectedApiKeyMetadata);
298306
// validate authentication still works after update
299307
doTestAuthenticationWithApiKey(apiKeyName, apiKeyId, apiKeyEncoded);
300308
}

0 commit comments

Comments
 (0)