mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-06-09 17:31:19 +00:00
🤫 chore: Quiet Repetitive Log Noise from Balance, CloudFront, and Capability Paths (#13461)
* chore: reduce auth and balance operational noise * chore: tighten balance and capability noise handling * chore: avoid balance 404s when disabled * chore: use response locals for balance handoff
This commit is contained in:
parent
67398e0a69
commit
e0c346c0a4
7 changed files with 139 additions and 20 deletions
|
|
@ -1,7 +1,13 @@
|
|||
const { findBalanceByUser } = require('~/models');
|
||||
|
||||
async function balanceController(req, res) {
|
||||
const balanceData = await findBalanceByUser(req.user.id);
|
||||
const balanceLocals = res.locals || {};
|
||||
|
||||
if (balanceLocals.balanceConfigEnabled === false) {
|
||||
return res.sendStatus(204);
|
||||
}
|
||||
|
||||
const balanceData = balanceLocals.balanceData ?? (await findBalanceByUser(req.user.id));
|
||||
|
||||
if (!balanceData) {
|
||||
return res.status(404).json({ error: 'Balance not found' });
|
||||
|
|
|
|||
72
api/server/controllers/Balance.spec.js
Normal file
72
api/server/controllers/Balance.spec.js
Normal file
|
|
@ -0,0 +1,72 @@
|
|||
jest.mock('~/models', () => ({
|
||||
findBalanceByUser: jest.fn(),
|
||||
}));
|
||||
|
||||
const { findBalanceByUser } = require('~/models');
|
||||
const balanceController = require('./Balance');
|
||||
|
||||
describe('balanceController', () => {
|
||||
const createResponse = () => ({
|
||||
locals: {},
|
||||
status: jest.fn().mockReturnThis(),
|
||||
json: jest.fn().mockReturnThis(),
|
||||
sendStatus: jest.fn().mockReturnThis(),
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
});
|
||||
|
||||
it('returns no content without reading balance when balance config is disabled', async () => {
|
||||
const req = {
|
||||
user: { id: 'user-1' },
|
||||
};
|
||||
const res = createResponse();
|
||||
res.locals.balanceConfigEnabled = false;
|
||||
|
||||
await balanceController(req, res);
|
||||
|
||||
expect(findBalanceByUser).not.toHaveBeenCalled();
|
||||
expect(res.sendStatus).toHaveBeenCalledWith(204);
|
||||
expect(res.status).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('uses balance data attached by middleware without a second read', async () => {
|
||||
const req = {
|
||||
user: { id: 'user-1' },
|
||||
};
|
||||
const res = createResponse();
|
||||
res.locals.balanceConfigEnabled = true;
|
||||
res.locals.balanceData = {
|
||||
_id: 'balance-1',
|
||||
user: 'user-1',
|
||||
tokenCredits: 100,
|
||||
autoRefillEnabled: false,
|
||||
};
|
||||
|
||||
await balanceController(req, res);
|
||||
|
||||
expect(findBalanceByUser).not.toHaveBeenCalled();
|
||||
expect(res.status).toHaveBeenCalledWith(200);
|
||||
expect(res.json).toHaveBeenCalledWith({
|
||||
user: 'user-1',
|
||||
tokenCredits: 100,
|
||||
autoRefillEnabled: false,
|
||||
});
|
||||
});
|
||||
|
||||
it('returns not found when balance is enabled and no record exists', async () => {
|
||||
findBalanceByUser.mockResolvedValue(null);
|
||||
const req = {
|
||||
user: { id: 'user-1' },
|
||||
};
|
||||
const res = createResponse();
|
||||
res.locals.balanceConfigEnabled = true;
|
||||
|
||||
await balanceController(req, res);
|
||||
|
||||
expect(findBalanceByUser).toHaveBeenCalledWith('user-1');
|
||||
expect(res.status).toHaveBeenCalledWith(404);
|
||||
expect(res.json).toHaveBeenCalledWith({ error: 'Balance not found' });
|
||||
});
|
||||
});
|
||||
|
|
@ -1,8 +1,17 @@
|
|||
const express = require('express');
|
||||
const { createSetBalanceConfig } = require('@librechat/api');
|
||||
const router = express.Router();
|
||||
const controller = require('../controllers/Balance');
|
||||
const { requireJwtAuth } = require('../middleware/');
|
||||
const { findBalanceByUser, upsertBalanceFields } = require('~/models');
|
||||
const { getAppConfig } = require('~/server/services/Config');
|
||||
|
||||
router.get('/', requireJwtAuth, controller);
|
||||
const setBalanceConfig = createSetBalanceConfig({
|
||||
getAppConfig,
|
||||
findBalanceByUser,
|
||||
upsertBalanceFields,
|
||||
});
|
||||
|
||||
router.get('/', requireJwtAuth, setBalanceConfig, controller);
|
||||
|
||||
module.exports = router;
|
||||
|
|
|
|||
|
|
@ -520,10 +520,6 @@ export function setCloudFrontCookies(
|
|||
path: '/',
|
||||
});
|
||||
|
||||
logger.debug(
|
||||
`[setCloudFrontCookies] Issued signed CloudFront cookies (paths=${signedCookieSets.length}, expiresInSec=${cookieExpiry}).`,
|
||||
);
|
||||
|
||||
return true;
|
||||
} catch (error) {
|
||||
logger.error('[setCloudFrontCookies] Failed to generate signed cookies:', error);
|
||||
|
|
@ -571,12 +567,6 @@ export function maybeRefreshCloudFrontAuthCookies(
|
|||
: getScopeRefreshReason(previousScope, effectiveScope, refreshWindowSec);
|
||||
|
||||
if (!refreshReason) {
|
||||
logger.debug('[maybeRefreshCloudFrontAuthCookies] CloudFront auth cookies still fresh', {
|
||||
attempted: false,
|
||||
refreshed: false,
|
||||
reason: 'fresh',
|
||||
refresh_window_sec: refreshWindowSec,
|
||||
});
|
||||
return {
|
||||
enabled: true,
|
||||
attempted: false,
|
||||
|
|
@ -599,10 +589,14 @@ export function maybeRefreshCloudFrontAuthCookies(
|
|||
};
|
||||
|
||||
if (cookiesSet) {
|
||||
logger.debug(
|
||||
'[maybeRefreshCloudFrontAuthCookies] CloudFront auth cookies refreshed',
|
||||
logPayload,
|
||||
);
|
||||
return {
|
||||
enabled: true,
|
||||
attempted: true,
|
||||
refreshed: true,
|
||||
reason: refreshReason,
|
||||
expiresInSec: timing.expiresInSec,
|
||||
refreshAfterSec: timing.refreshAfterSec,
|
||||
};
|
||||
} else {
|
||||
logger.warn(
|
||||
'[maybeRefreshCloudFrontAuthCookies] CloudFront auth cookie refresh failed',
|
||||
|
|
@ -613,8 +607,8 @@ export function maybeRefreshCloudFrontAuthCookies(
|
|||
return {
|
||||
enabled: true,
|
||||
attempted: true,
|
||||
refreshed: cookiesSet,
|
||||
reason: cookiesSet ? refreshReason : 'set_failed',
|
||||
refreshed: false,
|
||||
reason: 'set_failed',
|
||||
expiresInSec: timing.expiresInSec,
|
||||
refreshAfterSec: timing.refreshAfterSec,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -52,6 +52,7 @@ describe('createSetBalanceConfig', () => {
|
|||
});
|
||||
|
||||
const createMockResponse = (): Partial<ServerResponse> => ({
|
||||
locals: {},
|
||||
status: jest.fn().mockReturnThis(),
|
||||
json: jest.fn().mockReturnThis(),
|
||||
});
|
||||
|
|
@ -93,6 +94,8 @@ describe('createSetBalanceConfig', () => {
|
|||
expect(balanceRecord?.refillIntervalUnit).toBe('days');
|
||||
expect(balanceRecord?.refillAmount).toBe(500);
|
||||
expect(balanceRecord?.lastRefill).toBeInstanceOf(Date);
|
||||
expect((res.locals as { balanceConfigEnabled?: boolean }).balanceConfigEnabled).toBe(true);
|
||||
expect((res.locals as { balanceData?: IBalance }).balanceData?.tokenCredits).toBe(1000);
|
||||
});
|
||||
|
||||
test('should skip if balance config is not enabled', async () => {
|
||||
|
|
@ -115,6 +118,7 @@ describe('createSetBalanceConfig', () => {
|
|||
await middleware(req as ServerRequest, res as ServerResponse, mockNext);
|
||||
|
||||
expect(mockNext).toHaveBeenCalled();
|
||||
expect((res.locals as { balanceConfigEnabled?: boolean }).balanceConfigEnabled).toBe(false);
|
||||
|
||||
const balanceRecord = await Balance.findOne({ user: userId });
|
||||
expect(balanceRecord).toBeNull();
|
||||
|
|
@ -495,6 +499,7 @@ describe('createSetBalanceConfig', () => {
|
|||
|
||||
expect(mockNext).toHaveBeenCalled();
|
||||
expect(upsertSpy).not.toHaveBeenCalled();
|
||||
expect((res.locals as { balanceData?: IBalance }).balanceData?.tokenCredits).toBe(1000);
|
||||
});
|
||||
|
||||
test('should set tokenCredits for user with null tokenCredits', async () => {
|
||||
|
|
|
|||
|
|
@ -22,6 +22,11 @@ export interface BalanceMiddlewareOptions {
|
|||
upsertBalanceFields: (userId: string, fields: IBalanceUpdate) => Promise<IBalance | null>;
|
||||
}
|
||||
|
||||
type BalanceLocals = {
|
||||
balanceData?: IBalance | null;
|
||||
balanceConfigEnabled?: boolean;
|
||||
};
|
||||
|
||||
const balanceUpdateLocks = new Map<string, Promise<void>>();
|
||||
|
||||
async function runBalanceUpdate(userId: string, task: () => Promise<void>): Promise<void> {
|
||||
|
|
@ -117,6 +122,7 @@ export function createSetBalanceConfig({
|
|||
) => Promise<void> {
|
||||
return async (req: ServerRequest, res: ServerResponse, next: NextFunction): Promise<void> => {
|
||||
try {
|
||||
const balanceLocals = res.locals as BalanceLocals;
|
||||
const user = req.user as IUser & { _id: string | ObjectId };
|
||||
const appConfig = await getAppConfig({
|
||||
role: user?.role,
|
||||
|
|
@ -124,6 +130,7 @@ export function createSetBalanceConfig({
|
|||
tenantId: user?.tenantId,
|
||||
});
|
||||
const balanceConfig = getBalanceConfig(appConfig);
|
||||
balanceLocals.balanceConfigEnabled = balanceConfig?.enabled === true;
|
||||
if (!balanceConfig?.enabled) {
|
||||
return next();
|
||||
}
|
||||
|
|
@ -140,10 +147,11 @@ export function createSetBalanceConfig({
|
|||
const updateFields = buildUpdateFields(balanceConfig, userBalanceRecord, userId);
|
||||
|
||||
if (Object.keys(updateFields).length === 0) {
|
||||
balanceLocals.balanceData = userBalanceRecord;
|
||||
return;
|
||||
}
|
||||
|
||||
await upsertBalanceFields(userId, updateFields);
|
||||
balanceLocals.balanceData = await upsertBalanceFields(userId, updateFields);
|
||||
});
|
||||
|
||||
next();
|
||||
|
|
|
|||
|
|
@ -35,6 +35,10 @@ interface CapabilityStore {
|
|||
results: Map<string, boolean>;
|
||||
}
|
||||
|
||||
const DENIAL_WARN_INTERVAL_MS = 5 * 60 * 1000;
|
||||
const MAX_DENIAL_WARN_KEYS = 1000;
|
||||
const recentDenialWarnings = new Map<string, number>();
|
||||
|
||||
export type HasCapabilityFn = (
|
||||
user: CapabilityUser,
|
||||
capability: SystemCapability,
|
||||
|
|
@ -75,6 +79,24 @@ export function capabilityContextMiddleware(
|
|||
capabilityStore.run({ principals: new Map(), results: new Map() }, next);
|
||||
}
|
||||
|
||||
function warnDeniedCapabilityOnce(key: string, message: string): void {
|
||||
const now = Date.now();
|
||||
const lastLoggedAt = recentDenialWarnings.get(key);
|
||||
if (lastLoggedAt !== undefined && now - lastLoggedAt < DENIAL_WARN_INTERVAL_MS) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (recentDenialWarnings.size >= MAX_DENIAL_WARN_KEYS) {
|
||||
const oldestKey = recentDenialWarnings.keys().next().value;
|
||||
if (oldestKey !== undefined) {
|
||||
recentDenialWarnings.delete(oldestKey);
|
||||
}
|
||||
}
|
||||
|
||||
recentDenialWarnings.set(key, now);
|
||||
logger.warn(message);
|
||||
}
|
||||
|
||||
/**
|
||||
* Reads principals from the per-request ALS cache without side effects.
|
||||
* Returns `undefined` when called outside a request context or before
|
||||
|
|
@ -191,7 +213,10 @@ export function generateCapabilityCheck(deps: CapabilityDeps): {
|
|||
return;
|
||||
}
|
||||
|
||||
logger.warn(`[requireCapability] Forbidden: user ${id} missing capability '${capability}'`);
|
||||
warnDeniedCapabilityOnce(
|
||||
`missing-capability:${id}:${capability}`,
|
||||
`[requireCapability] Forbidden: user ${id} missing capability '${capability}'`,
|
||||
);
|
||||
res.status(403).json({ message: 'Forbidden' });
|
||||
} catch (err) {
|
||||
logger.error(`[requireCapability] Error checking capability: ${capability}`, err);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue