mirror of
https://github.com/penpot/penpot.git
synced 2026-11-05 01:20:38 +00:00
✨ Restrict management RPC methods to their declared shared keys (#12736)
* ✨ Restrict management RPC methods to their shared keys Management methods now declare the shared-key ids allowed to call them with ::rpc/shared-keys. A wrapper in wrap-management checks the key id from the request before opening a transaction, so a valid key outside the declared set answers 403 shared-key-not-allowed. Methods without the metadata keep accepting any configured key. Scope upload-tempfile to exporter and media-processor, file analysis to nexus, durable jobs to exporter, and the Nitrate methods to admin-console. The test helper injects a declared key for the allowed path, and a coverage test fails when a management method is missing or declares the wrong set. Closes #12730 AI-assisted-by: deepseek-v4.1-flash * ♻️ Harden shared-key authorization scope and tests Rename the per-method metadata to ::rpc/allowed-shared-keys so it reads as the allowed key ids, not the secrets. Fix the management test helper to resolve the key from the supplied request and to keep an explicit nil, which prevents a rejection test from silently using an allowed key. Strengthen coverage: assert every management namespace is scanned, check all four job methods reject a key they do not declare, verify a rejected complete-job leaves the row and its events untouched, and exercise both keys allowed on upload-tempfile. Log denied keys with the method name and key id, without the secret, and update the audit-log memory for push-audit-events. Closes #12730 AI-assisted-by: deepseek-v4.1-flash
This commit is contained in:
parent
43f3bda8b5
commit
41654aa1b7
@ -182,7 +182,7 @@ Who uses it:
|
||||
|
||||
Failure policy: every send is best-effort. A failure never rolls back the business action; it is only recorded for debugging.
|
||||
|
||||
Backend side (`backend/src/app/rpc/management/nitrate.clj` `::push-audit-events`): `::audit/skip`, `::rpc/auth false` (trust comes from the shared key, not from a user session). It builds context with `audit/prepare-context-from-request` + `request-id`, takes the given `profile-id` as-is (no coercion here, must already be a uuid), sets `tracked-at = request-at`, defaults `type` to `"action"`, and calls `audit/submit` (which defaults `source` to `"backend"`). So these rows land as **`source = backend`**: they are audited like backend events because a trusted service set the author.
|
||||
Backend side (`backend/src/app/rpc/management/nitrate.clj` `::push-audit-events`): `::audit/skip`, `::rpc/auth false` (trust comes from the shared key, not from a user session; only the `admin-console` key id is allowed). It builds context with `audit/prepare-context-from-request` + `request-id`, takes the given `profile-id` as-is (no coercion here, must already be a uuid), sets `tracked-at = request-at`, defaults `type` to `"action"`, and calls `audit/submit` (which defaults `source` to `"backend"`). So these rows land as **`source = backend`**: they are audited like backend events because a trusted service set the author.
|
||||
|
||||
### 7b. Browser channel (best-effort, like frontend events)
|
||||
|
||||
@ -301,7 +301,7 @@ So the full journey is: Penpot buffer → Nexus `audit_log` (via either archive
|
||||
|
||||
Penpot → admin-console archiving lives in its own job (`:audit-log-consumer-admin-console`, consumer `2`, flag `:admin-console`, params closed empty): it drains its own pending queue and ingests allowlisted events through `nitrate/call :ingest-audit-log`, enriched as described above. What also exists (do not confuse with archiving):
|
||||
|
||||
- **Nitrate → Penpot**: `POST /api/management/methods/push-audit-events` (`backend/src/app/rpc/management/nitrate.clj`) lets the trusted Nitrate backend inject events into the Penpot audit log (no RPC auth, `::audit/skip`, request context + `submit`). Section 7a is the main user.
|
||||
- **Nitrate → Penpot**: `POST /api/management/methods/push-audit-events` (`backend/src/app/rpc/management/nitrate.clj`) lets the trusted Nitrate backend inject events into the Penpot audit log (no RPC auth, `::audit/skip`, request context + `submit`; only the `admin-console` shared-key id can call it). Section 7a is the main user.
|
||||
- **Admin-console browser → collector**: usage events from the admin-console UI traveling through the normal public `push-audit-events` (section 7b).
|
||||
- **Penpot frontend Nitrate bits**: membership/subscription events (`frontend/src/app/main/data/nitrate.cljs`, `nitrate-audit/*`) through the same public endpoint.
|
||||
|
||||
|
||||
@ -14,6 +14,7 @@
|
||||
- Invitation tokens are verified through token issuers and only accepted when the token member id/email matches the authenticated profile; otherwise login proceeds without consuming the invitation.
|
||||
- When public registration is disabled, invitation-based password registration also requires a matching, non-expired `team_invitation` row; registration revalidates it under a row lock before creating the profile.
|
||||
- Access tokens (`app.http.access-token`): personal tokens have JWT issuer `access-token` and DB type other than `mcp`; MCP tokens have issuer `urn:penpot:mcp-token` (`actoken/mcp-token-issuer`) and DB type `mcp`. Authentication requires issuer and type to agree, and the claim `uid` to match the row's profile; other issuers and undecryptable tokens are treated as anonymous.
|
||||
- Management RPC (`/api/management/methods/:name`) is shared-key only. The middleware authenticates `<id> <secret>`; the RPC wrapper `wrap-shared-key-authorization` authorizes the key id against the method's `::rpc/allowed-shared-keys` set. Absent metadata accepts any configured key; all current methods declare one (`exporter`/`media-processor` for `upload-tempfile`, `nexus` for file analysis, `exporter` for jobs, `admin-console` for Nitrate). A valid key outside the set answers 403 `:shared-key-not-allowed`; denied calls do not reach the audit wrapper.
|
||||
- Personal tokens need flag `:access-tokens`. MCP tokens need `:mcp` or `:access-tokens`, and only on routes whose `actoken/authz` gets `::actoken/allow-mcp true` (the RPC routes); elsewhere (e.g. assets) they are anonymous.
|
||||
- On RPC, an MCP token reaches only commands with `::rpc/token-type "mcp"` (`wrap-authentication` in `app.rpc`); every other command answers 403 `:token-auth-required`. A command with that metadata also rejects sessions and personal tokens. Currently only `send-mcp-feedback`. Blocking a profile invalidates sessions, not tokens, so such commands check `:is-blocked`/`:is-active` themselves.
|
||||
- HTTP/session parsing details such as cookie/header precedence, JWT session token versions, and SameSite behavior are in `mem:backend/subtleties`.
|
||||
|
||||
@ -56,7 +56,7 @@
|
||||
- Request body parsing is mostly POST-oriented and supports Transit JSON plus plain JSON. Plain JSON request keys are kebab-decoded before being merged into `:params`. A body the parser cannot read is a client error, classified at the parse site and never by exception class (`clojure.data.json` signals its syntax errors with a bare `java.lang.Exception`): 400 `:validation`/`:malformed-json`, one `warn` line (`malformed request body`) carrying path, peer address and content-type instead of a stack trace, and `penpot_http_malformed_requests_total`. `RequestTooBigException` keeps its 413.
|
||||
- Response formatting negotiates with `Accept` or `_fmt=json`. Transit is the default for collection/boolean bodies; JSON encoding has special pointer-map handling.
|
||||
- Auth prefers the session cookie token before the `Authorization` header. Headers may be `Token` or `Bearer`. Only `kid=1`/`ver=1` tokens are decoded as session tokens; anything else is left unauthenticated (legacy v1 tokens were removed).
|
||||
- Shared-key auth requires `x-shared-key` as `<key-id> <key>` and stores the lowercased key id on the request. If no shared keys are configured it always rejects.
|
||||
- Shared-key auth requires `x-shared-key` as `<key-id> <key>` and stores the lowercased key id on the request. If no shared keys are configured it always rejects. Management methods declare the key ids they accept with `::rpc/allowed-shared-keys`; a valid key outside that set answers 403 `:shared-key-not-allowed`, and a method without the metadata accepts any configured key.
|
||||
- Session management uses DB storage unless the DB pool is read-only, then falls back to the in-memory manager. Sessions use only the v2 UUID model (`http_session_v2`); legacy string ids were removed.
|
||||
- Session cookies are renewed when `modified-at` is older than the 6h renewal interval. SameSite is `none` for CORS, otherwise strict/lax based on config. Session lifetime config and GC: `mem:backend/session-expiration`.
|
||||
|
||||
|
||||
@ -366,6 +366,28 @@
|
||||
(wrap-params-validation cfg $ mdata)
|
||||
(wrap-authentication cfg $ mdata)))
|
||||
|
||||
(defn- wrap-shared-key-authorization
|
||||
"Authorizes a shared-key caller against the method's `::allowed-shared-keys`.
|
||||
|
||||
A management method declares the shared-key ids allowed to call it with
|
||||
`::allowed-shared-keys`. When the metadata is absent any configured shared key is
|
||||
accepted, preserving the previous behavior. When it is present and empty no
|
||||
key is accepted."
|
||||
[_ f mdata]
|
||||
(if (contains? mdata ::allowed-shared-keys)
|
||||
(let [allowed-keys (::allowed-shared-keys mdata)]
|
||||
(fn [cfg params]
|
||||
(if (contains? allowed-keys (::auth-key-id params))
|
||||
(f cfg params)
|
||||
(do
|
||||
(l/wrn :hint "shared key is not allowed for this method"
|
||||
:handler-name (::handler-name params)
|
||||
:key-id (::auth-key-id params))
|
||||
(ex/raise :type :authorization
|
||||
:code :shared-key-not-allowed
|
||||
:hint "shared key is not allowed for this method")))))
|
||||
f))
|
||||
|
||||
(defn- wrap-management
|
||||
[cfg f mdata]
|
||||
(as-> f $
|
||||
@ -376,7 +398,10 @@
|
||||
(wrap-audit cfg $ mdata)
|
||||
(wrap-nitrate-sso cfg $ mdata)
|
||||
(wrap-params-validation cfg $ mdata)
|
||||
(wrap-authentication cfg $ mdata)))
|
||||
(wrap-authentication cfg $ mdata)
|
||||
;; Outermost on purpose: a denied key never opens a transaction,
|
||||
;; validates params or runs the method body.
|
||||
(wrap-shared-key-authorization cfg $ mdata)))
|
||||
|
||||
(defn- process-method
|
||||
[cfg wrap-fn [f mdata]]
|
||||
|
||||
@ -28,11 +28,16 @@
|
||||
schema:upload-tempfile-result
|
||||
[:map {:title "upload-templfile-result"}])
|
||||
|
||||
(def ^:private allowed-shared-key-ids
|
||||
"The exporter uploads its artifacts; the media worker uploads its results."
|
||||
#{:exporter :media-processor})
|
||||
|
||||
(sv/defmethod ::upload-tempfile
|
||||
{::doc/added "2.12"
|
||||
::sm/params schema:upload-tempfile-params
|
||||
::rpc/multipart-only-params [:content]
|
||||
::sm/result schema:upload-tempfile-result}
|
||||
::sm/result schema:upload-tempfile-result
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids}
|
||||
[cfg {:keys [::rpc/profile-id content]}]
|
||||
(media.v/validate-media-type! content cm/tempfile-types)
|
||||
(let [storage (sto/resolve cfg)
|
||||
|
||||
@ -28,6 +28,10 @@
|
||||
|
||||
(def projection-version 1)
|
||||
|
||||
(def ^:private allowed-shared-key-ids
|
||||
"Only the nexus file-security pipeline reads file projections."
|
||||
#{:nexus})
|
||||
|
||||
;; Bounds for the data that crosses to nexus on every sweep. The
|
||||
;; projection stays small by construction; anything past the caps sets
|
||||
;; `:truncated?` instead of growing the payload.
|
||||
@ -220,7 +224,8 @@
|
||||
{::doc/added "2.20"
|
||||
::sm/params schema:get-file-for-analysis-params
|
||||
::sm/result schema:get-file-for-analysis-result
|
||||
::rpc/auth false}
|
||||
::rpc/auth false
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids}
|
||||
[cfg {:keys [file-id]}]
|
||||
;; `realize?` loads the whole object tree eagerly: the projection
|
||||
;; walks every shape, so lazy pointers (no `load-fn` bound here)
|
||||
|
||||
@ -10,11 +10,12 @@
|
||||
the ledger. The methods are family-agnostic (they operate on any row
|
||||
of the `job` table) and use the plain JSON schemas of `app.jobs`.
|
||||
|
||||
Auth: every method declares ::rpc/auth false because these endpoints
|
||||
do not require a profile. Actual authentication is enforced by the
|
||||
management route resolver (resolve-management-methods in app.rpc),
|
||||
which mandates a valid shared-key before dispatching to these methods.
|
||||
If the route resolver changes, these methods must be updated accordingly."
|
||||
Auth: these endpoints take no profile, so every method declares
|
||||
::rpc/auth false. The route applies mw/shared-key-auth
|
||||
(app.http.middleware) to authenticate the `<id> <secret>` header, and
|
||||
wrap-management applies wrap-shared-key-authorization (app.rpc) to
|
||||
check the key id against ::rpc/allowed-shared-keys. If the route
|
||||
middleware or wrap-management changes, revisit these methods."
|
||||
(:require
|
||||
[app.common.exceptions :as ex]
|
||||
[app.common.schema :as sm]
|
||||
@ -24,6 +25,10 @@
|
||||
[app.rpc.doc :as doc]
|
||||
[app.util.services :as sv]))
|
||||
|
||||
(def ^:private allowed-shared-key-ids
|
||||
"Only the exporter worker claims and settles durable jobs today."
|
||||
#{:exporter})
|
||||
|
||||
;; ---- RPC METHOD: CLAIM-JOB
|
||||
|
||||
(def ^:private schema:claim-job-params
|
||||
@ -42,7 +47,8 @@
|
||||
{::doc/added "2.20"
|
||||
::sm/params schema:claim-job-params
|
||||
::sm/result schema:claim-job-result
|
||||
::rpc/auth false} ;; shared-key enforced by route resolver
|
||||
::rpc/auth false
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids}
|
||||
[cfg {:keys [job-id scheduled-at]}]
|
||||
(let [row (jobs/get-job cfg job-id)]
|
||||
;; Race condition: if the job is claimed between get-job and claim! by
|
||||
@ -74,7 +80,8 @@
|
||||
{::doc/added "2.20"
|
||||
::sm/params schema:report-job-progress-params
|
||||
::sm/result schema:report-job-progress-result
|
||||
::rpc/auth false} ;; shared-key enforced by route resolver
|
||||
::rpc/auth false
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids}
|
||||
[cfg {:keys [job-id progress]}]
|
||||
;; A lost race (row already terminal) reports :skip so the worker
|
||||
;; stops retrying a report that can never land, mirroring claim-job.
|
||||
@ -130,7 +137,8 @@
|
||||
{::doc/added "2.20"
|
||||
::sm/params schema:complete-job-params
|
||||
::sm/result schema:complete-job-result
|
||||
::rpc/auth false} ;; shared-key enforced by route resolver
|
||||
::rpc/auth false
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids}
|
||||
[cfg {:keys [job-id result resource-id]}]
|
||||
(if (pos? (jobs/complete cfg
|
||||
:job-id job-id
|
||||
@ -156,7 +164,8 @@
|
||||
{::doc/added "2.20"
|
||||
::sm/params schema:fail-job-params
|
||||
::sm/result schema:fail-job-result
|
||||
::rpc/auth false} ;; shared-key enforced by route resolver
|
||||
::rpc/auth false
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids}
|
||||
[cfg {:keys [job-id error]}]
|
||||
(if (pos? (jobs/fail cfg job-id error))
|
||||
{:action :run}
|
||||
|
||||
@ -46,6 +46,10 @@
|
||||
[cuerdas.core :as str]))
|
||||
|
||||
|
||||
(def ^:private allowed-shared-key-ids
|
||||
"The Nitrate admin-console is the only caller of this API."
|
||||
#{:admin-console})
|
||||
|
||||
(defn- profile-to-map [profile]
|
||||
{:id (:id profile)
|
||||
:name (:fullname profile)
|
||||
@ -58,6 +62,7 @@
|
||||
(sv/defmethod ::authenticate
|
||||
"Authenticate the current user"
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params [:map]
|
||||
::sm/result schema:profile
|
||||
::nitrate/sso false}
|
||||
@ -105,6 +110,7 @@
|
||||
(sv/defmethod ::get-penpot-version
|
||||
"Get the current Penpot version"
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params [:map]
|
||||
::sm/result schema:get-penpot-version-result
|
||||
::rpc/auth false}
|
||||
@ -124,6 +130,7 @@
|
||||
(sv/defmethod ::get-teams
|
||||
"List teams for which current user is owner"
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params [:map]
|
||||
::sm/result schema:get-teams-result
|
||||
::nitrate/sso false}
|
||||
@ -135,6 +142,7 @@
|
||||
(sv/defmethod ::get-member-teams
|
||||
"List all active teams where the current user is a member"
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params [:map]
|
||||
::sm/result schema:get-member-teams-result
|
||||
::nitrate/sso false}
|
||||
@ -151,6 +159,7 @@
|
||||
(sv/defmethod ::update-profile-theme
|
||||
"Update the current user's theme"
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params schema:update-profile-theme
|
||||
::db/transaction true
|
||||
::nitrate/sso false}
|
||||
@ -171,6 +180,7 @@
|
||||
(sv/defmethod ::update-profile-props
|
||||
"Merge writable properties into the current user's profile"
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params schema:update-profile-props
|
||||
::db/transaction true
|
||||
::nitrate/sso false}
|
||||
@ -193,6 +203,7 @@
|
||||
Accepts an optional previous-id to mark the old logo for garbage
|
||||
collection when replacing an existing one."
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params schema:upload-organization-logo
|
||||
::rpc/multipart-only-params [:content]
|
||||
::sm/result schema:upload-organization-logo-result
|
||||
@ -225,6 +236,7 @@
|
||||
(sv/defmethod ::notify-team-change
|
||||
"Notify to Penpot a team change from nitrate"
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params schema:notify-team-change
|
||||
::rpc/auth false}
|
||||
[cfg team]
|
||||
@ -245,6 +257,7 @@
|
||||
(sv/defmethod ::notify-user-added-to-organization
|
||||
"Notify to Penpot that an user has joined an organization from nitrate"
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params schema:notify-user-added-to-organization
|
||||
::rpc/auth false}
|
||||
[cfg {:keys [profile-id organization-id]}]
|
||||
@ -276,6 +289,7 @@
|
||||
(sv/defmethod ::get-managed-profiles
|
||||
"List profiles that belong to teams for which current user is owner"
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params [:map]
|
||||
::sm/result schema:managed-profile-result
|
||||
::nitrate/sso false}
|
||||
@ -316,6 +330,7 @@
|
||||
(sv/defmethod ::get-teams-summary
|
||||
"Get summary information for a list of teams"
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params schema:get-teams-summary-params
|
||||
::sm/result schema:get-teams-summary-result
|
||||
::nitrate/sso false}
|
||||
@ -437,6 +452,7 @@ RETURNING id, deleted_at;")
|
||||
"For a deleted organization, preserve organization teams and only prefix or delete
|
||||
imported Personal Projects before notifying connected users."
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params schema:notify-organization-deletion
|
||||
::rpc/auth false}
|
||||
[cfg {:keys [organization-id]}]
|
||||
@ -457,6 +473,7 @@ RETURNING id, deleted_at;")
|
||||
"For a given user, find all owned organizations and apply the deleted-organization
|
||||
transfer rules to their imported Personal Projects teams."
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params schema:notify-user-organizations-deletion
|
||||
::nitrate/sso false}
|
||||
[cfg {:keys [profile-id]}]
|
||||
@ -483,6 +500,7 @@ RETURNING id, deleted_at;")
|
||||
(sv/defmethod ::get-profile-by-email
|
||||
"Get profile by email"
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params [:map [:email ::sm/email]]
|
||||
::sm/result schema:profile
|
||||
::nitrate/sso false}
|
||||
@ -507,6 +525,7 @@ RETURNING id, deleted_at;")
|
||||
(sv/defmethod ::get-profile-by-id
|
||||
"Get profile by email"
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params [:map [:id ::sm/uuid]]
|
||||
::sm/result schema:profile
|
||||
::nitrate/sso false}
|
||||
@ -542,6 +561,7 @@ RETURNING id, deleted_at;")
|
||||
(sv/defmethod ::get-organization-member-team-counts
|
||||
"Get the number of non-default teams each profile belongs to within a set of teams."
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params schema:get-organization-member-team-counts-params
|
||||
::sm/result schema:get-organization-member-team-counts-result
|
||||
::rpc/auth false}
|
||||
@ -594,6 +614,7 @@ RETURNING id, deleted_at;")
|
||||
"Get the non-default teams a profile belongs to within a set of teams,
|
||||
with the profile's role in each one."
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params schema:get-organization-member-teams-params
|
||||
::sm/result schema:get-organization-member-teams-result
|
||||
::nitrate/sso false}
|
||||
@ -631,6 +652,7 @@ RETURNING id, deleted_at;")
|
||||
(sv/defmethod ::invite-to-organization
|
||||
"Invite to organization"
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params [:map
|
||||
[:email ::sm/email]
|
||||
[:organization cto/schema:organization-with-avatar]]
|
||||
@ -663,6 +685,7 @@ RETURNING id, deleted_at;")
|
||||
(sv/defmethod ::get-organization-invitations
|
||||
"Get valid invitations for an organization, returning at most one invitation per email."
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params schema:get-organization-invitations-params
|
||||
::sm/result schema:get-organization-invitations-result
|
||||
::nitrate/sso false}
|
||||
@ -691,6 +714,7 @@ RETURNING id, deleted_at;")
|
||||
(sv/defmethod ::delete-organization-invitations
|
||||
"Delete all invitations for one email in an organization scope (organization + organization teams)."
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params schema:delete-organization-invitations-params
|
||||
::nitrate/sso false}
|
||||
[cfg {:keys [organization-id email]}]
|
||||
@ -718,6 +742,7 @@ RETURNING id, deleted_at;")
|
||||
Called from Nitrate when an organization is about to be deleted, so users that click
|
||||
their invitation token hit the existing invalid-token landing page."
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params schema:delete-all-organization-invitations-params
|
||||
::rpc/auth false}
|
||||
[cfg {:keys [organization-id]}]
|
||||
@ -756,6 +781,7 @@ RETURNING id, deleted_at;")
|
||||
(sv/defmethod ::remove-from-organization
|
||||
"Remove an user from an organization"
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params [:map
|
||||
[:profile-id ::sm/uuid]
|
||||
[:organization-id ::sm/uuid]
|
||||
@ -801,6 +827,7 @@ RETURNING id, deleted_at;")
|
||||
"Get a summary of the teams that would be deleted, transferred, or exited
|
||||
if the user were removed from the organization"
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params [:map
|
||||
[:profile-id ::sm/uuid]
|
||||
[:organization-id ::sm/uuid]
|
||||
@ -836,6 +863,7 @@ RETURNING id, deleted_at;")
|
||||
(sv/defmethod ::send-renewal-email
|
||||
"Send an Enterprise subscription renewal notice email to a user."
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params schema:send-renewal-email-params
|
||||
::rpc/auth false}
|
||||
[cfg {:keys [profile-id user-email user-name renewal-date estimated-amount organizations]}]
|
||||
@ -912,6 +940,7 @@ RETURNING id, deleted_at;")
|
||||
(sv/defmethod ::exists-organization-team-invitations-for-non-members
|
||||
"Return if there are any team invitations for emails that are not organization members."
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params schema:organization-team-invitations-for-non-members-params
|
||||
::sm/result schema:exists-organization-team-invitations-for-non-members-result
|
||||
::nitrate/sso false}
|
||||
@ -922,6 +951,7 @@ RETURNING id, deleted_at;")
|
||||
(sv/defmethod ::delete-organization-team-invitations-for-non-members
|
||||
"Delete team invitations for emails that are not organization members."
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params schema:organization-team-invitations-for-non-members-params
|
||||
::db/transaction true
|
||||
::nitrate/sso false}
|
||||
@ -954,6 +984,7 @@ RETURNING id, deleted_at;")
|
||||
events)"
|
||||
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::audit/skip true
|
||||
::sm/params schema:push-audit-events-params
|
||||
::rpc/auth false}
|
||||
@ -1061,6 +1092,7 @@ RETURNING id, deleted_at;")
|
||||
"Get detailed information for all non-deleted teams in an organization,
|
||||
including owner info and project/file/member counts."
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params schema:get-teams-detail-params
|
||||
::sm/result schema:get-teams-detail-result
|
||||
::nitrate/sso false}
|
||||
@ -1091,6 +1123,7 @@ RETURNING id, deleted_at;")
|
||||
(customer-configured), so it is checked against the SSRF blocklist before
|
||||
any outbound request is attempted."
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params cto/schema:nitrate-sso
|
||||
::sm/result schema:check-organization-sso-result
|
||||
::rpc/auth false}
|
||||
@ -1104,6 +1137,7 @@ RETURNING id, deleted_at;")
|
||||
(sv/defmethod ::notify-organization-sso-change
|
||||
"Nitrate notifies that an organization sso values have changed"
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params [:map
|
||||
[:organization-id ::sm/uuid]
|
||||
[:updated-props ::sm/boolean]
|
||||
@ -1163,6 +1197,7 @@ RETURNING id, deleted_at;")
|
||||
email allow-list. Requires the `admin-console-bulk-create-profiles` flag, disabled
|
||||
by default so it is only available on test environments."
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params schema:bulk-create-profiles-params
|
||||
::sm/result schema:bulk-create-profiles-result
|
||||
::rpc/auth false}
|
||||
@ -1197,6 +1232,7 @@ RETURNING id, deleted_at;")
|
||||
(sv/defmethod ::get-air-gapped
|
||||
"Returns whether this Penpot instance runs in air-gapped mode."
|
||||
{::doc/added "2.18"
|
||||
::rpc/allowed-shared-keys allowed-shared-key-ids
|
||||
::sm/params [:map]
|
||||
::sm/result schema:get-air-gapped-result
|
||||
::rpc/auth false}
|
||||
|
||||
@ -547,13 +547,38 @@
|
||||
(method-fn params))))
|
||||
|
||||
(defn management-command!
|
||||
"Invokes a management RPC method.
|
||||
|
||||
Resolves the shared-key id from an explicit `::rpc/auth-key-id`, from
|
||||
`:app.http/auth-key-id` on the data or on the supplied request, and falls
|
||||
back to the first id the method declares. The fallback lets the existing
|
||||
tests exercise the allowed path without repeating the key; a test that
|
||||
checks a rejection declares the key explicitly, including `nil`."
|
||||
[{:keys [::type] :as data}]
|
||||
(let [[_ method-fn] (get-in *system* [:app.rpc/management-methods type])]
|
||||
(let [[mdata method-fn] (get-in *system* [:app.rpc/management-methods type])]
|
||||
(when-not method-fn
|
||||
(ex/raise :type :assertion
|
||||
:code :rpc-method-not-found
|
||||
:hint (str/ffmt "management rpc method '%' not found" (name type))))
|
||||
(let [params (prepare-rpc-params data)]
|
||||
(let [request (-> data meta :app.http/request)
|
||||
key-id (cond
|
||||
(contains? data ::rpc/auth-key-id)
|
||||
(::rpc/auth-key-id data)
|
||||
|
||||
(contains? data :app.http/auth-key-id)
|
||||
(:app.http/auth-key-id data)
|
||||
|
||||
(and (map? request)
|
||||
(contains? request :app.http/auth-key-id))
|
||||
(:app.http/auth-key-id request)
|
||||
|
||||
:else
|
||||
(first (::rpc/allowed-shared-keys mdata)))
|
||||
data (if (or (contains? data ::rpc/auth-key-id)
|
||||
(nil? key-id))
|
||||
data
|
||||
(assoc data ::rpc/auth-key-id key-id))
|
||||
params (prepare-rpc-params data)]
|
||||
(try-on! (method-fn params)))))
|
||||
|
||||
(def multipart-headers
|
||||
|
||||
@ -218,6 +218,14 @@
|
||||
(t/is (some? (:error out)))
|
||||
(t/is (= :not-found (th/ex-type (:error out))))))
|
||||
|
||||
(t/deftest get-file-for-analysis-rejects-a-key-it-does-not-declare
|
||||
(let [out (th/management-command! {::th/type :get-file-for-analysis
|
||||
::rpc/auth-key-id :exporter
|
||||
:file-id (uuid/random)})]
|
||||
(t/is (some? (:error out)))
|
||||
(t/is (= :authorization (th/ex-type (:error out))))
|
||||
(t/is (= :shared-key-not-allowed (th/ex-code (:error out))))))
|
||||
|
||||
;; --- Audit prop coverage (through the commands, into real events)
|
||||
;;
|
||||
;; NOTE: the test system neutralizes `audit/submit`, so no audit row
|
||||
|
||||
@ -13,6 +13,7 @@
|
||||
[app.db :as db]
|
||||
[app.jobs :as jobs]
|
||||
[app.metrics :as-alias mtx]
|
||||
[app.rpc :as-alias rpc]
|
||||
[backend-tests.helpers :as th]
|
||||
[clojure.test :as t]))
|
||||
|
||||
@ -76,6 +77,12 @@
|
||||
id])
|
||||
(mapv #(jobs/decode-progress (:payload %)))))
|
||||
|
||||
(defn- count-job-events
|
||||
[id]
|
||||
(-> (th/db-exec! ["SELECT count(*) AS count FROM job_event WHERE job_id = ?" id])
|
||||
first
|
||||
:count))
|
||||
|
||||
(defn- mgmt
|
||||
[type params]
|
||||
(th/management-command! (assoc params ::th/type type)))
|
||||
@ -134,6 +141,26 @@
|
||||
(let [out (mgmt :claim-job {:job-id (uuid/next)})]
|
||||
(t/is (= :validation (th/ex-type (:error out))))))
|
||||
|
||||
(t/deftest jobs-methods-reject-a-key-they-do-not-declare
|
||||
(doseq [type [:claim-job :report-job-progress :complete-job :fail-job]]
|
||||
(let [out (mgmt type {::rpc/auth-key-id :nexus})]
|
||||
(t/is (= :authorization (th/ex-type (:error out))) (name type))
|
||||
(t/is (= :shared-key-not-allowed (th/ex-code (:error out))) (name type)))))
|
||||
|
||||
(t/deftest complete-job-rejects-a-key-it-does-not-declare-without-side-effects
|
||||
(let [job-id (mk-job {})
|
||||
before (get-row job-id)
|
||||
events-before (count-job-events job-id)
|
||||
out (mgmt :complete-job {::rpc/auth-key-id :nexus
|
||||
:job-id job-id
|
||||
:result {:x 1}})
|
||||
after (get-row job-id)]
|
||||
(t/is (= :authorization (th/ex-type (:error out))))
|
||||
(t/is (= :shared-key-not-allowed (th/ex-code (:error out))))
|
||||
(t/is (= (:status before) (:status after)))
|
||||
(t/is (nil? (:completed-at after)))
|
||||
(t/is (= events-before (count-job-events job-id)))))
|
||||
|
||||
(t/deftest report-job-progress-appends-a-progress-event
|
||||
(let [job-id (mk-job {})
|
||||
_ (jobs/claim (make-cfg) job-id (:scheduled-at (th/db-get :job {:id job-id} :id :scheduled-at)))
|
||||
|
||||
@ -52,6 +52,13 @@
|
||||
(t/is (= :authentication (th/ex-type (:error out))))
|
||||
(t/is (= :authentication-required (th/ex-code (:error out))))))
|
||||
|
||||
(t/deftest authenticate-rejects-a-key-it-does-not-declare
|
||||
(let [out (th/management-command! {::th/type :authenticate
|
||||
::rpc/auth-key-id :exporter})]
|
||||
(t/is (not (th/success? out)))
|
||||
(t/is (= :authorization (th/ex-type (:error out))))
|
||||
(t/is (= :shared-key-not-allowed (th/ex-code (:error out))))))
|
||||
|
||||
(t/deftest create-and-update-organization-invitations-audit-props
|
||||
(let [owner-id-ref (atom nil)]
|
||||
(with-mocks [email-mock {:target 'app.email/send :return nil}
|
||||
|
||||
@ -0,0 +1,49 @@
|
||||
;; 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-management-shared-keys-test
|
||||
"Covers the shared-key scope declared by every management RPC method."
|
||||
(:require
|
||||
[app.rpc :as-alias rpc]
|
||||
[backend-tests.helpers :as th]
|
||||
[clojure.test :as t]))
|
||||
|
||||
(t/use-fixtures :once (t/compose-fixtures
|
||||
(partial th/init-config [:enable-admin-console])
|
||||
th/init-system))
|
||||
|
||||
(def ^:private expected-shared-keys
|
||||
{"app.rpc.management.exporter" #{:exporter :media-processor}
|
||||
"app.rpc.management.files" #{:nexus}
|
||||
"app.rpc.management.jobs" #{:exporter}
|
||||
"app.rpc.management.nitrate" #{:admin-console}})
|
||||
|
||||
(t/deftest every-management-method-declares-its-shared-keys
|
||||
(let [methods (get th/*system* :app.rpc/management-methods)
|
||||
seen (into #{} (map (comp :ns first val)) methods)]
|
||||
(t/is (seq methods))
|
||||
(t/is (= (set (keys expected-shared-keys)) seen)
|
||||
"every management namespace must be scanned")
|
||||
(doseq [[method-name [mdata _]] methods]
|
||||
(let [ns-name (:ns mdata)
|
||||
expected (get expected-shared-keys ns-name)]
|
||||
(t/is (some? expected)
|
||||
(str "unexpected management namespace for " method-name ": " ns-name))
|
||||
(t/is (= expected (::rpc/allowed-shared-keys mdata))
|
||||
(str "wrong ::rpc/allowed-shared-keys for " method-name " in " ns-name))))))
|
||||
|
||||
(t/deftest management-command-uses-the-key-from-the-request-metadata
|
||||
(let [out (th/management-command!
|
||||
(with-meta {::th/type :upload-tempfile}
|
||||
{:app.http/request {:app.http/auth-key-id :nexus}}))]
|
||||
(t/is (= :authorization (th/ex-type (:error out))))
|
||||
(t/is (= :shared-key-not-allowed (th/ex-code (:error out))))))
|
||||
|
||||
(t/deftest management-command-keeps-an-explicit-nil-key
|
||||
(let [out (th/management-command! {::th/type :upload-tempfile
|
||||
::rpc/auth-key-id nil})]
|
||||
(t/is (= :authorization (th/ex-type (:error out))))
|
||||
(t/is (= :shared-key-not-allowed (th/ex-code (:error out))))))
|
||||
@ -82,6 +82,29 @@
|
||||
(t/is (= :validation (th/ex-type (:error out))))
|
||||
(t/is (= :media-type-not-allowed (th/ex-code (:error out))))))
|
||||
|
||||
(t/deftest upload-tempfile-rejects-a-key-it-does-not-declare
|
||||
(let [out (th/management-command! {::th/type :upload-tempfile
|
||||
::rpc/auth-key-id :nexus})]
|
||||
(t/is (some? (:error out)))
|
||||
(t/is (= :authorization (th/ex-type (:error out))))
|
||||
(t/is (= :shared-key-not-allowed (th/ex-code (:error out))))))
|
||||
|
||||
(t/deftest upload-tempfile-accepts-both-declared-keys
|
||||
(let [profile (th/create-profile* 1 {:is-active true})]
|
||||
(doseq [key-id [:exporter :media-processor]]
|
||||
(let [path (fs/create-tempfile :dir "/tmp/penpot"
|
||||
:prefix (str "test-upload-" (name key-id) "-"))
|
||||
_ (io/write* path "content")
|
||||
out (th/multipart-management-command!
|
||||
{::th/type :upload-tempfile
|
||||
::rpc/auth-key-id key-id
|
||||
::rpc/profile-id (:id profile)
|
||||
:content {:filename "export.png"
|
||||
:path path
|
||||
:mtype "image/png"
|
||||
:size 7}})]
|
||||
(t/is (nil? (:error out)) (name key-id))))))
|
||||
|
||||
(t/deftest duplicate-file
|
||||
(let [storage (-> (:app.storage/storage th/*system*)
|
||||
(configure-storage-backend))
|
||||
|
||||
@ -7,6 +7,7 @@
|
||||
(ns backend-tests.rpc-test
|
||||
(:require
|
||||
[app.common.exceptions :as ex]
|
||||
[app.common.logging :as l]
|
||||
[app.common.schema :as sm]
|
||||
[app.common.time :as ct]
|
||||
[app.common.uuid :as uuid]
|
||||
@ -923,6 +924,92 @@
|
||||
(call-body (method {}) {:extras {::actoken/profile-id pid
|
||||
::actoken/type "personal"}}))))))
|
||||
|
||||
;; --- SHARED KEY AUTHORIZATION
|
||||
|
||||
(defn- call-shared-key-wrapper
|
||||
[mdata params]
|
||||
(let [wrapped (#'rpc/wrap-shared-key-authorization
|
||||
nil (fn [_ params] params) mdata)]
|
||||
(wrapped nil params)))
|
||||
|
||||
(t/deftest shared-key-authorization-allows-declared-keys
|
||||
(let [mdata {::rpc/allowed-shared-keys #{:exporter :media-processor}}]
|
||||
(t/is (= :exporter
|
||||
(::rpc/auth-key-id
|
||||
(call-shared-key-wrapper mdata {::rpc/auth-key-id :exporter}))))
|
||||
(t/is (= :media-processor
|
||||
(::rpc/auth-key-id
|
||||
(call-shared-key-wrapper mdata {::rpc/auth-key-id :media-processor}))))))
|
||||
|
||||
(t/deftest shared-key-authorization-rejects-other-key
|
||||
(let [ex (try (call-shared-key-wrapper {::rpc/allowed-shared-keys #{:exporter}}
|
||||
{::rpc/auth-key-id :nexus})
|
||||
(catch Exception e e))]
|
||||
(t/is (th/ex-of-type? ex :authorization))
|
||||
(t/is (th/ex-of-code? ex :shared-key-not-allowed))))
|
||||
|
||||
(t/deftest shared-key-authorization-rejects-missing-key
|
||||
(let [ex (try (call-shared-key-wrapper {::rpc/allowed-shared-keys #{:exporter}} {})
|
||||
(catch Exception e e))]
|
||||
(t/is (th/ex-of-type? ex :authorization))
|
||||
(t/is (th/ex-of-code? ex :shared-key-not-allowed))))
|
||||
|
||||
(t/deftest shared-key-authorization-without-metadata-allows-any-key
|
||||
(t/is (= :nexus
|
||||
(::rpc/auth-key-id
|
||||
(call-shared-key-wrapper {} {::rpc/auth-key-id :nexus}))))
|
||||
(t/is (nil? (::rpc/auth-key-id (call-shared-key-wrapper {} {})))))
|
||||
|
||||
(t/deftest shared-key-authorization-empty-set-denies-all
|
||||
(let [ex (try (call-shared-key-wrapper {::rpc/allowed-shared-keys #{}}
|
||||
{::rpc/auth-key-id :exporter})
|
||||
(catch Exception e e))]
|
||||
(t/is (th/ex-of-type? ex :authorization))
|
||||
(t/is (th/ex-of-code? ex :shared-key-not-allowed))))
|
||||
|
||||
(t/deftest shared-key-authorization-logs-the-denied-key
|
||||
(let [records (atom [])
|
||||
watch (fn [_ _ _ record] (swap! records conj record))]
|
||||
(add-watch l/log-record ::shared-key-denied watch)
|
||||
(try
|
||||
(try (call-shared-key-wrapper {::rpc/allowed-shared-keys #{:exporter}}
|
||||
{::rpc/auth-key-id :nexus
|
||||
::rpc/handler-name "test-method"})
|
||||
(catch Exception _))
|
||||
(let [deadline (+ (System/currentTimeMillis) 2000)
|
||||
logged? (fn []
|
||||
(some #(and (= :warn (::l/level %))
|
||||
(= "shared key is not allowed for this method"
|
||||
(get (::l/props %) :hint))
|
||||
(= :nexus (get (::l/props %) :key-id))
|
||||
(= "test-method" (get (::l/props %) :handler-name)))
|
||||
@records))]
|
||||
(loop []
|
||||
(when-not (or (logged?)
|
||||
(>= (System/currentTimeMillis) deadline))
|
||||
(Thread/sleep 10)
|
||||
(recur)))
|
||||
(t/is (some? (logged?))))
|
||||
(finally
|
||||
(remove-watch l/log-record ::shared-key-denied)))))
|
||||
|
||||
(t/deftest shared-key-authorization-is-applied-to-rpc-requests
|
||||
(let [mdata {::rpc/allowed-shared-keys #{:exporter}}
|
||||
wrapped (partial (#'rpc/wrap-shared-key-authorization
|
||||
nil (fn [_ params] params) mdata)
|
||||
nil)
|
||||
handler (make-handler {:shared-key-test [mdata wrapped]})]
|
||||
(t/is (= :exporter
|
||||
(::rpc/auth-key-id
|
||||
(call-body handler
|
||||
{:path-params {:method-name "shared-key-test"}
|
||||
:extras {::http/auth-key-id :exporter}}))))
|
||||
(t/is (th/ex-of-code?
|
||||
(call-ex handler
|
||||
{:path-params {:method-name "shared-key-test"}
|
||||
:extras {::http/auth-key-id :nexus}})
|
||||
:shared-key-not-allowed))))
|
||||
|
||||
;; --- PARAMS VALIDATION: SERVER AUTH CONTEXT VS CLIENT PARAMS
|
||||
|
||||
(t/deftest client-qualified-keys-do-not-override-server-auth-context
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user