mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-07-11 08:43:48 +00:00
🕒 feat: Track Terms Acceptance Timestamp (#10810)
* feat: add terms acceptance timestamp tracking and migration script * feat: update migration script to use countUsers method for user count * Update config/migrate-terms-timestamp.js Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * feat: enhance terms acceptance response to include acceptance timestamp * fix: make terms acceptance idempotent and fail migration on partial errors Preserve the original termsAcceptedAt on repeat accepts within a terms cycle so retried or duplicate requests no longer overwrite the first acceptance time. Exit the migration script with a non-zero status when any per-user update fails so partial failures are not reported as successful. * style: fix import ordering in data-provider mutations * refactor: record terms acceptance atomically to preserve first-accept time Replace the read-then-write in acceptTermsController with a single atomic acceptTerms method that conditionally stamps termsAcceptedAt via an $ifNull aggregation update. This removes the TOCTOU window where two concurrent first-time accepts could overwrite the earlier acceptance timestamp, while still preserving an existing timestamp and backfilling legacy accepted users. * fix: run terms timestamp migration under system tenant context Wrap the count, cursor scan, and per-user updates in runAsSystem so the tenant isolation plugin does not throw under TENANT_ISOLATION_STRICT or scope the cross-tenant migration to a non-existent tenant, matching the other maintenance migrations. * fix: guard terms backfill against concurrent acceptances Add the missing-timestamp predicate to the per-user updateOne filter so a user who accepts through the API between the cursor read and the write keeps their real acceptance time instead of being overwritten with createdAt. Track modified vs skipped so the summary reflects skips. * fix: scope terms backfill to still-accepted users Add termsAccepted: true to the per-user updateOne filter so a reset that clears acceptance between the cursor read and the write is not re-stamped with createdAt, which would otherwise poison the next acceptance cycle through the $ifNull preserve in acceptTerms. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
parent
9e74cc0e57
commit
b84e26671e
13 changed files with 299 additions and 9 deletions
|
|
@ -86,11 +86,14 @@ const getUserController = async (req, res) => {
|
|||
|
||||
const getTermsStatusController = async (req, res) => {
|
||||
try {
|
||||
const user = await db.getUserById(req.user.id, 'termsAccepted');
|
||||
const user = await db.getUserById(req.user.id, 'termsAccepted termsAcceptedAt');
|
||||
if (!user) {
|
||||
return res.status(404).json({ message: 'User not found' });
|
||||
}
|
||||
res.status(200).json({ termsAccepted: !!user.termsAccepted });
|
||||
res.status(200).json({
|
||||
termsAccepted: !!user.termsAccepted,
|
||||
termsAcceptedAt: user.termsAcceptedAt || null,
|
||||
});
|
||||
} catch (error) {
|
||||
logger.error('Error fetching terms acceptance status:', error);
|
||||
res.status(500).json({ message: 'Error fetching terms acceptance status' });
|
||||
|
|
@ -99,11 +102,14 @@ const getTermsStatusController = async (req, res) => {
|
|||
|
||||
const acceptTermsController = async (req, res) => {
|
||||
try {
|
||||
const user = await db.updateUser(req.user.id, { termsAccepted: true });
|
||||
const user = await db.acceptTerms(req.user.id);
|
||||
if (!user) {
|
||||
return res.status(404).json({ message: 'User not found' });
|
||||
}
|
||||
res.status(200).json({ message: 'Terms accepted successfully' });
|
||||
res.status(200).json({
|
||||
message: 'Terms accepted successfully',
|
||||
termsAcceptedAt: user.termsAcceptedAt,
|
||||
});
|
||||
} catch (error) {
|
||||
logger.error('Error accepting terms:', error);
|
||||
res.status(500).json({ message: 'Error accepting terms' });
|
||||
|
|
|
|||
|
|
@ -40,6 +40,7 @@ jest.mock('~/models', () => {
|
|||
deleteConvos: jest.fn().mockResolvedValue(undefined),
|
||||
deleteFiles: jest.fn().mockResolvedValue(undefined),
|
||||
updateUser: jest.fn(),
|
||||
acceptTerms: jest.fn(),
|
||||
getUserById: jest.fn().mockResolvedValue(null),
|
||||
findToken: jest.fn(),
|
||||
getFiles: jest.fn().mockResolvedValue([]),
|
||||
|
|
@ -111,11 +112,12 @@ afterEach(async () => {
|
|||
const {
|
||||
deleteUserController,
|
||||
getUserController,
|
||||
acceptTermsController,
|
||||
resendVerificationController,
|
||||
verifyEmailController,
|
||||
} = require('./UserController');
|
||||
const { Group } = require('~/db/models');
|
||||
const { deleteConvos } = require('~/models');
|
||||
const { deleteConvos, acceptTerms } = require('~/models');
|
||||
const { verifyEmail, resendVerificationEmail } = require('~/server/services/AuthService');
|
||||
|
||||
describe('verifyEmailController', () => {
|
||||
|
|
@ -256,6 +258,50 @@ describe('getUserController', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('acceptTermsController', () => {
|
||||
const mockRes = {
|
||||
status: jest.fn().mockReturnThis(),
|
||||
json: jest.fn().mockReturnThis(),
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
});
|
||||
|
||||
it('returns 404 when the user does not exist', async () => {
|
||||
acceptTerms.mockResolvedValueOnce(null);
|
||||
|
||||
await acceptTermsController({ user: { id: 'missing-user' } }, mockRes);
|
||||
|
||||
expect(acceptTerms).toHaveBeenCalledWith('missing-user');
|
||||
expect(mockRes.status).toHaveBeenCalledWith(404);
|
||||
expect(mockRes.json).toHaveBeenCalledWith({ message: 'User not found' });
|
||||
});
|
||||
|
||||
it('returns the recorded acceptance timestamp on success', async () => {
|
||||
const acceptedAt = new Date('2026-06-14T10:00:00.000Z');
|
||||
acceptTerms.mockResolvedValueOnce({ termsAccepted: true, termsAcceptedAt: acceptedAt });
|
||||
|
||||
await acceptTermsController({ user: { id: 'user-id' } }, mockRes);
|
||||
|
||||
expect(acceptTerms).toHaveBeenCalledWith('user-id');
|
||||
expect(mockRes.status).toHaveBeenCalledWith(200);
|
||||
expect(mockRes.json).toHaveBeenCalledWith({
|
||||
message: 'Terms accepted successfully',
|
||||
termsAcceptedAt: acceptedAt,
|
||||
});
|
||||
});
|
||||
|
||||
it('returns 500 when the update throws', async () => {
|
||||
acceptTerms.mockRejectedValueOnce(new Error('db down'));
|
||||
|
||||
await acceptTermsController({ user: { id: 'user-id' } }, mockRes);
|
||||
|
||||
expect(mockRes.status).toHaveBeenCalledWith(500);
|
||||
expect(mockRes.json).toHaveBeenCalledWith({ message: 'Error accepting terms' });
|
||||
});
|
||||
});
|
||||
|
||||
describe('deleteUserController', () => {
|
||||
const mockRes = {
|
||||
status: jest.fn().mockReturnThis(),
|
||||
|
|
|
|||
|
|
@ -1126,6 +1126,7 @@ export const useAcceptTermsMutation = (
|
|||
onSuccess: (data, variables, context) => {
|
||||
queryClient.setQueryData<t.TUserTermsResponse>([QueryKeys.userTerms], {
|
||||
termsAccepted: true,
|
||||
termsAcceptedAt: data.termsAcceptedAt,
|
||||
});
|
||||
options?.onSuccess?.(data, variables, context);
|
||||
},
|
||||
|
|
|
|||
|
|
@ -7,7 +7,10 @@ const connect = require('./connect');
|
|||
const listUsers = async () => {
|
||||
try {
|
||||
await connect();
|
||||
const users = await User.find({}, 'email provider avatar username name createdAt');
|
||||
const users = await User.find(
|
||||
{},
|
||||
'email provider avatar username name createdAt termsAccepted termsAcceptedAt',
|
||||
);
|
||||
|
||||
console.log('\nUser List:');
|
||||
console.log('----------------------------------------');
|
||||
|
|
@ -18,6 +21,10 @@ const listUsers = async () => {
|
|||
console.log(`Name: ${user.name || 'N/A'}`);
|
||||
console.log(`Provider: ${user.provider || 'email'}`);
|
||||
console.log(`Created: ${user.createdAt}`);
|
||||
console.log(`Terms Accepted: ${user.termsAccepted ? 'Yes' : 'No'}`);
|
||||
console.log(
|
||||
`Terms Accepted At: ${user.termsAcceptedAt ? user.termsAcceptedAt.toISOString() : 'N/A'}`,
|
||||
);
|
||||
console.log('----------------------------------------');
|
||||
});
|
||||
|
||||
|
|
|
|||
136
config/migrate-terms-timestamp.js
Normal file
136
config/migrate-terms-timestamp.js
Normal file
|
|
@ -0,0 +1,136 @@
|
|||
const path = require('path');
|
||||
const mongoose = require('mongoose');
|
||||
const { runAsSystem } = require('@librechat/data-schemas');
|
||||
const { User } = require('@librechat/data-schemas').createModels(mongoose);
|
||||
const { countUsers } = require('@librechat/data-schemas').createMethods(mongoose);
|
||||
require('module-alias')({ base: path.resolve(__dirname, '..', 'api') });
|
||||
const { askQuestion, silentExit } = require('./helpers');
|
||||
const connect = require('./connect');
|
||||
|
||||
/**
|
||||
* Migration script for Terms Acceptance Timestamp Tracking
|
||||
*
|
||||
* This script migrates existing users who have termsAccepted: true but no termsAcceptedAt timestamp.
|
||||
* For these users, it sets termsAcceptedAt to their account creation date (createdAt) as a fallback.
|
||||
*
|
||||
* Usage: npm run migrate:terms-timestamp
|
||||
*/
|
||||
(async () => {
|
||||
await connect();
|
||||
|
||||
console.purple('--------------------------');
|
||||
console.purple('Migrate Terms Acceptance Timestamps');
|
||||
console.purple('--------------------------');
|
||||
|
||||
// Count users that need migration. This script spans every tenant, so run
|
||||
// it under system context or the tenant isolation plugin throws under
|
||||
// TENANT_ISOLATION_STRICT=true and scopes to a non-existent tenant otherwise.
|
||||
const usersToMigrate = await runAsSystem(() =>
|
||||
countUsers({
|
||||
termsAccepted: true,
|
||||
$or: [{ termsAcceptedAt: null }, { termsAcceptedAt: { $exists: false } }],
|
||||
}),
|
||||
);
|
||||
|
||||
if (usersToMigrate === 0) {
|
||||
console.green(
|
||||
'No users need migration. All users with termsAccepted: true already have a termsAcceptedAt timestamp.',
|
||||
);
|
||||
silentExit(0);
|
||||
}
|
||||
|
||||
console.yellow(
|
||||
`Found ${usersToMigrate} user(s) with termsAccepted: true but no termsAcceptedAt timestamp.`,
|
||||
);
|
||||
console.yellow(
|
||||
'These users will have their termsAcceptedAt set to their account creation date (createdAt).',
|
||||
);
|
||||
|
||||
const confirm = await askQuestion('Are you sure you want to proceed? (y/n): ');
|
||||
|
||||
if (confirm.toLowerCase() !== 'y') {
|
||||
console.yellow('Operation cancelled.');
|
||||
silentExit(0);
|
||||
}
|
||||
|
||||
try {
|
||||
// Scan and update across every tenant under system context, matching the
|
||||
// other cross-tenant migrations, so the tenant isolation plugin does not
|
||||
// throw or scope queries to a non-existent tenant.
|
||||
await runAsSystem(async () => {
|
||||
const cursor = User.find({
|
||||
termsAccepted: true,
|
||||
$or: [{ termsAcceptedAt: null }, { termsAcceptedAt: { $exists: false } }],
|
||||
}).cursor();
|
||||
|
||||
let migratedCount = 0;
|
||||
let skippedCount = 0;
|
||||
let errorCount = 0;
|
||||
|
||||
for await (const user of cursor) {
|
||||
try {
|
||||
// Use createdAt as fallback for termsAcceptedAt
|
||||
const termsAcceptedAt = user.createdAt || new Date();
|
||||
if (!user.createdAt) {
|
||||
console.yellow(
|
||||
`Warning: User ${user._id} has no createdAt, using current date for termsAcceptedAt`,
|
||||
);
|
||||
}
|
||||
// Only backfill users who are still accepted and have no timestamp.
|
||||
// If they accept through the API or get reset between the cursor read
|
||||
// and this write, the filter no longer matches and their state is kept.
|
||||
const result = await User.updateOne(
|
||||
{
|
||||
_id: user._id,
|
||||
termsAccepted: true,
|
||||
$or: [{ termsAcceptedAt: null }, { termsAcceptedAt: { $exists: false } }],
|
||||
},
|
||||
{ $set: { termsAcceptedAt } },
|
||||
);
|
||||
|
||||
if (result.modifiedCount > 0) {
|
||||
migratedCount++;
|
||||
if (migratedCount % 100 === 0) {
|
||||
console.yellow(`Migrated ${migratedCount} users...`);
|
||||
}
|
||||
} else {
|
||||
skippedCount++;
|
||||
}
|
||||
} catch (error) {
|
||||
console.red(`Error migrating user ${user._id}: ${error.message}`);
|
||||
errorCount++;
|
||||
}
|
||||
}
|
||||
|
||||
console.green(`Migration complete!`);
|
||||
console.green(`Successfully migrated: ${migratedCount} user(s)`);
|
||||
if (skippedCount > 0) {
|
||||
console.yellow(
|
||||
`Skipped ${skippedCount} user(s) whose terms state changed during migration.`,
|
||||
);
|
||||
}
|
||||
if (errorCount > 0) {
|
||||
console.red(`Errors encountered: ${errorCount}`);
|
||||
silentExit(1);
|
||||
}
|
||||
});
|
||||
} catch (error) {
|
||||
console.red('Error during migration:', error);
|
||||
silentExit(1);
|
||||
}
|
||||
|
||||
silentExit(0);
|
||||
})();
|
||||
|
||||
process.on('uncaughtException', (err) => {
|
||||
if (!err.message.includes('fetch failed')) {
|
||||
console.error('There was an uncaught error:');
|
||||
console.error(err);
|
||||
}
|
||||
|
||||
if (err.message.includes('fetch failed')) {
|
||||
return;
|
||||
} else {
|
||||
process.exit(1);
|
||||
}
|
||||
});
|
||||
|
|
@ -21,7 +21,10 @@ const connect = require('./connect');
|
|||
}
|
||||
|
||||
try {
|
||||
const result = await User.updateMany({}, { $set: { termsAccepted: false } });
|
||||
const result = await User.updateMany(
|
||||
{},
|
||||
{ $set: { termsAccepted: false, termsAcceptedAt: null } },
|
||||
);
|
||||
console.green(`Updated ${result.modifiedCount} user(s).`);
|
||||
} catch (error) {
|
||||
console.red('Error resetting terms acceptance:', error);
|
||||
|
|
|
|||
|
|
@ -96,6 +96,7 @@
|
|||
"b:balance": "bun config/add-balance.js",
|
||||
"b:list-balances": "bun config/list-balances.js",
|
||||
"reset-terms": "node config/reset-terms.js",
|
||||
"migrate:terms-timestamp": "node config/migrate-terms-timestamp.js",
|
||||
"flush-cache": "node config/flush-cache.js",
|
||||
"migrate:agent-permissions:dry-run": "node config/migrate-agent-permissions.js --dry-run",
|
||||
"migrate:agent-permissions": "node config/migrate-agent-permissions.js",
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ const ALLOWED_USER_FIELDS = [
|
|||
'emailVerified',
|
||||
'twoFactorEnabled',
|
||||
'termsAccepted',
|
||||
'termsAcceptedAt',
|
||||
] as const;
|
||||
|
||||
type AllowedUserField = (typeof ALLOWED_USER_FIELDS)[number];
|
||||
|
|
|
|||
|
|
@ -740,10 +740,12 @@ export type TCustomConfigSpeechResponse = { [key: string]: string };
|
|||
|
||||
export type TUserTermsResponse = {
|
||||
termsAccepted: boolean;
|
||||
termsAcceptedAt: Date | string | null;
|
||||
};
|
||||
|
||||
export type TAcceptTermsResponse = {
|
||||
success: boolean;
|
||||
message: string;
|
||||
termsAcceptedAt: Date | string;
|
||||
};
|
||||
|
||||
export type TBannerResponse = TBanner | null;
|
||||
|
|
|
|||
|
|
@ -1,9 +1,9 @@
|
|||
import mongoose from 'mongoose';
|
||||
import { MongoMemoryServer } from 'mongodb-memory-server';
|
||||
import type * as t from '~/types';
|
||||
import balanceSchema from '~/schema/balance';
|
||||
import { createUserMethods } from './user';
|
||||
import userSchema from '~/schema/user';
|
||||
import balanceSchema from '~/schema/balance';
|
||||
|
||||
/** Mocking crypto for generateToken */
|
||||
jest.mock('~/crypto', () => ({
|
||||
|
|
@ -369,6 +369,63 @@ describe('User Methods - Database Tests', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('acceptTerms', () => {
|
||||
test('sets termsAccepted and stamps termsAcceptedAt on first acceptance', async () => {
|
||||
const user = await User.create({
|
||||
name: 'Terms User',
|
||||
email: 'terms@example.com',
|
||||
provider: 'local',
|
||||
});
|
||||
|
||||
const before = Date.now();
|
||||
const updated = await methods.acceptTerms(user._id?.toString() ?? '');
|
||||
const after = Date.now();
|
||||
|
||||
expect(updated?.termsAccepted).toBe(true);
|
||||
expect(updated?.termsAcceptedAt).toBeInstanceOf(Date);
|
||||
const stamped = (updated?.termsAcceptedAt as Date).getTime();
|
||||
expect(stamped).toBeGreaterThanOrEqual(before - 1000);
|
||||
expect(stamped).toBeLessThanOrEqual(after + 1000);
|
||||
});
|
||||
|
||||
test('preserves the original termsAcceptedAt on repeat acceptance', async () => {
|
||||
const originalAcceptedAt = new Date('2026-01-01T00:00:00.000Z');
|
||||
const user = await User.create({
|
||||
name: 'Repeat User',
|
||||
email: 'repeat@example.com',
|
||||
provider: 'local',
|
||||
termsAccepted: true,
|
||||
termsAcceptedAt: originalAcceptedAt,
|
||||
});
|
||||
|
||||
const updated = await methods.acceptTerms(user._id?.toString() ?? '');
|
||||
|
||||
expect(updated?.termsAccepted).toBe(true);
|
||||
expect((updated?.termsAcceptedAt as Date).getTime()).toBe(originalAcceptedAt.getTime());
|
||||
});
|
||||
|
||||
test('backfills termsAcceptedAt for a legacy accepted user without a timestamp', async () => {
|
||||
const user = await User.create({
|
||||
name: 'Legacy User',
|
||||
email: 'legacy@example.com',
|
||||
provider: 'local',
|
||||
termsAccepted: true,
|
||||
});
|
||||
|
||||
const updated = await methods.acceptTerms(user._id?.toString() ?? '');
|
||||
|
||||
expect(updated?.termsAccepted).toBe(true);
|
||||
expect(updated?.termsAcceptedAt).toBeInstanceOf(Date);
|
||||
});
|
||||
|
||||
test('returns null for non-existent user', async () => {
|
||||
const fakeId = new mongoose.Types.ObjectId();
|
||||
const result = await methods.acceptTerms(fakeId.toString());
|
||||
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('deleteUserById', () => {
|
||||
test('should delete user by ID', async () => {
|
||||
const user = await User.create({
|
||||
|
|
|
|||
|
|
@ -26,6 +26,7 @@ export function createUserMethods(mongoose: typeof import('mongoose')): {
|
|||
returnUser?: boolean,
|
||||
) => Promise<mongoose.Types.ObjectId | Partial<IUser>>;
|
||||
updateUser: (userId: string, updateData: Partial<IUser>) => Promise<IUser | null>;
|
||||
acceptTerms: (userId: string) => Promise<IUser | null>;
|
||||
searchUsers: ({
|
||||
searchPattern,
|
||||
limit,
|
||||
|
|
@ -253,6 +254,28 @@ export function createUserMethods(mongoose: typeof import('mongoose')): {
|
|||
}).lean<IUser>();
|
||||
}
|
||||
|
||||
/**
|
||||
* Atomically records terms acceptance for a user.
|
||||
* Sets termsAccepted and, only when no timestamp is already stored, stamps
|
||||
* termsAcceptedAt with the server time so the first acceptance within a terms
|
||||
* cycle is preserved across concurrent or repeated requests.
|
||||
*/
|
||||
async function acceptTerms(userId: string): Promise<IUser | null> {
|
||||
const User = mongoose.models.User;
|
||||
return await User.findByIdAndUpdate(
|
||||
userId,
|
||||
[
|
||||
{
|
||||
$set: {
|
||||
termsAccepted: true,
|
||||
termsAcceptedAt: { $ifNull: ['$termsAcceptedAt', '$$NOW'] },
|
||||
},
|
||||
},
|
||||
],
|
||||
{ new: true, runValidators: true },
|
||||
).lean<IUser>();
|
||||
}
|
||||
|
||||
/**
|
||||
* Retrieve a user by ID and convert the found user document to a plain object.
|
||||
*/
|
||||
|
|
@ -511,6 +534,7 @@ export function createUserMethods(mongoose: typeof import('mongoose')): {
|
|||
countUsers,
|
||||
createUser,
|
||||
updateUser,
|
||||
acceptTerms,
|
||||
searchUsers,
|
||||
getUserById,
|
||||
generateToken,
|
||||
|
|
|
|||
|
|
@ -127,6 +127,10 @@ const userSchema: Schema<IUser> = new Schema<IUser>(
|
|||
type: Boolean,
|
||||
default: false,
|
||||
},
|
||||
termsAcceptedAt: {
|
||||
type: Date,
|
||||
default: null,
|
||||
},
|
||||
personalization: {
|
||||
type: {
|
||||
memories: {
|
||||
|
|
|
|||
|
|
@ -48,6 +48,7 @@ export interface IUser extends Document {
|
|||
}>;
|
||||
expiresAt?: Date;
|
||||
termsAccepted?: boolean;
|
||||
termsAcceptedAt?: Date | null;
|
||||
personalization?: {
|
||||
memories?: boolean;
|
||||
};
|
||||
|
|
@ -93,6 +94,7 @@ export interface UpdateUserRequest {
|
|||
plugins?: string[];
|
||||
twoFactorEnabled?: boolean;
|
||||
termsAccepted?: boolean;
|
||||
termsAcceptedAt?: Date | null;
|
||||
personalization?: {
|
||||
memories?: boolean;
|
||||
};
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue