Notify about 'getAssertion' errors

RESOLVED FIXED in 1.4 S2 (28feb)

Status

defect
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: ferjm, Assigned: jedp)

Tracking

unspecified
1.4 S2 (28feb)
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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.

Updated

6 years ago
Blocks: 949053
Depends on: 938635
Jed, Lloyd, I would love to hear your thoughts about a potential addition of a 'onerror' callback to the mozId.watch() call.
Blocks: 938635
No longer depends on: 938635
Flags: needinfo?(lhilaiel)
Flags: needinfo?(jparsons)
+1 from me.  I would like this as well.
Flags: needinfo?(jparsons)
Hi, Fernando, would this be sufficient?

Lloyd, Sam, what do you think about the enhancement to the RP API?
Attachment #8363309 - Flags: feedback?(ferjmoreno)
(If the patch doesn't apply cleanly for you, try applying the DOM API patch from Bug 929386 first)

Comment 5

6 years ago
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 on attachment 8363309 [details] [diff] [review]
Add onerror handler to receive getAssertion errors

Thanks Jed!
Attachment #8363309 - Flags: feedback?(ferjmoreno) → feedback+
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
Attachment #8363309 - Flags: review?(ferjmoreno)
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 ?
Attachment #8363309 - Flags: review?(ferjmoreno)
Flags: needinfo?(lhilaiel)
Good point. Yes.

Updated

5 years ago
Blocks: 955951
Assignee: nobody → jparsons
Added test in toolkit/identity/tests/unit/test_firefox_accounts.js

Thanks for your review, Fernando!
Attachment #8363309 - Attachment is obsolete: true
Attachment #8378469 - Flags: review?(ferjmoreno)
Status: NEW → ASSIGNED
Attachment #8378469 - Flags: review?(ferjmoreno) → review+
Thank you, Fernando
Keywords: checkin-needed
With the bug # fixed in the commit message.
https://hg.mozilla.org/integration/b2g-inbound/rev/7a3702cd921d
Flags: in-testsuite+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/7a3702cd921d
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.4 S2 (28feb)
Depends on: 1008901
You need to log in before you can comment on or make changes to this bug.