[MobileID] First time an app requests a MobileID assertion for an already verified phone fails if it has no previous permission

VERIFIED FIXED in Firefox 34

Status

()

defect
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: ferjm, Assigned: ferjm)

Tracking

unspecified
mozilla35
x86
macOS
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking-b2g:2.0+, firefox33 wontfix, firefox34 fixed, firefox35 fixed, b2g-v2.0 verified, b2g-v2.0M verified, b2g-v2.1 verified, b2g-v2.2 verified)

Details

(Whiteboard: [Test-Run1])

Attachments

(2 attachments, 1 obsolete attachment)

No description provided.
Assignee: nobody → ferjmoreno
Posted patch v1 (obsolete) — Splinter Review
Fernando, what's the STR to reproduce this bug? It's not clear to me :(
Flags: needinfo?(ferjmoreno)
Ouch, of course, sorry.

STR with Loop:

1. Install and launch Loop.
2. Click on "Use phone number"
3. Verify a phone number and log into Loop with that number.
4. Uninstall, reinstall and launch Loop.
5. Click on "Use phone number".
6. Select the previously verified number.

Expected:

7. Log into Loop with that number.

Current:

7. The Mobile ID flow timeouts. Closing the Mobile ID dialog and
Flags: needinfo?(ferjmoreno)
Posted patch v1Splinter Review
Sam, this patch does two things:

1- Avoids making a /discover call when we already have credentials for a specific ICC. I saw this while looking into this issue so I thought I'd take the chance to also fix this.

2- Makes sure that we refresh the cached permission value before checking it after the user has made her choice and so she has already granted the permission. This was the main reason of this bug as we were triggering the prompt twice leaving the flow in an unstable state.

I also fixed a few tests that were failing. I am working on fixing bug 1073595 but unfortunately b2g emulator tests are really hard to launch :__( so it may take a few days.
Attachment #8497715 - Attachment is obsolete: true
Attachment #8499479 - Flags: review?(spenrose)

Comment 5

5 years ago
Comment on attachment 8499479 [details] [diff] [review]
v1

The edits are sensible in context and come with good commenting. By disabling 7 test functions (6 of them failing in the same way, presumably due to bug 1073595) I was able to get 96 checks to pass in the remaining test functions. The underlying state machine remains opaque to me. Thanks Fernando!
Attachment #8499479 - Flags: review?(spenrose) → review+
Thanks Sam!

The tests are green locally on the B2G emulator but for some reason not related with this bug are timing out on TBPL. I am working on it on bug 1073595.

https://hg.mozilla.org/integration/b2g-inbound/rev/2f99196c505e
Attachment #8499479 - Attachment description: bug1075070.mobileid.sicking → v1
https://hg.mozilla.org/mozilla-central/rev/2f99196c505e
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
[Blocking Requested - why for this release]: Failing the first time that we use the Mobile ID flow is a bad UX.
blocking-b2g: --- → 2.1?
Duplicate of this bug: 1077406
(In reply to Fernando Jiménez Moreno [:ferjm] from comment #8)
> [Blocking Requested - why for this release]: Failing the first time that we
> use the Mobile ID flow is a bad UX.

please seek aurora uplift when ready to land.
blocking-b2g: 2.1? → 2.1+
Flags: needinfo?(ferjmoreno)
Comment on attachment 8499479 [details] [diff] [review]
v1

Approval Request Comment
[Feature/regressing bug #]: Mobile ID.
[User impact if declined]: Without this patch a privileged app will fail to obtain a verified phone number the first time that it uses the Mobile ID flow. The second time it will work.
[Describe test coverage new/current, TBPL]: Unit tests added.
[Risks and why]: Low risk. We've been testing this feature on master and no regression has been found.
[String/UUID change made/needed]: None.
Attachment #8499479 - Flags: approval-mozilla-aurora?
Flags: needinfo?(ferjmoreno)
Attachment #8499479 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Duplicate of this bug: 1083214
[Blocking Requested - why for this release]: 
When this issue raised we had doubts about asking the uplift to 2.0, we considered that it should not happen very often for normal users as it does not happen the first time user insert a SIM and register in Loop. 

We were wrong, the problem is that we are going to upload the Loop application to the Market Place and can be downloaded in countries where the SIM change is really often (e.g in South America) and this could be an important issue for many of our target markets.

Apart of this, this issue is making us difficult the tests automation, something we need to improve and pay special attention as Loop application depends on MSIDN and Loop Server changes.
blocking-b2g: 2.1+ → 2.0?
You may also need uplift here.
blocking-b2g: 2.0? → 2.0+

Updated

5 years ago
Whiteboard: [Test-Run1]
Comment on attachment 8499479 [details] [diff] [review]
v1

Same as comment 12 plus the explanation at comment 14.
Attachment #8499479 - Flags: approval-mozilla-b2g32?
Hi Bhavana, we need the approval to uplift the patch to 2.0... can you help us with this?
Thanks a lot!!
Flags: needinfo?(bbajaj)
Flags: needinfo?(bbajaj)
Attachment #8499479 - Flags: approval-mozilla-b2g32? → approval-mozilla-b2g32+

Comment 20

5 years ago
Verified on FLAME (184based.B-43.Gecko-675d616.Gaia-2183b4f), Loop version 1.1: 5806319

It's still failing on FireE, v2.0-SW2E3-1 (waiting for uplifted).

Comment 21

5 years ago
Verified on FireE, firee-kk-v2.0-SW2E5-4,
Loop version, 1.1: cc87bd0
Status: RESOLVED → VERIFIED
This issue has been verified successfully on Flame 2.0, 2.1, 2.2 and woodduck 2.0
See attachment: 1412.MP4
Reproducing rate: 0/5

Step:
1. Install and launch Loop.
2. Click on "Use phone number".
3. Verify a phone number and log into Loop with that number.
4. Uninstall, reinstall and launch Loop.
5. Click on "Use phone number".
6. Select the previously verified number.

Actual result:
6. Log into Loop with that number.

Woodduck version:
Gaia-Rev        ead3b72a84512750bc5faff4e9e8faa1715c0d05
Gecko-Rev       8d40d6480ee0e628b0f7655dcd6ff79a2f2fbcfc
Build-ID        20141211050313
Version         32.0
Device-Name     jrdhz72_w_ff
FW-Release      4.4.2
FW-Incremental  1418245573
FW-Date         Thu Dec 11 05:06:41 CST 2014

Flame 2.1 version:
Gaia-Rev        c226db212db4d824c09617cd6dc407b2d4258d9b
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-2g34_v2_1/rev/cf8bebfa4703
Build-ID        20141210001201
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141210.035300
FW-Date         Wed Dec 10 03:53:11 EST 2014
Bootloader      L1TC00011880

Flame 2.2 version:
Gaia-Rev        e17c5656dbf517d48fb61ac9bc92119e023fd717
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/be1f49e80d2d
Build-ID        20141210040201
Version         37.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141210.074809
FW-Date         Wed Dec 10 07:48:20 EST 2014
Bootloader      L1TC00011880

Flame 2.0 version:
Gaia-Rev        856863962362030174bae4e03d59c3ebbc182473
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-2g32_v2_0/rev/2d0860bd0225
Build-ID        20141210000202
Version         32.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141210.034839
FW-Date         Wed Dec 10 03:48:50 EST 2014
Bootloader      L1TC00011880
The Loop version:bd8f1c2
You need to log in before you can comment on or make changes to this bug.