[MCP] Server configuration management: definitions, CRUD, tools & access model - #2027
Draft
wwidergoldpimcore wants to merge 20 commits into
Draft
Conversation
First step of #1309: a first-class, config-managed "MCP server" model, backend only (no UI, no endpoint yet). - McpServerDefinition / McpServerAccess value objects, with fromArray()/toArray() as the single (de)serialization boundary so the shipped symfony-config seed and the settings-store JSON map onto one shape. Access mirrors the SavedSearch/Grid sharing model (owner + shareGlobal + sharedUsers[] + sharedRoles[]), with users and roles in separate lists so a shared id is never ambiguous. - McpServerConfigRepository over Pimcore's LocationAwareConfigRepository, mirroring PerspectiveConfigRepository: shipped defaults from the new studio_mcp_servers node, runtime servers from the configured write target (settings-store or symfony-config, deployer-switchable via config_location.studio_mcp_servers). - Config tree + config_location node + prependCustomConfig wiring; the repository is registered and its config/storage args are set in the extension. Inert by default: the node defaults to an empty map and nothing consumes the repository yet. Tool registration, the per-URL endpoint, access enforcement and OAuth discovery follow in later steps. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…istry Second step of #1309: the mechanism by which tools become available to assign to a server. No server/endpoint yet. - McpToolInterface: a tool describes itself via getDefinition() (name, title, description, MCP annotations, JSON schemas) and runs via execute(). Native contract — the bundle does not depend on the MCP SDK; the per-server endpoint (later step) maps these onto the wire types. - Implementing the interface auto-applies the McpToolRegistry::TAG (via registerForAutoconfiguration in the bundle), and McpToolRegistry collects the tagged tools through an #[AutowireIterator], name-keyed, rejecting duplicates. McpToolPass guards against a hand-written tag on a non-tool service. - McpToolDefinition::requiredScope() derives the OAuth scope from the tool's readOnly annotation (read-only -> mcp:read, else mcp:write; unannotated defaults to write, fail-safe) — the basis for the operation-level scope enforcement tracked for a later step. Mirrors the agent bundle's PR #118 ToolAnnotations, authored and enforced server-side. - PingTool: a built-in, dependency-free read-only tool, so a server can be exercised end-to-end without the agent bundle. Inert by default: the registry is populated but nothing consumes it yet. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nd discovery
Makes step-1 testable end to end without a UI: a configured server is reachable
over MCP at /pimcore-mcp/studio/{server} under the shared pimcore_mcp firewall,
so it accepts the OAuth bearer.
- Adds the mcp/sdk dependency (^0.7, matching the agent bundle) and McpServerFactory,
which assembles a tools-only Mcp\Server per definition. Each assigned tool is
resolved from the registry and bridged onto the SDK: the native execute(array)
is wrapped in a handler that reads the call arguments (via RequestContext /
CallToolRequest) and maps the result to CallToolResult, so tools stay SDK-agnostic.
- McpServerController resolves the definition by URL slug, enforces per-server access
(McpServerAccessResolver — admin/global/owner/user/role, mirroring the bundle's
config sharing), and runs the streamable-HTTP transport with an explicit middleware
stack (dropping the SDK's Dns-rebinding middleware, incompatible with a proxy). The
route is namespaced under /studio/ and declared explicitly so it is neither swept
under the Studio API prefix nor colliding with other bundles' /pimcore-mcp/ routes.
- The extension advertises each enabled server as an RFC 9728 protected resource
(derived from the issuer), so the per-server 401 challenge and discovery resolve.
- A dedicated MCP session cache pool keeps sessions isolated from other bundles.
Verified live: unauthenticated -> 401 + WWW-Authenticate with the per-server
resource_metadata + scope; the resource metadata resolves 200; an authenticated
initialize -> tools/list -> tools/call ping returns "pong".
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Expose the location-aware MCP server configuration over the Studio API so a UI can manage servers without touching symfony-config or the settings store: - Servers: GET/POST/PUT/DELETE under /pimcore-studio/api/mcp/servers, one action per controller, guarded by a new `mcp_servers` user permission. - Tools: GET /pimcore-studio/api/mcp/tools returns the registry's tool catalogue (name, title, description, required scope, read-only/destructive hints) for assignment to a server. The service derives a server's advertised OAuth scopes from its tools' required scope, preserves the owner across updates, locks the url slug to the id, and builds the serving URL from the OAuth issuer. Response DTOs flatten the access model (owner/shareGlobal/sharedUsers/sharedRoles) and each carries a pre-response event. Adds the MCP OpenAPI tag and translation keys. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…g installs The mcp_servers permission is created by the installer on a fresh install, but that runs only once — instances updating from an earlier version never get the row. Without it the Studio permission voter cannot resolve the attribute and denies every user (admins included, since the admin bypass lives inside the vote which never runs for an unsupported attribute), so the MCP server management endpoints return 403. The migration inserts the definition idempotently (guarded on existence, so it is a no-op on a fresh install or a forward-merge replay) and drops the cached permission-key list in postUp so the change takes effect without a separate cache clear. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lists Rework the per-server access model on the agent bundle's run/update pattern, but kept deny-by-default and keyed by id: - Two levels (McpServerPermission Read/Write, write implies read). Read = see the server, view its config, copy the URL, connect a client at runtime; Write = read plus edit, re-share and delete. - Access is a grid: owner (implicit write) + global read flag + user/role share entries, each carrying a level (McpServerAccessEntry). The stored/submitted shapes tolerate the earlier flat id lists, reading them as read grants. - The resolver answers a requested level: admin, then owner, then an authoritative direct-user entry, then a granting role, then global-read, else deny. It backs both the Studio API and the runtime serving endpoint (which asks for Read). Endpoint gating follows the single-permission path: mcp_servers now gates only create and the tool catalogue. List/get are ungated and filtered/asserted by read access, so a user a server is shared with — with no manage permission — still sees it and copies its URL; update/delete assert write. The response carries the caller's resolved permissions (currentUserPermissions) plus the grid, so the UI can mirror the agent-bundle sharing editor. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two docs for the runtime MCP server feature: - Extending/Providing MCP Tools: the McpToolInterface contract, the auto-applied pimcore.studio_backend.mcp_tool tag, annotations-to-scope mapping, and McpToolResult. - Development Details/MCP Server Management: the mcp_servers permission, the settings-store write target, the Studio API surface, the read/write sharing model (deny-by-default, resolution order), and the Studio master/detail management UI including its read-only behavior. Both are cross-linked with the existing MCP infrastructure and OAuth docs and registered in their section indexes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Drop the bundle's parallel MCP tool framework in favour of the mcp/sdk types the agent bundle already uses, so studio tools and agent tools share one contract: - Delete McpToolInterface, McpToolDefinition, McpToolAnnotations, McpToolResult (and the now-dead DuplicateMcpToolException). A tool is now a plain service with an #[McpTool] method returning a CallToolResult. - Tools opt in explicitly with the pimcore.studio_backend.mcp_tool tag (no auto-tag-by-interface); McpToolPass reflects the attribute into the registry and builds a service locator. Tool names must be unique. - McpToolRegistry hands out McpToolReference descriptors (SDK Tool metadata + class/method); McpServerFactory registers them straight onto the SDK builder's addTool([class, method], ...) with a generated + normalized input schema — the former bridge closure and result mapping are gone. - The only Pimcore-specific concern kept is the OAuth scope, now a one-line McpScopes::forReadOnly() helper over the tool's readOnlyHint. - PingTool becomes the reference #[McpTool] example; the tool-authoring doc is rewritten around #[McpTool]/#[Schema]/CallToolResult. Server management, sharing, the mcp_servers permission, the API and routes are unchanged. Full cross-bundle unification of registries/routes is out of scope. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Configurations are meant to be portable across instances where the same user or role carries a different numeric id, so the sharing model now identifies users and roles by their (unique) name — matching how the agent bundle does it: - McpServerAccess.owner and McpServerAccessEntry are name-based; the stored shape and the API (McpServer.owner, McpServerAccessGrant.name) use names. - McpServerAccessResolver matches owner/user entries on the current user's name and resolves the user's role ids to names via RoleResolverInterface (the same id->name resolution the agent bundle performs). - The service stamps the owner from getCurrentUser()->getName(). The feature is experimental and unreleased, so no id->name back-compat is kept; the tolerant deserializer now reads a bare string as a read grant. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the single read/write level with three independent capabilities on an
MCP server, matching the two-checkbox sharing the UI wants:
- A share entry is now { name, canAccess, canEdit }; being listed at all grants
a read-only view. canAccess = connect a client at runtime; canEdit = change
the config. They no longer imply each other.
- The resolver returns { view, access, edit } (union of the user's own and role
entries): view = admin OR public OR listed; access = public OR a granting
entry (admins do NOT get access implicitly); edit = admin OR a granting entry.
- shareGlobal is the "public" flag: any authenticated user may view and use
(not edit) the server.
- The owner (creator) is auto-listed with full capabilities on save, so they
keep view/use/edit of their own server.
- The Studio API gates get on view, put/delete on edit; the runtime serving
endpoint requires access. The server list is filtered to viewable servers.
currentUserPermissions is now { canView, canAccess, canEdit }.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Studio-API write path moved MCP server access to name-based, two-capability
grants, but the symfony-config `studio_mcp_servers` tree still typed identities
as integers — so a YAML-configured server with a string `owner` failed at
container compile with `Expected "int", but got "string"`, and shared_users /
shared_roles could not carry the {name, can_access, can_edit} grid at all.
Align the file-config `access` node with the settings-store shape the repository
feeds into McpServerAccess::fromArray:
- owner is a scalar username (was integerNode)
- shared_users / shared_roles are grants of { name, can_access, can_edit }, and a
bare string is accepted as a view-only grant (was integerPrototype)
Regression test processes the node and the full tree with a string owner and the
capability grid.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Per the refined permission model (#1452), the owner is now symmetric with an admin: implicit Config Read + Config Edit, but MCP Server Access must be granted explicitly — so nobody, not even the owner or an admin, has default access to a server's runtime. - McpServerAccessResolver: the owner resolves to view + edit (not access), like an admin. Access stays public-or-explicit-entry only. - McpServerConfigurationService: stop seeding the owner into the sharing grid with full capabilities; the owner's read/edit is implicit, and they add themselves to the user list to grant access. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The MCP Server Management page still described the original two-level read/write sharing (owner with implicit write). Rewrite it to the current model: three independent capabilities — Config Read, Config Edit, MCP Server Access — where the owner and admins have implicit read+edit but must be granted access explicitly, and a public server grants read+access (not edit). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
View was derived from being listed, so read could not be withheld. Give each
grant its own canRead flag, so a user can hold Access without Read.
- McpServerAccessEntry: { name, canRead, canAccess, canEdit }; canRead normalises
to true when canEdit (edit implies read); fromMixed defaults canRead true so
grants stored before the flag keep "listed = read"; toArray adds can_read.
- McpServerAccessGrant: add canRead (required) + isCanRead().
- McpServerAccessResolver: view = admin || owner || public || a grant with
canRead — drops the "listed = view" shortcut. Access/edit unchanged.
- Hydrator passes canRead through; docs updated (Config Read now per-grant, with
the edit-implies-read invariant + back-compat default).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Admins and the owner always hold Config Read + Edit at resolve time, but the write path stored the grants verbatim — so an admin (or the owner) submitted with Edit unchecked was persisted, and then returned, as non-editable, which the UI faithfully rendered as a locked-down config. Patch user grants on write instead of trusting the client to have disabled the right checkboxes: any user entry belonging to the owner or an admin is forced to Read + Edit (Access is preserved and never made implicit). This keeps the stored config honest even when the frontend has a bug or the owner is somehow entered as read-only. Roles are left untouched — the admin/owner concept applies to users only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…er-config code Address the quality-gate violations on the MCP server-configuration feature and the one PHPStan error the "lowest deps" / 2026.x-dev static-analysis variants report: - S103 (line > 120): wrap the long #[Property] attributes, the docblock array-shape type and @PARAM lines; shorten two config-node info strings. - S1121 (assignment inside an expression): extract the memoise / lazy-init assignments in McpToolRegistry, McpServerConfigRepository and McpServerFactory. - PHPStan: extend the existing Symfony config-builder false-positive ignore for Configuration.php with `beforeNormalization`/`arrayPrototype` (the fluent chain in mcpAccessGrantListNode), which only the lowest-deps variants flag. No behaviour change (Mcp unit suite green, PHPStan + CS clean). The remaining gate items (S1192 duplicated SQL literal, S1185 redundant constructors) are acknowledged as won't-fix in SonarCloud. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The capability was added to the DTOs, hydrator and resolver but not to the config tree, so a YAML-defined server rejected the key its own settings-store form emits. Claude-Session: https://claude.ai/code/session_01Ej6YARj6HakERFuERFuDF9 Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* [MCP] Resolve the token audience from the endpoint's owner The authenticator derived the resource from a path hardcoded here, so only this bundle's own MCP servers could be reached with an audience-bound token; an endpoint registered by another bundle never matched. It now resolves the most specific registered resource covering the request, and declines when none does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ej6YARj6HakERFuERFuDF9 * [MCP] Document that registration decides which endpoints accept OAuth The authenticator now holds a token to the resource registered for the endpoint being called, so an endpoint whose owner registers none does not accept OAuth. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ej6YARj6HakERFuERFuDF9 * [MCP] Keep discovery on the issuer and rank matches by path Deriving the audience from the issuer while the 401 challenge and the metadata lookup still used the request host sent clients to a document that resolved to nothing. Ranking on the whole identifier also let a long query outrank the resource that actually matched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ej6YARj6HakERFuERFuDF9 * [MCP] Require oauth.issuer when the OAuth server is enabled The issuer is the authorization server's identity: stamped on tokens as `iss`, advertised in metadata, and the base for protected-resource URIs (`buildMcpServerResources` builds `<issuer>/pimcore-mcp/studio/<slug>`). Deriving it per request from the Host header is non-deterministic, and at container-build time yields no issuer at all — `buildMcpServerResources` returns an empty list, so no managed server registers as a resource and the authenticator then refuses OAuth for all of them. Reject `oauth.enabled: true` with a null issuer at config-compile time instead of failing silently at runtime, and document the issuer as the required public base URL (with reverse-proxy and env-var guidance). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * [MCP] Point the 401 challenge at the matched resource, not the request path McpAuthenticationEntryPoint built the RFC 9728 metadata URL from the raw request path, but the authenticator validates the token against the resource the RequestResourceResolver selects by longest prefix. When a broader resource covers the endpoint the two differ, and the challenge then advertised a metadata document that ProtectedResourceMetadataController's exact lookup answers with 404 — so the client could not discover where to authenticate. Resolve the request in the entry point too and build the metadata URL from the matched resource's path, falling back to the request path when nothing is registered. Regression test covers a broader-only registration. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * [MCP] Harden the issuer requirement and the root-resource challenge Two follow-ups from the review: - The enabled-issuer check only rejected null, so `issuer: ''` (and non-string scalars) slipped through, yielding relative resource URIs and an empty `iss`. Require a non-empty absolute origin (scheme + host); reject blank/malformed. - The 401 challenge conflated "no resource matched" with "matched an origin-only root resource": parse_url() returns a null path for the latter, so it fell back to the request path and reintroduced the mismatch. Branch on the null match instead, and use the matched resource's path (empty for a root resource). Adds config rejection cases (empty, non-absolute) and a root-resource challenge regression test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * [MCP] Validate the issuer as an http(s) origin on the scalar node The previous check lived on the parent oauth node and called parse_url() directly, which was wrong in both directions: - it rejected the documented `issuer: '%env(...)%'` form, because Symfony keeps env placeholders literal while the parent array node is validated; and - it accepted malformed issuers (path, query, fragment, userinfo, non-HTTP(S)), where a fragment or query swallows the appended `/pimcore-mcp/studio/...` path. Move the URL-shape check onto the scalar `issuer` node (Symfony skips it for unresolved placeholders, with an explicit `%…%` guard so it is also skipped in a bare Processor test) and require a bare http(s) origin — no userinfo, path, query or fragment. The parent node keeps only the enabled-without-issuer rule. Tests cover the env-backed form plus the malformed cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * [MCP] Document the issuer shape instead of validating it The issuer shape check was out of step with this configuration: nothing else here validates an operator-supplied URL, and client `redirect_uris` — the most security-sensitive URL in the OAuth config — is only checked for presence, with real enforcement at runtime. It also needed placeholder gymnastics to keep the documented `%env(...)%` form working. Drop the shape check and keep the part that matters: `issuer` is still required when OAuth is enabled, because omitting it registers no protected resource at all and every OAuth request then 401s with nothing pointing at the cause. The shape guidance moves to the documentation, including the one consequence that would otherwise be silent: a query or fragment collapses every MCP server onto the same token audience, defeating per-server isolation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Wilhelm Widergold <wilhelm.widergold@pimcore.com>
The authenticator section still said the resource URI is built from the issuer "when one is set", which predates oauth.issuer becoming required whenever the OAuth server is enabled. State the requirement instead of the stale conditional. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
wwidergoldpimcore
force-pushed
the
feature/mcp-1309-server-config-management
branch
from
September 9, 2026 12:40
28c90c3 to
9d09fa2
Compare
#2028 landed on a branch without src/Mcp/McpScopes.php, so McpScopeProvider had to spell the scope identifiers out. Now that both halves sit on one branch, take the constants instead, and do the same for the two other places that carried the same values behaviourally: the per-server scopes_supported fallback in the extension, and the scope advertised in the MCP firewall's 401 challenge. The remaining literals are OpenAPI `example:` strings and the generic oauth.resources default, which are documentation and configuration rather than this bundle's scope vocabulary. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes in this pull request
Adds user-configurable MCP servers to the Studio backend: define an MCP server, choose the tools it exposes, serve it over HTTP with OAuth-protected per-server access, and share it with other users/roles under a fine-grained capability model.
What it adds
studio_mcp_serversconfig node; runtime servers in the write target).#[McpTool]contract; a tool catalogue is exposed for the UI./pimcore-mcp/studio/{slug}with per-server access checks and RFC 9728 discovery.mcp_serverspermission on existing installs via a migration.The access / sharing model (evolved over the branch)
canRead/canAccess/canEdit), with the invariant that Edit implies Read.Docs
09_MCP_Server_Management.md(the capability model) and15_Providing_MCP_Tools.md(authoring tools), plus module README links. (OAuth-server docs deliberately live on the [Grid] Reuse Resolver in AdvancedColumnResolver #1308 base branch, not here.)Additional info
feature/oauth-1308-basic-integration([Grid] Reuse Resolver in AdvancedColumnResolver #1308); the OAuth authorization server and its docs are owned there.…/pimcore-mcp/studio/{slug}) this branch establishes.🤖 Generated with Claude Code