Skip to content

Commit

Permalink
✨ Redirect to original page after login, when clicking the login button
Browse files Browse the repository at this point in the history
Why:
- Continuing the migration from SPA to SSR.
- An anonymous user can be viewing the support or demo pages when they
  click the login button.
  • Loading branch information
luontola committed Mar 1, 2024
1 parent 4661b1c commit 4d61195
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 36 deletions.
15 changes: 9 additions & 6 deletions src/territory_bro/infra/auth0.clj
Original file line number Diff line number Diff line change
Expand Up @@ -96,16 +96,19 @@
(-> (getx config/env :public-url)
(str/replace "8080" "8081"))) ; TODO: remove me

(defn return-to-url [{:keys [uri query-string]}]
(if (some? query-string)
(str uri "?" query-string)
uri))

(defn login-url [request]
(str "/login?return-to-url=" (codec/url-encode (return-to-url request))))

(defn wrap-redirect-to-login [handler]
(fn [request]
(let [response (handler request)]
(if (= 401 (:status response))
(let [{:keys [uri query-string]} request
return-to-url (str uri
(when (some? query-string)
(str "?" query-string)))
login-url (str "/login?return-to-url=" (codec/url-encode return-to-url))]
(response/redirect login-url :see-other))
(response/redirect (login-url request) :see-other)
response))))

