Skip to content
This repository has been archived by the owner on Feb 27, 2024. It is now read-only.

Add ability to optionally store credentials for re-auth #239

Merged
merged 3 commits into from
Nov 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,8 @@ class TestBridgeConfig: BridgeConfig {
get() = PlatformConfig.BridgeEnvironment.PRODUCTION
override val osName: String
get() = "AssessmentModel-SDK Test"
override val cacheCredentials: Boolean
get() = false
override val osVersion: String
get() = "AssessmentModel-SDK Test"
override val deviceName: String
Expand Down
2 changes: 2 additions & 0 deletions bridge-client/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ kotlin {
api(libs.koin.core)
// Kermit
implementation(libs.touchlab.kermit)
implementation(libs.multiplatform.settings)
}
}

Expand All @@ -114,6 +115,7 @@ kotlin {
implementation(libs.androidx.work.runtimeKts)
implementation(libs.koin.android)
implementation(libs.koin.android.workmanager)
implementation(libs.androidx.security.crypto.ktx)
}
}
val androidUnitTest by getting {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ class AndroidBridgeConfig(context: Context) : BridgeConfig {
// TODO: emm 2021-08-18 Where should this value come from?
override val bridgeEnvironment: PlatformConfig.BridgeEnvironment = PlatformConfig.BridgeEnvironment.PRODUCTION

override val cacheCredentials: Boolean = applicationContext.resources.getBoolean(R.bool.cache_credentials)

override val osName: String = "Android"

override val osVersion: String = VERSION.RELEASE
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
package org.sagebionetworks.bridge.kmm.shared.di

import androidx.security.crypto.EncryptedSharedPreferences
import androidx.security.crypto.MasterKey
import app.cash.sqldelight.driver.android.AndroidSqliteDriver
import app.cash.sqldelight.db.SqlDriver
import com.russhwolf.settings.Settings
import com.russhwolf.settings.SharedPreferencesSettings
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.SupervisorJob
Expand All @@ -14,6 +18,8 @@ import org.sagebionetworks.bridge.kmm.shared.BridgeConfig
import org.sagebionetworks.bridge.kmm.shared.apis.HttpAndroidUtil
import org.sagebionetworks.bridge.kmm.shared.apis.HttpUtil
import org.sagebionetworks.bridge.kmm.shared.cache.BridgeResourceDatabase
import org.sagebionetworks.bridge.kmm.shared.cache.EncryptedSharedSettings
import org.sagebionetworks.bridge.kmm.shared.cache.EncryptedSharedSettingsImpl
import org.sagebionetworks.bridge.kmm.shared.upload.CoroutineUploadWorker

actual val platformModule: Module = module {
Expand All @@ -29,4 +35,16 @@ actual val platformModule: Module = module {

single<HttpUtil> {HttpAndroidUtil(get())}

single<Settings>(named(EncryptedSharedSettingsImpl.encryptedSettingsName)) {
SharedPreferencesSettings(EncryptedSharedPreferences.create(
get(),
EncryptedSharedSettingsImpl.ENCRYPTED_DATABASE_NAME,
MasterKey.Builder(get())
.setKeyScheme(MasterKey.KeyScheme.AES256_GCM)
.build(),
EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV,
EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM
), false)
}

}
1 change: 1 addition & 0 deletions bridge-client/src/androidMain/res/values/bridge_config.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@
<resources>
<string name="osb_app_id" translatable="false">bridge-client-kmm-integration</string>
<string name="osb_app_name">Bridge Client KMM Integration</string>
<bool name="cache_credentials">false</bool>

</resources>
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
package org.sagebionetworks.bridge.kmm.shared.integration

import org.koin.core.context.startKoin
import org.koin.core.qualifier.named
import org.koin.dsl.module
import org.koin.test.KoinTest
import org.sagebionetworks.bridge.kmm.shared.BaseTest
import org.sagebionetworks.bridge.kmm.shared.BridgeConfig
import org.sagebionetworks.bridge.kmm.shared.PlatformConfig
import org.sagebionetworks.bridge.kmm.shared.TestEncryptedSharedSettings
import org.sagebionetworks.bridge.kmm.shared.apis.HttpUtil
import org.sagebionetworks.bridge.kmm.shared.cache.EncryptedSharedSettings
import org.sagebionetworks.bridge.kmm.shared.cache.ResourceDatabaseHelper
import org.sagebionetworks.bridge.kmm.shared.di.bridgeClientkoinModules
import org.sagebionetworks.bridge.kmm.shared.repo.AuthenticationRepository
import org.sagebionetworks.bridge.kmm.shared.testDatabaseDriver
import kotlin.test.*

Expand All @@ -26,6 +30,16 @@ abstract class AbstractBaseIntegrationTest: BaseTest(), KoinTest {
return "en-US,en"
}
} }
//Need to override for tests since encryptedSharedSettings requires a Context
single<AuthenticationRepository> {
AuthenticationRepository(
authHttpClient = get(named("authHttpClient")),
bridgeConfig = get(),
database = get(),
backgroundScope = get(named("background")),
encryptedSharedSettings = TestEncryptedSharedSettings()
)
}

}
init {
Expand Down Expand Up @@ -58,10 +72,12 @@ abstract class AbstractBaseIntegrationTest: BaseTest(), KoinTest {
get() = PlatformConfig.BridgeEnvironment.PRODUCTION
override val osName: String
get() = "Android Integration Test"
override val cacheCredentials: Boolean
get() = true
override val osVersion: String
get() = "Android Integration Test"
override val deviceName: String
get() = "Android Integration Test"

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,13 @@ package org.sagebionetworks.bridge.kmm.shared.integration

import org.koin.core.component.inject
import org.sagebionetworks.bridge.kmm.shared.cache.ResourceResult
import org.sagebionetworks.bridge.kmm.shared.repo.AuthenticationRepository
import org.sagebionetworks.bridge.kmm.shared.models.UserSessionInfo
import kotlin.test.*
import org.sagebionetworks.bridge.kmm.shared.repo.AuthenticationRepository
import kotlin.test.Test
import kotlin.test.assertFalse
import kotlin.test.assertNotEquals
import kotlin.test.assertNotNull
import kotlin.test.assertTrue

class AuthenticationIntegrationTest: AbstractBaseIntegrationTest() {

Expand All @@ -31,6 +35,12 @@ class AuthenticationIntegrationTest: AbstractBaseIntegrationTest() {
val updatedSession = authRepo.session()
assertNotNull(updatedSession)
assertNotEquals(updatedSession.reauthToken, sessionInfo.reauthToken)
// Clear session token and reauth token
authRepo.updateCachedSession(null, updatedSession.copy(reauthToken = "invalid-token", sessionToken = "", authenticated = false))
//Test reauth using cached credentials
assertTrue(authRepo.reAuth())
assertTrue(authRepo.isAuthenticated())

//Test signout
authRepo.signOut()
assertFalse(authRepo.isAuthenticated())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ interface PlatformConfig {
val deviceName: String

val osName: String

val cacheCredentials: Boolean
}

fun PlatformConfig.buildClientData(input: JsonElement? = null, uploadId: String? = null): JsonElement {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package org.sagebionetworks.bridge.kmm.shared.cache

import com.russhwolf.settings.Settings
import com.russhwolf.settings.get
import com.russhwolf.settings.set

interface EncryptedSharedSettings {
var pwd: String?
}

class EncryptedSharedSettingsImpl (
private val encryptedSettings: Settings,
) : EncryptedSharedSettings {

override var pwd: String?
get() {
return encryptedSettings[PWD_KEY]
}
set(value) {
encryptedSettings[PWD_KEY] = value
}

companion object {
const val ENCRYPTED_DATABASE_NAME = "ENCRYPTED_SETTINGS"
const val encryptedSettingsName = "encryptedSettings"
// Keys of values stored in settings
private const val PWD_KEY = "PWD_KEY"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import org.koin.core.module.Module
import org.koin.core.qualifier.named
import org.koin.dsl.KoinAppDeclaration
import org.koin.dsl.module
import org.sagebionetworks.bridge.kmm.shared.cache.EncryptedSharedSettings
import org.sagebionetworks.bridge.kmm.shared.cache.EncryptedSharedSettingsImpl
import org.sagebionetworks.bridge.kmm.shared.cache.LocalJsonDataCache
import org.sagebionetworks.bridge.kmm.shared.cache.ResourceDatabaseHelper
import org.sagebionetworks.bridge.kmm.shared.repo.*
Expand All @@ -25,6 +27,8 @@ fun bridgeClientkoinModules(enableNetworkLogs: Boolean): List<Module> {
val commonModule = module {
single {ResourceDatabaseHelper(get())}

single<EncryptedSharedSettings> {EncryptedSharedSettingsImpl(get(named(EncryptedSharedSettingsImpl.encryptedSettingsName)))}

single<AssessmentConfigRepo> {AssessmentConfigRepo(get(), get(), get(named("background"))) }
single<ScheduleTimelineRepo> {ScheduleTimelineRepo(get(), get(), get(), get(), get(named("background")), null) }
single<ActivityEventsRepo> { ActivityEventsRepo(get(), get(), get(named("background")), get()) }
Expand All @@ -34,7 +38,8 @@ val commonModule = module {
authHttpClient = get(named("authHttpClient")),
bridgeConfig = get(),
database = get(),
backgroundScope = get(named("background"))
backgroundScope = get(named("background")),
encryptedSharedSettings = get()
)
}
single<AppConfigRepo> { AppConfigRepo(get(), get(), get(named("background")), get()) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ class AuthenticationRepository(
authHttpClient: HttpClient,
val bridgeConfig: BridgeConfig,
val database: ResourceDatabaseHelper,
private val backgroundScope: CoroutineScope
private val backgroundScope: CoroutineScope,
private val encryptedSharedSettings: EncryptedSharedSettings
) : KoinComponent, AuthenticationProvider {

internal companion object {
Expand Down Expand Up @@ -111,6 +112,7 @@ class AuthenticationRepository(
}
// Always clear database when signOut is called
database.clearDatabase()
clearCredentials()
}

/**
Expand Down Expand Up @@ -192,6 +194,21 @@ class AuthenticationRepository(
return signIn(signIn)
}

private fun storeCredentials(password: String) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're only storing/retrieving the password and not the account-identifying credential (id, email, etc)? What am I missing here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reauthWithCredentials method uses the currently cached user session to get email or externalID.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reauthWithCredentials method is an existing method intended to restore the session token without striping the participant's data. In other words, it will only attempt to reauthenticate against the current email/externalId.

if (bridgeConfig.cacheCredentials) {
encryptedSharedSettings.pwd = password
}
}

private fun clearCredentials() {
encryptedSharedSettings.pwd = null
}

private fun getCredentials() : String? {
return encryptedSharedSettings.pwd
}


suspend fun reauthWithCredentials(password: String) : ResourceResult<UserSessionInfo> {
val sessionInfo = session() ?: return ResourceResult.Failed(ResourceStatus.FAILED)
val signIn = SignIn(
Expand All @@ -207,6 +224,7 @@ class AuthenticationRepository(
} catch (err: Throwable) {
Logger.e("Error requesting reAuth with stored password: $err")
if (err is ResponseException && err.response.status == HttpStatusCode.NotFound) {
clearCredentials()
syoung-smallwisdom marked this conversation as resolved.
Show resolved Hide resolved
ResourceResult.Failed(ResourceStatus.FAILED)
} else {
ResourceResult.Failed(ResourceStatus.RETRY)
Expand All @@ -217,6 +235,9 @@ class AuthenticationRepository(
private suspend fun signIn(signIn: SignIn) : ResourceResult<UserSessionInfo> {
try {
val userSession = authenticationApi.signIn(signIn)
if (signIn.password != null && bridgeConfig.cacheCredentials) {
storeCredentials(signIn.password)
}
updateCachedSession(null, userSession)
return ResourceResult.Success(userSession, ResourceStatus.SUCCESS)
} catch (err: Throwable) {
Expand Down Expand Up @@ -290,6 +311,13 @@ class AuthenticationRepository(
sessionToken = ""
)
updateCachedSession(null, newSession)
val pwd = getCredentials()
if (bridgeConfig.cacheCredentials && pwd != null) {
val result = reauthWithCredentials(pwd)
if (result is ResourceResult.Success) {
success = true
}
}
} else {
// Some sort of network error leave the session alone so we can try again
Logger.i("User reauth failed. Ignoring. $err")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import kotlinx.serialization.json.Json
import org.sagebionetworks.bridge.kmm.shared.apis.RefreshTokenFeature
import org.sagebionetworks.bridge.kmm.shared.apis.SessionTokenFeature
import org.sagebionetworks.bridge.kmm.shared.apis.HttpUtil
import org.sagebionetworks.bridge.kmm.shared.cache.EncryptedSharedSettings
import org.sagebionetworks.bridge.kmm.shared.cache.ResourceDatabaseHelper
import org.sagebionetworks.bridge.kmm.shared.di.appendAuthConfig
import org.sagebionetworks.bridge.kmm.shared.di.appendDefaultConfig
Expand Down Expand Up @@ -98,3 +99,9 @@ fun getJsonReponseHandler(json: String, responseCode: HttpStatusCode = HttpStatu
respond(json, responseCode, headersOf("Content-Type" to listOf(ContentType.Application.Json.toString())))
}
}

class TestEncryptedSharedSettings : EncryptedSharedSettings {

override var pwd: String? = null

}
Loading
Loading