816564 - [User Story] Silent SMS to Authenticate Device for Mobile Billing

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, enhancement)

Description

[Kumar McMillan [:kumar]]

Write experimental JavaScript code within navigator.mozPay() to prove that we can or cannot use a silent SMS message to verify the identity of a phone for carrier billing. 

Concepts to prove:

- JavaScript running in the context of a navpay payment provider [1] can work with the WebSMS API [2] to send an SMS to an operator provided short code (any phone number could be used as stand-in)
- JavaScript running in the payment provider can receive an SMS reply that was returned from the operator in response to the short code. This might involve listening to events and/or scraping the SMS inbox.
- The user on the device should not see the operator SMS in her SMS application inbox

Ultimately, this feature intends to support the Marketplace payment provider (bug 794651) as a way to enable carrier billing as an alternate method of device identification. If the silent SMS can be sent and received then this would enable device identification for Bango/Telefonica. 

[1] https://wiki.mozilla.org/WebAPI/WebPaymentProvider
[2] https://wiki.mozilla.org/WebAPI/WebSMS

Comment 1

[David Lozano]

Let me propose you (tomorrow, today is already too late) a flow in which only the SMS sent from the device is necessary. We are currently working on that as an alternative to the traditional http header injection mechanism.

Comment 2

[Jason Smith [:jsmith]]

Disclaimer: The WebSMS Web API is only accessible to certified apps. So hosted content won't be able to make calls onto that API. Only certified apps can.

Comment 3

[Kumar McMillan [:kumar]]

(In reply to Jason Smith [:jsmith] from comment #2)
> Disclaimer: The WebSMS Web API is only accessible to certified apps. So
> hosted content won't be able to make calls onto that API. Only certified
> apps can.

The WebSMS capability would not be directly accessible to the hosted content. The content could get a private callback like doSilentSMS() similar to paymentSuccess() or paymentFailed(). The innards of those functions are currently executing under the system app. This is just an example. There are several ways it could be implemented.

Comment 4

[David Lozano]

At https://dl.dropbox.com/u/64338170/SMS%20MO-based%20MobileID%20%281%29.png I have uploaded our proposal for authenticating users with a single SMS MO sent from the mobile device to a short code.

As we use short codes (with "private" routing within the operator network) the procedure is secure to authenticate the origin phone number without having to use a SMS MT as response.

MobileID is the strategy we are going to use as a wrapper on top of net-based auth (header injection). Bango will receive MobileIDs which are temporal alias that allow to charge the user. By using the proposed solution for SMS, Bango will keep receiving MobileIDs and we stick to a single strategy in this regard.

Comment 5

[Jason Smith [:jsmith]]

Reading this bug, it sends off the message that an equivalent client-side bug is needed. Is that correct? If so, can someone please file this in dom: device interfaces and link to this bug.

Comment 6

[Kumar McMillan [:kumar]]

(In reply to David Lozano from comment #4)

Thanks David, this is really helpful. I linked to the flow here https://wiki.mozilla.org/Apps/ID_and_Payments#Sequence_Flow

Comment 7

[Jason Smith [:jsmith]]

Adding Jonas for input. Including Dan for the security perspective.

Talked quickly about this in irc + b2g platform meeting - apparently I'm not the only person who thinks something smells fishy here.

Jonas - Can you shed some light on this bug from the WebAPI Security Perspective with silent SMS?

Comment 8

[Daniel Coloma:dcoloma]

Can we stop any work that affects the device unless this is triaged as a blocker? I am afraid this is way too late and we all bigger problems to solve now.

Comment 9

[Jason Smith [:jsmith]]

(In reply to Daniel Coloma:dcoloma from comment #8)
> Can we stop any work that affects the device unless this is triaged as a
> blocker? I am afraid this is way too late and we all bigger problems to
> solve now.

+1 on that 300 times.

Comment 10

[Lucas Adamski [:ladamski]]

I do feel like the silent SMS scenario is something we could live without for basecamp.  Its not the ideal experience, but it doesn't actually block making a purchase.

Comment 11

[Kumar McMillan [:kumar]]

More info:

ferjm_ kumar, regarding the silent sms thing and your question about the sms permissions on the system app, take into account that even if the system app has permissions for sending sms (which is not the case, but easy to fix) the one requesting the sms would be the content of the trusted UI which has a different nsIPrincipal than the system app.
ferjm_ I mean, the trusted UI content has not the same permissions than the system app
kumar ferjm_: aha, ok
ferjm_ also, take into account that it is not only sending an SMS, but also hiding it from the user in the sms db
kumar ferjm_: it sounds like we may not need to retrieve an SMS, per Ernesto's explanation of the MobileID service
kumar we only need to send it
kumar ferjm_: so what possibilities do you know of to grant WebSMS permissions to the trusty ui?
ferjm_ yes, but sent SMSs are also stored in the database
ferjm_ kumar, I think that we would need to inject a new chrome function in the payment flow to allow sending SMSs from it
ferjm_ it would also probably require some work on the SMS API to allow sending SMSs without storing them in the database

Comment 12

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Since I was asked for input:

It's unclear to me what the security model is here. It's certainly technically possible to expose a doSilentSMS() function to the payment provider and have the backend of that function use WebSMS to send a text message.

However that doesn't really change anything from a security point of view. If we expose the ability to send text messages to the payment provider we have to trust that it won't abuse that since that can cost the user a lot of money.

Granted, payment providers should be used to handling end-users' money, so maybe we can trust them with the ability to send SMS messages.


But yes. I think it's much too late to do any of this for v1 since anything SMS related will require client-side changes.

Comment 13

[David Bialer [:dbialer]]

Added Requirements Doc https://docs.google.com/a/mozilla.com/document/d/1o1ABp0Q53yiLWmgeZCQVqjCwz9yuAHoxxXd_22fO2Jo/edit

Comment 14

[Jason Smith [:jsmith]]

I take it we're tracking this as a client-side fix then, right? Should I move this into the core then?

Comment 15

[Kumar McMillan [:kumar]]

Well, the bug originally was meant more as an exploration but I think we've discussed what to do so we can use it to track the client side changes, yes. We can file separately too if that's any easier.

Client changes are:
- expose javascript API to Payment Provider to send silent SMS to short code
- maybe present some user dialogs asking for permission
- prevent SMS visibility in sent/inbox

Comment 16

[Fernando Jiménez Moreno [:ferjm]]

This was listed as part of the user stories desired for 1.2

https://docs.google.com/spreadsheet/ccc?key=0AtVT90hlMtdSdEd4TVVjWXNfU3ctMlVhWFRrWkpweVE#gid=23

so I'm nominating for koi+

Comment 17

[Fernando Jiménez Moreno [:ferjm]]

Taking this as it has been planned for Comms group sprint 1 (July 8 ~ 26).

Comment 18

[Joe Cheng [:jcheng] (please needinfo)]

listed as must have for COMM team for v1.2. koi+

Comment 19

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Part 1: SilentSmsManager (WIP)

Comment 20

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Part 2: RIL (WIP)

Comment 21

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Part 3: mozPaymentProvider (WIP)

Comment 22

[Fernando Jiménez Moreno [:ferjm]]

With the above patches I've successfully sent a silent message with the mock payment provider at [1]. These patches are just a rough WIP but shows the basic structure of the silent SMS code.

As you can see these patches expose a window.SilentSms property *only* to privileged code (chrome code) and it is used from the "sendSilentSms()" function that is injected in the payment provider flow.

The idea is to also allow consumers to subscribe/unsubscribe to notifications of incoming messages given an specific phone number. The core of this feature will be implemented within the SilentSmsService.jsm module. That will allow the payment provider to do the required MT + MO flow.

I'd would love to hear some feedback from Gene and Vicamo since these patches touch parts of the SMS and RIL codes :). Please, do not stop too much on the details of the code as these patches are just an early WIP. I am more interested on hearing your feedback about the overall structure of the silent SMS feature :). Thanks!

[1] http://ferjm.github.io/simplepayprovider/page1.html

Comment 23

[Gene Lian (I already quit Mozilla)]

Just having a discussion with Vicamo. He will help conclude our questions later.

Comment 24

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 775677 [details] [diff] [review]
Part 1: SilentSmsManager (WIP)

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

Hi Fernando,

I wonder,

1) The discussion takes place only in dev-b2g mailing list, not in dev-webapi.  I'm not sure whether other DOM peers than Jonas agree with such new API.

2) Is that silent procedure a must to implement this API?  In the mails you mentioned that's a product call, but whose call actually?  Why must we take the disadvantage keeping an user in blind for, what's the purpose of that silence actually?

3) We don't want every app has the ability to send silent SMS, but the way injecting a new function into mozPaymentProvider doesn't really prevent this.  All apps has "silentsms" permission is allowed to send messages silently.  So I don't really find the necessity to create a new API rather than re-use the original one.  Bug 857005 is going to rename mozMobileMessage::send to mozMobileMessage::sendSMS and make it accept a SmsParameters dictionary object as mozMobileMessage::sendMMS does.  So how about having an additional boolean attribute "silent" in SmsParameters instead?  We can enforce the same permission checking on it for sure.  This way, all the IPC codes, DOM entries can be re-used, we just have to handle behaviour differences brought in by "silent" param.

