Great! Yes I was a bit iffy on the ³custom² approach myself given it
really didn¹t align with any authentication method. But if API
breakage is okay, this is definitely the cleanest way.

>>- Retrofit existing interfaces with the necessary data.
>>   a. Introduce void *sess_private in sess_t.
>It's not really sess_private, but authreg_private, right?

Yep agreed. Though what would be better is if we could somehow classify
it as per user per session data. Thinking of user_st.module_data in sm.h.
But authreg_private works if no other suggestions are found.

>>   int (*create_challenge)(authreg_t ar, sess_private *data,
>>     const char *username, const char *realm, const char *challenge,
>>     int maxlen);
>>   int (*check_response)(authreg_t ar, sess_private *data,
>>     const char *username, const char *realm, const char *resource,
>>      const char *challenge, const char *response);
>>   Pros: Maintain same methods but new parameters, faster approach.
>>   Cons: (BIG)breaks everyone out there. In some cases, other 3rd parties
>>     may want similar mechanism for plain text login as well and this
>>     approach wouldn't work for them.
>I think we should extend all authreg calls with a pointer to session
>attached, authreg private data.
>In the simplest case it could be even set to point to sess_st, for the
>mechanizm to dig in session by itself.
>This is how it is done all around jabberd2.

Yep agreed, providing sess_st avoids another API change down the road and
gives access to all the necessary data needed.

>Also good point, that create_challenge misses realm parameter.
>If we go for it, we will just release 2.4.x line which hints API
>breakage. ;-)

This is perfect. Though, just a heads up, I may have some more suggestions
on the storage_* side as well once I dig into it a bit more :)

>Let's just implement CRAM-MD5 properly, with all needed features, even
>if it means API changes.
>We're open source - we are not afraid to change things. :-)

I would change all the APIs and to pass in a pointer to the sess_t as I
also need it in check_passsword. Similar issues, I need to store the
authentication token for a successful verification. Also, this way the APIs
are uniform.

This is good stuff. These changes help any organization providing
centralized token based authentication where the resource specific and
user session needs to be stashed away properly.

I should be able to get a patch/pull reques back out by EOD today.


Reply via email to