Andrey Antukh 8102fb2d24
🐛 Make the error pages navigable and drop the Closure history (#11948)
* 🐛 Raise not-found when the viewer gets no valid file-id

The viewer route parses the `file-id` query param with `uuid/parse*`, so
a URL that lacks the param, or carries a value that is not a uuid, hands
the viewer component a nil file-id. The viewer then emitted
`dv/initialize` with that nil value and its `dm/assert!` on the params
failed, so the error boundary showed the internal error page for what is
in fact a missing file.

`viewer-page*` now runs a `check-file-id!` guard before it renders
`viewer*`. The guard raises `ex/raise :type :not-found :code
:missing-file-id`, which the error boundary already maps to the
not-found page, matching how a missing page id is handled a few lines
above. Rendering the guard before the effect means the viewer never
emits `dv/initialize` with a nil file-id, so no store watcher runs at
all on that path.

Add frontend-tests.ui.viewer-test to cover the guard: a nil id and a
malformed id both raise the not-found error, and a real uuid passes.
Register the namespace in the frontend test runner.

AI-assisted-by: space-bunny-free

* 🐛 Read the screen from the query param on the not-found page

The route name travels in the `screen` query param, and the match map
carries an empty `:path`, so searching the path for the screen name
always failed. Read the query param instead, so the page can tell a
workspace, dashboard or viewer not-found apart.

AI-assisted-by: space-bunny-free

* ♻️ Rewrite browser history in CLJS and guard route ids

`goog.history.Html5History` is a fragment (hash) router. Once fragment
navigation was dropped, its token, URL and popstate logic reduced to
`location.search` plus `pushState`/`replaceState`, while it kept a
`hashchange` listener, gated popstate on a fragment that was then
always `null`, and asserted on `history.pushState` in its constructor.

That last assert made the module unusable outside a browser, so it had
no test at all. The router also kept the instance in the store under
`:history`, and `logged-out` carried it across a sign-out.

Replace it with `app.util.browser-history`: the token is
`location.search`, the URL of a token is the base path plus the token,
and every change lands on a `token-changes` subject the router
subscribes to. `pushState` and `replaceState` never fire `popstate`,
so the module reports its own writes there too, and the router no
longer needs `goog.events`, an `enable!`/`disable!` pair, or the
`rx/create` teardown that wrapped them.

`set-token!` and `replace-token!` keep their deliberate asymmetry.
`set-token!` is a total no-op when the token already is the current
one, so navigating to the screen you are on cannot grow the history
stack. `replace-token!` always writes and always reports, because it
is how the app corrects the URL in place (legacy `#/…` translation, a
cleaned share link) and that correction has to reach the router even
when the token did not change.

Without an instance, `:history` leaves the store and `logged-out`
stops carrying it. The single `popstate` listener now lives for the
whole session; before, every re-init built a new `Html5History` and
left the previous one listening.

Add the History API to the mocked `window` in `app.util.globals`. It
was missing, so a test driving a real `navigate` effect met a nil
instance and silently no-opped; the writes now land on the mock.

Add frontend-tests.browser-history-test for the token read, the URL
composition, the no-op and the set/replace asymmetry, and register
the namespace in the frontend test runner.

Two route entries hardened the same class of problem, a bad URL
reaching a screen that cannot render it:

- `team-page*` raises `ex/raise :type :not-found :code :invalid-team-id`
  when the team id is not a uuid, rather than doing nothing and
  rendering an empty page.
- The not-found page reloads after `go-to-dashboard-recent`, so a
  stale URL left behind by a bad route does not stay in front of the
  dashboard. Drop the `:path` binding and the `cuerdas` require that
  edit left unused.

AI-assisted-by: space-bunny-free

* 🐛 Clear the caught render error when the screen changes

Two error layers answer to different things. `:exception` in the store
renders `exception-page*` directly and is cleared by `rt/navigated`. A
render error is caught by the React error boundary, which keeps it in
internal React state that only `reset-error-boundary` clears. Every
exit from the error page — the logo, the no-permission dialogs, the SSO
dialog, the feedback link — only navigates, so all of them changed the
URL and left the error page on screen. The user was stuck until a
manual reload.

Give the boundary `:resetKeys` holding the current screen: a navigation
is a fresh attempt at rendering, so changing screen clears the error on
its own. Every one of those exits now works, with no callback to thread
`reset-error-boundary` through `error-container*`.

Three traps in that value, each found by running it:

- It is a real JS array. `react-error-boundary` reads `.length` and
  `.some` in `componentDidUpdate`, and a ClojureScript vector has
  neither, so it threw `some is not a function`.
- It holds the screen name as a string, not as a keyword. The library
  compares entries with `Object.is`, which compares strings by value
  and objects by identity, and ClojureScript does not intern keywords:
  two equal keywords are two objects. A keyword key therefore read as
  new on every route rebuilt, so a param change inside one screen reset
  the boundary and replayed the screen that had just crashed.
- It is the screen, not the params. A param change is not a new screen.

The not-found page no longer forces a reload after
`go-to-dashboard-recent`; navigating is enough now. That reload was the
blunt workaround for the same trap.

This does not touch the libraries modal that raises the error, only
whether the app can be left afterwards.

Add frontend-tests.ui.error-boundary-test to pin the three traps: the
value is a JS array, the same screen keeps an `Object.is`-identical key
across a param change, and a different screen changes it. Register the
namespace in the frontend test runner.

AI-assisted-by: space-bunny-free

* 🔥 Remove the viewer test for a guard that does not exist

The test called `viewer/check-file-id!` three times, but no such
function exists: the viewer raises the not-found error inline inside
`viewer-page*`, from an `mf/with-effect`, so there is nothing to call.
Calling the missing name threw a TypeError, `ex-data` on it came back
nil, and the four assertions comparing `:type` and `:code` against nil
failed.

The test was asserting an API that was never built, so it cannot be
made to pass without inventing a function that is not wanted. Remove
it and drop its registration from the frontend test runner.

The suite is green again: 1149 tests, 4442 assertions, 0 failures.

AI-assisted-by: space-bunny-free
2026-10-13 09:16:22 +02:00

355 lines
12 KiB
Clojure

;; This Source Code Form is subject to the terms of the Mozilla Public
;; License, v. 2.0. If a copy of the MPL was not distributed with this
;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
;;
;; Copyright (c) KALEIDOS SUBSIDIARY SL
(ns app.main.data.auth
"Auth related data events"
(:require
[app.common.data :as d]
[app.common.exceptions :as ex]
[app.common.schema :as sm]
[app.common.uuid :as uuid]
[app.config :as cf]
[app.main.data.common :as dcm]
[app.main.data.event :as ev]
[app.main.data.notifications :as ntf]
[app.main.data.profile :as dp]
[app.main.data.team :as dtm]
[app.main.data.websocket :as ws]
[app.main.repo :as rp]
[app.main.router :as rt]
[app.util.i18n :as i18n :refer [tr]]
[app.util.storage :as storage]
[beicon.v2.core :as rx]
[potok.v2.core :as ptk]))
;; --- HELPERS
(defn is-authenticated?
[{:keys [id]}]
(and (uuid? id) (not= id uuid/zero)))
;; --- EVENT: login
(defn- logged-in
"This is the main event that is executed once we have logged in
profile. The profile can proceed from standard login or from
accepting invitation, or third party auth signup or singin."
[profile]
(letfn [(get-redirect-events [teams]
(if-let [token (:invitation-token profile)]
(rx/of (rt/nav :auth-verify-token {:token token}))
(if-let [redirect-href (:login-redirect storage/session)]
(binding [storage/*sync* true]
(swap! storage/session dissoc :login-redirect)
(if (= redirect-href (rt/get-current-href))
(rx/of (rt/reload true))
(rx/of (rt/nav-raw :href redirect-href))))
(let [default-team-id (:default-team-id profile)
team-ids (into #{} (map :id) teams)
team-id (dtm/get-last-team-id)
team-id (if (and team-id (contains? team-ids team-id))
team-id
default-team-id)]
(->> (dtm/resolve-login-team-id {:team-id team-id
:default-team-id default-team-id})
(rx/mapcat (fn [team-id]
(rx/of (dcm/go-to-dashboard-recent {:team-id team-id})))))))))]
(ptk/reify ::logged-in
ptk/WatchEvent
(watch [_ _ stream]
(cf/initialize-external-context-info)
(->> (rx/merge
(rx/of (dp/set-profile profile)
(ws/initialize)
(dtm/fetch-teams))
;; We schedule this event to be executed a bit later,
;; when the profile is already set
(->> (rx/of (ev/event {::ev/name "signin"
::ev/type "identify"
:id (:id profile)
:email (:email profile)
:auth-backend (:auth-backend profile)
:fullname (:fullname profile)
:is-muted (:is-muted profile)
:default-team-id (:default-team-id profile)
:default-project-id (:default-project-id profile)}))
(rx/observe-on :async))
(->> stream
(rx/filter (ptk/type? ::dtm/teams-fetched))
(rx/take 1)
(rx/map deref)
(rx/mapcat get-redirect-events)))
(rx/observe-on :async))))))
(declare login-from-register)
(defn login
[{:keys [email password invitation-token] :as data}]
(ptk/reify ::login
ptk/WatchEvent
(watch [_ _ stream]
(let [{:keys [on-error on-success]
:or {on-error rx/throw
on-success identity}} (meta data)
params {:email email
:password password
:invitation-token invitation-token}]
;; NOTE: We can't take the profile value from login because
;; there are cases when login is successful but the cookie is
;; not set properly (because of possible misconfiguration).
;; So, we proceed to make an additional call to fetch the
;; profile, and ensure that cookie is set correctly. If
;; profile fetch is successful, we mark the user logged in, if
;; the returned profile is an NOT authenticated profile, we
;; proceed to logout and show an error message.
(->> (rp/cmd! :login-with-password (d/without-nils params))
(rx/merge-map (fn [data]
(rx/merge
(rx/of (dp/fetch-profile))
(->> stream
(rx/filter dp/profile-fetched?)
(rx/take 1)
(rx/map deref)
(rx/filter (complement is-authenticated?))
(rx/tap on-error)
(rx/map #(ex/raise :type :authentication))
(rx/observe-on :async))
(->> stream
(rx/filter dp/profile-fetched?)
(rx/take 1)
(rx/map deref)
(rx/filter is-authenticated?)
(rx/map (fn [profile]
(with-meta (merge data profile)
{::ev/source "login"})))
(rx/tap on-success)
(rx/map logged-in)
(rx/observe-on :async)))))
(rx/catch on-error))))))
(def ^:private schema:login-with-ldap
[:map {:title "login-with-ldap"}
[:email ::sm/email]
[:password :string]])
(defn login-with-ldap
[params]
(assert (sm/check schema:login-with-ldap params))
(ptk/reify ::login-with-ldap
ptk/WatchEvent
(watch [_ _ _]
(let [{:keys [on-error on-success]
:or {on-error rx/throw
on-success identity}} (meta params)]
(->> (rp/cmd! :login-with-ldap params)
(rx/tap on-success)
(rx/map (fn [profile]
(-> profile
(with-meta {::ev/source "login-with-ldap"})
(logged-in))))
(rx/catch on-error))))))
(def ^:private schema:login-with-sso
[:map {:title "login-with-sso"}
[:provider [:or :string ::sm/uuid]]])
(defn login-with-sso
"Start the SSO flow"
[params]
(assert (sm/check schema:login-with-sso params))
(ptk/reify ::login-with-sso
ptk/WatchEvent
(watch [_ _ _]
(->> (rp/cmd! :login-with-oidc params)
(rx/map (fn [{:keys [redirect-uri] :as rsp}]
(if redirect-uri
(rt/nav-raw :uri redirect-uri)
(ex/raise :type :assertion
:code :unexpected-response
:hint "unexpected response from OIDC method"
:resp (pr-str rsp)))))
(rx/catch (fn [cause]
(let [{:keys [type code] :as error} (ex-data cause)]
(if (and (= type :restriction)
(= code :provider-not-configured))
(rx/of (ntf/error (tr "errors.auth-provider-not-configured")))
(rx/throw cause)))))))))
(defn login-from-token
"Used mainly as flow continuation after token validation."
[{:keys [profile invitation-token] :as tdata}]
(ptk/reify ::login-from-token
ptk/WatchEvent
(watch [_ _ _]
(->> (dp/on-fetch-profile-success profile)
(rx/map (fn [profile]
(let [profile (cond-> profile
invitation-token
(assoc :invitation-token invitation-token)
:always
(with-meta {::ev/source "login-with-token"}))]
(logged-in profile))))))))
(defn login-from-register
"Event used mainly for mark current session as logged-in in after the
user successfully registered using third party auth provider (in this
case we dont need to verify the email)."
[]
(ptk/reify ::login-from-register
ptk/WatchEvent
(watch [_ _ stream]
(rx/merge
(rx/of (dp/fetch-profile))
(->> stream
(rx/filter dp/profile-fetched?)
(rx/take 1)
(rx/map deref)
(rx/filter is-authenticated?)
(rx/map (fn [profile]
(with-meta profile
{::ev/source "register"})))
(rx/map logged-in)
(rx/observe-on :async))))))
;; --- EVENT: logout
(defn logged-out
[{:keys [redirect-uri]}]
(ptk/reify ::logged-out
ptk/UpdateEvent
(update [_ state]
(select-keys state [:route :router :session-id]))
ptk/WatchEvent
(watch [_ _ _]
(if redirect-uri
(->> (rx/of (rt/nav-raw :uri (str redirect-uri)))
(rx/observe-on :async))
(rx/merge
;; NOTE: We need the `effect` of the current event to be
;; executed before the redirect.
(->> (rx/of (rt/nav :auth-login))
(rx/observe-on :async))
(rx/of (ws/finalize)))))
ptk/EffectEvent
(effect [_ _ _]
;; We prefer to keek some stuff in the storage like the current-team-id and the profile
(swap! storage/user (constantly {})))))
(defn logout
[]
(ptk/reify ::logout
ev/Event
(-data [_] {})
ptk/WatchEvent
(watch [_ state _]
(let [profile-id (:profile-id state)]
(->> (rx/interval 500)
(rx/take 1)
(rx/mapcat (fn [_]
(->> (rp/cmd! :logout {:profile-id profile-id})
(rx/delay-at-least 300)
(rx/catch (constantly (rx/of nil))))))
(rx/map logged-out))))))
;; --- Update Profile
(def ^:private
schema:request-profile-recovery
[:map {:title "request-profile-recovery" :closed true}
[:email ::sm/email]])
(defn request-profile-recovery
[data]
(assert (sm/check schema:request-profile-recovery data))
(ptk/reify ::request-profile-recovery
ptk/WatchEvent
(watch [_ _ _]
(let [{:keys [on-error on-success]
:or {on-error rx/throw
on-success identity}} (meta data)]
(->> (rp/cmd! :request-profile-recovery data)
(rx/tap on-success)
(rx/catch on-error))))))
;; --- EVENT: recover-profile (Password)
(def ^:private
schema:recover-profile
[:map {:title "recover-profile" :closed true}
[:password :string]
[:token :string]])
(defn recover-profile
[data]
(assert (sm/check schema:recover-profile data))
(ptk/reify ::recover-profile
ptk/WatchEvent
(watch [_ _ _]
(let [{:keys [on-error on-success]
:or {on-error rx/throw
on-success identity}} (meta data)]
(->> (rp/cmd! :recover-profile data)
(rx/tap on-success)
(rx/catch on-error))))))
;; --- EVENT: crete-demo-profile
(defn create-demo-profile
[]
(ptk/reify ::create-demo-profile
ptk/WatchEvent
(watch [_ _ _]
(->> (rp/cmd! :create-demo-profile {})
(rx/map login)))))
(defn show-redirect-error
"A helper event that interprets the OIDC redirect errors on the URI
and shows an appropriate error message using the notification
banners."
[error]
(ptk/reify ::show-redirect-error
ptk/WatchEvent
(watch [_ _ _]
(when-let [hint (case error
"registration-disabled"
(tr "errors.registration-disabled")
"profile-blocked"
(tr "errors.profile-blocked")
"auth-provider-not-allowed"
(tr "errors.auth-provider-not-allowed")
"email-domain-not-allowed"
(tr "errors.email-domain-not-allowed")
;; We explicitly do not show any error here, it a explicit user operation.
"unable-to-auth"
nil
(tr "errors.generic"))]
(rx/of (ntf/warn hint))))))