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
