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

[Concurrency] Race condition when requesting >1 access_token using the same refresh_token #70

Open
SamuAlfageme opened this issue Aug 1, 2017 · 2 comments

Comments

@SamuAlfageme
Copy link
Contributor

SamuAlfageme commented Aug 1, 2017

Migrating from #65 (comment) and #65 (comment) as consequence from #65

Steps to Reproduce:

  1. Add the same account multiple times on the Desktop client (allowed by design; they currently use the same keychain entry: Different accounts may use same keychain entry client#5830 which will store the refresh token of the one included the last)
  2. Close the client and relaunch while monitoring the requests on startup.
    (Edit: The client no longer re-use the same token for different account on the same server since Credentials: Use per-account keychain entries #5830 client#6027)

Send twice the same request in parallel sharing the same shared_refresh_token:

curl \
[...] \
-H 'Content-Type:application/x-www-form-urlencoded'  \
-X POST '<server>/index.php/apps/oauth2/api/v1/token' \
--data-binary 'grant_type=refresh_token&refresh_token=<shared_refresh_token>'

Actual Behavior

Sometimes, more than one request is successful, each carrying on the reply a new, valid token pair (access/refresh).

Expected Behavior

Granting the token should be an atomic operation in the server - i.e. after having used a refresh_token already, additional requests must be replied with 401: Unauthorized.

Take what happens when the requests don't reach the server that simultaneously, the first POST succeeds, replied with a new set of tokens; second is replied:

{
    "error": "invalid_grant"
}

Assigning "lowest" priority available in the repo as pretty edge-case and currently happening only some of the times while sharing the refresh_token between requests (not that normal)

@guruz
Copy link

guruz commented May 23, 2019

@ogoffart @ckamm Do you see a solution there on the server side or shall we change the desktop's way of storing keychain entries? (which has its disavtanges too of course)

@ogoffart
Copy link

@guruz this is only a server issue.

The client is no longer re-using the token for two accounts on the same URL. I edited the description.

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

No branches or pull requests

3 participants