Use AccountManager.setAndVerifyUserData extension method, instead of setUserData (bitfireAT/davx5#344)

Hopefully fixes bitfireAT/davx5#308

* Add new setAndVerifyUserData extension function to AccountManager
* Use new setAndVerifyUserData extension function instead of insecure setUserData
* Update KDoc [skip CI]

---------

Co-authored-by: Ricki Hirner <hirner@bitfire.at>
This commit is contained in:
Sunik Kupfer 2023-08-15 17:21:24 +02:00 committed by Ricki Hirner
parent fcb9d7f560
commit 4ce33e949b
6 changed files with 49 additions and 23 deletions

View file

@ -21,6 +21,7 @@ import at.bitfire.davdroid.log.Logger
import at.bitfire.davdroid.settings.AccountSettings
import at.bitfire.davdroid.syncadapter.AccountUtils
import at.bitfire.davdroid.util.DavUtils
import at.bitfire.davdroid.util.setAndVerifyUserData
import at.bitfire.vcard4android.*
import java.io.ByteArrayOutputStream
import java.util.*
@ -170,8 +171,8 @@ open class LocalAddressBook(
}
set(newMainAccount) {
AccountManager.get(context).let { accountManager ->
accountManager.setUserData(account, USER_DATA_MAIN_ACCOUNT_NAME, newMainAccount.name)
accountManager.setUserData(account, USER_DATA_MAIN_ACCOUNT_TYPE, newMainAccount.type)
accountManager.setAndVerifyUserData(account, USER_DATA_MAIN_ACCOUNT_NAME, newMainAccount.name)
accountManager.setAndVerifyUserData(account, USER_DATA_MAIN_ACCOUNT_TYPE, newMainAccount.type)
}
_mainAccount = newMainAccount
@ -180,11 +181,11 @@ open class LocalAddressBook(
var url: String
get() = AccountManager.get(context).getUserData(account, USER_DATA_URL)
?: throw IllegalStateException("Address book has no URL")
set(url) = AccountManager.get(context).setUserData(account, USER_DATA_URL, url)
set(url) = AccountManager.get(context).setAndVerifyUserData(account, USER_DATA_URL, url)
override var readOnly: Boolean
get() = AccountManager.get(context).getUserData(account, USER_DATA_READ_ONLY) != null
set(readOnly) = AccountManager.get(context).setUserData(account, USER_DATA_READ_ONLY, if (readOnly) "1" else null)
set(readOnly) = AccountManager.get(context).setAndVerifyUserData(account, USER_DATA_READ_ONLY, if (readOnly) "1" else null)
override var lastSyncState: SyncState?
get() = syncState?.let { SyncState.fromString(String(it)) }

View file

@ -17,6 +17,7 @@ import at.bitfire.davdroid.log.Logger
import at.bitfire.davdroid.resource.LocalAddressBook
import at.bitfire.davdroid.syncadapter.PeriodicSyncWorker
import at.bitfire.davdroid.syncadapter.SyncUtils
import at.bitfire.davdroid.util.setAndVerifyUserData
import at.bitfire.ical4android.TaskProvider
import at.bitfire.vcard4android.GroupMethod
import dagger.hilt.EntryPoint
@ -194,14 +195,14 @@ class AccountSettings(
fun credentials(credentials: Credentials) {
// Basic/Digest auth
accountManager.setUserData(account, KEY_USERNAME, credentials.userName)
accountManager.setAndVerifyUserData(account, KEY_USERNAME, credentials.userName)
accountManager.setPassword(account, credentials.password)
// client certificate
accountManager.setUserData(account, KEY_CERTIFICATE_ALIAS, credentials.certificateAlias)
accountManager.setAndVerifyUserData(account, KEY_CERTIFICATE_ALIAS, credentials.certificateAlias)
// OAuth
accountManager.setUserData(account, KEY_AUTH_STATE, credentials.authState?.jsonSerializeString())
accountManager.setAndVerifyUserData(account, KEY_AUTH_STATE, credentials.authState?.jsonSerializeString())
}
@ -261,7 +262,7 @@ class AccountSettings(
else ->
throw IllegalArgumentException("Sync interval not applicable to authority $authority")
}
accountManager.setUserData(account, key, seconds.toString())
accountManager.setAndVerifyUserData(account, key, seconds.toString())
// update sync workers (needs already updated sync interval in AccountSettings)
updatePeriodicSyncWorker(authority, seconds, getSyncWifiOnly())
@ -323,7 +324,7 @@ class AccountSettings(
accountManager.getUserData(account, KEY_WIFI_ONLY) != null
fun setSyncWiFiOnly(wiFiOnly: Boolean) {
accountManager.setUserData(account, KEY_WIFI_ONLY, if (wiFiOnly) "1" else null)
accountManager.setAndVerifyUserData(account, KEY_WIFI_ONLY, if (wiFiOnly) "1" else null)
// update sync workers (needs already updated wifi-only flag in AccountSettings)
for (authority in SyncUtils.syncAuthorities(context))
@ -340,7 +341,7 @@ class AccountSettings(
} else
null
fun setSyncWifiOnlySSIDs(ssids: List<String>?) =
accountManager.setUserData(account, KEY_WIFI_ONLY_SSIDS, StringUtils.trimToNull(ssids?.joinToString(",")))
accountManager.setAndVerifyUserData(account, KEY_WIFI_ONLY_SSIDS, StringUtils.trimToNull(ssids?.joinToString(",")))
/**
* Updates the periodic sync worker of an authority according to
@ -382,7 +383,7 @@ class AccountSettings(
}
fun setTimeRangePastDays(days: Int?) =
accountManager.setUserData(account, KEY_TIME_RANGE_PAST_DAYS, (days ?: -1).toString())
accountManager.setAndVerifyUserData(account, KEY_TIME_RANGE_PAST_DAYS, (days ?: -1).toString())
/**
* Takes the default alarm setting (in this order) from
@ -407,7 +408,7 @@ class AccountSettings(
* start of every non-full-day event without reminder. *null*: No default reminders shall be created.
*/
fun setDefaultAlarm(minBefore: Int?) =
accountManager.setUserData(account, KEY_DEFAULT_ALARM,
accountManager.setAndVerifyUserData(account, KEY_DEFAULT_ALARM,
if (minBefore == settings.getIntOrNull(KEY_DEFAULT_ALARM)?.takeIf { it != -1 })
null
else
@ -418,14 +419,14 @@ class AccountSettings(
else
accountManager.getUserData(account, KEY_MANAGE_CALENDAR_COLORS) == null
fun setManageCalendarColors(manage: Boolean) =
accountManager.setUserData(account, KEY_MANAGE_CALENDAR_COLORS, if (manage) null else "0")
accountManager.setAndVerifyUserData(account, KEY_MANAGE_CALENDAR_COLORS, if (manage) null else "0")
fun getEventColors() = if (settings.containsKey(KEY_EVENT_COLORS))
settings.getBoolean(KEY_EVENT_COLORS)
else
accountManager.getUserData(account, KEY_EVENT_COLORS) != null
fun setEventColors(useColors: Boolean) =
accountManager.setUserData(account, KEY_EVENT_COLORS, if (useColors) "1" else null)
accountManager.setAndVerifyUserData(account, KEY_EVENT_COLORS, if (useColors) "1" else null)
// CardDAV settings
@ -442,7 +443,7 @@ class AccountSettings(
}
fun setGroupMethod(method: GroupMethod) {
accountManager.setUserData(account, KEY_CONTACT_GROUP_METHOD, method.name)
accountManager.setAndVerifyUserData(account, KEY_CONTACT_GROUP_METHOD, method.name)
}
@ -464,7 +465,7 @@ class AccountSettings(
}
fun setShowOnlyPersonal(showOnlyPersonal: Boolean) {
accountManager.setUserData(account, KEY_SHOW_ONLY_PERSONAL, if (showOnlyPersonal) "1" else null)
accountManager.setAndVerifyUserData(account, KEY_SHOW_ONLY_PERSONAL, if (showOnlyPersonal) "1" else null)
}
@ -487,7 +488,7 @@ class AccountSettings(
updateProc.invoke(migrations)
Logger.log.info("Account version update successful")
accountManager.setUserData(account, KEY_SETTINGS_VERSION, toVersion.toString())
accountManager.setAndVerifyUserData(account, KEY_SETTINGS_VERSION, toVersion.toString())
} catch (e: Exception) {
Logger.log.log(Level.SEVERE, "Couldn't update account settings", e)
}

View file

@ -25,6 +25,7 @@ import at.bitfire.davdroid.resource.LocalTask
import at.bitfire.davdroid.resource.TaskUtils
import at.bitfire.davdroid.syncadapter.SyncUtils
import at.bitfire.davdroid.util.closeCompat
import at.bitfire.davdroid.util.setAndVerifyUserData
import at.bitfire.ical4android.AndroidCalendar
import at.bitfire.ical4android.AndroidEvent
import at.bitfire.ical4android.TaskProvider
@ -229,7 +230,7 @@ class AccountSettingsMigrations(
TaskUtils.currentProvider(context)?.let { provider ->
val interval = accountSettings.getSyncInterval(provider.authority)
if (interval != null)
accountManager.setUserData(account,
accountManager.setAndVerifyUserData(account,
AccountSettings.KEY_SYNC_INTERVAL_TASKS, interval.toString())
}
}
@ -318,8 +319,8 @@ class AccountSettingsMigrations(
// update allowed WiFi settings key
val onlySSID = accountManager.getUserData(account, "wifi_only_ssid")
accountManager.setUserData(account, AccountSettings.KEY_WIFI_ONLY_SSIDS, onlySSID)
accountManager.setUserData(account, "wifi_only_ssid", null)
accountManager.setAndVerifyUserData(account, AccountSettings.KEY_WIFI_ONLY_SSIDS, onlySSID)
accountManager.setAndVerifyUserData(account, "wifi_only_ssid", null)
}
@Suppress("unused")
@ -381,7 +382,7 @@ class AccountSettingsMigrations(
}
// update version number so that further syncs don't repeat the migration
accountManager.setUserData(account, AccountSettings.KEY_SETTINGS_VERSION, "6")
accountManager.setAndVerifyUserData(account, AccountSettings.KEY_SETTINGS_VERSION, "6")
// request sync of new address book account
ContentResolver.setIsSyncable(account, context.getString(R.string.address_books_authority), 1)

View file

@ -9,6 +9,7 @@ import android.accounts.AccountManager
import android.content.Context
import android.os.Bundle
import at.bitfire.davdroid.log.Logger
import at.bitfire.davdroid.util.setAndVerifyUserData
object AccountUtils {
@ -42,7 +43,7 @@ object AccountUtils {
// https://forums.bitfire.at/post/11644
if (!verifyUserData(context, account, userData))
for (key in userData.keySet())
manager.setUserData(account, key, userData.getString(key))
manager.setAndVerifyUserData(account, key, userData.getString(key))
if (!verifyUserData(context, account, userData))
throw IllegalStateException("Android doesn't store user data in account")

View file

@ -13,6 +13,7 @@ import at.bitfire.davdroid.network.HttpClient
import at.bitfire.davdroid.log.Logger
import at.bitfire.davdroid.resource.LocalAddressBook
import at.bitfire.davdroid.settings.AccountSettings
import at.bitfire.davdroid.util.setAndVerifyUserData
import java.util.logging.Level
/**
@ -50,7 +51,7 @@ class ContactSyncer(context: Context): Syncer(context) {
addressBook.syncState = null
}
}
accountSettings.accountManager.setUserData(account, PREVIOUS_GROUP_METHOD, groupMethod)
accountSettings.accountManager.setAndVerifyUserData(account, PREVIOUS_GROUP_METHOD, groupMethod)
Logger.log.info("Synchronizing address book: ${addressBook.url}")
Logger.log.info("Taking settings from: ${addressBook.mainAccount}")

View file

@ -4,8 +4,29 @@
package at.bitfire.davdroid.util
import android.accounts.Account
import android.accounts.AccountManager
import android.content.ContentProviderClient
import android.os.Build
import at.bitfire.davdroid.log.Logger
/**
* [AccountManager.setUserData] has been found to be unreliable at times. This extension function
* checks whether the user data has actually been set and retries up to ten times before failing silently.
*
* Note: In the future we want to store accounts + associated data in the database, never calling
* so this method will become obsolete then.
*/
fun AccountManager.setAndVerifyUserData(account: Account, key: String, value: String?) {
for (i in 1..10) {
setUserData(account, key, value)
if (getUserData(account, key) == value)
return /* success */
Thread.sleep(100)
}
Logger.log.warning("AccountManager failed to set $account user data $key := $value")
}
@Suppress("DEPRECATION")
fun ContentProviderClient.closeCompat() {