♻️ Harden trusted alternate origins after review

Replace the OIDC-local build-public-uri helper with the canonical
cf/get-public-uri so every backend link shares one builder, and drop
the test that only covered the duplicate.

Assert the OIDC token-exchange redirect_uri under a trusted origin by
driving the callback with the HTTP client mocked instead of get-info;
the test fails if the exchange hardcodes the canonical base.

Document that removing an origin does not invalidate already-signed
OIDC states (up to four hours) and that MCP OAuth follows the stamped
origin the same way.

Bring MCP OAuth into scope: wrap its routes with wrap-trusted-origin,
stage the stamped origin in the Redis request record at authorize
time, and re-apply it in decision so issuer, resource, the approval
redirect and the code-exchange iss agree on one base. Unlisted
origins and requests without Origin keep the canonical base. Add a
tabulated trusted/untrusted end-to-end test over the real router.

AI-assisted-by: muse-spark-1.3-contributor

.serena/memories/backend/auth-permissions-product-domains.md

@@ -6,7 +6,7 @@
 - Public auth endpoints must explicitly set `::rpc/auth false`; RPC auth defaults to enabled. Session cookie creation/deletion is usually attached as an RPC response transform.
 - Basic Penpot registration is token staged: prepare/register creates or verifies temporary tokens, then profile creation/session setup is reused by other auth backends. The frontend `/auth/verify-token` flow is a hub for registration confirmation, email change, and invitation tokens.
 - OIDC-compatible providers share a generic flow: redirect to provider, validate callback/request token, fetch identity data, then login an existing profile or register a new one. Known providers may have hardcoded endpoints; generic OIDC can use discovery/configured endpoints.
-- Generic OIDC authorization stores a trusted alternate `Origin` (request `:app.http/trusted-origin`, see `mem:backend/subtleties`) as the `:origin` claim in the signed state; the callback re-applies it to the config public URI so the token-exchange `redirect_uri` and redirect targets stay on the alternate domain. Organization SSO mirrors this through `build-organization-sso-auth-redirect-uri :origin`.
+- Generic OIDC authorization stores a trusted alternate `Origin` (request `:app.http/trusted-origin`, see `mem:backend/subtleties`) as the `:origin` claim in the signed state; the callback re-applies it to the config public URI so the token-exchange `redirect_uri` and redirect targets stay on the alternate domain. Organization SSO mirrors this through `build-organization-sso-auth-redirect-uri :origin`. MCP OAuth (`app.http.mcp-oauth`) sits outside the root middleware chain with its own middleware, so it wraps its own routes with `wrap-trusted-origin` instead; `authorize` stages the stamped `:origin` in its Redis `:request` record and `decision` re-applies it for the approval redirect, `issuer`/`resource` metadata and the code-exchange `iss`, keeping the whole flow on one base.
 - LDAP login validates credentials against the external directory, fetches identity data, then logs in or registers a matching Penpot profile. LDAP registration is not a separate Penpot signup flow.
 - LDAP session identity MUST come from the directory-returned email (`info.email`): the profile matching the typed email can differ (aliases, UPNs, multi-valued `mail` attributes) and is used only for lockout checks, never to bind the session.
 - Account lockout (flag `:account-lockout`, `app.auth.login-lockout`) is Redis-backed and keyed per profile id. Password and LDAP flows check/increment on the profile derived from the typed email and clear on the profile that actually logs in.

backend/src/app/auth/oidc.clj

@@ -453,22 +453,9 @@
         [fitem & items]  (str/split path separator)]
     (into [(keyword (:type provider) (str/kebab fitem))] (map keyword) items)))
 
