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

Update JWT cookie on token refresh #97

Open
lemirep opened this issue Jun 18, 2020 · 13 comments
Open

Update JWT cookie on token refresh #97

lemirep opened this issue Jun 18, 2020 · 13 comments

Comments

@lemirep
Copy link

lemirep commented Jun 18, 2020

Hello,

First of all thanks for all your efforts on the project, that's greatly appreciated.

I'm hitting an issue and I'm not sure if it's a bug or my lack of knowledge. Please excuse me for the inconvenience if that's the latter.

I've been playing with dj-rest-auth and the JWT support with http cookie storage.
Log in and log out work great with my client app. Upon login I don't need to pass in the access_token because of it being stored in the http cookie. However after a few minutes, the access token expires and I believe the cookie as well. My client tests for authentication errors on API calls and tries to refresh the token which it successfully achieves using the token/refresh/ url.
However, for subsequent authenticated api calls to work, I need to start passing the access token as part of the request header. I believe I'm either missing the API entry point that refresh the access_token and updates/recreates a new JWT cookie or that this could be actually missing from the project.
Could you please tell me if I'm not misled?
If I'm not, I'd be happy to create a new API endpoint to refresh token and update JWT cookie.

Thanks,

Paul

@alichass
Copy link
Contributor

alichass commented Jun 26, 2020

Hi there, its currently missing from the dj-rest-auth docs but simple-jwt has a default expiry of 5 minutes.
https://django-rest-framework-simplejwt.readthedocs.io/en/latest/settings.html#access-token-lifetime
You can change this in your project settings with:

SIMPLE_JWT = {
    'ACCESS_TOKEN_LIFETIME': timedelta(days=1),
    'REFRESH_TOKEN_LIFETIME': timedelta(days=2),
}

I would also suggest setting SIGNING_KEY to something, as by default simple-jwt uses your django secret key.
If the jwt signing key does get exposed you want to be able to change it without dealing with the hassle of also changing your django secret key and the complications that come with that.

Thanks for adding cookie refresh to the token refresh view <3 is a good idea

lemirep added a commit to lemirep/dj-rest-auth that referenced this issue Jul 3, 2020
Add a dedicated view that subclasses django-rest-framework-simplejwt's
TokenRefreshView to update the JWT_AUTH_COOKIE on successful refreshes.

Fixes: iMerica#97
@Popa-93
Copy link

Popa-93 commented Dec 7, 2020

Hi,
I am interested in getting this issue fixed cleanly.
It seems to be resolved by lemirep but not merged.
Any reason?

Regards

@Wolf-Byte
Copy link
Contributor

The token refresh & verify endpoints are available in dj-rest-auth, to enable them you must enable REST_USE_JWT in your settings.py

Once enabled the following endpoints will be added

path('token/verify/', TokenVerifyView.as_view(), name='token_verify'),
path('token/refresh/', get_refresh_view().as_view(), name='token_refresh'),

Note: If you resgistered dj_rest_auth.urls with a prefix the e.g. /auth it will be:

/auth/token/verify
/auth/token/refresh

The issue you have when you call these endpoints they use simple_jwt which expects the token to be passed in the body. When you use HTTP only cookies the data is sent in the head. This is not a huge issue as we can use middleware to move the data from the head to the body.

When I submitted the pull request for #160 I did not include the middleware as the issue leans more towards simple-jwt rather than DJ-rest-auth (they have a PR).

Here is an old stack overflow post I used to base my solution.
https://stackoverflow.com/questions/56587690/django-rest-framework-using-httponly-cookie

middleware.py

from <your_project> import settings
from django.utils.deprecation import MiddlewareMixin
import json


class MoveJWTCookieIntoTheBody(MiddlewareMixin):
    """
    for Django Rest Framework JWT's POST "/token-refresh" endpoint --- check for a 'token' in the request.COOKIES
    and if, add it to the body payload.
    """

    def __init__(self, get_response):
        self.get_response = get_response

    def __call__(self, request):
        response = self.get_response(request)
        return response

    def process_view(self, request, view_func, *view_args, **view_kwargs):
        if request.path == '/auth/token/verify/' and settings.JWT_AUTH_COOKIE in request.COOKIES:

            if request.body != b'':
                data = json.loads(request.body)
                data['token'] = request.COOKIES[settings.JWT_AUTH_COOKIE]
                request._body = json.dumps(data).encode('utf-8')
            else:
                # I cannot create a body if it is not passed so the client must send '{}'
                pass

        return None


class MoveJWTRefreshCookieIntoTheBody(MiddlewareMixin):
    """
    for Django Rest Framework JWT's POST "/token-refresh" endpoint --- check for a 'token' in the request.COOKIES
    and if, add it to the body payload.
    """

    def __init__(self, get_response):
        self.get_response = get_response

    def __call__(self, request):
        response = self.get_response(request)
        return response

    def process_view(self, request, view_func, *view_args, **view_kwargs):
        if request.path == '/auth/token/refresh/' and settings.JWT_AUTH_REFRESH_COOKIE in request.COOKIES:

            if request.body != b'':
                data = json.loads(request.body)
                data['refresh'] = request.COOKIES[settings.JWT_AUTH_REFRESH_COOKIE]
                request._body = json.dumps(data).encode('utf-8')
            else:
                # I cannot create a body if it is not passed so the client must send '{}'
                pass

        return None

