near/airsonic-ui
1
0
Fork 0
mirror of https://github.com/heyarne/airsonic-ui.git synced 2026-05-07 02:33:39 +02:00
Code Issues Projects Releases Wiki Activity

Make login-flow more robust and add more tests

Browse source
This commit is contained in:
Arne Schlüter 2018-07-16 17:20:33 +02:00
commit 852a3193ab
9 changed files with 123 additions and 65 deletions
Download patch file Download diff file Expand all files Collapse all files

3
src/cljs/airsonic_ui/db.cljs
View file

@ -1,5 +1,4 @@
(ns airsonic-ui.db
(:require [airsonic-ui.routes :as routes]))
(ns airsonic-ui.db)
(def default-db
{:notifications (sorted-map)})

71
src/cljs/airsonic_ui/events.cljs
View file

@ -12,6 +12,11 @@
(fn [params]
(apply println params)))
(defn noop
"An event handler that can be used for clarity; doesn't do anything, but might
give a name to an event"
[cofx _] cofx)
;; ---
;; app boot flow
;; * restoring a previous session
@ -31,10 +36,9 @@
troubles with our router."
[{:keys [db store]} _]
(let [credentials (:credentials store)]
{:db (assoc db :credentials credentials)
:dispatch-n [(if credentials
{:dispatch-n [(if credentials
[:init-flow/credentials-found credentials]
[:init-flow/credentials-missing])]
[:init-flow/credentials-not-found])]
:routes/start-routing nil}))
(re-frame/reg-event-fx
@ -45,19 +49,21 @@
(defn credentials-found [_ [_ {:keys [u p server]}]]
{:dispatch [:credentials/verification-request u p server]})
(re-frame/reg-event-fx
:init-flow/credentials-found credentials-found)
(re-frame/reg-event-fx :init-flow/credentials-found credentials-found)
(re-frame/reg-event-fx
:init-flow/credentials-missing
;; we don't do anything special here, it's just for the sake of clarity
(fn [_ _] {}))
;; we don't do anything special here, it's just for the sake of clarity
(defn credentials-not-found
[cofx _]
(assoc-in cofx [:db :credentials] :credentials/not-found))
(re-frame/reg-event-fx :init-flow/credentials-not-found credentials-not-found)
;; ---
;; auth logic
;; ---
(defn-traced credentials-verification-request
(defn credentials-verification-request
"Tries to authenticate a user by pinging the server with credentials, saving
them when the request was successful. Bypasses the request when a user saved
their credentials."
@ -66,21 +72,25 @@
:uri (api/url server "ping" {:u user :p pass})
:response-format (ajax/json-response-format {:keywords? true})
:on-success [:credentials/verification-response user pass server]
:on-failure [:api/bad-response]}})
:on-failure [:credentials/verification-failure]}})
(re-frame/reg-event-fx
:credentials/verification-request credentials-verification-request)
(re-frame/reg-event-fx :credentials/verification-request credentials-verification-request)
(defn credentials-verification-response
"Since we don't get real status codes, we have to look into the server's
response and see whether we actually sent the correct credentials"
[fx [_ user pass server response]]
{:dispatch (if (api/is-error? response)
[:notification/show :error (api/error-msg (api/->exception response))]
[:credentials/verification-failure response]
[:credentials/verified user pass server])})
(re-frame/reg-event-fx
:credentials/verification-response credentials-verification-response)
(re-frame/reg-event-fx :credentials/verification-response credentials-verification-response)
(defn credentials-verification-failure [fx [_ response]]
(-> (assoc-in fx [:db :credentials] :credentials/verification-failure)
(assoc :dispatch [:notification/show :error (api/error-msg (api/->exception response))])))
(re-frame/reg-event-fx :credentials/verification-failure credentials-verification-failure)
(defn credentials-verified
"Gets called after the server indicates that the credentials entered by a user
@ -92,8 +102,7 @@
:db (assoc db :credentials credentials)
:dispatch [::logged-in]}))
(re-frame/reg-event-fx
:credentials/verified credentials-verified)
(re-frame/reg-event-fx :credentials/verified credentials-verified)
;; TODO: We have to find another solution for this once we have routes that
;; don't require a login but have the bottom controls
@ -103,10 +112,10 @@
(fn [_]
(.. js/document -documentElement -classList (add "has-navbar-fixed-bottom"))))
(defn logged-in
[cofx _]
(let [redirect (or (get-in cofx [:routes/from-query-param :redirect])
[::routes/main])]
(let [redirect (or (get-in cofx [:routes/from-query-param :redirect]) [::routes/main])]
{:routes/navigate redirect
:show-nav-bar nil}))
@ -117,17 +126,16 @@
(defn logout
"Clears all credentials and redirects the user to the login page"
[_ [_ & args]]
[cofx [_ & args]]
(let [args (apply hash-map args)]
{:routes/navigate (if-let [redirect (:redirect-to args)]
[::routes/login {} {:redirect (routes/encode-route redirect)}]
[::routes/login])
:routes/unset-credentials nil
:store nil
:db db/default-db}))
:db (merge (:db cofx) db/default-db {:credentials :credentials/logged-out})}))
(re-frame/reg-event-fx
::logout logout)
(re-frame/reg-event-fx ::logout logout)
;; ---
;; api interaction
@ -144,8 +152,7 @@
:on-success [:api/good-response]
:on-failure [:api/bad-response]}})
(re-frame/reg-event-fx
:api/request api-request)
(re-frame/reg-event-fx :api/request api-request)
(defn good-api-response [fx [_ response]]
(try
@ -153,15 +160,13 @@
(catch ExceptionInfo e
{:dispatch [:notification/show :error (api/error-msg e)]})))
(re-frame/reg-event-fx
:api/good-response good-api-response)
(re-frame/reg-event-fx :api/good-response good-api-response)
(defn bad-api-response [db event]
{:log ["API call gone bad; are CORS headers missing? check for :status 0" event]
:dispatch [:notification/show :error "Communication with server failed. Check browser logs for details."]})
(re-frame/reg-event-fx
:api/bad-response bad-api-response)
(re-frame/reg-event-fx :api/bad-response bad-api-response)
;; ---
;; musique
@ -256,12 +261,10 @@
:message message})
(assoc :dispatch-later (hide-later level))))))
(re-frame/reg-event-fx
:notification/show show-notification)
(re-frame/reg-event-fx :notification/show show-notification)
(defn hide-notification
[db [_ notification-id]]
(update db :notifications dissoc notification-id))
(re-frame/reg-event-db
:notification/hide hide-notification)
(re-frame/reg-event-db :notification/hide hide-notification)

