GNU bug report logs - #38284
27.0.50; [PATCH] Make auth-source-pass-search understand port lists

Previous Next

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

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

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

From: João Távora <joaotavora <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.0.50; [PATCH] Make auth-source-pass-search understand port lists
Date: Wed, 20 Nov 2019 00:20:28 +0000

[Message part 1 (text/plain, inline)]

Hi,

When trying to follow along a tutorial on setting up Gnus for GMAIL, I
tried to use auth-source-pass.el to access encrypted entries under
~/.password-store instead of the usual ~/.authinfo.gpg.

After much wrestling with the system, I couldn't figure out why my
entry:

   gmail:imap.gpg

whose contents are

   NotReallyThePassword
   host: imap.gmail.com
   user: joaotavora <at> gmail.com
   port: 993

weren't being understood by the new auth-source.  Eventually I came to
this patch, which seems to do the right thing.

[0001-Make-auth-source-pass-search-understand-port-lists.patch (text/x-diff, inline)]

From 4a6c24c23c9f7097807c1ef58688b51db330f503 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <joaotavora <at> gmail.com>
Date: Wed, 20 Nov 2019 00:11:00 +0000
Subject: [PATCH] Make auth-source-pass-search understand port lists

For cases such as a typical IMAP Gnus setup, auto-source-pass-search
will be passed a list of "port aliases" like (993 "imaps" "imap" "993"
"143") in hopes of finding a matching ~/.password-store entry.

This modification makes this library understand and unroll the port
list so that, i.e. "domain:993", "domain:imaps"", "domain:imap",
etc. are computed as potential suffixes.  Previously a nonsensical
string "domain:(993 imaps imap ...)" was return.

* lisp/auth-source-pass.el
(auth-source-pass--generate-entry-suffixes): Allow PORT to
be a list of ports.
---
 lisp/auth-source-pass.el | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/lisp/auth-source-pass.el b/lisp/auth-source-pass.el
index 524a72792c..cc0a6fe4de 100644
--- a/lisp/auth-source-pass.el
+++ b/lisp/auth-source-pass.el
@@ -269,10 +269,15 @@ auth-source-pass--generate-entry-suffixes
 
 Based on the supported pathname patterns for HOSTNAME, USER, &
 PORT, return a list of possible suffixes for matching entries in
