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

Setting Domain on cookies #238

Open
NPGiorgi opened this issue Mar 22, 2021 · 10 comments
Open

Setting Domain on cookies #238

NPGiorgi opened this issue Mar 22, 2021 · 10 comments

Comments

@NPGiorgi
Copy link

Hey,

I was just recently trying to use cookies to authenticate an SPA against a Django backend running DRF and DJ Rest Auth.
Everything worked locally just fine.

After deploying I found that the cookies were not being set on the browser. After closer examination I realized that the cookies don't have the domain name property set, which causes that the cookies coming from different domains don't get set.

For example my backend is located at: api.app.example.com
And my SPA located at: app.example.com

I tried using Django's default SESSION_COOKIE_DOMAIN but it seems that is not supported. I took a look at the code and it seems there is no way to pass the domain to the cookie as implemented currently:

def set_jwt_access_cookie(response, access_token):
    from rest_framework_simplejwt.settings import api_settings as jwt_settings
    cookie_name = getattr(settings, 'JWT_AUTH_COOKIE', None)
    access_token_expiration = (timezone.now() + jwt_settings.ACCESS_TOKEN_LIFETIME)
    cookie_secure = getattr(settings, 'JWT_AUTH_SECURE', False)
    cookie_httponly = getattr(settings, 'JWT_AUTH_HTTPONLY', True)
    cookie_samesite = getattr(settings, 'JWT_AUTH_SAMESITE', 'Lax')

    if cookie_name:
        response.set_cookie(
            cookie_name,
            access_token,
            expires=access_token_expiration,
            secure=cookie_secure,
            httponly=cookie_httponly,
            samesite=cookie_samesite
        )

I suppose I could write a middleware to force the domain name into the cookie, but it doesn't seem ideal. Is there any other way to set the domain for the cookie? Ideally I would set it to something like .example.com.

@iMerica
Copy link
Owner

iMerica commented Mar 22, 2021

The last time I considered this feature it seemed to have created more problems since browsers behave very differently. I ended up running everything through the same domain and using an ingress/proxy to route traffic between the backend and frontend at different base paths. Like the main site at / and the api at /api/.

That said, response. set_cookie does indeed accept a domain parameter. Maybe something simple like JWT_COOKIE_DOMAIN could be convenient that we can pass along.

@NPGiorgi
Copy link
Author

As far as I know it should work.

I tested using django session and the SESSION_COOKIE_DOMAIN setting and the browser set the cookies correctly.

To my knowledge the problem is using cookies locally when a domain is set. That is handled differently by different browsers.

The ideal to me would be to set the cookie domain using Django's default setting.

If the value is provided it should be passed in the domain parameter of the set cookie call.

If the value is not provided, then the domain parameter of the set cookie call should not be passed. As far as I could say setting the domain to anything like an empty string, localhost, null, None, in the domain param in the set cookie function makes the cookies not work locally.

I would also add a console warning if anyone tries to run the app with DEBUG = True and the cookie domain set.

@NPGiorgi
Copy link
Author

NPGiorgi commented Mar 22, 2021

Just to follow up on this.

I extended the refresh token view and the login view in order to set the domain in the cookies:

from dj_rest_auth.views import LoginView
from django.conf import settings
from django.utils import timezone
from rest_framework_simplejwt.views import TokenRefreshView
from rest_framework_simplejwt.settings import api_settings as jwt_settings


class Login(LoginView):
    authentication_classes = []

    def get_response(self):
        response = super(WorkoutLogin, self).get_response()

        # ...
        
        if getattr(settings, "REST_USE_JWT", False):
            cookie_name = getattr(settings, "JWT_AUTH_COOKIE", None)
            refresh_cookie_name = getattr(settings, "JWT_AUTH_REFRESH_COOKIE", None)
            refresh_cookie_path = getattr(settings, "JWT_AUTH_REFRESH_COOKIE_PATH", "/")
            cookie_secure = getattr(settings, "JWT_AUTH_SECURE", False)
            cookie_httponly = getattr(settings, "JWT_AUTH_HTTPONLY", True)
            cookie_samesite = getattr(settings, "JWT_AUTH_SAMESITE", "Lax")

            # read domain from django settings
            cookie_domain = getattr(settings, "SESSION_COOKIE_DOMAIN", None)

            if cookie_name:
                response.set_cookie(
                    cookie_name,
                    self.access_token,
                    expires=access_token_expiration,
                    secure=cookie_secure,
                    domain=cookie_domain,
                    httponly=cookie_httponly,
                    samesite=cookie_samesite,
                )

            if refresh_cookie_name:
                response.set_cookie(
                    refresh_cookie_name,
                    self.refresh_token,
                    expires=refresh_token_expiration,
                    secure=cookie_secure,
                    domain=cookie_domain,
                    httponly=cookie_httponly,
                    samesite=cookie_samesite,
                    path=refresh_cookie_path,
                )
        return response


