945278 - Notify about 'getAssertion' errors

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Fernando Jiménez Moreno [:ferjm]]

The FxAccountsManager module introduced in bug 929386 exposes a .getAssertion() method that allows RPs to get fxa assertions via the navigator.mozId API.

In order to get an assertion, there must be a valid and verified account in the device. If there is no account signed in the device, the sign in UI flow is triggered, but this flow can end up with an error or with a non verified signed in account. In both cases we need to inform about the result (1) to the user and (2) to the developer.

For (1), IMHO the most critical case is the unverified account one, where the user is asked to create an account after clicking in a RP "Sign In" button and after creating (or login with) an account the assertion is not provided to the RP and there is no notification about it. I would suggest to introduce a new system dialog explaining that the user is trying to log in in a RP with an unverified account.

For (2), it would be great if we could add an 'onerror' callback to the current navigator.id API. This way, devs will know why the .request calls are failing and will be able to react properly.

Comment 1

[Fernando Jiménez Moreno [:ferjm]]

Jed, Lloyd, I would love to hear your thoughts about a potential addition of a 'onerror' callback to the mozId.watch() call.

Comment 2

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

+1 from me.  I would like this as well.

Comment 3

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Attachment: Add onerror handler to receive getAssertion errors

Hi, Fernando, would this be sufficient?

Lloyd, Sam, what do you think about the enhancement to the RP API?

Comment 4

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

(If the patch doesn't apply cleanly for you, try applying the DOM API patch from Bug 929386 first)

Comment 5

[Sam Penrose]

I think we need it. +1 to patch direction.

(In reply to Jed Parsons (use needinfo, please) [:jedp, :jparsons] from comment #3)
> Lloyd, Sam, what do you think about the enhancement to the RP API?

Comment 6

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8363309 [details] [diff] [review]
Add onerror handler to receive getAssertion errors

Thanks Jed!

Comment 7

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Comment on attachment 8363309 [details] [diff] [review]
Add onerror handler to receive getAssertion errors

I'm hearing that this is an ok addition.

ferjm, can I ask you for r=ferjm this time?  still look good to you?

Thanks!
j

Comment 8

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8363309 [details] [diff] [review]
Add onerror handler to receive getAssertion errors

Thanks Jed. LGTM. Is it possible to also add a test to toolkit/identity/tests/unit/test_firefox_accounts.js ?

Comment 9

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Good point. Yes.

Comment 10

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Attachment: Add onerror handler to receive getAssertion errors

Added test in toolkit/identity/tests/unit/test_firefox_accounts.js

Thanks for your review, Fernando!

Comment 11

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Thank you, Fernando

Comment 12

[Ryan VanderMeulen [:RyanVM]]

With the bug # fixed in the commit message.
https://hg.mozilla.org/integration/b2g-inbound/rev/7a3702cd921d

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7a3702cd921d