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.

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

Remove the id field from the InvalidateApiKey API #66671

Merged
merged 1 commit into from
Dec 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,6 @@ public final class InvalidateApiKeyRequest implements Validatable, ToXContentObj
private final String name;
private final boolean ownedByAuthenticatedUser;

// pkg scope for testing
@Deprecated
InvalidateApiKeyRequest(@Nullable String realmName, @Nullable String userName, @Nullable String apiKeyId,
@Nullable String apiKeyName, boolean ownedByAuthenticatedUser) {
this(realmName, userName, apiKeyName, ownedByAuthenticatedUser, apiKeyIdToIds(apiKeyId));
}

InvalidateApiKeyRequest(@Nullable String realmName, @Nullable String userName,
@Nullable String apiKeyName, boolean ownedByAuthenticatedUser, @Nullable List<String> apiKeyIds) {
validateApiKeyIds(apiKeyIds);
Expand Down
15 changes: 5 additions & 10 deletions x-pack/docs/en/rest-api/security/invalidate-api-keys.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -30,34 +30,29 @@ invalidated using this API.
The following parameters can be specified in the body of a DELETE request and
pertain to invalidating api keys:

`id`::
deprecated:[7.12.0, "Use ids instead"]
(Optional, string) An API key id. This parameter cannot be used when any of
`ids`, `name`, `realm_name` or `username` are used.

`ids`::
(Optional, array of string) A list of API key ids. This parameter cannot be used
when any of `id`, `name`, `realm_name`, `username` are used
when any of `name`, `realm_name`, `username` are used

`name`::
(Optional, string) An API key name. This parameter cannot be used with any of
`id`, `realm_name` or `username` are used.
`ids`, `realm_name` or `username` are used.

`realm_name`::
(Optional, string) The name of an authentication realm. This parameter cannot be
used with either `id` or `name` or when `owner` flag is set to `true`.
used with either `ids` or `name` or when `owner` flag is set to `true`.

`username`::
(Optional, string) The username of a user. This parameter cannot be used with
either `id` or `name` or when `owner` flag is set to `true`.
either `ids` or `name` or when `owner` flag is set to `true`.

`owner`::
(Optional, Boolean) A boolean flag that can be used to query API keys owned
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", "ids", "name", "username" and "realm_name" must be specified
NOTE: At least one of "ids", "name", "username" and "realm_name" must be specified
if "owner" is "false" (default).

[[security-api-invalidate-api-key-response-body]]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,30 +55,6 @@ public InvalidateApiKeyRequest(StreamInput in) throws IOException {
}
}

@Deprecated
public InvalidateApiKeyRequest(@Nullable String realmName, @Nullable String userName, @Nullable String id,
@Nullable String name, boolean ownedByAuthenticatedUser) {
this(realmName, userName, name, ownedByAuthenticatedUser, Strings.hasText(id) ? new String[] { id } : null);
}

@Deprecated
public InvalidateApiKeyRequest(@Nullable String realmName, @Nullable String userName, @Nullable String id,
@Nullable String name, boolean ownedByAuthenticatedUser, @Nullable String[] ids) {
if (id != null && ids != null) {
throw new IllegalArgumentException("Must use either [id] or [ids], not both at the same time");
}
this.realmName = textOrNull(realmName);
this.userName = textOrNull(userName);
if (Strings.hasText(id)) {
this.ids = new String[]{id};
} else {
this.ids = ids;
}
validateIds(this.ids);
this.name = textOrNull(name);
this.ownedByAuthenticatedUser = ownedByAuthenticatedUser;
}

