mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-03-21 23:26:34 +01:00
* fix: use ACL ownership for prompt group cleanup on user deletion deleteUserPrompts previously called getAllPromptGroups with only an author filter, which defaults to searchShared=true and drops the author filter for shared/global project entries. This caused any user deleting their account to strip shared prompt group associations and ACL entries for other users. Replace the author-based query with ACL-based ownership lookup: - Find prompt groups where the user has OWNER permission (DELETE bit) - Only delete groups where the user is the sole owner - Preserve multi-owned groups and their ACL entries for other owners * fix: use ACL ownership for agent cleanup on user deletion deleteUserAgents used the deprecated author field to find and delete agents, then unconditionally removed all ACL entries for those agents. This could destroy ACL entries for agents shared with or co-owned by other users. Replace the author-based query with ACL-based ownership lookup: - Find agents where the user has OWNER permission (DELETE bit) - Only delete agents where the user is the sole owner - Preserve multi-owned agents and their ACL entries for other owners - Also clean up handoff edges referencing deleted agents * fix: add MCP server cleanup on user deletion User deletion had no cleanup for MCP servers, leaving solely-owned servers orphaned in the database with dangling ACL entries for other users. Add deleteUserMcpServers that follows the same ACL ownership pattern as prompt groups and agents: find servers with OWNER permission, check for sole ownership, and only delete those with no other owners. * style: fix prettier formatting in Prompt.spec.js * refactor: extract getSoleOwnedResourceIds to PermissionService The ACL sole-ownership detection algorithm was duplicated across deleteUserPrompts, deleteUserAgents, and deleteUserMcpServers. Centralizes the three-step pattern (find owned entries, find other owners, compute sole-owned set) into a single reusable utility. * refactor: use getSoleOwnedResourceIds in all deletion functions - Replace inline ACL queries with the centralized utility - Remove vestigial _req parameter from deleteUserPrompts - Use Promise.all for parallel project removal instead of sequential awaits - Disconnect live MCP sessions and invalidate tool cache before deleting sole-owned MCP server documents - Export deleteUserMcpServers for testability * test: improve deletion test coverage and quality - Move deleteUserPrompts call to beforeAll to eliminate execution-order dependency between tests - Standardize on test() instead of it() for consistency in Prompt.spec.js - Add assertion for deleting user's own ACL entry preservation on multi-owned agents - Add deleteUserMcpServers integration test suite with 6 tests covering sole-owner deletion, multi-owner preservation, session disconnect, cache invalidation, model-not-registered guard, and missing MCPManager - Add PermissionService mock to existing deleteUser.spec.js to fix import chain * fix: add legacy author-based fallback for unmigrated resources Resources created before the ACL system have author set but no AclEntry records. The sole-ownership detection returns empty for these, causing deleteUserPrompts, deleteUserAgents, and deleteUserMcpServers to silently skip them — permanently orphaning data on user deletion. Add a fallback that identifies author-owned resources with zero ACL entries (truly unmigrated) and includes them in the deletion set. This preserves the multi-owner safety of the ACL path while ensuring pre-ACL resources are still cleaned up regardless of migration status. * style: fix prettier formatting across all changed files * test: add resource type coverage guard for user deletion Ensures every ResourceType in the ACL system has a corresponding cleanup handler wired into deleteUserController. When a new ResourceType is added (e.g. WORKFLOW), this test fails immediately, preventing silent data orphaning on user account deletion. * style: fix import order in PermissionService destructure * test: add opt-out set and fix test lifecycle in coverage guard Add NO_USER_CLEANUP_NEEDED set for resource types that legitimately require no per-user deletion. Move fs.readFileSync into beforeAll so path errors surface as clean test failures instead of unhandled crashes.
306 lines
9.8 KiB
JavaScript
306 lines
9.8 KiB
JavaScript
const mockGetUserById = jest.fn();
|
|
const mockDeleteMessages = jest.fn();
|
|
const mockDeleteAllUserSessions = jest.fn();
|
|
const mockDeleteUserById = jest.fn();
|
|
const mockDeleteAllSharedLinks = jest.fn();
|
|
const mockDeletePresets = jest.fn();
|
|
const mockDeleteUserKey = jest.fn();
|
|
const mockDeleteConvos = jest.fn();
|
|
const mockDeleteFiles = jest.fn();
|
|
const mockGetFiles = jest.fn();
|
|
const mockUpdateUserPlugins = jest.fn();
|
|
const mockUpdateUser = jest.fn();
|
|
const mockFindToken = jest.fn();
|
|
const mockVerifyOTPOrBackupCode = jest.fn();
|
|
const mockDeleteUserPluginAuth = jest.fn();
|
|
const mockProcessDeleteRequest = jest.fn();
|
|
const mockDeleteToolCalls = jest.fn();
|
|
const mockDeleteUserAgents = jest.fn();
|
|
const mockDeleteUserPrompts = jest.fn();
|
|
|
|
jest.mock('@librechat/data-schemas', () => ({
|
|
logger: { error: jest.fn(), info: jest.fn() },
|
|
webSearchKeys: [],
|
|
}));
|
|
|
|
jest.mock('librechat-data-provider', () => ({
|
|
Tools: {},
|
|
CacheKeys: {},
|
|
Constants: { mcp_delimiter: '::', mcp_prefix: 'mcp_' },
|
|
FileSources: {},
|
|
}));
|
|
|
|
jest.mock('@librechat/api', () => ({
|
|
MCPOAuthHandler: {},
|
|
MCPTokenStorage: {},
|
|
normalizeHttpError: jest.fn(),
|
|
extractWebSearchEnvVars: jest.fn(),
|
|
}));
|
|
|
|
jest.mock('~/models', () => ({
|
|
deleteAllUserSessions: (...args) => mockDeleteAllUserSessions(...args),
|
|
deleteAllSharedLinks: (...args) => mockDeleteAllSharedLinks(...args),
|
|
updateUserPlugins: (...args) => mockUpdateUserPlugins(...args),
|
|
deleteUserById: (...args) => mockDeleteUserById(...args),
|
|
deleteMessages: (...args) => mockDeleteMessages(...args),
|
|
deletePresets: (...args) => mockDeletePresets(...args),
|
|
deleteUserKey: (...args) => mockDeleteUserKey(...args),
|
|
getUserById: (...args) => mockGetUserById(...args),
|
|
deleteConvos: (...args) => mockDeleteConvos(...args),
|
|
deleteFiles: (...args) => mockDeleteFiles(...args),
|
|
updateUser: (...args) => mockUpdateUser(...args),
|
|
findToken: (...args) => mockFindToken(...args),
|
|
getFiles: (...args) => mockGetFiles(...args),
|
|
}));
|
|
|
|
jest.mock('~/db/models', () => ({
|
|
ConversationTag: { deleteMany: jest.fn() },
|
|
AgentApiKey: { deleteMany: jest.fn() },
|
|
Transaction: { deleteMany: jest.fn() },
|
|
MemoryEntry: { deleteMany: jest.fn() },
|
|
Assistant: { deleteMany: jest.fn() },
|
|
AclEntry: { deleteMany: jest.fn() },
|
|
Balance: { deleteMany: jest.fn() },
|
|
Action: { deleteMany: jest.fn() },
|
|
Group: { updateMany: jest.fn() },
|
|
Token: { deleteMany: jest.fn() },
|
|
User: {},
|
|
}));
|
|
|
|
jest.mock('~/server/services/PluginService', () => ({
|
|
updateUserPluginAuth: jest.fn(),
|
|
deleteUserPluginAuth: (...args) => mockDeleteUserPluginAuth(...args),
|
|
}));
|
|
|
|
jest.mock('~/server/services/twoFactorService', () => ({
|
|
verifyOTPOrBackupCode: (...args) => mockVerifyOTPOrBackupCode(...args),
|
|
}));
|
|
|
|
jest.mock('~/server/services/AuthService', () => ({
|
|
verifyEmail: jest.fn(),
|
|
resendVerificationEmail: jest.fn(),
|
|
}));
|
|
|
|
jest.mock('~/config', () => ({
|
|
getMCPManager: jest.fn(),
|
|
getFlowStateManager: jest.fn(),
|
|
getMCPServersRegistry: jest.fn(),
|
|
}));
|
|
|
|
jest.mock('~/server/services/Config/getCachedTools', () => ({
|
|
invalidateCachedTools: jest.fn(),
|
|
}));
|
|
|
|
jest.mock('~/server/services/Files/S3/crud', () => ({
|
|
needsRefresh: jest.fn(),
|
|
getNewS3URL: jest.fn(),
|
|
}));
|
|
|
|
jest.mock('~/server/services/Files/process', () => ({
|
|
processDeleteRequest: (...args) => mockProcessDeleteRequest(...args),
|
|
}));
|
|
|
|
jest.mock('~/server/services/Config', () => ({
|
|
getAppConfig: jest.fn(),
|
|
}));
|
|
|
|
jest.mock('~/server/services/PermissionService', () => ({
|
|
getSoleOwnedResourceIds: jest.fn().mockResolvedValue([]),
|
|
}));
|
|
|
|
jest.mock('~/models/ToolCall', () => ({
|
|
deleteToolCalls: (...args) => mockDeleteToolCalls(...args),
|
|
}));
|
|
|
|
jest.mock('~/models/Prompt', () => ({
|
|
deleteUserPrompts: (...args) => mockDeleteUserPrompts(...args),
|
|
}));
|
|
|
|
jest.mock('~/models/Agent', () => ({
|
|
deleteUserAgents: (...args) => mockDeleteUserAgents(...args),
|
|
}));
|
|
|
|
jest.mock('~/cache', () => ({
|
|
getLogStores: jest.fn(),
|
|
}));
|
|
|
|
const { deleteUserController } = require('~/server/controllers/UserController');
|
|
|
|
function createRes() {
|
|
const res = {};
|
|
res.status = jest.fn().mockReturnValue(res);
|
|
res.json = jest.fn().mockReturnValue(res);
|
|
res.send = jest.fn().mockReturnValue(res);
|
|
return res;
|
|
}
|
|
|
|
function stubDeletionMocks() {
|
|
mockDeleteMessages.mockResolvedValue();
|
|
mockDeleteAllUserSessions.mockResolvedValue();
|
|
mockDeleteUserKey.mockResolvedValue();
|
|
mockDeletePresets.mockResolvedValue();
|
|
mockDeleteConvos.mockResolvedValue();
|
|
mockDeleteUserPluginAuth.mockResolvedValue();
|
|
mockDeleteUserById.mockResolvedValue();
|
|
mockDeleteAllSharedLinks.mockResolvedValue();
|
|
mockGetFiles.mockResolvedValue([]);
|
|
mockProcessDeleteRequest.mockResolvedValue();
|
|
mockDeleteFiles.mockResolvedValue();
|
|
mockDeleteToolCalls.mockResolvedValue();
|
|
mockDeleteUserAgents.mockResolvedValue();
|
|
mockDeleteUserPrompts.mockResolvedValue();
|
|
}
|
|
|
|
beforeEach(() => {
|
|
jest.clearAllMocks();
|
|
stubDeletionMocks();
|
|
});
|
|
|
|
describe('deleteUserController - 2FA enforcement', () => {
|
|
it('proceeds with deletion when 2FA is not enabled', async () => {
|
|
const req = { user: { id: 'user1', _id: 'user1', email: 'a@b.com' }, body: {} };
|
|
const res = createRes();
|
|
mockGetUserById.mockResolvedValue({ _id: 'user1', twoFactorEnabled: false });
|
|
|
|
await deleteUserController(req, res);
|
|
|
|
expect(res.status).toHaveBeenCalledWith(200);
|
|
expect(res.send).toHaveBeenCalledWith({ message: 'User deleted' });
|
|
expect(mockDeleteMessages).toHaveBeenCalled();
|
|
expect(mockVerifyOTPOrBackupCode).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it('proceeds with deletion when user has no 2FA record', async () => {
|
|
const req = { user: { id: 'user1', _id: 'user1', email: 'a@b.com' }, body: {} };
|
|
const res = createRes();
|
|
mockGetUserById.mockResolvedValue(null);
|
|
|
|
await deleteUserController(req, res);
|
|
|
|
expect(res.status).toHaveBeenCalledWith(200);
|
|
expect(res.send).toHaveBeenCalledWith({ message: 'User deleted' });
|
|
});
|
|
|
|
it('returns error when 2FA is enabled and verification fails with 400', async () => {
|
|
const req = { user: { id: 'user1', _id: 'user1' }, body: {} };
|
|
const res = createRes();
|
|
mockGetUserById.mockResolvedValue({
|
|
_id: 'user1',
|
|
twoFactorEnabled: true,
|
|
totpSecret: 'enc-secret',
|
|
});
|
|
mockVerifyOTPOrBackupCode.mockResolvedValue({ verified: false, status: 400 });
|
|
|
|
await deleteUserController(req, res);
|
|
|
|
expect(res.status).toHaveBeenCalledWith(400);
|
|
expect(mockDeleteMessages).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it('returns 401 when 2FA is enabled and invalid TOTP token provided', async () => {
|
|
const existingUser = {
|
|
_id: 'user1',
|
|
twoFactorEnabled: true,
|
|
totpSecret: 'enc-secret',
|
|
};
|
|
const req = { user: { id: 'user1', _id: 'user1' }, body: { token: 'wrong' } };
|
|
const res = createRes();
|
|
mockGetUserById.mockResolvedValue(existingUser);
|
|
mockVerifyOTPOrBackupCode.mockResolvedValue({
|
|
verified: false,
|
|
status: 401,
|
|
message: 'Invalid token or backup code',
|
|
});
|
|
|
|
await deleteUserController(req, res);
|
|
|
|
expect(mockVerifyOTPOrBackupCode).toHaveBeenCalledWith({
|
|
user: existingUser,
|
|
token: 'wrong',
|
|
backupCode: undefined,
|
|
});
|
|
expect(res.status).toHaveBeenCalledWith(401);
|
|
expect(res.json).toHaveBeenCalledWith({ message: 'Invalid token or backup code' });
|
|
expect(mockDeleteMessages).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it('returns 401 when 2FA is enabled and invalid backup code provided', async () => {
|
|
const existingUser = {
|
|
_id: 'user1',
|
|
twoFactorEnabled: true,
|
|
totpSecret: 'enc-secret',
|
|
backupCodes: [],
|
|
};
|
|
const req = { user: { id: 'user1', _id: 'user1' }, body: { backupCode: 'bad-code' } };
|
|
const res = createRes();
|
|
mockGetUserById.mockResolvedValue(existingUser);
|
|
mockVerifyOTPOrBackupCode.mockResolvedValue({
|
|
verified: false,
|
|
status: 401,
|
|
message: 'Invalid token or backup code',
|
|
});
|
|
|
|
await deleteUserController(req, res);
|
|
|
|
expect(mockVerifyOTPOrBackupCode).toHaveBeenCalledWith({
|
|
user: existingUser,
|
|
token: undefined,
|
|
backupCode: 'bad-code',
|
|
});
|
|
expect(res.status).toHaveBeenCalledWith(401);
|
|
expect(mockDeleteMessages).not.toHaveBeenCalled();
|
|
});
|
|
|
|
it('deletes account when valid TOTP token provided with 2FA enabled', async () => {
|
|
const existingUser = {
|
|
_id: 'user1',
|
|
twoFactorEnabled: true,
|
|
totpSecret: 'enc-secret',
|
|
};
|
|
const req = {
|
|
user: { id: 'user1', _id: 'user1', email: 'a@b.com' },
|
|
body: { token: '123456' },
|
|
};
|
|
const res = createRes();
|
|
mockGetUserById.mockResolvedValue(existingUser);
|
|
mockVerifyOTPOrBackupCode.mockResolvedValue({ verified: true });
|
|
|
|
await deleteUserController(req, res);
|
|
|
|
expect(mockVerifyOTPOrBackupCode).toHaveBeenCalledWith({
|
|
user: existingUser,
|
|
token: '123456',
|
|
backupCode: undefined,
|
|
});
|
|
expect(res.status).toHaveBeenCalledWith(200);
|
|
expect(res.send).toHaveBeenCalledWith({ message: 'User deleted' });
|
|
expect(mockDeleteMessages).toHaveBeenCalled();
|
|
});
|
|
|
|
it('deletes account when valid backup code provided with 2FA enabled', async () => {
|
|
const existingUser = {
|
|
_id: 'user1',
|
|
twoFactorEnabled: true,
|
|
totpSecret: 'enc-secret',
|
|
backupCodes: [{ codeHash: 'h1', used: false }],
|
|
};
|
|
const req = {
|
|
user: { id: 'user1', _id: 'user1', email: 'a@b.com' },
|
|
body: { backupCode: 'valid-code' },
|
|
};
|
|
const res = createRes();
|
|
mockGetUserById.mockResolvedValue(existingUser);
|
|
mockVerifyOTPOrBackupCode.mockResolvedValue({ verified: true });
|
|
|
|
await deleteUserController(req, res);
|
|
|
|
expect(mockVerifyOTPOrBackupCode).toHaveBeenCalledWith({
|
|
user: existingUser,
|
|
token: undefined,
|
|
backupCode: 'valid-code',
|
|
});
|
|
expect(res.status).toHaveBeenCalledWith(200);
|
|
expect(res.send).toHaveBeenCalledWith({ message: 'User deleted' });
|
|
expect(mockDeleteMessages).toHaveBeenCalled();
|
|
});
|
|
});
|