Add cookie domain support to Session and Auth snaplets

[sopvop]

Should be useful for correct cookie handling in custom session/auth implementations.

It sets same cookie parameters as setSecureCookie.

forgetSecureCookie didn't set httpOnly parameter on cookie, which could make some new browsers ignore it.

[sopvop]

Hm, I'm not sure info in snapframework/snap-core#212 was correct. I need to double check this.
Don't merge this yet.

[sopvop]

Ok, it works all right. Problem was elsewhere.
HttpOnly does not seem to affect firefox at all. But there were reports that it is important to some browsers.

[gregorycollins]

So do we need this function? I'm happy to merge if it's needed.

[sopvop]

It may be useful for symmetry with other functions in module.
But I have not confirmed that httponly flag is needed to expire cookies set with httponly.

[sopvop]

So, after doing some research, I've once again studied the rfc6265 which clearly states that HttpOnly and Secure flags don't affect stored cookie replacement.

11.  If the cookie store contains a cookie with the same name,
    domain, and path as the newly created cookie:

    1.  Let old-cookie be the existing cookie with the same name,
        domain, and path as the newly created cookie.  (Notice that
        this algorithm maintains the invariant that there is at most
        one such cookie.)

    2.  If the newly created cookie was received from a "non-HTTP"
        API and the old-cookie's http-only-flag is set, abort these
        steps and ignore the newly created cookie entirely.

    3.  Update the creation-time of the newly created cookie to
        match the creation-time of the old-cookie.

    4.  Remove the old-cookie from the cookie store.

However @reiddraper reported in snapframework/snap-core#212 that it does matter to some browsers. Maybe he can provide examples of browsers which this affects.

So, in my opinion this function should be added for symmetry with other exported functions, better code readability and future-proofing the implementation.

[sopvop]

Maybe actually rework these functions a bit.
Also allow session snaplet to set domain, as asked in #104

[mightybyte]

Seems reasonable to me. @ozataman?

[ozataman]

Seems reasonable to me as well. I agree with symmetricity comments - if we have it on secure cookies now, we may as well expose here also.

I am a bit annoyed, though, that the domain cant be dynamically determined in some Handler context. So you'd have to hardwire your production domain text into the codebase, which smells bad. I haven't thought about it enough, but that may be too large a redesign on the auth snaplet for the scope of change here.

On Jun 10, 2015, at 9:02 AM, Doug Beardsley [email protected] wrote:

Seems reasonable to me. @ozataman?

—
Reply to this email directly or view it on GitHub.

[reiddraper]

However @reiddraper reported in snapframework/snap-core#212 that it does matter to some browsers. Maybe he can provide examples of browsers which this affects.

I was using Chrome, somewhere near 43.0.2357.81 (it's probably been updated several times since reporting the issue). Definitely worth double-checking my findings.