Merge pull request #1520 from vector-im/feature/bma/sessionDb

Improve session db and improve deleted session behavior
This commit is contained in:
Benoit Marty 2023-10-11 16:56:54 +02:00 committed by GitHub
commit 88ca37984f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
43 changed files with 968 additions and 49 deletions

View file

@ -18,12 +18,18 @@ package io.element.android.libraries.matrix.api.auth
import io.element.android.libraries.matrix.api.MatrixClient
import io.element.android.libraries.matrix.api.core.SessionId
import io.element.android.libraries.sessionstorage.api.LoggedInState
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.StateFlow
interface MatrixAuthenticationService {
fun isLoggedIn(): Flow<Boolean>
fun loggedInStateFlow(): Flow<LoggedInState>
suspend fun getLatestSessionId(): SessionId?
/**
* Restore a session from a [sessionId].
* Do not restore anything it the access token is not valid anymore.
*/
suspend fun restoreSession(sessionId: SessionId): Result<MatrixClient>
fun getHomeserverDetails(): StateFlow<MatrixHomeServerDetails?>
suspend fun setHomeserver(homeserver: String): Result<Unit>

View file

@ -126,7 +126,16 @@ class RustMatrixClient constructor(
Timber.v("didReceiveAuthError -> do the cleanup")
//TODO handle isSoftLogout parameter.
appCoroutineScope.launch {
doLogout(doRequest = false)
val existingData = sessionStore.getSession(client.userId())
if (existingData != null) {
// Set isTokenValid to false
val newData = client.session().toSessionData(
isTokenValid = false,
loginType = existingData.loginType,
)
sessionStore.updateData(newData)
}
doLogout(doRequest = false, removeSession = false)
}
} else {
Timber.v("didReceiveAuthError -> already cleaning up")
@ -136,7 +145,12 @@ class RustMatrixClient constructor(
override fun didRefreshTokens() {
Timber.w("didRefreshTokens()")
appCoroutineScope.launch {
sessionStore.updateData(client.session().toSessionData())
val existingData = sessionStore.getSession(client.userId()) ?: return@launch
val newData = client.session().toSessionData(
isTokenValid = existingData.isTokenValid,
loginType = existingData.loginType,
)
sessionStore.updateData(newData)
}
}
}
@ -328,9 +342,9 @@ class RustMatrixClient constructor(
baseDirectory.deleteSessionDirectory(userID = sessionId.value, deleteCryptoDb = false)
}
override suspend fun logout(): String? = doLogout(doRequest = true)
override suspend fun logout(): String? = doLogout(doRequest = true, removeSession = true)
private suspend fun doLogout(doRequest: Boolean): String? {
private suspend fun doLogout(doRequest: Boolean, removeSession: Boolean): String? {
var result: String? = null
withContext(sessionDispatcher) {
if (doRequest) {
@ -342,7 +356,9 @@ class RustMatrixClient constructor(
}
close()
baseDirectory.deleteSessionDirectory(userID = sessionId.value, deleteCryptoDb = true)
sessionStore.removeSession(sessionId.value)
if (removeSession) {
sessionStore.removeSession(sessionId.value)
}
}
return result
}

View file

@ -30,6 +30,8 @@ import io.element.android.libraries.matrix.impl.RustMatrixClientFactory
import io.element.android.libraries.matrix.impl.exception.mapClientException
import io.element.android.libraries.matrix.impl.mapper.toSessionData
import io.element.android.libraries.network.useragent.UserAgentProvider
import io.element.android.libraries.sessionstorage.api.LoggedInState
import io.element.android.libraries.sessionstorage.api.LoginType
import io.element.android.libraries.sessionstorage.api.SessionStore
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
@ -62,7 +64,7 @@ class RustMatrixAuthenticationService @Inject constructor(
)
private var currentHomeserver = MutableStateFlow<MatrixHomeServerDetails?>(null)
override fun isLoggedIn(): Flow<Boolean> {
override fun loggedInStateFlow(): Flow<LoggedInState> {
return sessionStore.isLoggedIn()
}
@ -74,7 +76,11 @@ class RustMatrixAuthenticationService @Inject constructor(
runCatching {
val sessionData = sessionStore.getSession(sessionId.value)
if (sessionData != null) {
rustMatrixClientFactory.create(sessionData)
if (sessionData.isTokenValid) {
rustMatrixClientFactory.create(sessionData)
} else {
error("Token is not valid")
}
} else {
error("No session to restore with id $sessionId")
}
@ -102,7 +108,12 @@ class RustMatrixAuthenticationService @Inject constructor(
withContext(coroutineDispatchers.io) {
runCatching {
val client = authService.login(username, password, "Element X Android", null)
val sessionData = client.use { it.session().toSessionData() }
val sessionData = client.use {
it.session().toSessionData(
isTokenValid = true,
loginType = LoginType.PASSWORD,
)
}
sessionStore.storeData(sessionData)
SessionId(sessionData.userId)
}.mapFailure { failure ->
@ -144,7 +155,12 @@ class RustMatrixAuthenticationService @Inject constructor(
runCatching {
val urlForOidcLogin = pendingOidcAuthenticationData ?: error("You need to call `getOidcUrl()` first")
val client = authService.loginWithOidcCallback(urlForOidcLogin, callbackUrl)
val sessionData = client.use { it.session().toSessionData() }
val sessionData = client.use {
it.session().toSessionData(
isTokenValid = true,
loginType = LoginType.OIDC,
)
}
pendingOidcAuthenticationData?.close()
pendingOidcAuthenticationData = null
sessionStore.storeData(sessionData)

View file

@ -16,11 +16,15 @@
package io.element.android.libraries.matrix.impl.mapper
import io.element.android.libraries.sessionstorage.api.LoginType
import io.element.android.libraries.sessionstorage.api.SessionData
import org.matrix.rustcomponents.sdk.Session
import java.util.Date
internal fun Session.toSessionData() = SessionData(
internal fun Session.toSessionData(
isTokenValid: Boolean,
loginType: LoginType,
) = SessionData(
userId = userId,
deviceId = deviceId,
accessToken = accessToken,
@ -29,4 +33,6 @@ internal fun Session.toSessionData() = SessionData(
oidcData = oidcData,
slidingSyncProxy = slidingSyncProxy,
loginTimestamp = Date(),
isTokenValid = isTokenValid,
loginType = loginType,
)

View file

@ -22,6 +22,7 @@ import io.element.android.libraries.matrix.api.auth.MatrixHomeServerDetails
import io.element.android.libraries.matrix.api.auth.OidcDetails
import io.element.android.libraries.matrix.api.core.SessionId
import io.element.android.libraries.matrix.test.A_USER_ID
import io.element.android.libraries.sessionstorage.api.LoggedInState
import io.element.android.tests.testutils.simulateLongTask
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow
@ -38,8 +39,8 @@ class FakeAuthenticationService : MatrixAuthenticationService {
private var changeServerError: Throwable? = null
private var matrixClient: MatrixClient? = null
override fun isLoggedIn(): Flow<Boolean> {
return flowOf(false)
override fun loggedInStateFlow(): Flow<LoggedInState> {
return flowOf(LoggedInState.NotLoggedIn)
}
override suspend fun getLatestSessionId(): SessionId? {

View file

@ -0,0 +1,25 @@
/*
* Copyright (c) 2023 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.element.android.libraries.sessionstorage.api
sealed interface LoggedInState {
data object NotLoggedIn : LoggedInState
data class LoggedIn(
val sessionId: String,
val isTokenValid: Boolean,
) : LoggedInState
}

View file

@ -0,0 +1,43 @@
/*
* Copyright (c) 2023 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.element.android.libraries.sessionstorage.api
// Imported from Element Android, to be able to migrate from EA to EXA.
enum class LoginType {
PASSWORD,
OIDC,
SSO,
UNSUPPORTED,
CUSTOM,
DIRECT,
UNKNOWN,
QR;
companion object {
fun fromName(name: String) = when (name) {
PASSWORD.name -> PASSWORD
OIDC.name -> OIDC
SSO.name -> SSO
UNSUPPORTED.name -> UNSUPPORTED
CUSTOM.name -> CUSTOM
DIRECT.name -> DIRECT
QR.name -> QR
else -> UNKNOWN
}
}
}

View file

@ -27,4 +27,6 @@ data class SessionData(
val oidcData: String?,
val slidingSyncProxy: String?,
val loginTimestamp: Date?,
val isTokenValid: Boolean,
val loginType: LoginType,
)

View file

@ -20,7 +20,7 @@ import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.map
interface SessionStore {
fun isLoggedIn(): Flow<Boolean>
fun isLoggedIn(): Flow<LoggedInState>
fun sessionsFlow(): Flow<List<SessionData>>
suspend fun storeData(sessionData: SessionData)

View file

@ -16,6 +16,7 @@
package io.element.android.libraries.sessionstorage.impl.memory
import io.element.android.libraries.sessionstorage.api.LoggedInState
import io.element.android.libraries.sessionstorage.api.SessionData
import io.element.android.libraries.sessionstorage.api.SessionStore
import kotlinx.coroutines.flow.Flow
@ -26,8 +27,17 @@ class InMemorySessionStore : SessionStore {
private var sessionDataFlow = MutableStateFlow<SessionData?>(null)
override fun isLoggedIn(): Flow<Boolean> {
return sessionDataFlow.map { it != null }
override fun isLoggedIn(): Flow<LoggedInState> {
return sessionDataFlow.map {
if (it == null) {
LoggedInState.NotLoggedIn
} else {
LoggedInState.LoggedIn(
sessionId = it.userId,
isTokenValid = it.isTokenValid,
)
}
}
}
override fun sessionsFlow(): Flow<List<SessionData>> {

View file

@ -45,10 +45,18 @@ dependencies {
testImplementation(libs.test.turbine)
testImplementation(libs.coroutines.test)
testImplementation(libs.sqldelight.driver.jvm)
coreLibraryDesugaring(libs.android.desugar)
}
sqldelight {
database("SessionDatabase") {
// https://cashapp.github.io/sqldelight/1.5.4/multiplatform_sqlite/migrations/
// To generate a .db file from your latest schema, run this task
// ./gradlew generateDebugSessionDatabaseSchema
// Test migration by running
// ./gradlew verifySqlDelightMigration
schemaOutputDirectory = File("src/main/sqldelight/databases")
verifyMigrations = true
}
}

View file

@ -22,6 +22,7 @@ import com.squareup.sqldelight.runtime.coroutines.mapToList
import com.squareup.sqldelight.runtime.coroutines.mapToOneOrNull
import io.element.android.libraries.di.AppScope
import io.element.android.libraries.di.SingleIn
import io.element.android.libraries.sessionstorage.api.LoggedInState
import io.element.android.libraries.sessionstorage.api.SessionData
import io.element.android.libraries.sessionstorage.api.SessionStore
import kotlinx.coroutines.flow.Flow
@ -35,11 +36,20 @@ class DatabaseSessionStore @Inject constructor(
private val database: SessionDatabase,
) : SessionStore {
override fun isLoggedIn(): Flow<Boolean> {
override fun isLoggedIn(): Flow<LoggedInState> {
return database.sessionDataQueries.selectFirst()
.asFlow()
.mapToOneOrNull()
.map { it != null }
.map {
if (it == null) {
LoggedInState.NotLoggedIn
} else {
LoggedInState.LoggedIn(
sessionId = it.userId,
isTokenValid = it.isTokenValid == 1L
)
}
}
}
override suspend fun storeData(sessionData: SessionData) {

View file

@ -16,6 +16,7 @@
package io.element.android.libraries.sessionstorage.impl
import io.element.android.libraries.sessionstorage.api.LoginType
import io.element.android.libraries.sessionstorage.api.SessionData
import java.util.Date
import io.element.android.libraries.matrix.session.SessionData as DbSessionData
@ -30,6 +31,8 @@ internal fun SessionData.toDbModel(): DbSessionData {
oidcData = oidcData,
slidingSyncProxy = slidingSyncProxy,
loginTimestamp = loginTimestamp?.time,
isTokenValid = if (isTokenValid) 1L else 0L,
loginType = loginType.name,
)
}
@ -42,6 +45,8 @@ internal fun DbSessionData.toApiModel(): SessionData {
homeserverUrl = homeserverUrl,
oidcData = oidcData,
slidingSyncProxy = slidingSyncProxy,
loginTimestamp = loginTimestamp?.let { Date(it) }
loginTimestamp = loginTimestamp?.let { Date(it) },
isTokenValid = isTokenValid == 1L,
loginType = LoginType.fromName(loginType ?: LoginType.UNKNOWN.name),
)
}

View file

@ -6,7 +6,9 @@ CREATE TABLE SessionData (
homeserverUrl TEXT NOT NULL,
slidingSyncProxy TEXT,
loginTimestamp INTEGER,
oidcData TEXT
oidcData TEXT,
isTokenValid INTEGER NOT NULL,
loginType TEXT
);

View file

@ -0,0 +1,2 @@
ALTER TABLE SessionData ADD COLUMN isTokenValid INTEGER NOT NULL DEFAULT 1;
ALTER TABLE SessionData ADD COLUMN loginType TEXT;

View file

@ -20,6 +20,8 @@ import app.cash.turbine.test
import com.google.common.truth.Truth.assertThat
import com.squareup.sqldelight.sqlite.driver.JdbcSqliteDriver
import io.element.android.libraries.matrix.session.SessionData
import io.element.android.libraries.sessionstorage.api.LoggedInState
import io.element.android.libraries.sessionstorage.api.LoginType
import kotlinx.coroutines.test.runTest
import org.junit.Before
import org.junit.Test
@ -38,6 +40,8 @@ class DatabaseSessionStoreTests {
slidingSyncProxy = null,
loginTimestamp = null,
oidcData = "aOidcData",
isTokenValid = 1,
loginType = LoginType.UNKNOWN.name,
)
@Before
@ -63,11 +67,11 @@ class DatabaseSessionStoreTests {
@Test
fun `isLoggedIn emits true while there are sessions in the DB`() = runTest {
databaseSessionStore.isLoggedIn().test {
assertThat(awaitItem()).isFalse()
assertThat(awaitItem()).isEqualTo(LoggedInState.NotLoggedIn)
database.sessionDataQueries.insertSessionData(aSessionData)
assertThat(awaitItem()).isTrue()
assertThat(awaitItem()).isEqualTo(LoggedInState.LoggedIn(sessionId = aSessionData.userId, isTokenValid = true))
database.sessionDataQueries.removeSession(aSessionData.userId)
assertThat(awaitItem()).isFalse()
assertThat(awaitItem()).isEqualTo(LoggedInState.NotLoggedIn)
}
}
@ -121,6 +125,8 @@ class DatabaseSessionStoreTests {
slidingSyncProxy = "slidingSyncProxy",
loginTimestamp = 1,
oidcData = "aOidcData",
isTokenValid = 1,
loginType = null,
)
val secondSessionData = SessionData(
userId = "userId",
@ -131,6 +137,8 @@ class DatabaseSessionStoreTests {
slidingSyncProxy = "slidingSyncProxyAltered",
loginTimestamp = 2,
oidcData = "aOidcDataAltered",
isTokenValid = 1,
loginType = null,
)
assertThat(firstSessionData.userId).isEqualTo(secondSessionData.userId)
assertThat(firstSessionData.loginTimestamp).isNotEqualTo(secondSessionData.loginTimestamp)

View file

@ -64,6 +64,7 @@
<string name="action_send_message">"Send message"</string>
<string name="action_share">"Share"</string>
<string name="action_share_link">"Share link"</string>
<string name="action_sign_in_again">"Sign in again"</string>
<string name="action_skip">"Skip"</string>
<string name="action_start">"Start"</string>
<string name="action_start_chat">"Start chat"</string>