Send MCC and MNC on POST /sms/mt/verify

RESOLVED FIXED in Firefox 34, Firefox OS v2.0

Status

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: ferjm, Assigned: ferjm)

Tracking

unspecified
mozilla34
All
Gonk (Firefox OS)
Points:
---

Firefox Tracking Flags

(blocking-b2g:2.0+, firefox32 wontfix, firefox33 wontfix, firefox34 fixed, b2g-v2.0 fixed, b2g-v2.1 fixed)

Details

(Whiteboard: [qa?])

Attachments

(1 attachment)

(Assignee)

Comment 1

4 years ago
We need this change landed on 2.0 to allow the usage of Mobile ID on USA and Canada.
Assignee: nobody → ferjmoreno
blocking-b2g: --- → 2.0?
(Assignee)

Comment 2

4 years ago
Created attachment 8463429 [details] [diff] [review]
v1

Sam, this simple change adds the MCC and MNC (this one only when we know it) values to the already existing /sms/mt/verify request [1]. We need this addition to let the server decide which phone number to use to send the verification codes to the client via SMS. So far the server was using a default number, but that doesn't work for US and Canada (bug 1042866) and wouldn't scale on an scenario where we need to use an specific SMS sender for a specific MCC/MNC pair (i.e, if we have a carrier providing free SMS for their customers).

Thanks for your review!

[1] https://github.com/mozilla-services/msisdn-gateway/blob/master/API.md#post-v1msisdnsmsmtverify
Attachment #8463429 - Flags: review?(spenrose)

Comment 3

4 years ago
Comment on attachment 8463429 [details] [diff] [review]
v1

Review of attachment 8463429 [details] [diff] [review]:
-----------------------------------------------------------------

The code changes are clean, and fit with their contexts. The unit tests run and exercise at least part of the patch. r+ as-is.

Notes on the services/mobileid package: It's very helpful that it follows the pattern in services/fxaccounts. It could really use an explanation of the basic control flow. The names are a bit opaque and assume familiarity with the the caller and data types, for example the "MoMt" in "MobileIdentitySmsMoMtVerificationFlow", or the parameter names in this block:

+      this.activeVerificationFlow = new MobileIdentitySmsMtVerificationFlow({
+          origin: aOrigin,
+          msisdn: aToVerify.msisdn,
+          mcc: aToVerify.mcc,
+          mnc: aToVerify.mnc,
+          iccId: aToVerify.iccId,
+          external: aToVerify.serviceId === undefined,
+          mtSender: aToVerify.verificationDetails.mtSender
+        },
Attachment #8463429 - Flags: review?(spenrose) → review+
Whiteboard: [qa?]
(Assignee)

Comment 4

4 years ago
Thanks Sam!

(In reply to Sam Penrose from comment #3)
> Comment on attachment 8463429 [details] [diff] [review]
> v1
> 
> Review of attachment 8463429 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> The code changes are clean, and fit with their contexts. The unit tests run
> and exercise at least part of the patch. r+ as-is.
> 
> Notes on the services/mobileid package: It's very helpful that it follows
> the pattern in services/fxaccounts. It could really use an explanation of
> the basic control flow. The names are a bit opaque and assume familiarity
> with the the caller and data types, for example the "MoMt" in
> "MobileIdentitySmsMoMtVerificationFlow", or the parameter names in this
> block:
> 
> +      this.activeVerificationFlow = new
> MobileIdentitySmsMtVerificationFlow({
> +          origin: aOrigin,
> +          msisdn: aToVerify.msisdn,
> +          mcc: aToVerify.mcc,
> +          mnc: aToVerify.mnc,
> +          iccId: aToVerify.iccId,
> +          external: aToVerify.serviceId === undefined,
> +          mtSender: aToVerify.verificationDetails.mtSender
> +        },

Yes. I have a pending task about documenting all about the client implementation at https://wiki.mozilla.org/WebAPI/MobileIdentity#Implementation_details . I just need the time to find the time do it :)

Updated

4 years ago
blocking-b2g: 2.0? → 2.0+
https://hg.mozilla.org/mozilla-central/rev/5af3884cea83
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/e09a13eef6ae
status-b2g-v2.0: --- → fixed
status-b2g-v2.1: --- → fixed
status-firefox32: --- → wontfix
status-firefox33: --- → wontfix
status-firefox34: --- → fixed
You need to log in before you can comment on or make changes to this bug.