-(defn- build-public-uri
-  "Join a relative path onto the configured public URI, preserving any
-  subpath. Uses `u/ensure-path-slash` + `u/join` to match the pattern
-  used throughout the backend and frontend.
-  `path` must not start with `/` — a leading slash would be treated as
-  absolute by `u/join`, silently dropping the configured subpath."
-  [path]
-  {:pre [(not (str/starts-with? path "/"))]}
-  (-> (cf/get :public-uri)
-      (u/uri)
-      (u/ensure-path-slash)
-      (u/join path)))
-
 (defn- build-redirect-uri
   []
-  (str (build-public-uri "api/auth/oidc/callback")))
+  (cf/get-public-uri "api/auth/oidc/callback"))
 
 (defn build-auth-redirect-uri
   [provider token]

backend/src/app/http/mcp_oauth.clj

@@ -145,7 +145,12 @@
                                  :challenge (:code_challenge params)
                                  :state (:state params)
                                  :resource (resource)
-                                 :scope "mcp"}
+                                 :scope "mcp"
+                                 ;; The trusted alternate origin stamped on
+                                 ;; this request (nil on the canonical base).
+                                 ;; Decision re-applies it so every step of
+                                 ;; the flow agrees on one base.
+                                 :origin (:app.http/trusted-origin request)}
                                 600)]
       (redirect (append-query (str (cf/get-public-uri))
                               {:screen "mcp-authorize" :request handle})))))
@@ -215,9 +220,12 @@
                                                   (assoc :token-id (str (:id token))))
                                               120)]
                      {:code code}))]
