bug#64977: 29.1; `auth-source-macos-keychain-search' handles :user key incorrectly

bug#64977: 29.1; `auth-source-macos-keychain-search' handles :user key incorrectly

[LdBeth]

Right after switched from 28.2, my config that uses `auth-source.el'
to retrieve IRC password from macOS keychain stopped working.

Here is the backtrace, apparently there is a function in the chain
forget to handle the :user provided as a list when searching.

Debugger entered--Lisp error: (wrong-type-argument stringp ("ldb"))
  call-process("/usr/bin/security" nil t nil "find-internet-password" "-g" "-s" "irc.libera.chat" "-a" ("ldb") "-r" "\"irc\"")
  apply(call-process "/usr/bin/security" nil t nil ("find-internet-password" "-g" "-s" "irc.libera.chat" "-a" ("ldb") "-r" "\"irc\""))
  auth-source-macos-keychain-search-items("default" macos-keychain-internet 5000 "irc.libera.chat" "\"irc\"" :type macos-keychain-internet :require (:secret) :user ("ldb") :require (:secret))
  apply(auth-source-macos-keychain-search-items "default" macos-keychain-internet 5000 "irc.libera.chat" "\"irc\"" (:type macos-keychain-internet :require (:secret) :user ("ldb") :require (:secret)))
  auth-source-macos-keychain-search(:backend #<auth-source-backend auth-source-backend-1ff6005ea858> :type macos-keychain-internet :max 5000 :require (:secret) :create nil :delete nil :user ("ldb") :host ("irc.libera.chat") :port ("irc") :require (:secret) :max 5000)
  apply(auth-source-macos-keychain-search :backend #<auth-source-backend auth-source-backend-1ff6005ea858> :type macos-keychain-internet :max 5000 :require (:secret) :create nil :delete nil (:user ("ldb") :host ("irc.libera.chat") :port ("irc") :require (:secret) :max 5000))
  auth-source-search-backends((#<auth-source-backend auth-source-backend-1ff6005ea858> #<auth-source-backend auth-source-backend-1ff6005ea874>) (:user ("ldb") :host ("irc.libera.chat") :port ("irc") :require (:secret) :max 5000) 5000 nil nil (:secret))
  auth-source-search(:user ("ldb") :host ("irc.libera.chat") :port ("irc") :require (:secret) :max 5000)
  apply(auth-source-search (:user ("ldb") :host ("irc.libera.chat") :port ("irc") :require (:secret) :max 5000))
  erc--auth-source-search(:user ("ldb") :host ("irc.libera.chat") :port ("irc") :require (:secret))
  apply(erc--auth-source-search (:user ("ldb") :host ("irc.libera.chat") :port ("irc") :require (:secret)))
  erc-auth-source-search(:user "ldb")
  erc--compute-server-password(nil "ldb")
  erc-determine-parameters("irc.libera.chat" "6697" "ldb" "unknown" "user" nil)
  erc-open("irc.libera.chat" "6697" "ldb" "unknown" t nil nil nil nil nil "user" nil)
  erc-tls(:server "irc.libera.chat" :port "6697")
  #f(compiled-function (&rest _) #<bytecode -0x10737acf4eaa4b33>)((push-button :args nil :value #("Libera Chat" 0 11 (face font-lock-keyword-face)) :help-echo "Start ERC." :action #f(compiled-function (&rest _) #<bytecode -0x10737acf4eaa4b33>) :mouse-face highlight :follow-link "\15" :button-overlay #<overlay from 165 to 178 in *LaunchPad*> :from #<marker (moves after insertion) at 165 in *LaunchPad*> :to #<marker at 178 in *LaunchPad*>) nil)
  widget-apply((push-button :args nil :value #("Libera Chat" 0 11 (face font-lock-keyword-face)) :help-echo "Start ERC." :action #f(compiled-function (&rest _) #<bytecode -0x10737acf4eaa4b33>) :mouse-face highlight :follow-link "\15" :button-overlay #<overlay from 165 to 178 in *LaunchPad*> :from #<marker (moves after insertion) at 165 in *LaunchPad*> :to #<marker at 178 in *LaunchPad*>) :action nil)
  widget-apply-action((push-button :args nil :value #("Libera Chat" 0 11 (face font-lock-keyword-face)) :help-echo "Start ERC." :action #f(compiled-function (&rest _) #<bytecode -0x10737acf4eaa4b33>) :mouse-face highlight :follow-link "\15" :button-overlay #<overlay from 165 to 178 in *LaunchPad*> :from #<marker (moves after insertion) at 165 in *LaunchPad*> :to #<marker at 178 in *LaunchPad*>) nil)
  widget-button-press(165)
  funcall-interactively(widget-button-press 165)
  call-interactively(widget-button-press nil nil)
  command-execute(widget-button-press)


In GNU Emacs 29.1 (build 1, x86_64-apple-darwin21.6.0, NS
appkit-2113.60 Version 12.6.8 (Build 21G725)) of 2023-07-31 built on
Costume-Party.localWindowing system distributor 'Apple', version 10.3.2113
System Description:  macOS 12.6.8

Configured using:
 'configure --with-small-ja-dic CPPFLAGS=-I/opt/pkg/include
 LDFLAGS=-L/opt/pkg/lib'

Configured features:
ACL GMP GNUTLS JPEG JSON LIBXML2 MODULES NOTIFY KQUEUE NS PDUMPER PNG
SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Fundamental

Minor modes in effect:
  erc-list-mode: t
  erc-menu-mode: t
  erc-autojoin-mode: t
  erc-ring-mode: t
  erc-track-mode: t
  erc-track-minor-mode: t
  erc-match-mode: t
  erc-button-mode: t
  erc-fill-mode: t
  erc-stamp-mode: t
  erc-netsplit-mode: t
  erc-irccontrols-mode: t
  erc-noncommands-mode: t
  erc-move-to-prompt-mode: t
  erc-readonly-mode: t
  erc-networks-mode: t
  recentf-mode: t
  vertico-mode: t
  filladapt-mode: t
  global-page-break-lines-mode: t
  which-key-mode: t
  global-paren-face-mode: t
  override-global-mode: t
  ctrlf-mode: t
  ctrlf-local-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  line-number-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
/Users/ldbeth/.emacs.d/elpa/use-package-20230426.2324/use-package-jump hides /Users/ldbeth/Downloads/emacs-29.1/nextstep/Emacs.app/Contents/Resources/lisp/use-package/use-package-jump
/Users/ldbeth/.emacs.d/elpa/use-package-20230426.2324/use-package-ensure hides /Users/ldbeth/Downloads/emacs-29.1/nextstep/Emacs.app/Contents/Resources/lisp/use-package/use-package-ensure
/Users/ldbeth/.emacs.d/elpa/use-package-20230426.2324/use-package-core hides /Users/ldbeth/Downloads/emacs-29.1/nextstep/Emacs.app/Contents/Resources/lisp/use-package/use-package-core
/Users/ldbeth/.emacs.d/elpa/use-package-20230426.2324/use-package-delight hides /Users/ldbeth/Downloads/emacs-29.1/nextstep/Emacs.app/Contents/Resources/lisp/use-package/use-package-delight
/Users/ldbeth/.emacs.d/elpa/use-package-20230426.2324/use-package-diminish hides /Users/ldbeth/Downloads/emacs-29.1/nextstep/Emacs.app/Contents/Resources/lisp/use-package/use-package-diminish
/Users/ldbeth/.emacs.d/elpa/use-package-20230426.2324/use-package hides /Users/ldbeth/Downloads/emacs-29.1/nextstep/Emacs.app/Contents/Resources/lisp/use-package/use-package
/Users/ldbeth/.emacs.d/elpa/use-package-20230426.2324/use-package-bind-key hides /Users/ldbeth/Downloads/emacs-29.1/nextstep/Emacs.app/Contents/Resources/lisp/use-package/use-package-bind-key
/Users/ldbeth/.emacs.d/elpa/bind-key-20230203.2004/bind-key hides /Users/ldbeth/Downloads/emacs-29.1/nextstep/Emacs.app/Contents/Resources/lisp/use-package/bind-key
/Users/ldbeth/.emacs.d/elpa/use-package-20230426.2324/use-package-lint hides /Users/ldbeth/Downloads/emacs-29.1/nextstep/Emacs.app/Contents/Resources/lisp/use-package/use-package-lint
/Users/ldbeth/.emacs.d/elpa/flim-20230205.1423/sasl hides /Users/ldbeth/Downloads/emacs-29.1/nextstep/Emacs.app/Contents/Resources/lisp/net/sasl

Features:
(shadow mel-q-ccl ccl flyspell ispell modb-standard elmo-sendlog
elmo-internal elmo-rss elmo-maildir utf-7 elmo-imap4 time-stamp
mime-diff lsdb wl-fldmgr wl-template wl-demo wl-thread wl-action wl
wl-draft wl-folder elmo-nntp elmo-filter wl-summary wl-refile
wl-message wl-mime mime-play filename wl-e21 wl-highlight elmo-mime
mmelmo-buffer mmelmo-imap mmimap mmbuffer mmgeneric wl-address wl-util
wl-vars wl-version elmo-net elmo-cache elmo-map elmo-dop modb-generic
elmo-flag elmo-localdir elmo elmo-signal elmo-msgdb modb modb-entity
elmo-date elmo-util elmo-vars elmo-version mime-edit mime-image
mime-view mime-conf calist invisible inv-23 mime-setup mail-mime-setup
semi-setup advice semi-def mime-parse mime luna eword-encode
eword-decode mel path-util pces pces-e20 pces-20 mime-def alist
mcs-e20 mcs-20 mcharset std11 pccl pccl-20 broken static apel-ver
product timezone emacsbug message yank-media dired dired-loaddefs
rfc822 mml mml-sec epa derived epg rfc6068 epg-config gnus-util
mm-decode mm-bodies mm-encode mailabbrev gmm-utils mailheader sendmail
shortdoc bug-reference jka-compr mwim help-fns radix-tree cl-print
debug backtrace find-func cus-start cus-load rx orderless
vertico-directory erc-list erc-menu erc-join erc-ring erc-pcomplete
pcomplete comint ansi-osc ansi-color ring erc-track erc-match
erc-button erc-fill erc-stamp erc-netsplit erc-goodies erc format-spec
erc-backend erc-networks erc-common erc-compat erc-loaddefs mail-utils
gnutls network-stream url-http url-gw nsm url-auth textsec uni-scripts
mail-parse rfc2231 rfc2047 rfc2045 mm-util ietf-drums mail-prsvr
idna-mapping uni-confusable textsec-check url-queue url-cache shr
pixel-fill kinsoku url-file puny svg dom newst-plainview newst-ticker
newst-reader newst-backend iso8601 xml time-date recentf tree-widget
vc-git diff-mode vc-dispatcher bookmark text-property-search pp
disp-table wid-edit cl-extra help-mode edmacro kmacro vertico compat
compat-29 filladapt page-break-lines which-key ctrlf thingatpt hl-line
dim-paren tecoline spacemacs-buffer spacemacs-light-theme
spacemacs-dark-theme spacemacs-theme-autoloads core-autoloads bind-key
easy-mmode info package browse-url url url-proxy url-privacy
url-expand url-methods url-history url-cookie generate-lisp-file
url-domsuf url-util mailcap url-handlers url-parse auth-source cl-seq
eieio eieio-core cl-macs password-cache json subr-x map byte-opt gv
bytecomp byte-compile url-vars cl-loaddefs cl-lib rmc iso-transl
tooltip cconv eldoc paren electric uniquify ediff-hook vc-hooks
lisp-float-type elisp-mode mwheel term/ns-win ns-win ucs-normalize
mule-util term/common-win tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode lisp-mode prog-mode
register page tab-bar menu-bar rfn-eshadow isearch easymenu timer
select scroll-bar mouse jit-lock font-lock syntax font-core
term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew
greek romanian slovak czech european ethiopic indian cyrillic chinese
composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button
loaddefs theme-loaddefs faces cus-face macroexp files window
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget keymap hashtable-print-readable backquote threads kqueue
cocoa ns multi-tty make-network-process emacs)

Memory information:
((conses 16 356680 134770)
 (symbols 48 22848 15)
 (strings 32 86294 9713)
 (string-bytes 1 5357679)
 (vectors 16 41941)
 (vector-slots 8 739161 117204)
 (floats 8 406 749)
 (intervals 56 13370 879)
 (buffers 976 32))

bug#64977: 29.1; `auth-source-macos-keychain-search' handles :user key incorrectly

