>From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <jp@neverwas.me>
Date: Mon, 16 Aug 2021 04:38:18 -0700
Subject: [PATCH 19/28] Standardize auth-source queries in ERC

* lisp/erc/erc.el (erc-password): deprecate variable only used by
`erc-select-read-args'.  Server passwords are primarily used as
surrogates for other forms of authentication.  Such use is common but
nonstandard and often discouraged in favor of things like SASL.  Fans
of invoking `erc(-tls)' interactively should be coerced into using
auth-source instead.

(erc-select-read-args): Before this change, `erc-select-read-args'
offered to use the value of a non-nil `erc-password' as the :password
argument for `erc' and `erc-tls', referring to it as the "default"
password.  And when `erc-prompt-for-password' was nil and
`erc-password' wasn't, the latter was passed along unconditionally.
This only further complicated an already confusing situation for new
users, who in most cases shouldn't be worried about sending a PASS
command at all.  Until SASL arrives, they should provide server
passwords manually or learn to use auth-source.

(erc--auth-source-determine-params): New helper for
`erc--auth-source-search' with potential for wider role as default
value of custom function.  Favors :host and :port fields above others.

(erc--auth-source-search): New function for consulting auth-source and
sorting result as per default params provided by above functions.

(erc-server-join-channel): Use helper for consulting auth source
facility. Also accept nil for first argument (instead of server).  In
this case, allow default params option above to determine best course
of action.

(erc-cmd-JOIN): use above-mentioned facilities when joining new
channel.  Omit server when calling erc-server-join-channel.  Don't
filter target buffers twice.  Don't call `switch-to-buffer', which
would create phantom buffers with names like target/server that were
never used.  IOW, only switch to existing target buffers.

(erc-open, erc-determine-parameters, erc-compute-password): Move
password figuring from former to latter, and from there to erc-compute
password, which is a new function that figures out how to call
auth-source-search based on the value of the new option
erc-connect-auth-source-host.

(erc-connect-auth-source-host): Add new option for customizing the
:host param passed to auth-source-search while looking up the initial
PASS arg.  The default setting preserves existing behavior of matching
against the dialed host name or IP address stored in
erc-session-server.  Other options allow skipping auth-source lookup
altogether, or favoring network, when non-nil.

* test/lisp/erc/erc-tests.el: Add tests for new helpers.

* lisp/erc/erc-services.el (erc-nickserv-get-password): pass network
when looking up password in erc-nickserv-passwords and when formatting
prompt for user input.

* test/lisp/erc/erc-services-tests.el: add new test file for above
changes.
---
 lisp/erc/erc-services.el            |  41 +++----
 lisp/erc/erc.el                     | 174 +++++++++++++++++++---------
 test/lisp/erc/erc-services-tests.el |  63 ++++++++++
 test/lisp/erc/erc-tests.el          | 114 ++++++++++++++++++
 4 files changed, 309 insertions(+), 83 deletions(-)
 create mode 100644 test/lisp/erc/erc-services-tests.el

diff --git a/lisp/erc/erc-services.el b/lisp/erc/erc-services.el
index adb3f521cd..b0c58ae0ce 100644
--- a/lisp/erc/erc-services.el
+++ b/lisp/erc/erc-services.el
@@ -431,34 +431,19 @@ erc-nickserv-get-password
 lookups stops and this function returns it (or returns nil if it
 is empty).  Otherwise, no corresponding password was found, and
 it returns nil."
