a stateless "goldilocks" api

[seanmonstar]

keeps current watch() api, but makes it "stateless" if RP doesn't want
session management. to indicate this, simply don't pass an onlogout
handler to watch().

navigator.id.watch({ onlogin: function(assertion) {} });
navigator.id.request(optionsAsNormal);

Two notes:

1. Since there's really only 1 option to pass to watch() for this, I'd think it would be nice to allow passing a function instead of an object with an onlogin parameter.
2. There's some confusion over whether we're keeping the same function names, or introducing configure and prompt. Changing it in this PR is really easy, the only changes would be in include.js.

[callahad]

We'll keep the same function names.

[callahad]

Also, this is awesome. (Have not yet fully reviewed)

[seanmonstar]

Woops, completely forgot about adding this to example RP. Will do in the
morning.

[jaredhirsch]

Yay @seanmonstar! I'll have a look after looking at Shane's mondo logging pull.

[lloyd]

I just did a code skim. This is awesome and minimal.

I really like the idea of making a single function argument equivalent to that function in an options block as the onlogin property. The only thing I'm unclear of is if we will want to introduce more watch parameters in the future.

❤️

[Standard8]

The only comment we had was, its potentially confusing re-using the onlogin property as you're not really logging into persona now, just getting an email verification confirmation.

[jaredhirsch]

ok I lied. but will look today

[jaredhirsch]

welp, looks good to me, though it needs to be rebased to pick up some rpinfo changes.

where do we go from here? merge it I guess?

[callahad]

This reads well to me. Haven't tested directly. What happens if a site upgrades from normal Observer to Goldilocks? Could a user get stuck with "logged_in" set, leading to spurious automatic onlogin calls?

[seanmonstar]

@callahad probably... I could explicitly set loggedInUser to null if theres no onlogout.

[seanmonstar]

Oh right, I remember now. The proposed usage of goldilocks is to only call navigator.id.watch() if the user isn't logged in. Once they are, you no longer need to call watch().

Should I explicitly remove logged_in if rpAPI = stateless?

[callahad]

@seanmonstar I'm digging around in my head, and the best I can come up with is:

1. RPs should be allowed (but not forced) to call .watch on every page. Nothing should break of happen unexpectedly.
2. If a site transitions from Observer to Goldilocks, currently logged-in users should not be affected, even if the site continues to call .watch on every page.
3. We should throw if the user supplies onready, onmatch, or loggedInUser to the Goldilocks watch, since those aren't a part of that API.

If the best way to achieve # 2 is through nuking logged_in when we see rpAPI === 'stateless', then by all means, nuke it :)

[jaredhirsch]

@callahad, when you say "we should throw" in (3) above, do you mean console.log the error?

[callahad]

@6a68 Right now we throw an Error if you don't supply both onlogin and onlogout when calling .watch().

We should similarly throw if you supply an Observer option (loggedInUser, onmatch, etc.) to a Goldilocks .watch() invocation.

[shane-tomlinson]

We should similarly throw if you supply an Observer option (loggedInUser, onmatch, etc.) to a Goldilocks .watch() invocation.

Hey, ho, we started some tests for include.js in a PR from the other day. Who would have imagined? https://github.com/mozilla/browserid/blob/4f22fc06d62607012a046a744a47c35299c6f555/resources/static/test/cases/include.js#L48-L58 When these errors are added, please update the tests.

[jaredhirsch]

@callahad ah yes, good point.

wow, include.js tests!

[jaredhirsch]

this looks good to me. @callahad thoughts? want to pull the trigger?

[jaredhirsch]

nobody has objections, merging in.

[callahad]

👏