Skip to content

Conversation

uhoreg
Copy link
Member

@uhoreg uhoreg commented Sep 30, 2025

Fixes #4864

Content

Currently, on logging in, it will offer to verify against another device if the user has any other devices, even if they are not cross-signed. This change uses the function introduced in matrix-org/matrix-rust-sdk#5699 to check whether the user has any cross-signed devices, and will only offer to verify if one is available.

Motivation and context

#4864

Screenshots / GIFs

image

No "Use another device" button

Tests

  • Signed in a test user with Element Web, but did not verify
  • Ensured that the user had no other devices
  • Signed in to Element X, and check that "Confirm your dentity" screen did not offer to "Use another device"
  • Signed out of Element X
  • Reset cross-signing in Element Web
  • Signed in to Element X, and check that "Confirm your identity" screen now offers to "Use another device"

Tested devices

  • Physical
  • Emulator
  • OS version(s): Android 16 (API 36.0)

Checklist

  • Changes have been tested on an Android device or Android emulator with API 24
  • UI change has been tested on both light and dark themes
  • Accessibility has been taken into account. See https://github.com/element-hq/element-x-android/blob/develop/CONTRIBUTING.md#accessibility
  • Pull request is based on the develop branch
  • Pull request title will be used in the release note, it clearly define what will change for the user
  • Pull request includes screenshots or videos if containing UI changes
  • You've made a self review of your PR

Copy link
Contributor

Thank you for your contribution! Here are a few things to check in the PR to ensure it's reviewed as quickly as possible:

  • Your branch should be based on origin/develop, at least when it was created.
  • The title of the PR will be used for release notes, so it needs to describe the change visible to the user.
  • The test pass locally running ./gradlew test.
  • The code quality check suite pass locally running ./gradlew runQualityChecks.
  • If you modified anything related to the UI, including previews, you'll have to run the Record screenshots GH action in your forked repo: that will generate compatible new screenshots. However, given Github Actions limitations, it will prevent the CI from running temporarily, until you upload a new commit after that one. To do so, just pull the latest changes and push an empty commit.

Copy link

codecov bot commented Sep 30, 2025

Codecov Report

❌ Patch coverage is 86.95652% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.02%. Comparing base (43ad874) to head (d43e3da).
⚠️ Report is 95 commits behind head on develop.

Files with missing lines Patch % Lines
...es/matrix/impl/encryption/RustEncryptionService.kt 72.72% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #5433      +/-   ##
===========================================
- Coverage    81.03%   81.02%   -0.01%     
===========================================
  Files         2312     2312              
  Lines        63450    63463      +13     
  Branches      8012     8014       +2     
===========================================
+ Hits         51414    51419       +5     
- Misses        9099     9106       +7     
- Partials      2937     2938       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@uhoreg
Copy link
Member Author

uhoreg commented Sep 30, 2025

Trying to get Maestro running locally so I can look into the failure, because the CI log isn't very helpful. The code coverage seems to be complaining about missing a test on the mapRecoveryException line. Otherwise, I think this is pretty much ready for review.

@uhoreg uhoreg marked this pull request as ready for review September 30, 2025 22:39
@uhoreg uhoreg requested a review from a team as a code owner September 30, 2025 22:39
@uhoreg uhoreg requested review from bmarty and removed request for a team September 30, 2025 22:39
Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

isLastDevice is also used in LogoutView, to warn the user if this is the last session. I am wondering if the new hasDevicesToVerifyAgainst could be used there too. What do you think?

Also, having this screen:

image

with actually no options to confirm seems a bit annoying for the user. We may want to iterate on the design in this case.
Can't we enter the recovery key?


return ChooseSelfVerificationModeState(
isLastDevice = isLastDevice,
isLastDevice = !hasDevicesToVerifyAgainst,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For clarity / reduce tech debt, I think ChooseSelfVerificationModeState.isLastDevice could be renamed to canUseAnotherDevice. Do not forget to negate the boolean wherever this is necessary.

delay(5_000)
}
}
.stateIn(sessionCoroutineScope, SharingStarted.Eagerly, false)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add the same TODO than in the KDoc of isLastDevice. It is a bit annoying to request this value every 5s, when it's only used in the session verification screen.

@uhoreg
Copy link
Member Author

uhoreg commented Oct 1, 2025

Also, having this screen:
image

with actually no options to confirm seems a bit annoying for the user. We may want to iterate on the design in this case. Can't we enter the recovery key?