Comment 25

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 775678 [details] [diff] [review]
Part 2: RIL (WIP)

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

We'd really want to get rid of frame message manager in RIL code.  Re-use original SMS IPC code, maybe add a few new methods, can be as simple as all you want to do here.

Comment 26

[Fernando Jiménez Moreno [:ferjm]]

Thanks for the prompt feedback! :)

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #24)
> Comment on attachment 775677 [details] [diff] [review]
> Part 1: SilentSmsManager (WIP)
> 
> Review of attachment 775677 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Hi Fernando,
> 
> I wonder,
> 
> 1) The discussion takes place only in dev-b2g mailing list, not in
> dev-webapi.  I'm not sure whether other DOM peers than Jonas agree with such
> new API.
> 

Note that there is no public API expose to the DOM in general (at least not in the common way).

'window.SilentSms' is *only* exposed to chrome code. (Note the "JavaScript-global-privileged-property" in SilentSmsManager.manifest).

And "mozPaymentProvider.sendSilentSms()" is exposed (via frame script injection [1]) *only* to the payment provider.

I'd like you to understand that all this functionality is solely and exclusively exposed to the payment flow. There is no way that other web content can access to this functionality. Not even with a special permission. Maybe we want to expose it in the future (for example, for the Cost Control app), but that is not the current situation :).

So I don't think these changes could be of interest for any other DOM peer. In any case, I will forward the discussion to dev-webapi too :).

> 2) Is that silent procedure a must to implement this API?  In the mails you
> mentioned that's a product call, but whose call actually?  Why must we take
> the disadvantage keeping an user in blind for, what's the purpose of that
> silence actually?
> 

Yes, it is a must and the product requirements were exposed in the doc linked in comment 13. I am also asking needinfo to Ernesto Jiménez as he is the product owner of the payment world at Telefónica.

> 3) We don't want every app has the ability to send silent SMS, but the way
> injecting a new function into mozPaymentProvider doesn't really prevent
> this.

How is so?

As I said before, this functionality is injected *only* in the payment flow :) The same way that other payment functionality is. If you think that there is a way for other apps to access to this functionality, then we are in troubles (bigger than this bug).

> All apps has "silentsms" permission is allowed to send messages
> silently.  So I don't really find the necessity to create a new API rather
> than re-use the original one.

Sorry, I don't understand this statement. What's "silentsms" permission? Note that I am not adding any permission in these patches. I am explicitly granting this functionality ONLY to the payment provider.

> Bug 857005 is going to rename
> mozMobileMessage::send to mozMobileMessage::sendSMS and make it accept a
> SmsParameters dictionary object as mozMobileMessage::sendMMS does.  So how
> about having an additional boolean attribute "silent" in SmsParameters
> instead?  We can enforce the same permission checking on it for sure.  This
> way, all the IPC codes, DOM entries can be re-used, we just have to handle
> behaviour differences brought in by "silent" param.

That sounds reasonable to me, but it feels like a bigger change and a change in the public API that would probably require further discussion with the WebAPI team. It might also require and additional "silentsms" permission as you mention before (as I said, I am not introducing it with the current patches as we explicitly allow this functionality to the payment provider on demand).

I would like to avoid any change in the current API, specially because I don't think that it is actually required for our current needs.

The payment use case is a pretty special one, and the way we expose functionality to the payment provider is not the common one [2]. The payment provider is a regular web content loaded in a chrome frame that has no app privileges (at least the way we understand privileges) so it will never get access to a "sms" or a potential "silentsms" permission. We will still need to manually inject this functionality in the payment flow, even if it is exposed as part of the WebSMS API. We had a similar situation in bug 872987 where we needed to inject in the payment provider an already exposed functionality (via mozMobileConnection). I don't think we want to do the same here, as exposing the silent SMS functionality with the current WebSMS API is not a current requirement. Exposing it to the payment provider is a requirement.

I am also adding Jonas as needinfo as I would like to hear his opinion about this last suggestion.

Thanks again for your feedback!

[1] https://developer.mozilla.org/en-US/docs/The_message_manager#Content_scripts
[2] https://wiki.mozilla.org/WebAPI/WebPayment#Current_implementation

Comment 27

[Fernando Jiménez Moreno [:ferjm]]

(In reply to Fernando Jiménez Moreno [:ferjm] (needinfo instead of CC, please) from comment #26)
> I would like to avoid any change in the current API, specially because I
> don't think that it is actually required for our current needs.

When I say the current API I mean the current WebSMS API.

Comment 28

[Fernando Jiménez Moreno [:ferjm]]

Adding also Mounir to the loop, as he was the original author of the WebSMS API.

Comment 29

[Ernesto Jimenez]

As Fernando comments, dbialer's writeup linked in comment 13 is a good write up about the requirements.

When this kind of authentication flow is implemented in other platforms it is usually done silently to streamline the process and improve the UX. The exception usually is when the platform is not certain the silent SMS authentication will be free of charge to the customer in which case they prompt for confirmation.

That is aligned with dbialer's comments.

Comment 30

[Fernando Jiménez Moreno [:ferjm]]

I've just had a quick IRC chat with Jonas and Mounir about this:

[17:32]  <ferjm> mounir, hi! may I have your opinion about https://bugzilla.mozilla.org/show_bug.cgi?id=816564#c26 please?
[17:32]  <ferjm> also sicking ^  :)
[17:37]  * davehunt is now known as davehunt|away
[17:41]  <sicking> ferjm: which part specifically?
[17:42]  <sicking> ferjm: we discussed the general idea of a silent SMS API being exposed to payment providers somewhere. I definitely support that idea
[17:42]  <sicking> ferjm: not sure that sticking it on the window object is the best place though, it's so full of cruft as it is
[17:42]  <sicking> ferjm: even when just exposed to chrome
[17:43]  <ferjm> sicking, the proposal about changing the current WebSMS API to include a "silent" flag in SmsParameters done in the last part of https://bugzilla.mozilla.org/show_bug.cgi?id=816564#c26 I think there is no need to do that, as we can expose this functionality *only* for the payment provider.
[17:44]  <sicking> ferjm: i don't feel strongly about that. But yeah, i don't see a need. And i doubt that the standard API will include such a flag
[17:46]  <ferjm> sicking, the reason to expose it via the window object is to be able to implement nsIDOMGlobalPropertyInitializer to get a window instance to initialize the DOMRequestHelper. I can avoid doing that adding a window parameter to the SilentSmsManager.send() function, but that doesn't feel like the best option for consumers (even if we only have one consumer so far)
[17:46]  <sicking> ferjm: this is just exposed to chrome code?
[17:46]  <ferjm> yes
[17:47]  <sicking> ferjm: then can't you just create an XPCOM component, and give that component an init method which takes a window object?
[17:48]  <ferjm> sicking, yes, that's another option, but still feels a worst option for consumers as they'll need to call .init before .send. But I am ok with that if you think that's better that implementing nsIDOMGlobalPropertyInitializer
[17:48]  <sicking> ferjm: the only consumer is the payment API implementation, right?
[17:49]  <sicking> at least for now
[17:49]  <sicking> ?
[17:49]  <ferjm> sicking, yes. We might have also the identity API in the future
[17:49]  <sicking> ok
[17:49]  <sicking> yeah, i think that's a better approach then
[17:49]  <sicking> nsIDOMGlobalPropertyInitializer is really intended for things exposed to webpages
[17:50]  <ferjm> sicking, ok, cool. I'll change that then :) Would you mind adding a comment to the bug about the WebSms API modification thing, please?
[17:51]  <ferjm> sicking, well, nevermind, I'll just paste the IRC chat
[17:51]  <ferjm> sicking, thanks!
[17:51]  <sicking> thanks
[17:52]  * mounir assumes he is no longer needed
[17:52]  <ferjm> mounir, unless you have a different opinion :)
[17:53]  <ferjm> mounir, I though you'd be interested in this as you were the original author of the WebSms API
[17:53]  <mounir> ferjm: per law, I am not allowed to have a different opinion from sicking
[17:53]  <sicking> indeed
[17:53]  <ferjm> mounir, heh good to know
[17:53]  <mounir> ferjm: the WebSMS API is no longer my son
[17:54]  <mounir> since his puberty, he changed way too much, I don't regonize it anymore

Comment 31

[Fernando Jiménez Moreno [:ferjm]]

Vicamo, Gene, do the above comments make sense for you?

Comment 32

[Gene Lian (I already quit Mozilla)]

I agree with Jonas. Sounds good to me.

Comment 33

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Part 1: SilentSmsManager (WIP)

Comment 34

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Part 2: RIL (WIP)

Comment 35

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Part 3: mozPaymentProvider (WIP)

Comment 36

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Part 1: Silent Sms core (WIP)

Comment 37

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Part 2: RIL (WIP)

Comment 38

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Part 3: mozPaymentProvider (WIP)

Comment 39

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Part 1: Silent SMS core

Comment 40

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Part 2: RIL

Comment 41

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Part 3: mozPaymentProvider

Comment 42

[Fernando Jiménez Moreno [:ferjm]]

Try run:

https://tbpl.mozilla.org/?tree=Try&rev=518b38a9e994

Comment 43

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Part 3: mozPaymentProvider

Wrong patch, sorry for the noise.

Comment 44

[[:fabrice] Fabrice Desré]

Comment on attachment 779702 [details] [diff] [review]
Part 3: mozPaymentProvider

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

::: b2g/chrome/content/payment.js
@@ +40,5 @@
>      paymentSuccess: 'r',
>      paymentFailed: 'r',
> +    iccIds: 'r',
> +    sendSilentSms: 'r',
> +    observeSilentSms: 'r'

That needs to be #ifdef MOZ_RIL right?

@@ +80,5 @@
>      });
>  
> +    gBrowser.shell.sendChromeEvent(detail);
> +
> +    this.removeSilentSmsObservers();

That is also guarded by MOZ_RIL below.

Comment 45

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Part 3: mozPaymentProvider - v2

You are right!

Comment 46

[[:fabrice] Fabrice Desré]

Comment on attachment 779920 [details] [diff] [review]
Part 3: mozPaymentProvider - v2

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

::: b2g/chrome/content/payment.js
@@ +39,5 @@
>    __exposedProps__: {
>      paymentSuccess: 'r',
>      paymentFailed: 'r',
> +    iccIds: 'r',
> +#ifdef MOZ_B2G_RIL    

nit: trailing whitespace.

Comment 47

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 779700 [details] [diff] [review]
Part 2: RIL

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

Please add a boolean |silent| flag to |nsISmsService::send()| instead.  We have nsISmsService for SMS feature on each supported platform.  I don't want to dig another hole for it.  We're going to remove RILContentHelper as I've said in comment 25.

Comment 48

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 779699 [details] [diff] [review]
Part 1: Silent SMS core

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

Some carriers may ask for a certain provision app installed on CDMA phones.  This app sends a SMS message carrying necessary device identities such as MEID on first boot and then remove server streams provision data to your cell phone by OTASP.  The SMS message should neither be stored into database nor notify the user about its existence, delivery status, etc.  Sounds like a SILENT SMS you're going to add here, right?  That's why I thought we can expose this feature to DOM publicly.  Since Sicking had stated his opinions clearly, and that's not a mandatory feature in any time soon, I agree we can hide it for now.

But we have already nsISmsService and nsIMobileMessageDatabaseService.idl.  Every SMS request should be first served by either of them, not nsIRadioInterface.  Using cpmm/ppmm means we have to create similar message listeners to handle silent SMS requests for SMS-capable platforms.  Besides, in WebSMS we use IPDL for IPC; in this case we don't need IPC at all.  I thought you need cpmm/ppmm because you need IPC, but that has been proven false.  So is there any strong reason we must use cpmm/ppmm here?

::: dom/mobilemessage/src/gonk/SilentSmsManager.js
@@ +20,5 @@
> +                                 "SilentSmsService:NotifyObservers"];
> +
> +XPCOMUtils.defineLazyServiceGetter(this, "cpmm",
> +                                   "@mozilla.org/childprocessmessagemanager;1",
> +                                   "nsIMessageSender");

I really don't understand why you must need cpmm/ppmm if you're already in chrome process.  Why can't we just obtain a nsISmsService instance and call its |send()| function instead?  Besides, we will add more error checking code in the future, and all the error results will be passed through nsIMobileMessageCallback.  Please don't invent another channel for SMS stuff.

PS. nsIMobileMessageCallback is now tagged with |builtinclass|.  You might want to remove it and implement another one in javascript that fits your need.

Comment 49

[Vicamo Yang [:vicamo][:vyang]]

Something like this:
https://github.com/vicamo/b2g_releases-mozilla-central/tree/bugzilla/816564/silent-sms (tree)
https://github.com/vicamo/b2g_releases-mozilla-central/commit/71809d7 (commit)

Comment 50

[Fernando Jiménez Moreno [:ferjm]]

Awesome! Thanks Vicamo! I'll work based on your suggestions.

Comment 51

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Part 0: WebSMS v1

This is basically Vicamo's suggested patch with a few modifications to make it work

Comment 52

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Part 1: RIL v2

Comment 53

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Part 3: mozPaymentProvider v2

Comment 54

[Fernando Jiménez Moreno [:ferjm]]

I still need to check and fix WebSMS tests. I'll add them in another patch.

Comment 55

[Gene Lian (I already quit Mozilla)]

I think I should let Vicamo take these reviews. He knows more details than me regarding this bug. :)

Comment 56

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 781161 [details] [diff] [review]
Part 0: WebSMS v1

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

Have acknowledged Gene, just r=me :)

::: dom/mobilemessage/src/ipc/SmsParent.cpp
@@ +341,5 @@
>  }
>  
>  bool
> +SmsParent::RecvAddSilentNumber(const nsString& aNumber)
> +{

I missed a check here:

  if (mSilentNumbers.Contains(aNumber) {
    return true;
  }

Comment 57

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Fernando Jiménez Moreno [:ferjm] (PTO from 30/7 to 26/8) from comment #53)
> Created attachment 781164 [details] [diff] [review]
> Part 3: mozPaymentProvider v2

Maybe you want to use `hg mv` instead.  It generates a smaller patch and helps review a lot.

Comment 58

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 781162 [details] [diff] [review]
Part 1: RIL v2

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

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +1904,5 @@
>  
> +      Services.obs.notifyObservers(domMessage,
> +                                   kSilentSmsReceivedObserverTopic,
> +                                   null);
> +      return false;

Must return true to ACK this newly received SMS.

@@ +1997,5 @@
>        return;
>      }
>  
> +    if (options.silent) {
> +      //TODO: Set message delivery

Please fill the missing parts.  You'll also need some modifications to handleSmsSendFailed() and handleSmsDelivery().

@@ +3150,5 @@
>    },
>  
> +  sendSMS: function sendSMS(number, message, silent, request) {
> +
> +    function _notifyResult(rv, domMessage) {

We're going to have some jslint checks for RIL js files.  This make it complain "'options' not defined" at line 3192.  Please move this function definition right above |let sendingMessage = {|, and use:

  let notifyResult = function notifyResult(rv, domMessage) {

@@ +3152,5 @@
> +  sendSMS: function sendSMS(number, message, silent, request) {
> +
> +    function _notifyResult(rv, domMessage) {
> +      // TODO bug 832140 handle !Components.isSuccessCode(rv)
> +      Services.obs.notifyObservers(domMessage, kSmsSendingObserverTopic, null);

Should only notify observers if |silent| is false.

@@ +3188,5 @@
> +        return;
> +      }
> +
> +      // Keep current SMS message info for sent/delivered notifications
> +      options.envelopeId = this.createSmsEnvelope({

Here.

Comment 59

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 781164 [details] [diff] [review]
Part 3: mozPaymentProvider v2

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

::: b2g/chrome/content/paymentFlow.js
@@ +20,5 @@
> +Cu.import("resource://gre/modules/XPCOMUtils.jsm");
> +Cu.import("resource://gre/modules/Services.jsm");
> +
> +const kClosePaymentFlowEvent = "close-payment-flow-dialog";
> +const kSilentSmsReceivedTopic = "silent-sms-received";

#ifdef MOZ_B2G_RIL

@@ +198,5 @@
> +    }
> +
> +    let request = new SilentSmsRequest();
> +    smsService.send(aNumber, aMessage, true, request);
> +    return request;

Don't know whether returning an internal interface out a good idea, but I was planning something like:

  let domRequest = this.createRequest();
  let smsRequest = new SilentSmsRequest();
  smsRequest.domRequestId = this.getRequestId(domRequest);

  smsService.send(aNumber, aMessage, true, smsRequest);

  return domRequest;

Then you get a DOMRequest outside and can freely do whatever a DOMRequest allows you.

::: b2g/chrome/content/paymentFlow.manifest
@@ +1,2 @@
> +component {b484d8c9-6be4-4f94-ab60-c9c7ebcc853d} paymentFlow.js
> +contract @mozilla.org/mobilemessage/callback;1 {b484d8c9-6be4-4f94-ab60-c9c7ebcc853d}

I don't think you really need this.  We're not going to create a SilentSmsRequest instance outside paymentFlow.js.

Comment 60

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Part 2: mozPaymentProvider v3

Comment 61

[Fernando Jiménez Moreno [:ferjm]]

Thanks Vicamo!

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #59)
> Comment on attachment 781164 [details] [diff] [review]
> Part 3: mozPaymentProvider v2
> 
> @@ +198,5 @@
> > +    }
> > +
> > +    let request = new SilentSmsRequest();
> > +    smsService.send(aNumber, aMessage, true, request);
> > +    return request;
> 
> Don't know whether returning an internal interface out a good idea, but I
> was planning something like:

We are exposing only .onsuccess and .onerror callbacks.

> 
>   let domRequest = this.createRequest();
>   let smsRequest = new SilentSmsRequest();
>   smsRequest.domRequestId = this.getRequestId(domRequest);
> 
>   smsService.send(aNumber, aMessage, true, smsRequest);
> 
>   return domRequest;
> 
> Then you get a DOMRequest outside and can freely do whatever a DOMRequest
> allows you.
> 

I am afraid that's not possible, doing that it will raise a security error when trying to assign the DOMRequest .onsuccess/.onerror callbacks on the content side, that's why I needed to create and expose SilentSmsRequest callbacks.

Comment 62

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 781580 [details] [diff] [review]
Part 2: mozPaymentProvider v3

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

::: b2g/chrome/content/payment.js
@@ +8,5 @@
>  // navigator.pay API within the payment processor's scope.
>  
>  "use strict";
>  
> +let _DEBUG = true;

DEBUG flag should be default false, especially when you will print auth data at line 188.

@@ +184,5 @@
> +  _silentSmsObservers: null,
> +
> +  sendSilentSms: function sendSilentSms(aNumber, aMessage) {
> +    if (_DEBUG) {
> +      _debug("Sending silent message " + aNumber + " - " + aMessage);

Here.

Comment 63

[Fernando Jiménez Moreno [:ferjm]]

Attachment: Part 1: RIL v3

Thanks Vicamo! I think this patch addresses all your previous feedback.

Comment 64

[Kumar McMillan [:kumar]]

Would it be easy to add a setting to enabled debug messages? The native identity code lets us do:

toolkit.identity.debug = true

this is really helpful because we don't have to compile our own gecko for debugging.

Comment 65

[Fernando Jiménez Moreno [:ferjm]]

https://tbpl.mozilla.org/?tree=Try&rev=aa9e1dacebbf

Comment 66

[Fernando Jiménez Moreno [:ferjm]]

(In reply to Kumar McMillan [:kumar] from comment #64)
> Would it be easy to add a setting to enabled debug messages? The native
> identity code lets us do:
> 
> toolkit.identity.debug = true
> 
> this is really helpful because we don't have to compile our own gecko for
> debugging.

Bug 818317

Comment 67

[Fernando Jiménez Moreno [:ferjm]]

http://hg.mozilla.org/projects/birch/rev/933bd505fe4a
http://hg.mozilla.org/projects/birch/rev/bbbf11b2b3a8
http://hg.mozilla.org/projects/birch/rev/e660fb3f8481

Comment 68

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/933bd505fe4a
https://hg.mozilla.org/mozilla-central/rev/bbbf11b2b3a8
https://hg.mozilla.org/mozilla-central/rev/e660fb3f8481

Comment 69

[Fernando Jiménez Moreno [:ferjm]]

I wrote a few words for an email about what we did in this bug and I thought that it would also be useful here while we write the appropriate documentation:

*******

1 - We exposed three new methods in the mozPaymentProvider API:

*'sendSilentSms(number, message)', which allows the payment provider to send an MO SMS without storing it on the SMS database or requesting delivery status, which means that the SMS will not show any notifications and will not appear in any SMS application consuming the WebSMS API. This function returns a DOMRequest object so the caller can check that the SMS is successfully sent.

*'observeSilentSms(number, callback)', which allows to the payment provider to intercept any incoming MT SMS sent from the given number. The content of the intercepted SMS will be given to the payment provider through the callback provided as the second parameter in the form of a nsIDOMMozSmsMessage [2] . Same as before, this means that the SMS will be shown in SMS application or notification. The SMS will be silent.

*'removeSilentSmsObserver(number, callback)', which allows the payment provider to remove a previously observed number and its corresponding handler. This method will automatically be called with the completion of the payment flow (via 'paymentSuccess' or 'paymentFailed' calls or if the user manually closes the payment flow), so the payment provider doesn't need to do any manual clean up of the set up observers. This method is only exposed just in case the payment provider wants to remove a silent SMS observer before the completion of the whole payment flow (I'm guessing that we probably want to remove the observers as soon as we get the appropriate user information).

2 - A expected flow for an SMS MO + MT scenario with a fake 123 short code would be something like:
* As the payment provider will be "talking" with the short code application for 123, the first step should be start observing the messages coming from that number. This is done via 'mozPaymentProvider.observeSilentSms('123', onmessage);', where 'onmessage' is the callback that will handle the reception of an SMS coming from 123.

* Once it is ready to handle messages from 123, the payment provider requests to send an SMS via 'var req = mozPaymentProvider.sendSilentSms('123', uuid);'. Where 'uuid' is a variable containing the body of the SMS. I recommend to send a generated unique ID (uuid) as the message body, that will need to be sent back by the short code application, so the payment provider can match each send with its corresponding reply. The payment provider will need to keep track of these uuids and remove them as soon as a matching reply is received or after a timeout or send failure. The 'req' variable returned by 'sendSilentSms' is an instance of DOMRequest and allows the payment provider to check if the SMS is successfully sent or not.

* The server side steps (for the carrier + Bango scenario) are a bit obscure to me and can probably be done in several different ways, but I'm guessing something like: the message sent in the previous step is received by the short code application (on the carrier's side), which does what it needs to do to get an alias to authenticate the user (BlueVia MobileID or whatever) and shares this alias with Bango (and maybe also shares the uuid generated in the previous step that identifies the SMSs, so Bango can also match the authentication requests).
Once this is done the short code application sends back a message containing the uuid to the number of the sender of the previous message.

* The message sent by the short code application is intercepted by the payment provider and handled within the callback given to 'mozPaymentProvider.observeSilentSms' in the first step. If the uuid is correct, the payment provider should notify Bango about the successful completion of the flow. I'm guessing that via HTTP POST to a given URL.

* At this point, the payment provider should do its clean up for the uuid identifying the authentication flow. It can also call 'mozPaymentProvider.removeSilentSmsObserver', but as I mentioned above, it is not mandatory.

* Note that I am only guessing the server side steps for this flow. I am assuming that WebPay doesn't need to get the user authentication details (MobileID or whatever) and that is Bango the only one that needs it.
An SMS MO only flow is quite simpler and only requires a call to 'mozPaymentProvider.sendSilentSms'. The flow in this case is properly explained at [3].

3 - While we develop the server side for the SMS MO + MT flow in WebPay, we can easily improve the current SMS MT flow by intercepting and parsing the SMS sent by Bango. I guess you can imagine how this would work, but  a quick explanation of this flow would be something like:

* The payment provider needs to observe messages coming from Bango's short code (or number) via 'mozPaymentProvider.observeSilentSms'.

* We ask the user for her MSISDN, as we do in the current flow.

* We give this MSISDN to Bango, which sends an SMS to the device.

* This SMS MT is intercepted by the payment provider which parses the message to get the code sent by Bango.

* This code is sent to Bango to complete the user authentication.

* The user only needs to enter her MSISDN and wait for the authentication to be silently completed. We get rid of the step of forcing the user to read the message and write the received code back in the payment flow form.

Comment 70

[Fernando Jiménez Moreno [:ferjm]]

https://wiki.mozilla.org/WebAPI/WebPaymentProvider#Silent_SMS