Den tis 8 juli 2025 kl 22:23 skrev Dirk-Willem van Gulik < di...@webweaving.org>:
> On 8 Jul 2025, at 17:26, Branko Čibej <br...@apache.org> wrote: > > On 26. 6. 25 08:16, Branko Čibej wrote: > >> On 24. 6. 25 07:52, Branko Čibej wrote: > >>> On 14. 6. 25 21:23, Branko Čibej wrote: > >>>> Hi all, > >>>> > >>>> I've been digging into Serf's support for various authentication > schemes and I notices something that looks like a bit of a limitation. > >>>> > >>>> Unless I'm much mistaken, there's space for only one authentication > baton in Serf's context. It would seem that this is rather a blocker for > implementing multi-factor authentication flows, for example, Basic + OTP, > where the server would first require basic credentials and then, if those > were correct, go on to issue an OTP challenge. > >>>> > >>>> It seems to me that a simple solution for that would be to store an > authn baton per scheme, but I know on the close order of nothing about the > possible side effects. > >>>> > >>>> Yeah, I'm starting small, I have no wish to implement OAuth2 flow any > time soon. Still, a bit of insight from the knowledgeable would be welcome. > >>> > >>> > >>> With r1926674, I think the user-defined-authn branch is ready for > review. Here's a short summary of the changes on the branch: > >>> > >>> * The (private) struct serf__authn_scheme_t, defined in auth/auth.h, > >>> grew a number of new members to support user-defined schemes, among > >>> them five new user-visible callback functions. These, along with > >>> their interactions, are described in serf.h. The built-in > >>> authentication schemes don't need or use these new members. > >>> > >>> * The (private) table serf_authn_schemes, defined in auth/auth.c, has > >>> been extended to have room for as many schemes as there are bits in > >>> serf__authn_scheme_t::type. Locking was added to serialize access, > >>> because new schemes can be registered at runtime. > >>> > >>> * Private callbacks were added to implement the user-visible > >>> authentication flow. > >>> > >>> * Two new public APIs take care of registering and unregistering > >>> authentication schemes. The latter is tricky because we don't track > >>> references to the schemes in contexts and connections, only in the > >>> serf_authn_schemes table; this issue is documented. > >>> > >>> * A number of tests were added for all this new stuff. > >>> > >>> > >>> Please take a look if you have time. > >>> > >>> svn diff --internal-diff \ > >>> https://svn.apache.org/repos/asf/serf/trunk \ > >>> https://svn.apache.org/repos/asf/serf/branches/user-defined-authn \ > >>> | colordiff --color=always \ > >>> | less -R > >>> > >> > >> There are a couple more changes on that branch now: > >> > >> * The parameters in the authentication challenge (headers > >> WWW-Authenticate and Proxy-Authenticate) are now parsed into a > >> dictionary, using the rules described in RFC 9110. > >> > >> * The Authentication-Info and Proxy-Authentication-Info response > >> headers are now handled and their values parsed as above. > >> > >> These two changes should make life much easier for authn scheme > implementers. I decided to add this once I saw how many times we have > copy-pasted not-quite-compliant parsers throughout the auth code... > >> > >> (Note that these changes do not stop implementers from doing whatever > wrong-headed, non-compliant horror they can think of, as all the raw > information and headers are still available in the callbacks.) > > > > Any takers? It's been a couple of weeks now. I'd sure like to get > another pair of eyes on these changes before merging them to trunk, but I'd > rather not have the branch fall victim to bitrot, either. > > I'll bite - so I've tried it against some auth testing code we privately > maintain for legacy hardware keys - and it certainly gets rid of a lot of > the boilerplate/header parse code - without loss of functionality. And > oddly it passes now an EAGAIN/EINTR edge case that always broke our tests. > > Improvement overall. > > Dw Thanks for testing! It is on my todo list to review it but it seems there are always new issues / pr popping up and I've not been able to prioritise this. My goal is to do it before the end of summer vacations... /Daniel