Yes, if the user has a recovery key set up, they should have a button to use the recovery key. In this case, my test user did not have a recovery key, so no button was shown, and the user's only option was "Can't confirm?", which resets the user's cross-signing keys (which is the only thing that can be done in this case).

@uhoreg
Copy link
Member Author

uhoreg commented Oct 1, 2025

isLastDevice is also used in LogoutView, to warn the user if this is the last session. I am wondering if the new hasDevicesToVerifyAgainst could be used there too. What do you think?

I'm not sure. I just checked what Element Web did here, and it looks like it's only checking the backup status. As far as I can tell, it doesn't seem to check at all whether there are any other devices. So we might need to make a new issue to make the clients consistent, and figure out what criteria we should use to warn the user. Anyways, I don't think hasDevicesToVerifyAgainst will be quite right, because it specifically excludes dehydrated devices, and if the user has a dehydrated device, they shouldn't be shown a warning on logout.

@uhoreg
Copy link
Member Author

uhoreg commented Oct 2, 2025

Tried to reproduce the CI failures. For Test / Runs unit tests the failing task succeeds on my machine. For Maestro (local) / Maestro test suite I'm having trouble getting the Maestro suite to complete, but it gets past the login section, which is the only place my PR should affect.

@uhoreg uhoreg requested a review from bmarty October 2, 2025 21:01
aChooseSelfVerificationModeState(canUseAnotherDevice = true, canEnterRecoveryKey = true),
aChooseSelfVerificationModeState(canUseAnotherDevice = true, canEnterRecoveryKey = false),
aChooseSelfVerificationModeState(canUseAnotherDevice = false, canEnterRecoveryKey = true),
aChooseSelfVerificationModeState(canUseAnotherDevice = false, canEnterRecoveryKey = false),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The failing tests are related to the screenshot test, which are not run by default when running the test locally.
To avoid this problem, I think you need to use the opposite values for all boolean assigned to canUseAnotherDevice, and so the screenshot will be identical to the current version.

Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@bmarty bmarty merged commit d2e5b43 into element-hq:develop Oct 6, 2025
24 of 26 checks passed
raphael-chevallier added a commit to tchapgouv/tchap-x-android that referenced this pull request Oct 15, 2025
* Update dependency androidx.sqlite:sqlite-ktx to v2.6.1

* feature (space) : add trailing action to SpaceRoomItemView

