django-rest-knox icon indicating copy to clipboard operation
django-rest-knox copied to clipboard

Published 20 hours ago • verified publisher icon jazzband

knox: handle race condition on concurrent logout call

Open xrmx opened this issue 6 years ago • 6 comments

With AUTO_REFRESH enabled authentication is racing against token removal of logout. If a token gets removed updating its expiry would led to a DatabaseError raised by the Django ORM.

Fix that by ignoring DatabaseError exception returned by renew_token so that the last request will get a AuthenticationFailed exception instead of a 500 error.

xrmx avatar Jul 19 '19 11:07 xrmx

@johnraz Thanks for the review, it's based on master because develop is missing some 4.1.0 commits. Will add changelog.

xrmx avatar Jul 19 '19 12:07 xrmx

If you base your work on develop and create the PR against develop, missing commits from master should not be an issue. Those are only release related commits I believe.

Writing from my phone so I may be missing a point. Maybe some other contributors could have a look. Cheers.

johnraz avatar Jul 19 '19 12:07 johnraz

@johnraz yeah, but master commits should be merged back to develop or you'll have conflicts, e.g. like in this specific case in the CHANGELOG

xrmx avatar Jul 19 '19 12:07 xrmx

Yes agreed on the merge back to develop but please let’s do that in a separate PR, maybe @belugame can help, won’t have access to my computer soon (travelling right now).

johnraz avatar Jul 19 '19 13:07 johnraz

Updated PR:

  • rebased on top of develop
  • renamed tests and added some comments to help understand the scenario
  • rewritten commit message to make it hopefully more understandable
  • changed the continue to a break when handling the exception as we found the token

xrmx avatar Jul 23 '19 15:07 xrmx