-the password-store."
+the password-store.
+
+PORT may be a list of ports."
   (let ((domains (auth-source-pass--domains (split-string hostname "\\."))))
-    (seq-mapcat (lambda (n)
-                  (auth-source-pass--name-port-user-suffixes n user port))
+    (seq-mapcat (lambda (d)
+                  (seq-mapcat
+                   (lambda (p)
+                     (auth-source-pass--name-port-user-suffixes d user p))
+                   (if (listp port) port (list port))))
                 domains)))
 
 (defun auth-source-pass--domains (name-components)
-- 
2.24.0

[Message part 3 (text/plain, inline)]

Please have a look,
João

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: João Távora <joaotavora <at> gmail.com>
Cc: Damien Cassou <damien <at> cassou.me>, 38284 <at> debbugs.gnu.org,
 Nicolas Petton <nicolas <at> petton.fr>
Subject: Re: bug#38284: 27.0.50; [PATCH] Make auth-source-pass-search
 understand port lists
Date: Thu, 21 Nov 2019 14:47:54 +0100

João Távora <joaotavora <at> gmail.com> writes:

> This modification makes this library understand and unroll the port
> list so that, i.e. "domain:993", "domain:imaps"", "domain:imap",
> etc. are computed as potential suffixes.  Previously a nonsensical
> string "domain:(993 imaps imap ...)" was return.
>
>    (let ((domains (auth-source-pass--domains (split-string hostname "\\."))))
> -    (seq-mapcat (lambda (n)
> -                  (auth-source-pass--name-port-user-suffixes n user port))
> +    (seq-mapcat (lambda (d)
> +                  (seq-mapcat
> +                   (lambda (p)
> +                     (auth-source-pass--name-port-user-suffixes d user p))
> +                   (if (listp port) port (list port))))
>                  domains)))

Looks good to me, but I don't use password-store so I can't really
test.  If it works for you, please go ahead and apply.

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

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

From: Damien Cassou <damien <at> cassou.me>
To: João Távora <joaotavora <at> gmail.com>,
 38284 <at> debbugs.gnu.org
Cc: Nicolas Petton <nicolas <at> petton.fr>
Subject: Re: bug#38284: 27.0.50;
 [PATCH] Make auth-source-pass-search understand port lists
Date: Thu, 21 Nov 2019 19:27:34 +0100

Hi João,

João Távora <joaotavora <at> gmail.com> writes:
> […] Eventually I came to this patch, which seems to do the right
> thing.


great job, thank you. Some feedback below.

>    (let ((domains (auth-source-pass--domains (split-string hostname "\\."))))
> -    (seq-mapcat (lambda (n)
> -                  (auth-source-pass--name-port-user-suffixes n user port))
> +    (seq-mapcat (lambda (d)


can you please rename "d" to "domain"?


> +                  (seq-mapcat
> +                   (lambda (p)


same for "p".


> +                     (auth-source-pass--name-port-user-suffixes d user p))
> +                   (if (listp port) port (list port))))
>                  domains)))


Can you please add a unit test covering this new use-case?

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

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

From: João Távora <joaotavora <at> gmail.com>
To: Damien Cassou <damien <at> cassou.me>
Cc: 38284 <at> debbugs.gnu.org, Nicolas Petton <nicolas <at> petton.fr>
Subject: Re: bug#38284: 27.0.50; [PATCH] Make auth-source-pass-search
 understand port lists
Date: Thu, 21 Nov 2019 19:06:39 +0000

On Thu, Nov 21, 2019 at 6:27 PM Damien Cassou <damien <at> cassou.me> wrote:
>
> Hi João,
>
> João Távora <joaotavora <at> gmail.com> writes:
> > […] Eventually I came to this patch, which seems to do the right
> > thing.
>
>
> great job, thank you. Some feedback below.
>
> >    (let ((domains (auth-source-pass--domains (split-string hostname "\\."))))
> > -    (seq-mapcat (lambda (n)
> > -                  (auth-source-pass--name-port-user-suffixes n user port))
> > +    (seq-mapcat (lambda (d)
>
>
> can you please rename "d" to "domain"?

ok.  I do call your attention that it was already the
single letter n there, so I was following what I though
was shorthand convention, just adjusting it to the
first letter of the concept actually used.

> > +                  (seq-mapcat
> > +                   (lambda (p)
>
>
> same for "p".

ok.

> > +                     (auth-source-pass--name-port-user-suffixes d user p))
> > +                   (if (listp port) port (list port))))
> >                  domains)))
>
>
> Can you please add a unit test covering this new use-case?

No. This is too much work for such a trivial change that
can be reasoned about locally. I don't usually write tests
for those.  If you permit me to exagerate, it's like testing
that (+ 2 2) really equals 4.

But some people do write tests at this level, and I of
course don't mind if you do.

-- 
João Távora

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

From: Damien Cassou <damien <at> cassou.me>
To: João Távora <joaotavora <at> gmail.com>
Cc: 38284 <at> debbugs.gnu.org, Nicolas Petton <nicolas <at> petton.fr>
Subject: Re: bug#38284: 27.0.50;
 [PATCH] Make auth-source-pass-search understand port lists
Date: Fri, 22 Nov 2019 09:49:20 +0100

[Message part 1 (text/plain, inline)]

Hi João,

João Távora <joaotavora <at> gmail.com> writes:
>> can you please rename "d" to "domain"?
>
> ok.  I do call your attention that it was already the
> single letter n there


yes I saw. This should never have been merged like that. Thank you for
improving the code.

>> Can you please add a unit test covering this new use-case?
>
> No. This is too much work for such a trivial change

I care a lot about the automated testing of the code I write. I won't
try to convince you though. Can you please merge the attached patch with
yours?

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

[0001-WIP.patch (text/x-patch, attachment)]

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

From: João Távora <joaotavora <at> gmail.com>
To: Damien Cassou <damien <at> cassou.me>
Cc: 38284 <at> debbugs.gnu.org, Nicolas Petton <nicolas <at> petton.fr>
Subject: Re: bug#38284: 27.0.50; [PATCH] Make auth-source-pass-search
 understand port lists
Date: Fri, 22 Nov 2019 09:35:38 +0000

Damien Cassou <damien <at> cassou.me> writes:

> João Távora <joaotavora <at> gmail.com> writes:
>>> can you please rename "d" to "domain"?
>>
>> ok.  I do call your attention that it was already the
>> single letter n there
>
> yes I saw. This should never have been merged like that. Thank you for
> improving the code.

I think the single letter idiom is fine there.  It was just the wrong
letter.  By the way, I've left the p for the port, because calling it
"port", while it would work, would seriously confuse a reader.

>>> Can you please add a unit test covering this new use-case?
>>
>> No. This is too much work for such a trivial change
>
> I care a lot about the automated testing of the code I write.

Certainly, I care a lot, too.  I don't write tests for these changes out
of principle, not out of lazyness.  Most, if not all, the projects I
manage have automated tests.

> I won't try to convince you though. Can you please merge the attached
> patch with yours?

No, but you can do that, because it's your work (I can push it for you
though).  Anyway, now I read the test you wrote, I agree it's a good
test.  You are testing auth-source-pass-match-entry-p, much higher up
than auth-source-pass--generate-entry-suffixes, the function I changed.

