Skip to content

Commit 115ce52

Browse files
committed
Fix responses for the token APIs (elastic#54532)
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
1 parent 8afc8e6 commit 115ce52

File tree

18 files changed

+587
-121
lines changed

18 files changed

+587
-121
lines changed

client/rest-high-level/src/main/java/org/elasticsearch/client/SecurityClient.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -765,7 +765,7 @@ public Cancellable createTokenAsync(CreateTokenRequest request, RequestOptions o
765765
*/
766766
public InvalidateTokenResponse invalidateToken(InvalidateTokenRequest request, RequestOptions options) throws IOException {
767767
return restHighLevelClient.performRequestAndParseEntity(request, SecurityRequestConverters::invalidateToken, options,
768-
InvalidateTokenResponse::fromXContent, emptySet());
768+
InvalidateTokenResponse::fromXContent, singleton(404));
769769
}
770770

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

786786
/**

client/rest-high-level/src/main/java/org/elasticsearch/client/security/InvalidateTokenResponse.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,10 @@ public final class InvalidateTokenResponse {
6262
PARSER.declareInt(constructorArg(), PREVIOUSLY_INVALIDATED_TOKENS);
6363
PARSER.declareInt(constructorArg(), ERROR_COUNT);
6464
PARSER.declareObjectArray(optionalConstructorArg(), (p, c) -> ElasticsearchException.fromXContent(p), ERRORS);
65+
6566
}
6667

67-
public InvalidateTokenResponse(int invalidatedTokens, int previouslyInvalidatedTokens,
68-
@Nullable List<ElasticsearchException> errors) {
68+
public InvalidateTokenResponse(int invalidatedTokens, int previouslyInvalidatedTokens, @Nullable List<ElasticsearchException> errors) {
6969
this.invalidatedTokens = invalidatedTokens;
7070
this.previouslyInvalidatedTokens = previouslyInvalidatedTokens;
7171
if (null == errors) {

x-pack/docs/en/rest-api/security/oidc-logout-api.asciidoc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
=== OpenID Connect logout API
44

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

88
[[security-api-oidc-logout-request]]
99
==== {api-request-title}
@@ -48,7 +48,7 @@ POST /_security/oidc/logout
4848
"refresh_token": "vLBPvmAB6KvwvJZr27cS"
4949
}
5050
--------------------------------------------------
51-
// TEST[catch:unauthorized]
51+
// TEST[catch:request]
5252

5353
The following example output of the response contains the URI pointing to the
5454
End Session Endpoint of the OpenID Connect Provider with all the parameters of
@@ -60,4 +60,4 @@ the Logout Request, as HTTP GET parameters:
6060
"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"
6161
}
6262
--------------------------------------------------
63-
// NOTCONSOLE
63+
// NOTCONSOLE

x-pack/docs/en/security/authentication/oidc-guide.asciidoc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -672,7 +672,7 @@ POST /_security/oidc/logout
672672
"refresh_token": "vLBPvmAB6KvwvJZr27cS"
673673
}
674674
--------------------------------------------------
675-
// TEST[catch:unauthorized]
675+
// TEST[catch:request]
676676
+
677677
If the realm is configured accordingly, this may result in a response with a `redirect` parameter indicating where
678678
the user needs to be redirected in the OP in order to complete the logout process.

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

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.elasticsearch.common.io.stream.Writeable;
1515
import org.elasticsearch.common.xcontent.ToXContentObject;
1616
import org.elasticsearch.common.xcontent.XContentBuilder;
17+
import org.elasticsearch.rest.RestStatus;
1718

1819
import java.io.IOException;
1920
import java.util.Collections;
@@ -33,9 +34,10 @@ public class TokensInvalidationResult implements ToXContentObject, Writeable {
3334
private final List<String> invalidatedTokens;
3435
private final List<String> previouslyInvalidatedTokens;
3536
private final List<ElasticsearchException> errors;
37+
private RestStatus restStatus;
3638

3739
public TokensInvalidationResult(List<String> invalidatedTokens, List<String> previouslyInvalidatedTokens,
38-
@Nullable List<ElasticsearchException> errors) {
40+
@Nullable List<ElasticsearchException> errors, RestStatus restStatus) {
3941
Objects.requireNonNull(invalidatedTokens, "invalidated_tokens must be provided");
4042
this.invalidatedTokens = invalidatedTokens;
4143
Objects.requireNonNull(previouslyInvalidatedTokens, "previously_invalidated_tokens must be provided");
@@ -45,6 +47,7 @@ public TokensInvalidationResult(List<String> invalidatedTokens, List<String> pre
4547
} else {
4648
this.errors = Collections.emptyList();
4749
}
50+
this.restStatus = restStatus;
4851
}
4952

5053
public TokensInvalidationResult(StreamInput in) throws IOException {
@@ -54,10 +57,13 @@ public TokensInvalidationResult(StreamInput in) throws IOException {
5457
if (in.getVersion().before(Version.V_7_2_0)) {
5558
in.readVInt();
5659
}
60+
if (in.getVersion().onOrAfter(Version.V_8_0_0)) {
61+
this.restStatus = RestStatus.readFrom(in);
62+
}
5763
}
5864

59-
public static TokensInvalidationResult emptyResult() {
60-
return new TokensInvalidationResult(Collections.emptyList(), Collections.emptyList(), Collections.emptyList());
65+
public static TokensInvalidationResult emptyResult(RestStatus restStatus) {
66+
return new TokensInvalidationResult(Collections.emptyList(), Collections.emptyList(), Collections.emptyList(), restStatus);
6167
}
6268

6369

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

82+
public RestStatus getRestStatus() {
83+
return restStatus;
84+
}
85+
7686
@Override
7787
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
7888
builder.startObject()
@@ -100,5 +110,8 @@ public void writeTo(StreamOutput out) throws IOException {
100110
if (out.getVersion().before(Version.V_7_2_0)) {
101111
out.writeVInt(5);
102112
}
113+
if (out.getVersion().onOrAfter(Version.V_8_0_0)) {
114+
RestStatus.writeTo(out, restStatus);
115+
}
103116
}
104117
}

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/action/token/InvalidateTokenResponseTests.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.elasticsearch.common.xcontent.ToXContent;
1313
import org.elasticsearch.common.xcontent.XContentBuilder;
1414
import org.elasticsearch.common.xcontent.XContentFactory;
15+
import org.elasticsearch.rest.RestStatus;
1516
import org.elasticsearch.test.ESTestCase;
1617
import org.elasticsearch.xpack.core.security.authc.support.TokensInvalidationResult;
1718

@@ -29,7 +30,8 @@ public void testSerialization() throws IOException {
2930
TokensInvalidationResult result = new TokensInvalidationResult(Arrays.asList(generateRandomStringArray(20, 15, false)),
3031
Arrays.asList(generateRandomStringArray(20, 15, false)),
3132
Arrays.asList(new ElasticsearchException("foo", new IllegalArgumentException("this is an error message")),
32-
new ElasticsearchException("bar", new IllegalArgumentException("this is an error message2"))));
33+
new ElasticsearchException("bar", new IllegalArgumentException("this is an error message2"))),
34+
RestStatus.OK);
3335
InvalidateTokenResponse response = new InvalidateTokenResponse(result);
3436
try (BytesStreamOutput output = new BytesStreamOutput()) {
3537
response.writeTo(output);
@@ -45,7 +47,7 @@ public void testSerialization() throws IOException {
4547
}
4648

4749
result = new TokensInvalidationResult(Arrays.asList(generateRandomStringArray(20, 15, false)),
48-
Arrays.asList(generateRandomStringArray(20, 15, false)), Collections.emptyList());
50+
Arrays.asList(generateRandomStringArray(20, 15, false)), Collections.emptyList(), RestStatus.OK);
4951
response = new InvalidateTokenResponse(result);
5052
try (BytesStreamOutput output = new BytesStreamOutput()) {
5153
response.writeTo(output);
@@ -64,7 +66,7 @@ public void testToXContent() throws IOException {
6466
List previouslyInvalidatedTokens = Arrays.asList(generateRandomStringArray(20, 15, false));
6567
TokensInvalidationResult result = new TokensInvalidationResult(invalidatedTokens, previouslyInvalidatedTokens,
6668
Arrays.asList(new ElasticsearchException("foo", new IllegalArgumentException("this is an error message")),
67-
new ElasticsearchException("bar", new IllegalArgumentException("this is an error message2"))));
69+
new ElasticsearchException("bar", new IllegalArgumentException("this is an error message2"))), RestStatus.OK);
6870
InvalidateTokenResponse response = new InvalidateTokenResponse(result);
6971
XContentBuilder builder = XContentFactory.jsonBuilder();
7072
response.toXContent(builder, ToXContent.EMPTY_PARAMS);

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ public void doRun() {
7070
indicesWithTokens.add(securityMainIndex.aliasName());
7171
}
7272
if (indicesWithTokens.isEmpty()) {
73+
markComplete();
7374
return;
7475
}
7576
DeleteByQueryRequest expiredDbq = new DeleteByQueryRequest(indicesWithTokens.toArray(new String[0]));

0 commit comments

Comments
 (0)