887156 - Define which MSISDNs can send wap push messages to user

Status: VERIFIED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Ken Chang[:kenkai]]

As an operator I want to define which MSISDNs can send wap push messages to user so the user is not spammed by adverts.

Comment 1

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Two questions about the user story:
1. We have to support multiple MSISDNs, right?
2. Does the white list can be modified after shipping?
   This might be required if operator want to update the white list, or if we allow users to add white list of their own.

Comment 2

[Ken Chang[:kenkai]]

(In reply to Chuck Lee [:chucklee] from comment #1)
> Two questions about the user story:
> 1. We have to support multiple MSISDNs, right?
Need Bruce to answer.
> 2. Does the white list can be modified after shipping?
>    This might be required if operator want to update the white list, or if
> we allow users to add white list of their own.
No, this is a customization. User can not change it in shipping device.

Comment 3

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

(In reply to Ken Chang from comment #2)
> (In reply to Chuck Lee [:chucklee] from comment #1)
> > 2. Does the white list can be modified after shipping?
> >    This might be required if operator want to update the white list, or if
> > we allow users to add white list of their own.
> No, this is a customization. User can not change it in shipping device.

I think we have to make sure operators don't have the request to change it as well.
If they do, unless they are willing to modify gecko, we have to provide API for that, and we can't prevent user from writing app to do the same thing.

Comment 5

[Bruce Huang [:bhuang] (inactive)]

(In reply to Ken Chang from comment #2)
> (In reply to Chuck Lee [:chucklee] from comment #1)
> > Two questions about the user story:
> > 1. We have to support multiple MSISDNs, right?
> Need Bruce to answer.

Yes, we should allow a list of MSISDNs.

Comment 6

[Bruce Huang [:bhuang] (inactive)]

(In reply to Chuck Lee [:chucklee] from comment #3)
> (In reply to Ken Chang from comment #2)
> > (In reply to Chuck Lee [:chucklee] from comment #1)
> > > 2. Does the white list can be modified after shipping?
> > >    This might be required if operator want to update the white list, or if
> > > we allow users to add white list of their own.
> > No, this is a customization. User can not change it in shipping device.
> 
> I think we have to make sure operators don't have the request to change it
> as well.
> If they do, unless they are willing to modify gecko, we have to provide API
> for that, and we can't prevent user from writing app to do the same thing.

Is there a way to do it so OTA provisioning can modify the field as well?  Or can all customization be modified via user developed applications?

Comment 7

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

(In reply to bhuang from comment #6)
> Is there a way to do it so OTA provisioning can modify the field as well? 
> Or can all customization be modified via user developed applications?

There's no specific parameter in CP for this usage.
Modifying MSISDN filter can be done using settings, but I prefer we define a maximum number of MSISDN in white list.

Comment 8

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

I think of another method: put the filter in application instead of gecko.

Since operator need to provide an app for handling WAP Push(because we don't support app), and the method of getting new MSISDN filter list should be operator-specific. It's straightforward for the operator to integrate filter and filter update mechanism into provided WAP Push app.

Comment 9

[Ken Chang[:kenkai]]

(In reply to Chuck Lee [:chucklee] from comment #8)
> I think of another method: put the filter in application instead of gecko.
> 
> Since operator need to provide an app for handling WAP Push(because we don't
> support app), and the method of getting new MSISDN filter list should be
> operator-specific. It's straightforward for the operator to integrate filter
> and filter update mechanism into provided WAP Push app.

It makes sense to me.
Gabriele, what do you think?

Comment 10

[Gabriele Svelto [:gsvelto] (PTO)]

I'm not very familiar with the WAP Push code yet so bear with me if I'm getting this wrong.

To implement what's suggested in comment 8 we would need to adjust our API to expose the MSISDN for each push to content so a vendor-provided Gaia app will be able to filter the messages; is that correct? In this case we should restrict this API to certified apps otherwise another app might decide to receive all pushes and bypass the filtering. That being said if we go this way how would we handle the operator updating the whitelist? Should that be taken care of by the application too in a vendor-specific way?

If that's the case then there wouldn't be much else to do for us expect from exposing the right API. We might still want to make a reference application as an example for vendors to develop their own.

Comment 11

[Gabriele Svelto [:gsvelto] (PTO)]

BTW there's been a discussion on dev-platform recently about having a generic system for delivering updates to files such as certificate lists, page white-lists and such. Depending on the way we want to go here it might be relevant to this effort:

https://groups.google.com/d/msg/mozilla.dev.platform/p74X9iipfhg/MFeslNOteUAJ

Comment 12

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

(In reply to Gabriele Svelto [:gsvelto] from comment #10)
> I'm not very familiar with the WAP Push code yet so bear with me if I'm
> getting this wrong.
> 
> To implement what's suggested in comment 8 we would need to adjust our API
> to expose the MSISDN for each push to content so a vendor-provided Gaia app
> will be able to filter the messages; is that correct? In this case we should
> restrict this API to certified apps otherwise another app might decide to
> receive all pushes and bypass the filtering.

I think this can be done with a permission to wap push.
If an app receives all wap push and becomes annoying, it can be prevented from receiving wap push by removing its permission.

> That being said if we go this way how would we handle the operator updating
> the whitelist? Should that be taken care of by the application too in a 
> vendor-specific way?
> If that's the case then there wouldn't be much else to do for us expect from
> exposing the right API. We might still want to make a reference application
> as an example for vendors to develop their own.

I have no reference on how vendors update their white list, but I didn't find any standard for doing that.
So I assume this is vendor-specific function, vendors should implement it on their own.
In such case, I think we can only provide a reference on how to filter based on MSISDN, but no reference on the update protocol.

Comment 13

[Gabriele Svelto [:gsvelto] (PTO)]

(In reply to Chuck Lee [:chucklee] from comment #12)
> I think this can be done with a permission to wap push.
> If an app receives all wap push and becomes annoying, it can be prevented
> from receiving wap push by removing its permission.

OK, so you would like to have the API available also for non-certified apps? That means a user app might display all pushes including the ones not filtered by the app provided by the carrier.

> So I assume this is vendor-specific function, vendors should implement it on
> their own.
> In such case, I think we can only provide a reference on how to filter based
> on MSISDN, but no reference on the update protocol.

Agreed.

Comment 14

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

(In reply to Gabriele Svelto [:gsvelto] from comment #13)
> OK, so you would like to have the API available also for non-certified apps?

Yes, I think it's what these APIs(only a system message for WAP Push, actually) are designed for. Normal developers can build apps doing whatever they intent to, it's up to users to install that app or not.

> That means a user app might display all pushes including the ones not
> filtered by the app provided by the carrier.

There might be a user app showing all messages, but might also be a user app providing filter.
For those are interested in WAP Push message, or those who's phones doesn't have carrier provided app, user app is the only way of accessing WAP Push messages.

Comment 15

[Ken Chang[:kenkai]]

Gabriele, do you have any concern to implement this function in Gaia? If not, I will assign this bug to you.

Comment 16

[Gabriele Svelto [:gsvelto] (PTO)]

(In reply to Ken Chang from comment #15)
> Gabriele, do you have any concern to implement this function in Gaia? If
> not, I will assign this bug to you.

I haven't and in fact after discussing with :vingtetun I was already considering of developing an app to handle WAP Push messages in bug 891762 so it would be straightforward to add the filtering code to it.

The only problem is that the current API delivers all the messages irrespective of the MSISDN which sent them and there's no way in Gaia to discover from which MSISDN a messages was sent from. So we need at least a bit of functionality to expose the MSISDN to Gaia in order to be able to implement this. Do you know who was responsible for the platform WAP Push code? I can add the extra platform functionality myself if need be but it will take me a little bit more time obviously.

Comment 17

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

(In reply to Gabriele Svelto [:gsvelto] from comment #16)
> The only problem is that the current API delivers all the messages
> irrespective of the MSISDN which sent them and there's no way in Gaia to
> discover from which MSISDN a messages was sent from. So we need at least a
> bit of functionality to expose the MSISDN to Gaia in order to be able to
> implement this. 

I am doing it in bug 900331.
MSISDN will be carried as 'sender' in system message.

Comment 18

[Gabriele Svelto [:gsvelto] (PTO)]

(In reply to Chuck Lee [:chucklee] from comment #17)
> (In reply to Gabriele Svelto [:gsvelto] from comment #16)
> I am doing it in bug 900331.
> MSISDN will be carried as 'sender' in system message.

Great, thanks for the feedback and sorry for not spotting it earlier.

Comment 19

[Ken Chang[:kenkai]]

According to comment 18, we only need the Gaia's fix for this bug. Assign to Gabriele.

Comment 20

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: [PATCH] Add a mechanism to filter WAP Push messages based on a whitelist of source MSISDNs

This patch adds the whitelisting mechanism directly into the application as discussed in this bug. A vendor customizing it would only need to add the MSISDNs to whitelist to the apps/wappush/js/whitelist.json file as entries in a JSON array.

Comment 21

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 797892 [details] [diff] [review]
[PATCH] Add a mechanism to filter WAP Push messages based on a whitelist of source MSISDNs

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

The code looks good. However you would need to put /js/whitelist.json to ./apps/wappush/.gitignore and have it generated from the (in)famous ./build/applications-data.js.

Comment 22

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: [PATCH v2] Add a mechanism to filter WAP Push messages based on a whitelist of source MSISDNs

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #21)
> The code looks good. However you would need to put /js/whitelist.json to
> ./apps/wappush/.gitignore and have it generated from the (in)famous
> ./build/applications-data.js.

I've updated the patch with this change and rebased it on top of the changes in bug 887157. The rest of the code is identical and I've tested it on the emulator.

Comment 23

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: [PULL REQUEST] Add a mechanism to filter WAP Push messages based on a whitelist of source MSISDNs

Pointer to Github pull-request

Comment 24

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: [PATCH v3] Add a mechanism to filter WAP Push messages based on a whitelist of source MSISDNs

This is the same patch as before but rebased on the latest changes and with the logic to filter out messages refactored into a separate function.

Comment 25

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 802887 [details] [diff] [review]
[PATCH v3] Add a mechanism to filter WAP Push messages based on a whitelist of source MSISDNs

Create a new file at |./apps/wappush/.gitignore| and use that.
We should split app gitignores.

Comment 26

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: [PATCH v4] Add a mechanism to filter WAP Push messages based on a whitelist of source MSISDNs

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #25)
> Create a new file at |./apps/wappush/.gitignore| and use that.
> We should split app gitignores.

Thanks for the review! This is an updated patch with the .gitignore entry moved into apps/wappush/.gitignore; I'll merge it right away.

Comment 27

[Gabriele Svelto [:gsvelto] (PTO)]

Merged in master:

https://github.com/mozilla-b2g/gaia/commit/a7f11d14d1fa7988fa429fa87572e0072bf3c1a1

Comment 28

[Anthony Ricaud (:rik)]

Please fix this linter error.
----- FILE : /home/travis/build/mozilla-b2g/gaia/apps/wappush/js/wappush.js -----
Line 78, E:0001: Extra space before ":"

Comment 29

[Gabriele Svelto [:gsvelto] (PTO)]

(In reply to Anthony Ricaud (:rik) from comment #28)
> Please fix this linter error.
> ----- FILE : /home/travis/build/mozilla-b2g/gaia/apps/wappush/js/wappush.js
> -----
> Line 78, E:0001: Extra space before ":"

Awww... I remember having fixed it but I must have forgot to amend my commit. What's the less disruptive way of doing this? I've got more patches coming for this code so I can fold it in one of those instead of doing another patch here.

Comment 30

[Anthony Ricaud (:rik)]

The sooner the linter is fixed, the happier I'll be :) You can do an extra commit and put it in this bug.

Comment 31

[James Lal [:lightsofapollo]]

backed out:
https://github.com/mozilla-b2g/gaia/commit/67673ba6ca70e3e30f03e6f95b5115d415cf8463

Please check travis... linter errors are the easiest to check its obvious that the travis output was not looked at.

Comment 32

[Gabriele Svelto [:gsvelto] (PTO)]

(In reply to James Lal [:lightsofapollo] from comment #31)
> Please check travis... linter errors are the easiest to check its obvious
> that the travis output was not looked at.

Actually I ran the linter locally but forgot to update the PR :-/ Travis was failing all day long on intermittent errors so I must have not paid attention, sorry for that. Updated PR coming.

Comment 33

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: [PULL REQUEST] Add a mechanism to filter WAP Push messages based on a whitelist of source MSISDNs

Pointer to Github pull-request

Comment 34

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: [PATCH v5] Add a mechanism to filter WAP Push messages based on a whitelist of source MSISDNs

Fixed the extra whitespace, will be merging the updated PR soon.

Comment 35

[Gabriele Svelto [:gsvelto] (PTO)]

Merged to master again:

https://github.com/mozilla-b2g/gaia/commit/fe406c36bc28b2b33c9c08bcccf937137a643436

Comment 36

[Enpei]

Verify on following buid
Gaia: e58cfdac65fe644dc79fb3613fdaa9d9573537ac1378985810
Build: 20130912134644

Wap push from numbers in whitelist.json (ex:["+47xxxxxxxx"]) can be displayed by wap push message app.