[J.P.]

Hi LdBeth,

LdBeth <andpuke@foxmail.com> writes:

> Right after switched from 28.2, my config that uses `auth-source.el'
> to retrieve IRC password from macOS keychain stopped working.

A few comments to the extent this concerns ERC. (Feel free to ignore.)

In case you haven't noticed, ERC got pickier in 5.5 regarding the
auth-source back ends it supports. Quoting from "(erc) auth-source":

  See "(auth)Top" for general info on setting up various backends, but
  keep in mind that some of these may not be compatible. Those currently
  supported are netrc, plstore, json, secrets, and pass.

The recommended move for folks in your situation is to specify your own
query function. See "(erc) auth-source functions".

As for changing `auth-source-macos-keychain-search-items', that's above
my pay grade, but offering up a patch may be your best bet. And if by
chance you're keen on seeing ERC adopt macos-keychain as an official
back end, additional requirements likely apply. I say that because the
back ends comprising our current selection all provide a common set of
capabilities, namely, those needed to adapt IRC semantics to a
generalized secrets store. The auth-source side is somewhat spelled out
in the doc string for `auth-source-search', but ERC derives its idea of
the interface mostly from the behavior of the reference back end, netrc.
If interested, see test/lisp/erc/erc-services-tests.el and the latter
half of test/lisp/auth-source-pass-tests.el for all the particulars.

Thanks,
J.P.

P.S. If replying to this, please Cc. emacs-erc@gnu.org.

bug#64977: 29.1; `auth-source-macos-keychain-search' handles :user key incorrectly

