Skip to content

Commit

Permalink
Automatically save generated passwords (#81)
Browse files Browse the repository at this point in the history
Task/Issue URL: https://app.asana.com/0/0/1201969773101195/f
iOS PR: duckduckgo/iOS#1109
macOS PR: duckduckgo/macos-browser#500

Description:

This PR updates BSK to automatically save generated credentials when the user selects that option. Note that this PR does not require any changes on either of the client codebases.

If the user doesn't have a username entered, they will get prompted immediately. If they have already entered a username, the credential gets saved without prompting them. See parent task for context.
  • Loading branch information
samsymons authored May 6, 2022
1 parent ec7848a commit 7175260
Show file tree
Hide file tree
Showing 8 changed files with 476 additions and 68 deletions.
3 changes: 2 additions & 1 deletion .swiftlint.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
disabled_rules:
- trailing_whitespace
- nesting

line_length:
warning: 150
Expand All @@ -18,4 +19,4 @@ type_name:
error: 100

excluded:
- .build
- .build
Original file line number Diff line number Diff line change
Expand Up @@ -134,30 +134,35 @@ extension AutofillUserScript {

// MARK: - Requests

public struct IncomingCredentials {
public struct IncomingCredentials: Equatable {

private enum Constants {
static let credentialsKey = "credentials"
static let usernameKey = "username"
static let passwordKey = "password"
static let autogeneratedKey = "autogenerated"
}

let username: String?
let password: String
let autogenerated: Bool

init(username: String?, password: String) {
init(username: String?, password: String, autogenerated: Bool = false) {
self.username = username
self.password = password
self.autogenerated = autogenerated
}

init?(autofillDictionary: [String: Any]) {
guard let credentialsDictionary = autofillDictionary[Constants.credentialsKey] as? [String: String],
let password = credentialsDictionary[Constants.passwordKey] else {
guard let credentialsDictionary = autofillDictionary[Constants.credentialsKey] as? [String: Any],
let password = credentialsDictionary[Constants.passwordKey] as? String else {
return nil
}

// Usernames are optional, as the Autofill script can pass a generated password through without a corresponding username.
self.init(username: credentialsDictionary[Constants.usernameKey], password: password)
self.init(username: credentialsDictionary[Constants.usernameKey] as? String,
password: password,
autogenerated: (credentialsDictionary[Constants.autogeneratedKey] as? Bool) ?? false)
}

}
Expand All @@ -172,6 +177,10 @@ extension AutofillUserScript {
public let credentials: IncomingCredentials?
public let creditCard: SecureVaultModels.CreditCard?

var hasAutogeneratedPassword: Bool {
return credentials?.autogenerated ?? false
}

init(dictionary: [String: Any]) {
self.identity = .init(autofillDictionary: dictionary)
self.creditCard = .init(autofillDictionary: dictionary)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ extension ContentBlockerRulesManager {
Encapsulates compilation steps for a single Task
*/
class CompilationTask {
// swiftlint:disable:next nesting
typealias Completion = (_ success: Bool) -> Void
let workQueue: DispatchQueue
let rulesList: ContentBlockerRulesList
Expand Down
2 changes: 1 addition & 1 deletion Sources/BrowserServicesKit/Resources/duckduckgo-autofill
149 changes: 130 additions & 19 deletions Sources/BrowserServicesKit/SecureVault/SecureVaultManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public struct AutofillData {
public let identity: SecureVaultModels.Identity?
public let credentials: SecureVaultModels.WebsiteCredentials?
public let creditCard: SecureVaultModels.CreditCard?
public let automaticallySavedCredentials: Bool
}

public protocol SecureVaultManagerDelegate: SecureVaultErrorReporting {
Expand All @@ -39,14 +40,20 @@ public protocol SecureVaultManagerDelegate: SecureVaultErrorReporting {

func secureVaultManager(_: SecureVaultManager, didAutofill type: AutofillType, withObjectId objectId: Int64)

// swiftlint:disable:next identifier_name
func secureVaultManager(_: SecureVaultManager, didRequestAuthenticationWithCompletionHandler: @escaping (Bool) -> Void)

}

public class SecureVaultManager {

public weak var delegate: SecureVaultManagerDelegate?

private let vault: SecureVault?

public init() { }
public init(vault: SecureVault? = nil) {
self.vault = vault
}

}

Expand All @@ -60,7 +67,7 @@ extension SecureVaultManager: AutofillSecureVaultDelegate {
[SecureVaultModels.CreditCard]) -> Void) {

do {
let vault = try SecureVaultFactory.default.makeVault(errorReporter: self.delegate)
let vault = try self.vault ?? SecureVaultFactory.default.makeVault(errorReporter: self.delegate)
let accounts = try vault.accountsFor(domain: domain)
let identities = try vault.identities()
let cards = try vault.creditCards()
Expand All @@ -79,9 +86,14 @@ extension SecureVaultManager: AutofillSecureVaultDelegate {
/// Receives each of the types of data that the Autofill script has detected, and determines whether the user should be prompted to save them.
/// This involves checking each proposed object to determine whether it already exists in the store.
/// Currently, only one new type of data is presented to the user, but that decision is handled client-side so that it's easier to adapt in the future when multiple types are presented at once.
public func autofillUserScript(_: AutofillUserScript, didRequestStoreDataForDomain domain: String, data: AutofillUserScript.DetectedAutofillData) {
public func autofillUserScript(_: AutofillUserScript,
didRequestStoreDataForDomain domain: String,
data: AutofillUserScript.DetectedAutofillData) {
do {
let dataToPrompt = try existingEntries(for: domain, autofillData: data)
let automaticallySavedCredentials = try storeOrUpdateAutogeneratedCredentials(domain: domain, autofillData: data)
try updateExistingAutogeneratedCredentialsWithSubmittedValues(domain: domain, autofillData: data)

let dataToPrompt = try existingEntries(for: domain, autofillData: data, automaticallySavedCredentials: automaticallySavedCredentials)
delegate?.secureVaultManager(self, promptUserToStoreAutofillData: dataToPrompt)
} catch {
os_log(.error, "Error storing data: %{public}@", error.localizedDescription)
Expand All @@ -93,8 +105,8 @@ extension SecureVaultManager: AutofillSecureVaultDelegate {
completionHandler: @escaping ([SecureVaultModels.WebsiteAccount]) -> Void) {

do {
completionHandler(try SecureVaultFactory.default.makeVault(errorReporter: self.delegate)
.accountsFor(domain: domain))
let vault = try self.vault ?? SecureVaultFactory.default.makeVault(errorReporter: self.delegate)
completionHandler(try vault.accountsFor(domain: domain))
} catch {
os_log(.error, "Error requesting accounts: %{public}@", error.localizedDescription)
completionHandler([])
Expand All @@ -107,8 +119,8 @@ extension SecureVaultManager: AutofillSecureVaultDelegate {
completionHandler: @escaping (SecureVaultModels.WebsiteCredentials?) -> Void) {

do {
completionHandler(try SecureVaultFactory.default.makeVault(errorReporter: self.delegate)
.websiteCredentialsFor(accountId: accountId))
let vault = try self.vault ?? SecureVaultFactory.default.makeVault(errorReporter: self.delegate)
completionHandler(try vault.websiteCredentialsFor(accountId: accountId))
delegate?.secureVaultManager(self, didAutofill: .password, withObjectId: accountId)
} catch {
os_log(.error, "Error requesting credentials: %{public}@", error.localizedDescription)
Expand All @@ -121,7 +133,8 @@ extension SecureVaultManager: AutofillSecureVaultDelegate {
didRequestCreditCardWithId creditCardId: Int64,
completionHandler: @escaping (SecureVaultModels.CreditCard?) -> Void) {
do {
let card = try SecureVaultFactory.default.makeVault(errorReporter: self.delegate).creditCardFor(id: creditCardId)
let vault = try self.vault ?? SecureVaultFactory.default.makeVault(errorReporter: self.delegate)
let card = try vault.creditCardFor(id: creditCardId)

delegate?.secureVaultManager(self, didRequestAuthenticationWithCompletionHandler: { authenticated in
if authenticated {
Expand All @@ -142,23 +155,115 @@ extension SecureVaultManager: AutofillSecureVaultDelegate {
didRequestIdentityWithId identityId: Int64,
completionHandler: @escaping (SecureVaultModels.Identity?) -> Void) {
do {
completionHandler(try SecureVaultFactory.default.makeVault(errorReporter: self.delegate)
.identityFor(id: identityId))
let vault = try self.vault ?? SecureVaultFactory.default.makeVault(errorReporter: self.delegate)
completionHandler(try vault.identityFor(id: identityId))

delegate?.secureVaultManager(self, didAutofill: .identity, withObjectId: identityId)
} catch {
os_log(.error, "Error requesting identity: %{public}@", error.localizedDescription)
completionHandler(nil)
}
}

func existingEntries(for domain: String, autofillData: AutofillUserScript.DetectedAutofillData) throws -> AutofillData {
let vault = try SecureVaultFactory.default.makeVault(errorReporter: self.delegate)
/// Stores autogenerated credentials sent by the AutofillUserScript, or updates an existing row in the database if credentials already exist.
/// The Secure Vault only stores one generated password for a domain, which is updated any time the user selects a new generated password.
func storeOrUpdateAutogeneratedCredentials(domain: String, autofillData: AutofillUserScript.DetectedAutofillData) throws -> Bool {
guard autofillData.hasAutogeneratedPassword,
let autogeneratedCredentials = autofillData.credentials,
!(autogeneratedCredentials.username?.isEmpty ?? true),
let passwordData = autogeneratedCredentials.password.data(using: .utf8) else {
os_log("Did not meet conditions for silently saving autogenerated credentials, returning early", log: .passwordManager)
return false
}

let vault = try self.vault ?? SecureVaultFactory.default.makeVault(errorReporter: self.delegate)
let accounts = try vault.accountsFor(domain: domain)

if accounts.contains(where: { account in account.username == autogeneratedCredentials.username }) {
os_log("Tried to automatically save credentials for which an account already exists, returning early", log: .passwordManager)
return false
}

// As a precaution, check whether an account exists with the matching generated password _and_ a non-nil username.
// If so, then the user must have already saved the generated credentials and set a username.

for account in accounts where !account.username.isEmpty {
if let accountID = account.id,
let credentialsForAccount = try vault.websiteCredentialsFor(accountId: accountID),
credentialsForAccount.password == passwordData,
account.username == autogeneratedCredentials.username ?? "" {
os_log("Tried to save autogenerated password but it already exists, returning early", log: .passwordManager)
return false
}
}

let existingAccount = accounts.first(where: { $0.username == "" })
var account = existingAccount ?? SecureVaultModels.WebsiteAccount(username: "", domain: domain)

account.title = "Saved Password (\(domain))"
let generatedPassword = SecureVaultModels.WebsiteCredentials(account: account, password: passwordData)

os_log("Saving autogenerated password", log: .passwordManager)

try vault.storeWebsiteCredentials(generatedPassword)

return true
}

/// If credentials are sent via the AutofillUserScript, and there exists a credential row with empty username and matching password, then this function will update that credential row with the username.
/// This can happen if the user chooses to use a generated password when signing up for a service, then enters their own username and submits the form.
func updateExistingAutogeneratedCredentialsWithSubmittedValues(domain: String, autofillData: AutofillUserScript.DetectedAutofillData) throws {
let vault = try self.vault ?? SecureVaultFactory.default.makeVault(errorReporter: self.delegate)
let accounts = try vault.accountsFor(domain: domain)

guard let autofillCredentials = autofillData.credentials,
let autofillCredentialsUsername = autofillCredentials.username,
var existingAccount = accounts.first(where: { $0.username == "" }),
let existingAccountID = existingAccount.id else {
return
}

if accounts.contains(where: { $0.username == autofillCredentials.username }) {
os_log("ERROR: Tried to save generated credentials with an existing username, prompt the user to update instead.", log: .passwordManager)
return
}

let existingCredentials = try vault.websiteCredentialsFor(accountId: existingAccountID)

guard let existingPasswordData = existingCredentials?.password,
let autofillPasswordData = autofillCredentials.password.data(using: .utf8) else {
return
}

// If true, then the existing generated password matches the credentials sent by the script, so update and save the difference.
if existingPasswordData == autofillPasswordData {
os_log("Found matching autogenerated credentials in Secure Vault, updating with username", log: .passwordManager)

existingAccount.username = autofillCredentialsUsername
existingAccount.title = nil // Remove the "Saved Password" title so that the UI uses the default title format

let credentialsToSave = SecureVaultModels.WebsiteCredentials(account: existingAccount, password: autofillPasswordData)

try vault.storeWebsiteCredentials(credentialsToSave)
}
}

func existingEntries(for domain: String,
autofillData: AutofillUserScript.DetectedAutofillData,
automaticallySavedCredentials: Bool) throws -> AutofillData {
let vault = try self.vault ?? SecureVaultFactory.default.makeVault(errorReporter: self.delegate)

let proposedIdentity = try existingIdentity(with: autofillData, vault: vault)
let proposedCredentials = try existingCredentials(with: autofillData, domain: domain, vault: vault)
let proposedCredentials = try existingCredentials(with: autofillData,
domain: domain,
automaticallySavedCredentials: automaticallySavedCredentials,
vault: vault)
let proposedCard = try existingPaymentMethod(with: autofillData, vault: vault)

return AutofillData(identity: proposedIdentity, credentials: proposedCredentials, creditCard: proposedCard)
return AutofillData(identity: proposedIdentity,
credentials: proposedCredentials,
creditCard: proposedCard,
automaticallySavedCredentials: automaticallySavedCredentials)
}

private func existingIdentity(with autofillData: AutofillUserScript.DetectedAutofillData,
Expand All @@ -174,17 +279,23 @@ extension SecureVaultManager: AutofillSecureVaultDelegate {

private func existingCredentials(with autofillData: AutofillUserScript.DetectedAutofillData,
domain: String,
automaticallySavedCredentials: Bool,
vault: SecureVault) throws -> SecureVaultModels.WebsiteCredentials? {
if let credentials = autofillData.credentials, let passwordData = credentials.password.data(using: .utf8) {
if let account = try vault
.accountsFor(domain: domain)
.first(where: { $0.username == credentials.username }) {
.first(where: { $0.username == credentials.username ?? "" }) {

if let existingAccountID = account.id,
let existingCredentials = try vault.websiteCredentialsFor(accountId: existingAccountID),
existingCredentials.password == passwordData {
os_log("Found duplicate credentials, avoid prompting user", log: .passwordManager)
return nil
if automaticallySavedCredentials {
os_log("Found duplicate credentials which were just saved, notifying user", log: .passwordManager)
return SecureVaultModels.WebsiteCredentials(account: account, password: passwordData)
} else {
os_log("Found duplicate credentials which were previously saved, avoid notifying user", log: .passwordManager)
return nil
}
} else {
os_log("Found existing credentials to update", log: .passwordManager)
return SecureVaultModels.WebsiteCredentials(account: account, password: passwordData)
Expand All @@ -203,7 +314,7 @@ extension SecureVaultManager: AutofillSecureVaultDelegate {
}

private func existingPaymentMethod(with autofillData: AutofillUserScript.DetectedAutofillData,
vault: SecureVault) throws -> SecureVaultModels.CreditCard? {
vault: SecureVault) throws -> SecureVaultModels.CreditCard? {
if let card = autofillData.creditCard, try vault.existingCardForAutofill(matching: card) == nil {
os_log("Got new payment method to save", log: .passwordManager)
return card
Expand Down
Loading

0 comments on commit 7175260

Please sign in to comment.