Subject: Re: bug#58985: 29.0.50; Have auth-source-pass behave more like other back ends

From: Akib Azmain Turja <[email-masked]>
Date: Thu, 10 Nov 2022 14:11:31 +0600

"J.P." <[email-masked]> writes:

> Hi Akib,
>
> Akib Azmain Turja <[email-masked]> writes:
>
>> Michael Albinus <[email-masked]> writes:
>>
>>> "J.P." <[email-masked]> writes:
>>>
>>> Hi,
>>>
>>>> v2. Respect existing user option.
>>>
>>> I'm not familiar with the auth-source-pass.el implementation, so I
>>> cannot speak too much about your patch. Reading it roughly, I haven't
>>> found serious flaws, 'tho.
>>
>> It has a serious flaw AFAIK.  I have a password entry
>> "[email-masked]", and this legitimate search query doesn't find it:
>>
>> (auth-source-search :host "disroot.org")
>>
>> But if specify the user, it finds the entry:
>>
>> (auth-source-search :host "disroot.org" :user "akib")
>
> Hm, that's unfortunate. I specifically added a pair of tests just for
> this, namely
>
>   auth-source-pass-extra-query-keywords--netrc-akib
>   auth-source-pass-extra-query-keywords--akib
>
> Are you able to pinpoint why they're reporting a false positive by any
> chance (or give a minimal repro recipe with an FS tree layout of some
> ~/.password-store)? Also, and I'm not trying to be insulting here, but
> did you remember to rerun Make after applying the patch(es)?
>

Actually, I didn't review the patches in this email, I just commented on
the auth-source-pass in the master *right now*, not the patch.  Sorry
for the trouble.

>> And the entries can also be ambiguous.  For example, the entry at path
>> "foo.org/bar.net" might be interpreted as the password of bar.net, or
>> as the password of the user "bar.net" on "foo.org".  The current
>> implementation seems to interpret such entries unpredictably.
>
> Sounds convincing. What do you think about deprecating the /user form?
> (This may have to be spun off into a separate bug report.)
>
> At the end of the day, I'm more concerned about consistency (and thus
> predictability) than anything. IOW, I'd be okay with "foo.org/bar.net"
> being parsed either way, as long as it's the *same* way every time,
> which we could then document. If you're indeed finding otherwise, please
> provide an MRE for this as well (with patches applied, of course).
>
>>> - The name of this user option as well as its docstring are focussed on
>>>   the current behavior. People won't know what "mimic other auth-source
>>>   backends" would mean. Please describe the effect w/o that comparison,
>>>   and pls give it a name based on its effect, and not "...-standard-search".
>>
>> I agree.  This variable should be something like
>> "auth-source-pass-old-search" (or even "...-obsolete-search").
>
> Wait, but `auth-source-pass-old-search' sounds like we're regressing to
> describing a comparison rather than an effect. The name in the second
> (v2) iteration, `auth-source-pass-extra-query-keywords', was an attempt
> to rein in the scope of the option and convey no more than what it's
> claiming to offer.

Thanks for clarification.  I have written the same thing in my another
(actual) patch review email, feel free to ignore those parts.

>
>> And the default should be nil, because it fixes many bugs, and it's
>> pointless to disable the fixes by the default.
>
> Not sure I agree here, even though Damien seems to be in accord. In the
> interest of minimizing churn for Melpa's pass and password-store
> packages, I'd rather make this an opt-in for Emacs 29 if we end up
> including it at all.
>

How about communicating with them?

>>> - I'm missing the documentation in doc/misc/auth.texi and etc/NEWS.
>>
>> What documentation?  Of this change or anything else?  I think we should
>> focus on the implement before writing documentation.
>
> Hm, (again, not trying to insult here, but) did you somehow miss the
> patches attached to the email you replied to? It kind of looks that way
> based on your comments. If I'm wrong, though, please forgive; I
> appreciate your input regardless.

Yeah, you are right, I didn't notice those patches and just commented on
the auth-source-pass in the master *right now*, not the patch.  Please
forgive for the trouble.

> Thanks,
> J.P.

-- 
Akib Azmain Turja --- https://akib.codeberg.page/
GPG key: 70018CE5819F17A3BBA666AFE74F0EFA922AE7F5
Fediverse: [email-masked], Codeberg: akib
emailselfdefense.fsf.org | "Nothing can be secure without encryption."