Skip to content

Commit

Permalink
[BITAU-136] Sync Failure Message (#193)
Browse files Browse the repository at this point in the history
  • Loading branch information
brant-livefront authored Nov 21, 2024
1 parent 94fed8c commit 309d296
Show file tree
Hide file tree
Showing 15 changed files with 297 additions and 102 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ class AppSettingsStoreTests: AuthenticatorTestCase {
XCTAssertEqual(subject.defaultSaveOption, .none)
}

/// `defaultSaveOption` can be used to get and set the default save option..
/// `defaultSaveOption` can be used to get and set the default save option.
func test_defaultSaveOption_withValue() {
subject.defaultSaveOption = .saveToBitwarden
XCTAssertEqual(subject.defaultSaveOption, .saveToBitwarden)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,9 @@ class DefaultAuthenticatorItemRepository {
/// Service to fetch items from the shared CoreData store - shared from the main Bitwarden PM app.
private let sharedItemService: AuthenticatorBridgeItemService

/// Flag to indicate if there was an error with the data synced from the PM app.
private var syncError = false

/// A protocol wrapping the present time.
private let timeProvider: TimeProvider

Expand Down Expand Up @@ -174,10 +177,19 @@ class DefaultAuthenticatorItemRepository {
private func combinedSections(
localSections: [ItemListSection],
sharedItems: [AuthenticatorBridgeItemDataView]
) async throws -> [ItemListSection] {
) async -> [ItemListSection] {
guard await isPasswordManagerSyncActive() else {
return localSections
}
guard !syncError else {
var sections = localSections
sections.append(ItemListSection(
id: "SyncError",
items: [.syncError()],
name: ""
))
return sections
}

let groupedByAccount = Dictionary(
grouping: sharedItems,
Expand Down Expand Up @@ -244,10 +256,18 @@ class DefaultAuthenticatorItemRepository {
try await authenticatorItemService.authenticatorItemsPublisher()
.combineLatest(
sharedItemService.sharedItemsPublisher()
.catch { error -> AnyPublisher<[AuthenticatorBridgeItemDataView], any Error> in
self.syncError = true
self.errorReporter.log(error: error)

return Just([])
.setFailureType(to: Error.self)
.eraseToAnyPublisher()
}
)
.asyncTryMap { localItems, sharedItems in
let sections = try await self.itemListSections(from: localItems)
return try await self.combinedSections(localSections: sections, sharedItems: sharedItems)
return await self.combinedSections(localSections: sections, sharedItems: sharedItems)
}
.eraseToAnyPublisher()
}
Expand Down Expand Up @@ -292,13 +312,17 @@ extension DefaultAuthenticatorItemRepository: AuthenticatorItemRepository {
case let .sharedTotp(model):
let key = model.itemView.totpKey
keyModel = TOTPKeyModel(authenticatorKey: key)
case .syncError:
keyModel = nil // Should be filtered out, no need to refresh codes
case let .totp(model):
let key = model.itemView.totpKey
keyModel = TOTPKeyModel(authenticatorKey: key)
}
guard let keyModel else {
errorReporter.log(error: TOTPServiceError
.unableToGenerateCode("Unable to refresh TOTP code for list view item: \(item.id)"))
if item.itemType != .syncError {
errorReporter.log(error: TOTPServiceError
.unableToGenerateCode("Unable to refresh TOTP code for list view item: \(item.id)"))
}
return item
}
let code = try await totpService.getTotpCode(for: keyModel)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,30 +207,25 @@ class AuthenticatorItemRepositoryTests: AuthenticatorTestCase { // swiftlint:dis
let item = ItemListItem.fixture()
let sharedItem = ItemListItem.fixtureShared()

let result = try await subject.refreshTotpCodes(on: [item, sharedItem])
let result = try await subject.refreshTotpCodes(on: [item, sharedItem, .syncError()])
let actual = try XCTUnwrap(result[0])

XCTAssertEqual(actual.id, item.id)
XCTAssertEqual(actual.name, item.name)
XCTAssertEqual(actual.accountName, item.accountName)
switch actual.itemType {
case .sharedTotp:
XCTFail("Shared TOTP itemType found when expecting TOTP")
case let .totp(model):
XCTAssertEqual(model.totpCode, newCodeModel)
}
XCTAssertEqual(actual.totpCodeModel, newCodeModel)

let shared = try XCTUnwrap(result[1])

XCTAssertEqual(shared.id, sharedItem.id)
XCTAssertEqual(shared.name, sharedItem.name)
XCTAssertEqual(shared.accountName, sharedItem.accountName)
switch shared.itemType {
case let .sharedTotp(model):
XCTAssertEqual(model.totpCode, newCodeModel)
case .totp:
XCTFail("TOTP itemType found when expecting Shared TOTP")
}
XCTAssertEqual(shared.totpCodeModel, newCodeModel)

let syncError = try XCTUnwrap(result[2])
XCTAssertEqual(syncError, ItemListItem.syncError())

XCTAssertTrue(errorReporter.errors.isEmpty)
}

/// `saveTemporarySharedItem(_)` saves a temporary item into the Authenticator Bridge shared store.
Expand Down Expand Up @@ -387,6 +382,44 @@ class AuthenticatorItemRepositoryTests: AuthenticatorTestCase { // swiftlint:dis
)
}

/// `itemListPublisher()` returns a favorites section and a local codes section as normal. Adds a syncError section
/// when the sync process if throwing an error.
func test_itemListPublisher_syncError() async throws {
configService.featureFlagsBool[.enablePasswordManagerSync] = true
sharedItemService.syncOn = true
let items = [
AuthenticatorItem.fixture(id: "1", name: "One"),
AuthenticatorItem.fixture(favorite: true, id: "2", name: "Two"),
]
let sharedItem = AuthenticatorBridgeItemDataView.fixture(accountDomain: "Domain",
accountEmail: "[email protected]",
totpKey: "totpKey")
sharedItemService.storedItems = ["userId": [sharedItem]]
let unorganizedItem = itemListItem(from: items[0])
let favoritedItem = itemListItem(from: items[1])

authItemService.authenticatorItemsSubject.send(items)
sharedItemService.sharedItemsSubject.send(completion: .failure(AuthenticatorTestError.example))

var iterator = try await subject.itemListPublisher().makeAsyncIterator()
let sections = try await iterator.next()

XCTAssertEqual(
sections,
[
ItemListSection(id: "Favorites",
items: [favoritedItem],
name: Localizations.favorites),
ItemListSection(id: "LocalCodes",
items: [unorganizedItem],
name: Localizations.localCodes),
ItemListSection(id: "SyncError",
items: [.syncError()],
name: ""),
]
)
}

/// `itemListPublisher()` returns a favorites section as before, when the feature flag is enabled, but
/// the user has not yet enabled sync.
func test_itemListPublisher_syncOff() async throws {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,9 @@ enum TOTPExpirationCalculator {
timeProvider: any TimeProvider
) -> [Bool: [ItemListItem]] {
let sortedItems: [Bool: [ItemListItem]] = Dictionary(grouping: items, by: { item in
switch item.itemType {
case let .sharedTotp(model):
return hasCodeExpired(model.totpCode, timeProvider: timeProvider)
case let .totp(model):
return hasCodeExpired(model.totpCode, timeProvider: timeProvider)
}
guard let totpCode = item.totpCodeModel else { return false }

return hasCodeExpired(totpCode, timeProvider: timeProvider)
})
return sortedItems
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,3 +141,4 @@
"DefaultSaveOption" = "Default save option";
"SaveToBitwarden" = "Save to Bitwarden";
"None" = "None";
"UnableToSyncCodesFromTheBitwardenApp" = "Unable to sync codes from the Bitwarden app. Make sure both apps are up-to-date. You can still access your existing codes in the Bitwarden app.";
31 changes: 31 additions & 0 deletions AuthenticatorShared/UI/Vault/ItemList/ItemList/ItemListItem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ public struct ItemListItem: Equatable, Identifiable {
/// - model: The TOTP model
case sharedTotp(model: ItemListSharedTotpItem)

/// An item for displaying an error that occurred in syncing from the Password Manager app.
case syncError

/// A TOTP code item
///
/// - Parameters:
Expand All @@ -41,6 +44,19 @@ extension ItemListItem {
/// so that `code` is non-optional and always has a value.
private static let defaultTotpCode = "123456"

/// The associated `TOTPCodeModel` if this item is an `itemType` with an associated code (i.e. `.totp`
/// and `.sharedTotp`) or `nil` if there is no associated code (i.e. `.syncError`)
var totpCodeModel: TOTPCodeModel? {
switch itemType {
case let .sharedTotp(model):
return model.totpCode
case .syncError:
return nil
case let .totp(model):
return model.totpCode
}
}

/// Initialize an `ItemListItem` from an `AuthenticatorItemView`
///
/// - Parameters:
Expand Down Expand Up @@ -99,6 +115,19 @@ extension ItemListItem {
itemType: .sharedTotp(model: totpModel))
}

/// Initialize a `.syncError` `ItemListItem`.
///
/// - Returns: An`ItemListItem` of the `.syncError` `ItemType`.
///
public static func syncError() -> ItemListItem {
self.init(
id: "syncError",
name: Localizations.unableToSyncCodesFromTheBitwardenApp,
accountName: nil,
itemType: .syncError
)
}

/// Make a new `ItemListItem` that is a copy of the existing one, but with an updated `TOTPCodeModel`.
///
/// - Parameter newTotpModel: the new `TOTPCodeModel` to insert in this ItemListItem
Expand All @@ -116,6 +145,8 @@ extension ItemListItem {
accountName: accountName,
itemType: .sharedTotp(model: updatedModel)
)
case .syncError:
return self
case let .totp(oldModel):
var updatedModel = oldModel
updatedModel.totpCode = newTotpModel
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import XCTest

@testable import AuthenticatorShared

// MARK: - ItemListItemTests

class ItemListItemTests: AuthenticatorTestCase {
var subject: ItemListItem!

/// `totpCodeModel` returns the associated `TOTPCodeModel` for an item of type `.sharedTotp`.
func test_totpCodeModel_shared() {
let expected = TOTPCodeModel(code: "098765", codeGenerationDate: .now, period: 30)
subject = .fixtureShared(totp: .fixture(totpCode: expected))

XCTAssertEqual(expected, subject.totpCodeModel)
}

/// `totpCodeModel` returns `nil` for an item of type `.syncError`.
func test_totpCodeModel_syncError() {
subject = .syncError()

XCTAssertNil(subject.totpCodeModel)
}

/// `totpCodeModel` returns the associated `TOTPCodeModel` for an item of type `.totp`.
func test_totpCodeModel_totp() {
let expected = TOTPCodeModel(code: "098765", codeGenerationDate: .now, period: 30)
subject = .fixture(totp: .fixture(totpCode: expected))

XCTAssertEqual(expected, subject.totpCodeModel)
}

/// For the `.sharedTotp` case, `with(newTotpModel:)` returns a copy of the item with the new TOTP code.
func test_withNewTotpModel_shared() {
subject = .fixtureShared()
let newModel = TOTPCodeModel(
code: "098765",
codeGenerationDate: Date(),
period: 30
)

let newItem = subject.with(newTotpModel: newModel)
XCTAssertEqual(newItem.totpCodeModel, newModel)
}

/// For the `.syncError` case, `with(newTotpModel:)` simply returns a copy of the item.
func test_withNewTotpModel_syncError() {
subject = .syncError()
let newModel = TOTPCodeModel(
code: "098765",
codeGenerationDate: Date(),
period: 30
)

let newItem = subject.with(newTotpModel: newModel)
XCTAssertEqual(subject, newItem)
}

/// For the `.totp`case, `with(newTotpModel:)` returns a copy of the item with the new TOTP code.
func test_withNewTotpModel_totp() {
subject = .fixture()
let newModel = TOTPCodeModel(
code: "098765",
codeGenerationDate: Date(),
period: 30
)

let newItem = subject.with(newTotpModel: newModel)
XCTAssertEqual(newItem.totpCodeModel, newModel)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ final class ItemListProcessor: StateProcessor<ItemListState, ItemListAction, Ite
let totpKey = TOTPKeyModel(authenticatorKey: key)
else { return }
await generateAndCopyTotpCode(totpKey: totpKey)
case .syncError:
break // no action for this type
case let .totp(model):
guard let key = model.itemView.totpKey,
let totpKey = TOTPKeyModel(authenticatorKey: key)
Expand Down Expand Up @@ -119,14 +121,10 @@ final class ItemListProcessor: StateProcessor<ItemListState, ItemListAction, Ite
guard case let .totp(model) = item.itemType else { return }
coordinator.navigate(to: .editItem(item: model.itemView), context: self)
case let .itemPressed(item):
switch item.itemType {
case let .sharedTotp(model):
services.pasteboardService.copy(model.totpCode.code)
state.toast = Toast(text: Localizations.valueHasBeenCopied(Localizations.verificationCode))
case let .totp(model):
services.pasteboardService.copy(model.totpCode.code)
state.toast = Toast(text: Localizations.valueHasBeenCopied(Localizations.verificationCode))
}
guard let totpCode = item.totpCodeModel else { return }

services.pasteboardService.copy(totpCode.code)
state.toast = Toast(text: Localizations.valueHasBeenCopied(Localizations.verificationCode))
case let .searchStateChanged(isSearching: isSearching):
guard isSearching else {
state.searchText = ""
Expand Down Expand Up @@ -414,11 +412,8 @@ private class TOTPExpirationManager {
func configureTOTPRefreshScheduling(for items: [ItemListItem]) {
var newItemsByInterval = [UInt32: [ItemListItem]]()
items.forEach { item in
switch item.itemType {
case let .sharedTotp(model):
newItemsByInterval[model.totpCode.period, default: []].append(item)
case let .totp(model):
newItemsByInterval[model.totpCode.period, default: []].append(item)
if let totpCodeModel = item.totpCodeModel {
newItemsByInterval[totpCodeModel.period, default: []].append(item)
}
}
itemsByInterval = newItemsByInterval
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,16 @@ class ItemListProcessorTests: AuthenticatorTestCase { // swiftlint:disable:this
)
}

/// `perform(:_)` with `.copyPressed()` with a `.syncError` item does not throw
/// and produces no result.
func test_perform_copyPressed_syncError() async {
await assertAsyncDoesNotThrow {
await subject.perform(.copyPressed(.syncError()))
}
XCTAssertNil(subject.state.toast)
XCTAssertNil(pasteboardService.copiedString)
}

/// `perform(:_)` with `.moveToBitwardenPressed()` with a local item stores the item in the shared
/// store and launches the Bitwarden app via the new item deep link.
func test_perform_moveToBitwardenPressed_localItem() async throws {
Expand Down
Loading

0 comments on commit 309d296

Please sign in to comment.