penpot/backend/test/backend_tests/rpc_binfile_test.clj
Dr. Dominik Jain e20a0d748e
✨ Add OAuth support for MCP server (#12480)
* 🎉 Add OAuth support for remote MCP connections

Add MCP-specific discovery, client registration, and approval endpoints.
Exchange single-use authorization codes with PKCE for the fixed MCP user
token, enabling MCP and creating the token on approval when needed.

Use self-contained client IDs without new database tables or stored client
records. Keep temporary authorization state in Redis/Valkey.

Accept bearer credentials while preserving query-parameter connections.
Enable OAuth through PENPOT_MCP_AUTH_URI in multi-user mode. Document the
environment variable in the README and implementation details in Serena.

Explain protocol and concurrency decisions in the Clojure code. Cover token
provisioning, OAuth state, client registration, HTTP flows, and the approval
page with tests.

AI-assisted-by: gpt-6

* 🐛 Wait for devenv initialization before starting builds

Docker reports a container as Running as soon as its main process starts,
while the entrypoint may still be preparing the development environment.
manage.sh used that status to launch tmux jobs, allowing frontend builds
to race with ownership changes to /opt/rustup and Cargo setup.

On a fresh container, rustup could fail with Permission denied before the
frontend watcher started. Nginx would later serve the existing JavaScript,
leaving source changes absent from the running app despite restarting
devenv.

Wait for nginx's local health endpoint before launching the jobs. Nginx
starts after entrypoint setup, so its response confirms that setup has
finished. Allow up to 120 seconds and report a timeout without starting
the jobs if initialization does not complete.

Validation: shell syntax and whitespace checks pass.

AI-assisted-by: gpt-6

* ♻️ Share the login modal with MCP authorization

Extract the viewer login modal and its styles into a shared auth
component, retaining the viewer wrapper and its close behavior.

Use the modal on the MCP approval page and preserve the pending request
through login. Closing it clears the saved return URL and shows a
cancellation message without further actions.

Cover return URL handling and cancellation with frontend tests.

AI-assisted-by: gpt-6

* ✨ Show client icons in MCP authorization

Preserve HTTPS logo URLs from client registration and published metadata
and show the icon beside the application name on the approval page.
Ignore unusable URLs and hide failed images without blocking approval.

Update the access description to refer to MCP connections established
from Penpot. Cover logo metadata and approval responses in backend tests.

AI-assisted-by: gpt-6

* 🐛 Follow the system theme when no preference is configured

Default to the system theme in the shared resolver used by the app and
plugins. Preserve explicit light and dark choices, including the legacy
default value that means dark.

Reuse the reactive theme hook in progress widgets and show the system
preference in settings for accounts without a configured theme.

Verify preference resolution with regression tests and browser checks.
All 1,474 frontend tests pass.

AI-assisted-by: gpt-6

* 🐛 Replace expired MCP tokens during OAuth approval

Settings can give an MCP token an expiry date. OAuth approval reused
the newest MCP token without checking it, so an expired token reached
the client and never matched a plugin connection.

Approval now treats an expired token as missing and creates a new one.
Token exchange rejects a code whose token expired after approval with
invalid_grant.

AI-assisted-by: claude-opus-5-5

* 🐛 Log unexpected errors on MCP OAuth endpoints

The OAuth routes sit outside the shared HTTP middleware, so the shared
error handler never saw them, and their own handler turned unexpected
failures into a 503 without logging anything. Failures in the session
middleware of the approval endpoints had no handler at all.

Unexpected failures now also go through the shared error handler for
its logging, while clients still get an OAuth error body. The OAuth
error handler also wraps the session middleware of the approval
endpoints.

AI-assisted-by: claude-opus-5-5

* 🔧 Add devenv script to run the MCP server with OAuth

start-mcp-devenv-oauth starts the MCP server in multi-user mode with
PENPOT_MCP_AUTH_URI derived from PENPOT_PUBLIC_URI, to test OAuth in
the devenv. Agentic tooling still uses the single-user
start-mcp-devenv.

AI-assisted-by: claude-opus-5-5

* 🐛 Fix failing test case

---------

Co-authored-by: alonso.torres <alonso.torres@kaleidos.net>
2026-10-28 18:40:04 +01:00

227 lines
9.5 KiB
Clojure

;; This Source Code Form is subject to the terms of the Mozilla Public
;; License, v. 2.0. If a copy of the MPL was not distributed with this
;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
;;
;; Copyright (c) KALEIDOS SUBSIDIARY SL
(ns backend-tests.rpc-binfile-test
(:require
[app.binfile.common :as bfc]
[app.binfile.v3 :as bf.v3]
[app.common.schema :as sm]
[app.common.uuid :as uuid]
[app.config :as cf]
[app.db :as db]
[app.rpc :as-alias rpc]
[app.rpc.commands.binfile :as binfile]
[app.storage.tmp :as tmp]
[backend-tests.helpers :as th]
[clojure.test :as t]
[datoteka.fs :as fs]
[datoteka.io :as io]
[mockery.core :refer [with-mocks]]))
(t/use-fixtures :once th/state-init)
(t/use-fixtures :each th/database-reset)
(t/deftest import-binfile-schema-omits-file-id
;; N1-06: file-id parameter must be removed from schema for security
(let [schema @#'binfile/schema:import-binfile
validator (sm/lazy-validator schema)
valid-params
{:name "test"
:project-id (uuid/random)
:version 3
:upload-id (uuid/random)}
params-with-file-id
(assoc valid-params :file-id (uuid/random))]
(t/is (true? (validator valid-params))
"params without file-id should be valid")
(t/is (not (contains? (sm/keys (second schema)) :file-id))
"file-id should not be a declared parameter")
;; Params with file-id should fail (schema closed)
(t/is (false? (validator params-with-file-id))
"params with file-id should be rejected")))
(t/deftest import-binfile-schema-accepts-path-object
;; Documents the schema hole: a server-side Path object in :file passes
;; validation on shape alone. The multipart guard in the handler (not the
;; schema) is what rejects client-forged paths.
(let [schema @#'binfile/schema:import-binfile
validator (sm/lazy-validator schema)]
(t/is (true? (validator {:name "pwned"
:project-id (uuid/random)
:version 3
:file {:filename "evil.zip"
:size 123
:path (fs/path "/etc/passwd")}})))))
(t/deftest import-binfile-rejects-forged-file-without-multipart
;; Path-traversal PoC at command level: a transit body like
;; {"~:file": {"~:path": ["~#path", "/etc/passwd"]}} decodes to this exact
;; map. Without a multipart request it must fail at params validation,
;; before any file is read.
(let [prof (th/create-profile* 1)
out (th/command! {::th/type :import-binfile
::rpc/profile-id (:id prof)
:name "pwned"
:version 3
:project-id (:default-project-id prof)
:file {:filename "evil.zip"
:size 123
:path (fs/path "/etc/passwd")}})]
(t/is (th/ex-of-code? (:error out) :params-validation))))
(t/deftest import-binfile-rejects-forged-file-with-forged-flag
;; The multipart flag is server-side: smuggling it in the body must not
;; bypass the guard (prepare-rpc-params overwrites it from the request).
(let [prof (th/create-profile* 1)
out (th/command! {::th/type :import-binfile
::rpc/profile-id (:id prof)
:app.rpc/is-multipart true
:name "pwned"
:version 3
:project-id (:default-project-id prof)
:file {:filename "evil.zip"
:size 123
:path (fs/path "/etc/passwd")}})]
(t/is (th/ex-of-code? (:error out) :params-validation))))
(t/deftest import-binfile-schema-rejects-unsupported-version
;; T1-N2-03: version parameter should be restricted to supported values (1 or 3)
(let [schema @#'binfile/schema:import-binfile
validator (sm/lazy-validator schema)
base-params {:name "test"
:project-id (uuid/random)
:upload-id (uuid/random)}]
;; Version 1 should be accepted
(t/is (true? (validator (assoc base-params :version 1)))
"version 1 should be valid")
;; Version 3 should be accepted
(t/is (true? (validator (assoc base-params :version 3)))
"version 3 should be valid")
;; Version 2 should be rejected
(t/is (false? (validator (assoc base-params :version 2)))
"version 2 should be rejected")
;; Version 0 should be rejected
(t/is (false? (validator (assoc base-params :version 0)))
"version 0 should be rejected")
;; Negative version should be rejected
(t/is (false? (validator (assoc base-params :version -1)))
"negative version should be rejected")
;; Version 4 should be rejected
(t/is (false? (validator (assoc base-params :version 4)))
"version 4 should be rejected")))
(t/deftest import-binfile-audit-events-carry-entity-ids
;; The imported files are created inside the importer, so the command emits a
;; create-file event per file for a local projection.
(with-mocks [audit-mock {:target 'app.loggers.audit/submit :return nil}]
(with-redefs [cf/flags (conj cf/flags :audit-log)]
(let [profile (th/create-profile* 1 {:is-active true})
team (th/create-team* 51 {:profile-id (:id profile)})
project (th/create-project* 51 {:profile-id (:id profile)
:team-id (:id team)})
;; A real row: `submit-file-created-event` reads the light
;; columns back, so a phantom id would emit no event.
file (th/create-file* 99 {:profile-id (:id profile)
:project-id (:id project)})
file-id (:id file)]
(with-redefs [bf.v3/import-files! (fn [_cfg] {:file-ids [file-id]})]
(#'binfile/import-binfile
th/*system*
{::rpc/profile-id (:id profile)
:profile-id (:id profile)
:project-id (:id project)
:version 3
:name "imported"
:file {:filename "package.penpot"
:path (th/tempfile "backend_tests/test_files/svg-attrs-camel-case.penpot")
:mtype "application/zip"
:size 1}}))
(let [events (->> (:call-args-list @audit-mock)
(map second)
(filter #(= "create-file" (:name %))))]
(t/is (= 1 (count events)))
(t/is (= file-id (get-in (first events) [:props :id])))
(t/is (= (:id project) (get-in (first events) [:props :project-id])))
(t/is (= (:id team) (get-in (first events) [:props :team-id]))))))))
(defn- export-file
[profile]
(let [file (th/create-file* 1 {:profile-id (:id profile)
:project-id (:default-project-id profile)
:is-shared false})
output (tmp/tempfile :suffix ".zip")]
(bf.v3/export-files!
(-> th/*system*
(assoc ::bfc/ids #{(:id file)})
(assoc ::bfc/embed-assets false)
(assoc ::bfc/include-libraries false))
(io/output-stream output))
output))
(defn- import-file
[profile path]
(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))))
(defn- import-events
[mock]
(->> (:call-args-list @mock)
(map second)
(filter #(= "import-binfile" (:name %)))))
(t/deftest import-binfile-audit-event-records-file-ids
(with-mocks [mock {:target 'app.loggers.audit/submit :return nil}]
(let [profile (th/create-profile* 1 {:is-active true})
events (import-file profile (export-file profile))
file-ids (-> events last second :file-ids)
[event :as audit-events] (import-events mock)]
(t/is (= :end (-> events last first)))
(t/is (= 1 (count audit-events)))
(t/is (= 1 (count file-ids)))
(t/is (= (vec file-ids) (-> event :props :file-ids)))
(t/is (= 1 (-> event :props :file-count)))
(t/is (= "completed" (-> event :props :status)))
(t/is (= (:default-project-id profile) (-> event :props :project-id)))
(t/is (= "penpot" (-> event :props :referer)))
(t/is (= (:id profile) (:profile-id event)))
(t/is (= "command" (:type event)))
(t/is (not (contains? (:props event) :file))))))
(t/deftest import-binfile-audit-event-records-failure
(with-mocks [mock {:target 'app.loggers.audit/submit :return nil}]
(let [profile (th/create-profile* 1 {:is-active true})
path (export-file profile)
events (with-redefs [bf.v3/import-files!
(fn [_] (throw (ex-info "boom" {:type :internal})))]
(import-file profile path))
[event :as audit-events] (import-events mock)]
(t/is (= :error (-> events last first)))
(t/is (= 1 (count audit-events)))
(t/is (= "failed" (-> event :props :status)))
(t/is (not (contains? (:props event) :file-ids))))))