You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by "John K. Sterling" <jo...@sterls.com> on 2002/09/14 16:45:36 UTC
[PATCH] modules/aaa use optional function to register
Hi -
Here is the patch that changes the auth providers to use an optional
function. It delares the optional function in mod_auth.h, register it
in mod_auth_basic.c, and retrieve it in mod_authn_file.c and
mod_authn_dbm.c.
Notice how the only current auth_providers are dbm and file -- and
there are no authz providers. When those are switched, they simply
need to lookup the optional function in the pre_config phase. Even
though there are no authz providers, I optionalized the register
method. I did my best to implement this without modifying any current
logic, so it should be simple to read.
sterling
Index: modules/aaa/mod_auth.h
===================================================================
RCS file: /home/cvspublic/httpd-2.0/modules/aaa/mod_auth.h,v
retrieving revision 1.3
diff -u -r1.3 mod_auth.h
--- modules/aaa/mod_auth.h 13 Sep 2002 21:55:31 -0000 1.3
+++ modules/aaa/mod_auth.h 14 Sep 2002 14:33:24 -0000
@@ -58,7 +58,7 @@
#include "apr_pools.h"
#include "apr_hash.h"
-
+#include "apr_optional.h"
#include "httpd.h"
#ifdef __cplusplus
@@ -106,8 +106,10 @@
const char *realm, char **rethash);
} authn_provider;
+APR_DECLARE_OPTIONAL_FN(void, authn_register_provider, (apr_pool_t *p,
const char *name,
+ const
authn_provider *provider));
AAA_DECLARE(void) authn_register_provider(apr_pool_t *p, const char
*name,
- const authn_provider
*provider);
+ const authn_provider
*provider);
AAA_DECLARE(const authn_provider *) authn_lookup_provider(const char
*name);
typedef struct {
@@ -117,7 +119,12 @@
AAA_DECLARE(void) authz_register_provider(apr_pool_t *p, const char
*name,
const authz_provider
*provider);
+
+APR_DECLARE_OPTIONAL_FN(void, authz_register_provider,
+ (apr_pool_t *p, const char *name, const
authz_provider *provider)
+ );
AAA_DECLARE(const authz_provider *) authz_lookup_provider(const char
*name);
+
#ifdef __cplusplus
}
#endif
Index: modules/aaa/mod_auth_basic.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/modules/aaa/mod_auth_basic.c,v
retrieving revision 1.4
diff -u -r1.4 mod_auth_basic.c
--- modules/aaa/mod_auth_basic.c 10 Sep 2002 14:40:46 -0000 1.4
+++ modules/aaa/mod_auth_basic.c 14 Sep 2002 14:33:24 -0000
@@ -285,6 +285,8 @@
static void register_hooks(apr_pool_t *p)
{
+ APR_REGISTER_OPTIONAL_FN(authn_register_provider);
+ APR_REGISTER_OPTIONAL_FN(authz_register_provider);
ap_hook_check_user_id(authenticate_basic_user,NULL,NULL,APR_HOOK_MIDDLE)
;
}
Index: modules/aaa/mod_authn_dbm.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/modules/aaa/mod_authn_dbm.c,v
retrieving revision 1.3
diff -u -r1.3 mod_authn_dbm.c
--- modules/aaa/mod_authn_dbm.c 13 Sep 2002 17:33:27 -0000 1.3
+++ modules/aaa/mod_authn_dbm.c 14 Sep 2002 14:33:27 -0000
@@ -198,9 +198,23 @@
NULL, /* No realm support yet. */
};
+static int authn_dbm_pre_config(apr_pool_t *pconf, apr_pool_t *plog,
apr_pool_t *ptemp)
+{
+ APR_OPTIONAL_FN_TYPE(authn_register_provider) *register_provider;
+ register_provider =
APR_RETRIEVE_OPTIONAL_FN(authn_register_provider);
+ if( register_provider == NULL ) {
+ ap_log_error(APLOG_MARK, APLOG_ERR, NULL, NULL,
+ "Unable to register auth_dbm as a provider. This
is probably because auth_basic is not loaded");
+ return HTTP_INTERNAL_SERVER_ERROR;
+ } else {
+ register_provider(pconf, "dbm", &authn_dbm_provider);
+ }
+ return OK;
+}
+
static void register_hooks(apr_pool_t *p)
{
- authn_register_provider(p, "dbm", &authn_dbm_provider);
+ ap_hook_pre_config(authn_dbm_pre_config, NULL, NULL,
APR_HOOK_FIRST);
}
module AP_MODULE_DECLARE_DATA authn_dbm_module =
Index: modules/aaa/mod_authn_file.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/modules/aaa/mod_authn_file.c,v
retrieving revision 1.2
diff -u -r1.2 mod_authn_file.c
--- modules/aaa/mod_authn_file.c 10 Sep 2002 06:57:03 -0000 1.2
+++ modules/aaa/mod_authn_file.c 14 Sep 2002 14:33:27 -0000
@@ -224,9 +224,24 @@
&get_realm_hash,
};
+static int authn_file_pre_config(apr_pool_t *pconf, apr_pool_t *plog,
apr_pool_t *ptemp)
+{
+ APR_OPTIONAL_FN_TYPE(authn_register_provider) *register_provider;
+
+ register_provider =
APR_RETRIEVE_OPTIONAL_FN(authn_register_provider);
+ if( register_provider == NULL ) {
+ ap_log_error(APLOG_MARK, APLOG_ERR, NULL, NULL,
+ "Unable to register auth_file as a
provider. This is probably because auth_basic is not loaded");
+ return HTTP_INTERNAL_SERVER_ERROR;
+ } else {
+ register_provider(pconf, "file", &authn_file_provider);
+ }
+ return OK;
+}
+
static void register_hooks(apr_pool_t *p)
{
- authn_register_provider(p, "file", &authn_file_provider);
+ ap_hook_pre_config(authn_file_pre_config, NULL, NULL,
APR_HOOK_FIRST);
}
module AP_MODULE_DECLARE_DATA authn_file_module =
Re: [PATCH] modules/aaa use optional function to register
Posted by "John K. Sterling" <jo...@sterls.com>.
On Sunday, September 15, 2002, at 03:26 PM, Justin Erenkrantz wrote:
>
> If it's done right, mod_dav doesn't have to change any of its
> APIs - dav_register_provider/dav_lookup_provider can just wrap
> ap_register_provider/ap_lookup_provider with the DAV namespace.
>
> Do you feel brave enough to take this on? *nudge* ;-) -- justin
>
>
aight, I'll look into it (but no patches promised - i'm net sure i'm
sold on it yet :)
sterling
Re: [PATCH] modules/aaa use optional function to register
Posted by Justin Erenkrantz <je...@apache.org>.
On Sat, Sep 14, 2002 at 10:32:02PM -0400, John K. Sterling wrote:
> this. Only problem, as you implied, is the casting issue. Not so
> worried about the cast itself, but more that you lose some of the
> beauty of optional functions. With this scheme the consumers would
> need to cast the providers to a known type (so the consumers and
> providers would both have to have that structure defined).
Right. I doubt that the casting is a big deal though.
> That might not be so bad, though..... Looks to me like moving it into
> the core is an exercise in code re-use for dav. For the auth stuff it
> will definitely clean it up. Not sure how I feel about that.
If it's done right, mod_dav doesn't have to change any of its
APIs - dav_register_provider/dav_lookup_provider can just wrap
ap_register_provider/ap_lookup_provider with the DAV namespace.
Do you feel brave enough to take this on? *nudge* ;-) -- justin
Re: [PATCH] modules/aaa use optional function to register
Posted by "John K. Sterling" <jo...@sterls.com>.
On Saturday, September 14, 2002, at 09:58 PM, Justin Erenkrantz wrote:
> Hope that makes my last post clearer. While I do want to make
> mod_dav use these new providers in the long-run, mod_dav has its
> own independent provider API for a different purpose. We can take
> this opportunity to merge them together. -- justin
Ahh -
I see. Yeah, that code is kind of a duplicate. There are also some
other uses for this generic provider structure that come to mind. I've
already written a bunch of modules that define their own version of
this. Only problem, as you implied, is the casting issue. Not so
worried about the cast itself, but more that you lose some of the
beauty of optional functions. With this scheme the consumers would
need to cast the providers to a known type (so the consumers and
providers would both have to have that structure defined).
That might not be so bad, though..... Looks to me like moving it into
the core is an exercise in code re-use for dav. For the auth stuff it
will definitely clean it up. Not sure how I feel about that.
sterling
Re: [PATCH] modules/aaa use optional function to register
Posted by Justin Erenkrantz <je...@apache.org>.
On Sat, Sep 14, 2002 at 09:51:50PM -0400, John K. Sterling wrote:
> I'm not familiar with the dav requirements. I was kind of assuming
> these authn providers had a flexible enough api for dav to just use
> them - is that not the case? If it is actually not the case, I'm not
> sure I see what re-use we are going to get out of these new providers.
Oh, I mean that DAV has its own provider scheme that is completely
separate from aaa. See modules/dav/main/providers.c - the code
in modules/aaa/auth_provider.c is essentially a cut-and-paste of
the DAV code. Also see dav_cmd_dav in modules/dav/main/mod_dav.c.
("DAV filesystem", "DAV SVN", etc.)
Hope that makes my last post clearer. While I do want to make
mod_dav use these new providers in the long-run, mod_dav has its
own independent provider API for a different purpose. We can take
this opportunity to merge them together. -- justin
Re: [PATCH] modules/aaa use optional function to register
Posted by "John K. Sterling" <jo...@sterls.com>.
On Saturday, September 14, 2002, at 09:36 PM, Justin Erenkrantz wrote:
> Well, we could make a case for adding the provider API to the core
> since DAV has essentially the same semantics.
>
> Would (should) we use a two-key system? Such as:
I'm not familiar with the dav requirements. I was kind of assuming
these authn providers had a flexible enough api for dav to just use
them - is that not the case? If it is actually not the case, I'm not
sure I see what re-use we are going to get out of these new providers.
I would rather the 'authn_provider' interface be flexible enough that
dav can use it as well. maybe you can enlighten me on dav's
requirements.
sterling
Re: [PATCH] modules/aaa use optional function to register
Posted by Justin Erenkrantz <je...@apache.org>.
On Sat, Sep 14, 2002 at 09:28:44PM -0400, John K. Sterling wrote:
>
> On Saturday, September 14, 2002, at 09:17 PM, Justin Erenkrantz wrote:
> >
> >Hmm. Can we register an optional function twice? Does that mean
> >that any module that uses the provider API has to 'optionally'
> >register these functions? Eek.
>
> i was thinking they would conditionally register (i.e. if it wasn't
> there already)
>
> >I'm starting to like the idea of mod_auth_common. =) -- justin
>
> me too. unless this provider registration functionality gets added
> into the core.
Well, we could make a case for adding the provider API to the core
since DAV has essentially the same semantics.
Would (should) we use a two-key system? Such as:
ap_register_provider(apr_pool_t *p, const char *provider_group,
const char *provider_name, void *provider);
ap_lookup_provider(const char *provider_group,
const char *provider_name);
ap_register_provider(p, "authn", "file", authn_file_provider);
ap_register_provider(p, "dav", "file", dav_file_provider);
mod_dav:
myprov = (dav_provider*) ap_lookup_provider(p, "dav", "file");
mod_auth_basic:
myprov = (authn_provider*) ap_lookup_provider(p, "authn", "file");
The only drawback I see is the loss of type-safety, but that's a
red herring anyway, since the storage for the providers is a
hash table...
What do you think? -- justin
Re: [PATCH] modules/aaa use optional function to register
Posted by "John K. Sterling" <jo...@sterls.com>.
On Saturday, September 14, 2002, at 09:17 PM, Justin Erenkrantz wrote:
>
> Hmm. Can we register an optional function twice? Does that mean
> that any module that uses the provider API has to 'optionally'
> register these functions? Eek.
i was thinking they would conditionally register (i.e. if it wasn't
there already)
> I'm starting to like the idea of mod_auth_common. =) -- justin
me too. unless this provider registration functionality gets added
into the core.
sterling
Re: [PATCH] modules/aaa use optional function to register
Posted by Justin Erenkrantz <je...@apache.org>.
On Sat, Sep 14, 2002 at 09:13:22PM -0400, John K. Sterling wrote:
> >Here's the fundamental flaw - I can use providers with
> >mod_auth_digest as well.
> >
>
> Ouch -
>
> didn't catch that -
>
> If you don't want to have a specific module that registers the
> auth_provider optional functions, you could have both modules register
> the optional functions..... kinda yicky - but arguably cleaner than
> forcing the providers to be loaded after the basic/digest
> modules.........
>
> what do you think?
Hmm. Can we register an optional function twice? Does that mean
that any module that uses the provider API has to 'optionally'
register these functions? Eek.
I'm starting to like the idea of mod_auth_common. =) -- justin
Re: [PATCH] modules/aaa use optional function to register
Posted by "John K. Sterling" <jo...@sterls.com>.
On Saturday, September 14, 2002, at 08:07 PM, Justin Erenkrantz wrote:
> On Sat, Sep 14, 2002 at 10:45:36AM -0400, John K. Sterling wrote:
>> Hi -
>>
>> Here is the patch that changes the auth providers to use an optional
>> function. It delares the optional function in mod_auth.h, register it
>> in mod_auth_basic.c, and retrieve it in mod_authn_file.c and
>> mod_authn_dbm.c.
>
> Here's the fundamental flaw - I can use providers with
> mod_auth_digest as well.
>
Ouch -
didn't catch that -
If you don't want to have a specific module that registers the
auth_provider optional functions, you could have both modules register
the optional functions..... kinda yicky - but arguably cleaner than
forcing the providers to be loaded after the basic/digest
modules.........
what do you think?
sterling
Re: [PATCH] modules/aaa use optional function to register
Posted by Justin Erenkrantz <je...@apache.org>.
On Sat, Sep 14, 2002 at 10:45:36AM -0400, John K. Sterling wrote:
> Hi -
>
> Here is the patch that changes the auth providers to use an optional
> function. It delares the optional function in mod_auth.h, register it
> in mod_auth_basic.c, and retrieve it in mod_authn_file.c and
> mod_authn_dbm.c.
Here's the fundamental flaw - I can use providers with
mod_auth_digest as well.
There should be no requirement that in order to use mod_auth_digest,
we have to build in mod_auth_basic.
Not that I have an especially good solution, either as my idea for
a mod_auth_common isn't ideal too, but I can't really come up with
anything better... -- justin
Re: [PATCH] modules/aaa use optional function to register
Posted by Dirk-Willem van Gulik <di...@webweaving.org>.
> Here is the patch that changes the auth providers to use an optional
> function. It delares the optional function in mod_auth.h, register it
> in mod_auth_basic.c, and retrieve it in mod_authn_file.c and
> mod_authn_dbm.c.
Nice !
> Notice how the only current auth_providers are dbm and file -- and
> there are no authz providers. When those are switched, they simply
> need to lookup the optional function in the pre_config phase. Even
> though there are no authz providers, I optionalized the register
> method. I did my best to implement this without modifying any current
> logic, so it should be simple to read.
G33z - I am not sure I subscribe to this authZ naming stuff. With all
those Zzzz - it sure feels as if I am reading some script kiddie piece of
code :-)
Dw