1021595 - Set MobileID service https dev URL and force https

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Fernando Jiménez Moreno [:ferjm]]

Bug 988469 will land allowing http, we need to change that as soon as the server is https. Maybe we can keep it like that in eng builds.

Comment 1

[Frederik Braun [:freddy]]

What's blocking the server to move to HTTPS?
I want to make sure this goes in before we release.

Comment 2

[Fernando Jiménez Moreno [:ferjm]]

I believe bug 1020899 is the one blocking here

Comment 3

[Maria Angeles Oteo (:oteo)]

Nominating to 2.0, as the server is expected to used https when loop is released (2.0 FxOS version)

Comment 4

[Preeti Raghunath(:Preeti)]

blocking on 2.0.

Comment 5

[Jason Smith [:jsmith]]

Clearing blocking flag - this is technically feature work, not a bug. We need to set the feature-b2g flag here.

Comment 6

[Maria Angeles Oteo (:oteo)]

(In reply to Jason Smith [:jsmith] from comment #5)
> Clearing blocking flag - this is technically feature work, not a bug. We
> need to set the feature-b2g flag here.

Jason, one question, if I set feature-b2g:2.0, when Fernando has the patch, does he ask for approval? I want to understand if there is any problem or deadline with these feature-b2g:2.0 as FL finished 2 weeks ago
Thanks

Comment 7

[Jason Smith [:jsmith]]

(In reply to Maria Angeles Oteo (:oteo) from comment #6)
> (In reply to Jason Smith [:jsmith] from comment #5)
> > Clearing blocking flag - this is technically feature work, not a bug. We
> > need to set the feature-b2g flag here.
> 
> Jason, one question, if I set feature-b2g:2.0, when Fernando has the patch,
> does he ask for approval? I want to understand if there is any problem or
> deadline with these feature-b2g:2.0 as FL finished 2 weeks ago
> Thanks

Yeah, he'll need to ask for approval.

Comment 8

[Frederik Braun [:freddy]]

(In reply to Jason Smith [:jsmith] from comment #5)
> Clearing blocking flag - this is technically feature work, not a bug. We
> need to set the feature-b2g flag here.

Can we please make this block again?
The implication of this not landing with the webapi are truly dire:
All the security properties rely on a secure connection channel. We use it to send phone numbre and carrier information. (I am not a lawyer, but in some legislation this is PII)

Comment 9

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Jason - can you explain the difference between feature work and regular blocking ? I have not seen this before. Sending sensitive data unencrypted is likely to be a high security if that puts this in context.

Comment 10

[Jason Smith [:jsmith]]

(In reply to Paul Theriault [:pauljt] from comment #9)
> Jason - can you explain the difference between feature work and regular
> blocking ? I have not seen this before. Sending sensitive data unencrypted
> is likely to be a high security if that puts this in context.

Both flags indicate that this is required for 2.0. However, a blocking b2g flag allows the bug to automatically land on 2.0. A feature b2g flag requires approval for 2.0. Think of it as a gatekeeping approach to ensure that we don't crash land on 2.0.

Clearing the blocking flag since there's already a feature flag here.

Comment 11

[:shell escalante]

reset to blocking, per comment 8. 
Fernando- when do you expect this bug to land?

Comment 12

[Jason Smith [:jsmith]]

(In reply to sescalante from comment #11)
> reset to blocking, per comment 8. 
> Fernando- when do you expect this bug to land?

Please see the above comment that has clarified that this is feature work, not a bug. The feature-b2g flag is already set correctly here.

Comment 13

[Fernando Jiménez Moreno [:ferjm]]

(In reply to sescalante from comment #11)
> reset to blocking, per comment 8. 
> Fernando- when do you expect this bug to land?

As soon as the server is https, which I believe it will happen in bug 1020899

Comment 14

[Rémy Hubscher (:natim)]

Well the stage environment deployment defined in Bug 1020899 is not related to what we did for the dev environment.

We did deploy the dev environment behind SSL here: https://msisdn-dev.stage.mozaws.net

You can start pointing to https://msisdn-dev.stage.mozaws.net as soon as you are ready, ask me to switch the configuration from noSSL to useSSL to enable hawk.

Comment 15

[Fernando Jiménez Moreno [:ferjm]]

We also need to set the final production endpoint, so we can do it all in one patch as soon as the urls mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=1020899#c29 are working. This needs to land in 2.0 as we need to use the production server, so blocking 2.0?

Comment 16

[Tarek Ziadé (:tarek)]

Even if our production server is not deployed, We can have the client side point to https://msisdn.services.mozilla.com and route the calls to the current stage or dev servers for testings.

We can change the CNAME back to the real prod as soon as it is fully tested and ready

I will add a bug for this domain creation and routing

Comment 17

[Jason Smith [:jsmith]]

(In reply to Fernando Jiménez Moreno [:ferjm] from comment #15)
> We also need to set the final production endpoint, so we can do it all in
> one patch as soon as the urls mentioned in
> https://bugzilla.mozilla.org/show_bug.cgi?id=1020899#c29 are working. This
> needs to land in 2.0 as we need to use the production server, so blocking
> 2.0?

This is not a bug - it's required feature work to ship the feature in 2.0. The flags should not have been flipped here. I'm going to ask Bhavana to fix the feature-b2g flag here, since I don't the ability to set it.

Comment 18

[Fernando Jiménez Moreno [:ferjm]]

(In reply to Jason Smith [:jsmith] from comment #17)
> (In reply to Fernando Jiménez Moreno [:ferjm] from comment #15)
> > We also need to set the final production endpoint, so we can do it all in
> > one patch as soon as the urls mentioned in
> > https://bugzilla.mozilla.org/show_bug.cgi?id=1020899#c29 are working. This
> > needs to land in 2.0 as we need to use the production server, so blocking
> > 2.0?
> 
> This is not a bug - it's required feature work to ship the feature in 2.0.
> The flags should not have been flipped here. I'm going to ask Bhavana to fix
> the feature-b2g flag here, since I don't the ability to set it.

Sorry Jason, but IMHO this just makes us lose time by adding the extra step of requesting and waiting for the approval when we already know that this needs to land in 2.0 one way or the other. But whatever...

(In reply to Tarek Ziadé (:tarek) from comment #16)
> Even if our production server is not deployed, We can have the client side
> point to https://msisdn.services.mozilla.com and route the calls to the
> current stage or dev servers for testings.
> 
> We can change the CNAME back to the real prod as soon as it is fully tested
> and ready
> 
> I will add a bug for this domain creation and routing

Thanks Tarek. After talking with Rémy it seems that it is preferable to do this in two steps. First move to https with the dev server and after that add the final production url when it is ready. I'll file the bug for this last part and nominate it for 2.0 (feature 2.0).

Comment 19

[Jason Smith [:jsmith]]

(In reply to Fernando Jiménez Moreno [:ferjm] from comment #18)
> (In reply to Jason Smith [:jsmith] from comment #17)
> > (In reply to Fernando Jiménez Moreno [:ferjm] from comment #15)
> > > We also need to set the final production endpoint, so we can do it all in
> > > one patch as soon as the urls mentioned in
> > > https://bugzilla.mozilla.org/show_bug.cgi?id=1020899#c29 are working. This
> > > needs to land in 2.0 as we need to use the production server, so blocking
> > > 2.0?
> > 
> > This is not a bug - it's required feature work to ship the feature in 2.0.
> > The flags should not have been flipped here. I'm going to ask Bhavana to fix
> > the feature-b2g flag here, since I don't the ability to set it.
> 
> Sorry Jason, but IMHO this just makes us lose time by adding the extra step
> of requesting and waiting for the approval when we already know that this
> needs to land in 2.0 one way or the other. But whatever...

The whole point of the feature-b2g vs. blocking-b2g usage post FL is to actually ensure that we're placing approvals on late breaking feature requests so that we don't crash land on 2.0 while stability testing is ongoing. That's correct that it will land for 2.0 no matter what, it's just it needs to land cleanly on 2.1 first before getting uplifted. That's why a feature-b2g flag applies here, not a blocking flag.

Comment 20

[Fernando Jiménez Moreno [:ferjm]]

Attachment: v1

Comment 21

[Tarek Ziadé (:tarek)]

(In reply to Fernando Jiménez Moreno [:ferjm] from comment #18)
> Thanks Tarek. After talking with Rémy it seems that it is preferable to do
> this in two steps. First move to https with the dev server and after that
> add the final production url when it is ready. I'll file the bug for this
> last part and nominate it for 2.0 (feature 2.0).

That's fine as long as we are all in sync with the client pref final freeze.

Comment 22

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Comment on attachment 8449483 [details] [diff] [review]
v1

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

Looks good to me, Fernando.  I only have a suggestion about the default forceHttps behavior, but if anyone else agrees with me, that can be the topic of a follow-up.

::: b2g/app/b2g.js
@@ +991,5 @@
>  pref("identity.fxaccounts.enabled", true);
>  #endif
>  
> +// Mobile Identity API prefs.
> +pref("services.mobileid.server.uri", "https://msisdn-dev.stage.mozaws.net");

Per Comment 18, I see that you'll be able to make a final url change for production for 2.0, so that the stage url doesn't get shipped.

@@ +992,5 @@
>  #endif
>  
> +// Mobile Identity API prefs.
> +pref("services.mobileid.server.uri", "https://msisdn-dev.stage.mozaws.net");
> +pref("services.mobileid.forcehttps", true);

I feel like it would be better to have MobileIdentityClient.jsm default to forceHttps=true, in order to protect against misspelling or omitting the pref, and let people set this pref to false for local testing or whatnot.

Comment 23

[Fernando Jiménez Moreno [:ferjm]]

Thanks Jed!

https://hg.mozilla.org/integration/b2g-inbound/rev/1f24cbde93de

Comment 24

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8449483 [details] [diff] [review]
v1

Approval Request Comment
[Feature/regressing bug #]: Mobile ID
[User impact if declined]: Mobile ID won't work as it is using a deprecated endpoint.
[Describe test coverage new/current, TBPL]: Sadly no tests in TBPL yet. Unit tests will be added at bug 1022181 next week. We've tested it manually.
[Risks and why]: No risk. It is just pointing the requests to the right server and making sure that the server is https. 
[String/UUID change made/needed]: None

Comment 25

[Maire Reavy [:mreavy]]

FYI from Bhavana:  This will need tests (Bug 1022181) for this to be uplifted.

Comment 26

[Lawrence Mandel [:lmandel] (use needinfo)]

(In reply to Maire Reavy [:mreavy] (Plz needinfo me) from comment #25)
> FYI from Bhavana:  This will need tests (Bug 1022181) for this to be
> uplifted.

Fernando - In this case your patch flips the prefs to require SSL and use an SSL endpoint. If the tests that you'll be writing in bug 1022181 are more general for the feature and not specific to this change, I don't think there is a reason to wait after we see a green build on master. Do you intend to write tests specific to this pref change?

Comment 27

[Fernando Jiménez Moreno [:ferjm]]

(In reply to Lawrence Mandel [:lmandel] from comment #26)
> (In reply to Maire Reavy [:mreavy] (Plz needinfo me) from comment #25)
> > FYI from Bhavana:  This will need tests (Bug 1022181) for this to be
> > uplifted.
> 
> Fernando - In this case your patch flips the prefs to require SSL and use an
> SSL endpoint. If the tests that you'll be writing in bug 1022181 are more
> general for the feature and not specific to this change, I don't think there
> is a reason to wait after we see a green build on master. Do you intend to
> write tests specific to this pref change?

The tests at bug 1022181 that I was referring in comment 24 are to test the feature in a more generic way, not specific to this change. I don't think we should block this uplift on those tests. Sorry for not being more specific about this.

Comment 28

[bhavana bajaj [:bajaj]]

(In reply to Fernando Jiménez Moreno [:ferjm] from comment #27)
> (In reply to Lawrence Mandel [:lmandel] from comment #26)
> > (In reply to Maire Reavy [:mreavy] (Plz needinfo me) from comment #25)
> > > FYI from Bhavana:  This will need tests (Bug 1022181) for this to be
> > > uplifted.
> > 
> > Fernando - In this case your patch flips the prefs to require SSL and use an
> > SSL endpoint. If the tests that you'll be writing in bug 1022181 are more
> > general for the feature and not specific to this change, I don't think there
> > is a reason to wait after we see a green build on master. Do you intend to
> > write tests specific to this pref change?
> 
> The tests at bug 1022181 that I was referring in comment 24 are to test the
> feature in a more generic way, not specific to this change. I don't think we
> should block this uplift on those tests. Sorry for not being more specific
> about this.

Thanks for the clarification here, I'll approve the patch once the central landing happens within the next few hours.

Comment 29

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1f24cbde93de

Comment 30

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8449483 [details] [diff] [review]
v1

Aurora+

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ca9c1bf798be