cur_domain is effectively equivalent to '.' + cur_domain and therefor…

[ericatkin]

…e negates the effect of wild_domain

See PR #3586

[digitalresistor]

Argh... those build failures are not yours. Looks like pytest made some changes again. Please stand by, I'll get those fixed and have you rebase afterwards.

[digitalresistor]

@ericatkin would you please rebase this on top of the new master. Thank you!

[ericatkin]

So, I assumed that adding both cur_domain and '.' + cur_domain was a compatibility thing (perhaps some browsers have some behavior I'm not aware of), but if not, we could add just cur_domain for the same effect without the redundancy?!

[mmerickel]

I don't mean to wax nostalgic too much but wild_domain was my first PR I ever did for an open source project that I can recall. I caught @mcdonc whining about users who complain without contributing and felt I had to do something so that I could keep complaining. :-) It's been a journey, glad to see it didn't actually fix the problem. I wonder if some browsers treat it different or if I just didn't test it enough back then.

#98

[ericatkin]

I certainly misunderstood the issue at first. The standard seems backward to me, you specify a name to trigger wild card behavior, and an unspecified domain means the more restrictive behavior (so called "host-only" cookie).

…

On Thu, May 28, 2020, 7:33 PM Michael Merickel ***@***.***> wrote: I don't mean to wax nostalgic too much but wild_domain was my first PR I ever did for an open source project that I can recall. :-) It's been a journey, glad to see it didn't actually fix the problem. I wonder if some browsers treat it different or if I just didn't test it enough back then. #98 <#98> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3587 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAVJ6ECKD5RCIUCWGZPGIRTRT4GFBANCNFSM4NNNAG2A> .

[digitalresistor]

We should drop this line of the code, it is unnecessary and we should probably change the logic to either append cur_domain or append None, not do both. There is no reason to send two cookies in this case.

[digitalresistor]

Thanks for working with us @ericatkin. Re-reading the spec and what is expected, we should either set the Domain attribute of the cookie or we should not. Sending a cookie with the Domain attribute and without at the same time wastes bandwidth and doesn't actually change anything substantially (other than if the app is misconfigured and the Domain attribute would be wrong, but people should really make sure their WSGI environment matches what the browser is sending/expecting).

[shadow-light]

Here's the spec for those following along: https://tools.ietf.org/html/rfc6265#section-4.1.2.3

[digitalresistor]

This looks good to me, thank you!

[mmerickel]

Ok so I think we should backport this to 1.10. I can look at doing that this weekend if someone doesn't get to it first.

[nandoflorestan]

Is this why Pyramid is creating a 2nd, undesired auth_tkt cookie for a domain I did not request? I mean, if I have "my-domain.com", Pyramid is creating an extra auth_tkt cookie for ".my-domain.com" (with a dot at the start).

Can I just wait for this fix to be released and trust it will go away? I am on 1.10.4, the latest.

By the way, my code is this...

config.set_authentication_policy(
    AuthTktAuthenticationPolicy(
        secret,
        callback=effective_principals,
        wild_domain=False,
        parent_domain=False,
        secure=secure,
        samesite="strict",
    )
)

[mmerickel]

@nandoflorestan I'm not sure I see in the code how you'd be seeing .my-domain.com in the 1.10.4 codebase if wild_domain and parent_domain are false. Do you? Maybe you can debug that in your environment and tell us why that's happening for you.

https://github.com/Pylons/pyramid/blob/1.10.4/src/pyramid/authentication.py#L909-L932

[nandoflorestan]

@mmerickel

I edited /etc/hosts to add this line:

127.0.0.1       local.host

...because the issue only happens when the domain name contains a dot.

Testing on http://local.host:6543/ I saw the extra auth_tkt cookie appear when I logged in.

In order to debug this, it is necessary to restart waitress. I saw that the value of the variable "domains" is [None, 'local.host'], as you would expect.

But at the end, when profile.get_headers(value, **kw) was called with kw == {'domains': [None, 'local.host']}, it returned this:

[('Set-Cookie', 'auth_tkt=5cd21111c756ca02499eb331385ca6730e64d9f4fd21e05b55c39235b5bcb14d9e7d6987b2e7327e3e68315bf8665bd9e11c55af7cfacc6a9f25d33838e4d2d55f48f34a1!userid_type:int; Path=/; SameSite=strict'), ('Set-Cookie', 'auth_tkt=5cd21111c756ca02499eb331385ca6730e64d9f4fd21e05b55c39235b5bcb14d9e7d6987b2e7327e3e68315bf8665bd9e11c55af7cfacc6a9f25d33838e4d2d55f48f34a1!userid_type:int; Domain=local.host; Path=/; SameSite=strict')]

The above contains 2 cookies. The one without a domain is undesired.

profile is a webob.cookies.CookieProfile object. pip says my webob is up-to-date at 1.8.6.

The issue would be fixed by the removal of the line
https://github.com/Pylons/pyramid/blob/1.10.4/src/pyramid/authentication.py#L919

In fact, I don't know why that line would be there at all.

I am sorry if I am talking about something different from this issue's subject. If you want me to open a separate ticket, please say so.

[mmerickel]

@nandoflorestan that's the expected behavior. You made it sound originally like it was returning a cookie for .local.host, not local.host which would be relevant to this issue/PR we're hijacking. The former would be due to wild_domain or parent_domain depending on your use case and you had those off so I was concerned. There is not a way to disable the None cookie right now. You can open a separate issue about this if you want but there's no bug here it sounds like.

[nandoflorestan]

I am sorry for the noise. I have opened #3609

[mmerickel]

I've decided to not backport this because while it is a bug, it's also a bw-incompat change that apps may be inadvertently relying on. I don't want to change something that's been operating in that way for so many years in a bugfix release.