I'd like to namespace the shared cookies in a standard way (based on the cookie domain), to prevent getting cookies that can't be deleted on a sub-sub domain.
For example, we have environments like *.example.com, *.a.dev.example.com, *.demo.example.com, *.stage.example.com. Cookies from *.example.com have been encrypted with a different key, so at least they are invalid in these environments, but the subsites can't actually overwrite/delete the cookie from *.example.com, so if you have a cookie from there, you can't login.
I could be wrong on this, here's a stackoverflow q with answers seeming to agree though - http://stackoverflow.com/questions/3923285/how-to-remove-main-domain-coo....
The only solution I could come up with is to namespace the cookies with the cookie domain, e.g. EXAMPLE_COM_CHOCOLATECHIP.
Thoughts?
Comments
Comment #1
glennpratt CreditAttribution: glennpratt commentedMy other thought is to restructure bakery to not care if a cookie from another domain is set, overwrite it if it needs one, ignore it if it doesn't.
In testing though, I can't seem to make that work, the cookie from the parent domain appears to win.
Comment #2
glennpratt CreditAttribution: glennpratt commentedIt seems parent domain cookies override sub domain cookies no matter what I do, so I went with namespace as the original proposal. Basically:
$type . md5($cookie_domain);
Resulting in:
D6 patch, D7 to follow.
Comment #3
glennpratt CreditAttribution: glennpratt commentedSorry, included another patch. Still rolling the 7.x patch.
Comment #4
glennpratt CreditAttribution: glennpratt commented7.x patch.
These patches also have add some DRY-ness to the SSL cookie patch.
Comment #5
glennpratt CreditAttribution: glennpratt commentedI'm changing this to a bug, I'm sure it's not major, but it's pretty frustrating for our use case. Basically we can only have one Bakery environment on our domain without it (no demo, no dev-stage, etc).
Comment #6
coltraneI understand it's pretty frustrating, but it's a feature request to handle your specific use case. I haven't reviewed the patch yet, will think on this more. I understand you want to support your work flow but I hesitate in all cases of increasing the complexity of the code base.
Comment #7
glennpratt CreditAttribution: glennpratt commentedI hear you, though I really think the complexity is minimal, considering the SSL patch that is already in. Here is the cookie name function from the patch:
Comment #8
bcmiller0 CreditAttribution: bcmiller0 commentedre-rolled against alpha3 tag, and also adding some missing cookie_secure's.
Comment #9
drummI like this patch, it will make #1795118: Better support for mixed SSL environments a smaller patch. I haven't had a chance to test it, but I can confirm it applies nicely and looks okay.
Comment #10
glennpratt CreditAttribution: glennpratt commentedI would love to see this patch go in or get some feedback - it's hard to roll other patches around it.
If the namespace is a show stopper, then maybe we can settle on using a function to name cookies and not smattering code everywhere that does it... that will make the namespace patch much smaller.
Comment #11
coltraneNote, for others to help with testing there's a VM with cucumber tests https://github.com/bjeavons/Bakery-Chef
(/me needs to document using it)
Comment #12
coltraneRerolled for D7 and D6.
Comment #13
BarisW CreditAttribution: BarisW commentedHi coltrane,
thanks for the patches. I've spend days debugging to get Bakery get past Varnish and it seemed that Varnish strips out cookies that don't start with a specific name. See http://help.getpantheon.com/pantheon/topics/_cookie_cookiename_works_on_...
By adding SESS in front of the cookie name (and by making sure the cookiename is lowercase), Varnish lets the cookie pass.
Comment #14
glennpratt CreditAttribution: glennpratt commented@BarisW - You probably have something in your VCL that is looking for SESS. Just add something similar to allow CHOCOLATECHIP.
If you need help, post your VCL somewhere and ping me in IRC.
Comment #15
coltraneYes, you should change your Varnish configuration to let the Bakery cookies pass. It would be incorrect to prefix Bakery cookies with "SESS".
Comment #16
BarisW CreditAttribution: BarisW commentedNot everyone is able to configure their Varnish configuration. For example; at Pantheon I can submit a request to get it added, but I cannot do it myself.
What if we introduce a variable to do this? Or better, make it pluggable?
By changing this:
Into:
So I can just implement:
Comment #17
coltraneThat would be, ok. In fact this whole hashing addition to the cookie name could be implemented by a custom module for the use-case that Glenn has. Would that be ok with you Glenn?
Comment #18
coltraneNeeds work and reroll because #1915494: Prefix data signature and check prior to unserialize was committed. I'll work on this.
Comment #19
coltraneAh, drupal_alter() requires a full bootstrap of course. That's not a viable option.
Attached patch introduces new variable bakery_cookie_extension to alter the Bakery cookie name per-site. Also includes variable_del()s.
Comment #20
drummLooks good, we're using this on {association,portland2013,sydney2013}.drupal.org.
Comment #21
coltranehttp://drupalcode.org/project/bakery.git/commit/60c74ef and http://drupalcode.org/project/bakery.git/commit/bcc6100