Of course, only someone who was involved in the design would be able to
confidently place the tests at that correct level, the finding of which
is the most difficult part.

My advice and personal opinion is to later use this and more such tests
to perhaps redesign/cleanup the auth-source-pass.el library, which seems
needlessly complicated in the little stuff like the function I touched.
(to be fair it wasn't much helped by the style of the auth-source.el
parent library.)

João

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

From: Stefan Kangas <stefan <at> marxist.se>
To: João Távora <joaotavora <at> gmail.com>
Cc: Damien Cassou <damien <at> cassou.me>, 38284 <at> debbugs.gnu.org,
 Nicolas Petton <nicolas <at> petton.fr>
Subject: Re: bug#38284: 27.0.50; [PATCH] Make auth-source-pass-search
 understand port lists
Date: Mon, 20 Jan 2020 19:54:01 +0100

João Távora <joaotavora <at> gmail.com> writes:

> Damien Cassou <damien <at> cassou.me> writes:
>
>> Can you please merge the attached patch with yours?
>
> No, but you can do that, because it's your work (I can push it for you
> though).  Anyway, now I read the test you wrote, I agree it's a good
> test.  You are testing auth-source-pass-match-entry-p, much higher up
> than auth-source-pass--generate-entry-suffixes, the function I changed.
>
> Of course, only someone who was involved in the design would be able to
> confidently place the tests at that correct level, the finding of which
> is the most difficult part.

If the test is good, I think it should be installed.  Unfortunately,
it seems like it doesn't apply cleanly to current master.

Damien, could you please re-send the patch formatted by "git
format-patch -1"?  Please also include a commit message with a
ChangeLog entry as described in the CONTRIBUTE file.

Best regards,
Stefan Kangas

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

From: Damien Cassou <damien <at> cassou.me>
To: Stefan Kangas <stefan <at> marxist.se>, João Távora
 <joaotavora <at> gmail.com>
Cc: 38284 <at> debbugs.gnu.org, Nicolas Petton <nicolas <at> petton.fr>
Subject: Re: bug#38284: 27.0.50; [PATCH] Make auth-source-pass-search
 understand port lists
Date: Tue, 21 Jan 2020 20:25:08 +0100

[Message part 1 (text/plain, inline)]

Stefan Kangas <stefan <at> marxist.se> writes:
> If the test is good, I think it should be installed.  Unfortunately,
> it seems like it doesn't apply cleanly to current master.
>
> Damien, could you please re-send the patch formatted by "git
> format-patch -1"?  Please also include a commit message with a
> ChangeLog entry as described in the CONTRIBUTE file.

The patch was only meant to be added to João's own patch. Here is a
standalone one.

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

[0001-test-lisp-auth-source-pass-tests.el-Test-for-multipl.patch (text/x-patch, attachment)]

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Damien Cassou <damien <at> cassou.me>
Cc: 38284-done <at> debbugs.gnu.org, Nicolas Petton <nicolas <at> petton.fr>,
 João Távora <joaotavora <at> gmail.com>
Subject: Re: bug#38284: 27.0.50; [PATCH] Make auth-source-pass-search
 understand port lists
Date: Wed, 22 Jan 2020 09:02:45 +0100

Damien Cassou <damien <at> cassou.me> writes:

>> Damien, could you please re-send the patch formatted by "git
>> format-patch -1"?  Please also include a commit message with a
>> ChangeLog entry as described in the CONTRIBUTE file.
>
> The patch was only meant to be added to João's own patch.

Understood.

> Here is a standalone one.

Thanks, it seems to be working fine (the test passes), so I have now
pushed this to master as commit abb2515b0c.

I don't see anything more to do here, so I'm also closing this bug.

Best regards,
Stefan Kangas

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

From: Damien Cassou <damien <at> cassou.me>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 38284-done <at> debbugs.gnu.org, Nicolas Petton <nicolas <at> petton.fr>,
 João Távora <joaotavora <at> gmail.com>
Subject: Re: bug#38284: 27.0.50; [PATCH] Make auth-source-pass-search
 understand port lists
Date: Wed, 22 Jan 2020 10:22:08 +0100

Stefan Kangas <stefan <at> marxist.se> writes:
> I don't see anything more to do here, so I'm also closing this bug.

thank you

-- 
Damien Cassou

"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill

Previous Next

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