Skip to content

Commit

Permalink
Extract SecureVault code into SecureStorage module (#1382)
Browse files Browse the repository at this point in the history 
Task/Issue URL: https://app.asana.com/0/0/1205119749390493/f
Tech Design URL:
CC: @brindy @Bunn @jotaemepereira

Description:

This PR updates the macOS browser to use the new SecureStorage package.
  • Loading branch information
@samsymons
samsymons authored Jul 28, 2023
1 parent 434187f commit 157b5e5
Show file tree
Hide file tree
Showing 19 changed files with 304 additions and 35 deletions.
2 changes: 1 addition & 1 deletion DuckDuckGo.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10108,7 +10108,7 @@
repositoryURL = "https://github.com/duckduckgo/BrowserServicesKit";
requirement = {
kind = exactVersion;
version = 69.1.0;
version = 70.0.0;
};
};
AA06B6B52672AF8100F541C5 /* XCRemoteSwiftPackageReference "Sparkle" */ = {
Expand Down
4 changes: 2 additions & 2 deletions DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
"kind" : "remoteSourceControl",
"location" : "https://github.com/duckduckgo/BrowserServicesKit",
"state" : {
"revision" : "8743afc2846b72e4734e5bf9c97fa304423b94e4",
"version" : "69.1.0"
"revision" : "214db1a9aceb643cbb5bf070b61d8f65ec196e1c",
"version" : "70.0.0"
}
},
{
Expand Down
3 changes: 2 additions & 1 deletion DuckDuckGo/Autofill/ContentOverlayViewController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import Cocoa
import WebKit
import Combine
import BrowserServicesKit
import SecureStorage
import Autofill

@MainActor
Expand Down Expand Up @@ -263,7 +264,7 @@ extension ContentOverlayViewController: SecureVaultManagerDelegate {
}
}

