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 Private Email Addresses #396

Merged
merged 97 commits into from
Aug 3, 2023
Merged

Conversation

afterxleep
Copy link
Collaborator

@afterxleep afterxleep commented Jun 30, 2023

Please review the release process for BrowserServicesKit here.

Required:

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

Optional:

Description:
Allows users to manage their Private Email addresses from the browser.

Steps to test this PR:

  1. Check macOS/iOS PRs and test functionality
  2. Check iOS PR and smoke test password generation and credentials saving

Internal references:

Software Engineering Expectations
Technical Design Template

# Conflicts:
#	Sources/BrowserServicesKit/Email/EmailManager.swift
@afterxleep
Copy link
Collaborator Author

@amddg44

A few changes to fix the reported issues:

  1. Usernames are saved when generating a password (if present) + test

  2. When creating a private email address and then switching to a manual address, a prompt was presented, but you ended up with two accounts. I added a test for this which passes in the MockVaultProvider, but apparently the DefaultSecureVault fails. Moving temporary account deletion to the main function fixes the issue. This was likely caused by an unexpected vault dealloc. ??

  3. When generating a password on a site and navigating away to another login form in a different domain credentials were autosaved, even if no data was autogenerated. I have added logic to clear up autosaveAccount and autogenerated username and pass bools when navigating away, and added a test for coverage.

@afterxleep
Copy link
Collaborator Author

@amddg44

As for the reported issue "3f": (Existing account being auto-updated when generating password), the behavior on Android is to create another account.

While we cannot do that due to the unique username constraint, I'm no longer auto-updating data for those accounts, which should trigger a update dialog on submission. This is also covered by a unit test

Screen Recording 2023-08-01 at 8 27 28 PM

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! This is a significant improvement to the logic around autosaving; great work @afterxleep 🎉

afterxleep and others added 14 commits August 2, 2023 17:55
* replace assertion in assertObjectDeallocated with sigint

* add different options to ensureObjectDeallocated; fix some warnings
Task/Issue URL: https://app.asana.com/0/0/1205093266805582/f

Rework Sync initialization mechanism by moving sync data models initialization from
DDGSync (where it was tied to account signup/login flow) to data models themselves.
Sync Metadata is leveraged to store sync setup state per data model. Two values are
possible: needs remote data fetch (a.k.a initial sync) or ready to sync.
Decoupling account flow from data models initialization allows to initialize data models
independently of user turning on sync, which is a requirement to support initial sync
for new syncable models added with client app updates.
Also DataProvider class has been added – it's a dedicated sync data providers superclass
that also encapsulates common logic, making individual data providers simpler.
Task/Issue URL: https://app.asana.com/0/72649045549333/1203956617610891/f
Tech Design URL: https://app.asana.com/0/481882893211075/1204384679803788/f

Making Fire Window available to public

# Conflicts:
#	Sources/BrowserServicesKit/SecureVault/SecureVaultManager.swift
Task/Issue URL: https://app.asana.com/0/0/1205119749390492/f
Tech Design URL:
CC: @Bunn @jotaemepereira

Description:

This PR extracts the reusable SecureVault logic out into a new SecureStorage module.

This module is used by Autofill, and is usable by the client apps directly to implement secure storage on their own. The API for this new module matches the existing API as much as possible, and the implementation of the key/crypto providers is made easier by providing default implementations of the generic functions.

Note that not all areas of the code have been renamed; for example, there is still a struct called SecureVaultModels which I would like to rename AutofillModels, but in this case this symbol is referenced over 600 times, so I plan to do this in a follow-up renaming PR to keep this PR as small as possible (it's already quite large).
# Conflicts:
#	Sources/BrowserServicesKit/SecureVault/SecureVaultManager.swift
#	Tests/BrowserServicesKitTests/SecureVault/SecureVaultManagerTests.swift
Task/Issue URL: https://app.asana.com/0/414235014887631/1203017403159775/f
iOS PR: duckduckgo/iOS#1862
macOS PR: duckduckgo/macos-browser#1388

Description:
Class that aggregates detected Ad Attributions on a websites and stores that count over a certain time interval.
Co-authored-by: Shane Osbourne <sosbourne@duckduckgo.com>
* Use more swift concurrency in pinger tests

* Remove dead code and duped tests
Task/Issue URL: https://app.asana.com/0/414235014887631/1205157723964854/f
iOS PR: duckduckgo/iOS#1875
macOS PR: duckduckgo/macos-browser#1406

* os_log fixes

* add log visibility modifier to interpolated os_log

* fix os_log overloads
# Conflicts:
#	Sources/BrowserServicesKit/SecureVault/SecureVaultManager.swift
@afterxleep afterxleep merged commit 2931fce into main Aug 3, 2023
3 checks passed
@afterxleep afterxleep deleted the daniel/private-email-autofill branch August 3, 2023 11:10
afterxleep added a commit to duckduckgo/macos-browser that referenced this pull request Aug 3, 2023
Task/Issue URL:
https://app.asana.com/0/72649045549333/1204615490114675/f
BSK PR: duckduckgo/BrowserServicesKit#396

**Description**:
Enable/Disable Private Email Addresses from the Password management UI

---
###### Internal references:
[Pull Request Review
Checklist](https://app.asana.com/0/1202500774821704/1203764234894239/f)
[Software Engineering
Expectations](https://app.asana.com/0/59792373528535/199064865822552)
[Technical Design
Template](https://app.asana.com/0/59792373528535/184709971311943)
[Pull Request
Documentation](https://app.asana.com/0/1202500774821704/1204012835277482/f)
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.

10 participants