Skip to content

Conversation

@discobeta
Copy link
Contributor

@discobetadiscobeta commented Oct 12, 2023

Fixes#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

  • PR only contains one change (considered splitting up PR)
  • unit-test added
  • documentation updated
  • CHANGELOG.md updated (only for user relevant changes)
  • author name in AUTHORS

@codecov
Copy link

codecovbot commented Oct 19, 2023

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 
FilesCoverage Δ
oauth2_provider/oauth2_validators.py94.13% <100.00%> (+0.03%)⬆️

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

@dopry
Copy link
Member

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

@doprydopryforce-pushed the fix-access-token-500 branch from b458cc1 to eddbfaeCompareOctober 19, 2023 20:49
@discobeta
Copy link
ContributorAuthor

added additional tests

@dopry
Copy link
Member

@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
Copy link
ContributorAuthor

Added a test for a deleted token

@dopry
Copy link
Member

@discobeta
Copy link
ContributorAuthor

discobeta commented Oct 22, 2023 via email

@discobeta
Copy link
ContributorAuthor

@dopry
Copy link
Member

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

@discobeta
Copy link
ContributorAuthor

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

@doprydopry merged commit 854204b into django-oauth:masterOct 28, 2023
@n2ygkn2ygk changed the title Fix access token 500Fix access token 500 to properly handle expired or deleted refresh tokensMay 13, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refresh Token 500 Internal Server Error instead of the expected 401 Unauthorized

2 participants

@discobeta@dopry