jwt_authn: cleanup, separate verifier from the matcher

source/extensions/filters/http/jwt_authn/filter.cc

@@ -26,12 +26,12 @@ Http::FilterHeadersStatus Filter::decodeHeaders(Http::HeaderMap& headers, bool)
   state_ = Calling;
   stopped_ = false;
   // Verify the JWT token, onComplete() will be called when completed.
-  auto matcher = config_->findMatcher(headers);
-  if (!matcher) {
+  const auto* verifier = config_->findVerifier(headers);
+  if (!verifier) {
     onComplete(Status::Ok);
   } else {
     context_ = Verifier::createContext(headers, this);
-    matcher->verifier()->verify(context_);
+    verifier->verify(context_);
   }
 
   if (state_ == Complete) {

source/extensions/filters/http/jwt_authn/filter_config.h

@@ -6,6 +6,7 @@
 #include "envoy/thread_local/thread_local.h"
 
 #include "extensions/filters/http/jwt_authn/matcher.h"
+#include "extensions/filters/http/jwt_authn/verifier.h"
 
 namespace Envoy {
 namespace Extensions {
@@ -71,8 +72,9 @@ class FilterConfig : public Logger::Loggable<Logger::Id::config>, public AuthFac
     extractor_ = Extractor::create(proto_config_);
 
     for (const auto& rule : proto_config_.rules()) {
-      rule_matchers_.push_back(
-          Matcher::create(rule, proto_config_.providers(), *this, getExtractor()));
+      rule_pairs_.emplace_back(
+          Matcher::create(rule),
+          Verifier::create(rule.requires(), proto_config_.providers(), *this, getExtractor()));
     }
   }
 
@@ -94,10 +96,10 @@ class FilterConfig : public Logger::Loggable<Logger::Id::config>, public AuthFac
   const Extractor& getExtractor() const { return *extractor_; }
 
   // Finds the matcher that matched the header
-  virtual const MatcherConstSharedPtr findMatcher(const Http::HeaderMap& headers) const {
-    for (const auto& matcher : rule_matchers_) {
-      if (matcher->matches(headers)) {
-        return matcher;
+  virtual const Verifier* findVerifier(const Http::HeaderMap& headers) const {
+    for (const auto& pair : rule_pairs_) {
+      if (pair.matcher_->matches(headers)) {
+        return pair.verifier_.get();
       }
     }
     return nullptr;
@@ -117,6 +119,13 @@ class FilterConfig : public Logger::Loggable<Logger::Id::config>, public AuthFac
     return {ALL_JWT_AUTHN_FILTER_STATS(POOL_COUNTER_PREFIX(scope, final_prefix))};
   }
 
+  struct MatcherVerifierPair {
+    MatcherVerifierPair(MatcherConstPtr matcher, VerifierConstPtr verifier)
+        : matcher_(std::move(matcher)), verifier_(std::move(verifier)) {}
+    MatcherConstPtr matcher_;
+    VerifierConstPtr verifier_;
+  };
+
   // The proto config.
   ::envoy::config::filter::http::jwt_authn::v2alpha::JwtAuthentication proto_config_;
   // The stats for the filter.
@@ -128,7 +137,7 @@ class FilterConfig : public Logger::Loggable<Logger::Id::config>, public AuthFac
   // The object to extract tokens.
   ExtractorConstPtr extractor_;
   // The list of rule matchers.
-  std::vector<MatcherConstSharedPtr> rule_matchers_;
+  std::vector<MatcherVerifierPair> rule_pairs_;
   TimeSource& time_source_;
 };
 typedef std::shared_ptr<FilterConfig> FilterConfigSharedPtr;

source/extensions/filters/http/jwt_authn/matcher.cc

@@ -1,5 +1,6 @@
 #include "extensions/filters/http/jwt_authn/matcher.h"
 
+#include "common/common/logger.h"
 #include "common/router/config_impl.h"
 
 #include "absl/strings/match.h"
@@ -20,9 +21,7 @@ namespace {
  */
 class BaseMatcherImpl : public Matcher, public Logger::Loggable<Logger::Id::filter> {
 public:
-  BaseMatcherImpl(const RequirementRule& rule,
-                  const Protobuf::Map<ProtobufTypes::String, JwtProvider>& providers,
-                  const AuthFactory& factory, const Extractor& extractor)
+  BaseMatcherImpl(const RequirementRule& rule)
       : case_sensitive_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(rule.match(), case_sensitive, true)) {
 
     for (const auto& header_map : rule.match().headers()) {
@@ -32,8 +31,6 @@ class BaseMatcherImpl : public Matcher, public Logger::Loggable<Logger::Id::filt
     for (const auto& query_parameter : rule.match().query_parameters()) {
       config_query_parameters_.push_back(query_parameter);
     }
-
-    verifier_ = Verifier::create(rule.requires(), providers, factory, extractor);
   }
 
   // Check match for HeaderMatcher and QueryParameterMatcher
@@ -50,26 +47,21 @@ class BaseMatcherImpl : public Matcher, public Logger::Loggable<Logger::Id::filt
     return matches;
   }
 
-  const VerifierPtr& verifier() const override { return verifier_; }
-
 protected:
   const bool case_sensitive_;
 
 private:
   std::vector<Http::HeaderUtility::HeaderData> config_headers_;
   std::vector<Router::ConfigUtility::QueryParameterMatcher> config_query_parameters_;
-  VerifierPtr verifier_;
 };
 
 /**
  * Perform a match against any path with prefix rule.
  */
 class PrefixMatcherImpl : public BaseMatcherImpl {
 public:
-  PrefixMatcherImpl(const RequirementRule& rule,
-                    const Protobuf::Map<ProtobufTypes::String, JwtProvider>& providers,
-                    const AuthFactory& factory, const Extractor& extractor)
-      : BaseMatcherImpl(rule, providers, factory, extractor), prefix_(rule.match().prefix()) {}
+  PrefixMatcherImpl(const RequirementRule& rule)
+      : BaseMatcherImpl(rule), prefix_(rule.match().prefix()) {}
 
   bool matches(const Http::HeaderMap& headers) const override {
     if (BaseMatcherImpl::matchRoute(headers) &&
@@ -92,10 +84,8 @@ class PrefixMatcherImpl : public BaseMatcherImpl {
  */
 class PathMatcherImpl : public BaseMatcherImpl {
 public:
-  PathMatcherImpl(const RequirementRule& rule,
-                  const Protobuf::Map<ProtobufTypes::String, JwtProvider>& providers,
-                  const AuthFactory& factory, const Extractor& extractor)
-      : BaseMatcherImpl(rule, providers, factory, extractor), path_(rule.match().path()) {}
+  PathMatcherImpl(const RequirementRule& rule)
+      : BaseMatcherImpl(rule), path_(rule.match().path()) {}
 
   bool matches(const Http::HeaderMap& headers) const override {
     if (BaseMatcherImpl::matchRoute(headers)) {
@@ -122,11 +112,9 @@ class PathMatcherImpl : public BaseMatcherImpl {
  */
 class RegexMatcherImpl : public BaseMatcherImpl {
 public:
-  RegexMatcherImpl(const RequirementRule& rule,
-                   const Protobuf::Map<ProtobufTypes::String, JwtProvider>& providers,
-                   const AuthFactory& factory, const Extractor& extractor)
-      : BaseMatcherImpl(rule, providers, factory, extractor),
-        regex_(RegexUtil::parseRegex(rule.match().regex())), regex_str_(rule.match().regex()) {}
+  RegexMatcherImpl(const RequirementRule& rule)
+      : BaseMatcherImpl(rule), regex_(RegexUtil::parseRegex(rule.match().regex())),
+        regex_str_(rule.match().regex()) {}
 
   bool matches(const Http::HeaderMap& headers) const override {
     if (BaseMatcherImpl::matchRoute(headers)) {
@@ -149,17 +137,14 @@ class RegexMatcherImpl : public BaseMatcherImpl {
 
 } // namespace
 
-MatcherConstSharedPtr
-Matcher::create(const RequirementRule& rule,
-                const Protobuf::Map<ProtobufTypes::String, JwtProvider>& providers,
-                const AuthFactory& factory, const Extractor& extractor) {
+MatcherConstPtr Matcher::create(const RequirementRule& rule) {
   switch (rule.match().path_specifier_case()) {
   case RouteMatch::PathSpecifierCase::kPrefix:
-    return std::make_shared<PrefixMatcherImpl>(rule, providers, factory, extractor);
+    return std::make_unique<PrefixMatcherImpl>(rule);
   case RouteMatch::PathSpecifierCase::kPath:
-    return std::make_shared<PathMatcherImpl>(rule, providers, factory, extractor);
+    return std::make_unique<PathMatcherImpl>(rule);
   case RouteMatch::PathSpecifierCase::kRegex:
-    return std::make_shared<RegexMatcherImpl>(rule, providers, factory, extractor);
+    return std::make_unique<RegexMatcherImpl>(rule);
   // path specifier is required.
   case RouteMatch::PathSpecifierCase::PATH_SPECIFIER_NOT_SET:
   default:

source/extensions/filters/http/jwt_authn/matcher.h

@@ -1,16 +1,15 @@
 #pragma once
 
+#include "envoy/config/filter/http/jwt_authn/v2alpha/config.pb.h"
 #include "envoy/http/header_map.h"
 
-#include "extensions/filters/http/jwt_authn/verifier.h"
-
 namespace Envoy {
 namespace Extensions {
 namespace HttpFilters {
 namespace JwtAuthn {
 
 class Matcher;
-typedef std::shared_ptr<const Matcher> MatcherConstSharedPtr;
+typedef std::unique_ptr<const Matcher> MatcherConstPtr;
 
 /**
  * Supports matching a HTTP requests with JWT requirements.
@@ -28,27 +27,14 @@ class Matcher {
    */
   virtual bool matches(const Http::HeaderMap& headers) const PURE;
 
-  /**
-   * Returns the configured verifier for this route.
-   *
-   * @return reference to verifier pointer.
-   */
-  virtual const VerifierPtr& verifier() const PURE;
-
   /**
    * Factory method to create a shared instance of a matcher based on the rule defined.
    *
    * @param rule  the proto rule match message.
-   * @param providers  the provider name to config map
-   * @param factory  the Authenticator factory
    * @return the matcher instance.
    */
-  static MatcherConstSharedPtr
-  create(const ::envoy::config::filter::http::jwt_authn::v2alpha::RequirementRule& rule,
-         const Protobuf::Map<ProtobufTypes::String,
-                             ::envoy::config::filter::http::jwt_authn::v2alpha::JwtProvider>&
-             providers,
-         const AuthFactory& factory, const Extractor& extractor);
+  static MatcherConstPtr
+  create(const ::envoy::config::filter::http::jwt_authn::v2alpha::RequirementRule& rule);
 };
 
 } // namespace JwtAuthn

source/extensions/filters/http/jwt_authn/verifier.cc

@@ -179,10 +179,10 @@ class AllowFailedVerifierImpl : public BaseVerifierImpl {
   const Extractor& extractor_;
 };
 
-VerifierPtr innerCreate(const JwtRequirement& requirement,
-                        const Protobuf::Map<ProtobufTypes::String, JwtProvider>& providers,
-                        const AuthFactory& factory, const Extractor& extractor,
-                        const BaseVerifierImpl* parent);
+VerifierConstPtr innerCreate(const JwtRequirement& requirement,
+                             const Protobuf::Map<ProtobufTypes::String, JwtProvider>& providers,
+                             const AuthFactory& factory, const Extractor& extractor,
+                             const BaseVerifierImpl* parent);
 
 // Base verifier for requires all or any.
 class BaseGroupVerifierImpl : public BaseVerifierImpl {
@@ -201,7 +201,7 @@ class BaseGroupVerifierImpl : public BaseVerifierImpl {
 
 protected:
   // The list of requirement verifiers
-  std::vector<VerifierPtr> verifiers_;
+  std::vector<VerifierConstPtr> verifiers_;
 };
 
 // Requires any verifier.
@@ -264,10 +264,10 @@ class AllowAllVerifierImpl : public BaseVerifierImpl {
   }
 };
 
-VerifierPtr innerCreate(const JwtRequirement& requirement,
-                        const Protobuf::Map<ProtobufTypes::String, JwtProvider>& providers,
-                        const AuthFactory& factory, const Extractor& extractor_for_allow_fail,
-                        const BaseVerifierImpl* parent) {
+VerifierConstPtr innerCreate(const JwtRequirement& requirement,
+                             const Protobuf::Map<ProtobufTypes::String, JwtProvider>& providers,
+                             const AuthFactory& factory, const Extractor& extractor_for_allow_fail,
+                             const BaseVerifierImpl* parent) {
   std::string provider_name;
   std::vector<std::string> audiences;
   switch (requirement.requires_type_case()) {
@@ -311,10 +311,10 @@ ContextSharedPtr Verifier::createContext(Http::HeaderMap& headers, Callbacks* ca
   return std::make_shared<ContextImpl>(headers, callback);
 }
 
-VerifierPtr Verifier::create(const JwtRequirement& requirement,
-                             const Protobuf::Map<ProtobufTypes::String, JwtProvider>& providers,
-                             const AuthFactory& factory,
-                             const Extractor& extractor_for_allow_fail) {
+VerifierConstPtr
+Verifier::create(const JwtRequirement& requirement,
+                 const Protobuf::Map<ProtobufTypes::String, JwtProvider>& providers,
+                 const AuthFactory& factory, const Extractor& extractor_for_allow_fail) {
   return innerCreate(requirement, providers, factory, extractor_for_allow_fail, nullptr);
 }
 

source/extensions/filters/http/jwt_authn/verifier.h

@@ -8,7 +8,7 @@ namespace HttpFilters {
 namespace JwtAuthn {
 
 class Verifier;
-typedef std::unique_ptr<Verifier> VerifierPtr;
+typedef std::unique_ptr<const Verifier> VerifierConstPtr;
 
 /**
  * Supports verification of JWTs with configured requirments.
@@ -71,7 +71,7 @@ class Verifier {
   virtual void verify(ContextSharedPtr context) const PURE;
 
   // Factory method for creating verifiers.
-  static VerifierPtr
+  static VerifierConstPtr
   create(const ::envoy::config::filter::http::jwt_authn::v2alpha::JwtRequirement& requirement,
          const Protobuf::Map<ProtobufTypes::String,
                              ::envoy::config::filter::http::jwt_authn::v2alpha::JwtProvider>&

test/extensions/filters/http/jwt_authn/all_verifier_test.cc

@@ -29,7 +29,7 @@ class AllVerifierTest : public ::testing::Test {
 
   JwtAuthentication proto_config_;
   FilterConfigSharedPtr filter_config_;
-  VerifierPtr verifier_;
+  VerifierConstPtr verifier_;
   NiceMock<Server::Configuration::MockFactoryContext> mock_factory_ctx_;
   ContextSharedPtr context_;
   MockVerifierCallbacks mock_cb_;