Another pass

components/concept/sync/src/main/java/mozilla/components/concept/sync/OAuthAccount.kt

@@ -55,6 +55,42 @@ interface StatePersistenceCallback {
      */
     fun persist(data: String)
 }
+//
+//sealed class AuthType
+///**
+// * Account restored from hydrated state on disk.
+// */
+//object Existing : AuthSource()
+///**
+// * Account created via a shared account state from another app.
+// */
+//object Shared : AuthSource()
+//
+///**
+// * Existing account was recovered from an authentication problem.
+// */
+//object Recovered : AuthSource()
+///**
+// * Account was created from an external source (e.g. web login).
+// * @property action A kind of event that triggered account creation: e.g. signin, signup.
+// */
+//data class External(val action: String) : AuthSource()
+
+/**
+ * Describes source of an authentication event.
+ */
+enum class AuthType {
+    // Account restored from hydrated state on disk.
+    Existing,
+    // Account created in response to a sign-in.
+    Signin,
+    // Account created in response to a sign-up.
+    Signup,
+    // Account created via a shared account state from another app.
+    Shared,
+    // Existing account was recovered from an authentication problem.
+    Recovered,
+}
 
 /**
  * Observer interface which lets its users monitor account state changes and major events.
@@ -70,7 +106,7 @@ interface AccountObserver {
      * @param account An authenticated instance of a [OAuthAccount].
      * @param newAccount True if an account was just signed in.
      */
-    fun onAuthenticated(account: OAuthAccount, newAccount: Boolean) = Unit
+    fun onAuthenticated(account: OAuthAccount, authType: AuthType) = Unit
 
     /**
      * Account's profile is now available.

components/feature/accounts/src/main/java/mozilla/components/feature/accounts/FirefoxAccountsAuthFeature.kt

@@ -12,6 +12,8 @@ import kotlinx.coroutines.launch
 import mozilla.components.concept.engine.EngineSession
 import mozilla.components.concept.engine.request.RequestInterceptor
 import mozilla.components.service.fxa.manager.FxaAccountManager
+import mozilla.components.service.fxa.manager.FxaAuthData
+import mozilla.components.service.fxa.manager.toFxaAuthAction
 import kotlin.coroutines.CoroutineContext
 
 /**
@@ -66,10 +68,15 @@ class FirefoxAccountsAuthFeature(
                 val code = parsedUri.getQueryParameter("code")
 
                 if (code != null) {
+                    val action = parsedUri.getQueryParameter("action")!!.toFxaAuthAction()
                     val state = parsedUri.getQueryParameter("state") as String
 
                     // Notify the state machine about our success.
-                    accountManager.finishAuthenticationAsync(code, state)
+                    accountManager.finishAuthenticationAsync(FxaAuthData(
+                        action = action,
+                        code = code,
+                        state = state
+                    ))
 
                     return RequestInterceptor.InterceptionResponse.Url(redirectUrl)
                 }

components/feature/accounts/src/main/java/mozilla/components/feature/accounts/WebChannelFeature.kt

@@ -15,7 +15,10 @@ import mozilla.components.concept.engine.EngineSession
 import mozilla.components.concept.engine.webextension.MessageHandler
 import mozilla.components.concept.engine.webextension.Port
 import mozilla.components.concept.engine.webextension.WebExtension
+import mozilla.components.feature.accounts.WebChannelFeature.Companion.supportedSyncEnginesToJSON
 import mozilla.components.service.fxa.manager.FxaAccountManager
+import mozilla.components.service.fxa.manager.FxaAuthData
+import mozilla.components.service.fxa.manager.toFxaAuthAction
 import mozilla.components.service.fxa.toNativeString
 import mozilla.components.support.base.feature.LifecycleAwareFeature
 import mozilla.components.support.base.log.logger.Logger
@@ -100,9 +103,9 @@ class WebChannelFeature(
 
                 val messageId = messageObj.optString("messageId", "")
                 when (command) {
-                    ON_SIGN_UP_COMMAND -> onSignUpCommand(messageId, engineSession)
-                    ON_START_UP_COMMAND -> onStartUpCommand(accountManager, messageId, engineSession)
-                    ON_OAUTH_LOGIN_COMMAND -> onOauthLoginCommand(accountManager, messageObj)
+                    CAN_LINK_ACCOUNT_COMMAND -> onCanLinkAccountCommand(messageId, engineSession)
+                    FXA_STATUS_COMMAND -> onFxaStatusCommand(accountManager, messageId, engineSession)
+                    OAUTH_LOGIN_COMMAND -> onOauthLoginCommand(accountManager, messageObj)
                 }
             }
         }
@@ -126,24 +129,23 @@ class WebChannelFeature(
         internal const val CHANNEL_ID = "account_updates"
 
         /**
-         * Gets triggered after the user has signed up.
-         * For more information:
-         * https://github.com/mozilla/fxa/blob/master/packages/fxa-content-server/docs/relier-communication-protocols/fx-webchannel.md#fxaccountscan_link_account
+         * Gets triggered when user initiates a login within FxA web content.
+         * On Fx Desktop, this event triggers "a different user was previously signed in on this machine" warning.
          */
