From d814ba47822ba8e9184c60bf99408820e6f035e4 Mon Sep 17 00:00:00 2001 From: Sahan Dilshan Date: Thu, 25 Jun 2026 23:01:23 +0530 Subject: [PATCH] Add kind discriminator for MCP resource server tools and resources Model MCP tools and resources as actions distinguished by a kind ("tool"|"resource") stored in ACTION.PROPERTIES, surfaced as a flat Action.Kind field (mirroring the RESOURCE_SERVER delimiter pattern). - kind is optional on create for all resource server types; MCP-type servers default an omitted kind to "tool"; API/CUSTOM may set a kind but are not required to. A provided kind must be "tool" or "resource". - kind is immutable after create (the update path does not accept it). - MCP-gated cross-entity handle/permission collision guard, on the REST and declarative-import paths. - kind on the create request/response; GET .../actions?kind= filter (Postgres and SQLite variants) with a matching filtered count. - Declarative import/export preserve kind; OpenAPI updated. Refs #3465 --- api/resource.yaml | 182 +++++++-- backend/internal/authzen/service.go | 2 +- backend/internal/authzen/service_test.go | 23 +- .../ResourceServiceInterface_mock_test.go | 34 +- backend/internal/resource/composite_store.go | 22 +- .../internal/resource/composite_store_test.go | 109 +++-- .../internal/resource/declarative_resource.go | 63 ++- .../resource/declarative_resource_test.go | 372 +++++++++++++++++- backend/internal/resource/file_based_store.go | 11 +- .../resource/file_based_store_test.go | 65 ++- backend/internal/resource/handler.go | 34 +- backend/internal/resource/handler_test.go | 119 +++++- backend/internal/resource/model.go | 18 +- .../resourceStoreInterface_mock_test.go | 64 +-- backend/internal/resource/service.go | 76 +++- backend/internal/resource/service_test.go | 306 +++++++++++++- backend/internal/resource/store.go | 118 ++++-- backend/internal/resource/store_constants.go | 36 ++ backend/internal/resource/store_test.go | 165 +++++++- .../pkg/thunderidengine/providers/model.go | 28 ++ .../thunderidengine/providers/model_test.go | 38 ++ .../ResourceServiceInterface_mock.go | 34 +- .../resourceStoreInterface_mock.go | 64 +-- 23 files changed, 1736 insertions(+), 247 deletions(-) create mode 100644 backend/pkg/thunderidengine/providers/model_test.go diff --git a/api/resource.yaml b/api/resource.yaml index fa0412b684..d7cd42d381 100644 --- a/api/resource.yaml +++ b/api/resource.yaml @@ -1126,7 +1126,10 @@ paths: tags: - Actions summary: List actions at resource server level - description: Returns all actions defined at the resource server level + description: | + Returns all actions defined at the resource server level. Use the optional `kind` query + parameter to filter to a specific MCP primitive type. Omitting `kind` returns all actions + regardless of kind (the default, backward-compatible behavior). parameters: - in: path name: rsId @@ -1135,6 +1138,16 @@ paths: type: string format: uuid description: Resource server ID + - in: query + name: kind + required: false + schema: + type: string + enum: [tool, resource] + description: | + Filter actions by MCP primitive kind. Only `tool` and `resource` are valid values; + any other value (including future kinds) results in a 400 response. Omit the parameter + to return all actions regardless of kind. - $ref: '#/components/parameters/limitQueryParam' - $ref: '#/components/parameters/offsetQueryParam' responses: @@ -1144,24 +1157,47 @@ paths: application/json: schema: $ref: '#/components/schemas/ActionListResponse' - example: - totalResults: 5 - startIndex: 1 - count: 2 - actions: - - id: "9c6d3g0g-7e8b-4d82-b454-4d2ccg87gh5e" - name: "Create Booking" - description: "Create a new booking" - handle: "create" - permission: "create" - - id: "ad7e4h1h-8f9c-4e93-c565-5e3ddf98hi6f" - name: "List Bookings" - description: "List all bookings" - handle: "list" - permission: "list" - links: - - href: "resource-servers/3fa85f64-5717-4562-b3fc-2c963f66afa6/actions?offset=2&limit=2" - rel: "next" + examples: + unfiltered: + summary: All actions (no kind filter) + value: + totalResults: 5 + startIndex: 1 + count: 2 + actions: + - id: "9c6d3g0g-7e8b-4d82-b454-4d2ccg87gh5e" + name: "Create Booking" + description: "Create a new booking" + handle: "create" + permission: "create" + - id: "ad7e4h1h-8f9c-4e93-c565-5e3ddf98hi6f" + name: "List Bookings" + description: "List all bookings" + handle: "list" + permission: "list" + links: + - href: "resource-servers/3fa85f64-5717-4562-b3fc-2c963f66afa6/actions?offset=2&limit=2" + rel: "next" + kind-tool-filter: + summary: MCP tools only (kind=tool) + value: + totalResults: 2 + startIndex: 1 + count: 2 + actions: + - id: "9c6d3g0g-7e8b-4d82-b454-4d2ccg87gh5e" + name: "Create User" + description: "Creates a new user account" + handle: "create_user" + permission: "booking-mcp:create_user" + kind: "tool" + - id: "ad7e4h1h-8f9c-4e93-c565-5e3ddf98hi6f" + name: "Delete User" + description: "Deletes a user account" + handle: "delete_user" + permission: "booking-mcp:delete_user" + kind: "tool" + links: [] "400": description: Bad request content: @@ -1199,6 +1235,16 @@ paths: description: key: "error.resourceservice.result_limit_exceeded_in_composite_mode_description" defaultValue: "The total number of records exceeds the maximum limit in composite mode" + invalid-kind: + summary: Invalid kind parameter value + value: + code: "RES-1001" + message: + key: "error.resourceservice.invalid_request_format" + defaultValue: "Invalid request format" + description: + key: "error.resourceservice.invalid_request_format_description" + defaultValue: "The request body is malformed or contains invalid data" "404": description: Resource server not found content: @@ -1623,7 +1669,10 @@ paths: tags: - Actions summary: List actions at resource level - description: Returns all actions defined for a specific resource + description: | + Returns all actions defined for a specific resource. Use the optional `kind` query + parameter to filter to a specific MCP primitive type. Omitting `kind` returns all actions + regardless of kind (the default, backward-compatible behavior). parameters: - in: path name: rsId @@ -1639,6 +1688,16 @@ paths: type: string format: uuid description: Resource ID + - in: query + name: kind + required: false + schema: + type: string + enum: [tool, resource] + description: | + Filter actions by MCP primitive kind. Only `tool` and `resource` are valid values; + any other value (including future kinds) results in a 400 response. Omit the parameter + to return all actions regardless of kind. - $ref: '#/components/parameters/limitQueryParam' - $ref: '#/components/parameters/offsetQueryParam' responses: @@ -1648,24 +1707,41 @@ paths: application/json: schema: $ref: '#/components/schemas/ActionListResponse' - example: - totalResults: 6 - startIndex: 1 - count: 2 - actions: - - id: "be8f5i2i-9g0d-5fa4-d676-6f4eeg09ij7g" - name: "Create Reservation" - description: "Create a new reservation" - handle: "create" - permission: "reservations:create" - - id: "cf9g6j3j-0h1e-6gb5-e787-7g5ffh10jk8h" - name: "Read Reservation" - description: "Read reservation details" - handle: "read" - permission: "reservations:read" - links: - - href: "resource-servers/3fa85f64-5717-4562-b3fc-2c963f66afa6/resources/7a4b1f8e-5c69-4b60-9232-2b0aaf65ef3c/actions?offset=2&limit=2" - rel: "next" + examples: + unfiltered: + summary: All actions (no kind filter) + value: + totalResults: 6 + startIndex: 1 + count: 2 + actions: + - id: "be8f5i2i-9g0d-5fa4-d676-6f4eeg09ij7g" + name: "Create Reservation" + description: "Create a new reservation" + handle: "create" + permission: "reservations:create" + - id: "cf9g6j3j-0h1e-6gb5-e787-7g5ffh10jk8h" + name: "Read Reservation" + description: "Read reservation details" + handle: "read" + permission: "reservations:read" + links: + - href: "resource-servers/3fa85f64-5717-4562-b3fc-2c963f66afa6/resources/7a4b1f8e-5c69-4b60-9232-2b0aaf65ef3c/actions?offset=2&limit=2" + rel: "next" + kind-resource-filter: + summary: MCP resources only (kind=resource) + value: + totalResults: 1 + startIndex: 1 + count: 1 + actions: + - id: "cf9g6j3j-0h1e-6gb5-e787-7g5ffh10jk8h" + name: "User List" + description: "Read-only list of users" + handle: "user_list" + permission: "booking-mcp:user-mgmt:user_list" + kind: "resource" + links: [] "400": description: Bad request content: @@ -1703,6 +1779,16 @@ paths: description: key: "error.resourceservice.result_limit_exceeded_in_composite_mode_description" defaultValue: "The total number of records exceeds the maximum limit in composite mode" + invalid-kind: + summary: Invalid kind parameter value + value: + code: "RES-1001" + message: + key: "error.resourceservice.invalid_request_format" + defaultValue: "Invalid request format" + description: + key: "error.resourceservice.invalid_request_format_description" + defaultValue: "The request body is malformed or contains invalid data" "404": description: Resource or resource server not found content: @@ -2384,6 +2470,15 @@ components: permission: type: string description: Derived permission string based on handle hierarchy + kind: + type: string + enum: [tool, resource] + description: > + MCP primitive kind. "tool" represents a callable MCP tool (model-controlled); "resource" + represents a read-only MCP resource (application-controlled). Always present for type=MCP + actions (defaults to "tool" when omitted on create). For API and CUSTOM resource server + actions it is present only when a kind was provided on create, otherwise omitted. + Immutable after creation. CreateActionRequest: type: object @@ -2394,10 +2489,19 @@ components: description: Display name of the action handle: type: string - description: Immutable and unique under resource server or resource. Identifier used for permission derivation + maxLength: 100 + description: Immutable and unique under resource server or resource. Identifier used for permission derivation. Maximum length is 100 characters. description: type: string description: Optional description of the action + kind: + type: string + enum: [tool, resource] + description: > + MCP primitive kind. Optional for all resource server types; when provided it must be + "tool" or "resource". For type=MCP it defaults to "tool" when omitted. For API and + CUSTOM resource servers it is optional and, when omitted, stays empty. Immutable after + creation; omit on update requests (kind cannot be changed). UpdateActionRequest: type: object diff --git a/backend/internal/authzen/service.go b/backend/internal/authzen/service.go index b1e7b37f11..ea597d0a1b 100644 --- a/backend/internal/authzen/service.go +++ b/backend/internal/authzen/service.go @@ -356,7 +356,7 @@ func (s *authzenService) getAllActions( actions := make([]providers.Action, 0) for offset := 0; ; { actionList, svcErr := s.resourceService.GetActionList( - ctx, resourceServerID, resourceID, serverconst.MaxPageSize, offset) + ctx, resourceServerID, resourceID, "", serverconst.MaxPageSize, offset) if svcErr != nil { return nil, svcErr } diff --git a/backend/internal/authzen/service_test.go b/backend/internal/authzen/service_test.go index 3130414126..27b0c7d8d0 100644 --- a/backend/internal/authzen/service_test.go +++ b/backend/internal/authzen/service_test.go @@ -718,7 +718,8 @@ func (s *ServiceTestSuite) TestSearchActionsReturnsAuthorizedActions() { s.mockResourceServerHandle("booking") bookingResourceID := testBookingResourceID invoiceResourceID := "invoice1" - s.resourceMock.On("GetActionList", mock.Anything, testResourceServerID, (*string)(nil), mock.Anything, 0). + s.resourceMock.On("GetActionList", mock.Anything, testResourceServerID, (*string)(nil), + providers.ActionKind(""), mock.Anything, 0). Return(&resource.ActionList{ Actions: []providers.Action{ {Handle: "read", Permission: "booking:booking:read"}, @@ -732,13 +733,15 @@ func (s *ServiceTestSuite) TestSearchActionsReturnsAuthorizedActions() { {ID: invoiceResourceID}, }, }, nil) - s.resourceMock.On("GetActionList", mock.Anything, testResourceServerID, &bookingResourceID, mock.Anything, 0). + s.resourceMock.On("GetActionList", mock.Anything, testResourceServerID, &bookingResourceID, + providers.ActionKind(""), mock.Anything, 0). Return(&resource.ActionList{ Actions: []providers.Action{ {Handle: "delete", Permission: "booking:booking:delete"}, }, }, nil) - s.resourceMock.On("GetActionList", mock.Anything, testResourceServerID, &invoiceResourceID, mock.Anything, 0). + s.resourceMock.On("GetActionList", mock.Anything, testResourceServerID, &invoiceResourceID, + providers.ActionKind(""), mock.Anything, 0). Return(&resource.ActionList{ Actions: []providers.Action{ {Handle: "approve", Permission: "invoice:invoice:approve"}, @@ -779,7 +782,8 @@ func (s *ServiceTestSuite) TestSearchActionsPaginatesResourceServerActions() { s.entityProviderMock.On("GetTransitiveEntityGroups", "user1").Return([]providers.EntityGroup{}, nil) s.mockResourceServerHandle("booking") - s.resourceMock.On("GetActionList", mock.Anything, testResourceServerID, (*string)(nil), serverconst.MaxPageSize, 0). + s.resourceMock.On("GetActionList", mock.Anything, testResourceServerID, (*string)(nil), + providers.ActionKind(""), serverconst.MaxPageSize, 0). Return(&resource.ActionList{ TotalResults: serverconst.MaxPageSize + 1, Count: serverconst.MaxPageSize, @@ -788,7 +792,7 @@ func (s *ServiceTestSuite) TestSearchActionsPaginatesResourceServerActions() { }, }, nil) s.resourceMock.On("GetActionList", mock.Anything, testResourceServerID, (*string)(nil), - serverconst.MaxPageSize, serverconst.MaxPageSize). + providers.ActionKind(""), serverconst.MaxPageSize, serverconst.MaxPageSize). Return(&resource.ActionList{ TotalResults: serverconst.MaxPageSize + 1, Count: 1, @@ -829,7 +833,8 @@ func (s *ServiceTestSuite) TestSearchActionsReturnsEmptyResultsWhenDenied() { s.entityProviderMock.On("GetTransitiveEntityGroups", "user1").Return([]providers.EntityGroup{}, nil) s.mockResourceServerHandle("booking") - s.resourceMock.On("GetActionList", mock.Anything, testResourceServerID, (*string)(nil), mock.Anything, 0). + s.resourceMock.On("GetActionList", mock.Anything, testResourceServerID, (*string)(nil), + providers.ActionKind(""), mock.Anything, 0). Return(&resource.ActionList{ Actions: []providers.Action{ {Handle: "read", Permission: "booking:booking:read"}, @@ -906,7 +911,8 @@ func (s *ServiceTestSuite) TestSearchActionsResourceServiceError() { s.entityProviderMock.On("GetTransitiveEntityGroups", "user1").Return([]providers.EntityGroup{}, nil) s.mockResourceServerHandle("booking") - s.resourceMock.On("GetActionList", mock.Anything, testResourceServerID, (*string)(nil), mock.Anything, 0). + s.resourceMock.On("GetActionList", mock.Anything, testResourceServerID, (*string)(nil), + providers.ActionKind(""), mock.Anything, 0). Return((*resource.ActionList)(nil), &tidcommon.InternalServerError) resp, svcErr := s.service.SearchActions(context.Background(), req) @@ -924,7 +930,8 @@ func (s *ServiceTestSuite) TestSearchActionsAuthorizationServiceError() { s.entityProviderMock.On("GetTransitiveEntityGroups", "user1").Return([]providers.EntityGroup{}, nil) s.mockResourceServerHandle("booking") - s.resourceMock.On("GetActionList", mock.Anything, testResourceServerID, (*string)(nil), mock.Anything, 0). + s.resourceMock.On("GetActionList", mock.Anything, testResourceServerID, (*string)(nil), + providers.ActionKind(""), mock.Anything, 0). Return(&resource.ActionList{ Actions: []providers.Action{ {Handle: "read", Permission: "booking:booking:read"}, diff --git a/backend/internal/resource/ResourceServiceInterface_mock_test.go b/backend/internal/resource/ResourceServiceInterface_mock_test.go index 97f89bbd6e..76003f5752 100644 --- a/backend/internal/resource/ResourceServiceInterface_mock_test.go +++ b/backend/internal/resource/ResourceServiceInterface_mock_test.go @@ -615,8 +615,8 @@ func (_c *ResourceServiceInterfaceMock_GetAction_Call) RunAndReturn(run func(ctx } // GetActionList provides a mock function for the type ResourceServiceInterfaceMock -func (_mock *ResourceServiceInterfaceMock) GetActionList(ctx context.Context, resourceServerID string, resourceID *string, limit int, offset int) (*ActionList, *common.ServiceError) { - ret := _mock.Called(ctx, resourceServerID, resourceID, limit, offset) +func (_mock *ResourceServiceInterfaceMock) GetActionList(ctx context.Context, resourceServerID string, resourceID *string, kind providers.ActionKind, limit int, offset int) (*ActionList, *common.ServiceError) { + ret := _mock.Called(ctx, resourceServerID, resourceID, kind, limit, offset) if len(ret) == 0 { panic("no return value specified for GetActionList") @@ -624,18 +624,18 @@ func (_mock *ResourceServiceInterfaceMock) GetActionList(ctx context.Context, re var r0 *ActionList var r1 *common.ServiceError - if returnFunc, ok := ret.Get(0).(func(context.Context, string, *string, int, int) (*ActionList, *common.ServiceError)); ok { - return returnFunc(ctx, resourceServerID, resourceID, limit, offset) + if returnFunc, ok := ret.Get(0).(func(context.Context, string, *string, providers.ActionKind, int, int) (*ActionList, *common.ServiceError)); ok { + return returnFunc(ctx, resourceServerID, resourceID, kind, limit, offset) } - if returnFunc, ok := ret.Get(0).(func(context.Context, string, *string, int, int) *ActionList); ok { - r0 = returnFunc(ctx, resourceServerID, resourceID, limit, offset) + if returnFunc, ok := ret.Get(0).(func(context.Context, string, *string, providers.ActionKind, int, int) *ActionList); ok { + r0 = returnFunc(ctx, resourceServerID, resourceID, kind, limit, offset) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*ActionList) } } - if returnFunc, ok := ret.Get(1).(func(context.Context, string, *string, int, int) *common.ServiceError); ok { - r1 = returnFunc(ctx, resourceServerID, resourceID, limit, offset) + if returnFunc, ok := ret.Get(1).(func(context.Context, string, *string, providers.ActionKind, int, int) *common.ServiceError); ok { + r1 = returnFunc(ctx, resourceServerID, resourceID, kind, limit, offset) } else { if ret.Get(1) != nil { r1 = ret.Get(1).(*common.ServiceError) @@ -653,13 +653,14 @@ type ResourceServiceInterfaceMock_GetActionList_Call struct { // - ctx context.Context // - resourceServerID string // - resourceID *string +// - kind providers.ActionKind // - limit int // - offset int -func (_e *ResourceServiceInterfaceMock_Expecter) GetActionList(ctx interface{}, resourceServerID interface{}, resourceID interface{}, limit interface{}, offset interface{}) *ResourceServiceInterfaceMock_GetActionList_Call { - return &ResourceServiceInterfaceMock_GetActionList_Call{Call: _e.mock.On("GetActionList", ctx, resourceServerID, resourceID, limit, offset)} +func (_e *ResourceServiceInterfaceMock_Expecter) GetActionList(ctx interface{}, resourceServerID interface{}, resourceID interface{}, kind interface{}, limit interface{}, offset interface{}) *ResourceServiceInterfaceMock_GetActionList_Call { + return &ResourceServiceInterfaceMock_GetActionList_Call{Call: _e.mock.On("GetActionList", ctx, resourceServerID, resourceID, kind, limit, offset)} } -func (_c *ResourceServiceInterfaceMock_GetActionList_Call) Run(run func(ctx context.Context, resourceServerID string, resourceID *string, limit int, offset int)) *ResourceServiceInterfaceMock_GetActionList_Call { +func (_c *ResourceServiceInterfaceMock_GetActionList_Call) Run(run func(ctx context.Context, resourceServerID string, resourceID *string, kind providers.ActionKind, limit int, offset int)) *ResourceServiceInterfaceMock_GetActionList_Call { _c.Call.Run(func(args mock.Arguments) { var arg0 context.Context if args[0] != nil { @@ -673,20 +674,25 @@ func (_c *ResourceServiceInterfaceMock_GetActionList_Call) Run(run func(ctx cont if args[2] != nil { arg2 = args[2].(*string) } - var arg3 int + var arg3 providers.ActionKind if args[3] != nil { - arg3 = args[3].(int) + arg3 = args[3].(providers.ActionKind) } var arg4 int if args[4] != nil { arg4 = args[4].(int) } + var arg5 int + if args[5] != nil { + arg5 = args[5].(int) + } run( arg0, arg1, arg2, arg3, arg4, + arg5, ) }) return _c @@ -697,7 +703,7 @@ func (_c *ResourceServiceInterfaceMock_GetActionList_Call) Return(actionList *Ac return _c } -func (_c *ResourceServiceInterfaceMock_GetActionList_Call) RunAndReturn(run func(ctx context.Context, resourceServerID string, resourceID *string, limit int, offset int) (*ActionList, *common.ServiceError)) *ResourceServiceInterfaceMock_GetActionList_Call { +func (_c *ResourceServiceInterfaceMock_GetActionList_Call) RunAndReturn(run func(ctx context.Context, resourceServerID string, resourceID *string, kind providers.ActionKind, limit int, offset int) (*ActionList, *common.ServiceError)) *ResourceServiceInterfaceMock_GetActionList_Call { _c.Call.Return(run) return _c } diff --git a/backend/internal/resource/composite_store.go b/backend/internal/resource/composite_store.go index e4afc8fa85..d2144abaf6 100644 --- a/backend/internal/resource/composite_store.go +++ b/backend/internal/resource/composite_store.go @@ -416,8 +416,9 @@ func (c *compositeResourceStore) GetAction( // GetActionList returns a paginated, deduplicated action list from both stores. func (c *compositeResourceStore) GetActionList( - ctx context.Context, resServerID string, resID *string, limit, offset int) ([]providers.Action, error) { - merged, err := c.getMergedActions(ctx, resServerID, resID) + ctx context.Context, resServerID string, resID *string, kind providers.ActionKind, limit, offset int, +) ([]providers.Action, error) { + merged, err := c.getMergedActions(ctx, resServerID, resID, kind) if err != nil { return nil, err } @@ -435,10 +436,10 @@ func (c *compositeResourceStore) GetActionList( return merged[start:end], nil } -// GetActionListCount returns the deduplicated action count across both stores. +// GetActionListCount returns the deduplicated action count across both stores, optionally filtered by kind. func (c *compositeResourceStore) GetActionListCount( - ctx context.Context, resServerID string, resID *string) (int, error) { - merged, err := c.getMergedActions(ctx, resServerID, resID) + ctx context.Context, resServerID string, resID *string, kind providers.ActionKind) (int, error) { + merged, err := c.getMergedActions(ctx, resServerID, resID, kind) if err != nil { return 0, err } @@ -513,18 +514,19 @@ func (c *compositeResourceStore) getMergedResourcesByParent( ) } -// getMergedActions returns the deduplicated action list across both stores. +// getMergedActions returns the deduplicated action list across both stores, optionally filtered by kind. func (c *compositeResourceStore) getMergedActions( ctx context.Context, resServerID string, resID *string, + kind providers.ActionKind, ) ([]providers.Action, error) { - dbCount, err := c.dbStore.GetActionListCount(ctx, resServerID, resID) + dbCount, err := c.dbStore.GetActionListCount(ctx, resServerID, resID, kind) if err != nil { return nil, err } - fileCount, err := c.fileStore.GetActionListCount(ctx, resServerID, resID) + fileCount, err := c.fileStore.GetActionListCount(ctx, resServerID, resID, kind) if err != nil { return nil, err } @@ -533,10 +535,10 @@ func (c *compositeResourceStore) getMergedActions( dbCount, fileCount, func(count int) ([]providers.Action, error) { - return c.dbStore.GetActionList(ctx, resServerID, resID, count, 0) + return c.dbStore.GetActionList(ctx, resServerID, resID, kind, count, 0) }, func(count int) ([]providers.Action, error) { - return c.fileStore.GetActionList(ctx, resServerID, resID, count, 0) + return c.fileStore.GetActionList(ctx, resServerID, resID, kind, count, 0) }, mergeAndDeduplicateActions, ) diff --git a/backend/internal/resource/composite_store_test.go b/backend/internal/resource/composite_store_test.go index 1a94d8606e..1574457e7b 100644 --- a/backend/internal/resource/composite_store_test.go +++ b/backend/internal/resource/composite_store_test.go @@ -879,12 +879,41 @@ func (s *CompositeResourceStoreTestSuite) TestGetActionList_MergesBothStores() { {ID: "act-file1", Name: "File Action 1"}, } - s.dbStoreMock.On("GetActionListCount", s.ctx, "rs1", (*string)(nil)).Return(len(dbActions), nil) - s.fileStoreMock.On("GetActionListCount", s.ctx, "rs1", (*string)(nil)).Return(len(fileActions), nil) - s.dbStoreMock.On("GetActionList", s.ctx, "rs1", (*string)(nil), mock.Anything, 0).Return(dbActions, nil) - s.fileStoreMock.On("GetActionList", s.ctx, "rs1", (*string)(nil), mock.Anything, 0).Return(fileActions, nil) + s.dbStoreMock.On("GetActionListCount", s.ctx, "rs1", (*string)(nil), providers.ActionKind("")). + Return(len(dbActions), nil) + s.fileStoreMock.On("GetActionListCount", s.ctx, "rs1", (*string)(nil), providers.ActionKind("")). + Return(len(fileActions), nil) + s.dbStoreMock.On("GetActionList", s.ctx, "rs1", (*string)(nil), providers.ActionKind(""), mock.Anything, 0). + Return(dbActions, nil) + s.fileStoreMock.On("GetActionList", s.ctx, "rs1", (*string)(nil), providers.ActionKind(""), mock.Anything, 0). + Return(fileActions, nil) - result, err := s.compositeStore.GetActionList(s.ctx, "rs1", nil, 10, 0) + result, err := s.compositeStore.GetActionList(s.ctx, "rs1", nil, "", 10, 0) + + assert.NoError(s.T(), err) + assert.Len(s.T(), result, 2) + s.dbStoreMock.AssertExpectations(s.T()) + s.fileStoreMock.AssertExpectations(s.T()) +} + +func (s *CompositeResourceStoreTestSuite) TestGetActionList_ThreadsKindToBothStores() { + dbActions := []providers.Action{ + {ID: "act-db1", Name: "DB Tool", Kind: providers.ActionKindTool}, + } + fileActions := []providers.Action{ + {ID: "act-file1", Name: "File Tool", Kind: providers.ActionKindTool}, + } + + s.dbStoreMock.On("GetActionListCount", s.ctx, "rs1", (*string)(nil), providers.ActionKindTool). + Return(len(dbActions), nil) + s.fileStoreMock.On("GetActionListCount", s.ctx, "rs1", (*string)(nil), providers.ActionKindTool). + Return(len(fileActions), nil) + s.dbStoreMock.On("GetActionList", s.ctx, "rs1", (*string)(nil), providers.ActionKindTool, mock.Anything, 0). + Return(dbActions, nil) + s.fileStoreMock.On("GetActionList", s.ctx, "rs1", (*string)(nil), providers.ActionKindTool, mock.Anything, 0). + Return(fileActions, nil) + + result, err := s.compositeStore.GetActionList(s.ctx, "rs1", nil, providers.ActionKindTool, 10, 0) assert.NoError(s.T(), err) assert.Len(s.T(), result, 2) @@ -894,11 +923,12 @@ func (s *CompositeResourceStoreTestSuite) TestGetActionList_MergesBothStores() { func (s *CompositeResourceStoreTestSuite) TestGetActionList_DBError() { dbErr := errors.New("db error") - s.dbStoreMock.On("GetActionListCount", s.ctx, "rs1", (*string)(nil)).Return(1, nil) - s.fileStoreMock.On("GetActionListCount", s.ctx, "rs1", (*string)(nil)).Return(0, nil) - s.dbStoreMock.On("GetActionList", s.ctx, "rs1", (*string)(nil), mock.Anything, 0).Return(nil, dbErr) + s.dbStoreMock.On("GetActionListCount", s.ctx, "rs1", (*string)(nil), providers.ActionKind("")).Return(1, nil) + s.fileStoreMock.On("GetActionListCount", s.ctx, "rs1", (*string)(nil), providers.ActionKind("")).Return(0, nil) + s.dbStoreMock.On("GetActionList", s.ctx, "rs1", (*string)(nil), providers.ActionKind(""), mock.Anything, 0). + Return(nil, dbErr) - result, err := s.compositeStore.GetActionList(s.ctx, "rs1", nil, 10, 0) + result, err := s.compositeStore.GetActionList(s.ctx, "rs1", nil, "", 10, 0) assert.Error(s.T(), err) assert.Nil(s.T(), result) @@ -908,12 +938,14 @@ func (s *CompositeResourceStoreTestSuite) TestGetActionList_DBError() { func (s *CompositeResourceStoreTestSuite) TestGetActionList_FileError() { fileErr := errors.New("file error") - s.dbStoreMock.On("GetActionListCount", s.ctx, "rs1", (*string)(nil)).Return(0, nil) - s.fileStoreMock.On("GetActionListCount", s.ctx, "rs1", (*string)(nil)).Return(1, nil) - s.dbStoreMock.On("GetActionList", s.ctx, "rs1", (*string)(nil), mock.Anything, 0).Return([]providers.Action{}, nil) - s.fileStoreMock.On("GetActionList", s.ctx, "rs1", (*string)(nil), mock.Anything, 0).Return(nil, fileErr) + s.dbStoreMock.On("GetActionListCount", s.ctx, "rs1", (*string)(nil), providers.ActionKind("")).Return(0, nil) + s.fileStoreMock.On("GetActionListCount", s.ctx, "rs1", (*string)(nil), providers.ActionKind("")).Return(1, nil) + s.dbStoreMock.On("GetActionList", s.ctx, "rs1", (*string)(nil), providers.ActionKind(""), mock.Anything, 0). + Return([]providers.Action{}, nil) + s.fileStoreMock.On("GetActionList", s.ctx, "rs1", (*string)(nil), providers.ActionKind(""), mock.Anything, 0). + Return(nil, fileErr) - result, err := s.compositeStore.GetActionList(s.ctx, "rs1", nil, 10, 0) + result, err := s.compositeStore.GetActionList(s.ctx, "rs1", nil, "", 10, 0) assert.Error(s.T(), err) assert.Nil(s.T(), result) @@ -931,12 +963,16 @@ func (s *CompositeResourceStoreTestSuite) TestGetActionListCount_SumsBothStores( {ID: "act3", Name: "Action 3"}, } - s.dbStoreMock.On("GetActionListCount", s.ctx, "rs1", (*string)(nil)).Return(len(dbActions), nil) - s.fileStoreMock.On("GetActionListCount", s.ctx, "rs1", (*string)(nil)).Return(len(fileActions), nil) - s.dbStoreMock.On("GetActionList", s.ctx, "rs1", (*string)(nil), mock.Anything, 0).Return(dbActions, nil) - s.fileStoreMock.On("GetActionList", s.ctx, "rs1", (*string)(nil), mock.Anything, 0).Return(fileActions, nil) + s.dbStoreMock.On("GetActionListCount", s.ctx, "rs1", (*string)(nil), providers.ActionKind("")). + Return(len(dbActions), nil) + s.fileStoreMock.On("GetActionListCount", s.ctx, "rs1", (*string)(nil), providers.ActionKind("")). + Return(len(fileActions), nil) + s.dbStoreMock.On("GetActionList", s.ctx, "rs1", (*string)(nil), providers.ActionKind(""), mock.Anything, 0). + Return(dbActions, nil) + s.fileStoreMock.On("GetActionList", s.ctx, "rs1", (*string)(nil), providers.ActionKind(""), mock.Anything, 0). + Return(fileActions, nil) - count, err := s.compositeStore.GetActionListCount(s.ctx, "rs1", nil) + count, err := s.compositeStore.GetActionListCount(s.ctx, "rs1", nil, "") assert.NoError(s.T(), err) // Should return deduplicated count: act1, act2 (from db), act3 @@ -945,11 +981,36 @@ func (s *CompositeResourceStoreTestSuite) TestGetActionListCount_SumsBothStores( s.fileStoreMock.AssertExpectations(s.T()) } +func (s *CompositeResourceStoreTestSuite) TestGetActionListCount_FilteredByKind() { + dbActions := []providers.Action{ + {ID: "act1", Name: "DB Tool", Kind: providers.ActionKindTool}, + } + fileActions := []providers.Action{ + {ID: "act2", Name: "File Tool", Kind: providers.ActionKindTool}, + } + + s.dbStoreMock.On("GetActionListCount", s.ctx, "rs1", (*string)(nil), providers.ActionKindTool). + Return(len(dbActions), nil) + s.fileStoreMock.On("GetActionListCount", s.ctx, "rs1", (*string)(nil), providers.ActionKindTool). + Return(len(fileActions), nil) + s.dbStoreMock.On("GetActionList", s.ctx, "rs1", (*string)(nil), providers.ActionKindTool, mock.Anything, 0). + Return(dbActions, nil) + s.fileStoreMock.On("GetActionList", s.ctx, "rs1", (*string)(nil), providers.ActionKindTool, mock.Anything, 0). + Return(fileActions, nil) + + count, err := s.compositeStore.GetActionListCount(s.ctx, "rs1", nil, providers.ActionKindTool) + + assert.NoError(s.T(), err) + assert.Equal(s.T(), 2, count) + s.dbStoreMock.AssertExpectations(s.T()) + s.fileStoreMock.AssertExpectations(s.T()) +} + func (s *CompositeResourceStoreTestSuite) TestGetActionListCount_DBError() { dbErr := errors.New("db error") - s.dbStoreMock.On("GetActionListCount", s.ctx, "rs1", (*string)(nil)).Return(0, dbErr) + s.dbStoreMock.On("GetActionListCount", s.ctx, "rs1", (*string)(nil), providers.ActionKind("")).Return(0, dbErr) - count, err := s.compositeStore.GetActionListCount(s.ctx, "rs1", nil) + count, err := s.compositeStore.GetActionListCount(s.ctx, "rs1", nil, "") assert.Error(s.T(), err) assert.Equal(s.T(), 0, count) @@ -959,10 +1020,10 @@ func (s *CompositeResourceStoreTestSuite) TestGetActionListCount_DBError() { func (s *CompositeResourceStoreTestSuite) TestGetActionListCount_FileError() { fileErr := errors.New("file error") - s.dbStoreMock.On("GetActionListCount", s.ctx, "rs1", (*string)(nil)).Return(0, nil) - s.fileStoreMock.On("GetActionListCount", s.ctx, "rs1", (*string)(nil)).Return(0, fileErr) + s.dbStoreMock.On("GetActionListCount", s.ctx, "rs1", (*string)(nil), providers.ActionKind("")).Return(0, nil) + s.fileStoreMock.On("GetActionListCount", s.ctx, "rs1", (*string)(nil), providers.ActionKind("")).Return(0, fileErr) - count, err := s.compositeStore.GetActionListCount(s.ctx, "rs1", nil) + count, err := s.compositeStore.GetActionListCount(s.ctx, "rs1", nil, "") assert.Error(s.T(), err) assert.Equal(s.T(), 0, count) diff --git a/backend/internal/resource/declarative_resource.go b/backend/internal/resource/declarative_resource.go index a29f125116..92229d7181 100644 --- a/backend/internal/resource/declarative_resource.go +++ b/backend/internal/resource/declarative_resource.go @@ -109,7 +109,9 @@ func (e *resourceServerExporter) GetResourceByID(ctx context.Context, id string) ID: server.ID, Name: server.Name, Description: server.Description, + Handle: server.Handle, Identifier: server.Identifier, + Type: server.Type, OUID: server.OUID, Delimiter: server.Delimiter, Resources: []providers.Resource{}, @@ -144,7 +146,7 @@ func (e *resourceServerExporter) GetResourceByID(ctx context.Context, id string) // Get actions for this resource actOffset := 0 for { - actions, actErr := e.service.GetActionList(ctx, id, &res.ID, serverconst.MaxPageSize, actOffset) + actions, actErr := e.service.GetActionList(ctx, id, &res.ID, "", serverconst.MaxPageSize, actOffset) if actErr != nil { return nil, "", actErr } @@ -156,6 +158,7 @@ func (e *resourceServerExporter) GetResourceByID(ctx context.Context, id string) Name: action.Name, Handle: action.Handle, Description: action.Description, + Kind: action.Kind, }) } actOffset += len(actions.Actions) @@ -282,6 +285,24 @@ func parseToResourceServer(data []byte) (*providers.ResourceServer, error) { rs.Type = providers.ResourceServerTypeCustom } + // Apply the action kind discriminator rules (mirrors the REST path). The kind is optional for all + // resource server types; MCP actions default to "tool" when omitted, and any provided kind must be + // one of the supported values (tool|resource). + for i := range rs.Resources { + for j := range rs.Resources[i].Actions { + action := &rs.Resources[i].Actions[j] + if rs.Type == providers.ResourceServerTypeMCP && action.Kind == "" { + action.Kind = providers.ActionKindTool + } + if action.Kind != "" && !action.Kind.IsValid() { + return nil, fmt.Errorf( + "action %q in resource server '%s' has invalid kind %q (allowed: tool|resource)", + action.Handle, rs.Name, action.Kind, + ) + } + } + } + return &rs, nil } @@ -317,6 +338,46 @@ func ProcessResourceServer(rs *providers.ResourceServer) error { } } + // For MCP resource servers, a resource (group) and an action (tool/resource) in the same parent + // context can derive an identical permission string under exact-string RBAC, silently collapsing + // two distinct primitives. Mirror the REST cross-entity check (Rule 6) on the declarative path by + // failing on the first duplicate derived permission across all resources and their nested actions. + if rs.Type == providers.ResourceServerTypeMCP { + if err := checkDuplicateMCPPermissions(rs); err != nil { + return err + } + } + + return nil +} + +// checkDuplicateMCPPermissions detects duplicate derived permission strings across all resources +// (groups) and their nested actions (tools/resources) for an MCP resource server. It returns an +// error naming the colliding permission and handles on the first duplicate found. +func checkDuplicateMCPPermissions(rs *providers.ResourceServer) error { + seen := make(map[string]string) + for i := range rs.Resources { + res := &rs.Resources[i] + if existing, ok := seen[res.Permission]; ok { + return fmt.Errorf( + "duplicate permission '%s' derived for handles '%s' and '%s' in resource server '%s'", + res.Permission, existing, res.Handle, rs.ID, + ) + } + seen[res.Permission] = res.Handle + + for j := range res.Actions { + action := &res.Actions[j] + if existing, ok := seen[action.Permission]; ok { + return fmt.Errorf( + "duplicate permission '%s' derived for handles '%s' and '%s' in resource server '%s'", + action.Permission, existing, action.Handle, rs.ID, + ) + } + seen[action.Permission] = action.Handle + } + } + return nil } diff --git a/backend/internal/resource/declarative_resource_test.go b/backend/internal/resource/declarative_resource_test.go index f0ca490c7a..801ad0c519 100644 --- a/backend/internal/resource/declarative_resource_test.go +++ b/backend/internal/resource/declarative_resource_test.go @@ -32,6 +32,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" + "gopkg.in/yaml.v3" ) // ResourceServerExporterTestSuite tests the resourceServerExporter. @@ -159,6 +160,7 @@ func (s *ResourceServerExporterTestSuite) TestGetResourceByID_Success() { Handle: "read", Description: "Read action", Permission: "test-server:resource1:read", + Kind: providers.ActionKindTool, }, }, } @@ -166,7 +168,9 @@ func (s *ResourceServerExporterTestSuite) TestGetResourceByID_Success() { resourceID := "res1" s.mockService.EXPECT().GetResourceServer(ctx, serverID).Return(server, nil) s.mockService.EXPECT().GetAllResourceList(ctx, serverID).Return(resources, nil) - s.mockService.EXPECT().GetActionList(ctx, serverID, &resourceID, serverconst.MaxPageSize, 0).Return(actions, nil) + s.mockService.EXPECT(). + GetActionList(ctx, serverID, &resourceID, providers.ActionKind(""), serverconst.MaxPageSize, 0). + Return(actions, nil) result, name, err := s.exporter.GetResourceByID(ctx, serverID) @@ -180,6 +184,73 @@ func (s *ResourceServerExporterTestSuite) TestGetResourceByID_Success() { assert.Equal(s.T(), "Test Server", dto.Name) assert.Len(s.T(), dto.Resources, 1) assert.Len(s.T(), dto.Resources[0].Actions, 1) + assert.Equal(s.T(), providers.ActionKindTool, dto.Resources[0].Actions[0].Kind) +} + +func (s *ResourceServerExporterTestSuite) TestGetResourceByID_MCPExportImportRoundTrip() { + ctx := context.Background() + serverID := "rs-mcp" + + server := &providers.ResourceServer{ + ID: serverID, + Name: "Booking MCP", + Handle: "booking-mcp", + Identifier: "booking-mcp", + Type: providers.ResourceServerTypeMCP, + OUID: "ou1", + Delimiter: ":", + } + + resources := []providers.Resource{ + { + ID: "res1", + Name: "User Management", + Handle: "user-mgmt", + Parent: nil, + Permission: "booking-mcp:user-mgmt", + }, + } + + actions := &ActionList{ + TotalResults: 1, + Actions: []providers.Action{ + { + ID: "act1", + Name: "Create User", + Handle: "create_user", + Permission: "booking-mcp:user-mgmt:create_user", + Kind: providers.ActionKindTool, + }, + }, + } + + resourceID := "res1" + s.mockService.EXPECT().GetResourceServer(ctx, serverID).Return(server, nil) + s.mockService.EXPECT().GetAllResourceList(ctx, serverID).Return(resources, nil) + s.mockService.EXPECT(). + GetActionList(ctx, serverID, &resourceID, providers.ActionKind(""), serverconst.MaxPageSize, 0). + Return(actions, nil) + + result, _, err := s.exporter.GetResourceByID(ctx, serverID) + assert.Nil(s.T(), err) + + dto, ok := result.(*providers.ResourceServer) + assert.True(s.T(), ok) + + // Marshal the exported DTO to YAML, then re-import it. This guards the export->import + // lossless guarantee: type and handle must survive so the kind-vs-type import validation + // accepts the nested action carrying a kind. + yamlBytes, marshalErr := yaml.Marshal(dto) + assert.NoError(s.T(), marshalErr) + + imported, parseErr := parseToResourceServer(yamlBytes) + s.Require().NoError(parseErr) + s.Require().NotNil(imported) + assert.Equal(s.T(), providers.ResourceServerTypeMCP, imported.Type) + assert.Equal(s.T(), "booking-mcp", imported.Handle) + assert.Len(s.T(), imported.Resources, 1) + assert.Len(s.T(), imported.Resources[0].Actions, 1) + assert.Equal(s.T(), providers.ActionKindTool, imported.Resources[0].Actions[0].Kind) } func (s *ResourceServerExporterTestSuite) TestGetResourceByID_ServerNotFound() { @@ -340,6 +411,119 @@ ouId: "ou1" assert.Contains(t, err.Error(), "invalid type") } +func TestParseToResourceServer_MCPActionDefaultsKindToTool(t *testing.T) { + yamlData := []byte(` +id: "rs1" +name: "Test Server" +type: "MCP" +ouId: "ou1" +resources: + - name: "Users" + handle: "users" + actions: + - name: "Read" + handle: "read" +`) + + dto, err := parseToResourceServer(yamlData) + + assert.NoError(t, err) + assert.NotNil(t, dto) + assert.Equal(t, providers.ActionKindTool, dto.Resources[0].Actions[0].Kind) +} + +func TestParseToResourceServer_MCPActionWithKindSucceeds(t *testing.T) { + yamlData := []byte(` +id: "rs1" +name: "Test Server" +type: "MCP" +ouId: "ou1" +resources: + - name: "Users" + handle: "users" + actions: + - name: "Read" + handle: "read" + kind: "resource" + - name: "Create" + handle: "create" + kind: "tool" +`) + + dto, err := parseToResourceServer(yamlData) + + assert.NoError(t, err) + assert.NotNil(t, dto) + assert.Equal(t, providers.ActionKindResource, dto.Resources[0].Actions[0].Kind) + assert.Equal(t, providers.ActionKindTool, dto.Resources[0].Actions[1].Kind) +} + +func TestParseToResourceServer_NonMCPActionAllowsKind(t *testing.T) { + yamlData := []byte(` +id: "rs1" +name: "Test Server" +type: "API" +ouId: "ou1" +resources: + - name: "Users" + handle: "users" + actions: + - name: "Read" + handle: "read" + kind: "tool" +`) + + dto, err := parseToResourceServer(yamlData) + + assert.NoError(t, err) + assert.NotNil(t, dto) + assert.Equal(t, providers.ActionKindTool, dto.Resources[0].Actions[0].Kind) +} + +func TestParseToResourceServer_NonMCPActionNoKindStaysEmpty(t *testing.T) { + yamlData := []byte(` +id: "rs1" +name: "Test Server" +type: "CUSTOM" +ouId: "ou1" +resources: + - name: "Users" + handle: "users" + actions: + - name: "Read" + handle: "read" +`) + + dto, err := parseToResourceServer(yamlData) + + assert.NoError(t, err) + assert.NotNil(t, dto) + assert.Equal(t, providers.ActionKind(""), dto.Resources[0].Actions[0].Kind) +} + +func TestParseToResourceServer_ActionInvalidKindRejected(t *testing.T) { + yamlData := []byte(` +id: "rs1" +name: "Test Server" +type: "MCP" +ouId: "ou1" +resources: + - name: "Users" + handle: "users" + actions: + - name: "Read" + handle: "read" + kind: "prompt" +`) + + dto, err := parseToResourceServer(yamlData) + + assert.Error(t, err) + assert.Nil(t, dto) + assert.Contains(t, err.Error(), "read") + assert.Contains(t, err.Error(), "invalid kind") +} + func TestParseAndValidateResourceServerWrapper_TypeMCP(t *testing.T) { yamlData := []byte(` id: "rs1" @@ -518,6 +702,135 @@ func TestProcessResourceServer_DuplicateHandle(t *testing.T) { assert.Contains(t, err.Error(), "duplicate resource handle") } +func TestProcessResourceServer_MCPActionCollidesWithGroupPermission(t *testing.T) { + // An MCP resource server where a group (RESOURCE) nested under "ops" shares its handle with a + // tool (ACTION) nested under the same "ops" group: both derive "booking-mcp:ops:deploy". + rs := &providers.ResourceServer{ + ID: "rs-mcp", + Name: "Booking MCP", + Handle: "booking-mcp", + OUID: "ou1", + Type: providers.ResourceServerTypeMCP, + Resources: []providers.Resource{ + { + Name: "Ops", + Handle: "ops", + Actions: []providers.Action{ + {Name: "Deploy", Handle: "deploy", Kind: providers.ActionKindTool}, + }, + }, + { + Name: "Deploy Group", + Handle: "deploy", + ParentHandle: "ops", + }, + }, + } + + err := ProcessResourceServer(rs) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "duplicate permission") + assert.Contains(t, err.Error(), "booking-mcp:ops:deploy") +} + +func TestProcessResourceServer_MCPActionCollidesWithNestedGroupPermission(t *testing.T) { + // A tool nested under group "a" derives "mcp:a:b". A child group "b" nested under "a" derives the + // same "mcp:a:b". The cross-entity collision is caught even though the two collide via different + // nesting paths rather than at the same level. + rs := &providers.ResourceServer{ + ID: "rs-mcp", + Name: "Booking MCP", + Handle: "mcp", + OUID: "ou1", + Type: providers.ResourceServerTypeMCP, + Resources: []providers.Resource{ + { + Name: "Group A", + Handle: "a", + Actions: []providers.Action{ + {Name: "Tool B", Handle: "b", Kind: providers.ActionKindTool}, + }, + }, + { + Name: "Group B", + Handle: "b", + ParentHandle: "a", + }, + }, + } + + err := ProcessResourceServer(rs) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "duplicate permission") + assert.Contains(t, err.Error(), "mcp:a:b") +} + +func TestProcessResourceServer_MCPNoCollisionSucceeds(t *testing.T) { + rs := &providers.ResourceServer{ + ID: "rs-mcp", + Name: "Booking MCP", + Handle: "booking-mcp", + OUID: "ou1", + Type: providers.ResourceServerTypeMCP, + Resources: []providers.Resource{ + { + Name: "Ops", + Handle: "ops", + Actions: []providers.Action{ + {Name: "Deploy", Handle: "deploy", Kind: providers.ActionKindTool}, + {Name: "Status", Handle: "status", Kind: providers.ActionKindResource}, + }, + }, + { + Name: "Users", + Handle: "users", + Actions: []providers.Action{ + {Name: "Create", Handle: "create", Kind: providers.ActionKindTool}, + }, + }, + }, + } + + err := ProcessResourceServer(rs) + + assert.NoError(t, err) + assert.Equal(t, "booking-mcp:ops:deploy", rs.Resources[0].Actions[0].Permission) + assert.Equal(t, "booking-mcp:ops:status", rs.Resources[0].Actions[1].Permission) + assert.Equal(t, "booking-mcp:users:create", rs.Resources[1].Actions[0].Permission) +} + +func TestProcessResourceServer_NonMCPSkipsPermissionCollisionCheck(t *testing.T) { + // An API resource server with the same structure that collides for MCP must still succeed, + // since Rule 6 (cross-entity permission collision) applies only to MCP-type resource servers. + rs := &providers.ResourceServer{ + ID: "rs-api", + Name: "Booking API", + Handle: "booking-api", + OUID: "ou1", + Type: providers.ResourceServerTypeAPI, + Resources: []providers.Resource{ + { + Name: "Ops", + Handle: "ops", + Actions: []providers.Action{ + {Name: "Deploy", Handle: "deploy"}, + }, + }, + { + Name: "Deploy Group", + Handle: "deploy", + ParentHandle: "ops", + }, + }, + } + + err := ProcessResourceServer(rs) + + assert.NoError(t, err) +} + func TestProcessResource_SetsPermissions(t *testing.T) { root := &providers.Resource{Handle: "root"} resource := &providers.Resource{ @@ -578,6 +891,63 @@ resources: assert.Equal(t, "test-api:users:profile", rs.Resources[1].Permission) } +func TestParseAndValidateResourceServerWrapper_MCPPermissionCollisionRejected(t *testing.T) { + yamlData := []byte(` +id: "rs-mcp" +name: "Booking MCP" +handle: "booking-mcp" +type: "MCP" +ouId: "ou1" +resources: + - name: "Ops" + handle: "ops" + actions: + - name: "Deploy" + handle: "deploy" + kind: "tool" + - name: "Deploy Group" + handle: "deploy" + parent: "ops" +`) + + parser := parseAndValidateResourceServerWrapper(nil) + result, err := parser(yamlData) + + assert.Error(t, err) + assert.Nil(t, result) + assert.Contains(t, err.Error(), "duplicate permission") + assert.Contains(t, err.Error(), "booking-mcp:ops:deploy") +} + +func TestParseAndValidateResourceServerWrapper_MCPCleanSucceeds(t *testing.T) { + yamlData := []byte(` +id: "rs-mcp" +name: "Booking MCP" +handle: "booking-mcp" +type: "MCP" +ouId: "ou1" +resources: + - name: "Ops" + handle: "ops" + actions: + - name: "Deploy" + handle: "deploy" + kind: "tool" + - name: "Status" + handle: "status" + kind: "resource" +`) + + parser := parseAndValidateResourceServerWrapper(nil) + result, err := parser(yamlData) + + assert.NoError(t, err) + rs, ok := result.(*providers.ResourceServer) + assert.True(t, ok) + assert.Equal(t, "booking-mcp:ops:deploy", rs.Resources[0].Actions[0].Permission) + assert.Equal(t, "booking-mcp:ops:status", rs.Resources[0].Actions[1].Permission) +} + func TestParseAndValidateResourceServerWrapper_InvalidYAML(t *testing.T) { yamlData := []byte(`::invalid`) diff --git a/backend/internal/resource/file_based_store.go b/backend/internal/resource/file_based_store.go index 6e65801222..877db273b0 100644 --- a/backend/internal/resource/file_based_store.go +++ b/backend/internal/resource/file_based_store.go @@ -483,7 +483,8 @@ func (f *fileBasedResourceStore) GetAction( } func (f *fileBasedResourceStore) GetActionList( - ctx context.Context, resServerID string, resID *string, limit, offset int) ([]providers.Action, error) { + ctx context.Context, resServerID string, resID *string, kind providers.ActionKind, limit, offset int, +) ([]providers.Action, error) { list, err := f.GenericFileBasedStore.List() if err != nil { return nil, err @@ -510,6 +511,9 @@ func (f *fileBasedResourceStore) GetActionList( // Add all actions from this resource for _, action := range res.Actions { + if kind != "" && action.Kind != kind { + continue + } actionID := fmt.Sprintf("%s_%s_%s", rs.ID, res.Handle, action.Handle) actions = append(actions, providers.Action{ ID: actionID, @@ -517,6 +521,7 @@ func (f *fileBasedResourceStore) GetActionList( Handle: action.Handle, Description: action.Description, Permission: action.Permission, + Kind: action.Kind, }) } } @@ -543,8 +548,8 @@ func (f *fileBasedResourceStore) GetActionList( } func (f *fileBasedResourceStore) GetActionListCount( - ctx context.Context, resServerID string, resID *string) (int, error) { - actions, err := f.GetActionList(ctx, resServerID, resID, 1000, 0) + ctx context.Context, resServerID string, resID *string, kind providers.ActionKind) (int, error) { + actions, err := f.GetActionList(ctx, resServerID, resID, kind, 1000, 0) if err != nil { return 0, err } diff --git a/backend/internal/resource/file_based_store_test.go b/backend/internal/resource/file_based_store_test.go index 435361e2ef..087073ab5c 100644 --- a/backend/internal/resource/file_based_store_test.go +++ b/backend/internal/resource/file_based_store_test.go @@ -396,23 +396,80 @@ func (s *FileBasedResourceStoreTestSuite) TestGetActionListAndCounts_WithData() assert.NoError(s.T(), err) assert.Equal(s.T(), rootActionID, action.ID) - actions, err := s.store.GetActionList(s.ctx, "rs-data", nil, 10, 0) + actions, err := s.store.GetActionList(s.ctx, "rs-data", nil, "", 10, 0) assert.NoError(s.T(), err) assert.Len(s.T(), actions, 2) - actions, err = s.store.GetActionList(s.ctx, "rs-data", &rootID, 10, 0) + actions, err = s.store.GetActionList(s.ctx, "rs-data", &rootID, "", 10, 0) assert.NoError(s.T(), err) assert.Len(s.T(), actions, 1) - count, err := s.store.GetActionListCount(s.ctx, "rs-data", nil) + count, err := s.store.GetActionListCount(s.ctx, "rs-data", nil, "") assert.NoError(s.T(), err) assert.Equal(s.T(), 2, count) - count, err = s.store.GetActionListCount(s.ctx, "rs-data", &rootID) + count, err = s.store.GetActionListCount(s.ctx, "rs-data", &rootID, "") assert.NoError(s.T(), err) assert.Equal(s.T(), 1, count) } +func (s *FileBasedResourceStoreTestSuite) TestGetActionList_FilterByKind() { + fileStore, ok := s.store.(*fileBasedResourceStore) + assert.True(s.T(), ok) + + rs := &providers.ResourceServer{ + ID: "rs-mcp", + Name: "MCP Server", + OUID: "ou1", + Delimiter: ":", + Resources: []providers.Resource{ + { + Name: "Root", + Handle: "root", + Actions: []providers.Action{ + {Name: "Create User", Handle: "create_user", Kind: providers.ActionKindTool}, + {Name: "User List", Handle: "user_list", Kind: providers.ActionKindResource}, + }, + }, + }, + } + + err := fileStore.Create("rs-mcp", rs) + assert.NoError(s.T(), err) + + // Empty kind returns all actions. + all, err := s.store.GetActionList(s.ctx, "rs-mcp", nil, "", 10, 0) + assert.NoError(s.T(), err) + assert.Len(s.T(), all, 2) + + // Tool kind returns only the tool action. + tools, err := s.store.GetActionList(s.ctx, "rs-mcp", nil, providers.ActionKindTool, 10, 0) + assert.NoError(s.T(), err) + assert.Len(s.T(), tools, 1) + assert.Equal(s.T(), "create_user", tools[0].Handle) + assert.Equal(s.T(), providers.ActionKindTool, tools[0].Kind) + + // Resource kind returns only the resource action. + resources, err := s.store.GetActionList(s.ctx, "rs-mcp", nil, providers.ActionKindResource, 10, 0) + assert.NoError(s.T(), err) + assert.Len(s.T(), resources, 1) + assert.Equal(s.T(), "user_list", resources[0].Handle) + assert.Equal(s.T(), providers.ActionKindResource, resources[0].Kind) + + // The count honors the kind filter and matches the filtered list length. + allCount, err := s.store.GetActionListCount(s.ctx, "rs-mcp", nil, "") + assert.NoError(s.T(), err) + assert.Equal(s.T(), len(all), allCount) + + toolCount, err := s.store.GetActionListCount(s.ctx, "rs-mcp", nil, providers.ActionKindTool) + assert.NoError(s.T(), err) + assert.Equal(s.T(), len(tools), toolCount) + + resourceCount, err := s.store.GetActionListCount(s.ctx, "rs-mcp", nil, providers.ActionKindResource) + assert.NoError(s.T(), err) + assert.Equal(s.T(), len(resources), resourceCount) +} + func (s *FileBasedResourceStoreTestSuite) TestCheckActionHandleExists_WithData() { fileStore, ok := s.store.(*fileBasedResourceStore) assert.True(s.T(), ok) diff --git a/backend/internal/resource/handler.go b/backend/internal/resource/handler.go index 41cce6b14a..05f2274147 100644 --- a/backend/internal/resource/handler.go +++ b/backend/internal/resource/handler.go @@ -305,7 +305,13 @@ func (h *resourceHandler) HandleActionListAtResourceServerRequest(w http.Respons return } - result, svcErr := h.resourceService.GetActionList(ctx, rsID, nil, limit, offset) + kind, svcErr := parseActionKindParam(r.URL.Query()) + if svcErr != nil { + handleError(ctx, w, svcErr) + return + } + + result, svcErr := h.resourceService.GetActionList(ctx, rsID, nil, kind, limit, offset) if svcErr != nil { handleError(ctx, w, svcErr) return @@ -335,6 +341,7 @@ func (h *resourceHandler) HandleActionPostAtResourceServerRequest(w http.Respons Name: sanitized.Name, Handle: sanitized.Handle, Description: sanitized.Description, + Kind: sanitized.Kind, } result, svcErr := h.resourceService.CreateAction(ctx, rsID, nil, serviceReq) @@ -424,7 +431,13 @@ func (h *resourceHandler) HandleActionListAtResourceRequest(w http.ResponseWrite return } - result, svcErr := h.resourceService.GetActionList(ctx, rsID, &resourceID, limit, offset) + kind, svcErr := parseActionKindParam(r.URL.Query()) + if svcErr != nil { + handleError(ctx, w, svcErr) + return + } + + result, svcErr := h.resourceService.GetActionList(ctx, rsID, &resourceID, kind, limit, offset) if svcErr != nil { handleError(ctx, w, svcErr) return @@ -456,6 +469,7 @@ func (h *resourceHandler) HandleActionPostAtResourceRequest(w http.ResponseWrite Name: sanitized.Name, Handle: sanitized.Handle, Description: sanitized.Description, + Kind: sanitized.Kind, } result, svcErr := h.resourceService.CreateAction(ctx, rsID, &resourceID, serviceReq) @@ -556,6 +570,20 @@ func parsePaginationParams(query url.Values) (int, int, *tidcommon.ServiceError) return limit, offset, nil } +// parseActionKindParam parses the optional 'kind' query parameter for action-list endpoints. +// An omitted parameter yields an empty kind (no filter); any value other than the supported action +// kinds (tool|resource) is rejected with ErrorInvalidRequestFormat. +func parseActionKindParam(query url.Values) (providers.ActionKind, *tidcommon.ServiceError) { + if !query.Has("kind") { + return "", nil + } + kind := providers.ActionKind(query.Get("kind")) + if !kind.IsValid() { + return "", &ErrorInvalidRequestFormat + } + return kind, nil +} + // handleError writes an error response based on the provided service error. func handleError(ctx context.Context, w http.ResponseWriter, svcErr *tidcommon.ServiceError) { statusCode := http.StatusInternalServerError @@ -636,6 +664,7 @@ func sanitizeCreateActionRequest(req *CreateActionRequest) CreateActionRequest { Name: sysutils.SanitizeString(req.Name), Handle: sysutils.SanitizeString(req.Handle), Description: sysutils.SanitizeString(req.Description), + Kind: req.Kind, } } @@ -730,6 +759,7 @@ func toActionResponse(action *providers.Action) *ActionResponse { Handle: action.Handle, Description: action.Description, Permission: action.Permission, + Kind: action.Kind, } } diff --git a/backend/internal/resource/handler_test.go b/backend/internal/resource/handler_test.go index c31646aafe..66b7bf9eb7 100644 --- a/backend/internal/resource/handler_test.go +++ b/backend/internal/resource/handler_test.go @@ -401,7 +401,7 @@ func (suite *HandlerTestSuite) TestHandleActionListAtResourceServerRequest_Succe } var nilResourceID *string suite.mockService.On("GetActionList", mock.Anything, - "rs-123", nilResourceID, 30, 0).Return(&ActionList{ + "rs-123", nilResourceID, providers.ActionKind(""), 30, 0).Return(&ActionList{ TotalResults: 2, StartIndex: 1, Count: 2, @@ -422,6 +422,83 @@ func (suite *HandlerTestSuite) TestHandleActionListAtResourceServerRequest_Succe suite.Equal(2, resp.TotalResults) } +func (suite *HandlerTestSuite) TestHandleActionListAtResourceServerRequest_WithKindFilter() { + var nilResourceID *string + suite.mockService.On("GetActionList", mock.Anything, + "rs-123", nilResourceID, providers.ActionKindTool, 30, 0).Return(&ActionList{ + TotalResults: 1, + StartIndex: 1, + Count: 1, + Actions: []providers.Action{{ID: "action-1", Name: "Tool 1", Kind: providers.ActionKindTool}}, + }, nil) + + req := httptest.NewRequest("GET", "/resource-servers/rs-123/actions?kind=tool", nil) + req.SetPathValue("rsId", "rs-123") + w := httptest.NewRecorder() + + suite.handler.HandleActionListAtResourceServerRequest(w, req) + + suite.Equal(http.StatusOK, w.Code) + suite.mockService.AssertExpectations(suite.T()) +} + +func (suite *HandlerTestSuite) TestHandleActionListAtResourceServerRequest_InvalidKind() { + req := httptest.NewRequest("GET", "/resource-servers/rs-123/actions?kind=bogus", nil) + req.SetPathValue("rsId", "rs-123") + w := httptest.NewRecorder() + + suite.handler.HandleActionListAtResourceServerRequest(w, req) + + suite.Equal(http.StatusBadRequest, w.Code) + suite.mockService.AssertNotCalled(suite.T(), "GetActionList") +} + +func (suite *HandlerTestSuite) TestHandleActionListAtResourceServerRequest_KindPromptRejected() { + req := httptest.NewRequest("GET", "/resource-servers/rs-123/actions?kind=prompt", nil) + req.SetPathValue("rsId", "rs-123") + w := httptest.NewRecorder() + + suite.handler.HandleActionListAtResourceServerRequest(w, req) + + suite.Equal(http.StatusBadRequest, w.Code) + suite.mockService.AssertNotCalled(suite.T(), "GetActionList") +} + +func (suite *HandlerTestSuite) TestHandleActionListAtResourceRequest_WithKindFilter() { + resourceID := testResourceID + suite.mockService.On("GetActionList", mock.Anything, + "rs-123", &resourceID, providers.ActionKindResource, 30, 0).Return(&ActionList{ + TotalResults: 1, + Actions: []providers.Action{{ID: "action-1", Name: "Resource 1", Kind: providers.ActionKindResource}}, + }, nil) + + req := httptest.NewRequest( + "GET", "/resource-servers/rs-123/resources/res-123/actions?kind=resource", nil, + ) + req.SetPathValue("rsId", "rs-123") + req.SetPathValue("resourceId", testResourceID) + w := httptest.NewRecorder() + + suite.handler.HandleActionListAtResourceRequest(w, req) + + suite.Equal(http.StatusOK, w.Code) + suite.mockService.AssertExpectations(suite.T()) +} + +func (suite *HandlerTestSuite) TestHandleActionListAtResourceRequest_InvalidKind() { + req := httptest.NewRequest( + "GET", "/resource-servers/rs-123/resources/res-123/actions?kind=bogus", nil, + ) + req.SetPathValue("rsId", "rs-123") + req.SetPathValue("resourceId", testResourceID) + w := httptest.NewRecorder() + + suite.handler.HandleActionListAtResourceRequest(w, req) + + suite.Equal(http.StatusBadRequest, w.Code) + suite.mockService.AssertNotCalled(suite.T(), "GetActionList") +} + func (suite *HandlerTestSuite) TestHandleActionPostAtResourceServerRequest_Success() { reqBody := CreateActionRequest{ Name: "test-action", @@ -451,6 +528,40 @@ func (suite *HandlerTestSuite) TestHandleActionPostAtResourceServerRequest_Succe suite.Equal("test-handle", resp.Handle) } +func (suite *HandlerTestSuite) TestHandleActionPostAtResourceServerRequest_WithKind() { + reqBody := CreateActionRequest{ + Name: "test-tool", + Handle: "test-tool-handle", + Kind: providers.ActionKindTool, + } + + var nilResourceID *string + suite.mockService.On("CreateAction", mock.Anything, + "rs-123", nilResourceID, mock.MatchedBy(func(a providers.Action) bool { + return a.Kind == providers.ActionKindTool + })).Return(&providers.Action{ + ID: "action-123", + Name: "test-tool", + Handle: "test-tool-handle", + Kind: providers.ActionKindTool, + }, nil) + + body, _ := json.Marshal(reqBody) + req := httptest.NewRequest("POST", "/resource-servers/rs-123/actions", bytes.NewReader(body)) + req.SetPathValue("rsId", "rs-123") + w := httptest.NewRecorder() + + suite.handler.HandleActionPostAtResourceServerRequest(w, req) + + suite.Equal(http.StatusCreated, w.Code) + suite.Contains(w.Body.String(), `"kind":"tool"`) + var resp ActionResponse + err := json.Unmarshal(w.Body.Bytes(), &resp) + suite.NoError(err) + suite.Equal(providers.ActionKindTool, resp.Kind) + suite.mockService.AssertExpectations(suite.T()) +} + func (suite *HandlerTestSuite) TestHandleActionGetAtResourceServerRequest_Success() { var nilResourceID *string suite.mockService.On("GetAction", mock.Anything, @@ -517,7 +628,7 @@ func (suite *HandlerTestSuite) TestHandleActionListAtResourceRequest_Success() { resourceID := testResourceID suite.mockService.On("GetActionList", mock.Anything, - "rs-123", &resourceID, 30, 0).Return(&ActionList{ + "rs-123", &resourceID, providers.ActionKind(""), 30, 0).Return(&ActionList{ TotalResults: 1, Actions: actions, }, nil) @@ -1063,7 +1174,7 @@ func (suite *HandlerTestSuite) TestHandleActionListAtResourceServerRequest_Inval func (suite *HandlerTestSuite) TestHandleActionListAtResourceServerRequest_ServiceError() { var nilResourceID *string suite.mockService.On("GetActionList", mock.Anything, - "rs-123", nilResourceID, 30, 0).Return(nil, &tidcommon.InternalServerError) + "rs-123", nilResourceID, providers.ActionKind(""), 30, 0).Return(nil, &tidcommon.InternalServerError) req := httptest.NewRequest("GET", "/resource-servers/rs-123/actions", nil) req.SetPathValue("rsId", "rs-123") @@ -1201,7 +1312,7 @@ func (suite *HandlerTestSuite) TestHandleActionListAtResourceRequest_InvalidLimi func (suite *HandlerTestSuite) TestHandleActionListAtResourceRequest_ServiceError() { resourceID := testResourceID suite.mockService.On("GetActionList", mock.Anything, - "rs-123", &resourceID, 30, 0). + "rs-123", &resourceID, providers.ActionKind(""), 30, 0). Return(nil, &tidcommon.InternalServerError) req := httptest.NewRequest("GET", "/resource-servers/rs-123/resources/res-123/actions", nil) diff --git a/backend/internal/resource/model.go b/backend/internal/resource/model.go index 7a4381c6fd..e8a906c647 100644 --- a/backend/internal/resource/model.go +++ b/backend/internal/resource/model.go @@ -47,11 +47,12 @@ type ResourceResponse struct { // ActionResponse represents an action. type ActionResponse struct { - ID string `json:"id"` - Name string `json:"name"` - Handle string `json:"handle"` - Description string `json:"description,omitempty"` - Permission string `json:"permission"` + ID string `json:"id"` + Name string `json:"name"` + Handle string `json:"handle"` + Description string `json:"description,omitempty"` + Permission string `json:"permission"` + Kind providers.ActionKind `json:"kind,omitempty"` } // LinkResponse represents a pagination link. @@ -123,9 +124,10 @@ type UpdateResourceRequest struct { // CreateActionRequest represents the request to create an action. type CreateActionRequest struct { - Name string `json:"name" native:"required"` - Handle string `json:"handle" native:"required,max=50"` - Description string `json:"description,omitempty"` + Name string `json:"name" native:"required"` + Handle string `json:"handle" native:"required,max=100"` + Description string `json:"description,omitempty"` + Kind providers.ActionKind `json:"kind,omitempty"` } // UpdateActionRequest represents the request to update an action. diff --git a/backend/internal/resource/resourceStoreInterface_mock_test.go b/backend/internal/resource/resourceStoreInterface_mock_test.go index 42248f4d1b..9eae16e385 100644 --- a/backend/internal/resource/resourceStoreInterface_mock_test.go +++ b/backend/internal/resource/resourceStoreInterface_mock_test.go @@ -1145,8 +1145,8 @@ func (_c *resourceStoreInterfaceMock_GetAction_Call) RunAndReturn(run func(ctx c } // GetActionList provides a mock function for the type resourceStoreInterfaceMock -func (_mock *resourceStoreInterfaceMock) GetActionList(ctx context.Context, resServerID string, resID *string, limit int, offset int) ([]providers.Action, error) { - ret := _mock.Called(ctx, resServerID, resID, limit, offset) +func (_mock *resourceStoreInterfaceMock) GetActionList(ctx context.Context, resServerID string, resID *string, kind providers.ActionKind, limit int, offset int) ([]providers.Action, error) { + ret := _mock.Called(ctx, resServerID, resID, kind, limit, offset) if len(ret) == 0 { panic("no return value specified for GetActionList") @@ -1154,18 +1154,18 @@ func (_mock *resourceStoreInterfaceMock) GetActionList(ctx context.Context, resS var r0 []providers.Action var r1 error - if returnFunc, ok := ret.Get(0).(func(context.Context, string, *string, int, int) ([]providers.Action, error)); ok { - return returnFunc(ctx, resServerID, resID, limit, offset) + if returnFunc, ok := ret.Get(0).(func(context.Context, string, *string, providers.ActionKind, int, int) ([]providers.Action, error)); ok { + return returnFunc(ctx, resServerID, resID, kind, limit, offset) } - if returnFunc, ok := ret.Get(0).(func(context.Context, string, *string, int, int) []providers.Action); ok { - r0 = returnFunc(ctx, resServerID, resID, limit, offset) + if returnFunc, ok := ret.Get(0).(func(context.Context, string, *string, providers.ActionKind, int, int) []providers.Action); ok { + r0 = returnFunc(ctx, resServerID, resID, kind, limit, offset) } else { if ret.Get(0) != nil { r0 = ret.Get(0).([]providers.Action) } } - if returnFunc, ok := ret.Get(1).(func(context.Context, string, *string, int, int) error); ok { - r1 = returnFunc(ctx, resServerID, resID, limit, offset) + if returnFunc, ok := ret.Get(1).(func(context.Context, string, *string, providers.ActionKind, int, int) error); ok { + r1 = returnFunc(ctx, resServerID, resID, kind, limit, offset) } else { r1 = ret.Error(1) } @@ -1181,13 +1181,14 @@ type resourceStoreInterfaceMock_GetActionList_Call struct { // - ctx context.Context // - resServerID string // - resID *string +// - kind providers.ActionKind // - limit int // - offset int -func (_e *resourceStoreInterfaceMock_Expecter) GetActionList(ctx interface{}, resServerID interface{}, resID interface{}, limit interface{}, offset interface{}) *resourceStoreInterfaceMock_GetActionList_Call { - return &resourceStoreInterfaceMock_GetActionList_Call{Call: _e.mock.On("GetActionList", ctx, resServerID, resID, limit, offset)} +func (_e *resourceStoreInterfaceMock_Expecter) GetActionList(ctx interface{}, resServerID interface{}, resID interface{}, kind interface{}, limit interface{}, offset interface{}) *resourceStoreInterfaceMock_GetActionList_Call { + return &resourceStoreInterfaceMock_GetActionList_Call{Call: _e.mock.On("GetActionList", ctx, resServerID, resID, kind, limit, offset)} } -func (_c *resourceStoreInterfaceMock_GetActionList_Call) Run(run func(ctx context.Context, resServerID string, resID *string, limit int, offset int)) *resourceStoreInterfaceMock_GetActionList_Call { +func (_c *resourceStoreInterfaceMock_GetActionList_Call) Run(run func(ctx context.Context, resServerID string, resID *string, kind providers.ActionKind, limit int, offset int)) *resourceStoreInterfaceMock_GetActionList_Call { _c.Call.Run(func(args mock.Arguments) { var arg0 context.Context if args[0] != nil { @@ -1201,20 +1202,25 @@ func (_c *resourceStoreInterfaceMock_GetActionList_Call) Run(run func(ctx contex if args[2] != nil { arg2 = args[2].(*string) } - var arg3 int + var arg3 providers.ActionKind if args[3] != nil { - arg3 = args[3].(int) + arg3 = args[3].(providers.ActionKind) } var arg4 int if args[4] != nil { arg4 = args[4].(int) } + var arg5 int + if args[5] != nil { + arg5 = args[5].(int) + } run( arg0, arg1, arg2, arg3, arg4, + arg5, ) }) return _c @@ -1225,14 +1231,14 @@ func (_c *resourceStoreInterfaceMock_GetActionList_Call) Return(actions []provid return _c } -func (_c *resourceStoreInterfaceMock_GetActionList_Call) RunAndReturn(run func(ctx context.Context, resServerID string, resID *string, limit int, offset int) ([]providers.Action, error)) *resourceStoreInterfaceMock_GetActionList_Call { +func (_c *resourceStoreInterfaceMock_GetActionList_Call) RunAndReturn(run func(ctx context.Context, resServerID string, resID *string, kind providers.ActionKind, limit int, offset int) ([]providers.Action, error)) *resourceStoreInterfaceMock_GetActionList_Call { _c.Call.Return(run) return _c } // GetActionListCount provides a mock function for the type resourceStoreInterfaceMock -func (_mock *resourceStoreInterfaceMock) GetActionListCount(ctx context.Context, resServerID string, resID *string) (int, error) { - ret := _mock.Called(ctx, resServerID, resID) +func (_mock *resourceStoreInterfaceMock) GetActionListCount(ctx context.Context, resServerID string, resID *string, kind providers.ActionKind) (int, error) { + ret := _mock.Called(ctx, resServerID, resID, kind) if len(ret) == 0 { panic("no return value specified for GetActionListCount") @@ -1240,16 +1246,16 @@ func (_mock *resourceStoreInterfaceMock) GetActionListCount(ctx context.Context, var r0 int var r1 error - if returnFunc, ok := ret.Get(0).(func(context.Context, string, *string) (int, error)); ok { - return returnFunc(ctx, resServerID, resID) + if returnFunc, ok := ret.Get(0).(func(context.Context, string, *string, providers.ActionKind) (int, error)); ok { + return returnFunc(ctx, resServerID, resID, kind) } - if returnFunc, ok := ret.Get(0).(func(context.Context, string, *string) int); ok { - r0 = returnFunc(ctx, resServerID, resID) + if returnFunc, ok := ret.Get(0).(func(context.Context, string, *string, providers.ActionKind) int); ok { + r0 = returnFunc(ctx, resServerID, resID, kind) } else { r0 = ret.Get(0).(int) } - if returnFunc, ok := ret.Get(1).(func(context.Context, string, *string) error); ok { - r1 = returnFunc(ctx, resServerID, resID) + if returnFunc, ok := ret.Get(1).(func(context.Context, string, *string, providers.ActionKind) error); ok { + r1 = returnFunc(ctx, resServerID, resID, kind) } else { r1 = ret.Error(1) } @@ -1265,11 +1271,12 @@ type resourceStoreInterfaceMock_GetActionListCount_Call struct { // - ctx context.Context // - resServerID string // - resID *string -func (_e *resourceStoreInterfaceMock_Expecter) GetActionListCount(ctx interface{}, resServerID interface{}, resID interface{}) *resourceStoreInterfaceMock_GetActionListCount_Call { - return &resourceStoreInterfaceMock_GetActionListCount_Call{Call: _e.mock.On("GetActionListCount", ctx, resServerID, resID)} +// - kind providers.ActionKind +func (_e *resourceStoreInterfaceMock_Expecter) GetActionListCount(ctx interface{}, resServerID interface{}, resID interface{}, kind interface{}) *resourceStoreInterfaceMock_GetActionListCount_Call { + return &resourceStoreInterfaceMock_GetActionListCount_Call{Call: _e.mock.On("GetActionListCount", ctx, resServerID, resID, kind)} } -func (_c *resourceStoreInterfaceMock_GetActionListCount_Call) Run(run func(ctx context.Context, resServerID string, resID *string)) *resourceStoreInterfaceMock_GetActionListCount_Call { +func (_c *resourceStoreInterfaceMock_GetActionListCount_Call) Run(run func(ctx context.Context, resServerID string, resID *string, kind providers.ActionKind)) *resourceStoreInterfaceMock_GetActionListCount_Call { _c.Call.Run(func(args mock.Arguments) { var arg0 context.Context if args[0] != nil { @@ -1283,10 +1290,15 @@ func (_c *resourceStoreInterfaceMock_GetActionListCount_Call) Run(run func(ctx c if args[2] != nil { arg2 = args[2].(*string) } + var arg3 providers.ActionKind + if args[3] != nil { + arg3 = args[3].(providers.ActionKind) + } run( arg0, arg1, arg2, + arg3, ) }) return _c @@ -1297,7 +1309,7 @@ func (_c *resourceStoreInterfaceMock_GetActionListCount_Call) Return(n int, err return _c } -func (_c *resourceStoreInterfaceMock_GetActionListCount_Call) RunAndReturn(run func(ctx context.Context, resServerID string, resID *string) (int, error)) *resourceStoreInterfaceMock_GetActionListCount_Call { +func (_c *resourceStoreInterfaceMock_GetActionListCount_Call) RunAndReturn(run func(ctx context.Context, resServerID string, resID *string, kind providers.ActionKind) (int, error)) *resourceStoreInterfaceMock_GetActionListCount_Call { _c.Call.Return(run) return _c } diff --git a/backend/internal/resource/service.go b/backend/internal/resource/service.go index c86185a844..ad351e5f6b 100644 --- a/backend/internal/resource/service.go +++ b/backend/internal/resource/service.go @@ -102,7 +102,7 @@ type ResourceServiceInterface interface { ctx context.Context, resourceServerID string, resourceID *string, id string, ) (*providers.Action, *tidcommon.ServiceError) GetActionList( - ctx context.Context, resourceServerID string, resourceID *string, limit, offset int, + ctx context.Context, resourceServerID string, resourceID *string, kind providers.ActionKind, limit, offset int, ) (*ActionList, *tidcommon.ServiceError) UpdateAction( ctx context.Context, resourceServerID string, resourceID *string, id string, action providers.Action, @@ -230,6 +230,12 @@ func (rs *resourceService) CreateResourceServer( resourceServer.Type = providers.ResourceServerTypeCustom } + // MCP resource servers require a non-empty handle so server-level tools/resources derive a + // prefixed permission string and cannot collapse onto a bare action handle. + if resourceServer.Type == providers.ResourceServerTypeMCP && resourceServer.Handle == "" { + return nil, &ErrorInvalidRequestFormat + } + // Set default delimiter if not provided if resourceServer.Delimiter == "" { resourceServer.Delimiter = rs.defaultDelimiter @@ -438,6 +444,11 @@ func (rs *resourceService) UpdateResourceServer( return nil, &ErrorImmutableHandle } + // MCP resource servers require a non-empty handle. + if resourceServer.Type == providers.ResourceServerTypeMCP && resourceServer.Handle == "" { + return nil, &ErrorInvalidRequestFormat + } + // Identifier: preserve existing if not provided; check uniqueness if changed if resourceServer.Identifier == "" { resourceServer.Identifier = existingResServer.Identifier @@ -596,6 +607,21 @@ func (rs *resourceService) CreateResource( return nil, &ErrorHandleConflict } + // For MCP resource servers, a resource (group) and an action (tool/resource) in the same parent + // context must not share a handle, since they would derive an identical permission string. + if resourceServer.Type == providers.ResourceServerTypeMCP { + actionHandleExists, err := rs.resourceStore.CheckActionHandleExists( + ctx, resourceServerID, resource.Parent, resource.Handle, + ) + if err != nil { + rs.logger.Error(ctx, "Failed to check action handle", log.Error(err)) + return nil, &tidcommon.InternalServerError + } + if actionHandleExists { + return nil, &ErrorHandleConflict + } + } + // Derive permission string based on hierarchy resource.Permission = derivePermission(resourceServer, parentResource, resource.Handle) @@ -928,6 +954,13 @@ func (rs *resourceService) CreateAction( return nil, err } + if resourceServer.Type == providers.ResourceServerTypeMCP && action.Kind == "" { + action.Kind = providers.ActionKindTool + } + if svcErr := rs.validateActionKind(action.Kind); svcErr != nil { + return nil, svcErr + } + // Check handle uniqueness handleExists, err := rs.resourceStore.CheckActionHandleExists( ctx, resourceServerID, resourceID, action.Handle, @@ -940,6 +973,21 @@ func (rs *resourceService) CreateAction( return nil, &ErrorHandleConflict } + // For MCP resource servers, an action (tool/resource) and a resource (group) in the same parent + // context must not share a handle, since they would derive an identical permission string. + if resourceServer.Type == providers.ResourceServerTypeMCP { + resHandleExists, err := rs.resourceStore.CheckResourceHandleExists( + ctx, resourceServerID, action.Handle, resourceID, + ) + if err != nil { + rs.logger.Error(ctx, "Failed to check resource handle", log.Error(err)) + return nil, &tidcommon.InternalServerError + } + if resHandleExists { + return nil, &ErrorHandleConflict + } + } + // Derive permission string based on hierarchy action.Permission = derivePermission(resourceServer, resource, action.Handle) @@ -970,6 +1018,7 @@ func (rs *resourceService) CreateAction( Handle: action.Handle, Description: action.Description, Permission: action.Permission, + Kind: action.Kind, } return nil }); err != nil { @@ -1024,9 +1073,10 @@ func (rs *resourceService) GetAction( // GetActionList retrieves a paginated list of actions. // If resourceID is nil, retrieves actions at resource server level. // If resourceID is provided, retrieves actions at resource level. +// If kind is non-empty, only actions of that kind are returned. func (rs *resourceService) GetActionList( ctx context.Context, - resourceServerID string, resourceID *string, limit, offset int, + resourceServerID string, resourceID *string, kind providers.ActionKind, limit, offset int, ) (*ActionList, *tidcommon.ServiceError) { if err := validatePaginationParams(limit, offset); err != nil { return nil, err @@ -1056,7 +1106,7 @@ func (rs *resourceService) GetActionList( resID = resourceID } - totalCount, err := rs.resourceStore.GetActionListCount(ctx, resourceServerID, resID) + totalCount, err := rs.resourceStore.GetActionListCount(ctx, resourceServerID, resID, kind) if err != nil { if errors.Is(err, errResultLimitExceededInCompositeMode) { return nil, &ErrResultLimitExceededInCompositeMode @@ -1065,7 +1115,7 @@ func (rs *resourceService) GetActionList( return nil, &tidcommon.InternalServerError } - actions, err := rs.resourceStore.GetActionList(ctx, resourceServerID, resID, limit, offset) + actions, err := rs.resourceStore.GetActionList(ctx, resourceServerID, resID, kind, limit, offset) if err != nil { if errors.Is(err, errResultLimitExceededInCompositeMode) { return nil, &ErrResultLimitExceededInCompositeMode @@ -1138,11 +1188,17 @@ func (rs *resourceService) UpdateAction( return nil, &tidcommon.InternalServerError } - // Update only name and description (handle is immutable) + // Kind is immutable; reject any explicit change and preserve the stored value. + if action.Kind != "" && action.Kind != currentAction.Kind { + return nil, &ErrorInvalidRequestFormat + } + + // Update only name and description (handle and kind are immutable) updateAction := providers.Action{ Name: action.Name, Handle: currentAction.Handle, // Immutable - preserve Description: action.Description, + Kind: currentAction.Kind, // Immutable - preserve } // Use transaction for write operation @@ -1167,6 +1223,7 @@ func (rs *resourceService) UpdateAction( Name: updateAction.Name, Handle: updateAction.Handle, Description: updateAction.Description, + Kind: updateAction.Kind, } return nil }); err != nil { @@ -1460,6 +1517,15 @@ func (rs *resourceService) validateActionCreate(action providers.Action, delimit return nil } +// validateActionKind rejects a non-empty kind that is not one of the supported values (tool|resource). +// An empty kind is allowed for all resource server types; MCP defaulting is applied by the caller. +func (rs *resourceService) validateActionKind(kind providers.ActionKind) *tidcommon.ServiceError { + if kind != "" && !kind.IsValid() { + return &ErrorInvalidRequestFormat + } + return nil +} + // validatePaginationParams validates pagination parameters. func validatePaginationParams(limit, offset int) *tidcommon.ServiceError { if limit < 1 || limit > serverconst.MaxPageSize { diff --git a/backend/internal/resource/service_test.go b/backend/internal/resource/service_test.go index 6e51a3be95..f8c3128570 100644 --- a/backend/internal/resource/service_test.go +++ b/backend/internal/resource/service_test.go @@ -738,6 +738,35 @@ func (suite *ResourceServiceTestSuite) TestUpdateResourceServer_PreservesType() suite.mockStore.AssertExpectations(suite.T()) } +func (suite *ResourceServiceTestSuite) TestUpdateResourceServer_MCPRejectsEmptyHandle() { + // Guards pre-existing handle-less MCP rows (legacy rows created before the handle + // requirement) per spec ยง8: updating such a row without supplying a handle must be + // rejected so an MCP RS can never end up with an empty handle. + rs := providers.ResourceServer{ + Name: "updated-rs", + OUID: "ou-123", + } + + existingRS := providers.ResourceServer{ + ID: "rs-123", + Name: "old-name", + Handle: "", + Type: providers.ResourceServerTypeMCP, + OUID: "ou-123", + Delimiter: ":", + } + + suite.mockStore.On("IsResourceServerDeclarative", "rs-123").Return(false) + suite.mockStore.On("GetResourceServer", mock.Anything, "rs-123").Return(existingRS, nil) + + result, err := suite.service.UpdateResourceServer(context.Background(), "rs-123", rs) + + suite.Nil(result) + suite.NotNil(err) + suite.Equal(ErrorInvalidRequestFormat.Code, err.Code) + suite.Equal(tidcommon.ClientErrorType, err.Type) +} + func (suite *ResourceServiceTestSuite) TestUpdateResourceServer_NotFound() { rs := providers.ResourceServer{ Name: "test-rs", @@ -2790,6 +2819,245 @@ func (suite *ResourceServiceTestSuite) TestCreateActionAtResource_CheckResourceE suite.Equal(tidcommon.InternalServerError.Code, err.Code) } +// TestCreateAction_KindHandling covers kind defaulting and acceptance across resource server types: +// MCP defaults an omitted kind to "tool"; an explicit valid kind is preserved for any type; and an +// omitted kind on API/CUSTOM stays empty. +func (suite *ResourceServiceTestSuite) TestCreateAction_KindHandling() { + cases := []struct { + name string + rsID string + rsType providers.ResourceServerType + mcpCrossEnt bool + requestKind providers.ActionKind + expectedKind providers.ActionKind + }{ + {"MCP omitted defaults to tool", "rs-mcp", providers.ResourceServerTypeMCP, true, + "", providers.ActionKindTool}, + {"MCP explicit tool preserved", "rs-mcp", providers.ResourceServerTypeMCP, true, + providers.ActionKindTool, providers.ActionKindTool}, + {"MCP explicit resource preserved", "rs-mcp", providers.ResourceServerTypeMCP, true, + providers.ActionKindResource, providers.ActionKindResource}, + {"API explicit tool allowed", "rs-api", providers.ResourceServerTypeAPI, false, + providers.ActionKindTool, providers.ActionKindTool}, + {"API omitted stays empty", "rs-api", providers.ResourceServerTypeAPI, false, + "", ""}, + } + + for _, tc := range cases { + suite.Run(tc.name, func() { + suite.SetupTest() + suite.mockStore.On("GetResourceServer", mock.Anything, tc.rsID). + Return(providers.ResourceServer{Type: tc.rsType, Handle: "h-rs", Delimiter: ":"}, nil) + suite.mockStore.On("CheckActionHandleExists", mock.Anything, tc.rsID, (*string)(nil), "h"). + Return(false, nil) + if tc.mcpCrossEnt { + suite.mockStore.On("CheckResourceHandleExists", mock.Anything, tc.rsID, "h", (*string)(nil)). + Return(false, nil) + } + expectedKind := tc.expectedKind + suite.mockStore.On("CreateAction", mock.Anything, mock.AnythingOfType("string"), + tc.rsID, (*string)(nil), mock.MatchedBy(func(a providers.Action) bool { + return a.Handle == "h" && a.Kind == expectedKind && a.Permission != "" + })).Return(nil) + + result, err := suite.service.CreateAction(context.Background(), tc.rsID, nil, + providers.Action{Name: "n", Handle: "h", Kind: tc.requestKind}) + + suite.Nil(err) + suite.NotNil(result) + suite.Equal(tc.expectedKind, result.Kind) + suite.mockStore.AssertExpectations(suite.T()) + }) + } +} + +// TestCreateAction_InvalidKindRejected verifies a provided-but-unsupported kind is rejected with a +// 400 regardless of the resource server type. +func (suite *ResourceServiceTestSuite) TestCreateAction_InvalidKindRejected() { + cases := []struct { + name string + rsID string + rsType providers.ResourceServerType + }{ + {"MCP", "rs-mcp", providers.ResourceServerTypeMCP}, + {"API", "rs-api", providers.ResourceServerTypeAPI}, + } + + for _, tc := range cases { + suite.Run(tc.name, func() { + suite.SetupTest() + suite.mockStore.On("GetResourceServer", mock.Anything, tc.rsID). + Return(providers.ResourceServer{Type: tc.rsType, Handle: "h-rs", Delimiter: ":"}, nil) + + result, err := suite.service.CreateAction(context.Background(), tc.rsID, nil, + providers.Action{Name: "n", Handle: "h", Kind: providers.ActionKind("prompt")}) + + suite.Nil(result) + suite.NotNil(err) + suite.Equal(ErrorInvalidRequestFormat.Code, err.Code) + }) + } +} + +func (suite *ResourceServiceTestSuite) TestCreateAction_MCP_ActionHandleCollidesWithGroup() { + suite.mockStore.On("GetResourceServer", mock.Anything, "rs-mcp"). + Return(providers.ResourceServer{Type: providers.ResourceServerTypeMCP, Handle: "mcp", Delimiter: ":"}, nil) + suite.mockStore.On("CheckActionHandleExists", mock.Anything, "rs-mcp", (*string)(nil), "deploy"). + Return(false, nil) + suite.mockStore.On("CheckResourceHandleExists", mock.Anything, "rs-mcp", "deploy", (*string)(nil)). + Return(true, nil) + + result, err := suite.service.CreateAction(context.Background(), "rs-mcp", nil, + providers.Action{Name: "Deploy", Handle: "deploy", Kind: providers.ActionKindTool}) + + suite.Nil(result) + suite.NotNil(err) + suite.Equal(ErrorHandleConflict.Code, err.Code) +} + +func (suite *ResourceServiceTestSuite) TestCreateAction_API_NoCrossEntityCheck() { + suite.mockStore.On("GetResourceServer", mock.Anything, "rs-api"). + Return(providers.ResourceServer{Type: providers.ResourceServerTypeAPI, Handle: "api", Delimiter: ":"}, nil) + suite.mockStore.On("CheckActionHandleExists", mock.Anything, "rs-api", (*string)(nil), "deploy"). + Return(false, nil) + suite.mockStore.On("CreateAction", mock.Anything, mock.AnythingOfType("string"), + "rs-api", (*string)(nil), mock.MatchedBy(func(a providers.Action) bool { + return a.Handle == "deploy" && a.Permission != "" + })).Return(nil) + + result, err := suite.service.CreateAction(context.Background(), "rs-api", nil, + providers.Action{Name: "Deploy", Handle: "deploy"}) + + suite.Nil(err) + suite.NotNil(result) + suite.mockStore.AssertNotCalled(suite.T(), "CheckResourceHandleExists", + mock.Anything, mock.Anything, mock.Anything, mock.Anything) + suite.mockStore.AssertExpectations(suite.T()) +} + +func (suite *ResourceServiceTestSuite) TestCreateResource_MCP_GroupHandleCollidesWithAction() { + suite.mockStore.On("GetResourceServer", mock.Anything, "rs-mcp"). + Return(providers.ResourceServer{Type: providers.ResourceServerTypeMCP, Handle: "mcp", Delimiter: ":"}, nil) + suite.mockStore.On("CheckResourceHandleExists", mock.Anything, "rs-mcp", "deploy", (*string)(nil)). + Return(false, nil) + suite.mockStore.On("CheckActionHandleExists", mock.Anything, "rs-mcp", (*string)(nil), "deploy"). + Return(true, nil) + + result, err := suite.service.CreateResource(context.Background(), "rs-mcp", + providers.Resource{Name: "Deploy", Handle: "deploy"}) + + suite.Nil(result) + suite.NotNil(err) + suite.Equal(ErrorHandleConflict.Code, err.Code) +} + +func (suite *ResourceServiceTestSuite) TestCreateResource_API_NoCrossEntityCheck() { + suite.mockStore.On("GetResourceServer", mock.Anything, "rs-api"). + Return(providers.ResourceServer{Type: providers.ResourceServerTypeAPI, Handle: "api", Delimiter: ":"}, nil) + suite.mockStore.On("CheckResourceHandleExists", mock.Anything, "rs-api", "deploy", (*string)(nil)). + Return(false, nil) + suite.mockStore.On("CreateResource", mock.Anything, mock.AnythingOfType("string"), + "rs-api", (*string)(nil), mock.MatchedBy(func(r providers.Resource) bool { + return r.Handle == "deploy" && r.Permission != "" + })).Return(nil) + + result, err := suite.service.CreateResource(context.Background(), "rs-api", + providers.Resource{Name: "Deploy", Handle: "deploy"}) + + suite.Nil(err) + suite.NotNil(result) + suite.mockStore.AssertNotCalled(suite.T(), "CheckActionHandleExists", + mock.Anything, mock.Anything, mock.Anything, mock.Anything) + suite.mockStore.AssertExpectations(suite.T()) +} + +func (suite *ResourceServiceTestSuite) TestUpdateAction_KindImmutable() { + resID := testParentResourceID + currentAction := providers.Action{ + ID: "act-1", + Name: testOriginalName, + Handle: testOriginalHandle, + Permission: "mcp:g:h", + Kind: providers.ActionKindResource, + } + + suite.mockStore.On("IsResourceServerDeclarative", "rs-mcp").Return(false) + suite.mockStore.On("GetResourceServer", mock.Anything, "rs-mcp"). + Return(providers.ResourceServer{Type: providers.ResourceServerTypeMCP, Handle: "mcp", Delimiter: ":"}, nil) + suite.mockStore.On("GetResource", mock.Anything, resID, "rs-mcp").Return(providers.Resource{}, nil) + suite.mockStore.On("GetAction", mock.Anything, "act-1", "rs-mcp", &resID). + Return(currentAction, nil) + suite.mockStore.On("UpdateAction", mock.Anything, "act-1", "rs-mcp", &resID, + mock.MatchedBy(func(a providers.Action) bool { + return a.Kind == providers.ActionKindResource && a.Name == testUpdatedName + })).Return(nil) + + result, err := suite.service.UpdateAction(context.Background(), "rs-mcp", &resID, "act-1", + providers.Action{Name: testUpdatedName}) + + suite.Nil(err) + suite.NotNil(result) + suite.Equal(providers.ActionKindResource, result.Kind) + suite.mockStore.AssertExpectations(suite.T()) +} + +func (suite *ResourceServiceTestSuite) TestUpdateAction_KindChangeRejected() { + resID := testParentResourceID + currentAction := providers.Action{ + ID: "act-1", + Name: testOriginalName, + Handle: testOriginalHandle, + Permission: "mcp:g:h", + Kind: providers.ActionKindResource, + } + + suite.mockStore.On("IsResourceServerDeclarative", "rs-mcp").Return(false) + suite.mockStore.On("GetResourceServer", mock.Anything, "rs-mcp"). + Return(providers.ResourceServer{Type: providers.ResourceServerTypeMCP, Handle: "mcp", Delimiter: ":"}, nil) + suite.mockStore.On("GetResource", mock.Anything, resID, "rs-mcp").Return(providers.Resource{}, nil) + suite.mockStore.On("GetAction", mock.Anything, "act-1", "rs-mcp", &resID). + Return(currentAction, nil) + + result, err := suite.service.UpdateAction(context.Background(), "rs-mcp", &resID, "act-1", + providers.Action{Name: testUpdatedName, Kind: providers.ActionKindTool}) + + suite.Nil(result) + suite.NotNil(err) + suite.Equal(ErrorInvalidRequestFormat.Code, err.Code) +} + +func (suite *ResourceServiceTestSuite) TestCreateResourceServer_MCP_RequiresHandle() { + suite.mockOU.On("GetOrganizationUnit", mock.Anything, "ou-123"). + Return(providers.OrganizationUnit{ID: "ou-123"}, nil) + suite.mockStore.On("CheckResourceServerNameExists", mock.Anything, "x").Return(false, nil) + + result, err := suite.service.CreateResourceServer(context.Background(), + providers.ResourceServer{Name: "x", OUID: "ou-123", Type: providers.ResourceServerTypeMCP, Handle: ""}) + + suite.Nil(result) + suite.NotNil(err) + suite.Equal(ErrorInvalidRequestFormat.Code, err.Code) +} + +func (suite *ResourceServiceTestSuite) TestCreateResourceServer_MCP_WithHandleSucceeds() { + suite.mockOU.On("GetOrganizationUnit", mock.Anything, "ou-123"). + Return(providers.OrganizationUnit{ID: "ou-123"}, nil) + suite.mockStore.On("CheckResourceServerNameExists", mock.Anything, "x").Return(false, nil) + suite.mockStore.On("CheckResourceServerHandleExists", mock.Anything, "mcp").Return(false, nil) + suite.mockStore.On("CreateResourceServer", mock.Anything, mock.AnythingOfType("string"), + mock.MatchedBy(func(r providers.ResourceServer) bool { + return r.Type == providers.ResourceServerTypeMCP && r.Handle == "mcp" + })).Return(nil) + + result, err := suite.service.CreateResourceServer(context.Background(), + providers.ResourceServer{Name: "x", OUID: "ou-123", Type: providers.ResourceServerTypeMCP, Handle: "mcp"}) + + suite.Nil(err) + suite.NotNil(result) + suite.Equal(providers.ResourceServerTypeMCP, result.Type) + suite.mockStore.AssertExpectations(suite.T()) +} + func (suite *ResourceServiceTestSuite) TestGetActionAtResourceServer_Success() { expectedAction := providers.Action{ ID: "action-123", @@ -2857,11 +3125,11 @@ func (suite *ResourceServiceTestSuite) TestGetActionListAtResourceServer() { suite.mockStore.On("GetResourceServer", mock.Anything, "rs-123").Return(providers.ResourceServer{}, nil) suite.mockStore.On("GetActionListCount", mock.Anything, - "rs-123", (*string)(nil)).Return(2, nil) + "rs-123", (*string)(nil), providers.ActionKind("")).Return(2, nil) suite.mockStore.On("GetActionList", mock.Anything, - "rs-123", (*string)(nil), 30, 0).Return([]providers.Action{ - {ID: "action-1", Name: "providers.Action 1"}, - {ID: "action-2", Name: "providers.Action 2"}, + "rs-123", (*string)(nil), providers.ActionKind(""), 30, 0).Return([]providers.Action{ + {ID: "action-1", Name: "Action 1"}, + {ID: "action-2", Name: "Action 2"}, }, nil) }, expectedError: nil, @@ -2916,7 +3184,7 @@ func (suite *ResourceServiceTestSuite) TestGetActionListAtResourceServer() { suite.mockStore.On("GetResourceServer", mock.Anything, "rs-123").Return(providers.ResourceServer{}, nil) suite.mockStore.On("GetActionListCount", mock.Anything, - "rs-123", (*string)(nil)).Return(0, errors.New("database error")) + "rs-123", (*string)(nil), providers.ActionKind("")).Return(0, errors.New("database error")) }, expectedError: &tidcommon.InternalServerError, }, @@ -2930,9 +3198,9 @@ func (suite *ResourceServiceTestSuite) TestGetActionListAtResourceServer() { suite.mockStore.On("GetResourceServer", mock.Anything, "rs-123").Return(providers.ResourceServer{}, nil) suite.mockStore.On("GetActionListCount", mock.Anything, - "rs-123", (*string)(nil)).Return(2, nil) + "rs-123", (*string)(nil), providers.ActionKind("")).Return(2, nil) suite.mockStore.On("GetActionList", mock.Anything, - "rs-123", (*string)(nil), 30, 0).Return(nil, errors.New("database error")) + "rs-123", (*string)(nil), providers.ActionKind(""), 30, 0).Return(nil, errors.New("database error")) }, expectedError: &tidcommon.InternalServerError, }, @@ -2944,7 +3212,7 @@ func (suite *ResourceServiceTestSuite) TestGetActionListAtResourceServer() { tc.setupMocks() result, err := suite.service.GetActionList( - context.Background(), tc.resourceServerID, tc.resourceID, tc.limit, tc.offset, + context.Background(), tc.resourceServerID, tc.resourceID, "", tc.limit, tc.offset, ) if tc.expectedError != nil { @@ -3758,11 +4026,11 @@ func (suite *ResourceServiceTestSuite) TestGetActionListAtResource() { testParentResourceID, "rs-123").Return(providers.Resource{}, nil) resID := testParentResourceID suite.mockStore.On("GetActionListCount", mock.Anything, - "rs-123", &resID).Return(2, nil) + "rs-123", &resID, providers.ActionKind("")).Return(2, nil) suite.mockStore.On("GetActionList", mock.Anything, - "rs-123", &resID, 30, 0).Return([]providers.Action{ - {ID: "action-1", Name: "providers.Action 1"}, - {ID: "action-2", Name: "providers.Action 2"}, + "rs-123", &resID, providers.ActionKind(""), 30, 0).Return([]providers.Action{ + {ID: "action-1", Name: "Action 1"}, + {ID: "action-2", Name: "Action 2"}, }, nil) }, expectedError: nil, @@ -3785,9 +4053,9 @@ func (suite *ResourceServiceTestSuite) TestGetActionListAtResource() { testParentResourceID, "rs-123").Return(providers.Resource{}, nil) resID := testParentResourceID suite.mockStore.On("GetActionListCount", mock.Anything, - "rs-123", &resID).Return(0, nil) + "rs-123", &resID, providers.ActionKind("")).Return(0, nil) suite.mockStore.On("GetActionList", mock.Anything, - "rs-123", &resID, 30, 0).Return([]providers.Action{}, nil) + "rs-123", &resID, providers.ActionKind(""), 30, 0).Return([]providers.Action{}, nil) }, expectedError: nil, validateResponse: func(result *ActionList) { @@ -3876,7 +4144,7 @@ func (suite *ResourceServiceTestSuite) TestGetActionListAtResource() { Return(providers.Resource{}, nil) resID := testParentResourceID suite.mockStore.On("GetActionListCount", mock.Anything, - "rs-123", &resID). + "rs-123", &resID, providers.ActionKind("")). Return(0, errors.New("database error")) }, expectedError: &tidcommon.InternalServerError, @@ -3896,9 +4164,9 @@ func (suite *ResourceServiceTestSuite) TestGetActionListAtResource() { Return(providers.Resource{}, nil) resID := testParentResourceID suite.mockStore.On("GetActionListCount", mock.Anything, - "rs-123", &resID).Return(2, nil) + "rs-123", &resID, providers.ActionKind("")).Return(2, nil) suite.mockStore.On("GetActionList", mock.Anything, - "rs-123", &resID, 30, 0). + "rs-123", &resID, providers.ActionKind(""), 30, 0). Return(nil, errors.New("database error")) }, expectedError: &tidcommon.InternalServerError, @@ -3911,7 +4179,7 @@ func (suite *ResourceServiceTestSuite) TestGetActionListAtResource() { tc.setupMocks() result, err := suite.service.GetActionList( - context.Background(), tc.resourceServerID, tc.resourceID, tc.limit, tc.offset, + context.Background(), tc.resourceServerID, tc.resourceID, "", tc.limit, tc.offset, ) if tc.expectedError != nil { @@ -4167,7 +4435,7 @@ func (suite *ResourceServiceTestSuite) TestListMethods_PaginationValidationError suite.Run("GetActionList_"+tc.name, func() { suite.SetupTest() - result, err := suite.service.GetActionList(context.Background(), "rs-123", nil, tc.limit, tc.offset) + result, err := suite.service.GetActionList(context.Background(), "rs-123", nil, "", tc.limit, tc.offset) suite.Nil(result) suite.NotNil(err) diff --git a/backend/internal/resource/store.go b/backend/internal/resource/store.go index 760e0d4d94..82f1f7c30c 100644 --- a/backend/internal/resource/store.go +++ b/backend/internal/resource/store.go @@ -67,8 +67,12 @@ type resourceStoreInterface interface { // Action operations CreateAction(ctx context.Context, uuid string, resServerID string, resID *string, action providers.Action) error GetAction(ctx context.Context, id string, resServerID string, resID *string) (providers.Action, error) - GetActionList(ctx context.Context, resServerID string, resID *string, limit, offset int) ([]providers.Action, error) - GetActionListCount(ctx context.Context, resServerID string, resID *string) (int, error) + GetActionList( + ctx context.Context, resServerID string, resID *string, kind providers.ActionKind, limit, offset int, + ) ([]providers.Action, error) + GetActionListCount( + ctx context.Context, resServerID string, resID *string, kind providers.ActionKind, + ) (int, error) UpdateAction(ctx context.Context, id string, resServerID string, resID *string, action providers.Action) error UpdateActionPermission(ctx context.Context, id string, resServerID string, resID *string, permission string) error DeleteAction(ctx context.Context, id string, resServerID string, resID *string) error @@ -91,6 +95,11 @@ type resourceServerProperties struct { Delimiter string `json:"delimiter"` } +// actionProperties represents the JSON structure of the ACTION.PROPERTIES column. +type actionProperties struct { + Kind providers.ActionKind `json:"kind,omitempty"` +} + // newResourceStore creates a new instance of resourceStore. func newResourceStore() (resourceStoreInterface, transaction.Transactioner, error) { dbProvider := provider.GetDBProvider() @@ -643,15 +652,15 @@ func (s *resourceStore) CreateAction( _, err := dbClient.ExecuteContext( ctx, queryCreateAction, - uuid, // $1: ACTION_ID (UUID) - resServerID, // $2: RESOURCE_SERVER_ID (UUID FK) - resID, // $3: RESOURCE_ID (UUID FK or NULL) - action.Name, // $4: NAME - action.Handle, // $5: HANDLE - action.Description, // $6: DESCRIPTION - action.Permission, // $7: PERMISSION - "{}", // $8: PROPERTIES (empty JSON). - s.deploymentID, // $9: DEPLOYMENT_ID + uuid, // $1: ACTION_ID (UUID) + resServerID, // $2: RESOURCE_SERVER_ID (UUID FK) + resID, // $3: RESOURCE_ID (UUID FK or NULL) + action.Name, // $4: NAME + action.Handle, // $5: HANDLE + action.Description, // $6: DESCRIPTION + action.Permission, // $7: PERMISSION + buildActionPropertiesJSON(action), // $8: PROPERTIES (NULL when kind empty). + s.deploymentID, // $9: DEPLOYMENT_ID ) if err != nil { return fmt.Errorf("failed to create action: %w", err) @@ -687,17 +696,26 @@ func (s *resourceStore) GetAction( return action, err } -// GetActionList retrieves actions with pagination. +// GetActionList retrieves actions with pagination, optionally filtered by kind. func (s *resourceStore) GetActionList( ctx context.Context, - resServerID string, resID *string, limit, offset int, + resServerID string, resID *string, kind providers.ActionKind, limit, offset int, ) ([]providers.Action, error) { var actions []providers.Action err := s.withDBClient(func(dbClient provider.DBClientInterface) error { - results, err := dbClient.QueryContext( - ctx, queryGetActionList, resServerID, resID, limit, offset, - s.deploymentID, - ) + var results []map[string]interface{} + var err error + if kind != "" { + results, err = dbClient.QueryContext( + ctx, queryGetActionListByKind, resServerID, resID, limit, offset, + string(kind), s.deploymentID, + ) + } else { + results, err = dbClient.QueryContext( + ctx, queryGetActionList, resServerID, resID, limit, offset, + s.deploymentID, + ) + } if err != nil { return fmt.Errorf("failed to get action list: %w", err) } @@ -719,15 +737,23 @@ func (s *resourceStore) GetActionList( return actions, nil } -// GetActionListCount retrieves count of actions. +// GetActionListCount retrieves count of actions, optionally filtered by kind. func (s *resourceStore) GetActionListCount( - ctx context.Context, resServerID string, resID *string, + ctx context.Context, resServerID string, resID *string, kind providers.ActionKind, ) (int, error) { var count int err := s.withDBClient(func(dbClient provider.DBClientInterface) error { - results, err := dbClient.QueryContext( - ctx, queryGetActionListCount, resServerID, resID, s.deploymentID, - ) + var results []map[string]interface{} + var err error + if kind != "" { + results, err = dbClient.QueryContext( + ctx, queryGetActionListCountByKind, resServerID, resID, string(kind), s.deploymentID, + ) + } else { + results, err = dbClient.QueryContext( + ctx, queryGetActionListCount, resServerID, resID, s.deploymentID, + ) + } if err != nil { return fmt.Errorf("failed to get action count: %w", err) } @@ -747,13 +773,13 @@ func (s *resourceStore) UpdateAction( _, err := dbClient.ExecuteContext( ctx, queryUpdateAction, - action.Name, // $1: NAME - action.Description, // $2: DESCRIPTION - "{}", // $3: PROPERTIES (empty JSON). - id, // $4: ACTION_ID - resServerID, // $5: RESOURCE_SERVER_ID (UUID FK) - resID, // $6: RESOURCE_ID (UUID FK or NULL) - s.deploymentID, // $7: DEPLOYMENT_ID + action.Name, // $1: NAME + action.Description, // $2: DESCRIPTION + buildActionPropertiesJSON(action), // $3: PROPERTIES (NULL when kind empty). + id, // $4: ACTION_ID + resServerID, // $5: RESOURCE_SERVER_ID (UUID FK) + resID, // $6: RESOURCE_ID (UUID FK or NULL) + s.deploymentID, // $7: DEPLOYMENT_ID ) if err != nil { return fmt.Errorf("failed to update action: %w", err) @@ -1030,6 +1056,38 @@ func buildPropertiesJSON(rs providers.ResourceServer) interface{} { return json.RawMessage("{}") } +// resolveActionProperties extracts and sets the kind from the ACTION.PROPERTIES column. +func resolveActionProperties(row map[string]interface{}, a *providers.Action) { + if propsVal, ok := row["properties"]; ok && propsVal != nil { + var props actionProperties + var propsBytes []byte + + switch v := propsVal.(type) { + case string: + propsBytes = []byte(v) + case []byte: + propsBytes = v + } + + if len(propsBytes) > 0 { + if err := json.Unmarshal(propsBytes, &props); err == nil { + a.Kind = props.Kind + } + } + } +} + +// buildActionPropertiesJSON builds the PROPERTIES JSON for an Action, or NULL when kind is empty. +func buildActionPropertiesJSON(a providers.Action) interface{} { + if a.Kind == "" { + return nil + } + if propsJSON, err := json.Marshal(actionProperties{Kind: a.Kind}); err == nil { + return propsJSON + } + return nil +} + // buildResourceServerFromResultRow builds a providers.ResourceServer from a database result row. func buildResourceServerFromResultRow(row map[string]interface{}) (providers.ResourceServer, error) { rs := providers.ResourceServer{} @@ -1142,7 +1200,7 @@ func buildActionFromResultRow(row map[string]interface{}) (providers.Action, err action.Permission = permission } - // PROPERTIES column exists in DB but not mapped to model (store as empty JSON) + resolveActionProperties(row, &action) return action, nil } diff --git a/backend/internal/resource/store_constants.go b/backend/internal/resource/store_constants.go index a7d411d1a3..f1b82953f3 100644 --- a/backend/internal/resource/store_constants.go +++ b/backend/internal/resource/store_constants.go @@ -306,6 +306,25 @@ var ( ORDER BY a.CREATED_AT DESC LIMIT $3 OFFSET $4`, } + // queryGetActionListByKind retrieves actions filtered by kind (stored in PROPERTIES) with pagination. + queryGetActionListByKind = dbmodel.DBQuery{ + ID: "RSQ-RES_MGT-39", + PostgresQuery: `SELECT a.ID, a.NAME, a.HANDLE, a.DESCRIPTION, a.PERMISSION, a.PROPERTIES + FROM "ACTION" a + WHERE a.RESOURCE_SERVER_ID = $1 + AND (a.RESOURCE_ID = $2 OR (a.RESOURCE_ID IS NULL AND $2 IS NULL)) + AND a.PROPERTIES->>'kind' = $5 + AND a.DEPLOYMENT_ID = $6 + ORDER BY a.CREATED_AT DESC LIMIT $3 OFFSET $4`, + SQLiteQuery: `SELECT a.ID, a.NAME, a.HANDLE, a.DESCRIPTION, a.PERMISSION, a.PROPERTIES + FROM "ACTION" a + WHERE a.RESOURCE_SERVER_ID = $1 + AND (a.RESOURCE_ID = $2 OR (a.RESOURCE_ID IS NULL AND $2 IS NULL)) + AND json_extract(a.PROPERTIES, '$.kind') = $5 + AND a.DEPLOYMENT_ID = $6 + ORDER BY a.CREATED_AT DESC LIMIT $3 OFFSET $4`, + } + // queryGetActionListCount retrieves count of actions. queryGetActionListCount = dbmodel.DBQuery{ ID: "RSQ-RES_MGT-27", @@ -316,6 +335,23 @@ var ( AND a.DEPLOYMENT_ID = $3`, } + // queryGetActionListCountByKind retrieves count of actions filtered by kind (stored in PROPERTIES). + queryGetActionListCountByKind = dbmodel.DBQuery{ + ID: "RSQ-RES_MGT-40", + PostgresQuery: `SELECT COUNT(*) as total + FROM "ACTION" a + WHERE a.RESOURCE_SERVER_ID = $1 + AND (a.RESOURCE_ID = $2 OR (a.RESOURCE_ID IS NULL AND $2 IS NULL)) + AND a.PROPERTIES->>'kind' = $3 + AND a.DEPLOYMENT_ID = $4`, + SQLiteQuery: `SELECT COUNT(*) as total + FROM "ACTION" a + WHERE a.RESOURCE_SERVER_ID = $1 + AND (a.RESOURCE_ID = $2 OR (a.RESOURCE_ID IS NULL AND $2 IS NULL)) + AND json_extract(a.PROPERTIES, '$.kind') = $3 + AND a.DEPLOYMENT_ID = $4`, + } + // queryUpdateAction updates an action. queryUpdateAction = dbmodel.DBQuery{ ID: "RSQ-RES_MGT-28", diff --git a/backend/internal/resource/store_test.go b/backend/internal/resource/store_test.go index 114de15cad..8ca2109c7f 100644 --- a/backend/internal/resource/store_test.go +++ b/backend/internal/resource/store_test.go @@ -1981,7 +1981,7 @@ func (suite *ResourceStoreTestSuite) TestCreateAction() { suite.mockDBProvider.On("GetConfigDBClient").Return(suite.mockDBClient, nil) suite.mockDBClient.On("ExecuteContext", context.Background(), queryCreateAction, "action1", "rs1", resourceID, - "Test Action", "test-handle", "Test Description", "perm:act", "{}", "test-deployment"). + "Test Action", "test-handle", "Test Description", "perm:act", nil, "test-deployment"). Return(int64(1), nil) }, shouldErr: false, @@ -2001,7 +2001,29 @@ func (suite *ResourceStoreTestSuite) TestCreateAction() { suite.mockDBProvider.On("GetConfigDBClient").Return(suite.mockDBClient, nil) suite.mockDBClient.On("ExecuteContext", context.Background(), queryCreateAction, "action1", "rs1", (*string)(nil), - "Test Action", "test-handle", "Test Description", "perm:act", "{}", "test-deployment"). + "Test Action", "test-handle", "Test Description", "perm:act", nil, "test-deployment"). + Return(int64(1), nil) + }, + shouldErr: false, + }, + { + name: "Success_WithKind", + actionID: "action1", + resourceServerID: "rs1", + resourceID: nil, + action: providers.Action{ + Name: "Test Action", + Handle: "test-handle", + Description: "Test Description", + Permission: "perm:act", + Kind: providers.ActionKindTool, + }, + setupMocks: func(resourceID *string) { + suite.mockDBProvider.On("GetConfigDBClient").Return(suite.mockDBClient, nil) + suite.mockDBClient.On("ExecuteContext", context.Background(), + queryCreateAction, "action1", "rs1", (*string)(nil), + "Test Action", "test-handle", "Test Description", "perm:act", + []byte(`{"kind":"tool"}`), "test-deployment"). Return(int64(1), nil) }, shouldErr: false, @@ -2022,7 +2044,7 @@ func (suite *ResourceStoreTestSuite) TestCreateAction() { suite.mockDBProvider.On("GetConfigDBClient").Return(suite.mockDBClient, nil) suite.mockDBClient.On("ExecuteContext", context.Background(), queryCreateAction, "action1", "rs1", (*string)(nil), - "Test Action", "test-handle", "Test Description", "perm:act", "{}", "test-deployment"). + "Test Action", "test-handle", "Test Description", "perm:act", nil, "test-deployment"). Return(int64(0), execError) }, shouldErr: true, @@ -2191,6 +2213,7 @@ func (suite *ResourceStoreTestSuite) TestGetActionList() { name string resourceServerID string resourceID *string + kind providers.ActionKind limit int offset int setupMocks func(*string, int, int) @@ -2307,6 +2330,50 @@ func (suite *ResourceStoreTestSuite) TestGetActionList() { }, shouldErr: true, }, + { + name: "Success_WithKindFilter", + resourceServerID: "rs1", + resourceID: nil, + kind: providers.ActionKindTool, + limit: testLimit, + offset: testOffset, + setupMocks: func(resourceID *string, limit, offset int) { + var nilResID *string + suite.mockDBProvider.On("GetConfigDBClient").Return(suite.mockDBClient, nil) + suite.mockDBClient.On("QueryContext", context.Background(), + queryGetActionListByKind, "rs1", nilResID, + limit, offset, "tool", "test-deployment").Return([]map[string]interface{}{ + { + "id": "action1", + "resource_server_id": "rs1", + "name": "Tool Action", + "handle": "tool-action", + "description": "Tool Description", + "permission": "perm:tool", + "properties": `{"kind":"tool"}`, + }, + }, nil) + }, + expectedCount: 1, + shouldErr: false, + }, + { + name: "QueryError_WithKindFilter", + resourceServerID: "rs1", + resourceID: nil, + kind: providers.ActionKindTool, + limit: testLimit, + offset: testOffset, + setupMocks: func(resourceID *string, limit, offset int) { + var nilResID *string + queryError := errors.New("query error") + suite.mockDBProvider.On("GetConfigDBClient").Return(suite.mockDBClient, nil) + suite.mockDBClient.On("QueryContext", context.Background(), + queryGetActionListByKind, "rs1", nilResID, + limit, offset, "tool", "test-deployment").Return(nil, queryError) + }, + shouldErr: true, + }, } for _, tc := range testCases { @@ -2323,7 +2390,7 @@ func (suite *ResourceStoreTestSuite) TestGetActionList() { actions, err := suite.store.GetActionList(context.Background(), - tc.resourceServerID, tc.resourceID, tc.limit, tc.offset) + tc.resourceServerID, tc.resourceID, tc.kind, tc.limit, tc.offset) if tc.shouldErr { suite.Error(err) @@ -2347,6 +2414,7 @@ func (suite *ResourceStoreTestSuite) TestGetActionListCount() { name string resourceServerID string resourceID *string + kind providers.ActionKind setupMocks func(*string) expectedCount int shouldErr bool @@ -2411,6 +2479,39 @@ func (suite *ResourceStoreTestSuite) TestGetActionListCount() { expectedCount: 0, shouldErr: true, }, + { + name: "Success_WithKindFilter", + resourceServerID: "rs1", + resourceID: nil, + kind: providers.ActionKindTool, + setupMocks: func(resourceID *string) { + var nilResID *string + suite.mockDBProvider.On("GetConfigDBClient").Return(suite.mockDBClient, nil) + suite.mockDBClient.On("QueryContext", context.Background(), + queryGetActionListCountByKind, "rs1", nilResID, + "tool", "test-deployment").Return([]map[string]interface{}{ + {"total": int64(3)}, + }, nil) + }, + expectedCount: 3, + shouldErr: false, + }, + { + name: "QueryError_WithKindFilter", + resourceServerID: "rs1", + resourceID: nil, + kind: providers.ActionKindTool, + setupMocks: func(resourceID *string) { + var nilResID *string + queryError := errors.New("query error") + suite.mockDBProvider.On("GetConfigDBClient").Return(suite.mockDBClient, nil) + suite.mockDBClient.On("QueryContext", context.Background(), + queryGetActionListCountByKind, "rs1", nilResID, + "tool", "test-deployment").Return(nil, queryError) + }, + expectedCount: 0, + shouldErr: true, + }, } for _, tc := range testCases { @@ -2425,7 +2526,7 @@ func (suite *ResourceStoreTestSuite) TestGetActionListCount() { tc.setupMocks(tc.resourceID) count, err := suite.store.GetActionListCount(context.Background(), - tc.resourceServerID, tc.resourceID) + tc.resourceServerID, tc.resourceID, tc.kind) if tc.shouldErr { suite.Error(err) @@ -2462,7 +2563,7 @@ func (suite *ResourceStoreTestSuite) TestUpdateAction() { suite.mockDBProvider.On("GetConfigDBClient").Return(suite.mockDBClient, nil) suite.mockDBClient.On("ExecuteContext", context.Background(), queryUpdateAction, "Updated Action", - "Updated Description", "{}", "action1", "rs1", nilResID, "test-deployment"). + "Updated Description", nil, "action1", "rs1", nilResID, "test-deployment"). Return(int64(1), nil) }, shouldErr: false, @@ -2480,7 +2581,28 @@ func (suite *ResourceStoreTestSuite) TestUpdateAction() { suite.mockDBProvider.On("GetConfigDBClient").Return(suite.mockDBClient, nil) suite.mockDBClient.On("ExecuteContext", context.Background(), queryUpdateAction, "Updated Action", - "Updated Description", "{}", "action1", "rs1", resourceID, "test-deployment"). + "Updated Description", nil, "action1", "rs1", resourceID, "test-deployment"). + Return(int64(1), nil) + }, + shouldErr: false, + }, + { + name: "Success_WithKind", + actionID: "action1", + resourceServerID: "rs1", + resourceID: nil, + action: providers.Action{ + Name: "Updated Action", + Description: "Updated Description", + Kind: providers.ActionKindTool, + }, + setupMocks: func(resourceID *string) { + var nilResID *string + suite.mockDBProvider.On("GetConfigDBClient").Return(suite.mockDBClient, nil) + suite.mockDBClient.On("ExecuteContext", context.Background(), + queryUpdateAction, "Updated Action", + "Updated Description", []byte(`{"kind":"tool"}`), "action1", "rs1", nilResID, + "test-deployment"). Return(int64(1), nil) }, shouldErr: false, @@ -2500,7 +2622,7 @@ func (suite *ResourceStoreTestSuite) TestUpdateAction() { suite.mockDBProvider.On("GetConfigDBClient").Return(suite.mockDBClient, nil) suite.mockDBClient.On("ExecuteContext", context.Background(), queryUpdateAction, "Updated Action", - "Updated Description", "{}", "action1", "rs1", nilResID, "test-deployment"). + "Updated Description", nil, "action1", "rs1", nilResID, "test-deployment"). Return(int64(0), execError) }, shouldErr: true, @@ -2537,6 +2659,33 @@ func (suite *ResourceStoreTestSuite) TestUpdateAction() { } } +func (suite *ResourceStoreTestSuite) TestActionProperties_RoundTrip() { + // tool -> JSON -> back + props := buildActionPropertiesJSON(providers.Action{Kind: providers.ActionKindTool}) + suite.Equal([]byte(`{"kind":"tool"}`), props) + var toolAction providers.Action + resolveActionProperties(map[string]interface{}{"properties": props}, &toolAction) + suite.Equal(providers.ActionKindTool, toolAction.Kind) + + // resource -> JSON -> back + resProps := buildActionPropertiesJSON(providers.Action{Kind: providers.ActionKindResource}) + var resAction providers.Action + resolveActionProperties(map[string]interface{}{"properties": resProps}, &resAction) + suite.Equal(providers.ActionKindResource, resAction.Kind) + + // empty kind (API/CUSTOM) -> NULL -> stays empty + empty := buildActionPropertiesJSON(providers.Action{Kind: ""}) + suite.Nil(empty) + var emptyAction providers.Action + resolveActionProperties(map[string]interface{}{"properties": empty}, &emptyAction) + suite.Equal(providers.ActionKind(""), emptyAction.Kind) + + // PROPERTIES stored as TEXT (SQLite) round-trips through the string branch + var fromString providers.Action + resolveActionProperties(map[string]interface{}{"properties": `{"kind":"tool"}`}, &fromString) + suite.Equal(providers.ActionKindTool, fromString.Kind) +} + func (suite *ResourceStoreTestSuite) TestDeleteAction() { testCases := []struct { name string diff --git a/backend/pkg/thunderidengine/providers/model.go b/backend/pkg/thunderidengine/providers/model.go index 946d2aeadd..8e719786f5 100644 --- a/backend/pkg/thunderidengine/providers/model.go +++ b/backend/pkg/thunderidengine/providers/model.go @@ -127,6 +127,32 @@ func (t ResourceServerType) IsValid() bool { return false } +// ActionKind discriminates MCP primitives stored as actions. +type ActionKind string + +const ( + // ActionKindTool represents an MCP tool. + ActionKindTool ActionKind = "tool" + // ActionKindResource represents an MCP resource. + ActionKindResource ActionKind = "resource" +) + +// supportedActionKinds lists all the supported action kinds. +var supportedActionKinds = []ActionKind{ + ActionKindTool, + ActionKindResource, +} + +// IsValid reports whether the action kind is one of the supported values. +func (k ActionKind) IsValid() bool { + for _, supported := range supportedActionKinds { + if k == supported { + return true + } + } + return false +} + // Consolidated resource models for YAML parsing, processing, and service layer // These models use: // - yaml tags for YAML parsing (serialize/deserialize) @@ -140,6 +166,8 @@ type Action struct { Handle string `yaml:"handle" json:"handle"` Description string `yaml:"description,omitempty" json:"description,omitempty"` Permission string `yaml:"-" json:"-"` // Computed permission string, not serialized to YAML + // Kind is empty ("") for API/CUSTOM actions; "tool"|"resource" for MCP actions. + Kind ActionKind `yaml:"kind,omitempty" json:"-"` } // Resource represents a resource in both declarative resources and service layer. diff --git a/backend/pkg/thunderidengine/providers/model_test.go b/backend/pkg/thunderidengine/providers/model_test.go new file mode 100644 index 0000000000..71a0127df0 --- /dev/null +++ b/backend/pkg/thunderidengine/providers/model_test.go @@ -0,0 +1,38 @@ +/* + * Copyright (c) 2026, WSO2 LLC. (https://www.wso2.com). + * + * WSO2 LLC. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package providers + +import "testing" + +func TestActionKind_IsValid(t *testing.T) { + cases := []struct { + kind ActionKind + want bool + }{ + {ActionKindTool, true}, + {ActionKindResource, true}, + {ActionKind("prompt"), false}, + {ActionKind(""), false}, + } + for _, c := range cases { + if got := c.kind.IsValid(); got != c.want { + t.Errorf("ActionKind(%q).IsValid() = %v, want %v", c.kind, got, c.want) + } + } +} diff --git a/backend/tests/mocks/resourcemock/ResourceServiceInterface_mock.go b/backend/tests/mocks/resourcemock/ResourceServiceInterface_mock.go index f1662949d1..0cc2d8252d 100644 --- a/backend/tests/mocks/resourcemock/ResourceServiceInterface_mock.go +++ b/backend/tests/mocks/resourcemock/ResourceServiceInterface_mock.go @@ -616,8 +616,8 @@ func (_c *ResourceServiceInterfaceMock_GetAction_Call) RunAndReturn(run func(ctx } // GetActionList provides a mock function for the type ResourceServiceInterfaceMock -func (_mock *ResourceServiceInterfaceMock) GetActionList(ctx context.Context, resourceServerID string, resourceID *string, limit int, offset int) (*resource.ActionList, *common.ServiceError) { - ret := _mock.Called(ctx, resourceServerID, resourceID, limit, offset) +func (_mock *ResourceServiceInterfaceMock) GetActionList(ctx context.Context, resourceServerID string, resourceID *string, kind providers.ActionKind, limit int, offset int) (*resource.ActionList, *common.ServiceError) { + ret := _mock.Called(ctx, resourceServerID, resourceID, kind, limit, offset) if len(ret) == 0 { panic("no return value specified for GetActionList") @@ -625,18 +625,18 @@ func (_mock *ResourceServiceInterfaceMock) GetActionList(ctx context.Context, re var r0 *resource.ActionList var r1 *common.ServiceError - if returnFunc, ok := ret.Get(0).(func(context.Context, string, *string, int, int) (*resource.ActionList, *common.ServiceError)); ok { - return returnFunc(ctx, resourceServerID, resourceID, limit, offset) + if returnFunc, ok := ret.Get(0).(func(context.Context, string, *string, providers.ActionKind, int, int) (*resource.ActionList, *common.ServiceError)); ok { + return returnFunc(ctx, resourceServerID, resourceID, kind, limit, offset) } - if returnFunc, ok := ret.Get(0).(func(context.Context, string, *string, int, int) *resource.ActionList); ok { - r0 = returnFunc(ctx, resourceServerID, resourceID, limit, offset) + if returnFunc, ok := ret.Get(0).(func(context.Context, string, *string, providers.ActionKind, int, int) *resource.ActionList); ok { + r0 = returnFunc(ctx, resourceServerID, resourceID, kind, limit, offset) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*resource.ActionList) } } - if returnFunc, ok := ret.Get(1).(func(context.Context, string, *string, int, int) *common.ServiceError); ok { - r1 = returnFunc(ctx, resourceServerID, resourceID, limit, offset) + if returnFunc, ok := ret.Get(1).(func(context.Context, string, *string, providers.ActionKind, int, int) *common.ServiceError); ok { + r1 = returnFunc(ctx, resourceServerID, resourceID, kind, limit, offset) } else { if ret.Get(1) != nil { r1 = ret.Get(1).(*common.ServiceError) @@ -654,13 +654,14 @@ type ResourceServiceInterfaceMock_GetActionList_Call struct { // - ctx context.Context // - resourceServerID string // - resourceID *string +// - kind providers.ActionKind // - limit int // - offset int -func (_e *ResourceServiceInterfaceMock_Expecter) GetActionList(ctx interface{}, resourceServerID interface{}, resourceID interface{}, limit interface{}, offset interface{}) *ResourceServiceInterfaceMock_GetActionList_Call { - return &ResourceServiceInterfaceMock_GetActionList_Call{Call: _e.mock.On("GetActionList", ctx, resourceServerID, resourceID, limit, offset)} +func (_e *ResourceServiceInterfaceMock_Expecter) GetActionList(ctx interface{}, resourceServerID interface{}, resourceID interface{}, kind interface{}, limit interface{}, offset interface{}) *ResourceServiceInterfaceMock_GetActionList_Call { + return &ResourceServiceInterfaceMock_GetActionList_Call{Call: _e.mock.On("GetActionList", ctx, resourceServerID, resourceID, kind, limit, offset)} } -func (_c *ResourceServiceInterfaceMock_GetActionList_Call) Run(run func(ctx context.Context, resourceServerID string, resourceID *string, limit int, offset int)) *ResourceServiceInterfaceMock_GetActionList_Call { +func (_c *ResourceServiceInterfaceMock_GetActionList_Call) Run(run func(ctx context.Context, resourceServerID string, resourceID *string, kind providers.ActionKind, limit int, offset int)) *ResourceServiceInterfaceMock_GetActionList_Call { _c.Call.Run(func(args mock.Arguments) { var arg0 context.Context if args[0] != nil { @@ -674,20 +675,25 @@ func (_c *ResourceServiceInterfaceMock_GetActionList_Call) Run(run func(ctx cont if args[2] != nil { arg2 = args[2].(*string) } - var arg3 int + var arg3 providers.ActionKind if args[3] != nil { - arg3 = args[3].(int) + arg3 = args[3].(providers.ActionKind) } var arg4 int if args[4] != nil { arg4 = args[4].(int) } + var arg5 int + if args[5] != nil { + arg5 = args[5].(int) + } run( arg0, arg1, arg2, arg3, arg4, + arg5, ) }) return _c @@ -698,7 +704,7 @@ func (_c *ResourceServiceInterfaceMock_GetActionList_Call) Return(actionList *re return _c } -func (_c *ResourceServiceInterfaceMock_GetActionList_Call) RunAndReturn(run func(ctx context.Context, resourceServerID string, resourceID *string, limit int, offset int) (*resource.ActionList, *common.ServiceError)) *ResourceServiceInterfaceMock_GetActionList_Call { +func (_c *ResourceServiceInterfaceMock_GetActionList_Call) RunAndReturn(run func(ctx context.Context, resourceServerID string, resourceID *string, kind providers.ActionKind, limit int, offset int) (*resource.ActionList, *common.ServiceError)) *ResourceServiceInterfaceMock_GetActionList_Call { _c.Call.Return(run) return _c } diff --git a/backend/tests/mocks/resourcemock/resourceStoreInterface_mock.go b/backend/tests/mocks/resourcemock/resourceStoreInterface_mock.go index a40f6cbfbc..cd39712545 100644 --- a/backend/tests/mocks/resourcemock/resourceStoreInterface_mock.go +++ b/backend/tests/mocks/resourcemock/resourceStoreInterface_mock.go @@ -1145,8 +1145,8 @@ func (_c *resourceStoreInterfaceMock_GetAction_Call) RunAndReturn(run func(ctx c } // GetActionList provides a mock function for the type resourceStoreInterfaceMock -func (_mock *resourceStoreInterfaceMock) GetActionList(ctx context.Context, resServerID string, resID *string, limit int, offset int) ([]providers.Action, error) { - ret := _mock.Called(ctx, resServerID, resID, limit, offset) +func (_mock *resourceStoreInterfaceMock) GetActionList(ctx context.Context, resServerID string, resID *string, kind providers.ActionKind, limit int, offset int) ([]providers.Action, error) { + ret := _mock.Called(ctx, resServerID, resID, kind, limit, offset) if len(ret) == 0 { panic("no return value specified for GetActionList") @@ -1154,18 +1154,18 @@ func (_mock *resourceStoreInterfaceMock) GetActionList(ctx context.Context, resS var r0 []providers.Action var r1 error - if returnFunc, ok := ret.Get(0).(func(context.Context, string, *string, int, int) ([]providers.Action, error)); ok { - return returnFunc(ctx, resServerID, resID, limit, offset) + if returnFunc, ok := ret.Get(0).(func(context.Context, string, *string, providers.ActionKind, int, int) ([]providers.Action, error)); ok { + return returnFunc(ctx, resServerID, resID, kind, limit, offset) } - if returnFunc, ok := ret.Get(0).(func(context.Context, string, *string, int, int) []providers.Action); ok { - r0 = returnFunc(ctx, resServerID, resID, limit, offset) + if returnFunc, ok := ret.Get(0).(func(context.Context, string, *string, providers.ActionKind, int, int) []providers.Action); ok { + r0 = returnFunc(ctx, resServerID, resID, kind, limit, offset) } else { if ret.Get(0) != nil { r0 = ret.Get(0).([]providers.Action) } } - if returnFunc, ok := ret.Get(1).(func(context.Context, string, *string, int, int) error); ok { - r1 = returnFunc(ctx, resServerID, resID, limit, offset) + if returnFunc, ok := ret.Get(1).(func(context.Context, string, *string, providers.ActionKind, int, int) error); ok { + r1 = returnFunc(ctx, resServerID, resID, kind, limit, offset) } else { r1 = ret.Error(1) } @@ -1181,13 +1181,14 @@ type resourceStoreInterfaceMock_GetActionList_Call struct { // - ctx context.Context // - resServerID string // - resID *string +// - kind providers.ActionKind // - limit int // - offset int -func (_e *resourceStoreInterfaceMock_Expecter) GetActionList(ctx interface{}, resServerID interface{}, resID interface{}, limit interface{}, offset interface{}) *resourceStoreInterfaceMock_GetActionList_Call { - return &resourceStoreInterfaceMock_GetActionList_Call{Call: _e.mock.On("GetActionList", ctx, resServerID, resID, limit, offset)} +func (_e *resourceStoreInterfaceMock_Expecter) GetActionList(ctx interface{}, resServerID interface{}, resID interface{}, kind interface{}, limit interface{}, offset interface{}) *resourceStoreInterfaceMock_GetActionList_Call { + return &resourceStoreInterfaceMock_GetActionList_Call{Call: _e.mock.On("GetActionList", ctx, resServerID, resID, kind, limit, offset)} } -func (_c *resourceStoreInterfaceMock_GetActionList_Call) Run(run func(ctx context.Context, resServerID string, resID *string, limit int, offset int)) *resourceStoreInterfaceMock_GetActionList_Call { +func (_c *resourceStoreInterfaceMock_GetActionList_Call) Run(run func(ctx context.Context, resServerID string, resID *string, kind providers.ActionKind, limit int, offset int)) *resourceStoreInterfaceMock_GetActionList_Call { _c.Call.Run(func(args mock.Arguments) { var arg0 context.Context if args[0] != nil { @@ -1201,20 +1202,25 @@ func (_c *resourceStoreInterfaceMock_GetActionList_Call) Run(run func(ctx contex if args[2] != nil { arg2 = args[2].(*string) } - var arg3 int + var arg3 providers.ActionKind if args[3] != nil { - arg3 = args[3].(int) + arg3 = args[3].(providers.ActionKind) } var arg4 int if args[4] != nil { arg4 = args[4].(int) } + var arg5 int + if args[5] != nil { + arg5 = args[5].(int) + } run( arg0, arg1, arg2, arg3, arg4, + arg5, ) }) return _c @@ -1225,14 +1231,14 @@ func (_c *resourceStoreInterfaceMock_GetActionList_Call) Return(actions []provid return _c } -func (_c *resourceStoreInterfaceMock_GetActionList_Call) RunAndReturn(run func(ctx context.Context, resServerID string, resID *string, limit int, offset int) ([]providers.Action, error)) *resourceStoreInterfaceMock_GetActionList_Call { +func (_c *resourceStoreInterfaceMock_GetActionList_Call) RunAndReturn(run func(ctx context.Context, resServerID string, resID *string, kind providers.ActionKind, limit int, offset int) ([]providers.Action, error)) *resourceStoreInterfaceMock_GetActionList_Call { _c.Call.Return(run) return _c } // GetActionListCount provides a mock function for the type resourceStoreInterfaceMock -func (_mock *resourceStoreInterfaceMock) GetActionListCount(ctx context.Context, resServerID string, resID *string) (int, error) { - ret := _mock.Called(ctx, resServerID, resID) +func (_mock *resourceStoreInterfaceMock) GetActionListCount(ctx context.Context, resServerID string, resID *string, kind providers.ActionKind) (int, error) { + ret := _mock.Called(ctx, resServerID, resID, kind) if len(ret) == 0 { panic("no return value specified for GetActionListCount") @@ -1240,16 +1246,16 @@ func (_mock *resourceStoreInterfaceMock) GetActionListCount(ctx context.Context, var r0 int var r1 error - if returnFunc, ok := ret.Get(0).(func(context.Context, string, *string) (int, error)); ok { - return returnFunc(ctx, resServerID, resID) + if returnFunc, ok := ret.Get(0).(func(context.Context, string, *string, providers.ActionKind) (int, error)); ok { + return returnFunc(ctx, resServerID, resID, kind) } - if returnFunc, ok := ret.Get(0).(func(context.Context, string, *string) int); ok { - r0 = returnFunc(ctx, resServerID, resID) + if returnFunc, ok := ret.Get(0).(func(context.Context, string, *string, providers.ActionKind) int); ok { + r0 = returnFunc(ctx, resServerID, resID, kind) } else { r0 = ret.Get(0).(int) } - if returnFunc, ok := ret.Get(1).(func(context.Context, string, *string) error); ok { - r1 = returnFunc(ctx, resServerID, resID) + if returnFunc, ok := ret.Get(1).(func(context.Context, string, *string, providers.ActionKind) error); ok { + r1 = returnFunc(ctx, resServerID, resID, kind) } else { r1 = ret.Error(1) } @@ -1265,11 +1271,12 @@ type resourceStoreInterfaceMock_GetActionListCount_Call struct { // - ctx context.Context // - resServerID string // - resID *string -func (_e *resourceStoreInterfaceMock_Expecter) GetActionListCount(ctx interface{}, resServerID interface{}, resID interface{}) *resourceStoreInterfaceMock_GetActionListCount_Call { - return &resourceStoreInterfaceMock_GetActionListCount_Call{Call: _e.mock.On("GetActionListCount", ctx, resServerID, resID)} +// - kind providers.ActionKind +func (_e *resourceStoreInterfaceMock_Expecter) GetActionListCount(ctx interface{}, resServerID interface{}, resID interface{}, kind interface{}) *resourceStoreInterfaceMock_GetActionListCount_Call { + return &resourceStoreInterfaceMock_GetActionListCount_Call{Call: _e.mock.On("GetActionListCount", ctx, resServerID, resID, kind)} } -func (_c *resourceStoreInterfaceMock_GetActionListCount_Call) Run(run func(ctx context.Context, resServerID string, resID *string)) *resourceStoreInterfaceMock_GetActionListCount_Call { +func (_c *resourceStoreInterfaceMock_GetActionListCount_Call) Run(run func(ctx context.Context, resServerID string, resID *string, kind providers.ActionKind)) *resourceStoreInterfaceMock_GetActionListCount_Call { _c.Call.Run(func(args mock.Arguments) { var arg0 context.Context if args[0] != nil { @@ -1283,10 +1290,15 @@ func (_c *resourceStoreInterfaceMock_GetActionListCount_Call) Run(run func(ctx c if args[2] != nil { arg2 = args[2].(*string) } + var arg3 providers.ActionKind + if args[3] != nil { + arg3 = args[3].(providers.ActionKind) + } run( arg0, arg1, arg2, + arg3, ) }) return _c @@ -1297,7 +1309,7 @@ func (_c *resourceStoreInterfaceMock_GetActionListCount_Call) Return(n int, err return _c } -func (_c *resourceStoreInterfaceMock_GetActionListCount_Call) RunAndReturn(run func(ctx context.Context, resServerID string, resID *string) (int, error)) *resourceStoreInterfaceMock_GetActionListCount_Call { +func (_c *resourceStoreInterfaceMock_GetActionListCount_Call) RunAndReturn(run func(ctx context.Context, resServerID string, resID *string, kind providers.ActionKind) (int, error)) *resourceStoreInterfaceMock_GetActionListCount_Call { _c.Call.Return(run) return _c }