Skip to content

Support concurrent refresh of refresh tokens #39559

Merged
jkakavas merged 2 commits intoelastic:7.xfrom
jkakavas:support-concurrent-refresh-7.x
Mar 1, 2019
Merged

Support concurrent refresh of refresh tokens #39559
jkakavas merged 2 commits intoelastic:7.xfrom
jkakavas:support-concurrent-refresh-7.x

Conversation

@jkakavas
Copy link
Copy Markdown
Contributor

@jkakavas jkakavas commented Mar 1, 2019

This is a backport of #38382
This change adds supports for the concurrent refresh of access
tokens as described in #36872
In short it allows subsequent client requests to refresh the same token that
come within a predefined window of 60 seconds to be handled as duplicates
of the original one and thus receive the same response with the same newly
issued access token and refresh token.
In order to support that, two new fields are added in the token document. One
contains the instant (in epoqueMillis) when a given refresh token is refreshed
and one that contains a pointer to the token document that stores the new
refresh token and access token that was created by the original refresh.
A side effect of this change, that was however also a intended enhancement
for the token service, is that we needed to stop encrypting the string
representation of the UserToken while serializing. ( It was necessary as we
correctly used a new IV for every time we encrypted a token in serialization, so
subsequent serializations of the same exact UserToken would produce
different access token strings)

This change also handles the serialization/deserialization BWC logic:

  • In mixed clusters we keep creating tokens in the old format and
    consume only old format tokens
  • In upgraded clusters, we start creating tokens in the new format but
    still remain able to consume old format tokens (that could have been
    created during the rolling upgrade and are still valid)

Resolves #36872

Co-authored-by: Jay Modi jaymode@users.noreply.github.com

jkakavas and others added 2 commits March 1, 2019 12:51
This change adds supports for the concurrent refresh of access
tokens as described in elastic#36872
In short it allows subsequent client requests to refresh the same token that
come within a predefined window of 60 seconds to be handled as duplicates
of the original one and thus receive the same response with the same newly
issued access token and refresh token.
In order to support that, two new fields are added in the token document. One
contains the instant (in epoqueMillis) when a given refresh token is refreshed
and one that contains a pointer to the token document that stores the new
refresh token and access token that was created by the original refresh.
A side effect of this change, that was however also a intended enhancement
for the token service, is that we needed to stop encrypting the string
representation of the UserToken while serializing. ( It was necessary as we
correctly used a new IV for every time we encrypted a token in serialization, so
subsequent serializations of the same exact UserToken would produce
different access token strings)

This change also handles the serialization/deserialization BWC logic:

- In mixed clusters we keep creating tokens in the old format and
consume only old format tokens
- In upgraded clusters, we start creating tokens in the new format but
still remain able to consume old format tokens (that could have been
created during the rolling upgrade and are still valid)

Resolves elastic#36872

Co-authored-by: Jay Modi jaymode@users.noreply.github.com
@jkakavas jkakavas added >enhancement :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) backport v7.2.0 labels Mar 1, 2019
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-security

@jkakavas jkakavas merged commit e259921 into elastic:7.x Mar 1, 2019
tlrx added a commit that referenced this pull request Mar 1, 2019
@tlrx
Copy link
Copy Markdown
Member

tlrx commented Mar 1, 2019

Sorry @jkakavas, I had to revert this change in 0c6b7cf because it is not compatible with #38382 from master and prevent me from reenabling the bwc tests in master.

As we talked via another channel, the serialization bwc logic in TokensInvalidationResult might be the cause.

@jkakavas
Copy link
Copy Markdown
Contributor Author

jkakavas commented Mar 1, 2019

Thanks a ton @tlrx. I'll take a look once I'm back at a desk and make sure the change is properly tested

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport >enhancement :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) v7.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants