🗂️ fix: Allow Empty-Overrides Scope Creation in Admin Config (#12492)

* fix: Allow empty-overrides scope creation when priority is provided

The upsertConfigOverrides handler short-circuited when overrides was
empty, returning a plain message instead of creating the config document.
This broke the admin panel's "create blank scope" flow which sends
`{ overrides: {}, priority: N }` — the missing `config` property in the
response caused an `_id` error on the client.

The early return now only triggers when both overrides are empty and no
priority is provided. Per-section permission checks are scoped to cases
where override sections are actually present.

* test: Add tests for empty-overrides scope creation with priority

* test: Address review nits for empty-overrides scope tests

- Add res.statusCode/res.body assertions to capability-check test
- Add 403/401 tests for empty overrides + priority path
- Use mockResolvedValue(null) for consistency on bare jest.fn()
- Remove narrating comment; fold intent into test name
This commit is contained in:
Danny Avila 2026-03-31 21:46:48 -04:00 committed by GitHub
parent 2451bf54cf
commit f8405e731b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 98 additions and 9 deletions

View file

@ -535,6 +535,93 @@ describe('createAdminConfigHandlers', () => {
});
});
describe('upsertConfigOverrides — empty-overrides scope creation', () => {
it('creates config document when overrides is empty but priority is provided', async () => {
const upsertConfig = jest.fn().mockResolvedValue({
_id: 'c1',
principalType: 'role',
principalId: 'admin',
overrides: {},
configVersion: 1,
});
const { handlers } = createHandlers({ upsertConfig });
const req = mockReq({
params: { principalType: 'role', principalId: 'admin' },
body: { overrides: {}, priority: 5 },
});
const res = mockRes();
await handlers.upsertConfigOverrides(req, res);
expect(res.statusCode).toBe(201);
expect(res.body).toHaveProperty('config');
expect(res.body?.config).toHaveProperty('_id', 'c1');
expect(upsertConfig).toHaveBeenCalledWith('role', 'admin', expect.anything(), {}, 5);
});
it('returns no-op message when overrides is empty and no priority is provided', async () => {
const upsertConfig = jest.fn().mockResolvedValue(null);
const { handlers } = createHandlers({ upsertConfig });
const req = mockReq({
params: { principalType: 'role', principalId: 'admin' },
body: { overrides: {} },
});
const res = mockRes();
await handlers.upsertConfigOverrides(req, res);
expect(res.statusCode).toBe(200);
expect(res.body).toHaveProperty('message', 'No actionable override sections provided');
expect(upsertConfig).not.toHaveBeenCalled();
});
it('calls general manage check exactly once when overrides is empty with priority', async () => {
const hasConfigCapability = jest.fn().mockResolvedValue(true);
const { handlers } = createHandlers({ hasConfigCapability });
const req = mockReq({
params: { principalType: 'role', principalId: 'admin' },
body: { overrides: {}, priority: 3 },
});
const res = mockRes();
await handlers.upsertConfigOverrides(req, res);
expect(hasConfigCapability).toHaveBeenCalledTimes(1);
expect(hasConfigCapability).toHaveBeenCalledWith(expect.anything(), null, 'manage');
expect(res.statusCode).toBe(201);
expect(res.body).toHaveProperty('config');
});
it('returns 403 for empty overrides with priority when user lacks MANAGE_CONFIGS', async () => {
const { handlers } = createHandlers({
hasConfigCapability: jest.fn().mockResolvedValue(false),
});
const req = mockReq({
params: { principalType: 'role', principalId: 'admin' },
body: { overrides: {}, priority: 5 },
});
const res = mockRes();
await handlers.upsertConfigOverrides(req, res);
expect(res.statusCode).toBe(403);
});
it('returns 401 for empty overrides with priority when unauthenticated', async () => {
const { handlers } = createHandlers();
const req = mockReq({
params: { principalType: 'role', principalId: 'admin' },
body: { overrides: {}, priority: 5 },
user: undefined,
});
const res = mockRes();
await handlers.upsertConfigOverrides(req, res);
expect(res.statusCode).toBe(401);
});
});
// ── Invariant tests: rules that must hold across ALL handlers ──────
const MUTATION_HANDLERS: Array<{

View file

@ -333,18 +333,20 @@ export function createAdminConfigHandlers(deps: AdminConfigDeps) {
const overrideSections = Object.keys(filteredOverrides);
if (overrideSections.length === 0) {
if (overrideSections.length === 0 && priority == null) {
return res.status(200).json({ message: 'No actionable override sections provided' });
}
const allowed = await Promise.all(
overrideSections.map((s) => hasConfigCapability(user, s as ConfigSection, 'manage')),
);
const denied = overrideSections.find((_, i) => !allowed[i]);
if (denied) {
return res.status(403).json({
error: `Insufficient permissions for config section: ${denied}`,
});
if (overrideSections.length > 0) {
const allowed = await Promise.all(
overrideSections.map((s) => hasConfigCapability(user, s as ConfigSection, 'manage')),
);
const denied = overrideSections.find((_, i) => !allowed[i]);
if (denied) {
return res.status(403).json({
error: `Insufficient permissions for config section: ${denied}`,
});
}
}
const config = await upsertConfig(