GNU bug report logs - #46777
28.0.50; ERC: NickServ identification: Prompt for password after other sources, overall simplifications

Previous Next

Package: emacs;

Reported by: Olivier Certner <ocert.dev <at> free.fr>

Date: Thu, 25 Feb 2021 17:35:01 UTC

Severity: normal

Tags: patch

Found in version 28.0.50

Done: Amin Bandali <bandali <at> gnu.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 46777 in the body.
You can then email your comments to 46777 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#46777; Package emacs. (Thu, 25 Feb 2021 17:35:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Olivier Certner <ocert.dev <at> free.fr>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Thu, 25 Feb 2021 17:35:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Olivier Certner <ocert.dev <at> free.fr>
To: bug-gnu-emacs <at> gnu.org
Subject: 28.0.50; ERC: NickServ identification: Prompt for password after other
 sources, overall simplifications
Date: Thu, 25 Feb 2021 18:33:53 +0100
When `erc-prompt-for-nickserv-password' is true, don't ignore the
other forms of identification.  Instead, process them first, and
prompt for the password last.  Separate concerns (determination of the
nick to use, of the password to use, and actual message sending).

Note that the user can be interactively prompted for a password on
reception of a Nickserv request, as before (on
`erc-prompt-for-nickserv-password').

This is a follow-up to #45340 (see end of discussion there for additional 
context).

Pull request with the single commit to be posted after the bug is open. 
Changes rebased on master.

-- 
Olivier Certner






Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46777; Package emacs. (Thu, 25 Feb 2021 17:39:01 GMT) Full text and rfc822 format available.

Message #8 received at 46777 <at> debbugs.gnu.org (full text, mbox):

From: Olivier Certner <ocert.dev <at> free.fr>
To: 46777 <at> debbugs.gnu.org
Subject: 28.0.50; ERC: NickServ identification: Prompt for password after other
 sources, overall simplifications
Date: Thu, 25 Feb 2021 18:38:45 +0100
Corresponding "pull request" (just for showing the diff):
https://github.com/OlCe2/emacs/pull/2

-- 
Olivier Certner






Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46777; Package emacs. (Thu, 25 Feb 2021 17:48:01 GMT) Full text and rfc822 format available.

Message #11 received at 46777 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Olivier Certner <ocert.dev <at> free.fr>
Cc: 46777 <at> debbugs.gnu.org
Subject: Re: bug#46777: 28.0.50; ERC: NickServ identification: Prompt for
 password after other sources, overall simplifications
Date: Thu, 25 Feb 2021 17:47:31 +0000
Olivier Certner <ocert.dev <at> free.fr> writes:

> Corresponding "pull request" (just for showing the diff):
> https://github.com/OlCe2/emacs/pull/2

Thanks, but I think it's generally easier/preferred to review patches
here if you send them as attachments.  See the file CONTRIBUTE.

-- 
Basil




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46777; Package emacs. (Thu, 25 Feb 2021 18:23:01 GMT) Full text and rfc822 format available.

Message #14 received at 46777 <at> debbugs.gnu.org (full text, mbox):

From: Olivier Certner <ocert.dev <at> free.fr>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: 46777 <at> debbugs.gnu.org
Subject: Re: bug#46777: 28.0.50;
 ERC: NickServ identification: Prompt for password after other
 sources, overall simplifications
Date: Thu, 25 Feb 2021 19:22:04 +0100
[Message part 1 (text/plain, inline)]
Sure. Attached.

-- 
Olivier Certner
[0001-NickServ-identification-Prompt-for-password-last-ove.patch (text/x-patch, attachment)]