-        internal const val ON_SIGN_UP_COMMAND = "fxaccounts:can_link_account"
+        internal const val CAN_LINK_ACCOUNT_COMMAND = "fxaccounts:can_link_account"
 
         /**
          * Gets triggered when a user successfully authenticates via OAuth.
          */
-        internal const val ON_OAUTH_LOGIN_COMMAND = "fxaccounts:oauth_login"
+        internal const val OAUTH_LOGIN_COMMAND = "fxaccounts:oauth_login"
 
         /**
          * Gets triggered when the communication between the app and the web-channel starts.
          * This gives the app an opportunity to indicate which capabilities it supports e.g History, Bookmarks ... etc.
          * For more information:
          * https://github.com/mozilla/fxa/blob/master/packages/fxa-content-server/docs/relier-communication-protocols/fx-webchannel.md#fxaccountsfxa_status
          */
-        internal const val ON_START_UP_COMMAND = "fxaccounts:fxa_status"
+        internal const val FXA_STATUS_COMMAND = "fxaccounts:fxa_status"
 
         @Volatile
         internal var installedWebExt: WebExtension? = null
@@ -186,63 +188,58 @@ class WebChannelFeature(
         }
 
         /**
-         * Handles the [ON_SIGN_UP_COMMAND] event from the web-channel.
+         * Handles the [CAN_LINK_ACCOUNT_COMMAND] event from the web-channel.
+         * Currently this always response with 'true'.
+         * On Fx Desktop, this event prompts a possible "another user was previously logged in on
+         * this device" warning. Currently we don't support propagating this warning to a consuming application.
          */
