From bbe869ee4e814a2cd0b8b846b2fd5ee7ce48f811 Mon Sep 17 00:00:00 2001 From: Fahim Masud Choudhury Date: Mon, 18 Aug 2025 13:19:51 +0600 Subject: [PATCH] fix: resolve issue with Murena and Google account sync, and fix authState update logic For a non-SSO Murena account already set up, if users try to add a Google or Yahoo account later, Account Manager would incorrectly update the Murena account's authState with Google/Yahoo's authState. Users would see no Google/Yahoo account added in the Account Manager and the affected Murena account would behave incorrectly too. The fix checks the new account's name and type with the existing non-SSO Murena accounts and only proceeds if there's a match. Additionally, it resolved auth state updating logic. updateAuthState() method now correctly updates the associated account and saves them inside user data. Previously, for re-auth and SSO migration, the update method would **incorrectly** replace all existing account's auth state with the latest one. --- .../ui/setup/AccountDetailsFragment.kt | 95 +++++++++++-------- 1 file changed, 56 insertions(+), 39 deletions(-) diff --git a/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/AccountDetailsFragment.kt b/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/AccountDetailsFragment.kt index 27883cc30..b976a9e0d 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/AccountDetailsFragment.kt +++ b/app/src/main/kotlin/at/bitfire/davdroid/ui/setup/AccountDetailsFragment.kt @@ -49,8 +49,6 @@ import at.bitfire.davdroid.servicedetection.RefreshCollectionsWorker import at.bitfire.davdroid.settings.AccountSettings import at.bitfire.davdroid.settings.SettingsManager import at.bitfire.davdroid.syncadapter.AccountUtils -import at.bitfire.davdroid.syncadapter.AccountUtils.isLoggedInWithMurenaSso -import at.bitfire.davdroid.syncadapter.AccountUtils.isMurenaAccount import at.bitfire.davdroid.syncadapter.SyncAllAccountWorker import at.bitfire.davdroid.syncadapter.SyncWorker import at.bitfire.davdroid.util.AuthStatePrefUtils @@ -175,14 +173,7 @@ class AccountDetailsFragment : Fragment() { val idx = v.contactGroupMethod.selectedItemPosition val groupMethodName = resources.getStringArray(R.array.settings_contact_group_method_values)[idx] - val existingNonSsoMurenaAccount = AccountUtils.getMainAccounts(requireContext()) - .firstOrNull { account -> - isMurenaAccount(requireContext(), account) && - !isLoggedInWithMurenaSso(requireContext(), account) - } - existingNonSsoMurenaAccount?.let { - Logger.log.info("Found non-SSO Murena account: $it") - } + val accountToUpdate = findBasicAuthMurenaAccount(name, providedAccountType) model.createOrUpdateAccount( requireActivity(), @@ -190,7 +181,7 @@ class AccountDetailsFragment : Fragment() { loginModel.credentials!!, config, GroupMethod.valueOf(groupMethodName), - existingNonSsoMurenaAccount, + accountToUpdate, ).observe(viewLifecycleOwner, Observer { success -> if (success) { Toast.makeText(context, R.string.message_account_added_successfully, Toast.LENGTH_LONG).show() @@ -225,6 +216,32 @@ class AccountDetailsFragment : Fragment() { return v.root } + private fun findBasicAuthMurenaAccount(accountName: String, accountType: String?): Account? { + val requiredAccountType = requireContext().getString(R.string.eelo_account_type) + if (accountType != requiredAccountType) { + return null + } + + val account = AccountManager.get(requireContext()) + .getAccountsByType(requiredAccountType) + .firstOrNull() ?: return null + + return when { + !matchesAccountName(account, accountName) -> null + !AccountUtils.isMurenaAccount(requireContext(), account) -> null + AccountUtils.isLoggedInWithMurenaSso(requireContext(), account) -> null + else -> account + } + } + + private fun matchesAccountName(account: Account, accountName: String): Boolean { + // In Murena account's `name` field, non-SSO accounts contain domain name i.e. `user.name@example.com`, + // whereas SSO accounts have `user.name`. + // Only the name part without the domain needs to be extracted and compared. + + return account.name.substringBefore("@") == accountName.substringBefore("@") + } + private fun handlePostMurenaSsoMigrationOperations() { val authState = requireActivity().intent.getStringExtra(LoginActivity.AUTH_STATE) val isMigratedToMurenaSso = !authState.isNullOrBlank() @@ -359,14 +376,13 @@ class AccountDetailsFragment : Fragment() { val accountManager = AccountManager.get(context) if (accountToUpdate != null) { - Logger.log.info("Updating auth state for existing non-SSO Murena account: $account") - updateAuthState(userData, accountManager, account) + val authState = credentials?.authState + if (authState != null) { + updateAuthState(userData, account) + Logger.log.info("Updated auth state for $account") - // Clear password for SSO account - val authState = AuthStatePrefUtils.loadAuthState(context, name, accountType) - if (!authState.isNullOrBlank()) { accountManager.clearPassword(account) - Logger.log.info("Cleared password for account: $account") + Logger.log.info("Cleared password for $account") } } else { if (!AccountUtils.createAccount( @@ -377,7 +393,8 @@ class AccountDetailsFragment : Fragment() { ) ) { if (accountType in AccountUtils.getOpenIdMainAccountTypes(context) && credentials?.authState != null) { - updateAuthState(userData, accountManager, account) + updateAuthState(userData, account) + Logger.log.info("Updated auth state for re-authenticating $account") } else { result.postValue(false) return@launch @@ -530,7 +547,7 @@ class AccountDetailsFragment : Fragment() { ) // add entries for account to service DB - Logger.log.log(Level.INFO, "Writing account configuration to database", config) + Logger.log.log(Level.INFO, "Writing $account configuration to database", config) try { val accountSettings = AccountSettings(context, account) @@ -618,7 +635,7 @@ class AccountDetailsFragment : Fragment() { tasksSyncEnabled ) accountSettings.setSyncInterval(taskProvider.authority, tasksSyncInterval, false) - Logger.log.info("Tasks provider ${taskProvider.authority} found. Tasks sync enabled.") + Logger.log.info("Tasks provider ${taskProvider.authority} found for $account. Tasks sync enabled.") } else { Logger.log.info("No tasks provider found. Did not enable tasks sync.") } @@ -696,29 +713,29 @@ class AccountDetailsFragment : Fragment() { private fun updateAuthState( userData: Bundle, - accountManager: AccountManager, - account: Account + providedAccount: Account ) { - for (openIdAccount in AccountUtils.getOpenIdMainAccounts(context)) { - if (userData.get(AccountSettings.KEY_EMAIL_ADDRESS) == accountManager - .getUserData(account, AccountSettings.KEY_EMAIL_ADDRESS) - ) { - val authState = userData.getString(AccountSettings.KEY_AUTH_STATE) - - accountManager.setUserData( - openIdAccount, AccountSettings.KEY_AUTH_STATE, - authState - ) - accountManager.setUserData( - openIdAccount, AccountSettings.KEY_CLIENT_SECRET, - userData.getString(AccountSettings.KEY_CLIENT_SECRET) - ) + val accountManager = AccountManager.get(context) - AuthStatePrefUtils.saveAuthState(context, openIdAccount, authState) - } - } + val account = accountManager.getAccountsByType(providedAccount.type) + .firstOrNull { matchesEmail(userData, accountManager, it) } ?: return + + val authState = userData.getString(AccountSettings.KEY_AUTH_STATE) + accountManager.setUserData(account, AccountSettings.KEY_AUTH_STATE, authState) + accountManager.setUserData( + account, + AccountSettings.KEY_CLIENT_SECRET, + userData.getString(AccountSettings.KEY_CLIENT_SECRET) + ) } + private fun matchesEmail( + userData: Bundle, + accountManager: AccountManager, + account: Account + ) = userData.getString(AccountSettings.KEY_EMAIL_ADDRESS) == accountManager + .getUserData(account, AccountSettings.KEY_EMAIL_ADDRESS) + private fun insertService( accountName: String, authState: String?, -- GitLab