bug#62952: 28.2.50; secrets.el unlocking items

["Philipp Uhl" <git@ph-uhl.com>]

[-- Attachment #1: Type: text/plain, Size: 4566 bytes --]

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.

[-- Attachment #2: Type: text/html, Size: 10082 bytes --]