Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should IAuthenticationPolicy implementors add Vary headers? #1617

Closed
dstufft opened this issue Mar 24, 2015 · 4 comments
Closed

Should IAuthenticationPolicy implementors add Vary headers? #1617

dstufft opened this issue Mar 24, 2015 · 4 comments

Comments

@dstufft
Copy link
Contributor

dstufft commented Mar 24, 2015

Currently if you're using something like the BasicAuthenticationPolicy and you have a view that accesses request.unauthenticated_userid (either via the ACL system, or directly) you're going to have a view where the response is almost certainly dependent on the value of the Authorization header. The same is true of the SessionAuthenticationPolicy with (most likely? Is there ever a case it isnt?) the Cookie header.

I feel like these policies should possibly automatically (or by default?) add Vary headers to the response. Doing it anywhere else but within the IAuthenticationPolicy, such as a view, would require tight coupling with the type of IAuthenticationPolicy which in use which I don't think is the right approach to handling that.

@mmerickel
Copy link
Member

You may want to read and consider the comments in #1348.

On a side note, simply accessing the userid is not indicative of the content of the response. This would require an opt-out.

@dstufft
Copy link
Contributor Author

dstufft commented Mar 24, 2015

I'll take a look at that.

An opt out would be fine, I do think it should be an opt out though (versus an opt in), because the failure mode is a lot more forgiving:

  • If you add a Vary header when you didn't need to, the failure is that caches are less effective than they could otherwise be.
  • If you don't add a Vary header when you needed to, the cache is going to serve wrong responses.

@mcdonc
Copy link
Member

mcdonc commented Jun 6, 2015

We spoke about this a while in IRC a few days back and I think I came down on the side of "working as designed". For better or worse, folks who are relying heavily on reverse proxy caching, and whom need to as a result issue Vary headers in certain places should have their applications issue those Vary headers instead of the framework doing it on their behalf when almost certainly sometimes it will be issued when the framework user doesn't intend it to be, and might cause just as many problems. I won't pass judgment on which problem is worse, I'll just leave it up to the app developer to decide when and how to issue those headers, as he's the only one who really can.

A description of the problem and a drop-n-drool solution would be welcome in the cookbook, however.

@mmerickel
Copy link
Member

For now we're treating this as a documentation problem until some real patterns emerge that we may be able to codeify. This information should be included in #1845.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants