Allow expressions in require statements.

[wwaaron]

LocationMatch config statements allow regex match groups to be used in Require expressions. However, mod_auth_openidc was not parsing these expressions, and instead the raw expression text was used directly. This patch uses existing Apache2 methods to parse and evaluate these expressions. These changes are modeled after the implementation in the Apache2 ldap plugin.

If the expression syntax might cause problems for existing statements, then a config option could be introduced to enable or disable expression parsing in order to not break existing configs. However, it seems unlikely that existing Require statements would be misinterpreted after introducing this change.

Unfortunately, this functionality was difficult to test given the requirement to stub the ap_* methods that actually perform expression parsing. Because of this, presently the unit tests simply bypass the expression parsing and use the raw require_args by hiding the require_args in the parsed_require_args->filename pointer and directly returning that pointer as the "parsed" expression. I have manually tested these changes using an expression and observed them to work correctly, but unfortunately they're not well-tested by the test suite, and there doesn't appear to be a simple way to do so.

[zandbelt]

that's interesting and this looks like a pretty clean patch; I'm just curious about an example/use case of how and when one would use this; can you present one?

[wwaaron]

What I'd like to use it for is for a simple dynamic proxy where the request path dictates what backend a request is proxied to, and also dictates the Keycloak role the user requires to access that backend. For example:

  ProxyPassMatch "^/(.*)-backend/(.*)$" "https://$1/$2"
  <LocationMatch "^/(?<backend>.*)-backend/">
    Require claim "roles:%{env:MATCH_BACKEND}"
    AuthType openid-connect
  </LocationMatch>

With this configuration, if someone sends a request to https://myproxy/foo-backend/bar/baz then the request will be proxied to https://foo/bar/baz if and only if the authenticated user has the foo role in Keycloak. Instead of needing to make a block for each backend defined like this, it is useful to have a generic way to do it. This also alleviates the need to reload the Apache2 config when a new backend is brought up, which is very useful if backends are spun up and down somewhat rapidly. This approach appears to work well from my limited testing.

Another, less functional motivation is that since the Apache2 docs indicate that you can use expressions like this in Require statements, but doesn't mention that it requires plugin support to parse expressions, users could get confused if they see this usage in the docs and attempt to apply it to this plugin without knowing that it doesn't support expressions. That's what happened in my case, and it took a bit of sleuthing to figure out why.

There may be a simpler way to do what I'd like to do, but this is a way I found that works.

[zandbelt]

I think it needs:

diff --git a/src/config.c b/src/config.c
index 23f755a..2096f9f 100644
--- a/src/config.c
+++ b/src/config.c
@@ -2359,6 +2359,7 @@
 	return oidc_config_check_merged_vhost_configs(pool, s);
 }
 
+#if MODULE_MAGIC_NUMBER_MAJOR >= 20100714
 static const char *oidc_parse_config(cmd_parms *cmd, const char *require_line,
 		const void **parsed_require_line) {
 	const char *expr_err = NULL;
@@ -2377,7 +2378,6 @@
 	return NULL;
 }
 
-#if MODULE_MAGIC_NUMBER_MAJOR >= 20100714
 static const authz_provider oidc_authz_claim_provider = {
 		&oidc_authz_checker_claim,
 		&oidc_parse_config,