Skip to content

Commit

Permalink
Merge pull request #5398 from vector-im/bugfix/eric/softlogout-ux-broken
Browse files Browse the repository at this point in the history
Fixes broken SoftLogout UX for homeservers that support both Password and SSO
  • Loading branch information
ericdecanini authored Jul 1, 2022
2 parents d5b375e + 4cf97d4 commit bdb49f5
Show file tree
Hide file tree
Showing 37 changed files with 1,058 additions and 150 deletions.
1 change: 1 addition & 0 deletions changelog.d/5398.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Adds LoginType to SessionParams to fix soft logout form not showing for SSO and Password type
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Copyright (c) 2022 The Matrix.org Foundation C.I.C.
*
* 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 org.matrix.android.sdk.api.auth

enum class LoginType {
PASSWORD,
SSO,
UNSUPPORTED,
CUSTOM,
DIRECT,
UNKNOWN;

companion object {

fun fromName(name: String) = when (name) {
PASSWORD.name -> PASSWORD
SSO.name -> SSO
UNSUPPORTED.name -> UNSUPPORTED
CUSTOM.name -> CUSTOM
DIRECT.name -> DIRECT
else -> UNKNOWN
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package org.matrix.android.sdk.api.auth.data

import org.matrix.android.sdk.api.auth.LoginType

/**
* This data class holds necessary data to open a session.
* You don't have to manually instantiate it.
Expand All @@ -34,7 +36,12 @@ data class SessionParams(
/**
* Set to false if the current token is not valid anymore. Application should not have to use this info.
*/
val isTokenValid: Boolean
val isTokenValid: Boolean,

/**
* The authentication method that was used to create the session.
*/
val loginType: LoginType,
) {
/*
* Shortcuts. Usually the application should only need to use these shortcuts
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ internal abstract class AuthModule {
@Binds
abstract fun bindSessionCreator(creator: DefaultSessionCreator): SessionCreator

@Binds
abstract fun bindSessionParamsCreator(creator: DefaultSessionParamsCreator): SessionParamsCreator

@Binds
abstract fun bindDirectLoginTask(task: DefaultDirectLoginTask): DirectLoginTask

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import okhttp3.OkHttpClient
import org.matrix.android.sdk.api.MatrixPatterns
import org.matrix.android.sdk.api.MatrixPatterns.getServerName
import org.matrix.android.sdk.api.auth.AuthenticationService
import org.matrix.android.sdk.api.auth.LoginType
import org.matrix.android.sdk.api.auth.data.Credentials
import org.matrix.android.sdk.api.auth.data.HomeServerConnectionConfig
import org.matrix.android.sdk.api.auth.data.LoginFlowResult
Expand Down Expand Up @@ -361,7 +362,7 @@ internal class DefaultAuthenticationService @Inject constructor(
homeServerConnectionConfig: HomeServerConnectionConfig,
credentials: Credentials
): Session {
return sessionCreator.createSession(credentials, homeServerConnectionConfig)
return sessionCreator.createSession(credentials, homeServerConnectionConfig, LoginType.SSO)
}

override suspend fun getWellKnownData(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,69 +16,41 @@

package org.matrix.android.sdk.internal.auth

import android.net.Uri
import org.matrix.android.sdk.api.auth.LoginType
import org.matrix.android.sdk.api.auth.data.Credentials
import org.matrix.android.sdk.api.auth.data.HomeServerConnectionConfig
import org.matrix.android.sdk.api.auth.data.SessionParams
import org.matrix.android.sdk.api.extensions.tryOrNull
import org.matrix.android.sdk.api.session.Session
import org.matrix.android.sdk.internal.SessionManager
import timber.log.Timber
import javax.inject.Inject

internal interface SessionCreator {
suspend fun createSession(credentials: Credentials, homeServerConnectionConfig: HomeServerConnectionConfig): Session

suspend fun createSession(
credentials: Credentials,
homeServerConnectionConfig: HomeServerConnectionConfig,
loginType: LoginType,
): Session
}

internal class DefaultSessionCreator @Inject constructor(
private val sessionParamsStore: SessionParamsStore,
private val sessionManager: SessionManager,
private val pendingSessionStore: PendingSessionStore,
private val isValidClientServerApiTask: IsValidClientServerApiTask
private val sessionParamsCreator: SessionParamsCreator,
) : SessionCreator {

/**
* Credentials can affect the homeServerConnectionConfig, override homeserver url and/or
* identity server url if provided in the credentials.
*/
override suspend fun createSession(credentials: Credentials, homeServerConnectionConfig: HomeServerConnectionConfig): Session {
override suspend fun createSession(
credentials: Credentials,
homeServerConnectionConfig: HomeServerConnectionConfig,
loginType: LoginType,
): Session {
// We can cleanup the pending session params
pendingSessionStore.delete()

val overriddenUrl = credentials.discoveryInformation?.homeServer?.baseURL
// remove trailing "/"
?.trim { it == '/' }
?.takeIf { it.isNotBlank() }
// It can be the same value, so in this case, do not check again the validity
?.takeIf { it != homeServerConnectionConfig.homeServerUriBase.toString() }
?.also { Timber.d("Overriding homeserver url to $it (will check if valid)") }
?.let { Uri.parse(it) }
?.takeIf {
// Validate the URL, if the configuration is wrong server side, do not override
tryOrNull {
isValidClientServerApiTask.execute(
IsValidClientServerApiTask.Params(
homeServerConnectionConfig.copy(homeServerUriBase = it)
)
)
.also { Timber.d("Overriding homeserver url: $it") }
} ?: true // In case of other error (no network, etc.), consider it is valid...
}

val sessionParams = SessionParams(
credentials = credentials,
homeServerConnectionConfig = homeServerConnectionConfig.copy(
homeServerUriBase = overriddenUrl ?: homeServerConnectionConfig.homeServerUriBase,
identityServerUri = credentials.discoveryInformation?.identityServer?.baseURL
// remove trailing "/"
?.trim { it == '/' }
?.takeIf { it.isNotBlank() }
?.also { Timber.d("Overriding identity server url to $it") }
?.let { Uri.parse(it) }
?: homeServerConnectionConfig.identityServerUri
),
isTokenValid = true)

val sessionParams = sessionParamsCreator.create(credentials, homeServerConnectionConfig, loginType)
sessionParamsStore.save(sessionParams)
return sessionManager.getOrCreateSession(sessionParams)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/*
* Copyright (c) 2022 The Matrix.org Foundation C.I.C.
*
* 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 org.matrix.android.sdk.internal.auth

import android.net.Uri
import org.matrix.android.sdk.api.auth.LoginType
import org.matrix.android.sdk.api.auth.data.Credentials
import org.matrix.android.sdk.api.auth.data.HomeServerConnectionConfig
import org.matrix.android.sdk.api.auth.data.SessionParams
import org.matrix.android.sdk.api.extensions.tryOrNull
import timber.log.Timber
import javax.inject.Inject

internal interface SessionParamsCreator {

suspend fun create(
credentials: Credentials,
homeServerConnectionConfig: HomeServerConnectionConfig,
loginType: LoginType,
): SessionParams
}

internal class DefaultSessionParamsCreator @Inject constructor(
private val isValidClientServerApiTask: IsValidClientServerApiTask
) : SessionParamsCreator {

override suspend fun create(
credentials: Credentials,
homeServerConnectionConfig: HomeServerConnectionConfig,
loginType: LoginType,
) = SessionParams(
credentials = credentials,
homeServerConnectionConfig = homeServerConnectionConfig.overrideWithCredentials(credentials),
isTokenValid = true,
loginType = loginType,
)

private suspend fun HomeServerConnectionConfig.overrideWithCredentials(credentials: Credentials) = copy(
homeServerUriBase = credentials.getHomeServerUri(this) ?: homeServerUriBase,
identityServerUri = credentials.getIdentityServerUri() ?: identityServerUri
)

private suspend fun Credentials.getHomeServerUri(homeServerConnectionConfig: HomeServerConnectionConfig) =
discoveryInformation?.homeServer?.baseURL
?.trim { it == '/' }
?.takeIf { it.isNotBlank() }
// It can be the same value, so in this case, do not check again the validity
?.takeIf { it != homeServerConnectionConfig.homeServerUriBase.toString() }
?.also { Timber.d("Overriding homeserver url to $it (will check if valid)") }
?.let { Uri.parse(it) }
?.takeIf { validateUri(it, homeServerConnectionConfig) }

private suspend fun validateUri(uri: Uri, homeServerConnectionConfig: HomeServerConnectionConfig) =
// Validate the URL, if the configuration is wrong server side, do not override
tryOrNull {
performClientServerApiValidation(uri, homeServerConnectionConfig)
} ?: true // In case of other error (no network, etc.), consider it is valid...

private suspend fun performClientServerApiValidation(uri: Uri, homeServerConnectionConfig: HomeServerConnectionConfig) =
isValidClientServerApiTask.execute(
IsValidClientServerApiTask.Params(homeServerConnectionConfig.copy(homeServerUriBase = uri))
).also { Timber.d("Overriding homeserver url: $it") }

private fun Credentials.getIdentityServerUri() = discoveryInformation?.identityServer?.baseURL
?.trim { it == '/' }
?.takeIf { it.isNotBlank() }
?.also { Timber.d("Overriding identity server url to $it") }
?.let { Uri.parse(it) }
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import org.matrix.android.sdk.internal.auth.db.migration.MigrateAuthTo001
import org.matrix.android.sdk.internal.auth.db.migration.MigrateAuthTo002
import org.matrix.android.sdk.internal.auth.db.migration.MigrateAuthTo003
import org.matrix.android.sdk.internal.auth.db.migration.MigrateAuthTo004
import org.matrix.android.sdk.internal.auth.db.migration.MigrateAuthTo005
import timber.log.Timber
import javax.inject.Inject

Expand All @@ -33,7 +34,7 @@ internal class AuthRealmMigration @Inject constructor() : RealmMigration {
override fun equals(other: Any?) = other is AuthRealmMigration
override fun hashCode() = 4000

val schemaVersion = 4L
val schemaVersion = 5L

override fun migrate(realm: DynamicRealm, oldVersion: Long, newVersion: Long) {
Timber.d("Migrating Auth Realm from $oldVersion to $newVersion")
Expand All @@ -42,5 +43,6 @@ internal class AuthRealmMigration @Inject constructor() : RealmMigration {
if (oldVersion < 2) MigrateAuthTo002(realm).perform()
if (oldVersion < 3) MigrateAuthTo003(realm).perform()
if (oldVersion < 4) MigrateAuthTo004(realm).perform()
if (oldVersion < 5) MigrateAuthTo005(realm).perform()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,6 @@ internal open class SessionParamsEntity(
var homeServerConnectionConfigJson: String = "",
// Set to false when the token is invalid and the user has been soft logged out
// In case of hard logout, this object is deleted from DB
var isTokenValid: Boolean = true
var isTokenValid: Boolean = true,
var loginType: String = "",
) : RealmObject()
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.matrix.android.sdk.internal.auth.db

import com.squareup.moshi.Moshi
import org.matrix.android.sdk.api.auth.LoginType
import org.matrix.android.sdk.api.auth.data.Credentials
import org.matrix.android.sdk.api.auth.data.HomeServerConnectionConfig
import org.matrix.android.sdk.api.auth.data.SessionParams
Expand All @@ -37,7 +38,7 @@ internal class SessionParamsMapper @Inject constructor(moshi: Moshi) {
if (credentials == null || homeServerConnectionConfig == null) {
return null
}
return SessionParams(credentials, homeServerConnectionConfig, entity.isTokenValid)
return SessionParams(credentials, homeServerConnectionConfig, entity.isTokenValid, LoginType.fromName(entity.loginType))
}

fun map(sessionParams: SessionParams?): SessionParamsEntity? {
Expand All @@ -54,7 +55,8 @@ internal class SessionParamsMapper @Inject constructor(moshi: Moshi) {
sessionParams.userId,
credentialsJson,
homeServerConnectionConfigJson,
sessionParams.isTokenValid
sessionParams.isTokenValid,
sessionParams.loginType.name,
)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* Copyright (c) 2022 The Matrix.org Foundation C.I.C.
*
* 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 org.matrix.android.sdk.internal.auth.db.migration

import io.realm.DynamicRealm
import org.matrix.android.sdk.api.auth.LoginType
import org.matrix.android.sdk.internal.auth.db.SessionParamsEntityFields
import org.matrix.android.sdk.internal.util.database.RealmMigrator
import timber.log.Timber

internal class MigrateAuthTo005(realm: DynamicRealm) : RealmMigrator(realm, 5) {

override fun doMigrate(realm: DynamicRealm) {
Timber.d("Update SessionParamsEntity to add LoginType")

realm.schema.get("SessionParamsEntity")
?.addField(SessionParamsEntityFields.LOGIN_TYPE, String::class.java)
?.setRequired(SessionParamsEntityFields.LOGIN_TYPE, true)
?.transform { it.set(SessionParamsEntityFields.LOGIN_TYPE, LoginType.UNKNOWN.name) }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.matrix.android.sdk.internal.auth.login

import android.util.Patterns
import org.matrix.android.sdk.api.auth.LoginType
import org.matrix.android.sdk.api.auth.login.LoginProfileInfo
import org.matrix.android.sdk.api.auth.login.LoginWizard
import org.matrix.android.sdk.api.auth.registration.RegisterThreePid
Expand Down Expand Up @@ -78,7 +79,7 @@ internal class DefaultLoginWizard(
authAPI.login(loginParams)
}

return sessionCreator.createSession(credentials, pendingSessionData.homeServerConnectionConfig)
return sessionCreator.createSession(credentials, pendingSessionData.homeServerConnectionConfig, LoginType.PASSWORD)
}

/**
Expand All @@ -92,15 +93,15 @@ internal class DefaultLoginWizard(
authAPI.login(loginParams)
}

return sessionCreator.createSession(credentials, pendingSessionData.homeServerConnectionConfig)
return sessionCreator.createSession(credentials, pendingSessionData.homeServerConnectionConfig, LoginType.SSO)
}

override suspend fun loginCustom(data: JsonDict): Session {
val credentials = executeRequest(null) {
authAPI.login(data)
}

return sessionCreator.createSession(credentials, pendingSessionData.homeServerConnectionConfig)
return sessionCreator.createSession(credentials, pendingSessionData.homeServerConnectionConfig, LoginType.CUSTOM)
}

override suspend fun resetPassword(email: String) {
Expand Down
Loading

0 comments on commit bdb49f5

Please sign in to comment.