* bug#62952: 28.2.50; secrets.el unlocking items
@ 2023-04-19 12:23 Philipp Uhl
2023-04-20 11:23 ` Michael Albinus
0 siblings, 1 reply; 6+ messages in thread
From: Philipp Uhl @ 2023-04-19 12:23 UTC (permalink / raw)
To: 62952
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.
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.
Cheers,
-----------------------------
Philipp Uhl
git@ph-uhl.com
^ permalink raw reply [flat|nested] 6+ messages in thread* bug#62952: 28.2.50; secrets.el unlocking items 2023-04-19 12:23 bug#62952: 28.2.50; secrets.el unlocking items Philipp Uhl @ 2023-04-20 11:23 ` Michael Albinus 2023-05-02 10:05 ` Philipp Uhl 0 siblings, 1 reply; 6+ messages in thread From: Michael Albinus @ 2023-04-20 11:23 UTC (permalink / raw) To: Philipp Uhl; +Cc: 62952 "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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#62952: 28.2.50; secrets.el unlocking items 2023-04-20 11:23 ` Michael Albinus @ 2023-05-02 10:05 ` Philipp Uhl 2023-05-02 11:44 ` Michael Albinus 0 siblings, 1 reply; 6+ messages in thread From: Philipp Uhl @ 2023-05-02 10:05 UTC (permalink / raw) To: Michael Albinus; +Cc: 62952 [-- 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 --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#62952: 28.2.50; secrets.el unlocking items 2023-05-02 10:05 ` Philipp Uhl @ 2023-05-02 11:44 ` Michael Albinus 2023-05-08 11:42 ` Michael Albinus 0 siblings, 1 reply; 6+ messages in thread From: Michael Albinus @ 2023-05-02 11:44 UTC (permalink / raw) To: Philipp Uhl; +Cc: 62952 "Philipp Uhl" <git@ph-uhl.com> writes: > Hi Michael, Hi Philipp, > thanks for your response. Here is the secrets-lock-item function: Thanks. I'll check it next days. >> 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. See test/lisp/net/secrets-tests.el in the git repository. Tests are performed using ERT, see (info "(ert) Top") for the manual. >> 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. Good. So we can leave it out, until somebody hollers. >> Would you like to sign FSF copyright papers in order to contribute to >> Emacs? > > Yes. Would digitally suffice? What exactly do I have to sign? Template sent off-list. > Cheers, Philipp Best regards, Michael. ^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#62952: 28.2.50; secrets.el unlocking items 2023-05-02 11:44 ` Michael Albinus @ 2023-05-08 11:42 ` Michael Albinus 2023-05-09 8:15 ` Philipp Uhl 0 siblings, 1 reply; 6+ messages in thread From: Michael Albinus @ 2023-05-08 11:42 UTC (permalink / raw) To: Philipp Uhl; +Cc: 62952-done Version: 30.1 Michael Albinus <michael.albinus@gmx.de> writes: Hi Philipp, >> thanks for your response. Here is the secrets-lock-item function: > > Thanks. I'll check it next days. I've played this morning with your changes. Everything looks fine (in my environment), so I have pushed them to the Emacs master branch. >>> 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. > > See test/lisp/net/secrets-tests.el in the git repository. Tests are > performed using ERT, see (info "(ert) Top") for the manual. I've tried to see how it works with this, but it looks like the temporary "session" session of Gnome keyring, which I use, doesn't care about locking/unlocking of single items. So likely it isn't worth to extend secrets-tests.el. I'm closing the bug. >> Cheers, Philipp Best regards, Michael. ^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#62952: 28.2.50; secrets.el unlocking items 2023-05-08 11:42 ` Michael Albinus @ 2023-05-09 8:15 ` Philipp Uhl 0 siblings, 0 replies; 6+ messages in thread From: Philipp Uhl @ 2023-05-09 8:15 UTC (permalink / raw) To: Michael Albinus; +Cc: 62952-done Hi Michael, thank you. Sounds great. Best, Philipp ----------------------------- Philipp Uhl git@ph-uhl.com Am Mo, 8. Mai 2023, um 13:42, schrieb Michael Albinus: > Version: 30.1 > > Michael Albinus <michael.albinus@gmx.de> writes: > > Hi Philipp, > >>> thanks for your response. Here is the secrets-lock-item function: >> >> Thanks. I'll check it next days. > > I've played this morning with your changes. Everything looks fine (in my > environment), so I have pushed them to the Emacs master branch. > >>>> 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. >> >> See test/lisp/net/secrets-tests.el in the git repository. Tests are >> performed using ERT, see (info "(ert) Top") for the manual. > > I've tried to see how it works with this, but it looks like the > temporary "session" session of Gnome keyring, which I use, doesn't care > about locking/unlocking of single items. So likely it isn't worth to > extend secrets-tests.el. > > I'm closing the bug. > >>> Cheers, Philipp > > Best regards, Michael. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-05-09 8:15 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-04-19 12:23 bug#62952: 28.2.50; secrets.el unlocking items Philipp Uhl 2023-04-20 11:23 ` Michael Albinus 2023-05-02 10:05 ` Philipp Uhl 2023-05-02 11:44 ` Michael Albinus 2023-05-08 11:42 ` Michael Albinus 2023-05-09 8:15 ` Philipp Uhl
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).