1570496 - Forward compatibility with upcoming FxA changes for the 'action' parameter

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Firefox Accounts, task, P1)

Description

[:Grisha Kruglov]

Bug 1568605, in a bit of a haste, landed a solution for distinguishing a signin vs signup in a way that's not forward-compatible with upcoming FxA changes. In the (near) future, FxA may change the underlying assumptions - by offering a selection of sync data during a sign-in, not just sign-up - and will provide an explicit action parameter as part of the fxaccounts:login webchannel message.

Comment 1

[:Grisha Kruglov]

Attachment: Bug 1570496 - Forward-compatible treatment of 'action' WebChannel payload param r=nalexander

Comment 2

[:Grisha Kruglov]

Re-tested various signin, signup, change password (locally & remotely) and reconnect scenarios, it all seems to work as expected. Requesting landing.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bcdd64c29e02
Forward-compatible treatment of 'action' WebChannel payload param r=nalexander

Comment 4

[Bogdan Tara[:bogdan_tara | bogdant]]

Backed out changeset bcdd64c29e02 (Bug 1570496) for ES Lint failure on FxAccountsWebChannel.jsm

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&fromchange=bcdd64c29e02149acaad0587c06310186927b88d&tochange=76154667c2f1adbb405d2ff5a7dcb5453478b41b&selectedJob=259286849

Backout link: https://hg.mozilla.org/integration/autoland/rev/76154667c2f1adbb405d2ff5a7dcb5453478b41b

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=259286849&repo=autoland&lineNumber=436

[setup 2019-07-31T23:16:11.908Z] GECKO_PATH is /builds/worker/checkouts/gecko
[task 2019-07-31T23:16:11.908Z] executing ['bash', '-cx', 'cd /builds/worker/checkouts/gecko/ && cp -r /build/node_modules_eslint node_modules && ln -s ../tools/lint/eslint/eslint-plugin-mozilla node_modules && ln -s ../tools/lint/eslint/eslint-plugin-spidermonkey-js node_modules && ./mach lint -l eslint -f treeherder --quiet -f json:/builds/worker/mozlint.json\n']
[task 2019-07-31T23:16:11.910Z] + cd /builds/worker/checkouts/gecko/
[task 2019-07-31T23:16:11.910Z] + cp -r /build/node_modules_eslint node_modules
[task 2019-07-31T23:16:12.246Z] + ln -s ../tools/lint/eslint/eslint-plugin-mozilla node_modules
[task 2019-07-31T23:16:12.248Z] + ln -s ../tools/lint/eslint/eslint-plugin-spidermonkey-js node_modules
[task 2019-07-31T23:16:12.249Z] + ./mach lint -l eslint -f treeherder --quiet -f json:/builds/worker/mozlint.json
[task 2019-07-31T23:16:13.125Z] New python executable in /builds/worker/checkouts/gecko/obj-x86_64-pc-linux-gnu/_virtualenvs/init/bin/python2.7
[task 2019-07-31T23:16:13.125Z] Also creating executable in /builds/worker/checkouts/gecko/obj-x86_64-pc-linux-gnu/_virtualenvs/init/bin/python
[task 2019-07-31T23:16:16.421Z] Installing setuptools, pip, wheel...done.
[task 2019-07-31T23:16:17.662Z] running build_ext
[task 2019-07-31T23:16:17.662Z] building 'psutil._psutil_linux' extension
[task 2019-07-31T23:16:17.662Z] creating build
[task 2019-07-31T23:16:17.662Z] creating build/temp.linux-x86_64-2.7
[task 2019-07-31T23:16:17.662Z] creating build/temp.linux-x86_64-2.7/psutil
[task 2019-07-31T23:16:17.662Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_common.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_common.o
[task 2019-07-31T23:16:17.662Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_posix.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o
[task 2019-07-31T23:16:17.662Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_linux.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_linux.o
[task 2019-07-31T23:16:17.662Z] creating build/lib.linux-x86_64-2.7
[task 2019-07-31T23:16:17.662Z] creating build/lib.linux-x86_64-2.7/psutil
[task 2019-07-31T23:16:17.662Z] x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wl,-Bsymbolic-functions -Wl,-z,relro -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security build/temp.linux-x86_64-2.7/psutil/_psutil_common.o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o build/temp.linux-x86_64-2.7/psutil/_psutil_linux.o -o build/lib.linux-x86_64-2.7/psutil/_psutil_linux.so
[task 2019-07-31T23:16:17.662Z] building 'psutil._psutil_posix' extension
[task 2019-07-31T23:16:17.662Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_common.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_common.o
[task 2019-07-31T23:16:17.662Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_posix.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o
[task 2019-07-31T23:16:17.662Z] x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wl,-Bsymbolic-functions -Wl,-z,relro -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security build/temp.linux-x86_64-2.7/psutil/_psutil_common.o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o -o build/lib.linux-x86_64-2.7/psutil/_psutil_posix.so
[task 2019-07-31T23:16:17.662Z] copying build/lib.linux-x86_64-2.7/psutil/_psutil_linux.so -> psutil
[task 2019-07-31T23:16:17.662Z] copying build/lib.linux-x86_64-2.7/psutil/_psutil_posix.so -> psutil
[task 2019-07-31T23:16:17.662Z]
[task 2019-07-31T23:16:17.662Z] Error processing command. Ignoring because optional. (optional:packages.txt:comm/build/virtualenv_packages.txt)
[task 2019-07-31T23:31:54.217Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/mobile/android/modules/FxAccountsWebChannel.jsm:403:75 | Insert ?????????????????????? (prettier/prettier)
[taskcluster 2019-07-31 23:31:54.553Z] === Task Finished ===
[taskcluster 2019-07-31 23:31:55.335Z] Unsuccessful task run with exit code: 1 completed in 1543.573 seconds

Comment 5

[:Grisha Kruglov]

Updated patch so that ESLint passes locally. Let's try this again.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1795156aec4f
Forward-compatible treatment of 'action' WebChannel payload param r=nalexander

Comment 7

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/1795156aec4f

Comment 8

[Chris Peterson [:cpeterson]]

firefox-esr68=affected because we need to up this WebChannel fix to Fennec ESR 68.1 (as part of Leanplum bug 1568605).

Comment 9

[:Grisha Kruglov]

Comment on attachment 9082105 [details]
Bug 1570496 - Forward-compatible treatment of 'action' WebChannel payload param r=nalexander

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is needed for the Skyline project, allowing us to better track performance of the accounts onboarding UIs.
• User impact if declined: None for the user; mozilla will be unable to differentiate sign-ups from signins in the leanplum telemetry.
• Fix Landed on Version: 70
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Not risky; builds upon 1568605 (and needs to be uplifted along with it!). Patch takes into the account current state of the webchannel, as well as the upcoming changes that will happen in fxa-web-content repos. FxA side is aware of these changes, and will act accordingly in the future to accommodate them.
• String or UUID changes made by this patch: n/a

Comment 10

[Julien Cristau [:jcristau]]

Comment on attachment 9082105 [details]
Bug 1570496 - Forward-compatible treatment of 'action' WebChannel payload param r=nalexander

followup for bug 1568605, approved for fennec 68.1

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-esr68/rev/b870bfa864cd

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9082105 [details]
Bug 1570496 - Forward-compatible treatment of 'action' WebChannel payload param r=nalexander

We want this on Beta for parity as well.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/5db9809ede16