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

Manage Duck Addresses #1828

Merged
merged 37 commits into from
Aug 3, 2023
Merged

Manage Duck Addresses #1828

merged 37 commits into from
Aug 3, 2023

Conversation

afterxleep
Copy link
Collaborator

@afterxleep afterxleep commented Jul 13, 2023

Task/Issue URL: https://app.asana.com/0/1142021229838617/1205057443326659/f

Description:
Allows you to manage your private Duck addresses

Steps to test this PR:

  1. Enable Autofill
  2. Create a login that includes a valid private Duck Address
  3. Observe the management UI for the address is available
  4. Enable and disable the duck Address
  5. Observe state changes properly
  6. Update the username of the login so the private address is no longer valid
  7. Observe the management UI is not available anymore

Other tests

  • Smoke test email and password generation

Demo:
demo-2

Orientation Testing:

  • Portrait
  • Landscape

Device Testing:

  • iPhone SE (1st Gen)
  • iPhone 8
  • iPhone X
  • iPhone 14 Pro
  • iPad

OS Testing:

  • iOS 14
  • iOS 15
  • iOS 16

Theme Testing:

  • Light theme
  • Dark theme

Internal references:

Software Engineering Expectations
Technical Design Template

@github-actions
Copy link

github-actions bot commented Jul 17, 2023

Warnings
⚠️ PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS against 7d50ee9

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.

@afterxleep it looks like there is still an issue with state not being properly reset. I was running through your test steps and found (and can reproduce consistently):

  • If I test 3b from a fresh app launch, everything is good
  • If I test 3a, and then 3b in the same session my personal duck address saves automatically

I spent some time digging into this and it looks to me that autogeneratedCredentials in SecureVaultManager is not being reset correctly to false after 3a

My exact test steps were:

  1. On fill.dev I generated a private address and entered a manual password
  2. Credentials were saved automatically ✅
  3. I deleted this login and went back and chose my personal duck address
  4. Before entering a password or tapping submit, I checked Logins and found a partial login saved with my personal duck address

@afterxleep
Copy link
Collaborator Author

@amddg44 Looking into this

@afterxleep
Copy link
Collaborator Author

afterxleep commented Jul 27, 2023

  • test 3a, and then 3b

@amddg44 I've made a couple minor updates to split the logic between autogenerated usernames and passwords, updated a test and added a new one to cover for this particular scenario:

RocketSim_Recording_iPhone_14_Pro_2023-07-27_20 45 10

@afterxleep afterxleep requested a review from amddg44 July 27, 2023 18:49
@afterxleep
Copy link
Collaborator Author

@amddg44

Thanks again for the review. I've double checked and fixed 3c, which as expected was also caused by the form submission autogenerated key not working on iOS as it does on Mac. I also added a test for this, and manually tested 3d, 3e and 3f.

Changes to BSK here

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.

@afterxleep nice work! just a couple of nits (mostly removing some dead code) 🙂

DuckDuckGo/TabViewController.swift Show resolved Hide resolved
DuckDuckGo/TabViewController.swift Show resolved Hide resolved
DuckDuckGo/AutofillLoginDetailsView.swift Outdated Show resolved Hide resolved
# Conflicts:
#	DuckDuckGo.xcodeproj/project.pbxproj
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
#	DuckDuckGo/AutofillLoginDetailsViewModel.swift
#	DuckDuckGo/TabViewController.swift
afterxleep added a commit to duckduckgo/BrowserServicesKit that referenced this pull request Aug 3, 2023
Task/Issue URL: https://app.asana.com/0/72649045549333/1204615490114675/f
iOS PR: duckduckgo/iOS#1828
macOS PR: duckduckgo/macos-browser#1319
What kind of version bump will this require?: Minor

Description:
Allows users to manage their Private Email addresses from the browser.
# Conflicts:
#	DuckDuckGo.xcodeproj/project.pbxproj
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
#	DuckDuckGo/UserText.swift
#	DuckDuckGo/en.lproj/Localizable.strings
@afterxleep afterxleep merged commit 8c556e5 into develop Aug 3, 2023
6 checks passed
@afterxleep afterxleep deleted the daniel/private-emails branch August 3, 2023 13:34
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