From 4233d13a5cbfb434630363cf081a73500be7bbf5 Mon Sep 17 00:00:00 2001 From: Jiaen Ren Date: Thu, 27 Aug 2026 16:31:21 -0700 Subject: [PATCH] Retire the mcp:Access action The action mapped to exactly one path, /mcp, and ext_authz is disabled on that route, so it was never evaluated. A role granting it therefore implied an access control that did not run -- worse than not having the action at all. The wholesale gate is the identity-provider scope: JWTVerifier rejects a token without it, so a caller cannot reach the protocol endpoint at all. Per-operation authorization is unchanged, because every tool call reaches OSMO through /api where gateway JWT validation and semantic RBAC apply exactly as they do for the CLI and UI. resourceTypeMCP and ResourceTypeMCP go too; nothing outside the registry referenced either. The two tests removed existed only to assert this action's path mapping. Existing deployments need no migration. IsValidAction has no callers, so nothing validates stored role actions against the registry: a role row still granting mcp:Access keeps an action that matches no path, which is already true today. --- deployments/charts/osmo/values.yaml | 1 - deployments/charts/service/values.yaml | 1 - src/service/mcp/README.md | 4 +- src/utils/connectors/postgres.py | 1 - .../connectors/tests/test_default_roles.py | 1 - src/utils/roles/action_registry.go | 10 --- src/utils/roles/action_registry_test.go | 84 ------------------- 7 files changed, 2 insertions(+), 100 deletions(-) diff --git a/deployments/charts/osmo/values.yaml b/deployments/charts/osmo/values.yaml index fc6b6c508..d8217e343 100644 --- a/deployments/charts/osmo/values.yaml +++ b/deployments/charts/osmo/values.yaml @@ -1644,7 +1644,6 @@ configuration: - app:* - auth:Token - credentials:* - - mcp:Access - pool:List - profile:Read - profile:Update diff --git a/deployments/charts/service/values.yaml b/deployments/charts/service/values.yaml index 3f6811d45..fbcc89e49 100644 --- a/deployments/charts/service/values.yaml +++ b/deployments/charts/service/values.yaml @@ -769,7 +769,6 @@ services: - "app:*" - "auth:Token" - "credentials:*" - - "mcp:Access" - "pool:List" - "profile:Read" - "profile:Update" diff --git a/src/service/mcp/README.md b/src/service/mcp/README.md index 8ec245428..564e935ef 100644 --- a/src/service/mcp/README.md +++ b/src/service/mcp/README.md @@ -411,8 +411,8 @@ failures. ## Deployment validation The MCP smoke target requires an MCP-enabled deployment with JWT -authentication. Its token needs `mcp:Access`, `profile:Read`, and -`workflow:Create` for `OETF_POOL`. +authentication. Its token needs `profile:Read` and `workflow:Create` +for `OETF_POOL`. ```bash bazel run //test/oetf:run -- --env --tags mcp diff --git a/src/utils/connectors/postgres.py b/src/utils/connectors/postgres.py index 64359ecb6..04517f1d6 100644 --- a/src/utils/connectors/postgres.py +++ b/src/utils/connectors/postgres.py @@ -4824,7 +4824,6 @@ def merge_default_role_policies(existing_role: Role, default_role: Role) -> bool 'app:*', 'auth:Token', 'credentials:*', - 'mcp:Access', 'pool:List', 'profile:Read', 'profile:Update', diff --git a/src/utils/connectors/tests/test_default_roles.py b/src/utils/connectors/tests/test_default_roles.py index a043643c8..749981547 100644 --- a/src/utils/connectors/tests/test_default_roles.py +++ b/src/utils/connectors/tests/test_default_roles.py @@ -343,7 +343,6 @@ def test_osmo_user_default_role_allows_only_workflow_read_list_on_all_pools(self 'app:*', 'auth:Token', 'credentials:*', - 'mcp:Access', 'pool:List', 'profile:Read', 'profile:Update', diff --git a/src/utils/roles/action_registry.go b/src/utils/roles/action_registry.go index eb13574d8..a3c64730d 100644 --- a/src/utils/roles/action_registry.go +++ b/src/utils/roles/action_registry.go @@ -43,7 +43,6 @@ const ( resourceTypeConfig = "config" resourceTypeProfile = "profile" resourceTypeWorkflow = "workflow" - resourceTypeMCP = "mcp" resourceTypeInternal = "internal" ) @@ -60,7 +59,6 @@ const ( ResourceTypeConfig ResourceType = resourceTypeConfig ResourceTypeProfile ResourceType = resourceTypeProfile ResourceTypeWorkflow ResourceType = resourceTypeWorkflow - ResourceTypeMCP ResourceType = resourceTypeMCP ResourceTypeInternal ResourceType = resourceTypeInternal ) @@ -111,9 +109,6 @@ const ( ActionAuthRefresh = resourceTypeAuth + ":Refresh" ActionAuthToken = resourceTypeAuth + ":Token" - // MCP actions - ActionMCPAccess = resourceTypeMCP + ":Access" - // System actions (public) ActionSystemHealth = resourceTypeSystem + ":Health" ActionSystemVersion = resourceTypeSystem + ":Version" @@ -288,11 +283,6 @@ var ActionRegistry = map[string][]EndpointPattern{ {Path: "/api/auth/user/*/access_token/*", Methods: []string{"*"}}, }, - // ==================== MCP ==================== - ActionMCPAccess: { - {Path: "/mcp", Methods: []string{"GET", "POST", "DELETE"}}, - }, - // ==================== SYSTEM (PUBLIC) ==================== ActionSystemHealth: { {Path: "/health", Methods: []string{"*"}}, diff --git a/src/utils/roles/action_registry_test.go b/src/utils/roles/action_registry_test.go index 4b1239fcc..f506e9e61 100644 --- a/src/utils/roles/action_registry_test.go +++ b/src/utils/roles/action_registry_test.go @@ -42,90 +42,6 @@ func TestGetAllActions(t *testing.T) { } } -func TestMCPActionRegistry(t *testing.T) { - tests := []struct { - name string - path string - method string - wantAction string - }{ - {name: "GET", path: "/mcp", method: "GET", wantAction: ActionMCPAccess}, - {name: "POST", path: "/mcp", method: "POST", wantAction: ActionMCPAccess}, - {name: "DELETE", path: "/mcp", method: "DELETE", wantAction: ActionMCPAccess}, - {name: "unsupported PUT", path: "/mcp", method: "PUT"}, - {name: "unsupported PATCH", path: "/mcp", method: "PATCH"}, - {name: "unsupported OPTIONS", path: "/mcp", method: "OPTIONS"}, - {name: "nested path", path: "/mcp/tools", method: "GET"}, - {name: "adjacent path", path: "/mcp-extra", method: "GET"}, - { - name: "protected resource metadata path", - path: "/.well-known/oauth-protected-resource/mcp", - method: "GET", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - action, resource := ResolvePathToAction( - context.Background(), tt.path, tt.method, nil, - ) - if action != tt.wantAction { - t.Errorf("ResolvePathToAction(%q, %q) action = %q, want %q", - tt.path, tt.method, action, tt.wantAction) - } - if resource != "" { - t.Errorf("ResolvePathToAction(%q, %q) resource = %q, want empty", - tt.path, tt.method, resource) - } - }) - } -} - -func TestMCPActionAuthorization(t *testing.T) { - osmoUser := &Role{ - Name: "osmo-user", - Policies: []RolePolicy{ - { - Effect: EffectAllow, - Actions: RoleActions{{Action: ActionMCPAccess}}, - Resources: []string{"*"}, - }, - }, - } - roleWithoutMCP := &Role{ - Name: "role-without-mcp", - Policies: []RolePolicy{ - { - Effect: EffectAllow, - Actions: RoleActions{{Action: ActionProfileRead}}, - Resources: []string{"*"}, - }, - }, - } - - for _, method := range []string{"GET", "POST", "DELETE"} { - t.Run(method, func(t *testing.T) { - result := CheckRolesAccess( - context.Background(), []*Role{osmoUser}, "/mcp", method, nil, - ) - if !result.Allowed { - t.Fatalf("osmo-user should be allowed to %s /mcp", method) - } - if result.MatchedAction != ActionMCPAccess { - t.Errorf("MatchedAction = %q, want %q", result.MatchedAction, ActionMCPAccess) - } - - result = CheckRolesAccess( - context.Background(), []*Role{roleWithoutMCP}, "/mcp", method, nil, - ) - if result.Allowed { - t.Errorf("role without %s should not be allowed to %s /mcp", - ActionMCPAccess, method) - } - }) - } -} - func TestMatchMethodRegistry(t *testing.T) { tests := []struct { name string