bug#62952: 28.2.50; secrets.el unlocking items

[Philipp Uhl]

Hi Michael,

thanks for your response. Here is the secrets-lock-item function:

(defun secrets-lock-item (collection item)

    "Lock collection item labeled ITEM in COLLECTION.

If successful, return the object path of the item. Does not lock

the collection."

    (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

           "Lock" `(:array :object-path ,item-path)))))

      item-path))

> Bonus points for respective tests

> in secrets-tests.el.

I didn't find any secrets-tests.el in the Emacs repository. Also I am not really familiar with writing test code in Elisp. But I did manually test the code and it works.

> 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.

I don't. For my purposes the code as shown before suffices.

> 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.

Yes. Would digitally suffice? What exactly do I have to sign?

Cheers, Philipp

-----------------------------

  Philipp Uhl

  git@ph-uhl.com

Am Do, 20. Apr 2023, um 13:23, schrieb 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.