From 229eeae6dbcae4d8d0da94ac87c4c9ecb5a1b893 Mon Sep 17 00:00:00 2001 From: Pablo Alba Date: Thu, 12 Sep 2024 16:11:15 +0200 Subject: [PATCH] :bug: Fix bad redirect on new oops page with social login --- frontend/src/app/main/data/users.cljs | 22 ++-- frontend/src/app/main/ui/auth/login.cljs | 14 ++- frontend/src/app/main/ui/static.cljs | 3 +- frontend/src/app/util/router.cljs | 14 ++- frontend/src/app/util/storage.cljs | 151 +++++++++++++++++------ 5 files changed, 154 insertions(+), 50 deletions(-) diff --git a/frontend/src/app/main/data/users.cljs b/frontend/src/app/main/data/users.cljs index e8369a2f9..9cf0f9966 100644 --- a/frontend/src/app/main/data/users.cljs +++ b/frontend/src/app/main/data/users.cljs @@ -168,13 +168,21 @@ accepting invitation, or third party auth signup or singin." [profile] (letfn [(get-redirect-events [] - (let [team-id (get-current-team-id profile) - welcome-file-id (get-in profile [:props :welcome-file-id])] - (if (some? welcome-file-id) - (rx/of - (rt/nav' :workspace {:project-id (:default-project-id profile) - :file-id welcome-file-id}) - (update-profile-props {:welcome-file-id nil})) + (let [team-id (get-current-team-id profile) + welcome-file-id (dm/get-in profile [:props :welcome-file-id]) + redirect-href (:login-redirect @s/session)] + (cond + (some? redirect-href) + (binding [s/*sync* true] + (swap! s/session dissoc :login-redirect) + (rx/of (rt/nav-raw :href redirect-href))) + + (some? welcome-file-id) + (rx/of (rt/nav' :workspace {:project-id (:default-project-id profile) + :file-id welcome-file-id}) + (update-profile-props {:welcome-file-id nil})) + + :else (rx/of (rt/nav' :dashboard-projects {:team-id team-id})))))] (ptk/reify ::logged-in diff --git a/frontend/src/app/main/ui/auth/login.cljs b/frontend/src/app/main/ui/auth/login.cljs index 0d0b587a9..5533a9171 100644 --- a/frontend/src/app/main/ui/auth/login.cljs +++ b/frontend/src/app/main/ui/auth/login.cljs @@ -23,6 +23,7 @@ [app.util.i18n :refer [tr]] [app.util.keyboard :as k] [app.util.router :as rt] + [app.util.storage :as s] [beicon.v2.core :as rx] [rumext.v2 :as mf])) @@ -47,10 +48,21 @@ (defn- login-with-oidc [event provider params] (dom/prevent-default event) + + (binding [s/*sync* true] + (if (some? (:save-login-redirect params)) + ;; Save the current login raw uri for later redirect user back to + ;; the same page, we need it to be synchronous because the user is + ;; going to be redirected instantly to the oidc provider uri + (swap! s/session assoc :login-redirect (rt/get-current-href)) + ;; Clean the login redirect + (swap! s/session dissoc :login-redirect))) + + ;; FIXME: this code should be probably moved outside of the UI (->> (rp/cmd! :login-with-oidc (assoc params :provider provider)) (rx/subs! (fn [{:keys [redirect-uri] :as rsp}] (if redirect-uri - (.replace js/location redirect-uri) + (st/emit! (rt/nav-raw :uri redirect-uri)) (log/error :hint "unexpected response from OIDC method" :resp (pr-str rsp)))) (fn [cause] diff --git a/frontend/src/app/main/ui/static.cljs b/frontend/src/app/main/ui/static.cljs index 06333087c..08c6ec06b 100644 --- a/frontend/src/app/main/ui/static.cljs +++ b/frontend/src/app/main/ui/static.cljs @@ -118,7 +118,8 @@ [:div {:class (stl/css :logo-title)} (tr "labels.login")] [:div {:class (stl/css :logo-subtitle)} (tr "not-found.login.free")] [:& login-methods {:on-recovery-request set-section-recovery - :on-success-callback success-login}] + :on-success-callback success-login + :params {:save-login-redirect true}}] [:hr {:class (stl/css :separator)}] [:div {:class (stl/css :change-section)} (tr "auth.register") diff --git a/frontend/src/app/util/router.cljs b/frontend/src/app/util/router.cljs index b556bd84b..9180d5d77 100644 --- a/frontend/src/app/util/router.cljs +++ b/frontend/src/app/util/router.cljs @@ -151,11 +151,20 @@ (set! (.-href globals/location) "/")) (defn nav-raw - [href] + [& {:keys [href uri]}] (ptk/reify ::nav-raw ptk/EffectEvent (effect [_ _ _] - (set! (.-href globals/location) href)))) + (cond + (string? uri) + (.replace globals/location uri) + + (string? href) + (set! (.-href globals/location) href))))) + +(defn get-current-href + [] + (.-href globals/location)) (defn get-current-path [] @@ -164,6 +173,7 @@ (subs hash 1) hash))) + ;; --- History API (defn initialize-history diff --git a/frontend/src/app/util/storage.cljs b/frontend/src/app/util/storage.cljs index df08e0eac..2be417230 100644 --- a/frontend/src/app/util/storage.cljs +++ b/frontend/src/app/util/storage.cljs @@ -10,75 +10,148 @@ [app.common.transit :as t] [app.util.functions :as fns] [app.util.globals :as g] - [cuerdas.core :as str])) + [cuerdas.core :as str] + [okulary.util :as ou])) ;; Using ex/ignoring because can receive a DOMException like this when ;; importing the code as a library: Failed to read the 'localStorage' ;; property from 'Window': Storage is disabled inside 'data:' URLs. -(defonce ^:private local-storage +(defonce ^:private local-storage-backend (ex/ignoring (unchecked-get g/global "localStorage"))) +(defonce ^:private session-storage-backend + (ex/ignoring (unchecked-get g/global "sessionStorage"))) + +(def ^:dynamic *sync* + "Dynamic variable which determines the mode of operation of the + storage mutatio actions. By default is asynchronous." + false) + (defn- encode-key - [k] + [prefix k] (assert (keyword? k) "key must be keyword") (let [kns (namespace k) kn (name k)] - (str "penpot:" kns "/" kn))) + (str prefix ":" kns "/" kn))) (defn- decode-key - [k] - (when (str/starts-with? k "penpot:") - (let [k (subs k 7)] + [prefix k] + (when (str/starts-with? k prefix) + (let [l (+ (count prefix) 1) + k (subs k l)] (if (str/starts-with? k "/") (keyword (subs k 1)) (let [[kns kn] (str/split k "/" 2)] (keyword kns kn)))))) (defn- lookup-by-index - [result index] + [backend prefix result index] (try - (let [key (.key ^js local-storage index) - key' (decode-key key)] + (let [key (.key ^js backend index) + key' (decode-key prefix key)] (if key' - (let [val (.getItem ^js local-storage key)] + (let [val (.getItem ^js backend key)] (assoc! result key' (t/decode-str val))) result)) (catch :default _ result))) -(defn- load - [] - (when (some? local-storage) - (let [length (.-length ^js local-storage)] +(defn- load-data + [backend prefix] + (if (some? backend) + (let [length (.-length ^js backend)] (loop [index 0 result (transient {})] (if (< index length) (recur (inc index) - (lookup-by-index result index)) - (persistent! result)))))) + (lookup-by-index backend prefix result index)) + (persistent! result)))) + {})) -(defonce ^:private latest-state (load)) +(defn create-storage + [backend prefix] + (let [initial (load-data backend prefix) + curr-data #js {:content initial} + last-data #js {:content initial} + watches (js/Map.) -(defn- on-change* - [curr-state] - (let [prev-state latest-state] - (try - (run! (fn [key] - (let [prev-val (get prev-state key) - curr-val (get curr-state key)] - (when-not (identical? curr-val prev-val) - (if (some? curr-val) - (.setItem ^js local-storage (encode-key key) (t/encode-str curr-val)) - (.removeItem ^js local-storage (encode-key key)))))) - (into #{} (concat (keys curr-state) - (keys prev-state)))) - (finally - (set! latest-state curr-state))))) + update-key + (fn [key val] + (when (some? backend) + (if (some? val) + (.setItem ^js backend (encode-key prefix key) (t/encode-str val)) + (.removeItem ^js backend (encode-key prefix key))))) -(defonce on-change - (fns/debounce on-change* 2000)) + on-change* + (fn [curr-state] + (let [prev-state (unchecked-get last-data "content")] + (try + (run! (fn [key] + (let [prev-val (get prev-state key) + curr-val (get curr-state key)] + (when-not (identical? curr-val prev-val) + (update-key key curr-val)))) + (into #{} (concat (keys curr-state) + (keys prev-state)))) + (finally + (unchecked-set last-data "content" curr-state))))) -(defonce storage (atom latest-state)) -(add-watch storage :persistence - (fn [_ _ _ curr-state] - (on-change curr-state))) + on-change + (fns/debounce on-change* 2000)] + + (reify + IAtom + + IDeref + (-deref [_] (unchecked-get curr-data "content")) + + ILookup + (-lookup [coll k] + (-lookup coll k nil)) + (-lookup [_ k not-found] + (let [state (unchecked-get curr-data "content")] + (-lookup state k not-found))) + + IReset + (-reset! [self newval] + (let [oldval (unchecked-get curr-data "content")] + (unchecked-set curr-data "content" newval) + (if *sync* + (on-change* newval) + (on-change newval)) + (when (> (.-size watches) 0) + (-notify-watches self oldval newval)) + newval)) + + ISwap + (-swap! [self f] + (let [state (unchecked-get curr-data "content")] + (-reset! self (f state)))) + (-swap! [self f x] + (let [state (unchecked-get curr-data "content")] + (-reset! self (f state x)))) + (-swap! [self f x y] + (let [state (unchecked-get curr-data "content")] + (-reset! self (f state x y)))) + (-swap! [self f x y more] + (let [state (unchecked-get curr-data "content")] + (-reset! self (apply f state x y more)))) + + IWatchable + (-notify-watches [self oldval newval] + (ou/doiter + (.entries watches) + (fn [n] + (let [f (aget n 1) + k (aget n 0)] + (f k self oldval newval))))) + + (-add-watch [self key f] + (.set watches key f) + self) + + (-remove-watch [_ key] + (.delete watches key))))) + +(defonce storage (create-storage local-storage-backend "penpot")) +(defonce session (create-storage session-storage-backend "penpot"))