bug#45340: erc-services.el: Auth-source support for passwords

[Amin Bandali <bandali@gnu.org>]

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

Hello Leon, Lars,

Thanks for your patch, Leon.  I've left some stylistic comments below,
until I get a chance to have a closer look at your patch and take it for
a test run in the next week or so. :-)

                                 * * *

Leon Vack writes:

>> Have you already started the assignment process, or do you need the
>> assignment form to get started?
>
> Yes, I have the form and started the assignment process with it. Thanks
> for asking.

Great.  Please let me know when you hear from the copyright clerk about
the completion of the process.

                                 * * *

Leon Vack writes:

> Adds an option to erc-services.el to query auth-source for NickServ
> passwords.
>
> This is my first patch to emacs, so please let me know if I missed any
> standards.  I am about to start the process of assigning the copyright
> to the FSF.

Thanks!

To begin with, please use spaces for indenting on the lines you change
or the new lines you add throughout your patch.

>>From 367593cc4cbfdfbdac778fd15ceb79fd61d7f64c Mon Sep 17 00:00:00 2001
> From: Leon Vack <dev@lgcl.de>
> Date: Sun, 20 Dec 2020 10:53:33 +0100
> Subject: [PATCH] erc-services.el: Auth-source support for passwords

Please rewrite the subject to conform to the conventions laid out in
emacs.git's CONTRIBUTE file.  Perhaps something like this:

Support using auth-source for NickServ passwords in ERC

> * lisp/etc/erc-services.el (erc-nickserv-passwords): Document that
> the passwords are only used when erc-prompt-for-nickserv-password is
> nil.
> * lisp/etc/erc-services.el
> (erc-use-auth-source-for-nickserv-password): New customizable
> variable to enable checking auth-source for NickServ passwords.
> * lisp/etc/erc-services.el (etc-nickserv-get-password): New function
> to handle the lookup of the NickServ password from both auth-source
> and the erc-nickserv-passwords variable.
> * lisp/etc/erc-services.el (erc-nickserv-call-identify-function):
> Use new erc-nickserv-get-password function to lookup NickServ
> passwords.
> * lisp/etc/erc-services.el (erc-nickserv-identify-autodetect)
> (erc-nickserv-identify-on-connect)
> (erc-nickserv-identify-on-nick-change): Call
> erc-nickserv-call-identify-function when
> erc-use-auth-source-for-nickserv-password is set.

You don't have to repeat the "* lisp/etc/erc-services.el" at the
beginning of each of those subsequent lines.  You can simply start those
lines with parenthesized name of the modified function/variable,
followed by a colon, followed by the change description.

Also, for the last entry, you can merge the multiple parenthesized names
into one, like so:

  (erc-nickserv-identify-autodetect, erc-nickserv-identify-on-connect,
  erc-nickserv-identify-on-nick-change): Call ......

Lastly, please add a simple line like "* etc/NEWS: Document change."
about the etc/NEWS update also.

> ---
>  etc/NEWS                 |  4 ++++
>  lisp/erc/erc-services.el | 50 ++++++++++++++++++++++++++++++++--------
>  2 files changed, 45 insertions(+), 9 deletions(-)
>
> diff --git a/etc/NEWS b/etc/NEWS
> index 4a8e70e6a6..c9e0fa7459 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -1283,6 +1283,10 @@ https://www.w3.org/TR/xml/#charsets).  Now it rejects such strings.
>  
>  ** erc
>  
> +*** erc-services.el now supports NickServ passwords from auth-source.
> +The 'erc-use-auth-source-for-nickserv-password' enables querying
> +auth-source for NickServ passwords.
> +
>  ---
>  *** The '/ignore' command will now ask for a timeout to stop ignoring the user.
>  Allowed inputs are seconds or ISO8601-like periods like "1h" or "4h30m".

Please consider adding example(s) of use to the NEWS entry.

