mozilla-mobile / android-components

⚠️ This project moved to a new repository. It is now developed and maintained at: https://github.com/mozilla-mobile/firefox-android
https://github.com/mozilla-mobile/firefox-android
Mozilla Public License 2.0
2.02k stars 472 forks source link

Enable WebChannel messages for custom FxA servers #6225

Closed eoger closed 2 years ago

eoger commented 4 years ago

Right now setting up a different FxA server on Fenix will not work, because WebChannel messages can't get through. (JavaScript Error: "account_updates error message. No Such Channel")

This is caused by the fxawebchannel.js content script only getting injected on the prod server pages.

What we could do is either:

@grigoryk @csadilek @vladikoff thoughts?

┆Issue is synchronized with this Jira Task

rfk commented 4 years ago

In a different (possibly out-of-band?) conversation, I recall @csadilek suggesting to make a couple of different copies of the fxa-webchannel support module - one that is used by default and hard-codes the prod FxA domain, one that can be used when configured to talk to stage, and perhaps one that has slightly different perf characteristics that can be used with any arbitrary domain. This sounds like a lot of potential for code duplication but perhaps we could avoid that with careful factoring.

Another option is to disable webchannels for non-prod servers and fall back to the plain OAuth flow, at the cost of losing CWTS during signup. We probably shouldn't do this for stage (since it changes QA experience between stage and prod) but we could do it for self-hosters.

eoger commented 4 years ago

In a different (possibly out-of-band?) conversation, I recall @csadilek suggesting to make a couple of different copies of the fxa-webchannel support module - one that is used by default and hard-codes the prod FxA domain, one that can be used when configured to talk to stage, and perhaps one that has slightly different perf characteristics that can be used with any arbitrary domain. This sounds like a lot of potential for code duplication but perhaps we could avoid that with careful factoring.

FYIW I'd be fine with either making a version of the extension or even changing the behavior the current one to set itself up (with the connectNative call and other listeners) only after it receives a WebChannelMessageToChrome event. If I remember right FxA always starts off the communication as the chrome code cannot send a message out of the blue anyway.

rfk commented 4 years ago

If I remember right FxA always starts off the communication as the chrome code cannot send a message out of the blue anyway.

This is accurate.

eoger commented 4 years ago

even changing the behavior the current one

Just be extra clear, this would change the current extension to be injected for all domains, while the Kotlin code would have to grow the capability to decide whether or not it wants to listen from a particular domain. (e.g. by comparing msg.origin with origin(fxAccountManager.getContentUrl))

csadilek commented 4 years ago

In a different (possibly out-of-band?) conversation, I recall @csadilek suggesting to make a couple of different copies of the fxa-webchannel support module...

Other conversation was here: https://github.com/mozilla-mobile/android-components/issues/6012. I think we can combine and close one of these tickets.

Duplication can be minimal and would definitely address this concern: https://github.com/mozilla-mobile/android-components/issues/6012#issuecomment-589341798

But other solutions welcome, of course :).

eoger commented 4 years ago

Summary of the latest developments:

eoger commented 4 years ago

Renaming the ticket to acknowledge this ticket is about allowing arbitrary FxA servers to send WebChannel messages to Kotlin. #6012 is for Mozilla-owned non-prod servers.

rfk commented 4 years ago

Inject the script everywhere and move the origin checking in FxaWebChannelFeature.kt.

We've done part of this now - the origin-checking is now performed in FxaWebChannelFeature.kt, but we're only injecting the webextension on a handful of known domains rather than injecting it everywhere.

jackyzy823 commented 4 years ago

Hello there. I'm working on self hosting fxa stack and i try to use @eoger 's second method to make fenix work with self hosting fxa.

* However we may a found a better way: lgreco on Slack suggested we could use [contentScripts.register()](https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/contentScripts). @csadilek says we could load a background script on every webpage, that checks the content URL and sends a message back to Kotlin to ask whether or not it can use `contentScripts.register`.

However I cannot make this work.
Here's my changes.

diff --git a/components/feature/accounts/src/main/assets/extensions/fxawebchannel/background.js b/components/feature/accounts/src/main/assets/extensions/fxawebchannel/background.js
new file mode 100644
index 000000000..b3ce7148e
--- /dev/null
+++ b/components/feature/accounts/src/main/assets/extensions/fxawebchannel/background.js
@@ -0,0 +1,18 @@
+/*
+Establish communication with native application.
+*/
+let port = browser.runtime.connectNative("mozacWebchannelBackground");
+/*
+Handle messages from native application, dispatch them to FxA via an event.
+*/
+port.onMessage.addListener( event => {
+  if(event.type == "overrideFxAServer"){
+    // TODO: unregister
+    browser.contentScripts.register({
+      "matches": [ event.url+"/*" ],
+      "js": [{file: "fxawebchannel.js"}],
+      "runAt": "document_start"
+    })
+  }
+});
+
diff --git a/components/feature/accounts/src/main/assets/extensions/fxawebchannel/manifest.json b/components/feature/accounts/src/main/assets/extensions/fxawebchannel/manifest.json
index 007b9bc1c..264179844 100644
--- a/components/feature/accounts/src/main/assets/extensions/fxawebchannel/manifest.json
+++ b/components/feature/accounts/src/main/assets/extensions/fxawebchannel/manifest.json
@@ -15,6 +15,9 @@
       "run_at": "document_start"
     }
   ],
