Add forced logout flow when the proxy is no longer available #3458

jmartinesp · 2024-09-13T13:15:04Z

Content

Added MatrixClient.isSlidingSyncProxyAvailable() function.
Modifies ErrorDialog component so it can be made non-dismissable if needed, added a separate submit action from the dismiss one.
Check if a forced migration to native sliding sync is needed (the user is using the proxy, which is no longer available), and display the error dialog on top of all screens.
Rework the login process so it tries to log in using native sliding sync if available and enabled, then using the proxy. Also enable native sliding sync support by default.

Motivation and context

Second part of #3427.

Screenshots / GIFs

Tests

This is quite difficult to test unless you either change the check in LoggedInFlowNode or make the return value of Client::available_sliding_sync_versions() in Rust to just the native one. If you manage to do one of them:

Open the app, the dialog should be displayed.
It must not be dismissable and should prevent using the app.
Restarting the app should make no change.
Tapping on logout and upgrade should do exactly that.

Tested devices

Physical
Emulator
OS version(s): 14

Checklist

Changes have been tested on an Android device or Android emulator with API 23
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
Pull request includes a sign off
You've made a self review of your PR

…or the submit action. Also make the title text optional and dismissing the dialog by tapping outside/going back configurable.

…longer available. In that case, display the non-dismissable dialog and force the user to log out after enabling SSS.

1. Always try native/simplified sliding sync login first, if available. 2. Then, if it wasn't available or failed with an sliding sync not supported error, try with the proxy instead (either discovered proxy or forced custom one).

github-actions · 2024-09-13T13:25:04Z

📱 Scan the QR code below to install the build (arm64 only) for this PR.

If you can't scan the QR code you can install the build via this link: https://i.diawi.com/n5VjnC

codecov · 2024-09-13T13:40:51Z

Codecov Report

Attention: Patch coverage is 88.23529% with 6 lines in your changes missing coverage. Please review.

Project coverage is 82.63%. Comparing base (da3f5e0) to head (b6bc7cb).
Report is 1 commits behind head on develop.

Files with missing lines	Patch %	Lines
...io/element/android/appnav/loggedin/LoggedInView.kt	84.61%	1 Missing and 1 partial ⚠️
...ement/android/appnav/loggedin/LoggedInPresenter.kt	80.00%	0 Missing and 1 partial ⚠️
...nt/android/features/leaveroom/api/LeaveRoomView.kt	0.00%	1 Missing ⚠️
...d/features/lockscreen/impl/unlock/PinUnlockView.kt	50.00%	1 Missing ⚠️
...nfirmaccountprovider/ConfirmAccountProviderView.kt	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3458      +/-   ##
===========================================
+ Coverage    82.62%   82.63%   +0.01%     
===========================================
  Files         1701     1701              
  Lines        40015    40047      +32     
  Branches      4868     4872       +4     
===========================================
+ Hits         33061    33092      +31     
- Misses        5233     5234       +1     
  Partials      1721     1721

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

ganfra

Some remarks, otherwise LGTM

ganfra · 2024-09-13T15:33:07Z

...ies/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt

+    override suspend fun isSlidingSyncProxySupported(): Boolean {
+        return client.availableSlidingSyncVersions().any { it is SlidingSyncVersion.Proxy }


Can't this fail?

Apparently not, if the SDK can't load the data (network error, one of the config files not existing...) it just returns an empty list.

ganfra · 2024-09-13T15:42:03Z

appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt

Can we try to move logic to the LoggedInPresenter and LoggedInView?

I can try 🤞 .

ganfra · 2024-09-13T15:42:50Z

features/login/impl/src/main/res/values-be/translations.xml

PR should avoid updating all translations, just the ones you need for your work would be better :)

That's actually what I did, but I had to remove the translations in other languages for a couple of strings that are no longer in the project because otherwise the code quality checks wouldn't pass.

This also allows some extra testing

sonarcloud · 2024-09-16T08:40:37Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

jmartinesp and others added 9 commits September 13, 2024 12:51

Add MatrixClient.isSlidingSyncProxySupported function

216b421

Update localazy strings

df4867f

Modify ErrorDialog to have an onSubmit call, which will be used f…

185e223

…or the submit action. Also make the title text optional and dismissing the dialog by tapping outside/going back configurable.