-  (let (network server port)
-    ;; Fill in local vars, switching to the server buffer once only
-    (erc-with-server-buffer
-     (setq network erc-network
-           server erc-session-server
-           port erc-session-port))
-    (let ((ret
-           (or
-            (when erc-nickserv-passwords
-              (cdr (assoc nick
-                          (cl-second (assoc network
-                                            erc-nickserv-passwords)))))
-            (when erc-use-auth-source-for-nickserv-password
-              (let ((secret (cl-first (auth-source-search
-                                       :max 1 :require '(:secret)
-                                       :host server
-                                       ;; Ensure a string for :port
-                                       :port (format "%s" port)
-                                       :user nick))))
-                (when secret
-                  (let ((passwd (plist-get secret :secret)))
-                    (if (functionp passwd) (funcall passwd) passwd)))))
-            (when erc-prompt-for-nickserv-password
-              (read-passwd
-               (format "NickServ password for %s on %s (RET to cancel): "
-                       nick network))))))
-      (when (and ret (not (string= ret "")))
-        ret))))
+  (when-let*
+      ((esid (erc-network))
+       (ret (or (when erc-nickserv-passwords
+                  (assoc-default nick
+                                 (cadr (assq esid erc-nickserv-passwords))))
+                (when erc-use-auth-source-for-nickserv-password
+                  (erc--auth-source-search :user nick))
+                (when erc-prompt-for-nickserv-password
+                  (read-passwd
+                   (format "NickServ password for %s on %s (RET to cancel): "
+                           nick esid)))))
+       ((not (string-empty-p ret))))
+    ret))
 
 (defvar erc-auto-discard-away)
 
