Re: c2s per session user data authreg auth API extension

2014-08-14 Thread Tomasz Sterna 
Dnia 2014-08-14, czw o godzinie 04:27 +, Shawn Debnath pisze:
 - Build a hash table of relevant data and store it in the authreg_t
   private data member.

Agreed, that needed internal bookkeeping makes it not feasible.


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


   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.

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

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


   /* Extension for custom authentication providers */
   int (*custom_auth_get)(authreg_t ar, authdata_t data);
   int (*custom_auth_set)(authreg_t ar, authdata_t data);

I don't like this approach for two reasons.
- custom_auth does not really mean anything. as it is now it is clean -
either we have password verification, or challenge/response.
- custom_auth is used in ar_mechs  AR_MECH_TRAD_CRAMMD5, so it is not
really custom, but CRAM-MD5, right?


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

-- 
smk

Re: c2s per session user data authreg auth API extension

2014-08-14 Thread Shawn Debnath 

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.

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

Thanks,
Shawn

Re: c2s per session user data authreg auth API extension

2014-08-14 Thread Tomasz Sterna 
Dnia 2014-08-14, czw o godzinie 16:20 +, Shawn Debnath pisze:
 I would change all the APIs and to pass in a pointer to the sess_t as
 I also need it in check_passsword.

I would advise to include sess_t* in authreg_private then.

It's OK for authreg to dig around session data, but the API should be
flexible enough to give option to pass anything as authreg_private, not
only sess_t*.


-- 
Tomasz Sterna   :(){ :|:};:
Instant Messaging Consultant   Open Source Developer 
http://abadcafe.pl/  http://xiaoka.com/portfolio



signature.asc
Description: This is a digitally signed message part

Re: c2s per session user data authreg auth API extension

2014-08-14 Thread Tomasz Sterna 
Dnia 2014-08-14, czw o godzinie 23:45 +, Shawn Debnath pisze:
 I have modified the
 APIs to pass sess_t and then the implementation can choose to pack it
 in their private authreg_private data if they so choose.

WFM :-)


-- 
Tomasz Sterna   :(){ :|:};:
Instant Messaging Consultant   Open Source Developer 
http://abadcafe.pl/  http://xiaoka.com/portfolio



signature.asc
Description: This is a digitally signed message part

Re: c2s per session user data authreg auth API extension

2014-08-14 Thread Shawn Debnath 
I have modified the
 APIs to pass sess_t and then the implementation can choose to pack it
 in their private authreg_private data if they so choose.

WFM :-)

Changes almost ready to go, will submit a pull request within the hour.

Re: c2s per session user data authreg auth API extension

2014-08-14 Thread Shawn Debnath 
Changes almost ready to go, will submit a pull request within the hour.

Pull request submitted: https://github.com/jabberd2/jabberd2/pull/72
Issue create to track (perhaps excessive):
https://github.com/jabberd2/jabberd2/issues/71

Looking forward to comments and feedback!

Thanks,
Shawn