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

Properly handle non-existent tokens in Token Invalidate API #53323

Closed
jkakavas opened this issue Mar 10, 2020 · 1 comment · Fixed by #54532
Closed

Properly handle non-existent tokens in Token Invalidate API #53323

jkakavas opened this issue Mar 10, 2020 · 1 comment · Fixed by #54532
Labels
>bug :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc)

Comments

@jkakavas
Copy link
Member

Invalidate Token API does not handle malformed and non-existent tokens correctly. It returns a 400 could not refresh the requested token if it can decode the refresh token and a 401 token malformed if the token document doesn't exist. The latter can happen in cases where we have already deleted the token document because the access token is expired/invalidated and the caller is not aware, or when we get a token that is malformed or not ours.

In all above cases we should be returning 200 with a body of

{
  "invalidated_tokens":0, 
  "previously_invalidated_tokens":0, 
  "error_count":0
}
@jkakavas jkakavas added >bug :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) labels Mar 10, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (:Security/Authentication)

jkakavas added a commit that referenced this issue Apr 16, 2020
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: #53323
jkakavas added a commit to jkakavas/elasticsearch that referenced this issue Apr 16, 2020
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
jkakavas added a commit that referenced this issue Apr 16, 2020
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: #53323
jkakavas added a commit to jkakavas/elasticsearch that referenced this issue May 14, 2020
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants