diff --git a/cms/themes/rise/src/systems/bread/alpha/cms/theme/rise.cljc b/cms/themes/rise/src/systems/bread/alpha/cms/theme/rise.cljc index a3828f2e..467c41bf 100644 --- a/cms/themes/rise/src/systems/bread/alpha/cms/theme/rise.cljc +++ b/cms/themes/rise/src/systems/bread/alpha/cms/theme/rise.cljc @@ -666,7 +666,7 @@ (ErrorMessage {:message (i18n/t i18n error-key)}))]) (defmethod Section :save [{:keys [i18n]} _] - (Submit (:account/save i18n) :name :action :value "update")) + (Submit (:account/save i18n) :name :action :value "update-details")) (defn- CustomizingSection [_] {:id :customizing diff --git a/plugins/auth/account.i18n.edn b/plugins/auth/account.i18n.edn index f20ca0f4..4706d914 100644 --- a/plugins/auth/account.i18n.edn +++ b/plugins/auth/account.i18n.edn @@ -8,6 +8,7 @@ :account-updated "Account details have been updated." :datetime-format-default "EEE, LLL d 'at' h:mm a" :date-format-default "EEE, LLL d" + :invalid-action "Invalid action." :leave-passwords-blank "Leave password fields blank to keep your current password." :name "Name" :email "Email" @@ -15,6 +16,7 @@ :pronouns "Pronouns" :pronouns-example "they/them/theirs" :save "Save" + :session-deleted "Session deleted." :timezone "Timezone" :your-sessions "Your login sessions" } diff --git a/plugins/auth/systems/bread/alpha/plugin/account.cljc b/plugins/auth/systems/bread/alpha/plugin/account.cljc index dc10f033..b65dad3b 100644 --- a/plugins/auth/systems/bread/alpha/plugin/account.cljc +++ b/plugins/auth/systems/bread/alpha/plugin/account.cljc @@ -155,14 +155,20 @@ (update $ :user/preferences edn/read-string) (s/transform [:user/sessions s/ALL :session/data] edn/read-string $)))) -(defmulti account-action (fn [{:keys [params]}] (keyword (:action params)))) +(defmulti effects (fn [{:keys [params]}] (keyword (:action params)))) -(defmethod account-action :delete-session [{:keys [params]}] - (when-let [id (try (Integer. (:dbid params)) (catch Throwable _ nil))] - [[:db/retract id :session/id] - [:db/retract id :session/data] - [:db/retract id :thing/created-at] - [:db/retract id :thing/updated-at]])) +(defmethod effects :default [_] + ;; TODO standardize generic error-key? + (throw (ex-info "Invalid action" {:error-key :account/invalid-action}))) + +(defmethod effects :delete-session [{:as req :keys [params]}] + (when (try (Integer. (:dbid params)) (catch Throwable _ nil)) + [{:effect/name [::update :delete-session] + :effect/description "Update account state" + :effect/key :delete-session + :success-key :account/session-deleted + :params params + :conn (db/connection req)}])) (defn validate-password-fields [{:auth/keys [min-password-length max-password-length]} @@ -170,6 +176,9 @@ "Returns an error code as a keyword if the :password and/or :password-confirmation params are invalid." (cond + ;; If the user submitted only the password confirmation, assume they intended + ;; to update password but forgot to fill out both fields. + (and (seq password-confirmation) (empty? password)) :auth/passwords-must-match (empty? password) :auth/password-required (not= password password-confirmation) :auth/passwords-must-match (< (count password) min-password-length) @@ -180,65 +189,83 @@ (defn- hook-preference [req [k v]] [k (bread/hook req [::preference k] v)]) -(defmethod account-action :update [{:as req :keys [params session] ::bread/keys [config]}] +(defmethod effects :update-details + [{:as req :keys [params session] ::bread/keys [config]}] (let [{:keys [password password-confirmation]} params - update-password? (seq password) + update-password? (or (seq password) (seq password-confirmation)) error-key (when update-password? (validate-password-fields config params)) hash-algo (when update-password? (:auth/hash-algorithm config)) preferences (dissoc params :action :name :lang :password :password-confirmation)] (when error-key (throw (ex-info "Invalid password" {:error-key error-key}))) - [(cond-> {:db/id (:db/id (:user session)) :user/name (:name params)} - (:lang params) (assoc :user/lang (keyword (:lang params))) - update-password? (assoc :user/password - (hashers/derive password {:alg hash-algo})) - (seq preferences) (assoc :user/preferences (->> preferences - (map (partial hook-preference req)) - (into {}) - pr-str)))])) + [{:effect/name ::db/transact + :effect/description "Update account details" + :effect/key :update-details + :success-key :account/account-updated + :conn (db/connection req) + :txs + [(cond-> {:db/id (:db/id (:user session)) :user/name (:name params)} + (:lang params) (assoc :user/lang (keyword (:lang params))) + update-password? (assoc :user/password + (hashers/derive password {:alg hash-algo})) + (seq preferences) (assoc :user/preferences (->> preferences + (map (partial hook-preference req)) + (into {}) + pr-str)))]}])) + +(defmethod bread/effect [::update :delete-session] + [{k :effect/key :keys [conn params success-key]} {user :user}] + (let [session-id (try (Integer. (:dbid params)) (catch Throwable _ nil)) + valid-ids (set (map :db/id (:user/sessions user))) + valid? (contains? valid-ids session-id)] + (when valid? + {:effects + [{:effect/name ::db/transact + :effect/description "Delete a user session." + :effect/key k + :success-key success-key + :conn conn + :txs [[:db/retractEntity session-id]]}]}))) (defmethod bread/dispatch ::account=> - [{:as req :keys [params request-method session] ::bread/keys [config dispatcher]}] - (if (= :post request-method) - ;; Account update. - (let [action (keyword (:action params)) - account-update? (= :update action) - [txs error-key] (try - [(account-action req) nil] - (catch clojure.lang.ExceptionInfo e - [nil (-> e ex-data :error-key)])) - success-key (cond - account-update? :account-updated)] - (if txs - {:effects - [(db/txs->effect req txs :effect/description "Update account details")] - :hooks - {::bread/expand - [{:action/name ::ring/redirect - :to (bread/config req :account/account-uri) - :flash (when account-update? {:success-key :account/account-updated}) - :action/description - "Redirect to account page after taking an account action"}]}} - {:hooks - {::bread/expand - [{:action/name ::ring/redirect - :to (bread/config req :account/account-uri) - :flash (when account-update? {:error-key error-key}) - :action/description - "Redirect to account page after an error"}]}})) - ;; Rendering the account page. - (let [id (:db/id (:user session)) - pull (:dispatcher/pull dispatcher)] + [{:as req :keys [params request-method session] ::bread/keys [dispatcher]}] + (let [id (:db/id (:user session)) + pull (:dispatcher/pull dispatcher) + user-pull {:find [(list 'pull '?e pull) '.] :in '[$ ?e]} + query-user {:expansion/key :user + :expansion/name ::db/query + :expansion/description "Query for all user account data" + :expansion/db (db/database req) + :expansion/args [user-pull id]} + expand-user {:expansion/key :user + :expansion/name ::user + :expansion/description "Expand user data"}] + (if (= :post request-method) + ;; Account update. + (let [[effects error-key] (try + [(effects req) nil] + (catch clojure.lang.ExceptionInfo e + [nil (-> e ex-data :error-key)]))] + (if error-key + {:hooks + {::bread/render + [{:action/name ::ring/redirect + :to (bread/config req :account/account-uri) + :flash (when error-key {:error-key error-key}) + :action/description + "Redirect to account page after an error"}]}} + {:expansions [query-user expand-user] + :effects effects + :hooks + {::bread/render + [{:action/name ::ring/effect-redirect + :to (bread/config req :account/account-uri) + :effect/key (keyword (:action params)) + :action/description + "Redirect to account page after taking an account action"}]}})) + ;; Rendering the account page. {:expansions - [{:expansion/key :user - :expansion/name ::db/query - :expansion/description "Query for all user account data" - :expansion/db (db/database req) - :expansion/args [{:find [(list 'pull '?e pull) '.] - :in '[$ ?e]} - id]} - {:expansion/key :user - :expansion/name ::user - :expansion/description "Expand user data"} + [query-user + expand-user {;; TODO => i18n :expansion/key :supported-langs :expansion/name ::bread/value @@ -247,6 +274,7 @@ {;; TODO => i18n :expansion/key :lang-names :expansion/name ::bread/value + :expansion/description "Language names for display" :expansion/value (bread/config req :i18n/lang-names)}]}))) (defn plugin [{:keys [account-uri html-account-header html-account-form diff --git a/src/systems/bread/alpha/component.cljc b/src/systems/bread/alpha/component.cljc index e6c97a37..eeb73d91 100644 --- a/src/systems/bread/alpha/component.cljc +++ b/src/systems/bread/alpha/component.cljc @@ -130,10 +130,10 @@ (defmethod bread/action ::render [{:keys [::bread/data body] :as res} _ _] (if body - res - (let [component (match res) - body (render component data)] - (assoc res :body body)))) + res + (let [component (match res) + body (render component data)] + (assoc res :body body)))) (defmethod bread/action ::hook-fn [req _ _] diff --git a/src/systems/bread/alpha/database.cljc b/src/systems/bread/alpha/database.cljc index 95261b67..3ddd3e64 100644 --- a/src/systems/bread/alpha/database.cljc +++ b/src/systems/bread/alpha/database.cljc @@ -118,8 +118,13 @@ (contains? ks (migration-key migration)))) (defmethod bread/effect ::transact - [{:keys [conn txs]} _] - (transact conn {:tx-data txs})) + [{:keys [conn txs success-key error-key]} _] + (try + {:db (transact conn {:tx-data txs}) + :flash {:success-key success-key}} + (catch Throwable e + {:ex e + :flash {:error-key error-key}}))) (defn txs->effect [req txs & {desc :effect/description k :effect/key diff --git a/test/cms/systems/bread/alpha/plugin/account_test.clj b/test/cms/systems/bread/alpha/plugin/account_test.clj new file mode 100644 index 00000000..bf05d791 --- /dev/null +++ b/test/cms/systems/bread/alpha/plugin/account_test.clj @@ -0,0 +1,475 @@ +(ns systems.bread.alpha.plugin.account-test + (:require + [buddy.hashers :as hashers] + [clojure.test :refer [deftest are]] + + [systems.bread.alpha.test-helpers :refer [db->plugin + plugins->loaded + use-db]] + [systems.bread.alpha.core :as bread] + [systems.bread.alpha.database :as db] + [systems.bread.alpha.dispatcher :as dispatcher] + [systems.bread.alpha.i18n :as i18n] + [systems.bread.alpha.plugin.account :as account] + [systems.bread.alpha.plugin.auth :as auth] + [systems.bread.alpha.ring :as ring] + [systems.bread.alpha.schema :as schema])) + +(def db-config + {:db/type :datahike + :db/migrations schema/initial + :db/config {:store {:backend :mem :id "account-test-db"}}}) + +(use-db :each db-config) + +(deftest test-account=> + (let [db-plugin (db->plugin ::FAKEDB) + db-conn (:db/connection (:config db-plugin)) + query-user + {:expansion/args ['{:find [(pull ?e [:user/attrs]) .] + :in [$ ?e]} + 123] + :expansion/db ::FAKEDB + :expansion/description "Query for all user account data" + :expansion/key :user + :expansion/name ::db/query} + expand-user + {:expansion/description "Expand user data" + :expansion/key :user + :expansion/name ::account/user} + success-hook + {:action/name ::ring/effect-redirect + :action/description "Redirect to account page after taking an account action" + :effect/key :update-details + :to "/account"}] + (are + [expected config req] + (= expected (let [dispatcher {:dispatcher/type ::account/account=> + :dispatcher/pull [:user/attrs]} + {:keys [account-config auth-config]} config + app (plugins->loaded [db-plugin + (i18n/plugin + {:supported-langs [:be :de] + :lang-names {:be "Belarusian" + :de "Deutsche"}}) + (auth/plugin auth-config) + (account/plugin account-config)]) + req* (merge app req {::bread/dispatcher dispatcher})] + (with-redefs [hashers/derive (fn [pw {:keys [alg]}] + (str "[" alg "+" pw "]"))] + (bread/dispatch req*)))) + + ;; Just loading the account page. + {:expansions [query-user + expand-user + {:expansion/description "Supported languages" + :expansion/key :supported-langs + :expansion/name ::bread/value + :expansion/value [:be :de]} + {:expansion/key :lang-names + :expansion/name ::bread/value + :expansion/description "Language names for display" + :expansion/value {:be "Belarusian" + :de "Deutsche"}}]} + {} + {:request-method :get + :session {:user {:db/id 123}}} + + ;; Updating name. + {:expansions + [query-user expand-user] + :effects + [{:effect/name ::db/transact + :effect/key :update-details + :effect/description "Update account details" + :success-key :account/account-updated + :txs [{:db/id 123 :user/name "Spongebob Squarepants"}] + :conn db-conn}] + :hooks + {::bread/render [success-hook]}} + {} + {:request-method :post + :params {:action "update-details" + :name "Spongebob Squarepants"} + :session {:user {:db/id 123}}} + + ;; Updating preferences. + {:expansions [query-user expand-user] + :effects [{:effect/name ::db/transact + :effect/key :update-details + :effect/description "Update account details" + :success-key :account/account-updated + :txs [{:db/id 123 + :user/name "Spongebob Squarepants" + :user/preferences (pr-str {:pronouns "he/they" + :timezone "America/Los_Angeles"})}] + :conn db-conn}] + :hooks {::bread/render [success-hook]}} + {} + {:request-method :post + :params {:action "update-details" + :name "Spongebob Squarepants" + :pronouns "he/they" + :timezone "America/Los_Angeles"} + :session {:user {:db/id 123}}} + + ;; Updating preferences; custom account-uri. + {:expansions [query-user expand-user] + :effects [{:effect/name ::db/transact + :effect/key :update-details + :effect/description "Update account details" + :success-key :account/account-updated + :txs [{:db/id 123 + :user/name "Spongebob Squarepants" + :user/preferences (pr-str {:pronouns "he/they" + :timezone "America/Los_Angeles"})}] + :conn db-conn}] + :hooks {::bread/render [(assoc success-hook :to "/custom")]}} + {:account-config {:account-uri "/custom"}} + {:request-method :post + :params {:action "update-details" + :name "Spongebob Squarepants" + :pronouns "he/they" + :timezone "America/Los_Angeles"} + :session {:user {:db/id 123}}} + + ;; Invalid action. + {:hooks {::bread/render [{:action/name ::ring/redirect + :action/description + "Redirect to account page after an error" + :flash {:error-key :account/invalid-action} + :to "/account"}]}} + {} + {:request-method :post + :params {:action "booboo" + :name "Spongebob Squarepants" + :pronouns "he/they" + :timezone "America/Los_Angeles"} + :session {:user {:db/id 123}}} + + ;; Invalid action; custom URI. + {:hooks {::bread/render [{:action/name ::ring/redirect + :action/description + "Redirect to account page after an error" + :flash {:error-key :account/invalid-action} + :to "/custom"}]}} + {:account-config {:account-uri "/custom"}} + {:request-method :post + :params {:action "booboo" + :name "Spongebob Squarepants" + :pronouns "he/they" + :timezone "America/Los_Angeles"} + :session {:user {:db/id 123}}} + + ;; Any custom fields on the account form are treated as preferences. + {:expansions [query-user expand-user] + :effects [{:effect/name ::db/transact + :effect/key :update-details + :effect/description "Update account details" + :success-key :account/account-updated + :txs [{:db/id 123 + :user/name "Spongebob Squarepants" + :user/preferences (pr-str {:custom "something" + :other "something else"})}] + :conn db-conn}] + :hooks {::bread/render [success-hook]}} + {} + {:request-method :post + :params {:action "update-details" + :name "Spongebob Squarepants" + :custom "something" + :other "something else"} + :session {:user {:db/id 123}}} + + ;; Updating password - mismatched passwords error. + {:hooks {::bread/render [{:action/name ::ring/redirect + :action/description + "Redirect to account page after an error" + :flash {:error-key :auth/passwords-must-match} + :to "/account"}]}} + {} + {:request-method :post + :params {:action "update-details" + :name "Spongebob Squarepants" + :password "xyz" + :password-confirmation ""} + :session {:user {:db/id 123}}} + + ;; Updating password - only confirmation submitted. + {:hooks {::bread/render [{:action/name ::ring/redirect + :action/description + "Redirect to account page after an error" + :flash {:error-key :auth/passwords-must-match} + :to "/account"}]}} + {} + {:request-method :post + :params {:action "update-details" + :name "Spongebob Squarepants" + :password "" + :password-confirmation "xyz"} + :session {:user {:db/id 123}}} + + ;; Updating password - both fields submitted, but still mismatched + {:hooks {::bread/render [{:action/name ::ring/redirect + :action/description + "Redirect to account page after an error" + :flash {:error-key :auth/passwords-must-match} + :to "/account"}]}} + {} + {:request-method :post + :params {:action "update-details" + :name "Spongebob Squarepants" + :password "abc" + :password-confirmation "xyz"} + :session {:user {:db/id 123}}} + + ;; Updating password - too short. + {:hooks {::bread/render [{:action/name ::ring/redirect + :action/description + "Redirect to account page after an error" + ;; 12 is the default from auth + :flash {:error-key [:auth/password-must-be-at-least 12]} + :to "/account"}]}} + {} + {:request-method :post + :params {:action "update-details" + :name "Spongebob Squarepants" + :password "tooshort" + :password-confirmation "tooshort"} + :session {:user {:db/id 123}}} + + ;; Updating password - too short with custom auth config. + {:hooks {::bread/render [{:action/name ::ring/redirect + :action/description + "Redirect to account page after an error" + ;; 12 is the default from auth + :flash {:error-key [:auth/password-must-be-at-least 8]} + :to "/account"}]}} + {:auth-config {:min-password-length 8}} + {:request-method :post + :params {:action "update-details" + :name "Spongebob Squarepants" + :password "2shrt" + :password-confirmation "2shrt"} + :session {:user {:db/id 123}}} + + ;; Updating password - just long enough with custom config. + {:expansions [query-user expand-user] + :effects [{:effect/name ::db/transact + :effect/description "Update account details" + :effect/key :update-details + :success-key :account/account-updated + :txs [{:db/id 123 + :user/name "Spongebob Squarepants" + :user/password "[:bcrypt+blake2b-512+password]"}] + :conn db-conn}] + :hooks {::bread/render [success-hook]}} + {:auth-config {:min-password-length 8}} + {:request-method :post + :params {:action "update-details" + :name "Spongebob Squarepants" + :password "password" + :password-confirmation "password"} + :session {:user {:db/id 123}}} + + ;; Updating password - meets default 12-char minimum. + {:expansions [query-user expand-user] + :effects [{:effect/name ::db/transact + :effect/description "Update account details" + :effect/key :update-details + :success-key :account/account-updated + :txs [{:db/id 123 + :user/name "Spongebob Squarepants" + :user/password "[:bcrypt+blake2b-512+password1234]"}] + :conn db-conn}] + :hooks {::bread/render [success-hook]}} + {} + {:request-method :post + :params {:action "update-details" + :name "Spongebob Squarepants" + :password "password1234" + :password-confirmation "password1234"} + :session {:user {:db/id 123}}} + + ;; Updating password - meets default 12-char minimum. + {:expansions [query-user expand-user] + :effects [{:effect/name ::db/transact + :effect/description "Update account details" + :effect/key :update-details + :success-key :account/account-updated + :txs [{:db/id 123 + :user/name "Spongebob Squarepants" + :user/password (str + "[:bcrypt+blake2b-512+" + "twelve_chars" + "twelve_chars" + "twelve_chars" + "twelve_chars" + "twelve_chars" + "twelve_chars" + "]")}] + :conn db-conn}] + :hooks {::bread/render [success-hook]}} + {} + (let [;; 6 * 12 = 72 + long-password (apply str (repeat 6 "twelve_chars"))] + {:request-method :post + :params {:action "update-details" + :name "Spongebob Squarepants" + :password long-password + :password-confirmation long-password} + :session {:user {:db/id 123}}}) + + ;; Updating password - too long! + {:hooks {::bread/render [{:action/name ::ring/redirect + :action/description + "Redirect to account page after an error" + ;; 12 is the default from auth + :flash {:error-key [:auth/password-must-be-at-most 72]} + :to "/account"}]}} + {} + (let [;; 1 + 6 * 12 = 73 + long-password (apply str "x" (repeat 6 "twelve_chars"))] + {:request-method :post + :params {:action "update-details" + :name "Spongebob Squarepants" + :password long-password + :password-confirmation long-password} + :session {:user {:db/id 123}}}) + + ;; Updating password - too long with custom auth config. + {:hooks {::bread/render [{:action/name ::ring/redirect + :action/description + "Redirect to account page after an error" + ;; 12 is the default from auth + :flash {:error-key [:auth/password-must-be-at-most 71]} + :to "/account"}]}} + {:auth-config {:max-password-length 71}} + (let [;; 6 * 12 = 72 + long-password (apply str (repeat 6 "twelve_chars"))] + {:request-method :post + :params {:action "update-details" + :name "Spongebob Squarepants" + :password long-password + :password-confirmation long-password} + :session {:user {:db/id 123}}}) + + ;; Deleting a session. + {:expansions [query-user expand-user] + :effects [{:effect/name [::account/update :delete-session] + :effect/description "Update account state" + :effect/key :delete-session + :success-key :account/session-deleted + :params {:action "delete-session" :dbid "456"} + :conn db-conn}] + :hooks {::bread/render [(assoc success-hook :effect/key :delete-session)]}} + {} + {:request-method :post + :params {:action "delete-session" + :dbid "456"} + :session {:user {:db/id 123}}} + + ,))) + +(deftest test-delete-session-effect + (are + [expected effect data] + (= expected (bread/effect effect data)) + + ;; Anonymous, no params. + nil + {:effect/name [::account/update :delete-session] + :effect/description "Update account state" + :effect/key :delete-session + :success-key :account/session-deleted + :params nil + :conn ::FAKEDB} + {:user nil} + + ;; Anonymous attempt to delete a session. + nil + {:effect/name [::account/update :delete-session] + :effect/description "Update account state" + :effect/key :delete-session + :success-key :account/session-deleted + :params {:dbid "345"} + :conn ::FAKEDB} + {:user nil} + + ;; Invalid session id. + nil + {:effect/name [::account/update :delete-session] + :effect/description "Update account state" + :effect/key :delete-session + :success-key :account/session-deleted + :params {:dbid "invalid"} + :conn ::FAKEDB} + {:user {:user/sessions [{:db/id 234} {:db/id 456} {:db/id 567}]}} + + ;; Invalid session id. + nil + {:effect/name [::account/update :delete-session] + :effect/description "Update account state" + :effect/key :delete-session + :success-key :account/session-deleted + :params {:dbid ""} + :conn ::FAKEDB} + {:user {:user/sessions [{:db/id 234} {:db/id 456} {:db/id 567}]}} + + ;; Invalid session id. + nil + {:effect/name [::account/update :delete-session] + :effect/description "Update account state" + :effect/key :delete-session + :success-key :account/session-deleted + :params {:dbid nil} + :conn ::FAKEDB} + {:user {:user/sessions [{:db/id 234} {:db/id 456} {:db/id 567}]}} + + ;; Attempt to delete a different user's session. + nil + {:effect/name [::account/update :delete-session] + :effect/description "Update account state" + :effect/key :delete-session + :success-key :account/session-deleted + :params {:dbid "345"} + :conn ::FAKEDB} + {:user {:user/sessions [{:db/id 234} {:db/id 456} {:db/id 567}]}} + + ;; Happy path. + {:effects + [{:effect/name ::db/transact + :effect/description "Delete a user session." + :effect/key :delete-session + :success-key :account/session-deleted + :conn ::FAKEDB + :txs [[:db/retractEntity 123]]}]} + {:effect/name [::account/update :delete-session] + :effect/description "Update account state" + :effect/key :delete-session + :success-key :account/session-deleted + :params {:dbid "123"} + :conn ::FAKEDB} + {:user {:user/sessions [{:db/id 123}]}} + + ;; Happy path; more than one session. + {:effects + [{:effect/name ::db/transact + :effect/description "Delete a user session." + :effect/key :delete-session + :success-key :account/session-deleted + :conn ::FAKEDB + :txs [[:db/retractEntity 456]]}]} + {:effect/name [::account/update :delete-session] + :effect/description "Update account state" + :effect/key :delete-session + :success-key :account/session-deleted + :params {:dbid "456"} + :conn ::FAKEDB} + {:user {:user/sessions [{:db/id 123} {:db/id 456}]}} + + ,)) + +(comment + (require '[kaocha.repl :as k]) + (k/run {:color? false}))