From 0449c423a2fcbec7c150c693673392ae124fcd57 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Mon, 11 May 2026 09:39:58 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=97=9D=EF=B8=8F=20fix:=20Enforce=20Skill?= =?UTF-8?q?=20Share=20Role=20Permission=20(#13062)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: enforce skill share role permission * fix: preserve share capability bypass * refactor: move share policy middleware to api package * style: order share middleware imports * fix: satisfy share middleware type checks * test: cover share policy resource types --- .../middleware/checkSharePublicAccess.js | 90 +----- .../middleware/checkSharePublicAccess.spec.js | 150 +++++++++- api/server/routes/accessPermissions.js | 8 +- .../accessPermissions.sharePolicy.test.js | 211 ++++++++++++++ packages/api/src/middleware/index.ts | 1 + packages/api/src/middleware/share.spec.ts | 264 ++++++++++++++++++ packages/api/src/middleware/share.ts | 248 ++++++++++++++++ 7 files changed, 882 insertions(+), 90 deletions(-) create mode 100644 api/server/routes/accessPermissions.sharePolicy.test.js create mode 100644 packages/api/src/middleware/share.spec.ts create mode 100644 packages/api/src/middleware/share.ts diff --git a/api/server/middleware/checkSharePublicAccess.js b/api/server/middleware/checkSharePublicAccess.js index 945d5521b7..7b9fae395c 100644 --- a/api/server/middleware/checkSharePublicAccess.js +++ b/api/server/middleware/checkSharePublicAccess.js @@ -1,86 +1,8 @@ -const { logger } = require('@librechat/data-schemas'); -const { ResourceType, PermissionTypes, Permissions } = require('librechat-data-provider'); +const { createSharePolicyMiddleware } = require('@librechat/api'); +const { hasCapability } = require('~/server/middleware/roles/capabilities'); const { getRoleByName } = require('~/models'); -/** - * Maps resource types to their corresponding permission types - */ -const resourceToPermissionType = { - [ResourceType.AGENT]: PermissionTypes.AGENTS, - [ResourceType.PROMPTGROUP]: PermissionTypes.PROMPTS, - [ResourceType.MCPSERVER]: PermissionTypes.MCP_SERVERS, - [ResourceType.REMOTE_AGENT]: PermissionTypes.REMOTE_AGENTS, - [ResourceType.SKILL]: PermissionTypes.SKILLS, -}; - -/** - * Middleware to check if user has SHARE_PUBLIC permission for a resource type - * Only enforced when request body contains `public: true` - * @param {import('express').Request} req - Express request - * @param {import('express').Response} res - Express response - * @param {import('express').NextFunction} next - Express next function - */ -const checkSharePublicAccess = async (req, res, next) => { - try { - const { public: isPublic } = req.body; - - // Only check if trying to enable public sharing - if (!isPublic) { - return next(); - } - - const user = req.user; - if (!user || !user.role) { - return res.status(401).json({ - error: 'Unauthorized', - message: 'Authentication required', - }); - } - - const { resourceType } = req.params; - const permissionType = resourceToPermissionType[resourceType]; - - if (!permissionType) { - return res.status(400).json({ - error: 'Bad Request', - message: `Unsupported resource type for public sharing: ${resourceType}`, - }); - } - - const role = await getRoleByName(user.role); - if (!role || !role.permissions) { - return res.status(403).json({ - error: 'Forbidden', - message: 'No permissions configured for user role', - }); - } - - const resourcePerms = role.permissions[permissionType] || {}; - const canSharePublic = resourcePerms[Permissions.SHARE_PUBLIC] === true; - - if (!canSharePublic) { - logger.warn( - `[checkSharePublicAccess][${user.id}] User denied SHARE_PUBLIC for ${resourceType}`, - ); - return res.status(403).json({ - error: 'Forbidden', - message: `You do not have permission to share ${resourceType} resources publicly`, - }); - } - - next(); - } catch (error) { - logger.error( - `[checkSharePublicAccess][${req.user?.id}] Error checking SHARE_PUBLIC permission`, - error, - ); - return res.status(500).json({ - error: 'Internal Server Error', - message: 'Failed to check public sharing permissions', - }); - } -}; - -module.exports = { - checkSharePublicAccess, -}; +module.exports = createSharePolicyMiddleware({ + getRoleByName, + hasCapability, +}); diff --git a/api/server/middleware/checkSharePublicAccess.spec.js b/api/server/middleware/checkSharePublicAccess.spec.js index 605de2049e..762f013962 100644 --- a/api/server/middleware/checkSharePublicAccess.spec.js +++ b/api/server/middleware/checkSharePublicAccess.spec.js @@ -1,8 +1,15 @@ -const { ResourceType, PermissionTypes, Permissions } = require('librechat-data-provider'); -const { checkSharePublicAccess } = require('./checkSharePublicAccess'); -const { getRoleByName } = require('~/models'); +jest.mock('~/models', () => ({ + getRoleByName: jest.fn(), +})); -jest.mock('~/models'); +jest.mock('~/server/middleware/roles/capabilities', () => ({ + hasCapability: jest.fn(), +})); + +const { ResourceType, PermissionTypes, Permissions } = require('librechat-data-provider'); +const { hasCapability } = require('~/server/middleware/roles/capabilities'); +const { checkShareAccess, checkSharePublicAccess } = require('./checkSharePublicAccess'); +const { getRoleByName } = require('~/models'); describe('checkSharePublicAccess middleware', () => { let mockReq; @@ -11,6 +18,7 @@ describe('checkSharePublicAccess middleware', () => { beforeEach(() => { jest.clearAllMocks(); + hasCapability.mockResolvedValue(false); mockReq = { user: { id: 'user123', role: 'USER' }, params: { resourceType: ResourceType.AGENT }, @@ -162,3 +170,137 @@ describe('checkSharePublicAccess middleware', () => { }); }); }); + +describe('checkShareAccess middleware', () => { + let mockReq; + let mockRes; + let mockNext; + + beforeEach(() => { + jest.clearAllMocks(); + hasCapability.mockResolvedValue(false); + mockReq = { + user: { id: 'user123', role: 'USER' }, + params: { resourceType: ResourceType.SKILL }, + body: { updated: [] }, + }; + mockRes = { + status: jest.fn().mockReturnThis(), + json: jest.fn(), + }; + mockNext = jest.fn(); + }); + + it('should return 403 when user role has no SHARE permission for skills', async () => { + getRoleByName.mockResolvedValue({ + permissions: { + [PermissionTypes.SKILLS]: { + [Permissions.SHARE]: false, + [Permissions.SHARE_PUBLIC]: false, + }, + }, + }); + + await checkShareAccess(mockReq, mockRes, mockNext); + + expect(mockRes.status).toHaveBeenCalledWith(403); + expect(mockRes.json).toHaveBeenCalledWith({ + error: 'Forbidden', + message: `You do not have permission to share ${ResourceType.SKILL} resources`, + }); + expect(mockNext).not.toHaveBeenCalled(); + }); + + it('should call next() when user has resource management capability for skills', async () => { + hasCapability.mockResolvedValue(true); + + await checkShareAccess(mockReq, mockRes, mockNext); + + expect(hasCapability).toHaveBeenCalledWith(mockReq.user, 'manage:skills'); + expect(getRoleByName).not.toHaveBeenCalled(); + expect(mockNext).toHaveBeenCalled(); + expect(mockRes.status).not.toHaveBeenCalled(); + }); + + it('should call next() when user role has SHARE permission for skills', async () => { + getRoleByName.mockResolvedValue({ + permissions: { + [PermissionTypes.SKILLS]: { + [Permissions.SHARE]: true, + [Permissions.SHARE_PUBLIC]: false, + }, + }, + }); + + await checkShareAccess(mockReq, mockRes, mockNext); + + expect(hasCapability).toHaveBeenCalledWith(mockReq.user, 'manage:skills'); + expect(mockNext).toHaveBeenCalled(); + expect(mockRes.status).not.toHaveBeenCalled(); + }); + + it('should return 401 when user is not authenticated', async () => { + mockReq.user = null; + + await checkShareAccess(mockReq, mockRes, mockNext); + + expect(mockRes.status).toHaveBeenCalledWith(401); + expect(mockRes.json).toHaveBeenCalledWith({ + error: 'Unauthorized', + message: 'Authentication required', + }); + expect(mockNext).not.toHaveBeenCalled(); + }); + + it('should return 400 for unsupported resource type', async () => { + mockReq.params = { resourceType: 'unsupported' }; + + await checkShareAccess(mockReq, mockRes, mockNext); + + expect(mockRes.status).toHaveBeenCalledWith(400); + expect(mockRes.json).toHaveBeenCalledWith({ + error: 'Bad Request', + message: 'Unsupported resource type for sharing: unsupported', + }); + }); + + it('should return 403 when role has no permissions object', async () => { + getRoleByName.mockResolvedValue({ permissions: null }); + + await checkShareAccess(mockReq, mockRes, mockNext); + + expect(mockRes.status).toHaveBeenCalledWith(403); + expect(mockNext).not.toHaveBeenCalled(); + }); + + it('should return 500 on error', async () => { + getRoleByName.mockRejectedValue(new Error('Database error')); + + await checkShareAccess(mockReq, mockRes, mockNext); + + expect(mockRes.status).toHaveBeenCalledWith(500); + expect(mockRes.json).toHaveBeenCalledWith({ + error: 'Internal Server Error', + message: 'Failed to check sharing permissions', + }); + }); + + it('should reuse the role permission lookup for public sharing checks', async () => { + mockReq.body = { updated: [], public: true }; + getRoleByName.mockResolvedValue({ + permissions: { + [PermissionTypes.SKILLS]: { + [Permissions.SHARE]: true, + [Permissions.SHARE_PUBLIC]: true, + }, + }, + }); + + await checkShareAccess(mockReq, mockRes, mockNext); + await checkSharePublicAccess(mockReq, mockRes, mockNext); + + expect(getRoleByName).toHaveBeenCalledTimes(1); + expect(mockNext).toHaveBeenCalledTimes(2); + expect(mockRes.status).not.toHaveBeenCalled(); + }); +}); diff --git a/api/server/routes/accessPermissions.js b/api/server/routes/accessPermissions.js index 01e3196b8f..e53d0ef1a7 100644 --- a/api/server/routes/accessPermissions.js +++ b/api/server/routes/accessPermissions.js @@ -8,9 +8,12 @@ const { getResourceRoles, searchPrincipals, } = require('~/server/controllers/PermissionsController'); +const { + checkShareAccess, + checkSharePublicAccess, +} = require('~/server/middleware/checkSharePublicAccess'); const { requireJwtAuth, checkBan, uaParser, canAccessResource } = require('~/server/middleware'); const { checkPeoplePickerAccess } = require('~/server/middleware/checkPeoplePickerAccess'); -const { checkSharePublicAccess } = require('~/server/middleware/checkSharePublicAccess'); const { findMCPServerByObjectId, getSkillById } = require('~/models'); const router = express.Router(); @@ -104,12 +107,13 @@ router.get( /** * PUT /api/permissions/{resourceType}/{resourceId} * Bulk update permissions for a specific resource - * SECURITY: Requires SHARE permission to modify resource permissions + * SECURITY: Requires resource ACL SHARE and role SHARE to modify resource permissions * SECURITY: Requires SHARE_PUBLIC permission to enable public sharing */ router.put( '/:resourceType/:resourceId', checkResourcePermissionAccess(PermissionBits.SHARE), + checkShareAccess, checkSharePublicAccess, updateResourcePermissions, ); diff --git a/api/server/routes/accessPermissions.sharePolicy.test.js b/api/server/routes/accessPermissions.sharePolicy.test.js new file mode 100644 index 0000000000..0fc7a90dea --- /dev/null +++ b/api/server/routes/accessPermissions.sharePolicy.test.js @@ -0,0 +1,211 @@ +jest.mock('~/models', () => ({ + getRoleByName: jest.fn(), + findMCPServerByObjectId: jest.fn(), + getSkillById: jest.fn(), +})); + +jest.mock('~/server/middleware', () => ({ + requireJwtAuth: (req, _res, next) => next(), + checkBan: (_req, _res, next) => next(), + uaParser: (_req, _res, next) => next(), + canAccessResource: jest.fn(() => (_req, _res, next) => next()), +})); + +jest.mock('~/server/middleware/checkPeoplePickerAccess', () => ({ + checkPeoplePickerAccess: jest.fn((_req, _res, next) => next()), +})); + +jest.mock('~/server/middleware/roles/capabilities', () => ({ + hasCapability: jest.fn(), +})); + +jest.mock('~/server/controllers/PermissionsController', () => ({ + getUserEffectivePermissions: jest.fn((_req, res) => res.json({ permissions: [] })), + getAllEffectivePermissions: jest.fn((_req, res) => res.json({ permissions: [] })), + updateResourcePermissions: jest.fn((_req, res) => res.json({ success: true })), + getResourcePermissions: jest.fn((_req, res) => res.json({ permissions: [] })), + getResourceRoles: jest.fn((_req, res) => res.json({ roles: [] })), + searchPrincipals: jest.fn((_req, res) => res.json({ principals: [] })), +})); + +const express = require('express'); +const request = require('supertest'); +const { + SystemRoles, + ResourceType, + AccessRoleIds, + PrincipalType, + PermissionBits, + PermissionTypes, + Permissions, +} = require('librechat-data-provider'); + +const { updateResourcePermissions } = require('~/server/controllers/PermissionsController'); +const { hasCapability } = require('~/server/middleware/roles/capabilities'); +const { canAccessResource } = require('~/server/middleware'); +const accessPermissionsRouter = require('./accessPermissions'); +const { getRoleByName } = require('~/models'); + +describe('Access permissions share policy', () => { + let app; + + const resourceId = '507f1f77bcf86cd799439011'; + const sharePolicyCases = [ + { + label: 'agent', + resourceType: ResourceType.AGENT, + permissionType: PermissionTypes.AGENTS, + accessRoleId: AccessRoleIds.AGENT_VIEWER, + middlewareOptions: { + resourceType: ResourceType.AGENT, + requiredPermission: PermissionBits.SHARE, + resourceIdParam: 'resourceId', + }, + }, + { + label: 'prompt group', + resourceType: ResourceType.PROMPTGROUP, + permissionType: PermissionTypes.PROMPTS, + accessRoleId: AccessRoleIds.PROMPTGROUP_VIEWER, + middlewareOptions: { + resourceType: ResourceType.PROMPTGROUP, + requiredPermission: PermissionBits.SHARE, + resourceIdParam: 'resourceId', + }, + }, + { + label: 'MCP server', + resourceType: ResourceType.MCPSERVER, + permissionType: PermissionTypes.MCP_SERVERS, + accessRoleId: AccessRoleIds.MCPSERVER_VIEWER, + middlewareOptions: { + resourceType: ResourceType.MCPSERVER, + requiredPermission: PermissionBits.SHARE, + resourceIdParam: 'resourceId', + idResolver: expect.any(Function), + }, + }, + { + label: 'remote agent', + resourceType: ResourceType.REMOTE_AGENT, + permissionType: PermissionTypes.REMOTE_AGENTS, + accessRoleId: AccessRoleIds.REMOTE_AGENT_VIEWER, + middlewareOptions: { + resourceType: ResourceType.REMOTE_AGENT, + requiredPermission: PermissionBits.SHARE, + resourceIdParam: 'resourceId', + }, + }, + { + label: 'skill', + resourceType: ResourceType.SKILL, + permissionType: PermissionTypes.SKILLS, + accessRoleId: AccessRoleIds.SKILL_VIEWER, + middlewareOptions: { + resourceType: ResourceType.SKILL, + requiredPermission: PermissionBits.SHARE, + resourceIdParam: 'resourceId', + idResolver: expect.any(Function), + }, + }, + ]; + + const createUpdatedPrincipal = (accessRoleId) => ({ + type: PrincipalType.USER, + id: 'target-user', + accessRoleId, + }); + + beforeEach(() => { + jest.clearAllMocks(); + hasCapability.mockResolvedValue(false); + + app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + req.user = { id: 'skill-owner', role: SystemRoles.USER }; + next(); + }); + app.use('/api/permissions', accessPermissionsRouter); + }); + + it.each(sharePolicyCases)( + 'blocks non-public $label sharing when ACL SHARE passes but role SHARE is disabled', + async ({ resourceType, permissionType, accessRoleId, middlewareOptions }) => { + getRoleByName.mockResolvedValue({ + permissions: { + [permissionType]: { + [Permissions.SHARE]: false, + [Permissions.SHARE_PUBLIC]: false, + }, + }, + }); + + const response = await request(app) + .put(`/api/permissions/${resourceType}/${resourceId}`) + .send({ updated: [createUpdatedPrincipal(accessRoleId)], public: false }); + + expect(response.status).toBe(403); + expect(response.body).toEqual({ + error: 'Forbidden', + message: `You do not have permission to share ${resourceType} resources`, + }); + expect(canAccessResource).toHaveBeenCalledWith(middlewareOptions); + expect(updateResourcePermissions).not.toHaveBeenCalled(); + }, + ); + + it('allows non-public skill sharing when both ACL SHARE and role SKILLS.SHARE pass', async () => { + getRoleByName.mockResolvedValue({ + permissions: { + [PermissionTypes.SKILLS]: { + [Permissions.SHARE]: true, + [Permissions.SHARE_PUBLIC]: false, + }, + }, + }); + + const response = await request(app) + .put(`/api/permissions/${ResourceType.SKILL}/${resourceId}`) + .send({ updated: [createUpdatedPrincipal(AccessRoleIds.SKILL_VIEWER)], public: false }); + + expect(response.status).toBe(200); + expect(response.body).toEqual({ success: true }); + expect(updateResourcePermissions).toHaveBeenCalledTimes(1); + }); + + it('preserves resource management capability bypass for non-public skill sharing', async () => { + hasCapability.mockResolvedValue(true); + + const response = await request(app) + .put(`/api/permissions/${ResourceType.SKILL}/${resourceId}`) + .send({ updated: [createUpdatedPrincipal(AccessRoleIds.SKILL_VIEWER)], public: false }); + + expect(response.status).toBe(200); + expect(response.body).toEqual({ success: true }); + expect(getRoleByName).not.toHaveBeenCalled(); + expect(updateResourcePermissions).toHaveBeenCalledTimes(1); + }); + + it('still requires SHARE_PUBLIC when enabling public skill sharing', async () => { + getRoleByName.mockResolvedValue({ + permissions: { + [PermissionTypes.SKILLS]: { + [Permissions.SHARE]: true, + [Permissions.SHARE_PUBLIC]: false, + }, + }, + }); + + const response = await request(app) + .put(`/api/permissions/${ResourceType.SKILL}/${resourceId}`) + .send({ public: true, publicAccessRoleId: AccessRoleIds.SKILL_VIEWER }); + + expect(response.status).toBe(403); + expect(response.body).toEqual({ + error: 'Forbidden', + message: `You do not have permission to share ${ResourceType.SKILL} resources publicly`, + }); + expect(updateResourcePermissions).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/api/src/middleware/index.ts b/packages/api/src/middleware/index.ts index 9fccfa6780..f14e65fc49 100644 --- a/packages/api/src/middleware/index.ts +++ b/packages/api/src/middleware/index.ts @@ -10,3 +10,4 @@ export { preAuthTenantMiddleware } from './preAuthTenant'; export * from './concurrency'; export * from './checkBalance'; export * from './remoteAgentAuth'; +export * from './share'; diff --git a/packages/api/src/middleware/share.spec.ts b/packages/api/src/middleware/share.spec.ts new file mode 100644 index 0000000000..3924d4bf6c --- /dev/null +++ b/packages/api/src/middleware/share.spec.ts @@ -0,0 +1,264 @@ +jest.mock('@librechat/data-schemas', () => ({ + ...jest.requireActual('@librechat/data-schemas'), + logger: { + warn: jest.fn(), + error: jest.fn(), + }, +})); + +import { Permissions, PermissionTypes, ResourceType } from 'librechat-data-provider'; +import type { NextFunction, Response } from 'express'; +import type { IRole } from '@librechat/data-schemas'; +import type { ServerRequest } from '~/types/http'; +import type { SharePolicyDeps } from './share'; +import { createSharePolicyMiddleware } from './share'; + +type ShareTestRequest = ServerRequest & { + params: { + resourceType?: string; + }; + body: ServerRequest['body'] & { + public?: boolean; + }; +}; + +const createResponse = (): Response => { + const res = { + status: jest.fn().mockReturnThis(), + json: jest.fn(), + } as Partial; + + return res as Response; +}; + +const createRequest = (overrides: Partial = {}): ShareTestRequest => + ({ + user: { id: 'user123', role: 'USER' }, + params: { resourceType: ResourceType.SKILL }, + body: {}, + ...overrides, + }) as ShareTestRequest; + +const createRole = (permissions: IRole['permissions']): IRole => + ({ + permissions, + }) as IRole; + +describe('createSharePolicyMiddleware', () => { + let getRoleByName: jest.MockedFunction; + let hasCapability: jest.MockedFunction; + let next: jest.MockedFunction; + + beforeEach(() => { + getRoleByName = jest.fn(); + hasCapability = jest.fn().mockResolvedValue(false); + next = jest.fn(); + }); + + it('skips public sharing checks when public is not true', async () => { + const { checkSharePublicAccess } = createSharePolicyMiddleware({ + getRoleByName, + hasCapability, + }); + const req = createRequest({ body: { public: false } }); + const res = createResponse(); + + await checkSharePublicAccess(req, res, next); + + expect(next).toHaveBeenCalled(); + expect(res.status).not.toHaveBeenCalled(); + expect(getRoleByName).not.toHaveBeenCalled(); + }); + + it('blocks non-public skill sharing when role SKILLS.SHARE is disabled', async () => { + const { checkShareAccess } = createSharePolicyMiddleware({ + getRoleByName, + hasCapability, + }); + getRoleByName.mockResolvedValue( + createRole({ + [PermissionTypes.SKILLS]: { + [Permissions.SHARE]: false, + [Permissions.SHARE_PUBLIC]: false, + }, + }), + ); + const req = createRequest(); + const res = createResponse(); + + await checkShareAccess(req, res, next); + + expect(res.status).toHaveBeenCalledWith(403); + expect(res.json).toHaveBeenCalledWith({ + error: 'Forbidden', + message: `You do not have permission to share ${ResourceType.SKILL} resources`, + }); + expect(next).not.toHaveBeenCalled(); + }); + + it('allows non-public skill sharing when role SKILLS.SHARE is enabled', async () => { + const { checkShareAccess } = createSharePolicyMiddleware({ + getRoleByName, + hasCapability, + }); + getRoleByName.mockResolvedValue( + createRole({ + [PermissionTypes.SKILLS]: { + [Permissions.SHARE]: true, + [Permissions.SHARE_PUBLIC]: false, + }, + }), + ); + const req = createRequest(); + const res = createResponse(); + + await checkShareAccess(req, res, next); + + expect(hasCapability).toHaveBeenCalledWith( + { id: 'user123', role: 'USER', tenantId: undefined }, + 'manage:skills', + ); + expect(next).toHaveBeenCalled(); + expect(res.status).not.toHaveBeenCalled(); + }); + + it('preserves resource management capability bypass for skills', async () => { + const { checkShareAccess } = createSharePolicyMiddleware({ + getRoleByName, + hasCapability, + }); + hasCapability.mockResolvedValue(true); + const req = createRequest(); + const res = createResponse(); + + await checkShareAccess(req, res, next); + + expect(hasCapability).toHaveBeenCalledWith( + { id: 'user123', role: 'USER', tenantId: undefined }, + 'manage:skills', + ); + expect(getRoleByName).not.toHaveBeenCalled(); + expect(next).toHaveBeenCalled(); + expect(res.status).not.toHaveBeenCalled(); + }); + + it('still requires SHARE_PUBLIC when public sharing is enabled', async () => { + const { checkSharePublicAccess } = createSharePolicyMiddleware({ + getRoleByName, + hasCapability, + }); + getRoleByName.mockResolvedValue( + createRole({ + [PermissionTypes.SKILLS]: { + [Permissions.SHARE]: true, + [Permissions.SHARE_PUBLIC]: false, + }, + }), + ); + const req = createRequest({ body: { public: true } }); + const res = createResponse(); + + await checkSharePublicAccess(req, res, next); + + expect(res.status).toHaveBeenCalledWith(403); + expect(res.json).toHaveBeenCalledWith({ + error: 'Forbidden', + message: `You do not have permission to share ${ResourceType.SKILL} resources publicly`, + }); + expect(next).not.toHaveBeenCalled(); + }); + + it('reuses the role permission lookup for public sharing checks', async () => { + const { checkShareAccess, checkSharePublicAccess } = createSharePolicyMiddleware({ + getRoleByName, + hasCapability, + }); + getRoleByName.mockResolvedValue( + createRole({ + [PermissionTypes.SKILLS]: { + [Permissions.SHARE]: true, + [Permissions.SHARE_PUBLIC]: true, + }, + }), + ); + const req = createRequest({ body: { public: true } }); + const res = createResponse(); + + await checkShareAccess(req, res, next); + await checkSharePublicAccess(req, res, next); + + expect(getRoleByName).toHaveBeenCalledTimes(1); + expect(next).toHaveBeenCalledTimes(2); + expect(res.status).not.toHaveBeenCalled(); + }); + + it('returns 401 when user is not authenticated', async () => { + const { checkShareAccess } = createSharePolicyMiddleware({ + getRoleByName, + hasCapability, + }); + const req = createRequest({ user: undefined }); + const res = createResponse(); + + await checkShareAccess(req, res, next); + + expect(res.status).toHaveBeenCalledWith(401); + expect(res.json).toHaveBeenCalledWith({ + error: 'Unauthorized', + message: 'Authentication required', + }); + expect(next).not.toHaveBeenCalled(); + }); + + it('returns 400 for unsupported resource type', async () => { + const { checkShareAccess } = createSharePolicyMiddleware({ + getRoleByName, + hasCapability, + }); + const req = createRequest({ params: { resourceType: 'unsupported' } }); + const res = createResponse(); + + await checkShareAccess(req, res, next); + + expect(res.status).toHaveBeenCalledWith(400); + expect(res.json).toHaveBeenCalledWith({ + error: 'Bad Request', + message: 'Unsupported resource type for sharing: unsupported', + }); + }); + + it('returns 403 when role has no permissions object', async () => { + const { checkShareAccess } = createSharePolicyMiddleware({ + getRoleByName, + hasCapability, + }); + const role = createRole({}); + Object.defineProperty(role, 'permissions', { value: null }); + getRoleByName.mockResolvedValue(role); + const req = createRequest(); + const res = createResponse(); + + await checkShareAccess(req, res, next); + + expect(res.status).toHaveBeenCalledWith(403); + expect(next).not.toHaveBeenCalled(); + }); + + it('returns 500 when role lookup fails', async () => { + const { checkShareAccess } = createSharePolicyMiddleware({ + getRoleByName, + hasCapability, + }); + getRoleByName.mockRejectedValue(new Error('Database error')); + const req = createRequest(); + const res = createResponse(); + + await checkShareAccess(req, res, next); + + expect(res.status).toHaveBeenCalledWith(500); + expect(res.json).toHaveBeenCalledWith({ + error: 'Internal Server Error', + message: 'Failed to check sharing permissions', + }); + }); +}); diff --git a/packages/api/src/middleware/share.ts b/packages/api/src/middleware/share.ts new file mode 100644 index 0000000000..ec5074a6cc --- /dev/null +++ b/packages/api/src/middleware/share.ts @@ -0,0 +1,248 @@ +import { logger, ResourceCapabilityMap } from '@librechat/data-schemas'; +import { Permissions, PermissionTypes, ResourceType } from 'librechat-data-provider'; +import type { NextFunction, Response } from 'express'; +import type { IRole } from '@librechat/data-schemas'; +import type { CapabilityUser, HasCapabilityFn } from './capabilities'; +import type { RequestBody, ServerRequest } from '~/types/http'; + +type ShareResourcePermissions = Partial>; + +interface SharePermissionCache { + cacheKey: string; + resourcePerms: ShareResourcePermissions; +} + +type ShareRequest = ServerRequest & { + params: { + resourceType?: string; + }; + body: RequestBody & { + public?: boolean; + }; + sharePermissionContext?: SharePermissionCache; +}; + +interface ShareContext { + user: CapabilityUser; + resourceType: ResourceType; + permissionType: PermissionTypes; +} + +export interface SharePolicyDeps { + getRoleByName: (roleName: string, fieldsToSelect?: string | string[]) => Promise; + hasCapability: HasCapabilityFn; +} + +type ShareMiddleware = ( + req: ShareRequest, + res: Response, + next: NextFunction, +) => Promise; + +const resourceToPermissionType: Record = { + [ResourceType.AGENT]: PermissionTypes.AGENTS, + [ResourceType.PROMPTGROUP]: PermissionTypes.PROMPTS, + [ResourceType.MCPSERVER]: PermissionTypes.MCP_SERVERS, + [ResourceType.REMOTE_AGENT]: PermissionTypes.REMOTE_AGENTS, + [ResourceType.SKILL]: PermissionTypes.SKILLS, +}; + +function formatError(error: unknown): string { + if (error instanceof Error) { + return error.message; + } + return String(error); +} + +function getShareContext(req: ShareRequest, res: Response, action: string): ShareContext | null { + const { user } = req; + const role = user?.role; + if (!user || !role) { + res.status(401).json({ + error: 'Unauthorized', + message: 'Authentication required', + }); + return null; + } + + const resourceType = req.params.resourceType as ResourceType | undefined; + const permissionType = resourceType ? resourceToPermissionType[resourceType] : undefined; + if (!resourceType || !permissionType) { + res.status(400).json({ + error: 'Bad Request', + message: `Unsupported resource type for ${action}: ${req.params.resourceType}`, + }); + return null; + } + + return { + user: { + id: user.id, + role, + tenantId: user.tenantId, + }, + resourceType, + permissionType, + }; +} + +export function createSharePolicyMiddleware({ getRoleByName, hasCapability }: SharePolicyDeps): { + checkShareAccess: ShareMiddleware; + checkSharePublicAccess: ShareMiddleware; +} { + async function getResourcePerms( + req: ShareRequest, + res: Response, + action: string, + context?: ShareContext, + ): Promise<{ + user: CapabilityUser; + resourceType: ResourceType; + resourcePerms: ShareResourcePermissions; + } | null> { + const resolvedContext = context ?? getShareContext(req, res, action); + if (!resolvedContext) { + return null; + } + + const { user, resourceType, permissionType } = resolvedContext; + const cacheKey = `${user.role}:${resourceType}`; + const cached = req.sharePermissionContext; + if (cached?.cacheKey === cacheKey) { + return { + user, + resourceType, + resourcePerms: cached.resourcePerms, + }; + } + + const role = await getRoleByName(user.role); + if (!role?.permissions) { + res.status(403).json({ + error: 'Forbidden', + message: 'No permissions configured for user role', + }); + return null; + } + + const resourcePerms = role.permissions[permissionType] ?? {}; + req.sharePermissionContext = { + cacheKey, + resourcePerms, + }; + + return { + user, + resourceType, + resourcePerms, + }; + } + + async function hasResourceManagementCapability( + user: CapabilityUser, + resourceType: ResourceType, + ): Promise { + const capability = ResourceCapabilityMap[resourceType]; + if (!capability) { + return false; + } + + try { + return await hasCapability(user, capability); + } catch (error) { + logger.warn( + `[checkShareAccess] capability check failed, denying bypass: ${formatError(error)}`, + ); + return false; + } + } + + async function checkShareAccess( + req: ShareRequest, + res: Response, + next: NextFunction, + ): Promise { + try { + const context = getShareContext(req, res, 'sharing'); + if (!context) { + return; + } + + if (await hasResourceManagementCapability(context.user, context.resourceType)) { + return next(); + } + + const result = await getResourcePerms(req, res, 'sharing', context); + if (!result) { + return; + } + + const { user, resourceType, resourcePerms } = result; + const canShare = resourcePerms[Permissions.SHARE] === true; + + if (!canShare) { + logger.warn(`[checkShareAccess][${user.id}] User denied SHARE for ${resourceType}`); + return res.status(403).json({ + error: 'Forbidden', + message: `You do not have permission to share ${resourceType} resources`, + }); + } + + return next(); + } catch (error) { + logger.error(`[checkShareAccess][${req.user?.id}] Error checking SHARE permission`, error); + return res.status(500).json({ + error: 'Internal Server Error', + message: 'Failed to check sharing permissions', + }); + } + } + + async function checkSharePublicAccess( + req: ShareRequest, + res: Response, + next: NextFunction, + ): Promise { + try { + const { public: isPublic } = req.body; + + if (!isPublic) { + return next(); + } + + const result = await getResourcePerms(req, res, 'public sharing'); + if (!result) { + return; + } + + const { user, resourceType, resourcePerms } = result; + const canSharePublic = resourcePerms[Permissions.SHARE_PUBLIC] === true; + + if (!canSharePublic) { + logger.warn( + `[checkSharePublicAccess][${user.id}] User denied SHARE_PUBLIC for ${resourceType}`, + ); + return res.status(403).json({ + error: 'Forbidden', + message: `You do not have permission to share ${resourceType} resources publicly`, + }); + } + + return next(); + } catch (error) { + logger.error( + `[checkSharePublicAccess][${req.user?.id}] Error checking SHARE_PUBLIC permission`, + error, + ); + return res.status(500).json({ + error: 'Internal Server Error', + message: 'Failed to check public sharing permissions', + }); + } + } + + return { + checkShareAccess, + checkSharePublicAccess, + }; +}