Skip to content

Commit

Permalink
Promote password import in autofill menu (#976)
Browse files Browse the repository at this point in the history 
Please review the release process for BrowserServicesKit
[here](https://app.asana.com/0/1200194497630846/1200837094583426).

**Required**:

Task/Issue URL:
https://app.asana.com/0/72649045549333/1207617229892922/f
iOS PR: duckduckgo/iOS#3332
macOS PR: duckduckgo/macos-browser#3220
What kind of version bump will this require?: Major

**Optional**:

Tech Design URL:
https://app.asana.com/0/481882893211075/1208037181667142

**Description**:

When adopting a new browser after a long time using something else, you
expect your browser to have all your data. If you skipped import during
onboarding you will find out about the missing data at the time when
you're trying to use the password. This creates huge friction right in
the middle of a task. See [Comment by Peter Dolanjski on Password and
autofill
issues](https://app.asana.com/0/0/1207533847538452/1207544369978642/f).
Reminding users of the option to import right when that friction happens
could be a way to quickly give recourse and solve the problem for all
subsequent browsing needs.

This feature promotes the import in autofill scenarios when the user has
few or no passwords saved.

**Steps to test this PR**:
(Same as macOS PR)
https://app.asana.com/0/72649045549333/1208245520043990/f

**OS Testing**:

* [ ] iOS 14
* [ ] iOS 15
* [ ] iOS 16
* [ ] macOS 10.15
* [ ] macOS 11
* [ ] macOS 12

---
###### Internal references:
[Software Engineering
Expectations](https://app.asana.com/0/59792373528535/199064865822552)
[Technical Design
Template](https://app.asana.com/0/59792373528535/184709971311943)
  • Loading branch information
@graeme
graeme authored Sep 10, 2024
1 parent 3dc2650 commit e036123
Show file tree
Hide file tree
Showing 13 changed files with 415 additions and 44 deletions.
4 changes: 2 additions & 2 deletions Package.resolved
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
"kind" : "remoteSourceControl",
"location" : "https://github.com/duckduckgo/duckduckgo-autofill.git",
"state" : {
"revision" : "1f12b78d9bac4a1d9b6bad18dc2ef0593bed34a3",
"version" : "13.0.0"
"revision" : "1fee787458d13f8ed07f9fe81aecd6e59609339e",
"version" : "13.1.0"
}
},
{
Expand Down
2 changes: 1 addition & 1 deletion Package.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ let package = Package(
.library(name: "Onboarding", targets: ["Onboarding"])
],
dependencies: [
.package(url: "https://github.com/duckduckgo/duckduckgo-autofill.git", exact: "13.0.0"),
.package(url: "https://github.com/duckduckgo/duckduckgo-autofill.git", exact: "13.1.0"),
.package(url: "https://github.com/duckduckgo/GRDB.swift.git", exact: "2.4.0"),
.package(url: "https://github.com/duckduckgo/TrackerRadarKit", exact: "3.0.0"),
.package(url: "https://github.com/duckduckgo/sync_crypto", exact: "0.2.0"),
Expand Down
119 changes: 107 additions & 12 deletions Sources/BrowserServicesKit/Autofill/AutofillUserScript+SecureVault.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,17 @@ public enum RequestVaultCredentialsAction: String, Codable {

public protocol AutofillSecureVaultDelegate: AnyObject {

typealias SecureVaultLoginsCount = Int

var autofillWebsiteAccountMatcher: AutofillWebsiteAccountMatcher? { get }
var tld: TLD? { get }

func autofillUserScript(_: AutofillUserScript, didRequestAutoFillInitDataForDomain domain: String, completionHandler: @escaping (
[SecureVaultModels.WebsiteCredentials],
[SecureVaultModels.Identity],
[SecureVaultModels.CreditCard],
SecureVaultModels.CredentialsProvider
SecureVaultModels.CredentialsProvider,
SecureVaultLoginsCount
) -> Void)

func autofillUserScript(_: AutofillUserScript, didRequestCreditCardsManagerForDomain domain: String)
Expand Down Expand Up @@ -75,6 +78,20 @@ public protocol AutofillSecureVaultDelegate: AnyObject {

}

public protocol AutofillLoginImportStateProvider {
var isNewDDGUser: Bool { get }
var hasImportedLogins: Bool { get }
var credentialsImportPromptPresentationCount: Int { get }
var isAutofillEnabled: Bool { get }
func hasNeverPromptWebsitesFor(_ domain: String) -> Bool
}

public protocol AutofillPasswordImportDelegate: AnyObject {
func autofillUserScriptDidRequestPasswordImportFlow(_ completion: @escaping () -> Void)
func autofillUserScriptDidFinishImportWithImportedCredentialForCurrentDomain()
func autofillUserScriptWillDisplayOverlay(_ serializedInputContext: String)
}

extension AutofillUserScript {

// MARK: - Response Objects
Expand Down Expand Up @@ -314,6 +331,7 @@ extension AutofillUserScript {
let creditCards: AvailableInputTypesCreditCards
let email: Bool
let credentialsProviderStatus: CredentialProviderStatus
let credentialsImport: Bool

}

Expand Down Expand Up @@ -433,19 +451,57 @@ extension AutofillUserScript {

func getAvailableInputTypes(_ message: UserScriptMessage, _ replyHandler: @escaping MessageReplyHandler) {
let domain = hostForMessage(message)
Self.domainOfMostRecentGetAvailableInputsMessage = domain
let email = emailDelegate?.autofillUserScriptDidRequestSignedInStatus(self) ?? false
vaultDelegate?.autofillUserScript(self, didRequestAutoFillInitDataForDomain: domain) { credentials, identities, cards, credentialsProvider in
vaultDelegate?.autofillUserScript(self, didRequestAutoFillInitDataForDomain: domain) { [weak self] credentials, identities, cards, credentialsProvider, totalCredentialsCount in
guard let self else {
replyHandler("")
return
}
let credentialsImport = self.shouldShowPasswordImportDialog(domain: domain, credentials: credentials, credentialsProvider: credentialsProvider, totalCredentialsCount: totalCredentialsCount)
let response = RequestAvailableInputTypesResponse(credentials: credentials,
identities: identities,
cards: cards,
email: email,
credentialsProvider: credentialsProvider)
credentialsProvider: credentialsProvider,
credentialsImport: credentialsImport)
if let json = try? JSONEncoder().encode(response), let jsonString = String(data: json, encoding: .utf8) {
replyHandler(jsonString)
}
}
}

private func shouldShowPasswordImportDialog(domain: String, credentials: [SecureVaultModels.WebsiteCredentials], credentialsProvider: SecureVaultModels.CredentialsProvider, totalCredentialsCount: Int) -> Bool {
guard loginImportStateProvider.isAutofillEnabled else {
return false
}
guard credentialsProvider.name != .bitwarden else {
return false
}
guard !isBurnerWindow else {
return false
}
guard loginImportStateProvider.credentialsImportPromptPresentationCount < 5 else {
return false
}
guard credentials.isEmpty else {
return false
}
guard totalCredentialsCount < 10 else {
return false
}
guard !loginImportStateProvider.hasImportedLogins else {
return false
}
guard loginImportStateProvider.isNewDDGUser else {
return false
}
guard !loginImportStateProvider.hasNeverPromptWebsitesFor(domain) else {
return false
}
return true
}

// https://github.com/duckduckgo/duckduckgo-autofill/blob/main/src/deviceApiCalls/schemas/getAutofillData.params.json
struct GetAutofillDataRequest: Codable {
let mainType: GetAutofillDataMainType
Expand Down Expand Up @@ -515,7 +571,7 @@ extension AutofillUserScript {

func pmGetAutoFillInitData(_ message: UserScriptMessage, _ replyHandler: @escaping MessageReplyHandler) {
let domain = hostForMessage(message)
vaultDelegate?.autofillUserScript(self, didRequestAutoFillInitDataForDomain: domain) { credentials, identities, cards, credentialsProvider in
vaultDelegate?.autofillUserScript(self, didRequestAutoFillInitDataForDomain: domain) { credentials, identities, cards, credentialsProvider, _ in
let credentialObjects: [CredentialObject]
if credentialsProvider.locked {
credentialObjects = [CredentialObject(id: "provider_locked", username: "", credentialsProvider: credentialsProvider.name.rawValue)]
Expand Down Expand Up @@ -715,6 +771,25 @@ extension AutofillUserScript {
replyHandler(nil)
}

// MARK: Credentials Import Flow

func startCredentialsImportFlow(_ message: UserScriptMessage, replyHandler: @escaping MessageReplyHandler) {
passwordImportDelegate?.autofillUserScriptDidRequestPasswordImportFlow { [weak self] in
NotificationCenter.default.post(name: .passwordImportDidCloseImportDialog, object: nil)
guard let self else {
replyHandler(nil)
return
}
let domain = Self.domainOfMostRecentGetAvailableInputsMessage ?? ""
vaultDelegate?.autofillUserScript(self, didRequestAccountsForDomain: domain, completionHandler: { [weak self] credentials, _ in
if !credentials.isEmpty {
self?.passwordImportDelegate?.autofillUserScriptDidFinishImportWithImportedCredentialForCurrentDomain()
}
replyHandler(nil)
})
}
}

// MARK: Pixels

public struct JSPixel: Equatable {
Expand All @@ -728,6 +803,10 @@ extension AutofillUserScript {
case autofillIdentity = "autofill_identity"
}

private enum CredentialsImportPromotionPixelName: String {
case promotionShown = "autofill_import_credentials_prompt_shown"
}

/// The pixel name sent by the JS layer. This name does not include the platform on which it was sent.
private let originalPixelName: String

Expand All @@ -753,6 +832,13 @@ extension AutofillUserScript {
return false
}

public var isCredentialsImportPromotionPixel: Bool {
if case CredentialsImportPromotionPixelName.promotionShown.rawValue = originalPixelName {
return true
}
return false
}

public var pixelName: String {
switch originalPixelName {
case EmailPixelName.autofillPersonalAddress.rawValue:
Expand Down Expand Up @@ -782,13 +868,18 @@ extension AutofillUserScript {
}
}

public extension Notification.Name {
static let passwordImportDidCloseImportDialog = Notification.Name("com.duckduckgo.browserServicesKit.PasswordImportDidCloseImportDialog")
}

extension AutofillUserScript.RequestAvailableInputTypesResponse {

init(accounts: [SecureVaultModels.WebsiteAccount],
identities: [SecureVaultModels.Identity],
cards: [SecureVaultModels.CreditCard],
email: Bool,
credentialsProvider: SecureVaultModels.CredentialsProvider) {
credentialsProvider: SecureVaultModels.CredentialsProvider,
credentialsImport: Bool) {
let credentialObjects: [AutofillUserScript.CredentialObject] = accounts.compactMap {
guard let id = $0.id, let username = $0.username else { return nil }
return .init(id: id, username: username, credentialsProvider: credentialsProvider.name.rawValue)
Expand All @@ -803,7 +894,8 @@ extension AutofillUserScript.RequestAvailableInputTypesResponse {
identities: identities,
creditCards: cards,
email: email,
credentialsProviderStatus: credentialsProvider.locked ? .locked : .unlocked
credentialsProviderStatus: credentialsProvider.locked ? .locked : .unlocked,
credentialsImport: credentialsImport
)
self.init(success: success, error: nil)
}
Expand All @@ -812,7 +904,8 @@ extension AutofillUserScript.RequestAvailableInputTypesResponse {
identities: [SecureVaultModels.Identity],
cards: [SecureVaultModels.CreditCard],
email: Bool,
credentialsProvider: SecureVaultModels.CredentialsProvider) {
credentialsProvider: SecureVaultModels.CredentialsProvider,
credentialsImport: Bool) {
let username = credentialsProvider.locked || credentials.hasAtLeastOneUsername
let password = credentialsProvider.locked || credentials.hasAtLeastOnePassword
let credentials = AutofillUserScript.AvailableInputTypesSuccess.AvailableInputTypesCredentials(username: username, password: password)
Expand All @@ -821,7 +914,8 @@ extension AutofillUserScript.RequestAvailableInputTypesResponse {
identities: AutofillUserScript.AvailableInputTypesSuccess.AvailableInputTypesIdentities(identities: identities),
creditCards: AutofillUserScript.AvailableInputTypesSuccess.AvailableInputTypesCreditCards(creditCards: cards),
email: email,
credentialsProviderStatus: credentialsProvider.locked ? .locked : .unlocked
credentialsProviderStatus: credentialsProvider.locked ? .locked : .unlocked,
credentialsImport: credentialsImport
)
self.init(success: success, error: nil)
}
Expand Down Expand Up @@ -919,10 +1013,11 @@ extension AutofillUserScript.AskToUnlockProviderResponse {
credentialsProvider: SecureVaultModels.CredentialsProvider) {

let availableInputTypesResponse = AutofillUserScript.RequestAvailableInputTypesResponse(credentials: credentials,
identities: identities,
cards: cards,
email: email,
credentialsProvider: credentialsProvider)
identities: identities,
cards: cards,
email: email,
credentialsProvider: credentialsProvider,
credentialsImport: false)
let status = credentialsProvider.locked ? AutofillUserScript.CredentialProviderStatus.locked : .unlocked
let credentialsArray: [AutofillUserScript.CredentialResponse] = credentials.compactMap { credential in
guard let id = credential.account.id,
Expand Down
20 changes: 17 additions & 3 deletions Sources/BrowserServicesKit/Autofill/AutofillUserScript.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ public class AutofillUserScript: NSObject, UserScript, UserScriptMessageEncrypti
case getIncontextSignupDismissedAt
case startEmailProtectionSignup
case closeEmailProtectionTab

case startCredentialsImportFlow
}

/// Represents if the autofill is loaded into the top autofill context.
Expand All @@ -69,11 +71,16 @@ public class AutofillUserScript: NSObject, UserScript, UserScriptMessageEncrypti
/// once the user selects a field to open, we store field type and other contextual information to be initialized into the top autofill.
public var serializedInputContext: String?

/// Represents whether the webView is part of a burner window
public var isBurnerWindow: Bool = false

public var sessionKey: String?

public weak var emailDelegate: AutofillEmailDelegate?
public weak var vaultDelegate: AutofillSecureVaultDelegate?
public weak var passwordImportDelegate: AutofillPasswordImportDelegate?

internal let loginImportStateProvider: AutofillLoginImportStateProvider
internal var scriptSourceProvider: AutofillUserScriptSourceProvider

internal lazy var autofillDomainNameUrlMatcher: AutofillDomainNameUrlMatcher = {
Expand All @@ -98,6 +105,9 @@ public class AutofillUserScript: NSObject, UserScript, UserScriptMessageEncrypti
return true
}

// Temporary only for Pixel purposes. Do not rely on this for any functional logic
static var domainOfMostRecentGetAvailableInputsMessage: String?

public var messageNames: [String] {
return MessageName.allCases.map(\.rawValue)
}
Expand Down Expand Up @@ -156,6 +166,7 @@ public class AutofillUserScript: NSObject, UserScript, UserScriptMessageEncrypti
case .getIncontextSignupDismissedAt: return getIncontextSignupDismissedAt
case .startEmailProtectionSignup: return startEmailProtectionSignup
case .closeEmailProtectionTab: return closeEmailProtectionTab
case .startCredentialsImportFlow: return startCredentialsImportFlow
}
}

Expand All @@ -167,19 +178,22 @@ public class AutofillUserScript: NSObject, UserScript, UserScriptMessageEncrypti
return hostProvider.hostForMessage(message)
}

public convenience init(scriptSourceProvider: AutofillUserScriptSourceProvider) {
public convenience init(scriptSourceProvider: AutofillUserScriptSourceProvider, loginImportStateProvider: AutofillLoginImportStateProvider) {
self.init(scriptSourceProvider: scriptSourceProvider,
encrypter: AESGCMUserScriptEncrypter(),
hostProvider: SecurityOriginHostProvider())
hostProvider: SecurityOriginHostProvider(),
loginImportStateProvider: loginImportStateProvider)
}

init(scriptSourceProvider: AutofillUserScriptSourceProvider,
encrypter: UserScriptEncrypter = AESGCMUserScriptEncrypter(),
hostProvider: UserScriptHostProvider = SecurityOriginHostProvider()) {
hostProvider: UserScriptHostProvider = SecurityOriginHostProvider(),
loginImportStateProvider: AutofillLoginImportStateProvider) {
self.scriptSourceProvider = scriptSourceProvider
self.hostProvider = hostProvider
self.encrypter = encrypter
self.isTopAutofillContext = false
self.loginImportStateProvider = loginImportStateProvider
}
}

Expand Down
4 changes: 2 additions & 2 deletions Sources/BrowserServicesKit/Autofill/OverlayAutofillUserScript.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ public class OverlayAutofillUserScript: AutofillUserScript {
}

/// Used to create a top autofill context script for injecting into a ContentOverlay
public convenience init(scriptSourceProvider: AutofillUserScriptSourceProvider, overlay: OverlayAutofillUserScriptPresentationDelegate) {
self.init(scriptSourceProvider: scriptSourceProvider, encrypter: AESGCMUserScriptEncrypter(), hostProvider: SecurityOriginHostProvider())
public convenience init(scriptSourceProvider: AutofillUserScriptSourceProvider, overlay: OverlayAutofillUserScriptPresentationDelegate, loginImportStateProvider: AutofillLoginImportStateProvider) {
self.init(scriptSourceProvider: scriptSourceProvider, encrypter: AESGCMUserScriptEncrypter(), hostProvider: SecurityOriginHostProvider(), loginImportStateProvider: loginImportStateProvider)
self.isTopAutofillContext = true
self.contentOverlay = overlay
}
Expand Down
1 change: 1 addition & 0 deletions Sources/BrowserServicesKit/Autofill/WebsiteAutofillUserScript.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ public class WebsiteAutofillUserScript: AutofillUserScript {
}
// Sets the last message host, so we can check when it messages back
lastOpenHost = hostProvider.hostForMessage(message)
passwordImportDelegate?.autofillUserScriptWillDisplayOverlay(serializedInputContext)

currentOverlayTab.websiteAutofillUserScript(self,
willDisplayOverlayAtClick: clickPoint,
Expand Down
Loading

0 comments on commit e036123

Please sign in to comment.