(TK-497) Allow requiring authorization via config setting

documentation/configuration.md

@@ -80,6 +80,10 @@ metrics: {
             # Default is true.
             enabled: false
 
+            # Enable or disable authorization for metrics endpoint
+            # Default is true.
+            require-auth: true
+
             # Configure any of the settings listed at:
             #   https://jolokia.org/reference/html/agents.html#war-agent-installation
             servlet-init-params: {

src/clj/puppetlabs/trapperkeeper/services/metrics/jolokia.clj

@@ -84,7 +84,10 @@
                        {:authorized true :message ""}))]
     (if auth-check-fn
       (log/info "Metrics access control using trapperkeeper-authorization is enabled.")
-      (log/warn "Metrics access control using trapperkeeper-authorization is disabled. Add the authorization service to the trapperkeeper bootstrap configuration file to enable it."))
+      (log/warn (str "Metrics access control using trapperkeeper-authorization is disabled. "
+                     "To enable it, add the authorization service to the trapperkeeper bootstrap "
+                     "configuration file and set the trapperkeeper-metrics config setting "
+                     "metrics.metrics-webservice.jolokia.require-auth to true (default).")))
     (proxy [AgentServlet] []
      ;; NOTE: An alternative to this method override would be to use defrecord
      ;; to create a class that can be set as `:logHandlerClass` in the servlet

src/clj/puppetlabs/trapperkeeper/services/metrics/metrics_core.clj

@@ -27,6 +27,7 @@
 
 (def JolokiaApiConfig
   {(schema/optional-key :enabled) schema/Bool
+   (schema/optional-key :require-auth) schema/Bool
    (schema/optional-key :servlet-init-params) jolokia/JolokiaConfig})
 
 (def MbeansApiConfig

src/clj/puppetlabs/trapperkeeper/services/metrics/metrics_service.clj

@@ -81,12 +81,21 @@
             options (if (nil? server)
                       {:servlet-init-params config}
                       {:servlet-init-params config :server-id (keyword server)})
-            auth-service (tk-services/maybe-get-service this :AuthorizationService)
-            auth-check-fn (if auth-service (partial tk-auth/authorization-check auth-service))]
-        (add-servlet-handler
-         (jolokia/create-servlet auth-check-fn)
-         route
-         options)))
+            require-auth? (get-in-config [:metrics :metrics-webservice :jolokia :require-auth] true)]
+        (if-not require-auth?
+          (add-servlet-handler (jolokia/create-servlet nil) route options)
+          (try
+            (let [auth-service (tk-services/get-service this :AuthorizationService)
+                  auth-check-fn (partial tk-auth/authorization-check auth-service)
+                  servlet (jolokia/create-servlet auth-check-fn)]
+              (add-servlet-handler servlet route options))
+            ;; Fail fast when auth is required but the auth service could not load
+            (catch IllegalArgumentException e
+              (log/error "metrics.metrics-webservice.jolokia.require-auth set to true (default)"
+                         "but trapperkeeper-authorization service was not found. To disable"
+                         "authorization change this setting to false.")
+              (throw e))))))
+
 
     context)
 

test/puppetlabs/trapperkeeper/services/metrics/metrics_service_test.clj

@@ -63,6 +63,7 @@
 
 (def metrics-service-config
   {:metrics {:server-id "localhost"
+             :metrics-webservice {:jolokia {:require-auth false}}
              :registries {:pl.test.reg {:reporters {:jmx {:enabled true}}}
                           :pl.other.reg {:reporters {:jmx {:enabled true}}}}}
    :webserver {:port 8180
@@ -226,15 +227,31 @@
             (is (= 403 (get body "status")))))))))
 
 (deftest metrics-service-with-tk-auth
-  (testing "tk-auth works when included in bootstrap"
-    (with-app-with-config
-      app
-      (conj services authorization-service/authorization-service)
-      (merge metrics-service-config auth-config ssl-webserver-config)
-      (let [resp (http-client/get "https://localhost:8180/metrics/v2/list" ssl-opts)]
-        (is (= 200 (:status resp))))
-      (let [resp (http-client/get "https://localhost:8180/metrics/v2" ssl-opts)]
-        (is (= 403 (:status resp)))))))
+  (let [config-with-require-auth
+        (-> (merge metrics-service-config auth-config ssl-webserver-config)
+          (assoc-in [:metrics :metrics-webservice :jolokia :require-auth] true))]
+
+    (testing "tk-auth works when included in bootstrap"
+      (with-app-with-config
+        app
+        (conj services authorization-service/authorization-service)
+        ;; This test requires authentication so we need to override the require-auth
+        ;; option that is set to false in metrics-service-config
+        config-with-require-auth
+        (let [resp (http-client/get "https://localhost:8180/metrics/v2/list" ssl-opts)]
+          (is (= 200 (:status resp))))
+        (let [resp (http-client/get "https://localhost:8180/metrics/v2" ssl-opts)]
+          (is (= 403 (:status resp))))))
+
+    (testing "exception occurs when require-auth option is true but auth service was not found"
+      ;; This test is like the previous, except the auth service is not listed as a dependency
+      (is (thrown-with-msg? IllegalArgumentException
+                            #"service ':AuthorizationService' does not exist"
+                            (with-app-with-config
+                              app
+                              ;; The auth service is purposefully left out here
+                              services
+                              config-with-require-auth))))))
 
 (deftest metrics-v1-endpoint-disabled-by-default
   (testing "metrics/v1 is disabled by default, returns 404"