Skip to content
This repository has been archived by the owner. It is now read-only.

watch() API logs out on page load with iOS 7 Safari #3905

Closed
jamesshore opened this issue Sep 20, 2013 · 53 comments
Closed

watch() API logs out on page load with iOS 7 Safari #3905

jamesshore opened this issue Sep 20, 2013 · 53 comments

Comments

@jamesshore
Copy link

@jamesshore jamesshore commented Sep 20, 2013

I'm getting early reports from users that they can't login when using Safari on iOS 7. Chrome on iOS 7 works fine. I'm using the watch() API. I'll update this issue as I have more information.

Update: Issue confirmed. To reproduce the issue, use Safari on iOS 7 to log into a site, such as http://123done.org, that uses the navigator.id.watch() API. After logging in, reload the page. (Some sites will reload the page automatically when you login.) Persona will call the onlogout function and log you out.

Original report:

Here's what I know so far:

  1. I saw several users (but not all) show up in my logs with a login immediately followed by a logout, repeated several times. It looks like users are logging in, automatically being logged out, and then manually trying again.
  2. I emailed every user that experienced this problem about an hour ago. I've gotten the following replies so far:

"That happened in Safari. I'll check it when I get a chance. Thanks."

"it seems the new Safari 7 on iOS 7 has some trouble with the new Persona API. Sorry, I don't know what goes wrong there, but I upgraded the iPad to iOS 7 just yesterday. Anyway, Chrome works fine, even on iOS 7."

  1. (Possibly unrelated.) I recently upgraded to the watch() API, so it's possible there's a bug in my implementation of it. My code is very simple, though. Here it is:
// Copyright (c) 2012-2013 Titanium I.T. LLC. All rights reserved.
/*global window, document, navigator, XMLHttpRequest, alert, setupPersona, $*/

(function() {
    "use strict";

    window.setupPersona = function(email) {
        $(".tdjs-signin").click(onSignin);
        $(".tdjs-signout").click(onSignout);
        navigator.id.watch({
            loggedInUser: email,
            onlogin: postLoginRequest,
            onlogout: postLogoutRequest
        });
    };

    function onSignin(event) {
        event.preventDefault();
        navigator.id.request({
            siteName: "Let’s Code JavaScript"
        });
    }

    function onSignout(event) {
        event.preventDefault();
        navigator.id.logout();
    }

    function postLoginRequest(assertion) {
        var post = $.post("/v3/auth/login", "assertion=" + assertion);
        post.done(function(data) {
            if (data === "ok") {
                window.location.reload();
            }
            else {
                alert("Login failed. Please try again.");
                // Must logout on failure or Persona will keep trying to validate assertion. But that will trigger
                // onlogout (postLogoutRequest()) synchronously, which will cause a window reload, so the logout must be
                // the last thing we do in this function.
                navigator.id.logout();
            }
        });
        post.fail(function(data, textStatus) {
            alert("Could not contact server to log in. Please try again.");
        });
    }

    function postLogoutRequest() {
        var post = $.post("/v3/auth/logout", "ios6_cache_buster=" + (new Date()).valueOf());
        post.done(function(data) {
            window.location.reload();
        });
        post.fail(function(data, textStatus) {
            alert("Could not contact server to log out. Please try again.");
        });
    }

}());
@edwindotcom
Copy link
Contributor

@edwindotcom edwindotcom commented Sep 20, 2013

Just a data point, I was able to sign in on the testing site, 123done.org successfully on iOS 7 ipad mini (most recent)

@jamesshore
Copy link
Author

@jamesshore jamesshore commented Sep 20, 2013

@edmoz I believe 123done.org uses a different version of the API, which may be relevant. I'll ask my users to try logging into 123done.org as well, though. You could also try logging into my site, http://www.letscodejavsacript.com .

@seanmonstar
Copy link
Member

@seanmonstar seanmonstar commented Sep 20, 2013

123done.org uses the production version of the watch() API. The dev version is at dev.123done.org.
Nothing sticks out in your code. I wonder, could Safari be too cachy w.r.t. location.reload()? Or... did their cookie policy change slightly?

@jrgm
Copy link
Contributor

@jrgm jrgm commented Sep 20, 2013

With IOS 7.0 (11A465) on an iphone 4S, 123done.org works to me. However, I tried two other RP's that use the watch API, and with both, I am initially signed in (I can see UI that reflects that state), but then I get logged out again.

