Refresh Token Reuse Protection#1452
Merged
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#1404
Description of the Change
This PR adds a new setting / feature
REFRESH_TOKEN_REUSE_PROTECTION. It allows django-oauth-toolkit to revoke related refresh and access tokens whenever an already revoked refresh token is used again. This indicates a breach - for details see #1404.The implementation does it by adding a new column
token_familyto the refresh token table. When a new refresh token is created with an existing refresh token, the new refresh token get's assigned the sametoken_familyvalue as the original token. This ensures that all tokens from the same session share the same family. Otherwise (first token of a session, gathered by e.g. username & password), thetoken_familyis a random value.Whenever a revoked refresh token is presented, the server will query for all tokens with the same
token_familyvalue, and call the.revoke()method on it.To discuss: The "select all tokens of a family and revoke" code is always being run when a revoked token is presented. Maybe this could provide a vector for a denial of service attack. Therefore, I was wondering if we should set the
token_familyvalue to NULL after the whole family has been revoked. Another thought to ease database load was furthermore to useobjects.filter(...).delete(), however I was afraid that the logic inside the.revoke()method was relevant and shouldn't be skipped lightheartedly. Furthermore, do you think an index on that new column would be required?Checklist
CHANGELOG.mdupdated (only for user relevant changes)AUTHORS