Skip to content

Commit

Permalink
PM-14547: Remove soft logged out account (#1150)
Browse files Browse the repository at this point in the history
Co-authored-by: Ezimet Ozkhan <[email protected]>
  • Loading branch information
matt-livefront and ezimet-livefront authored Dec 12, 2024
1 parent 74baaeb commit 361cc87
Show file tree
Hide file tree
Showing 10 changed files with 439 additions and 24 deletions.
62 changes: 48 additions & 14 deletions BitwardenShared/UI/Auth/Extensions/Alert+Auth.swift
Original file line number Diff line number Diff line change
Expand Up @@ -77,37 +77,49 @@ extension Alert {
)
}

/// Display the options to log out of or lock the selected profile switcher item.
/// Display the options to log out of, lock, or remove the selected profile switcher item.
///
/// - Parameters:
/// - item: The selected item from the profile switcher view.
/// - lockAction: The action to perform if the user chooses to lock the account.
/// - logoutAction: The action to perform if the user chooses to log out of the account.
/// - removeAccountAction: The action to perform if the user chooses to remove the account.
///
/// - Returns: An alert displaying the options for the item.
///
static func accountOptions(
_ item: ProfileSwitcherItem,
lockAction: @escaping () async -> Void,
logoutAction: @escaping () async -> Void
logoutAction: @escaping () async -> Void,
removeAccountAction: @escaping () async -> Void
) -> Alert {
// All accounts have the option to log out, but only display the lock option if
// the account is not currently locked.
var alertActions = [
AlertAction(
title: Localizations.logOut,
style: .default,
handler: { _, _ in await logoutAction() }
),
]
var alertActions = [AlertAction]()

if item.isUnlocked, item.canBeLocked {
alertActions.insert(
alertActions.append(
AlertAction(
title: Localizations.lock,
style: .default,
handler: { _, _ in await lockAction() }
),
at: 0
)
)
}

if item.isLoggedOut {
alertActions.append(
AlertAction(
title: Localizations.removeAccount,
style: .default,
handler: { _, _ in await removeAccountAction() }
)
)
} else {
alertActions.append(
AlertAction(
title: Localizations.logOut,
style: .default,
handler: { _, _ in await logoutAction() }
)
)
}

Expand Down Expand Up @@ -215,6 +227,28 @@ extension Alert {
)
}

/// An alert that is displayed to confirm the user wants to remove the account.
///
/// - Parameters:
/// - profile: The profile switcher item to remove.
/// - action: An action to perform when the user taps `Yes`, to confirm removing the account.
/// - Returns: An alert that is displayed to confirm the user wants to remove the account.
///
static func removeAccountConfirmation(
_ profile: ProfileSwitcherItem,
action: @escaping () async -> Void
) -> Alert {
Alert(
title: Localizations.removeAccount,
message: Localizations.removeAccountConfirmation + "\n\n"
+ [profile.email, profile.webVault].joined(separator: "\n"),
alertActions: [
AlertAction(title: Localizations.yes, style: .default) { _ in await action() },
AlertAction(title: Localizations.cancel, style: .cancel),
]
)
}

/// An alert confirming that the user wants to finish setting up autofill later in settings.
///
/// - Parameter action: The action taken when the user taps on Confirm to finish setting up
Expand Down
75 changes: 70 additions & 5 deletions BitwardenShared/UI/Auth/Extensions/AlertAuthTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@ import XCTest
@testable import BitwardenShared

