At 01:30 AM 3/4/2005, Justin Erenkrantz wrote:
>On Thu, Mar 03, 2005 at 08:40:22PM -0600, William A. Rowe, Jr. wrote:
>> And attached is the module for comment.  I have no time till this
>> weekend if then, so I've got the build system changes and will
>> commit if we like.
>
>My question as to how this would interact with the auth providers is still
>unanswered.

Correct.  Needs Study.

>Remember that the auth providers don't implement the check_user_id hook - only
>the auth mechanisms (basic/digest) implement those hooks.  So, this module
>acts counter to the entire notion of providers by just blindly re-running the
>entire hook process instead.  (check_user_id now becomes recursive - yikes!)

Actually, to depth == 1.  We have a recursion block to prevent further
recursion.

One interesting side effect on this patch.  If we don't find the auth
we required in the specific per-dir config, we unset it before we
return declined from the module.  If a provider accepts, we leave that
per-dir config set (that -is- where we were authorized!)

The side effect is this, on the subsequent hooks, per-dir still points
at this second provider, so now we leverage the identical config for
gaining groups, fixups and other such tweakage.

Remember, we -only- recursed IIF everyone on pass one declined.  Our
hook isn't invoked at all because it's RUN_FIRST, and if someone is
willing to authenticate the user, we succeed.

>We'd now incur the overhead of the auth mechanism hooks when there is little
>need to do so.  

No overhead, anything but DECLINED drops us out of the hook.

>Plus, we lose the ability to sanely chain providers as was the
>original intent.

Agreed - this - is what needs study.

>I still maintain the better way to do this is to handle it in the provider
>modules themselves by leveraging the provider API instead.  

NO and I will veto.  IF you introduce code complexity into seven
provider modules which could be made more complex in the core, in 
a single add-in module, or extending only the basic/digest provider,
then you are rolling dice in one area I don't like to play poker.

>To reiterate, in my mind, the ideal syntax is:
>
><Location /foo>
>  <LDAPProvider ldap1>
>      ...config options for mod_authnz_ldap...
>  </LDAPProvider>
>  <LDAPProvider ldap2>
>      ...config options for mod_authnz_ldap...
>  </LDAPProvider>
>  <DBDProvider my_db>
>      ...config options for hypothetical mod_authn_dbd...
>  </DBDProvider>
>
>  ...config options for mod_authnz_ldap...
>
>  AuthUserFile conf/foo
>
>  AuthBasicProvider ldap1 ldap2 ldap file my_db
></Location>
>
>This isolates the config directly to the module, and if we so desire, we could
>add helper functions which promote re-use of this strategy by other provider
>modules as needed.  -- justin

Unnecessarily complicates all the providers.

What about
  <AuthConfig LDAP ldap1>
  ...
  <AuthConfig LDAP ldap2>
  ...

  AuthBasicProvider ldap1 ldap2 file my_db

where we teach auth_basic and only auth_basic about <AuthConfig >
sections, and keep the cruft out of each and every provider instance?

Bill 

Reply via email to