Bug 139683 - "Allow from current website only" privacy setting strips cookies from 302 redirects
Summary: "Allow from current website only" privacy setting strips cookies from 302 red...
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.10
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-12-16 10:54 PST by Patrick Toomey
Modified: 2015-09-01 16:39 PDT (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick Toomey 2014-12-16 10:54:03 PST
We are seeing cookies stripped from requests that result from a 302 redirect for users using Safari 8 with the "Allow from current website only" privacy setting. This was discovered while debugging an issue where users were unable to authorize an application using OAuth with Safari. A typical OAuth flow is:

1. Client application/site does a 302 redirect (or directly links to) the OAuth provider.
2. OAuth provider validates the user is logged in and lets the user authorize the client application.
3. OAuth provider does a 302 redirect (or meta-refresh) back to the client application with a `code` to be used to exchange for a valid OAuth token.

We are seeing cookies stripped from the requests resulting from the 302 redirect that occurs in step 1 and step 3. Despite them being functionally similar, the meta-refresh redirection approach does not strip cookies from the request back to the client application. From a privacy perspective it seems these two approaches (302 vs. meta-refresh) should work the same (i.e. they should either both strip cookies or both not strip cookies). From a functional perspective, many sites rely on cookie bearing 302 redirects for OAuth (and likely other use cases). So, the hope is that this is a bug and that cookies should be sent along with requests that result from a 302 redirect (or meta-refresh).
Comment 1 Patrick Toomey 2015-09-01 13:11:55 PDT
In order to work around this issue for our users we changed all our OAuth redirects to use a meta refresh instead of performing a 302 redirect. This generally works, but there are scenarios where it is not ideal. For example, we found one scenario where a page embedded something like the following

`<img src="https://some_oauth_app/generate_some_image?width=50&length=50">`

The above is a reference to an OAuth app that will perform the OAuth dance with GitHub.com if the user doesn't currently have an active session. Unfortunately, it looks like subresource requests like this will not follow meta refresh based redirects. So, the only way to not break these uses is to use a 302 redirect. But, that brings us back to the original issue described above.

This seems to be an issue a number of people have encountered:

https://github.com/intridea/omniauth-oauth2/issues/72
https://github.com/zotonic/zotonic/issues/902

People seem to generally be working around it using this meta refresh approach. This Safari behavior seems to break OAuth in a fundamental way. OAuth applications are required to prevent CSRF during the OAuth dance by passing a `state` value. This value is generally tracked in the originating site's session. However, by stripping cookies on the final redirect the originating site has no session with which to validate `state`.
Comment 2 Alexey Proskuryakov 2015-09-01 15:40:02 PDT
Thank you for the reminder! This turned out to be an issue below WebKit, which should be fixed in El Capitan public beta.

Closing as INVALID since it was not a WebKit bug.
Comment 3 Patrick Toomey 2015-09-01 15:57:56 PDT
That is great news! Can you clarify one point? When I first submitted this radar I only noticed the cookie stripping on a 302 redirect during the oauth flow. But, I recently noticed that the cookie stripping seems to occur on any third-party request. For example, if I set an image source to https://some_site/authenticated_image it also doesn't send previously set cookies. Will this upcoming fix work with subresources in addition to 302 redirects?
Comment 4 Alexey Proskuryakov 2015-09-01 16:39:10 PDT
I encourage you to test with the public beta. I would encourage you to test with a public beta to see if it works.