-      (response {:redirect-uri (append-query (:redirect-uri value)
-                                             (merge result {:state (:state value)
-                                                            :iss (issuer)}))}))))
+      (binding [cf/config (cf/with-public-uri (:origin value) cf/config)]
+        ;; Re-applied from the staged request so :iss matches the
+        ;; authorize-time base (nil keeps the canonical one).
+        (response {:redirect-uri (append-query (:redirect-uri value)
+                                               (merge result {:state (:state value)
+                                                              :iss (issuer)}))})))))
 
 (defn- client-authentication
   "Extract one OAuth client-authentication method, rejecting mixed credentials.
@@ -336,7 +344,10 @@
 ;; middleware. Only the approval UI endpoints require a Penpot login session.
 (defmethod ig/init-key ::routes
   [_ cfg]
-  [""
+  ;; Trusted-origin runs outermost so a listed request Origin is honored
+  ;; without inheriting the RPC transforms above (form parsing, protocol
+  ;; JSON names and session handling stay OAuth-specific below).
+  ["" {:middleware [[mw/trusted-origin]]}
    ["/.well-known/oauth-authorization-server/api/mcp/oauth"
     (endpoint cfg (fn [_ _] (response (authorization-metadata))) :get true)]
    ["/api/mcp/oauth"

backend/test/backend_tests/auth_oidc_test.clj

@@ -234,11 +234,6 @@
           loc    (get-in result [::yres/headers "location"])]
       (t/is (.contains loc "http://localhost:3449/my-app?screen=auth-verify-token&")))))
 
-(t/deftest build-public-uri-no-subpath
-  (binding [cf/config {:public-uri "http://localhost:3449"}]
-    (t/is (= "http://localhost:3449/api/auth/oidc/callback"
-             (str (#'oidc/build-public-uri "api/auth/oidc/callback"))))))
-
 (t/deftest fetch-user-info-returns-decoded-body-on-success
   (let [cfg      {}
         provider {:user-uri "https://provider.example.com/userinfo"}
@@ -953,6 +948,44 @@
           (t/is (= "https://alt.example.com" @seen))
           (t/is (.contains loc "https://alt.example.com?screen=auth-verify-token&")))))))
 
+(t/deftest callback-token-exchange-uses-stored-trusted-origin
+  ;; Unlike callback-applies-stored-trusted-origin, this drives the real
+  ;; token exchange (mocking app.http.client/req instead of get-info) so
+  ;; the redirect_uri sent to the provider is asserted, not only the
+  ;; auth-request one and the post-exchange redirect.
+  (let [cfg      (dissoc base-cfg :app.email/blacklist :app.email/whitelist)
+        bodies   (atom [])
+        provider {:type "oidc" :id "oidc"
+                  :user-info-source "userinfo"
+                  :token-uri "https://idp.example.com/token"
+                  :user-uri "https://idp.example.com/userinfo"}
+        state    (make-state-token cfg {:origin "https://alt.example.com"})
+        request  (default-request cfg :state state)]
+    (binding [cf/config {:public-uri "http://localhost:3449"
+                         :trusted-origins #{"https://alt.example.com"}}]
+      (with-redefs [app.auth.oidc/resolve-provider        (constantly provider)
+                    app.auth.oidc/get-profile             (constantly test-profile)
+                    app.auth.oidc/update-profile-with-info (fn [_ profile _] profile)
+                    app.loggers.audit/submit              (constantly nil)]
+        (with-mocks [http-mock {:target 'app.http.client/req
+                                :return (fn [_ request & _]
+                                          (swap! bodies conj request)
+                                          (if (= (:uri request) (:token-uri provider))
+                                            {:status 200
+                                             :body "{\"access_token\":\"at\",\"token_type\":\"Bearer\"}"}
+                                            {:status 200
+                                             :body "{\"email\":\"u@e.com\",\"name\":\"U\"}"}))}]
+          (let [result     (#'oidc/callback-handler cfg request)
+                token-body (->> @bodies
+                                (filter #(= (:uri %) (:token-uri provider)))
+                                (first)
+                                (:body)
+                                (u/query-string->map))]
+            (t/is (= "https://alt.example.com/api/auth/oidc/callback"
+                     (:redirect_uri token-body)))
+            (t/is (.contains (redirect-location result)
+                             "https://alt.example.com?screen=auth-verify-token&"))))))))
+
 (t/deftest callback-error-uses-stored-trusted-origin
   (let [cfg     (dissoc base-cfg :app.email/blacklist :app.email/whitelist)
         state   (make-state-token cfg {:origin "https://alt.example.com"})

backend/test/backend_tests/mcp_oauth_http_test.clj

@@ -251,3 +251,75 @@
                                       :redirect_uri "https://attacker.example/callback"}))]
     (t/is (= 400 (::yres/status reply)))
     (t/is (nil? (get-in reply [::yres/headers "location"])))))
+
+(t/deftest trusted-origin-flow
+  (binding [cf/config (assoc cf/config
+                             :public-uri "http://localhost:3449"
+                             :trusted-origins #{"https://alt.example.com"})]
+    (let [alt "https://alt.example.com"]
+      ;; Discovery honors a trusted Origin and ignores anything else.
+      (doseq [{:keys [origin expected]} [{:origin alt :expected alt}
+                                         {:origin "https://evil.example.com"
+                                          :expected "http://localhost:3449"}
+                                         {:origin nil
+                                          :expected "http://localhost:3449"}]]
+        (let [headers  (cond-> {} (some? origin) (assoc "origin" origin))
+              metadata (body (request "/.well-known/oauth-authorization-server/api/mcp/oauth"
+                                      :headers headers))
+              resource (body (request "/api/mcp/oauth/resource" :headers headers))]
+          (t/is (= (str expected "/api/mcp/oauth") (:issuer metadata)) origin)
+          (t/is (= (str expected "/mcp/stream") (:resource resource)) origin)))
+      ;; An unlisted Origin keeps the canonical approval redirect.
+      (let [client (clients/register! th/*system* {:redirect_uris ["https://example.com/callback"]
+                                                   :token_endpoint_auth_method "none"})
+            params {:client_id (:client_id client)
+                    :redirect_uri (first (:redirect_uris client))
+                    :response_type "code" :scope "mcp"
+                    :resource (oauth/resource)
+                    :state "client-state"
+                    :code_challenge (state/digest (apply str (repeat 43 "a")))
+                    :code_challenge_method "S256"}
+            reply  (request "/api/mcp/oauth/authorize"
+                            :headers {"origin" "https://evil.example.com"}
+                            :query (form params))]
+        (t/is (= 303 (::yres/status reply)))
+        (t/is (str/starts-with? (get-in reply [::yres/headers "location"])
+                                "http://localhost:3449/?screen=mcp-authorize")))
+      ;; Full flow on the alternate base: authorize, approve and exchange.
+      (let [profile  (th/create-profile* 1 {:is-active true})
+            cookies  (login-cookie profile)
+            client   (clients/register! th/*system* {:redirect_uris ["https://example.com/callback"]
+                                                     :token_endpoint_auth_method "none"})
+            reply    (request "/api/mcp/oauth/authorize"
+                              :headers {"origin" alt}
+                              :query (form {:client_id (:client_id client)
+                                            :redirect_uri (first (:redirect_uris client))
+                                            :response_type "code" :scope "mcp"
+                                            :resource (str alt "/mcp/stream")
+                                            :state "client-state"
+                                            :code_challenge (state/digest (apply str (repeat 43 "a")))
+                                            :code_challenge_method "S256"}))
+            location (get-in reply [::yres/headers "location"])
+            handle   (get (uri/query-string->map (.getQuery (java.net.URI. location)))
+                          :request)
+            details  (body (request "/api/mcp/oauth/request"
+                                    :query (form {:request handle}) :cookies cookies))
+            submit   #(request "/api/mcp/oauth/decision" :method :post :cookies cookies
+                               :headers {"content-type" "application/x-www-form-urlencoded"
+                                         "origin" alt}
+                               :body-bytes (form %))
+            approved (submit {:request handle :decision "approve" :csrf (:csrf details)})
+            params   (uri/query-string->map
+                      (.getQuery (java.net.URI. (:redirect-uri (body approved)))))
+            exchange {:grant_type "authorization_code" :code (:code params)
+                      :client_id (:client_id client) :redirect_uri "https://example.com/callback"
+                      :resource (str alt "/mcp/stream")
+                      :code_verifier (apply str (repeat 43 "a"))}
+            token    (request "/api/mcp/oauth/token" :method :post
+                              :headers {"content-type" "application/x-www-form-urlencoded"}
+                              :body-bytes (form exchange))]
+        (t/is (= 303 (::yres/status reply)))
+        (t/is (str/starts-with? location (str alt "/?screen=mcp-authorize")))
+        (t/is (= 200 (::yres/status approved)))
+        (t/is (= (str alt "/api/mcp/oauth") (:iss params)))
+        (t/is (= 200 (::yres/status token)))))))

docs/technical-guide/configuration.md

@@ -431,7 +431,9 @@ instead. When a request carries an `Origin` header that matches one of the liste
 origins, the backend uses that origin as the base for the links it generates while
 serving the request (transactional emails, asset URLs, audit and webhook link fields,
 and redirects). Origins that are not listed, and requests without an `Origin` header,
-keep using `PENPOT_PUBLIC_URI`.
+keep using `PENPOT_PUBLIC_URI`. Removing an origin from the list does not
+invalidate OIDC states already signed with it; they keep using it until they
+expire (up to four hours).
 
 ```bash
 # Backend
@@ -449,6 +451,9 @@ PENPOT_TRUSTED_ORIGINS: https://alt.penpot.mycompany.com, https://staging.penpot
 - Notifications sent later by background workers keep the canonical `PENPOT_PUBLIC_URI`,
   because they run outside the request that triggered them. The same applies to
   server-to-server calls (for example the management API) that carry no `Origin`.
+- MCP OAuth endpoints (discovery metadata, the approval redirect and the
+  code-exchange `iss`) follow the stamped origin the same way; without a trusted
+  `Origin` they keep the canonical `PENPOT_PUBLIC_URI`.
 
 ## Email configuration