mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-02-15 15:08:10 +01:00
🐘 feat: FerretDB Compatibility (#11769)
* feat: replace unsupported MongoDB aggregation operators for FerretDB compatibility Replace $lookup, $unwind, $sample, $replaceRoot, and $addFields aggregation stages which are unsupported on FerretDB v2.x (postgres-documentdb backend). - Prompt.js: Replace $lookup/$unwind/$project pipelines with find().select().lean() + attachProductionPrompts() batch helper. Replace $group/$replaceRoot/$sample in getRandomPromptGroups with distinct() + Fisher-Yates shuffle. - Agent/Prompt migration scripts: Replace $lookup anti-join pattern with distinct() + $nin two-step queries for finding un-migrated resources. All replacement patterns verified against FerretDB v2.7.0. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: use $pullAll for simple array removals, fix memberIds type mismatches Replace $pull with $pullAll for exact-value scalar array removals. Both operators work on MongoDB and FerretDB, but $pullAll is more explicit for exact matching (no condition expressions). Fix critical type mismatch bugs where ObjectId values were used against String[] memberIds arrays in Group queries: - config/delete-user.js: use string uid instead of ObjectId user._id - e2e/setup/cleanupUser.ts: convert userId.toString() before query Harden PermissionService.bulkUpdateResourcePermissions abort handling to prevent crash when abortTransaction is called after commitTransaction. All changes verified against FerretDB v2.7.0 and MongoDB Memory Server. Co-authored-by: Cursor <cursoragent@cursor.com> * fix: harden transaction support probe for FerretDB compatibility Commit the transaction before aborting in supportsTransactions probe, and wrap abortTransaction in try-catch to prevent crashes when abort is called after a successful commit (observed behavior on FerretDB). Co-authored-by: Cursor <cursoragent@cursor.com> * feat: add FerretDB compatibility test suite, retry utilities, and CI config Add comprehensive FerretDB integration test suite covering: - $pullAll scalar array operations - $pull with subdocument conditions - $lookup replacement (find + manual join) - $sample replacement (distinct + Fisher-Yates) - $bit and $bitsAllSet operations - Migration anti-join pattern - Multi-tenancy (useDb, scaling, write amplification) - Sharding proof-of-concept - Production operations (backup/restore, schema migration, deadlock retry) Add production retryWithBackoff utility for deadlock recovery during concurrent index creation on FerretDB/DocumentDB backends. Add UserController.spec.js tests for deleteUserController (runs in CI). Configure jest and eslint to isolate FerretDB tests from CI pipelines: - packages/data-schemas/jest.config.mjs: ignore misc/ directory - eslint.config.mjs: ignore packages/data-schemas/misc/ Include Docker Compose config for local FerretDB v2.7 + postgres-documentdb, dedicated jest/tsconfig for the test files, and multi-tenancy findings doc. Co-authored-by: Cursor <cursoragent@cursor.com> * style: brace formatting in aclEntry.ts modifyPermissionBits Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: reorganize retry utilities and update imports - Moved retryWithBackoff utility to a new file `retry.ts` for better structure. - Updated imports in `orgOperations.ferretdb.spec.ts` to reflect the new location of retry utilities. - Removed old import statement for retryWithBackoff from index.ts to streamline exports. * test: add $pullAll coverage for ConversationTag and PermissionService Add integration tests for deleteConversationTag verifying $pullAll removes tags from conversations correctly, and for syncUserEntraGroupMemberships verifying $pullAll removes user from non-matching Entra groups while preserving local group membership. --------- Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
parent
dc489e7b25
commit
3398f6a17a
35 changed files with 4727 additions and 347 deletions
|
|
@ -266,11 +266,7 @@ const deleteUserController = async (req, res) => {
|
|||
await deleteUserPrompts(req, user.id); // delete user prompts
|
||||
await Action.deleteMany({ user: user.id }); // delete user actions
|
||||
await Token.deleteMany({ userId: user.id }); // delete user OAuth tokens
|
||||
await Group.updateMany(
|
||||
// remove user from all groups
|
||||
{ memberIds: user.id },
|
||||
{ $pull: { memberIds: user.id } },
|
||||
);
|
||||
await Group.updateMany({ memberIds: user.id }, { $pullAll: { memberIds: [user.id] } });
|
||||
await AclEntry.deleteMany({ principalId: user._id }); // delete user ACL entries
|
||||
logger.info(`User deleted account. Email: ${user.email} ID: ${user.id}`);
|
||||
res.status(200).send({ message: 'User deleted' });
|
||||
|
|
|
|||
208
api/server/controllers/UserController.spec.js
Normal file
208
api/server/controllers/UserController.spec.js
Normal file
|
|
@ -0,0 +1,208 @@
|
|||
const mongoose = require('mongoose');
|
||||
const { MongoMemoryServer } = require('mongodb-memory-server');
|
||||
|
||||
jest.mock('@librechat/data-schemas', () => {
|
||||
const actual = jest.requireActual('@librechat/data-schemas');
|
||||
return {
|
||||
...actual,
|
||||
logger: {
|
||||
debug: jest.fn(),
|
||||
error: jest.fn(),
|
||||
warn: jest.fn(),
|
||||
info: jest.fn(),
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
jest.mock('~/models', () => ({
|
||||
deleteAllUserSessions: jest.fn().mockResolvedValue(undefined),
|
||||
deleteAllSharedLinks: jest.fn().mockResolvedValue(undefined),
|
||||
updateUserPlugins: jest.fn(),
|
||||
deleteUserById: jest.fn().mockResolvedValue(undefined),
|
||||
deleteMessages: jest.fn().mockResolvedValue(undefined),
|
||||
deletePresets: jest.fn().mockResolvedValue(undefined),
|
||||
deleteUserKey: jest.fn().mockResolvedValue(undefined),
|
||||
deleteConvos: jest.fn().mockResolvedValue(undefined),
|
||||
deleteFiles: jest.fn().mockResolvedValue(undefined),
|
||||
updateUser: jest.fn(),
|
||||
findToken: jest.fn(),
|
||||
getFiles: jest.fn().mockResolvedValue([]),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/services/PluginService', () => ({
|
||||
updateUserPluginAuth: jest.fn(),
|
||||
deleteUserPluginAuth: jest.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/services/AuthService', () => ({
|
||||
verifyEmail: jest.fn(),
|
||||
resendVerificationEmail: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/services/Files/S3/crud', () => ({
|
||||
needsRefresh: jest.fn(),
|
||||
getNewS3URL: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/services/Files/process', () => ({
|
||||
processDeleteRequest: jest.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/services/Config', () => ({
|
||||
getAppConfig: jest.fn().mockResolvedValue({}),
|
||||
getMCPManager: jest.fn(),
|
||||
getFlowStateManager: jest.fn(),
|
||||
getMCPServersRegistry: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('~/models/ToolCall', () => ({
|
||||
deleteToolCalls: jest.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
jest.mock('~/models/Prompt', () => ({
|
||||
deleteUserPrompts: jest.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
jest.mock('~/models/Agent', () => ({
|
||||
deleteUserAgents: jest.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
jest.mock('~/cache', () => ({
|
||||
getLogStores: jest.fn(),
|
||||
}));
|
||||
|
||||
let mongoServer;
|
||||
|
||||
beforeAll(async () => {
|
||||
mongoServer = await MongoMemoryServer.create();
|
||||
await mongoose.connect(mongoServer.getUri());
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await mongoose.disconnect();
|
||||
await mongoServer.stop();
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
const collections = mongoose.connection.collections;
|
||||
for (const key in collections) {
|
||||
await collections[key].deleteMany({});
|
||||
}
|
||||
});
|
||||
|
||||
const { deleteUserController } = require('./UserController');
|
||||
const { Group } = require('~/db/models');
|
||||
const { deleteConvos } = require('~/models');
|
||||
|
||||
describe('deleteUserController', () => {
|
||||
const mockRes = {
|
||||
status: jest.fn().mockReturnThis(),
|
||||
send: jest.fn().mockReturnThis(),
|
||||
json: jest.fn().mockReturnThis(),
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
});
|
||||
|
||||
it('should return 200 on successful deletion', async () => {
|
||||
const userId = new mongoose.Types.ObjectId();
|
||||
const req = { user: { id: userId.toString(), _id: userId, email: 'test@test.com' } };
|
||||
|
||||
await deleteUserController(req, mockRes);
|
||||
|
||||
expect(mockRes.status).toHaveBeenCalledWith(200);
|
||||
expect(mockRes.send).toHaveBeenCalledWith({ message: 'User deleted' });
|
||||
});
|
||||
|
||||
it('should remove the user from all groups via $pullAll', async () => {
|
||||
const userId = new mongoose.Types.ObjectId();
|
||||
const userIdStr = userId.toString();
|
||||
const otherUser = new mongoose.Types.ObjectId().toString();
|
||||
|
||||
await Group.create([
|
||||
{ name: 'Group A', memberIds: [userIdStr, otherUser], source: 'local' },
|
||||
{ name: 'Group B', memberIds: [userIdStr], source: 'local' },
|
||||
{ name: 'Group C', memberIds: [otherUser], source: 'local' },
|
||||
]);
|
||||
|
||||
const req = { user: { id: userIdStr, _id: userId, email: 'del@test.com' } };
|
||||
await deleteUserController(req, mockRes);
|
||||
|
||||
const groups = await Group.find({}).sort({ name: 1 }).lean();
|
||||
expect(groups[0].memberIds).toEqual([otherUser]);
|
||||
expect(groups[1].memberIds).toEqual([]);
|
||||
expect(groups[2].memberIds).toEqual([otherUser]);
|
||||
});
|
||||
|
||||
it('should handle user that exists in no groups', async () => {
|
||||
const userId = new mongoose.Types.ObjectId();
|
||||
await Group.create({ name: 'Empty', memberIds: ['someone-else'], source: 'local' });
|
||||
|
||||
const req = { user: { id: userId.toString(), _id: userId, email: 'no-groups@test.com' } };
|
||||
await deleteUserController(req, mockRes);
|
||||
|
||||
expect(mockRes.status).toHaveBeenCalledWith(200);
|
||||
const group = await Group.findOne({ name: 'Empty' }).lean();
|
||||
expect(group.memberIds).toEqual(['someone-else']);
|
||||
});
|
||||
|
||||
it('should remove duplicate memberIds if the user appears more than once', async () => {
|
||||
const userId = new mongoose.Types.ObjectId();
|
||||
const userIdStr = userId.toString();
|
||||
|
||||
await Group.create({
|
||||
name: 'Dupes',
|
||||
memberIds: [userIdStr, 'other', userIdStr],
|
||||
source: 'local',
|
||||
});
|
||||
|
||||
const req = { user: { id: userIdStr, _id: userId, email: 'dupe@test.com' } };
|
||||
await deleteUserController(req, mockRes);
|
||||
|
||||
const group = await Group.findOne({ name: 'Dupes' }).lean();
|
||||
expect(group.memberIds).toEqual(['other']);
|
||||
});
|
||||
|
||||
it('should still succeed when deleteConvos throws', async () => {
|
||||
const userId = new mongoose.Types.ObjectId();
|
||||
deleteConvos.mockRejectedValueOnce(new Error('no convos'));
|
||||
|
||||
const req = { user: { id: userId.toString(), _id: userId, email: 'convos@test.com' } };
|
||||
await deleteUserController(req, mockRes);
|
||||
|
||||
expect(mockRes.status).toHaveBeenCalledWith(200);
|
||||
expect(mockRes.send).toHaveBeenCalledWith({ message: 'User deleted' });
|
||||
});
|
||||
|
||||
it('should return 500 when a critical operation fails', async () => {
|
||||
const userId = new mongoose.Types.ObjectId();
|
||||
const { deleteMessages } = require('~/models');
|
||||
deleteMessages.mockRejectedValueOnce(new Error('db down'));
|
||||
|
||||
const req = { user: { id: userId.toString(), _id: userId, email: 'fail@test.com' } };
|
||||
await deleteUserController(req, mockRes);
|
||||
|
||||
expect(mockRes.status).toHaveBeenCalledWith(500);
|
||||
expect(mockRes.json).toHaveBeenCalledWith({ message: 'Something went wrong.' });
|
||||
});
|
||||
|
||||
it('should use string user.id (not ObjectId user._id) for memberIds removal', async () => {
|
||||
const userId = new mongoose.Types.ObjectId();
|
||||
const userIdStr = userId.toString();
|
||||
const otherUser = 'other-user-id';
|
||||
|
||||
await Group.create({
|
||||
name: 'StringCheck',
|
||||
memberIds: [userIdStr, otherUser],
|
||||
source: 'local',
|
||||
});
|
||||
|
||||
const req = { user: { id: userIdStr, _id: userId, email: 'stringcheck@test.com' } };
|
||||
await deleteUserController(req, mockRes);
|
||||
|
||||
const group = await Group.findOne({ name: 'StringCheck' }).lean();
|
||||
expect(group.memberIds).toEqual([otherUser]);
|
||||
expect(group.memberIds).not.toContain(userIdStr);
|
||||
});
|
||||
});
|
||||
|
|
@ -557,7 +557,6 @@ describe('Agent Controllers - Mass Assignment Protection', () => {
|
|||
|
||||
const updatedAgent = mockRes.json.mock.calls[0][0];
|
||||
expect(updatedAgent).toBeDefined();
|
||||
// Note: updateAgentProjects requires more setup, so we just verify the handler doesn't crash
|
||||
});
|
||||
|
||||
test('should validate tool_resources in updates', async () => {
|
||||
|
|
|
|||
|
|
@ -536,7 +536,7 @@ const syncUserEntraGroupMemberships = async (user, accessToken, session = null)
|
|||
memberIds: user.idOnTheSource,
|
||||
idOnTheSource: { $nin: allGroupIds },
|
||||
},
|
||||
{ $pull: { memberIds: user.idOnTheSource } },
|
||||
{ $pullAll: { memberIds: [user.idOnTheSource] } },
|
||||
sessionOptions,
|
||||
);
|
||||
} catch (error) {
|
||||
|
|
@ -788,7 +788,15 @@ const bulkUpdateResourcePermissions = async ({
|
|||
return results;
|
||||
} catch (error) {
|
||||
if (shouldEndSession && supportsTransactions) {
|
||||
await localSession.abortTransaction();
|
||||
try {
|
||||
await localSession.abortTransaction();
|
||||
} catch (transactionError) {
|
||||
/** best-effort abort; may fail if commit already succeeded */
|
||||
logger.error(
|
||||
`[PermissionService.bulkUpdateResourcePermissions] Error aborting transaction:`,
|
||||
transactionError,
|
||||
);
|
||||
}
|
||||
}
|
||||
logger.error(`[PermissionService.bulkUpdateResourcePermissions] Error: ${error.message}`);
|
||||
throw error;
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ const {
|
|||
} = require('librechat-data-provider');
|
||||
const {
|
||||
bulkUpdateResourcePermissions,
|
||||
syncUserEntraGroupMemberships,
|
||||
getEffectivePermissions,
|
||||
findAccessibleResources,
|
||||
getAvailableRoles,
|
||||
|
|
@ -26,7 +27,11 @@ jest.mock('@librechat/data-schemas', () => ({
|
|||
|
||||
// Mock GraphApiService to prevent config loading issues
|
||||
jest.mock('~/server/services/GraphApiService', () => ({
|
||||
entraIdPrincipalFeatureEnabled: jest.fn().mockReturnValue(false),
|
||||
getUserOwnedEntraGroups: jest.fn().mockResolvedValue([]),
|
||||
getUserEntraGroups: jest.fn().mockResolvedValue([]),
|
||||
getGroupMembers: jest.fn().mockResolvedValue([]),
|
||||
getGroupOwners: jest.fn().mockResolvedValue([]),
|
||||
}));
|
||||
|
||||
// Mock the logger
|
||||
|
|
@ -1933,3 +1938,134 @@ describe('PermissionService', () => {
|
|||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('syncUserEntraGroupMemberships - $pullAll on Group.memberIds', () => {
|
||||
const {
|
||||
entraIdPrincipalFeatureEnabled,
|
||||
getUserEntraGroups,
|
||||
} = require('~/server/services/GraphApiService');
|
||||
const { Group } = require('~/db/models');
|
||||
|
||||
const userEntraId = 'entra-user-001';
|
||||
const user = {
|
||||
openidId: 'openid-sub-001',
|
||||
idOnTheSource: userEntraId,
|
||||
provider: 'openid',
|
||||
};
|
||||
|
||||
beforeEach(async () => {
|
||||
await Group.deleteMany({});
|
||||
entraIdPrincipalFeatureEnabled.mockReturnValue(true);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
entraIdPrincipalFeatureEnabled.mockReturnValue(false);
|
||||
getUserEntraGroups.mockResolvedValue([]);
|
||||
});
|
||||
|
||||
it('should add user to matching Entra groups and remove from non-matching ones', async () => {
|
||||
await Group.create([
|
||||
{ name: 'Group A', source: 'entra', idOnTheSource: 'entra-group-a', memberIds: [] },
|
||||
{
|
||||
name: 'Group B',
|
||||
source: 'entra',
|
||||
idOnTheSource: 'entra-group-b',
|
||||
memberIds: [userEntraId],
|
||||
},
|
||||
{
|
||||
name: 'Group C',
|
||||
source: 'entra',
|
||||
idOnTheSource: 'entra-group-c',
|
||||
memberIds: [userEntraId],
|
||||
},
|
||||
]);
|
||||
|
||||
getUserEntraGroups.mockResolvedValue(['entra-group-a', 'entra-group-c']);
|
||||
|
||||
await syncUserEntraGroupMemberships(user, 'fake-access-token');
|
||||
|
||||
const groups = await Group.find({ source: 'entra' }).sort({ name: 1 }).lean();
|
||||
expect(groups[0].memberIds).toContain(userEntraId);
|
||||
expect(groups[1].memberIds).not.toContain(userEntraId);
|
||||
expect(groups[2].memberIds).toContain(userEntraId);
|
||||
});
|
||||
|
||||
it('should not modify groups when API returns empty list (early return)', async () => {
|
||||
await Group.create([
|
||||
{
|
||||
name: 'Group X',
|
||||
source: 'entra',
|
||||
idOnTheSource: 'entra-x',
|
||||
memberIds: [userEntraId, 'other-user'],
|
||||
},
|
||||
{ name: 'Group Y', source: 'entra', idOnTheSource: 'entra-y', memberIds: [userEntraId] },
|
||||
]);
|
||||
|
||||
getUserEntraGroups.mockResolvedValue([]);
|
||||
|
||||
await syncUserEntraGroupMemberships(user, 'fake-token');
|
||||
|
||||
const groups = await Group.find({ source: 'entra' }).sort({ name: 1 }).lean();
|
||||
expect(groups[0].memberIds).toContain(userEntraId);
|
||||
expect(groups[0].memberIds).toContain('other-user');
|
||||
expect(groups[1].memberIds).toContain(userEntraId);
|
||||
});
|
||||
|
||||
it('should remove user from groups not in the API response via $pullAll', async () => {
|
||||
await Group.create([
|
||||
{ name: 'Keep', source: 'entra', idOnTheSource: 'entra-keep', memberIds: [userEntraId] },
|
||||
{
|
||||
name: 'Remove',
|
||||
source: 'entra',
|
||||
idOnTheSource: 'entra-remove',
|
||||
memberIds: [userEntraId, 'other-user'],
|
||||
},
|
||||
]);
|
||||
|
||||
getUserEntraGroups.mockResolvedValue(['entra-keep']);
|
||||
|
||||
await syncUserEntraGroupMemberships(user, 'fake-token');
|
||||
|
||||
const keep = await Group.findOne({ idOnTheSource: 'entra-keep' }).lean();
|
||||
const remove = await Group.findOne({ idOnTheSource: 'entra-remove' }).lean();
|
||||
expect(keep.memberIds).toContain(userEntraId);
|
||||
expect(remove.memberIds).not.toContain(userEntraId);
|
||||
expect(remove.memberIds).toContain('other-user');
|
||||
});
|
||||
|
||||
it('should not modify local groups', async () => {
|
||||
await Group.create([
|
||||
{ name: 'Local Group', source: 'local', memberIds: [userEntraId] },
|
||||
{
|
||||
name: 'Entra Group',
|
||||
source: 'entra',
|
||||
idOnTheSource: 'entra-only',
|
||||
memberIds: [userEntraId],
|
||||
},
|
||||
]);
|
||||
|
||||
getUserEntraGroups.mockResolvedValue([]);
|
||||
|
||||
await syncUserEntraGroupMemberships(user, 'fake-token');
|
||||
|
||||
const localGroup = await Group.findOne({ source: 'local' }).lean();
|
||||
expect(localGroup.memberIds).toContain(userEntraId);
|
||||
});
|
||||
|
||||
it('should early-return when feature is disabled', async () => {
|
||||
entraIdPrincipalFeatureEnabled.mockReturnValue(false);
|
||||
|
||||
await Group.create({
|
||||
name: 'Should Not Touch',
|
||||
source: 'entra',
|
||||
idOnTheSource: 'entra-safe',
|
||||
memberIds: [userEntraId],
|
||||
});
|
||||
|
||||
getUserEntraGroups.mockResolvedValue([]);
|
||||
await syncUserEntraGroupMemberships(user, 'fake-token');
|
||||
|
||||
const group = await Group.findOne({ idOnTheSource: 'entra-safe' }).lean();
|
||||
expect(group.memberIds).toContain(userEntraId);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue