Code

Opened 3 years ago

Closed 3 years ago

#15618 closed (fixed)

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

Reported by: lamby Owned by: nobody
Component: Contrib apps Version: 1.2
Severity: Keywords:
Cc: lamby, 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 lamby 3 years ago.
03-delete-cookie-from-correct-domain.diff (672 bytes) - added by lamby 3 years ago.
15618_message_cookies_domain.diff (3.3 KB) - added by julien 3 years ago.

Download all attachments as: .zip

Change History (9)

Changed 3 years ago by lamby

comment:1 Changed 3 years ago by adrian

  • Resolution set to fixed
  • Status changed from new to closed

In [15848]:

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

comment:2 Changed 3 years ago by adrian

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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 3 years ago by lamby

  • Resolution fixed deleted
  • Status changed from closed to reopened

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

Changed 3 years ago by lamby

comment:4 Changed 3 years ago by niels

  • Cc niels.busch@… added

Changed 3 years ago by julien

comment:5 Changed 3 years ago by julien

  • Triage Stage changed from Unreviewed to Ready for checkin

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

comment:6 Changed 3 years ago by jezdez

  • Resolution set to fixed
  • Status changed from reopened to closed

In [15885]:

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

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.