Skip to content

Commit

Permalink
Fix responses for the token APIs (elastic#54532)
Browse files Browse the repository at this point in the history
This commit fixes our behavior regarding the responses we
return in various cases for the use of token related APIs.
More concretely:

- In the Get Token API with the `refresh` grant, when an invalid
(already deleted, malformed, unknown) refresh token is used in the
body of the request, we respond with `400` HTTP status code
 and an `error_description` header with the message "could not
refresh the requested token".
Previously we would return erroneously return a  `401` with "token
malformed" message.

- In the Invalidate Token API, when using an invalid (already
deleted, malformed, unknown) access or refresh token, we respond
with `404` and a body that shows that no tokens were invalidated:
   ```
   {
     "invalidated_tokens":0,
     "previously_invalidated_tokens":0,
      "error_count":0
   }
   ```
   The previous behavior would be to erroneously return
a `400` or `401` ( depending on the case ).

- In the Invalidate Token API, when the tokens index doesn't
exist or is closed, we return `400` because we assume this is
a user issue either because they tried to invalidate a token
when there is no tokens index yet ( i.e. no tokens have
been created yet or the tokens index has been deleted ) or the
index is closed.

- In the Invalidate Token API, when the tokens index is
unavailable, we return a `503` status code because
we want to signal to the caller of the API that the token they
tried to invalidate was not invalidated and we can't be sure
if it is still valid or not, and that they should try the request
again.

Resolves: elastic#53323
  • Loading branch information
jkakavas committed Apr 16, 2020
1 parent 8afc8e6 commit 115ce52
Show file tree
Hide file tree
Showing 18 changed files with 587 additions and 121 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -765,7 +765,7 @@ public Cancellable createTokenAsync(CreateTokenRequest request, RequestOptions o
*/
public InvalidateTokenResponse invalidateToken(InvalidateTokenRequest request, RequestOptions options) throws IOException {
return restHighLevelClient.performRequestAndParseEntity(request, SecurityRequestConverters::invalidateToken, options,
InvalidateTokenResponse::fromXContent, emptySet());
InvalidateTokenResponse::fromXContent, singleton(404));
}

/**
Expand All @@ -780,7 +780,7 @@ public InvalidateTokenResponse invalidateToken(InvalidateTokenRequest request, R
public Cancellable invalidateTokenAsync(InvalidateTokenRequest request, RequestOptions options,
ActionListener<InvalidateTokenResponse> listener) {
return restHighLevelClient.performRequestAsyncAndParseEntity(request, SecurityRequestConverters::invalidateToken, options,
InvalidateTokenResponse::fromXContent, listener, emptySet());
InvalidateTokenResponse::fromXContent, listener, singleton(404));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,10 @@ public final class InvalidateTokenResponse {
PARSER.declareInt(constructorArg(), PREVIOUSLY_INVALIDATED_TOKENS);
PARSER.declareInt(constructorArg(), ERROR_COUNT);
PARSER.declareObjectArray(optionalConstructorArg(), (p, c) -> ElasticsearchException.fromXContent(p), ERRORS);

}

public InvalidateTokenResponse(int invalidatedTokens, int previouslyInvalidatedTokens,
@Nullable List<ElasticsearchException> errors) {
public InvalidateTokenResponse(int invalidatedTokens, int previouslyInvalidatedTokens, @Nullable List<ElasticsearchException> errors) {
this.invalidatedTokens = invalidatedTokens;
this.previouslyInvalidatedTokens = previouslyInvalidatedTokens;
if (null == errors) {
Expand Down
6 changes: 3 additions & 3 deletions x-pack/docs/en/rest-api/security/oidc-logout-api.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
=== OpenID Connect logout API

Submits a request to invalidate a refresh token and an access token that was
generated as a response to a call to `/_security/oidc/authenticate`.
generated as a response to a call to `/_security/oidc/authenticate`.

[[security-api-oidc-logout-request]]
==== {api-request-title}
Expand Down Expand Up @@ -48,7 +48,7 @@ POST /_security/oidc/logout
"refresh_token": "vLBPvmAB6KvwvJZr27cS"
}
--------------------------------------------------
// TEST[catch:unauthorized]
// TEST[catch:request]

The following example output of the response contains the URI pointing to the
End Session Endpoint of the OpenID Connect Provider with all the parameters of
Expand All @@ -60,4 +60,4 @@ the Logout Request, as HTTP GET parameters:
"redirect" : "https://op-provider.org/logout?id_token_hint=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c&post_logout_redirect_uri=http%3A%2F%2Foidc-kibana.elastic.co%2Floggedout&state=lGYK0EcSLjqH6pkT5EVZjC6eIW5YCGgywj2sxROO"
}
--------------------------------------------------
// NOTCONSOLE
// NOTCONSOLE
2 changes: 1 addition & 1 deletion x-pack/docs/en/security/authentication/oidc-guide.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ POST /_security/oidc/logout
"refresh_token": "vLBPvmAB6KvwvJZr27cS"
}
--------------------------------------------------
// TEST[catch:unauthorized]
// TEST[catch:request]
+
If the realm is configured accordingly, this may result in a response with a `redirect` parameter indicating where
the user needs to be redirected in the OP in order to complete the logout process.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.rest.RestStatus;

import java.io.IOException;
import java.util.Collections;
Expand All @@ -33,9 +34,10 @@ public class TokensInvalidationResult implements ToXContentObject, Writeable {
private final List<String> invalidatedTokens;
private final List<String> previouslyInvalidatedTokens;
private final List<ElasticsearchException> errors;
private RestStatus restStatus;

public TokensInvalidationResult(List<String> invalidatedTokens, List<String> previouslyInvalidatedTokens,
@Nullable List<ElasticsearchException> errors) {
@Nullable List<ElasticsearchException> errors, RestStatus restStatus) {
Objects.requireNonNull(invalidatedTokens, "invalidated_tokens must be provided");
this.invalidatedTokens = invalidatedTokens;
Objects.requireNonNull(previouslyInvalidatedTokens, "previously_invalidated_tokens must be provided");
Expand All @@ -45,6 +47,7 @@ public TokensInvalidationResult(List<String> invalidatedTokens, List<String> pre
} else {
this.errors = Collections.emptyList();
}
this.restStatus = restStatus;
}

public TokensInvalidationResult(StreamInput in) throws IOException {
Expand All @@ -54,10 +57,13 @@ public TokensInvalidationResult(StreamInput in) throws IOException {
if (in.getVersion().before(Version.V_7_2_0)) {
in.readVInt();
}
if (in.getVersion().onOrAfter(Version.V_8_0_0)) {
this.restStatus = RestStatus.readFrom(in);
}
}

public static TokensInvalidationResult emptyResult() {
return new TokensInvalidationResult(Collections.emptyList(), Collections.emptyList(), Collections.emptyList());
public static TokensInvalidationResult emptyResult(RestStatus restStatus) {
return new TokensInvalidationResult(Collections.emptyList(), Collections.emptyList(), Collections.emptyList(), restStatus);
}


Expand All @@ -73,6 +79,10 @@ public List<ElasticsearchException> getErrors() {
return errors;
}

public RestStatus getRestStatus() {
return restStatus;
}

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
builder.startObject()
Expand Down Expand Up @@ -100,5 +110,8 @@ public void writeTo(StreamOutput out) throws IOException {
if (out.getVersion().before(Version.V_7_2_0)) {
out.writeVInt(5);
}
if (out.getVersion().onOrAfter(Version.V_8_0_0)) {
RestStatus.writeTo(out, restStatus);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentFactory;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.core.security.authc.support.TokensInvalidationResult;

Expand All @@ -29,7 +30,8 @@ public void testSerialization() throws IOException {
TokensInvalidationResult result = new TokensInvalidationResult(Arrays.asList(generateRandomStringArray(20, 15, false)),
Arrays.asList(generateRandomStringArray(20, 15, false)),
Arrays.asList(new ElasticsearchException("foo", new IllegalArgumentException("this is an error message")),
new ElasticsearchException("bar", new IllegalArgumentException("this is an error message2"))));
new ElasticsearchException("bar", new IllegalArgumentException("this is an error message2"))),
RestStatus.OK);
InvalidateTokenResponse response = new InvalidateTokenResponse(result);
try (BytesStreamOutput output = new BytesStreamOutput()) {
response.writeTo(output);
Expand All @@ -45,7 +47,7 @@ public void testSerialization() throws IOException {
}

result = new TokensInvalidationResult(Arrays.asList(generateRandomStringArray(20, 15, false)),
Arrays.asList(generateRandomStringArray(20, 15, false)), Collections.emptyList());
Arrays.asList(generateRandomStringArray(20, 15, false)), Collections.emptyList(), RestStatus.OK);
response = new InvalidateTokenResponse(result);
try (BytesStreamOutput output = new BytesStreamOutput()) {
response.writeTo(output);
Expand All @@ -64,7 +66,7 @@ public void testToXContent() throws IOException {
List previouslyInvalidatedTokens = Arrays.asList(generateRandomStringArray(20, 15, false));
TokensInvalidationResult result = new TokensInvalidationResult(invalidatedTokens, previouslyInvalidatedTokens,
Arrays.asList(new ElasticsearchException("foo", new IllegalArgumentException("this is an error message")),
new ElasticsearchException("bar", new IllegalArgumentException("this is an error message2"))));
new ElasticsearchException("bar", new IllegalArgumentException("this is an error message2"))), RestStatus.OK);
InvalidateTokenResponse response = new InvalidateTokenResponse(result);
XContentBuilder builder = XContentFactory.jsonBuilder();
response.toXContent(builder, ToXContent.EMPTY_PARAMS);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ public void doRun() {
indicesWithTokens.add(securityMainIndex.aliasName());
}
if (indicesWithTokens.isEmpty()) {
markComplete();
return;
}
DeleteByQueryRequest expiredDbq = new DeleteByQueryRequest(indicesWithTokens.toArray(new String[0]));
Expand Down
Loading

0 comments on commit 115ce52

Please sign in to comment.