(defn login-handler [ring-request]
Expand Down
7 changes: 5 additions & 2 deletions src/territory_bro/ui/layout.clj
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
(:require [clojure.string :as str]
[hiccup2.core :as h]
[territory-bro.api :as api]
[territory-bro.infra.auth0 :as auth0]
[territory-bro.infra.authentication :as auth]
[territory-bro.infra.config :as config]
[territory-bro.infra.resources :as resources]
Expand All @@ -23,6 +24,8 @@
:congregation congregation
:user (when (auth/logged-in?)
auth/*user*)
:login-url (when-not (auth/logged-in?)
(auth0/login-url request))
:dev? (getx config/env :dev)})))


Expand Down Expand Up @@ -102,14 +105,14 @@
:icon "🛟"
:title (i18n/t "SupportPage.title")})]])))

(defn authentication-panel [{:keys [user dev?]}]
(defn authentication-panel [{:keys [user login-url dev?]}]
(if (some? user)
(h/html [:i.fa-solid.fa-user-large {:style {:font-size "1.25em"
:vertical-align "middle"}}]
" " (:name user) " "
[:a#logout-button.pure-button {:href "/logout"}
(i18n/t "Navigation.logout")])
(h/html [:a#login-button.pure-button {:href "/login"}
(h/html [:a#login-button.pure-button {:href login-url}
(i18n/t "Navigation.login")]
(when dev?
(h/html " " [:a#dev-login-button.pure-button {:href "/dev-login?sub=developer&name=Developer&[email protected]"}
Expand Down
46 changes: 26 additions & 20 deletions test/territory_bro/browser_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -53,29 +53,35 @@

(deftest login-and-logout-test
(with-per-test-postmortem
(testing "login with username and password"
(doto *driver*
(b/go *base-url*)
(b/wait-visible :login-button)
(b/click :login-button)
(submit-auth0-login-form)
(b/wait-visible :logout-button)))

(testing "shows user is logged in"
(is (b/has-text? *driver* {:css "nav"} "Logout"))
(is (b/has-text? *driver* {:css "nav"} auth0-username)))

(testing "logout"
(doto *driver*
(b/click :logout-button)
(b/wait-visible :login-button)))

(testing "shows the user is logged out"
(is (b/has-text? *driver* {:css "nav"} "Login")))))
(let [original-page-url (str *base-url* "/support?foo=bar&gazonk")] ; also query string should be restored

(testing "login with Auth0"
(doto *driver*
(b/go original-page-url)
(b/wait-visible :login-button)
(b/click :login-button)
(submit-auth0-login-form)
(b/wait-visible :logout-button)))

(testing "shows user is logged in"
(is (b/has-text? *driver* {:css "nav"} "Logout"))
(is (b/has-text? *driver* {:css "nav"} auth0-username)))

(testing "after login, redirects to the page where the user came from"
(is (= original-page-url (b/get-url *driver*))))

(testing "logout"
(doto *driver*
(b/click :logout-button)
(b/wait-visible :login-button)))

(testing "shows the user is logged out"
(is (b/has-text? *driver* {:css "nav"} "Login"))))))

(deftest login-via-entering-restricted-page-test
(with-per-test-postmortem
(let [restricted-page-url (str *base-url* "/congregation/" (UUID/randomUUID) "?foo=bar&gazonk")]
(let [restricted-page-url (str *base-url* "/congregation/" (UUID/randomUUID) "?foo=bar&gazonk")] ; also query string should be restored

(testing "enter restricted page, redirects to login"
(doto *driver*
(b/go restricted-page-url)
Expand Down
30 changes: 22 additions & 8 deletions test/territory_bro/ui/layout_test.clj
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
;; The license text is at http://www.apache.org/licenses/LICENSE-2.0

(ns territory-bro.ui.layout-test
(:require [clojure.test :refer :all]
(:require [clojure.string :as str]
[clojure.test :refer :all]
[hiccup2.core :as h]
[territory-bro.api-test :as at]
[territory-bro.infra.authentication :as auth]
Expand All @@ -18,24 +19,29 @@
{:title "the title"
:congregation nil
:user nil
;; the page path and query string should both be included in the return-to-url
:login-url "/login?return-to-url=%2Fsome%2Fpage%3Ffoo%3Dbar%26gazonk"
:dev? false})
(def developer-model
{:title "the title"
:congregation nil
:user nil
:login-url "/login?return-to-url=%2F"
:dev? true})
(def logged-in-model
{:title "the title"
:congregation nil
:user {:user/id (UUID. 0 2)
:name "John Doe"}
:login-url nil
:dev? false})
(def congregation-model
{:title "the title"
:congregation {:id (UUID. 0 1)
:name "the congregation"}
:user {:user/id (UUID. 0 2)
:name "John Doe"}
:login-url nil
:dev? false})

(deftest ^:slow model!-test
Expand All @@ -45,24 +51,30 @@
cong-id (at/create-congregation! session "the congregation")]

(testing "top level, anonymous"
(let [request {}]
(let [request {:uri "/some/page"
:query-string "foo=bar&gazonk"}]
(is (= anonymous-model
(layout/model! request {:title "the title"})))))

(testing "top level, developer"
(binding [config/env (replace-in config/env [:dev] false true)]
(let [request {}]
(let [request {:uri "/"
:query-string nil}]
(is (= developer-model
(layout/model! request {:title "the title"}))))))

(testing "top level, logged in"
(let [request {:session (auth/user-session {:name "John Doe"} user-id)}]
(let [request {:uri "/"
:query-string nil
:session (auth/user-session {:name "John Doe"} user-id)}]
(is (= (-> logged-in-model
(replace-in [:user :user/id] (UUID. 0 2) user-id))
(layout/model! request {:title "the title"})))))

(testing "congregation level"
(let [request {:params {:congregation (str cong-id)}
(let [request {:uri "/"
:query-string nil
:params {:congregation (str cong-id)}
:session (auth/user-session {:name "John Doe"} user-id)}]
(is (= (-> congregation-model
(replace-in [:congregation :id] (UUID. 0 1) cong-id)
Expand Down Expand Up @@ -162,9 +174,11 @@

(deftest authentication-panel-test
(testing "anonymous"
(is (= "Login"
(html/visible-text
(layout/authentication-panel anonymous-model)))))
(let [html (layout/authentication-panel anonymous-model)]
(is (= "Login"
(html/visible-text html)))
(is (str/includes? (str html) "href=\"/login?return-to-url=%2Fsome%2Fpage%3Ffoo%3Dbar%26gazonk\"")
"the login link will redirect back to the current page after login")))

(testing "developer"
(is (= (html/normalize-whitespace
Expand Down

0 comments on commit 4d61195

Please sign in to comment.