Skip to content

Commit

Permalink
Fix unresponsive soft-logout in the legacy flow. (#6329)
Browse files Browse the repository at this point in the history
- Delegate logic to the OnboardingCoordinator like the new flow does.
- Reset the credentials on every presentation of authVC.
- Make sure not to set the credentials after deleting the data.
  • Loading branch information
pixlwave authored and stefanceriu committed Jun 28, 2022
1 parent d164834 commit 19c0656
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 61 deletions.
5 changes: 5 additions & 0 deletions Riot/Modules/Authentication/AuthenticationViewController.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,9 @@
andPassword:(NSString * _Nullable)password
orSSOIdentityProvider:(SSOIdentityProvider * _Nullable)identityProvider;

/**
Notifies the delegate that the user would like to clear all data following a soft logout.
@param authenticationViewController The view controller that the user was shown.
*/
- (void)authenticationViewControllerDidRequestClearAllData:(AuthenticationViewController * _Nonnull)authenticationViewController;
@end;
44 changes: 1 addition & 43 deletions Riot/Modules/Authentication/AuthenticationViewController.m
Original file line number Diff line number Diff line change
Expand Up @@ -698,48 +698,6 @@ - (void)updateSoftLogoutClearDataContainerVisibility
}
}

- (void)showClearDataAfterSoftLogoutConfirmation
{
// Request confirmation
if (alert)
{
[alert dismissViewControllerAnimated:NO completion:nil];
}

alert = [UIAlertController alertControllerWithTitle:[VectorL10n authSoftlogoutClearDataSignOutTitle]
message:[VectorL10n authSoftlogoutClearDataSignOutMsg]
preferredStyle:UIAlertControllerStyleAlert];


[alert addAction:[UIAlertAction actionWithTitle:[VectorL10n authSoftlogoutClearDataSignOut]
style:UIAlertActionStyleDestructive
handler:^(UIAlertAction * action)
{
[self clearDataAfterSoftLogout];
}]];

MXWeakify(self);
[alert addAction:[UIAlertAction actionWithTitle:[VectorL10n cancel]
style:UIAlertActionStyleDefault
handler:^(UIAlertAction * action)
{
MXStrongifyAndReturnIfNil(self);
self->alert = nil;
}]];

[self presentViewController:alert animated:YES completion:nil];
}

- (void)clearDataAfterSoftLogout
{
MXLogDebug(@"[AuthenticationVC] clearDataAfterSoftLogout %@", self.softLogoutCredentials.userId);

// Use AppDelegate so that we reset app settings and this auth screen
[[AppDelegate theDelegate] logoutSendingRequestServer:YES completion:^(BOOL isLoggedOut) {
MXLogDebug(@"[AuthenticationVC] Complete. isLoggedOut: %@", @(isLoggedOut));
}];
}

