From ce64c2439dbab2e8d7d9731dd394aafe8e6279cc Mon Sep 17 00:00:00 2001 From: Mugurell Date: Thu, 17 Jun 2021 12:49:38 +0300 Subject: [PATCH] For #19797 - Use the FXA email only if the account is authenticated An account may exist but it may need to be re-authenticated. In this case also the email should not be exposed to the app. --- .../accounts/FenixAccountManager.kt | 11 +++++- .../components/toolbar/DefaultToolbarMenu.kt | 12 +----- .../java/org/mozilla/fenix/home/HomeMenu.kt | 12 +----- .../accounts/FenixAccountManagerTest.kt | 39 ++++++++++++------- 4 files changed, 38 insertions(+), 36 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/components/accounts/FenixAccountManager.kt b/app/src/main/java/org/mozilla/fenix/components/accounts/FenixAccountManager.kt index 0c53d519f4..f47b3ab3e9 100644 --- a/app/src/main/java/org/mozilla/fenix/components/accounts/FenixAccountManager.kt +++ b/app/src/main/java/org/mozilla/fenix/components/accounts/FenixAccountManager.kt @@ -18,8 +18,15 @@ open class FenixAccountManager(context: Context) { val authenticatedAccount get() = accountManager.authenticatedAccount() != null - val accountProfileEmail - get() = accountManager.accountProfile()?.email + /** + * Returns the Firefox Account email if authenticated in the app, `null` otherwise. + */ + val accountProfileEmail: String? + get() = if (accountState == AccountState.AUTHENTICATED) { + accountManager.accountProfile()?.email + } else { + null + } /** * The current state of the Firefox Account. See [AccountState]. diff --git a/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt b/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt index cf00b50871..85c575033c 100644 --- a/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt +++ b/app/src/main/java/org/mozilla/fenix/components/toolbar/DefaultToolbarMenu.kt @@ -332,16 +332,8 @@ open class DefaultToolbarMenu( onItemTapped.invoke(ToolbarMenu.Item.Quit) } - private fun getSyncItemTitle(): String { - val authenticatedAccount = accountManager.authenticatedAccount - val email = accountManager.accountProfileEmail - - return if (authenticatedAccount && !email.isNullOrEmpty()) { - email - } else { - context.getString(R.string.sync_menu_sign_in) - } - } + private fun getSyncItemTitle() = + accountManager.accountProfileEmail ?: context.getString(R.string.sync_menu_sign_in) val syncMenuItem = BrowserMenuImageText( getSyncItemTitle(), diff --git a/app/src/main/java/org/mozilla/fenix/home/HomeMenu.kt b/app/src/main/java/org/mozilla/fenix/home/HomeMenu.kt index 6746aed99b..56cd69cdce 100644 --- a/app/src/main/java/org/mozilla/fenix/home/HomeMenu.kt +++ b/app/src/main/java/org/mozilla/fenix/home/HomeMenu.kt @@ -91,16 +91,8 @@ class HomeMenu( } } - private fun getSyncItemTitle(): String { - val authenticatedAccount = accountManager.authenticatedAccount - val email = accountManager.accountProfileEmail - - return if (authenticatedAccount && !email.isNullOrEmpty()) { - email - } else { - context.getString(R.string.sync_menu_sign_in) - } - } + private fun getSyncItemTitle(): String = + accountManager.accountProfileEmail ?: context.getString(R.string.sync_menu_sign_in) private val syncSignInMenuItem = BrowserMenuImageText( getSyncItemTitle(), diff --git a/app/src/test/java/org/mozilla/fenix/components/accounts/FenixAccountManagerTest.kt b/app/src/test/java/org/mozilla/fenix/components/accounts/FenixAccountManagerTest.kt index c53a0bf304..dd72e4bd66 100644 --- a/app/src/test/java/org/mozilla/fenix/components/accounts/FenixAccountManagerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/components/accounts/FenixAccountManagerTest.kt @@ -14,11 +14,8 @@ import org.junit.Assert.assertEquals import org.junit.Assert.assertSame import org.junit.Before import org.junit.Test -import org.junit.runner.RunWith import org.mozilla.fenix.ext.components -import org.mozilla.fenix.helpers.FenixRobolectricTestRunner -@RunWith(FenixRobolectricTestRunner::class) class FenixAccountManagerTest { private lateinit var fenixFxaManager: FenixAccountManager @@ -36,30 +33,44 @@ class FenixAccountManagerTest { } @Test - fun `GIVEN an account exists THEN fetch the associated email address`() { + fun `GIVEN an account does not exist WHEN accountProfileEmail is called THEN it returns null`() { + every { accountManagerComponent.authenticatedAccount() } returns null + every { accountManagerComponent.accountProfile() } returns null + every { context.components.backgroundServices.accountManager } returns accountManagerComponent + fenixFxaManager = FenixAccountManager(context) + + val result = fenixFxaManager.accountProfileEmail + + assertEquals(null, result) + } + + @Test + fun `GIVEN an account exists but needs to be re-authenticated WHEN accountProfileEmail is called THEN it returns null`() { every { accountManagerComponent.authenticatedAccount() } returns account every { accountManagerComponent.accountProfile() } returns profile + every { accountManagerComponent.accountProfile()?.email } returns "firefoxIsFun@test.com" + every { accountManagerComponent.accountNeedsReauth() } returns true every { context.components.backgroundServices.accountManager } returns accountManagerComponent - fenixFxaManager = FenixAccountManager(context) - val emailAddress = "firefoxIsFun@test.com" - every { accountManagerComponent.accountProfile()?.email } returns emailAddress - val result = fenixFxaManager.accountProfileEmail - assertEquals(emailAddress, result) + + assertEquals(null, result) } @Test - fun `GIVEN an account does not exist THEN return null when fetching the associated email address`() { - every { accountManagerComponent.authenticatedAccount() } returns null - every { accountManagerComponent.accountProfile() } returns null + fun `GIVEN an account exists and doesn't need to be re-authenticated WHEN accountProfileEmail is called THEN it returns the associated email address`() { + every { accountManagerComponent.authenticatedAccount() } returns account + every { accountManagerComponent.accountProfile() } returns profile + val accountEmail = "firefoxIsFun@test.com" + every { accountManagerComponent.accountProfile()?.email } returns accountEmail + every { accountManagerComponent.accountNeedsReauth() } returns false every { context.components.backgroundServices.accountManager } returns accountManagerComponent - fenixFxaManager = FenixAccountManager(context) val result = fenixFxaManager.accountProfileEmail - assertEquals(null, result) + + assertEquals(accountEmail, result) } @Test