Opened 13 years ago

Closed 13 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: no UI/UX: no

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 13 years ago.
03-delete-cookie-from-correct-domain.diff (672 bytes ) - added by Chris Lamb 13 years ago.
15618_message_cookies_domain.diff (3.3 KB ) - added by Julien Phalip 13 years ago.

Download all attachments as: .zip

Change History (9)

by Chris Lamb, 13 years ago

comment:1 by Adrian Holovaty, 13 years ago

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 by Adrian Holovaty, 13 years ago

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 by Chris Lamb, 13 years ago

Resolution: fixed
Status: closedreopened

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

comment:4 by Niels Sandholt Busch, 13 years ago

Cc: niels.busch@… added

by Julien Phalip, 13 years ago

comment:5 by Julien Phalip, 13 years ago

Triage Stage: UnreviewedReady for checkin

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

comment:6 by Jannis Leidel, 13 years ago

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