Fixes broken SoftLogout UX for homeservers that support both Password and SSO #5398
Conversation
Thanks for the comments, I pushed some changes addressing them. Regarding testing against a real setup, I agree that this would be ideal to do before merging. I'll try tomorrow if I can get this sandbox homeserver set up. As for remembering social login used, I tried this but this needs much more consideration from a technical standpoint. We could persist which social was used in the database but I think this is the wrong way to go about it. I can see in the code that a recover session through SSO has been set up but not fully implemented. I tried to use this but the implementation was too unclear for me to extend |
…gout-ux-broken # Conflicts: # vector/src/main/java/im/vector/app/features/login/LoginActivity.kt # vector/src/main/java/im/vector/app/features/signout/soft/SoftLogoutController.kt
|
@ericdecanini I tried this out, and it no longer crashes but it also doesnt fully solve the issue. It now continues in a login loop. edit: I just also read your last comment, it seems that the |
@mikhail5555 Thanks for testing it! Yeah setting a low |
|
@ericdecanini can you contact me on matrix at |
|
@ericdecanini I managed to setup a test server for you which i can provide you with a test account. With the only (major) config difference being the the session time is extreme short (1m). Please contact me through matrix and we can exchange some credentials and other info ;) |
…gout-ux-broken # Conflicts: # matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/db/SessionParamsMapper.kt # matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/registration/DefaultRegistrationWizard.kt # vector/src/main/java/im/vector/app/features/login/LoginActivity.kt # vector/src/main/java/im/vector/app/features/signout/soft/SoftLogoutController.kt # vector/src/main/java/im/vector/app/features/signout/soft/SoftLogoutViewModel.kt
|
It must be known that nobody in the team has the capacity at the moment to take care of the issue of SSO login after soft logout causing an infinite loop. After a discussion with @bmarty we determined that this is at least a better outcome than the previous crashing the app had (which also covered soft logout with password which should now be working on this branch) so we'll work towards getting this merged. We can then open a new issue to address the sso infinite loop in isolation |
There was a problem hiding this comment.
LGTM, thanks.
@BillCarsonFr can you also review so that we can merge this PR? Thanks!
| @@ -252,7 +253,7 @@ open class LoginActivity : VectorBaseActivity<ActivityLoginBinding>(), UnlockedA | |||
| // It depends on the LoginMode | |||
| when (state.loginMode) { | |||
| LoginMode.Unknown, | |||
| is LoginMode.Sso -> error("Developer error") | |||
| is LoginMode.Sso -> launchSsoFlow() | |||
…gout-ux-broken # Conflicts: # matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/DefaultAuthenticationService.kt # matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/registration/DefaultRegistrationWizard.kt # vector/src/main/java/im/vector/app/features/login/LoginActivity.kt # vector/src/main/java/im/vector/app/features/signout/soft/SoftLogoutController.kt
ericdecanini
force-pushed
the
bugfix/eric/softlogout-ux-broken
branch
from
0ccc8ba
to
4cf97d4
Compare
|
Force pushed after failed rebase (resulting in 1.8k files changed) |
|
SonarCloud Quality Gate failed. |
Type of change
Content
Fixes an error in SoftLogout where if your LoginMode was SsoAndPassword, no form would show up on the soft logout screen.
This was done by adding a new field LoginType in SessionParamsEntity which remembers the method the user used to log in.
Motivation and context
Merging this PR closes #5311
I also took this as an opportunity to refactor some classes around the session creation in order to make them easier to read and more testable & maintainable.
I did try to add tests for this too, but due to hardware constraints I couldn't test them on my local machine and using the CI didn't give me the info I need to properly write them to pass (failing because of MockkException, NoClassDefFoundException, etc) so I removed them in this commit and added an issue to get back to this as soon as I'm able to.
Lastly, this PR also fixes a bug where SSO login after a soft logout would crash the app. I would believe the SSO journey to be fixed but currently there's no true way to test this with both the client and server knowing that the user had soft logged out. The closest we can achieve is to throw a global error on the client side as per the test instructions below.
I attached a gif showing this journey which ultimately ends back in the soft logout screen (assumably because the global error gets thrown again or there's no log in actually happen because the user is already logged in according to the server) so we can only base this fix off of assumptions here
Screenshots / GIFs
SSO Journey:
Tests
There is no current way to properly test this without mocking the situation with a few code changes.
In SoftLogoutController above line 155 add this line:
globalErrorReceiver.handleGlobalError(GlobalError.InvalidToken(softLogout = true))
Run the app and login. You'll immediately be taken to the Soft Logout screen as shown above (though the server doesn't know of any logout happening, it only shows in the app)
Test this with a password account and an SSO account
Tested devices
Checklist