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

Autofill/subdomain matching #1122

Merged
merged 38 commits into from
May 17, 2023
Merged

Autofill/subdomain matching #1122

merged 38 commits into from
May 17, 2023

Conversation

afterxleep
Copy link
Contributor

@afterxleep afterxleep commented Apr 13, 2023

Task/Issue URL: https://app.asana.com/0/1204099484721401/1204214992981893/f

Description:

This is the integration branch for subdomain support in autofill-related changes.

Test Instructions

  • Tests should be 🟢

Merges to this branch have already been tested, but here are a few steps to smoke-test autofill TLD support. At the minimum, please test the basics and multiple logins.

Basics:

  • Go to Netflix.com, log in, and save your credentials

  • Manually add another login (different username and pass) using netflix.com as the site

  • Logout/Login to netflix.con and observe both logins are available.

  • Visit a TLD for which you have multiple logins

  • Observe all are presented as suggestions

  • Confirm that sorting and duplicate removal in the Autofill suggestion box works as expected

Feeling adventurous? Try these tests:

Sorting and Duplicate Removal:

  • Confirm sorting order for multiple credentials as follows:

Sorting logic:

  • Exact match
  • TLD (+ www.TLD)
  • Other subdomains

Each group is sorted by precedence:

  • Recently Updated (newest first, ⚠️ not incluiding time — just date)
  • Domain name (alpha)
  • User name (alpha)
  • Empty username

