Re: bug#29108: 25.3; ERC SASL support

From f406d32c6ea6a5e08f660bafd4ea30767936b799 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <[email protected]>
Date: Wed, 23 Nov 2022 21:31:19 -0800
Subject: [PATCH 1/3] Add erc-sasl-auth-source-function to cached options

* lisp/erc/erc-sasl.el (erc-sasl--read-password): Consult cached
options instead of `erc-sasl-auth-source-function'.
(erc-sasl--init): Add `erc-sasl-auth-source-function' to
`erc-sasl--options'.

* test/lisp/erc/erc-sasl-tests.el (erc-sasl--read-password--basic,
erc-sasl--read-password--auth-source): Look for original value of
`erc-sasl-auth-source-function' in `erc-sasl--options' under the
`authfn' key.
---
 lisp/erc/erc-sasl.el            | 10 ++++++----
 test/lisp/erc/erc-sasl-tests.el | 31 ++++++++++++++++---------------
 2 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/lisp/erc/erc-sasl.el b/lisp/erc/erc-sasl.el
index 9084d873ce..5ee7169de5 100644
--- a/lisp/erc/erc-sasl.el
+++ b/lisp/erc/erc-sasl.el
@@ -137,10 +137,11 @@ erc-sasl--read-password
       ((found (pcase (alist-get 'password erc-sasl--options)
                 (:password erc-session-password)
                 ((and (pred stringp) v) (unless (string-empty-p v) v))
-                ((and (guard erc-sasl-auth-source-function)
-                      v (let host
-                          (or v (erc-networks--id-given erc-networks--id))))
-                 (apply erc-sasl-auth-source-function
+                ((and (let fn (alist-get 'authfn erc-sasl--options))
+                      (guard fn) v
+                      (let host
+                        (or v (erc-networks--id-given erc-networks--id))))
+                 (apply fn
                         :user (erc-sasl--get-user)
                         (and host (list :host (symbol-name host))))))))
       (copy-sequence (erc--unfun found))
@@ -293,6 +294,7 @@ erc-sasl--init
             `((user . ,erc-sasl-user)
               (password . ,erc-sasl-password)
               (mechanism . ,erc-sasl-mechanism)
+              (authfn . ,erc-sasl-auth-source-function)
               (authzid . ,erc-sasl-authzid)))))
 
 (defun erc-sasl--mechanism-offered-p (offered)
diff --git a/test/lisp/erc/erc-sasl-tests.el b/test/lisp/erc/erc-sasl-tests.el
index 64593ca270..a0e871979a 100644
--- a/test/lisp/erc/erc-sasl-tests.el
+++ b/test/lisp/erc/erc-sasl-tests.el
@@ -42,17 +42,17 @@ erc-sasl--read-password--basic
           (erc-sasl--options '((password . :password))))
       (should (string= (erc-sasl--read-password nil) "foo"))))
 
