From 49934adaaf76703879ed05b6d1d098ece867d99e Mon Sep 17 00:00:00 2001 From: kolaente Date: Tue, 26 May 2026 22:58:53 +0200 Subject: [PATCH] feat(mcp): register mcp:access api token scope Adds the mcp scope group with a single access permission so it shows up in GET /api/v1/routes (and therefore in the frontend token form). Adds APIToken.HasMCPAccess() mirroring the caldav/feeds helpers. The MCP endpoint will use POST, GET, and DELETE on the same path for the streamable-HTTP transport, which CanDoAPIRoute's exact (method, path) match cannot gate. The token middleware therefore skips the route check for /api/v1/mcp and any sub-path; the actual authorization is delegated to an inline HasMCPAccess() call in the MCP handler (added in the next task). Fixtures gain two MCP tokens for user 1: one mcp-only and one with mcp:access plus projects read scopes for the per-tool scope filter tests. --- pkg/db/fixtures/api_tokens.yml | 20 ++++++++++ pkg/models/api_routes.go | 12 ++++++ pkg/models/api_routes_test.go | 18 +++++++++ pkg/models/api_tokens.go | 12 ++++++ pkg/models/api_tokens_test.go | 36 ++++++++++++++++- pkg/routes/api_tokens.go | 9 ++++- pkg/webtests/api_tokens_test.go | 71 +++++++++++++++++++++++++++++++++ 7 files changed, 175 insertions(+), 3 deletions(-) diff --git a/pkg/db/fixtures/api_tokens.yml b/pkg/db/fixtures/api_tokens.yml index 46a97e7f5..f38d9619b 100644 --- a/pkg/db/fixtures/api_tokens.yml +++ b/pkg/db/fixtures/api_tokens.yml @@ -78,3 +78,23 @@ owner_id: 13 created: 2024-01-01 00:00:00 # token in plaintext is tk_feeds_access_token_user_0013_feed0013 +- id: 9 + title: 'mcp access token for user 1' + token_salt: mCpAcCs9R1 + token_hash: d57d7084733dee8e76c81ed4220bb4f9147e39b7966c7c435ced7437b2e4e09c9d4595d544b9dcd613c179e9866074f64a87 + token_last_eight: 0mcp0001 + permissions: '{"mcp":["access"]}' + expires_at: 2099-01-01 00:00:00 + owner_id: 1 + created: 2024-01-01 00:00:00 + # token in plaintext is tk_mcp_access_token_test_0000000000mcp0001 +- id: 10 + title: 'mcp access token with mixed scopes for user 1' + token_salt: mCpMxSc8R2 + token_hash: 8c34b5ca2154ee6515900650600d260c1246b98c28e7d56ab6f247aeea81b0fd65d433a4fd8c162149ebe2ff751e020bd8c8 + token_last_eight: pmixed02 + permissions: '{"mcp":["access"],"projects":["read_one","read_all"]}' + expires_at: 2099-01-01 00:00:00 + owner_id: 1 + created: 2024-01-01 00:00:00 + # token in plaintext is tk_mcp_mixed_scope_token_test_00mcpmixed02 diff --git a/pkg/models/api_routes.go b/pkg/models/api_routes.go index 607274092..1e03efefb 100644 --- a/pkg/models/api_routes.go +++ b/pkg/models/api_routes.go @@ -41,6 +41,18 @@ func init() { Method: "GET", }, } + // The MCP endpoint serves the streamable-HTTP transport, which uses + // POST, GET and DELETE on the same path. CanDoAPIRoute only matches one + // (method, path) pair per RouteDetail, so the actual gate lives behind + // skipRouteCheck + an inline HasMCPAccess() call in the MCP handler. + // This entry only exists so the scope appears in GET /api/v1/routes + // and PermissionsAreValid accepts it. + apiTokenRoutes["mcp"] = APITokenRoute{ + "access": &RouteDetail{ + Path: "/api/v1/mcp", + Method: "ANY", + }, + } } type APITokenRoute map[string]*RouteDetail diff --git a/pkg/models/api_routes_test.go b/pkg/models/api_routes_test.go index 3dd9e7d99..c4330bf72 100644 --- a/pkg/models/api_routes_test.go +++ b/pkg/models/api_routes_test.go @@ -24,6 +24,24 @@ import ( "github.com/stretchr/testify/require" ) +func TestAPITokenRoutes_MCPAccessRegistered(t *testing.T) { + routes := GetAPITokenRoutes() + + group, has := routes["mcp"] + require.True(t, has, "mcp scope group should be registered") + + detail, has := group["access"] + require.True(t, has, "mcp.access permission should be registered") + require.NotNil(t, detail, "mcp.access RouteDetail should not be nil") + assert.NotEmpty(t, detail.Path, "mcp.access path should not be empty") + assert.NotEmpty(t, detail.Method, "mcp.access method should not be empty") +} + +func TestPermissionsAreValid_MCPAccess(t *testing.T) { + err := PermissionsAreValid(APIPermissions{"mcp": {"access"}}) + require.NoError(t, err) +} + func TestCanDoAPIRoute_BulkLabelTask(t *testing.T) { // Reset apiTokenRoutes to isolate this test apiTokenRoutes = make(map[string]APITokenRoute) diff --git a/pkg/models/api_tokens.go b/pkg/models/api_tokens.go index 8bbe48550..06b85c02b 100644 --- a/pkg/models/api_tokens.go +++ b/pkg/models/api_tokens.go @@ -216,6 +216,18 @@ func (t *APIToken) HasFeedsAccess() bool { return slices.Contains(perms, "access") } +// HasMCPAccess checks whether the token has the mcp access permission. +// The MCP endpoint uses POST, GET, and DELETE on the same path (streamable-HTTP +// transport), so CanDoAPIRoute can't gate it — the MCP entry handler calls +// this directly after the middleware skips the route check. +func (t *APIToken) HasMCPAccess() bool { + perms, has := t.APIPermissions["mcp"] + if !has { + return false + } + return slices.Contains(perms, "access") +} + // GetTokenFromTokenString returns the full token object from the original token string. func GetTokenFromTokenString(s *xorm.Session, token string) (apiToken *APIToken, err error) { lastEight := token[len(token)-8:] diff --git a/pkg/models/api_tokens_test.go b/pkg/models/api_tokens_test.go index 261de26bd..4be6f19b6 100644 --- a/pkg/models/api_tokens_test.go +++ b/pkg/models/api_tokens_test.go @@ -39,11 +39,13 @@ func TestAPIToken_ReadAll(t *testing.T) { require.NoError(t, err) tokens, is := result.([]*APIToken) assert.Truef(t, is, "tokens are not of type []*APIToken") - assert.Len(t, tokens, 2) + assert.Len(t, tokens, 4) assert.Len(t, tokens, count) - assert.Equal(t, int64(2), total) + assert.Equal(t, int64(4), total) assert.Equal(t, int64(1), tokens[0].ID) assert.Equal(t, int64(2), tokens[1].ID) + assert.Equal(t, int64(9), tokens[2].ID) + assert.Equal(t, int64(10), tokens[3].ID) } func TestAPIToken_CanDelete(t *testing.T) { @@ -155,6 +157,36 @@ func TestAPIToken_HasFeedsAccess(t *testing.T) { }) } +func TestAPIToken_HasMCPAccess(t *testing.T) { + t.Run("has mcp access", func(t *testing.T) { + token := &APIToken{ + APIPermissions: APIPermissions{"mcp": {"access"}}, + } + assert.True(t, token.HasMCPAccess()) + }) + t.Run("no mcp group", func(t *testing.T) { + token := &APIToken{ + APIPermissions: APIPermissions{"tasks": {"read_all"}}, + } + assert.False(t, token.HasMCPAccess()) + }) + t.Run("mcp group but wrong permission", func(t *testing.T) { + token := &APIToken{ + APIPermissions: APIPermissions{"mcp": {"read_all"}}, + } + assert.False(t, token.HasMCPAccess()) + }) + t.Run("mcp access among other permissions", func(t *testing.T) { + token := &APIToken{ + APIPermissions: APIPermissions{ + "tasks": {"read_all", "update"}, + "mcp": {"access"}, + }, + } + assert.True(t, token.HasMCPAccess()) + }) +} + func TestAPIToken_GetTokenFromTokenString(t *testing.T) { t.Run("valid token", func(t *testing.T) { s := db.NewSession() diff --git a/pkg/routes/api_tokens.go b/pkg/routes/api_tokens.go index 0e29911c9..834f98c63 100644 --- a/pkg/routes/api_tokens.go +++ b/pkg/routes/api_tokens.go @@ -46,7 +46,14 @@ func SetupTokenMiddleware() echo.MiddlewareFunc { for _, s := range authHeader { if strings.HasPrefix(s, "Bearer "+models.APITokenPrefix) { - skipRouteCheck := c.Request().URL.Path == "/api/v1/token/test" + path := c.Request().URL.Path + // The MCP endpoint uses POST, GET, and DELETE on the same path + // (streamable-HTTP transport). CanDoAPIRoute does an exact + // (method, path) match per permission, so we skip it here + // and gate inside the MCP handler via token.HasMCPAccess(). + skipRouteCheck := path == "/api/v1/token/test" || + path == "/api/v1/mcp" || + strings.HasPrefix(path, "/api/v1/mcp/") err := checkAPITokenAndPutItInContext(s, c, skipRouteCheck) return err == nil } diff --git a/pkg/webtests/api_tokens_test.go b/pkg/webtests/api_tokens_test.go index e78d09ef9..1307bd475 100644 --- a/pkg/webtests/api_tokens_test.go +++ b/pkg/webtests/api_tokens_test.go @@ -52,6 +52,77 @@ func TestAPITokenRoutesIncludesCaldav(t *testing.T) { assert.Contains(t, res.Body.String(), `"access"`) } +func TestAPITokenRoutesIncludesMCP(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + + s := db.NewSession() + defer s.Close() + u, err := user.GetUserByID(s, 1) + require.NoError(t, err) + jwt, err := auth.NewUserJWTAuthtoken(u, "test-session-id") + require.NoError(t, err) + + req := httptest.NewRequest(http.MethodGet, "/api/v1/routes", nil) + req.Header.Set(echo.HeaderAuthorization, "Bearer "+jwt) + res := httptest.NewRecorder() + e.ServeHTTP(res, req) + + assert.Equal(t, http.StatusOK, res.Code) + assert.Contains(t, res.Body.String(), `"mcp"`) + assert.Contains(t, res.Body.String(), `"access"`) +} + +func TestAPITokenMiddleware_SkipsRouteCheckForMCPPath(t *testing.T) { + // The MCP endpoint needs to accept POST, GET, and DELETE on the same path + // (streamable-HTTP transport). CanDoAPIRoute is exact (method, path) match, + // so we skip the route check for /api/v1/mcp and any sub-path; the + // HasMCPAccess() gate is applied inside the MCP handler instead. + for _, method := range []string{http.MethodGet, http.MethodPost, http.MethodDelete} { + t.Run(method, func(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + req := httptest.NewRequest(method, "/api/v1/mcp", nil) + res := httptest.NewRecorder() + c := e.NewContext(req, res) + + called := false + h := routes.SetupTokenMiddleware()(func(_ *echo.Context) error { + called = true + return nil + }) + + // Token 1 only has {tasks: [read_all, update]} — no mcp scope. + // With the skipRouteCheck, the middleware must still pass the + // request through to the wrapped handler. The MCP-specific + // authorization (HasMCPAccess) is enforced inside the handler, + // not here. + req.Header.Set(echo.HeaderAuthorization, "Bearer tk_2eef46f40ebab3304919ab2e7e39993f75f29d2e") + require.NoError(t, h(c)) + assert.True(t, called, "wrapped handler should run because /api/v1/mcp skips route check") + assert.NotEqual(t, http.StatusUnauthorized, res.Code) + }) + } +} + +func TestAPITokenMiddleware_SkipsRouteCheckForMCPSubPath(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + req := httptest.NewRequest(http.MethodPost, "/api/v1/mcp/anything", nil) + res := httptest.NewRecorder() + c := e.NewContext(req, res) + + called := false + h := routes.SetupTokenMiddleware()(func(_ *echo.Context) error { + called = true + return nil + }) + + req.Header.Set(echo.HeaderAuthorization, "Bearer tk_2eef46f40ebab3304919ab2e7e39993f75f29d2e") + require.NoError(t, h(c)) + assert.True(t, called, "sub-paths under /api/v1/mcp should also skip the route check") +} + func TestAPIToken(t *testing.T) { t.Run("valid token", func(t *testing.T) { e, err := setupTestEnv()