On Sep 5, 2005, at 3:15 AM, Graham Leggett wrote:
Ryan Morgan wrote:
require ldap-group cn=Engineering,ou=Groups,o=SomeCompany,c=US
require ldap-group cn=QA,ou=Groups,o=SomeCompany,c=US
satisfy all
Could someone provide feedback on whether this is a feature that
could be
added to the ldap module?
Definitely a +1 in concept. Do the other authz modules handle
satisfy all in the same way?
Great.. Thanks for taking a look Graham.
Other than mod_access, none of the other authz modules handle the
satisfy
directive. mod_access uses it to specify how to handle authorization
when
both the require and allow directives are used.
This patch builds on that concept, but handles the case where
multiple require
lines are present. I figured using satisfy made more sense than
adding another
directive to the ldap module. It's entirely possible that satisfy
was not meant
to be used this way, but it seems to fit in nicely.
A quick eyeball of the patch shows up some C++ comments - can you
convert them to C comments?
Sorry about that, attached is an updated patch.
-Ryan
Index: modules/aaa/mod_authnz_ldap.c
===================================================================
--- modules/aaa/mod_authnz_ldap.c (revision 278618)
+++ modules/aaa/mod_authnz_ldap.c (working copy)
@@ -477,6 +477,9 @@
const apr_array_header_t *reqs_arr = ap_requires(r);
require_line *reqs = reqs_arr ? (require_line *)reqs_arr->elts : NULL;
+ int satisfy = ap_satisfies(r);
+ int require_failed = 0; /* flag for satisfy all */
+ int req_user_success, req_group_success, req_attribute_success;
register int x;
const char *t;
@@ -602,9 +605,13 @@
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
"[%" APR_PID_T_FMT "] auth_ldap authorise: "
"require user: authorisation successful",
getpid());
- return OK;
+ if (satisfy != SATISFY_ALL) {
+ return OK;
+ }
+ break;
}
default: {
+ require_failed = 1;
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
"[%" APR_PID_T_FMT "] auth_ldap authorise:
require user: "
"authorisation failed [%s][%s]", getpid(),
@@ -614,24 +621,31 @@
/*
* Now break apart the line and compare each word on it
*/
+ req_user_success = 0;
while (t[0]) {
w = ap_getword_conf(r->pool, &t);
result = util_ldap_cache_compare(r, ldc, sec->url, req->dn,
sec->attribute, w);
- switch(result) {
- case LDAP_COMPARE_TRUE: {
- ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
- "[%" APR_PID_T_FMT "] auth_ldap
authorise: "
- "require user: authorisation
successful", getpid());
+
+ if (result == LDAP_COMPARE_TRUE) {
+ ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
+ "[%" APR_PID_T_FMT "] auth_ldap authorise: "
+ "require user: authorisation successful",
getpid());
+ if (satisfy != SATISFY_ALL) {
return OK;
}
- default: {
- ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
- "[%" APR_PID_T_FMT "] auth_ldap
authorise: "
- "require user: authorisation failed
[%s][%s]",
- getpid(), ldc->reason,
ldap_err2string(result));
- }
+ req_user_success = 1;
+ break;
+ } else {
+ ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
+ "[%" APR_PID_T_FMT "] auth_ldap authorise: "
+ "require user: authorisation failed
[%s][%s]",
+ getpid(), ldc->reason,
ldap_err2string(result));
}
}
+ if (!req_user_success) {
+ /* all user checks failed for this require line */
+ require_failed = 1;
+ }
}
else if (strcmp(w, "ldap-dn") == 0) {
if (req->dn == NULL || strlen(req->dn) == 0) {
@@ -648,9 +662,13 @@
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
"[%" APR_PID_T_FMT "] auth_ldap authorise: "
"require dn: authorisation successful",
getpid());
- return OK;
+ if (satisfy != SATISFY_ALL) {
+ return OK;
+ }
+ break;
}
default: {
+ require_failed = 1;
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
"[%" APR_PID_T_FMT "] auth_ldap authorise: "
"require dn \"%s\": LDAP error [%s][%s]",
@@ -684,6 +702,7 @@
"testing for group membership in \"%s\"",
getpid(), t);
+ req_group_success = 0;
for (i = 0; i < sec->groupattr->nelts; i++) {
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
"[%" APR_PID_T_FMT "] auth_ldap authorise:
require group: "
@@ -692,22 +711,27 @@
result = util_ldap_cache_compare(r, ldc, sec->url, t,
ent[i].name,
sec->group_attrib_is_dn ? req->dn :
req->user);
- switch(result) {
- case LDAP_COMPARE_TRUE: {
- ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
- "[%" APR_PID_T_FMT "] auth_ldap
authorise: require group: "
- "authorisation successful (attribute %s)
[%s][%s]",
- getpid(), ent[i].name, ldc->reason,
ldap_err2string(result));
+ if (result == LDAP_COMPARE_TRUE) {
+ ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
+ "[%" APR_PID_T_FMT "] auth_ldap authorise:
require group: "
+ "authorisation successful (attribute %s)
[%s][%s]",
+ getpid(), ent[i].name, ldc->reason,
ldap_err2string(result));
+ if (satisfy != SATISFY_ALL) {
return OK;
}
- default: {
- ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
- "[%" APR_PID_T_FMT "] auth_ldap
authorise: require group \"%s\": "
- "authorisation failed [%s][%s]",
- getpid(), t, ldc->reason,
ldap_err2string(result));
- }
+ req_group_success = 1;
+ break;
+ } else {
+ ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
+ "[%" APR_PID_T_FMT "] auth_ldap authorise:
require group \"%s\": "
+ "authorisation failed [%s][%s]",
+ getpid(), t, ldc->reason,
ldap_err2string(result));
}
}
+ if (!req_group_success) {
+ /* all group checks failed for this require line */
+ require_failed = 1;
+ }
}
else if (strcmp(w, "ldap-attribute") == 0) {
if (req->dn == NULL || strlen(req->dn) == 0) {
@@ -717,6 +741,8 @@
getpid());
return sec->auth_authoritative? HTTP_UNAUTHORIZED : DECLINED;
}
+
+ req_attribute_success = 0;
while (t[0]) {
w = ap_getword(r->pool, &t, '=');
value = ap_getword_conf(r->pool, &t);
@@ -726,23 +752,29 @@
" %s has value %s", getpid(), w, value);
result = util_ldap_cache_compare(r, ldc, sec->url, req->dn,
w, value);
- switch(result) {
- case LDAP_COMPARE_TRUE: {
- ap_log_rerror(APLOG_MARK, APLOG_DEBUG,
- 0, r, "[%" APR_PID_T_FMT "] auth_ldap
authorise: "
- "require attribute: authorisation "
- "successful", getpid());
+
+ if (result == LDAP_COMPARE_TRUE) {
+ ap_log_rerror(APLOG_MARK, APLOG_DEBUG,
+ 0, r, "[%" APR_PID_T_FMT "] auth_ldap
authorise: "
+ "require attribute: authorisation "
+ "successful", getpid());
+ if (satisfy != SATISFY_ALL) {
return OK;
}
- default: {
- ap_log_rerror(APLOG_MARK, APLOG_DEBUG,
- 0, r, "[%" APR_PID_T_FMT "] auth_ldap
authorise: "
- "require attribute: authorisation "
- "failed [%s][%s]", getpid(),
- ldc->reason, ldap_err2string(result));
- }
+ req_attribute_success = 1;
+ break;
+ } else {
+ ap_log_rerror(APLOG_MARK, APLOG_DEBUG,
+ 0, r, "[%" APR_PID_T_FMT "] auth_ldap
authorise: "
+ "require attribute: authorisation "
+ "failed [%s][%s]", getpid(),
+ ldc->reason, ldap_err2string(result));
}
}
+ if (!req_attribute_success) {
+ /* all attributes failed for this require line */
+ require_failed = 1;
+ }
}
else if (strcmp(w, "ldap-filter") == 0) {
if (req->dn == NULL || strlen(req->dn) == 0) {
@@ -779,9 +811,13 @@
0, r, "[%" APR_PID_T_FMT "] auth_ldap
authorise: "
"require ldap-filter: authorisation "
"successful", getpid());
- return OK;
+ if (satisfy != SATISFY_ALL) {
+ return OK;
+ }
+ break;
}
case LDAP_FILTER_ERROR: {
+ require_failed = 1;
ap_log_rerror(APLOG_MARK, APLOG_DEBUG,
0, r, "[%" APR_PID_T_FMT "] auth_ldap
authorise: "
"require ldap-filter: %s authorisation "
@@ -790,6 +826,7 @@
break;
}
default: {
+ require_failed = 1;
ap_log_rerror(APLOG_MARK, APLOG_DEBUG,
0, r, "[%" APR_PID_T_FMT "] auth_ldap
authorise: "
"require ldap-filter: authorisation "
@@ -808,6 +845,19 @@
return OK;
}
+ if (satisfy == SATISFY_ALL) {
+ if (require_failed) {
+ ap_log_rerror(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r,
+ "[%d] auth_ldap authorise: satisfy all specified, "
+ "but not all requirements were met.", getpid());
+ } else {
+ ap_log_rerror(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r,
+ "[%d] auth_ldap authorise: user satisfies all "
+ "requirements", getpid());
+ return OK;
+ }
+ }
+
if (!sec->auth_authoritative) {
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
"[%" APR_PID_T_FMT "] auth_ldap authorise: declining to
authorise", getpid());
