1267760 - Send push public key and auth secret when registering/updating a device

Status: RESOLVED FIXED

(Firefox :: Sync, defect, P2)

Description

[Edouard Oger [:eoger]]

We need these keys to use push payloads on the server side.

Comment 1

[Edouard Oger [:eoger]]

Extended description: We want the Fxa server to be able to send different types of notifications to the desktop client (new device joined, account verified, etc) in the future.
Sending data along a push notification helps us differentiate these commands with a single endpoint.
In order to send data payloads, we need to provide a couple of crypto keys to the server to encrypt the message when we register or update a device.

Comment 2

[Edouard Oger [:eoger]]

Attachment: bug-1267760.patch

We shouldn't merge this until the FxA servers accept push keys again

Comment 3

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8745805 [details] [diff] [review]
bug-1267760.patch

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

Looks OK to me, but I suspect Kit better understands if this is the right thing to do.

::: services/fxaccounts/tests/xpcshell/test_accounts_device_registration.js
@@ +106,5 @@
>            resolve({
> +            endpoint: "http://mochi.test:8888",
> +            getKey: function(type) {
> +              if (type === "auth") {
> +                return "GSsIiaD2Mr83iPqwFNK4rw";

it seems like these should be constants?

Comment 4

[Edouard Oger [:eoger]]

Attachment: bug-1267760.patch

I refactored the tests

Comment 5

[Edouard Oger [:eoger]]

Note to myself: I'll have to change the reviewer name on the commit message.

Comment 6

[Lina Butler (ex-Mozilla)]

Comment on attachment 8745816 [details] [diff] [review]
bug-1267760.patch

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

Thanks, looks great!

::: services/fxaccounts/FxAccounts.jsm
@@ +1498,5 @@
>        if (subscription && subscription.endpoint) {
>          deviceOptions.pushCallback = subscription.endpoint;
> +        let publicKey = subscription.getKey('p256dh');
> +        let authKey = subscription.getKey('auth');
> +        let urlsafeBase64Encode = key => ChromeUtils.base64URLEncode(new Uint8Array(key), { pad: false });

Nit: This could be a top-level function, but leaving it here is OK, too.

::: services/fxaccounts/FxAccountsClient.jsm
@@ +367,5 @@
>     *         Extra device options
>     * @param  [options.pushCallback]
>     *         `pushCallback` push endpoint callback
> +   * @param  [options.pushPublicKey]
> +   *         `pushPublicKey` push public key

Nit: A comment that these are Base64 URL-encoded would be good.

Comment 7

[Edouard Oger [:eoger]]

Attachment: bug-1267760.patch

Thanks kit! Carrying r+ forward

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/678d2aa3a3a6

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/678d2aa3a3a6

Comment 10

[Edouard Oger [:eoger]]

I asked Kit to back this out since I contradicted myself (see comment 2)

Comment 11

[Pulsebot]

Backout:
https://hg.mozilla.org/mozilla-central/rev/6adc822f5e27

Comment 12

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8746238 [details] [diff] [review]
bug-1267760.patch

I'm fairly sure that already registered devices will not register with this new info.

Comment 13

[Edouard Oger [:eoger]]

Attachment: bug-1267760.patch

Rebased and bumped the DEVICE_REGISTRATION_VERSION constant we introduced in bug 1268761

Comment 14

[Edouard Oger [:eoger]]

Attachment: MozReview Request: Bug 1267760 - Send push public key and auth secret when registering/updating a device. r=kitcambridge

Review commit: https://reviewboard.mozilla.org/r/54962/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/54962/

Comment 15

[Edouard Oger [:eoger]]

Rebased in prevision of the new fxa-auth-server release this week (with keys support!). ni myself to remind me to land this.

Comment 16

[Lina Butler (ex-Mozilla)]

Comment on attachment 8756118 [details]
MozReview Request: Bug 1267760 - Send push public key and auth secret when registering/updating a device. r=kitcambridge

https://reviewboard.mozilla.org/r/54962/#review51632

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/7520b940afc8
Send push public key and auth secret when registering/updating a device. r=kitcambridge

Comment 18

[Carsten Book [:Tomcat]]

sorry had to back this out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=9681002&repo=fx-team

Comment 19

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/e3d409d10dea
Backed out changeset 7520b940afc8 for eslint failure

Comment 20

[Edouard Oger [:eoger]]

Comment on attachment 8756118 [details]
MozReview Request: Bug 1267760 - Send push public key and auth secret when registering/updating a device. r=kitcambridge

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/54962/diff/1-2/

Comment 21

[Edouard Oger [:eoger]]

My bad, here it is corrected

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/2df619ad3af0
Send push public key and auth secret when registering/updating a device. r=kitcambridge

Comment 23

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2df619ad3af0