keepassxreboot / keepassxc

KeePassXC is a cross-platform community-driven port of the Windows application “Keepass Password Safe”.
https://keepassxc.org/
Other
21.4k stars 1.48k forks source link

Add command line option to prevent window focus (--quiet or --background) #7055

Open sphh opened 3 years ago

sphh commented 3 years ago

Summary

An --unlock command line option would

  1. Start KeePassXC, if it is not already running.
  2. Present the unlock screen, if KeePassXC is already running, but the database is locked.
  3. Do nothing, if KeePassXC is already running and the database is currently unlocked.

In all these cases the program should return,

  1. after the database is unlocked or
  2. the Cancel button was pressed

Context

I use autofs/afuse to mount a server with the help of sshfs automatically. For this to work, sshfs must be able to mount the server without any password.

I have my private key stored in KeePassXC, which adds this key to ssh-agent when unlocked and removes the key, when it locks again.

That means, that for automounting to work, I must have KeePassXC running and it must be unlocked. Sometimes I forget to start it or it became locked after the screensaver kicked in and I cannot automount the server. It would be great, if in these cases the KeePassXC starts automatically, so that I can unlock the database.

I am aware, that this would only work in a graphical desktop requirement …

I tried the following:

  1. Call keepassxc everytime, when afuse tries to mount a server: This would also open the GUI, when the database is already unlocked. This becomes quite annoying.
  2. Check, if there is a keepassxc process running. This works for the case, if KeePassXC is not running yet, but does not work, if the database is locked.

Alternative

Have a command line option, which checks, if the database is unlocked.

sphh commented 3 years ago

I managed to simulate this behaviour with the following bash script:

#!/bin/bash

if ! ssh-add -l | grep ${USER}@$(hostname) > /dev/null; then
  # Key not available - start KeePassXC
  keepassxc &
  # Wait a bit
  sleep 1
  # KeePassXC shows a dialog for password input. If this dialog is discarded,
  # the main window is available - check for it
  # Note, the main window can be found by grepping for 'KeePassXC' instead
  # of 'keepassxc'.
  while ! xwininfo -children -root | grep "KeePassXC" > /dev/null; do
    # Main window not available, check, if keepassxc is actually running
    if ! xwininfo -children -root | grep "keepassxc" > /dev/null; then
      # KeePassXC not running, exit
      exit 1
    fi
    # Wait a bit, before we check again
    sleep 1
  done
fi

Unfortunately it relies on the fact, that the KeePassXC main window shows KeePassXC and the dialogs keepassxc, so this script can only seen as a workaround to an --unlock command line option at most …

michaelk83 commented 3 years ago

If you're on Linux, you can use DBus to unlock a specific database. If credentials aren't provided, it should display the unlock screen, I guess. See https://github.com/keepassxreboot/keepassxc/wiki/Using-DBus-with-KeePassXC . You can probably also use DBus to check if an instance is already running. Also see #7033 to start a minimized instance (or minimize the current one).

sphh commented 3 years ago

Good idea to use DBus!

After rewriting the qdbus call given in the link to this gdbus call:

gdbus call --session --dest org.keepassxc.KeePassXC.MainWindow --object-path /keepassxc --method <command> [<args>]

I now can

but unfortunately neither

gdbus call --session --dest org.keepassxc.KeePassXC.MainWindow --object-path /keepassxc --method org.keepassxc.MainWindow.openDatabase

nor appending the database

gdbus call --session --dest org.keepassxc.KeePassXC.MainWindow --object-path /keepassxc --method org.keepassxc.MainWindow.openDatabase /path/to/database.xdbx

present the unlock screen. 😞

On the positive side, calling

gdbus call --session --dest org.keepassxc.KeePassXC.MainWindow --object-path /keepassxc --method org.keepassxc.MainWindow.openDatabase /path/to/database.xdbx

without KeePassXC running returns an error code of 1, which can help to start KeePassXC.

yan12125 commented 3 years ago

I'm working on adding an --unlock option. With the current implementation (https://github.com/yan12125/keepassxc/commits/unlock-command), keepassxc --unlock returns immediately without waiting for the unlock dialog. Is waiting desired?

sphh commented 3 years ago

Excellent! :bow:

In my case I would prefer that keepassxc --unlock only returns, if the database is unlocked (return code 0) or the unlock dialog is dismissed (return code >0). If someone needs it to return immediately, s/he could always use keepassxc --unlock &!? That would be my preferred behaviour …

