GNU bug report logs - #72526
31.0.50; [PATCH] Fix url-basic-auth secret search when passing username and/or port

Previous Next

Full log

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

From: Björn Bidar <bjorn.bidar <at> thaodan.de>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 72526 <at> debbugs.gnu.org
Subject: Re: bug#72526: 31.0.50; [PATCH] Fix url-basic-auth secret search
 when passing username and/or port
Date: Sun, 18 Aug 2024 15:30:22 +0300

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Björn Bidar <bjorn.bidar <at> thaodan.de>
>> Cc: 72526 <at> debbugs.gnu.org
>> Date: Sat, 17 Aug 2024 23:50:51 +0300
>> 
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> 
>> >> > Sorry, I don't see any experts around to ask to do that.
>> >> 
>> >> Maybe the maintainer of the url package?
>> >
>> > Whom did you have in mind?  url.el says "emacs-devel", which is
>> > basically no one and everyone.
>> 
>> I don't know, the person that usually deals with the package?
>
> I couldn't find him or her in the recent logs.  I concluded we didn't
> have such a person/
>
>> >> > Maybe if you'd posted a more detailed description of the problem and
>> >> > its context, someone could follow your arguments and do a meaningful
>> >> > review.  E.g., it sounds from your description like the case of URLs
>> >> > where it currently fails was not meant to be supported by this
>> >> > library?  If so, perhaps an alternative is to submit to this library
>> >> > only URLs that it supports, like after stripping the port part?
>> >> 
>> >> The problem is that the user in url-basic-auth when handling urls like <uri-type>://<user>@<host> isn't
>> >> forwarded to auth-source. Further it also appends to port to the
>> >> hostname of host which means that the host is invalid since the hostname
>> >> includes the port number.
>> >> 
>> >> >From what I read when looking at url-auth.el at line 84 it does support
>> >> this kind of case of url as it already handles the same type of url when
>> >> it deals with <uri-type>://<user>:<password>@<host>.
>> >
>> > So how come this code was not fixed since the day it was added to
>> > Emacs, so long ago?
>> 
>> I don't know I assume it was never an issue at that time?
>> In any case amending the port to the :host key seems like a bug to me.
>> Similarly when the user specifies the user in the url it should be
>> passed to auth-source so it can find the credentials.
>
> So please take me through your changes with more detailed
> explanations, without assuming I know my way around this code.  A few
> things bother me just by looking at the diffs: (1) why do we need to
> calculate 'server' more than once using the same code in the same
> function, and (2) will auth-source-search as called in
> url-do-auth-source-search DTRT when called with ':user nil', which
> will happen if the last argument is omitted.  I also wonder what is
> the semantics of the call to auth-source-search in the current code
> where the user is omitted.  AFAIU, in that case Emacs will prompt the
> user for username?  If so, why is it a good idea to pass to
> auth-source-search USER derived by url-basic-auth, instead of
> prompting?

1. url-basic-auth-store uses the 'server' as in the '<server>:<port>' in
   url-basic-auth-storage. I did not want to change the existing format
   as I don't know the implications.
2. I tested calling auth-source-search with :user nil and without :user
   in both cases the result was the same, from this I imply that calling
   auth-source-search with :user nil is ok.
   Yes if auth-source-search doesn't find a user for the url
   url-basic-auth will prompt the user for a user.
   Why is it a good idea to derive the user by url-basic-auth?
   Because HTTP basic authentication uses the as specific in RFC 3986
   section 3.2.1. Using it in this function to infer the user from the
   url just follows the standard as already in other programs/Emacs
   packages.
   If the user has specified the username they want to identify with
   at the server asking for it would be redundant and not confirming to
   the standard.

> IOW, I'd like to open what the current code and your changes do for a
> more detailed discussion, so we could be sure the change is TRT before
> we decide what changes to install.
>
> Thanks.

PS: Reading your message was quite hard as a non-native speaker of
English, had to search so many of the acronyms.

Previous Next

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