cli: show: add --attributes flag #1289

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 [email protected]

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

@@ -49,6 +49,13 @@ int Show::execute(QStringList arguments)
                               QObject::tr("Key file of the database."),
                               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

@@ -49,6 +49,13 @@ int Show::execute(QStringList arguments)
                               QObject::tr("Key file of the database."),
                               QObject::tr("path"));
    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 <[email protected]>

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cli: show: add --attributes flag #1289

cli: show: add --attributes flag #1289

cyphar commented Dec 15, 2017 •

edited

Loading

droidmonkey commented Dec 15, 2017

cyphar commented Dec 15, 2017 •

edited

Loading

louib commented Dec 15, 2017

louib commented Dec 15, 2017

louib Dec 15, 2017

louib Dec 15, 2017

cyphar commented Dec 16, 2017 •

edited

Loading

cyphar commented Dec 16, 2017 •

edited

Loading

louib Dec 16, 2017

cyphar Dec 16, 2017

louib commented Dec 16, 2017

cyphar commented Dec 16, 2017 •

edited

Loading

droidmonkey commented Dec 16, 2017

louib left a comment

cli: show: add --attributes flag #1289

cli: show: add --attributes flag #1289

Conversation

cyphar commented Dec 15, 2017 • edited Loading

Motivation and context

How has this been tested?

Screenshots (if appropriate):

Types of changes

Checklist:

droidmonkey commented Dec 15, 2017

cyphar commented Dec 15, 2017 • edited Loading

louib commented Dec 15, 2017

louib commented Dec 15, 2017

louib Dec 15, 2017

Choose a reason for hiding this comment

louib Dec 15, 2017

Choose a reason for hiding this comment

cyphar commented Dec 16, 2017 • edited Loading

cyphar commented Dec 16, 2017 • edited Loading

louib Dec 16, 2017

Choose a reason for hiding this comment

cyphar Dec 16, 2017

Choose a reason for hiding this comment

louib commented Dec 16, 2017

cyphar commented Dec 16, 2017 • edited Loading

droidmonkey commented Dec 16, 2017

louib left a comment

Choose a reason for hiding this comment

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