diff --git a/.serena/memories/mcp/core.md b/.serena/memories/mcp/core.md index 04bfef910e..551ef961c6 100644 --- a/.serena/memories/mcp/core.md +++ b/.serena/memories/mcp/core.md @@ -13,19 +13,26 @@ enabling advanced AI-driven features in Penpot. - Build Tool: TypeScript Compiler (tsc) + esbuild - Package Manager: pnpm -## General Principles +## Principles & Code Style IMPORTANT: Use an idiomatic, object-oriented style. -In particular, this implies that, for any non-trivial interfaces, you use interfaces that expect explicitly typed abstractions -rather than mere functions (i.e. use the strategy pattern, for example). +* You keep each concern in exactly one home. + A mechanism whose parts are only correct in combination is implemented as a single component/class, and its parts are private to it; + expose a minimal public surface. Java-like encapsulation: If helper functions and constants are only used by one abstraction, + make them internal to the respective class. +* For any non-trivial interfaces, you use functions/methods that expect explicitly typed abstractions + rather than mere functions (i.e. use the strategy pattern, for example). +* Prefer declaring and instantiating concrete classes over defining only interfaces and using object literals. Comments: -When describing parameters, methods/functions and classes, you use a precise style, where the initial (elliptical) phrase -clearly defines *what* it is. Any details then follow in subsequent sentences. +* When describing parameters, methods/functions and classes, you use a precise style, where the initial (elliptical) phrase + clearly defines *what* it is. Any details then follow in subsequent sentences. +* When describing what blocks of code do, you also use an elliptical style and start with a lower-case letter unless + the comment is a lengthy explanation with at least two sentences (in which case you start with a capital letter, as is + required for sentences). -When describing what blocks of code do, you also use an elliptical style and start with a lower-case letter unless -the comment is a lengthy explanation with at least two sentences (in which case you start with a capital letter, as is -required for sentences). +Syntax: +* Never use if blocks (or other blocks) without braces, even for single-line cases. ## Project Structure (Excerpt) @@ -102,6 +109,10 @@ From the `mcp/` directory, run stdout directly. `NODE_TEST_CONTEXT` is the env var `node --test` sets. * Cross-cutting testing principles and anti-patterns: `mem:testing`. +## OAuth for remote MCP clients + +- Authorization/discovery URLs, self-contained client IDs, consent and PKCE/code binding, backend Redis state, deployment wiring, and compatibility limits: `mem:mcp/oauth`. + ## Devenv plugin/server wiring In the normal Penpot devenv MCP path, the browser plugin does not discover or route through Postgres. The frontend provides the plugin extension API with `mcp.getServerUrl()`, currently derived from `frontend/src/app/config.cljs` as `penpotMcpServerURI` if set, otherwise `/mcp/ws`. The MCP plugin opens a direct WebSocket to that URL and appends the current MCP access token as a query parameter. diff --git a/.serena/memories/mcp/oauth.md b/.serena/memories/mcp/oauth.md new file mode 100644 index 0000000000..097bdc1186 --- /dev/null +++ b/.serena/memories/mcp/oauth.md @@ -0,0 +1,47 @@ +# MCP OAuth + +## Contract and scope + +- MCP-specific authorization; existing Penpot login identifies the user on the approval page. No application vetting or persistent consent policy. Access requires user approval; registration alone grants nothing. +- Return the existing fixed MCP user token. On approval, enable `:mcp-enabled` and create the token only if no unexpired one exists; preserve an existing unexpired token. Settings can give MCP tokens an expiry, and an expired one is replaced. No refresh token, `expires_in`, or new token lifetime. +- No new client database table or schema migration. Client IDs must remain usable across separate authorization flows without server-side registration storage. Do not replace them with expiring Redis registrations. +- MCP transport continues matching user tokens to connected Penpot plugins; no new per-request token introspection or independent semantic validation. The logged-in Penpot instance remains the content-access boundary. Do not silently add resource-server validation or change token semantics. +- README scope: user/developer usage instructions; document `PENPOT_MCP_AUTH_URI` in its environment-variable table. Keep OAuth architecture and protocol details here and non-obvious invariants in source comments. + +## Deployment and discovery + +- MCP process: `PENPOT_MCP_AUTH_URI=https://penpot.example/api/mcp/oauth` with `--multi-user`. Unset/empty disables OAuth; configured OAuth requires multi-user. Absolute HTTPS, or HTTP on loopback for development; no userinfo, query, or fragment. Invalid configuration fails startup; startup logs OAuth status. +- Devenv: `mcp/scripts/start-mcp-devenv-oauth` starts multi-user mode with `PENPOT_MCP_AUTH_URI` derived from `PENPOT_PUBLIC_URI`. Manual opt-in only: agentic tooling needs the single-user `start-mcp-devenv`. +- Public MCP transport: `/mcp/stream`, proxied to Node `/mcp`. Missing credentials with OAuth enabled: 401 plus `WWW-Authenticate` referencing `/resource` and scope `mcp`. +- Bearer header takes precedence over legacy `?userToken=`. Malformed headers do not fall back to the query token. Existing query-token clients remain supported. +- Protected-resource metadata: `/api/mcp/oauth/resource`; issuer: `/api/mcp/oauth`; authorization-server metadata: `/.well-known/oauth-authorization-server/api/mcp/oauth`. All reach the backend. Bundled devenv/production nginx configs forward the well-known path; custom proxies must do likewise. Keep Penpot public URI and external URLs consistent. +- Node implementation: `mcp/packages/server/src/McpAuthentication.ts`, integrated by `PenpotMcpServer.ts`; OAuth-enabled tools advertise `_meta.securitySchemes`. + +## Client identification + +- `backend/src/app/mcp/oauth_clients.clj`: dynamic registration at `/api/mcp/oauth/register`, HTTPS client metadata documents (CIMD), and registrations obtained in advance via `(app.mcp.oauth-clients/register! {} metadata)`. +- DCR returns `penpot-mcp-v1.` plus unpadded base64url JSON: normalized callback URLs, optional name, authentication method, random nonce, optional secret hash. Fixed grant/response/scope values are reconstructed. Nonce distinguishes registrations with otherwise identical metadata. +- IDs are public, unsigned, unencrypted, non-expiring metadata; validate on every resolution. Client secrets are generated randomly and returned separately; only the hash enters the ID. No signing/encryption key or client record in PostgreSQL/Redis. Old experimental database-backed UUID IDs require registration again. +- An edited payload changes the ID. Authorization-code exchange compares the exact ID string against the approved request before resolving metadata. Do not replace this with comparison of decoded fields; code ownership and PKCE binding must survive metadata edits. +- Supported authentication: `none`, `client_secret_basic`, `client_secret_post`; no `private_key_jwt`. CIMD selects public-client `none` from the document's supported methods; other listed methods do not imply support here. Scope `mcp`; grant `authorization_code`; response `code`. CIMD may advertise additional grants (e.g. Codex lists `refresh_token`); require `authorization_code` support and normalize to the grant Penpot implements, rather than reject broader client capabilities. +- Entire ID capped at 2,048 ASCII characters. DCR returns `invalid_client_metadata` if encoded metadata exceeds this; shorter/fewer callback URLs reduce size. OAuth defines no fixed ID-length maximum (RFC 6749 §2.2); stateless IDs are explicitly described by RFC 7591 §A.5.2. Installed MCP client SDK 2.3.1 accepts a 2,048-character ID; this does not establish Codex/Claude compatibility. +- Redirect matching is exact except loopback HTTP port changes for native clients (`127.0.0.1`, `[::1]`, exact `localhost`). Host, path and query must still match; reject fragments/userinfo and lookalike localhost domains. Allow HTTPS, loopback HTTP, and reverse-domain native schemes. +- CIMD: fetch with existing SSRF-checked `app.http.client/req`, no redirect following, require matching document `client_id`, bounded 64-KiB body and request/body-read timeouts. Cache max 256 entries/one minute, further limited by HTTP max-age; no-store/no-cache/no max-age means no reuse. + +## Approval and exchange + +- `backend/src/app/http/mcp_oauth.clj`: separate router branch from ordinary RPC middleware; preserve OAuth snake_case JSON and form encoding. Session auth applies to approval request/decision only. Public metadata/register/token endpoints allow noncredentialed CORS; MCP transport CORS was not added. +- Errors: every failure answers with an OAuth `{"error": ...}` body, never the shared Penpot error shape. Failures without `:oauth-error` are also passed to `app.http.errors/handle` for logging only; its response is discarded. `wrap-errors` also wraps the session middleware of `/request` and `/decision`. Safe to log because these routes never set request `:params`. +- `/authorize`: validate client/callback, `code`, PKCE S256, scope/resource/state; store pending request for 600 seconds; redirect to `?screen=mcp-authorize&request=`. +- `frontend/src/app/main/ui/mcp_authorize.cljs`: existing login with return URL; show application and account details; submit explicit Allow/Cancel. No fixed MCP token in frontend state or browser redirect. Approval refreshes profile and broadcasts MCP enablement; does not connect a plugin automatically. +- `/request`: first authenticated read binds request to profile plus hashed login-session ID and CSRF value, using compare-and-replace. Re-read after the attempt: concurrent GETs may win/lose binding; only the matching session may proceed. +- `/decision`: check exact Origin, session binding and CSRF, then atomically consume the request. Denial creates no token/enables nothing. Approval calls transactional `access-token/ensure-mcp-access!`, then stores code for 120 seconds. Code state holds token row ID, never the fixed token. Callback echoes OAuth `state` and RFC 9207 `iss`. +- `/token`: validate grant, exact client ID/callback/resource and PKCE; resolve current client metadata/authentication method and verify any secret; atomically consume validated code; fetch the unexpired token by original row ID/profile/type. Regeneration deletes the old token row, so a pending code cannot silently yield the replacement token. Return fixed `access_token`, `token_type: Bearer`, scope `mcp`. +- `backend/src/app/mcp/oauth_state.clj` and Lua: tenant/kind/hash-prefixed keys; random handles; SET NX EX must succeed before returning handle. Reads retain raw JSON for atomic comparison. Consume/delete and bind/replace compare the validated raw record in Lua; replacement preserves TTL. Redis failures fail closed. Backend Redis is required independently of Node's optional Redis plugin bridge. +- `backend/src/app/rpc/commands/access_token.clj`: lock profile row in both MCP generation and OAuth provisioning, including when no token exists. Serializes simultaneous approvals and settings regeneration. Provisioning must run within a transaction. + +## Verification and limits + +- Tests: `mcp_oauth_clients_test.clj`, `mcp_oauth_http_test.clj`, `mcp_oauth_state_test.clj`, OAuth provisioning in `rpc_access_tokens_test.clj`; MCP authentication/HTTP tests; frontend approval tests. Cover no-storage registration, size boundaries, edited-ID rejection, code replay/concurrency, session/CSRF binding, and fixed-token reuse. +- Live OpenAI/Codex/Claude compatibility and marketplace acceptance remain unverified. In particular, verify token persistence without `expires_in` and acceptance of the existing matching-only resource boundary; do not claim protocol conformance from SDK tests alone. +- Protocol references: https://www.rfc-editor.org/rfc/rfc6749.html#section-2.2 ; https://www.rfc-editor.org/rfc/rfc7591.html#appendix-A.5.2 ; https://www.rfc-editor.org/rfc/rfc7636.html ; https://www.rfc-editor.org/rfc/rfc9207.html . diff --git a/backend/src/app/http.clj b/backend/src/app/http.clj index aa6950fdeb..83af60c3d2 100644 --- a/backend/src/app/http.clj +++ b/backend/src/app/http.clj @@ -19,6 +19,7 @@ [app.http.errors :as errors] [app.http.link-preview :as-alias link-preview] [app.http.management :as mgmt] + [app.http.mcp-oauth :as-alias mcp-oauth] [app.http.middleware :as mw] [app.http.security :as sec] [app.http.session :as session] @@ -236,6 +237,7 @@ [::ws/routes schema:routes] [::rpc/routes schema:routes] [::oidc/routes schema:routes] + [::mcp-oauth/routes schema:routes] [::assets/routes schema:routes] [::link-preview/routes schema:routes] [::debug/routes schema:routes] @@ -278,4 +280,7 @@ (::ws/routes cfg) (::oidc/routes cfg) - (::rpc/routes cfg)]])) + (::rpc/routes cfg)] + ;; OAuth owns form parsing, protocol JSON names and cross-origin handling; + ;; it must not inherit the RPC transformations and session middleware above. + (::mcp-oauth/routes cfg)])) diff --git a/backend/src/app/http/mcp_oauth.clj b/backend/src/app/http/mcp_oauth.clj new file mode 100644 index 0000000000..5184cc7e65 --- /dev/null +++ b/backend/src/app/http/mcp_oauth.clj @@ -0,0 +1,353 @@ +;; 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.http.mcp-oauth + "MCP-specific OAuth authorization server. + + The authorize endpoint stages a request; the existing Penpot session identifies + the user on the approval page. Approval creates a one-use code, and token + exchange returns the existing fixed MCP user token after verifying PKCE. + Client registration, login sessions, and authorization codes are distinct." + (:require + [app.common.uuid :as uuid] + [app.config :as cf] + [app.db :as db] + [app.http.errors :as errors] + [app.http.middleware :as mw] + [app.http.session :as session] + [app.mcp.oauth-clients :as clients] + [app.mcp.oauth-state :as state] + [app.rpc.commands.access-token :as access-token] + [app.util.json :as json] + [clojure.string :as str] + [integrant.core :as ig] + [yetti.request :as yreq] + [yetti.response :as yres]) + (:import + java.io.InputStream + java.net.URI + java.net.URLDecoder + java.net.URLEncoder + java.nio.charset.StandardCharsets + java.util.Base64)) + +(defn issuer [] (str (cf/get-public-uri "api/mcp/oauth"))) +(defn resource [] (str (cf/get-public-uri "mcp/stream"))) + +(defn authorization-metadata + "OAuth capabilities and endpoints advertised to MCP clients." + [] + {:issuer (issuer) + :authorization_endpoint (str (issuer) "/authorize") + :token_endpoint (str (issuer) "/token") + :registration_endpoint (str (issuer) "/register") + :response_types_supported ["code"] + :grant_types_supported ["authorization_code"] + :code_challenge_methods_supported ["S256"] + :token_endpoint_auth_methods_supported ["none" "client_secret_basic" "client_secret_post"] + :scopes_supported ["mcp"] + :authorization_response_iss_parameter_supported true + :client_id_metadata_document_supported true}) + +(defn resource-metadata + "Protected-resource metadata referenced by the MCP server's 401 challenge." + [] + {:resource (resource) + :authorization_servers [(issuer)] + :scopes_supported ["mcp"] + :bearer_methods_supported ["header"] + :resource_name "Penpot MCP"}) + +(defn- response + ([body] (response 200 body)) + ([status body] + {::yres/status status + ::yres/headers {"content-type" "application/json" + "cache-control" "no-store" + "pragma" "no-cache" + "referrer-policy" "no-referrer"} + ::yres/body (json/encode-str body)})) + +(defn- redirect + [url] + {::yres/status 303 + ::yres/headers {"location" url "cache-control" "no-store" + "referrer-policy" "no-referrer"} + ::yres/body ""}) + +(defn- encode [^String value] (URLEncoder/encode value StandardCharsets/UTF_8)) +(defn- decode + [^String value] + (try (URLDecoder/decode value StandardCharsets/UTF_8) + (catch IllegalArgumentException _ (clients/invalid! "invalid_request")))) + +(defn- parse-form + "Bounded OAuth form/query parser preserving protocol parameter names. + Rejects duplicate keys so validation never depends on which value wins." + [query] + (when (> (count query) 16384) (clients/invalid! "invalid_request")) + (reduce (fn [result part] + (let [[key value] (str/split part #"=" 2) + key (keyword (decode key))] + (when (contains? result key) (clients/invalid! "invalid_request")) + (assoc result key (decode (or value ""))))) + {} + (remove str/blank? (str/split (or query "") #"&")))) + +(defn- read-body + [request content-type] + (when-not (str/starts-with? (or (yreq/get-header request "content-type") "") content-type) + (clients/invalid! "invalid_request")) + (with-open [^InputStream body (yreq/body request)] + (when-not body (clients/invalid! "invalid_request")) + (let [bytes (.readNBytes body 16385)] + (when (> (alength bytes) 16384) (clients/invalid! "invalid_request")) + (String. bytes StandardCharsets/UTF_8)))) + +(defn- append-query + [url params] + (str url (if (str/includes? url "?") "&" "?") + (str/join "&" (for [[key value] params :when (some? value)] + (str (encode (name key)) "=" (encode value)))))) + +(defn- limit! + [cfg request kind limit] + (when-not (state/allow-request! cfg kind (str (yreq/remote-addr request)) limit) + (clients/invalid! "temporarily_unavailable" 429))) + +(defn- authorize + "Validate and stage an authorization request before user login or consent. + The browser receives only an opaque handle to the ten-minute Redis record." + [cfg request] + (limit! cfg request :authorize-rate 60) + (let [params (parse-form (yreq/query request)) + client (clients/resolve-client cfg (:client_id params)) + callback (:redirect_uri params)] + ;; Never redirect until the client and its callback have been checked. + (when-not (clients/redirect-matches? client callback) + (clients/invalid! "invalid_request")) + (when-not (and (= "code" (:response_type params)) + (= "S256" (:code_challenge_method params)) + (string? (:code_challenge params)) + (re-matches #"[A-Za-z0-9_-]{43}" (:code_challenge params)) + (= "mcp" (get params :scope "mcp")) + (= (resource) (get params :resource (resource))) + (<= (count (:state params)) 2048)) + (clients/invalid! "invalid_request")) + (let [handle (state/create! cfg :request + {:client-id (:client_id client) + :client-name (:client_name client) + :logo-uri (:logo_uri client) + :redirect-uri callback + :challenge (:code_challenge params) + :state (:state params) + :resource (resource) + :scope "mcp"} + 600)] + (redirect (append-query (str (cf/get-public-uri)) + {:screen "mcp-authorize" :request handle}))))) + +(defn- session-binding + "Profile and login-session identity used to bind one approval request." + [request] + (let [session (session/get-session request)] + (when-not (:profile-id session) (clients/invalid! "login_required" 401)) + {:profile-id (str (:profile-id session)) + :session-id (state/digest (str (:id session)))})) + +(defn- approval-request + "Bind a pending request to its first authenticated session and return UI data. + Compare-and-replace prevents another session from taking over that request." + [cfg request] + (let [binding (session-binding request) + handle (:request (parse-form (yreq/query request))) + record (state/read! cfg :request handle)] + (when-not record (clients/invalid! "invalid_request")) + (let [value (:value record)] + (if (:session-id value) + (when-not (= binding (select-keys value [:profile-id :session-id])) + (clients/invalid! "access_denied" 403)) + (state/replace! cfg :request handle record + (merge value binding {:csrf (state/random-secret)})))) + ;; Another GET may have won the binding race. Read the winning record so + ;; same-session retries share its CSRF value; other sessions cannot proceed. + (let [value (:value (state/read! cfg :request handle))] + (when-not (= binding (select-keys value [:profile-id :session-id])) + (clients/invalid! "invalid_request")) + (response (select-keys value [:client-id :client-name :logo-uri :csrf :scope]))))) + +(defn- same-origin? + [request] + (let [public (URI. (str (cf/get-public-uri))) + origin (str (.getScheme public) "://" (.getRawAuthority public))] + (= origin (yreq/get-header request "origin")))) + +(defn- decision + "Accept one session-bound approval/denial and return the client callback. + Origin and CSRF checks protect the cookie-authenticated decision endpoint." + [cfg request] + (when-not (same-origin? request) (clients/invalid! "access_denied" 403)) + (let [binding (session-binding request) + params (parse-form (read-body request "application/x-www-form-urlencoded")) + handle (:request params) + record (state/read! cfg :request handle) + value (:value record)] + (when-not (and record + (= binding (select-keys value [:profile-id :session-id])) + (state/secret= (:csrf value) (:csrf params)) + (contains? #{"approve" "deny"} (:decision params))) + (clients/invalid! "invalid_request")) + ;; Consume before provisioning: concurrent clicks must not issue two codes. + ;; A later failure requires a fresh authorization attempt. + (when-not (state/consume! cfg :request handle record) + (clients/invalid! "invalid_request")) + (let [result (if (= "deny" (:decision params)) + {:error "access_denied"} + (let [token (db/tx-run! cfg access-token/ensure-mcp-access! + (uuid/parse (:profile-id value))) + ;; Retain the token row ID, never the fixed token itself. + code (state/create! cfg :code + (-> value + (dissoc :csrf :session-id :client-name) + (assoc :token-id (str (:id token)))) + 120)] + {:code code}))] + (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. + Basic credentials are form-decoded after splitting the decoded header." + [request params] + (let [header (yreq/get-header request "authorization")] + (if header + (do + (when (or (:client_secret params) (not (str/starts-with? header "Basic "))) + (clients/invalid! "invalid_client" 401)) + (let [decoded (try (String. (.decode (Base64/getDecoder) (subs header 6)) + StandardCharsets/UTF_8) + (catch IllegalArgumentException _ + (clients/invalid! "invalid_client" 401))) + [id secret] (str/split decoded #":" 2) + id (decode id)] + (when (or (nil? secret) + (and (:client_id params) (not= id (:client_id params)))) + (clients/invalid! "invalid_client" 401)) + {:id id :secret (decode secret) :method "client_secret_basic"})) + {:id (:client_id params) :secret (:client_secret params) + :method (if (:client_secret params) "client_secret_post" "none")}))) + +(defn- token + "Exchange a code for the fixed MCP token after client and PKCE verification." + [cfg request] + (limit! cfg request :token-rate 120) + (let [params (parse-form (read-body request "application/x-www-form-urlencoded")) + auth (client-authentication request params) + record (state/read! cfg :code (:code params)) + value (:value record)] + (when-not (= "authorization_code" (:grant_type params)) + (clients/invalid! "unsupported_grant_type")) + ;; Compare the exact client ID, not decoded metadata: a self-contained ID + ;; edited after approval identifies a different client for this exchange. + (when-not (and record + (= (:id auth) (:client-id value)) + (= (:redirect_uri params) (:redirect-uri value)) + (= (:resource value) (get params :resource (:resource value))) + (string? (:code_verifier params)) + (re-matches #"[A-Za-z0-9._~-]{43,128}" (:code_verifier params)) + (state/secret= (:challenge value) (state/digest (:code_verifier params)))) + (clients/invalid! "invalid_grant")) + ;; Resolve again because an HTTPS metadata document may have changed + ;; since approval; its current callback and authentication method apply. + (let [client (clients/resolve-client cfg (:id auth))] + (when-not (and (= (:method auth) (:token_endpoint_auth_method client)) + (clients/redirect-matches? client (:redirect-uri value)) + (or (= "none" (:method auth)) + (and (string? (:secret auth)) + (state/secret= (:secret-hash client) + (state/digest (:secret auth)))))) + (clients/invalid! "invalid_client" 401))) + ;; Validate before consuming so an invalid verifier/secret cannot burn the + ;; code. Atomic consumption allows only one valid exchange to succeed. + (when-not (state/consume! cfg :code (:code params) record) + (clients/invalid! "invalid_grant")) + ;; Regeneration deletes the old token row. Never substitute the profile's + ;; latest token for the one this approval authorized, nor return it expired. + (let [row (db/exec-one! (::db/pool cfg) + [(str "SELECT token FROM access_token" + " WHERE id = ? AND profile_id = ? AND type = 'mcp'" + " AND (expires_at IS NULL OR expires_at > now())") + (uuid/parse (:token-id value)) + (uuid/parse (:profile-id value))])] + (when-not row (clients/invalid! "invalid_grant")) + (response {:access_token (:token row) :token_type "Bearer" :scope "mcp"})))) + +(defn- register + "Issue a self-contained client registration; does not grant user access." + [cfg request] + (limit! cfg request :registration-rate 10) + (let [body (read-body request "application/json") + metadata (try (json/decode body) + (catch Exception _ (clients/invalid! "invalid_request")))] + (response 201 (clients/register! cfg metadata)))) + +(defn- wrap-errors + "Answer every failure with an OAuth error body. Unexpected failures also go + through the shared HTTP error handler, only for its logging; these routes + never set request params, so codes and credentials stay out of the log." + [handler] + (fn [request] + (try (handler request) + (catch Exception cause + (let [{:keys [oauth-error status]} (ex-data cause) + status (or status (if (and oauth-error (not= oauth-error "temporarily_unavailable")) 400 503))] + (when-not oauth-error + (errors/handle cause request)) + (cond-> (response status {:error (or oauth-error "temporarily_unavailable")}) + (and (= status 401) (= oauth-error "invalid_client")) + (assoc-in [::yres/headers "www-authenticate"] "Basic realm=\"Penpot MCP\""))))))) + +(defn- wrap-public + "CORS for discovery, registration and exchange without browser credentials. + Cookie-authenticated approval endpoints must not use this wrapper." + [handler] + (fn [request] + (let [response (if (= :options (yreq/method request)) + {::yres/status 204} + (handler request))] + (update response ::yres/headers merge + {"access-control-allow-origin" "*" + "access-control-allow-methods" "GET, POST, OPTIONS" + "access-control-allow-headers" "Content-Type, Authorization" + "access-control-expose-headers" "WWW-Authenticate"})))) + +(defn- endpoint + [cfg handler method public?] + {:handler (partial handler cfg) + :middleware (into (if public? [{:compile (constantly wrap-public)}] []) + [{:compile (constantly wrap-errors)} [mw/restrict-methods]]) + :allowed-methods (cond-> #{method} public? (conj :options))}) + +;; OAuth endpoints parse forms and emit snake_case JSON independently of RPC +;; middleware. Only the approval UI endpoints require a Penpot login session. +(defmethod ig/init-key ::routes + [_ cfg] + ["" + ["/.well-known/oauth-authorization-server/api/mcp/oauth" + (endpoint cfg (fn [_ _] (response (authorization-metadata))) :get true)] + ["/api/mcp/oauth" + ["/resource" (endpoint cfg (fn [_ _] (response (resource-metadata))) :get true)] + ["/authorize" (endpoint cfg authorize :get false)] + ["/register" (endpoint cfg register :post true)] + ["/token" (endpoint cfg token :post true)] + ;; Error handling wraps the session middleware too, so its failures also + ;; get OAuth error bodies and logging. + ["" {:middleware [{:compile (constantly wrap-errors)} + [mw/auth {:cookie (partial session/decode-token cfg)}] + [session/authz cfg]]} + ["/request" (endpoint cfg approval-request :get false)] + ["/decision" (endpoint cfg decision :post false)]]]]) diff --git a/backend/src/app/main.clj b/backend/src/app/main.clj index f8d856fb76..9267c889f4 100644 --- a/backend/src/app/main.clj +++ b/backend/src/app/main.clj @@ -22,6 +22,7 @@ [app.http.debug :as-alias http.debug] [app.http.link-preview :as-alias http.link-preview] [app.http.management :as mgmt] + [app.http.mcp-oauth :as-alias http.mcp-oauth] [app.http.session :as session] [app.http.websocket :as http.ws] [app.jobs :as-alias jobs] @@ -540,9 +541,17 @@ ::http.debug/routes (ig/ref ::http.debug/routes) ::http.assets/routes (ig/ref ::http.assets/routes) ::http.link-preview/routes (ig/ref ::http.link-preview/routes) + ::http.mcp-oauth/routes (ig/ref ::http.mcp-oauth/routes) ::http.ws/routes (ig/ref ::http.ws/routes) ::http.awsns/routes (ig/ref ::http.awsns/routes)} + ::http.mcp-oauth/routes + {::db/pool (ig/ref ::db/pool) + ::rds/pool (ig/ref ::rds/pool) + ::setup/props (ig/ref ::setup/props) + ::session/manager (ig/ref ::session/manager) + ::http.client/client (ig/ref ::http.client/client)} + ::http.link-preview/routes {::db/pool (ig/ref ::db/pool)} diff --git a/backend/src/app/mcp/oauth_clients.clj b/backend/src/app/mcp/oauth_clients.clj new file mode 100644 index 0000000000..2e36fa6751 --- /dev/null +++ b/backend/src/app/mcp/oauth_clients.clj @@ -0,0 +1,220 @@ +;; 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.mcp.oauth-clients + "Client metadata from self-contained registrations or HTTPS documents. + Registration describes an application; it does not authorize a Penpot user. + Both sources resolve to the same metadata shape without a client database." + (:require + [app.http.client :as http] + [app.mcp.oauth-state :as state] + [app.util.cache :as cache] + [app.util.json :as json] + [clojure.string :as str]) + (:import + java.io.InputStream + java.net.URI + java.util.Base64)) + +(defn invalid! + ([error] (invalid! error 400)) + ([error status] + (throw (ex-info "MCP OAuth request rejected" {:oauth-error error :status status})))) + +(defn uri + [value] + (when (and (string? value) (<= 1 (count value) 2048)) + (try (URI. value) (catch Exception _ nil)))) + +(defn- loopback? + [^URI uri] + (and (= "http" (.getScheme uri)) + (contains? #{"127.0.0.1" "[::1]" "localhost"} (.getHost uri)))) + +(defn redirect-uri? + [value] + (when-let [uri (uri value)] + (and (nil? (.getFragment uri)) + (nil? (.getUserInfo uri)) + (or (and (= "https" (.getScheme uri)) (some? (.getHost uri))) + (loopback? uri) + ;; Native applications use a reverse-domain scheme. + (and (some? (.getScheme uri)) + (str/includes? (.getScheme uri) ".") + (not (.isOpaque uri))))))) + +(defn redirect-matches? + "Exact callback match, except for the port of loopback HTTP URLs. + Native applications may allocate a different local port for each login." + [registered requested] + (and (redirect-uri? requested) + (some (fn [value] + (or (= value requested) + (let [a (uri value) + b (uri requested)] + (and (loopback? a) (loopback? b) + (= (.getHost a) (.getHost b)) + (= (.getRawPath a) (.getRawPath b)) + (= (.getRawQuery a) (.getRawQuery b)))))) + (:redirect_uris registered)))) + +(defn- logo-uri? + [value] + (when-let [uri (uri value)] + (and (= "https" (.getScheme uri)) + (some? (.getHost uri)) + (nil? (.getUserInfo uri)) + (nil? (.getFragment uri))))) + +(defn validate-metadata + "Validate untrusted metadata and retain the fields this server supports. + CIMD selects public-client authentication from the advertised methods; + dynamic registration also supports Basic and form-post client secrets." + [metadata cimd?] + (let [redirects (:redirect_uris metadata) + grants (get metadata :grant_types ["authorization_code"]) + methods (if cimd? + (or (:token_endpoint_auth_methods_supported metadata) + [(get metadata :token_endpoint_auth_method "none")]) + [(get metadata :token_endpoint_auth_method "client_secret_basic")]) + method (if cimd? "none" (first methods))] + (when-not (and (map? metadata) + (vector? redirects) (<= 1 (count redirects) 20) + (every? redirect-uri? redirects) + (or (nil? (:client_name metadata)) + (and (string? (:client_name metadata)) + (<= 1 (count (:client_name metadata)) 200))) + ;; Published metadata describes client capabilities across + ;; servers. Extra grants do not require us to issue them. + (if cimd? + (and (vector? grants) (every? string? grants) + (some #{"authorization_code"} grants)) + (= ["authorization_code"] grants)) + (= ["code"] (get metadata :response_types ["code"])) + (or (nil? (:scope metadata)) (= "mcp" (:scope metadata))) + (if cimd? + (and (vector? methods) (some #{"none"} methods)) + (contains? #{"none" "client_secret_basic" "client_secret_post"} method))) + (invalid! "invalid_client_metadata")) + (cond-> {:redirect_uris redirects + :grant_types ["authorization_code"] + :response_types ["code"] + :scope "mcp" + :token_endpoint_auth_method method} + (:client_name metadata) (assoc :client_name (:client_name metadata)) + ;; Logos are optional browser-loaded images. Ignore unusable URLs + ;; instead of preventing authorization because of cosmetic metadata. + (logo-uri? (:logo_uri metadata)) (assoc :logo_uri (:logo_uri metadata))))) + +(defn register! + "Issue a reusable client ID and, when applicable, a separate client secret. + No client record is stored. The public ID carries only the secret hash; + the random nonce distinguishes registrations with otherwise equal metadata." + [_cfg metadata] + (let [metadata (validate-metadata metadata false) + secret (when-not (= "none" (:token_endpoint_auth_method metadata)) + (state/random-secret)) + ;; Omit fixed protocol values to keep IDs short; validation restores + ;; them on resolution. The version prefix defines the payload format. + payload (cond-> (-> metadata + (dissoc :grant_types :response_types :scope) + (assoc :nonce (state/random-secret))) + secret (assoc :secret-hash (state/digest secret))) + id (str "penpot-mcp-v1." + (.encodeToString (.withoutPadding (Base64/getUrlEncoder)) + (json/encode payload)))] + ;; Enforce the resolver's size cap here rather than issue an unusable ID. + (when (> (count id) 2048) + (invalid! "invalid_client_metadata")) + (cond-> (assoc metadata :client_id id + :client_id_issued_at (quot (System/currentTimeMillis) 1000)) + secret (assoc :client_secret secret :client_secret_expires_at 0)))) + +(defn- decode-client-id + [client-id] + ;; Open registration accepts client-supplied metadata, not a trusted identity. + ;; Changing these details changes the ID; codes are bound to the exact ID + ;; approved by the user, so an edited ID cannot redeem an existing code. + (try + (let [[_ encoded] (re-matches #"penpot-mcp-v1\.([A-Za-z0-9_-]+)" client-id) + _ (when-not encoded (invalid! "invalid_client")) + payload (json/decode (.decode (Base64/getUrlDecoder) ^String encoded)) + metadata (validate-metadata payload false) + nonce (:nonce payload) + secret-hash (:secret-hash payload)] + (when-not (and (string? nonce) (re-matches #"[A-Za-z0-9_-]{43}" nonce) + (if (= "none" (:token_endpoint_auth_method metadata)) + (nil? secret-hash) + (and (string? secret-hash) + (re-matches #"[A-Za-z0-9_-]{43}" secret-hash)))) + (invalid! "invalid_client")) + (cond-> (assoc metadata :client_id client-id) + secret-hash (assoc :secret-hash secret-hash))) + (catch Exception _ + (invalid! "invalid_client")))) + +(defonce ^:private metadata-cache + (cache/create :expire "1m" :max-size 256)) + +(defn- fetch-metadata + "Fetch a client's own HTTPS metadata document with bounded size and freshness. + The shared HTTP client checks SSRF targets and does not follow redirects." + [cfg client-id] + (let [response (http/req cfg {:method :get :uri client-id :timeout 5000 + :headers {"accept" "application/json"}} + {:response-type :input-stream})] + (with-open [^InputStream body (:body response)] + (when-not (and (= 200 (:status response)) + (str/starts-with? (get-in response [:headers "content-type"] "") + "application/json")) + (invalid! "invalid_client")) + ;; Bound both bytes read and time spent receiving the response body. + (let [reader (future (.readNBytes body 65537)) + bytes (deref reader 5000 ::timeout)] + (when (= ::timeout bytes) + (future-cancel reader) + (invalid! "invalid_client")) + (when (> (alength ^bytes bytes) 65536) + (invalid! "invalid_client")) + (let [metadata (json/decode bytes) + control (get-in response [:headers "cache-control"] "") + ;; The local cache ceiling never extends the publisher's TTL. + ;; Missing cache lifetime is treated as immediately stale. + max-age (some-> (re-find #"(?i)max-age=(\d+)" control) second parse-long) + ttl (if (re-find #"(?i)no-store|no-cache" control) + 0 + (min 60 (or max-age 0)))] + (when-not (= client-id (:client_id metadata)) + (invalid! "invalid_client")) + {:client (assoc (validate-metadata metadata true) :client_id client-id) + :expires (+ (System/currentTimeMillis) (* ttl 1000))}))))) + +(defn resolve-client + "Resolve a client ID to validated metadata for authorization or exchange. + HTTPS IDs use the document cache; versioned IDs decode locally without + PostgreSQL or Redis. Neither form establishes user approval." + [cfg client-id] + (when-not (and (string? client-id) (<= 1 (count client-id) 2048)) + (invalid! "invalid_client")) + (if-let [url (when (str/starts-with? client-id "https://") (uri client-id))] + (do + (when-not (and (.getHost url) + (nil? (.getUserInfo url)) + (nil? (.getFragment url)) + (not (str/blank? (.getPath url)))) + (invalid! "invalid_client")) + (try + (let [cached (cache/get metadata-cache client-id) + entry (if (and cached (> (:expires cached) (System/currentTimeMillis))) + cached + (fetch-metadata cfg client-id))] + (when-not (identical? cached entry) + (cache/invalidate metadata-cache client-id) + (cache/get metadata-cache client-id (constantly entry))) + (:client entry)) + (catch Exception _ + (invalid! "invalid_client")))) + (decode-client-id client-id))) diff --git a/backend/src/app/mcp/oauth_rate.lua b/backend/src/app/mcp/oauth_rate.lua new file mode 100644 index 0000000000..0a8a168cfc --- /dev/null +++ b/backend/src/app/mcp/oauth_rate.lua @@ -0,0 +1,5 @@ +-- Copyright (c) KALEIDOS SUBSIDIARY SL +-- MPL-2.0 +local count = redis.call('INCR', KEYS[1]) +if count == 1 then redis.call('EXPIRE', KEYS[1], ARGV[1]) end +return {count} diff --git a/backend/src/app/mcp/oauth_state.clj b/backend/src/app/mcp/oauth_state.clj new file mode 100644 index 0000000000..6f27a3b05e --- /dev/null +++ b/backend/src/app/mcp/oauth_state.clj @@ -0,0 +1,105 @@ +;; 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.mcp.oauth-state + "Temporary approval requests and authorization codes in Redis/Valkey. + Handles are opaque; tenant, record kind and handle hash form the key. + Mutations compare the exact record that the HTTP handler validated." + (:require + [app.config :as cf] + [app.redis :as rds] + [app.redis.script :as-alias script] + [app.util.json :as json]) + (:import + java.nio.charset.StandardCharsets + java.security.MessageDigest + java.security.SecureRandom + java.util.Base64)) + +(defn random-secret + "32 random bytes encoded as a 43-character, unpadded base64url string." + [] + (let [bytes (byte-array 32)] + (.nextBytes (SecureRandom.) bytes) + (.encodeToString (.withoutPadding (Base64/getUrlEncoder)) bytes))) + +(defn digest + "Unpadded base64url SHA-256 digest, also used for PKCE S256 challenges." + [^String value] + (-> (.digest (MessageDigest/getInstance "SHA-256") + (.getBytes value StandardCharsets/UTF_8)) + (#(.encodeToString (.withoutPadding (Base64/getUrlEncoder)) %)))) + +(defn secret= + [a b] + (and (string? a) (string? b) + (MessageDigest/isEqual (.getBytes ^String a StandardCharsets/UTF_8) + (.getBytes ^String b StandardCharsets/UTF_8)))) + +(defn- state-key + [kind handle] + (str "penpot:" (cf/get :tenant) ":mcp-oauth:" (name kind) ":" (digest handle))) + +(defn create! + "Store a new record for ttl seconds and return its random handle. + The handle is returned only after Redis confirms SET NX EX succeeded." + [cfg kind value ttl] + (let [handle (random-secret) + result (rds/run! cfg + (fn [{::rds/keys [conn]}] + (rds/set conn (state-key kind handle) + (json/encode-str value) {:ex ttl :nx true})))] + (when-not (= "OK" result) + (throw (ex-info "Unable to store OAuth state" {:oauth-error "temporarily_unavailable"}))) + handle)) + +(defn read! + "Read an unexpired record, or nil for a missing/malformed handle. + Return both decoded :value for validation and original :raw for atomic + comparison; serializing the decoded map again is not an equivalent check." + [cfg kind handle] + (when (and (string? handle) (re-matches #"[A-Za-z0-9_-]{43}" handle)) + (when-let [raw (rds/run! cfg (fn [{::rds/keys [conn]}] + (rds/get conn (state-key kind handle))))] + {:raw raw :value (json/decode raw)}))) + +(defn- compare-and-update! + "Atomically change only the exact record the caller read and validated. + Lua deletes for nil replacement, or replaces while preserving the TTL. + False means the record expired or another request changed/consumed it." + [cfg kind handle record replacement] + (let [result (rds/run! cfg + (fn [{::rds/keys [conn]}] + (rds/eval conn + {::script/name ::compare-and-update + ::script/path "app/mcp/oauth_state.lua" + ::script/keys [(state-key kind handle)] + ::script/vals [(:raw record) (or replacement "")]})))] + (= [1] result))) + +(defn consume! + "Delete a validated record once; true only for the request that consumes it." + [cfg kind handle record] + (compare-and-update! cfg kind handle record nil)) + +(defn replace! + "Replace a validated record without extending its original expiration." + [cfg kind handle record value] + (compare-and-update! cfg kind handle record (json/encode-str value))) + +(defn allow-request! + "Limit requests per endpoint kind, tenant and remote address. + The one-minute window begins with the first request; subsequent requests + increment the count without extending that window." + [cfg kind remote-address limit] + (let [[count] (rds/run! cfg + (fn [{::rds/keys [conn]}] + (rds/eval conn + {::script/name ::rate + ::script/path "app/mcp/oauth_rate.lua" + ::script/keys [(state-key kind (or remote-address "unknown"))] + ::script/vals [60]})))] + (and (number? count) (<= count limit)))) diff --git a/backend/src/app/mcp/oauth_state.lua b/backend/src/app/mcp/oauth_state.lua new file mode 100644 index 0000000000..b7bcd02330 --- /dev/null +++ b/backend/src/app/mcp/oauth_state.lua @@ -0,0 +1,11 @@ +-- Copyright (c) KALEIDOS SUBSIDIARY SL +-- MPL-2.0 +if redis.call('GET', KEYS[1]) ~= ARGV[1] then + return {0} +end +if ARGV[2] == '' then + redis.call('DEL', KEYS[1]) +else + redis.call('SET', KEYS[1], ARGV[2], 'KEEPTTL') +end +return {1} diff --git a/backend/src/app/rpc/commands/access_token.clj b/backend/src/app/rpc/commands/access_token.clj index 09aa9189c2..6681af3cd7 100644 --- a/backend/src/app/rpc/commands/access_token.clj +++ b/backend/src/app/rpc/commands/access_token.clj @@ -12,6 +12,7 @@ [app.db :as db] [app.main :as-alias main] [app.rpc :as-alias rpc] + [app.rpc.commands.profile :as profile] [app.rpc.doc :as-alias doc] [app.rpc.quotes :as quotes] [app.setup :as-alias setup] @@ -28,12 +29,23 @@ AND id != ? AND type = 'mcp'") +(def ^:private sql:get-valid-mcp-token + "SELECT * FROM access_token + WHERE profile_id = ? + AND type = 'mcp' + AND (expires_at IS NULL OR expires_at > now()) + ORDER BY created_at DESC + LIMIT 1") + (defn create-access-token "Create an access token with empty perms. Elevated permissions (e.g. error-reports:read) are not assignable via the public API; grant them with SQL or `repl:grant-access-token-perm`." [{:keys [::db/conn] :as cfg} profile-id name expiration type] + ;; Serialize regeneration with OAuth provisioning for the same profile. + (when (= type "mcp") + (db/get-by-id conn :profile profile-id {::db/for-update true})) (let [token-id (uuid/next) expires-at (some-> expiration (ct/in-future)) created-at (ct/now) @@ -62,6 +74,21 @@ (decode-row token))) +(defn ensure-mcp-access! + "Enable MCP and return the profile's unexpired token, creating it if absent. + Must run in a transaction. Lock the profile even when no token exists, so + concurrent approvals and settings regeneration use the same lock. Enabling + MCP must not regenerate a token already held by a connected client; an + expired token is replaced because no client can use it." + [{:keys [::db/conn] :as cfg} profile-id] + (let [profile (profile/get-profile conn profile-id ::db/for-update true) + token (db/exec-one! conn [sql:get-valid-mcp-token profile-id])] + (when-not (:mcp-enabled (:props profile)) + (profile/update-profile-props cfg profile-id {:mcp-enabled true})) + (if token + (decode-row token) + (create-access-token cfg profile-id "MCP" nil "mcp")))) + (defn repl:create-access-token [cfg profile-id name expiration] (db/tx-run! cfg create-access-token profile-id name expiration)) diff --git a/backend/test/backend_tests/mcp_oauth_clients_test.clj b/backend/test/backend_tests/mcp_oauth_clients_test.clj new file mode 100644 index 0000000000..04db71a48b --- /dev/null +++ b/backend/test/backend_tests/mcp_oauth_clients_test.clj @@ -0,0 +1,219 @@ +;; 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 backend-tests.mcp-oauth-clients-test + (:require + [app.mcp.oauth-clients :as clients] + [app.mcp.oauth-state :as state] + [app.util.json :as json] + [backend-tests.helpers :as th] + [clojure.string :as str] + [clojure.test :as t] + [mockery.core :refer [with-mocks]])) + +(t/use-fixtures :once th/state-init) + +(t/deftest registered-clients + (let [registration (clients/register! {} + {:client_name "Example" + :redirect_uris ["https://client.example/callback"]}) + client (clients/resolve-client {} (:client_id registration))] + (t/is (string? (:client_secret registration))) + (t/is (nil? (:client_secret client))) + (t/is (state/secret= (:secret-hash client) (state/digest (:client_secret registration)))) + (t/is (clients/redirect-matches? client "https://client.example/callback")) + (doseq [redirect ["https://client.example/callback?extra=1" + "https://client.example/callback/../evil" + "https://evil.example/callback"]] + (t/is (not (clients/redirect-matches? client redirect)))))) + +(t/deftest registrations-are-self-contained-and-distinct + (doseq [method ["none" "client_secret_basic" "client_secret_post"]] + (let [metadata {:client_name "Example α" + :logo_uri "https://client.example/logo.png" + :redirect_uris ["https://client.example/callback?source=mcp&mode=oauth"] + :token_endpoint_auth_method method} + first (clients/register! {} metadata) + second (clients/register! {} metadata) + client (clients/resolve-client {} (:client_id first))] + (t/is (not= (:client_id first) (:client_id second))) + (t/is (<= (count (:client_id first)) 2048)) + (t/is (= metadata (select-keys client (keys metadata)))) + (t/is (= (:client_id first) (:client_id client))) + (t/is (nil? (:client_secret client))) + (if (= method "none") + (do (t/is (nil? (:client_secret first))) + (t/is (nil? (:secret-hash client)))) + (t/is (state/secret= (:secret-hash client) + (state/digest (:client_secret first)))))))) + +(t/deftest oversized-registration-is-rejected + (let [metadata {:redirect_uris [(str "https://client.example/" (apply str (repeat 1800 "a")))]}] + (t/is (= "invalid_client_metadata" + (try (clients/register! {} metadata) + (catch clojure.lang.ExceptionInfo cause (:oauth-error (ex-data cause)))))))) + +(t/deftest unusable-client-logos-are-ignored + ;; An optional image must not prevent a client from authorizing. + (doseq [logo [nil "" 42 "/logo.png" "http://client.example/logo.png" + "javascript:alert(1)" "data:image/svg+xml," + "https://user:secret@client.example/logo.png" + (str "https://client.example/" (apply str (repeat 2048 "a")))]] + (let [client (clients/validate-metadata + {:redirect_uris ["https://client.example/callback"] + :logo_uri logo} + false)] + (t/is (not (contains? client :logo_uri)))))) + +(t/deftest client-id-size-boundary + (let [callback "https://client.example/callback/" + metadata {:redirect_uris [callback] :token_endpoint_auth_method "none"} + registration (clients/register! {} metadata) + [_ encoded] (str/split (:client_id registration) #"\." 2) + payload-size (alength (.decode (java.util.Base64/getUrlDecoder) encoded)) + ;; 1,525 payload bytes encode to 2,034 characters plus the 14-char prefix. + padding (apply str (repeat (- 1525 payload-size) "a")) + metadata (assoc metadata :redirect_uris [(str callback padding)]) + largest (clients/register! {} metadata)] + (t/is (= 2048 (count (:client_id largest)))) + (t/is (= (:redirect_uris metadata) + (:redirect_uris (clients/resolve-client {} (:client_id largest))))) + (t/is (= "invalid_client_metadata" + (try (clients/register! {} (update-in metadata [:redirect_uris 0] str "a")) + (catch clojure.lang.ExceptionInfo cause (:oauth-error (ex-data cause)))))))) + +(t/deftest malformed-client-identifiers-are-rejected + (doseq [id ["unknown" "penpot-mcp-v2.e30" "penpot-mcp-v1.e30" + "penpot-mcp-v1.W10" "penpot-mcp-v1.%%" "penpot-mcp-v1.bm90LWpzb24" + (str "penpot-mcp-v1." (apply str (repeat 2048 "a")))]] + (t/is (= "invalid_client" + (try (clients/resolve-client {} id) + (catch clojure.lang.ExceptionInfo cause (:oauth-error (ex-data cause)))))))) + +(t/deftest client-metadata-validation + (doseq [redirect ["https://client.example/callback#fragment" "javascript:alert(1)" + "http://client.example/callback" "https://user:secret@client.example/cb"]] + (t/is (thrown? clojure.lang.ExceptionInfo + (clients/validate-metadata {:redirect_uris [redirect]} false)))) + (t/is (clients/redirect-matches? {:redirect_uris ["http://127.0.0.1:3000/callback"]} + "http://127.0.0.1:12345/callback")) + (t/is (= "none" (:token_endpoint_auth_method + (clients/validate-metadata + {:redirect_uris ["https://chatgpt.com/connector_platform_oauth_redirect"] + :token_endpoint_auth_methods_supported ["none" "private_key_jwt"]} + true)))) + (t/is (thrown? clojure.lang.ExceptionInfo + (clients/validate-metadata + {:redirect_uris ["https://example.com/cb"] + :grant_types ["authorization_code" "refresh_token"]} false)))) + +(t/deftest client-metadata-documents-can-advertise-additional-grants + (let [metadata {:redirect_uris ["https://client.example/callback"] + :grant_types ["authorization_code" "refresh_token"]}] + (t/is (= ["authorization_code"] + (:grant_types (clients/validate-metadata metadata true)))))) + +(t/deftest client-metadata-documents-require-authorization-code-support + (doseq [grants [[] ["refresh_token"] "authorization_code" + ["authorization_code" nil] ["authorization_code" 1]]] + (t/is (thrown? clojure.lang.ExceptionInfo + (clients/validate-metadata + {:redirect_uris ["https://client.example/callback"] + :grant_types grants} + true))))) + +(t/deftest localhost-callbacks-allow-only-port-changes + (let [client {:redirect_uris ["http://localhost/callback"]}] + (t/is (clients/redirect-uri? "http://localhost/callback")) + (t/is (clients/redirect-matches? client "http://localhost:52351/callback")) + (doseq [redirect ["http://localhost.evil.example:52351/callback" + "http://127.0.0.1:52351/callback" + "http://localhost:52351/other" + "http://localhost:52351/callback?extra=1" + "http://localhost:52351/callback#fragment" + "http://user@localhost:52351/callback"]] + (t/is (not (clients/redirect-matches? client redirect)))) + (doseq [redirect ["http://localhost.evil.example/callback" + "http://user@localhost/callback"]] + (t/is (not (clients/redirect-uri? redirect)))))) + +(t/deftest codex-published-client-metadata + ;; Public metadata from https://chatgpt.com/oauth/codex/client.json. + ;; Keep this fixture offline so upstream changes cannot make CI flaky. + (let [client-id "https://chatgpt.com/oauth/codex/client.json" + metadata {:client_id client-id + :client_uri "https://chatgpt.com/codex" + :application_type "native" + :redirect_uris ["http://127.0.0.1/callback" "http://localhost/callback"] + :token_endpoint_auth_method "none" + :token_endpoint_auth_methods_supported ["none"] + :grant_types ["authorization_code" "refresh_token"] + :response_types ["code"] + :client_name "Codex" + :logo_uri "https://persistent.oaistatic.com/sonic/misc/openai-logo.png"}] + (with-mocks [dns {:target 'app.util.ssrf/resolve-host + :return (into-array java.net.InetAddress + [(java.net.InetAddress/getByName "93.184.216.34")])} + sender {:target 'app.http.client/send! + :return (fn [& _] + {:status 200 + :headers {"content-type" "application/json" "cache-control" "no-store"} + :body (java.io.ByteArrayInputStream. (json/encode metadata))})}] + (let [client (clients/resolve-client th/*system* client-id)] + (t/is (= client-id (:client_id client))) + (t/is (= (:logo_uri metadata) (:logo_uri client))) + (t/is (= ["authorization_code"] (:grant_types client))) + (t/is (= "none" (:token_endpoint_auth_method client))) + (t/is (clients/redirect-matches? client "http://127.0.0.1:52351/callback")))))) + +(t/deftest client-metadata-documents + (let [client-id "https://client.example/client.json" + calls (atom 0) + metadata (atom {:client_id client-id + :client_name "Example" + :redirect_uris ["https://client.example/callback"] + :token_endpoint_auth_methods_supported ["none" "private_key_jwt"]})] + (with-mocks [dns {:target 'app.util.ssrf/resolve-host + :return (into-array java.net.InetAddress + [(java.net.InetAddress/getByName "93.184.216.34")])} + sender {:target 'app.http.client/send! + :return (fn [_ _ _] + (swap! calls inc) + {:status 200 + :headers {"content-type" "application/json" "cache-control" "no-store"} + :body (java.io.ByteArrayInputStream. + (json/encode @metadata))})}] + (t/is (= "none" (:token_endpoint_auth_method (clients/resolve-client th/*system* client-id)))) + (swap! metadata assoc :redirect_uris ["https://client.example/changed"]) + (t/is (not (clients/redirect-matches? (clients/resolve-client th/*system* client-id) + "https://client.example/callback"))) + (t/is (= 2 @calls)) + (swap! metadata assoc :client_id "https://different.example/client.json") + (t/is (thrown? clojure.lang.ExceptionInfo (clients/resolve-client th/*system* client-id))) + (swap! metadata assoc :client_id client-id :client_name (apply str (repeat 65537 "a"))) + (t/is (thrown? clojure.lang.ExceptionInfo (clients/resolve-client th/*system* client-id)))))) + +(t/deftest client-metadata-rejects-unsafe-fetches + (with-mocks [sender {:target 'app.http.client/send! + :return (fn [& _] (throw (AssertionError. "Unsafe request reached HTTP client")))}] + (doseq [client-id ["https://127.0.0.1/client.json" + "https://169.254.169.254/client.json" + "https://[::1]/client.json" + "https://user:secret@example.com/client.json" + "https://example.com/client.json#fragment"]] + (t/is (thrown? clojure.lang.ExceptionInfo (clients/resolve-client th/*system* client-id)))) + (t/is (not (:called? @sender))))) + +(t/deftest client-metadata-rejects-redirects + (with-mocks [dns {:target 'app.util.ssrf/resolve-host + :return (into-array java.net.InetAddress + [(java.net.InetAddress/getByName "93.184.216.34")])} + sender {:target 'app.http.client/send! + :return {:status 302 + :headers {"location" "https://127.0.0.1/internal"} + :body (java.io.ByteArrayInputStream. (byte-array 0))}}] + (t/is (thrown? clojure.lang.ExceptionInfo + (clients/resolve-client th/*system* "https://redirect.example/client.json"))))) diff --git a/backend/test/backend_tests/mcp_oauth_http_test.clj b/backend/test/backend_tests/mcp_oauth_http_test.clj new file mode 100644 index 0000000000..3a19d8bd93 --- /dev/null +++ b/backend/test/backend_tests/mcp_oauth_http_test.clj @@ -0,0 +1,253 @@ +;; 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 backend-tests.mcp-oauth-http-test + (:require + [app.common.uri :as uri] + [app.config :as cf] + [app.db :as db] + [app.http :as http] + [app.http.errors :as errors] + [app.http.mcp-oauth :as oauth] + [app.http.session :as session] + [app.mcp.oauth-clients :as clients] + [app.mcp.oauth-state :as state] + [app.rpc.commands.access-token :as access-token] + [app.setup.clock :as clock] + [app.util.json :as json] + [backend-tests.helpers :as th] + [clojure.string :as str] + [clojure.test :as t] + [yetti.response :as yres]) + (:import java.net.URLEncoder java.nio.charset.StandardCharsets)) + +(t/use-fixtures :once th/state-init) +(t/use-fixtures :each th/database-reset + (fn [run] + (binding [cf/config (assoc cf/config :tenant (str (java.util.UUID/randomUUID)))] + (run)))) + +(defn- form [params] + (str/join "&" (for [[key value] params] + (str (name key) "=" (URLEncoder/encode (str value) StandardCharsets/UTF_8))))) + +(defn- request [path & {:as options}] + (let [handler (#'http/router-handler (:app.http/router th/*system*))] + (handler (th/make-dummy-request (assoc options :path path))))) + +(defn- body [response] (json/decode (::yres/body response))) + +(defn- login-cookie [profile] + (let [session (session/create-session (:app.http.session/manager th/*system*) + {:profile-id (:id profile)}) + token (:token (#'session/assign-token th/*system* session))] + {(cf/get :auth-token-cookie-name) token})) + +(defn- begin [client] + (let [reply (request "/api/mcp/oauth/authorize" + :query (form {: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"}))] + (t/is (= 303 (::yres/status reply))) + (get (uri/query-string->map (.getQuery (java.net.URI. (get-in reply [::yres/headers "location"])))) + :request))) + +(t/deftest discovery-and-registration + (let [metadata (body (request "/.well-known/oauth-authorization-server/api/mcp/oauth")) + resource (body (request "/api/mcp/oauth/resource")) + registered (request "/api/mcp/oauth/register" :method :post + :headers {"content-type" "application/json"} + :body-bytes (json/encode-str {:redirect_uris ["https://example.com/callback"] + :token_endpoint_auth_method "none"}))] + (t/is (= (oauth/issuer) (:issuer metadata))) + (t/is (= ["S256"] (:code_challenge_methods_supported metadata))) + (t/is (= (oauth/resource) (:resource resource))) + (t/is (= 201 (::yres/status registered))) + (t/is (= "none" (:token_endpoint_auth_method (body registered)))) + (t/is (nil? (:client_secret (body registered)))))) + +(t/deftest complete-authorization-flow + (let [profile (th/create-profile* 1 {:is-active true}) + cookies (login-cookie profile) + client (clients/register! th/*system* {:redirect_uris ["https://example.com/callback"] + :logo_uri "https://example.com/logo.png" + :token_endpoint_auth_method "none"}) + handle (begin client) + details (request "/api/mcp/oauth/request" :query (form {:request handle}) :cookies cookies) + origin (str (cf/get-public-uri)) + origin (str/replace origin #"/$" "") + decision {:request handle :decision "approve" :csrf (:csrf (body details))} + submit #(request "/api/mcp/oauth/decision" :method :post :cookies cookies + :headers {"content-type" "application/x-www-form-urlencoded" "origin" origin} + :body-bytes (form %))] + (t/is (= 401 (::yres/status (request "/api/mcp/oauth/request" :query (form {:request handle}))))) + (t/is (= 200 (::yres/status details))) + (t/is (= "https://example.com/logo.png" (:logo-uri (body details)))) + (t/is (= 400 (::yres/status (submit (assoc decision :csrf "wrong"))))) + (let [approved (submit decision) + callback (java.net.URI. (:redirect-uri (body approved))) + params (uri/query-string->map (.getQuery callback)) + exchange {:grant_type "authorization_code" :code (:code params) + :client_id (:client_id client) :redirect_uri "https://example.com/callback" + :resource (oauth/resource) :code_verifier (apply str (repeat 43 "a"))} + redeem #(request "/api/mcp/oauth/token" :method :post + :headers {"content-type" "application/x-www-form-urlencoded"} + :body-bytes (form %))] + (t/is (= 200 (::yres/status approved))) + (t/is (= "client-state" (:state params))) + (t/is (= (oauth/issuer) (:iss params))) + (t/is (= 400 (::yres/status (submit decision)))) + ;; Metadata is public and editable, but changing it changes the client ID. + ;; Even with the correct verifier, an edited ID cannot use this approval. + (let [[prefix encoded] (str/split (:client_id client) #"\." 2) + payload (json/decode (.decode (java.util.Base64/getUrlDecoder) encoded))] + (doseq [changed [(assoc payload :client_name "Different application") + (assoc payload :redirect_uris ["https://attacker.example/callback"])]] + (let [id (str prefix "." (.encodeToString (.withoutPadding (java.util.Base64/getUrlEncoder)) + (json/encode changed)))] + (t/is (string? (:client_id (clients/resolve-client {} id)))) + (t/is (= "invalid_grant" (:error (body (redeem (assoc exchange :client_id id))))))))) + (t/is (= 400 (::yres/status (redeem (assoc exchange :code_verifier (apply str (repeat 43 "b"))))))) + (let [token-response (redeem exchange) + token (body token-response) + stored (first (db/query th/*pool* :access-token {:profile-id (:id profile) :type "mcp"}))] + (t/is (= 200 (::yres/status token-response))) + (t/is (= "no-store" (get-in token-response [::yres/headers "cache-control"]))) + (t/is (= (:token stored) (:access_token token))) + (t/is (= "Bearer" (:token_type token))) + (t/is (nil? (:expires_in token))) + (t/is (nil? (:refresh_token token))) + (t/is (= 400 (::yres/status (redeem exchange)))))))) + +(t/deftest denial-and-session-binding + (let [profile (th/create-profile* 1 {:is-active true}) + other (th/create-profile* 2 {:is-active true}) + cookies (login-cookie profile) + client (clients/register! th/*system* {:redirect_uris ["https://example.com/callback"] + :token_endpoint_auth_method "none"}) + handle (begin client) + details (body (request "/api/mcp/oauth/request" :query (form {:request handle}) :cookies cookies)) + params {:request handle :csrf (:csrf details) :decision "deny"} + submit #(request "/api/mcp/oauth/decision" :method :post :cookies cookies + :headers {"content-type" "application/x-www-form-urlencoded" "origin" %} + :body-bytes (form params))] + (t/is (= 403 (::yres/status + (request "/api/mcp/oauth/request" :query (form {:request handle}) + :cookies (login-cookie other))))) + (t/is (= 403 (::yres/status (submit "https://attacker.example")))) + (let [denied (submit (str/replace (str (cf/get-public-uri)) #"/$" "")) + callback (java.net.URI. (:redirect-uri (body denied))) + result (uri/query-string->map (.getQuery callback))] + (t/is (= "access_denied" (:error result))) + (t/is (= "client-state" (:state result))) + (t/is (empty? (db/query th/*pool* :access-token {:profile-id (:id profile)}))) + (t/is (not (:mcp-enabled (db/decode-transit-pgobject + (:props (th/db-get :profile {:id (:id profile)}))))))))) + +(t/deftest oauth-input-and-cors-errors + (doseq [data ["{" "[]" (apply str (repeat 16385 "a"))]] + (let [reply (request "/api/mcp/oauth/register" :method :post + :headers {"content-type" "application/json"} + :body-bytes data)] + (t/is (= 400 (::yres/status reply))) + (t/is (= "*" (get-in reply [::yres/headers "access-control-allow-origin"]))))) + (let [reply (request "/api/mcp/oauth/token" :method :post + :headers {"content-type" "application/x-www-form-urlencoded"} + :body-bytes "code=a&code=b")] + (t/is (= 400 (::yres/status reply)))) + (t/is (= 204 (::yres/status (request "/api/mcp/oauth/token" :method :options))))) + +(t/deftest confidential-client-exchange + (let [profile (th/create-profile* 1 {:is-active true}) + stored (db/tx-run! th/*system* access-token/ensure-mcp-access! (:id profile)) + verifier (apply str (repeat 43 "a"))] + (doseq [method ["client_secret_basic" "client_secret_post"]] + (let [client (clients/register! th/*system* + {:redirect_uris ["https://example.com/callback"] + :token_endpoint_auth_method method}) + code (state/create! th/*system* :code + {:client-id (:client_id client) + :redirect-uri "https://example.com/callback" + :resource (oauth/resource) + :challenge (state/digest verifier) + :profile-id (str (:id profile)) + :token-id (str (:id stored))} + 120) + params {:grant_type "authorization_code" :client_id (:client_id client) + :code code :code_verifier verifier :redirect_uri "https://example.com/callback"} + redeem (fn [secret] + (request "/api/mcp/oauth/token" :method :post + :headers (cond-> {"content-type" "application/x-www-form-urlencoded"} + (= method "client_secret_basic") + (assoc "authorization" + (str "Basic " (.encodeToString + (java.util.Base64/getEncoder) + (.getBytes (str (:client_id client) ":" secret) "UTF-8"))))) + :body-bytes (form (cond-> params + (= method "client_secret_post") + (assoc :client_secret secret)))))] + (t/is (= 401 (::yres/status (redeem "wrong")))) + (t/is (= (:token stored) (:access_token (body (redeem (:client_secret client)))))))))) + +(t/deftest exchange-rejects-expired-token + (let [profile (th/create-profile* 1 {:is-active true}) + stored (db/tx-run! th/*system* access-token/ensure-mcp-access! (:id profile)) + verifier (apply str (repeat 43 "a")) + client (clients/register! th/*system* {:redirect_uris ["https://example.com/callback"] + :token_endpoint_auth_method "none"}) + code (state/create! th/*system* :code + {:client-id (:client_id client) + :redirect-uri "https://example.com/callback" + :resource (oauth/resource) + :challenge (state/digest verifier) + :profile-id (str (:id profile)) + :token-id (str (:id stored))} + 120)] + (db/exec! th/*pool* ["UPDATE access_token SET expires_at = now() - interval '1 day' WHERE id = ?" + (:id stored)]) + (let [reply (request "/api/mcp/oauth/token" :method :post + :headers {"content-type" "application/x-www-form-urlencoded"} + :body-bytes (form {:grant_type "authorization_code" :code code + :client_id (:client_id client) :code_verifier verifier + :redirect_uri "https://example.com/callback"}))] + (t/is (= 400 (::yres/status reply))) + (t/is (= "invalid_grant" (:error (body reply))))))) + +(t/deftest unexpected-errors-are-logged + (let [logged (atom []) + cookies (login-cookie (th/create-profile* 1 {:is-active true})) + fail (fn [& _] (throw (RuntimeException. "unexpected")))] + (with-redefs [errors/handle (fn [cause _] (swap! logged conj (ex-message cause))) + clients/register! fail + clock/get-clock fail] + (t/testing "endpoint failures" + (let [reply (request "/api/mcp/oauth/register" :method :post + :headers {"content-type" "application/json"} + :body-bytes (json/encode-str {:redirect_uris ["https://example.com/callback"]}))] + (t/is (= 503 (::yres/status reply))) + (t/is (= {:error "temporarily_unavailable"} (body reply))) + (t/is (= "*" (get-in reply [::yres/headers "access-control-allow-origin"]))))) + (t/testing "session middleware failures" + (let [reply (request "/api/mcp/oauth/request" :query (form {:request "x"}) :cookies cookies)] + (t/is (= 503 (::yres/status reply))) + (t/is (= {:error "temporarily_unavailable"} (body reply))))) + (t/testing "OAuth rejections are not logged" + (t/is (= 400 (::yres/status (request "/api/mcp/oauth/token" :method :post + :headers {"content-type" "application/x-www-form-urlencoded"} + :body-bytes "code=a&code=b")))))) + (t/is (= ["unexpected" "unexpected"] @logged)))) + +(t/deftest authorization-rejects-unregistered-callback + (let [client (clients/register! th/*system* {:redirect_uris ["https://example.com/callback"]}) + reply (request "/api/mcp/oauth/authorize" + :query (form {:client_id (:client_id client) + :redirect_uri "https://attacker.example/callback"}))] + (t/is (= 400 (::yres/status reply))) + (t/is (nil? (get-in reply [::yres/headers "location"]))))) diff --git a/backend/test/backend_tests/mcp_oauth_state_test.clj b/backend/test/backend_tests/mcp_oauth_state_test.clj new file mode 100644 index 0000000000..196c8378a2 --- /dev/null +++ b/backend/test/backend_tests/mcp_oauth_state_test.clj @@ -0,0 +1,53 @@ +;; 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 backend-tests.mcp-oauth-state-test + (:require + [app.config :as cf] + [app.mcp.oauth-state :as state] + [backend-tests.helpers :as th] + [clojure.test :as t] + [mockery.core :refer [with-mocks]])) + +(t/use-fixtures :once th/state-init) + +(t/deftest authorization-state + (let [cfg th/*system* + handle (state/create! cfg :code {:client-id "test"} 60) + record (state/read! cfg :code handle)] + (t/is (= {:client-id "test"} (:value record))) + (t/is (nil? (state/read! cfg :request handle))) + (t/is (false? (state/consume! cfg :code handle (assoc record :raw "wrong")))) + (t/is (= record (state/read! cfg :code handle))) + (let [results (mapv deref (repeatedly 8 #(future (state/consume! cfg :code handle record))))] + (t/is (= 1 (count (filter true? results))))) + (t/is (nil? (state/read! cfg :code handle))))) + +(t/deftest bind-pending-request-once + (let [cfg th/*system* + handle (state/create! cfg :request {:client-id "test"} 60) + record (state/read! cfg :request handle)] + (t/is (true? (state/replace! cfg :request handle record {:session-id "first"}))) + (t/is (false? (state/replace! cfg :request handle record {:session-id "second"}))) + (t/is (= {:session-id "first"} (:value (state/read! cfg :request handle)))))) + +(t/deftest pkce-and-secrets + ;; RFC 7636 appendix B + (t/is (= "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM" + (state/digest "dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk")))) + +(t/deftest expiry-and-tenant-isolation + (let [cfg th/*system* + handle (state/create! cfg :request {:test true} 1)] + (binding [cf/config (assoc cf/config :tenant "another-tenant")] + (t/is (nil? (state/read! cfg :request handle)))) + (Thread/sleep 1100) + (t/is (nil? (state/read! cfg :request handle))))) + +(t/deftest state-storage-fails-closed + (with-mocks [store {:target 'app.redis/set :return nil}] + (t/is (thrown? clojure.lang.ExceptionInfo + (state/create! th/*system* :request {:test true} 60))))) diff --git a/backend/test/backend_tests/rpc_access_tokens_test.clj b/backend/test/backend_tests/rpc_access_tokens_test.clj index 5fa3a1496a..e5fcaf5348 100644 --- a/backend/test/backend_tests/rpc_access_tokens_test.clj +++ b/backend/test/backend_tests/rpc_access_tokens_test.clj @@ -11,6 +11,7 @@ [app.http :as http] [app.http.access-token :as actoken] [app.rpc :as-alias rpc] + [app.rpc.commands.access-token :as access-token] [app.storage :as sto] [backend-tests.helpers :as th] [clojure.test :as t] @@ -232,3 +233,40 @@ decoded (actoken/decode-token cfg access-token)] (t/is (some? decoded)) (t/is (= (:id prof) (:uid decoded))))))) + +(t/deftest oauth-mcp-token-provisioning + (let [prof (th/create-profile* 1 {:is-active true}) + cfg th/*system* + provision #(db/tx-run! cfg access-token/ensure-mcp-access! (:id prof))] + (t/testing "approval enables MCP and creates a fixed token" + (let [token (provision) + props (-> (th/db-get :profile {:id (:id prof)}) + :props db/decode-transit-pgobject)] + (t/is (string? (:token token))) + (t/is (nil? (:expires-at token))) + (t/is (true? (:mcp-enabled props))) + (t/is (= token (provision))))) + (t/testing "approval preserves a token generated in settings" + (let [{:keys [result error]} + (th/command! {::th/type :create-access-token + ::rpc/profile-id (:id prof) + :type "mcp" + :name "From settings"})] + (t/is (nil? error)) + (t/is (= (:token result) (:token (provision)))))) + (t/testing "approval replaces an expired token" + (db/exec! th/*pool* [(str "UPDATE access_token SET expires_at = now() - interval '1 day'" + " WHERE profile_id = ?") + (:id prof)]) + (let [expired (first (db/query th/*pool* :access-token {:profile-id (:id prof) :type "mcp"})) + token (provision)] + (t/is (not= (:token expired) (:token token))) + (t/is (nil? (:expires-at token))) + (t/is (= [(:id token)] + (map :id (db/query th/*pool* :access-token {:profile-id (:id prof) :type "mcp"})))))) + (t/testing "concurrent approvals share one token" + (db/delete! th/*pool* :access-token {:profile-id (:id prof)}) + (let [results (mapv deref (repeatedly 4 #(future (provision))))] + (t/is (= 1 (count (set (map :token results))))) + (t/is (= 1 (count (db/query th/*pool* :access-token + {:profile-id (:id prof) :type "mcp"})))))))) diff --git a/backend/test/backend_tests/rpc_binfile_test.clj b/backend/test/backend_tests/rpc_binfile_test.clj index e684d5b7e0..f15a261040 100644 --- a/backend/test/backend_tests/rpc_binfile_test.clj +++ b/backend/test/backend_tests/rpc_binfile_test.clj @@ -175,14 +175,14 @@ (defn- import-file [profile path] - (let [out (th/command! {::th/type :import-binfile - ::rpc/profile-id (:id profile) - :project-id (:default-project-id profile) - :name "imported" - :file {:filename "imported.penpot" - :size (fs/size path) - :path path - :mtype "application/zip"}})] + (let [out (th/multipart-command! {::th/type :import-binfile + ::rpc/profile-id (:id profile) + :project-id (:default-project-id profile) + :name "imported" + :file {:filename "imported.penpot" + :size (fs/size path) + :path path + :mtype "application/zip"}})] (t/is (nil? (:error out))) (th/consume-sse (:result out)))) diff --git a/docker/devenv/files/nginx.conf b/docker/devenv/files/nginx.conf index 5a0a81f091..9a3cfd3222 100644 --- a/docker/devenv/files/nginx.conf +++ b/docker/devenv/files/nginx.conf @@ -141,6 +141,11 @@ http { proxy_pass http://127.0.0.1:6061; } + location = /.well-known/oauth-authorization-server/api/mcp/oauth { + proxy_pass http://127.0.0.1:6060; + proxy_http_version 1.1; + } + location /api { proxy_pass http://127.0.0.1:6060/api; proxy_http_version 1.1; diff --git a/docker/images/files/nginx.conf.template b/docker/images/files/nginx.conf.template index c8800b0503..54d62e18eb 100644 --- a/docker/images/files/nginx.conf.template +++ b/docker/images/files/nginx.conf.template @@ -155,6 +155,11 @@ http { proxy_pass $PENPOT_EXPORTER_URI; } + location = /.well-known/oauth-authorization-server/api/mcp/oauth { + proxy_set_header Host $proxy_host; + proxy_pass $PENPOT_BACKEND_URI$request_uri; + } + location /api { proxy_set_header Host $proxy_host; proxy_pass $PENPOT_BACKEND_URI/api; diff --git a/frontend/playwright/ui/specs/email-verification.spec.js b/frontend/playwright/ui/specs/email-verification.spec.js index 65c1a9cb99..f15be88e20 100644 --- a/frontend/playwright/ui/specs/email-verification.spec.js +++ b/frontend/playwright/ui/specs/email-verification.spec.js @@ -1,32 +1,33 @@ import { test, expect } from "@playwright/test"; import { RegisterPage } from "../pages/RegisterPage"; -// Regression test for the bug where a freshly verified account (whose -// profile never had a theme persisted) ended up with an empty string as -// its theme instead of falling back to the dark default: the workspace -// switched to light mode and Settings > UI Theme showed a blank field. +// A freshly verified account has no saved theme preference. It must follow +// the system theme and show that preference in settings, not a blank field. test.beforeEach(async ({ page }) => { await RegisterPage.initWithLoggedOutUser(page); }); test.describe("Email verification", () => { - test("Newly verified account defaults to the dark theme", async ({ - page, - }) => { - const registerPage = new RegisterPage(page); - await registerPage.setupEmailVerificationSuccess(); + for (const colorScheme of ["light", "dark"]) { + test(`Newly verified account follows the ${colorScheme} system theme`, async ({ + page, + }) => { + await page.emulateMedia({ colorScheme }); + const registerPage = new RegisterPage(page); + await registerPage.setupEmailVerificationSuccess(); - await registerPage.goToVerifyToken(); - await page.waitForURL(/screen=dashboard/); + await registerPage.goToVerifyToken(); + await page.waitForURL(/screen=dashboard/); - // `default` is the body class applied for dark theme, `light` for - // light theme (see app.util.theme/set-color-scheme). - await expect(page.locator("body")).toHaveClass(/default/); - await expect(page.locator("body")).not.toHaveClass(/light/); - }); + // The dark theme uses the legacy body class "default". + await expect(page.locator("body")).toHaveClass( + colorScheme === "dark" ? "default" : "light", + ); + }); + } - test("Settings > UI Theme shows Penpot Dark (default) selected, not blank", async ({ + test("Settings > UI Theme shows System theme selected, not blank", async ({ page, }) => { const registerPage = new RegisterPage(page); @@ -40,6 +41,6 @@ test.describe("Email verification", () => { // The language select is the first combobox on the page, the theme // select is the second one. const themeSelect = page.getByRole("combobox").nth(1); - await expect(themeSelect).toHaveText("Penpot Dark (default)"); + await expect(themeSelect).toHaveText("System theme"); }); }); diff --git a/frontend/playwright/ui/specs/workspace.spec.js b/frontend/playwright/ui/specs/workspace.spec.js index 051530b877..b59266f5eb 100644 --- a/frontend/playwright/ui/specs/workspace.spec.js +++ b/frontend/playwright/ui/specs/workspace.spec.js @@ -107,6 +107,8 @@ test("Selection size badge appears on selection and hides on deselect", async ({ test("Selection size badge uses component color for component selection", async ({ page, }) => { + // The expected component color below is the dark palette value. + await page.emulateMedia({ colorScheme: "dark" }); const workspacePage = new WasmWorkspacePage(page); await workspacePage.setupEmptyFile(); await workspacePage.mockGetFile("components/get-file-13267.json"); diff --git a/frontend/src/app/main/ui.cljs b/frontend/src/app/main/ui.cljs index bbae3fec99..5e2075082c 100644 --- a/frontend/src/app/main/ui.cljs +++ b/frontend/src/app/main/ui.cljs @@ -20,6 +20,7 @@ [app.main.ui.error-boundary :refer [error-boundary*]] [app.main.ui.exports.files] [app.main.ui.frame-preview :as frame-preview] + [app.main.ui.mcp-authorize :refer [mcp-authorize*]] [app.main.ui.nitrate.entry :as nitrate-entry] [app.main.ui.notifications :as notifications] [app.main.ui.onboarding.questions :refer [questions-modal]] @@ -118,6 +119,9 @@ :auth-verify-token [:? [:> verify-token-page* {:route route}]] + :mcp-authorize + [:> mcp-authorize* {:route route :profile profile}] + :nitrate-entry [:> nitrate-entry/nitrate-entry-page* {:profile profile}] diff --git a/frontend/src/app/main/ui/auth/login.cljs b/frontend/src/app/main/ui/auth/login.cljs index d200c3746d..b43a954986 100644 --- a/frontend/src/app/main/ui/auth/login.cljs +++ b/frontend/src/app/main/ui/auth/login.cljs @@ -70,7 +70,7 @@ [:string {:min 1}]]]) (mf/defc login-form* - [{:keys [params handle-redirect on-success-callback on-recovery-request origin] :as props}] + [{:keys [params handle-redirect on-success-callback on-recovery-request origin hide-recovery-link] :as props}] (let [initial (mf/with-memo [params] params) form (fm/use-form :schema schema:login-form @@ -193,7 +193,8 @@ :label (tr "auth.password") :class (stl/css :form-field)}]]) - (when (and (not= origin :viewer) + (when (and (not hide-recovery-link) + (not= origin :viewer) (or (contains? cf/flags :login) (contains? cf/flags :login-with-password))) [:div {:class (stl/css :form-row :forgot-password-row)} diff --git a/frontend/src/app/main/ui/auth/login_modal.cljs b/frontend/src/app/main/ui/auth/login_modal.cljs new file mode 100644 index 0000000000..ee192893a4 --- /dev/null +++ b/frontend/src/app/main/ui/auth/login_modal.cljs @@ -0,0 +1,146 @@ +;; 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.ui.auth.login-modal + (:require-macros [app.main.style :as stl]) + (:require + [app.config :as cf] + [app.main.router :as rt] + [app.main.store :as st] + [app.main.ui.auth.login :refer [login-dialog*]] + [app.main.ui.auth.recovery-request :refer [recovery-request-page*]] + [app.main.ui.auth.register :refer [register-methods* register-success-page* + register-validate-form* terms-service-privacy-policy*]] + [app.main.ui.ds.buttons.icon-button :refer [icon-button*]] + [app.main.ui.ds.foundations.assets.icon :as i] + [app.util.dom :as dom] + [app.util.i18n :as i18n :refer [tr]] + [app.util.keyboard :as k] + [rumext.v2 :as mf])) + +(mf/defc login-register-modal* + [{:keys [params on-close on-register]}] + (let [user-email (mf/use-state "") + register-token (mf/use-state "") + + current-section* (mf/use-state :login) + current-section (deref current-section*) + + set-current-section + (mf/use-fn + #(reset! current-section* %)) + + set-section + (mf/use-fn + (fn [event] + (let [section (-> (dom/get-current-target event) + (dom/get-data "value") + (keyword))] + (set-current-section section)))) + + go-back-to-login + (mf/use-fn + #(set-current-section :login)) + + main-section (or + (= current-section :login) + (= current-section :register) + (= current-section :register-validate)) + close + (fn [event] + (dom/prevent-default event) + (dom/stop-propagation event) + (on-close)) + + success-email-sent + (fn [email] + (reset! user-email email) + (set-current-section :email-sent)) + + success-login + (fn [& _] + (.reload js/window.location true)) + + success-register + (fn [data] + (cond + (:invitation-token data) + (st/emit! (rt/nav :auth-verify-token {:token (:invitation-token data)})) + + (:is-active data) + (if on-register (on-register) (on-close)) + + :else + (do + (reset! user-email (:email data)) + (set-current-section :email-sent))))] + + [:div {:class (stl/css :modal-overlay) + :on-key-down #(when (k/esc? %) (close %))} + [:div {:class (stl/css :modal-container) + :role "dialog" + :aria-modal true + :aria-labelledby "login-modal-title"} + [:div {:class (stl/css :modal-header)} + [:h2 {:class (stl/css :modal-header-title) + :id "login-modal-title"} (tr "labels.continue-with-penpot")] + [:> icon-button* {:variant "ghost" + :class (stl/css :modal-close) + :aria-label (tr "labels.close") + :on-click close + :icon i/close}]] + + [:div {:class (stl/css :modal-content)} + (case current-section + :login + [:div {:class (stl/css :login-form)} + [:> login-dialog* {:params params + :on-success-callback success-login + :hide-recovery-link true}] + [:div {:class (stl/css :login-links)} + [:div + [:a {:on-click set-section + :data-value "recovery-request"} + (tr "auth.forgot-password")]] + (when (contains? cf/flags :registration) + [:div + [:span + (tr "auth.register") " "] + [:a {:on-click set-section + :data-value "register"} + (tr "auth.register-submit")]])]] + + :register + [:div {:class (stl/css :login-form)} + [:> register-methods* {:params params :on-success-callback success-register}] + [:div {:class (stl/css :login-links)} + [:div + [:span (tr "auth.already-have-account") " "] + [:a {:on-click set-section + :data-value "login"} + (tr "auth.login-here")]]]] + + :register-validate + [:div {:class (stl/css :login-form)} + [:> register-validate-form* {:params {:token @register-token} + :on-success-callback success-email-sent}] + [:div {:class (stl/css :login-links)} + [:div + [:a {:on-click set-section + :data-value "register"} + (tr "labels.go-back")]]]] + + :recovery-request + [:> recovery-request-page* {:go-back-callback go-back-to-login + :on-success-callback success-email-sent}] + + :email-sent + [:div {:class (stl/css :login-form)} + [:> register-success-page* {:params {:email @user-email}}]]) + + (when main-section + [:div {:class (stl/css :login-links)} + [:> terms-service-privacy-policy*]])]]])) diff --git a/frontend/src/app/main/ui/viewer/login.scss b/frontend/src/app/main/ui/auth/login_modal.scss similarity index 100% rename from frontend/src/app/main/ui/viewer/login.scss rename to frontend/src/app/main/ui/auth/login_modal.scss diff --git a/frontend/src/app/main/ui/components/progress.cljs b/frontend/src/app/main/ui/components/progress.cljs index 52fb0b2393..35ae9f3101 100644 --- a/frontend/src/app/main/ui/components/progress.cljs +++ b/frontend/src/app/main/ui/components/progress.cljs @@ -31,8 +31,8 @@ [] (let [state (mf/deref refs/progress) profile (mf/deref refs/profile) - theme (get profile :theme theme/default) - default-theme? (= theme/default theme) + theme (theme/use-theme profile) + dark-theme? (= "dark" theme) error? (:error state) healthy? (:healthy state) visible? (:visible state) @@ -48,13 +48,13 @@ color (cond error? clr/new-danger - healthy? (if default-theme? + healthy? (if dark-theme? clr/new-primary clr/new-primary-light) (not healthy?) clr/new-warning) background-clr - (if default-theme? + (if dark-theme? clr/background-quaternary clr/background-quaternary-light) diff --git a/frontend/src/app/main/ui/exports/assets.cljs b/frontend/src/app/main/ui/exports/assets.cljs index eb842c1f6b..6f7f2311aa 100644 --- a/frontend/src/app/main/ui/exports/assets.cljs +++ b/frontend/src/app/main/ui/exports/assets.cljs @@ -207,8 +207,8 @@ [] (let [state (mf/deref refs/export) profile (mf/deref refs/profile) - theme (or (:theme profile) theme/default) - is-default-theme? (= theme/default theme) + theme (theme/use-theme profile) + dark-theme? (= "dark" theme) error? (:error state) ;; The exporter is at capacity: worth its own wording, so the user ;; knows retrying later is the thing to do. @@ -245,13 +245,13 @@ error? clr/new-danger (or cancelling? cancelled?) clr/new-warning - healthy? (if is-default-theme? + healthy? (if dark-theme? clr/new-primary clr/new-primary-light) (not healthy?) clr/new-warning) background-clr - (if is-default-theme? + (if dark-theme? clr/background-quaternary clr/background-quaternary-light) diff --git a/frontend/src/app/main/ui/mcp_authorize.cljs b/frontend/src/app/main/ui/mcp_authorize.cljs new file mode 100644 index 0000000000..5f47fd5df6 --- /dev/null +++ b/frontend/src/app/main/ui/mcp_authorize.cljs @@ -0,0 +1,162 @@ +;; 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.ui.mcp-authorize + (:require-macros [app.main.style :as stl]) + (:require + [app.common.uri :as uri] + [app.config :as cf] + [app.main.broadcast :as mbc] + [app.main.data.profile :as profile] + [app.main.router :as rt] + [app.main.store :as st] + [app.main.ui.auth.login-modal :refer [login-register-modal*]] + [app.main.ui.ds.buttons.button :refer [button*]] + [app.main.ui.ds.foundations.typography :as typography] + [app.main.ui.ds.foundations.typography.heading :refer [heading*]] + [app.main.ui.ds.foundations.typography.text :refer [text*]] + [app.main.ui.ds.product.loader :refer [loader*]] + [app.util.http :as http] + [app.util.i18n :refer [tr]] + [app.util.storage :as storage] + [beicon.v2.core :as rx] + [rumext.v2 :as mf])) + +(defn- endpoint + [path] + (str (uri/join cf/public-uri (str "api/mcp/oauth/" path)))) + +(defn- prepare-login! + [] + (let [href (rt/get-current-href)] + ;; Save the return URL even when only SSO buttons are enabled and the + ;; password form (which normally stores it) never mounts. + (binding [storage/*sync* true] + (swap! storage/session assoc :login-redirect href)) + {:callback-url href})) + +(defn- cancel-login! + [] + ;; Do not let a later, unrelated login resume this cancelled attempt. + (when (= (:login-redirect storage/session) (rt/get-current-href)) + (binding [storage/*sync* true] + (swap! storage/session dissoc :login-redirect)))) + +(defn- oauth-request + [method path params] + (->> (http/send! (cond-> {:method method + :uri (endpoint path) + :response-type :json + :omit-default-headers true} + (= method :get) (assoc :query params) + (= method :post) (assoc :headers {"content-type" "application/x-www-form-urlencoded"} + :body (uri/map->query-string params)))) + (rx/map #(update % :body js->clj :keywordize-keys true)))) + +(mf/defc client-logo* + {::mf/private true} + [{:keys [uri]}] + (let [failed (mf/use-state false)] + (when-not @failed + [:img {:class (stl/css :client-logo) + :src uri + :alt "" + :referrer-policy "no-referrer" + :on-error #(reset! failed true)}]))) + +(mf/defc mcp-authorize* + [{:keys [route profile]}] + (let [handle (get-in route [:query-params :request]) + details (mf/use-state nil) + failed? (mf/use-state false) + login-params (mf/use-state nil) + cancelled? (mf/use-state false) + busy? (mf/use-state false) + ;; The ref also blocks a second click before React renders disabled buttons. + submitting (mf/use-memo (fn [] (atom false))) + + decide! + (mf/use-fn + (mf/deps handle @details) + (fn [decision] + (when (compare-and-set! submitting false true) + (reset! busy? true) + (->> (oauth-request :post "decision" + {:request handle :csrf (:csrf @details) :decision decision}) + (rx/subs! + (fn [{:keys [status body]}] + (if (= status 200) + (do + (when (= decision "approve") + (st/emit! (profile/refresh-profile) + (mbc/event :mcp/enable {}))) + (st/emit! (rt/nav-raw :href (:redirect-uri body)))) + (reset! failed? true))) + (fn [_] (reset! failed? true)))))))] + + (mf/with-effect [handle] + (reset! details nil) + (reset! failed? false) + (reset! login-params nil) + (reset! cancelled? false) + (reset! busy? false) + (reset! submitting false) + (let [subscription + (->> (oauth-request :get "request" {:request handle}) + (rx/subs! + (fn [{:keys [status body]}] + (case status + 200 (reset! details body) + 401 (reset! login-params (prepare-login!)) + (reset! failed? true))) + (fn [_] (reset! failed? true))))] + #(rx/dispose! subscription))) + + [:main {:class (stl/css :page)} + (if @login-params + [:> login-register-modal* + {:params @login-params + :on-register #(st/emit! (rt/reload true)) + :on-close (fn [] + (cancel-login!) + (reset! login-params nil) + (reset! cancelled? true))}] + [:section {:class (stl/css :card) :aria-labelledby "mcp-approval-title"} + [:> heading* {:level 1 :typography typography/title-large :id "mcp-approval-title"} + (tr "mcp.oauth.title")] + (cond + @cancelled? + [:> text* {:typography typography/body-large :role "status"} + (tr "mcp.oauth.cancelled")] + + @failed? + [:> text* {:typography typography/body-large :role "alert"} + (tr "mcp.oauth.failed")] + + (nil? @details) + [:> loader* {:title (tr "labels.loading")}] + + :else + [:* + [:div {:class (stl/css :client)} + (when-let [logo-uri (:logo-uri @details)] + [:> client-logo* {:key logo-uri :uri logo-uri}]) + [:div {:class (stl/css :client-details)} + [:> text* {:typography typography/body-large :class (stl/css :client-text)} + (tr "mcp.oauth.request" (or (:client-name @details) (:client-id @details)))] + [:> text* {:typography typography/body-small :class (stl/css :client-text)} + (:client-id @details)]]] + [:> text* {:typography typography/body-medium} + (tr "mcp.oauth.account" (:email profile))] + [:> text* {:typography typography/body-large} + (tr "mcp.oauth.access")] + [:> text* {:typography typography/body-medium} + (tr "mcp.oauth.enable")] + [:div {:class (stl/css :actions)} + [:> button* {:variant "secondary" :disabled @busy? :on-click #(decide! "deny")} + (tr "labels.cancel")] + [:> button* {:variant "primary" :disabled @busy? :on-click #(decide! "approve")} + (tr "mcp.oauth.approve")]]])])])) diff --git a/frontend/src/app/main/ui/mcp_authorize.scss b/frontend/src/app/main/ui/mcp_authorize.scss new file mode 100644 index 0000000000..64eed408fa --- /dev/null +++ b/frontend/src/app/main/ui/mcp_authorize.scss @@ -0,0 +1,61 @@ +// 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 + +@use "ds/_sizes.scss" as *; + +.page { + display: grid; + place-items: center; + min-block-size: 100%; + padding: var(--sp-xl); + background: var(--color-background-primary); + color: var(--color-foreground-primary); +} + +.card { + display: flex; + flex-direction: column; + gap: var(--sp-l); + max-inline-size: 36rem; + padding: var(--sp-xxl); + border: 1px solid var(--color-background-quaternary); + border-radius: var(--br-8); +} + +.client { + display: flex; + align-items: center; + gap: var(--sp-l); + margin-block-end: var(--sp-s); + overflow-wrap: anywhere; +} + +.client-logo { + inline-size: $sz-48; + block-size: $sz-48; + flex-shrink: 0; + object-fit: contain; + border-radius: var(--br-8); +} + +.client-details { + display: flex; + flex-direction: column; + gap: calc(var(--sp-s) - var(--sp-xxs)); + min-inline-size: 0; +} + +.client-text { + margin: 0; +} + +.actions { + display: flex; + flex-wrap: wrap; + justify-content: flex-end; + gap: var(--sp-m); + margin-block-start: var(--sp-l); +} diff --git a/frontend/src/app/main/ui/routes.cljs b/frontend/src/app/main/ui/routes.cljs index 8096051d40..fa9926911b 100644 --- a/frontend/src/app/main/ui/routes.cljs +++ b/frontend/src/app/main/ui/routes.cljs @@ -33,7 +33,8 @@ "Enabled route names. Query-string routing: the `screen` query param carries the route name (`?screen=¶ms`); it is router-owned and reserved, every other param travels as a plain query param." - (into #{:auth-login + (into #{:mcp-authorize + :auth-login :auth-register :auth-register-validate :auth-register-success diff --git a/frontend/src/app/main/ui/settings/options.cljs b/frontend/src/app/main/ui/settings/options.cljs index c4d711ba79..12ba30ef6a 100644 --- a/frontend/src/app/main/ui/settings/options.cljs +++ b/frontend/src/app/main/ui/settings/options.cljs @@ -49,7 +49,7 @@ (update :lang #(or % "")) (update :theme #(if (= % "default") "dark" - (or % "dark"))))) + (or % theme/default))))) form (fm/use-form :schema schema:options-form :initial initial)] diff --git a/frontend/src/app/main/ui/viewer/login.cljs b/frontend/src/app/main/ui/viewer/login.cljs index e99e7e3383..fca78a1aa4 100644 --- a/frontend/src/app/main/ui/viewer/login.cljs +++ b/frontend/src/app/main/ui/viewer/login.cljs @@ -5,140 +5,13 @@ ;; Copyright (c) KALEIDOS SUBSIDIARY SL (ns app.main.ui.viewer.login - (:require-macros [app.main.style :as stl]) (:require - [app.common.logging :as log] - [app.config :as cf] [app.main.data.modal :as modal] - [app.main.router :as rt] - [app.main.store :as st] - [app.main.ui.auth.login :refer [login-dialog*]] - [app.main.ui.auth.recovery-request :refer [recovery-request-page*]] - [app.main.ui.auth.register :refer [register-methods* register-success-page* - register-validate-form* terms-service-privacy-policy*]] - [app.main.ui.ds.buttons.icon-button :refer [icon-button*]] - [app.main.ui.ds.foundations.assets.icon :as i] - [app.util.dom :as dom] - [app.util.i18n :as i18n :refer [tr]] + [app.main.ui.auth.login-modal :refer [login-register-modal*]] [rumext.v2 :as mf])) -(log/set-level! :warn) - (mf/defc login-register-modal {::mf/register modal/components ::mf/register-as :login-register} [_] - (let [user-email (mf/use-state "") - register-token (mf/use-state "") - - current-section* (mf/use-state :login) - current-section (deref current-section*) - - set-current-section - (mf/use-fn - #(reset! current-section* %)) - - set-section - (mf/use-fn - (fn [event] - (let [section (-> (dom/get-current-target event) - (dom/get-data "value") - (keyword))] - (set-current-section section)))) - - go-back-to-login - (mf/use-fn - #(set-current-section :login)) - - main-section (or - (= current-section :login) - (= current-section :register) - (= current-section :register-validate)) - close - (fn [event] - (dom/prevent-default event) - (st/emit! (modal/hide))) - - success-email-sent - (fn [email] - (reset! user-email email) - (set-current-section :email-sent)) - - success-login - (fn [] - (.reload js/window.location true)) - - success-register - (fn [data] - (cond - (:invitation-token data) - (st/emit! (rt/nav :auth-verify-token {:token (:invitation-token data)})) - - (:is-active data) - (st/emit! (modal/hide)) - - :else - (do - (reset! user-email (:email data)) - (set-current-section :email-sent))))] - - [:div {:class (stl/css :modal-overlay)} - [:div {:class (stl/css :modal-container)} - [:div {:class (stl/css :modal-header)} - [:h2 {:class (stl/css :modal-header-title)} (tr "labels.continue-with-penpot")] - [:> icon-button* {:variant "ghost" - :class (stl/css :modal-close) - :aria-label (tr "labels.close") - :on-click close - :icon i/close}]] - - [:div {:class (stl/css :modal-content)} - (case current-section - :login - [:div {:class (stl/css :login-form)} - [:> login-dialog* {:on-success-callback success-login - :origin :viewer}] - [:div {:class (stl/css :login-links)} - [:div - [:a {:on-click set-section - :data-value "recovery-request"} - (tr "auth.forgot-password")]] - (when (contains? cf/flags :registration) - [:div - [:span - (tr "auth.register") " "] - [:a {:on-click set-section - :data-value "register"} - (tr "auth.register-submit")]])]] - - :register - [:div {:class (stl/css :login-form)} - [:> register-methods* {:on-success-callback success-register}] - [:div {:class (stl/css :login-links)} - [:div - [:span (tr "auth.already-have-account") " "] - [:a {:on-click set-section - :data-value "login"} - (tr "auth.login-here")]]]] - - :register-validate - [:div {:class (stl/css :login-form)} - [:> register-validate-form* {:params {:token @register-token} - :on-success-callback success-email-sent}] - [:div {:class (stl/css :login-links)} - [:div - [:a {:on-click set-section - :data-value "register"} - (tr "labels.go-back")]]]] - - :recovery-request - [:> recovery-request-page* {:go-back-callback go-back-to-login - :on-success-callback success-email-sent}] - - :email-sent - [:div {:class (stl/css :login-form)} - [:> register-success-page* {:params {:email @user-email}}]]) - - (when main-section - [:div {:class (stl/css :login-links)} - [:> terms-service-privacy-policy*]])]]])) + [:> login-register-modal* {:on-close modal/hide!}]) diff --git a/frontend/src/app/util/theme.cljs b/frontend/src/app/util/theme.cljs index 75c0837d40..b5c0437a6e 100644 --- a/frontend/src/app/util/theme.cljs +++ b/frontend/src/app/util/theme.cljs @@ -17,7 +17,7 @@ (defonce ^:private color-scheme-listeners* (atom #{})) -(def ^:const default "dark") +(def ^:const default "system") (defn get-system-theme [] @@ -47,16 +47,17 @@ (defn resolve-theme "Resolves the profile's theme setting to the effective UI theme ('dark' or - 'light'): 'system' follows the given system theme, 'default' and an unset - theme mean dark, and any other value is taken as-is. Single source of truth - for the app's own theme and the theme reported to plugins." + 'light'). An unset preference or 'system' follows the system theme; + the legacy 'default' preference means dark. Used by the app and plugins." [profile-theme system-theme] - (cond - (= profile-theme "system") system-theme - (= profile-theme "default") "dark" - :else (d/nilv profile-theme "dark"))) + (let [preference (d/nilv profile-theme default)] + (case preference + "system" system-theme + "default" "dark" + preference))) -(defn use-initialize +(defn use-theme + "Return the effective theme and follow system preference changes." [{profile-theme :theme}] (let [system-theme* (mf/use-state get-system-theme) system-theme (deref system-theme*)] @@ -68,6 +69,11 @@ (fn [] (rx/dispose! s)))) - (mf/with-effect [system-theme profile-theme] - (set-color-scheme (resolve-theme profile-theme system-theme)) + (resolve-theme profile-theme system-theme))) + +(defn use-initialize + [profile] + (let [color (use-theme profile)] + (mf/with-effect [color] + (set-color-scheme color) (notify-color-scheme-listeners!)))) diff --git a/frontend/test/frontend_tests/runner.cljs b/frontend/test/frontend_tests/runner.cljs index e23f915520..bdc9193088 100644 --- a/frontend/test/frontend_tests/runner.cljs +++ b/frontend/test/frontend_tests/runner.cljs @@ -131,6 +131,7 @@ [frontend-tests.ui.inspect-stroke-tokens-test] [frontend-tests.ui.jobs-progress-test] [frontend-tests.ui.layout-container-multiple-test] + [frontend-tests.ui.mcp-authorize-test] [frontend-tests.ui.measurements-test] [frontend-tests.ui.measures-menu-props-test] [frontend-tests.ui.organization-team-switch-test] @@ -152,6 +153,7 @@ [frontend-tests.util-simple-math-test] [frontend-tests.util-text-clipboard-test] [frontend-tests.util-text-editor-test] + [frontend-tests.util-theme-test] [frontend-tests.util-webapi-test] [frontend-tests.util-zip-test] [frontend-tests.util.code-gen-markup-svg-wasm-test] @@ -301,6 +303,7 @@ 'frontend-tests.ui.measurements-test 'frontend-tests.ui.measures-menu-props-test 'frontend-tests.ui.organization-team-switch-test + 'frontend-tests.ui.mcp-authorize-test 'frontend-tests.ui.routes-test 'frontend-tests.render-dimensions-test 'frontend-tests.text-editor-paste-guard-test @@ -321,6 +324,7 @@ 'frontend-tests.util-simple-math-test 'frontend-tests.util-text-clipboard-test 'frontend-tests.util-text-editor-test + 'frontend-tests.util-theme-test 'frontend-tests.util-webapi-test 'frontend-tests.util.code-gen-markup-svg-wasm-test 'frontend-tests.util.dom.dnd-test diff --git a/frontend/test/frontend_tests/ui/mcp_authorize_test.cljs b/frontend/test/frontend_tests/ui/mcp_authorize_test.cljs new file mode 100644 index 0000000000..b91e3cbef2 --- /dev/null +++ b/frontend/test/frontend_tests/ui/mcp_authorize_test.cljs @@ -0,0 +1,88 @@ +;; 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 frontend-tests.ui.mcp-authorize-test + (:require + [app.common.uri :as uri] + [app.main.router :as rt] + [app.main.store :as st] + [app.main.ui.mcp-authorize :as oauth] + [app.util.http :as http] + [app.util.storage :as storage] + [beicon.v2.core :as rx] + [cljs.test :as t :include-macros true] + [frontend-tests.helpers.async :as a] + [frontend-tests.helpers.mock :as mock])) + +(t/deftest ^:async approval-uses-form-encoding-and-preserves-response-keys + (let [sent (atom nil)] + (await + (mock/with-mocks* + {http/send! (mock/stub + (fn [request] + (reset! sent request) + (->> (rx/of {:status 200 :body #js {"redirect-uri" "https://client.example/cb?code=abc"}}) + (rx/observe-on :async))))} + (let [response (await (a/->promise + (#'oauth/oauth-request :post "decision" + {:request "opaque-handle" :csrf "a+b&c" :decision "approve"})))] + (t/is (= :post (:method @sent))) + (t/is (= "application/x-www-form-urlencoded" (get-in @sent [:headers "content-type"]))) + (t/is (= {:request "opaque-handle" :csrf "a+b&c" :decision "approve"} + (uri/query-string->map (:body @sent)))) + (t/is (= "https://client.example/cb?code=abc" (get-in response [:body :redirect-uri]))) + (t/is (nil? (get-in response [:body :access_token])))))))) + +(t/deftest ^:async approval-request-preserves-login-required-response + (await + (mock/with-mocks* + {http/send! (mock/stub + (fn [_] + (->> (rx/of {:status 401 :body #js {:error "login_required"}}) + (rx/observe-on :async))))} + (let [response (await (a/->promise (#'oauth/oauth-request :get "request" {:request "opaque-handle"})))] + (t/is (= 401 (:status response))) + (t/is (= "login_required" (get-in response [:body :error]))))))) + +(t/deftest ^:async login-keeps-the-approval-return-url-without-navigation + (let [href "https://penpot.example/?screen=mcp-authorize&request=opaque-handle" + saved (:login-redirect storage/session) + emitted (atom [])] + (try + (await + (mock/with-mocks* + {rt/get-current-href (mock/stub (fn [] href)) + st/emit! (mock/stub (fn [event] (swap! emitted conj event)))} + (let [params (#'oauth/prepare-login!)] + (await (a/settle)) + (t/is (= href (:callback-url params))) + (t/is (= href (:login-redirect storage/session))) + (t/is (empty? @emitted))))) + (finally + (binding [storage/*sync* true] + (if saved + (swap! storage/session assoc :login-redirect saved) + (swap! storage/session dissoc :login-redirect))))))) + +(t/deftest ^:async cancelling-login-clears-only-this-attempts-return-url + (let [href "https://penpot.example/?screen=mcp-authorize&request=opaque-handle" + saved (:login-redirect storage/session)] + (try + (await + (mock/with-mocks* + {rt/get-current-href (mock/stub (fn [] href))} + (#'oauth/prepare-login!) + (#'oauth/cancel-login!) + (t/is (nil? (:login-redirect storage/session))) + (binding [storage/*sync* true] + (swap! storage/session assoc :login-redirect "https://penpot.example/other")) + (#'oauth/cancel-login!) + (t/is (= "https://penpot.example/other" (:login-redirect storage/session))))) + (finally + (binding [storage/*sync* true] + (if saved + (swap! storage/session assoc :login-redirect saved) + (swap! storage/session dissoc :login-redirect))))))) diff --git a/frontend/test/frontend_tests/util_theme_test.cljs b/frontend/test/frontend_tests/util_theme_test.cljs new file mode 100644 index 0000000000..83889bab96 --- /dev/null +++ b/frontend/test/frontend_tests/util_theme_test.cljs @@ -0,0 +1,23 @@ +;; 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 frontend-tests.util-theme-test + (:require + [app.util.theme :as theme] + [cljs.test :as t])) + +(t/deftest unset-theme-follows-system + (doseq [system ["light" "dark"]] + (t/is (= system (theme/resolve-theme nil system))) + (t/is (= system (theme/resolve-theme theme/default system))) + (t/is (= system (theme/resolve-theme "system" system))))) + +(t/deftest explicit-theme-overrides-system + (doseq [system ["light" "dark"]] + (t/is (= "light" (theme/resolve-theme "light" system))) + (t/is (= "dark" (theme/resolve-theme "dark" system))) + ;; Older profiles store the dark preference as "default". + (t/is (= "dark" (theme/resolve-theme "default" system))))) diff --git a/frontend/translations/en.po b/frontend/translations/en.po index 09b305cc19..8d5af490a5 100644 --- a/frontend/translations/en.po +++ b/frontend/translations/en.po @@ -9716,3 +9716,27 @@ msgstr "" #: src/app/main/ui/workspace/sidebar/versions.cljs:462 msgid "workspace.versions.warning.text" msgstr "Autosaved versions will be kept for %s days." + +msgid "mcp.oauth.title" +msgstr "Connect to Penpot MCP" + +msgid "mcp.oauth.request" +msgstr "%s wants to connect to Penpot." + +msgid "mcp.oauth.account" +msgstr "You are signed in as %s." + +msgid "mcp.oauth.access" +msgstr "This client can read and change designs through MCP connections established from Penpot." + +msgid "mcp.oauth.enable" +msgstr "Allowing access enables the MCP integration and creates your MCP token if needed." + +msgid "mcp.oauth.approve" +msgstr "Allow access" + +msgid "mcp.oauth.failed" +msgstr "This request has expired, was already used, or could not be completed. Start the connection again from your MCP client." + +msgid "mcp.oauth.cancelled" +msgstr "Connection cancelled" diff --git a/frontend/translations/es.po b/frontend/translations/es.po index 94ac75a0a6..0e75450d92 100644 --- a/frontend/translations/es.po +++ b/frontend/translations/es.po @@ -9820,3 +9820,9 @@ msgstr "Si quieres aumentar este límite, contáctanos en [support@penpot.app](% #: src/app/main/ui/workspace/sidebar/versions.cljs:462 msgid "workspace.versions.warning.text" msgstr "Los autoguardados duran %s días." + +msgid "mcp.oauth.access" +msgstr "Este cliente puede leer y modificar diseños mediante conexiones MCP establecidas desde Penpot." + +msgid "mcp.oauth.cancelled" +msgstr "Conexión cancelada" diff --git a/manage.sh b/manage.sh index f9a5824bfe..9ef5fb72ad 100755 --- a/manage.sh +++ b/manage.sh @@ -784,13 +784,16 @@ function start-instance { instance-compose "$instance" up -d main - # Wait briefly for main to be reachable; the tmux session lives inside. + # Running does not mean entrypoint setup has finished. Nginx starts only + # after ownership and Cargo setup, so wait for its local health endpoint + # before launching builds as penpot (especially on a fresh container). local container deadline container=$(devenv-main-container "$instance") - deadline=$(( SECONDS + 30 )) - while ! docker inspect -f '{{.State.Running}}' "$container" 2>/dev/null | grep -q true; do + deadline=$(( SECONDS + 120 )) + while ! docker exec "$container" curl --fail --silent --max-time 2 \ + --output /dev/null http://127.0.0.1:8082/stub_status 2>/dev/null; do [[ $SECONDS -ge $deadline ]] && { - echo "[${instance}] main container did not reach Running within 30s" >&2 + echo "[${instance}] main container initialization did not finish within 120s; check docker logs $container" >&2 return 1 } sleep 1 diff --git a/mcp/README.md b/mcp/README.md index 59d60aaf61..b38b60d502 100644 --- a/mcp/README.md +++ b/mcp/README.md @@ -299,6 +299,7 @@ The Penpot MCP server can be configured using environment variables. | `PENPOT_MCP_DEVENV` | Enable Penpot development environment tools in local single-user mode. Set to `true` to enable. | `false` | | `PENPOT_MCP_TOOL_TIMEOUT_S` | Timeout, in seconds, for tool calls dispatched to the Penpot plugin | `120` | | `PENPOT_MCP_EXPORT_SHAPE_MAX_PARALLEL_REQUESTS` | Maximum number of parallel export shape requests (multi-user mode only). | `0` (no limit) | +| `PENPOT_MCP_AUTH_URI` | Public base URL of the MCP authorization service, e.g. `https://penpot.example/api/mcp/oauth`. Set to enable OAuth; requires `--multi-user`. Use HTTPS (HTTP is allowed on loopback for development), without credentials, query parameters, or a fragment. | (unset; OAuth disabled) | | `PENPOT_MCP_REDIS_URI` | Redis connection URI (e.g. `redis://host:6379`) enabling multi-instance horizontal scaling via Redis pub/sub task routing (multi-user mode only). When unset, the server runs in single-instance mode, requiring the plugin and MCP client to connect to the same instance. | (unset) | ### Logging Configuration diff --git a/mcp/packages/server/src/McpAuthentication.test.ts b/mcp/packages/server/src/McpAuthentication.test.ts new file mode 100644 index 0000000000..715e60fdc9 --- /dev/null +++ b/mcp/packages/server/src/McpAuthentication.test.ts @@ -0,0 +1,81 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; +import express from "express"; +import { McpAuthentication } from "./McpAuthentication"; + +test("OAuth is opt-in and requires multi-user mode", () => { + assert.equal(new McpAuthentication(undefined, false).isOAuthEnabled, false); + assert.equal(new McpAuthentication("", true).isOAuthEnabled, false); + assert.throws(() => new McpAuthentication("https://penpot.example/api/mcp/oauth", false), /multi-user/); + for (const uri of [ + "not a URL", + "http://remote.example/auth", + "https://user:secret@example.com/auth", + "https://example.com/auth?x=1", + "https://example.com/auth#fragment", + ]) { + assert.throws(() => new McpAuthentication(uri, true), /PENPOT_MCP_AUTH_URI/); + } + assert.equal(new McpAuthentication("http://localhost:3449/api/mcp/oauth", true).isOAuthEnabled, true); +}); + +async function withEndpoint(uri: string | undefined, run: (base: string) => Promise) { + const auth = new McpAuthentication(uri, true); + const app = express(); + app.all("/mcp", (req, res) => { + const result = auth.authenticate(req, res); + if (!result.isRejected) res.json({ token: result.userToken ?? null }); + }); + const server = app.listen(0, "127.0.0.1"); + await new Promise((resolve) => server.once("listening", resolve)); + const address = server.address(); + assert.ok(address && typeof address !== "string"); + try { + await run(`http://127.0.0.1:${address.port}/mcp`); + } finally { + await new Promise((resolve, reject) => server.close((error) => (error ? reject(error) : resolve()))); + } +} + +test("missing credentials challenge the client only when OAuth is enabled", async () => { + await withEndpoint(undefined, async (base) => { + const response = await fetch(base); + assert.equal(response.status, 200); + assert.deepEqual(await response.json(), { token: null }); + }); + await withEndpoint("https://penpot.example/api/mcp/oauth/", async (base) => { + const response = await fetch(base); + assert.equal(response.status, 401); + assert.equal( + response.headers.get("www-authenticate"), + 'Bearer resource_metadata="https://penpot.example/api/mcp/oauth/resource", scope="mcp"' + ); + assert.equal(response.headers.get("cache-control"), "no-store"); + }); +}); + +test("bearer credentials take precedence and query credentials remain supported", async () => { + for (const uri of [undefined, "https://penpot.example/api/mcp/oauth"]) { + await withEndpoint(uri, async (base) => { + const bearer = await fetch(base + "?userToken=query-token", { + headers: { Authorization: "bEaReR header-token" }, + }); + assert.deepEqual(await bearer.json(), { token: "header-token" }); + const query = await fetch(base + "?userToken=query-token"); + assert.deepEqual(await query.json(), { token: "query-token" }); + }); + } +}); + +test("malformed credentials never fall back to another token", async () => { + await withEndpoint("https://penpot.example/api/mcp/oauth", async (base) => { + for (const header of ["Basic abc", "Bearer", "Bearer two tokens", "Bearer a, Bearer b"]) { + const response = await fetch(base + "?userToken=valid", { headers: { Authorization: header } }); + assert.equal(response.status, 401, header); + assert.deepEqual(await response.json(), { error: "invalid_token" }); + } + for (const query of ["?userToken=a&userToken=b", "?userToken=", "?userToken=a%20b"]) { + assert.equal((await fetch(base + query)).status, 401, query); + } + }); +}); diff --git a/mcp/packages/server/src/McpAuthentication.ts b/mcp/packages/server/src/McpAuthentication.ts new file mode 100644 index 0000000000..6df14f6733 --- /dev/null +++ b/mcp/packages/server/src/McpAuthentication.ts @@ -0,0 +1,120 @@ +import type { Request, Response } from "express"; + +/** + * Result of processing credentials from an MCP HTTP request. + */ +export class McpAuthenticationResult { + /** + * @param userToken the extracted user token, if present + * @param isRejected whether the request was rejected and its HTTP error response sent + */ + constructor( + public readonly userToken: string | undefined, + public readonly isRejected: boolean = false + ) {} +} + +/** + * HTTP credential extraction and optional OAuth discovery for MCP clients. + * + * Tokens retain their existing role of matching requests to Penpot connections. + */ +export class McpAuthentication { + public readonly isOAuthEnabled: boolean; + private readonly resourceMetadataUri?: string; + + /** + * Creates a new MCP authentication handler. + * + * @param authUri the authentication URI from PENPOT_MCP_AUTH_URI, or undefined/empty to disable OAuth + * @param multiUser whether the server is running in multi-user mode + */ + constructor(authUri: string | undefined, multiUser: boolean) { + if (!authUri?.trim()) { + this.isOAuthEnabled = false; + } else { + if (!multiUser) { + throw new Error("PENPOT_MCP_AUTH_URI (which enables OAuth) requires multi-user mode"); + } + + // validate the authentication URI + let uri: URL; + try { + uri = new URL(authUri); + } catch { + throw new Error("PENPOT_MCP_AUTH_URI must be an absolute HTTP(S) URL"); + } + const loopback = ["localhost", "127.0.0.1", "[::1]"].includes(uri.hostname); + if ( + (uri.protocol !== "https:" && !(uri.protocol === "http:" && loopback)) || + uri.username || + uri.password || + uri.search || + uri.hash + ) { + throw new Error( + "PENPOT_MCP_AUTH_URI must use HTTPS (HTTP is allowed on loopback) without credentials, query, or fragment" + ); + } + + this.resourceMetadataUri = uri.href.replace(/\/$/, "") + "/resource"; + this.isOAuthEnabled = true; + } + } + + /** + * Extracts credentials. + * If credentials are absent while OAuth is enabled, sends an authentication error. + */ + public authenticate(req: Request, res: Response): McpAuthenticationResult { + let userToken: string | undefined; + + // check for a Bearer token in the Authorization header + const header = req.headers.authorization; + if (header !== undefined) { + const headerCount = req.rawHeaders.filter( + (_, i) => i % 2 === 0 && req.rawHeaders[i].toLowerCase() === "authorization" + ).length; + const match = /^Bearer +([A-Za-z0-9\-._~+/]+=*)$/i.exec(header); + if (headerCount !== 1 || !match) { + this.rejectOAuth(res, "invalid_token"); + return new McpAuthenticationResult(undefined, true); + } + userToken = match[1]; + } + // check for a userToken query parameter + else if (req.query.userToken !== undefined) { + const value = req.query.userToken; + if (typeof value !== "string" || !/^[A-Za-z0-9\-._~+/]+=*$/.test(value)) { + this.rejectOAuth(res, "invalid_token"); + return new McpAuthenticationResult(undefined, true); + } + userToken = value; + } + + // NOTE: The MCP server does not validate userToken against the backend. + // It uses the token to match MCP requests to a connection established + // from the Penpot webapp. Without that matching connection, requests + // cannot access or modify designs. The user must be logged in to Penpot + // to establish that connection; Penpot login and file permissions thus + // provide the authentication and access control boundary. + + if (this.isOAuthEnabled && !userToken) { + this.rejectOAuth(res); + return new McpAuthenticationResult(undefined, true); + } else { + return new McpAuthenticationResult(userToken); + } + } + + private rejectOAuth(res: Response, error?: "invalid_token"): void { + let challenge = "Bearer"; + if (this.resourceMetadataUri) { + challenge += ` resource_metadata="${this.resourceMetadataUri}", scope="mcp"`; + } + if (error) challenge += `${this.resourceMetadataUri ? "," : ""} error="${error}"`; + res.setHeader("WWW-Authenticate", challenge); + res.setHeader("Cache-Control", "no-store"); + res.status(401).json({ error: error ?? "unauthorized" }); + } +} diff --git a/mcp/packages/server/src/PenpotMcpServer.http.test.ts b/mcp/packages/server/src/PenpotMcpServer.http.test.ts index 2a72ed5305..693e3197ab 100644 --- a/mcp/packages/server/src/PenpotMcpServer.http.test.ts +++ b/mcp/packages/server/src/PenpotMcpServer.http.test.ts @@ -16,6 +16,7 @@ beforeEach(async () => { process.env.PENPOT_MCP_DEVENV = "false"; process.env.PENPOT_MCP_REPL_ENABLE = "false"; delete process.env.PENPOT_MCP_REDIS_URI; + delete process.env.PENPOT_MCP_AUTH_URI; server = new PenpotMcpServer(true); baseUrl = `http://127.0.0.1:${server.port}`; await server.start(); @@ -26,7 +27,7 @@ afterEach(async () => { process.env = previousEnv; }); -async function modernRequest(method: string, params: Record = {}, query = "") { +async function modernRequest(method: string, params: Record = {}, query = "", bearer?: string) { return fetch(`${baseUrl}/mcp${query}`, { method: "POST", headers: { @@ -34,6 +35,7 @@ async function modernRequest(method: string, params: Record = { Accept: "application/json, text/event-stream", "MCP-Protocol-Version": "2026-07-28", "Mcp-Method": method, + ...(bearer ? { Authorization: `Bearer ${bearer}` } : {}), ...(typeof params.name === "string" ? { "Mcp-Name": params.name } : {}), }, body: JSON.stringify({ @@ -93,7 +95,8 @@ test("isolates user tokens across overlapping tool calls with the same request I name: "execute_code", arguments: { code: "return 1;" }, }, - `?userToken=${token}` + token === "alice" ? "" : `?userToken=${token}`, + token === "alice" ? token : undefined ); const body = (await response.json()) as { result: CallToolResult }; assert.equal(response.status, 200, JSON.stringify(body)); @@ -135,3 +138,30 @@ test("supports older Streamable HTTP clients without allocating a session", asyn await client.close(); } }); + +test("OAuth challenges missing credentials and accepts bearer and query tokens", async () => { + await server.stop(); + process.env.PENPOT_MCP_AUTH_URI = "https://penpot.example/api/mcp/oauth"; + server = new PenpotMcpServer(true); + await server.start(); + + const missing = await modernRequest("tools/list"); + assert.equal(missing.status, 401); + assert.match(missing.headers.get("www-authenticate") ?? "", /resource_metadata=/); + + for (const useBearer of [true, false]) { + const client = new Client({ name: "oauth-test", version: "1" }); + const transport = new StreamableHTTPClientTransport( + new URL(`${baseUrl}/mcp${useBearer ? "" : "?userToken=alice"}`), + useBearer ? { requestInit: { headers: { Authorization: "Bearer alice" } } } : undefined + ); + try { + await client.connect(transport); + const result = await client.listTools(); + const tool = result.tools.find((tool) => tool.name === "execute_code"); + assert.ok(tool, "authenticated clients can discover execute_code"); + } finally { + await client.close(); + } + } +}); diff --git a/mcp/packages/server/src/PenpotMcpServer.ts b/mcp/packages/server/src/PenpotMcpServer.ts index 73ffd83a9f..2ec5cbe146 100644 --- a/mcp/packages/server/src/PenpotMcpServer.ts +++ b/mcp/packages/server/src/PenpotMcpServer.ts @@ -22,6 +22,7 @@ import { ReadTaigaIssueTool } from "./tools/ReadTaigaIssueTool"; import { NreplClient } from "./NreplClient"; import { ReplServer } from "./ReplServer"; import { ApiDocs } from "./ApiDocs"; +import { McpAuthentication } from "./McpAuthentication"; /** * Session context for request-scoped data. @@ -111,6 +112,7 @@ export class PenpotMcpServer { private app!: Express; private httpServer?: HttpServer; private readonly mcpHandler: McpHttpHandler; + private readonly authentication: McpAuthentication; public readonly pluginBridge: PluginBridge; private readonly replServer: ReplServer | null; private apiDocs: ApiDocs; @@ -144,6 +146,7 @@ export class PenpotMcpServer { private readonly tenant: string; constructor(private isMultiUser: boolean = false) { + this.authentication = new McpAuthentication(process.env.PENPOT_MCP_AUTH_URI, isMultiUser); // read port configuration from environment variables this.host = process.env.PENPOT_MCP_SERVER_HOST ?? "localhost"; this.port = parseInt(process.env.PENPOT_MCP_SERVER_PORT ?? "4401", 10); @@ -298,11 +301,16 @@ export class PenpotMcpServer { private setupHttpEndpoints(): void { const handleMcpRequest = toNodeHandler(this.mcpHandler); this.app.all("/mcp", async (req, res) => { - const userToken = req.query.userToken as string | undefined; + const authResult = this.authentication.authenticate(req, res); + if (authResult.isRejected) { + return; + } this.logger.info( - `Received MCP request: method=${req.body?.method ?? ""}; userTokenFp=${PenpotMcpServer.tokenFingerprint(userToken)}` + `Received MCP request: method=${req.body?.method ?? ""}; userTokenFp=${PenpotMcpServer.tokenFingerprint(authResult.userToken)}` + ); + await this.sessionContext.run({ userToken: authResult.userToken }, () => + handleMcpRequest(req, res, req.body) ); - await this.sessionContext.run({ userToken }, () => handleMcpRequest(req, res, req.body)); }); } @@ -316,6 +324,7 @@ export class PenpotMcpServer { return new Promise((resolve, reject) => { this.httpServer = this.app.listen(this.port, this.host, async () => { this.logger.info(`Multi-user mode: ${this.isMultiUserMode()}`); + this.logger.info(`OAuth enabled: ${this.authentication.isOAuthEnabled}`); this.logger.info( `Multi-instance mode with Redis-backed transport: ${this.redisBridge ? "true" : "false"}` ); diff --git a/mcp/scripts/start-mcp-devenv-oauth b/mcp/scripts/start-mcp-devenv-oauth new file mode 100755 index 0000000000..230dbf4029 --- /dev/null +++ b/mcp/scripts/start-mcp-devenv-oauth @@ -0,0 +1,19 @@ +#!/bin/sh + +# This starts the MCP server in multi-user mode with OAuth enabled, for +# testing OAuth in the Penpot development environment (assuming devenv). +# Agentic tooling expects the single-user server from start-mcp-devenv. + +set -e + +if [ -z "$PENPOT_PUBLIC_URI" ]; then + echo "PENPOT_PUBLIC_URI is not set; run this inside the devenv container." >&2 + exit 1 +fi + +PENPOT_MCP_AUTH_URI="${PENPOT_PUBLIC_URI%/}/api/mcp/oauth" + +echo "OAuth authorization URI: $PENPOT_MCP_AUTH_URI" +echo "MCP endpoint for clients: ${PENPOT_PUBLIC_URI%/}/mcp/stream" + +PENPOT_MCP_SERVER_HOST=0.0.0.0 PENPOT_MCP_REMOTE_MODE=true PENPOT_MCP_AUTH_URI="$PENPOT_MCP_AUTH_URI" pnpm run bootstrap:multi-user