2
src/cljs/airsonic_ui/routes.cljs
View file

@ -100,7 +100,7 @@
;; this allows us to encode a complete route in a url fragment; useful for
;; doing redirects
(let [[_ _ query] (current-route)
from-param (decode-route (get query param))]
from-param (some-> (get query param) (decode-route))]
(assoc-in coeffects [:routes/from-query-param param] from-param))))
(defn start-routing!

8
src/cljs/airsonic_ui/subs.cljs
View file

@ -2,6 +2,14 @@
(:require [re-frame.core :as re-frame :refer [subscribe]]
[airsonic-ui.utils.api :as api]))
(defn is-booting?
"Predicate to tell whether our app is still in the process of initialization"
[{:keys [credentials]} _]
(and (not (map? credentials))
(not (#{:credentials/not-found :credentials/verification-failure :credentials/logged-out} credentials))))
(re-frame/reg-sub ::is-booting? is-booting?)
;; can be used to query the user's credentials
(re-frame/reg-sub

11
src/cljs/airsonic_ui/views.cljs
View file

@ -68,10 +68,11 @@
[bottom-bar]])))
(defn main-panel []
(let [[route params query] @(subscribe [::subs/current-route])
notifications @(subscribe [::subs/notifications])]
(let [notifications @(subscribe [::subs/notifications])
is-booting? @(subscribe [::subs/is-booting?])
[route params query] @(subscribe [::subs/current-route])]
[:div
[notification-list notifications]
(if route
[app route params query]
[:div.app-loading>div.loader])]))
(if is-booting?
[:div.app-loading>div.loader]
[app route params query] )]))

