Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix documentation regarding rotate_refresh_token setting #1250

Merged
merged 2 commits into from
Feb 12, 2023

Conversation

JordiNeil
Copy link
Contributor

@JordiNeil JordiNeil commented Feb 11, 2023

Fixes 1240

Description of the Change

Removed 'known bug' from documentation due to it is not happening.

The known bug states that when the ROTATE_REFRESH_TOKEN setting is set to False, was causing that both refresh token and access token get revoked

image

For testing that this is working correctly I set the ROTATE_REFRESH_TOKEN to False in the OAUTH2_PROVIDER setting in settings.py file and then:

  1. Created a password based application
  2. Requested an access token using password as grant_type
  3. Used the returned refresh token to run a new request but this time with refresh_token as grant_type

Tested that:

  • The response included the same refresh_token in the response, and updated the access_token with a new value.
  • Revoked the old access_token.
  • The initially originated refresh token can be used multiple times for refreshing tokens.
  • If send an invalid refresh token, it's failing the refresh.

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

@JordiNeil JordiNeil self-assigned this Feb 11, 2023
@codecov
Copy link

codecov bot commented Feb 11, 2023

Codecov Report

Merging #1250 (34a8b9f) into master (62f9261) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1250   +/-   ##
=======================================
  Coverage   96.97%   96.97%           
=======================================
  Files          31       31           
  Lines        1821     1821           
=======================================
  Hits         1766     1766           
  Misses         55       55           

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

@n2ygk
Copy link
Member

n2ygk commented Feb 12, 2023

Ugh some unpinned version (probably) cause flake8 to fail on this PR. This is unrelated to the change here but requires digging in to seeing what now is causing this.

  flake8: commands[0]> flake8 /home/runner/work/django-oauth-toolkit/django-oauth-toolkit
  /home/runner/work/django-oauth-toolkit/django-oauth-toolkit/oauth2_provider/oauth2_validators.py:575:1: BLK100 Black would make changes.
  /home/runner/work/django-oauth-toolkit/django-oauth-toolkit/oauth2_provider/models.py:752:1: BLK100 Black would make changes.
  /home/runner/work/django-oauth-toolkit/django-oauth-toolkit/oauth2_provider/views/oidc.py:89:1: BLK100 Black would make changes.
  /home/runner/work/django-oauth-toolkit/django-oauth-toolkit/tests/test_oauth2_validators.py:163:1: BLK100 Black would make changes.
  flake8: exit 1 (4.34 seconds) 

@n2ygk
Copy link
Member

n2ygk commented Feb 12, 2023

Ugh some unpinned version (probably) cause flake8 to fail on this PR. This is unrelated to the change here but requires digging in to seeing what now is causing this.

  flake8: commands[0]> flake8 /home/runner/work/django-oauth-toolkit/django-oauth-toolkit
  /home/runner/work/django-oauth-toolkit/django-oauth-toolkit/oauth2_provider/oauth2_validators.py:575:1: BLK100 Black would make changes.
  /home/runner/work/django-oauth-toolkit/django-oauth-toolkit/oauth2_provider/models.py:752:1: BLK100 Black would make changes.
  /home/runner/work/django-oauth-toolkit/django-oauth-toolkit/oauth2_provider/views/oidc.py:89:1: BLK100 Black would make changes.
  /home/runner/work/django-oauth-toolkit/django-oauth-toolkit/tests/test_oauth2_validators.py:163:1: BLK100 Black would make changes.
  flake8: exit 1 (4.34 seconds) 

Fixing this now by merging #1247, #1248

@n2ygk n2ygk added this to the Future milestone Feb 12, 2023
@n2ygk n2ygk merged commit fc50ff1 into master Feb 12, 2023
@n2ygk n2ygk modified the milestones: Future, 2.3.0 May 31, 2023
Sign up for free to 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.

ROTATE_REFRESH_TOKEN Known Bug
2 participants