diff --git a/lisp/erc/erc.el b/lisp/erc/erc.el
index 2fefa00778..8ebe50fa89 100644
--- a/lisp/erc/erc.el
+++ b/lisp/erc/erc.el
@@ -206,9 +206,14 @@ erc-rename-buffers
                         "old behavior when t now permanent" "29.1")
 
 (defvar erc-password nil
-  "Password to use when authenticating to an IRC server.
-It is not strictly necessary to provide this, since ERC will
-prompt you for it.")
+  "Password to use when authenticating to an IRC server interactively.
+
+This variable only exists for legacy reasons.  It's not customizable and
+is limited to a single server password.  Users looking for similar
+functionality should consider auth-source instead.  See info
+node `(auth) Top' and info node `(erc) Connecting'.")
+
+(make-obsolete-variable 'erc-password "use auth-source instead" "29.1")
 
 (defcustom erc-user-mode "+i"
   ;; +i "Invisible".  Hides user from global /who and /names.
@@ -219,10 +224,30 @@ erc-user-mode
 
 
 (defcustom erc-prompt-for-password t
-  "Asks before using the default password, or whether to enter a new one."
+  "Ask for a server password when invoking `erc-tls' interactively."
   :group 'erc
   :type 'boolean)
 
+(defcustom erc-connect-auth-source-host 'server
+  "Host \"type\" for querying auth-source when first connecting.
+This is for determining the \"server password\" argument of the IRC
+\"PASS\" command sent to the server.  The entry points `erc' and
+`erc-tls' query auth-source for such a password when a :password
+argument isn't provided.  Because ERC also interfaces with auth-source
+for other secrets, such as NickServ passwords and channel keys,
+additional ways of selecting entries are sometimes necessary.  See info
+node `(auth) Top'.
+
+Note that there aren't any options for specifying a network, like
+Libera.Chat, or a network-specific server, such as foo.libera.chat,
+because such information isn't available until after initial
+introductions have completed (\"registration\" in IRC speak)."
+  :group 'erc
+  :type '(choice (const :tag "Don't query auth-source" nil)
+                 (const :tag "Dialed host name or IP address" server)
+                 (const :tag "Prompt for a machine/host value" prompt)
+                 (string :tag "Literal value to use for :host")))
+
 (defcustom erc-warn-about-blank-lines t
   "Warn the user if they attempt to send a blank line."
   :group 'erc
@@ -2217,22 +2242,6 @@ erc-open
     (setq erc-logged-in nil)
     ;; The local copy of `erc-nick' - the list of nicks to choose
     (setq erc-default-nicks (if (consp erc-nick) erc-nick (list erc-nick)))
-    ;; password stuff
-    (setq erc-session-password
-          (or passwd
-              (let ((secret
-                     (plist-get
-                      (nth 0
-                           (auth-source-search :host server
-                                               :max 1
-                                               :user nick
-                                               ;; secrets.el wouldn’t accept a number
-                                               :port (if (numberp port) (number-to-string port) port)
-                                               :require '(:secret)))
-                      :secret)))
-                (if (functionp secret)
-                    (funcall secret)
-                  secret))))
     ;; client certificate (only useful if connecting over TLS)
     (setq erc-session-client-certificate client-certificate)
     (setq erc--session (if connect
@@ -2252,7 +2261,7 @@ erc-open
       (erc-display-prompt)
       (goto-char (point-max)))
 
-    (erc-determine-parameters server port nick full-name)
+    (erc-determine-parameters server port nick full-name passwd)
 
     ;; Saving log file on exit
     (run-hook-with-args 'erc-connect-pre-hook buffer)
@@ -2350,11 +2359,9 @@ erc-select-read-args
     (setq server user-input)
 
     (setq passwd (if erc-prompt-for-password
-                     (if (and erc-password
-                              (y-or-n-p "Use the default password? "))
-                         erc-password
-                       (read-passwd "Password: "))
-                   erc-password))
+                     (read-passwd "Server password: ")
+                   (with-suppressed-warnings ((obsolete erc-password))
+                     erc-password)))
     (when (and passwd (string= "" passwd))
       (setq passwd nil))
 
@@ -3366,22 +3373,71 @@ erc-cmd-HELP
 (defalias 'erc-cmd-H #'erc-cmd-HELP)
 (put 'erc-cmd-HELP 'process-not-needed t)
 
+;; Users may want to override this.  We could convert it to the
+;; default value of a -function option (or use a defmethod).
+
+(defun erc--auth-source-determine-params ()
+  "Return a plist of default args to pass to `auth-source-search'.
+Favor a discovered network name over an announced server unless
+`erc--buffer-target' is a local channel.  Consider the dialed server
+address as a fallback for the announced name in both cases."
+  (let* ((net (and-let* ((net (erc-network))) (symbol-name net)))
+         (localp (and erc--buffer-target
+                      (erc--target-local-p erc--buffer-target)))
+         (hosts (if localp
+                    (list erc-server-announced-name erc-session-server net)
+                  (list net erc-server-announced-name erc-session-server)))
+         (ports (list (cl-typecase erc-session-port
+                        (integer (number-to-string erc-session-port))
+                        (string (and (string= erc-session-port "irc")
+                                     erc-session-port)) ; or nil
+                        (t erc-session-port))
+                      "irc")))
+    (list :host (delq nil hosts)
+          :port (delq nil ports)
+          :require (list :secret))))
+
+(defun erc--auth-source-search (&rest plist)
+  "Ask auth-source for a secret and return it if found.
+Favor overrides in PLIST, if any.  Otherwise, use whatever's present in
+the list returned by `erc--auth-source-determine-params'.  Return a
+string if found or nil otherwise."
+  (let* ((defaults (erc--auth-source-determine-params))
+         priority
+         (test (lambda (a b)
+                 (catch 'done
+                   (dolist (key priority)
+                     (let* ((d (plist-get defaults key))
+                            (default-value (if (listp d) d (list d)))
+                            ;; featurep 'seq via auth-source > json > map
+                            (p (seq-position default-value (plist-get a key)))
+                            (q (seq-position default-value (plist-get b key))))
+                       (unless (eql p q)
+                         (throw 'done (when p (or (not q) (< p q)))))))))))
+    (cl-loop for (key value) on defaults by #'cddr
+             when value unless (plist-get plist key)
+             do (setq plist (plist-put plist key value)))
+    (let ((keys (nreverse (map-keys defaults))))
+      (dolist (key (map-keys plist))
+        (cl-pushnew key keys))
+      (setq priority (nreverse keys)))
+    (unless (plist-get plist :max) ; from `auth-source-netrc-parse'
+      (setq plist (plist-put plist :max 5000)))
+    (when-let* ((sorted (sort (apply #'auth-source-search plist) test))
+                (secret (plist-get (car sorted) :secret)))
+      (if (functionp secret) (funcall secret) secret))))
+
 (defun erc-server-join-channel (server channel &optional secret)
-  (let* ((secret (or secret
-		     (plist-get (nth 0 (auth-source-search
-					:max 1
-					:host server
-					:port "irc"
-					:user channel))
-				:secret)))
-	 (password (if (functionp secret)
-		       (funcall secret)
-		     secret)))
-    (erc-log (format "cmd: JOIN: %s" channel))
-    (erc-server-send (concat "JOIN " channel
-			     (if password
-				 (concat " " password)
-			       "")))))
+  "Join CHANNEL, optionally with SECRET.
+Without SECRET, consult auth source, using SERVER if non-nil."
+  (unless secret
+    (unless server
+      (when (and erc-server-announced-name (erc-valid-local-channel-p channel))
+        (setq server erc-server-announced-name)))
+    (let ((args `(,@(when server (list :host server)) :user channel)))
+      (setq secret (apply #'erc--auth-source-search args))))
+  (erc-log (format "cmd: JOIN: %s" channel))
+  (erc-server-send (concat "JOIN " channel (when secret (concat " " secret)))))
 
 (defun erc-valid-local-channel-p (channel)
   "Non-nil when channel is server-local on a network that allows them."
@@ -3401,18 +3457,11 @@ erc-cmd-JOIN
       (setq chnl (erc-ensure-channel-name channel)))
     (when chnl
       ;; Prevent double joining of same channel on same server.
-      (let* ((joined-channels
-              (mapcar (lambda (chanbuf)
-                        (with-current-buffer chanbuf (erc-default-target)))
-                      (erc-channel-list erc-server-process)))
-             (server (with-current-buffer (process-buffer erc-server-process)
-		       (or erc-session-server erc-server-announced-name)))
-             (chnl-name (car (erc-member-ignore-case chnl joined-channels))))
-        (if chnl-name
-            (switch-to-buffer (if (get-buffer chnl-name)
-                                  chnl-name
-                                (concat chnl-name "/" server)))
-	  (erc-server-join-channel server chnl key)))))
+      (if-let* ((existing (erc-get-buffer chnl erc-server-process))
+                ((with-current-buffer existing
+                   (erc-get-channel-user (erc-current-nick)))))
+          (switch-to-buffer existing)
+        (erc-server-join-channel nil chnl key))))
   t)
 
 (defalias 'erc-cmd-CHANNEL #'erc-cmd-JOIN)
@@ -6368,7 +6417,7 @@ erc-login
 
 ;; connection properties' heuristics
 
-(defun erc-determine-parameters (&optional server port nick name)
+(defun erc-determine-parameters (&optional server port nick name passwd)
   "Determine the connection and authentication parameters.
 Sets the buffer local variables:
 
@@ -6376,11 +6425,13 @@ erc-determine-parameters
 - `erc-session-server'
 - `erc-session-port'
 - `erc-session-user-full-name'
+- `erc-session-password'
 - `erc-server-current-nick'"
   (setq erc-session-connector erc-server-connect-function
         erc-session-server (erc-compute-server server)
         erc-session-port (or port erc-default-port)
-        erc-session-user-full-name (erc-compute-full-name name))
+        erc-session-user-full-name (erc-compute-full-name name)
+        erc-session-password (erc-compute-server-password passwd nick))
   (erc-set-current-nick (erc-compute-nick nick)))
 
 (defun erc-compute-server (&optional server)
@@ -6413,6 +6464,19 @@ erc-compute-nick
       (getenv "IRCNICK")
       (user-login-name)))
 
+(defun erc-compute-server-password (password nick)
+  "Determine initial PASSWORD value for IRC PASS command.
+Use the value of `erc-connect-auth-source-host' to determine the
+machine/host query param.  Use NICK for the user/login query param."
+  (or password
+      (when erc-connect-auth-source-host
+        (let* ((host (pcase erc-connect-auth-source-host
+                       ('server erc-session-server)
+                       ((and (pred stringp) v) v)
+                       ('prompt (read-string "Auth-source host: "
+                                             nil t (list nil)))))
+               (args `(,@(when host (list :host host)) :user ,nick)))
+          (apply #'erc--auth-source-search args)))))
 
 (defun erc-compute-full-name (&optional full-name)
   "Return user's full name.
diff --git a/test/lisp/erc/erc-services-tests.el b/test/lisp/erc/erc-services-tests.el
new file mode 100644
index 0000000000..c646b1c69d
--- /dev/null
+++ b/test/lisp/erc/erc-services-tests.el
@@ -0,0 +1,63 @@
+;;; erc-services-tests.el --- Tests for erc-services.  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2020-2021 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+;;
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published
+;; by the Free Software Foundation, either version 3 of the License,
+;; or (at your option) any later version.
+;;
+;; GNU Emacs is distributed in the hope that it will be useful, but
+;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+;; General Public License for more details.
+;;
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(require 'ert-x)
+(require 'erc-services)
+
+(ert-deftest erc-nickserv-get-password ()
+  (should erc-prompt-for-nickserv-password)
+  (let* ((entries (list
+                   "machine GNU/chat port 6697 user bob password spam"
+                   "machine FSF.chat port 6697 user bob password sesame"
+                   "machine MyHost port irc password 123"))
+         (netrc-file (make-temp-file "auth-source-test" nil nil
+                                     (mapconcat 'identity entries "\n")))
+         (auth-sources (list netrc-file))
+         (auth-source-do-cache nil)
+         (erc-nickserv-passwords '((FSF.chat (("alice" . "foo")
+                                              ("joe" . "bar")))))
+         (erc-use-auth-source-for-nickserv-password t)
+         (erc-session-server "irc.gnu.org")
+         (erc-server-announced-name "west.gnu.org")
+         (erc-network 'FSF.chat)
+         (erc-session-port 6697))
+
+    (ert-info ("Lookup custom option")
+      (should (string= (erc-nickserv-get-password "alice") "foo")))
+
+    (unwind-protect
+        (ert-info ("Auth source")
+
+          (ert-info ("Network")
+            (should (string= (erc-nickserv-get-password "bob") "sesame"))))
+
+      (delete-file netrc-file))
+
+    (ert-info ("Read input")
+      (should (string=
+               (ert-simulate-keys "baz\r" (erc-nickserv-get-password "mike"))
+               "baz")))
+
+    (ert-info ("Failed")
+      (should-not (ert-simulate-keys "\r"
+                    (erc-nickserv-get-password "fake"))))))
+
+;;; erc-services-tests.el ends here
diff --git a/test/lisp/erc/erc-tests.el b/test/lisp/erc/erc-tests.el
index 8d078a2053..a337195898 100644
--- a/test/lisp/erc/erc-tests.el
+++ b/test/lisp/erc/erc-tests.el
@@ -238,6 +238,120 @@ erc--target-from-string
     (should (equal (erc--target-from-string "&Bitlbee")
                    #s(erc--target "&Bitlbee" &bitlbee t t)))))
 
+;; This stuff is related to bug#23438.
+
+(defvar erc-join-tests--auth-source-entries
+  '("machine irc.gnu.org port irc user \"#chan\" password bar"
+    "machine west.gnu.org port irc user \"#chan\" password baz"
+    "machine GNU.chat port irc user \"#chan\" password foo"))
+
+(ert-deftest erc--auth-source-search--standard ()
+  (let* ((entries (sort (copy-sequence erc-join-tests--auth-source-entries)
+                        (lambda (&rest _) (zerop (random 2)))))
+         (netrc-file (make-temp-file "auth-source-test" nil nil
+                                     (mapconcat 'identity entries "\n")))
+         (auth-sources (list netrc-file))
+         (auth-source-do-cache nil))
+
+    (unwind-protect
+
+        (ert-info ("Normal ordering")
+
+          (ert-info ("Network wins")
+            (let ((erc-session-server "irc.gnu.org")
+                  (erc-server-announced-name "west.gnu.org")
+                  (erc-session-port 6697)
+                  (erc-network 'GNU.chat))
+              (should (string= (erc--auth-source-search :user "#chan")
+                               "foo"))))
+
+          (ert-info ("Announced wins")
+            (let ((erc-session-server "irc.gnu.org")
+                  (erc-server-announced-name "west.gnu.org")
+                  (erc-session-port 6697)
+                  erc-network)
+              (should (string= (erc--auth-source-search :user "#chan")
+                               "baz")))))
+
+      (delete-file netrc-file))))
+
+(ert-deftest erc--auth-source-search--announced ()
+  (let* ((entries (sort (copy-sequence erc-join-tests--auth-source-entries)
+                        (lambda (&rest _) (zerop (random 2)))))
+         (netrc-file (make-temp-file "auth-source-test" nil nil
+                                     (mapconcat 'identity entries "\n")))
+         (auth-sources (list netrc-file))
+         (auth-source-do-cache nil)
+         (erc-isupport-parameters '((CHANTYPES "&#")))
+         (erc--buffer-target (erc--target-from-string "&chan")))
+
+    (unwind-protect
+
+        (ert-info ("Announced prioritized")
+
+          (ert-info ("Announced wins")
+            (let ((erc-session-server "irc.gnu.org")
+                  (erc-server-announced-name "west.gnu.org")
+                  (erc-session-port 6697)
+                  (erc-network 'GNU.chat))
+              (should (string= (erc--auth-source-search :user "#chan")
+                               "baz"))))
+
+          (ert-info ("Peer next")
+            (let ((erc-server-announced-name "irc.gnu.org")
+                  (erc-session-port 6697)
+                  (erc-network 'GNU.chat))
+              (should (string= (erc--auth-source-search :user "#chan")
+                               "bar"))))
+
+          (ert-info ("Network used as fallback")
+            (let ((erc-session-port 6697)
+                  (erc-network 'GNU.chat))
+              (should (string= (erc--auth-source-search :user "#chan")
+                               "foo")))))
+
+      (delete-file netrc-file))))
+
+(ert-deftest erc--auth-source-search--overrides ()
+  (let* ((extra (list
+                 "machine GNU.chat port 6697 user \"#chan\" password spam"
+                 "machine west.gnu.org port irc user \"#fsf\" password 42"
+                 "machine irc.gnu.org port 6667 password sesame"
+                 "machine MyHost port irc password 456"
+                 "machine MyHost port 6667 password 123"))
+         (entries (sort (append erc-join-tests--auth-source-entries extra)
+                        (lambda (&rest _) (zerop (random 2)))))
+         (netrc-file (make-temp-file "auth-source-test" nil nil
+                                     (mapconcat 'identity entries "\n")))
+         (auth-sources (list netrc-file))
+         (auth-source-do-cache nil)
+         (erc-session-server "irc.gnu.org")
+         (erc-server-announced-name "west.gnu.org")
+         (erc-network 'GNU.chat)
+         (erc-session-port 6667))
+
+    (unwind-protect
+        (ert-info ("Specificity and overrides")
+
+          (ert-info ("More specific port")
+            (let ((erc-session-port 6697))
+              (should (string= (erc--auth-source-search :user "#chan")
+                               "spam"))))
+
+          (ert-info ("Network wins")
+            (should (string= (erc--auth-source-search :user '("#fsf"))
+                             "42")))
+
+          (ert-info ("Actual override")
+            (should (string= (erc--auth-source-search :port "6667")
+                             "sesame")))
+
+          (ert-info ("Overrides don't interfere with post-processing")
+            (should (string= (erc--auth-source-search :host "MyHost")
+                             "123"))))
+
+      (delete-file netrc-file))))
+
 (ert-deftest erc-ring-previous-command-base-case ()
   (ert-info ("Create ring when nonexistent and do nothing")
     (let (erc-input-ring
-- 
2.31.1