cli: show: add --attributes flag by cyphar · Pull Request #1289 · keepassxreboot/keepassxc

cyphar · 2017-12-15T10:13:14Z

In order for scripting to be much simpler with keepassxc-cli show,
provide a simple --show-attribute API which effectively is just a CLI
interface for entry->attributes()->value(...). This allows for more
extensibility and prevents changes in our output formatting from
breaking existing users of keepassxc-cli (if they use --show-attribute).

Signed-off-by: Aleksa Sarai cyphar@cyphar.com

Motivation and context

I've recently started scripting around keepassxc-cli and I wanted to be able to extract particular fields (especially custom attribute fields) and to avoid having to parse the (unstable) output format.

How has this been tested?

I tested this on my machine with my existing database. It suffers from #1260, but once #1280 is merged this could be rebased.

Screenshots (if appropriate):

Types of changes

✅ New feature (non-breaking change which adds functionality)

Checklist:

✅ I have read the CONTRIBUTING document. [REQUIRED]
✅ My code follows the code style of this project. [REQUIRED]
✅ All new and existing tests passed. [REQUIRED]
✅ I have compiled and verified my code with -DWITH_ASAN=ON. [REQUIRED]
✅ My change requires a change to the documentation and I have updated it accordingly.

droidmonkey · 2017-12-15T11:42:34Z

For this PR and the other can you significantly shorten the commit log message. Its not meant to be a play by play.

cyphar · 2017-12-15T11:56:19Z

How much shorter would you like? I guess I'm just used to the kernel commit style, where the explanation of a patch is part of the commit (rather than it being a separate PR description). I'm also a little confused why it would matter all too much -- most git log aliases people have only print the description line anyway.

louib · 2017-12-15T18:35:49Z

@droidmonkey what's wrong with detailed commit messages?

louib · 2017-12-15T18:38:42Z

@cyphar I'm happy that you give feedback (and contribute) on the CLI! I added the main commands some time ago and did not receive that much feedback since then.

Can you add a screenshot of the output with the default attributes and with custom attributes?

louib · 2017-12-15T18:39:51Z

src/cli/Show.cpp

                               QObject::tr("path"));
    parser.addOption(keyFile);
+    QCommandLineOption fields(QStringList() << "a"
+                                            << "show-attribute",


@cyphar what about -a and --attributes ? I find it redundant to have both the show command and show in the name of the option.

louib · 2017-12-15T18:40:59Z

src/cli/Show.cpp

    parser.addOption(keyFile);
+    QCommandLineOption fields(QStringList() << "a"
+                                            << "show-attribute",
+                              QObject::tr("Attribute name to extract and output. "


@cyphar what about names of the attributes to show, since show is the name of the command.

cyphar · 2017-12-16T08:41:49Z

@louib Fixed your comments, and added screenshots. The only question is whether we should provide a way to get a list of attributes.

cyphar · 2017-12-16T09:40:38Z

As an aside, we really should also allow people to provide passwords in a non-interactive manner (like a password file, which can be made secure by a user by using a FIFO or process substitution).

louib · 2017-12-16T13:32:45Z

src/cli/Show.cpp

+        if (showAttributeNames) {
+            outputTextStream << attribute << ": ";
+        }
+        outputTextStream << entry->attributes()->value(attribute) << endl;


@cyphar what's the behaviour when that attribute does not exist?

At the moment it will output nothing I believe, but we should be erroring out and doing a return EXIT_FAILURE.

louib · 2017-12-16T13:35:26Z

@cyphar

The only question is whether we should provide a way to get a list of attributes.

You mean using a comma separated list of attributes? --attributes title,url,password?

As an aside, we really should also allow people to provide passwords in a non-interactive manner (like a password file, which can be made secure by a user by using a FIFO or process substitution).

Can you open an issue for that please?

One last comment on your PR, otherwise 👍

cyphar · 2017-12-16T13:46:17Z

@louib

You mean using a comma separated list of attributes?

At the moment, you can get more than one attribute by specifying -a more than once (though as I mentioned above, that could get confusing if you have custom attributes that have newlines in them).

What I meant by "get a list of attributes" is to do something like keepassxc-cli list --type entry $db Some/Entry/Path which then gives you a list of the attributes that the object has (including custom ones). This would basically be a concatenation of entry->attributes()->customKeys() and EntryAttributes::DefaultAttributes. To give an example, for my test-entry it would look something like

% keepassxc-cli list --type entry ./test-database.kdbx some-entry
Title
UserName
Password
URL
Notes
additional_attribute_1
additional_attribute_2

But maybe I should propose this separately.

Can you open an issue for that please?

Will do. I've opened #1297.

droidmonkey · 2017-12-16T14:56:56Z

Sorry i was looking at the commits using FastHub which doesnt distinguish the first line from subsequent. I thought you had run on the first line. Carry on with your excellent contributions! 😁

In order for scripting to be much simpler with `keepassxc-cli show`, provide a simple --attributesk API which effectively is just a CLI interface for entry->attributes()->value(...). This allows for more extensibility and prevents changes in our output formatting from breaking existing users of keepassxc-cli (if they use --attributes). Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>

louib

@cyphar thanks for the contribution!

TheZ3ro requested review from TheZ3ro and louib December 15, 2017 10:50

louib reviewed Dec 15, 2017

View reviewed changes

cyphar changed the title ~~cli: show: add --show-attribute flag~~ cli: show: add --attribute flag Dec 16, 2017

louib reviewed Dec 16, 2017

View reviewed changes

cyphar changed the title ~~cli: show: add --attribute flag~~ cli: show: add --attributes flag Dec 16, 2017

louib approved these changes Dec 16, 2017

View reviewed changes

louib merged commit 8e231df into keepassxreboot:develop Dec 17, 2017

cyphar deleted the keepasscli-field-flag branch December 17, 2017 08:04

cyphar mentioned this pull request Dec 17, 2017

keepassxc-cli show: resolve references in output #1280

Merged

liar666 mentioned this pull request Aug 22, 2018

CLI missing fields on Add/Edit #2220

Open

bricewge mentioned this pull request Mar 8, 2023

Allow setting attributes via CLI #9212

Open

Uh oh!

Conversation

cyphar commented Dec 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation and context

How has this been tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Uh oh!

droidmonkey commented Dec 15, 2017

Uh oh!

cyphar commented Dec 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

louib commented Dec 15, 2017

Uh oh!

louib commented Dec 15, 2017

Uh oh!

louib Dec 15, 2017

Choose a reason for hiding this comment

Uh oh!

louib Dec 15, 2017

Choose a reason for hiding this comment

Uh oh!

cyphar commented Dec 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cyphar commented Dec 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

louib Dec 16, 2017

Choose a reason for hiding this comment

Uh oh!

cyphar Dec 16, 2017

Choose a reason for hiding this comment

Uh oh!

louib commented Dec 16, 2017

Uh oh!

cyphar commented Dec 16, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

droidmonkey commented Dec 16, 2017

Uh oh!

louib left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cyphar commented Dec 15, 2017 •

edited

Loading

cyphar commented Dec 15, 2017 •

edited

Loading

cyphar commented Dec 16, 2017 •

edited

Loading

cyphar commented Dec 16, 2017 •

edited

Loading

cyphar commented Dec 16, 2017 •

edited

Loading