Closed Bug 1101444 Opened 5 years ago Closed 5 years ago

[Loop] For each invalid inserted verification code, Device receives a valid verification code.

Categories

(Firefox OS Graveyard :: Gaia::Loop, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

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

RESOLVED FIXED
2.2 S1 (5dec)
blocking-b2g 2.0+
Tracking Status
firefox34 --- wontfix
firefox35 --- wontfix
firefox36 --- fixed
b2g-v2.0 --- verified
b2g-v2.0M --- verified
b2g-v2.1 --- verified
b2g-v2.2 --- verified

People

(Reporter: lolimartinezcr, Assigned: ferjm)

References

Details

(Whiteboard: [mobile app])

Attachments

(2 files, 2 obsolete files)

flame
user
v2.0
188based
Gecko-de95811
Gaia-1ede266

Loop versión: 1.1, 38eadf0

STRs:
1. In device A, clicks Loop application.
2. In device A, clicks "Use Phone Number" button.
3. In device A, clicks "Or add your phone number" link
4. Write a phone number (different to phone number in SIM card) and clicks "Share" button -> Device B (with phone number inserted) receives SMS with autentication code.
5. In device A, inserts an invalid autentication code and clicks "Verify" button. 

Actual result:
 For each invalid inserted verification code, Device B receives a SMS with a valid verification code. (See attached video: http://youtu.be/FbufNSfRm6w)

Expected result:
When user inserts invalid verification code, in device A should shown a warning and not send other autentication code.
Blocks: 1036490
Whiteboard: [mobile app]
Fernando, as far as I know, the MSISDN server should not send a new verification code when you fails in entering the code. I would expect that it notifies about the fault and, only, when the user taps on "Resend" button a new verification button would be sent.
Wdyt? Has there been any change in the MSISDN server lately?
Flags: needinfo?(ferjmoreno)
Yes, this doesn't seem to be working fine. AFAIK no changes has been done that may have caused this. I'll take a look as soon as I can.
Flags: needinfo?(ferjmoreno)
Assignee: nobody → ferjmoreno
[Blocking Requested - why for this release]: Important failure and Fernando points that seems to be a platform issue. 
The MSISDN server should not send a new verification code when you fails in entering the code. I would expect that it notifies about the fault and, only, when the user taps on "Resend" button a new verification button would be sent.
blocking-b2g: --- → 2.0?
Attached patch v1 (obsolete) — Splinter Review
This patch fixes the issue. I am working on the tests for this now.
Attached patch v1 (obsolete) — Splinter Review
And now with the test.

I enabled the tests in TBPL disabling the MobileIdentityManger test and triggered a try run https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=f1ddb9f62f96

I still need to enable the tests for MobileIdentityManager in TBPL, but that's another story that should happen at bug 1073595
Attachment #8529017 - Attachment is obsolete: true
Attached patch v1Splinter Review
Sam, could you take a look at this patch, please?

It looks like a lot of changes, but it is basically moving blocks of codes from one place to another. The reason of this issue was that we were restarting the verification process (MobileIdentityVerificationFlow.doVerification) every time the user entered an invalid verification code. Now we only do that when the verification timeout expires. If the verification code entered by the user is wrong we just restart part of the verification flow (MobileIdentityVerificationFlow._doVerification).
As I mentioned in the comment above, I also added a test for this and enabled the tests in TBPL. It seems again like a lot of changes in the tests but what I did was moving the common part of what we had in test_mobileid_manager.js to head.js so we can use the mock helpers in the new test as well.
Thanks!
Attachment #8529159 - Attachment is obsolete: true
Attachment #8529168 - Flags: review?(spenrose)
Comment on attachment 8529168 [details] [diff] [review]
v1

Hi Fernando --

Thanks for the explanation of the patch. It makes sense, reads clearly, and builds cleanly. I have not run the xpcshell tests.
Attachment #8529168 - Flags: review?(spenrose) → review+
https://hg.mozilla.org/mozilla-central/rev/dce264e3c9f1
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Tested and working in:
flame
user
master 
188based
Gecko-8d8a729
Gaia-f20b72c
Loop version: master, fe0b50f

Will it be resolved in 2.0? Pending 2.0
Hi Wesly,
as you are reviewing 2.0 nominations, could you please consider also this bug? It's quite important for Loop application as it's using registration via Mobile ID.
Flags: needinfo?(wehuang)
Fernando, can you please ask for the approval for b2g32 and b2g34? This is an important bug for loop Mobile client
Flags: needinfo?(ferjmoreno)
Sorry for late, Maria!

[Triage] tag due to the importance of Loop, also a fix is available.
blocking-b2g: 2.0? → 2.0+
Flags: needinfo?(wehuang)
Comment on attachment 8529168 [details] [diff] [review]
v1

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): MobileID
User impact if declined: Wrong behavior and UX of the verification process that causes unnecessary costs to Mozilla and potential costs to the user.
Testing completed: Manual tests and unit tests added and running on TBPL.
Risk to taking this patch (and alternatives if risky): Low risk.
String or UUID changes made by this patch: None.
Flags: needinfo?(ferjmoreno)
Attachment #8529168 - Flags: approval-mozilla-b2g34?
Attachment #8529168 - Flags: approval-mozilla-b2g32?
Comment on attachment 8529168 [details] [diff] [review]
v1

Apologies on the late approval here, I agree on taking this.

Loli, Can you please help with verification once this lands on 2.0 branach.
Attachment #8529168 - Flags: approval-mozilla-b2g34?
Attachment #8529168 - Flags: approval-mozilla-b2g34+
Attachment #8529168 - Flags: approval-mozilla-b2g32?
Attachment #8529168 - Flags: approval-mozilla-b2g32+
(In reply to bhavana bajaj [:bajaj] from comment #15)
> Comment on attachment 8529168 [details] [diff] [review]
> v1
> 
> Apologies on the late approval here, I agree on taking this.
> 
> Loli, Can you please help with verification once this lands on 2.0 branach.

Hi Ryan, yes I will help with verification!
This bug has been successfully verified on woodduck 2.0 and Flame v2.0&2.1&2.2.
See attachment: verified_v2.1.mp4.
Reproduce rate: 0/5.

STR:
1. In device A, click Loop application.
2. In device A, click "Use Phone Number" button.
3. In device A, click "Or add your phone number" link(on flame,user can direct to input number)
4. Write a phone number (different to phone number in SIM card) and click "Share" button -> Device B (with phone number inserted) receives SMS with autentication code.
5. In device A, insert an invalid autentication code and clicks "Verify" button. 
**When user insert invalid verification code, device A shows the warning with the red input box flashing, and not send other autentication code.

Flame 2.2 build:
Gaia-Rev        f5e481d4caf9ffa561720a6fc9cf521a28bd8439
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/bb8d6034f5f2
Build-ID        20150111010223
Version         37.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150111.043244
FW-Date         Sun Jan 11 04:32:55 EST 2015
Bootloader      L1TC000118D0

Flame 2.1 build:
ia-Rev        64db236bea9a0510567ab7ced2f2b4688737123c
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/273f24a1d1fe
Build-ID        20150111001202
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150111.035122
FW-Date         Sun Jan 11 03:51:33 EST 2015
Bootloader      L1TC000118D0

Flame 2.0 build:
Gaia-Rev        31d6c9422cd0a8213df9f96019c9ab7168ec3ab3
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/a05a5378cb1f
Build-ID        20150111000203
Version         32.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150111.034429
FW-Date         Sun Jan 11 03:44:40 EST 2015
Bootloader      L1TC000118D0

Woodduck 2.0 build:
aia-Rev        7e55f20bc0f82397207cac6b5b477948652da62c
Gecko-Rev       5e2c8611f0705e5ee656e3bfad8e8ee9bde228fe
Build-ID        20150112050313
Version         32.0
Device-Name     jrdhz72_w_ff
FW-Release      4.4.2
FW-Incremental  1421010380
FW-Date         Mon Jan 12 05:06:48 CST 2015
You need to log in before you can comment on or make changes to this bug.