Styria-Digital / django-rest-framework-jwt

JSON Web Token Authentication support for Django REST Framework
https://styria-digital.github.io/django-rest-framework-jwt/
MIT License
192 stars 60 forks source link

add JWT_AUTH_COOKIE_* settings paralleling django SESSION_COOKIE_* #29

Closed nigoroll closed 4 years ago

nigoroll commented 4 years ago

We add settings analogous to SESSION_COOKIE_* for the JWT cookie:

'JWT_AUTH_COOKIE_DOMAIN': None
'JWT_AUTH_COOKIE_PATH': None
'JWT_AUTH_COOKIE_SECURE': True
'JWT_AUTH_COOKIE_SAMESITE': 'Lax'

with the following differences to django:

BREAKING CHANGES with this patch:

This changes the default Secure attribute from False (actually None as in not present in Set-Cookie) to True. Users wishing to use JWT cookies over http (as in no TLS) need to set JWT_AUTH_COOKIE_SECURE to False.

This change is intentional to follow common best common practice.

CHANGES:

Adds the default Samesite attribute Lax

nigoroll commented 4 years ago

d-oh, the samesite argument to .HttpResponse.set_cookie was added with django 2.1 Do we need to be compatible?

nigoroll commented 4 years ago

I have added support for Django < 2.1 Feel free to omit this commit if you are dropping support for earlier versions anyway

codecov-io commented 4 years ago

Codecov Report

Merging #29 into master will decrease coverage by 0.69%. The diff coverage is 84.61%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master     #29     +/-   ##
========================================
- Coverage     100%   99.3%   -0.7%     
========================================
  Files           8       8             
  Lines         281     289      +8     
  Branches       28      29      +1     
========================================
+ Hits          281     287      +6     
- Misses          0       1      +1     
- Partials        0       1      +1
Flag Coverage Δ
#codecov 99.3% <84.61%> (-0.7%) :arrow_down:
#dj111 99.3% <84.61%> (-0.7%) :arrow_down:
#dj20 99.3% <84.61%> (-0.7%) :arrow_down:
#dj21 100% <ø> (ø) :arrow_up:
#dj22 99.3% <84.61%> (-0.7%) :arrow_down:
#dj30 99.3% <84.61%> (-0.7%) :arrow_down:
#drf310 99.3% <84.61%> (-0.7%) :arrow_down:
#drf311 99.3% <84.61%> (-0.7%) :arrow_down:
#drf37 99.3% <84.61%> (-0.7%) :arrow_down:
#drf38 99.3% <84.61%> (-0.7%) :arrow_down:
#drf39 99.3% <84.61%> (-0.7%) :arrow_down:
#py27 99.3% <84.61%> (-0.7%) :arrow_down:
#py34 99.3% <84.61%> (-0.7%) :arrow_down:
#py35 99.3% <84.61%> (-0.7%) :arrow_down:
#py36 99.3% <84.61%> (-0.7%) :arrow_down:
#py37 99.3% <84.61%> (-0.7%) :arrow_down:
#py38 100% <ø> (ø) :arrow_up:
Impacted Files Coverage Δ
src/rest_framework_jwt/settings.py 100% <ø> (ø) :arrow_up:
src/rest_framework_jwt/views.py 100% <100%> (ø) :arrow_up:
src/rest_framework_jwt/compat.py 86.66% <80%> (-13.34%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ddbf51c...543108c. Read the comment docs.

nigoroll commented 4 years ago

@fitodic thank you very much for your comprehensive and helpful review. I have taken in most of your suggestions, commented on them in detail, force-pushed the PR branch and hope to have not missed anything else (sorry for the changelog oversight). Other than that, please feel free to make any changes to these suggestions as you like.

Thank you again

nigoroll commented 4 years ago

force pushed

fitodic commented 4 years ago

Thanks for the pull request and the changes. I'll create the new release shortly so you can start using these changes right away.