+  "background": {
+    "scripts": ["background.js"]
+  },
   "permissions": [
     "mozillaAddons",
     "geckoViewAddons",
diff --git a/components/feature/accounts/src/main/java/mozilla/components/feature/accounts/FxaWebChannelFeature.kt b/components/feature/accounts/src/main/java/mozilla/components/feature/accounts/FxaWebChannelFeature.kt
index 421f4fd1a..263d1bac6 100644
--- a/components/feature/accounts/src/main/java/mozilla/components/feature/accounts/FxaWebChannelFeature.kt
+++ b/components/feature/accounts/src/main/java/mozilla/components/feature/accounts/FxaWebChannelFeature.kt
@@ -73,7 +73,7 @@ class FxaWebChannelFeature(

     override fun start() {
         extensionController.install(runtime)
-
+   extensionController.registerBackgroundMessageHandler(WebChannelViewBackgroundMessageHandler(serverConfig),"mozacWebchannelBackground")
         scope = store.flowScoped { flow ->
             flow.mapNotNull { state -> state.findCustomTabOrSelectedTab(customTabSessionId) }
                 .ifChanged { it.engineState.engineSession }
@@ -161,6 +161,17 @@ class FxaWebChannelFeature(
         extensionController.registerContentMessageHandler(engineSession, messageHandler)
     }

+    private class WebChannelViewBackgroundMessageHandler(
+        private val serverConfig: ServerConfig
+    ) : MessageHandler {
+        @SuppressWarnings("ComplexMethod")
+        override fun onPortConnected(port: Port) {
+            logger.debug("in  WebChannelViewBackgroundMessageHandler onPortConnected begin")
+            port.postMessage(JSONObject().put("type", "overrideFxAServer").put("url",serverConfig.contentUrl))
+            logger.debug("in  WebChannelViewBackgroundMessageHandler onPortConnected done")
+        }
+    }
+
     @VisibleForTesting
     companion object {
         private val logger = Logger("mozac-fxawebchannel")

Any help would be appericated. Thanks.

immanuelfodor commented 3 years ago

Using the Firefox Beta app, the custom account and sync server URLs can be displayed but even when correct self-hosted FXA/sync URLs are added (using @jackyzy823 's https://github.com/jackyzy823/fxa-selfhosting project), the login page just wouldn't load.

Self-hosting FXA/sync is a good thing between desktop browsers but it would be great if it could work between desktop and mobile apps. I even started self-hosting it to be able to transfer links between desktop and mobile and continue reading on either devices. Why on Earth would the Beta app display the custom URL fields if it's useless because some other settings are hard-coded? :grinning: The "old" Firefox app was working fine with the self-hosted FXA/sync stack, please allow Fenix to work with it, too.

These are the settings we can set in Firefox Desktop about:config to make the self-hosted FXA/sync work:

These are the settings that we need in Firefox Android (Beta) about:config to make the self-hosted FXA/sync work:

https://www.fxa.local and https://token.fxa.local can be added to Firefox Android (Beta) with the debug settings enabled, but the webchannel whitelist is still missing. Please allow setting it in the Beta, too.

defkev commented 3 years ago

@immanuelfodor's observation still applies for 87.0.0-rc.1

Orhideous commented 3 years ago

Are there any chance to get this merged?

grigoryk commented 3 years ago

https://github.com/mozilla-mobile/android-components/pull/9827 merged (apologies to all for the delay there), so in theory, the next Fenix Nightly should work well with a self-hosted syncserver + self-hosted account combo.

samr7 commented 3 years ago

9827 merged (apologies to all for the delay there), so in theory, the next Fenix Nightly should work well with a self-hosted syncserver + self-hosted account combo.

So, Firefox Nightly is supposed to allow me to log in to a self-hosted FxA server now? I will report that it most certainly does not and gives me the spinner of death, using 96.0a1 build 2015843595 / AC 96.0.20211102153458, a31fe20eea

pennae commented 2 years ago

still a problem in 102: setting a custom auth server in fenix does not work properly, the UI presented by the auth server cannot access the webchannel

pennae commented 2 years ago

it looks like https://github.com/mozilla-mobile/android-components/blob/main/components/feature/accounts/src/main/assets/extensions/fxawebchannel/background.js#L15 is never triggered for the account server url (as set in the secet config menu).

opening the account server page and sending a fxaccounts:fxa_status message results in a No Such Channel error. if we run the content script registration fragment from the webchannel extension inspector (substituting our account server uri for event.url) we can successfully query the accounts webchannel after reloading the UI page that accesses it. in that case we receive two replies to the same message though, one No Such Channel error and the expected response containing fxa login state.

pennae commented 2 years ago

we've installed a fresh copy of firefox from the fenix github repository (103.2.0) to make sure we have an absolutely clean slate and the same happens, although running the full fragment does not seem to be required. just executing 1 in the debug console of the webchannel extension is sufficient to receive two replies to a status query message, again one an error and one the expected reply. a reboot does not change this behavior. sometimes just reloading the page seems to do it as well, and we're now at the point where we cannot reproduce not also getting the expected reply any more (even after clearing all firefox data and reinstalling the app or using a new user account on the phone!)

the one commonality we see among all those is that the first reply to a status query is always an error, if the actual reply is sent it is sent second.

edit: after much more investigation i think we figured out what happened.

so this is not so much a bug that this feature isn't working, it's more that fenix always sends a channel error first, before the actual reply.

csadilek commented 2 years ago

Thanks @pennae for the summary, and glad to read you figured this out! The "No such channel" errors come from outdated functionality. I will close this ticket as completed.

We have https://bugzilla.mozilla.org/show_bug.cgi?id=1616635 (which I will re-open).