Skip to content

Commit

Permalink
fix: try catch shared pref creation on android (#2013)
Browse files Browse the repository at this point in the history
* fix: try catch Exceptions when initializing encrypted shared pref

* ignore the test

* revert unwanted change

* detekt
  • Loading branch information
MohamadJaara authored and ohassine committed Oct 25, 2023
1 parent 51abd95 commit 3640dff
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 30 deletions.
2 changes: 1 addition & 1 deletion cryptography/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ kotlin {
addCommonKotlinJvmSourceDir()
dependencies {
implementation(libs.cryptoboxAndroid)
implementation(libs.javaxCrypto)
implementation(libs.androidCrypto)
implementation(libs.coreCryptoAndroid)
}
}
Expand Down
4 changes: 1 addition & 3 deletions gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ compose-ui = "1.3.2"
compose-material = "1.3.1"
cryptobox4j = "1.3.0"
cryptobox-android = "1.1.3"
javax-crypto = "1.1.0-alpha06"
android-security = "1.1.0-alpha06"
ktor = "2.3.1"
okio = "3.2.0"
Expand Down Expand Up @@ -92,7 +91,6 @@ composeTooling = { module = "androidx.compose.ui:ui-tooling", version.ref = "com
coroutinesAndroid = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-android", version.ref = "coroutines" }
ktor = { module = "io.ktor:ktor-client-android", version.ref = "ktor" }
paging3 = { module = "androidx.paging:paging-runtime", version.ref = "android-paging3" }
securityCrypto = { module = "androidx.security:security-crypto", version.ref = "android-security" }
desugarJdkLibs = { module = "com.android.tools:desugar_jdk_libs", version.ref = "desugar-jdk" }
annotation = { module = "androidx.annotation:annotation", version.ref = "annotation" }

Expand All @@ -114,7 +112,7 @@ coroutines-test = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-test", ve
# cryptobox and crypto dependencies
cryptoboxAndroid = { module = "com.wire:cryptobox-android", version.ref = "cryptobox-android" }
cryptobox4j = { module = "com.wire:cryptobox4j", version.ref = "cryptobox4j" }
javaxCrypto = { module = "androidx.security:security-crypto-ktx", version.ref = "javax-crypto" }
androidCrypto = { module = "androidx.security:security-crypto-ktx", version.ref = "android-security" }
coreCrypto = { module = "com.wire:core-crypto", version.ref = "core-crypto-multiplatform" }
coreCryptoJvm = { module = "com.wire:core-crypto-jvm", version.ref = "core-crypto" }
coreCryptoAndroid = { module = "com.wire:core-crypto-android", version.ref = "core-crypto" }
Expand Down
2 changes: 1 addition & 1 deletion persistence/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ kotlin {
val jsTest by getting
val androidMain by getting {
dependencies {
implementation(libs.securityCrypto)
implementation(libs.androidCrypto)
implementation(libs.sqldelight.androidDriver)
implementation(libs.sqldelight.androidxPaging)
implementation(libs.paging3)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,11 @@ package com.wire.kalium.persistence.kmmSettings

import android.content.Context
import androidx.test.core.app.ApplicationProvider
import com.russhwolf.settings.set
import com.wire.kalium.persistence.dao.UserIDEntity
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.joinAll
import kotlinx.coroutines.launch
import kotlinx.coroutines.test.StandardTestDispatcher
import kotlinx.coroutines.test.TestScope
import kotlinx.coroutines.test.UnconfinedTestDispatcher
import kotlinx.coroutines.test.advanceUntilIdle
Expand All @@ -41,7 +39,6 @@ import kotlin.test.Test
class EncryptedSettingsBuilderTest {

private val coroutineDispatcher = UnconfinedTestDispatcher()
private val testScope = TestScope(coroutineDispatcher)

@BeforeTest
fun before() {
Expand All @@ -50,25 +47,23 @@ class EncryptedSettingsBuilderTest {

@Ignore
@Test
fun givenShouldEncryptDataIsTrue_whenEncryptingData_thenShouldEncryptWithoutFailing() = runTest {
fun givenShouldEncryptDataIsTrue_whenEncryptingData_thenShouldEncryptWithoutFailing() = runTest(coroutineDispatcher) {
val context = ApplicationProvider.getApplicationContext<Context>()
(1..500).map {
testScope.launch {
(1..5000).map {
launch() {
EncryptedSettingsBuilder.build(
options = SettingOptions.UserSettings(
shouldEncryptData = true,
userIDEntity = UserIDEntity(
value = "userValue",
value = "userValue$it",
domain = "domainValue"
)
),
param = EncryptedSettingsPlatformParam(context)
).also {
it["key$it"] = "value$it"
}
)
}
}.joinAll()
testScope.advanceUntilIdle()
advanceUntilIdle()
}

@AfterTest
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package com.wire.kalium.persistence.kmmSettings

import android.content.Context
import android.content.SharedPreferences
import androidx.security.crypto.EncryptedSharedPreferences
import androidx.security.crypto.MasterKey
import com.russhwolf.settings.Settings
Expand All @@ -30,7 +31,10 @@ private fun SettingOptions.keyAlias(): String = when (this) {
}

internal actual object EncryptedSettingsBuilder {
private fun getOrCreateMasterKey(context: Context, keyAlias: String = MasterKey.DEFAULT_MASTER_KEY_ALIAS): MasterKey =
private fun getOrCreateMasterKey(
context: Context,
keyAlias: String
): MasterKey =
MasterKey
.Builder(context, keyAlias)
.setKeyScheme(MasterKey.KeyScheme.AES256_GCM)
Expand All @@ -41,19 +45,35 @@ internal actual object EncryptedSettingsBuilder {
options: SettingOptions,
param: EncryptedSettingsPlatformParam
): Settings = synchronized(this) {
SharedPreferencesSettings(
if (options.shouldEncryptData) {
EncryptedSharedPreferences.create(
param.appContext,
options.fileName,
getOrCreateMasterKey(param.appContext, options.keyAlias()),
EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV,
EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM,
)
} else {
param.appContext.getSharedPreferences(options.fileName, Context.MODE_PRIVATE)
}, false
)
val settings = if (options.shouldEncryptData) {
encryptedSharedPref(options, param, false)
} else {
param.appContext.getSharedPreferences(options.fileName, Context.MODE_PRIVATE)
}
SharedPreferencesSettings(settings, false)
}

private fun encryptedSharedPref(
options: SettingOptions,
param: EncryptedSettingsPlatformParam,
isRetry: Boolean
): SharedPreferences {
@Suppress("TooGenericExceptionCaught")
return try {
val masterKey = getOrCreateMasterKey(param.appContext, options.keyAlias())
EncryptedSharedPreferences.create(
param.appContext,
options.fileName,
masterKey,
EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV,
EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM
)
} catch (e: Exception) {
if (isRetry) {
throw e
}
encryptedSharedPref(options, param, true)
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@

package com.wire.kalium.persistence.util

import kotlinx.serialization.ExperimentalSerializationApi
import kotlinx.serialization.json.Json

internal object JsonSerializer {

@OptIn(ExperimentalSerializationApi::class)
private val instance: Json by lazy {
Json {
encodeDefaults = true
Expand Down

0 comments on commit 3640dff

Please sign in to comment.