-  (ert-info ("Fallback to prompt skip auth-source")
-    (should-not erc-sasl-auth-source-function)
-    (let ((erc-session-password "bar")
-          (erc-networks--id (erc-networks--id-create nil)))
+  (ert-info ("Prompt when no authfn and :password resolves to nil")
+    (let ((erc-session-password nil)
+          (erc-sasl--options
+           '((password . :password) (user . :user) (authfn))))
       (should (string= (ert-simulate-keys "bar\r"
                          (erc-sasl--read-password "?"))
                        "bar"))))
 
-  (ert-info ("Prompt when auth-source fails and `erc-sasl-password' null")
-    (let ((erc-sasl--options '((password)))
-          (erc-sasl-auth-source-function #'ignore))
+  (ert-info ("Prompt when auth-source fails and `erc-session-password' null")
+    (should-not erc-session-password)
+    (let ((erc-sasl--options '((password) (authfn . ignore))))
       (should (string= (ert-simulate-keys "baz\r"
                          (erc-sasl--read-password "pwd:"))
                        "baz")))))
@@ -71,36 +71,37 @@ erc-sasl--read-password--auth-source
            (erc-session-port 6697)
            (erc-networks--id (erc-networks--id-create nil))
            calls
-           (erc-sasl-auth-source-function
-            (lambda (&rest r)
-              (push r calls)
-              (apply #'erc--auth-source-search r)))
+           (fn (lambda (&rest r)
+                 (push r calls)
+                 (apply #'erc--auth-source-search r)))
            erc-server-announced-name ; too early
            auth-source-do-cache)
 
       (ert-info ("Symbol as password specifies machine")
-        (let ((erc-sasl--options '((user . "bob") (password . FSF.chat)))
+        (let ((erc-sasl--options
+               `((user . "bob") (password . FSF.chat) (authfn . ,fn)))
               (erc-networks--id (make-erc-networks--id)))
           (should (string= (erc-sasl--read-password nil) "sesame"))
           (should (equal (pop calls) '(:user "bob" :host "FSF.chat")))))
 
       (ert-info ("ID for :host and `erc-session-username' for :user") ; *1
         (let ((erc-session-username "bob")
-              (erc-sasl--options '((user . :user) (password)))
+              (erc-sasl--options `((user . :user) (password) (authfn . ,fn)))
               (erc-networks--id (erc-networks--id-create 'GNU/chat)))
           (should (string= (erc-sasl--read-password nil) "spam"))
           (should (equal (pop calls) '(:user "bob" :host "GNU/chat")))))
 
       (ert-info ("ID for :host and current nick for :user") ; *1
         (let ((erc-server-current-nick "bob")
-              (erc-sasl--options '((user . :nick) (password)))
+              (erc-sasl--options `((user . :nick) (password) (authfn . ,fn)))
               (erc-networks--id (erc-networks--id-create 'GNU/chat)))
           (should (string= (erc-sasl--read-password nil) "spam"))
           (should (equal (pop calls) '(:user "bob" :host "GNU/chat")))))
 
       (ert-info ("Symbol as password, entry lacks user field")
         (let ((erc-server-current-nick "fake")
-              (erc-sasl--options '((user . :nick) (password . MyHost)))
+              (erc-sasl--options
+               `((user . :nick) (password . MyHost) (authfn . ,fn)))
               (erc-networks--id (erc-networks--id-create 'GNU/chat)))
           (should (string= (erc-sasl--read-password nil) "123"))
           (should (equal (pop calls) '(:user "fake" :host "MyHost"))))))))
-- 
2.38.1

From 2ac3d4eb39b53256edfe6ddba541da2c81d64fc1 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <[email protected]>
Date: Wed, 23 Nov 2022 21:31:19 -0800
Subject: [PATCH 2/3] * doc/misc/erc.texi: Revise SASL and modules chapters.

---
 doc/misc/erc.texi | 82 ++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 78 insertions(+), 4 deletions(-)

diff --git a/doc/misc/erc.texi b/doc/misc/erc.texi
index f86465fed7..5317a3e5aa 100644
--- a/doc/misc/erc.texi
+++ b/doc/misc/erc.texi
@@ -392,10 +392,14 @@ Modules
 There is a spiffy customize interface, which may be reached by typing
 @kbd{M-x customize-option @key{RET} erc-modules @key{RET}}.
 When removing a module outside of the Custom ecosystem, you may wish
-to ensure it's disabled by invoking its associated minor-mode toggle,
-such as @kbd{M-x erc-spelling-mode @key{RET}}.  Note that, these days,
-calling @code{erc-update-modules} in an init file is typically
-unnecessary.
+to ensure it's disabled by invoking its associated minor-mode toggle
+with a nonpositive prefix argument, for example, @kbd{C-u - M-x
+erc-spelling-mode @key{RET}}.  Additionally, if you plan on loading
+third-party modules that perform atypical setup on activation, you may
+need to arrange for calling @code{erc-update-modules} in your init
+file.  Examples of such setup might include registering an
+@code{erc-before-connect} hook, advising @code{erc-open}, and
+modifying @code{erc-modules} itself.
 
 The following is a list of available modules.
 
@@ -1082,6 +1086,76 @@ SASL
 leave this set to @code{nil}.
 @end defopt
 
+@subheading Examples
+
+@itemize @bullet
+@item
+Defaults
+
+@lisp
+(erc-tls :server "irc.libera.chat" :port 6697
+         :nick "aph"
+         :user "APHacker"
+         :password "changeme")
+@end lisp
+
+Here, after adding @code{sasl} to @code{erc-modules} via the Customize
+interface, you authenticate to Libera using the @samp{PLAIN} mechanism
+and your NickServ credentials, @samp{APHacker} and @samp{changeme}.
+
+@item
+External
+
+@lisp
+(setopt erc-sasl-mechanism 'external)
+
+(erc-tls :server "irc.libera.chat" :port 6697 :nick "aph"
+         :client-certificate
+         '("/home/aph/my.key" "/home/aph/my.crt"))
+@end lisp
+
+You decide to switch things up and try out the @samp{EXTERNAL}
+mechanism.  You follow your network's instructions for telling
+NickServ about your client-certificate's fingerprint, and you
+authenticate successfully.
+
+@item
+Multiple networks
+
+@example
+# ~/.authinfo.gpg
+
+machine irc.libera.chat key /home/aph/my.key cert /home/aph/my.crt
+machine Example.Net login alyssa password sEcReT
+machine Example.Net login aph-bot password sesame
+@end example
+
+@lisp
+;; init.el
+
+(defun my-erc-up (network)
+  (interactive "Snetwork: ")
+
+  (pcase network
+    ('libera
+     (let ((erc-sasl-mechanism 'external))
+       (erc-tls :server "irc.libera.chat" :port 6697
+                :client-certificate t)))
+    ('example
+     (let ((erc-sasl-auth-source-function #'erc-auth-source-search)
+           (erc-sasl-password 'Example.Net))
+       (erc-tls :server "irc.example.net" :port 6697
+                :user "alyssa")))))
+@end lisp
+
+You've started storing your credentials with auth-source and have
+decided to try SASL on another network as well.  But there's a catch:
+this network doesn't support @samp{EXTERNAL}.  You use
+@code{let}-binding to get around this and successfully authenticate to
+both networks.
+
+@end itemize
+
 @subheading Troubleshooting
 
 @strong{Warning:} ERC's SASL offering is currently limited by a lack
-- 
2.38.1

From 936c25dd844079edcc474a50cc82a1fca2b196f4 Mon Sep 17 00:00:00 2001
From: "F. Jason Park" <[email protected]>
Date: Wed, 23 Nov 2022 21:31:19 -0800
Subject: [PATCH 3/3] Simplify erc-sasl's auth-source API

* doc/misc/erc.texi: Revise descriptions in SASL chapter to reflect
simplified auth-source options.

* lisp/erc/erc-sasl.el (erc-sasl-password,
erc-sasl-auth-source-function): Revise doc strings.
(erc-sasl-auth-source-password-as-host): New function to serve as
more useful choice for option `erc-sasl-auth-source-function'.
(erc-sasl--read-password): Promote auth-source to poll position, above
an explicit string and `:password'.

* test/lisp/erc/erc-sasl-tests.el (erc-sasl--read-password--basic):
Massage tests to conform to simplified `erc-sasl-password' API.
---
 doc/misc/erc.texi               | 40 +++++++++++-------
 lisp/erc/erc-sasl.el            | 74 ++++++++++++++++++++-------------
 test/lisp/erc/erc-sasl-tests.el | 38 +++++++++++------
 3 files changed, 96 insertions(+), 56 deletions(-)

diff --git a/doc/misc/erc.texi b/doc/misc/erc.texi
index 5317a3e5aa..6f9656ca6b 100644
--- a/doc/misc/erc.texi
+++ b/doc/misc/erc.texi
@@ -1055,17 +1055,10 @@ SASL
 @code{erc-tls} with @code{:password} set to your NickServ password.
 
 You can also set this to a nonemtpy string, and ERC will send that
-when needed, no questions asked.  If you instead give a non-@code{nil}
-symbol (other than @code{:password}), like @samp{Libera.Chat}, ERC
-will use it for the @code{:host} field in an auth-source query.
-Actually, the same goes for when this option is @code{nil} but an
-explicit session ID is already on file (@pxref{Network Identifier}).
-For all such queries, ERC specifies the resolved value of
-@code{erc-sasl-user} for the @code{:user} (@code{:login}) param.  Keep
-in mind that none of this matters unless
-@code{erc-sasl-auth-source-function} holds a function, and it's
-@code{nil} by default.  As a last resort, ERC will prompt you for
-input.
+when needed, no questions asked.  There is one catch, though: if you
+set @code{erc-sasl-auth-source-function} to a function, ERC will
+perform an auth-source query instead.  As last resort in all cases,
+ERC will prompt you for input.
 
 Lastly, if your mechanism is @code{ecdsa-nist256p-challenge}, this
 option should instead hold the file name of your key.
@@ -1075,7 +1068,23 @@ SASL
 This is nearly identical to the other ERC @samp{auth-source} function
 options (@pxref{ERC auth-source functions}) except that the default
 value here is @code{nil}, meaning you have to set it to something like
-@code{erc-auth-source-search} for queries to be performed.
+@code{erc-auth-source-search} for queries to be performed.  For
+convenience, this module provides the following as a possible value:
+
+@defun erc-sasl-auth-source-password-as-host &rest plist
+Setting @code{erc-sasl-auth-source-function} to this function tells
+ERC to use @code{erc-sasl-password} for the @code{:host} field when
+querying auth-source, even if its value is the default
+@code{:password}, in which case ERC knows to ``resolve'' it to
+@code{erc-session-password} and use that as long as it's
+non-@code{nil}.  Otherwise, ERC just defers to
+@code{erc-auth-source-search} to determine the @code{:host}, along
+with everything else.
+@end defun
+
+Regardless, so long as this option specifies a function, ERC will pass
+it the ``resolved'' value of @code{erc-sasl-user} for the auth-source
+@code{:user} parameter.
 @end defopt
 
 @defopt erc-sasl-authzid
@@ -1142,10 +1151,11 @@ SASL
        (erc-tls :server "irc.libera.chat" :port 6697
                 :client-certificate t)))
     ('example
-     (let ((erc-sasl-auth-source-function #'erc-auth-source-search)
-           (erc-sasl-password 'Example.Net))
+     (let ((erc-sasl-auth-source-function
+            #'erc-sasl-auth-source-password-as-host))
        (erc-tls :server "irc.example.net" :port 6697
-                :user "alyssa")))))
+                :user "alyssa"
+                :password "Example.Net")))))
 @end lisp
 
 You've started storing your credentials with auth-source and have
diff --git a/lisp/erc/erc-sasl.el b/lisp/erc/erc-sasl.el
index 5ee7169de5..e149c94085 100644
--- a/lisp/erc/erc-sasl.el
+++ b/lisp/erc/erc-sasl.el
@@ -77,15 +77,14 @@ erc-sasl-user
 
 (defcustom erc-sasl-password :password
   "Optional account password to send when authenticating.
-When the value is a string, ERC will use it unconditionally for
-most mechanisms.  Likewise with `:password', except ERC will
-instead use the \"session password\" on file, which often
-originates from the entry-point commands `erc' or `erc-tls'.
-Otherwise, when `erc-sasl-auth-source-function' is a function,
-ERC will attempt an auth-source query, possibly using a non-nil
-symbol for the suggested `:host' parameter if set as this
-option's value or passed as an `:id' to `erc-tls'.  Failing that,
-ERC will prompt for input.
+When `erc-sasl-auth-source-function' is a function, ERC will
+attempt an auth-source query and prompt for input if it fails.
+Otherwise, when the value is a nonempty string, ERC will use it
+unconditionally for most mechanisms.  Likewise with `:password',
+except ERC will instead use the \"session password\" on file, if
+any, which often originates from the entry-point commands `erc'
+or `erc-tls'.  As with auth-source, ERC will prompt for input as
+a fallback.
 
 Note that, with `:password', ERC will forgo sending a traditional
 server password via the IRC \"PASS\" command.  Also, when
@@ -95,15 +94,18 @@ erc-sasl-password
 
 (defcustom erc-sasl-auth-source-function nil
   "Function to query auth-source for an SASL password.
-Called with keyword params known to `auth-source-search', which
-includes `erc-sasl-user' for the `:user' field and
-`erc-sasl-password' for the `:host' field, when the latter option
-is a non-nil, non-keyword symbol.  In return, ERC expects a
-string to send as the SASL password, or nil, to move on to the
-next approach, as described in the doc string for the option
-`erc-sasl-password'.  See info node `(erc) Connecting' for
-details on ERC's auth-source integration."
-  :type '(choice (function-item erc-auth-source-search)
+If provided, this function should expect to be called with any
+number of keyword params known to `auth-source-search', even
+though, as of ERC 5.5, these consists only of `:user' paired with
+a \"resolved\" `erc-sasl-user' value.  Additionally, all user
+options defined this library, such as `erc-sasl-password', are
+bound to their original values from module initialization.  In
+return, ERC expects a string to send as the SASL password, or
+nil, in which case, ERC will prompt the for input.  See info
+node `(erc) Connecting' for details on ERC's auth-source
+integration."
+  :type '(choice (function-item erc-sasl-auth-source-password-as-host)
+                 (function-item erc-auth-source-search)
                  (const nil)
                  function))
 
@@ -130,20 +132,34 @@ erc-sasl--get-user
     (:nick (erc-downcase (erc-current-nick)))
     (v v)))
 
+(defun erc-sasl-auth-source-password-as-host (&rest plist)
+  "Call `erc-auth-source-search' with `erc-sasl-password' as `:host'.
+But only do so when it's a string or a non-nil symbol, unless
+that symbol is `:password', in which case, use a non-nil
+`erc-session-password' instead.  Otherwise, just defer to
+`erc-auth-source-search' to pick a suitable `:host'."
+  (when erc-sasl-password
+    (when-let ((host (if (eq :password erc-sasl-password)
+                         (and (not (functionp erc-session-password))
+                              erc-session-password)
+                       erc-sasl-password)))
+      (setq plist `(,@plist :host ,(format "%s" host)))))
+  (apply #'erc-auth-source-search plist))
+
 (defun erc-sasl--read-password (prompt)
   "Return configured option or server password.
 PROMPT is passed to `read-passwd' if necessary."
-  (if-let
-      ((found (pcase (alist-get 'password erc-sasl--options)
-                (:password erc-session-password)
-                ((and (pred stringp) v) (unless (string-empty-p v) v))
-                ((and (let fn (alist-get 'authfn erc-sasl--options))
-                      (guard fn) v
-                      (let host
-                        (or v (erc-networks--id-given erc-networks--id))))
-                 (apply fn
-                        :user (erc-sasl--get-user)
-                        (and host (list :host (symbol-name host))))))))
+  (if-let ((found (pcase (alist-get 'password erc-sasl--options)
+                    ((guard (alist-get 'authfn erc-sasl--options))
+                     (let-alist erc-sasl--options
+                       (let ((erc-sasl-user .user)
+                             (erc-sasl-password .password)
+                             (erc-sasl-mechanism .mechanism)
+                             (erc-sasl-authzid .authzid)
+                             (erc-sasl-auth-source-function .authfn))
+                         (funcall .authfn :user (erc-sasl--get-user)))))
+                    (:password erc-session-password)
+                    ((and (pred stringp) v) (unless (string-empty-p v) v)))))
       (copy-sequence (erc--unfun found))
     (read-passwd prompt)))
 
diff --git a/test/lisp/erc/erc-sasl-tests.el b/test/lisp/erc/erc-sasl-tests.el
index a0e871979a..0e5ea60e5f 100644
--- a/test/lisp/erc/erc-sasl-tests.el
+++ b/test/lisp/erc/erc-sasl-tests.el
@@ -57,6 +57,8 @@ erc-sasl--read-password--basic
                          (erc-sasl--read-password "pwd:"))
                        "baz")))))
 
+;; This mainly tests `erc-sasl-auth-source-password-as-host'.
+
 (ert-deftest erc-sasl--read-password--auth-source ()
   (ert-with-temp-file netrc-file
     :text (string-join
@@ -70,33 +72,42 @@ erc-sasl--read-password--auth-source
            (erc-session-server "irc.gnu.org")
            (erc-session-port 6697)
            (erc-networks--id (erc-networks--id-create nil))
-           calls
-           (fn (lambda (&rest r)
-                 (push r calls)
-                 (apply #'erc--auth-source-search r)))
            erc-server-announced-name ; too early
-           auth-source-do-cache)
+           auth-source-do-cache
+           ;;
+           (fn #'erc-sasl-auth-source-password-as-host)
+           calls)
+
+      (advice-add 'erc-auth-source-search :before
+                  (lambda (&rest r) (push r calls))
+                  '((name . erc-sasl--read-password--auth-source)))
 
       (ert-info ("Symbol as password specifies machine")
         (let ((erc-sasl--options
-               `((user . "bob") (password . FSF.chat) (authfn . ,fn)))
-              (erc-networks--id (make-erc-networks--id)))
+               `((user . "bob") (password . FSF.chat) (authfn . ,fn))))
           (should (string= (erc-sasl--read-password nil) "sesame"))
           (should (equal (pop calls) '(:user "bob" :host "FSF.chat")))))
 
-      (ert-info ("ID for :host and `erc-session-username' for :user") ; *1
+      (ert-info (":password as password resolved to machine")
+        (let ((erc-session-password "FSF.chat")
+              (erc-sasl--options
+               `((user . "bob") (password . :password) (authfn . ,fn))))
+          (should (string= (erc-sasl--read-password nil) "sesame"))
+          (should (equal (pop calls) '(:user "bob" :host "FSF.chat")))))
+
+      (ert-info (":user resolved to `erc-session-username'") ; *1
         (let ((erc-session-username "bob")
               (erc-sasl--options `((user . :user) (password) (authfn . ,fn)))
               (erc-networks--id (erc-networks--id-create 'GNU/chat)))
           (should (string= (erc-sasl--read-password nil) "spam"))
-          (should (equal (pop calls) '(:user "bob" :host "GNU/chat")))))
+          (should (equal (pop calls) '(:user "bob")))))
 
-      (ert-info ("ID for :host and current nick for :user") ; *1
+      (ert-info (":user resolved to current nick") ; *1
         (let ((erc-server-current-nick "bob")
               (erc-sasl--options `((user . :nick) (password) (authfn . ,fn)))
               (erc-networks--id (erc-networks--id-create 'GNU/chat)))
           (should (string= (erc-sasl--read-password nil) "spam"))
-          (should (equal (pop calls) '(:user "bob" :host "GNU/chat")))))
+          (should (equal (pop calls) '(:user "bob")))))
 
       (ert-info ("Symbol as password, entry lacks user field")
         (let ((erc-server-current-nick "fake")
@@ -104,7 +115,10 @@ erc-sasl--read-password--auth-source
                `((user . :nick) (password . MyHost) (authfn . ,fn)))
               (erc-networks--id (erc-networks--id-create 'GNU/chat)))
           (should (string= (erc-sasl--read-password nil) "123"))
-          (should (equal (pop calls) '(:user "fake" :host "MyHost"))))))))
+          (should (equal (pop calls) '(:user "fake" :host "MyHost")))))
+
+      (advice-remove 'erc-auth-source-search
+                     'erc-sasl--read-password--auth-source))))
 
 (ert-deftest erc-sasl-create-client--plain ()
   (let* ((erc-session-password "password123")
-- 
2.38.1