Added tag(s) patch. Request was from Lars Ingebrigtsen <larsi <at> gnus.org> to control <at> debbugs.gnu.org. (Fri, 26 Feb 2021 09:24:01 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46777; Package emacs. (Tue, 08 Jun 2021 02:24:02 GMT) Full text and rfc822 format available.

Message #19 received at 46777 <at> debbugs.gnu.org (full text, mbox):

From: "J.P." <jp <at> neverwas.me>
To: Olivier Certner <ocert.dev <at> free.fr>
Cc: emacs-erc <at> gnu.org, 46777 <at> debbugs.gnu.org
Subject: Re: bug#46777: 28.0.50; ERC: NickServ identification: Prompt for
 password after other sources, overall simplifications
Date: Mon, 07 Jun 2021 19:23:39 -0700
Hi Olivier,

I'm sure you know this, but for others, both files have changed since
this patch was created. It applies cleanly for me atop this commit:

  297c0e0306f111c1e7564b2bb49a7e1a925a55bb

Okay, the layout of this module is a bit confusing to me. Perhaps that's
because it's regarded as an entry point of some kind and therefore
special? In the commentary, it says to enable it explicitly using the
minor mode interface (rather than add it to erc-modules). However, I
notice that the :set function for the option erc-nickserv-identify-mode
goes ahead and activates it in all the ways that matter (but the minor-
mode variable).

So no matter what, the function of the same name is called, which then
sets the same custom option (possibly again). I guess that's what that
comment about avoiding "recursive load at startup" is all about? For
now, I'll just assume something fancy's going on I don't yet comprehend.
(Maybe I'll check the old mailing list for clues.)

In general, I'm somewhat inclined to regard this module as nonessential
and legacy focused because it's not loaded by default and because these
days, things seem to be trending toward fewer interactions with nick
services beyond initial setup (where manual piloting is required
anyway). However, I think this module receives a fair amount of
attention on #emacs and elsewhere, so we might as well abide. Because I
don't use it myself, I'll spare you any dubious hands on feedback and
stick to the self-interested stuff affecting those improvements I'd like
to see in ERC for this coming release.

So, despite its specialness, I'm rather confident this module and your
changes to it will be spared the brunt of the library-wide modifications
I have in store. Basically, this would be a reorienting of ERC's notion
of connection identities toward a more network-centric view.

This module already depends on erc-networks, which is good. This means
most of what I'll be tweaking will be auth-source related. But I won't
touch any options concerning the when and the why of it all, which is
what you and Leon have addressed. I'll instead likely only be messing
with the arguments to the one auth-source-search invocation. If you're
interested in details, please follow bug #48598.

A couple specifics. In erc-nickserv-get-password,

    (erc-with-server-buffer
     (setq network erc-network
                   ^~~~~~~~~~~~
           server erc-session-server
           port erc-session-port))

would you mind using the function form of erc-network instead? I'm
focusing a lot on that one symbol in particular, and it'd be nice to
keep things consistent for now, if it's all the same to you.

My other note concerns erc-nickserv-identify. Assuming debug-on-error is
nil, it looks like this dings whenever erc-nickserv-get-password comes
up empty, which I guess can only happen when the three main
password-related user options are all nil (or the prompt gets
dismissed).

So, worst case scenario, people get dinged a few times straight away:
maybe once just after MOTD and once just before, in the case of an
initial re-NICK, and maybe again from a "please identify"/"nick taken"
NOTICE. But being Emacs users, they'd know to check *Messages* for
details (is that the idea?). If there's a realistic chance of a more
intense onslaught, I suppose one alternative might be to print something
to the active buffer using erc-display-error-notice instead. But you
know better than I, having actually used this.

Not sure if you're aware, but there's a bit of an integration going on
between erc-join.el and this module via erc-nickserv-identified-hook.
The autojoin module is pretty confusing, and my current bug addresses
some of that. My question for you is: do networks punish folks for
repeated failed JOIN attempts while unauthed? IOW, any clue whether
major IRCds or service daemons auto-TKLINE (or similar) for such
behavior? If there *is* a risk, I'd rather fix things on the autojoin
side because inhibiting timers during read-passwd would affect PONGs and
the outgoing flood queue, etc.

BTW, have you considered maybe generalizing this entire module (while
preserving the interface) to make it work with *any* services bot, e.g.,
FooServ, and not just nick-related stuff?

So, yeah, now comes the part where I admit to not having actually fired
this up and put it through its paces. But I'm certainly willing to if
you'd like the extra peace of mind. Shouldn't take more than a couple
days (i.e., nanoseconds to the yogi you've surely become by now). Let me
know, and thanks.

J.P.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46777; Package emacs. (Wed, 09 Jun 2021 13:32:01 GMT) Full text and rfc822 format available.

Message #22 received at 46777 <at> debbugs.gnu.org (full text, mbox):

From: Olivier Certner <ocert.dev <at> free.fr>
To: "J.P." <jp <at> neverwas.me>
Cc: emacs-erc <at> gnu.org, 46777 <at> debbugs.gnu.org
Subject: Re: bug#46777: 28.0.50;
 ERC: NickServ identification: Prompt for password after other
 sources, overall simplifications
Date: Wed, 09 Jun 2021 15:30:55 +0200
Hi,

> I'm sure you know this, but for others, both files have changed since
> this patch was created. It applies cleanly for me atop this commit: 
>   297c0e0306f111c1e7564b2bb49a7e1a925a55bb

I didn't check for a long time because of the legal part taking so long (more 
than 7 months, I opted for public domain first but switched to regular 
copyright assignment thanks to the grant back) and because I'm maintaining 
private patches over Emacs 27.1.

I have very limited time at the moment, so I'm sure I'm not going to be able 
to look at it before next week, and perhaps even before next month. Are you 
(or is someone) aware of the expected release cycle for 28.1?

> Okay, the layout of this module is a bit confusing to me. Perhaps that's
> because it's regarded as an entry point of some kind and therefore
> special? In the commentary, it says to enable it explicitly using the
> minor mode interface (rather than add it to erc-modules). However, I
> notice that the :set function for the option erc-nickserv-identify-mode
> goes ahead and activates it in all the ways that matter (but the minor-
> mode variable).

Didn't dig that, but can tell you "services" indeed works when added to `erc-
modules', since this is how I'm using it. If I don't put it there, no autojoin 
happens when NickServ responds.

> In general, I'm somewhat inclined to regard this module as nonessential
> and legacy focused because it's not loaded by default and because these
> days, things seem to be trending toward fewer interactions with nick
> services beyond initial setup (where manual piloting is required
> anyway). However, I think this module receives a fair amount of
> attention on #emacs and elsewhere, so we might as well abide. Because I
> don't use it myself, I'll spare you any dubious hands on feedback and
> stick to the self-interested stuff affecting those improvements I'd like
> to see in ERC for this coming release.

I've seen you sent a big mail entitled "buffer-naming collisions involving 
bouncers in ERC" but only had time for a quick glance.

As for interaction with NickServ, every new direct connection to servers need 
to authenticate with NickServ when using a registered nick. I don't know how 
bouncers work, but I suspect they do authentication differently, so I suspect 
your view is that of a user of bouncers.

Architecturally, I don't know (yet) if having this module separate is a good 
thing going ahead, but on the other hand I think the need for auto-joining is 
very real (again, may be wrong with bouncers; do they automatically forward 
all messages from all channels they are in to clients connecting to them? or 
is there a specific mechanism to obtain the messages from specific channels?).
And autojoining cannot work effectively if users are not automatically 
identified before (when using registered nicks). So the module/architecture 
may be obsolete, but I don't think the needs themselves are.
 
> This module already depends on erc-networks, which is good. This means
> most of what I'll be tweaking will be auth-source related. But I won't
> touch any options concerning the when and the why of it all, which is
> what you and Leon have addressed. I'll instead likely only be messing
> with the arguments to the one auth-source-search invocation. If you're
> interested in details, please follow bug #48598.

Given my limited time at the moment, yes, it would be best that your changes 
are quite small if you want me to review them.

I'll follow up with #48598 when I can.

So here's a first proposal:
1. I rebase the changes on current master.
2. We address what needs to be addressed with respect to your other patches.

Provided, indeed, that I have time to do it quickly enough for 28.1's release. 
If not we'll have to find another plan.
 
> A couple specifics. In erc-nickserv-get-password,
> 
>     (erc-with-server-buffer
>      (setq network erc-network
>                    ^~~~~~~~~~~~
>            server erc-session-server
>            port erc-session-port))
> 
> would you mind using the function form of erc-network instead? I'm
> focusing a lot on that one symbol in particular, and it'd be nice to
> keep things consistent for now, if it's all the same to you.

In principle I don't mind. But I also prefer simplicity as much as possible. 
If I chose this, it's probably because using the function had no added value. 
If the buffer-local variable and the function are indeed going to differ 
(hopefully you have good reasons for this) in your plans, of course we can 
switch.

In ERC in general, I've found that there are too many buffer-local variables 
and accessor functions in the sense that they are redundant, that you don't 
necessarily know in which buffer to look for which variable (current buffer? 
server buffer? other?), and that multiple variables seem to contain 
approximately the same information (but not exactly, that would be too 
simple). Proper accessors could solve part of these problems (doing that with 
a minimal amount of buffer switching is more work, but can wait for later).

> My other note concerns erc-nickserv-identify. Assuming debug-on-error is
> nil, it looks like this dings whenever erc-nickserv-get-password comes
> up empty, which I guess can only happen when the three main
> password-related user options are all nil (or the prompt gets
> dismissed).
> 
> So, worst case scenario, people get dinged a few times straight away:
> maybe once just after MOTD and once just before, in the case of an
> initial re-NICK, and maybe again from a "please identify"/"nick taken"
> NOTICE. But being Emacs users, they'd know to check *Messages* for
> details (is that the idea?). If there's a realistic chance of a more
> intense onslaught, I suppose one alternative might be to print something
> to the active buffer using erc-display-error-notice instead. But you
> know better than I, having actually used this.

I get prompted once only (in fact, now not at all, since I use auth-source). I 
never re-NICK. What are re-NICKs used for? To wear cloaks after regular log 
in? In this case, I assume NickServ authentication is needed for logging in, 
but not to switch to the cloak? Sorry, I'm not yet very versed in the 
subtleties of IRC, contrary to what you may think.

I don't think a priori that there is a problem in dinging the user per se, 
even a few times. The problem is rather whether the process should take place 
or not (i.e., should NickServ identification happen in the first place? see 
questions on re-NICK above) and when (dinging is OK if close to an event 
triggered by the user, which if I remember correctly is the case now except 
for delayed autojoining, if the delay is too long).

> Not sure if you're aware, but there's a bit of an integration going on
> between erc-join.el and this module via erc-nickserv-identified-hook.
> The autojoin module is pretty confusing, and my current bug addresses
> some of that. My question for you is: do networks punish folks for
> repeated failed JOIN attempts while unauthed? IOW, any clue whether
> major IRCds or service daemons auto-TKLINE (or similar) for such
> behavior? If there *is* a risk, I'd rather fix things on the autojoin
> side because inhibiting timers during read-passwd would affect PONGs and
> the outgoing flood queue, etc.

Not sure what you are referring to here. Maybe after fully reading bugs and 
mails, things will get clear, but for now they are rather obscure. Trying 
anyway:
1. I can't answer your question about being banned if attempting too much to 
join while not authenticated. But why is this even a problem? In which case 
are we going to repeatedly join some channel after a failed attempt?
2. `read-passwd' doesn't inhibit timers (AFAIK). Why would you want to inhibit 
timers there? Having a time out in `erc-nickserv-identify' would be great yes. 
Is this what you are talking about?

> BTW, have you considered maybe generalizing this entire module (while
> preserving the interface) to make it work with *any* services bot, e.g.,
> FooServ, and not just nick-related stuff?

Not at this point, because I don't know IRC enough, and I'm not using other 
services bot. What would doing this buy us? I can't answer that now.

That said, I think we (at least, I) should rather focus on needs and 
mechanisms (e.g., such as authentication), and then deal with actual 
implementations, possibly involving bots (e.g., NickServ for auth).

-- 
Olivier Certner






Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46777; Package emacs. (Thu, 10 Jun 2021 04:01:02 GMT) Full text and rfc822 format available.

Message #25 received at 46777 <at> debbugs.gnu.org (full text, mbox):

From: "J.P." <jp <at> neverwas.me>
To: Olivier Certner <ocert.dev <at> free.fr>
Cc: emacs-erc <at> gnu.org, 46777 <at> debbugs.gnu.org
Subject: Re: bug#46777: 28.0.50; ERC: NickServ identification: Prompt for
 password after other sources, overall simplifications
Date: Wed, 09 Jun 2021 20:59:54 -0700
> I have very limited time at the moment, so I'm sure I'm not going to be able 
> to look at it before next week, and perhaps even before next month. Are you 
> (or is someone) aware of the expected release cycle for 28.1?

Thanks for the time window. It would be nice to get the patches you have
open into ERC before the next release, but I'm clueless as to when
that's slated to occur.

> Didn't dig that, but can tell you "services" indeed works when added to `erc-
> modules', since this is how I'm using it. If I don't put it there, no autojoin 
> happens when NickServ responds.

I wasn't worried about it not supporting the modules interface. I was
concerned that setting `erc-nickserv-identify-mode' in a config via
`set-variable' or similar would effectively turn it "on" by registering
its hooks even though `erc-services-mode' (the variable) would still be
nil (because this module isn't loaded by default). Regardless, shouldn't
we try to keep those in sync? If not, and module-based minor modes
shouldn't be used to detect whether a module is active and its features
enabled, then we've got to fix that in the code (right?), perhaps
starting with

  (unless erc-networks-mode
    ;; Force-enable networks module, because we need it to set
    ;; erc-network for us.
    (erc-networks-enable))

in (the function) `erc-nickserv-identify-mode' itself.

> I've seen you sent a big mail entitled "buffer-naming collisions involving 
> bouncers in ERC" but only had time for a quick glance.
>
> As for interaction with NickServ, every new direct connection to servers need 
> to authenticate with NickServ when using a registered nick. I don't know how 
> bouncers work, but I suspect they do authentication differently, so I suspect 
> your view is that of a user of bouncers.

I should apologize for not de-emphasizing "bouncer" in reference to my
concerns being influenced by that bug (#48598), which for me is merely a
convenient means of addressing larger fundamental problems in ERC.

> Architecturally, I don't know (yet) if having this module separate is a good 
> thing going ahead, but on the other hand I think the need for auto-joining is 
> very real (again, may be wrong with bouncers; do they automatically forward 
> all messages from all channels they are in to clients connecting to them? or 
> is there a specific mechanism to obtain the messages from specific channels?).
> And autojoining cannot work effectively if users are not automatically 
> identified before (when using registered nicks). So the module/architecture 
> may be obsolete, but I don't think the needs themselves are.

The real problem (to my knowledge) is that there's no consensus around
how an IRC server should tell a client it's been authenticated (work may
be ongoing in this area). For now though, since there's no formal
concept of "accounts," you're either granted the nick you requested
during opening introductions or you're not.

How one goes about creating the conditions for such "granting" to occur
successfully depends on the various methods available to the client and
the server. Extensions like SASL support (part of the v3 initiative) and
Cert FP, which uses client certificates, are two examples of methods
currently employed by networks and servers to address this.

Personally, I wouldn't like to see this module loaded by default unless
we can state confidently that the way in which it goes about solving
this problem, namely engaging nick services with heuristics-driven,
TCL-expect style exchanges (which is fine) is the right thing for most
users (it very well may be).

More on this general topic of what determines a session in my bug's
thread (#48598).

> Given my limited time at the moment, yes, it would be best that your changes 
> are quite small if you want me to review them.

For this module, that's looking likely.

> I'll follow up with #48598 when I can.
>
> So here's a first proposal:
> 1. I rebase the changes on current master.
> 2. We address what needs to be addressed with respect to your other patches.

Step 2 isn't really necessary unless you feel up to it. I'm fine with
just dropping it rather than having us waste time coordinating around
something that's still mostly fluid.

>> A couple specifics. In erc-nickserv-get-password,
>> [...]
>> would you mind using the function form of erc-network instead?

Don't bother with this. I shouldn't have brought it up.

> In ERC in general, I've found that there are too many buffer-local variables 
> and accessor functions in the sense that they are redundant, that you don't 
> necessarily know in which buffer to look for which variable (current buffer? 
> server buffer? other?), and that multiple variables seem to contain 
> approximately the same information (but not exactly, that would be too 
> simple). Proper accessors could solve part of these problems (doing that with 
> a minimal amount of buffer switching is more work, but can wait for later).

We definitely agree on this point. At the moment though, I'm trying to
resist refactoring in this area in full. Instead, I'd like to turn this
corner in stages, the first adding whatever's necessary to tackle #48598
(which again, has to do with much more than just bouncers).

> I get prompted once only (in fact, now not at all, since I use auth-source). I 
> never re-NICK. What are re-NICKs used for? To wear cloaks after regular log 
> in? In this case, I assume NickServ authentication is needed for logging in, 
> but not to switch to the cloak? Sorry, I'm not yet very versed in the 
> subtleties of IRC, contrary to what you may think.

By re-NICK, I meant the server sending you a

  :you`!~you <at> yours NICK :you

once you've been authenticated. Do you not get these?

In terms of dinging, I wasn't really referring to your personal
experience but rather a worst case for an unlucky user. As long as the
error message and how it's displayed is sufficient for getting someone
on their way toward fixing the issue, then fine by me.

> 1. I can't answer your question about being banned if attempting too much to 
> join while not authenticated. But why is this even a problem? In which case 
> are we going to repeatedly join some channel after a failed attempt?

Don't worry about the ban thing. This was based on my carelessly missing
the fact that `erc-nickserv-identified-hook' only runs after you're
successfully authenticated. For whatever reason, I thought it ran on 376
when `erc-nickserv-identify-on-connect' fires, but that's not the case.

Currently, the first autojoin timer is only set on 376 and fires after
30 seconds. In the most unlikely scenario, an unlucky user with no auth
source and no options configured and who can't get their act together
before the timer runs will only log a single failed attempt. Clearly not
something to worry about. I doubt any public network would consider that
an infraction.

Also fueling my original concern was this phenomenon we've been seeing
of various networks enacting stricter policies for some IP ranges and
individuals. That this would somehow involve attempts to use the network
before being auth'd was unfounded speculation on my part.

> 2. `read-passwd' doesn't inhibit timers (AFAIK). Why would you want to inhibit 
> timers there? Having a time out in `erc-nickserv-identify' would be great yes. 
> Is this what you are talking about?

Sorry, that wasn't clear. I didn't mean to imply `read-passwd' inhibited
timers while running. I meant we shouldn't do that as a sad workaround
for postponing JOIN timers.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46777; Package emacs. (Tue, 06 Jul 2021 14:59:04 GMT) Full text and rfc822 format available.

Message #28 received at 46777 <at> debbugs.gnu.org (full text, mbox):

From: Olivier Certner <olce.emacs <at> certner.fr>
To: 46777 <at> debbugs.gnu.org
Subject: Updated patch
Date: Tue, 06 Jul 2021 16:52:06 +0200
[Message part 1 (text/plain, inline)]
Patch slightly edited (commit message, doc text) and rebased on top of a 
recent 'master'. Ready to apply. Easy to backport to 27 as well.

-- 
Olivier Certner
[0001-ERC-NickServ-Prompt-for-password-last-overall-simpli.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46777; Package emacs. (Thu, 22 Jul 2021 12:31:01 GMT) Full text and rfc822 format available.

Message #31 received at 46777 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Olivier Certner <olce.emacs <at> certner.fr>
Cc: Amin Bandali <bandali <at> gnu.org>, 46777 <at> debbugs.gnu.org
Subject: Re: bug#46777: 28.0.50; ERC: NickServ identification: Prompt for
 password after other sources, overall simplifications
Date: Thu, 22 Jul 2021 14:29:56 +0200
Olivier Certner <olce.emacs <at> certner.fr> writes:

> Patch slightly edited (commit message, doc text) and rebased on top of a 
> recent 'master'. Ready to apply. Easy to backport to 27 as well.

Amin, have you found time to look at this patch from Olivier (and the
other three (I think) patches)?  If you can't find the time, I can
examine them and get them applied.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46777; Package emacs. (Mon, 26 Jul 2021 07:40:01 GMT) Full text and rfc822 format available.

Message #34 received at 46777 <at> debbugs.gnu.org (full text, mbox):

From: "J.P." <jp <at> neverwas.me>
To: Olivier Certner <olce.emacs <at> certner.fr>
Cc: emacs-erc <at> gnu.org, 46777 <at> debbugs.gnu.org
Subject: Re: bug#46777: 28.0.50; ERC: NickServ identification: Prompt for
 password after other sources, overall simplifications
Date: Mon, 26 Jul 2021 00:39:39 -0700
Olivier Certner <olce.emacs <at> certner.fr> writes:

> * lisp/erc/erc-services.el (erc-nickserv-identify): Don't take the
> password anymore as an argument (and don't prompt for it
> interactively).  On the contrary, now take the nick to use for
> identification (interactively, ask for it, defaulting to the current
> one).  Move actual message sending into the new
> `erc-nickserv-send-identify', and password prompting into
> `erc-nickserv-get-password'.
>
> [...]
>
> -;;;###autoload
> -(defun erc-nickserv-identify (password)
> [...]
> +;;;###autoload
> +(defun erc-nickserv-identify (&optional nick)
> +  "Identify to NickServ immediately.

Hi Olivier, this concerns your changes to the autoloaded command
`erc-nickserv-identify'. Someone seeking help for a loosely related
issue shared this on Libera today:

  (defun uh-erc-identify ()
    (let ((pass
           (let ((s (shell-command-to-string "pass malc <at> irc.libera.chat")))
             (substring s 0 -1))))
      (erc-nickserv-identify pass)))

It strikes me that other folks may be doing the same, namely, using this
function in lisp code. So just to settle some imaginary nerves, it might
help to flesh out exactly why there's little risk of something
unfortunate/scary happening as a result of this interface change.

As I see it, the worst case scenario is that your password is sent in
place of your nick and ends up in the network's logs (where passwords
are normally redacted). If that's a realistic concern, perhaps you
should consider keeping the password param in its original position
for the sake of compatibility. Just a thought.

Also, I think the addition of the nick param bears some justifying too
because it may not be clear from your commit log and emails (or #45340)
why this is necessary. For me, the param is needed because some IRCds
and services daemons won't let you auth by passing your currently
assigned (perhaps unregistered) nick. In other words:

  PRIVMSG NickServ :IDENTIFY bob` mypass

or

  PRIVMSG NickServ :IDENTIFY mypass

won't work if you're trying to identify as "bob" and are currently
"bob`". But without the nick param you've added, a user would be forced
to issue the IRC command manually. Hope that's clear.

BTW, I've noticed that many services automatically re-NICK you as
confirmation of success, triggering `erc-nick-changed-functions', which
`erc-nickserv-identify-on-nick-change' is a member of by default. So you
get prompted again for no reason. (As you may be aware, double prompting
has also come up recently in the ERC channel, but I suspect the two are
unrelated.)

Anyway, I doubt you'll recall, but I tried (rather unconvincingly) to
raise this point earlier in this bug thread. If addressing this isn't in
the cards, then hopefully we can get a ruling on this patch and start
putting the days of scraping NickServ replies behind us (and begin
embracing the evolving standard). Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46777; Package emacs. (Fri, 30 Jul 2021 12:28:01 GMT) Full text and rfc822 format available.

Message #37 received at 46777 <at> debbugs.gnu.org (full text, mbox):

From: Amin Bandali <bandali <at> gnu.org>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: Olivier Certner <olce.emacs <at> certner.fr>, 46777 <at> debbugs.gnu.org
Subject: Re: bug#46777: 28.0.50; ERC: NickServ identification: Prompt for
 password after other sources, overall simplifications
Date: Fri, 30 Jul 2021 08:27:26 -0400
Hi Lars, all,

Lars Ingebrigtsen writes:

> Olivier Certner <olce.emacs <at> certner.fr> writes:
>
>> Patch slightly edited (commit message, doc text) and rebased on top of a 
>> recent 'master'. Ready to apply. Easy to backport to 27 as well.
>
> Amin, have you found time to look at this patch from Olivier (and the
> other three (I think) patches)?  If you can't find the time, I can
> examine them and get them applied.

Apologies for my slow reply and absence.  I haven't been well for a few
weeks, and didn't have the time and energy to do much outside of work.
Thankfully I am better now, and expect to be able to review Olivier's
patches and hopefully get them applied in the coming days, starting
tomorrow.  If not, I'll let you all know so you could review and apply
them, thanks!




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46777; Package emacs. (Fri, 30 Jul 2021 12:44:02 GMT) Full text and rfc822 format available.

Message #40 received at 46777 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Amin Bandali <bandali <at> gnu.org>
Cc: Olivier Certner <olce.emacs <at> certner.fr>, 46777 <at> debbugs.gnu.org
Subject: Re: bug#46777: 28.0.50; ERC: NickServ identification: Prompt for
 password after other sources, overall simplifications
Date: Fri, 30 Jul 2021 14:43:46 +0200
Amin Bandali <bandali <at> gnu.org> writes:

> Apologies for my slow reply and absence.  I haven't been well for a few
> weeks, and didn't have the time and energy to do much outside of work.
> Thankfully I am better now, and expect to be able to review Olivier's
> patches and hopefully get them applied in the coming days, starting
> tomorrow.  If not, I'll let you all know so you could review and apply
> them, thanks!

Great!  (Well, not great that you haven't been feeling well, but that
you're better.  :-))

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46777; Package emacs. (Tue, 14 Sep 2021 09:22:02 GMT) Full text and rfc822 format available.

Message #43 received at 46777 <at> debbugs.gnu.org (full text, mbox):

From: "J.P." <jp <at> neverwas.me>
To: Olivier Certner <olce.emacs <at> certner.fr>
Cc: Amin Bandali <bandali <at> gnu.org>, emacs-erc <at> gnu.org, 46777 <at> debbugs.gnu.org
Subject: Re: bug#46777: 28.0.50; ERC: NickServ identification: Prompt for
 password after other sources, overall simplifications
Date: Tue, 14 Sep 2021 02:20:55 -0700
[Message part 1 (text/plain, inline)]
"J.P." <jp <at> neverwas.me> writes:

> If that's a realistic concern, perhaps you should consider keeping the
> password param in its original position for the sake of compatibility.

Recent discussions have left me with the impression that at least one
consequential voice may be harboring reservations about the proposed
interface change to `erc-nickserv-identify'. I'd like to offer some
thoughts towards salvaging (at a minimum) the "prompting as a fallback"
portion of this patch, which I feel may be helpful to new users.

As a side note, the few changes that appear motivated mostly by
promoting a separation of concerns seem at worst benign and otherwise
potentially beneficial beyond maintenance matters (IMO). For example,
adding a helper, like the proposed `erc-nickserv-send-identify', to
encapsulate the actual request-making may improve this module's utility
as a traditional library. (A few bots may even appreciate that.)

Regarding the perceived minor bone of contention, my instincts favor the
bold move, as usual. But that's contingent on all things being equal,
which they likely aren't here (because passwords), meaning progressive
attitudes may have to take a back seat. So as a compromise, how about an
ugly chimera like the one in the attached example?

[0000-NOT-A-PATCH-discussion-example.diff (text/x-patch, attachment)]
[0001-ERC-NickServ-Prompt-for-password-last-overall-simpli.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46777; Package emacs. (Thu, 16 Sep 2021 05:31:01 GMT) Full text and rfc822 format available.

Message #46 received at 46777 <at> debbugs.gnu.org (full text, mbox):

From: Amin Bandali <bandali <at> gnu.org>
To: "J.P." <jp <at> neverwas.me>, Olivier Certner <olce.emacs <at> certner.fr>, Lars
 Ingebrigtsen <larsi <at> gnus.org>
Cc: emacs-erc <at> gnu.org, 46777 <at> debbugs.gnu.org
Subject: Re: bug#46777: 28.0.50; ERC: NickServ identification: Prompt for
 password after other sources, overall simplifications
Date: Thu, 16 Sep 2021 01:30:18 -0400
Hi all,

J.P. writes:

> "J.P." <jp <at> neverwas.me> writes:
>
>> If that's a realistic concern, perhaps you should consider keeping the
>> password param in its original position for the sake of compatibility.
>
> Recent discussions have left me with the impression that at least one
> consequential voice may be harboring reservations about the proposed
> interface change to `erc-nickserv-identify'. I'd like to offer some
> thoughts towards salvaging (at a minimum) the "prompting as a fallback"
> portion of this patch, which I feel may be helpful to new users.
>
> As a side note, the few changes that appear motivated mostly by
> promoting a separation of concerns seem at worst benign and otherwise
> potentially beneficial beyond maintenance matters (IMO). For example,
> adding a helper, like the proposed `erc-nickserv-send-identify', to
> encapsulate the actual request-making may improve this module's utility
> as a traditional library. (A few bots may even appreciate that.)
>
> Regarding the perceived minor bone of contention, my instincts favor the
> bold move, as usual. But that's contingent on all things being equal,
> which they likely aren't here (because passwords), meaning progressive
> attitudes may have to take a back seat. So as a compromise, how about an
> ugly chimera like the one in the attached example?

After a closer look, I am indeed hesitant about merging the original
patch, since it changes the interface of `erc-nickserv-identify' -- a
function as old as time, as far as ERC is concerned, and likely in use
by many -- especially since it deals with passwords.  Instead, I would
prefer one of the following approaches:

1. change the function's arguments in a backwards-compatible way
   (i.e. by adding new &optional args as needed), and ignore the
   arg we don't need anymore (the password in this case, and
   make sure we don't accidentally expose it instead of a nick);

2. gradually phase out the function if we must: either declare it
   obsolete and make it a wrapper around a new function with the
   desired API (I did this recently for one of Olivier's patches for
   erc-track) (this might very well involve option 1 as well), and
   remove the function at some later point after some major releases
   of Emacs; or

3. save the backward-incompatible interface change for a major ERC
   version bump.

We could do one or a mix of the above three options gradually, giving
users enough time and notice to adapt, and avoid putting them at risk.
For this case, option 1 seems straightforward to do, but so does 2.

From a quick look at J.P.'s revised version of Olivier's patch, it
looks quite favourable to me, in its preserving backward compatibility
in erc-nickserv-identify's interface and obsoleting (rather than
deleting) erc-nickserv-call-identify-function and making it a wrapper
around erc-nickserv-identify, and could probably be merged with few
minor tweaks.

J.P., Olivier, Lars, thoughts?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46777; Package emacs. (Thu, 16 Sep 2021 12:43:02 GMT) Full text and rfc822 format available.

Message #49 received at 46777 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Amin Bandali <bandali <at> gnu.org>
Cc: Olivier Certner <olce.emacs <at> certner.fr>, emacs-erc <at> gnu.org,
 46777 <at> debbugs.gnu.org, "J.P." <jp <at> neverwas.me>
Subject: Re: bug#46777: 28.0.50; ERC: NickServ identification: Prompt for
 password after other sources, overall simplifications
Date: Thu, 16 Sep 2021 14:42:42 +0200
Amin Bandali <bandali <at> gnu.org> writes:

> 2. gradually phase out the function if we must: either declare it
>    obsolete and make it a wrapper around a new function with the
>    desired API

This is usually the way to handle things like this, and it looks like
the right solution here, too.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46777; Package emacs. (Fri, 17 Sep 2021 02:18:02 GMT) Full text and rfc822 format available.

Message #52 received at 46777 <at> debbugs.gnu.org (full text, mbox):

From: "J.P." <jp <at> neverwas.me>
To: Amin Bandali <bandali <at> gnu.org>
Cc: Olivier Certner <olce.emacs <at> certner.fr>, emacs-erc <at> gnu.org,
 46777 <at> debbugs.gnu.org, Lars Ingebrigtsen <larsi <at> gnus.org>
Subject: Re: bug#46777: 28.0.50; ERC: NickServ identification: Prompt for
 password after other sources, overall simplifications
Date: Thu, 16 Sep 2021 19:16:54 -0700
Amin Bandali <bandali <at> gnu.org> writes:

> 1. change the function's arguments in a backwards-compatible way
>    (i.e. by adding new &optional args as needed), and ignore the
>    arg we don't need anymore (the password in this case, and
>    make sure we don't accidentally expose it instead of a nick);

I guess ignoring the password arg means some folks currently using this
entry point in lisp code may have to call `erc-nickserv-send-identify'
directly instead, depending on their needs. Works for me, but if that's
the plan, I don't think the version I sent fits the bill entirely.

> From a quick look at J.P.'s revised version of Olivier's patch, it
> looks quite favourable to me

Um, that was mostly me trying out my bandali impersonation (blasphemous,
I know).




Reply sent to Amin Bandali <bandali <at> gnu.org>:
You have taken responsibility. (Fri, 17 Sep 2021 04:46:01 GMT) Full text and rfc822 format available.

Notification sent to Olivier Certner <ocert.dev <at> free.fr>:
bug acknowledged by developer. (Fri, 17 Sep 2021 04:46:01 GMT) Full text and rfc822 format available.

Message #57 received at 46777-done <at> debbugs.gnu.org (full text, mbox):

From: Amin Bandali <bandali <at> gnu.org>
To: 46777-done <at> debbugs.gnu.org
Cc: Olivier Certner <olce.emacs <at> certner.fr>, emacs-erc <at> gnu.org,
 Lars Ingebrigtsen <larsi <at> gnus.org>, "J.P." <jp <at> neverwas.me>
Subject: Re: bug#46777: 28.0.50; ERC: NickServ identification: Prompt for
 password after other sources, overall simplifications
Date: Fri, 17 Sep 2021 00:45:24 -0400
Hi all,

Lars Ingebrigtsen writes:

> Amin Bandali <bandali <at> gnu.org> writes:
>
>> 2. gradually phase out the function if we must: either declare it
>>    obsolete and make it a wrapper around a new function with the
>>    desired API
>
> This is usually the way to handle things like this, and it looks like
> the right solution here, too.

Thanks, sounds good.

                                 * * *

J.P. writes:

> Amin Bandali <bandali <at> gnu.org> writes:
>
>> 1. change the function's arguments in a backwards-compatible way
>>    (i.e. by adding new &optional args as needed), and ignore the
>>    arg we don't need anymore (the password in this case, and
>>    make sure we don't accidentally expose it instead of a nick);
>
> I guess ignoring the password arg means some folks currently using this
> entry point in lisp code may have to call `erc-nickserv-send-identify'
> directly instead, depending on their needs. Works for me, but if that's
> the plan, I don't think the version I sent fits the bill entirely.

Right.  I think your *not* ignoring the password here is actually more
familiar and desirable, and I prefer it over what I'd suggested. :)

>> From a quick look at J.P.'s revised version of Olivier's patch, it
>> looks quite favourable to me
>
> Um, that was mostly me trying out my bandali impersonation (blasphemous,
> I know).

Ha, you give me too much credit. :)

Okay so having tested and reviewed J.P.'s revised patch, it looks good
to me; so I went ahead and merged it with some minor tweaks, including
listing J.P. as co-author:

https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=c6eb114a42922af18818dce7317238e0af776958

Thank you all for your help and bearing with me until I finally got
around to this.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46777; Package emacs. (Fri, 17 Sep 2021 07:58:02 GMT) Full text and rfc822 format available.

Message #60 received at 46777 <at> debbugs.gnu.org (full text, mbox):

From: Olivier Certner <olce.emacs <at> certner.fr>
To: 46777 <at> debbugs.gnu.org, bandali <at> gnu.org, jp <at> neverwas.me
Cc: larsi <at> gnus.org
Subject: Re: bug#46777: 28.0.50;
 ERC: NickServ identification: Prompt for password after other
 sources, overall simplifications
Date: Fri, 17 Sep 2021 09:57:29 +0200
Hi all,

Sorry for not having much time to respond, and thanks for having handled the 
compatibility concern through proposals 1 & 2, which I agree with. In 
particular, it is best that PASSWORD remains the first parameter to `erc-
nickserv-identify', for compatibility but practical use also , since the nick 
is normally already set when this function is called (non-interactively).

The only thing I might have done differently is deleting `erc-nickserv-call-
identify-function' instead of obsoleting it since, contrary to `erc-nickserv-
identify', I don't think people would use it directly (the name sounds too 
much "internal"; I may be wrong on its actual use, though). But your approach 
is the most prudent one.

I'll try to hang around in #erc and respond to your mails a bit more quickly, 
at least for matters that do not take much time.

Thanks,
Olivier







Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#46777; Package emacs. (Sun, 19 Sep 2021 15:27:02 GMT) Full text and rfc822 format available.

Message #63 received at 46777 <at> debbugs.gnu.org (full text, mbox):

From: Amin Bandali <bandali <at> gnu.org>
To: Olivier Certner <olce.emacs <at> certner.fr>
Cc: larsi <at> gnus.org, 46777 <at> debbugs.gnu.org, jp <at> neverwas.me
Subject: Re: bug#46777: 28.0.50; ERC: NickServ identification: Prompt for
 password after other sources, overall simplifications
Date: Sun, 19 Sep 2021 11:26:20 -0400
Hi Olivier,

Olivier Certner writes:

> Hi all,
>
> Sorry for not having much time to respond, and thanks for having handled the 
> compatibility concern through proposals 1 & 2, which I agree with. In 
> particular, it is best that PASSWORD remains the first parameter to `erc-
> nickserv-identify', for compatibility but practical use also , since the nick 
> is normally already set when this function is called (non-interactively).
>
> The only thing I might have done differently is deleting `erc-nickserv-call-
> identify-function' instead of obsoleting it since, contrary to `erc-nickserv-
> identify', I don't think people would use it directly (the name sounds too 
> much "internal"; I may be wrong on its actual use, though). But your approach 
> is the most prudent one.

No worries, and thanks.  Yeah we probably could have indeed deleted
`erc-nickserv-call-identify-function', but decided to play it safe(r)
and keep it for now. :)

> I'll try to hang around in #erc and respond to your mails a bit more quickly, 
> at least for matters that do not take much time.
>
> Thanks,
> Olivier

Thanks!

-amin




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Mon, 18 Oct 2021 11:24:08 GMT) Full text and rfc822 format available.

This bug report was last modified 2 years and 182 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.