Closes #10924 - Cleanup SavedLoginsAuthFragment (#10930)

app/src/main/java/org/mozilla/fenix/settings/logins/SyncLoginsPreferenceView.kt

@@ -0,0 +1,105 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+package org.mozilla.fenix.settings.logins
+
+import androidx.lifecycle.LifecycleOwner
+import androidx.navigation.NavController
+import androidx.preference.Preference
+import mozilla.components.concept.sync.AccountObserver
+import mozilla.components.concept.sync.AuthType
+import mozilla.components.concept.sync.OAuthAccount
+import mozilla.components.service.fxa.SyncEngine
+import mozilla.components.service.fxa.manager.FxaAccountManager
+import mozilla.components.service.fxa.manager.SyncEnginesStorage
+import org.mozilla.fenix.R
+import org.mozilla.fenix.settings.logins.fragment.SavedLoginsAuthFragmentDirections
+
+/**
+ * Helper to manage the [R.string.pref_key_password_sync_logins] preference.
+ */
+class SyncLoginsPreferenceView(
+    private val syncLoginsPreference: Preference,
+    lifecycleOwner: LifecycleOwner,
+    accountManager: FxaAccountManager,
+    private val navController: NavController
+) {
+
+    init {
+        accountManager.register(object : AccountObserver {
+            override fun onAuthenticated(account: OAuthAccount, authType: AuthType) =
+                updateSyncPreferenceStatus()
+            override fun onLoggedOut() = updateSyncPreferenceNeedsLogin()
+            override fun onAuthenticationProblems() = updateSyncPreferenceNeedsReauth()
+        }, owner = lifecycleOwner)
+
+        val accountExists = accountManager.authenticatedAccount() != null
+        val needsReauth = accountManager.accountNeedsReauth()
+        when {
+            needsReauth -> updateSyncPreferenceNeedsReauth()
+            accountExists -> updateSyncPreferenceStatus()
+            !accountExists -> updateSyncPreferenceNeedsLogin()
+        }
+    }
+
+    /**
+     * Show the current status of the sync preference (on/off) for the logged in user.
+     */
+    private fun updateSyncPreferenceStatus() {
+        syncLoginsPreference.apply {
+            val syncEnginesStatus = SyncEnginesStorage(context).getStatus()
+            val loginsSyncStatus = syncEnginesStatus.getOrElse(SyncEngine.Passwords) { false }
+            summary = context.getString(
+                if (loginsSyncStatus) R.string.preferences_passwords_sync_logins_on
+                else R.string.preferences_passwords_sync_logins_off
+            )
+            setOnPreferenceClickListener {
+                navigateToAccountSettingsFragment()
+                true
+            }
+        }
+    }
+
+    /**
+     * Indicate that the user can sign in to turn on sync.
+     */
+    private fun updateSyncPreferenceNeedsLogin() {
+        syncLoginsPreference.apply {
+            summary = context.getString(R.string.preferences_passwords_sync_logins_sign_in)
+            setOnPreferenceClickListener {
+                navigateToTurnOnSyncFragment()
+                true
+            }
+        }
+    }
+
+    /**
+     * Indicate that the user can fix their account problems to turn on sync.
+     */
+    private fun updateSyncPreferenceNeedsReauth() {
+        syncLoginsPreference.apply {
+            summary = context.getString(R.string.preferences_passwords_sync_logins_reconnect)
+            setOnPreferenceClickListener {
+                navigateToAccountProblemFragment()
+                true
+            }
+        }
+    }
+
+    private fun navigateToAccountSettingsFragment() {
+        val directions =
+            SavedLoginsAuthFragmentDirections.actionGlobalAccountSettingsFragment()
+        navController.navigate(directions)
+    }
+
+    private fun navigateToAccountProblemFragment() {
+        val directions = SavedLoginsAuthFragmentDirections.actionGlobalAccountProblemFragment()
+        navController.navigate(directions)
+    }
+
+    private fun navigateToTurnOnSyncFragment() {
+        val directions = SavedLoginsAuthFragmentDirections.actionSavedLoginsAuthFragmentToTurnOnSyncFragment()
+        navController.navigate(directions)
+    }
+}

app/src/main/java/org/mozilla/fenix/settings/logins/fragment/SavedLoginsAuthFragment.kt

@@ -4,15 +4,15 @@
 
 package org.mozilla.fenix.settings.logins.fragment
 
-import android.annotation.TargetApi
 import android.app.Activity.RESULT_OK
 import android.app.KeyguardManager
-import android.content.Context.KEYGUARD_SERVICE
+import android.content.Context
 import android.content.DialogInterface
 import android.content.Intent
-import android.os.Build
+import android.os.Build.VERSION.SDK_INT
 import android.os.Build.VERSION_CODES.M
 import android.os.Bundle
+import android.provider.Settings.ACTION_SECURITY_SETTINGS
 import android.util.Log
 import androidx.appcompat.app.AlertDialog
 import androidx.biometric.BiometricManager
@@ -27,11 +27,6 @@ import androidx.preference.SwitchPreference
 import kotlinx.coroutines.Dispatchers.Main
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.launch
-import mozilla.components.concept.sync.AccountObserver
-import mozilla.components.concept.sync.AuthType
-import mozilla.components.concept.sync.OAuthAccount
-import mozilla.components.service.fxa.SyncEngine
-import mozilla.components.service.fxa.manager.SyncEnginesStorage
 import org.mozilla.fenix.R
 import org.mozilla.fenix.addons.runIfFragmentIsAttached
 import org.mozilla.fenix.components.metrics.Event
@@ -41,24 +36,33 @@ import org.mozilla.fenix.ext.secure
 import org.mozilla.fenix.ext.settings
 import org.mozilla.fenix.ext.showToolbar
 import org.mozilla.fenix.settings.SharedPreferenceUpdater
+import org.mozilla.fenix.settings.logins.SyncLoginsPreferenceView
 import org.mozilla.fenix.settings.requirePreference
-import java.util.concurrent.Executor
 