public InvalidateApiKeyRequest(@Nullable String realmName, @Nullable String userName,
@Nullable String name, boolean ownedByAuthenticatedUser, @Nullable String[] ids) {
validateIds(ids);
Expand Down Expand Up @@ -145,7 +121,7 @@ public static InvalidateApiKeyRequest usingRealmAndUserName(String realmName, St
}

/**
* Creates invalidate API key request for given api key id
* Creates invalidate API key request for given api key ids
*
* @param id api key id
* @param ownedByAuthenticatedUser set {@code true} if the request is only for the API keys owned by current authenticated user else
Expand All @@ -156,6 +132,18 @@ public static InvalidateApiKeyRequest usingApiKeyId(String id, boolean ownedByAu
return new InvalidateApiKeyRequest(null, null, null, ownedByAuthenticatedUser, new String[]{ id });
}

/**
* Creates invalidate API key request for given api key id
*
* @param ids array of api key ids
* @param ownedByAuthenticatedUser set {@code true} if the request is only for the API keys owned by current authenticated user else
* {@code false}
* @return {@link InvalidateApiKeyRequest}
*/
public static InvalidateApiKeyRequest usingApiKeyIds(String[] ids, boolean ownedByAuthenticatedUser) {
return new InvalidateApiKeyRequest(null, null, null, ownedByAuthenticatedUser, ids);
}

/**
* Creates invalidate api key request for given api key name
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,7 @@

public class InvalidateApiKeyRequestTests extends ESTestCase {

public void testCannotSpecifyBothIdAndIds() {
final IllegalArgumentException e =
expectThrows(IllegalArgumentException.class, () -> new InvalidateApiKeyRequest(
randomFrom(randomNullOrEmptyString(), randomAlphaOfLength(8)),
randomFrom(randomNullOrEmptyString(), randomAlphaOfLength(8)),
randomAlphaOfLength(12),
randomFrom(randomNullOrEmptyString(), randomAlphaOfLength(8)),
false,
new String[]{randomAlphaOfLength(12)}));
assertThat(e.getMessage(), containsString("Must use either [id] or [ids], not both at the same time"));
}

public void testNonNullIdsCannotBeEmptyNorContainBlankId() {

ActionRequestValidationException validationException =
expectThrows(ActionRequestValidationException.class, () -> new InvalidateApiKeyRequest(
randomFrom(randomNullOrEmptyString(), randomAlphaOfLength(8)),
Expand All @@ -69,9 +56,9 @@ public void testEmptyStringsAreCoercedToNull() {
final InvalidateApiKeyRequest request = new InvalidateApiKeyRequest(
randomBlankString.get(), // realm name
randomBlankString.get(), // user name
randomBlankString.get(), // key id
randomBlankString.get(), // key name
randomBoolean() // owned by user
randomBoolean(), // owned by user
null
);
assertThat(request.getRealmName(), nullValue());
assertThat(request.getUserName(), nullValue());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,14 @@
public final class RestInvalidateApiKeyAction extends SecurityBaseRestHandler {
static final ConstructingObjectParser<InvalidateApiKeyRequest, Void> PARSER = new ConstructingObjectParser<>("invalidate_api_key",
a -> {
return new InvalidateApiKeyRequest((String) a[0], (String) a[1], (String) a[2], (String) a[3],
(a[4] == null) ? false : (Boolean) a[4],
(a[5] == null) ? null : ((List<String>) a[5]).toArray(new String[0]));
return new InvalidateApiKeyRequest((String) a[0], (String) a[1], (String) a[2],
(a[3] == null) ? false : (Boolean) a[3],
(a[4] == null) ? null : ((List<String>) a[4]).toArray(new String[0]));
});

static {
PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), new ParseField("realm_name"));
PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), new ParseField("username"));
PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), new ParseField("id").withAllDeprecated("ids"));
PARSER.declareString(ConstructingObjectParser.optionalConstructorArg(), new ParseField("name"));
PARSER.declareBoolean(ConstructingObjectParser.optionalConstructorArg(), new ParseField("owner"));
PARSER.declareStringArray(ConstructingObjectParser.optionalConstructorArg(), new ParseField("ids"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -594,25 +594,14 @@ public void testSecurityConfigChangeEventFormattingForApiKeyInvalidation() throw
final AuthorizationInfo authorizationInfo = () -> Collections.singletonMap(PRINCIPAL_ROLES_FIELD_NAME, expectedRoles);
final Authentication authentication = createAuthentication();

final InvalidateApiKeyRequest invalidateApiKeyRequest;
if (randomBoolean()) {
invalidateApiKeyRequest = new InvalidateApiKeyRequest(
randomFrom(randomAlphaOfLength(8), null),
randomFrom(randomAlphaOfLength(8), null),
null,
randomFrom(randomAlphaOfLength(8), null),
randomBoolean(),
randomFrom(randomArray(1,3, String[]::new, () -> randomAlphaOfLength(8)), null)
);
} else {
invalidateApiKeyRequest = new InvalidateApiKeyRequest(
randomFrom(randomAlphaOfLength(8), null),
randomFrom(randomAlphaOfLength(8), null),
randomAlphaOfLength(8),
randomFrom(randomAlphaOfLength(8), null),
randomBoolean()
);
}
final InvalidateApiKeyRequest invalidateApiKeyRequest = new InvalidateApiKeyRequest(
randomFrom(randomAlphaOfLength(8), null),
randomFrom(randomAlphaOfLength(8), null),
randomFrom(randomAlphaOfLength(8), null),
randomBoolean(),
randomFrom(randomArray(1,3, String[]::new, () -> randomAlphaOfLength(8)), null)
);

auditTrail.accessGranted(requestId, authentication, InvalidateApiKeyAction.NAME, invalidateApiKeyRequest, authorizationInfo);
List<String> output = CapturingLogger.output(logger.getName(), Level.INFO);
assertThat(output.size(), is(2));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,9 @@ public void testInvalidateApiKey() throws Exception {
final String json1 = "{ \"realm_name\" : \"realm-1\", \"username\": \"user-x\" }";
final String json2 = "{ \"realm_name\" : \"realm-1\" }";
final String json3 = "{ \"username\": \"user-x\" }";
final String json4 = "{ \"id\" : \"api-key-id-1\" }";
final String json5 = "{ \"name\" : \"api-key-name-1\" }";
final String json6 = "{ \"ids\" : [\"api-key-id-1\"] }";
final String json = randomFrom(json1, json2, json3, json4, json5, json6);
final boolean assertDeprecationWarning = json == json4; // we want object identity comparison here
final String json = randomFrom(json1, json2, json3, json5, json6);
final FakeRestRequest restRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY)
.withContent(new BytesArray(json), XContentType.JSON).build();

Expand Down Expand Up @@ -119,9 +117,6 @@ void doExecute(ActionType<Response> action, Request request, ActionListener<Resp
assertThat(actual.getPreviouslyInvalidatedApiKeys(),
equalTo(invalidateApiKeyResponseExpected.getPreviouslyInvalidatedApiKeys()));
assertThat(actual.getErrors(), equalTo(invalidateApiKeyResponseExpected.getErrors()));
if (assertDeprecationWarning) {
assertWarnings("Deprecated field [id] used, replaced by [ids]");
}
}

}
Expand Down