From 2e3d66cfe288be3ca75f26eee2fad3b482dd4803 Mon Sep 17 00:00:00 2001 From: Dustin Healy <54083382+dustinhealy@users.noreply.github.com> Date: Thu, 26 Mar 2026 14:36:18 -0700 Subject: [PATCH] =?UTF-8?q?=F0=9F=91=A5=20feat:=20Admin=20Groups=20API=20E?= =?UTF-8?q?ndpoints=20(#12387)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: add listGroups and deleteGroup methods to userGroup * feat: add admin groups handler factory and Express routes * fix: address convention violations in admin groups handlers * fix: address Copilot review findings in admin groups handlers - Escape regex in listGroups to prevent injection/ReDoS - Validate ObjectId format in all handlers accepting id/userId params - Replace N+1 findUser loop with batched findUsers query - Remove unused findGroupsByMemberId from dep interface - Map Mongoose ValidationError to 400 in create/update handlers - Validate name in updateGroupHandler (reject empty/whitespace) - Handle null updateGroupById result (race condition) - Tighten error message matching in add/remove member handlers * test: add unit tests for admin groups handlers * fix: address code review findings for admin groups Atomic delete/update handlers (single DB trip), pass through idOnTheSource, add removeMemberById for non-ObjectId members, deduplicate member results, fix error message exposure, add hard cap/sort to listGroups, replace GroupListFilter with Pick of GroupFilterOptions, validate memberIds as array, trim name in update, fix import order, and improve test hygiene with fresh IDs per test. * fix: cascade cleanup, pagination, and test coverage for admin groups Add deleteGrantsForPrincipal to systemGrant data layer and wire cascade cleanup (Config, AclEntry, SystemGrant) into deleteGroupHandler. Add limit/offset pagination to getGroupMembers. Guard empty PATCH bodies with 400. Remove dead type guard and unnecessary type cast. Add 11 new tests covering cascade delete, idempotent member removal, empty update, search filter, 500 error paths, and pagination. * fix: harden admin groups with cascade resilience, type safety, and fallback removal Wrap cascade cleanup in inner try/catch so partial failure logs but still returns 200 (group is already deleted). Replace Record on deleteAclEntries with proper typed filter. Log warning for unmapped user ObjectIds in createGroup memberIds. Add removeMemberById fallback when removeUserFromGroup throws User not found for ObjectId-format userId. Extract VALID_GROUP_SOURCES constant. Add 3 new tests (60 total). * refactor: add countGroups, pagination, and projection type to data layer Extract buildGroupQuery helper, add countGroups method, support limit/offset/skip in listGroups, standardize session handling to .session(session ?? null), and tighten projection parameter from Record to Record. * fix: cascade resilience, pagination, validation, and error clarity for admin groups - Use Promise.allSettled for cascade cleanup so all steps run even if one fails; log individual rejections - Echo deleted group id in delete response - Add countGroups dep and wire limit/offset pagination for listGroups - Deduplicate memberIds before computing total in getGroupMembers - Use { memberIds: 1 } projection in getGroupMembers - Cap memberIds at 500 entries in createGroup - Reject search queries exceeding 200 characters - Clarify addGroupMember error for non-ObjectId userId - Document deleted-user fallback limitation in removeGroupMember * test: extend handler and DB-layer test coverage for admin groups Handler tests: projection assertion, dedup total, memberIds cap, search max length, non-ObjectId memberIds passthrough, cascade partial failure resilience, dedup scenarios, echo id in delete response. DB-layer tests: listGroups sort/filter/pagination, countGroups, deleteGroup, removeMemberById, deleteGrantsForPrincipal. * fix: cast group principalId to ObjectId for ACL entry cleanup deleteAclEntries is a thin deleteMany wrapper with no type casting, but grantPermission stores group principalId as ObjectId. Passing the raw string from req.params would leave orphaned ACL entries on group deletion. * refactor: remove redundant pagination clamping from DB listGroups Handler already clamps limit/offset at the API boundary. The DB method is a general-purpose building block and should not re-validate. * fix: add source and name validation, import order, and test coverage for admin groups - Validate source against VALID_GROUP_SOURCES in createGroupHandler - Cap name at 500 characters in both create and update handlers - Document total as upper bound in getGroupMembers response - Document ObjectId requirement for deleteAclEntries in cascade - Fix import ordering in test file (local value after type imports) - Add tests for updateGroup with description, email, avatar fields - Add tests for invalid source and name max-length in both handlers * fix: add field length caps, flatten nested try/catch, and fix logger level in admin groups Add max-length validation for description, email, avatar, and idOnTheSource in create/update handlers. Extract removeObjectIdMember helper to flatten nested try/catch per never-nesting convention. Downgrade unmapped-memberIds log from error to warn. Fix type import ordering and add missing await in removeMemberById for consistency. --- api/server/index.js | 1 + api/server/routes/admin/groups.js | 41 + api/server/routes/index.js | 2 + packages/api/src/admin/groups.spec.ts | 1348 +++++++++++++++++ packages/api/src/admin/groups.ts | 482 ++++++ packages/api/src/admin/index.ts | 2 + .../src/methods/systemGrant.spec.ts | 62 + .../data-schemas/src/methods/systemGrant.ts | 16 + packages/data-schemas/src/methods/user.ts | 14 + .../src/methods/userGroup.methods.spec.ts | 149 ++ .../data-schemas/src/methods/userGroup.ts | 102 +- 11 files changed, 2216 insertions(+), 3 deletions(-) create mode 100644 api/server/routes/admin/groups.js create mode 100644 packages/api/src/admin/groups.spec.ts create mode 100644 packages/api/src/admin/groups.ts diff --git a/api/server/index.js b/api/server/index.js index de99f06701..4ecc966476 100644 --- a/api/server/index.js +++ b/api/server/index.js @@ -144,6 +144,7 @@ const startServer = async () => { app.use('/api/auth', routes.auth); app.use('/api/admin', routes.adminAuth); app.use('/api/admin/config', routes.adminConfig); + app.use('/api/admin/groups', routes.adminGroups); app.use('/api/actions', routes.actions); app.use('/api/keys', routes.keys); app.use('/api/api-keys', routes.apiKeys); diff --git a/api/server/routes/admin/groups.js b/api/server/routes/admin/groups.js new file mode 100644 index 0000000000..7ca93acaa2 --- /dev/null +++ b/api/server/routes/admin/groups.js @@ -0,0 +1,41 @@ +const express = require('express'); +const { createAdminGroupsHandlers } = require('@librechat/api'); +const { SystemCapabilities } = require('@librechat/data-schemas'); +const { requireCapability } = require('~/server/middleware/roles/capabilities'); +const { requireJwtAuth } = require('~/server/middleware'); +const db = require('~/models'); + +const router = express.Router(); + +const requireAdminAccess = requireCapability(SystemCapabilities.ACCESS_ADMIN); +const requireReadGroups = requireCapability(SystemCapabilities.READ_GROUPS); +const requireManageGroups = requireCapability(SystemCapabilities.MANAGE_GROUPS); + +const handlers = createAdminGroupsHandlers({ + listGroups: db.listGroups, + countGroups: db.countGroups, + findGroupById: db.findGroupById, + createGroup: db.createGroup, + updateGroupById: db.updateGroupById, + deleteGroup: db.deleteGroup, + addUserToGroup: db.addUserToGroup, + removeUserFromGroup: db.removeUserFromGroup, + removeMemberById: db.removeMemberById, + findUsers: db.findUsers, + deleteConfig: db.deleteConfig, + deleteAclEntries: db.deleteAclEntries, + deleteGrantsForPrincipal: db.deleteGrantsForPrincipal, +}); + +router.use(requireJwtAuth, requireAdminAccess); + +router.get('/', requireReadGroups, handlers.listGroups); +router.post('/', requireManageGroups, handlers.createGroup); +router.get('/:id', requireReadGroups, handlers.getGroup); +router.patch('/:id', requireManageGroups, handlers.updateGroup); +router.delete('/:id', requireManageGroups, handlers.deleteGroup); +router.get('/:id/members', requireReadGroups, handlers.getGroupMembers); +router.post('/:id/members', requireManageGroups, handlers.addGroupMember); +router.delete('/:id/members/:userId', requireManageGroups, handlers.removeGroupMember); + +module.exports = router; diff --git a/api/server/routes/index.js b/api/server/routes/index.js index b1f16d5e3c..f9a088649c 100644 --- a/api/server/routes/index.js +++ b/api/server/routes/index.js @@ -3,6 +3,7 @@ const assistants = require('./assistants'); const categories = require('./categories'); const adminAuth = require('./admin/auth'); const adminConfig = require('./admin/config'); +const adminGroups = require('./admin/groups'); const endpoints = require('./endpoints'); const staticRoute = require('./static'); const messages = require('./messages'); @@ -33,6 +34,7 @@ module.exports = { auth, adminAuth, adminConfig, + adminGroups, keys, apiKeys, user, diff --git a/packages/api/src/admin/groups.spec.ts b/packages/api/src/admin/groups.spec.ts new file mode 100644 index 0000000000..42e32152d9 --- /dev/null +++ b/packages/api/src/admin/groups.spec.ts @@ -0,0 +1,1348 @@ +import { Types } from 'mongoose'; +import { PrincipalType } from 'librechat-data-provider'; +import type { IGroup, IUser } from '@librechat/data-schemas'; +import type { Response } from 'express'; +import type { ServerRequest } from '~/types/http'; +import type { AdminGroupsDeps } from './groups'; +import { createAdminGroupsHandlers } from './groups'; + +jest.mock('@librechat/data-schemas', () => ({ + ...jest.requireActual('@librechat/data-schemas'), + logger: { error: jest.fn(), warn: jest.fn() }, +})); + +describe('createAdminGroupsHandlers', () => { + let validId: string; + let validUserId: string; + + beforeEach(() => { + validId = new Types.ObjectId().toString(); + validUserId = new Types.ObjectId().toString(); + }); + + function mockGroup(overrides: Partial = {}): IGroup { + return { + _id: new Types.ObjectId(validId), + name: 'Test Group', + source: 'local', + memberIds: [], + createdAt: new Date(), + updatedAt: new Date(), + ...overrides, + } as IGroup; + } + + function mockUser(overrides: Partial = {}): IUser { + return { + _id: new Types.ObjectId(validUserId), + name: 'Test User', + email: 'test@example.com', + avatar: 'https://example.com/avatar.png', + ...overrides, + } as IUser; + } + + function createReqRes( + overrides: { + params?: Record; + query?: Record; + body?: Record; + } = {}, + ) { + const req = { + params: overrides.params ?? {}, + query: overrides.query ?? {}, + body: overrides.body ?? {}, + } as unknown as ServerRequest; + + const json = jest.fn(); + const status = jest.fn().mockReturnValue({ json }); + const res = { status, json } as unknown as Response; + + return { req, res, status, json }; + } + + function createDeps(overrides: Partial = {}): AdminGroupsDeps { + return { + listGroups: jest.fn().mockResolvedValue([]), + countGroups: jest.fn().mockResolvedValue(0), + findGroupById: jest.fn().mockResolvedValue(null), + createGroup: jest.fn().mockResolvedValue(mockGroup()), + updateGroupById: jest.fn().mockResolvedValue(mockGroup()), + deleteGroup: jest.fn().mockResolvedValue(mockGroup()), + addUserToGroup: jest.fn().mockResolvedValue({ user: mockUser(), group: mockGroup() }), + removeUserFromGroup: jest.fn().mockResolvedValue({ user: mockUser(), group: mockGroup() }), + removeMemberById: jest.fn().mockResolvedValue(mockGroup()), + findUsers: jest.fn().mockResolvedValue([]), + deleteConfig: jest.fn().mockResolvedValue(null), + deleteAclEntries: jest.fn().mockResolvedValue({ deletedCount: 0 }), + deleteGrantsForPrincipal: jest.fn().mockResolvedValue(undefined), + ...overrides, + }; + } + + describe('listGroups', () => { + it('returns groups with total, limit, offset', async () => { + const groups = [mockGroup()]; + const deps = createDeps({ + listGroups: jest.fn().mockResolvedValue(groups), + countGroups: jest.fn().mockResolvedValue(1), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ query: {} }); + + await handlers.listGroups(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ groups, total: 1, limit: 50, offset: 0 }); + }); + + it('passes source and search filters with pagination', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res } = createReqRes({ + query: { source: 'entra', search: 'engineering', limit: '20', offset: '10' }, + }); + + await handlers.listGroups(req, res); + + expect(deps.listGroups).toHaveBeenCalledWith({ + source: 'entra', + search: 'engineering', + limit: 20, + offset: 10, + }); + expect(deps.countGroups).toHaveBeenCalledWith({ + source: 'entra', + search: 'engineering', + }); + }); + + it('passes search filter alone', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res } = createReqRes({ query: { search: 'eng' } }); + + await handlers.listGroups(req, res); + + expect(deps.listGroups).toHaveBeenCalledWith({ search: 'eng', limit: 50, offset: 0 }); + expect(deps.countGroups).toHaveBeenCalledWith({ search: 'eng' }); + }); + + it('ignores invalid source values', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res } = createReqRes({ query: { source: 'invalid' } }); + + await handlers.listGroups(req, res); + + expect(deps.listGroups).toHaveBeenCalledWith({ limit: 50, offset: 0 }); + expect(deps.countGroups).toHaveBeenCalledWith({}); + }); + + it('clamps limit and offset', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res } = createReqRes({ query: { limit: '999', offset: '-5' } }); + + await handlers.listGroups(req, res); + + expect(deps.listGroups).toHaveBeenCalledWith({ limit: 200, offset: 0 }); + }); + + it('returns 400 when search exceeds max length', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + query: { search: 'a'.repeat(201) }, + }); + + await handlers.listGroups(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'search must not exceed 200 characters' }); + expect(deps.listGroups).not.toHaveBeenCalled(); + }); + + it('returns 500 when countGroups fails', async () => { + const deps = createDeps({ + countGroups: jest.fn().mockRejectedValue(new Error('count failed')), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes(); + + await handlers.listGroups(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(json).toHaveBeenCalledWith({ error: 'Failed to list groups' }); + }); + + it('returns 500 on error', async () => { + const deps = createDeps({ listGroups: jest.fn().mockRejectedValue(new Error('db down')) }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes(); + + await handlers.listGroups(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(json).toHaveBeenCalledWith({ error: 'Failed to list groups' }); + }); + }); + + describe('getGroup', () => { + it('returns group with 200', async () => { + const group = mockGroup(); + const deps = createDeps({ findGroupById: jest.fn().mockResolvedValue(group) }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { id: validId } }); + + await handlers.getGroup(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ group }); + }); + + it('returns 400 for invalid ID', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { id: 'not-an-id' } }); + + await handlers.getGroup(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'Invalid group ID format' }); + expect(deps.findGroupById).not.toHaveBeenCalled(); + }); + + it('returns 404 when group not found', async () => { + const deps = createDeps({ findGroupById: jest.fn().mockResolvedValue(null) }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { id: validId } }); + + await handlers.getGroup(req, res); + + expect(status).toHaveBeenCalledWith(404); + expect(json).toHaveBeenCalledWith({ error: 'Group not found' }); + }); + + it('returns 500 on error', async () => { + const deps = createDeps({ + findGroupById: jest.fn().mockRejectedValue(new Error('db down')), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { id: validId } }); + + await handlers.getGroup(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(json).toHaveBeenCalledWith({ error: 'Failed to get group' }); + }); + }); + + describe('createGroup', () => { + it('creates group and returns 201', async () => { + const group = mockGroup(); + const deps = createDeps({ createGroup: jest.fn().mockResolvedValue(group) }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + body: { name: 'New Group', description: 'A group' }, + }); + + await handlers.createGroup(req, res); + + expect(status).toHaveBeenCalledWith(201); + expect(json).toHaveBeenCalledWith({ group }); + expect(deps.createGroup).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'New Group', + description: 'A group', + source: 'local', + memberIds: [], + }), + ); + }); + + it('normalizes memberIds to idOnTheSource values', async () => { + const userId = new Types.ObjectId().toString(); + const user = { _id: new Types.ObjectId(userId), idOnTheSource: 'ext-norm-1' } as IUser; + const group = mockGroup(); + const deps = createDeps({ + createGroup: jest.fn().mockResolvedValue(group), + findUsers: jest.fn().mockResolvedValue([user]), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status } = createReqRes({ + body: { name: 'With Members', memberIds: [userId] }, + }); + + await handlers.createGroup(req, res); + + expect(status).toHaveBeenCalledWith(201); + expect(deps.findUsers).toHaveBeenCalledWith({ _id: { $in: [userId] } }, 'idOnTheSource'); + expect(deps.createGroup).toHaveBeenCalledWith( + expect.objectContaining({ memberIds: ['ext-norm-1'] }), + ); + }); + + it('logs warning when memberIds contain non-existent user ObjectIds', async () => { + const { logger } = jest.requireMock('@librechat/data-schemas'); + const unknownId = new Types.ObjectId().toString(); + const group = mockGroup(); + const deps = createDeps({ + createGroup: jest.fn().mockResolvedValue(group), + findUsers: jest.fn().mockResolvedValue([]), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status } = createReqRes({ + body: { name: 'With Unknown', memberIds: [unknownId] }, + }); + + await handlers.createGroup(req, res); + + expect(status).toHaveBeenCalledWith(201); + expect(logger.warn).toHaveBeenCalledWith( + '[adminGroups] createGroup: memberIds contain unknown user ObjectIds:', + [unknownId], + ); + }); + + it('passes idOnTheSource when provided', async () => { + const group = mockGroup(); + const deps = createDeps({ createGroup: jest.fn().mockResolvedValue(group) }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status } = createReqRes({ + body: { name: 'Entra Group', source: 'entra', idOnTheSource: 'ent-abc-123' }, + }); + + await handlers.createGroup(req, res); + + expect(status).toHaveBeenCalledWith(201); + expect(deps.createGroup).toHaveBeenCalledWith( + expect.objectContaining({ idOnTheSource: 'ent-abc-123', source: 'entra' }), + ); + }); + + it('returns 400 for invalid source value', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + body: { name: 'Bad Source', source: 'azure' }, + }); + + await handlers.createGroup(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'Invalid source value' }); + expect(deps.createGroup).not.toHaveBeenCalled(); + }); + + it('returns 400 when name exceeds max length', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + body: { name: 'a'.repeat(501) }, + }); + + await handlers.createGroup(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'name must not exceed 500 characters' }); + expect(deps.createGroup).not.toHaveBeenCalled(); + }); + + it('returns 400 when description exceeds max length', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + body: { name: 'Valid', description: 'x'.repeat(2001) }, + }); + + await handlers.createGroup(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ + error: 'description must not exceed 2000 characters', + }); + expect(deps.createGroup).not.toHaveBeenCalled(); + }); + + it('returns 400 when email exceeds max length', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + body: { name: 'Valid', email: 'x'.repeat(501) }, + }); + + await handlers.createGroup(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'email must not exceed 500 characters' }); + expect(deps.createGroup).not.toHaveBeenCalled(); + }); + + it('returns 400 when avatar exceeds max length', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + body: { name: 'Valid', avatar: 'x'.repeat(2001) }, + }); + + await handlers.createGroup(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'avatar must not exceed 2000 characters' }); + expect(deps.createGroup).not.toHaveBeenCalled(); + }); + + it('returns 400 when idOnTheSource exceeds max length', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + body: { name: 'Valid', idOnTheSource: 'x'.repeat(501) }, + }); + + await handlers.createGroup(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ + error: 'idOnTheSource must not exceed 500 characters', + }); + expect(deps.createGroup).not.toHaveBeenCalled(); + }); + + it('returns 400 when memberIds exceeds cap', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const memberIds = Array.from({ length: 501 }, (_, i) => `ext-${i}`); + const { req, res, status, json } = createReqRes({ + body: { name: 'Too Many Members', memberIds }, + }); + + await handlers.createGroup(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'memberIds must not exceed 500 entries' }); + expect(deps.createGroup).not.toHaveBeenCalled(); + }); + + it('passes non-ObjectId memberIds through unchanged', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status } = createReqRes({ + body: { name: 'Ext Group', memberIds: ['ext-1', 'ext-2'] }, + }); + + await handlers.createGroup(req, res); + + expect(deps.findUsers).not.toHaveBeenCalled(); + expect(deps.createGroup).toHaveBeenCalledWith( + expect.objectContaining({ memberIds: ['ext-1', 'ext-2'] }), + ); + expect(status).toHaveBeenCalledWith(201); + }); + + it('returns 400 when name is missing', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ body: {} }); + + await handlers.createGroup(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'name is required' }); + expect(deps.createGroup).not.toHaveBeenCalled(); + }); + + it('returns 400 when name is whitespace-only', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ body: { name: ' ' } }); + + await handlers.createGroup(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'name is required' }); + }); + + it('returns 400 on ValidationError', async () => { + const validationError = new Error('source must be local or entra'); + validationError.name = 'ValidationError'; + const deps = createDeps({ createGroup: jest.fn().mockRejectedValue(validationError) }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ body: { name: 'Test' } }); + + await handlers.createGroup(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'source must be local or entra' }); + }); + + it('returns 500 on unexpected error', async () => { + const deps = createDeps({ createGroup: jest.fn().mockRejectedValue(new Error('db crash')) }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ body: { name: 'Test' } }); + + await handlers.createGroup(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(json).toHaveBeenCalledWith({ error: 'Failed to create group' }); + }); + }); + + describe('updateGroup', () => { + it('updates group and returns 200', async () => { + const group = mockGroup({ name: 'Updated' }); + const deps = createDeps({ + updateGroupById: jest.fn().mockResolvedValue(group), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: validId }, + body: { name: 'Updated' }, + }); + + await handlers.updateGroup(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ group }); + }); + + it('updates description only', async () => { + const group = mockGroup({ description: 'New desc' }); + const deps = createDeps({ updateGroupById: jest.fn().mockResolvedValue(group) }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status } = createReqRes({ + params: { id: validId }, + body: { description: 'New desc' }, + }); + + await handlers.updateGroup(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(deps.updateGroupById).toHaveBeenCalledWith(validId, { description: 'New desc' }); + }); + + it('updates email only', async () => { + const group = mockGroup({ email: 'team@co.com' }); + const deps = createDeps({ updateGroupById: jest.fn().mockResolvedValue(group) }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status } = createReqRes({ + params: { id: validId }, + body: { email: 'team@co.com' }, + }); + + await handlers.updateGroup(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(deps.updateGroupById).toHaveBeenCalledWith(validId, { email: 'team@co.com' }); + }); + + it('updates avatar only', async () => { + const group = mockGroup({ avatar: 'https://img.co/a.png' }); + const deps = createDeps({ updateGroupById: jest.fn().mockResolvedValue(group) }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status } = createReqRes({ + params: { id: validId }, + body: { avatar: 'https://img.co/a.png' }, + }); + + await handlers.updateGroup(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(deps.updateGroupById).toHaveBeenCalledWith(validId, { + avatar: 'https://img.co/a.png', + }); + }); + + it('updates multiple fields at once', async () => { + const group = mockGroup({ name: 'New', description: 'Desc', email: 'a@b.com' }); + const deps = createDeps({ updateGroupById: jest.fn().mockResolvedValue(group) }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status } = createReqRes({ + params: { id: validId }, + body: { name: ' New ', description: 'Desc', email: 'a@b.com' }, + }); + + await handlers.updateGroup(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(deps.updateGroupById).toHaveBeenCalledWith(validId, { + name: 'New', + description: 'Desc', + email: 'a@b.com', + }); + }); + + it('returns 400 for invalid ID', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: 'bad' }, + body: { name: 'Updated' }, + }); + + await handlers.updateGroup(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'Invalid group ID format' }); + }); + + it('returns 400 when name is empty string', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: validId }, + body: { name: '' }, + }); + + await handlers.updateGroup(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'name must be a non-empty string' }); + }); + + it('returns 400 when name is whitespace-only', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: validId }, + body: { name: ' ' }, + }); + + await handlers.updateGroup(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'name must be a non-empty string' }); + }); + + it('returns 400 when name exceeds max length', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: validId }, + body: { name: 'a'.repeat(501) }, + }); + + await handlers.updateGroup(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'name must not exceed 500 characters' }); + expect(deps.updateGroupById).not.toHaveBeenCalled(); + }); + + it('returns 400 when description exceeds max length', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: validId }, + body: { description: 'x'.repeat(2001) }, + }); + + await handlers.updateGroup(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ + error: 'description must not exceed 2000 characters', + }); + expect(deps.updateGroupById).not.toHaveBeenCalled(); + }); + + it('returns 400 when email exceeds max length', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: validId }, + body: { email: 'x'.repeat(501) }, + }); + + await handlers.updateGroup(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'email must not exceed 500 characters' }); + expect(deps.updateGroupById).not.toHaveBeenCalled(); + }); + + it('returns 400 when avatar exceeds max length', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: validId }, + body: { avatar: 'x'.repeat(2001) }, + }); + + await handlers.updateGroup(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'avatar must not exceed 2000 characters' }); + expect(deps.updateGroupById).not.toHaveBeenCalled(); + }); + + it('returns 400 when no valid fields provided', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: validId }, + body: {}, + }); + + await handlers.updateGroup(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'No valid fields to update' }); + expect(deps.updateGroupById).not.toHaveBeenCalled(); + }); + + it('returns 404 when updateGroupById returns null', async () => { + const deps = createDeps({ + updateGroupById: jest.fn().mockResolvedValue(null), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: validId }, + body: { name: 'Updated' }, + }); + + await handlers.updateGroup(req, res); + + expect(status).toHaveBeenCalledWith(404); + expect(json).toHaveBeenCalledWith({ error: 'Group not found' }); + }); + + it('returns 400 on ValidationError', async () => { + const validationError = new Error('invalid field'); + validationError.name = 'ValidationError'; + const deps = createDeps({ + updateGroupById: jest.fn().mockRejectedValue(validationError), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: validId }, + body: { name: 'Updated' }, + }); + + await handlers.updateGroup(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'invalid field' }); + }); + + it('returns 500 on error', async () => { + const deps = createDeps({ + updateGroupById: jest.fn().mockRejectedValue(new Error('db down')), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: validId }, + body: { name: 'Updated' }, + }); + + await handlers.updateGroup(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(json).toHaveBeenCalledWith({ error: 'Failed to update group' }); + }); + }); + + describe('deleteGroup', () => { + it('deletes group and returns 200 with id', async () => { + const deps = createDeps({ deleteGroup: jest.fn().mockResolvedValue(mockGroup()) }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { id: validId } }); + + await handlers.deleteGroup(req, res); + + expect(deps.deleteGroup).toHaveBeenCalledWith(validId); + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ success: true, id: validId }); + }); + + it('returns 400 for invalid ID', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { id: 'bad-id' } }); + + await handlers.deleteGroup(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'Invalid group ID format' }); + }); + + it('returns 404 when deleteGroup returns null', async () => { + const deps = createDeps({ deleteGroup: jest.fn().mockResolvedValue(null) }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { id: validId } }); + + await handlers.deleteGroup(req, res); + + expect(status).toHaveBeenCalledWith(404); + expect(json).toHaveBeenCalledWith({ error: 'Group not found' }); + expect(deps.deleteConfig).not.toHaveBeenCalled(); + }); + + it('returns 500 on error', async () => { + const deps = createDeps({ + deleteGroup: jest.fn().mockRejectedValue(new Error('db down')), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { id: validId } }); + + await handlers.deleteGroup(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(json).toHaveBeenCalledWith({ error: 'Failed to delete group' }); + }); + + it('returns 200 even when cascade cleanup partially fails', async () => { + const deps = createDeps({ + deleteGroup: jest.fn().mockResolvedValue(mockGroup()), + deleteAclEntries: jest.fn().mockRejectedValue(new Error('cleanup failed')), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { id: validId } }); + + await handlers.deleteGroup(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ success: true, id: validId }); + expect(deps.deleteConfig).toHaveBeenCalledWith(PrincipalType.GROUP, validId); + expect(deps.deleteAclEntries).toHaveBeenCalledWith({ + principalType: PrincipalType.GROUP, + principalId: new Types.ObjectId(validId), + }); + expect(deps.deleteGrantsForPrincipal).toHaveBeenCalledWith(PrincipalType.GROUP, validId); + }); + + it('cleans up Config, AclEntry, and SystemGrant on group delete', async () => { + const deps = createDeps({ deleteGroup: jest.fn().mockResolvedValue(mockGroup()) }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status } = createReqRes({ params: { id: validId } }); + + await handlers.deleteGroup(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(deps.deleteConfig).toHaveBeenCalledWith(PrincipalType.GROUP, validId); + expect(deps.deleteAclEntries).toHaveBeenCalledWith({ + principalType: PrincipalType.GROUP, + principalId: new Types.ObjectId(validId), + }); + expect(deps.deleteGrantsForPrincipal).toHaveBeenCalledWith(PrincipalType.GROUP, validId); + }); + }); + + describe('getGroupMembers', () => { + it('fetches group with memberIds projection only', async () => { + const group = mockGroup({ memberIds: [] }); + const deps = createDeps({ findGroupById: jest.fn().mockResolvedValue(group) }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res } = createReqRes({ params: { id: validId } }); + + await handlers.getGroupMembers(req, res); + + expect(deps.findGroupById).toHaveBeenCalledWith(validId, { memberIds: 1 }); + }); + + it('returns empty members for group with no memberIds', async () => { + const group = mockGroup({ memberIds: [] }); + const deps = createDeps({ findGroupById: jest.fn().mockResolvedValue(group) }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { id: validId } }); + + await handlers.getGroupMembers(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ members: [], total: 0, limit: 50, offset: 0 }); + expect(deps.findUsers).not.toHaveBeenCalled(); + }); + + it('batches member lookup with $or query', async () => { + const user = mockUser({ idOnTheSource: 'ext-123' }); + const group = mockGroup({ memberIds: [validUserId, 'ext-123'] }); + const deps = createDeps({ + findGroupById: jest.fn().mockResolvedValue(group), + findUsers: jest.fn().mockResolvedValue([user]), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { id: validId } }); + + await handlers.getGroupMembers(req, res); + + expect(deps.findUsers).toHaveBeenCalledWith( + { + $or: [ + { idOnTheSource: { $in: [validUserId, 'ext-123'] } }, + { _id: { $in: [validUserId] } }, + ], + }, + 'name email avatar idOnTheSource', + ); + expect(status).toHaveBeenCalledWith(200); + const members = json.mock.calls[0][0].members; + expect(members).toHaveLength(1); + }); + + it('skips _id condition when no valid ObjectIds in memberIds', async () => { + const group = mockGroup({ memberIds: ['ext-1', 'ext-2'] }); + const deps = createDeps({ + findGroupById: jest.fn().mockResolvedValue(group), + findUsers: jest.fn().mockResolvedValue([]), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res } = createReqRes({ params: { id: validId } }); + + await handlers.getGroupMembers(req, res); + + expect(deps.findUsers).toHaveBeenCalledWith( + { $or: [{ idOnTheSource: { $in: ['ext-1', 'ext-2'] } }] }, + 'name email avatar idOnTheSource', + ); + }); + + it('falls back to memberId when user not found', async () => { + const group = mockGroup({ memberIds: ['unknown-member'] }); + const deps = createDeps({ + findGroupById: jest.fn().mockResolvedValue(group), + findUsers: jest.fn().mockResolvedValue([]), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, json } = createReqRes({ params: { id: validId } }); + + await handlers.getGroupMembers(req, res); + + expect(json.mock.calls[0][0].members).toEqual([ + { userId: 'unknown-member', name: 'unknown-member', email: '', avatarUrl: undefined }, + ]); + }); + + it('deduplicates when identical memberId appears twice', async () => { + const user = mockUser({ idOnTheSource: validUserId }); + const group = mockGroup({ memberIds: [validUserId, validUserId] }); + const deps = createDeps({ + findGroupById: jest.fn().mockResolvedValue(group), + findUsers: jest.fn().mockResolvedValue([user]), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, json } = createReqRes({ params: { id: validId } }); + + await handlers.getGroupMembers(req, res); + + const result = json.mock.calls[0][0]; + expect(result.members).toHaveLength(1); + expect(result.total).toBe(1); + }); + + it('deduplicates when objectId and idOnTheSource both present for same user', async () => { + const extId = 'ext-dedup-123'; + const user = mockUser({ idOnTheSource: extId }); + const group = mockGroup({ memberIds: [validUserId, extId] }); + const deps = createDeps({ + findGroupById: jest.fn().mockResolvedValue(group), + findUsers: jest.fn().mockResolvedValue([user]), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, json } = createReqRes({ params: { id: validId } }); + + await handlers.getGroupMembers(req, res); + + expect(json.mock.calls[0][0].members).toHaveLength(1); + }); + + it('reports deduplicated total for duplicate memberIds', async () => { + const group = mockGroup({ memberIds: ['m1', 'm2', 'm1', 'm3', 'm2'] }); + const deps = createDeps({ + findGroupById: jest.fn().mockResolvedValue(group), + findUsers: jest.fn().mockResolvedValue([]), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, json } = createReqRes({ params: { id: validId } }); + + await handlers.getGroupMembers(req, res); + + const result = json.mock.calls[0][0]; + expect(result.total).toBe(3); + expect(result.members).toHaveLength(3); + }); + + it('paginates members with limit and offset', async () => { + const ids = ['m1', 'm2', 'm3', 'm4', 'm5']; + const group = mockGroup({ memberIds: ids }); + const deps = createDeps({ + findGroupById: jest.fn().mockResolvedValue(group), + findUsers: jest.fn().mockResolvedValue([]), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, json } = createReqRes({ + params: { id: validId }, + query: { limit: '2', offset: '1' }, + }); + + await handlers.getGroupMembers(req, res); + + const result = json.mock.calls[0][0]; + expect(result.total).toBe(5); + expect(result.limit).toBe(2); + expect(result.offset).toBe(1); + expect(result.members).toHaveLength(2); + expect(result.members[0].userId).toBe('m2'); + expect(result.members[1].userId).toBe('m3'); + }); + + it('caps limit at 200', async () => { + const ids = Array.from({ length: 5 }, (_, i) => `m${i}`); + const group = mockGroup({ memberIds: ids }); + const deps = createDeps({ + findGroupById: jest.fn().mockResolvedValue(group), + findUsers: jest.fn().mockResolvedValue([]), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, json } = createReqRes({ + params: { id: validId }, + query: { limit: '999' }, + }); + + await handlers.getGroupMembers(req, res); + + const result = json.mock.calls[0][0]; + expect(result.limit).toBe(200); + }); + + it('returns empty when offset exceeds total', async () => { + const group = mockGroup({ memberIds: ['m1', 'm2'] }); + const deps = createDeps({ + findGroupById: jest.fn().mockResolvedValue(group), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, json } = createReqRes({ + params: { id: validId }, + query: { offset: '10' }, + }); + + await handlers.getGroupMembers(req, res); + + const result = json.mock.calls[0][0]; + expect(result.members).toHaveLength(0); + expect(result.total).toBe(2); + expect(deps.findUsers).not.toHaveBeenCalled(); + }); + + it('returns 400 for invalid group ID', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { id: 'nope' } }); + + await handlers.getGroupMembers(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'Invalid group ID format' }); + }); + + it('returns 404 when group not found', async () => { + const deps = createDeps({ findGroupById: jest.fn().mockResolvedValue(null) }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { id: validId } }); + + await handlers.getGroupMembers(req, res); + + expect(status).toHaveBeenCalledWith(404); + expect(json).toHaveBeenCalledWith({ error: 'Group not found' }); + }); + + it('returns 500 on error', async () => { + const deps = createDeps({ + findGroupById: jest.fn().mockRejectedValue(new Error('db down')), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ params: { id: validId } }); + + await handlers.getGroupMembers(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(json).toHaveBeenCalledWith({ error: 'Failed to get group members' }); + }); + }); + + describe('addGroupMember', () => { + it('adds member and returns 200', async () => { + const group = mockGroup(); + const deps = createDeps({ + addUserToGroup: jest.fn().mockResolvedValue({ user: mockUser(), group }), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: validId }, + body: { userId: validUserId }, + }); + + await handlers.addGroupMember(req, res); + + expect(deps.addUserToGroup).toHaveBeenCalledWith(validUserId, validId); + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ group }); + }); + + it('returns 400 for invalid group ID', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: 'bad' }, + body: { userId: validUserId }, + }); + + await handlers.addGroupMember(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'Invalid group ID format' }); + }); + + it('returns 400 when userId is missing', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: validId }, + body: {}, + }); + + await handlers.addGroupMember(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'userId is required' }); + }); + + it('returns 400 for non-ObjectId userId', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: validId }, + body: { userId: 'not-valid' }, + }); + + await handlers.addGroupMember(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ + error: 'Only native user ObjectIds can be added via this endpoint', + }); + }); + + it('returns 404 when addUserToGroup returns null group', async () => { + const deps = createDeps({ + addUserToGroup: jest.fn().mockResolvedValue({ user: mockUser(), group: null }), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: validId }, + body: { userId: validUserId }, + }); + + await handlers.addGroupMember(req, res); + + expect(status).toHaveBeenCalledWith(404); + expect(json).toHaveBeenCalledWith({ error: 'Group not found' }); + }); + + it('returns 404 for "User not found" error', async () => { + const deps = createDeps({ + addUserToGroup: jest.fn().mockRejectedValue(new Error('User not found')), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: validId }, + body: { userId: validUserId }, + }); + + await handlers.addGroupMember(req, res); + + expect(status).toHaveBeenCalledWith(404); + expect(json).toHaveBeenCalledWith({ error: 'User not found' }); + }); + + it('returns 500 for unrelated errors', async () => { + const deps = createDeps({ + addUserToGroup: jest.fn().mockRejectedValue(new Error('connection lost')), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: validId }, + body: { userId: validUserId }, + }); + + await handlers.addGroupMember(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(json).toHaveBeenCalledWith({ error: 'Failed to add member' }); + }); + + it('does not misclassify errors containing "not found" substring', async () => { + const deps = createDeps({ + addUserToGroup: jest.fn().mockRejectedValue(new Error('Permission not found in config')), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status } = createReqRes({ + params: { id: validId }, + body: { userId: validUserId }, + }); + + await handlers.addGroupMember(req, res); + + expect(status).toHaveBeenCalledWith(500); + }); + }); + + describe('removeGroupMember', () => { + it('removes member and returns 200', async () => { + const deps = createDeps({ + removeUserFromGroup: jest.fn().mockResolvedValue({ user: mockUser(), group: mockGroup() }), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: validId, userId: validUserId }, + }); + + await handlers.removeGroupMember(req, res); + + expect(deps.removeUserFromGroup).toHaveBeenCalledWith(validUserId, validId); + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ success: true }); + }); + + it('returns 400 for invalid group ID', async () => { + const deps = createDeps(); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: 'bad', userId: validUserId }, + }); + + await handlers.removeGroupMember(req, res); + + expect(status).toHaveBeenCalledWith(400); + expect(json).toHaveBeenCalledWith({ error: 'Invalid group ID format' }); + }); + + it('removes non-ObjectId member via removeMemberById', async () => { + const deps = createDeps({ + removeMemberById: jest.fn().mockResolvedValue(mockGroup()), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: validId, userId: 'ent-abc-123' }, + }); + + await handlers.removeGroupMember(req, res); + + expect(deps.removeMemberById).toHaveBeenCalledWith(validId, 'ent-abc-123'); + expect(deps.removeUserFromGroup).not.toHaveBeenCalled(); + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ success: true }); + }); + + it('returns 404 when removeMemberById returns null', async () => { + const deps = createDeps({ + removeMemberById: jest.fn().mockResolvedValue(null), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: validId, userId: 'ent-abc-123' }, + }); + + await handlers.removeGroupMember(req, res); + + expect(status).toHaveBeenCalledWith(404); + expect(json).toHaveBeenCalledWith({ error: 'Group not found' }); + }); + + it('falls back to removeMemberById when ObjectId userId not found as user', async () => { + const deps = createDeps({ + removeUserFromGroup: jest.fn().mockRejectedValue(new Error('User not found')), + removeMemberById: jest.fn().mockResolvedValue(mockGroup()), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: validId, userId: validUserId }, + }); + + await handlers.removeGroupMember(req, res); + + expect(deps.removeUserFromGroup).toHaveBeenCalledWith(validUserId, validId); + expect(deps.removeMemberById).toHaveBeenCalledWith(validId, validUserId); + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ success: true }); + }); + + it('returns 404 when removeUserFromGroup returns null group', async () => { + const deps = createDeps({ + removeUserFromGroup: jest.fn().mockResolvedValue({ user: mockUser(), group: null }), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: validId, userId: validUserId }, + }); + + await handlers.removeGroupMember(req, res); + + expect(status).toHaveBeenCalledWith(404); + expect(json).toHaveBeenCalledWith({ error: 'Group not found' }); + }); + + it('returns 404 when fallback removeMemberById also returns null', async () => { + const deps = createDeps({ + removeUserFromGroup: jest.fn().mockRejectedValue(new Error('User not found')), + removeMemberById: jest.fn().mockResolvedValue(null), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: validId, userId: validUserId }, + }); + + await handlers.removeGroupMember(req, res); + + expect(status).toHaveBeenCalledWith(404); + expect(json).toHaveBeenCalledWith({ error: 'Group not found' }); + }); + + it('returns 500 for unrelated errors', async () => { + const deps = createDeps({ + removeUserFromGroup: jest.fn().mockRejectedValue(new Error('timeout')), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: validId, userId: validUserId }, + }); + + await handlers.removeGroupMember(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(json).toHaveBeenCalledWith({ error: 'Failed to remove member' }); + }); + + it('returns 200 when removing ObjectId member not in group (idempotent delete)', async () => { + const group = mockGroup({ memberIds: [] }); + const deps = createDeps({ + removeUserFromGroup: jest.fn().mockResolvedValue({ user: mockUser(), group }), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: validId, userId: validUserId }, + }); + + await handlers.removeGroupMember(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ success: true }); + }); + + it('returns 200 when removing non-ObjectId member not in group (idempotent delete)', async () => { + const group = mockGroup({ memberIds: [] }); + const deps = createDeps({ + removeMemberById: jest.fn().mockResolvedValue(group), + }); + const handlers = createAdminGroupsHandlers(deps); + const { req, res, status, json } = createReqRes({ + params: { id: validId, userId: 'ext-not-in-group' }, + }); + + await handlers.removeGroupMember(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(json).toHaveBeenCalledWith({ success: true }); + }); + }); +}); diff --git a/packages/api/src/admin/groups.ts b/packages/api/src/admin/groups.ts new file mode 100644 index 0000000000..58ff4d9782 --- /dev/null +++ b/packages/api/src/admin/groups.ts @@ -0,0 +1,482 @@ +import { Types } from 'mongoose'; +import { PrincipalType } from 'librechat-data-provider'; +import { logger, isValidObjectIdString } from '@librechat/data-schemas'; +import type { + IGroup, + IUser, + IConfig, + CreateGroupRequest, + UpdateGroupRequest, + GroupFilterOptions, +} from '@librechat/data-schemas'; +import type { FilterQuery, ClientSession, DeleteResult } from 'mongoose'; +import type { Response } from 'express'; +import type { ValidationError } from '~/types/error'; +import type { ServerRequest } from '~/types/http'; + +type GroupListFilter = Pick; + +const VALID_GROUP_SOURCES: ReadonlySet = new Set(['local', 'entra']); +const MAX_CREATE_MEMBER_IDS = 500; +const MAX_SEARCH_LENGTH = 200; +const MAX_NAME_LENGTH = 500; +const MAX_DESCRIPTION_LENGTH = 2000; +const MAX_EMAIL_LENGTH = 500; +const MAX_AVATAR_LENGTH = 2000; +const MAX_EXTERNAL_ID_LENGTH = 500; + +interface GroupIdParams { + id: string; +} + +interface GroupMemberParams extends GroupIdParams { + userId: string; +} + +export interface AdminGroupsDeps { + listGroups: ( + filter?: GroupListFilter & { limit?: number; offset?: number }, + session?: ClientSession, + ) => Promise; + countGroups: (filter?: GroupListFilter, session?: ClientSession) => Promise; + findGroupById: ( + groupId: string | Types.ObjectId, + projection?: Record, + session?: ClientSession, + ) => Promise; + createGroup: (groupData: Partial, session?: ClientSession) => Promise; + updateGroupById: ( + groupId: string | Types.ObjectId, + data: Partial>, + session?: ClientSession, + ) => Promise; + deleteGroup: ( + groupId: string | Types.ObjectId, + session?: ClientSession, + ) => Promise; + addUserToGroup: ( + userId: string | Types.ObjectId, + groupId: string | Types.ObjectId, + session?: ClientSession, + ) => Promise<{ user: IUser; group: IGroup | null }>; + removeUserFromGroup: ( + userId: string | Types.ObjectId, + groupId: string | Types.ObjectId, + session?: ClientSession, + ) => Promise<{ user: IUser; group: IGroup | null }>; + removeMemberById: ( + groupId: string | Types.ObjectId, + memberId: string, + session?: ClientSession, + ) => Promise; + findUsers: ( + searchCriteria: FilterQuery, + fieldsToSelect?: string | string[] | null, + ) => Promise; + deleteConfig: ( + principalType: PrincipalType, + principalId: string | Types.ObjectId, + ) => Promise; + deleteAclEntries: (filter: { + principalType: PrincipalType; + principalId: string | Types.ObjectId; + }) => Promise; + deleteGrantsForPrincipal: ( + principalType: PrincipalType, + principalId: string | Types.ObjectId, + ) => Promise; +} + +export function createAdminGroupsHandlers(deps: AdminGroupsDeps) { + const { + listGroups, + countGroups, + findGroupById, + createGroup, + updateGroupById, + deleteGroup, + addUserToGroup, + removeUserFromGroup, + removeMemberById, + findUsers, + deleteConfig, + deleteAclEntries, + deleteGrantsForPrincipal, + } = deps; + + async function listGroupsHandler(req: ServerRequest, res: Response) { + try { + const { search, source } = req.query as { search?: string; source?: string }; + const filter: GroupListFilter = {}; + if (source && VALID_GROUP_SOURCES.has(source)) { + filter.source = source as IGroup['source']; + } + if (search && search.length > MAX_SEARCH_LENGTH) { + return res + .status(400) + .json({ error: `search must not exceed ${MAX_SEARCH_LENGTH} characters` }); + } + if (search) { + filter.search = search; + } + const limit = Math.min(Math.max(Number(req.query.limit) || 50, 1), 200); + const offset = Math.max(Number(req.query.offset) || 0, 0); + const [groups, total] = await Promise.all([ + listGroups({ ...filter, limit, offset }), + countGroups(filter), + ]); + return res.status(200).json({ groups, total, limit, offset }); + } catch (error) { + logger.error('[adminGroups] listGroups error:', error); + return res.status(500).json({ error: 'Failed to list groups' }); + } + } + + async function getGroupHandler(req: ServerRequest, res: Response) { + try { + const { id } = req.params as GroupIdParams; + if (!isValidObjectIdString(id)) { + return res.status(400).json({ error: 'Invalid group ID format' }); + } + const group = await findGroupById(id); + if (!group) { + return res.status(404).json({ error: 'Group not found' }); + } + return res.status(200).json({ group }); + } catch (error) { + logger.error('[adminGroups] getGroup error:', error); + return res.status(500).json({ error: 'Failed to get group' }); + } + } + + async function createGroupHandler(req: ServerRequest, res: Response) { + try { + const body = req.body as CreateGroupRequest; + if (!body.name || typeof body.name !== 'string' || !body.name.trim()) { + return res.status(400).json({ error: 'name is required' }); + } + if (body.name.trim().length > MAX_NAME_LENGTH) { + return res + .status(400) + .json({ error: `name must not exceed ${MAX_NAME_LENGTH} characters` }); + } + if (body.source && !VALID_GROUP_SOURCES.has(body.source)) { + return res.status(400).json({ error: 'Invalid source value' }); + } + if (body.description && body.description.length > MAX_DESCRIPTION_LENGTH) { + return res + .status(400) + .json({ error: `description must not exceed ${MAX_DESCRIPTION_LENGTH} characters` }); + } + if (body.email && body.email.length > MAX_EMAIL_LENGTH) { + return res + .status(400) + .json({ error: `email must not exceed ${MAX_EMAIL_LENGTH} characters` }); + } + if (body.avatar && body.avatar.length > MAX_AVATAR_LENGTH) { + return res + .status(400) + .json({ error: `avatar must not exceed ${MAX_AVATAR_LENGTH} characters` }); + } + if (body.idOnTheSource && body.idOnTheSource.length > MAX_EXTERNAL_ID_LENGTH) { + return res + .status(400) + .json({ error: `idOnTheSource must not exceed ${MAX_EXTERNAL_ID_LENGTH} characters` }); + } + + const rawIds = Array.isArray(body.memberIds) ? body.memberIds : []; + if (rawIds.length > MAX_CREATE_MEMBER_IDS) { + return res + .status(400) + .json({ error: `memberIds must not exceed ${MAX_CREATE_MEMBER_IDS} entries` }); + } + let memberIds = rawIds; + const objectIds = rawIds.filter(isValidObjectIdString); + if (objectIds.length > 0) { + const users = await findUsers({ _id: { $in: objectIds } }, 'idOnTheSource'); + const idMap = new Map(); + for (const user of users) { + const uid = user._id?.toString() ?? ''; + idMap.set(uid, user.idOnTheSource || uid); + } + const unmapped = objectIds.filter((oid) => !idMap.has(oid)); + if (unmapped.length > 0) { + logger.warn( + '[adminGroups] createGroup: memberIds contain unknown user ObjectIds:', + unmapped, + ); + } + memberIds = rawIds.map((id) => idMap.get(id) || id); + } + + const group = await createGroup({ + name: body.name.trim(), + description: body.description, + email: body.email, + avatar: body.avatar, + source: body.source || 'local', + memberIds, + ...(body.idOnTheSource ? { idOnTheSource: body.idOnTheSource } : {}), + }); + return res.status(201).json({ group }); + } catch (error) { + if ((error as ValidationError).name === 'ValidationError') { + return res.status(400).json({ error: (error as ValidationError).message }); + } + logger.error('[adminGroups] createGroup error:', error); + return res.status(500).json({ error: 'Failed to create group' }); + } + } + + async function updateGroupHandler(req: ServerRequest, res: Response) { + try { + const { id } = req.params as GroupIdParams; + if (!isValidObjectIdString(id)) { + return res.status(400).json({ error: 'Invalid group ID format' }); + } + const body = req.body as UpdateGroupRequest; + + if ( + body.name !== undefined && + (!body.name || typeof body.name !== 'string' || !body.name.trim()) + ) { + return res.status(400).json({ error: 'name must be a non-empty string' }); + } + if (body.name !== undefined && body.name.trim().length > MAX_NAME_LENGTH) { + return res + .status(400) + .json({ error: `name must not exceed ${MAX_NAME_LENGTH} characters` }); + } + if (body.description !== undefined && body.description.length > MAX_DESCRIPTION_LENGTH) { + return res + .status(400) + .json({ error: `description must not exceed ${MAX_DESCRIPTION_LENGTH} characters` }); + } + if (body.email !== undefined && body.email.length > MAX_EMAIL_LENGTH) { + return res + .status(400) + .json({ error: `email must not exceed ${MAX_EMAIL_LENGTH} characters` }); + } + if (body.avatar !== undefined && body.avatar.length > MAX_AVATAR_LENGTH) { + return res + .status(400) + .json({ error: `avatar must not exceed ${MAX_AVATAR_LENGTH} characters` }); + } + + const updateData: Partial> = {}; + if (body.name !== undefined) { + updateData.name = body.name.trim(); + } + if (body.description !== undefined) { + updateData.description = body.description; + } + if (body.email !== undefined) { + updateData.email = body.email; + } + if (body.avatar !== undefined) { + updateData.avatar = body.avatar; + } + + if (Object.keys(updateData).length === 0) { + return res.status(400).json({ error: 'No valid fields to update' }); + } + + const group = await updateGroupById(id, updateData); + if (!group) { + return res.status(404).json({ error: 'Group not found' }); + } + return res.status(200).json({ group }); + } catch (error) { + if ((error as ValidationError).name === 'ValidationError') { + return res.status(400).json({ error: (error as ValidationError).message }); + } + logger.error('[adminGroups] updateGroup error:', error); + return res.status(500).json({ error: 'Failed to update group' }); + } + } + + async function deleteGroupHandler(req: ServerRequest, res: Response) { + try { + const { id } = req.params as GroupIdParams; + if (!isValidObjectIdString(id)) { + return res.status(400).json({ error: 'Invalid group ID format' }); + } + const deleted = await deleteGroup(id); + if (!deleted) { + return res.status(404).json({ error: 'Group not found' }); + } + /** + * deleteAclEntries is a raw deleteMany wrapper with no type casting. + * grantPermission stores group principalId as ObjectId, so we must + * cast here. deleteConfig and deleteGrantsForPrincipal normalize internally. + */ + const cleanupResults = await Promise.allSettled([ + deleteConfig(PrincipalType.GROUP, id), + deleteAclEntries({ + principalType: PrincipalType.GROUP, + principalId: new Types.ObjectId(id), + }), + deleteGrantsForPrincipal(PrincipalType.GROUP, id), + ]); + for (const result of cleanupResults) { + if (result.status === 'rejected') { + logger.error('[adminGroups] cascade cleanup step failed for group:', id, result.reason); + } + } + return res.status(200).json({ success: true, id }); + } catch (error) { + logger.error('[adminGroups] deleteGroup error:', error); + return res.status(500).json({ error: 'Failed to delete group' }); + } + } + + async function getGroupMembersHandler(req: ServerRequest, res: Response) { + try { + const { id } = req.params as GroupIdParams; + if (!isValidObjectIdString(id)) { + return res.status(400).json({ error: 'Invalid group ID format' }); + } + const group = await findGroupById(id, { memberIds: 1 }); + if (!group) { + return res.status(404).json({ error: 'Group not found' }); + } + + /** + * `total` counts unique raw memberId strings. After user resolution, two + * distinct strings may map to the same user, so `members.length` can be + * less than the page size. Write paths prevent this for well-formed data. + */ + const allMemberIds = [...new Set(group.memberIds || [])]; + const total = allMemberIds.length; + const limit = Math.min(Math.max(Number(req.query.limit) || 50, 1), 200); + const offset = Math.max(Number(req.query.offset) || 0, 0); + + if (total === 0 || offset >= total) { + return res.status(200).json({ members: [], total, limit, offset }); + } + + const memberIds = allMemberIds.slice(offset, offset + limit); + + const validObjectIds = memberIds.filter(isValidObjectIdString); + const conditions: FilterQuery[] = [{ idOnTheSource: { $in: memberIds } }]; + if (validObjectIds.length > 0) { + conditions.push({ _id: { $in: validObjectIds } }); + } + const users = await findUsers({ $or: conditions }, 'name email avatar idOnTheSource'); + + const userMap = new Map(); + for (const user of users) { + if (user.idOnTheSource) { + userMap.set(user.idOnTheSource, user); + } + if (user._id) { + userMap.set(user._id.toString(), user); + } + } + + const seen = new Set(); + const members: { userId: string; name: string; email: string; avatarUrl?: string }[] = []; + for (const memberId of memberIds) { + const user = userMap.get(memberId); + const userId = user?._id?.toString() ?? memberId; + if (seen.has(userId)) { + continue; + } + seen.add(userId); + members.push({ + userId, + name: user?.name ?? memberId, + email: user?.email ?? '', + avatarUrl: user?.avatar, + }); + } + + return res.status(200).json({ members, total, limit, offset }); + } catch (error) { + logger.error('[adminGroups] getGroupMembers error:', error); + return res.status(500).json({ error: 'Failed to get group members' }); + } + } + + async function addGroupMemberHandler(req: ServerRequest, res: Response) { + try { + const { id } = req.params as GroupIdParams; + if (!isValidObjectIdString(id)) { + return res.status(400).json({ error: 'Invalid group ID format' }); + } + const { userId } = req.body as { userId: string }; + if (!userId || typeof userId !== 'string') { + return res.status(400).json({ error: 'userId is required' }); + } + if (!isValidObjectIdString(userId)) { + return res + .status(400) + .json({ error: 'Only native user ObjectIds can be added via this endpoint' }); + } + + const { group } = await addUserToGroup(userId, id); + if (!group) { + return res.status(404).json({ error: 'Group not found' }); + } + return res.status(200).json({ group }); + } catch (error) { + const message = error instanceof Error ? error.message : ''; + const isNotFound = message === 'User not found' || message.startsWith('User not found:'); + if (isNotFound) { + return res.status(404).json({ error: 'User not found' }); + } + logger.error('[adminGroups] addGroupMember error:', error); + return res.status(500).json({ error: 'Failed to add member' }); + } + } + + /** + * Attempt removal of an ObjectId-format member: first via removeUserFromGroup + * (which resolves the user), falling back to a raw $pull if the user record + * no longer exists. Returns null only when the group itself is not found. + */ + async function removeObjectIdMember(groupId: string, userId: string): Promise { + try { + const { group } = await removeUserFromGroup(userId, groupId); + return group; + } catch (err) { + const msg = err instanceof Error ? err.message : ''; + if (msg === 'User not found' || msg.startsWith('User not found:')) { + return removeMemberById(groupId, userId); + } + throw err; + } + } + + async function removeGroupMemberHandler(req: ServerRequest, res: Response) { + try { + const { id, userId } = req.params as GroupMemberParams; + if (!isValidObjectIdString(id)) { + return res.status(400).json({ error: 'Invalid group ID format' }); + } + + const group = isValidObjectIdString(userId) + ? await removeObjectIdMember(id, userId) + : await removeMemberById(id, userId); + + if (!group) { + return res.status(404).json({ error: 'Group not found' }); + } + return res.status(200).json({ success: true }); + } catch (error) { + logger.error('[adminGroups] removeGroupMember error:', error); + return res.status(500).json({ error: 'Failed to remove member' }); + } + } + + return { + listGroups: listGroupsHandler, + getGroup: getGroupHandler, + createGroup: createGroupHandler, + updateGroup: updateGroupHandler, + deleteGroup: deleteGroupHandler, + getGroupMembers: getGroupMembersHandler, + addGroupMember: addGroupMemberHandler, + removeGroupMember: removeGroupMemberHandler, + }; +} diff --git a/packages/api/src/admin/index.ts b/packages/api/src/admin/index.ts index bf48ce7345..d833c7e2b0 100644 --- a/packages/api/src/admin/index.ts +++ b/packages/api/src/admin/index.ts @@ -1,2 +1,4 @@ export { createAdminConfigHandlers } from './config'; +export { createAdminGroupsHandlers } from './groups'; export type { AdminConfigDeps } from './config'; +export type { AdminGroupsDeps } from './groups'; diff --git a/packages/data-schemas/src/methods/systemGrant.spec.ts b/packages/data-schemas/src/methods/systemGrant.spec.ts index b17285c761..49b4f7269e 100644 --- a/packages/data-schemas/src/methods/systemGrant.spec.ts +++ b/packages/data-schemas/src/methods/systemGrant.spec.ts @@ -702,6 +702,68 @@ describe('systemGrant methods', () => { }); }); + describe('deleteGrantsForPrincipal', () => { + it('deletes all grants for a principal', async () => { + const groupId = new Types.ObjectId(); + + await methods.grantCapability({ + principalType: PrincipalType.GROUP, + principalId: groupId, + capability: SystemCapabilities.READ_USERS, + }); + await methods.grantCapability({ + principalType: PrincipalType.GROUP, + principalId: groupId, + capability: SystemCapabilities.READ_CONFIGS, + }); + + await methods.deleteGrantsForPrincipal(PrincipalType.GROUP, groupId); + + const remaining = await SystemGrant.countDocuments({ + principalType: PrincipalType.GROUP, + principalId: groupId, + }); + expect(remaining).toBe(0); + }); + + it('is a no-op for principal with no grants', async () => { + const groupId = new Types.ObjectId(); + + await expect( + methods.deleteGrantsForPrincipal(PrincipalType.GROUP, groupId), + ).resolves.not.toThrow(); + }); + + it('does not affect other principals', async () => { + const groupA = new Types.ObjectId(); + const groupB = new Types.ObjectId(); + + await methods.grantCapability({ + principalType: PrincipalType.GROUP, + principalId: groupA, + capability: SystemCapabilities.READ_USERS, + }); + await methods.grantCapability({ + principalType: PrincipalType.GROUP, + principalId: groupB, + capability: SystemCapabilities.READ_USERS, + }); + + await methods.deleteGrantsForPrincipal(PrincipalType.GROUP, groupA); + + const remainingA = await SystemGrant.countDocuments({ + principalType: PrincipalType.GROUP, + principalId: groupA, + }); + const remainingB = await SystemGrant.countDocuments({ + principalType: PrincipalType.GROUP, + principalId: groupB, + }); + expect(remainingA).toBe(0); + expect(remainingB).toBe(1); + }); + }); + describe('schema validation', () => { it('rejects null tenantId at the schema level', async () => { await expect( diff --git a/packages/data-schemas/src/methods/systemGrant.ts b/packages/data-schemas/src/methods/systemGrant.ts index 6071dd38c5..4954f50c16 100644 --- a/packages/data-schemas/src/methods/systemGrant.ts +++ b/packages/data-schemas/src/methods/systemGrant.ts @@ -246,12 +246,28 @@ export function createSystemGrantMethods(mongoose: typeof import('mongoose')) { } } + /** + * Delete all system grants for a principal. + * Used for cascade cleanup when a principal (group, role) is deleted. + */ + async function deleteGrantsForPrincipal( + principalType: PrincipalType, + principalId: string | Types.ObjectId, + session?: ClientSession, + ): Promise { + const SystemGrant = mongoose.models.SystemGrant as Model; + const normalizedPrincipalId = normalizePrincipalId(principalId, principalType); + const options = session ? { session } : {}; + await SystemGrant.deleteMany({ principalType, principalId: normalizedPrincipalId }, options); + } + return { grantCapability, seedSystemGrants, revokeCapability, hasCapabilityForPrincipals, getCapabilitiesForPrincipal, + deleteGrantsForPrincipal, }; } diff --git a/packages/data-schemas/src/methods/user.ts b/packages/data-schemas/src/methods/user.ts index 74cb4a1e1c..137c01d0cd 100644 --- a/packages/data-schemas/src/methods/user.ts +++ b/packages/data-schemas/src/methods/user.ts @@ -44,6 +44,19 @@ export function createUserMethods(mongoose: typeof import('mongoose')) { return (await query.lean()) as IUser | null; } + async function findUsers( + searchCriteria: FilterQuery, + fieldsToSelect?: string | string[] | null, + ): Promise { + const User = mongoose.models.User; + const normalizedCriteria = normalizeEmailInCriteria(searchCriteria); + const query = User.find(normalizedCriteria); + if (fieldsToSelect) { + query.select(fieldsToSelect); + } + return (await query.lean()) as IUser[]; + } + /** * Count the number of user documents in the collection based on the provided filter. */ @@ -323,6 +336,7 @@ export function createUserMethods(mongoose: typeof import('mongoose')) { return { findUser, + findUsers, countUsers, createUser, updateUser, diff --git a/packages/data-schemas/src/methods/userGroup.methods.spec.ts b/packages/data-schemas/src/methods/userGroup.methods.spec.ts index 8a31544018..51848de091 100644 --- a/packages/data-schemas/src/methods/userGroup.methods.spec.ts +++ b/packages/data-schemas/src/methods/userGroup.methods.spec.ts @@ -600,6 +600,155 @@ describe('UserGroup Methods - Detailed Tests', () => { }); }); + describe('listGroups', () => { + beforeEach(async () => { + await Group.create([ + { name: 'Beta', source: 'local', memberIds: [], email: 'beta@test.com' }, + { name: 'Alpha', source: 'local', memberIds: [], description: 'first group' }, + { name: 'Gamma', source: 'entra', idOnTheSource: 'ext-g', memberIds: [] }, + ]); + }); + + test('returns groups sorted by name', async () => { + const groups = await methods.listGroups(); + + expect(groups).toHaveLength(3); + expect(groups[0].name).toBe('Alpha'); + expect(groups[1].name).toBe('Beta'); + expect(groups[2].name).toBe('Gamma'); + }); + + test('filters by source', async () => { + const groups = await methods.listGroups({ source: 'entra' }); + + expect(groups).toHaveLength(1); + expect(groups[0].name).toBe('Gamma'); + }); + + test('filters by search (name)', async () => { + const groups = await methods.listGroups({ search: 'alpha' }); + + expect(groups).toHaveLength(1); + expect(groups[0].name).toBe('Alpha'); + }); + + test('filters by search (email)', async () => { + const groups = await methods.listGroups({ search: 'beta@test' }); + + expect(groups).toHaveLength(1); + expect(groups[0].name).toBe('Beta'); + }); + + test('filters by search (description)', async () => { + const groups = await methods.listGroups({ search: 'first group' }); + + expect(groups).toHaveLength(1); + expect(groups[0].name).toBe('Alpha'); + }); + + test('respects limit and offset', async () => { + const groups = await methods.listGroups({ limit: 1, offset: 1 }); + + expect(groups).toHaveLength(1); + expect(groups[0].name).toBe('Beta'); + }); + + test('returns empty for no matches', async () => { + const groups = await methods.listGroups({ search: 'nonexistent' }); + + expect(groups).toHaveLength(0); + }); + }); + + describe('countGroups', () => { + beforeEach(async () => { + await Group.create([ + { name: 'A', source: 'local', memberIds: [] }, + { name: 'B', source: 'local', memberIds: [] }, + { name: 'C', source: 'entra', idOnTheSource: 'ext-c', memberIds: [] }, + ]); + }); + + test('returns total count', async () => { + const count = await methods.countGroups(); + + expect(count).toBe(3); + }); + + test('respects source filter', async () => { + const count = await methods.countGroups({ source: 'local' }); + + expect(count).toBe(2); + }); + + test('respects search filter', async () => { + const count = await methods.countGroups({ search: 'A' }); + + expect(count).toBe(1); + }); + }); + + describe('deleteGroup', () => { + test('returns deleted group', async () => { + const group = await Group.create({ name: 'ToDelete', source: 'local', memberIds: [] }); + + const deleted = await methods.deleteGroup(group._id as mongoose.Types.ObjectId); + + expect(deleted).toBeDefined(); + expect(deleted?.name).toBe('ToDelete'); + const remaining = await Group.findById(group._id); + expect(remaining).toBeNull(); + }); + + test('returns null for non-existent ID', async () => { + const fakeId = new mongoose.Types.ObjectId(); + const result = await methods.deleteGroup(fakeId); + + expect(result).toBeNull(); + }); + }); + + describe('removeMemberById', () => { + test('removes member from memberIds array', async () => { + const group = await Group.create({ + name: 'Test', + source: 'local', + memberIds: ['m1', 'm2', 'm3'], + }); + + const updated = await methods.removeMemberById( + group._id as mongoose.Types.ObjectId, + 'm2', + ); + + expect(updated).toBeDefined(); + expect(updated?.memberIds).toEqual(['m1', 'm3']); + }); + + test('is idempotent when memberId not present', async () => { + const group = await Group.create({ + name: 'Test', + source: 'local', + memberIds: ['m1'], + }); + + const updated = await methods.removeMemberById( + group._id as mongoose.Types.ObjectId, + 'nonexistent', + ); + + expect(updated).toBeDefined(); + expect(updated?.memberIds).toEqual(['m1']); + }); + + test('returns null for non-existent group', async () => { + const fakeId = new mongoose.Types.ObjectId(); + const result = await methods.removeMemberById(fakeId, 'any-id'); + + expect(result).toBeNull(); + }); + }); + describe('sortPrincipalsByRelevance', () => { test('should sort principals by relevance score', async () => { const principals = [ diff --git a/packages/data-schemas/src/methods/userGroup.ts b/packages/data-schemas/src/methods/userGroup.ts index a41358337c..948542e6de 100644 --- a/packages/data-schemas/src/methods/userGroup.ts +++ b/packages/data-schemas/src/methods/userGroup.ts @@ -1,8 +1,9 @@ import { Types } from 'mongoose'; import { PrincipalType } from 'librechat-data-provider'; import type { TUser, TPrincipalSearchResult } from 'librechat-data-provider'; -import type { Model, ClientSession } from 'mongoose'; +import type { Model, ClientSession, FilterQuery } from 'mongoose'; import type { IGroup, IRole, IUser } from '~/types'; +import { escapeRegExp } from '~/utils/string'; export function createUserGroupMethods(mongoose: typeof import('mongoose')) { /** @@ -14,7 +15,7 @@ export function createUserGroupMethods(mongoose: typeof import('mongoose')) { */ async function findGroupById( groupId: string | Types.ObjectId, - projection: Record = {}, + projection: Record = {}, session?: ClientSession, ): Promise { const Group = mongoose.models.Group as Model; @@ -36,7 +37,7 @@ export function createUserGroupMethods(mongoose: typeof import('mongoose')) { async function findGroupByExternalId( idOnTheSource: string, source: 'entra' | 'local' = 'entra', - projection: Record = {}, + projection: Record = {}, session?: ClientSession, ): Promise { const Group = mongoose.models.Group as Model; @@ -658,6 +659,97 @@ export function createUserGroupMethods(mongoose: typeof import('mongoose')) { return Group.updateMany(filter, update, options || {}); } + function buildGroupQuery(filter: { + source?: 'local' | 'entra'; + search?: string; + }): FilterQuery { + const query: FilterQuery = {}; + if (filter.source) { + query.source = filter.source; + } + if (filter.search) { + const regex = new RegExp(escapeRegExp(filter.search), 'i'); + query.$or = [{ name: regex }, { email: regex }, { description: regex }]; + } + return query; + } + + /** + * List groups with optional source, search, and pagination filters. + * Results are sorted by name. + * @param filter - Optional filter with source, search, limit, and offset fields + * @param session - Optional MongoDB session for transactions + */ + async function listGroups( + filter: { + source?: 'local' | 'entra'; + search?: string; + limit?: number; + offset?: number; + } = {}, + session?: ClientSession, + ): Promise { + const Group = mongoose.models.Group as Model; + const query = buildGroupQuery(filter); + const limit = filter.limit ?? 50; + const offset = filter.offset ?? 0; + return await Group.find(query) + .sort({ name: 1 }) + .skip(offset) + .limit(limit) + .session(session ?? null) + .lean(); + } + + /** + * Count groups matching optional source and search filters. + * @param filter - Optional filter with source and search fields + * @param session - Optional MongoDB session for transactions + */ + async function countGroups( + filter: { source?: 'local' | 'entra'; search?: string } = {}, + session?: ClientSession, + ): Promise { + const Group = mongoose.models.Group as Model; + const query = buildGroupQuery(filter); + return await Group.countDocuments(query).session(session ?? null); + } + + /** + * Delete a group by its ID. + * @param groupId - The group's ObjectId + * @param session - Optional MongoDB session for transactions + */ + async function deleteGroup( + groupId: string | Types.ObjectId, + session?: ClientSession, + ): Promise { + const Group = mongoose.models.Group as Model; + const options = session ? { session } : {}; + return await Group.findByIdAndDelete(groupId, options).lean(); + } + + /** + * Remove a member from a group by raw memberId string ($pull from memberIds). + * Unlike removeUserFromGroup, this does not look up the user first. + * @param groupId - The group's ObjectId + * @param memberId - The raw memberId string to remove (ObjectId or idOnTheSource) + * @param session - Optional MongoDB session for transactions + */ + async function removeMemberById( + groupId: string | Types.ObjectId, + memberId: string, + session?: ClientSession, + ): Promise { + const Group = mongoose.models.Group as Model; + const options = { new: true, ...(session ? { session } : {}) }; + return await Group.findByIdAndUpdate( + groupId, + { $pull: { memberIds: memberId } }, + options, + ).lean(); + } + return { findGroupById, findGroupByExternalId, @@ -677,6 +769,10 @@ export function createUserGroupMethods(mongoose: typeof import('mongoose')) { searchPrincipals, calculateRelevanceScore, sortPrincipalsByRelevance, + listGroups, + countGroups, + deleteGroup, + removeMemberById, }; }