Fix #211, #217: Remove support for auto-login/logout.

[Osmose]

Removes logout callbacks altogether, and updates the login callback to only do anything if requestDeferred is set, which only happens when the page calls getAssertion, effectively disabling automatic login and logout.

[Osmose]

@willkg r?

@callahad It'd be great if you could take a look at this as well and let me know if there's anything I'm missing with regards to how I'm ignoring auto-login and logout with the API.

[callahad]

After a quick skim, I think you'll still get login loops. Since you're not setting loggedInUser, Persona's going to call onlogin or onlogout every time you load the page.

Also, you need to make a working onlogin available to Persona if you want Persona to work on browsers that require redirects instead of popups (Windows Phone, Chrome for iOS).

I'd maybe suggest something closer to:

/*
Goals:
  - Suppress automatic login/logout callbacks.
  - Don't break browsers that use redirects + onlogin instead of popups (Chrome/iOS, Windows Phone)
Method:
  - We always tell Persona that nobody (`null`) is logged in.
  - On login, we immediately tell Persona to log that user back out.
  - We don't *actually* log the user out; we just make Persona believe that nobody is logged in.
  - Because loggedInUser is `none`, all automatic `onlogout` calls are suppressed.
  - Because we wait until we see `onlogin` to clear Persona's state, browsers that need to autonomously fire onlogin are still able to do so.
Notes:
  - We need to make sure Persona is done logging the user out before we proceed with actually logging them in.
  - We can do that by abusing the `onlogout` callback to actually log the user in.
  - This won't break anything because we've effectively disabled `onlogout`.
*/

function actuallyLogin(assertion) {
  /* verify assertion, etc. */
}

var _assertion;

navigator.id.watch({
  loggedInUser: null,
  onlogin: function(assertion) {
    _assertion = assertion; /* Stash the assertion. */
    navigator.id.logout();  /* Clear Persona's state (then actually log the user in). */
  },
  onlogout: function() {
    /* Only fires once Persona finishes updating its state. */
    actuallyLogin(_assertion);
  }
});

[Osmose]

If we provide a working onlogin though, won't that mean that if someone has multiple tabs to the same site open, they'll all make login attempts in each tab? That's part of what we're trying to avoid here.

What if we store a flag in sessionStorage that marks if a login attempt is in progress? If the redirect login process stays in the same tab, it should be accessible when Persona redirects the user back to the page. That way, when onlogin is called, we only take action if the flag is set, meaning other tabs won't trigger an automatic login.

[callahad]

Gating onlogin with a sessionStorage flag, in addition to my proposed structure, might work to avoid multiple tab races, so long as sessionStorage actually persists across the RP -> Persona -> RP redirect cycle.

That might end up blocking first-time fallback users on Chrome for iOS and Windows Phone, since they'll loop out to their email client, then likely come back in a new window (or potentially a new browser)

[Osmose]

That might end up blocking first-time fallback users on Chrome for iOS and Windows Phone, since they'll loop out to their email client, then likely come back in a new window (or potentially a new browser)

It would only block them from auto-login after they click the email, hit Persona, and get redirected back to the site, wouldn't it? They'd just have to click login again. It's not a great experience, but may be worth it to avoid multiple tab races.

[willkg]

First off, I apologize for replying two weeks later.

Second, I read through this PR and the ensuing discussion in the comments and I think I'm pretty out of my depth. I don't have time right now to familiarize myself with the bits involved to be able to talk intelligently here. I might in April, but my April is beginning to look like the 12 Labors of Hercules.

[Osmose]

@callahad I've added a commit with the sessionStorage changes, but when I had solarce help me test it on Chrome for iOS it didn't work. Do you see anything obviously wrong?

(I don't have a device to test this out on, by the way. Know of any way of forcing the redirect flow?)

[Osmose]

@seanmonstar Perhaps you can help with my questions? ^

[jaredhirsch]

@Osmose Hey dude, you said the current implementation's not working on Chrome/iOS? Do you have a stage server someplace I can poke at?

[Osmose]

Finally got someone with a device to help me test, turns out I was just being dumb and forgetting that sessionStorage converts values into strings. DOH!

PR is updated with the latest code. It seems to work everywhere I've tested it.

[Osmose]

@callahad says r+ via IRC <3

[jaredhirsch]

:beers: awesome man! congrats!