public func secureVaultInitFailed(_ error: SecureVaultError) {
public func secureVaultInitFailed(_ error: SecureStorageError) {
SecureVaultErrorReporter.shared.secureVaultInitFailed(error)
}

Expand Down
4 changes: 2 additions & 2 deletions DuckDuckGo/DataExport/CSVLoginExporter.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ final class CSVLoginExporter {
case failedToEncodeLogins
}

private let secureVault: SecureVault
private let secureVault: any AutofillSecureVault
private let fileStore: FileStore

init(secureVault: SecureVault, fileStore: FileStore = FileManager.default) {
init(secureVault: any AutofillSecureVault, fileStore: FileStore = FileManager.default) {
self.secureVault = secureVault
self.fileStore = fileStore
}
Expand Down
11 changes: 6 additions & 5 deletions DuckDuckGo/DataImport/Logins/SecureVault/SecureVaultLoginImporter.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,18 @@

import Foundation
import BrowserServicesKit
import SecureStorage

final class SecureVaultLoginImporter: LoginImporter {

private let secureVault: SecureVault
private let secureVault: any AutofillSecureVault

init(secureVault: SecureVault) {
init(secureVault: any AutofillSecureVault) {
self.secureVault = secureVault
}

func importLogins(_ logins: [ImportedLoginCredential]) throws -> DataImport.CompletedLoginsResult {
let vault = try SecureVaultFactory.default.makeVault(errorReporter: SecureVaultErrorReporter.shared)
let vault = try AutofillSecureVaultFactory.makeVault(errorReporter: SecureVaultErrorReporter.shared)

var successful: [String] = []
var duplicates: [String] = []
Expand All @@ -47,10 +48,10 @@ final class SecureVaultLoginImporter: LoginImporter {
}

do {
try vault.storeWebsiteCredentials(credentials)
_ = try vault.storeWebsiteCredentials(credentials)
successful.append(importSummaryValue)
} catch {
if case .duplicateRecord = error as? SecureVaultError {
if case .duplicateRecord = error as? SecureStorageError {
duplicates.append(importSummaryValue)
} else {
failed.append(importSummaryValue)
Expand Down
4 changes: 2 additions & 2 deletions DuckDuckGo/DataImport/View/DataImportViewController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ final class DataImportViewController: NSViewController {
}

private func secureVaultImporter() throws -> SecureVaultLoginImporter {
let secureVault = try SecureVaultFactory.default.makeVault(errorReporter: SecureVaultErrorReporter.shared)
let secureVault = try AutofillSecureVaultFactory.makeVault(errorReporter: SecureVaultErrorReporter.shared)
return SecureVaultLoginImporter(secureVault: secureVault)
}

Expand Down Expand Up @@ -460,7 +460,7 @@ extension DataImportViewController: FileImportViewControllerDelegate {
}

do {
let secureVault = try SecureVaultFactory.default.makeVault(errorReporter: SecureVaultErrorReporter.shared)
let secureVault = try AutofillSecureVaultFactory.makeVault(errorReporter: SecureVaultErrorReporter.shared)
let secureVaultImporter = SecureVaultLoginImporter(secureVault: secureVault)
self.dataImporter = CSVImporter(fileURL: url,
loginImporter: secureVaultImporter,
Expand Down
5 changes: 3 additions & 2 deletions DuckDuckGo/Fire/Model/Fire.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import BrowserServicesKit
import DDGSync
import PrivacyDashboard
import WebKit
import SecureStorage

final class Fire {

Expand All @@ -38,7 +39,7 @@ final class Fire {
let bookmarkManager: BookmarkManager
let syncService: DDGSyncing?
let tabCleanupPreparer = TabCleanupPreparer()
let secureVaultFactory: SecureVaultFactory
let secureVaultFactory: AutofillVaultFactory
let tld: TLD

private var dispatchGroup: DispatchGroup?
Expand Down Expand Up @@ -95,7 +96,7 @@ final class Fire {
tld: TLD,
bookmarkManager: BookmarkManager = LocalBookmarkManager.shared,
syncService: DDGSyncing? = nil,
secureVaultFactory: SecureVaultFactory = SecureVaultFactory.default
secureVaultFactory: AutofillVaultFactory = AutofillSecureVaultFactory
) {
self.webCacheManager = cacheManager
self.historyCoordinating = historyCoordinating
Expand Down
4 changes: 2 additions & 2 deletions DuckDuckGo/HomePage/Model/DataImportStatusProviding.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ protocol DataImportStatusProviding {

final class BookmarksAndPasswordsImportStatusProvider: DataImportStatusProviding {

let secureVault: SecureVault?
let secureVault: (any AutofillSecureVault)?
let bookmarkManager: BookmarkManager

init(secureVault: SecureVault? = try? SecureVaultFactory.default.makeVault(errorReporter: SecureVaultErrorReporter.shared),
init(secureVault: (any AutofillSecureVault)? = try? AutofillSecureVaultFactory.makeVault(errorReporter: SecureVaultErrorReporter.shared),
bookmarkManager: BookmarkManager = LocalBookmarkManager.shared) {
self.secureVault = secureVault
self.bookmarkManager = bookmarkManager
Expand Down
6 changes: 3 additions & 3 deletions DuckDuckGo/Menus/MainMenuActions.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ extension AppDelegate {
savePanel.beginSheetModal(for: window) { response in
guard response == .OK, let selectedURL = savePanel.url else { return }

let vault = try? SecureVaultFactory.default.makeVault(errorReporter: SecureVaultErrorReporter.shared)
let vault = try? AutofillSecureVaultFactory.makeVault(errorReporter: SecureVaultErrorReporter.shared)
let exporter = CSVLoginExporter(secureVault: vault!)
do {
try exporter.exportVaultLogins(to: selectedURL)
Expand Down Expand Up @@ -614,7 +614,7 @@ extension MainViewController {
}

@IBAction func resetSecureVaultData(_ sender: Any?) {
let vault = try? SecureVaultFactory.default.makeVault(errorReporter: SecureVaultErrorReporter.shared)
let vault = try? AutofillSecureVaultFactory.makeVault(errorReporter: SecureVaultErrorReporter.shared)

let accounts = (try? vault?.accounts()) ?? []
for accountID in accounts.compactMap(\.id) {
Expand Down Expand Up @@ -846,7 +846,7 @@ extension AppDelegate: NSMenuItemValidation {
}

private var areTherePasswords: Bool {
let vault = try? SecureVaultFactory.default.makeVault(errorReporter: SecureVaultErrorReporter.shared)
let vault = try? AutofillSecureVaultFactory.makeVault(errorReporter: SecureVaultErrorReporter.shared)
guard let vault else {
return false
}
Expand Down
3 changes: 2 additions & 1 deletion DuckDuckGo/SecureVault/SecureVaultErrorReporter.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,13 @@

import Foundation
import BrowserServicesKit
import SecureStorage

final class SecureVaultErrorReporter: SecureVaultErrorReporting {
static let shared = SecureVaultErrorReporter()
private init() {}

func secureVaultInitFailed(_ error: SecureVaultError) {
func secureVaultInitFailed(_ error: SecureStorageError) {
#if DEBUG
guard !NSApp.isRunningUnitTests else { return }
#endif
Expand Down
7 changes: 4 additions & 3 deletions DuckDuckGo/SecureVault/View/PasswordManagementViewController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import Foundation
import Combine
import SwiftUI
import BrowserServicesKit
import SecureStorage

protocol PasswordManagementDelegate: AnyObject {

Expand Down Expand Up @@ -139,8 +140,8 @@ final class PasswordManagementViewController: NSViewController {
}
}

var secureVault: SecureVault? {
try? SecureVaultFactory.default.makeVault(errorReporter: SecureVaultErrorReporter.shared)
var secureVault: (any AutofillSecureVault)? {
try? AutofillSecureVaultFactory.makeVault(errorReporter: SecureVaultErrorReporter.shared)
}

private let passwordManagerCoordinator: PasswordManagerCoordinating = PasswordManagerCoordinator.shared
Expand Down Expand Up @@ -464,7 +465,7 @@ final class PasswordManagementViewController: NSViewController {
postChange()

} catch {
if let window = view.window, case SecureVaultError.duplicateRecord = error {
if let window = view.window, case SecureStorageError.duplicateRecord = error {
NSAlert.passwordManagerDuplicateLogin().beginSheetModal(for: window)
}
}
Expand Down
2 changes: 1 addition & 1 deletion DuckDuckGo/SecureVault/View/SaveCredentialsViewController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ final class SaveCredentialsViewController: NSViewController {
}
}
} else {
try SecureVaultFactory.default.makeVault(errorReporter: SecureVaultErrorReporter.shared).storeWebsiteCredentials(credentials)
_ = try AutofillSecureVaultFactory.makeVault(errorReporter: SecureVaultErrorReporter.shared).storeWebsiteCredentials(credentials)
}
} catch {
os_log("%s:%s: failed to store credentials %s", type: .error, className, #function, error.localizedDescription)
Expand Down
2 changes: 1 addition & 1 deletion DuckDuckGo/SecureVault/View/SaveIdentityViewController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ final class SaveIdentityViewController: NSViewController {
identity.title = UserText.pmDefaultIdentityAutofillTitle

do {
try SecureVaultFactory.default.makeVault(errorReporter: SecureVaultErrorReporter.shared).storeIdentity(identity)
try AutofillSecureVaultFactory.makeVault(errorReporter: SecureVaultErrorReporter.shared).storeIdentity(identity)
Pixel.fire(.autofillItemSaved(kind: .identity))
} catch {
os_log("%s:%s: failed to store identity %s", type: .error, className, #function, error.localizedDescription)
Expand Down
2 changes: 1 addition & 1 deletion DuckDuckGo/SecureVault/View/SavePaymentMethodViewController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ final class SavePaymentMethodViewController: NSViewController {
paymentMethod.title = CreditCardValidation.type(for: paymentMethod.cardNumber).displayName

do {
try SecureVaultFactory.default.makeVault(errorReporter: SecureVaultErrorReporter.shared).storeCreditCard(paymentMethod)
try AutofillSecureVaultFactory.makeVault(errorReporter: SecureVaultErrorReporter.shared).storeCreditCard(paymentMethod)
} catch {
os_log("%s:%s: failed to store payment method %s", type: .error, className, #function, error.localizedDescription)
}
Expand Down
3 changes: 2 additions & 1 deletion DuckDuckGo/Tab/TabExtensions/AutofillTabExtension.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import BrowserServicesKit
import Combine
import Foundation
import SecureStorage

final class AutofillTabExtension: TabExtension {

Expand Down Expand Up @@ -113,7 +114,7 @@ extension AutofillTabExtension: SecureVaultManagerDelegate {
}
}

func secureVaultInitFailed(_ error: SecureVaultError) {
func secureVaultInitFailed(_ error: SecureStorageError) {
SecureVaultErrorReporter.shared.secureVaultInitFailed(error)
}

Expand Down
2 changes: 1 addition & 1 deletion LocalPackages/NetworkProtectionUI/Package.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ let package = Package(
dependencies: [
// Dependencies declare other packages that this package depends on.
// .package(url: /* package url */, from: "1.0.0"),
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "69.1.0"),
.package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "70.0.0"),
.package(path: "../SwiftUIExtensions")
],
targets: [
Expand Down
4 changes: 2 additions & 2 deletions UnitTests/DataExport/CSVLoginExporterTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ import BrowserServicesKit

class CSVLoginExporterTests: XCTestCase {

func testWhenExportingLogins_ThenLoginsArePersistedToDisk() {
func testWhenExportingLogins_ThenLoginsArePersistedToDisk() throws {
let mockFileStore = FileStoreMock()
let vault = MockSecureVault()
let vault = try MockSecureVaultFactory.makeVault(errorReporter: nil)

let credentials = websiteCredentials(identifiers: [1])
vault.storedAccounts = credentials.map(\.value.account)
Expand Down
Loading

0 comments on commit 157b5e5

Please sign in to comment.