[LdBeth]

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


Finally I get some time looking at this issue.

I think the bug is caused by `auth-source-macos-keychain-search-items'
forget the fact that :users can be a list of string instead of a string.

The `auth-source-macos-keychain-search-items' also did
not handle the protocol argument correctly. The `security' command
needs the "-r" argument to be a string of 4 characters. This is also
fixed in the patch attached.

Best wishes,
ldb


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 6329 bytes --]

--- auth-source.el.old	2023-08-08 16:37:41.000000000 -0500
+++ auth-source.el	2023-08-08 17:08:23.000000000 -0500
@@ -1958,20 +1958,23 @@
          (hosts (if (and hosts (listp hosts)) hosts `(,hosts)))
          (ports (plist-get spec :port))
          (ports (if (and ports (listp ports)) ports `(,ports)))
+         (users (plist-get spec :user))
+         (users (if (and users (listp users)) users `(,users)))
          ;; Loop through all combinations of host/port and pass each of these to
          ;; auth-source-macos-keychain-search-items
          (items (catch 'match
                   (dolist (host hosts)
                     (dolist (port ports)
-                      (let* ((port (if port (format "%S" port)))
-                             (items (apply #'auth-source-macos-keychain-search-items
-                                           coll
-                                           type
-                                           max
-                                           host port
-                                           search-spec)))
-                        (when items
-                          (throw 'match items)))))))
+                      (dolist (user users)
+                        (let ((items (apply
+                                      #'auth-source-macos-keychain-search-items
+                                      coll
+                                      type
+                                      max
+                                      host port user
+                                      search-spec)))
+                          (when items
+                            (throw 'match items))))))))
 
          ;; ensure each item has each key in `returned-keys'
          (items (mapcar (lambda (plist)
@@ -2003,8 +2006,9 @@
                      collect var))
      'utf-8)))
 
-(cl-defun auth-source-macos-keychain-search-items (coll _type _max host port
-                                                   &key label type user
+(cl-defun auth-source-macos-keychain-search-items (coll _type _max
+                                                        host port user
+                                                   &key label type
                                                    &allow-other-keys)
   (let* ((keychain-generic (eq type 'macos-keychain-generic))
          (args `(,(if keychain-generic
@@ -2022,47 +2026,47 @@
     (when port
       (if keychain-generic
           (setq args (append args (list "-s" port)))
-        (setq args (append args (list
-                                 (if (string-match "[0-9]+" port) "-P" "-r")
-                                 port)))))
-
-      (unless (equal coll "default")
-        (setq args (append args (list coll))))
-
-      (with-temp-buffer
-        (apply #'call-process "/usr/bin/security" nil t nil args)
-        (goto-char (point-min))
-        (while (not (eobp))
-          (cond
-           ((looking-at "^password: \\(?:0x[0-9A-F]+\\)? *\"\\(.+\\)\"")
-            (setq ret (auth-source-macos-keychain-result-append
-                       ret
-                       keychain-generic
-                       "secret"
-                       (let ((v (auth-source--decode-octal-string
-                                 (match-string 1))))
-                         (lambda () v)))))
-           ;; TODO: check if this is really the label
-           ;; match 0x00000007 <blob>="AppleID"
-           ((looking-at
-             "^[ ]+0x00000007 <blob>=\\(?:0x[0-9A-F]+\\)? *\"\\(.+\\)\"")
-            (setq ret (auth-source-macos-keychain-result-append
-                       ret
-                       keychain-generic
-                       "label"
-                       (auth-source--decode-octal-string (match-string 1)))))
-           ;; match "crtr"<uint32>="aapl"
-           ;; match "svce"<blob>="AppleID"
-           ((looking-at
-             "^[ ]+\"\\([a-z]+\\)\"[^=]+=\\(?:0x[0-9A-F]+\\)? *\"\\(.+\\)\"")
-            (setq ret (auth-source-macos-keychain-result-append
-                       ret
-                       keychain-generic
-                       (auth-source--decode-octal-string (match-string 1))
-                       (auth-source--decode-octal-string (match-string 2))))))
-          (forward-line)))
-      ;; return `ret' iff it has the :secret key
-      (and (plist-get ret :secret) (list ret))))
+        (setq args (append args (if (string-match "[0-9]+" port)
+                                    (list "-P" port)
+                                  (list "-r" (format "%-4s" port)))))))
+
+    (unless (equal coll "default")
+      (setq args (append args (list coll))))
+
+    (with-temp-buffer
+      (apply #'call-process "/usr/bin/security" nil t nil args)
+      (goto-char (point-min))
+      (while (not (eobp))
+        (cond
+         ((looking-at "^password: \\(?:0x[0-9A-F]+\\)? *\"\\(.+\\)\"")
+          (setq ret (auth-source-macos-keychain-result-append
+                     ret
+                     keychain-generic
+                     "secret"
+                     (let ((v (auth-source--decode-octal-string
+                               (match-string 1))))
+                       (lambda () v)))))
+         ;; TODO: check if this is really the label
+         ;; match 0x00000007 <blob>="AppleID"
+         ((looking-at
+           "^[ ]+0x00000007 <blob>=\\(?:0x[0-9A-F]+\\)? *\"\\(.+\\)\"")
+          (setq ret (auth-source-macos-keychain-result-append
+                     ret
+                     keychain-generic
+                     "label"
+                     (auth-source--decode-octal-string (match-string 1)))))
+         ;; match "crtr"<uint32>="aapl"
+         ;; match "svce"<blob>="AppleID"
+         ((looking-at
+           "^[ ]+\"\\([a-z]+\\)\"[^=]+=\\(?:0x[0-9A-F]+\\)? *\"\\(.+\\)\"")
+          (setq ret (auth-source-macos-keychain-result-append
+                     ret
+                     keychain-generic
+                     (auth-source--decode-octal-string (match-string 1))
+                     (auth-source--decode-octal-string (match-string 2))))))
+        (forward-line)))
+    ;; return `ret' iff it has the :secret key
+    (and (plist-get ret :secret) (list ret))))
 
 (defun auth-source-macos-keychain-result-append (result generic k v)
   (push v result)

bug#64977: 29.1; `auth-source-macos-keychain-search' handles :user key incorrectly

[Eli Zaretskii]

> Date: Tue, 08 Aug 2023 17:20:32 -0500
> From: LdBeth <andpuke@foxmail.com>
> 
> Finally I get some time looking at this issue.
> 
> I think the bug is caused by `auth-source-macos-keychain-search-items'
> forget the fact that :users can be a list of string instead of a string.
> 
> The `auth-source-macos-keychain-search-items' also did
> not handle the protocol argument correctly. The `security' command
> needs the "-r" argument to be a string of 4 characters. This is also
> fixed in the patch attached.

Michael, any comments to the problem and the patch?

bug#64977: 29.1; `auth-source-macos-keychain-search' handles :user key incorrectly

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

Hi Eli,

>> Finally I get some time looking at this issue.
>>
>> I think the bug is caused by `auth-source-macos-keychain-search-items'
>> forget the fact that :users can be a list of string instead of a string.
>>
>> The `auth-source-macos-keychain-search-items' also did
>> not handle the protocol argument correctly. The `security' command
>> needs the "-r" argument to be a string of 4 characters. This is also
>> fixed in the patch attached.
>
> Michael, any comments to the problem and the patch?

The change in :user seems to be OK. I cannot comment the change in
:port, because I don't use macOS, and cannot test anything.

Best regards, Michael.

bug#64977: 29.1; `auth-source-macos-keychain-search' handles :user key incorrectly

[LdBeth]

From man security(1):

    -r protocol     Specify protocol (optional four-character SecProtocolType, e.g. "http", "ftp ")

https://www.unix.com/man-page/osx/1/security/

To test this function, first add password using:

$ security add-internet-password -s irc.libera.chat -a ldb -r 'irc ' -w 'passwd'

In emacs, use something like

(auth-source-search :user "ldb" :port "irc")

should about to retrieve the password.


ldb

>>>>> In <87leegqyk3.fsf@gmx.de> 
>>>>>	Michael Albinus <michael.albinus@gmx.de> wrote:
> Eli Zaretskii <eliz@gnu.org> writes:

> Hi Eli,

>>> Finally I get some time looking at this issue.
>>>
>>> I think the bug is caused by `auth-source-macos-keychain-search-items'
>>> forget the fact that :users can be a list of string instead of a string.
>>>
>>> The `auth-source-macos-keychain-search-items' also did
>>> not handle the protocol argument correctly. The `security' command
>>> needs the "-r" argument to be a string of 4 characters. This is also
>>> fixed in the patch attached.
>>
>> Michael, any comments to the problem and the patch?

> The change in :user seems to be OK. I cannot comment the change in
> :port, because I don't use macOS, and cannot test anything.

> Best regards, Michael.

bug#64977: 29.1; `auth-source-macos-keychain-search' handles :user key incorrectly

[Michael Albinus]

LdBeth <andpuke@foxmail.com> writes:

Hi,

> To test this function, first add password using:
>
> $ security add-internet-password -s irc.libera.chat -a ldb -r 'irc ' -w 'passwd'
>
> In emacs, use something like
>
> (auth-source-search :user "ldb" :port "irc")
>
> should about to retrieve the password.

Thanbks.

Do you believe it is possible to add corresponding tests to
auth-source-tests.el, like we have for the netrc and secrets backends?

> ldb

Best regards, Michael.

bug#64977: 29.1; `auth-source-macos-keychain-search' handles :user key incorrectly

[LdBeth]

I think it is possible to use cl-left to redefine
`auth-source-macos-keychain-search-items' to avoid calling external
security command and just check the argument passed to
`call-process'.

Something like:

(cl-letf (((symbol-function 'call-process) (lambda (&rest r) (print r))))
  (auth-source-search :user "ldb" :port "irc"))

("/usr/bin/security" nil t nil "find-internet-password" "-g" "-a" "ldb" "-r" "irc ")

("/usr/bin/security" nil t nil "find-generic-password" "-g" "-a" "ldb" "-s" "irc")

ldb

>>>>> In <87wmy0z9lp.fsf@gmx.de> 
>>>>>	Michael Albinus <michael.albinus@gmx.de> wrote:
> LdBeth <andpuke@foxmail.com> writes:

> Hi,

>> To test this function, first add password using:
>>
>> $ security add-internet-password -s irc.libera.chat -a ldb -r 'irc ' -w 'passwd'
>>
>> In emacs, use something like
>>
>> (auth-source-search :user "ldb" :port "irc")
>>
>> should about to retrieve the password.

> Thanbks.

> Do you believe it is possible to add corresponding tests to
> auth-source-tests.el, like we have for the netrc and secrets backends?

>> ldb

> Best regards, Michael.

bug#64977: 29.1; `auth-source-macos-keychain-search' handles :user key incorrectly

[Michael Albinus]

LdBeth <andpuke@foxmail.com> writes:

Hi,

> I think it is possible to use cl-left to redefine
> `auth-source-macos-keychain-search-items' to avoid calling external
> security command and just check the argument passed to
> `call-process'.
>
> Something like:
>
> (cl-letf (((symbol-function 'call-process) (lambda (&rest r) (print r))))
>   (auth-source-search :user "ldb" :port "irc"))
>
> ("/usr/bin/security" nil t nil "find-internet-password" "-g" "-a" "ldb" "-r" "irc ")
>
> ("/usr/bin/security" nil t nil "find-generic-password" "-g" "-a" "ldb" "-s" "irc")

Yes, like this. And in the lambda function, you could check the expected
arguments of the "/usr/bin/security" call by `should' and friends.

Would you like to add such test(s)? This would give us more confidence
that nothing is or will be broken, because people using macOS would test
this by default when running "make check", even if they don't care the
macOS keychain.

> ldb

Best regards, Michael.

bug#64977: 29.1; `auth-source-macos-keychain-search' handles :user key incorrectly

[LdBeth]

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


Sure. I have made the test. I also found a issue not covered in my
previous patch, that if the protocol is longer than 4 characters it
should be truncated. Please have a look at them.

ldb


[-- Attachment #2: patch1 --]
[-- Type: text/plain, Size: 6439 bytes --]

--- auth-source.el.old	2023-08-08 16:37:41.000000000 -0500
+++ auth-source.el	2023-08-08 17:08:23.000000000 -0500
@@ -1958,20 +1958,23 @@
          (hosts (if (and hosts (listp hosts)) hosts `(,hosts)))
          (ports (plist-get spec :port))
          (ports (if (and ports (listp ports)) ports `(,ports)))
+         (users (plist-get spec :user))
+         (users (if (and users (listp users)) users `(,users)))
          ;; Loop through all combinations of host/port and pass each of these to
          ;; auth-source-macos-keychain-search-items
          (items (catch 'match
                   (dolist (host hosts)
                     (dolist (port ports)
-                      (let* ((port (if port (format "%S" port)))
-                             (items (apply #'auth-source-macos-keychain-search-items
-                                           coll
-                                           type
-                                           max
-                                           host port
-                                           search-spec)))
-                        (when items
-                          (throw 'match items)))))))
+                      (dolist (user users)
+                        (let ((items (apply
+                                      #'auth-source-macos-keychain-search-items
+                                      coll
+                                      type
+                                      max
+                                      host port user
+                                      search-spec)))
+                          (when items
+                            (throw 'match items))))))))
 
          ;; ensure each item has each key in `returned-keys'
          (items (mapcar (lambda (plist)
@@ -2003,8 +2006,9 @@
                      collect var))
      'utf-8)))
 
-(cl-defun auth-source-macos-keychain-search-items (coll _type _max host port
-                                                   &key label type user
+(cl-defun auth-source-macos-keychain-search-items (coll _type _max
+                                                        host port user
+                                                   &key label type
                                                    &allow-other-keys)
   (let* ((keychain-generic (eq type 'macos-keychain-generic))
          (args `(,(if keychain-generic
@@ -2022,47 +2026,49 @@
     (when port
       (if keychain-generic
           (setq args (append args (list "-s" port)))
-        (setq args (append args (list
-                                 (if (string-match "[0-9]+" port) "-P" "-r")
-                                 port)))))
-
-      (unless (equal coll "default")
-        (setq args (append args (list coll))))
-
-      (with-temp-buffer
-        (apply #'call-process "/usr/bin/security" nil t nil args)
-        (goto-char (point-min))
-        (while (not (eobp))
-          (cond
-           ((looking-at "^password: \\(?:0x[0-9A-F]+\\)? *\"\\(.+\\)\"")
-            (setq ret (auth-source-macos-keychain-result-append
-                       ret
-                       keychain-generic
-                       "secret"
-                       (let ((v (auth-source--decode-octal-string
-                                 (match-string 1))))
-                         (lambda () v)))))
-           ;; TODO: check if this is really the label
-           ;; match 0x00000007 <blob>="AppleID"
-           ((looking-at
-             "^[ ]+0x00000007 <blob>=\\(?:0x[0-9A-F]+\\)? *\"\\(.+\\)\"")
-            (setq ret (auth-source-macos-keychain-result-append
-                       ret
-                       keychain-generic
-                       "label"
-                       (auth-source--decode-octal-string (match-string 1)))))
-           ;; match "crtr"<uint32>="aapl"
-           ;; match "svce"<blob>="AppleID"
-           ((looking-at
-             "^[ ]+\"\\([a-z]+\\)\"[^=]+=\\(?:0x[0-9A-F]+\\)? *\"\\(.+\\)\"")
-            (setq ret (auth-source-macos-keychain-result-append
-                       ret
-                       keychain-generic
-                       (auth-source--decode-octal-string (match-string 1))
-                       (auth-source--decode-octal-string (match-string 2))))))
-          (forward-line)))
-      ;; return `ret' iff it has the :secret key
-      (and (plist-get ret :secret) (list ret))))
+        (setq args (append args (if (string-match "[0-9]+" port)
+                                    (list "-P" port)
+                                  (list "-r" (substring
+                                              (format "%-4s" port)
+                                              0 4)))))))
+
+    (unless (equal coll "default")
+      (setq args (append args (list coll))))
+
+    (with-temp-buffer
+      (apply #'call-process "/usr/bin/security" nil t nil args)
+      (goto-char (point-min))
+      (while (not (eobp))
+        (cond
+         ((looking-at "^password: \\(?:0x[0-9A-F]+\\)? *\"\\(.+\\)\"")
+          (setq ret (auth-source-macos-keychain-result-append
+                     ret
+                     keychain-generic
+                     "secret"
+                     (let ((v (auth-source--decode-octal-string
+                               (match-string 1))))
+                       (lambda () v)))))
+         ;; TODO: check if this is really the label
+         ;; match 0x00000007 <blob>="AppleID"
+         ((looking-at
+           "^[ ]+0x00000007 <blob>=\\(?:0x[0-9A-F]+\\)? *\"\\(.+\\)\"")
+          (setq ret (auth-source-macos-keychain-result-append
+                     ret
+                     keychain-generic
+                     "label"
+                     (auth-source--decode-octal-string (match-string 1)))))
+         ;; match "crtr"<uint32>="aapl"
+         ;; match "svce"<blob>="AppleID"
+         ((looking-at
+           "^[ ]+\"\\([a-z]+\\)\"[^=]+=\\(?:0x[0-9A-F]+\\)? *\"\\(.+\\)\"")
+          (setq ret (auth-source-macos-keychain-result-append
+                     ret
+                     keychain-generic
+                     (auth-source--decode-octal-string (match-string 1))
+                     (auth-source--decode-octal-string (match-string 2))))))
+        (forward-line)))
+    ;; return `ret' iff it has the :secret key
+    (and (plist-get ret :secret) (list ret))))
 
 (defun auth-source-macos-keychain-result-append (result generic k v)
   (push v result)

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



[-- Attachment #4: patch2 --]
[-- Type: text/plain, Size: 1527 bytes --]

--- auth-source-tests.el.old	2023-08-12 12:55:52.000000000 -0500
+++ auth-source-tests.el	2023-08-12 13:58:28.000000000 -0500
@@ -435,5 +435,25 @@
             '((("machine" . "XM") ("login" . "XL") ("password" . "XP"))
               (("machine" . "YM") ("login" . "YL") ("password" . "YP")))))))
 
+(ert-deftest test-macos-keychain-search ()
+  "Test if the constructed command line arglist is correct."
+  (let ((auth-sources '(macos-keychain-internet macos-keychain-generic)))
+    ;; Redefine `call-process' to check command line arguments.
+    (cl-letf (((symbol-function 'call-process)
+               (lambda (_program _infile _destination _display
+                                 &rest args)
+                 ;; Arguments must be all strings
+                 (should (cl-every #'stringp args))
+                 ;; Argument number should be even
+                 (should (cl-evenp (length args)))
+                 (should (cond ((string= (car args) "find-internet-password")
+                                (let ((protocol (cl-member "-r" args :test #'string=)))
+                                  (if protocol
+                                      (= 4 (length (cadr protocol)))
+                                    t)))
+                               ((string= (car args) "find-generic-password")
+                                t))))))
+      (auth-source-search :user '("a" "b") :host '("example.org") :port '("irc" "ftp" "https")))))
+
 (provide 'auth-source-tests)
 ;;; auth-source-tests.el ends here

[-- Attachment #5: Type: text/plain, Size: 1090 bytes --]



>>>>> In <87r0o8z1sc.fsf@gmx.de> 
>>>>>	Michael Albinus <michael.albinus@gmx.de> wrote:
> LdBeth <andpuke@foxmail.com> writes:

> Hi,

>> I think it is possible to use cl-left to redefine
>> `auth-source-macos-keychain-search-items' to avoid calling external
>> security command and just check the argument passed to
>> `call-process'.
>>
>> Something like:
>>
>> (cl-letf (((symbol-function 'call-process) (lambda (&rest r) (print r))))
>>   (auth-source-search :user "ldb" :port "irc"))
>>
>> ("/usr/bin/security" nil t nil "find-internet-password" "-g" "-a" "ldb" "-r" "irc ")
>>
>> ("/usr/bin/security" nil t nil "find-generic-password" "-g" "-a" "ldb" "-s" "irc")

> Yes, like this. And in the lambda function, you could check the expected
> arguments of the "/usr/bin/security" call by `should' and friends.

> Would you like to add such test(s)? This would give us more confidence
> that nothing is or will be broken, because people using macOS would test
> this by default when running "make check", even if they don't care the
> macOS keychain.

>> ldb

> Best regards, Michael.

bug#64977: 29.1; `auth-source-macos-keychain-search' handles :user key incorrectly

[Michael Albinus]

Version: 30.1

LdBeth <andpuke@foxmail.com> writes:

Hi,

> Sure. I have made the test. I also found a issue not covered in my
> previous patch, that if the protocol is longer than 4 characters it
> should be truncated. Please have a look at them.

Thanks, I've pushed your patches to master. Closing the bug.

> ldb

Best regards, Michael.