class AlertAuthTests: BitwardenTestCase {
/// `accountOptions(_:lockAction:logoutAction:)`
func test_accountOptions() {
/// `accountOptions(_:lockAction:logoutAction:removeAccountAction:)`
func test_accountOptions() async throws {
var actions = [String]()
let subject = Alert.accountOptions(
.fixture(email: "[email protected]", isUnlocked: true, webVault: "secureVault.example.com"),
lockAction: {},
logoutAction: {}
lockAction: { actions.append(Localizations.lock) },
logoutAction: { actions.append(Localizations.logOut) },
removeAccountAction: { actions.append(Localizations.removeAccount) }
)

XCTAssertEqual(subject.title, "[email protected]\nsecureVault.example.com")
Expand All @@ -18,6 +20,43 @@ class AlertAuthTests: BitwardenTestCase {
XCTAssertEqual(subject.alertActions[0].title, Localizations.lock)
XCTAssertEqual(subject.alertActions[1].title, Localizations.logOut)
XCTAssertEqual(subject.alertActions[2].title, Localizations.cancel)

try await subject.tapAction(title: Localizations.lock)
XCTAssertEqual(actions, [Localizations.lock])
actions.removeAll()

try await subject.tapAction(title: Localizations.logOut)
XCTAssertEqual(actions, [Localizations.logOut])
actions.removeAll()

try await subject.tapAction(title: Localizations.cancel)
XCTAssertTrue(actions.isEmpty)
}

/// `accountOptions(_:lockAction:logoutAction:removeAccountAction:)` shows the account options
/// for a logged out account.
func test_accountOptions_loggedOut() async throws {
var actions = [String]()
let subject = Alert.accountOptions(
.fixture(email: "[email protected]", isLoggedOut: true, webVault: "secureVault.example.com"),
lockAction: { actions.append(Localizations.lock) },
logoutAction: { actions.append(Localizations.logOut) },
removeAccountAction: { actions.append(Localizations.removeAccount) }
)

XCTAssertEqual(subject.title, "[email protected]\nsecureVault.example.com")
XCTAssertNil(subject.message)
XCTAssertEqual(subject.preferredStyle, .actionSheet)
XCTAssertEqual(subject.alertActions.count, 2)
XCTAssertEqual(subject.alertActions[0].title, Localizations.removeAccount)
XCTAssertEqual(subject.alertActions[1].title, Localizations.cancel)

try await subject.tapAction(title: Localizations.removeAccount)
XCTAssertEqual(actions, [Localizations.removeAccount])
actions.removeAll()

try await subject.tapAction(title: Localizations.cancel)
XCTAssertTrue(actions.isEmpty)
}

/// `accountOptions(_:lockAction:logoutAction:)` shows the account options for can not be locked account.
Expand All @@ -31,7 +70,8 @@ class AlertAuthTests: BitwardenTestCase {
webVault: "secureVault.example.com"
),
lockAction: {},
logoutAction: {}
logoutAction: {},
removeAccountAction: {}
)

XCTAssertEqual(subject.title, "[email protected]\nsecureVault.example.com")
Expand Down Expand Up @@ -187,6 +227,31 @@ class AlertAuthTests: BitwardenTestCase {
await fulfillment(of: [expectation], timeout: 3)
}

/// `removeAccountConfirmation(action:)` constructs an `Alert` used to confirm that the user
/// wants to remove the account.
func test_removeAccountConfirmation() async throws {
var actionCalled = false
let subject = Alert.removeAccountConfirmation(.fixture(email: "[email protected]")) {
actionCalled = true
}

XCTAssertEqual(subject.title, Localizations.removeAccount)
XCTAssertEqual(
subject.message,
Localizations.removeAccountConfirmation + "\n\n" + "[email protected]\nvault.bitwarden.com"
)
XCTAssertEqual(subject.preferredStyle, .alert)
XCTAssertEqual(subject.alertActions.count, 2)
XCTAssertEqual(subject.alertActions[0].title, Localizations.yes)
XCTAssertEqual(subject.alertActions[1].title, Localizations.cancel)

try await subject.tapAction(title: Localizations.cancel)
XCTAssertFalse(actionCalled)

try await subject.tapAction(title: Localizations.yes)
XCTAssertTrue(actionCalled)
}

/// `setUpAutoFillLater(action:)` builds an `Alert` for setting up autofill later.
func test_setUpAutoFillLater() async throws {
var actionCalled = false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ import SwiftUI
enum ProfileSwitcherAccessibilityAction: Equatable {
/// The account should be logged out.
case logout(ProfileSwitcherItem)

/// The account should be removed.
case remove(ProfileSwitcherItem)
}

// MARK: - ProfileSwitcherAction
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ extension ProfileSwitcherHandler {
switch accessibilityAction {
case let .logout(account):
confirmLogout(account)
case let .remove(account):
confirmRemoveAccount(account)
}
case .backgroundPressed:
profileSwitcherState.isVisible = false
Expand Down Expand Up @@ -139,6 +141,19 @@ private extension ProfileSwitcherHandler {
)
}

/// Confirms that the user would like to log out of an account by presenting an alert.
///
/// - Parameter profile: The profile switcher item for the account to be logged out.
///
func confirmRemoveAccount(_ profile: ProfileSwitcherItem) {
showAlert(
.removeAccountConfirmation(profile) { [weak self] in
guard let self else { return }
await removeAccount(profile)
}
)
}

/// Handles a long press of an account in the profile switcher.
///
/// - Parameter account: The `ProfileSwitcherItem` long pressed by the user.
Expand All @@ -153,6 +168,9 @@ private extension ProfileSwitcherHandler {
},
logoutAction: {
self.confirmLogout(account)
},
removeAccountAction: {
self.confirmRemoveAccount(account)
}
)
)
Expand Down Expand Up @@ -203,6 +221,34 @@ private extension ProfileSwitcherHandler {
}
}

/// Remove an account.
///
/// - Parameter account: The profile switcher item for the account to be removed.
///
func removeAccount(_ account: ProfileSwitcherItem) async {
do {
let activeAccountId = try await profileServices.authRepository.getUserId()

if account.userId == activeAccountId {
// If the active account is being removed, forward it to the router to handle
// removing the account and any navigation associated with it (e.g. switch to next
// active account).
// A user-initiated logout functions the same as removing the account.
await handleAuthEvent(.action(.logout(userId: account.userId, userInitiated: true)))
} else {
// Otherwise, if it's an inactive account, it can be removed directly.
// A user-initiated logout functions the same as removing the account.
try await profileServices.authRepository.logout(userId: account.userId, userInitiated: true)
toast = Toast(title: Localizations.accountRemovedSuccessfully)

// Update the profile switcher view.
await refreshProfileState()
}
} catch {
profileServices.errorReporter.log(error: error)
}
}

/// A profile switcher row appeared.
///
/// - Parameter rowType: The row type that appeared.
Expand Down
Loading

0 comments on commit 361cc87

Please sign in to comment.