-@Suppress("TooManyFunctions", "LargeClass")
+@Suppress("TooManyFunctions")
-class SavedLoginsAuthFragment : PreferenceFragmentCompat(), AccountObserver {
+class SavedLoginsAuthFragment : PreferenceFragmentCompat() {
 
-    @TargetApi(M)
-    private lateinit var biometricPromptCallback: BiometricPrompt.AuthenticationCallback
-
-    @TargetApi(M)
-    private lateinit var executor: Executor
-
-    @TargetApi(M)
     private lateinit var biometricPrompt: BiometricPrompt
-
-    @TargetApi(M)
     private lateinit var promptInfo: BiometricPrompt.PromptInfo
 
+    private val biometricPromptCallback = object : BiometricPrompt.AuthenticationCallback() {
+
+        override fun onAuthenticationError(errorCode: Int, errString: CharSequence) {
+            Log.e(LOG_TAG, "onAuthenticationError $errString")
+            togglePrefsEnabledWhileAuthenticating(enabled = true)
+        }
+
+        override fun onAuthenticationSucceeded(result: BiometricPrompt.AuthenticationResult) {
+            Log.d(LOG_TAG, "onAuthenticationSucceeded")
+            navigateToSavedLogins()
+        }
+
+        override fun onAuthenticationFailed() {
+            Log.e(LOG_TAG, "onAuthenticationFailed")
+            togglePrefsEnabledWhileAuthenticating(enabled = true)
+        }
+    }
+
     override fun onCreatePreferences(savedInstanceState: Bundle?, rootKey: String?) {
         setPreferencesFromResource(R.xml.logins_preferences, rootKey)
     }
@@ -88,25 +92,7 @@ class SavedLoginsAuthFragment : PreferenceFragmentCompat(), AccountObserver {
     override fun onCreate(savedInstanceState: Bundle?) {
         super.onCreate(savedInstanceState)
 
-        biometricPromptCallback = object : BiometricPrompt.AuthenticationCallback() {
+        val executor = ContextCompat.getMainExecutor(requireContext())
-
-            override fun onAuthenticationError(errorCode: Int, errString: CharSequence) {
-                Log.e(LOG_TAG, "onAuthenticationError $errString")
-                togglePrefsEnabledWhileAuthenticating(enabled = true)
-            }
-
-            override fun onAuthenticationSucceeded(result: BiometricPrompt.AuthenticationResult) {
-                Log.d(LOG_TAG, "onAuthenticationSucceeded")
-                navigateToSavedLogins()
-            }
-
-            override fun onAuthenticationFailed() {
-                Log.e(LOG_TAG, "onAuthenticationFailed")
-                togglePrefsEnabledWhileAuthenticating(enabled = true)
-            }
-        }
-
-        executor = ContextCompat.getMainExecutor(requireContext())
 
         biometricPrompt = BiometricPrompt(this, executor, biometricPromptCallback)
 
@@ -116,7 +102,6 @@ class SavedLoginsAuthFragment : PreferenceFragmentCompat(), AccountObserver {
             .build()
     }
 
-    @Suppress("ComplexMethod")
     override fun onResume() {
         super.onResume()
         showToolbar(getString(R.string.preferences_passwords_logins_and_passwords))
@@ -144,7 +129,7 @@ class SavedLoginsAuthFragment : PreferenceFragmentCompat(), AccountObserver {
             isChecked = context.settings().shouldAutofillLogins
             onPreferenceChangeListener = object : SharedPreferenceUpdater() {
                 override fun onPreferenceChange(preference: Preference, newValue: Any?): Boolean {
-                    context?.components?.core?.engine?.settings?.loginAutofillEnabled =
+                    context.components.core.engine.settings.loginAutofillEnabled =
                         newValue as Boolean
                     return super.onPreferenceChange(preference, newValue)
                 }
@@ -152,155 +137,95 @@ class SavedLoginsAuthFragment : PreferenceFragmentCompat(), AccountObserver {
         }
 
         requirePreference<Preference>(R.string.pref_key_saved_logins).setOnPreferenceClickListener {
-            if (Build.VERSION.SDK_INT >= M && isHardwareAvailable && hasBiometricEnrolled) {
+            verifyCredentialsOrShowSetupWarning(it.context)
-                togglePrefsEnabledWhileAuthenticating(enabled = false)
-                biometricPrompt.authenticate(promptInfo)
-            } else {
-                verifyPinOrShowSetupWarning()
-            }
             true
         }
 
-        val accountManager = requireComponents.backgroundServices.accountManager
+        SyncLoginsPreferenceView(
-        accountManager.register(this, owner = this)
+            requirePreference(R.string.pref_key_password_sync_logins),
-
+            lifecycleOwner = viewLifecycleOwner,
-        val accountExists = accountManager.authenticatedAccount() != null
+            accountManager = requireComponents.backgroundServices.accountManager,
-        val needsReauth = accountManager.accountNeedsReauth()
+            navController = findNavController()
-        when {
+        )
-            needsReauth -> updateSyncPreferenceNeedsReauth()
-            accountExists -> updateSyncPreferenceStatus()
-            !accountExists -> updateSyncPreferenceNeedsLogin()
-        }
 
         togglePrefsEnabledWhileAuthenticating(enabled = true)
     }
 
-    override fun onAuthenticated(account: OAuthAccount, authType: AuthType) =
+    private fun canUseBiometricPrompt(context: Context): Boolean {
-        updateSyncPreferenceStatus()
+        return if (SDK_INT >= M) {
+            val manager = BiometricManager.from(context)
+            val canAuthenticate = manager.canAuthenticate()
 
-    override fun onLoggedOut() = updateSyncPreferenceNeedsLogin()
+            val hardwareUnavailable = canAuthenticate == BiometricManager.BIOMETRIC_ERROR_NO_HARDWARE ||
+                canAuthenticate == BiometricManager.BIOMETRIC_ERROR_HW_UNAVAILABLE
+            val biometricsEnrolled = canAuthenticate == BiometricManager.BIOMETRIC_SUCCESS
 
-    override fun onAuthenticationProblems() = updateSyncPreferenceNeedsReauth()
+            !hardwareUnavailable && biometricsEnrolled
-
-    private val isHardwareAvailable: Boolean by lazy {
-        if (Build.VERSION.SDK_INT >= M) {
-            context?.let {
-                val bm = BiometricManager.from(it)
-                val canAuthenticate = bm.canAuthenticate()
-                !(canAuthenticate == BiometricManager.BIOMETRIC_ERROR_NO_HARDWARE ||
-                        canAuthenticate == BiometricManager.BIOMETRIC_ERROR_HW_UNAVAILABLE)
-            } ?: false
         } else {
             false
         }
     }
 
-    private val hasBiometricEnrolled: Boolean by lazy {
+    private fun verifyCredentialsOrShowSetupWarning(context: Context) {
-        if (Build.VERSION.SDK_INT >= M) {
+        // Use the BiometricPrompt first
-            context?.let {
+        if (canUseBiometricPrompt(context)) {
-                val bm = BiometricManager.from(it)
+            biometricPrompt.authenticate(promptInfo)
-                val canAuthenticate = bm.canAuthenticate()
+            return
-                (canAuthenticate == BiometricManager.BIOMETRIC_SUCCESS)
+        }
-            } ?: false
+
+        // Fallback to prompting for password with the KeyguardManager
+        val manager = context.getSystemService<KeyguardManager>()
+        if (manager?.isKeyguardSecure == true) {
+            showPinVerification(manager)
         } else {
-            false
+            // Warn that the device has not been secured
-        }
+            if (context.settings().shouldShowSecurityPinWarning) {
-    }
+                showPinDialogWarning(context)
-
-    private fun updateSyncPreferenceStatus() {
-        requirePreference<Preference>(R.string.pref_key_password_sync_logins).apply {
-            val syncEnginesStatus = SyncEnginesStorage(requireContext()).getStatus()
-            val loginsSyncStatus = syncEnginesStatus.getOrElse(SyncEngine.Passwords) { false }
-            summary = getString(
-                if (loginsSyncStatus) R.string.preferences_passwords_sync_logins_on
-                else R.string.preferences_passwords_sync_logins_off
-            )
-            setOnPreferenceClickListener {
-                navigateToAccountSettingsFragment()
-                true
-            }
-        }
-    }
-
-    private fun updateSyncPreferenceNeedsLogin() {
-        requirePreference<Preference>(R.string.pref_key_password_sync_logins).apply {
-            summary = getString(R.string.preferences_passwords_sync_logins_sign_in)
-            setOnPreferenceClickListener {
-                navigateToTurnOnSyncFragment()
-                true
-            }
-        }
-    }
-
-    private fun updateSyncPreferenceNeedsReauth() {
-        requirePreference<Preference>(R.string.pref_key_password_sync_logins).apply {
-            summary = getString(R.string.preferences_passwords_sync_logins_reconnect)
-            setOnPreferenceClickListener {
-                navigateToAccountProblemFragment()
-                true
-            }
-        }
-    }
-
-    private fun verifyPinOrShowSetupWarning() {
-        val manager = activity?.getSystemService(KEYGUARD_SERVICE) as KeyguardManager
-        if (manager.isKeyguardSecure) {
-            showPinVerification()
-        } else {
-            if (context?.settings()?.shouldShowSecurityPinWarning == true) {
-                showPinDialogWarning()
             } else {
                 navigateToSavedLoginsFragment()
             }
         }
     }
 
-    private fun showPinDialogWarning() {
+    private fun showPinDialogWarning(context: Context) {
-        context?.let {
+        AlertDialog.Builder(context).apply {
-            AlertDialog.Builder(it).apply {
+            setTitle(getString(R.string.logins_warning_dialog_title))
-                setTitle(getString(R.string.logins_warning_dialog_title))
+            setMessage(
-                setMessage(
+                getString(R.string.logins_warning_dialog_message)
-                    getString(R.string.logins_warning_dialog_message)
+            )
-                )
 
-                setNegativeButton(getString(R.string.logins_warning_dialog_later)) { _: DialogInterface, _ ->
+            setNegativeButton(getString(R.string.logins_warning_dialog_later)) { _: DialogInterface, _ ->
-                    navigateToSavedLoginsFragment()
+                navigateToSavedLoginsFragment()
-                }
+            }
 
-                setPositiveButton(getString(R.string.logins_warning_dialog_set_up_now)) { it: DialogInterface, _ ->
+            setPositiveButton(getString(R.string.logins_warning_dialog_set_up_now)) { it: DialogInterface, _ ->
-                    it.dismiss()
+                it.dismiss()
-                    val intent = Intent(
+                val intent = Intent(ACTION_SECURITY_SETTINGS)
-                        android.provider.Settings.ACTION_SECURITY_SETTINGS
+                startActivity(intent)
-                    )
+            }
-                    startActivity(intent)
+            create()
-                }
+        }.show().secure(activity)
-                create()
+        context.settings().incrementShowLoginsSecureWarningCount()
-            }.show().secure(activity)
-            it.settings().incrementShowLoginsSecureWarningCount()
-        }
     }
 
-    @Suppress("Deprecation")
+    @Suppress("Deprecation") // This is only used when BiometricPrompt is unavailable
-    private fun showPinVerification() {
+    private fun showPinVerification(manager: KeyguardManager) {
-        val manager = activity?.getSystemService<KeyguardManager>()!!
         val intent = manager.createConfirmDeviceCredentialIntent(
             getString(R.string.logins_biometric_prompt_message_pin),
             getString(R.string.logins_biometric_prompt_message)
         )
-        startActivityForResult(
+        startActivityForResult(intent, PIN_REQUEST)
-            intent,
-            PIN_REQUEST
-        )
     }
 
     override fun onActivityResult(requestCode: Int, resultCode: Int, data: Intent?) {
         if (requestCode == PIN_REQUEST && resultCode == RESULT_OK) {
             navigateToSavedLoginsFragment()
-        } else {
-            super.onActivityResult(requestCode, resultCode, data)
         }
     }
 
+    /**
+     * Called when authentication succeeds.
+     */
     private fun navigateToSavedLoginsFragment() {
         context?.components?.analytics?.metrics?.track(Event.OpenLogins)
         val directions =
@@ -308,24 +233,6 @@ class SavedLoginsAuthFragment : PreferenceFragmentCompat(), AccountObserver {
         findNavController().navigate(directions)
     }
 
-    private fun navigateToAccountSettingsFragment() {
-        val directions =
-            SavedLoginsAuthFragmentDirections.actionGlobalAccountSettingsFragment()
-        findNavController().navigate(directions)
-    }
-
-    private fun navigateToAccountProblemFragment() {
-        val directions =
-            SavedLoginsAuthFragmentDirections.actionGlobalAccountProblemFragment()
-        findNavController().navigate(directions)
-    }
-
-    private fun navigateToTurnOnSyncFragment() {
-        val directions =
-            SavedLoginsAuthFragmentDirections.actionSavedLoginsAuthFragmentToTurnOnSyncFragment()
-        findNavController().navigate(directions)
-    }
-
     private fun navigateToSaveLoginSettingFragment() {
         val directions =
             SavedLoginsAuthFragmentDirections.actionSavedLoginsAuthFragmentToSavedLoginsSettingFragment()

app/src/test/java/org/mozilla/fenix/settings/logins/SyncLoginsPreferenceViewTest.kt

@@ -0,0 +1,146 @@
+package org.mozilla.fenix.settings.logins
+
+import android.content.Context
+import androidx.lifecycle.LifecycleOwner
+import androidx.navigation.NavController
+import androidx.preference.Preference
+import io.mockk.CapturingSlot
+import io.mockk.MockKAnnotations
+import io.mockk.Runs
+import io.mockk.every
+import io.mockk.impl.annotations.MockK
+import io.mockk.just
+import io.mockk.mockk
+import io.mockk.mockkConstructor
+import io.mockk.slot
+import io.mockk.unmockkConstructor
+import io.mockk.verify
+import mozilla.components.concept.sync.AccountObserver
+import mozilla.components.service.fxa.SyncEngine
+import mozilla.components.service.fxa.manager.FxaAccountManager
+import mozilla.components.service.fxa.manager.SyncEnginesStorage
+import org.junit.After
+import org.junit.Assert.assertTrue
+import org.junit.Before
+import org.junit.Test
+import org.mozilla.fenix.R
+import org.mozilla.fenix.settings.logins.fragment.SavedLoginsAuthFragmentDirections
+
+class SyncLoginsPreferenceViewTest {
+
+    @MockK private lateinit var syncLoginsPreference: Preference
+    @MockK private lateinit var lifecycleOwner: LifecycleOwner
+    @MockK private lateinit var accountManager: FxaAccountManager
+    @MockK(relaxed = true) private lateinit var navController: NavController
+    private lateinit var accountObserver: CapturingSlot<AccountObserver>
+    private lateinit var clickListener: CapturingSlot<Preference.OnPreferenceClickListener>
+
+    @Before
+    fun setup() {
+        MockKAnnotations.init(this)
+        mockkConstructor(SyncEnginesStorage::class)
+
+        accountObserver = slot()
+        clickListener = slot()
+        val context = mockk<Context> {
+            every { getString(R.string.preferences_passwords_sync_logins_reconnect) } returns "Reconnect"
+            every { getString(R.string.preferences_passwords_sync_logins_sign_in) } returns "Sign in to Sync"
+            every { getString(R.string.preferences_passwords_sync_logins_on) } returns "On"
+            every { getString(R.string.preferences_passwords_sync_logins_off) } returns "Off"
+        }
+
+        every { syncLoginsPreference.summary = any() } just Runs
+        every { syncLoginsPreference.onPreferenceClickListener = capture(clickListener) } just Runs
+        every { syncLoginsPreference.context } returns context
+        every { accountManager.register(capture(accountObserver), owner = lifecycleOwner) } just Runs
+        every { anyConstructed<SyncEnginesStorage>().getStatus() } returns emptyMap()
+    }
+
+    @After
+    fun teardown() {
+        unmockkConstructor(SyncEnginesStorage::class)
+    }
+
+    @Test
+    fun `needs reauth ui on init`() {
+        every { accountManager.authenticatedAccount() } returns mockk()
+        every { accountManager.accountNeedsReauth() } returns true
+        createView()
+
+        verify { syncLoginsPreference.summary = "Reconnect" }
+        assertTrue(clickListener.captured.onPreferenceClick(syncLoginsPreference))
+
+        verify {
+            navController.navigate(
+                SavedLoginsAuthFragmentDirections.actionGlobalAccountProblemFragment()
+            )
+        }
+    }
+
+    @Test
+    fun `needs reauth ui on init even if null account`() {
+        every { accountManager.authenticatedAccount() } returns null
+        every { accountManager.accountNeedsReauth() } returns true
+        createView()
+
+        verify { syncLoginsPreference.summary = "Reconnect" }
+    }
+
+    @Test
+    fun `needs login if account does not exist`() {
+        every { accountManager.authenticatedAccount() } returns null
+        every { accountManager.accountNeedsReauth() } returns false
+        createView()
+
+        verify { syncLoginsPreference.summary = "Sign in to Sync" }
+        assertTrue(clickListener.captured.onPreferenceClick(syncLoginsPreference))
+
+        verify {
+            navController.navigate(
+                SavedLoginsAuthFragmentDirections.actionSavedLoginsAuthFragmentToTurnOnSyncFragment()
+            )
+        }
+    }
+
+    @Test
+    fun `show status for existing account`() {
+        every { accountManager.authenticatedAccount() } returns mockk()
+        every { accountManager.accountNeedsReauth() } returns false
+        createView()
+
+        verify { syncLoginsPreference.summary = "Off" }
+        assertTrue(clickListener.captured.onPreferenceClick(syncLoginsPreference))
+
+        verify {
+            navController.navigate(
+                SavedLoginsAuthFragmentDirections.actionGlobalAccountSettingsFragment()
+            )
+        }
+    }
+
+    @Test
+    fun `show status for existing account with passwords`() {
+        every { anyConstructed<SyncEnginesStorage>().getStatus() } returns mapOf(
+            SyncEngine.Passwords to true
+        )
+        every { accountManager.authenticatedAccount() } returns mockk()
+        every { accountManager.accountNeedsReauth() } returns false
+        createView()
+
+        verify { syncLoginsPreference.summary = "On" }
+        assertTrue(clickListener.captured.onPreferenceClick(syncLoginsPreference))
+
+        verify {
+            navController.navigate(
+                SavedLoginsAuthFragmentDirections.actionGlobalAccountSettingsFragment()
+            )
+        }
+    }
+
+    private fun createView() = SyncLoginsPreferenceView(
+        syncLoginsPreference,
+        lifecycleOwner,
+        accountManager,
+        navController
+    )
+}