oauth2 icon indicating copy to clipboard operation
oauth2 copied to clipboard

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

Open SamuAlfageme opened this issue 8 years ago • 2 comments

Migrating from https://github.com/owncloud/oauth2/pull/65#issuecomment-318595228 and https://github.com/owncloud/oauth2/pull/65#issuecomment-318641919 as consequence from https://github.com/owncloud/oauth2/pull/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: https://github.com/owncloud/client/issues/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 https://github.com/owncloud/client/pull/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)

SamuAlfageme avatar Aug 01 '17 15:08 SamuAlfageme

@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)

guruz avatar May 23 '19 09:05 guruz

@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.

ogoffart avatar May 23 '19 11:05 ogoffart