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

Import passwords from Chrome #77

Merged
merged 2 commits into from
Apr 3, 2018
Merged

Import passwords from Chrome #77

merged 2 commits into from
Apr 3, 2018

Conversation

garrettr
Copy link
Contributor

@garrettr garrettr commented Mar 30, 2018

Submitter Checklist:

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).
  • Tagged reviewers and labelled the pull request as needed.
  • Request a security/privacy review as needed.

Test Plan:

  1. Create a test Chrome profile.
  2. Log in to any website and allow Chrome to save the password when prompted.
  3. Open Brave
  4. Go to Brave → Import Bookmarks and Settings...
  5. Choose your test Chrome profile, make sure Saved passwords option is checked, click Import.
  6. "Allow" any prompts for access to "Brave Safe Storage" or "Chrome Safe Storage."
  7. Attempt to login to the website from earlier. The form fields should be autofilled (highlighted yellow) and a key icon should be visible in the address bar.

I spent some time working on automated tests for this branch, but they were complicated by the design of OSCrypt and interactions with the various system keychains. To keep momentum going, I'm posting this branch for review without tests. I think I may be able to get a reasonable unit test working in the near future, which could be included in this PR or a follow-up.

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions

@garrettr garrettr requested a review from darkdh March 30, 2018 15:24
@darkdh
Copy link
Member

darkdh commented Apr 2, 2018

Please rebase against master, thx

Garrett Robinson added 2 commits April 2, 2018 17:14
We want to reuse Chrome's code as much as possible when importing encrypted
data (e.g. saved passwords, cookies) from a Chrome profile. The encryption is
managed by OSCrypt, which has an unfortunate singleton design that makes it
difficult to modify its behavior at runtime. This commit uses Muon's approach
of using a command line switch as a proxy for a global variable to change the
behavior of OSCrypt in the importer helper process.
@garrettr garrettr force-pushed the import-chrome-passwords branch from 2f9b051 to 74fa96c Compare April 3, 2018 02:39
@garrettr
Copy link
Contributor Author

garrettr commented Apr 3, 2018

@darkdh Rebased!

Copy link
Member

@darkdh darkdh left a comment

Choose a reason for hiding this comment

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

++

@darkdh darkdh merged commit f40fbfd into master Apr 3, 2018
@bsclifton bsclifton deleted the import-chrome-passwords branch June 18, 2018 06:29
NejcZdovc pushed a commit that referenced this pull request Dec 10, 2018
make production/verbose extern
bbondy added a commit that referenced this pull request Feb 18, 2019
Add sitehacks and cosmetic filters from browser-laptop
fmarier pushed a commit that referenced this pull request Oct 29, 2019
petemill pushed a commit that referenced this pull request Jul 27, 2020
petemill pushed a commit that referenced this pull request Jul 28, 2020
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