@jamesshore
Copy link
Author

@jamesshore jamesshore commented Sep 21, 2013

I just upgraded my iPad to 7.0 and I've reproduced the problem on letscodejavascript.com. I've also confirmed that it doesn't occur on 123done.org. I'm digging into it now.

@pdehaan
Copy link
Contributor

@pdehaan pdehaan commented Sep 21, 2013

I'm able to repro on my iPad gen2 w/ Safari and iOS7 and letscodejavascript.com.
I can repro on the latest Chrome on iOS as well. (v 30.0.1599.12)
I can also repro on my iPhone 4S w/ iOS 7 and latest Safari.

@jamesshore
Copy link
Author

@jamesshore jamesshore commented Sep 21, 2013

It's almost certainly an issue with the watch() API. Here's the function flow I see when running the above letscodejavascript.com code on iOS 7.

(User loads page — page says user is logged out)
  setupPersona()
(User presses login button)
  onSignin()
  (User signs in via Persona UI)
  postLoginRequest()
  postLoginRequest.done() callback
    window.location.reload()
(Window reloads — page says user is logged in)
  setupPersona()
  postLogoutRequest() — note that no user action triggered this call
  postLogoutRequest.done() callback
    window.location.reload()
(Window reloads — page says user is logged out)
  setupPersona()
  (done)
@jamesshore
Copy link
Author

@jamesshore jamesshore commented Sep 21, 2013

I've confirmed that the issue does not occur with the navigator.id.get() API. (No surprise.)

@jamesshore
Copy link
Author

@jamesshore jamesshore commented Sep 21, 2013

I've reproduced the issue on 123done.org. Although 123done.org appears to work at first, if you refresh the page, you will be logged out.

When you login, 123done.org doesn't do window.location.reload(), which makes it seem like it's working, even on iOS 7. The problem is more apparent on letscodejavascript.com because it does an immediate reload() when the login state changes.

Now that we know how to reproduce the issue, I'm reverting letscodejavascript.com to the navigator.id.get() API, so you won't be able to reproduce it there.

@seanmonstar
Copy link
Member

@seanmonstar seanmonstar commented Sep 21, 2013

So essentially Persona is broken on iOS7 Safari. That's not good. This is
rightly a 5star. Will need to hotfix it.

@jamesshore
Copy link
Author

@jamesshore jamesshore commented Sep 21, 2013

Agreed. I've updated the issue title to be more specific.

@callahad
Copy link
Contributor

@callahad callahad commented Sep 21, 2013

As a quick fix, could we detect the iOS 7 UA string and use the redirect flow?

@seanmonstar
Copy link
Member

@seanmonstar seanmonstar commented Sep 21, 2013

Since on reload the comm iframe triggers a logout, I'm thinking redirect
would hit the same problem.

Did iOS7 make its cookie policy default to block all 3rd party?
On Sep 20, 2013 7:33 PM, "Dan Callahan" notifications@github.com wrote:

As a quick fix, could we detect the iOS 7 UA string and use the redirect
flow?


Reply to this email directly or view it on GitHubhttps://github.com//issues/3905#issuecomment-24854473
.

@jamesshore
Copy link
Author

@jamesshore jamesshore commented Sep 21, 2013

I just checked my recently-upgraded iPad and, yes, it was set to "Block cookies: From third parties and advertisers." (I can't say if that was the default or not, though.)

I changed the setting to "Block cookies: Never" and 123done.org started working properly.

@jaredhirsch
Copy link
Member

@jaredhirsch jaredhirsch commented Sep 21, 2013

@seanmonstar @jamesshore Yep, this is a change in cookie policy in iOS 7, I just confirmed it. Looks like "from visited" doesn't matter any longer: if i visit persona.org, then sign into 123done, then reload 123done, I see the login button.

Interestingly, I don't see any menu to allow third-party whitelisting. So either every third-party provider is broken, or you enable useful third-parties and the scumbag advertisers sneak in as well :-\

edit: I confirmed it by upgrading my phone and reproing login problems, not talking to MobileSafari folks.

@jaredhirsch
Copy link
Member

@jaredhirsch jaredhirsch commented Sep 21, 2013

ah, nice. "buggiest safari since 1.0". I'm not sure what we do with this bug, but it's definitely not a 5-star--we haven't regressed, MobileSafari did.

@callahad
Copy link
Contributor

@callahad callahad commented Sep 21, 2013

@6a68 This means that primaries are broken, too, right?

@jaredhirsch
Copy link
Member

@jaredhirsch jaredhirsch commented Sep 21, 2013

another data point: chrome update for iOS 7 doesn't have a third-party cookie option. It's just cookies enabled or disabled. And both browsers have Do Not Track preffed off post-upgrade, of course :-P

@callahad
Copy link
Contributor

@callahad callahad commented Sep 21, 2013

To restate the problem: Mobile Safari in iOS 7 defaults to "Block all third party cookies," which prevents session cookies from being transmitted over the communication iframe or the provisioning iframe. This means that:

  1. Persona can't see any indication that the user is logged in, and thus automatically fires onlogout on every page load. It is impossible to keep a user logged in across page loads.
  2. (Theoretically, but evidently not actually happening? See below.) Native Identity Providers, like Vinz Clortho or Eyedee.me, can't see any indication that the user is logged in, and thus will always raise a provisioning failure. It is impossible to get provisioned by a native IdP.

What are our options for short-term mitigation?

  1. Add a new error screen telling users to adjust their cookie policy.
    Fixes both, requires user action.
  2. Add a new error screen telling users to use Chrome for iOS.
    Fixes both, requires user action.
  3. Email persona-notices and tell RPs to insert a hack that ignores calls to onlogout before onready fires.
    Fixes logout loop, does not fix IdPs, requires RP action.
  4. Stop sending onlogout events to Mobile Safari on iOS.
    Fixes logout loop, does not fix IdPs, breaks sites expecting Persona-driven session management.

Am I missing anything?

@jaredhirsch
Copy link
Member

@jaredhirsch jaredhirsch commented Sep 21, 2013

hmm, primaries are working. i need to clear cache and poke at this a little more, but have to go afk for a while. i'll update later.

@callahad
Copy link
Contributor

@callahad callahad commented Sep 21, 2013

Woah, really? Real primaries shouldn't work. (Bridges should, since they're under *.login.persona.org)...

@jaredhirsch
Copy link
Member

@jaredhirsch jaredhirsch commented Sep 21, 2013

yeah, this is odd. mozilla LDAP and eyedee.me are both logging me into persona.org. Very strange.

To clarify: I can log into 123done, but on reload, the session is lost. If I click the login button again, I get the email picker from persona, implying my session is active. I pick an email, go back to 123done, it shows me logged in. On page refresh, it's lost again.

@jamesshore
Copy link
Author

@jamesshore jamesshore commented Sep 21, 2013

@6a68 "I'm not sure what we do with this bug, but it's definitely not a 5-star--we haven't regressed, MobileSafari did."

This is what drives me crazy about using Persona, and what makes me hesitant to recommend it to others. You're in charge of a critical piece of infrastructure. It doesn't matter whose fault it is. It has to work.

This is the third major problem I've seen come down the pike with Persona (the Yahoo alias problem; Google Chrome on iOS; and now this). Each time, the Mozilla attitude has been sort of laissez-faire. Meanwhile, I'm scrambling to take care of my customers and make sure they get the best experience possible. Because that's what you do when critical infrastructure doesn't work, because it doesn't matter who's to blame.

I'm just hoping this doesn't take weeks to fix like the other major breakages did.

@seanmonstar
Copy link
Member

@seanmonstar seanmonstar commented Sep 21, 2013

