Review feedback

src/nl/surf/eduhub/validator/service/api.clj

@@ -96,9 +96,9 @@
       (wrap-json-response)
       (wrap-defaults api-defaults)))
 
-(defn private-routes [{:keys [introspection-endpoint-url introspection-basic-auth allowed-client-ids] :as config} auth-enabled]
+(defn private-routes [{:keys [introspection-endpoint-url introspection-basic-auth allowed-client-ids] :as config} auth-disabled]
   (let [allowed-client-id-set (set (str/split allowed-client-ids #","))
-        auth-opts             {:auth-enabled (boolean auth-enabled)}]
+        auth-opts             {:auth-disabled (boolean auth-disabled)}]
     (-> (compojure.core/routes
           (POST "/endpoints/:endpoint-id/config" [endpoint-id]
             (checker/check-endpoint endpoint-id config))
@@ -109,8 +109,8 @@
         (wrap-defaults api-defaults))))
 
 ;; Compose the app from the routes and the wrappers. Authentication can be disabled for testing purposes.
-(defn compose-app [config auth-enabled]
+(defn compose-app [config auth-disabled]
   (compojure.core/routes
     (public-routes config)
-    (private-routes config auth-enabled)
+    (private-routes config auth-disabled)
     (route/not-found "Not Found")))

src/nl/surf/eduhub/validator/service/authentication.clj

@@ -40,8 +40,7 @@
             [cheshire.core :as json]
             [clojure.core.memoize :as memo]
             [clojure.tools.logging :as log]
-            [nl.jomco.http-status-codes :as http-status]
-            [ring.util.response :as response]))
+            [nl.jomco.http-status-codes :as http-status]))
 
 (defn bearer-token
   [{{:strs [authorization]} :headers}]
@@ -101,15 +100,12 @@
 
   If no bearer token is provided, the request is executed without a client-id."
   ; auth looks like {:user client-id :pass client-secret}
-  [app introspection-endpoint auth allowed-client-id-set {:keys [auth-enabled]}]
+  [app introspection-endpoint auth allowed-client-id-set {:keys [auth-disabled]}]
   (let [authenticator (memo/ttl (make-token-authenticator introspection-endpoint auth) :ttl/threshold 60000)] ; 1 minute
     (fn authentication [request]
-      (if (not auth-enabled)
-        (app request)
-        (let [token (bearer-token request)
-              client-id (some-> (bearer-token request)
-                                (and token (authenticator token)))]
-          (if (allowed-client-id-set client-id)
-            (app request)
-            {:body   (if client-id "Unknown client id" "No client-id found")
-             :status http-status/forbidden}))))))
+      (let [client-id (some-> request bearer-token authenticator)]
+        (if (or auth-disabled
+                (allowed-client-id-set client-id))
+          (app request)
+          {:body   (if client-id "Unknown client id" "No client-id found")
+           :status http-status/forbidden})))))

src/nl/surf/eduhub/validator/service/checker.clj

@@ -29,7 +29,7 @@
 (defn check-endpoint [endpoint-id config]
   (try
     (let [{:keys [status body]} (validate/check-endpoint endpoint-id config)]
-      ;; If the client credentials for the validator are incorrect, the wrap-allowed-clients-checker
+      ;; If the client credentials for the validator are incorrect, the wrap-authentication
       ;; middleware has already returned 401 forbidden and execution doesn't get here.
       (handle-check-endpoint-response status body endpoint-id))
     (catch Throwable e

src/nl/surf/eduhub/validator/service/main.clj

@@ -47,4 +47,4 @@
                                (let [job-args (assoc-in (vec (:args job)) [2 :config] config)
                                      new-job (assoc job :args job-args)]
                                  (app opts new-job))))))
-    (start-server (api/compose-app config :auth-enabled) config)))
+    (start-server (api/compose-app config (not :auth-disabled)) config)))

test/nl/surf/eduhub/validator/service/authentication_test.clj

@@ -68,8 +68,7 @@
   (-> (fn auth-handler [_req]
         {:status http-status/ok
          :body   {}})
-      (authentication/wrap-authentication introspection-endpoint basic-auth {:auth-enabled true})
-      (authentication/wrap-allowed-clients-checker allowed-client-id-set {:auth-enabled true})
+      (authentication/wrap-authentication introspection-endpoint basic-auth allowed-client-id-set {:auth-disabled false})
       wrap-observe-client-id))
 
 (deftest token-validator
@@ -80,7 +79,7 @@
       (reset! count-calls 0)
       (let [handler (make-handler introspection-endpoint basic-auth #{"institution_client_id"})]
         (is (= {:status    http-status/ok
-                :body      {:client "institution_client_id"}}
+                :body      {}}
                (handler {:headers {"authorization" (str "Bearer " valid-token)}}))
             "Ok when valid token provided")
 
@@ -97,7 +96,7 @@
 
         (reset! count-calls 0)
         (is (= {:status    http-status/ok
-                :body      {:client "institution_client_id"}}
+                :body      {}}
                (handler {:headers {"authorization" (str "Bearer " valid-token)}}))
             "CACHED: Ok when valid token provided")
 

test/nl/surf/eduhub/validator/service/checker_test.clj

@@ -28,7 +28,7 @@
 (def test-config
   (first (config/load-config-from-env (merge config-test/default-env env))))
 
-(def app (api/compose-app test-config false))
+(def app (api/compose-app test-config :auth-disabled))
 
 (defn- response-match [actual req]
   (is (= actual

test/nl/surf/eduhub/validator/service/jobs/client_test.clj

@@ -13,7 +13,7 @@
 (def test-config
   (first (config/load-config-from-env (merge config-test/default-env env))))
 
-(def app (api/compose-app test-config false))
+(def app (api/compose-app test-config :auth-disabled))
 
 (defn- make-status-call [uuid]
   (let [{:keys [status body]}