Let EnterpriseService prevent usage of homeserver (#4682)

* Add check on homeserver url.

* Update screenshots

* Add missing test

* Update submodule link.

---------

Co-authored-by: ElementBot <android@element.io>
This commit is contained in:
Benoit Marty
2025-05-06 19:25:07 +02:00
committed by GitHub
parent 8bbe4a55d2
commit 93eb70bd83
14 changed files with 108 additions and 5 deletions

View File

@@ -14,6 +14,7 @@ interface EnterpriseService {
val isEnterpriseBuild: Boolean
suspend fun isEnterpriseUser(sessionId: SessionId): Boolean
fun defaultHomeserver(): String?
suspend fun isAllowedToConnectToHomeserver(homeserverUrl: String): Boolean
fun semanticColorsLight(): SemanticColors
fun semanticColorsDark(): SemanticColors

View File

@@ -23,6 +23,7 @@ class DefaultEnterpriseService @Inject constructor() : EnterpriseService {
override suspend fun isEnterpriseUser(sessionId: SessionId) = false
override fun defaultHomeserver() = null
override suspend fun isAllowedToConnectToHomeserver(homeserverUrl: String) = true
override fun semanticColorsLight(): SemanticColors = compoundColorsLight

View File

@@ -8,6 +8,7 @@
package io.element.android.features.enterprise.impl
import com.google.common.truth.Truth.assertThat
import io.element.android.libraries.matrix.test.A_HOMESERVER_URL
import io.element.android.libraries.matrix.test.A_SESSION_ID
import kotlinx.coroutines.test.runTest
import org.junit.Test
@@ -25,6 +26,12 @@ class DefaultEnterpriseServiceTest {
assertThat<String?>(defaultEnterpriseService.defaultHomeserver()).isNull()
}
@Test
fun `isAllowedToConnectToHomeserver is true for all homeserver urls`() = runTest {
val defaultEnterpriseService = DefaultEnterpriseService()
assertThat(defaultEnterpriseService.isAllowedToConnectToHomeserver(A_HOMESERVER_URL)).isTrue()
}
@Test
fun `isEnterpriseUser always return false`() = runTest {
val defaultEnterpriseService = DefaultEnterpriseService()

View File

@@ -17,6 +17,7 @@ class FakeEnterpriseService(
override val isEnterpriseBuild: Boolean = false,
private val isEnterpriseUserResult: (SessionId) -> Boolean = { lambdaError() },
private val defaultHomeserverResult: () -> String? = { A_FAKE_HOMESERVER },
private val isAllowedToConnectToHomeserverResult: (String) -> Boolean = { lambdaError() },
private val semanticColorsLightResult: () -> SemanticColors = { lambdaError() },
private val semanticColorsDarkResult: () -> SemanticColors = { lambdaError() },
private val firebasePushGatewayResult: () -> String? = { lambdaError() },
@@ -30,6 +31,10 @@ class FakeEnterpriseService(
return defaultHomeserverResult()
}
override suspend fun isAllowedToConnectToHomeserver(homeserverUrl: String): Boolean = simulateLongTask {
isAllowedToConnectToHomeserverResult(homeserverUrl)
}
override fun semanticColorsLight(): SemanticColors {
return semanticColorsLightResult()
}

View File

@@ -12,6 +12,7 @@ import androidx.compose.runtime.MutableState
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberCoroutineScope
import io.element.android.features.enterprise.api.EnterpriseService
import io.element.android.features.login.impl.accountprovider.AccountProvider
import io.element.android.features.login.impl.accountprovider.AccountProviderDataSource
import io.element.android.features.login.impl.error.ChangeServerError
@@ -26,6 +27,7 @@ import javax.inject.Inject
class ChangeServerPresenter @Inject constructor(
private val authenticationService: MatrixAuthenticationService,
private val accountProviderDataSource: AccountProviderDataSource,
private val enterpriseService: EnterpriseService,
) : Presenter<ChangeServerState> {
@Composable
override fun present(): ChangeServerState {
@@ -53,6 +55,9 @@ class ChangeServerPresenter @Inject constructor(
changeServerAction: MutableState<AsyncData<Unit>>,
) = launch {
suspend {
if (enterpriseService.isAllowedToConnectToHomeserver(data.url).not()) {
throw UnauthorizedAccountProviderException(data)
}
authenticationService.setHomeserver(data.url).map {
authenticationService.getHomeserverDetails().value!!
// Valid, remember user choice

View File

@@ -8,6 +8,7 @@
package io.element.android.features.login.impl.changeserver
import androidx.compose.ui.tooling.preview.PreviewParameterProvider
import io.element.android.features.login.impl.accountprovider.anAccountProvider
import io.element.android.features.login.impl.error.ChangeServerError
import io.element.android.libraries.architecture.AsyncData
import io.element.android.libraries.ui.strings.CommonStrings
@@ -18,6 +19,7 @@ open class ChangeServerStateProvider : PreviewParameterProvider<ChangeServerStat
aChangeServerState(),
aChangeServerState(changeServerAction = AsyncData.Failure(ChangeServerError.Error(CommonStrings.error_unknown))),
aChangeServerState(changeServerAction = AsyncData.Failure(ChangeServerError.SlidingSyncAlert)),
aChangeServerState(changeServerAction = AsyncData.Failure(ChangeServerError.UnauthorizedAccountProvider(anAccountProvider()))),
)
}

View File

@@ -12,7 +12,9 @@ import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.getValue
import androidx.compose.runtime.rememberUpdatedState
import androidx.compose.ui.Modifier
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.tooling.preview.PreviewParameter
import io.element.android.features.login.impl.R
import io.element.android.features.login.impl.dialogs.SlidingSyncNotSupportedDialog
import io.element.android.features.login.impl.error.ChangeServerError
import io.element.android.libraries.architecture.AsyncData
@@ -20,6 +22,7 @@ import io.element.android.libraries.designsystem.components.ProgressDialog
import io.element.android.libraries.designsystem.components.dialogs.ErrorDialog
import io.element.android.libraries.designsystem.preview.ElementPreview
import io.element.android.libraries.designsystem.preview.PreviewsDayNight
import io.element.android.libraries.designsystem.theme.LocalBuildMeta
@Composable
fun ChangeServerView(
@@ -31,7 +34,7 @@ fun ChangeServerView(
val eventSink = state.eventSink
when (state.changeServerAction) {
is AsyncData.Failure -> {
when (val error = state.changeServerAction.error) {
when (val error = state.changeServerAction.error as? ChangeServerError) {
is ChangeServerError.Error -> {
ErrorDialog(
modifier = modifier,
@@ -53,6 +56,20 @@ fun ChangeServerView(
}
)
}
is ChangeServerError.UnauthorizedAccountProvider -> {
ErrorDialog(
modifier = modifier,
content = stringResource(
id = R.string.screen_change_server_error_unauthorized_homeserver,
LocalBuildMeta.current.applicationName,
error.accountProvider.title,
),
onSubmit = {
eventSink.invoke(ChangeServerEvents.ClearError)
}
)
}
null -> Unit
}
}
is AsyncData.Loading -> ProgressDialog()

View File

@@ -0,0 +1,14 @@
/*
* Copyright 2025 New Vector Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
* Please see LICENSE files in the repository root for full details.
*/
package io.element.android.features.login.impl.changeserver
import io.element.android.features.login.impl.accountprovider.AccountProvider
class UnauthorizedAccountProviderException(
val accountProvider: AccountProvider,
) : Exception()

View File

@@ -11,6 +11,8 @@ import androidx.annotation.StringRes
import androidx.compose.runtime.Composable
import androidx.compose.ui.res.stringResource
import io.element.android.features.login.impl.R
import io.element.android.features.login.impl.accountprovider.AccountProvider
import io.element.android.features.login.impl.changeserver.UnauthorizedAccountProviderException
import io.element.android.libraries.matrix.api.auth.AuthenticationException
import io.element.android.libraries.ui.strings.CommonStrings
@@ -23,12 +25,17 @@ sealed class ChangeServerError : Throwable() {
fun message(): String = messageStr ?: stringResource(messageId ?: CommonStrings.error_unknown)
}
data class UnauthorizedAccountProvider(
val accountProvider: AccountProvider,
) : ChangeServerError()
data object SlidingSyncAlert : ChangeServerError()
companion object {
fun from(error: Throwable): ChangeServerError = when (error) {
is AuthenticationException.SlidingSyncVersion -> SlidingSyncAlert
is AuthenticationException.Oidc -> Error(messageStr = error.message)
is UnauthorizedAccountProviderException -> UnauthorizedAccountProvider(error.accountProvider)
else -> Error(messageId = R.string.screen_change_server_error_invalid_homeserver)
}
}

View File

@@ -14,9 +14,10 @@
<string name="screen_change_account_provider_subtitle">"Use a different account provider, such as your own private server or a work account."</string>
<string name="screen_change_account_provider_title">"Change account provider"</string>
<string name="screen_change_server_error_invalid_homeserver">"We couldn\'t reach this homeserver. Please check that you have entered the homeserver URL correctly. If the URL is correct, contact your homeserver administrator for further help."</string>
<string name="screen_change_server_error_invalid_well_known">"Server isn\'t available due to an issue in the well-known file:
<string name="screen_change_server_error_invalid_well_known">"Server isn\'t available due to an issue in the .well-known file:
%1$s"</string>
<string name="screen_change_server_error_no_sliding_sync_message">"The selected account provider does not support sliding sync. An upgrade to the server is needed to use %1$s."</string>
<string name="screen_change_server_error_unauthorized_homeserver">"%1$s is not allowed to connect to %2$s."</string>
<string name="screen_change_server_form_header">"Homeserver URL"</string>
<string name="screen_change_server_form_notice">"Enter a domain address."</string>
<string name="screen_change_server_subtitle">"What is the address of your server?"</string>

View File

@@ -8,14 +8,18 @@
package io.element.android.features.login.impl.changeserver
import com.google.common.truth.Truth.assertThat
import io.element.android.features.enterprise.api.EnterpriseService
import io.element.android.features.enterprise.test.FakeEnterpriseService
import io.element.android.features.login.impl.accountprovider.AccountProvider
import io.element.android.features.login.impl.accountprovider.AccountProviderDataSource
import io.element.android.features.login.impl.error.ChangeServerError
import io.element.android.libraries.architecture.AsyncData
import io.element.android.libraries.matrix.test.A_HOMESERVER
import io.element.android.libraries.matrix.test.A_HOMESERVER_URL
import io.element.android.libraries.matrix.test.auth.FakeMatrixAuthenticationService
import io.element.android.tests.testutils.WarmUpRule
import io.element.android.tests.testutils.lambda.lambdaRecorder
import io.element.android.tests.testutils.lambda.value
import io.element.android.tests.testutils.test
import kotlinx.coroutines.test.runTest
import org.junit.Rule
@@ -38,6 +42,9 @@ class ChangeServerPresenterTest {
val authenticationService = FakeMatrixAuthenticationService()
createPresenter(
authenticationService = authenticationService,
enterpriseService = FakeEnterpriseService(
isAllowedToConnectToHomeserverResult = { true },
),
).test {
val initialState = awaitItem()
assertThat(initialState.changeServerAction).isEqualTo(AsyncData.Uninitialized)
@@ -52,7 +59,11 @@ class ChangeServerPresenterTest {
@Test
fun `present - change server error`() = runTest {
createPresenter().test {
createPresenter(
enterpriseService = FakeEnterpriseService(
isAllowedToConnectToHomeserverResult = { true },
),
).test {
val initialState = awaitItem()
assertThat(initialState.changeServerAction).isEqualTo(AsyncData.Uninitialized)
initialState.eventSink.invoke(ChangeServerEvents.ChangeServer(AccountProvider(url = A_HOMESERVER_URL)))
@@ -67,11 +78,37 @@ class ChangeServerPresenterTest {
}
}
@Test
fun `present - change server not allowed error`() = runTest {
val isAllowedToConnectToHomeserverResult = lambdaRecorder<String, Boolean> { false }
createPresenter(
enterpriseService = FakeEnterpriseService(
isAllowedToConnectToHomeserverResult = isAllowedToConnectToHomeserverResult,
),
).test {
val initialState = awaitItem()
assertThat(initialState.changeServerAction).isEqualTo(AsyncData.Uninitialized)
val anAccountProvider = AccountProvider(url = A_HOMESERVER_URL)
initialState.eventSink.invoke(ChangeServerEvents.ChangeServer(anAccountProvider))
val loadingState = awaitItem()
assertThat(loadingState.changeServerAction).isInstanceOf(AsyncData.Loading::class.java)
val failureState = awaitItem()
assertThat(
(failureState.changeServerAction.errorOrNull() as ChangeServerError.UnauthorizedAccountProvider).accountProvider
).isEqualTo(anAccountProvider)
isAllowedToConnectToHomeserverResult.assertions()
.isCalledOnce()
.with(value(A_HOMESERVER_URL))
}
}
private fun createPresenter(
authenticationService: FakeMatrixAuthenticationService = FakeMatrixAuthenticationService(),
accountProviderDataSource: AccountProviderDataSource = AccountProviderDataSource(FakeEnterpriseService()),
enterpriseService: EnterpriseService = FakeEnterpriseService(),
) = ChangeServerPresenter(
authenticationService = authenticationService,
accountProviderDataSource = accountProviderDataSource
accountProviderDataSource = accountProviderDataSource,
enterpriseService = enterpriseService,
)
}