Skip to content

Commit

Permalink
BIT-2366: Alert user to switch to existing account with matching emai…
Browse files Browse the repository at this point in the history
…l on login (#688)
  • Loading branch information
matt-livefront authored Jun 27, 2024
1 parent eca5364 commit 54f855c
Show file tree
Hide file tree
Showing 16 changed files with 307 additions and 71 deletions.
19 changes: 19 additions & 0 deletions BitwardenShared/Core/Auth/Repositories/AuthRepository.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,14 @@ protocol AuthRepository: AnyObject {
///
func deleteAccount(otp: String?, passwordText: String?) async throws

/// Returns the user ID of an existing account that is already logged in on the device matching
/// the specified email.
///
/// - Parameter email: The email for the account to check.
/// - Returns: The user ID of the account that is already logged in on the device, or `nil` otherwise.
///
func existingAccountUserId(email: String) async -> String?

/// Gets the account for a user id.
///
/// - Parameter userId: The user Id to be mapped to an account.
Expand Down Expand Up @@ -464,6 +472,17 @@ extension DefaultAuthRepository: AuthRepository {
await vaultTimeoutService.remove(userId: nil)
}

func existingAccountUserId(email: String) async -> String? {
let matchingUserIds = await stateService.getUserIds(email: email)
for userId in matchingUserIds {
if let baseUrl = try? await stateService.getEnvironmentUrls(userId: userId)?.base,
baseUrl == environmentService.baseURL {
return userId
}
}
return nil
}

func getAccount(for userId: String?) async throws -> Account {
try await stateService.getAccount(userId: userId)
}
Expand Down
62 changes: 62 additions & 0 deletions BitwardenShared/Core/Auth/Repositories/AuthRepositoryTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,68 @@ class AuthRepositoryTests: BitwardenTestCase { // swiftlint:disable:this type_bo
XCTAssertEqual(client.requests[0].url, URL(string: "https://example.com/api/accounts"))
}

/// `existingAccountUserId(email:)` returns the user ID of the existing account with the same
/// email and base URLs.
func test_existingAccountUserId() async throws {
environmentService.baseURL = try XCTUnwrap(EnvironmentUrlData.defaultUS.base)
stateService.activeAccount = .fixture(profile: .fixture(email: "[email protected]", userId: "1"))
stateService.environmentUrls["1"] = .defaultUS
stateService.userIds = ["1"]

let userId = await subject.existingAccountUserId(email: "[email protected]")

XCTAssertEqual(userId, "1")
}

/// `existingAccountUserId(email:)` returns `nil` if getting the environment URLs throws an error.
func test_existingAccountUserId_getEnvironmentUrlsError() async throws {
environmentService.baseURL = try XCTUnwrap(EnvironmentUrlData.defaultUS.base)
stateService.activeAccount = .fixture(profile: .fixture(email: "[email protected]", userId: "1"))
stateService.environmentUrlsError = StateServiceError.noAccounts
stateService.userIds = ["1"]

let userId = await subject.existingAccountUserId(email: "[email protected]")

XCTAssertNil(userId)
}

/// `existingAccountUserId(email:)` returns `nil` if there's an existing account with the same
/// email but the base URLs are different.
func test_existingAccountUserId_matchingAccountDifferentBaseUrl() async throws {
environmentService.baseURL = try XCTUnwrap(EnvironmentUrlData.defaultEU.base)
stateService.activeAccount = .fixture(profile: .fixture(email: "[email protected]", userId: "1"))
stateService.environmentUrls["1"] = .defaultUS
stateService.userIds = ["1"]

let userId = await subject.existingAccountUserId(email: "[email protected]")

XCTAssertNil(userId)
}

/// `existingAccountUserId(email:)` returns the matching user ID with the same base URL, if
/// there are multiple matches for the user's email.
func test_existingAccountUserId_multipleMatching() async throws {
stateService.activeAccount = .fixture(profile: .fixture(email: "[email protected]", userId: "1"))
stateService.environmentUrls["1"] = .defaultUS
stateService.environmentUrls["2"] = .defaultEU
stateService.userIds = ["1", "2"]

environmentService.baseURL = try XCTUnwrap(EnvironmentUrlData.defaultUS.base)
var userId = await subject.existingAccountUserId(email: "[email protected]")
XCTAssertEqual(userId, "1")

environmentService.baseURL = try XCTUnwrap(EnvironmentUrlData.defaultEU.base)
userId = await subject.existingAccountUserId(email: "[email protected]")
XCTAssertEqual(userId, "2")
}

/// `existingAccountUserId(email:)` returns `nil` if there isn't an account that matches the email.
func test_existingAccountUserId_noMatchingAccount() async throws {
let userId = await subject.existingAccountUserId(email: "[email protected]")

XCTAssertNil(userId)
}

/// `allowBioMetricUnlock(:)` throws an error if required.
func test_allowBioMetricUnlock_biometricsRepositoryError() async throws {
biometricsRepository.setBiometricUnlockKeyError = BiometricsServiceError.setAuthKeyFailed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ class MockAuthRepository: AuthRepository { // swiftlint:disable:this type_body_l
var deviceId: String = ""
var email: String = ""
var encryptedPin: String = "123"
var existingAccountUserIdEmail: String?
var existingAccountUserIdResult: String?
var fingerprintPhraseResult: Result<String, Error> = .success("fingerprint")
var activeAccount: Account?
var altAccounts = [Account]()
Expand Down Expand Up @@ -99,6 +101,11 @@ class MockAuthRepository: AuthRepository { // swiftlint:disable:this type_body_l
try deleteAccountResult.get()
}

func existingAccountUserId(email: String) async -> String? {
existingAccountUserIdEmail = email
return existingAccountUserIdResult
}

func getAccount(for userId: String?) async throws -> Account {
if let getAccountError {
throw getAccountError
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ protocol EnvironmentService {
/// The URL for the API.
var apiURL: URL { get }

/// The environment's base URL.
var baseURL: URL { get }

/// The URL for the events API.
var eventsURL: URL { get }

Expand Down Expand Up @@ -109,6 +112,10 @@ extension DefaultEnvironmentService {
environmentUrls.apiURL
}

var baseURL: URL {
environmentUrls.baseURL
}

var eventsURL: URL {
environmentUrls.eventsURL
}
Expand Down
13 changes: 13 additions & 0 deletions BitwardenShared/Core/Platform/Services/StateService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,13 @@ protocol StateService: AnyObject {
///
func getUserHasMasterPassword(userId: String?) async throws -> Bool

/// Gets the user ID of any accounts with the specified email.
///
/// - Parameter email: The email of the account.
/// - Returns: A list of user IDs for the accounts with a matching email.
///
func getUserIds(email: String) async -> [String]

/// Gets the username generation options for a user ID.
///
/// - Parameter userId: The user ID associated with the username generation options.
Expand Down Expand Up @@ -1186,6 +1193,12 @@ actor DefaultStateService: StateService { // swiftlint:disable:this type_body_le
try getAccount(userId: userId).profile.userDecryptionOptions?.hasMasterPassword ?? true
}

func getUserIds(email: String) async -> [String] {
guard let state = appSettingsStore.state else { return [] }
let userIds = state.accounts.values.filter { $0.profile.email == email }.map(\.profile.userId)
return userIds
}

func getUsernameGenerationOptions(userId: String?) async throws -> UsernameGenerationOptions? {
let userId = try userId ?? getActiveAccountUserId()
return appSettingsStore.usernameGenerationOptions(userId: userId)
Expand Down
49 changes: 49 additions & 0 deletions BitwardenShared/Core/Platform/Services/StateServiceTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -729,6 +729,55 @@ class StateServiceTests: BitwardenTestCase { // swiftlint:disable:this type_body
XCTAssertTrue(userHasMasterPassword)
}

/// `getUserIds(email:)` returns the user ID of any users with a matching email.
func test_getUserIds() async {
appSettingsStore.state = State(
accounts: [
"1": .fixture(profile: .fixture(email: "[email protected]", userId: "1")),
"2": .fixture(profile: .fixture(email: "[email protected]", userId: "2")),
"3": .fixture(profile: .fixture(email: "[email protected]", userId: "3")),
]
)

let user1Ids = await subject.getUserIds(email: "[email protected]")
XCTAssertEqual(user1Ids, ["1"])

let user3Ids = await subject.getUserIds(email: "[email protected]")
XCTAssertEqual(user3Ids, ["3"])
}

/// `getUserIds(email:)` returns multiple user IDs if they all have a matching email.
func test_getUserIds_multiple() async {
appSettingsStore.state = State(
accounts: [
"1": .fixture(profile: .fixture(email: "[email protected]", userId: "1")),
"2": .fixture(profile: .fixture(email: "[email protected]", userId: "2")),
"3": .fixture(profile: .fixture(email: "[email protected]", userId: "3")),
]
)

let userIds = await subject.getUserIds(email: "[email protected]")
XCTAssertEqual(userIds.sorted(), ["1", "2", "3"])
}

/// `getUserIds(email:)` returns `nil` if there isn't a user with a matching email.
func test_getUserIds_noMatchingUser() async {
appSettingsStore.state = State(
accounts: [
"1": .fixture(profile: .fixture(email: "[email protected]", userId: "1")),
]
)

let userIds = await subject.getUserIds(email: "[email protected]")
XCTAssertTrue(userIds.isEmpty)
}

/// `getUserIds(email:)` returns `nil` if there are no other users.
func test_getUserIds_noUsers() async {
let userIds = await subject.getUserIds(email: "[email protected]")
XCTAssertTrue(userIds.isEmpty)
}

/// `getUsernameGenerationOptions()` gets the saved username generation options for the account.
func test_getUsernameGenerationOptions() async throws {
let options1 = UsernameGenerationOptions(plusAddressedEmail: "[email protected]")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ class MockEnvironmentService: EnvironmentService {
var setPreAuthEnvironmentUrlsData: EnvironmentUrlData?

var apiURL = URL(string: "https://example.com/api")!
var baseURL = URL(string: "https://example.com")!
var eventsURL = URL(string: "https://example.com/events")!
var iconsURL = URL(string: "https://example.com/icons")!
var identityURL = URL(string: "https://example.com/identity")!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class MockStateService: StateService { // swiftlint:disable:this type_body_lengt
var doesActiveAccountHavePremiumResult: Result<Bool, Error> = .success(true)
var encryptedPinByUserId = [String: String]()
var environmentUrls = [String: EnvironmentUrlData]()
var environmentUrlsError: Error?
var forcePasswordResetReason = [String: ForcePasswordResetReason]()
var lastActiveTime = [String: Date]()
var loginRequest: LoginRequestNotification?
Expand Down Expand Up @@ -58,6 +59,7 @@ class MockStateService: StateService { // swiftlint:disable:this type_body_lengt
var updateProfileResponse: ProfileResponseModel?
var updateProfileUserId: String?
var userHasMasterPassword = [String: Bool]()
var userIds = [String]()
var usernameGenerationOptions = [String: UsernameGenerationOptions]()
var vaultTimeout = [String: SessionTimeoutValue]()

Expand Down Expand Up @@ -177,6 +179,9 @@ class MockStateService: StateService { // swiftlint:disable:this type_body_lengt
}

func getEnvironmentUrls(userId: String?) async throws -> EnvironmentUrlData? {
if let environmentUrlsError {
throw environmentUrlsError
}
let userId = try unwrapUserId(userId)
return environmentUrls[userId]
}
Expand Down Expand Up @@ -251,6 +256,10 @@ class MockStateService: StateService { // swiftlint:disable:this type_body_lengt
return userHasMasterPassword[userId] ?? true
}

func getUserIds(email: String) async -> [String] {
userIds
}

func getUsernameGenerationOptions(userId: String?) async throws -> UsernameGenerationOptions? {
let userId = try unwrapUserId(userId)
return usernameGenerationOptions[userId]
Expand Down
20 changes: 20 additions & 0 deletions BitwardenShared/UI/Auth/Extensions/Alert+Auth.swift
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,26 @@ extension Alert {
)
}

/// An alert asking the user if they want to switch to the already existing account when adding
/// a new account.
///
/// - Parameter action: The action taken if the user wants to switch to the existing account.
/// - Returns: An alert asking the user if they want to switch to the already existing account
/// when adding a new account.
///
static func switchToExistingAccount(
action: @escaping () async -> Void
) -> Alert {
Alert(
title: Localizations.accountAlreadyAdded,
message: Localizations.switchToAlreadyAddedAccountConfirmation,
alertActions: [
AlertAction(title: Localizations.cancel, style: .cancel),
AlertAction(title: Localizations.yes, style: .default) { _ in await action() },
]
)
}

/// An alert notifying the user that they have unassigned ciphers.
///
/// - Parameters:
Expand Down
21 changes: 21 additions & 0 deletions BitwardenShared/UI/Auth/Extensions/AlertAuthTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -164,4 +164,25 @@ class AlertAuthTests: BitwardenTestCase {

await fulfillment(of: [expectation], timeout: 3)
}

/// `switchToExistingAccount(action:)` builds an `Alert` for switching to an existing account.
func test_switchToExistingAccount() async throws {
var actionCalled = false
let subject = Alert.switchToExistingAccount {
actionCalled = true
}

XCTAssertEqual(subject.title, Localizations.accountAlreadyAdded)
XCTAssertEqual(subject.message, Localizations.switchToAlreadyAddedAccountConfirmation)
XCTAssertEqual(subject.preferredStyle, .alert)
XCTAssertEqual(subject.alertActions.count, 2)
XCTAssertEqual(subject.alertActions[0].title, Localizations.cancel)
XCTAssertEqual(subject.alertActions[1].title, Localizations.yes)

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

try await subject.tapAction(title: Localizations.yes)
XCTAssertTrue(actionCalled)
}
}
3 changes: 0 additions & 3 deletions BitwardenShared/UI/Auth/Landing/LandingAction.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@

/// Actions that can be processed by a `LandingProcessor`.
enum LandingAction: Equatable {
/// The continue button was pressed.
case continuePressed

/// The create account button was pressed.
case createAccountPressed

Expand Down
5 changes: 4 additions & 1 deletion BitwardenShared/UI/Auth/Landing/LandingEffect.swift
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
// MARK: - LandingEffect

/// Effects that can be processed by a `LandingProcessor`.
enum LandingEffect {
enum LandingEffect: Equatable {
/// The vault list appeared on screen.
case appeared

/// The continue button was pressed.
case continuePressed

/// A Profile Switcher Effect.
case profileSwitcher(ProfileSwitcherEffect)
}
15 changes: 11 additions & 4 deletions BitwardenShared/UI/Auth/Landing/LandingProcessor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,16 @@ class LandingProcessor: StateProcessor<LandingState, LandingAction, LandingEffec
case .appeared:
await loadRegion()
await refreshProfileState()
case .continuePressed:
updateRememberedEmail()
await validateEmailAndContinue()
case let .profileSwitcher(profileEffect):
await handleProfileSwitcherEffect(profileEffect)
}
}

override func receive(_ action: LandingAction) {
switch action {
case .continuePressed:
updateRememberedEmail()
validateEmailAndContinue()
case .createAccountPressed:
coordinator.navigate(to: .createAccount)
case let .emailChanged(newValue):
Expand Down Expand Up @@ -103,13 +103,20 @@ class LandingProcessor: StateProcessor<LandingState, LandingAction, LandingEffec

/// Validate the currently entered email address and navigate to the login screen.
///
private func validateEmailAndContinue() {
private func validateEmailAndContinue() async {
let email = state.email.trimmingCharacters(in: .whitespacesAndNewlines).lowercased()
guard email.isValidEmail else {
coordinator.showAlert(.invalidEmail)
return
}

if let userId = await services.authRepository.existingAccountUserId(email: email) {
coordinator.showAlert(.switchToExistingAccount {
await self.coordinator.handleEvent(.action(.switchAccount(isAutomatic: false, userId: userId)))
})
return
}

coordinator.navigate(to: .login(username: email))
}

Expand Down
Loading

0 comments on commit 54f855c

Please sign in to comment.