-        private fun onSignUpCommand(messageId: String, engineSession: EngineSession) {
-            // TODO don't allow linking if we're logged in already? This is requested after user
-            // entered their credentials.
-
-            val data = JSONObject()
-            // TODO should just return `accountManager.authenticatedAccount() != null`
-            // But, fxa-status message isn't currently working correctly - we can't reply
-            // with a sessionToken, because we don't have access to it.
-            // This means that if we return 'false' when we have an account in the browser,
-            // an fxa content page running in a "regular tab" (not part of browser auth flow)
-            // will not be able to login correctly.
-            // The expected behaviour for a logged-in browser is that it's never necessary
-            // to login manually into accounts.firefox.com (fxa-status message takes care of
-            // auto-login).
-            data.put("ok", true)
-
-            val status =
-                WebChannelHelper.createResponseObject(messageId, ON_SIGN_UP_COMMAND, data)
+        private fun onCanLinkAccountCommand(messageId: String, engineSession: EngineSession) {
+            // TODO profile this; it's in a hot-path during sign-in flow.
+            val data = JSONObject("""
+                {"ok": true}
+            """.trimIndent())
+
+            val status = WebChannelHelper.createResponseObject(messageId, CAN_LINK_ACCOUNT_COMMAND, data)
             sendContentMessage(status, engineSession)
         }
 
         /**
-         * Handles the [ON_START_UP_COMMAND] event from the web-channel.
+         * Handles the [FXA_STATUS_COMMAND] event from the web-channel.
          * Provides the initial information like which capabilities this app supports to the web-channel.
          */
-        private fun onStartUpCommand(
+        private fun onFxaStatusCommand(
             accountManager: FxaAccountManager,
             messageId: String,
             engineSession: EngineSession
         ) {
-
-            val data = JSONObject(
-                """{
-                "capabilities":{
+            // TODO profile this. We're in a hot path for displaying FxA Login UI. As is, this code is very inefficient.
+            val data = JSONObject("""
+                {"capabilities": {
                     "engines": ${accountManager.supportedSyncEnginesToJSON()}
-                }
-            }""".trimIndent()
-            )
+                }}
+            """.trimIndent())
 
             // Since accountManager currently can't provide us with a sessionToken, this is
             // hard-coded to null.
             // Fix this once https://github.com/mozilla/application-services/issues/1669 is resolved.
             data.put("signedInUser", null)
 
-            val status = WebChannelHelper.createResponseObject(messageId, ON_START_UP_COMMAND, data)
+            val status = WebChannelHelper.createResponseObject(messageId, FXA_STATUS_COMMAND, data)
             sendContentMessage(status, engineSession)
         }
 
-        //  TODO Define what "code" and "data" means as there is not are docs yet
         private fun onOauthLoginCommand(accountManager: FxaAccountManager, messageObj: JSONObject) {
             val data = messageObj.getJSONObject("data")
+
+            val action = data.getString("action").toFxaAuthAction()
             val code = data.getString("code")
             val state = data.getString("state")
 
-            accountManager.finishAuthenticationAsync(code, state)
+            accountManager.finishAuthenticationAsync(FxaAuthData(
+                action = action,
+                code = code,
+                state = state
+            ))
         }
 
         private fun sendContentMessage(msg: Any, engineSession: EngineSession) {
@@ -266,11 +263,11 @@ internal object WebChannelHelper {
         command: String,
         data: JSONObject
     ): JSONObject {
+        // TODO this is very inefficient.
         return JSONObject(
             """{
                 "id": "${WebChannelFeature.CHANNEL_ID}",
                 "message":{
-                    "messageId":"$messageId",
                     "messageId":"$messageId",
                     "command":"$command",
                     "data":$data

components/feature/accounts/src/test/java/mozilla/components/feature/accounts/WebChannelFeatureTest.kt

@@ -14,6 +14,8 @@ import mozilla.components.concept.engine.webextension.Port
 import mozilla.components.concept.engine.webextension.WebExtension
 import mozilla.components.service.fxa.SyncEngine
 import mozilla.components.service.fxa.manager.FxaAccountManager
+import mozilla.components.service.fxa.manager.FxaAuthAction
+import mozilla.components.service.fxa.manager.FxaAuthData
 import mozilla.components.service.fxa.toNativeString
 import mozilla.components.support.test.any
 import mozilla.components.support.test.argumentCaptor
@@ -185,7 +187,7 @@ class WebChannelFeatureTest {
         val jsonToWebChannel = JSONObject(
             """{
              "message":{
-                "command": "${WebChannelFeature.ON_START_UP_COMMAND}",
+                "command": "${WebChannelFeature.FXA_STATUS_COMMAND}",
                 "messageId":123
              }
             }""".trimIndent()
@@ -201,7 +203,7 @@ class WebChannelFeatureTest {
     }
 
     @Test
-    fun `ON_START_UP_COMMAND web-channel must get the capabilities`() {
+    fun `FXA_STATUS_COMMAND web-channel must get the capabilities`() {
         val sessionManager = mock<SessionManager>()
         val accountManager = mock<FxaAccountManager>()
         val session = mock<Session>()
@@ -231,7 +233,7 @@ class WebChannelFeatureTest {
         val jsonToWebChannel = JSONObject(
             """{
              "message":{
-                "command": "${WebChannelFeature.ON_START_UP_COMMAND}",
+                "command": "fxaccounts:fxa_status",
                 "messageId":123
              }
             }""".trimIndent()
@@ -245,7 +247,7 @@ class WebChannelFeatureTest {
     }
 
     @Test
-    fun `ON_SIGN_UP_COMMAND with no capabilities configured must provided an empty list of capabilities to the web-channel`() {
+    fun `CAN_LINK_ACCOUNT_COMMAND with no capabilities configured must provided an empty list of capabilities to the web-channel`() {
         val sessionManager = mock<SessionManager>()
         val accountManager = mock<FxaAccountManager>()
         val session = mock<Session>()
@@ -276,7 +278,7 @@ class WebChannelFeatureTest {
         val jsonToWebChannel = JSONObject(
             """{
              "message":{
-                "command": "${WebChannelFeature.ON_SIGN_UP_COMMAND}",
+                "command": "fxaccounts:can_link_account",
                 "messageId":123
              }
             }""".trimIndent()
@@ -289,7 +291,7 @@ class WebChannelFeatureTest {
     }
 
     @Test
-    fun `ON_OAUTH_LOGIN_COMMAND web-channel must be processed through the accountManager`() {
+    fun `OAUTH_LOGIN_COMMAND web-channel must be processed through the accountManager`() {
         val sessionManager = mock<SessionManager>()
         val accountManager = mock<FxaAccountManager>()
         val session = mock<Session>()
@@ -313,12 +315,14 @@ class WebChannelFeatureTest {
         )
         messageHandler.value.onPortConnected(port)
 
-        val jsonToWebChannel = JSONObject(
+        var jsonToWebChannel = JSONObject(
             """{
              "message":{
-                "command": "${WebChannelFeature.ON_OAUTH_LOGIN_COMMAND}",
+                "command": "fxaccounts:oauth_login",
                 "messageId":123,
                 "data":{
+                    "action":"signin",
+                    "redirect":"urn:ietf:wg:oauth:2.0:oob:oauth-redirect-webchannel",
                     "code":"b1b4887c1",
                     "state":"vHao1p6"
                 }
@@ -328,7 +332,36 @@ class WebChannelFeatureTest {
 
         messageHandler.value.onPortMessage(jsonToWebChannel, mock())
 
-        verify(accountManager).finishAuthenticationAsync(any(), any())
+        var expectedAuthData = FxaAuthData(
+            action = FxaAuthAction.Signin,
+            code = "b1b4887c1",
+            state = "vHao1p6"
+        )
+        verify(accountManager).finishAuthenticationAsync(expectedAuthData)
+
+        jsonToWebChannel = JSONObject(
+            """{
+             "message":{
+                "command": "fxaccounts:oauth_login",
+                "messageId":123,
+                "data":{
+                    "action":"signup",
+                    "redirect":"urn:ietf:wg:oauth:2.0:oob:oauth-redirect-webchannel",
+                    "code":"anotherCode1",
+                    "state":"anotherState2"
+                }
+             }
+            }""".trimIndent()
+        )
+
+        messageHandler.value.onPortMessage(jsonToWebChannel, mock())
+
+        expectedAuthData = FxaAuthData(
+            action = FxaAuthAction.Signup,
+            code = "anotherCode1",
+            state = "anotherState2"
+        )
+        verify(accountManager).finishAuthenticationAsync(expectedAuthData)
     }
 
     private fun JSONObject.getOk(): Boolean {

components/feature/sendtab/src/main/java/mozilla/components/feature/sendtab/SendTabFeature.kt

@@ -10,6 +10,7 @@ import androidx.lifecycle.LifecycleOwner
 import androidx.lifecycle.ProcessLifecycleOwner
 import mozilla.components.concept.push.Bus
 import mozilla.components.concept.push.PushService
+import mozilla.components.concept.sync.AuthType
 import mozilla.components.concept.sync.AccountObserver as SyncAccountObserver
 import mozilla.components.concept.sync.Device
 import mozilla.components.concept.sync.DeviceConstellation
@@ -116,10 +117,10 @@ internal class AccountObserver(
 ) : SyncAccountObserver {
     private val logger = Logger("AccountObserver")
 
-    override fun onAuthenticated(account: OAuthAccount, newAccount: Boolean) {
+    override fun onAuthenticated(account: OAuthAccount, authType: AuthType) {
         // We need a new subscription only when we have a new account.
         // This is removed when an account logs out.
-        if (newAccount) {
+        if (authType != AuthType.Existing) {
             logger.debug("Subscribing for ${PushType.Services} events.")
 
             feature?.subscribeForType(PushType.Services)

components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/FirefoxAccount.kt

@@ -4,6 +4,7 @@
 
 package mozilla.components.service.fxa
 
+import android.net.Uri
 import kotlinx.coroutines.async
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
@@ -110,7 +111,7 @@ class FirefoxAccount internal constructor(
         return scope.async {
             handleFxaExceptions(logger, "begin oauth flow", { null }) {
                 val url = inner.beginOAuthFlow(scopes.toTypedArray())
-                val state = URL(url).queryParam("state")!!
+                val state = Uri.parse(url).getQueryParameter("state")!!
                 AuthFlowUrl(state, url)
             }
         }
@@ -120,7 +121,7 @@ class FirefoxAccount internal constructor(
         return scope.async {
             handleFxaExceptions(logger, "begin oauth pairing flow", { null }) {
                 val url = inner.beginPairingFlow(pairingUrl, scopes.toTypedArray())
-                val state = URL(url).queryParam("state")!!
+                val state = Uri.parse(url).getQueryParameter("state")!!
                 AuthFlowUrl(state, url)
             }
         }