mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-05-13 16:07:30 +00:00
🚫 fix: Hide Delete Account Button When ALLOW_ACCOUNT_DELETION Is Disabled (#12568)
* fix: hide Delete Account button when ALLOW_ACCOUNT_DELETION is false * fix: add admin bypass, inline env read, and tests for allowAccountDeletion - Show delete button for admin users even when ALLOW_ACCOUNT_DELETION=false, matching the canDeleteAccount middleware's ACCESS_ADMIN bypass - Move env var read inline in buildSharedPayload() for per-request evaluation - Add 4 frontend tests for Account conditional rendering - Add 3 backend tests for allowAccountDeletion config field * fix: use server-side ACCESS_ADMIN capability check instead of frontend role check - Replace frontend SystemRoles.ADMIN check with server-side hasCapability() in the authenticated config route, matching canDeleteAccount middleware exactly - Admin bypass now evaluates ACCESS_ADMIN capability per-user in GET /api/config, so users with the grant (regardless of role) see the button, and admins without the grant do not - Add 3 authenticated backend tests: without capability, with capability, and skip-when-already-enabled - Simplify frontend to pure config check (no role logic) - Remove redundant jest-dom import; add inline env var comment * test: add missing toHaveBeenCalled assertion in ACCESS_ADMIN test
This commit is contained in:
parent
223065c411
commit
d350c58633
5 changed files with 164 additions and 4 deletions
|
|
@ -9,6 +9,11 @@ jest.mock('~/server/services/Config/ldap', () => ({
|
|||
getLdapConfig: jest.fn(() => null),
|
||||
}));
|
||||
|
||||
const mockHasCapability = jest.fn();
|
||||
jest.mock('~/server/middleware/roles/capabilities', () => ({
|
||||
hasCapability: (...args) => mockHasCapability(...args),
|
||||
}));
|
||||
|
||||
const mockGetTenantId = jest.fn(() => undefined);
|
||||
jest.mock('@librechat/data-schemas', () => ({
|
||||
...jest.requireActual('@librechat/data-schemas'),
|
||||
|
|
@ -76,6 +81,7 @@ afterEach(() => {
|
|||
delete process.env.SAML_ISSUER;
|
||||
delete process.env.SAML_CERT;
|
||||
delete process.env.SAML_SESSION_SECRET;
|
||||
delete process.env.ALLOW_ACCOUNT_DELETION;
|
||||
});
|
||||
|
||||
describe('GET /api/config', () => {
|
||||
|
|
@ -181,6 +187,35 @@ describe('GET /api/config', () => {
|
|||
expect(response.body).toHaveProperty('serverDomain');
|
||||
});
|
||||
|
||||
it('should default allowAccountDeletion to true when env var is unset', async () => {
|
||||
mockGetAppConfig.mockResolvedValue(baseAppConfig);
|
||||
const app = createApp(null);
|
||||
|
||||
const response = await request(app).get('/api/config');
|
||||
|
||||
expect(response.body.allowAccountDeletion).toBe(true);
|
||||
});
|
||||
|
||||
it('should set allowAccountDeletion to false when ALLOW_ACCOUNT_DELETION=false', async () => {
|
||||
process.env.ALLOW_ACCOUNT_DELETION = 'false';
|
||||
mockGetAppConfig.mockResolvedValue(baseAppConfig);
|
||||
const app = createApp(null);
|
||||
|
||||
const response = await request(app).get('/api/config');
|
||||
|
||||
expect(response.body.allowAccountDeletion).toBe(false);
|
||||
});
|
||||
|
||||
it('should set allowAccountDeletion to true when ALLOW_ACCOUNT_DELETION=true', async () => {
|
||||
process.env.ALLOW_ACCOUNT_DELETION = 'true';
|
||||
mockGetAppConfig.mockResolvedValue(baseAppConfig);
|
||||
const app = createApp(null);
|
||||
|
||||
const response = await request(app).get('/api/config');
|
||||
|
||||
expect(response.body.allowAccountDeletion).toBe(true);
|
||||
});
|
||||
|
||||
it('should return 500 when getAppConfig throws', async () => {
|
||||
mockGetAppConfig.mockRejectedValue(new Error('Config service failure'));
|
||||
const app = createApp(null);
|
||||
|
|
@ -277,6 +312,40 @@ describe('GET /api/config', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('should set allowAccountDeletion to false for authenticated users without ACCESS_ADMIN', async () => {
|
||||
process.env.ALLOW_ACCOUNT_DELETION = 'false';
|
||||
mockGetAppConfig.mockResolvedValue(baseAppConfig);
|
||||
mockHasCapability.mockResolvedValue(false);
|
||||
const app = createApp(mockUser);
|
||||
|
||||
const response = await request(app).get('/api/config');
|
||||
|
||||
expect(response.body.allowAccountDeletion).toBe(false);
|
||||
expect(mockHasCapability).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should override allowAccountDeletion to true for users with ACCESS_ADMIN capability', async () => {
|
||||
process.env.ALLOW_ACCOUNT_DELETION = 'false';
|
||||
mockGetAppConfig.mockResolvedValue(baseAppConfig);
|
||||
mockHasCapability.mockResolvedValue(true);
|
||||
const app = createApp(mockUser);
|
||||
|
||||
const response = await request(app).get('/api/config');
|
||||
|
||||
expect(response.body.allowAccountDeletion).toBe(true);
|
||||
expect(mockHasCapability).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should not call hasCapability when allowAccountDeletion is already true', async () => {
|
||||
mockGetAppConfig.mockResolvedValue(baseAppConfig);
|
||||
const app = createApp(mockUser);
|
||||
|
||||
const response = await request(app).get('/api/config');
|
||||
|
||||
expect(response.body.allowAccountDeletion).toBe(true);
|
||||
expect(mockHasCapability).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should return 500 when getAppConfig throws', async () => {
|
||||
mockGetAppConfig.mockRejectedValue(new Error('Config service failure'));
|
||||
const app = createApp(mockUser);
|
||||
|
|
|
|||
|
|
@ -1,7 +1,8 @@
|
|||
const express = require('express');
|
||||
const { isEnabled, getBalanceConfig } = require('@librechat/api');
|
||||
const { defaultSocialLogins } = require('librechat-data-provider');
|
||||
const { logger, getTenantId } = require('@librechat/data-schemas');
|
||||
const { logger, getTenantId, SystemCapabilities } = require('@librechat/data-schemas');
|
||||
const { hasCapability } = require('~/server/middleware/roles/capabilities');
|
||||
const { getLdapConfig } = require('~/server/services/Config/ldap');
|
||||
const { getAppConfig } = require('~/server/services/Config/app');
|
||||
|
||||
|
|
@ -77,6 +78,10 @@ function buildSharedPayload() {
|
|||
publicSharedLinksEnabled,
|
||||
analyticsGtmId: process.env.ANALYTICS_GTM_ID,
|
||||
openidReuseTokens,
|
||||
/** Read inline (not module-level) for per-request evaluation and test isolation */
|
||||
allowAccountDeletion:
|
||||
process.env.ALLOW_ACCOUNT_DELETION === undefined ||
|
||||
isEnabled(process.env.ALLOW_ACCOUNT_DELETION),
|
||||
};
|
||||
|
||||
const minPasswordLength = parseInt(process.env.MIN_PASSWORD_LENGTH, 10);
|
||||
|
|
@ -172,6 +177,23 @@ router.get('/', async function (req, res) {
|
|||
payload.webSearch = webSearch;
|
||||
}
|
||||
|
||||
if (!payload.allowAccountDeletion) {
|
||||
try {
|
||||
const userId = req.user.id ?? req.user._id?.toString();
|
||||
if (userId) {
|
||||
const canDelete = await hasCapability(
|
||||
{ id: userId, role: req.user.role ?? '', tenantId: req.user.tenantId },
|
||||
SystemCapabilities.ACCESS_ADMIN,
|
||||
);
|
||||
if (canDelete) {
|
||||
payload.allowAccountDeletion = true;
|
||||
}
|
||||
}
|
||||
} catch (err) {
|
||||
logger.warn(`[config] ACCESS_ADMIN capability check failed: ${err.message}`);
|
||||
}
|
||||
}
|
||||
|
||||
return res.status(200).send(payload);
|
||||
} catch (err) {
|
||||
logger.error('Error in startup config', err);
|
||||
|
|
|
|||
|
|
@ -0,0 +1,64 @@
|
|||
import React from 'react';
|
||||
import { SystemRoles } from 'librechat-data-provider';
|
||||
import { render, screen } from '@testing-library/react';
|
||||
import type { TUser } from 'librechat-data-provider';
|
||||
import Account from './Account';
|
||||
|
||||
jest.mock('./DisplayUsernameMessages', () => () => <div data-testid="display-username" />);
|
||||
jest.mock('./Avatar', () => () => <div data-testid="avatar" />);
|
||||
jest.mock('./TwoFactorAuthentication', () => () => <div data-testid="two-factor" />);
|
||||
jest.mock('./BackupCodesItem', () => () => <div data-testid="backup-codes" />);
|
||||
jest.mock('./DeleteAccount', () => () => <div data-testid="delete-account" />);
|
||||
|
||||
const mockUseAuthContext = jest.fn();
|
||||
const mockUseGetStartupConfig = jest.fn();
|
||||
|
||||
jest.mock('~/hooks', () => ({
|
||||
useAuthContext: () => mockUseAuthContext(),
|
||||
}));
|
||||
|
||||
jest.mock('~/data-provider', () => ({
|
||||
useGetStartupConfig: () => mockUseGetStartupConfig(),
|
||||
}));
|
||||
|
||||
const baseUser: TUser = {
|
||||
id: 'user-123',
|
||||
username: 'testuser',
|
||||
email: 'test@example.com',
|
||||
name: 'Test User',
|
||||
avatar: '',
|
||||
role: SystemRoles.USER,
|
||||
provider: 'local',
|
||||
createdAt: '2023-01-01T00:00:00.000Z',
|
||||
updatedAt: '2023-01-01T00:00:00.000Z',
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
mockUseAuthContext.mockReturnValue({ user: baseUser });
|
||||
mockUseGetStartupConfig.mockReturnValue({ data: { allowAccountDeletion: true } });
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
jest.resetAllMocks();
|
||||
});
|
||||
|
||||
describe('Account', () => {
|
||||
describe('DeleteAccount visibility', () => {
|
||||
it('renders DeleteAccount when allowAccountDeletion is true', () => {
|
||||
render(<Account />);
|
||||
expect(screen.getByTestId('delete-account')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('hides DeleteAccount when allowAccountDeletion is false', () => {
|
||||
mockUseGetStartupConfig.mockReturnValue({ data: { allowAccountDeletion: false } });
|
||||
render(<Account />);
|
||||
expect(screen.queryByTestId('delete-account')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('shows DeleteAccount when startup config is still loading', () => {
|
||||
mockUseGetStartupConfig.mockReturnValue({ data: undefined });
|
||||
render(<Account />);
|
||||
expect(screen.getByTestId('delete-account')).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -4,10 +4,12 @@ import DeleteAccount from './DeleteAccount';
|
|||
import Avatar from './Avatar';
|
||||
import EnableTwoFactorItem from './TwoFactorAuthentication';
|
||||
import BackupCodesItem from './BackupCodesItem';
|
||||
import { useGetStartupConfig } from '~/data-provider';
|
||||
import { useAuthContext } from '~/hooks';
|
||||
|
||||
function Account() {
|
||||
const { user } = useAuthContext();
|
||||
const { data: startupConfig } = useGetStartupConfig();
|
||||
|
||||
return (
|
||||
<div className="flex flex-col gap-3 p-1 text-sm text-text-primary">
|
||||
|
|
@ -29,9 +31,11 @@ function Account() {
|
|||
)}
|
||||
</>
|
||||
)}
|
||||
<div className="pb-3">
|
||||
<DeleteAccount />
|
||||
</div>
|
||||
{startupConfig?.allowAccountDeletion !== false && (
|
||||
<div className="pb-3">
|
||||
<DeleteAccount />
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -859,6 +859,7 @@ export type TStartupConfig = {
|
|||
sharePointPickerGraphScope?: string;
|
||||
sharePointPickerSharePointScope?: string;
|
||||
openidReuseTokens?: boolean;
|
||||
allowAccountDeletion: boolean;
|
||||
minPasswordLength?: number;
|
||||
webSearch?: {
|
||||
searchProvider?: SearchProviders;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue