diff --git a/src/oph/ehoks/common/api.clj b/src/oph/ehoks/common/api.clj index 9a0e95daa..1f0536f3c 100644 --- a/src/oph/ehoks/common/api.clj +++ b/src/oph/ehoks/common/api.clj @@ -14,6 +14,11 @@ [_ __ ___] (response/not-found {:reason "Route not found"})) +(defn unauthorized-handler + "Käsittelee tapauksen, jossa ei (ole/saada tarkistettua) käyttöoikeuksia." + [_ __ ___] + (response/unauthorized {:reason "Unable to check access rights"})) + (defn log-exception "Logittaa virheen." [ex data] @@ -41,6 +46,7 @@ ::c-ex/response-validation (c-ex/with-logging c-ex/http-response-handler :error) :not-found not-found-handler + :unauthorized unauthorized-handler ::c-ex/default exception-handler}) (defn create-app diff --git a/src/oph/ehoks/external/cas.clj b/src/oph/ehoks/external/cas.clj index 6773dd48b..27ae850d4 100644 --- a/src/oph/ehoks/external/cas.clj +++ b/src/oph/ehoks/external/cas.clj @@ -64,7 +64,24 @@ (nil? (:url @grant-ticket)) (t/after? (t/now) (:expires @grant-ticket))) (refresh-grant-ticket!)) - (let [ticket (get-service-ticket (:url @grant-ticket) service)] + (let [ticket (try + (get-service-ticket (:url @grant-ticket) service) + (catch Exception e + (if (= 404 (:status (ex-data e))) + ; Refresh Ticket Granting Ticket and retry, + ; when fetch Service Ticket call returns 404. + ; This potentially happens after CAS service restart. + (do + (refresh-grant-ticket!) + (try + (get-service-ticket (:url @grant-ticket) service) + (catch Exception ex + (throw (ex-info "Error getting Service Ticket" + {:type :unauthorized} + ex))))) + (throw (ex-info "Error getting Service Ticket" + {:type :unauthorized} + e)))))] (-> data (assoc-in [:headers "accept"] "*/*") (assoc-in [:headers "ticket"] ticket) diff --git a/test/oph/ehoks/external/cas_test.clj b/test/oph/ehoks/external/cas_test.clj index ca396fda8..b146ab063 100644 --- a/test/oph/ehoks/external/cas_test.clj +++ b/test/oph/ehoks/external/cas_test.clj @@ -1,5 +1,6 @@ (ns oph.ehoks.external.cas-test (:require [clojure.test :refer [deftest testing is use-fixtures]] + [clojure.string :as s] [oph.ehoks.external.cas :as c] [oph.ehoks.config :refer [config]] [oph.ehoks.external.http-client :as client] @@ -27,8 +28,8 @@ (t/minutes (inc (:ext-cache-lifetime-minutes config))))}}) -(deftest test-refresh-service-ticket - (testing "Refresh service ticket successfully" +(deftest test-refresh-grant-ticket + (testing "Refresh grant ticket successfully" (reset! c/grant-ticket {:url nil :expires nil}) (is (= (deref c/grant-ticket) {:url nil :expires nil})) (client/set-post! @@ -43,10 +44,17 @@ (is (= (:url (deref c/grant-ticket)) "test-url")) (is (some? (:expires (deref c/grant-ticket))))) - (testing "Refresh service ticket unsuccessfully" + (testing "Refresh grant ticket unsuccessfully (404)" (reset! c/grant-ticket {:url nil :expires nil}) (client/set-post! (fn [_ options] - {:status 404})) + (throw (ex-info "HTTP Exception" {:status 404})))) + (is (thrown-with-msg? clojure.lang.ExceptionInfo + #"HTTP Exception" + (c/refresh-grant-ticket!)))) + (testing "Refresh grant ticket unsuccessfully (missing location header)" + (reset! c/grant-ticket {:url nil :expires nil}) + (client/set-post! (fn [_ options] + {:status 201})) (is (thrown-with-msg? clojure.lang.ExceptionInfo #"Failed to refresh CAS Service Ticket" (c/refresh-grant-ticket!))))) @@ -62,14 +70,87 @@ "test-ticket")))) (deftest test-add-cas-ticket - (testing "Add service ticket" + (testing "Add service ticket successfully" (client/set-post! (fn [_ options] {:body "test-ticket"})) (reset! c/grant-ticket {:url "http://ticket.url" :expires (t/plus (t/now) (t/hours 2))}) (let [data (c/add-cas-ticket {} "http://test-service")] (is (= (get-in data [:headers "accept"]) "*/*")) - (is (= (get-in data [:query-params :ticket]) "test-ticket"))))) + (is (= (get-in data [:query-params :ticket]) "test-ticket")))) + (testing "Add service ticket retries when get ST returns 404 once" + (let [get-st-returns-404 (atom true) + get-st-call-count (atom 0)] + (client/set-post! + (fn [url _] + (cond + ; refresh ticket granting ticket + (.endsWith url "/v1/tickets") + {:status 201 :headers {"location" "http://ticket.url"}} + ; get service ticket returns 404 once + (= url "http://ticket.url") + (do + (swap! get-st-call-count inc) + (if @get-st-returns-404 + (do + (reset! get-st-returns-404 false) + (throw + (ex-info + "Test HTTP Exception" + {:body + "TGT-nnn could not be found or is considered invalid" + :status 404}))) + {:status 201 :body "test-ticket"}))))) + (reset! c/grant-ticket {:url "http://ticket.url" + :expires (t/plus (t/now) (t/hours 2))}) + (let [data (c/add-cas-ticket {} "http://test-service")] + (is (= (get-in data [:headers "accept"]) "*/*")) + (is (= (get-in data [:query-params :ticket]) "test-ticket"))) + (is (= @get-st-call-count 2)))) + (testing "Add service ticket eventually throws" + (let [get-st-call-count (atom 0)] + (client/set-post! + (fn [url _] + (cond + ; refresh ticket granting ticket + (.endsWith url "/v1/tickets") + {:status 201 :headers {"location" "http://ticket.url"}} + ; get service ticket returns 404 continuously + (= url "http://ticket.url") + (do + (swap! get-st-call-count inc) + (throw + (ex-info + "Test HTTP Exception" + {:status 404 + :body + "TGT-nnn could not be found or is considered invalid"})))))) + (reset! c/grant-ticket {:url "http://ticket.url" + :expires (t/plus (t/now) (t/hours 2))}) + (let [result (try + (c/add-cas-ticket {} "http://test-service") + nil + (catch Exception e + e))] + (is (= :unauthorized (:type (ex-data result))))) + (is (= @get-st-call-count 2)))) + (testing "Add service ticket immediately throws when get ST returns 500" + (let [get-st-call-count (atom 0)] + (client/set-post! (fn [_ __] + (swap! get-st-call-count inc) + (throw (ex-info "Test HTTP Exception" + {:status 500 + :body "Infernal server error"})))) + (reset! c/grant-ticket {:url "http://ticket.url" + :expires (t/plus (t/now) (t/hours 2))}) + (let [result (try + (c/add-cas-ticket {} "http://test-service") + nil + (catch Exception e + e))] + (is (= :unauthorized (:type (ex-data result))))) + ; not trying to retry in this case + (is (= @get-st-call-count 1))))) (deftest test-with-service-ticket (testing "Request with API headers" diff --git a/test/oph/ehoks/hoks/hoks_handler_test.clj b/test/oph/ehoks/hoks/hoks_handler_test.clj index 0ad210c17..3dc20f917 100644 --- a/test/oph/ehoks/hoks/hoks_handler_test.clj +++ b/test/oph/ehoks/hoks/hoks_handler_test.clj @@ -864,3 +864,44 @@ (update-in [:osaamisen-osoittaminen 0] dissoc :module-id) (dissoc :id :module-id)) hato)))))) + +(deftest test-handles-unauthorized + (testing "Responds with unauthorized when not able to get ST for käyttöoikeus" + (client/set-post! + (fn [url _] + (cond + (.endsWith url "/v1/tickets") + {:status 201 + :headers {"location" "http://localhost/TGT-1234"}} + (= url "http://localhost/TGT-1234") + (throw + (ex-info + "Test HTTP Exception" + {:status 404 + :body + "TGT-1234 could not be found or is considered invalid"}))))) + (client/set-get! + (fn [url options] + (cond + (.endsWith url "/serviceValidate") + {:status 200 + :body + (str "" + "ehoks" + "" + "false" + "" + "false" + "2019-02-20T10:14:24.046+02:00" + "" + "")}))) + (let [app (hoks-utils/create-app nil) + response (app (-> (mock/request + :get + (str hoks-utils/base-url "/1")) + (mock/header "Caller-Id" "test") + (mock/header "ticket" "ST-testitiketti")))] + (is (= (:status response) 401)) + (is (= (utils/parse-body (:body response)) + {:reason "Unable to check access rights"})))))