Duplicate removal

  • Create duplicate logins. (We consider a duplicate any login that is the same username and password for different TLD or subdomains. i.e. netflix.com and www.netflixcom
  • Observe that duplicate logins are removed in the autofill suggestion box, keeping only the most relevant option, based on the following conditions:

Exact match

  • Recently Updated (newest) OR
  • Domain name OR
  • User Name

- TLD (+ www.TLD)

  • Recently Updated (newest (Date only, not time)
  • Domain name (alpha)
  • User name (alpha)

Other subdomains

  • Most similar subdomain name

Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
When ready for review, remember to post the PR in MM

afterxleep and others added 4 commits April 5, 2023 17:34
Task/Issue URL:
https://app.asana.com/0/1204099484721401/1204318806095423/f

**Description**:
- We no longer remove the www prefix when saving passwords, as it's a
valid subdomain.
- We no longer pre-populate the title with the host name if no custom
title was provided
Task/Issue URL: https://app.asana.com/0/0/1204214992981892/f

Description:
Adding support for new autofill subdomain matching, sorting and new 'Manage Logins' action from the overlay form
Task/Issue URL: https://app.asana.com/0/1204099484721401/1204318806095429/f

Description:
We currently prevent favicons from Bookmarks and Fireproofed websites from being deleted. These changes also prevent the deletion of Favicons from non-fireproofed websites for which the user has login credentials stored.
@amddg44 amddg44 assigned amddg44 and unassigned amddg44 Apr 17, 2023
Task/Issue URL: https://app.asana.com/0/1204099484721401/1204318806095421/f

Description:
Summary: Sort login items by Title if one exists, otherwise sort by eTLD+1 then subdomain

Display the Title if it's not blank, otherwise, display subdomain.domain. Sort by title if present, then by domain, then by subdomain, example list of logins in order:
Task/Issue URL: https://app.asana.com/0/0/1204318806095419/f

Description:
If the URL is an exact match for the title, drop it on import.
sk/Issue URL: https://app.asana.com/0/0/1204319942044704/f

Description:
Prevent the user from saving an empty login.
Task/Issue URL: https://app.asana.com/0/1204099484721401/1204318806095433/f

Description:
In some cases, opening the Autofill popover causes the scroll to be janky or locked (see this video).

This issue has been around for a long time. We've tried fixing without luck here without luck.

It's still unclear why this is happening, but it may be related to the lazy stack rendering..

A. The view takes too long to render when you have several hundreds of login items, causing the selection/scroll to have issues.
B. The model not returning the list of items before we trigger the selection, which happens onAppear.

I've implemented a few changes and moved the selection to .onChange, adding a dynamic delay based on the list of items in the user's vault, so the list has more time to render with bigger datasets.

Still, not an ideal solution until Apple implements a proper .onLayout method for SwiftUI views.
Task/Issue URL:https://app.asana.com/0/1204099484721401/1204337437827879/f

Description:
Opens Bitwarden (if enabled) when selecting "Manage Logins..." from the password overlay.
# Conflicts:
#	DuckDuckGo.xcodeproj/project.pbxproj
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
#	DuckDuckGo/Fire/Model/Fire.swift
#	DuckDuckGo/Preferences/Model/AutofillPreferencesModel.swift
Task/Issue URL: https://app.asana.com/0/1204099484721401/1204318806095418/f

Description:
When importing logins, remove the title when it matches the URL or the Title matches a specific pattern.
# Conflicts:
#	DuckDuckGo.xcodeproj/project.pbxproj
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
# Conflicts:
#	DuckDuckGo.xcodeproj/project.pbxproj
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
# Conflicts:
#	DuckDuckGo.xcodeproj/project.pbxproj
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
Task/Issue URL:
https://app.asana.com/0/1204099484721401/1204538675714894/f

**Description**:
Selects the best matching option from the autofill UI, even if there is
not an exact match to the visited site.
afterxleep added a commit to duckduckgo/BrowserServicesKit that referenced this pull request May 17, 2023
Task/Issue URL: https://app.asana.com/0/1204099484721401/1204529101794549/f
iOS PR: duckduckgo/iOS#1702
macOS PR: duckduckgo/macos-browser#1122
What kind of version bump will this require?: Minor

Description:

Add custom sorting/filtering extension to [SecureVaultmodels.WebsiteAccount] for suggested domains.
New tests to validate sorting works as expected
Updates the password hashing keychain store to use the kSecClassInternetPassword category
@afterxleep
Copy link
Contributor Author

@amddg44 Updated to point the BSK integration branch

Copy link
Contributor

@amddg44 amddg44 left a comment

Choose a reason for hiding this comment

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

LGTM! Nice work @afterxleep 😄

afterxleep pushed a commit to duckduckgo/BrowserServicesKit that referenced this pull request May 17, 2023
Task/Issue URL: https://app.asana.com/0/0/1204214992981892/f
iOS PR: duckduckgo/iOS#1702
macOS PR: duckduckgo/macos-browser#1122

Description:
Adding support for new autofill subdomain matching, sorting, and new 'Manage Logins' action from the overlay form. This is the final PR from the integration branch. Merges have already been reviewed.
@afterxleep afterxleep merged commit cfe3d7b into develop May 17, 2023
@afterxleep afterxleep deleted the autofill/subdomain-matching branch May 17, 2023 15:42
samsymons added a commit that referenced this pull request May 23, 2023
* upstream_develop:
  sync device connected names (#1205)
  Autofill/subdomain matching (#1122)
  add-windows-browser-download-link-privacy-config (#1198)
  Update metadata
  Fix creating DMG from release workflow (#1200)
  Unit tests to cover sensitive areas of Burner Window (#1169)
  [Hackdays] Share via QR Code (#1177)
  Fix sync connect flow  (#1195)
  Update to BSK adding new remote message image (#1194)
  Set version to 1.39.0
  Update embedded files
  Update old Dax asset (#1193)
  WebKit find in page (#1188)
  modify manage bookmarks button (#1175)
  sync update device model / poll for devices (#1191)
samsymons added a commit that referenced this pull request May 27, 2023
* release/1.42.0: (26 commits)
  Bump version to 1.42.0 (28)
  Update embedded files
  Network Protection (#1211)
  Update BSK with autofill 7.1.0 (#1225)
  Autofill password generation support for iOS (#1212)
  Sync Engine with support for syncing bookmarks (#1203)
  Bump BSK for messaging updates (#1213)
  add newTab pixel, fire pixels only for new users first time (#1219)
  Update tds endpoint (#1218)
  prevent devices from being fetch when sync not visible (#1214)
  License and contributor guidelines for open-sourcing (#1189)
  Add a unique Bitwarden decryption failure pixel (#1197)
  Bump version to 1.41.0 (27)
  Update embedded files
  Check that the source and target frame security origins are equal (#1207)
  Add activation points pixels (#1206)
  Add Login export links to settings and Autofill 3 dot menu (#1183)
  sync device connected names (#1205)
  Autofill/subdomain matching (#1122)
  add-windows-browser-download-link-privacy-config (#1198)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants