JWT Cookie Authentication #75
Conversation
Codecov Report
@@ Coverage Diff @@
## master #75 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 17 17
Lines 280 280
Branches 31 31
=====================================
Hits 280 280
Continue to review full report at Codecov.
|
docs/cookie_authentication.rst
Outdated
| from django.urls import path | ||
|
|
||
| from graphql_extensions.views import GraphQLView | ||
| from graphql_jwt.decoratos import jwt_cookie |
There was a problem hiding this comment.
typo in from graphql_jwt.decorators import jwt_cookie
graphql_jwt/decorators.py
Outdated
| @@ -62,14 +64,14 @@ def check_perms(user): | |||
|
|
|||
| def token_auth(f): | |||
| @wraps(f) | |||
| @setup_jwt_cookie | |||
| def wrapper(cls, root, info, password, **kwargs): | |||
| def on_resolve(values): | |||
| user, payload = values | |||
| payload.token = get_token(user, info.context) | |||
There was a problem hiding this comment.
Don't we need to get the token from cookie in get_token?
There was a problem hiding this comment.
Not here, this is to request a new token.
There was a problem hiding this comment.
I was talking about adding the logic inside get_token function. Though it seems like not the right place. I did it like that: mtszsobczak@928fc4c looks like it is working ;)
There was a problem hiding this comment.
Yeah, that should work :)
| @@ -0,0 +1,13 @@ | |||
| Cookie authentication | |||
There was a problem hiding this comment.
I am really glad that you are working on this! Thanks :). I did some testing and it seems that when token is only in cookie request.user is an AnonymousUser. Cookie can be seen on request.cookies though. I am not very familiar with the code base so I am not sure whether the middleware or just get_token need extension (or none :) ). I am happy to help with this one.
There was a problem hiding this comment.
Thanks @mtszsobczak !,
I forgot to read the token from the cookie 😅, I'll update the PR soon.
| response = view_func(request, *args, **kwargs) | ||
|
|
||
| if hasattr(request, 'jwt'): | ||
| expiration = datetime.utcnow() + jwt_settings.JWT_EXPIRATION_DELTA |
| Cookie authentication | ||
| ===================== | ||
|
|
||
| When a token is requested and ``jwt_cookie`` decorator is set, the response will set the given cookie with the token string:: |
There was a problem hiding this comment.
What do you think about mentioning here (or somewhere else) that CSRF token should be used when using cookie as transport mechanism for jwt token?
| if hasattr(request, 'jwt'): | ||
| expiration = datetime.utcnow() + jwt_settings.JWT_EXPIRATION_DELTA | ||
|
|
||
| response.set_cookie( |
There was a problem hiding this comment.
What do you think about making the cookie secure only as an opt-in? Similar to https://docs.djangoproject.com/en/2.1/ref/settings/#std:setting-SESSION_COOKIE_SECURE
|
Tested and it seems to be working. Great work! |
Bumps [strawberry-graphql](https://github.com/strawberry-graphql/strawberry) from 0.76.0 to 0.76.1. - [Release notes](https://github.com/strawberry-graphql/strawberry/releases) - [Changelog](https://github.com/strawberry-graphql/strawberry/blob/main/CHANGELOG.md) - [Commits](strawberry-graphql/strawberry@0.76.0...0.76.1) --- updated-dependencies: - dependency-name: strawberry-graphql dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Resolve #55
When a token is requested and
jwt_cookiedecorator is set, the response will set the given cookie with the token string:Update: Used
graphene_django.viewsinstead ofgraphql_extensions.views.