Select group when adding new credentials by varjolintu · Pull Request #396 · keepassxreboot/keepassxc-browser

varjolintu · 2019-01-23T07:48:46Z

Allows selecting a group when adding new credentials to KeePassXC from the popup dialog.

A new command get-database-groups is added to the protocol. If older version than KeePassXC 2.4.0 is used, credentials are saved to the default KeePassXC-Browser Passwords group. For saving, both name and group UUID are used.

The groups are read from a dynamic array where each group can contain N children:

{
    name: '<group name>',
    uuid: '<group UUID>',
    children: [...]
}

Fixes #382.
Fixes #388.
Fixes #427.

Related KeePassXC PR: keepassxreboot/keepassxc#2637.

Also, a default group can be set, or the group can be asked every time:

droidmonkey · 2019-01-23T19:23:58Z

This is not scalable and could get very confusing for people with many nested groups. Would it be possible to use an accordion type widget that shows only the top level groups, when that group is clicked it should the child, etc etc?

Alternatively a select form element would be better since it would only ever show one group and the whole contents would be visible when dropped down.

varjolintu · 2019-01-24T13:01:37Z

I updated the PR to include a tree view. I'll update the KeePassXC side soon.

droidmonkey · 2019-01-24T14:05:22Z

Is the tree collapsed by default? (Should be) the arrows don't seem to indicate the correct state.

varjolintu · 2019-01-24T14:10:44Z

@droidmonkey It's not collapsed, or cannot be collapsed. It's just a tree view. What do you suggest instead of the arrows (maybe arrow down like in KeePassXC)? Or should it definitely be able to collapse?

varjolintu · 2019-01-24T14:48:27Z

I changed the icon to arrow down, indicating open tree. For now.

keepassxc-browser/background/keepass.js

droidmonkey · 2019-01-24T18:15:23Z

keepassxc-browser/background/keepass.js

