Opened 6 years ago

Closed 6 years ago

#15618 closed (fixed)

django.contrib.messages.storage.fallback.CookieStorage does not behave properly with subdomains

Reported by: Chris Lamb Owned by: nobody
Component: Contrib apps Version: 1.2
Severity: Keywords:
Cc: Chris Lamb, niels.busch@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Imagine you have two domains "www.example.com" and "special.example.com". Your SESSION_COOKIE_DOMAIN is set to ".example.com" so that users are logged-in across these two subdomains.

The problem arises when a page on "www.example.com" sets a django.contrib.message and redirects to "special.example.com", the user will not see it unless they return to "www.example.com" as the default domain of the cookie is the current one. This naturally causes confusion as actions users have performed in the past suddenly are being confirmed (!).

This happens with FallbackStorage too as it wraps CookieStorage.

Patch attached that sets the domain of the CookieStorage cookie to SESSION_COOKIE_DOMAIN. Whilst this works, it might be better to not couple sessions and messages in this way, so we could alternatively introduce a new setting under a the MESSAGE_STORAGE_ namespace.

Attachments (3)

03-domain-of-messages-cookie.diff (612 bytes) - added by Chris Lamb 6 years ago.
03-delete-cookie-from-correct-domain.diff (672 bytes) - added by Chris Lamb 6 years ago.
15618_message_cookies_domain.diff (3.3 KB) - added by Julien Phalip 6 years ago.

Download all attachments as: .zip

Change History (9)

Changed 6 years ago by Chris Lamb

comment:1 Changed 6 years ago by Adrian Holovaty

Resolution: fixed
Status: newclosed

In [15848]:

Fixed #15618 -- CookieStorage storage in messages framework now honors SESSION_COOKIE_DOMAIN. Thanks for the report and patch, lamby

comment:2 Changed 6 years ago by Adrian Holovaty

Note that I opted to use SESSION_COOKIE_DOMAIN here even though this technically isn't for django.contrib.sessions. I didn't want to add another setting, and I couldn't think of a case where you might want the cookie domain to be different for messages vs. sessions. Down the road we may want to rename SESSION_COOKIE_DOMAIN to COOKIE_DOMAIN.

comment:3 Changed 6 years ago by Chris Lamb

Resolution: fixed
Status: closedreopened

Oops, we need to delete the cookie from the right domain too. Attaching patch.

Changed 6 years ago by Chris Lamb

comment:4 Changed 6 years ago by Niels Sandholt Busch

Cc: niels.busch@… added

Changed 6 years ago by Julien Phalip

comment:5 Changed 6 years ago by Julien Phalip

Triage Stage: UnreviewedReady for checkin

Thanks for spotting this. See attached patch with the final fix and tests for the whole thing.

comment:6 Changed 6 years ago by Jannis Leidel

Resolution: fixed
Status: reopenedclosed

In [15885]:

Fixed #15618 -- Fixed regression introduced in r15848 regarding not properly deleting messages cookies when honoring SESSION_COOKIE_DOMAIN. Thanks, Julien.

Note: See TracTickets for help on using tickets.
Back to Top