Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

feat(app): basic password authentication #2940

Merged
merged 34 commits into from
Oct 11, 2019
Merged

feat(app): basic password authentication #2940

merged 34 commits into from
Oct 11, 2019

Conversation

korhaliv
Copy link
Member

@korhaliv korhaliv commented Oct 7, 2019 •
edited
Loading

Description:

Continues #2860

This PR adds a number of improvements:

  1. Stores hashes of password in OS secure storage (keychain on macOS, credentials manager on Windows, gnome keyring on Ubuntu)
  2. Adds ability to change password via dialog
  3. Adds ability to disable/enable password via dialog
  4. Adds infrastructure for password protected functionality (password prompt)
    Resolves security for wallet (pin / password) #1387

How Has This Been Tested?

Manually

Screenshots (if appropriate):

image
image
image

Types of changes:

New feature

Checklist:

  • My code follows the code style of this project.
  • I have reviewed and updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes where needed.
  • All new and existing tests passed.
  • My commits have been squashed into a concise set of changes.

@korhaliv korhaliv added type: enhancement New feature or request scope: security issues that are security related labels Oct 7, 2019
@korhaliv korhaliv added this to the v0.6.0-beta milestone Oct 7, 2019
@korhaliv korhaliv requested a review from mrfelton October 7, 2019 11:29
@korhaliv korhaliv self-assigned this Oct 7, 2019
@korhaliv korhaliv force-pushed the feat/app-password branch 3 times, most recently from 504b823 to e03f278 Compare October 7, 2019 17:17
@mrfelton mrfelton mentioned this pull request Oct 7, 2019
Closed
6 tasks
mrfelton
mrfelton suggested changes Oct 8, 2019
View reviewed changes
Copy link
Member

@mrfelton mrfelton left a comment

Choose a reason for hiding this comment

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

Some comments

renderer/containers/Initializer.js Show resolved Hide resolved
utils/crypto.js Outdated Show resolved Hide resolved
test/unit/components/UI/__snapshots__/Countdown.spec.js.snap Outdated Show resolved Hide resolved
renderer/store/schema/account.js Outdated Show resolved Hide resolved
renderer/reducers/modal.js Show resolved Hide resolved
renderer/reducers/account.js Outdated Show resolved Hide resolved
renderer/reducers/account.js Outdated Show resolved Hide resolved
renderer/reducers/account.js Outdated Show resolved Hide resolved
renderer/components/Settings/Security/hooks.js Outdated Show resolved Hide resolved
renderer/components/Settings/Security/PasswordState.js Outdated Show resolved Hide resolved
@korhaliv
Copy link
Member Author

korhaliv commented Oct 8, 2019

@mrfelton PR updated

@korhaliv korhaliv requested a review from mrfelton October 8, 2019 20:37
mrfelton
mrfelton suggested changes Oct 9, 2019
View reviewed changes
Copy link
Member

@mrfelton mrfelton left a comment

Choose a reason for hiding this comment

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

Nice one. A few things @korhaliv

renderer/components/Login/Login.js Outdated Show resolved Hide resolved
electron/secureStorage/service.js Outdated Show resolved Hide resolved
renderer/reducers/utils/waitForIpc.js Outdated Show resolved Hide resolved
test/unit/utils/sha256.spec.js Show resolved Hide resolved
translations/de-DE.json Outdated Show resolved Hide resolved
@korhaliv korhaliv force-pushed the feat/app-password branch 4 times, most recently from 467e1e3 to 7687671 Compare October 9, 2019 13:58
@korhaliv korhaliv requested a review from mrfelton October 9, 2019 13:58
@korhaliv korhaliv added scope: devops issues that affect devops scope: tests issues relating to the test suit and removed scope: devops issues that affect devops labels Oct 9, 2019
@korhaliv korhaliv requested a review from JimmyMow October 9, 2019 14:00
@coveralls
Copy link

coveralls commented Oct 9, 2019 •
edited
Loading

Coverage Status

Coverage increased (+0.03%) to 22.911% when pulling 673418c on korhaliv:feat/app-password into 39fea5d on LN-Zap:next.

