bug#62952: 28.2.50; secrets.el unlocking items

[Michael Albinus]

"Philipp Uhl" <git@ph-uhl.com> writes:

Hi Philipp,

> The secrets.el implementation lacks support for unlocking specific
> items. It only unlocks collections. This does not work well with certain
> password managers (e.g. in my case KeepassXC, accessed through secret
> service). When receiving a secret through
>
> (secrets-get-secret  "MyPws" "MyEntry")
>
> with the setting "Confirm when passwords are retrieved by clients"
> turned on in KeepassXC, secrets-get-secret will just say IsLocked.

Thanks for the report.

> Instead, secrets-get-secret should try to unlock the entry itself before
> retrieving.
>
> Here is a proof of concept:
>
> +  ;; New function, analogously to secrets-unlock-collection, that
> +  ;; specifically unlocks the item
> +  (defun secrets-unlock-item (collection item)
> +    "Unlock item labeled ITEM from collection labeled COLLECTION.
> +  If successful, return the object path of the item."
> +    (let ((item-path (secrets-item-path collection item)))
> +      (unless (secrets-empty-path item-path)
> +        (secrets-prompt
> +         (cadr
> +          (dbus-call-method
> +           :session secrets-service secrets-path secrets-interface-service
> +           "Unlock" `(:array :object-path ,item-path)))))
> +      item-path))
>
>   (defun secrets-get-secret (collection item)
>     "Return the secret of item labeled ITEM in COLLECTION.
>   If there are several items labeled ITEM, it is undefined which
>   one is returned.  If there is no such item, return nil.
>
>   ITEM can also be an object path, which is used if contained in COLLECTION."
> -    (let ((item-path (secrets-item-path collection item)))
> +    (let ((item-path (secrets-unlock-item collection item)))
>       (unless (secrets-empty-path item-path)
>         (dbus-byte-array-to-string
>          (nth 2
>               (dbus-call-method
>                :session secrets-service item-path secrets-interface-item
>                "GetSecret" :object-path secrets-session-path))))))
>
> To make this function a bit more similar to how it was before, one could
> concider to explicitly wait for the IsLocked event before unlocking the
> item. That way, if the password manager does not support unlocking of
> items, this would not be braking.

LGTM. Well, I don't know how relevant it is to wait for the IsLocked
event. If you have use cases where it is needed, we shall do.

When we add secrets-unlock-item, we should also add secrets-lock-item as
counterpart. Like we have done it with secrets-(un)?lock-collection.
Would you like to add this function? Bonus points for respective tests
in secrets-tests.el.

All these changes exceed the limit for tiny changes in Emacs, which
could be submitted w/o legal work. Would you like to sign FSF copyright
papers in order to contribute to Emacs? See
<https://git.savannah.gnu.org/cgit/gnulib.git/plain/doc/Copyright/conditions.text>
which explains the reasons.

> Cheers,
>   Philipp Uhl

Best regards, Michael.