jazzband / django-oauth-toolkit

OAuth2 goodies for the Djangonauts!
https://django-oauth-toolkit.readthedocs.io
Other
3.06k stars 777 forks source link

Fix access token 500 to properly handle expired or deleted refresh tokens #1337

Closed discobeta closed 8 months ago

discobeta commented 8 months ago

Fixes https://github.com/jazzband/django-oauth-toolkit/issues/1318

Description of the Change

We are now try / except when attempting to find a vali token to avoid a 500 oauth2_provider.models.AccessToken.DoesNotExist: AccessToken matching query does not exist

Checklist

codecov[bot] commented 8 months ago

Codecov Report

Merging #1337 (9bbd884) into master (9b91d79) will increase coverage by 0.00%. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1337   +/-   ##
=======================================
  Coverage   97.54%   97.55%           
=======================================
  Files          32       32           
  Lines        2120     2123    +3     
=======================================
+ Hits         2068     2071    +3     
  Misses         52       52           
Files Coverage Δ
oauth2_provider/oauth2_validators.py 94.13% <100.00%> (+0.03%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

dopry commented 8 months ago

@discobeta could you review your tests? It looks like the new change isn't covered.

discobeta commented 8 months ago

added additional tests

dopry commented 8 months ago

@discobeta, It looks like https://github.com/jazzband/django-oauth-toolkit/pull/1337/checks?check_run_id=17910869256 still isn't covered. You probably need to setup a scenario where you make a valid AccessToken, delete the token, then make a request.

discobeta commented 8 months ago

Added a test for a deleted token

dopry commented 8 months ago

The except branch added by your patch is still not covered, https://app.codecov.io/gh/jazzband/django-oauth-toolkit/pull/1337?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=checks&utm_campaign=pr+comments&utm_term=jazzband

discobeta commented 8 months ago

I pushed an update that covers this change. Can you please approve the workflow to recheck coverage?

On Sat, Oct 21, 2023 at 12:42 PM dopry @.***> wrote:

The except branch added by your patch is still not covered, https://app.codecov.io/gh/jazzband/django-oauth-toolkit/pull/1337?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=checks&utm_campaign=pr+comments&utm_term=jazzband

— Reply to this email directly, view it on GitHub https://github.com/jazzband/django-oauth-toolkit/pull/1337#issuecomment-1773854584, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABRERCSQIQNPU6PITIYSTN3YAP3OLAVCNFSM6AAAAAA55WNPIKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZTHA2TINJYGQ . You are receiving this because you were mentioned.Message ID: @.***>

discobeta commented 8 months ago

@dopry It looks like this is now covered. Thank you for your recommendation and support with this.

https://app.codecov.io/gh/jazzband/django-oauth-toolkit/pull/1337?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=jazzband#diff-b2F1dGgyX3Byb3ZpZGVyL29hdXRoMl92YWxpZGF0b3JzLnB5

dopry commented 8 months ago

I think the last thing we need is a changelog entry and adding yourself to Authors if you're not in there already.

discobeta commented 8 months ago

@dopry can you please take a look here and see if there are any further changes required?