yan12125 commented 3 years ago

The return code is a good idea! More hacking :)

sphh commented 2 years ago

@yan12125: May I ask what has happened with your branch implementing the "--unlock" command line parameter?

yan12125 commented 2 years ago

Sorry, I abandoned my fork as I don't need that feature anymore. Here is the last version of my patches, which are released under the same license as KeePassXC. unlock-command.tgz

droidmonkey commented 2 years ago

I don't see a need for a --unlock flag, but a --quiet or --background flag would be more descriptive. Basically you provide the kdbx file and credentials as usual and if you add one of the mentioned flags it won't focus or unminimize the window.

yan12125 commented 2 years ago

From https://github.com/keepassxreboot/keepassxc/issues/7055#issue-1030422950,

  1. Present the unlock screen, if KeePassXC is already running, but the database is locked.

--unlock in my version can do that, but it's kind of non-intuitive if --quiet/--background does that.

droidmonkey commented 2 years ago

--unlock is not descriptive for the use cases above. By providing a KDBX and credentials you are asking to unlock. If no credentials are provided then it would make sense to show the window to type them in. If you don't want the window to appear then use the --quiet/--background flag. If you want to unlock in the background provide credentials and that flag.

The actual use case for this is very narrow, likely people using the single app feature to act as a "raise keepassxc" using a keyboard shortcut. I do intend to implement that shortcut eventually.

Also if you are automating things, you really should be using keepassxc-cli.

sphh commented 2 years ago

@droidmonkey: Let me start, that I do not mind, how the command line parameter is called …

I would have thought, that the use case I described in generic terms is quite a common one. Here are some ideas, when you encounter it:

I hope, this all makes sense.

I am well aware, that these cases are probably not, what the everyday user uses, but nonetheless it would be nice to have.

@droidmonkey: If this already possible, could you please be so kind to point out, how this is done.

droidmonkey commented 2 years ago

Your use case is very much is wrapped up in ssh agent, which is admittedly a niche but useful feature. Browser extension, autotype, and fdosecrets all handle locked databases gracefully because they specifically request access. Due to the nature of ssh-agent, that isn't possible. It might be good to add a global shortcut that specifically requests adding ssh keys and would present an unlock dialog similar to the other methods.

sphh commented 2 years ago

Honestly, I do not mind, how it is done. Can you please describe your approach in more detail, so that I am able to implement it. Thanks.

droidmonkey commented 2 years ago

Add a global keyboard shortcut that requests to unlock the loaded databases. If they are all unlocked it does nothing. We need another global keyboard shortcut to being KeePassXC to the foreground.

sphh commented 2 years ago

Mmh, I already have a global keyboard shortcut to start/request unlocking the loaded database. If I press it, it will start KeePassXC, if it is not started yet and I can unlock the database. If I press it, when KeePassXC has been already started, it shows KeePassXC / bring it to the front.

That is not what I need, because I have to check, if KeePassXC is running, before hitting the global keyboard shortcut:

  1. KeePassXC is not running: Hit the key combination.
  2. KeePassXC is running, but locked: Hit the key combination.
  3. KeePassXC is running and is not locked: Do not hit the key combination.

What is the problem to implement this very behaviour with a command line parameter directly in KeePassXC?

sphh commented 2 years ago

Thinking about it, I can see two options:

  1. The behaviour described above could be the default behaviour. And if you want to see the KeePassXC window, use a command line parameter, e.g. --foreground. This would be convenient for people, who only want to run KeePassXC in the background – but would break current behaviour.
  2. Make a command line parameter to switch to the behaviour described above. Call it --quiet or --background or whatever you think is appropriate. Then people who just want to run KeePassXC to supply passwords (without looking these up themselves) add this parameter to their command line.

What do you think about this?

droidmonkey commented 2 years ago

By default most people expect the window to be shown when they invoke the application (either via command line, script, or desktop shortcut). Option 2 is the only viable one, and in my opinion would invoke the database unlock dialog if the current active database is locked.

sphh commented 2 years ago

Exactly!

But this is the behaviour, @yan12125 has implemented – with the command line argument --unlock. Let us take his implementation and change the command line parameter to whatever you think more appropriate.

Perhaps the https://github.com/keepassxreboot/keepassxc/issues/7055#issuecomment-973858495 can be implemented too?