jaraco / keyring

MIT License
1.26k stars 160 forks source link

libsecret headless check? #561

Open davegaeddert opened 2 years ago

davegaeddert commented 2 years ago

Describe the bug

I'm trying to debug an issue in another library and it seems to stem from the libsecret backend failing in get_password: https://github.com/jaraco/keyring/blob/eb1a04a42617131a35ed8b6089f8889e6054bcb1/keyring/backends/libsecret.py#L65-L68

The environment is a GitHub Action (ubuntu), and the error is g-io-error-quark: Cannot autolaunch D-Bus without X11 $DISPLAY (0) . Here's a link to the action output: https://github.com/dropseed/workhorse/runs/5160584552?check_suite_focus=true#step:3:263

I found some other discussion about headless, but I guess what I'm wondering is whether there could be some kind of check in libsecret priority (or elsewhere) that would prevent it from using that keyring in the first place? https://github.com/jaraco/keyring/blob/eb1a04a42617131a35ed8b6089f8889e6054bcb1/keyring/backends/libsecret.py#L45-L52

I'm not familiar enough with the details, but I see some stuff that looks a little like that in the kwallet backend: https://github.com/jaraco/keyring/blob/eb1a04a42617131a35ed8b6089f8889e6054bcb1/keyring/backends/kwallet.py#L40-L58

Or is there some way that it is their responsibility to use get_keyring differently? https://github.com/python-poetry/poetry/blob/7cc684981983963dc202e1a249a4b66667b468bd/src/poetry/utils/password_manager.py#L86-L95

Thanks!

jaraco commented 2 years ago

Ideally, the backend should not have a non-zero priority if it is unlikely to work when queried. I'd welcome a fix to make it so.

davegaeddert commented 2 years ago

@jaraco do you have any suggestions for a fix? It's not something I know much about. The only thing I could come up with was to actually do a dummy password search during priority and catch the specific error... which doesn't seem like a great idea?

takluyver commented 2 years ago

Probably the best approach is for the priority method to actually try connecting to D-Bus and catch the same error there. This is what the SecretStorage and kwallet backends already do:

https://github.com/jaraco/keyring/blob/d1e37a5efb56ab4b805b721847c87fb94b9ca57b/keyring/backends/SecretService.py#L41-L46

https://github.com/jaraco/keyring/blob/d1e37a5efb56ab4b805b721847c87fb94b9ca57b/keyring/backends/kwallet.py#L45-L52

But I don't know exactly what to call in libsecret to do something equivalent.

If not, then I think that if neither DBUS_SESSION_BUS_ADDRESS nor DISPLAY are set as environment variables (i.e. there's no D-Bus session bus and no X server), it's likely that none of the Linux backends (libsecret, SecretStorage, kwallet) will work. The D-Bus spec specifically recommends against checking like this, though:

Note that this mechanism is not recommended for attempting to determine if a daemon is running. It is inherently racy to attempt to make this determination, since the bus daemon may be started just before or just after the determination is made. Therefore, it is recommended that applications do not try to make this determination for their functionality purposes, and instead they should attempt to start the server.