Skip to content

Check homeserver when login using qr code #4708

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
May 15, 2025
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 @@ -21,8 +21,10 @@ open class AccountProviderProvider : PreviewParameterProvider<AccountProvider> {
)
}

fun anAccountProvider() = AccountProvider(
url = AuthenticationConfig.MATRIX_ORG_URL,
fun anAccountProvider(
url: String = AuthenticationConfig.MATRIX_ORG_URL,
) = AccountProvider(
url = url,
subtitle = "Matrix.org is an open network for secure, decentralized communication.",
isPublic = true,
isMatrixOrg = true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,10 @@ class ChangeServerPresenter @Inject constructor(
) = launch {
suspend {
if (enterpriseService.isAllowedToConnectToHomeserver(data.url).not()) {
throw UnauthorizedAccountProviderException(data)
throw UnauthorizedAccountProviderException(
unauthorisedAccountProviderTitle = data.title,
authorisedAccountProviderTitles = listOfNotNull(enterpriseService.defaultHomeserver())
)
}
authenticationService.setHomeserver(data.url).map {
authenticationService.getHomeserverDetails().value!!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
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
Expand All @@ -19,7 +18,14 @@ 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()))),
aChangeServerState(
changeServerAction = AsyncData.Failure(
ChangeServerError.UnauthorizedAccountProvider(
unauthorisedAccountProviderTitle = "example.com",
authorisedAccountProviderTitles = listOf("element.io", "element.org"),
)
)
),
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ fun ChangeServerView(
content = stringResource(
id = R.string.screen_change_server_error_unauthorized_homeserver,
LocalBuildMeta.current.applicationName,
error.accountProvider.title,
error.unauthorisedAccountProviderTitle,
),
onSubmit = {
eventSink.invoke(ChangeServerEvents.ClearError)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@

package io.element.android.features.login.impl.changeserver

import io.element.android.features.login.impl.accountprovider.AccountProvider

class UnauthorizedAccountProviderException(
val accountProvider: AccountProvider,
val unauthorisedAccountProviderTitle: String,
val authorisedAccountProviderTitles: List<String>,
) : Exception()
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ 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
Expand All @@ -26,7 +25,8 @@ sealed class ChangeServerError : Throwable() {
}

data class UnauthorizedAccountProvider(
val accountProvider: AccountProvider,
val unauthorisedAccountProviderTitle: String,
val authorisedAccountProviderTitles: List<String>,
) : ChangeServerError()

data object SlidingSyncAlert : ChangeServerError()
Expand All @@ -35,7 +35,10 @@ sealed class ChangeServerError : Throwable() {
fun from(error: Throwable): ChangeServerError = when (error) {
is AuthenticationException.SlidingSyncVersion -> SlidingSyncAlert
is AuthenticationException.Oidc -> Error(messageStr = error.message)
is UnauthorizedAccountProviderException -> UnauthorizedAccountProvider(error.accountProvider)
is UnauthorizedAccountProviderException -> UnauthorizedAccountProvider(
unauthorisedAccountProviderTitle = error.unauthorisedAccountProviderTitle,
authorisedAccountProviderTitles = error.authorisedAccountProviderTitles,
)
else -> Error(messageId = R.string.screen_change_server_error_invalid_homeserver)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.runtime.setValue
import io.element.android.features.enterprise.api.EnterpriseService
import io.element.android.features.login.impl.changeserver.UnauthorizedAccountProviderException
import io.element.android.features.login.impl.qrcode.QrCodeLoginManager
import io.element.android.libraries.architecture.AsyncAction
import io.element.android.libraries.architecture.Presenter
Expand All @@ -36,6 +38,7 @@ class QrCodeScanPresenter @Inject constructor(
private val qrCodeLoginDataFactory: MatrixQrCodeLoginDataFactory,
private val qrCodeLoginManager: QrCodeLoginManager,
private val coroutineDispatchers: CoroutineDispatchers,
private val enterpriseService: EnterpriseService,
) : Presenter<QrCodeScanState> {
private var isScanning by mutableStateOf(true)

Expand Down Expand Up @@ -90,9 +93,17 @@ class QrCodeScanPresenter @Inject constructor(

launch(coroutineDispatchers.computation) {
suspend {
qrCodeLoginDataFactory.parseQrCodeData(code).onFailure {
val data = qrCodeLoginDataFactory.parseQrCodeData(code).onFailure {
Timber.e(it, "Error parsing QR code data")
}.getOrThrow()
val serverName = data.serverName()
if (serverName != null && enterpriseService.isAllowedToConnectToHomeserver(serverName).not()) {
throw UnauthorizedAccountProviderException(
unauthorisedAccountProviderTitle = serverName,
authorisedAccountProviderTitles = listOfNotNull(enterpriseService.defaultHomeserver())
)
}
data
}.runCatchingUpdatingState(codeScannedAction)
}.invokeOnCompletion {
isProcessingCode.set(false)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
package io.element.android.features.login.impl.screens.qrcode.scan

import androidx.compose.ui.tooling.preview.PreviewParameterProvider
import io.element.android.features.login.impl.changeserver.UnauthorizedAccountProviderException
import io.element.android.libraries.architecture.AsyncAction
import io.element.android.libraries.matrix.api.auth.qrlogin.MatrixQrCodeLoginData
import io.element.android.libraries.matrix.api.auth.qrlogin.QrLoginException
Expand All @@ -19,6 +20,15 @@ open class QrCodeScanStateProvider : PreviewParameterProvider<QrCodeScanState> {
aQrCodeScanState(isScanning = false, authenticationAction = AsyncAction.Loading),
aQrCodeScanState(isScanning = false, authenticationAction = AsyncAction.Failure(Exception("Error"))),
aQrCodeScanState(isScanning = false, authenticationAction = AsyncAction.Failure(QrLoginException.OtherDeviceNotSignedIn)),
aQrCodeScanState(
isScanning = false,
authenticationAction = AsyncAction.Failure(
UnauthorizedAccountProviderException(
unauthorisedAccountProviderTitle = "example.com",
authorisedAccountProviderTitles = listOf("element.io", "element.org"),
)
)
),
// Add other state here
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import androidx.compose.ui.unit.dp
import io.element.android.compound.theme.ElementTheme
import io.element.android.compound.tokens.generated.CompoundIcons
import io.element.android.features.login.impl.R
import io.element.android.features.login.impl.changeserver.UnauthorizedAccountProviderException
import io.element.android.libraries.architecture.AsyncAction
import io.element.android.libraries.designsystem.atomic.pages.FlowStepPage
import io.element.android.libraries.designsystem.components.BigIcon
Expand Down Expand Up @@ -144,6 +145,12 @@ private fun ColumnScope.Buttons(
Spacer(modifier = Modifier.width(4.dp))
Text(
text = when (error) {
is UnauthorizedAccountProviderException -> {
stringResource(
id = R.string.screen_change_server_error_unauthorized_homeserver_title,
error.unauthorisedAccountProviderTitle,
)
}
is QrLoginException.OtherDeviceNotSignedIn -> {
stringResource(R.string.screen_qr_code_login_device_not_signed_in_scan_state_subtitle)
}
Expand All @@ -156,6 +163,12 @@ private fun ColumnScope.Buttons(
}
Text(
text = when (error) {
is UnauthorizedAccountProviderException -> {
stringResource(
id = R.string.screen_change_server_error_unauthorized_homeserver_content,
error.authorisedAccountProviderTitles.joinToString(),
)
}
is QrLoginException.OtherDeviceNotSignedIn -> {
stringResource(R.string.screen_qr_code_login_device_not_signed_in_scan_state_description)
}
Expand Down
2 changes: 2 additions & 0 deletions features/login/impl/src/main/res/values/localazy.xml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
%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_error_unauthorized_homeserver_content">"This app has been configured to allow: %1$s."</string>
<string name="screen_change_server_error_unauthorized_homeserver_title">"Account provider %1$s not allowed."</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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ class ChangeServerPresenterTest {
createPresenter(
enterpriseService = FakeEnterpriseService(
isAllowedToConnectToHomeserverResult = isAllowedToConnectToHomeserverResult,
defaultHomeserverResult = { "element.io" },
),
).test {
val initialState = awaitItem()
Expand All @@ -94,8 +95,11 @@ class ChangeServerPresenterTest {
assertThat(loadingState.changeServerAction).isInstanceOf(AsyncData.Loading::class.java)
val failureState = awaitItem()
assertThat(
(failureState.changeServerAction.errorOrNull() as ChangeServerError.UnauthorizedAccountProvider).accountProvider
).isEqualTo(anAccountProvider)
(failureState.changeServerAction.errorOrNull() as ChangeServerError.UnauthorizedAccountProvider).unauthorisedAccountProviderTitle
).isEqualTo(anAccountProvider.title)
assertThat(
(failureState.changeServerAction.errorOrNull() as ChangeServerError.UnauthorizedAccountProvider).authorisedAccountProviderTitles
).containsExactly("element.io")
isAllowedToConnectToHomeserverResult.assertions()
.isCalledOnce()
.with(value(A_HOMESERVER_URL))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,17 @@ import app.cash.molecule.RecompositionMode
import app.cash.molecule.moleculeFlow
import app.cash.turbine.test
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.changeserver.UnauthorizedAccountProviderException
import io.element.android.features.login.impl.qrcode.FakeQrCodeLoginManager
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
import io.element.android.libraries.matrix.api.auth.qrlogin.QrCodeLoginStep
import io.element.android.libraries.matrix.api.auth.qrlogin.QrLoginException
import io.element.android.libraries.matrix.test.auth.qrlogin.FakeMatrixQrCodeLoginData
import io.element.android.libraries.matrix.test.auth.qrlogin.FakeMatrixQrCodeLoginDataFactory
import io.element.android.tests.testutils.lambda.lambdaRecorder
import io.element.android.tests.testutils.test
import io.element.android.tests.testutils.testCoroutineDispatchers
import kotlinx.coroutines.test.TestScope
import kotlinx.coroutines.test.runTest
Expand All @@ -38,10 +43,22 @@ class QrCodeScanPresenterTest {

@Test
fun `present - scanned QR code successfully`() = runTest {
val presenter = createQrCodeScanPresenter()
moleculeFlow(RecompositionMode.Immediate) {
presenter.present()
}.test {
val qrCodeLoginDataFactory = FakeMatrixQrCodeLoginDataFactory(
parseQrCodeLoginDataResult = {
Result.success(
FakeMatrixQrCodeLoginData(
serverNameResult = { "example.com" }
)
)
}
)
val presenter = createQrCodeScanPresenter(
qrCodeLoginDataFactory = qrCodeLoginDataFactory,
enterpriseService = FakeEnterpriseService(
isAllowedToConnectToHomeserverResult = { true },
)
)
presenter.test {
val initialState = awaitItem()
initialState.eventSink(QrCodeScanEvents.QrCodeScanned(byteArrayOf()))
assertThat(awaitItem().isScanning).isFalse()
Expand All @@ -50,6 +67,38 @@ class QrCodeScanPresenterTest {
}
}

@Test
fun `present - scanned QR code successfully, but homeserver not allowed`() = runTest {
val qrCodeLoginDataFactory = FakeMatrixQrCodeLoginDataFactory(
parseQrCodeLoginDataResult = {
Result.success(
FakeMatrixQrCodeLoginData(
serverNameResult = { "example.com" }
)
)
}
)
val presenter = createQrCodeScanPresenter(
qrCodeLoginDataFactory = qrCodeLoginDataFactory,
enterpriseService = FakeEnterpriseService(
isAllowedToConnectToHomeserverResult = { false },
defaultHomeserverResult = { "element.io" }
)
)
presenter.test {
val initialState = awaitItem()
initialState.eventSink(QrCodeScanEvents.QrCodeScanned(byteArrayOf()))
assertThat(awaitItem().isScanning).isFalse()
assertThat(awaitItem().authenticationAction.isLoading()).isTrue()
awaitItem().also { state ->
assertThat((state.authenticationAction.errorOrNull() as UnauthorizedAccountProviderException).unauthorisedAccountProviderTitle)
.isEqualTo("example.com")
assertThat((state.authenticationAction.errorOrNull() as UnauthorizedAccountProviderException).authorisedAccountProviderTitles)
.containsExactly("element.io")
}
}
}

@Test
fun `present - scanned QR code failed and can be retried`() = runTest {
val qrCodeLoginDataFactory = FakeMatrixQrCodeLoginDataFactory(
Expand Down Expand Up @@ -103,9 +152,11 @@ class QrCodeScanPresenterTest {
qrCodeLoginDataFactory: FakeMatrixQrCodeLoginDataFactory = FakeMatrixQrCodeLoginDataFactory(),
coroutineDispatchers: CoroutineDispatchers = testCoroutineDispatchers(),
qrCodeLoginManager: FakeQrCodeLoginManager = FakeQrCodeLoginManager(),
enterpriseService: EnterpriseService = FakeEnterpriseService(),
) = QrCodeScanPresenter(
qrCodeLoginDataFactory = qrCodeLoginDataFactory,
qrCodeLoginManager = qrCodeLoginManager,
coroutineDispatchers = coroutineDispatchers,
enterpriseService = enterpriseService,
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,6 @@

package io.element.android.libraries.matrix.api.auth.qrlogin

interface MatrixQrCodeLoginData
interface MatrixQrCodeLoginData {
fun serverName(): String?
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,8 @@ import org.matrix.rustcomponents.sdk.QrCodeData as RustQrCodeData

class SdkQrCodeLoginData(
internal val rustQrCodeData: RustQrCodeData,
) : MatrixQrCodeLoginData
) : MatrixQrCodeLoginData {
override fun serverName(): String? {
return rustQrCodeData.serverName()
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* 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.libraries.matrix.impl.auth.qrlogin

import com.google.common.truth.Truth.assertThat
import io.element.android.libraries.matrix.impl.fixtures.fakes.FakeQrCodeData
import io.element.android.libraries.matrix.test.A_HOMESERVER_URL
import org.junit.Test

class SdkQrCodeLoginDataTest {
@Test
fun `getServer reads the value from the Rust side, null case`() {
val sut = SdkQrCodeLoginData(
rustQrCodeData = FakeQrCodeData(
serverNameResult = { null },
),
)
assertThat(sut.serverName()).isNull()
}

@Test
fun `getServer reads the value from the Rust side`() {
val sut = SdkQrCodeLoginData(
rustQrCodeData = FakeQrCodeData(
serverNameResult = { A_HOMESERVER_URL },
),
)
assertThat(sut.serverName()).isEqualTo(A_HOMESERVER_URL)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* 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.libraries.matrix.impl.fixtures.fakes

import io.element.android.tests.testutils.lambda.lambdaError
import org.matrix.rustcomponents.sdk.NoPointer
import org.matrix.rustcomponents.sdk.QrCodeData

class FakeQrCodeData(
private val serverNameResult: () -> String? = { lambdaError() },
) : QrCodeData(NoPointer) {
override fun serverName(): String? {
return serverNameResult()
}
}
Loading
Loading