a big +1...

On Mon, Aug 26, 2002 at 11:44:32PM -0700, Justin Erenkrantz wrote:
>...
> My point is that I need to add another front end authentication
> module (namely within mod_dav).  I think it'd be pointless to
> duplicate all of the backend work done in mod_auth* so that
> mod_dav can authenticate users.

Eh? I thought you wanted *access* to the user information, not to perform
authentication.

And/or maybe you need the data for authorization?

> The current authentication API
> can't work as it combines the front and back-ends.  The answer we
> give to people is, "cut-and-paste."  Ick.  Therefore, yes, I think
> we have to introduce another level as what we have now is
> insufficient.

Absolutely. I recall Ken and I talking about this kind of separation over
three years ago.

>...
> And, it doesn't mandate that third-party modules have to move to this
> scheme - it's opt-in.

Yup. Unless you kill the helper functions from server/protocol.c. :-)

>...
> Of course, I would expect the API to become better as we play with it
> more and determine what is missing.  However, this API is currently
> sufficient for the two providers that are already included - enough
> so that I think future work warrants inclusion in the tree.  As we

Absolutely. You have to start somewhere. If you want for perfection, then
you'll never get anything done.


Some random nits/comments about the code itself:

* get_user_groups() should return a hash rather than a table, right? Or do
  you truly need the case insensitively? If so, then couldn't you mandate
  the provider insert lower-cased values, and then you can look them up that
  way?

* need the various guards/wrappers in mod_auth.h

* some of your files refer to "Portions of this software based on..." I
  seriously doubt that is true for your new files :-)  [altho it probably
  *is* true for mod_auth derivatives]

* mod_auth_file/dbm.c: the module rec has a comment about "command
  apr_table_t"... it isn't a apr_table_t. some old search/replace comment?

* mod_auth_dbm.c: obsolete comment at the top (between license and first
  #include line)

* mod_auth_file.c: constify the values in auth_file_config_rec


Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Reply via email to