feat(core): Fix user access control logic (#29481)

This commit is contained in:
Sandra Zollner 2026-04-29 17:42:09 +02:00 committed by GitHub
parent 3791db782b
commit 484cb2efba
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 6 additions and 27 deletions

View File

@ -17,7 +17,6 @@ import { mock } from 'jest-mock-extended';
import { v4 as uuid } from 'uuid';
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
import { ForbiddenError } from '@/errors/response-errors/forbidden.error';
import { NotFoundError } from '@/errors/response-errors/not-found.error';
import { UrlService } from '@/services/url.service';
import { UserService } from '@/services/user.service';
@ -656,15 +655,12 @@ describe('UserService', () => {
});
describe('assertGetUsersAccess', () => {
it('should allow project admin to list all users', async () => {
it('should allow global member to list all users without project filter', async () => {
const member = Object.assign(new User(), { role: GLOBAL_MEMBER_ROLE });
projectService.getProjectIdsWithScope.mockResolvedValueOnce(['project-1']);
await expect(userService.assertGetUsersAccess(member)).resolves.toBeUndefined();
expect(projectService.getProjectIdsWithScope).toHaveBeenCalledWith(member, [
'project:update',
]);
expect(projectService.getProjectIdsWithScope).not.toHaveBeenCalled();
});
it('should allow non-admin members to list users by projectId', async () => {
@ -678,13 +674,6 @@ describe('UserService', () => {
]);
});
it('should throw ForbiddenError for member without project admin scope', async () => {
const member = Object.assign(new User(), { role: GLOBAL_MEMBER_ROLE });
projectService.getProjectIdsWithScope.mockResolvedValueOnce([]);
await expect(userService.assertGetUsersAccess(member)).rejects.toThrow(ForbiddenError);
});
it('should throw NotFoundError when filtering by unknown projectId', async () => {
const member = Object.assign(new User(), { role: GLOBAL_MEMBER_ROLE });
projectService.getProjectWithScope.mockResolvedValueOnce(null);

View File

@ -23,7 +23,6 @@ import type { IUserSettings } from 'n8n-workflow';
import { UserError } from 'n8n-workflow';
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
import { ForbiddenError } from '@/errors/response-errors/forbidden.error';
import { InternalServerError } from '@/errors/response-errors/internal-server.error';
import { NotFoundError } from '@/errors/response-errors/not-found.error';
import { EventService } from '@/events/event.service';
@ -80,17 +79,6 @@ export class UserService {
}
return;
}
const isInstanceAdmin = ['global:owner', 'global:admin'].includes(user.role.slug);
if (isInstanceAdmin) {
return;
}
const hasProjectUpdateScope =
(await this.projectService.getProjectIdsWithScope(user, ['project:update'])).length > 0;
if (!hasProjectUpdateScope) {
throw new ForbiddenError(
'Listing all users is limited to instance administrators and project admins. Filter by project to list project members.',
);
}
}
async updateSettings(userId: string, newSettings: Partial<IUserSettings>) {

View File

@ -48,11 +48,13 @@ describe('With license unlimited quota:users', () => {
await authOwnerAgent.get('/users').expect(401);
});
test('should forbid global user list for a member API key', async () => {
test('should allow global user list for a member API key with user:list scope', async () => {
const member = await createMemberWithApiKey();
await createUser();
await testServer.publicApiAgentFor(member).get('/users').expect(403);
const response = await testServer.publicApiAgentFor(member).get('/users').expect(200);
expect(response.body.data.length).toBe(2);
});
test('should allow member to list users of a project they belong to', async () => {