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

Hi Akib,

Akib Azmain Turja <[email protected]> writes:

> Michael Albinus <[email protected]> writes:
>
>> "J.P." <[email protected]> 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 protected]", 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)?

> 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.

> 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.

>> - 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.

Thanks,
J.P.