httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chris Darroch <chr...@pearsoncmg.com>
Subject Re: Authz directives
Date Tue, 09 Dec 2008 18:30:47 GMT
Roy T. Fielding wrote:

> I totally understand the desire to make the implementation more
> modular and to make a more sensible Satisfy logic, but I don't
> understand the need for Match (as opposed to just extending Require)
> and the odd changes in defaults (multiple Require defaults to
> MatchAny semantics, but multiple Match defaults to MatchAll).

   Match came up only because negated containers cause the meaning
of a plain old Require to be inverted and to have the opposite meaning
from what it appears.  If we drop negated containers that problem
goes away.  The negated containers, as I mentioned previously, were
just something which fell out easily from the refactoring; they're
hardly worth keeping if they just cause trouble.


> 1) make the new directives self-documenting
> 
>     remove MatchNotAll (nobody needs this)
> 
>     s/MatchAny/RequireAny/ig;
>     s/MatchAll/RequireAll/ig;
>     s/MatchNotAny/RequireNone/ig;
> 
>     s/(MergeAuthz|AuthzMerge)/AuthMerging/ig;   (off | and | or)
> 
> 2) move new Match functionality to Require
> 
> 3) default for multiple Require* is RequireAny
>     - implies that "Require" and "Require not" are only mixed when
>       used within a RequireAll or RequireNone container.

   This is all fairly simple, I think, especially if
MatchNotAny/RequireNone is removed as well so that Require retains its
apparent meaning everywhere.  See if the patch below meets your
expectations; if so, I'll commit it and update the docs.  The next
item on the agenda, then, is probably PR #46214, i.e., getting the
various providers to quiet down their log messages.

Chris.

=========================================================
--- modules/aaa/mod_authz_core.c	(revision 724744)
+++ modules/aaa/mod_authz_core.c	(working copy)
@@ -52,17 +52,19 @@
 
 #define FORMAT_AUTHZ_COMMAND(p,section) \
     ((section)->provider                  \
-     ? apr_pstrcat((p),                     \
+     ? apr_pstrcat((p), "Require ",         \
                    ((section)->negate ?       \
-                    "Match not " : "Match "),    \
+                    "not " : ""),               \
                    (section)->provider_name, " ", \
                    (section)->provider_args, NULL)  \
-     : apr_pstrcat((p), "<Match",                     \
+     : apr_pstrcat((p), "<Require",                   \
                    ((section)->negate ? "Not" : ""),    \
                    (((section)->op == AUTHZ_LOGIC_AND)    \
                     ? "All" : "Any"),                       \
-                   ">", NULL))                                \
+                   ">", NULL))
 
+#undef AUTHZ_NEGATIVE_CONFIGS
+
 typedef struct provider_alias_rec {
     char *provider_name;
     char *provider_alias;
@@ -95,7 +97,6 @@
 struct authz_core_dir_conf {
     authz_section_conf *section;
     authz_logic_op op;
-    int old_require;
     authz_core_dir_conf *next;
 };
 
@@ -314,12 +315,11 @@
     return errmsg;
 }
 
-static authz_section_conf* create_default_section(apr_pool_t *p,
-                                                  int old_require)
+static authz_section_conf* create_default_section(apr_pool_t *p)
 {
     authz_section_conf *section = apr_pcalloc(p, sizeof(*section));
 
-    section->op = old_require ? AUTHZ_LOGIC_OR : AUTHZ_LOGIC_AND;
+    section->op = AUTHZ_LOGIC_OR;
 
     return section;
 }
@@ -331,21 +331,9 @@
     authz_section_conf *section = apr_pcalloc(cmd->pool, sizeof(*section));
     authz_section_conf *child;
 
-    if (!strcasecmp(cmd->cmd->name, "Require")) {
-        if (conf->section && !conf->old_require) {
-            return "Require directive not allowed with "
-                   "Match and related directives";
-        }
-
-        conf->old_require = 1;
-    }
-    else if (conf->old_require) {
-        return "Match directive not allowed with Require directives";
-    }
-
     section->provider_name = ap_getword_conf(cmd->pool, &args);
 
-    if (!conf->old_require && !strcasecmp(section->provider_name, "not")) {
+    if (!strcasecmp(section->provider_name, "not")) {
         section->provider_name = ap_getword_conf(cmd->pool, &args);
         section->negate = 1;
     }
@@ -377,7 +365,7 @@
     section->limited = cmd->limited;
 
     if (!conf->section) {
-        conf->section = create_default_section(cmd->pool, conf->old_require);
+        conf->section = create_default_section(cmd->pool);
     }
 
     if (section->negate && conf->section->op == AUTHZ_LOGIC_OR) {
@@ -416,12 +404,6 @@
     apr_int64_t old_limited = cmd->limited;
     const char *errmsg;
 
-    if (conf->old_require) {
-        return apr_pstrcat(cmd->pool, cmd->cmd->name,
-                           "> directive not allowed with "
-                           "Require directives", NULL);
-    }
-
     if (endp == NULL) {
         return apr_pstrcat(cmd->pool, cmd->cmd->name,
                            "> directive missing closing '>'", NULL);
@@ -437,13 +419,14 @@
 
     section = apr_pcalloc(cmd->pool, sizeof(*section));
 
-    if (!strcasecmp(cmd->cmd->name, "<MatchAll")) {
+    if (!strcasecmp(cmd->cmd->name, "<RequireAll")) {
         section->op = AUTHZ_LOGIC_AND;
     }
-    else if (!strcasecmp(cmd->cmd->name, "<MatchAny")) {
+    else if (!strcasecmp(cmd->cmd->name, "<RequireAny")) {
         section->op = AUTHZ_LOGIC_OR;
     }
-    else if (!strcasecmp(cmd->cmd->name, "<MatchNotAll")) {
+#ifdef AUTHZ_NEGATIVE_CONFIGS
+    else if (!strcasecmp(cmd->cmd->name, "<RequireNotAll")) {
         section->op = AUTHZ_LOGIC_AND;
         section->negate = 1;
     }
@@ -451,6 +434,7 @@
         section->op = AUTHZ_LOGIC_OR;
         section->negate = 1;
     }
+#endif
 
     conf->section = section;
 
@@ -473,7 +457,7 @@
         authz_section_conf *child;
 
         if (!old_section) {
-            old_section = conf->section = create_default_section(cmd->pool, 0);
+            old_section = conf->section = create_default_section(cmd->pool);
         }
 
         if (section->negate && old_section->op == AUTHZ_LOGIC_OR) {
@@ -521,15 +505,15 @@
     if (!strcasecmp(arg, "Off")) {
         conf->op = AUTHZ_LOGIC_OFF;
     }
-    else if (!strcasecmp(arg, "MatchAll")) {
+    else if (!strcasecmp(arg, "And")) {
         conf->op = AUTHZ_LOGIC_AND;
     }
-    else if (!strcasecmp(arg, "MatchAny")) {
+    else if (!strcasecmp(arg, "Or")) {
         conf->op = AUTHZ_LOGIC_OR;
     }
     else {
         return apr_pstrcat(cmd->pool, cmd->cmd->name, " must be one of: "
-                           "Off | MatchAll | MatchAny", NULL);
+                           "Off | And | Or", NULL);
     }
 
     return NULL;
@@ -612,7 +596,7 @@
     authz_core_dir_conf *conf = authz_core_first_dir_conf;
 
     while (conf) {
-        if (conf->section && !conf->old_require) {
+        if (conf->section) {
             if (authz_core_check_section(p, s, conf->section, 1) != OK) {
                 return !OK;
             }
@@ -631,28 +615,26 @@
                      "container for grouping an authorization provider's "
                      "directives under a provider alias"),
     AP_INIT_RAW_ARGS("Require", add_authz_provider, NULL, OR_AUTHCFG,
-                     "specifies legacy authorization directives "
-                     "of which one must pass "
-                     "for a request to suceeed"),
-    AP_INIT_RAW_ARGS("Match", add_authz_provider, NULL, OR_AUTHCFG,
-                     "specifies authorization directives that must pass "
-                     "(or not) for a request to suceeed"),
-    AP_INIT_RAW_ARGS("<MatchAll", add_authz_section, NULL, OR_AUTHCFG,
+                     "specifies authorization directives "
+                     "which one must pass (or not) for a request to suceeed"),
+    AP_INIT_RAW_ARGS("<RequireAll", add_authz_section, NULL, OR_AUTHCFG,
                      "container for grouping authorization directives "
                      "of which none must fail and at least one must pass "              
            "for a request to succeed"),
-    AP_INIT_RAW_ARGS("<MatchAny", add_authz_section, NULL, OR_AUTHCFG,
+    AP_INIT_RAW_ARGS("<RequireAny", add_authz_section, NULL, OR_AUTHCFG,
                      "container for grouping authorization directives "
                      "of which one must pass "
                      "for a request to succeed"),
-    AP_INIT_RAW_ARGS("<MatchNotAll", add_authz_section, NULL, OR_AUTHCFG,
+#ifdef AUTHZ_NEGATIVE_CONFIGS
+    AP_INIT_RAW_ARGS("<RequireNotAll", add_authz_section, NULL, OR_AUTHCFG,
                      "container for grouping authorization directives "
                      "of which some must fail or none must pass "
                      "for a request to succeed"),
-    AP_INIT_RAW_ARGS("<MatchNotAny", add_authz_section, NULL, OR_AUTHCFG,
+    AP_INIT_RAW_ARGS("<RequireNotAny", add_authz_section, NULL, OR_AUTHCFG,
                      "container for grouping authorization directives "
                      "of which none must pass "
                      "for a request to succeed"),
-    AP_INIT_TAKE1("MergeAuthz", authz_merge_sections, NULL, OR_AUTHCFG,
+#endif
+    AP_INIT_TAKE1("AuthMerging", authz_merge_sections, NULL, OR_AUTHCFG,
                   "controls how a <Directory>, <Location>, or similar "
                   "directive's authorization directives are combined with "
                   "those of its predecessor"),

Mime
View raw message