class RefreshToken(TokenRefreshView):
    def post(self, request, *args, **kwargs):
        response = super().post(request, *args, **kwargs)
        cookie_name = getattr(settings, "JWT_AUTH_COOKIE", None)
        if cookie_name and response.status_code == 200 and "access" in response.data:
            cookie_secure = getattr(settings, "JWT_AUTH_SECURE", False)
            cookie_httponly = getattr(settings, "JWT_AUTH_HTTPONLY", True)
            cookie_samesite = getattr(settings, "JWT_AUTH_SAMESITE", "Lax")

            # read domain from django settings
            cookie_domain = getattr(settings, "SESSION_COOKIE_DOMAIN", None)
            
            token_expiration = timezone.now() + jwt_settings.ACCESS_TOKEN_LIFETIME
            response.set_cookie(
                cookie_name,
                response.data["access"],
                expires=token_expiration,
                secure=cookie_secure,
                domain=cookie_domain,
                httponly=cookie_httponly,
                samesite=cookie_samesite,
            )

            response.data["access_token_expiration"] = token_expiration
        return response

And in the urls file:

urlpatterns = [
    path("auth/token/refresh/", api.RefreshToken.as_view(), name="token_refresh"),
    path("auth/login/", api.Login.as_view()),
    path("auth/", include("dj_rest_auth.urls")),
]

And settings:

if ENVIRONMENT !== "local":
    JWT_AUTH_SECURE = True
    SESSION_COOKIE_DOMAIN = ".example.com"

Tested after deploying, with the domain setup stated above and HTTPS enabled and required. The browser successfully sets the cookies.

@J-Priebe
Copy link
Contributor

Seconding this. For a package that is meant to bridge to gap between allauth and drf I don't think JWT can be called "supported" without this feature. 90% of users are SPAs running on a different domain/subdomain.

@theodormarcu
Copy link

theodormarcu commented Mar 9, 2023

+1 - this is a table stakes feature for supporting JWTs. Makes me lose long-term confidence in this library to not see such a table stakes feature being supported.

@ShahriarDhruvo
Copy link

ShahriarDhruvo commented Sep 3, 2023

@iMerica you said "Maybe something simple like JWT_COOKIE_DOMAIN could be convenient that we can pass along." So, do you plan to do that? And why didn't the merge request #294 was closed without merging! We really need this man! Just as @NPGiorgi said if the dev find it difficult to run in the local setting for this config. They should already know what they are trying to enable from the doc or you can also print some sort of warning in the "runserver" command's console.

It should be the defacto standard to pass domain. Can you go into more details why you decided to opt-out from including this feature?

@smallfish06
Copy link

Are there any further discussion with this issue?
@iMerica

@adrenaline681
Copy link
Contributor

adrenaline681 commented Nov 13, 2023

I'm also using a SPA and I need to set my auth token on all subdomains .example.com instead of just on my backend domain api.example.com.

Is there really no way of setting the domain of the cookies? It seems like it should be a must-have feature for a package that deals with setting auth tokens inside cookies.

@adrenaline681
Copy link
Contributor

The last time I considered this feature it seemed to have created more problems since browsers behave very differently. I ended up running everything through the same domain and using an ingress/proxy to route traffic between the backend and frontend at different base paths. Like the main site at / and the api at /api/.

That said, response. set_cookie does indeed accept a domain parameter. Maybe something simple like JWT_COOKIE_DOMAIN could be convenient that we can pass along.

Please include this feature, and make it optional for people who want to use it. It's a must-have for people who use SPA like ReactJs, Vue, etc.

@adrenaline681
Copy link
Contributor

adrenaline681 commented Nov 14, 2023

I created a pull request that fixes this issue, hopefully they accept it 🙏
#568

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

No branches or pull requests

7 participants