/**
Filter and prioritise flows supported by the app.
Expand Down Expand Up @@ -1010,7 +968,7 @@ - (IBAction)onButtonPressed:(id)sender
}
else if (sender == self.softLogoutClearDataButton)
{
[self showClearDataAfterSoftLogoutConfirmation];
[self.authVCDelegate authenticationViewControllerDidRequestClearAllData:self];
}
else
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ final class LegacyAuthenticationCoordinator: NSObject, AuthenticationCoordinator
self.canPresentAdditionalScreens = parameters.canPresentAdditionalScreens

let authenticationViewController = AuthenticationViewController()
authenticationViewController.softLogoutCredentials = authenticationService.softLogoutCredentials
self.authenticationViewController = authenticationViewController

// Preload the view as this can a second and lock up the UI at presentation.
Expand All @@ -77,6 +76,8 @@ final class LegacyAuthenticationCoordinator: NSObject, AuthenticationCoordinator
func start() {
// Listen to the end of the authentication flow.
authenticationViewController.authVCDelegate = self
// Set (or clear) any soft-logout credentials.
authenticationViewController.softLogoutCredentials = authenticationService.softLogoutCredentials
// Listen for changes from deep links.
AuthenticationService.shared.delegate = self
}
Expand Down Expand Up @@ -207,6 +208,10 @@ extension LegacyAuthenticationCoordinator: AuthenticationViewControllerDelegate
authenticationFlow: authenticationViewController.authType.flow,
authenticationType: authenticationType))
}

func authenticationViewControllerDidRequestClearAllData(_ authenticationViewController: AuthenticationViewController) {
callback?(.clearAllData)
}
}

// MARK: - KeyVerificationCoordinatorDelegate
Expand Down
29 changes: 15 additions & 14 deletions Riot/Modules/Onboarding/OnboardingCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -233,13 +233,7 @@ final class OnboardingCoordinator: NSObject, OnboardingCoordinatorProtocol {
case .didComplete:
self.authenticationCoordinatorDidComplete(coordinator)
case .clearAllData:
self.showClearAllDataConfirmation {
MXLog.debug("[OnboardingCoordinator] beginAuthentication: clear all data after soft logout")
self.authenticationService.reset()
self.authenticationFinished = false
self.cancelAuthentication(flow: .login)
AppDelegate.theDelegate().logoutSendingRequestServer(true, completion: nil)
}
self.showClearAllDataConfirmation()
case .cancel(let flow):
self.cancelAuthentication(flow: flow)
}
Expand All @@ -264,7 +258,9 @@ final class OnboardingCoordinator: NSObject, OnboardingCoordinatorProtocol {
self.authenticationCoordinator(coordinator, didLoginWith: session, and: authenticationFlow, using: authenticationType)
case .didComplete:
self.authenticationCoordinatorDidComplete(coordinator)
case .didStart, .clearAllData, .cancel:
case .clearAllData:
self.showClearAllDataConfirmation()
case .didStart, .cancel:
// These results are only sent by the new flow.
break
}
Expand Down Expand Up @@ -599,16 +595,21 @@ final class OnboardingCoordinator: NSObject, OnboardingCoordinatorProtocol {
loadingIndicator = nil
}

/// Show confirmation to clear all data
/// - Parameter confirmed: Callback to be called when confirmed.
private func showClearAllDataConfirmation(_ confirmed: (() -> Void)?) {
/// Shows a confirmation to clear all data, and proceeds to do so if the user confirms.
private func showClearAllDataConfirmation() {
let alertController = UIAlertController(title: VectorL10n.authSoftlogoutClearDataSignOutTitle,
message: VectorL10n.authSoftlogoutClearDataSignOutMsg,
preferredStyle: .alert)
alertController.addAction(UIAlertAction(title: VectorL10n.cancel, style: .cancel, handler: nil))
alertController.addAction(UIAlertAction(title: VectorL10n.authSoftlogoutClearDataSignOut, style: .default, handler: { action in
confirmed?()
}))
alertController.addAction(UIAlertAction(title: VectorL10n.authSoftlogoutClearDataSignOut, style: .destructive) { [weak self] action in
guard let self = self else { return }
MXLog.debug("[OnboardingCoordinator] showClearAllDataConfirmation: clear all data after soft logout")
self.authenticationService.reset()
self.authenticationFinished = false
self.cancelAuthentication(flow: .login)
AppDelegate.theDelegate().logoutSendingRequestServer(true, completion: nil)
})

navigationRouter.present(alertController, animated: true)
}
}
Expand Down
12 changes: 9 additions & 3 deletions Riot/Modules/TabBar/MasterTabBarController.m
Original file line number Diff line number Diff line change
Expand Up @@ -487,9 +487,15 @@ - (void)showSoftLogoutOnboardingFlowWithCredentials:(MXCredentials*)credentials;
{
MXLogDebug(@"[MasterTabBarController] showAuthenticationScreenAfterSoftLogout");

AuthenticationService.shared.softLogoutCredentials = credentials;

[self showOnboardingFlowAndResetSessionFlags:NO];
// This method can be called after the user chooses to clear their data as the MXSession
// is opened to call logout from. So we only set the credentials when authentication isn't
// in progress to prevent a second soft logout screen being shown.
if (!self.onboardingCoordinatorBridgePresenter && !self.isOnboardingCoordinatorPreparing)
{
AuthenticationService.shared.softLogoutCredentials = credentials;

[self showOnboardingFlowAndResetSessionFlags:NO];
}
}

- (void)showOnboardingFlowAndResetSessionFlags:(BOOL)resetSessionFlags
Expand Down
1 change: 1 addition & 0 deletions changelog.d/5881.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Soft logout: Fix a bug where clearing all data from soft logout didn't present the login screen.

0 comments on commit 19c0656

Please sign in to comment.