@korhaliv korhaliv force-pushed the feat/app-password branch 2 times, most recently from c862ded to 62d9922 Compare October 10, 2019 18:04
mrfelton
mrfelton suggested changes Oct 10, 2019
View reviewed changes
Copy link
Member

@mrfelton mrfelton left a comment

Choose a reason for hiding this comment

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

A few minor comments on the code @korhaliv .

renderer/components/Login/messages.js Outdated Show resolved Hide resolved
renderer/components/Login/messages.js Outdated Show resolved Hide resolved
webpack/webpack.config.e2e.js Show resolved Hide resolved
renderer/reducers/utils/waitForIpc.js Outdated Show resolved Hide resolved
renderer/reducers/utils/waitForIpc.js Outdated Show resolved Hide resolved
electron/secureStorage/ipcCRUD.js Outdated Show resolved Hide resolved
renderer/reducers/account.js Outdated Show resolved Hide resolved
renderer/reducers/account.js Outdated Show resolved Hide resolved
renderer/reducers/account.js Outdated Show resolved Hide resolved
renderer/reducers/account.js Outdated Show resolved Hide resolved
mrfelton and others added 25 commits October 11, 2019 12:45
@mrfelton @korhaliv
refactor: show loading bolt if login is required
4152467
@mrfelton @korhaliv
docs(storybook): add Login container example
a1f670e
@korhaliv
fix(ui): add password field tooltip
a94fb9d
@korhaliv
fix(ui): prevent Toggle from selecting text when clicked
8d15568
@korhaliv
refactor(utils): add sha256digest util
98cbe6a
@korhaliv
test(utils): add sha256digest util test
26ec01c
@korhaliv
refactor: introduce secureStorage service
1816c48
@korhaliv
refactor: add PasswordState component
828f68d
@korhaliv
refactor: add PasswordPromptDialog component
49eac06
@korhaliv
refactor: add ChangePasswordDialog component
3e666a4
@korhaliv
docs(storybook): add password dialog stories
ab2d78f
@korhaliv
refactor: introduce common dialog reducer
32ff953
@korhaliv
feat(ui): allow changing and toggling app password via dialogs
892d592
@korhaliv
refactor: add useFormError hook
7e1930e
@korhaliv
test(unit): update snapshots
c0ce1c2
@korhaliv
refactor(utils): introduce waitForIpc util
2630b56
@korhaliv
refactor: use keystore as single source of truth for password state
c04d21d
@korhaliv
test(e2e): ensure e2e won't encounter app password
6bfd014
@korhaliv
refactor: introduce CRUD provider for the secure storage <-> ipc
b406a22
@korhaliv
test: add integration tests for ipcCRUD provider
d9cd1d5
@korhaliv
refactor: add LoginNotAllowed component
488ada5
@korhaliv
docs(storybook): add LoginNotAllowed component
8ca8d63
@korhaliv
feat(ui): prohibit login if secure storage is not available
56f2169
@korhaliv
test(e2e): use mock keytar when running e2e on linux
404036a
@korhaliv
refactor: make account a persistent reducer
dc95517
mrfelton
mrfelton approved these changes Oct 11, 2019
View reviewed changes
Copy link
Member

@mrfelton mrfelton left a comment

Choose a reason for hiding this comment

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

Tested ACK dc95517

@mrfelton mrfelton merged commit 12fcf31 into LN-Zap:next Oct 11, 2019
@korhaliv korhaliv mentioned this pull request Oct 15, 2019
Closed
@bolatovumar bolatovumar mentioned this pull request Oct 17, 2019
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mrfelton mrfelton mrfelton approved these changes

@JimmyMow JimmyMow Awaiting requested review from JimmyMow

Assignees

@korhaliv korhaliv

Labels
scope: security issues that are security related scope: tests issues relating to the test suit type: enhancement New feature or request
Projects
None yet
Milestone
v0.6.0-beta
Development

Successfully merging this pull request may close these issues.
3 participants
@korhaliv @coveralls @mrfelton