@jamesshore don't worry, we're scrambling. (we're all thinking and discussing this on a Saturday). iOS Safari is huge. What he meant is that we're not yet sure what to do. The .get() API works fine, since it doesn't use any cookies, but for all the sites that use watch(), how do we fix Safari for them?

@callahad
Copy link
Contributor

@callahad callahad commented Sep 21, 2013

@jamesshore That criticism is well-deserved. I sincerely apologize, again, for the difficulty.

That this is a regression in Mobile Safari is beside the point; The regression breaks our users, and it is incumbent upon us to find a way to mitigate it in the next few days.

Longer term, it's clear that we've over-engineered and over-complicated the Observer API. Before we leave Beta, we'll revamp our API to make this kind of session management opt-in to avoid similar issues in the future.

@callahad
Copy link
Contributor

@callahad callahad commented Sep 21, 2013

@jamesshore You can mitigate this by hacking around Persona-driven session management on your site. Here's a patch that disables Persona-initiated automatic logouts: https://gist.github.com/callahad/6654015/revisions

@jamesshore
Copy link
Author

@jamesshore jamesshore commented Sep 21, 2013

Thanks, @callahad. I do appreciate the work all of you are putting into Persona.

@jaredhirsch
Copy link
Member

@jaredhirsch jaredhirsch commented Sep 23, 2013

@jamesshore I didn't mean to give you the impression I was saying "WONTFIX/not my problem", it's just that the fix here is removing the need for third-party cookies at all, which would be a major, not-easy-to-hotfix kind of change. It's hard to see a quick fix that solves this for users, and that's really unfortunate.

@callahad
Copy link
Contributor

@callahad callahad commented Sep 23, 2013

@6a68 Have you been able to re-verify that true primaries are working? If so, why the heck are those third party cookies working while the ones to our communication iframe aren't?

Also, any thoughts on refusing to automatically trigger onlogout for iOS 7 as a temporary mitigation while we investigate further?

@jaredhirsch
Copy link
Member

@jaredhirsch jaredhirsch commented Sep 24, 2013

Here's the results from francois' troubleshooting page. This is from the iOS 7 simulator, but I get the same results on my phone:

screen shot 2013-09-23 at 9 39 26 pm

@lloyd
Copy link
Contributor

@lloyd lloyd commented Sep 24, 2013

At callahad's fist pumping, shane and I did some digging. It appears like the new default safari privacy setting on iOS7 has changed behavior. When you set "Block cookies" to "From third parties and advertisers", then:

  1. Cookies can be read and written when you're in a first party context
  2. Local storage can be read and written when you're in a first party context
  3. Cookies can be read (written - unknown) when you're in a third party context (iframe)
  4. Local storage is sandboxed in a third party context (iframe) and can be read and written, but has no relationship to local storage in the first party context.

bottom line - communication iframe can't access local storage values written by the dialog.

Suggested quick fix: disable onlogout events from comm iframe on ios7.

@callahad
Copy link
Contributor

@callahad callahad commented Sep 24, 2013

This also means that automatic login will also be broken -- basically, we can't do automatic session management / extension because we can't access the key material, right?

@lloyd
Copy link
Contributor

@lloyd lloyd commented Sep 24, 2013

correct.

@lloyd
Copy link
Contributor

@lloyd lloyd commented Sep 24, 2013

nor post redirect sign in, nor sso.

@callahad
Copy link
Contributor

@callahad callahad commented Sep 24, 2013

Derail...

Thankfully we don't need redirect-flow on Mobile Safari. Now we just have two cases: Chrome iOS must be redirect, Safari iOS must not be redirect. I'm cool with that.

Persona without session management is still a very compelling product. It's what the overwhelming majority of RPs want. Let's decouple the two. Standardize a minimal Persona API, without session management, for Strawplan. Rev the spec to include session management later, once we've had some more time to experiment and let things like SSO bake...

lloyd added a commit that referenced this issue Sep 24, 2013
@jaredhirsch
Copy link
Member

@jaredhirsch jaredhirsch commented Sep 24, 2013

holy crap I love having devs in europe, we're a 24-hour support shop now :-)

lloyd added a commit that referenced this issue Sep 24, 2013
lloyd added a commit that referenced this issue Sep 24, 2013
@lloyd
Copy link
Contributor

@lloyd lloyd commented Sep 24, 2013

test the mitigation on http://ios7.personatest.org - report back. please feel free to try lots of browsers to ensure no regression.

lloyd added a commit that referenced this issue Sep 24, 2013
@jaredhirsch
Copy link
Member

@jaredhirsch jaredhirsch commented Sep 24, 2013

@jamesshore we've merged in a fix and are discussing how soon we can hotfix it into production, more updates shortly

@jaredhirsch
Copy link
Member

@jaredhirsch jaredhirsch commented Sep 24, 2013

The fix works for me on these platforms, testing against ios7.123done.org:

  • Safari/iOS7, default cookie settings (3p disabled)
  • Chrome/iOS7, default cookie settings (cookies enabled, no 3p option)
  • IE8/XP, default cookie settings (yay p3p middleware)
  • Chrome/MacOS 10.8.5, default cookie settings and 3p disabled cookie settings
  • Firefox 24/MacOS 10.8.5, 3p cookies set to "Never", "Always", and "From Visited"

123done can't update session across desktop browser tabs with third-party cookies disabled, but does catch updates on refresh. This is the same as current production behavior.

@jamesshore
Copy link
Author

@jamesshore jamesshore commented Sep 24, 2013

@callahad Agreed about decoupling session management. My interest in Persona is for authentication. Session management is cool but scary because of the complexity. I would actually much prefer to use the get() API over the watch() API for that reason, if it worked on iOS Chrome.

@6a68 Thanks for the update, and sorry for beating on you. :-)

@edwindotcom
Copy link
Contributor

@edwindotcom edwindotcom commented Sep 24, 2013

main test: using ios7.123done.org - sign in - refresh - expect still logged in
ipad mini: Safari/iOS7, default cookie settings (Always, 3p disabled, Never) - PASS
samsung s4: Fx23/Android 4.2, No 3rd party cookies - FAIL can't sign in with primary (eyedee.me) but works with a persona as fallback idp (same as current PROD behavior)

@jaredhirsch
Copy link
Member

@jaredhirsch jaredhirsch commented Sep 24, 2013

@jamesshore no worries, I'd rather you tell us you're pissed than quit using Persona. (long time fan of your writing btw :-)

@jrgm any updates on the hotfix timeline?

@pdehaan
Copy link
Contributor

@pdehaan pdehaan commented Sep 24, 2013

Confirmed on latest iOS7 w/ latest Safari and Chome w/ default cookie settings on an iPhone 4S and iPad 2.
Safari: Do not track (off), Block cookies (from third parties and advertisers)

@lloyd
Copy link
Contributor

@lloyd lloyd commented Sep 25, 2013

tearing down ios7 vm, sounds like sufficient testing has been done to ensure we improved behavior on ios7 without regressing other platforms.

@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Sep 26, 2013

@jamesshore - the fix for iOS7 users is scheduled to go out on Monday, September 30th.

@jaredhirsch
Copy link
Member

@jaredhirsch jaredhirsch commented Sep 27, 2013

One final note, the front-end devs on signin are all in Apple's dev program now, so we can stay ahead of these things in the future and avoid drama/heroics/etc. 🍻

@jaredhirsch
Copy link
Member

@jaredhirsch jaredhirsch commented Sep 27, 2013

link to #3916: same issue, but on Safari desktop 6.1 on 10.8. The fix applies there as well.

@callahad
Copy link
Contributor

@callahad callahad commented Oct 1, 2013

This fix is now live in production.

@callahad callahad closed this Oct 1, 2013
@saverchenkov
Copy link

@saverchenkov saverchenkov commented Oct 4, 2013

We are using persona to sign in user inside an iframe on a page.

  • Main page
    • iframe (different domain)
      • include persona
      • navigator.id.watch()
      • navigator.id.request()

This approach works quite well in desktop browsers, including Safari (tested up to 6.1) and iOS6 (all versions).

However, this fix is still not working in this scenario on iOS7. Similar behavior occurs. User signs in and persona calls onready, but does not even call onlogin or onlogout callbacks. What is interesting is that neither onlogin nor onlogout callbacks are called. If I change block 3d party cookies settings to "never", it obviously works, so the issue is related.

I tried taking latest version of persona.js and patching it to force redirect flow instead of popup. It had no effect on iOS7 (using the default settings of blocking 3d party cookies). (Although it does work in iOS6 and desktop browsers).

You can test it live on http://udropr.com
(It is currently set to use redirect flow)

@floatingatoll
Copy link
Contributor

@floatingatoll floatingatoll commented Oct 8, 2013

For the record, the fixes for iOS 7 Safari with third-party cookie blocking also repaired my instance of desktop Safari 7.0 (9537.71) with the same "[x] From third parties and advertisers" option selected. See also https://bugzilla.mozilla.org/show_bug.cgi?id=918893

@michaellenaghan
Copy link

@michaellenaghan michaellenaghan commented Feb 10, 2014

In case others run into the same problem I'm seeing in Safari 7 and Mobile Safari 7: the sample code on the navigator.id API page doesn't call navigator.id.watch() until after making a call to the home server to get the current login state. That's what http://123done.org also does. That seems reasonable, but neither the Quick Setup Guide nor the Implementor's Guide suggest it. (I discovered this while commenting on a similar issue in a different thread.)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet