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"),
