Bug #18808
closed
OIDC token check can fail due to concurrent requests
Description
If two or more concurrent requests present the same OIDC token, and that token has not already been cached as an Arvados token in the database, multiple threads may try to insert a new token, in which case all but one will fail due to the unique constraint on the api_token column.
user supplied log excerpt:
"respBody":"{\"errors\":[\"error adding OIDC access token to database: pq: duplicate key value violates unique constraint \\\"index_api_client_authorizations_on_api_token\\\"\"]}\n"
In this situation the losing thread should detect the race and use the token that was added by the winning thread, instead of erroring out.
Updated by Tom Clegg almost 3 years ago
18808-new-token-race @ fb348442584fc62bb670c378f80ab4c943e875cc --
Updated by Lucas Di Pentima almost 3 years ago
I have just one question: Why do we need to remove the row after doing the rollback? Wouldn't that remove the valid api_client_auth row that the winning thread set up?
Updated by Tom Clegg almost 3 years ago
In the race situation, winning & losing threads each add a new row, with random UUID and api_token. The winner changes api_token to hmac(oidctoken) on the row it created, which ensures oidctoken is accepted as an Arvados token. The loser can't update api_token on its row, so that row is just an extra UUID and random api_token that haven't been given to anyone. So it behaves as if it had called "select ... from a_c_a where api_token=$1" a little bit later, in which case it would have returned ("token is already in the database") without creating a new UUID.
Updated by Lucas Di Pentima almost 3 years ago
Thanks, that clarifies my confusion! LGTM.
Updated by Tom Clegg almost 3 years ago
- Status changed from In Progress to Resolved
Applied in changeset arvados|d05b16ce4d4eea6841bf105081191f9964b485b7.