> diff --git a/lisp/erc/erc-services.el b/lisp/erc/erc-services.el
> index c0011f9808..0710813e9d 100644
> --- a/lisp/erc/erc-services.el
> +++ b/lisp/erc/erc-services.el
> @@ -168,9 +168,23 @@ erc-prompt-for-nickserv-password
>    :group 'erc-services
>    :type 'boolean)
>  
> +(defcustom erc-use-auth-source-for-nickserv-password nil
> +  "Query auth-source for a password when identifiying to NickServ.
> +
> +
> +This option has an no effect `erc-prompt-for-nickserv-password'
                               ^
                               add missing 'if'
> +is not nil and passwords from `erc-nickserv-passwords' take
             ^
             add missing comma ','
> +precedence."
> +  :version "28.1"
> +  :group 'erc-services
> +  :type 'boolean)
> +
>  (defcustom erc-nickserv-passwords nil
>    "Passwords used when identifying to NickServ automatically.
>  
> +`erc-prompt-for-nickserv-password' must be nil for these
> +passwords to be used.
> +
>  Example of use:
>    (setq erc-nickserv-passwords
>          \\='((freenode ((\"nick-one\" . \"password\")
> @@ -199,7 +213,7 @@ erc-nickserv-passwords
>  			(cons :tag "Identity"
>  			      (string :tag "Nick")
>  			      (string :tag "Password"
> -                                      :secret ?*))))))
> +				      :secret ?*))))))
>  
>  ;; Variables:
>  
> @@ -375,7 +389,8 @@ erc-nickserv-identify-autodetect
>  If `erc-prompt-for-nickserv-password' is non-nil, prompt the user for the
>  password for this nickname, otherwise try to send it automatically."
>    (unless (and (null erc-nickserv-passwords)
> -	       (null erc-prompt-for-nickserv-password))
> +	       (null erc-prompt-for-nickserv-password)
> +	       (null erc-use-auth-source-for-nickserv-password))
>      (let* ((network (erc-network))
>  	   (sender (erc-nickserv-alist-sender network))
>  	   (identify-regex (erc-nickserv-alist-regexp network))
> @@ -394,7 +409,8 @@ erc-nickserv-identify-autodetect
>  (defun erc-nickserv-identify-on-connect (_server nick)
>    "Identify to Nickserv after the connection to the server is established."
>    (unless (or (and (null erc-nickserv-passwords)
> -		   (null erc-prompt-for-nickserv-password))
> +		   (null erc-prompt-for-nickserv-password)
> +		   (null erc-use-auth-source-for-nickserv-password))
>  	      (and (eq erc-nickserv-identify-mode 'both)
>  		   (erc-nickserv-alist-regexp (erc-network))))
>      (erc-nickserv-call-identify-function nick)))
> @@ -402,22 +418,38 @@ erc-nickserv-identify-on-connect
>  (defun erc-nickserv-identify-on-nick-change (nick _old-nick)
>    "Identify to Nickserv whenever your nick changes."
>    (unless (or (and (null erc-nickserv-passwords)
> -		   (null erc-prompt-for-nickserv-password))
> +		   (null erc-prompt-for-nickserv-password)
> +		   (null erc-use-auth-source-for-nickserv-password))
>  	      (and (eq erc-nickserv-identify-mode 'both)
>  		   (erc-nickserv-alist-regexp (erc-network))))
>      (erc-nickserv-call-identify-function nick)))
>  
> +(defun erc-nickserv-get-password (nickname)
> +  "Return the password for NICKNAME from configured sources.
> +
> +It uses `erc-nickserv-passwords' and additionally auth-source
> +when `erc-use-auth-source-for-nickserv-password' is not nil."
> +  (or (when erc-nickserv-passwords
> +	(cdr (assoc nickname

Strange formatting/indentation.  I would prefer it if
"(when erc-nickserv-passwords" started on its own line, and
"(cdr (assoc nickname" on the line below it, and the region
indented using spaces and properly, in a way that the lines
fit the 70-character line length convention.

> +		    (nth 1 (assoc (erc-network)
> +				  erc-nickserv-passwords)))))
> +      (when erc-use-auth-source-for-nickserv-password
> +	(let* ((secret (nth 0 (auth-source-search
> +			       :max 1 :require '(:secret)
> +			       :host (erc-with-server-buffer erc-session-server)
> +			       :port (format ; ensure we have a string
> +				      "%s" (erc-with-server-buffer erc-session-port))
> +			       :user nickname))))
> +	  (when secret (let ((passwd (plist-get secret :secret)))
> +			 (if (functionp passwd) (funcall passwd) passwd)))))))
> +
>  (defun erc-nickserv-call-identify-function (nickname)
>    "Call `erc-nickserv-identify'.
>  Either call it interactively or run it with NICKNAME's password,
>  depending on the value of `erc-prompt-for-nickserv-password'."
>    (if erc-prompt-for-nickserv-password
>        (call-interactively 'erc-nickserv-identify)
> -    (when erc-nickserv-passwords
> -      (erc-nickserv-identify
> -       (cdr (assoc nickname
> -		   (nth 1 (assoc (erc-network)
> -				 erc-nickserv-passwords))))))))
> +    (when password (erc-nickserv-identify (erc-nickserv-get-password nickname)))))

This line is too long; please reformat.

>  
>  (defvar erc-auto-discard-away)

                                 * * *

Thanks!

-- 
https://bndl.org
Free Software activist | GNU maintainer & webmaster
GPG: BE62 7373 8E61 6D6D 1B3A  08E8 A21A 0202 4881 6103

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]