* Update dependency io.element.android:element-call-embedded to v0.16.0 (element-hq#5408)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update dependency org.matrix.rustcomponents:sdk-android to v25.9.25 (element-hq#5412)

* Update dependency org.matrix.rustcomponents:sdk-android to v25.9.25

* Adapt to SDK changes:

`MessageLikeEventType` is now a sealed interface and has a new `data class Other(val v1: String)` case.

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Jorge Martín <[email protected]>

* Update kotlin (element-hq#5317)

* Update kotlin

* Upgrade Metro and add new `@Origin` annotation

* Suppress warnings in overridden method as nothing else would work

* "Fix" quality warnings about reusing the same string literal

* Don't use `compat` version for `datetime` dependency

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Jorge Martín <[email protected]>

* Update metro to v0.6.7 (element-hq#5416)

* Update metro to v0.6.7

* Replace `@Inject` with `@AssistedInject` where needed

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Jorge Martín <[email protected]>

* Update dependency app.cash.molecule:molecule-runtime to v2.2.0 (element-hq#5413)

* Update dependency app.cash.molecule:molecule-runtime to v2.2.0

* Fix compilation warnings

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Benoit Marty <[email protected]>

* feature (space) : allow joining children from space screen

* Use shared recent emoji reactions from account data (element-hq#5402)

* Use shared recent emoji reactions from account data

- Add `AddRecentEmoji` and `GetRecentEmojis` use cases to avoid injecting the whole `MatrixClient` for just one of these operations.
- Update the UI and logic of the emoji picker and message context menu to include the recent emojis.
- Add `CoroutineDispatchers.Default` with the defaults coroutines to use in the app for ease of use.

* Instead of replacing suggested emojis, concatenate recent ones removing duplicates

* Update screenshots

---------

Co-authored-by: ElementBot <[email protected]>

* Leave space  - Add screen to leave a space.

* Remove translations (key values have changed).
Translations will be back during the next Localazy sync.

* Add the (Admin) info.

* Add unit test on LeaveSpaceState

* Select all rooms by default

* Update UI

* Update tests

* Cleanup to be able to merge.

* Metro now have `@AssistedInject`.

* Update screenshots

* Multi accounts - experimental first implementation (element-hq#5285)

* Multi account - Do not reset analytics store on sign out.

Else when 1 of many accounts is removed, the analytics opt in screen is displayed again.

* Multi accounts - first implementation.

* Multi accounts - Prevent user from logging twice with the same account

* Multi accounts - ignore automatic GoBack in case of error.

* Multi accounts - update first view when adding an account.

* Rename method storeData to addSession.

* Multi accounts - handle account switch when coming from a notification

* Multi accounts - handle login link when there is already an account.

* Multi accounts - handle click on push history for not current account.

* Multi accounts - improve layout and add preview.

* Add accountselect modules

* Multi accounts - incoming share with account selection

* Multi accounts - check the feature flag before allowing login using login link.

* Multi accounts - swipe on account icon

* Cleanup

* Multi accounts - fix other implementation of SessionStore

* Multi accounts - fix PreferencesRootPresenterTest

* Multi accounts - Add test on AccountSelectPresenter

* Multi accounts - Fix test on HomePresenter - WIP

* Update database to be able to sort accounts by creation date.

* Add unit test on takeCurrentUserWithNeighbors

* Fix test and improve code.

* Add exception

* Multi accounts - handle permalink

* Code quality

* Multi accounts - localization

* Fix issue after rebase on develop

* Fix issue after rebase on develop

* Fix tests

* Fix tests

* Fix tests

* Fix tests

* Update Multi accounts flag details.

* Add missing test on DatabaseSessionStore

* Add missing preview on LoginModeView

* Remove dead code.

* Add missing preview on PushHistoryView

* Document API.

* Rename API and update test.

* Remove MatrixAuthenticationService.loggedInStateFlow()

* Update screenshots

* Remove unused import

* Add exception

* Fix compilation issue after rebase on develop.

* Update screenshots

* Fix test

* Avoid calling getLatestSession() twice

* Rename `matrixUserAndNeighbors` to `currentUserAndNeighbors`

* Extract code to its own class.

* Add comment to clarify the code.

* Init current user profile with what we now have in the database.

It allows having the cached data (user display name and avatar) when starting the application when no network is available.

* Let the RustMatrixClient update the profile in the session database

* Fix test.

* When logging out from Pin code screen, logout from all the sessions.

tom

* Make PushData.clientSecret mandatory.
Also do not restore the last session as a fallback, it can lead to error in a multi account context, or even when a ghost pusher send a Push.

* Change test in RustMatrixAuthenticationServiceTest

* Do not use MatrixAuthenticationService in RootFlowNode, only use SessionStore

* Remove MatrixAuthenticationService.getLatestSessionId()

* Fix compilation issue after merging develop

* Add test on DefaultAccountSelectEntryPoint

* Fix compilation issue after merging develop

* Introduce LoggedInAccountSwitcherNode, to improve animation when switching between accounts.

* Rename Node to follow naming convention.

* Fix navigation issue after login.

* Remove unused import

* Revert "Fix navigation issue after login."

This reverts commit e409630.

* Revert "Rename Node to follow naming convention."

This reverts commit 883b1f3.

* Revert "Introduce LoggedInAccountSwitcherNode, to improve animation when switching between accounts."

This reverts commit 9c698ff.

* Metro now have `@AssistedInject`.

* Update screenshots

* Introduce DelegateTransitionHandler and use it in RootFlowNode

---------

Co-authored-by: ElementBot <[email protected]>
Co-authored-by: ganfra <[email protected]>

* Update screenshots

* Address review comments.

* Update dependency com.posthog:posthog-android to v3.22.0 (element-hq#5415)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update metro to v0.6.8

* Add Konsist test to check Metro annotation.

* Use AssistedInject instead of Inject.

* Create a LoadingNode to reduce code duplication.

* Fix compilation issue.

* Fix issue after rebase.

* Do not use plurals in this case since it can lead to lint issues since there is no %d in the value for one and this triggers a warning in some languages.

* Update screenshots

* Add a11y preview for incoming verification request.
We want to see the string a11y_session_verification_time_limited_action_required.

tom

* SessionVerificationRequestDetails: map deviceDisplayName.

* Remove code duplication around UserProfile mapper.

* Fix mapping issue.

* Fix layout issue

* Improve fun VerificationUserProfileContent

* Update screenshots

* Update test.

* Sync Strings from Localazy (element-hq#5427)

Co-authored-by: bmarty <[email protected]>

* Update the strings for the device verification flow (element-hq#5419)

* Update the strings for the device verification flow

Part of element-hq/element-meta#2898

* Update screenshots

* feature (space) : manage failures to join in Space screen

* feature (space) : fix breaking tests after rebase

* feature (space) : some code clean up

* Follow permalinks to and from threads (element-hq#5414)

* Implement navigation to event inside a thread when a permalink is used

* Fix permalink navigation in threads to rooms

* Fix navigating to a different thread from a permalink in an existing thread

* Fix tests

* Add missing tests for thread navigation

* Reduce number of diff between ThreadedMessagesNode.kt and MessagesNode.kt

* Navigate back to the room when a link to the current room is clicked in a thread.

---------

Co-authored-by: Benoit Marty <[email protected]>
Co-authored-by: Benoit Marty <[email protected]>

* No need of DI for the LoadingNode.

* loadingNode: hide ProgressIndicator in some cases.

* Remove Progress from splashscreen to follow design requirements.

* Add Preview

* Update screenshots

* Add default value

* feature (space) : handle accept decline invite

* di : clean some code

* Improve Previews.

* misc (space) : ensure SpaceRoomList is destroyed

* Update screenshots

* Remove code duplication.

* Add modifier parameter.

* Add unit test on SpaceState

* misc (space) : update tests after rework

* fix(deps): update dependency net.java.dev.jna:jna to v5.18.1

* fix(deps): update dependency org.matrix.rustcomponents:sdk-android to v25.10.1

* Add unit test on SpacePresenter

* Add unit test on SpaceView

* We do not need CurrentSessionIdHolder anymore.
The SessionId can be provided by SessionMatrixModule and injected in constructors directly.

* fix(deps): update dependency io.mockk:mockk to v1.14.6

* Improve SignedOutPresenter.
We can now provide a SessionId in the Factory.

* Fix test.

* Remove unused dependency on `javax.inject:javax.inject`

* feature(space) : ensure RoomSummaryRow can display space invites

* Update SDK

* Let SpaceId be an alias of RoomId

* Enable leave space entry point.

* Leave space: Fix UI issue on top bar.

* Leave space: use the SDK API.

* feature(space) : filter space manually so we can show space invites

* Update screenshots

* Update screenshots

* Leave space: notify the room membership change

* Format

* Add Composable for a Beta label

* Add a way to use the primary color for the icon.

* Announcement for Spaces

* Update screenshots

* Add unit test on AnnouncementPresenter

* Add unit test on SpaceAnnouncementPresenter

* Remove unused Node

* Add test on DefaultAnnouncementService

* Use onContinue method in the back handler

* Add test on SpaceAnnouncementView

* Do not expose `AnnouncementState` in the api module

* Introduce Announcement enum.

* Improve code.

* Improve LeaveSpacePresenter and add a retry mechanism if loading the rooms fails.

* Use semantics colors.

* Space announcement: iterate on wording.

* Update screenshots

* Update screenshots

* Fix lint issue by removing old translations

* chore(deps): update gradle/actions action to v5 (element-hq#5444)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* fix(deps): update dependency io.sentry:sentry-android to v8.23.0 (element-hq#5442)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* Update dependency org.maplibre.gl:android-sdk to v12 (element-hq#5455)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* feature(space) : keep space children in the presenter

* Import Compound code from project https://github.com/element-hq/compound-android

* Import Compound scripts from project https://github.com/element-hq/compound-android

* fix(deps): update telephoto to v0.18.0

* Update compound scripts.

Ensure drawable folder is emptied before importing tokens and import again.

* We do not need to use support library

* Fix some quality issue

* Exclude generated files from being analyzed

* Add exception for Konsist test.

* Update ref in enterprise module.

* Fix lint issue

* Import Compound tests from project https://github.com/element-hq/compound-android

* Fix CI on screeshot recording

* Enable Spaces by default.

* Sync Strings from Localazy (element-hq#5460)

Co-authored-by: bmarty <[email protected]>

* Only offer to verify if a cross-signed device is available (element-hq#5433)

* Only offer to verify if a cross-signed device is available

* Fix tests

* use the right exception mapper

* adjust flag name and logic in ChooseSelfVerificationState

* add comment

* switch order of states to match previous logic

* Fix tests.

* fix(deps): update dependency com.posthog:posthog-android to v3.23.0 (element-hq#5463)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* fix(deps): update roborazzi to v1.50.0 (element-hq#5464)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

* feature(space): compute space room name locally

* Let MatrixClient exposes `val` instead of `fun` for the services.

* Naming convention

* Naming convention and use MatrixMediaLoader instead of MatrixClient for Coil factories.

* Use base type.

* Remove unused getCacheSize File receiver.

The path are manager by the sessionData now.

* appCoroutineScope does not have to be a class member.

* feature(space): introduce SpaceRoomVisibility and remove room count

* Update screenshots

* feature(space): fix space tests compilation

* Add Labs screen for beta testing of public features (element-hq#5465)

* Add Labs screen:

- Make `Feature` have an `isInLabs` boolean to distinguish private feature flags from public ones.
- Have `FeatureFlagsService` provide the list of available flags.
- Display the labs item in the settings screen only if there are available public features.
- Remove public feature toggles from developer options.
- Implement the labs screen with the public features.
- Add a clear cache step to the threads feature toggle
- Update screenshots

---------

Co-authored-by: ElementBot <[email protected]>

* Use "BETA" word from Localazy and ensure layout is correct in IconTitleSubtitleMolecule if the title is long.

* Improve current push provider test: give info about the distributor.

* Set a sound to the noisy notification channel

* Update Localazy config and sync all the strings.

* New notification sound banner

* Add isFreshInstall Boolean to allow the miration to behave in a different way for an application upgrade or a fresh install.
We cannot restore the previous code which existed because of element-hq#3535

* Show new notification sound banner logic

* Update screenshots

* Fix compilation issue in tests.

* fix(deps): update dependency org.matrix.rustcomponents:sdk-android to v25.10.7

* Remove duplicate Import class in RecoveryException

* Remove duplicated code.

* Fix API break.

* Map SpaceRoom.displayName

* feature(space): use SpaceRoom.displayName from sdk

* Update screenshots

* Reduce number of changes in preview.

* Update screenshots

* Improve AnnouncementService.

* Rename method.

* Rename key and value

* Improve InMemoryAnnouncementStore

* Extract AnnouncementStatus to its own file.

* Add missing tests on DefaultAnnouncementService

* Clean up and add unit test on `Theme.isDark()`

* Add a preview for all icons.
It will help to investigate icon rendering issue using Showkase browser.

* Update screenshots

* Fix import ordering

* Sync Strings from Localazy

* Remove dead code.

* feature(space): makes sure SpaceRoom is marked as Immutable

* feature(space): use room heroes for avatar

* Add Konsist test to check that `toPersistentList()` is not used.

Same for `toPersistentSet()` and `toPersistentMap()`.
Fix existing issues.

* Let SpaceRoom be stable the proper way.

* Improve and add previews.

* Update screenshots

* Format code.

* Disable Avatar cluster for now.

* Update screenshots

* feature(space): make sure to handle topic properly

* Reduce number of Preview for Avatar.
It will cover more cases, and it will limit the risk of conflict on screenshot, which happen each time we touch AvatarSize.

* Update screenshots

* misc(design) : introduce SimpleModalBottomSheet component

* feature(space): use the new SimpleModalBottomSheet for TopicViewer

* feature(space): better previews for Space screen

* Filter out direct room in the leave space screen. Closes element-hq#5496

* Update screenshots

* feature(space): add missing tests on SpaceEvents topic

* Fix Detekt issue.

* Sync strings.

* Fix compilation issue after string key renaming.

* Fix tests.

* Fix tests again

* feature(space): add SpaceViewTest related to topic

* Setting version for the release 25.10.0

* Adding fastlane file for version 25.10.0

* Fix conflicts

* Fix isDebugBuild

* Fix access_rules

* Build script using release mode

* Add builded SDK

* Import tokens

* Replace Workspace icons by Space

* Import tokens Script using Tchap repository

* Remove old compound-android submodule

* Fix Screenshots generation

* Version 0.2.0

* Update screenshots

* Fix PR

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: ganfra <[email protected]>
Co-authored-by: ganfra <[email protected]>
Co-authored-by: Jorge Martín <[email protected]>
Co-authored-by: Benoit Marty <[email protected]>
Co-authored-by: ElementBot <[email protected]>
Co-authored-by: Benoit Marty <[email protected]>
Co-authored-by: Benoit Marty <[email protected]>
Co-authored-by: ElementBot <[email protected]>
Co-authored-by: bmarty <[email protected]>
Co-authored-by: Andy Balaam <[email protected]>
Co-authored-by: Hubert Chathi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EXA: When no other devices are verified, don't offer "Use another device" for verification

2 participants