Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

De-duplicate passwords imports #2920

Merged
merged 21 commits into from
Jul 5, 2024
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 48 additions & 2 deletions DuckDuckGo.xcodeproj/project.pbxproj

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
"kind" : "remoteSourceControl",
"location" : "https://github.com/duckduckgo/BrowserServicesKit",
"state" : {
"revision" : "2df7f9d9063c9f8f8f07ccb80c95d7e35738d1ea",
"version" : "165.0.1"
"branch" : "graeme/deduplicate-passwords",
"revision" : "2d0332335e0f53399d8d852321565cadb19eeb93"
}
},
{
Expand Down
25 changes: 5 additions & 20 deletions DuckDuckGo/Common/Extensions/OptionalExtension.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,11 @@

import Foundation

protocol OptionalProtocol {
associatedtype Wrapped

var isNil: Bool { get }

/// instantiate a Concrete-Typed `Optional<Wrapped>.none as T` from an `AnyOptionalType`
/// can be used to return nil value for a maybe-optional Generic Type
/// usage: `(T.self as? AnyOptionalType)?.none as? T`
static var none: Self { get }
}
typealias AnyOptional = any OptionalProtocol
typealias AnyOptionalType = any OptionalProtocol.Type

extension Optional: OptionalProtocol {

var isNil: Bool {
if case .none = self {
return true
extension Optional where Wrapped == String {
var isNilOrEmpty: Bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏼

if case .some(let wrapped) = self {
return wrapped.isEmpty
}
return false
return true
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,15 @@

final class SecureVaultLoginImporter: LoginImporter {

static var featureFlagger: FeatureFlagger {
NSApp.delegateTyped.featureFlagger
}

private enum ImporterError: Error {
case duplicate
}

func importLogins(_ logins: [ImportedLoginCredential], progressCallback: @escaping (Int) throws -> Void) throws -> DataImport.DataTypeSummary {

Check failure on line 33 in DuckDuckGo/DataImport/Logins/SecureVault/SecureVaultLoginImporter.swift

View workflow job for this annotation

GitHub Actions / SwiftLint

Function body should span 50 lines or less excluding comments and whitespace: currently spans 51 lines (function_body_length)
let vault = try AutofillSecureVaultFactory.makeVault(reporter: SecureVaultReporter.shared)

var successful: [String] = []
Expand All @@ -32,6 +40,14 @@
let encryptionKey = try vault.getEncryptionKey()
let hashingSalt = try vault.getHashingSalt()

let accounts: [SecureVaultModels.WebsiteAccount]

if let results = try? vault.accounts() {
accounts = results
} else {
accounts = .init()
}

try vault.inDatabaseTransaction { database in
for (idx, login) in logins.enumerated() {
let title = login.title
Expand All @@ -46,11 +62,22 @@
}

do {
if Self.featureFlagger.isFeatureOn(.deduplicateLoginsOnImport),
let signature = try vault.encryptPassword(for: credentials, key: encryptionKey, salt: hashingSalt).account.signature {
let isDuplicate = accounts.contains {
$0.isDuplicateOf(accountToBeImported: account, signatureOfAccountToBeImported: signature, passwordToBeImported: login.password)
}
if isDuplicate {
throw ImporterError.duplicate
}
}
_ = try vault.storeWebsiteCredentials(credentials, in: database, encryptedUsing: encryptionKey, hashedUsing: hashingSalt)
successful.append(importSummaryValue)
} catch {
if case .duplicateRecord = error as? SecureStorageError {
duplicates.append(importSummaryValue)
} else if case .duplicate = error as? ImporterError {
duplicates.append(importSummaryValue)
} else {
failed.append(importSummaryValue)
}
Expand All @@ -68,3 +95,26 @@
}

}

extension SecureVaultModels.WebsiteAccount {

// Deduplication rules: https://app.asana.com/0/0/1207598052765977/f
func isDuplicateOf(accountToBeImported: Self, signatureOfAccountToBeImported: String, passwordToBeImported: String?) -> Bool {
guard signature == signatureOfAccountToBeImported || passwordToBeImported.isNilOrEmpty else {
return false
}
guard username == accountToBeImported.username || accountToBeImported.username.isNilOrEmpty else {
return false
}
guard domain == accountToBeImported.domain || accountToBeImported.domain.isNilOrEmpty else {
return false
}
guard notes == accountToBeImported.notes || accountToBeImported.notes.isNilOrEmpty else {
return false
}
guard patternMatchedTitle() == accountToBeImported.patternMatchedTitle() || accountToBeImported.patternMatchedTitle().isEmpty else {
return false
}
return true
}
}
9 changes: 8 additions & 1 deletion DuckDuckGo/DataImport/View/DataImportSummaryView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func bookmarksSuccessSummary(_ summary: DataImport.DataTypeSummary) -> some View
if summary.duplicate > 0 {
lineSeparator()
importSummaryRow(image: .failed,
text: "Duplicate Bookmarks Skipped:",
text: "Duplicates Skipped:",
comment: "Data import summary format of how many duplicate bookmarks (%lld) were skipped during import.",
count: summary.duplicate)
}
Expand All @@ -138,6 +138,13 @@ private func passwordsSuccessSummary(_ summary: DataImport.DataTypeSummary) -> s
comment: "Data import summary format of how many passwords (%lld) failed to import.",
count: summary.failed)
}
if summary.duplicate > 0 {
lineSeparator()
importSummaryRow(image: .failed,
text: "Duplicates Skipped: ",
comment: "Data import summary format of how many passwords (%lld) were skipped due to being duplicates.",
count: summary.duplicate)
}
}
}

Expand Down
5 changes: 5 additions & 0 deletions DuckDuckGo/FeatureFlagging/Model/FeatureFlag.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ public enum FeatureFlag: String {
/// Add experimental atb parameter to SERP queries for internal users to display Privacy Reminder
/// https://app.asana.com/0/1199230911884351/1205979030848528/f
case appendAtbToSerpQueries

// https://app.asana.com/0/72649045549333/1207597760316574/f
case deduplicateLoginsOnImport
}

extension FeatureFlag: FeatureFlagSourceProviding {
Expand All @@ -37,6 +40,8 @@ extension FeatureFlag: FeatureFlagSourceProviding {
return .internalOnly
case .sslCertificatesBypass:
return .remoteReleasable(.subfeature(sslCertificatesSubfeature.allowBypass))
case .deduplicateLoginsOnImport:
return .remoteReleasable(.subfeature(AutofillSubfeature.deduplicateLoginsOnImport))
}
}
}
Expand Down
7 changes: 7 additions & 0 deletions DuckDuckGo/Localizable.xcstrings
Original file line number Diff line number Diff line change
Expand Up @@ -17237,6 +17237,7 @@
},
"Duplicate Bookmarks Skipped:" : {
"comment" : "Data import summary format of how many duplicate bookmarks (%lld) were skipped during import.",
"extractionState" : "stale",
"localizations" : {
"de" : {
"stringUnit" : {
Expand Down Expand Up @@ -17347,6 +17348,12 @@
}
}
}
},
"Duplicates Skipped:" : {

},
"Duplicates Skipped: " : {

},
"edit" : {
"comment" : "Edit button",
Expand Down
93 changes: 93 additions & 0 deletions IntegrationTests/DataImport/CSVImporterIntegrationTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
//
// CSVImporterIntegrationTests.swift
//
// Copyright © 2024 DuckDuckGo. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

import Foundation
import XCTest
@testable import DuckDuckGo_Privacy_Browser
import BrowserServicesKit

final class CSVImporterIntegrationTests: XCTestCase {

override func setUpWithError() throws {
super.setUp()
try clearDB()
executionTimeAllowance = 10
}

override func tearDownWithError() throws {
try clearDB()
super.tearDown()
}

func clearDB() throws {
let vault = try AutofillSecureVaultFactory.makeVault(reporter: SecureVaultReporter.shared)

let accounts = try vault.accounts()
for accountID in accounts.compactMap(\.id) {
if let accountID = Int64(accountID) {
try vault.deleteWebsiteCredentialsFor(accountId: accountID)
}
}
}

func testImportPasswordsPerformance() async throws {
let csvURL = Bundle(for: Self.self).url(forResource: "mock_login_data_large", withExtension: "csv")!
let csvImporter = CSVImporter(
fileURL: csvURL,
loginImporter: SecureVaultLoginImporter(),
defaultColumnPositions: nil
)
let importTask = csvImporter.importData(types: [.passwords])

// No baseline set, but should be no more than 0.3 seconds on an M1 Max with 32GB memory
measureMetrics([.wallClockTime], automaticallyStartMeasuring: false) {
let expectation = expectation(description: "Measure finished")
Task {
startMeasuring()
let result = await importTask.result
_ = try result.get()
stopMeasuring()
expectation.fulfill()
}
wait(for: [expectation])
}
}

// Deduplication rules: https://app.asana.com/0/0/1207598052765977/f
func testImportingPasswords_deduplicatesAccordingToDefinedRules() async throws {
let startingDataURL = Bundle(for: Self.self).url(forResource: "login_deduplication_starting_data", withExtension: "csv")!
let startingDataImporter = CSVImporter(
fileURL: startingDataURL,
loginImporter: SecureVaultLoginImporter(),
defaultColumnPositions: nil
)
_ = await startingDataImporter.importData(types: [.passwords]).result

let testDataURL = Bundle(for: Self.self).url(forResource: "login_deduplication_test_data", withExtension: "csv")!
let testDataImporter = CSVImporter(
fileURL: testDataURL,
loginImporter: SecureVaultLoginImporter(),
defaultColumnPositions: nil
)
let importTask = testDataImporter.importData(types: [.passwords])
let result = await importTask.result
let summary = try result.get()[.passwords]?.get()

XCTAssertEqual(summary?.duplicate, 4)
}
}
18 changes: 18 additions & 0 deletions IntegrationTests/DataImport/login_deduplication_starting_data.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
title,username,password,url,notes
Strict duplicate,isillitoe0,iL8>{M(t,http://lycos.com,"Quisque porta volutpat erat. Quisque erat eros, viverra eget, congue eget, semper rutrum, nulla. Nunc purus. Phasellus in felis. Donec semper sapien a libero. Nam dui. Proin leo odio, porttitor id, consequat in, consequat ut, nulla. Sed accumsan felis."
Strict antithesis,aschermick1,"hS0#,JU>",https://icio.us,"Donec ut dolor. Morbi vel lectus in quam fringilla rhoncus. Mauris enim leo, rhoncus sed, vestibulum sit amet, cursus id, turpis."
Differing usernames,karran3,rP9*0X%Xj<iBOVX~,http://angelfire.com,"Aliquam augue quam, sollicitudin vitae, consectetuer eget, rutrum at, lorem. Integer tincidunt ante vel ipsum. Praesent blandit lacinia erat. Vestibulum sed magna at nunc commodo placerat."
Nil stored username,,rW4|)T2GagA!|NHc,http://blinklist.com,Etiam faucibus cursus urna. Ut tellus. Nulla ut erat id mauris vulputate elementum. Nullam varius. Nulla facilisi. Cras non velit nec nisi vulputate nonummy. Maecenas tincidunt lacus at velit. Vivamus vel nulla eget eros elementum pellentesque. Quisque porta volutpat erat.
Nil imported username,ratlee5,xB0)X*hFWr,http://un.org,Nullam sit amet turpis elementum ligula vehicula consequat. Morbi a ipsum. Integer a nibh. In quis justo. Maecenas rhoncus aliquam lacus. Morbi quis tortor id nulla ultrices aliquet.
Differing passwords,rinkin6,oK8*r5Q%O,https://psu.edu,"Donec posuere metus vitae ipsum. Aliquam non mauris. Morbi non lectus. Aliquam sit amet diam in magna bibendum imperdiet. Nullam orci pede, venenatis non, sodales sed, tincidunt eu, felis. Fusce posuere felis sed lacus. Morbi sem mauris, laoreet ut, rhoncus aliquet, pulvinar sed, nisl."
Nil stored password,ccowan7,,http://canalblog.com,"Nulla mollis molestie lorem. Quisque ut erat. Curabitur gravida nisi at nibh. In hac habitasse platea dictumst. Aliquam augue quam, sollicitudin vitae, consectetuer eget, rutrum at, lorem. Integer tincidunt ante vel ipsum. Praesent blandit lacinia erat. Vestibulum sed magna at nunc commodo placerat. Praesent blandit."
Nil imported password,mmonan8,oT3(a2cW$,http://symantec.com,"Phasellus in felis. Donec semper sapien a libero. Nam dui. Proin leo odio, porttitor id, consequat in, consequat ut, nulla. Sed accumsan felis. Ut at dolor quis odio consequat varius. Integer ac leo. Pellentesque ultrices mattis odio. Donec vitae nisi."
Differing URL,hfranceschi9,iS3_XQ=}k)cvuy/,http://un.org,"Proin eu mi. Nulla ac enim. In tempor, turpis nec euismod scelerisque, quam turpis adipiscing lorem, vitae mattis nibh ligula nec sem. Duis aliquam convallis nunc. Proin at turpis a pede posuere nonummy."
Nil stored URL,hcanepea,"kP3&*y7~26q,Veb",,In hac habitasse platea dictumst.
Nil imported URL,ayanshonokb,rY7}IB6@,https://delicious.com,"Pellentesque at nulla. Suspendisse potenti. Cras in purus eu magna vulputate luctus. Cum sociis natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Vivamus vestibulum sagittis sapien. Cum sociis natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Etiam vel augue. Vestibulum rutrum rutrum neque. Aenean auctor gravida sem."
Differing notes,nlepardc,hX4'<+Q.&<z,http://indiegogo.com,Vestibulum ante ipsum primis in faucibus orci luctus et ultrices posuere cubilia Curae; Mauris viverra diam vitae quam. Suspendisse potenti. Nullam porttitor lacus at turpis. Donec posuere metus vitae ipsum.
Nil stored notes,tschalld,iK6%k&n?}OG_d,http://mysql.com,
Nil imported notes,lhowe,gA8)oS53,http://latimes.com,"Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Proin interdum mauris non ligula pellentesque ultrices. Phasellus id sapien in sapien iaculis congue."
Differing titles,sparncuttf,"sU1""v$c<J7<",http://statcounter.com,Curabitur convallis. Duis consequat dui nec nisi volutpat eleifend. Donec ut dolor.
,lsambrookg,nM6)CyX&/hy,https://mapquest.com,Nil stored title
Nil imported title,abrimbleh,iU3*~0DzuU9>0sD,https://gov.uk,Nil imported title
18 changes: 18 additions & 0 deletions IntegrationTests/DataImport/login_deduplication_test_data.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
title,username,password,url,notes,,,,
Strict duplicate,isillitoe0,iL8>{M(t,http://lycos.com,"Quisque porta volutpat erat. Quisque erat eros, viverra eget, congue eget, semper rutrum, nulla. Nunc purus. Phasellus in felis. Donec semper sapien a libero. Nam dui. Proin leo odio, porttitor id, consequat in, consequat ut, nulla. Sed accumsan felis.",,,,
Complete antithesis,abrimbleh,iU3*~0DzuU9>0sD,https://icio.us,"Quisque erat eros, viverra eget, congue eget, semper rutrum, nulla. Nunc purus. Phasellus in felis. Donec semper sapien a libero. Nam dui. Proin leo odio, porttitor id, consequat in, consequat ut, nulla. Sed accumsan felis. Ut at dolor quis odio consequat varius. Integer ac leo. Pellentesque ultrices mattis odio.Hatity",caizic2,yN1{Yv%nkJ=,http://aboutads.info/diam/nam/tristique/tortor.png?ac=sapien&nulla=a,"Duis consequat dui nec nisi volutpat eleifend. Donec ut dolor. Morbi vel lectus in quam fringilla rhoncus. Mauris enim leo, rhoncus sed, vestibulum sit amet, cursus id, turpis. Integer aliquet, massa id lobortis convallis, tortor risus dapibus augue, vel accumsan tellus nisi eu orci. Mauris lacinia sapien quis libero. Nullam sit amet turpis elementum ligula vehicula consequat."
Differing usernames,wblindmann1,rP9*0X%Xj<iBOVX~,http://angelfire.com,"Aliquam augue quam, sollicitudin vitae, consectetuer eget, rutrum at, lorem. Integer tincidunt ante vel ipsum. Praesent blandit lacinia erat. Vestibulum sed magna at nunc commodo placerat.",,,,
Nil stored username,dprescott4,rW4|)T2GagA!|NHc,http://blinklist.com,Etiam faucibus cursus urna. Ut tellus. Nulla ut erat id mauris vulputate elementum. Nullam varius. Nulla facilisi. Cras non velit nec nisi vulputate nonummy. Maecenas tincidunt lacus at velit. Vivamus vel nulla eget eros elementum pellentesque. Quisque porta volutpat erat.,,,,
Nil imported username,,xB0)X*hFWr,http://un.org,Nullam sit amet turpis elementum ligula vehicula consequat. Morbi a ipsum. Integer a nibh. In quis justo. Maecenas rhoncus aliquam lacus. Morbi quis tortor id nulla ultrices aliquet.,,,,
Differing passwords,rinkin6,yT7/=or7WhIX<,https://psu.edu,"Donec posuere metus vitae ipsum. Aliquam non mauris. Morbi non lectus. Aliquam sit amet diam in magna bibendum imperdiet. Nullam orci pede, venenatis non, sodales sed, tincidunt eu, felis. Fusce posuere felis sed lacus. Morbi sem mauris, laoreet ut, rhoncus aliquet, pulvinar sed, nisl.",,,,
Nil stored password,ccowan7,"qZ0""2*S?uO/k{",http://canalblog.com,"Nulla mollis molestie lorem. Quisque ut erat. Curabitur gravida nisi at nibh. In hac habitasse platea dictumst. Aliquam augue quam, sollicitudin vitae, consectetuer eget, rutrum at, lorem. Integer tincidunt ante vel ipsum. Praesent blandit lacinia erat. Vestibulum sed magna at nunc commodo placerat. Praesent blandit.",,,,
Nil imported password,mmonan8,,http://symantec.com,"Phasellus in felis. Donec semper sapien a libero. Nam dui. Proin leo odio, porttitor id, consequat in, consequat ut, nulla. Sed accumsan felis. Ut at dolor quis odio consequat varius. Integer ac leo. Pellentesque ultrices mattis odio. Donec vitae nisi.",,,,
Differing URL,hfranceschi9,iS3_XQ=}k)cvuy/,https://sogou.com,"Proin eu mi. Nulla ac enim. In tempor, turpis nec euismod scelerisque, quam turpis adipiscing lorem, vitae mattis nibh ligula nec sem. Duis aliquam convallis nunc. Proin at turpis a pede posuere nonummy.",,,,
Nil stored URL,hcanepea,"kP3&*y7~26q,Veb",https://icq.com,In hac habitasse platea dictumst.,,,,
Nil imported URL,ayanshonokb,rY7}IB6@,,"Pellentesque at nulla. Suspendisse potenti. Cras in purus eu magna vulputate luctus. Cum sociis natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Vivamus vestibulum sagittis sapien. Cum sociis natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Etiam vel augue. Vestibulum rutrum rutrum neque. Aenean auctor gravida sem.",,,,
Differing notes,nlepardc,hX4'<+Q.&<z,http://indiegogo.com,"Integer aliquet, massa id lobortis convallis, tortor risus dapibus augue, vel accumsan tellus nisi eu orci. Mauris lacinia sapien quis libero. Nullam sit amet turpis elementum ligula vehicula consequat. Morbi a ipsum.",,,,
Nil stored notes,tschalld,iK6%k&n?}OG_d,http://mysql.com,"Nullam varius. Nulla facilisi. Cras non velit nec nisi vulputate nonummy. Maecenas tincidunt lacus at velit. Vivamus vel nulla eget eros elementum pellentesque. Quisque porta volutpat erat. Quisque erat eros, viverra eget, congue eget, semper rutrum, nulla. Nunc purus.",,,,
Nil imported notes,lhowe,gA8)oS53,http://latimes.com,,,,,
Varying titles,sparncuttf,"sU1""v$c<J7<",http://statcounter.com,Curabitur convallis. Duis consequat dui nec nisi volutpat eleifend. Donec ut dolor.,,,,
Nil stored title,lsambrookg,nM6)CyX&/hy,https://mapquest.com,Nil stored title,,,,
,abrimbleh,iU3*~0DzuU9>0sD,https://gov.uk,Nil imported title,,,,
Loading
Loading