Repository: CSCfi/rems (clojure)

Issue:
Consider renaming ForbiddenException and NotAuthorizedException to match HTTP error codes.
- ForbiddenException is ok
- [x] NotAuthorizedException should be UnauthorizedException and message should tell that the user is unauthenticated

Pull request:
to match the name of the corresponding http code

closes #815

Patch:
diff --git a/src/clj/rems/api.clj b/src/clj/rems/api.clj
index e604f52967..0042628baa 100644
--- a/src/clj/rems/api.clj
+++ b/src/clj/rems/api.clj
@@ -20,7 +20,7 @@
             [ring.middleware.cors :refer [wrap-cors]]
             [ring.util.http-response :refer :all]
             [schema.core :as s])
-  (:import [rems.auth ForbiddenException NotAuthorizedException]
+  (:import [rems.auth ForbiddenException UnauthorizedException]
            rems.InvalidRequestException))
 
 (defn unauthorized-handler
@@ -82,7 +82,7 @@
      :formats muuntaja
      :middleware [cors-middleware
                   transaction-middleware]
-     :exceptions {:handlers {NotAuthorizedException unauthorized-handler
+     :exceptions {:handlers {UnauthorizedException unauthorized-handler
                              ForbiddenException forbidden-handler
                              InvalidRequestException invalid-handler
                              ;; java.lang.Throwable (ex/with-logging debug-handler) ; optional Debug handler
diff --git a/src/clj/rems/auth/util.clj b/src/clj/rems/auth/util.clj
index b4842db12b..3e0b0b0e60 100644
--- a/src/clj/rems/auth/util.clj
+++ b/src/clj/rems/auth/util.clj
@@ -1,10 +1,10 @@
 (ns rems.auth.util
-  (:import [rems.auth NotAuthorizedException ForbiddenException]))
+  (:import [rems.auth UnauthorizedException ForbiddenException]))
 
 (defn throw-unauthorized
-  "Helper for throwing `NotAuthorizedException`."
+  "Helper for throwing `UnauthorizedException`."
   []
-  (throw (NotAuthorizedException.)))
+  (throw (UnauthorizedException.)))
 
 (defn throw-forbidden
   "Helper for throwing `ForbiddenException`."
diff --git a/src/clj/rems/middleware.clj b/src/clj/rems/middleware.clj
index 8994806311..9a89e48f59 100644
--- a/src/clj/rems/middleware.clj
+++ b/src/clj/rems/middleware.clj
@@ -27,7 +27,7 @@
             [ring.util.http-response :refer [unauthorized]]
             [ring.util.response :refer [redirect header]])
   (:import [javax.servlet ServletContext]
-           [rems.auth ForbiddenException NotAuthorizedException]))
+           [rems.auth ForbiddenException UnauthorizedException]))
 
 (defn calculate-root-path [request]
   (if-let [context (:servlet-context request)]
@@ -160,7 +160,7 @@
   (fn [req]
     (try
       (handler req)
-      (catch NotAuthorizedException e
+      (catch UnauthorizedException e
         (on-unauthorized-error req))
       (catch ForbiddenException e
         (on-forbidden-error req)))))
diff --git a/src/java/rems/auth/NotAuthorizedException.java b/src/java/rems/auth/NotAuthorizedException.java
deleted file mode 100644
index 62598bb181..0000000000
--- a/src/java/rems/auth/NotAuthorizedException.java
+++ /dev/null
@@ -1,22 +0,0 @@
-package rems.auth;
-
-/**
- * Thrown when an operation is not authorized.
- */
-public class NotAuthorizedException extends Exception {
-
-    public NotAuthorizedException() {
-    }
-
-    public NotAuthorizedException(String message) {
-        super(message);
-    }
-
-    public NotAuthorizedException(String message, Throwable cause) {
-        super(message, cause);
-    }
-
-    public NotAuthorizedException(Throwable cause) {
-        super(cause);
-    }
-}
diff --git a/src/java/rems/auth/UnauthorizedException.java b/src/java/rems/auth/UnauthorizedException.java
new file mode 100644
index 0000000000..3e79aeca06
--- /dev/null
+++ b/src/java/rems/auth/UnauthorizedException.java
@@ -0,0 +1,22 @@
+package rems.auth;
+
+/**
+ * Thrown when an operation is not authorized.
+ */
+public class UnauthorizedException extends Exception {
+
+    public UnauthorizedException() {
+    }
+
+    public UnauthorizedException(String message) {
+        super(message);
+    }
+
+    public UnauthorizedException(String message, Throwable cause) {
+        super(message, cause);
+    }
+
+    public UnauthorizedException(Throwable cause) {
+        super(cause);
+    }
+}


Test patch:
diff --git a/test/clj/rems/api/test_util.clj b/test/clj/rems/api/test_util.clj
index 163065baaa..67e7c585ba 100644
--- a/test/clj/rems/api/test_util.clj
+++ b/test/clj/rems/api/test_util.clj
@@ -4,7 +4,7 @@
             [rems.api.util :refer :all]
             [rems.context :as context]
             [ring.util.http-response :refer :all])
-  (:import (rems.auth ForbiddenException NotAuthorizedException)))
+  (:import (rems.auth ForbiddenException UnauthorizedException)))
 
 (deftest route-role-check-test
   (testing "no roles required"
@@ -42,7 +42,7 @@
       (testing "but user is not logged in"
         (binding [context/*roles* #{:role1}
                   context/*user* nil]
-          (is (thrown? NotAuthorizedException
+          (is (thrown? UnauthorizedException
                        (route {:request-method :get
                                :uri "/foo"})))))))