+                    keepass.updateLastUsed(keepass.databaseHash);
+                    callback(groups);
+                } else {
+                    console.log('getDatabaseGroups rejected');


Would you want to show a proper error at this point? Would the CANNOT_DECRYPT_MESSAGE be appropriate here?

It's not appropriate here. The actual decryption is made much earlier. The extension actually doesn't have an error message for failed verification. I'll add it as a separate PR later, because that is something that every protocol command will need.

Something like "Invalid or corrupt message sent from KeePassXC"

Something like that yes. Currently the verifyResponse() returns false from multiple reasons, so the message must be a general one. The one you proposed or something like: "Message sent from KeePassXC cannot be verified".

varjolintu · 2019-03-09T12:06:56Z

I added one more thing to the PR. A default group can be specified, or the user can set an option that asks for the group every time when saving new credentials.

droidmonkey · 2019-03-12T20:21:50Z

There is still a problem. When defining the group to save to, if it does not exist then the entry is silently dropped. The group should be created to store the new entry if it doesn't already exist. This will likely require changes on KeePassXC side.

varjolintu · 2019-03-13T14:38:14Z

It also seems that triggering notifications from the popup doesn't work. Some changes are needed for that too. Probably we need to call the background script to activate them.

varjolintu · 2019-03-14T09:05:36Z

@droidmonkey That is now done. KeePassXC side support is here: keepassxreboot/keepassxc#2790.

keepassxc-browser/_locales/en/messages.json

droidmonkey · 2019-03-15T14:52:21Z

I thought of a little vulnerability with create entry/group. There is really nothing stopping abuse of the API to create an entry or group. We prevent retrieving information from the database by showing allow/deny dialog, but we do not prevent writing to the database (from KeePassXC).

varjolintu · 2019-03-15T16:08:28Z

@droidmonkey Yes. We need to restrict this in some way.

droidmonkey · 2019-03-16T02:41:52Z

We can introduce the protections in 2.4.1

droidmonkey

I really like this functionality

varjolintu added the enhancement label Jan 23, 2019

varjolintu added this to the Next milestone Jan 23, 2019

varjolintu requested review from droidmonkey and phoerious January 23, 2019 07:48

This was referenced Jan 23, 2019

Select group when adding credentials from browser extension keepassxreboot/keepassxc#2637

Merged

Allow selection of KeePassXC group when creating new entry #382

Closed

varjolintu force-pushed the feature/group_selection branch 7 times, most recently from 8a2641c to ece93dc Compare January 24, 2019 13:32

varjolintu force-pushed the feature/group_selection branch from ece93dc to 05a7c0b Compare January 24, 2019 14:47

droidmonkey approved these changes Jan 24, 2019

View reviewed changes

varjolintu force-pushed the feature/group_selection branch from 05a7c0b to e0f0008 Compare January 25, 2019 06:27

varjolintu mentioned this pull request Jan 27, 2019

Cannot save authentication data in Chrome+Vivaldi #388

Closed

varjolintu added the work in progress label Jan 27, 2019

varjolintu force-pushed the feature/group_selection branch from e0f0008 to ca888af Compare January 27, 2019 09:15

varjolintu removed the work in progress label Jan 27, 2019

varjolintu modified the milestones: Next, 1.3.3 Feb 1, 2019

varjolintu force-pushed the feature/group_selection branch from ca888af to c01d331 Compare February 5, 2019 08:22

varjolintu mentioned this pull request Feb 21, 2019

Translations from Transifex #418

Closed

varjolintu mentioned this pull request Mar 2, 2019

Set default group for saving new logins #427

Closed

varjolintu force-pushed the feature/group_selection branch 2 times, most recently from 78b48c8 to 0c17791 Compare March 9, 2019 12:11

varjolintu mentioned this pull request Mar 14, 2019

Allow creating new groups with Browser Integration keepassxreboot/keepassxc#2790

Merged

droidmonkey requested changes Mar 15, 2019

View reviewed changes

keepassxc-browser/_locales/en/messages.json Show resolved Hide resolved

keepassxc-browser/_locales/en/messages.json Show resolved Hide resolved

Select group when adding new credentials

34362ff

varjolintu force-pushed the feature/group_selection branch from a9d5d33 to 34362ff Compare March 19, 2019 05:31

droidmonkey approved these changes Mar 22, 2019

View reviewed changes

varjolintu merged commit 72014b8 into develop Mar 23, 2019

varjolintu deleted the feature/group_selection branch March 23, 2019 07:17

varjolintu mentioned this pull request Apr 7, 2019

redesign, upgrade to Bootstrap 4 and Fork Awesome #322

Closed

16 tasks

Uh oh!

Conversation

varjolintu commented Jan 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

droidmonkey commented Jan 23, 2019

Uh oh!

varjolintu commented Jan 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

droidmonkey commented Jan 24, 2019

Uh oh!

varjolintu commented Jan 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

varjolintu commented Jan 24, 2019

Uh oh!

Uh oh!

droidmonkey Jan 24, 2019

Choose a reason for hiding this comment

Uh oh!

varjolintu Jan 25, 2019

Choose a reason for hiding this comment

Uh oh!

droidmonkey Jan 25, 2019

Choose a reason for hiding this comment

Uh oh!

varjolintu Jan 25, 2019

Choose a reason for hiding this comment

Uh oh!

varjolintu commented Mar 9, 2019

Uh oh!

droidmonkey commented Mar 12, 2019

Uh oh!

varjolintu commented Mar 13, 2019

Uh oh!

varjolintu commented Mar 14, 2019

Uh oh!

Uh oh!

Uh oh!

droidmonkey commented Mar 15, 2019

Uh oh!

varjolintu commented Mar 15, 2019

Uh oh!

droidmonkey commented Mar 16, 2019

Uh oh!

droidmonkey left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

varjolintu commented Jan 23, 2019 •

edited

Loading

varjolintu commented Jan 24, 2019 •

edited

Loading

varjolintu commented Jan 24, 2019 •

edited

Loading