38
test/cljs/airsonic_ui/events_test.cljs
View file

@ -13,18 +13,17 @@
(letfn [(no-previous-session []
(events/restore-previous-session {} [:_]))
(has-previous-session []
(events/restore-previous-session {:store {:u "test"
(events/restore-previous-session {:store {:credentials {:u "test"
:p "test"
:server "https://demo.airsonic.io/"}} [:_]))]
:server "https://demo.airsonic.io/"}}} [:_]))]
(testing "Should initialize routing after checking for previous credentials"
(is (contains? (no-previous-session) :routes/start-routing))
(is (contains? (has-previous-session) :routes/start-routing)))
(testing "Should indicate success or failure"
(is (dispatches? (no-previous-session)) :init-flow/credentials-missing)
(is (dispatches? (has-previous-session)) :init-flow/credentials-found))
(is (true? (dispatches? (no-previous-session) :init-flow/credentials-not-found)))
(is (true? (dispatches? (has-previous-session) :init-flow/credentials-found))))
(testing "Should send an auth request on success"
(is (dispatches? (events/credentials-found {} [:_]) :credentials/verification-request)))
(testing "Should redirect to the login form when there's no previous session to be restored")))
(is (true? (dispatches? (events/credentials-found {} [:_]) :credentials/verification-request))))))
(deftest authentication
(testing "Server ping for verifications"
@ -32,20 +31,23 @@
fx (events/credentials-verification-request {} [:_ "user" "pass" server])
request (:http-xhrio fx)]
(testing "uses correct server url"
(is (str/starts-with? (:uri request) server))
(is (str/includes? (:uri request) "/ping"))
(is (str/includes? (:uri request) "p=pass"))
(is (str/includes? (:uri request) "u=user")))
(let [uri (:uri request)]
(is (true? (str/starts-with? uri server)))
(is (true? (str/includes? uri "/ping")))
(is (true? (str/includes? uri "p=pass")))
(is (true? (str/includes? uri "u=user")))))
(testing "invokes correct success callback"
(is (= :credentials/verification-response (first (:on-success request)))))))
(testing "Auth response verification"
(let [server "https://localhost"
fx (events/credentials-verification-response {} [:_ "user" "pass" server (:error responses)])]
(is (= (dispatches? fx :notification/show))
"shows an error when we have a bad response"))
(testing "Auth response"
(testing "verification for bad responses"
(let [ev [:_ "user" "pass" "https://localhost"]
invalid-credentials (events/credentials-verification-response {} (conj ev (:auth-failure responses)))
verification-failure (events/credentials-verification-failure {} [:_ (:auth-failure responses)])]
(is (true? (dispatches? invalid-credentials :credentials/verification-failure)) "fails for bad responses")
(is (true? (dispatches? verification-failure :notification/show)) "shows the failure the the user")))
(let [server "https://localhost"
fx (events/credentials-verification-response {} [:_ "username" "password" server (:auth-success responses)])]
(is (dispatches? fx [:credentials/verified "username" "password" server]))))
(is (true? (dispatches? fx [:credentials/verified "username" "password" server])))))
(testing "On succesful response"
(let [credentials {:u "user" :p "pass" :server "https://localhost"}
fx (events/credentials-verified {} [:_ (:u credentials) (:p credentials) (:server credentials)])]
@ -54,7 +56,7 @@
(testing "credentials are saved in the global state"
(is (= credentials (get-in fx [:db :credentials]))))
(testing "the login process is finalized"
(is (dispatches? fx ::events/logged-in))))))
(is (true? (dispatches? fx ::events/logged-in)))))))
(deftest logout
(let [fx (events/logout {} [:_])]
@ -65,7 +67,7 @@
(testing "Should unset authentication in the router"
(is (contains? fx :routes/unset-credentials)))
(testing "Should reset the app-db"
(is (= db/default-db (:db fx)))))
(is (= (every? #(= (get db/default-db %) (get-in fx [:db %])) (keys db/default-db))))))
(testing "Should be able to keep a redirection parameter"
(let [redirect [:route {:with-data #{1 2 3 4 5}}]
fx (events/logout {} [:_ :redirect-to redirect])]

10
test/cljs/airsonic_ui/fixtures.cljs
View file

@ -1,8 +1,8 @@
(ns airsonic-ui.fixtures)
(def responses {:error {:subsonic-response
{:error {:code 40
:message "Wrong username or password"}
{:error {:code 50
:message "Incompatible Airsonic REST protocol version. Server must upgrade."}
:status "failed"
:version "1.15.0"}}
:ok {:subsonic-response
@ -11,7 +11,11 @@
:status "ok"
:version "1.15.0"}}
:auth-success {:subsonic-response {:status "ok"
:version "1.15.0"}}})
:version "1.15.0"}}
:auth-failure {:subsonic-response {:status "failed"
:version "1.15.0"
:error {:code 40
:message "Wrong username or password."}}}})
(def song
{:artistId 42,

35
test/cljs/airsonic_ui/subs_test.cljs
View file

@ -1,9 +1,42 @@
(ns airsonic-ui.subs-test
(:require [cljs.test :refer [deftest testing is]]
[airsonic-ui.fixtures :refer [song]]
[airsonic-ui.db :as db]
[airsonic-ui.fixtures :refer [song] :as fixtures]
[airsonic-ui.utils.api :as api]
[airsonic-ui.events :as ev]
[airsonic-ui.subs :as subs]))
(def creds {:credentials {:u "test"
:p "test"
:server "https://demo.airsonic.io/"}})
(deftest is-booting
(testing "Should be true when provided the initial state"
(is (true? (subs/is-booting? db/default-db [:_]))))
(testing "Should be true when we have credentials but no response yet"
(is (true? (-> (ev/restore-previous-session {:store creds} [:_])
(ev/credentials-found [:_])
:db
(subs/is-booting? [:_])))))
(testing "Should be false when the login screen is shown"
(is (false? (-> (ev/restore-previous-session {} [:_])
(ev/credentials-not-found [:_])
:db
(subs/is-booting? [:_])))))
(let [{:keys [u p server]} (:credentials creds)]
(testing "Should be false after we verified our credentials with the server"
(is (false? (-> (ev/credentials-verified {:db {}} [:_ u p server])
:db
(subs/is-booting? [:_])))))
(testing "Should be false after the server rejected our credentials"
(is (false? (-> (ev/credentials-verification-failure {} [:_ (:auth-failure fixtures/responses)])
:db
(subs/is-booting? [:_]))))))
(testing "Should be false when a user logged out voluntarily"
(is (false? (-> (ev/logout {} [:_])
:db
(subs/is-booting? [:_]))))))
(deftest cover-images
(let [credentials {:server "https://foo.bar"
:u "test-user"

8
test/cljs/airsonic_ui/utils/api_test.cljs
View file

@ -47,3 +47,11 @@
(api/unwrap-response error-response)
(catch ExceptionInfo e
(= (:error error-response) (ex-data e)))))))
(deftest error-recognition
(testing "Should detect error responses"
(is (true? (api/is-error? (:error responses))))
(is (true? (api/is-error? (:auth-failure responses)))))
(testing "Should pass on good responses"
(is (false? (api/is-error? (:ok responses))))
(is (false? (api/is-error? (:auth-success responses))))))