test, oauth2: Make sure config test runs field validation (envoyproxy…

…#13496)

This makes sure config tests for oauth2 filter checks fields
validations. This also handles nullptr values for token_secret and
hmac_secret fields due to misconfigurations (for example: token_secret:
{}). Example config in the docs is also updated.

Signed-off-by: Dhi Aurrahman <dio@tetrate.io>

docs/root/configuration/http/http_filters/oauth2_filter.rst

@@ -14,36 +14,82 @@ OAuth2
 Example configuration
 ---------------------
 
-.. code-block::
-
-   http_filters:
-   - name: oauth2
-     typed_config:
-       "@type": type.googleapis.com/envoy.extensions.filters.http.oauth2.v3alpha.OAuth2
-       token_endpoint:
-         cluster: oauth
-         uri: oauth.com/token
-         timeout: 3s
-       authorization_endpoint: https://oauth.com/oauth/authorize/
-       redirect_uri: "%REQ(:x-forwarded-proto)%://%REQ(:authority)%/callback"
-       redirect_path_matcher:
-         path:
-           exact: /callback
-       signout_path:
-         path:
-           exact: /signout
-      credentials:
-        client_id: foo
-        token_secret:
-          name: token
-        hmac_secret:
-          name: hmac
+The following is an example configuring the filter.
+
+.. validated-code-block:: yaml
+  :type-name: envoy.extensions.filters.http.oauth2.v3alpha.OAuth2
+
+  config:
+    token_endpoint:
+      cluster: oauth
+      uri: oauth.com/token
       timeout: 3s
-   - name: envoy.router
+    authorization_endpoint: https://oauth.com/oauth/authorize/
+    redirect_uri: "%REQ(:x-forwarded-proto)%://%REQ(:authority)%/callback"
+    redirect_path_matcher:
+      path:
+        exact: /callback
+    signout_path:
+      path:
+        exact: /signout
+    credentials:
+      client_id: foo
+      token_secret:
+        name: token
+        sds_config:
+          path: "/etc/envoy/token-secret.yaml"
+      hmac_secret:
+        name: hmac
+        sds_config:
+          path: "/etc/envoy/hmac.yaml"
+
+And the below code block is an example of how we employ it as one of
+:ref:`HttpConnectionManager HTTP filters
+<envoy_v3_api_field_extensions.filters.network.http_connection_manager.v3.HttpConnectionManager.http_filters>`
+
+.. code-block:: yaml
+
+  static_resources:
+    listeners:
+    - name:
+      address:
+    filter_chains:
+    - filters:
+      - name: envoy.filters.network.http_connection_manager
+        typed_config:
+          "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
+          http_filters:
+          - name: envoy.filters.http.oauth2
+            typed_config:
+              "@type": type.googleapis.com/envoy.extensions.filters.http.oauth2.v3alpha.OAuth2
+              config:
+                token_endpoint:
+                  cluster: oauth
+                  uri: oauth.com/token
+                  timeout: 3s
+                authorization_endpoint: https://oauth.com/oauth/authorize/
+                redirect_uri: "%REQ(:x-forwarded-proto)%://%REQ(:authority)%/callback"
+                redirect_path_matcher:
+                  path:
+                    exact: /callback
+                signout_path:
+                  path:
+                    exact: /signout
+                credentials:
+                  client_id: foo
+                  token_secret:
+                    name: token
+                    sds_config:
+                      path: "/etc/envoy/token-secret.yaml"
+                  hmac_secret:
+                    name: hmac
+                    sds_config:
+                      path: "/etc/envoy/hmac.yaml"
+          - name: envoy.router
 
   clusters:
   - name: service
-    ...
+    # ...
   - name: auth
     connect_timeout: 5s
     type: LOGICAL_DNS
@@ -53,21 +99,25 @@ Example configuration
       endpoints:
       - lb_endpoints:
         - endpoint:
-            address: { socket_address: { address: auth.example.com, port_value: 443 }}
-    tls_context: { sni: auth.example.com }
+            address:
+              socket_address:
+                address: auth.example.com
+                port_value: 443
+    tls_context:
+      sni: auth.example.com
 
 Notes
 -----
 
-This module does not currently provide much Cross-Site-Request-Forgery protection for the redirect loop
-to the OAuth server and back.
+This module does not currently provide much Cross-Site-Request-Forgery protection for the redirect
+loop to the OAuth server and back.
 
 The service must be served over HTTPS for this filter to work, as the cookies use `;secure`.
 
 Statistics
 ----------
 
-The OAuth filter outputs statistics in the *<stat_prefix>.* namespace.
+The OAuth2 filter outputs statistics in the *<stat_prefix>.* namespace.
 
 .. csv-table::
   :header: Name, Type, Description

source/extensions/filters/http/oauth2/config.cc

@@ -51,24 +51,32 @@ Http::FilterFactoryCb OAuth2Config::createFilterFactoryFromProtoTyped(
   const auto& token_secret = credentials.token_secret();
   const auto& hmac_secret = credentials.hmac_secret();
 
-  auto& secret_manager = context.clusterManager().clusterManagerFactory().secretManager();
+  auto& cluster_manager = context.clusterManager();
+  auto& secret_manager = cluster_manager.clusterManagerFactory().secretManager();
   auto& transport_socket_factory = context.getTransportSocketFactoryContext();
   auto secret_provider_token_secret =
       secretsProvider(token_secret, secret_manager, transport_socket_factory);
+  if (secret_provider_token_secret == nullptr) {
+    throw EnvoyException("invalid token secret configuration");
+  }
   auto secret_provider_hmac_secret =
       secretsProvider(hmac_secret, secret_manager, transport_socket_factory);
+  if (secret_provider_hmac_secret == nullptr) {
+    throw EnvoyException("invalid HMAC secret configuration");
+  }
 
   auto secret_reader = std::make_shared<SDSSecretReader>(
       secret_provider_token_secret, secret_provider_hmac_secret, context.api());
-  auto config = std::make_shared<FilterConfig>(proto_config, context.clusterManager(),
-                                               secret_reader, context.scope(), stats_prefix);
+  auto config = std::make_shared<FilterConfig>(proto_config, cluster_manager, secret_reader,
+                                               context.scope(), stats_prefix);
 
-  return [&context, config](Http::FilterChainFactoryCallbacks& callbacks) -> void {
-    std::unique_ptr<OAuth2Client> oauth_client =
-        std::make_unique<OAuth2ClientImpl>(context.clusterManager(), config->oauthTokenEndpoint());
-    callbacks.addStreamDecoderFilter(
-        std::make_shared<OAuth2Filter>(config, std::move(oauth_client), context.timeSource()));
-  };
+  return
+      [&context, config, &cluster_manager](Http::FilterChainFactoryCallbacks& callbacks) -> void {
+        std::unique_ptr<OAuth2Client> oauth_client =
+            std::make_unique<OAuth2ClientImpl>(cluster_manager, config->oauthTokenEndpoint());
+        callbacks.addStreamDecoderFilter(
+            std::make_shared<OAuth2Filter>(config, std::move(oauth_client), context.timeSource()));
+      };
 }
 
 /*

source/extensions/filters/http/well_known_names.h

@@ -81,7 +81,7 @@ class HttpFilterNameValues {
   // AWS Lambda filter
   const std::string AwsLambda = "envoy.filters.http.aws_lambda";
   // OAuth filter
-  const std::string OAuth = "envoy.filters.http.oauth";
+  const std::string OAuth = "envoy.filters.http.oauth2";
 };
 
 using HttpFilterNames = ConstSingleton<HttpFilterNameValues>;

test/extensions/filters/http/oauth2/config_test.cc

@@ -22,33 +22,102 @@ namespace Oauth2 {
 using testing::NiceMock;
 using testing::Return;
 
+namespace {
+
+// This loads one of the secrets in credentials, and fails the other one.
+void expectInvalidSecretConfig(const std::string& failed_secret_name,
+                               const std::string& exception_message) {
+  const std::string yaml = R"EOF(
+config:
+  token_endpoint:
+    cluster: foo
+    uri: oauth.com/token
+    timeout: 3s
+  credentials:
+    client_id: "secret"
+    token_secret:
+      name: token
+    hmac_secret:
+      name: hmac
+  authorization_endpoint: https://oauth.com/oauth/authorize/
+  redirect_uri: "%REQ(:x-forwarded-proto)%://%REQ(:authority)%/callback"
+  redirect_path_matcher:
+    path:
+      exact: /callback
+  signout_path:
+    path:
+      exact: /signout
+    )EOF";
+
+  OAuth2Config factory;
+  ProtobufTypes::MessagePtr proto_config = factory.createEmptyConfigProto();
+  TestUtility::loadFromYaml(yaml, *proto_config);
+  NiceMock<Server::Configuration::MockFactoryContext> context;
+
+  auto& secret_manager = context.cluster_manager_.cluster_manager_factory_.secretManager();
+  ON_CALL(secret_manager,
+          findStaticGenericSecretProvider(failed_secret_name == "token" ? "hmac" : "token"))
+      .WillByDefault(Return(std::make_shared<Secret::GenericSecretConfigProviderImpl>(
+          envoy::extensions::transport_sockets::tls::v3::GenericSecret())));
+
+  EXPECT_THROW_WITH_MESSAGE(factory.createFilterFactoryFromProto(*proto_config, "stats", context),
+                            EnvoyException, exception_message);
+}
+
+} // namespace
+
 TEST(ConfigTest, CreateFilter) {
   const std::string yaml = R"EOF(
 config:
-    token_endpoint:
-      cluster: foo
-      uri: oauth.com/token
-      timeout: 3s
-    authorization_endpoint: https://oauth.com/oauth/authorize/
-    redirect_uri: "%REQ(:x-forwarded-proto)%://%REQ(:authority)%/callback"
-    signout_path: 
-      path:
-        exact: /signout
+  token_endpoint:
+    cluster: foo
+    uri: oauth.com/token
+    timeout: 3s
+  credentials:
+    client_id: "secret"
+    token_secret:
+      name: token
+    hmac_secret:
+      name: hmac
+  authorization_endpoint: https://oauth.com/oauth/authorize/
+  redirect_uri: "%REQ(:x-forwarded-proto)%://%REQ(:authority)%/callback"
+  redirect_path_matcher:
+    path:
+      exact: /callback
+  signout_path:
+    path:
+      exact: /signout
     )EOF";
 
-  envoy::extensions::filters::http::oauth2::v3alpha::OAuth2 proto_config;
-  MessageUtil::loadFromYaml(yaml, proto_config, ProtobufMessage::getStrictValidationVisitor());
-  NiceMock<Server::Configuration::MockFactoryContext> factory_context;
-  auto& secret_manager = factory_context.cluster_manager_.cluster_manager_factory_.secretManager();
+  OAuth2Config factory;
+  ProtobufTypes::MessagePtr proto_config = factory.createEmptyConfigProto();
+  TestUtility::loadFromYaml(yaml, *proto_config);
+  Server::Configuration::MockFactoryContext context;
+
+  // This returns non-nullptr for token_secret and hmac_secret.
+  auto& secret_manager = context.cluster_manager_.cluster_manager_factory_.secretManager();
   ON_CALL(secret_manager, findStaticGenericSecretProvider(_))
       .WillByDefault(Return(std::make_shared<Secret::GenericSecretConfigProviderImpl>(
           envoy::extensions::transport_sockets::tls::v3::GenericSecret())));
 
-  OAuth2Config config;
-  auto cb = config.createFilterFactoryFromProtoTyped(proto_config, "whatever", factory_context);
+  EXPECT_CALL(context, messageValidationVisitor());
+  EXPECT_CALL(context, clusterManager());
+  EXPECT_CALL(context, scope());
+  EXPECT_CALL(context, timeSource());
+  EXPECT_CALL(context, api());
+  EXPECT_CALL(context, getTransportSocketFactoryContext());
+  Http::FilterFactoryCb cb = factory.createFilterFactoryFromProto(*proto_config, "stats", context);
+  Http::MockFilterChainFactoryCallbacks filter_callback;
+  EXPECT_CALL(filter_callback, addStreamDecoderFilter(_));
+  cb(filter_callback);
+}
+
+TEST(ConfigTest, InvalidTokenSecret) {
+  expectInvalidSecretConfig("token", "invalid token secret configuration");
+}
 
-  NiceMock<Http::MockFilterChainFactoryCallbacks> filter_callbacks;
-  cb(filter_callbacks);
+TEST(ConfigTest, InvalidHmacSecret) {
+  expectInvalidSecretConfig("hmac", "invalid HMAC secret configuration");
 }
 
 TEST(ConfigTest, CreateFilterMissingConfig) {
@@ -65,4 +134,4 @@ TEST(ConfigTest, CreateFilterMissingConfig) {
 } // namespace Oauth2
 } // namespace HttpFilters
 } // namespace Extensions
-} // namespace Envoy
+} // namespace Envoy