jwt_authn: add missing path_match_policy matcher implementation (envo…

…yproxy#35072) Commit Message: Added a matcher implementation for JWT. Right now the [documentation](https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto.html#config-route-v3-routematch) says that `path_match_policy` may be used, but currently it throws exception. Additional Description: Previously I implemented it for [rbac only](envoyproxy#31447). Would be nice if it could be included as a bug fix in the current Envoy version. Risk Level: Low? Testing: unit test Docs Changes: do not know how to handle it if it would be merged before new major release. Release Notes: Platform Specific Features: --------- Signed-off-by: jan.kozlowski <[email protected]> Signed-off-by: kozjan <[email protected]> Co-authored-by: yanavlasov <[email protected]>
martinduke · Aug 8, 2024 · b1dca5a · b1dca5a
1 parent 511e4e5
commit b1dca5a
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 4 deletions.
diff --git a/changelogs/current.yaml b/changelogs/current.yaml
@@ -122,6 +122,9 @@ removed_config_or_runtime:
     Removed ``envoy.reloadable_features.normalize_host_for_preresolve_dfp_dns`` runtime flag and legacy code paths.
 
 new_features:
+- area: jwt_authn
+  change: |
+    Added missing implementation to jwt_authn matchers to allow glob pattern matching.
 - area: tls
   change: |
     Added :ref:`prefer_client_ciphers

diff --git a/source/extensions/filters/http/jwt_authn/matcher.cc b/source/extensions/filters/http/jwt_authn/matcher.cc
@@ -7,6 +7,7 @@
 #include "source/common/common/matchers.h"
 #include "source/common/common/regex.h"
 #include "source/common/http/path_utility.h"
+#include "source/common/protobuf/message_validator_impl.h"
 #include "source/common/router/config_impl.h"
 
 #include "absl/strings/match.h"
@@ -188,6 +189,43 @@ class PathSeparatedPrefixMatcherImpl : public BaseMatcherImpl {
 };
 } // namespace
 
+class PathMatchPolicyMatcherImpl : public BaseMatcherImpl {
+public:
+  PathMatchPolicyMatcherImpl(const RequirementRule& rule,
+                             Server::Configuration::CommonFactoryContext& context)
+      : BaseMatcherImpl(rule, context), uri_template_matcher_(createUriTemplateMatcher(rule)) {}
+
+  bool matches(const Http::RequestHeaderMap& headers) const override {
+    if (BaseMatcherImpl::matchRoute(headers) &&
+        uri_template_matcher_->match(headers.getPathValue())) {
+      ENVOY_LOG(debug, "Path match policy requirement '{}' matched.",
+                uri_template_matcher_->uriTemplate());
+      return true;
+    }
+
+    return false;
+  }
+
+private:
+  const Router::PathMatcherSharedPtr uri_template_matcher_;
+
+  static Router::PathMatcherSharedPtr createUriTemplateMatcher(const RequirementRule& rule) {
+    auto& factory = Config::Utility::getAndCheckFactory<Router::PathMatcherFactory>(
+        rule.match().path_match_policy());
+    ProtobufTypes::MessagePtr config = Envoy::Config::Utility::translateAnyToFactoryConfig(
+        rule.match().path_match_policy().typed_config(),
+        ProtobufMessage::getStrictValidationVisitor(), factory);
+
+    absl::StatusOr<Router::PathMatcherSharedPtr> matcher = factory.createPathMatcher(*config);
+
+    if (!matcher.ok()) {
+      throw EnvoyException(std::string(matcher.status().message()));
+    }
+
+    return matcher.value();
+  }
+};
+
 MatcherConstPtr Matcher::create(const RequirementRule& rule,
                                 Server::Configuration::CommonFactoryContext& context) {
   switch (rule.match().path_specifier_case()) {
@@ -201,10 +239,9 @@ MatcherConstPtr Matcher::create(const RequirementRule& rule,
     return std::make_unique<ConnectMatcherImpl>(rule, context);
   case RouteMatch::PathSpecifierCase::kPathSeparatedPrefix:
     return std::make_unique<PathSeparatedPrefixMatcherImpl>(rule, context);
-  case RouteMatch::PathSpecifierCase::kPathMatchPolicy:
-    // TODO(silverstar194): Implement matcher for template based match
-    throw EnvoyException("RouteMatch: path_match_policy is not supported");
-    break;
+  case RouteMatch::PathSpecifierCase::kPathMatchPolicy: {
+    return std::make_unique<PathMatchPolicyMatcherImpl>(rule, context);
+  }
   case RouteMatch::PathSpecifierCase::PATH_SPECIFIER_NOT_SET:
     break; // Fall through to PANIC.
   }

diff --git a/test/extensions/filters/http/jwt_authn/matcher_test.cc b/test/extensions/filters/http/jwt_authn/matcher_test.cc
@@ -253,6 +253,30 @@ TEST_F(MatcherTest, TestMatchPathSeparatedPrefixBaseCondition) {
   EXPECT_FALSE(matcher->matches(headers));
 }
 
+TEST_F(MatcherTest, TestMatchPathMatchPolicy) {
+  const char config[] = R"(match:
+  path_match_policy:
+    name: envoy.path.match.uri_template.uri_template_matcher
+    typed_config:
+      "@type": type.googleapis.com/envoy.extensions.path.match.uri_template.v3.UriTemplateMatchConfig
+      path_template: "/bar/*/foo"
+  )";
+  MatcherConstPtr matcher = createMatcher(config);
+  auto headers = TestRequestHeaderMapImpl{{":path", "/bar/test/foo"}};
+  EXPECT_TRUE(matcher->matches(headers));
+}
+
+TEST_F(MatcherTest, TestMatchPathMatchPolicyError) {
+  const char config[] = R"(match:
+  path_match_policy:
+    name: envoy.path.match.uri_template.uri_template_matcher
+    typed_config:
+      "@type": type.googleapis.com/envoy.extensions.path.match.uri_template.v3.UriTemplateMatchConfig
+      wrong_key: "/bar/*/foo"
+  )";
+  EXPECT_THROW_WITH_REGEX(createMatcher(config), EnvoyException, "INVALID_ARGUMENT")
+}
+
 } // namespace
 } // namespace JwtAuthn
 } // namespace HttpFilters