Skip to content

Commit

Permalink
Enable credentials import promo for all users (#1014)
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/1202926619870900/1208282574906775/f
iOS PR: duckduckgo/iOS#3427
macOS PR: duckduckgo/macos-browser#3383
What kind of version bump will this require?: Major

**Optional**:

Task/Issue URL:
https://app.asana.com/0/1202926619870900/1208282574906775/f
Tech Design URL:
https://app.asana.com/0/1202926619870900/1208502988382869/f

**Description**:

- We're rolling out [✓ Promote password import in autofill menu
[2w]](https://app.asana.com/0/0/1201480346793878) and the conversion
rate seems pretty decent. If we expand the addressable userbase we're
likely to improve the user experience of many more users.

- With [✓ Promote password import in autofill menu
[2w]](https://app.asana.com/0/0/1201480346793878) we're promoting import
to <7 days users with less than 10 passwords saved.
- To make this promotion even more impactful, we can expand the coverage
to existing users, but to make it impactful we'll need more lax
heuristics and a way for users to dismiss this promo.

**Steps to test this PR**:
[Test
Instructions](https://app.asana.com/0/1202926619870900/1208543152554655)

<!--
Before submitting a PR, please ensure you have tested the combinations
you expect the reviewer to test, then delete configurations you *know*
do not need explicit testing.

Using a simulator where a physical device is unavailable is acceptable.
-->

**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 authored Oct 21, 2024
1 parent f1a033c commit 9f62aac
Show file tree
Hide file tree
Showing 12 changed files with 60 additions and 175 deletions.
4 changes: 2 additions & 2 deletions Package.resolved
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" : "1fee787458d13f8ed07f9fe81aecd6e59609339e",
"version" : "13.1.0"
"revision" : "945ac09a0189dc6736db617867fde193ea984b20",
"version" : "15.0.0"
}
},
{
Expand Down
2 changes: 1 addition & 1 deletion Package.swift
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.1.0"),
.package(url: "https://github.com/duckduckgo/duckduckgo-autofill.git", exact: "15.0.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
Original file line number Diff line number Diff line change
Expand Up @@ -78,18 +78,12 @@ 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 autofillUserScriptShouldShowPasswordImportDialog(domain: String, credentials: [SecureVaultModels.WebsiteCredentials], credentialsProvider: SecureVaultModels.CredentialsProvider, totalCredentialsCount: Int) -> Bool
func autofillUserScriptDidRequestPasswordImportFlow(_ completion: @escaping () -> Void)
func autofillUserScriptDidFinishImportWithImportedCredentialForCurrentDomain()
func autofillUserScriptWillDisplayOverlay(_ serializedInputContext: String)
func autofillUserScriptShouldDisplayOverlay(_ serializedInputContext: String, for domain: String) -> Bool
func autofillUserScriptDidRequestPermanentCredentialsImportPromptDismissal()
}

extension AutofillUserScript {
Expand Down Expand Up @@ -458,7 +452,7 @@ extension AutofillUserScript {
replyHandler("")
return
}
let credentialsImport = self.shouldShowPasswordImportDialog(domain: domain, credentials: credentials, credentialsProvider: credentialsProvider, totalCredentialsCount: totalCredentialsCount)
let credentialsImport = self.passwordImportDelegate?.autofillUserScriptShouldShowPasswordImportDialog(domain: domain, credentials: credentials, credentialsProvider: credentialsProvider, totalCredentialsCount: totalCredentialsCount) ?? false
let response = RequestAvailableInputTypesResponse(credentials: credentials,
identities: identities,
cards: cards,
Expand All @@ -471,37 +465,6 @@ extension AutofillUserScript {
}
}

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 @@ -790,6 +753,12 @@ extension AutofillUserScript {
}
}

func credentialsImportFlowPermanentlyDismissed(_ message: UserScriptMessage, replyHandler: @escaping MessageReplyHandler) {
passwordImportDelegate?.autofillUserScriptDidRequestPermanentCredentialsImportPromptDismissal()
replyHandler(nil)
NotificationCenter.default.post(name: .passwordImportDidCloseImportDialog, object: nil)
}

// MARK: Pixels

public struct JSPixel: Equatable {
Expand Down
12 changes: 5 additions & 7 deletions Sources/BrowserServicesKit/Autofill/AutofillUserScript.swift
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ public class AutofillUserScript: NSObject, UserScript, UserScriptMessageEncrypti
case closeEmailProtectionTab

case startCredentialsImportFlow
case credentialsImportFlowPermanentlyDismissed
}

/// Represents if the autofill is loaded into the top autofill context.
Expand All @@ -80,7 +81,6 @@ public class AutofillUserScript: NSObject, UserScript, UserScriptMessageEncrypti
public weak var vaultDelegate: AutofillSecureVaultDelegate?
public weak var passwordImportDelegate: AutofillPasswordImportDelegate?

internal let loginImportStateProvider: AutofillLoginImportStateProvider
internal var scriptSourceProvider: AutofillUserScriptSourceProvider

internal lazy var autofillDomainNameUrlMatcher: AutofillDomainNameUrlMatcher = {
Expand Down Expand Up @@ -167,6 +167,7 @@ public class AutofillUserScript: NSObject, UserScript, UserScriptMessageEncrypti
case .startEmailProtectionSignup: return startEmailProtectionSignup
case .closeEmailProtectionTab: return closeEmailProtectionTab
case .startCredentialsImportFlow: return startCredentialsImportFlow
case .credentialsImportFlowPermanentlyDismissed: return credentialsImportFlowPermanentlyDismissed
}
}

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

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

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

Expand Down
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, loginImportStateProvider: AutofillLoginImportStateProvider) {
self.init(scriptSourceProvider: scriptSourceProvider, encrypter: AESGCMUserScriptEncrypter(), hostProvider: SecurityOriginHostProvider(), loginImportStateProvider: loginImportStateProvider)
public convenience init(scriptSourceProvider: AutofillUserScriptSourceProvider, overlay: OverlayAutofillUserScriptPresentationDelegate) {
self.init(scriptSourceProvider: scriptSourceProvider, encrypter: AESGCMUserScriptEncrypter(), hostProvider: SecurityOriginHostProvider())
self.isTopAutofillContext = true
self.contentOverlay = overlay
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,10 @@ public class WebsiteAutofillUserScript: AutofillUserScript {
}
// Sets the last message host, so we can check when it messages back
lastOpenHost = hostProvider.hostForMessage(message)
passwordImportDelegate?.autofillUserScriptWillDisplayOverlay(serializedInputContext)
if passwordImportDelegate?.autofillUserScriptShouldDisplayOverlay(serializedInputContext, for: hostForMessage(message)) != true {
replyHandler(nil)
return
}

currentOverlayTab.websiteAutofillUserScript(self,
willDisplayOverlayAtClick: clickPoint,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ public enum AutofillSubfeature: String, PrivacySubfeature {
case onByDefault
case onForExistingUsers
case unknownUsernameCategorization
case credentialsImportPromotionForExistingUsers
}

public enum DBPSubfeature: String, Equatable, PrivacySubfeature {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class AutofillEmailUserScriptTests: XCTestCase {
properties: properties)
.withJSLoading()
.build()
return AutofillUserScript(scriptSourceProvider: sourceProvider, encrypter: MockEncrypter(), hostProvider: SecurityOriginHostProvider(), loginImportStateProvider: MockAutofillLoginImportStateProvider())
return AutofillUserScript(scriptSourceProvider: sourceProvider, encrypter: MockEncrypter(), hostProvider: SecurityOriginHostProvider())
}()
let userContentController = WKUserContentController()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class AutofillVaultUserScriptTests: XCTestCase {
let sourceProvider = DefaultAutofillSourceProvider(privacyConfigurationManager: privacyConfig,
properties: properties,
isDebug: false)
return AutofillUserScript(scriptSourceProvider: sourceProvider, hostProvider: hostProvider, loginImportStateProvider: MockAutofillLoginImportStateProvider())
return AutofillUserScript(scriptSourceProvider: sourceProvider, hostProvider: hostProvider)
}()

let userContentController = WKUserContentController()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class EmailManagerTests: XCTestCase {
sessionKey: "1234",
featureToggles: ContentScopeFeatureToggles.allTogglesOn),
isDebug: false)
let userScript = AutofillUserScript(scriptSourceProvider: sourceProvider, loginImportStateProvider: MockAutofillLoginImportStateProvider())
let userScript = AutofillUserScript(scriptSourceProvider: sourceProvider)
return userScript
}

Expand Down
Loading

0 comments on commit 9f62aac

Please sign in to comment.