mirror of
https://github.com/Novaloop-AG/element-x-android.git
synced 2026-11-03 06:55:41 +00:00
Configure the account provider homeserver only once when signing in
This commit is contained in:
+5
-2
@@ -22,6 +22,7 @@ import io.element.android.libraries.architecture.AsyncData
|
|||||||
import io.element.android.libraries.architecture.Presenter
|
import io.element.android.libraries.architecture.Presenter
|
||||||
import io.element.android.libraries.architecture.runCatchingUpdatingState
|
import io.element.android.libraries.architecture.runCatchingUpdatingState
|
||||||
import io.element.android.libraries.matrix.api.auth.MatrixAuthenticationService
|
import io.element.android.libraries.matrix.api.auth.MatrixAuthenticationService
|
||||||
|
import io.element.android.libraries.matrix.api.auth.MatrixHomeServerDetails
|
||||||
|
|
||||||
@Inject
|
@Inject
|
||||||
class ChangeServerPresenter(
|
class ChangeServerPresenter(
|
||||||
@@ -32,7 +33,7 @@ class ChangeServerPresenter(
|
|||||||
) : Presenter<ChangeServerState> {
|
) : Presenter<ChangeServerState> {
|
||||||
@Composable
|
@Composable
|
||||||
override fun present(): ChangeServerState {
|
override fun present(): ChangeServerState {
|
||||||
val changeServerAction: MutableState<AsyncData<Unit>> = remember {
|
val changeServerAction: MutableState<AsyncData<MatrixHomeServerDetails>> = remember {
|
||||||
mutableStateOf(AsyncData.Uninitialized)
|
mutableStateOf(AsyncData.Uninitialized)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -59,7 +60,7 @@ class ChangeServerPresenter(
|
|||||||
|
|
||||||
private suspend fun changeServer(
|
private suspend fun changeServer(
|
||||||
data: AccountProvider,
|
data: AccountProvider,
|
||||||
changeServerAction: MutableState<AsyncData<Unit>>,
|
changeServerAction: MutableState<AsyncData<MatrixHomeServerDetails>>,
|
||||||
) {
|
) {
|
||||||
suspend {
|
suspend {
|
||||||
defaultAccountProviderAccessControl.assertIsAllowedToConnectToAccountProvider(
|
defaultAccountProviderAccessControl.assertIsAllowedToConnectToAccountProvider(
|
||||||
@@ -72,6 +73,8 @@ class ChangeServerPresenter(
|
|||||||
}
|
}
|
||||||
// Homeserver is valid, remember user choice
|
// Homeserver is valid, remember user choice
|
||||||
accountProviderDataSource.setAccountProvider(data)
|
accountProviderDataSource.setAccountProvider(data)
|
||||||
|
// Return the resolved details so the caller can sign in without configuring the homeserver again.
|
||||||
|
details
|
||||||
}.runCatchingUpdatingState(changeServerAction, errorTransform = ChangeServerError::from)
|
}.runCatchingUpdatingState(changeServerAction, errorTransform = ChangeServerError::from)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
+4
-1
@@ -9,10 +9,13 @@
|
|||||||
package io.element.android.features.login.impl.changeserver
|
package io.element.android.features.login.impl.changeserver
|
||||||
|
|
||||||
import io.element.android.libraries.architecture.AsyncData
|
import io.element.android.libraries.architecture.AsyncData
|
||||||
|
import io.element.android.libraries.matrix.api.auth.MatrixHomeServerDetails
|
||||||
import io.element.android.libraries.permissions.api.localnetwork.LocalNetworkPermissionDialog
|
import io.element.android.libraries.permissions.api.localnetwork.LocalNetworkPermissionDialog
|
||||||
|
|
||||||
data class ChangeServerState(
|
data class ChangeServerState(
|
||||||
val changeServerAction: AsyncData<Unit>,
|
// On success, carries the resolved homeserver details so the caller can proceed with login without
|
||||||
|
// configuring (and re-networking) the homeserver a second time.
|
||||||
|
val changeServerAction: AsyncData<MatrixHomeServerDetails>,
|
||||||
val localNetworkPermissionDialog: LocalNetworkPermissionDialog,
|
val localNetworkPermissionDialog: LocalNetworkPermissionDialog,
|
||||||
val eventSink: (ChangeServerEvents) -> Unit
|
val eventSink: (ChangeServerEvents) -> Unit
|
||||||
)
|
)
|
||||||
|
|||||||
+2
-1
@@ -11,6 +11,7 @@ package io.element.android.features.login.impl.changeserver
|
|||||||
import androidx.compose.ui.tooling.preview.PreviewParameterProvider
|
import androidx.compose.ui.tooling.preview.PreviewParameterProvider
|
||||||
import io.element.android.features.login.impl.error.ChangeServerError
|
import io.element.android.features.login.impl.error.ChangeServerError
|
||||||
import io.element.android.libraries.architecture.AsyncData
|
import io.element.android.libraries.architecture.AsyncData
|
||||||
|
import io.element.android.libraries.matrix.api.auth.MatrixHomeServerDetails
|
||||||
import io.element.android.libraries.permissions.api.localnetwork.LocalNetworkPermissionDialog
|
import io.element.android.libraries.permissions.api.localnetwork.LocalNetworkPermissionDialog
|
||||||
|
|
||||||
open class ChangeServerStateProvider : PreviewParameterProvider<ChangeServerState> {
|
open class ChangeServerStateProvider : PreviewParameterProvider<ChangeServerState> {
|
||||||
@@ -45,7 +46,7 @@ open class ChangeServerStateProvider : PreviewParameterProvider<ChangeServerStat
|
|||||||
}
|
}
|
||||||
|
|
||||||
fun aChangeServerState(
|
fun aChangeServerState(
|
||||||
changeServerAction: AsyncData<Unit> = AsyncData.Uninitialized,
|
changeServerAction: AsyncData<MatrixHomeServerDetails> = AsyncData.Uninitialized,
|
||||||
localNetworkPermissionDialog: LocalNetworkPermissionDialog = LocalNetworkPermissionDialog.None,
|
localNetworkPermissionDialog: LocalNetworkPermissionDialog = LocalNetworkPermissionDialog.None,
|
||||||
) = ChangeServerState(
|
) = ChangeServerState(
|
||||||
changeServerAction = changeServerAction,
|
changeServerAction = changeServerAction,
|
||||||
|
|||||||
+6
@@ -7,12 +7,18 @@
|
|||||||
|
|
||||||
package io.element.android.features.login.impl.login
|
package io.element.android.features.login.impl.login
|
||||||
|
|
||||||
|
import io.element.android.libraries.matrix.api.auth.MatrixHomeServerDetails
|
||||||
|
|
||||||
sealed interface LoginModeEvent {
|
sealed interface LoginModeEvent {
|
||||||
data class Submit(
|
data class Submit(
|
||||||
val isAccountCreation: Boolean,
|
val isAccountCreation: Boolean,
|
||||||
val homeserverUrl: String,
|
val homeserverUrl: String,
|
||||||
val resolvedHomeserverUrl: String?,
|
val resolvedHomeserverUrl: String?,
|
||||||
val loginHint: String?,
|
val loginHint: String?,
|
||||||
|
// When the caller has already configured the homeserver (e.g. the account provider screen validated
|
||||||
|
// it first), these are its resolved details. When set, login reuses them instead of calling
|
||||||
|
// setHomeserver again, avoiding a redundant network round-trip.
|
||||||
|
val preConfiguredDetails: MatrixHomeServerDetails? = null,
|
||||||
) : LoginModeEvent
|
) : LoginModeEvent
|
||||||
|
|
||||||
data object ClearError : LoginModeEvent
|
data object ClearError : LoginModeEvent
|
||||||
|
|||||||
+20
-18
@@ -75,26 +75,28 @@ class LoginModePresenter(
|
|||||||
|
|
||||||
private suspend fun performSubmit(request: LoginModeEvent.Submit, loginMode: MutableState<AsyncData<LoginMode>>) {
|
private suspend fun performSubmit(request: LoginModeEvent.Submit, loginMode: MutableState<AsyncData<LoginMode>>) {
|
||||||
suspend {
|
suspend {
|
||||||
authenticationService.setHomeserver(request.homeserverUrl).recoverCatching {
|
// Reuse the details the caller already resolved when it configured the homeserver, so we don't
|
||||||
// Fallback to the well-known-resolved URL if the primary URL failed and the caller supplied one.
|
// run setHomeserver (a network round-trip) again. Otherwise configure the homeserver ourselves.
|
||||||
if (request.resolvedHomeserverUrl != null && request.resolvedHomeserverUrl != request.homeserverUrl) {
|
val matrixHomeServerDetails = request.preConfiguredDetails
|
||||||
authenticationService.setHomeserver(request.resolvedHomeserverUrl).getOrThrow()
|
?: authenticationService.setHomeserver(request.homeserverUrl).recoverCatching {
|
||||||
} else {
|
// Fallback to the well-known-resolved URL if the primary URL failed and the caller supplied one.
|
||||||
throw it
|
if (request.resolvedHomeserverUrl != null && request.resolvedHomeserverUrl != request.homeserverUrl) {
|
||||||
}
|
authenticationService.setHomeserver(request.resolvedHomeserverUrl).getOrThrow()
|
||||||
}.map { matrixHomeServerDetails ->
|
} else {
|
||||||
when {
|
throw it
|
||||||
matrixHomeServerDetails.supportsOAuthLogin -> {
|
|
||||||
val oAuthPrompt = if (request.isAccountCreation) OAuthPrompt.Create else OAuthPrompt.Login
|
|
||||||
LoginMode.OAuth(
|
|
||||||
authenticationService.getOAuthUrl(prompt = oAuthPrompt, loginHint = request.loginHint).getOrThrow()
|
|
||||||
)
|
|
||||||
}
|
}
|
||||||
request.isAccountCreation -> throw AccountCreationNotSupported()
|
}.getOrThrow()
|
||||||
matrixHomeServerDetails.supportsPasswordLogin -> LoginMode.PasswordLogin
|
when {
|
||||||
else -> error("Unsupported authentication flow")
|
matrixHomeServerDetails.supportsOAuthLogin -> {
|
||||||
|
val oAuthPrompt = if (request.isAccountCreation) OAuthPrompt.Create else OAuthPrompt.Login
|
||||||
|
LoginMode.OAuth(
|
||||||
|
authenticationService.getOAuthUrl(prompt = oAuthPrompt, loginHint = request.loginHint).getOrThrow()
|
||||||
|
)
|
||||||
}
|
}
|
||||||
}.getOrThrow()
|
request.isAccountCreation -> throw AccountCreationNotSupported()
|
||||||
|
matrixHomeServerDetails.supportsPasswordLogin -> LoginMode.PasswordLogin
|
||||||
|
else -> error("Unsupported authentication flow")
|
||||||
|
}
|
||||||
}.runCatchingUpdatingState(
|
}.runCatchingUpdatingState(
|
||||||
state = loginMode,
|
state = loginMode,
|
||||||
errorTransform = {
|
errorTransform = {
|
||||||
|
|||||||
+4
-2
@@ -27,7 +27,6 @@ import io.element.android.features.login.impl.changeserver.ChangeServerEvents
|
|||||||
import io.element.android.features.login.impl.changeserver.ChangeServerState
|
import io.element.android.features.login.impl.changeserver.ChangeServerState
|
||||||
import io.element.android.features.login.impl.login.LoginModeEvent
|
import io.element.android.features.login.impl.login.LoginModeEvent
|
||||||
import io.element.android.features.login.impl.login.LoginModeState
|
import io.element.android.features.login.impl.login.LoginModeState
|
||||||
import io.element.android.libraries.architecture.AsyncData
|
|
||||||
import io.element.android.libraries.architecture.Presenter
|
import io.element.android.libraries.architecture.Presenter
|
||||||
import io.element.android.libraries.core.uri.ensureProtocol
|
import io.element.android.libraries.core.uri.ensureProtocol
|
||||||
import io.element.android.libraries.preferences.api.store.AppPreferencesStore
|
import io.element.android.libraries.preferences.api.store.AppPreferencesStore
|
||||||
@@ -81,14 +80,17 @@ class ConfirmAccountProviderPresenter(
|
|||||||
val latestSubmittedAccountProvider by rememberUpdatedState(submittedAccountProvider)
|
val latestSubmittedAccountProvider by rememberUpdatedState(submittedAccountProvider)
|
||||||
|
|
||||||
// Once the chosen account provider has been validated and persisted, proceed with the actual sign in / sign up.
|
// Once the chosen account provider has been validated and persisted, proceed with the actual sign in / sign up.
|
||||||
|
// Reuse the details resolved while validating, so login does not configure (and re-network) the homeserver again.
|
||||||
LaunchedEffect(changeServerState.changeServerAction) {
|
LaunchedEffect(changeServerState.changeServerAction) {
|
||||||
if (changeServerState.changeServerAction is AsyncData.Success) {
|
val homeServerDetails = changeServerState.changeServerAction.dataOrNull()
|
||||||
|
if (homeServerDetails != null) {
|
||||||
loginModeState.eventSink(
|
loginModeState.eventSink(
|
||||||
LoginModeEvent.Submit(
|
LoginModeEvent.Submit(
|
||||||
isAccountCreation = params.isAccountCreation,
|
isAccountCreation = params.isAccountCreation,
|
||||||
homeserverUrl = latestSubmittedAccountProvider.orEmpty().trim(),
|
homeserverUrl = latestSubmittedAccountProvider.orEmpty().trim(),
|
||||||
resolvedHomeserverUrl = null,
|
resolvedHomeserverUrl = null,
|
||||||
loginHint = null,
|
loginHint = null,
|
||||||
|
preConfiguredDetails = homeServerDetails,
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
+2
-2
@@ -73,7 +73,7 @@ class ChangeServerPresenterTest {
|
|||||||
val loadingState = awaitItem()
|
val loadingState = awaitItem()
|
||||||
assertThat(loadingState.changeServerAction).isInstanceOf(AsyncData.Loading::class.java)
|
assertThat(loadingState.changeServerAction).isInstanceOf(AsyncData.Loading::class.java)
|
||||||
val successState = awaitItem()
|
val successState = awaitItem()
|
||||||
assertThat(successState.changeServerAction).isEqualTo(AsyncData.Success(Unit))
|
assertThat(successState.changeServerAction).isEqualTo(AsyncData.Success(aMatrixHomeServerDetails(supportsOAuthLogin = true)))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -252,7 +252,7 @@ class ChangeServerPresenterTest {
|
|||||||
while (finalState.changeServerAction !is AsyncData.Success) {
|
while (finalState.changeServerAction !is AsyncData.Success) {
|
||||||
finalState = awaitItem()
|
finalState = awaitItem()
|
||||||
}
|
}
|
||||||
assertThat(finalState.changeServerAction).isEqualTo(AsyncData.Success(Unit))
|
assertThat(finalState.changeServerAction).isEqualTo(AsyncData.Success(aMatrixHomeServerDetails(supportsOAuthLogin = true)))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+23
@@ -396,6 +396,29 @@ class ConfirmAccountProviderPresenterTest {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `present - continue configures the homeserver only once`() = runTest {
|
||||||
|
// The consolidated Continue flow validates the account provider and then signs in. Login reuses the
|
||||||
|
// details resolved during validation, so setHomeserver runs exactly once rather than once per phase.
|
||||||
|
val submittedUrls = mutableListOf<String>()
|
||||||
|
val authenticationService = FakeMatrixAuthenticationService(
|
||||||
|
setHomeserverResult = { url ->
|
||||||
|
submittedUrls.add(url)
|
||||||
|
Result.success(aMatrixHomeServerDetails(supportsOAuthLogin = true))
|
||||||
|
},
|
||||||
|
)
|
||||||
|
val presenter = createConfirmAccountProviderPresenter(
|
||||||
|
matrixAuthenticationService = authenticationService,
|
||||||
|
)
|
||||||
|
presenter.test {
|
||||||
|
val initialState = awaitItem()
|
||||||
|
initialState.eventSink(ConfirmAccountProviderEvents.Continue(AuthenticationConfig.MATRIX_ORG_URL))
|
||||||
|
awaitLoginMode { it is AsyncData.Success }
|
||||||
|
assertThat(submittedUrls).containsExactly(AuthenticationConfig.MATRIX_ORG_URL)
|
||||||
|
cancelAndIgnoreRemainingEvents()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Awaits until the emitted state's login mode matches [predicate], skipping the intermediate
|
* Awaits until the emitted state's login mode matches [predicate], skipping the intermediate
|
||||||
* account-provider validation states, and returns that state.
|
* account-provider validation states, and returns that state.
|
||||||
|
|||||||
Reference in New Issue
Block a user