Closed
Bug 1044792
Opened 10 years ago
Closed 10 years ago
Send MCC and MNC on POST /sms/mt/verify
Categories
(Cloud Services Graveyard :: MobileID, defect)
Tracking
(blocking-b2g:2.0+, firefox32 wontfix, firefox33 wontfix, firefox34 fixed, b2g-v2.0 fixed, b2g-v2.1 fixed)
People
(Reporter: ferjm, Assigned: ferjm)
References
Details
(Whiteboard: [qa?])
Attachments
(1 file)
13.90 KB,
patch
|
spenrose
:
review+
|
Details | Diff | Splinter Review |
The rationale is at https://bugzilla.mozilla.org/show_bug.cgi?id=1042866
Assignee | ||
Comment 1•10 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•10 years ago
|
||
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•10 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+
Updated•10 years ago
|
Whiteboard: [qa?]
Assignee | ||
Comment 4•10 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 :)
Assignee | ||
Comment 5•10 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/5af3884cea83
Updated•10 years ago
|
blocking-b2g: 2.0? → 2.0+
https://hg.mozilla.org/mozilla-central/rev/5af3884cea83
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Comment 7•10 years ago
|
||
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
Updated•5 years ago
|
Product: Cloud Services → Cloud Services Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•