If anyone has a solution to create an object with the token if no body data is sent that would be great. You will see from the code above if b'' is received the middleware fails. To work around this you need to ensure the client sends {}

Next, you need to add the middleware to your settings.py

MIDDLEWARE = [
    ...
    'django.middleware.clickjacking.XFrameOptionsMiddleware',

    'authentication.middleware.MoveJWTCookieIntoTheBody',
    'authentication.middleware.MoveJWTRefreshCookieIntoTheBody'
]

I hope this helps

@Wolf-Byte
Copy link
Contributor

Note the URLs in my snippet above are hardcoded, you could improve this by using reverse() on the url name to get the path dynamically.

@Popa-93
Copy link

Popa-93 commented Dec 8, 2020

Thank you so much for these explanations.
My understanding of the issue was wrong.
That's crystal clear now.

It's so easy now to setup a nice JWT token use .
May be this should be added to the doc even it's a workaround for an issue in a dependancy.

@Popa-93 Popa-93 closed this as completed Dec 10, 2020
@Popa-93
Copy link

Popa-93 commented Dec 10, 2020

As precised by @Wolf-Byte this issue concerns two packages:

  • dj-rest-auth's JWT cookies support was incomplete. This issue is fixed since version 2.1.1
  • django-rest-framework-simplejwt current implementation allows token in request's body only, not in headers. This has not been fixed yet. @Wolf-Byte proposed a workaround using some middlewares(explained in this thread).

Thank to @lemirep and @Wolf-Byte for their work.

@alexerhardt
Copy link

@Wolf-Byte I can see that this is simple-jwt problem, but after all simple-jwt is prescribed by dj-rest-auth... And so is the http-only cookie default, which I agree with.

I understand your solution, but how is this not included by default in dj-rest-auth?

Now every time I use the library with JWT (which will be nearly all of the time), I have to copy/paste your middleware in order to have a fully-closed request/response loop with http-only cookie.

Isn't this highly unergonomic? Why can't your solution or that of @lemirep be baked into the library?

@Popa-93
Copy link

Popa-93 commented Dec 14, 2020

@alexerhardt I agree with you.
I have closed this issue a little bit too fast.
It seems many users have troubles using dj-rest_auth with JWT due to this issue.
It would be good to find a more durable solution to ease dj-rest-auth use and develop the userbase.

We have 3 options :

  • enrich the documentation with the workaround
  • deliver the "middleware workaround" within dj-rest_auth + enrich the documentation for settings.py update.
  • handle the token in headers directly in dj-rest-auth at least temporary until this is fixed in simple-jwt

I have a preference for the last solution as it is straight forward for the users.
Let me know your opinion.

@Popa-93 Popa-93 reopened this Dec 14, 2020
@alexerhardt
Copy link

My two cents is that if you want a good development experience out of the package, it should be either option 2 or 3 - or anything that bakes the solution into the library, really.

Like I said, it's quite awkward that you have JWT as an option - albeit through an external lib - and that you enable http-only but then have to go through significant middleware patching to make it all work... (And BTW, I want to insist that the http-only cookie default is a great idea, it has nudged me to go for a hardened client design instead of sloppily saving tokens in localStorage)

However, if all this was baked in, the dev experience would be "holy sh*@, it works" - which is what everyone would like out of a library!

I am fairly new to Django so I'm not sure I should contribute code (though perhaps, as I think I have a good grasp on the issue), but I'd be happy to help with documentation or additional feedback.

I would also definitely contribute a more complete SPA React example if you are interested, complete with automated refresh token cycles via an axios interceptor. I think however it doesn't make sense as it stands because with dj-rest-auth as it currently stands one has to set the cookie on the client on refresh, thus breaking the http-only loop.

@Popa-93
Copy link

Popa-93 commented Dec 17, 2020

@alexerhardt, thank for your feedback.

As I am also inexperienced on this topic, I think it would be nice to have @iMerica opinion on the proposed solution.

@alexerhardt, your contribution proposal is very welcome, there is a issue about this with the tag Help wanted : Minimal example for SPA implementation of social login #147.

@hnnazm
Copy link

hnnazm commented Aug 25, 2022

As @Wolf-Byte said, the issue was the refresh_token endpoint is a POST method and expect a body.

Can the refresh token endpoint simply change to GET method? Since the access_token and refresh_token will be sent through cookies anyway.

I think anyone with SPA website using this library with JWT will hit this issue?

@ZunCreative
Copy link

I've encountered the same problem hence why I'm here.

Digging around a bit more on Reddit and Google, I realized using HttpOnly cookies for JWT is pretty close to just using the session authentication instead. So I went with that. I don't want to start a debate here. But can someone explain to me what advantage JWT cookies have over the standard session cookies from DRF?

@colehorvitz
Copy link

Is this still an issue in 2023? This is the only library that provides "out of the box" support for JWT in HttpOnly cookies, yet it seems that its implementation is still only half-baked. Pinging @iMerica, it would be great to have this fully supported instead of copy-pasting a three year old patch.

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 a pull request may close this issue.

8 participants