diff --git a/backend/src/app/auth/oidc.clj b/backend/src/app/auth/oidc.clj index c20260f2e6..c32f3bbf75 100644 --- a/backend/src/app/auth/oidc.clj +++ b/backend/src/app/auth/oidc.clj @@ -652,17 +652,10 @@ (defn- redirect-with-organization-sso-error [{:keys [dest-url organization-id]}] - (let [uri (u/uri (or dest-url (cf/get :public-uri))) - params (d/without-nils {:sso-error true :organization-id organization-id}) - qs (u/map->query-string params) - fragment (or (:fragment uri) "") - parsed-frag (when-not (str/blank? fragment) (u/parse fragment)) - new-fragment (if parsed-frag - (let [existing-query (or (:query parsed-frag) "") - combined (if (str/blank? existing-query) qs (str existing-query "&" qs))] - (str (u/uri (assoc parsed-frag :query combined)))) - (str "?" qs))] - (redirect-response (assoc uri :fragment new-fragment)))) + (-> (str (or dest-url (cf/get :public-uri))) + (u/append-query-param :sso-error true) + (u/append-query-param :organization-id organization-id) + (redirect-response))) (defn- redirect-to-register [cfg info provider] @@ -901,6 +894,39 @@ {::yres/status 200 ::yres/body {:redirect-uri uri}})) +(defn- organization-sso-callback-handler + "Handle the organization-SSO branch of the OIDC callback: state carries + :dest-url — exchange the authorization code with the OIDC provider to + verify authentication actually occurred, then redirect back to dest-url." + [cfg request state code] + (let [dest-url (:dest-url state)] + (try + (let [organization-id (:organization-id state) + sso (nitrate/call cfg :get-organization-sso {:organization-id organization-id}) + provider (prepare-organization-sso-provider cfg sso) + info (get-info cfg provider state code) + session (session/get-session request) + exp (or (:sso-token-exp info) (ct/in-future {:hours 48}))] + (when (and session organization-id) + (let [props (-> (or (:props session) {}) + (update :sso assoc organization-id exp))] + (session/update-session (::session/manager cfg) (assoc session :props props)))) + (redirect-response dest-url)) + (catch Throwable cause + (let [{:keys [code]} (ex-data cause)] + (binding [l/*context* (errors/request->context request)] + (if (some? code) + (l/warn :hint "organization sso callback failed" + :code code + :message (ex-message cause) + :organization-id (:organization-id state)) + (l/err :hint "unexpected error on organization sso callback" + :organization-id (:organization-id state) + :cause cause)))) + (redirect-with-organization-sso-error + {:dest-url dest-url + :organization-id (:organization-id state)}))))) + (defn- callback-handler [cfg {:keys [params] :as request}] (if-let [error (get params :error)] @@ -912,33 +938,8 @@ ;; Organization SSO flow: state carries :dest-url — exchange the authorization ;; code with the OIDC provider to verify authentication actually occurred. - (if-let [dest-url (:dest-url state)] - (try - (let [organization-id (:organization-id state) - sso (nitrate/call cfg :get-organization-sso {:organization-id organization-id}) - provider (prepare-organization-sso-provider cfg sso) - info (get-info cfg provider state code) - session (session/get-session request) - exp (or (:sso-token-exp info) (ct/in-future {:hours 48}))] - (when (and session organization-id) - (let [props (-> (or (:props session) {}) - (update :sso assoc organization-id exp))] - (session/update-session (::session/manager cfg) (assoc session :props props)))) - (redirect-response dest-url)) - (catch Throwable cause - (let [{:keys [code]} (ex-data cause)] - (binding [l/*context* (errors/request->context request)] - (if (some? code) - (l/warn :hint "organization sso callback failed" - :code code - :message (ex-message cause) - :organization-id (:organization-id state)) - (l/err :hint "unexpected error on organization sso callback" - :organization-id (:organization-id state) - :cause cause)))) - (redirect-with-organization-sso-error - {:dest-url dest-url - :organization-id (:organization-id state)}))) + (if (:dest-url state) + (organization-sso-callback-handler cfg request state code) (let [provider (resolve-provider cfg state) info (get-info cfg provider state code) diff --git a/common/src/app/common/uri.cljc b/common/src/app/common/uri.cljc index b82b5c0e74..c61cc69737 100644 --- a/common/src/app/common/uri.cljc +++ b/common/src/app/common/uri.cljc @@ -67,6 +67,36 @@ path (str path "/"))))) +(defn- update-query-params + "Apply `f` to the query-params map of `url`, returning the updated URL string. + Handles both plain query strings and fragment-based (hash) URLs." + [url f] + (let [transform (fn [parsed] + (update parsed :query + (fn [q] + (-> (query-string->map (or q "")) + f + map->query-string)))) + parsed (uri url) + fragment (:fragment parsed)] + (if (str/blank? fragment) + (str (transform parsed)) + (-> parsed + (assoc :fragment (str (transform (parse fragment)))) + str)))) + +(defn append-query-param + "Return a new URL string with the given query parameter added or replaced. + Handles both plain query strings and fragment-based (hash) URLs." + [url key value] + (update-query-params url #(assoc % key value))) + +(defn remove-query-param + "Return a new URL string with the given query parameter removed. + Handles both plain query strings and fragment-based (hash) URLs." + [url key] + (update-query-params url #(dissoc % key))) + #?(:clj (defmethod print-method lambdaisland.uri.URI [^URI this ^java.io.Writer writer] (.write writer "#") diff --git a/frontend/src/app/main/data/nitrate.cljs b/frontend/src/app/main/data/nitrate.cljs index dc6ce4615b..3470dcc81d 100644 --- a/frontend/src/app/main/data/nitrate.cljs +++ b/frontend/src/app/main/data/nitrate.cljs @@ -1,5 +1,6 @@ (ns app.main.data.nitrate (:require + [app.common.data :as d] [app.common.data.macros :as dm] [app.common.types.organization :as cto] [app.common.uri :as u] @@ -354,13 +355,16 @@ (defn retry-organization-sso "Retries the organization SSO login flow after a failed attempt, reusing the same check-nitrate-sso RPC used elsewhere to move the user through - the organization's identity provider. Falls back to navigating straight + the organization's identity provider. Passing `team-id` enables the + backend's non-member short-circuit. Falls back to navigating straight to `dest-url` when no fresh SSO redirect is needed or available." - [{:keys [organization-id dest-url]}] + [{:keys [team-id organization-id dest-url]}] (ptk/reify ::retry-organization-sso ptk/WatchEvent (watch [_ _ _] - (->> (rp/cmd! :check-nitrate-sso {:organization-id organization-id :url dest-url}) + (->> (rp/cmd! :check-nitrate-sso (d/without-nils {:team-id team-id + :organization-id organization-id + :url dest-url})) (rx/map (fn [{:keys [redirect-uri]}] (rt/nav-raw :uri (or redirect-uri dest-url)))) (rx/catch (fn [_] diff --git a/frontend/src/app/main/ui/routes.cljs b/frontend/src/app/main/ui/routes.cljs index 77e8ae2814..5b5c985043 100644 --- a/frontend/src/app/main/ui/routes.cljs +++ b/frontend/src/app/main/ui/routes.cljs @@ -126,7 +126,9 @@ (defn- handle-sso-error-and-navigate "Check if the current route has an SSO error marker. If so, assign an exception with type :sso-error and organization-id from query params, - then proceed with normal navigation." + and deliberately do NOT proceed with normal navigation: emitting + `rt/navigated` would clear the exception that was just assigned. + Otherwise, delegate to `check-sso-and-navigate`." [match send-event-info? url] (let [route-name (name (get-in match [:data :name])) sso-error? (some? (get-in match [:query-params :sso-error])) diff --git a/frontend/src/app/main/ui/static.cljs b/frontend/src/app/main/ui/static.cljs index a8ce419c50..9232c6598c 100644 --- a/frontend/src/app/main/ui/static.cljs +++ b/frontend/src/app/main/ui/static.cljs @@ -483,6 +483,7 @@ "Shown in place of the dashboard/workspace (same static skeleton and `request-dialog*` used by the no-permission dialogs) when the organization SSO exchange with the identity provider fails." + {::mf/private true} [{:keys [organization-id team-id profile is-workspace is-dashboard]}] (let [clean-url (mf/with-memo [] @@ -500,15 +501,18 @@ (mf/deps profile) (fn [] ;; Land on the user's own default team - (st/emit! (dcm/go-to-dashboard-recent :team-id (:default-team-id profile))))) + (st/emit! (rt/assign-exception nil) + (dcm/go-to-dashboard-recent :team-id (:default-team-id profile))))) on-retry (mf/use-fn (mf/deps organization-id team-id clean-url) (fn [] - (if team-id - ;; Retry with team-id to trigger SSO check for that specific team - (st/emit! (dnt/retry-organization-sso {:organization-id organization-id + (st/emit! (rt/assign-exception nil)) + (if (or team-id organization-id) + ;; Retry with team-id and/or organization-id to trigger SSO check + (st/emit! (dnt/retry-organization-sso {:team-id team-id + :organization-id organization-id :dest-url clean-url})) ;; Fallback: just navigate to clean URL (st/emit! (rt/nav-raw :uri clean-url)))))] diff --git a/frontend/src/app/util/dom.cljs b/frontend/src/app/util/dom.cljs index 6bdc663f42..6a0f514018 100644 --- a/frontend/src/app/util/dom.cljs +++ b/frontend/src/app/util/dom.cljs @@ -875,35 +875,17 @@ [url] (.replaceState (.-history globals/window) nil "" url)) -(defn- update-query-params - "Apply `f` to the query-params map of `url`, returning the updated URL string. - Handles both plain query strings and fragment-based (hash) URLs." - [url f] - (let [transform (fn [parsed] - (update parsed :query - (fn [q] - (-> (u/query-string->map (or q "")) - f - u/map->query-string)))) - parsed (u/uri url) - fragment (:fragment parsed)] - (if (str/blank? fragment) - (str (transform parsed)) - (-> parsed - (assoc :fragment (str (transform (u/parse fragment)))) - str)))) - (defn append-query-param "Return a new URL string with the given query parameter added or replaced. Handles both plain query strings and fragment-based (hash) URLs." [url key value] - (update-query-params url #(assoc % key value))) + (u/append-query-param url key value)) (defn remove-query-param "Return a new URL string with the given query parameter removed. Handles both plain query strings and fragment-based (hash) URLs." [url key] - (update-query-params url #(dissoc % key))) + (u/remove-query-param url key)) (defn reload-current-window ([]