Check if a forced migration to SSS is needed because the proxy is no …

b004ac3

…longer available. In that case, display the non-dismissable dialog and force the user to log out after enabling SSS.

Enable native/simplified sliding sync by default.

8bc8e6f

Refactor the login to make sure we:

c45930c

1. Always try native/simplified sliding sync login first, if available. 2. Then, if it wasn't available or failed with an sliding sync not supported error, try with the proxy instead (either discovered proxy or forced custom one).

Fix sample project

3802f69

Fix translations no longer having a default value

e87ec5a

Update screenshots

ab92c8f

jmartinesp added the PR-Feature For a new feature label Sep 13, 2024

jmartinesp requested a review from a team as a code owner September 13, 2024 13:15

jmartinesp requested review from ganfra and removed request for a team September 13, 2024 13:15

ganfra approved these changes Sep 13, 2024

View reviewed changes

SpeedyFireCyclone mentioned this pull request Sep 14, 2024

Can't sign in via Authentik SSO, only username/password #3452

Closed

Move logic to LoggedInPresenter and the UI to LoggedInView

b6b286a

This also allows some extra testing

jmartinesp added the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Sep 16, 2024

github-actions bot removed the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Sep 16, 2024

Update screenshots

b6bc7cb

jmartinesp added the Run-Maestro Starts a Maestro Cloud session to run integration tests label Sep 16, 2024

github-actions bot removed the Run-Maestro Starts a Maestro Cloud session to run integration tests label Sep 16, 2024

jmartinesp enabled auto-merge (squash) September 16, 2024 08:50

jmartinesp merged commit 663362a into develop Sep 16, 2024
30 checks passed

jmartinesp deleted the feat/jme/implement-forced-logout-for-proxy-not-available-use-case branch September 16, 2024 09:13

manuroe mentioned this pull request Sep 17, 2024

[Task] SSS Migration: Force logout if the SS proxy is no more available #3456

Closed

tonkku107 mentioned this pull request Sep 23, 2024

Native sliding sync migration banner does not show up on an old session #3468

Closed

jmartinesp mentioned this pull request Sep 24, 2024

Simplified Sliding Sync should be enabled by default if your homeserver supports it (and doesn't have a SS proxy). #3256

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add forced logout flow when the proxy is no longer available #3458

Add forced logout flow when the proxy is no longer available #3458

jmartinesp commented Sep 13, 2024

github-actions bot commented Sep 13, 2024 •

edited

Loading

codecov bot commented Sep 13, 2024 •

edited

Loading

ganfra left a comment

ganfra Sep 13, 2024

jmartinesp Sep 16, 2024

ganfra Sep 13, 2024

jmartinesp Sep 16, 2024

jmartinesp Sep 16, 2024

ganfra Sep 13, 2024

jmartinesp Sep 16, 2024 •

edited

Loading

sonarcloud bot commented Sep 16, 2024

		override suspend fun isSlidingSyncProxySupported(): Boolean {
		return client.availableSlidingSyncVersions().any { it is SlidingSyncVersion.Proxy }

Add forced logout flow when the proxy is no longer available #3458

Add forced logout flow when the proxy is no longer available #3458

Conversation

jmartinesp commented Sep 13, 2024

Content

Motivation and context

Screenshots / GIFs

Tests

Tested devices

Checklist

github-actions bot commented Sep 13, 2024 • edited Loading

codecov bot commented Sep 13, 2024 • edited Loading

Codecov Report

ganfra left a comment

Choose a reason for hiding this comment

ganfra Sep 13, 2024

Choose a reason for hiding this comment

jmartinesp Sep 16, 2024

Choose a reason for hiding this comment

ganfra Sep 13, 2024

Choose a reason for hiding this comment

jmartinesp Sep 16, 2024

Choose a reason for hiding this comment

jmartinesp Sep 16, 2024

Choose a reason for hiding this comment

ganfra Sep 13, 2024

Choose a reason for hiding this comment

jmartinesp Sep 16, 2024 • edited Loading

Choose a reason for hiding this comment

sonarcloud bot commented Sep 16, 2024

Quality Gate passed

github-actions bot commented Sep 13, 2024 •

edited

Loading

codecov bot commented Sep 13, 2024 •

edited

Loading

jmartinesp Sep 16, 2024 •

edited

Loading