feat: Replace owner checks with scope checks (no-changelog) (#7846)

Github issue / Community forum post (link here to close automatically):
This commit is contained in:
Val 2023-11-29 14:48:36 +00:00 committed by GitHub
parent d5762a7539
commit 1cb92ffe16
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
26 changed files with 136 additions and 78 deletions

View file

@ -30,7 +30,7 @@ export type CommunityPackageScope = ResourceScope<
'install' | 'uninstall' | 'update' | 'list' | 'manage' 'install' | 'uninstall' | 'update' | 'list' | 'manage'
>; >;
export type CredentialScope = ResourceScope<'credential', DefaultOperations | 'share'>; export type CredentialScope = ResourceScope<'credential', DefaultOperations | 'share'>;
export type ExternalSecretScope = ResourceScope<'externalSecret', 'list'>; export type ExternalSecretScope = ResourceScope<'externalSecret', 'list' | 'use'>;
export type ExternalSecretProviderScope = ResourceScope< export type ExternalSecretProviderScope = ResourceScope<
'externalSecretsProvider', 'externalSecretsProvider',
DefaultOperations | 'sync' DefaultOperations | 'sync'
@ -46,9 +46,9 @@ export type OrchestrationScope = ResourceScope<'orchestration', 'read' | 'list'>
export type SamlScope = ResourceScope<'saml', 'manage'>; export type SamlScope = ResourceScope<'saml', 'manage'>;
export type SourceControlScope = ResourceScope<'sourceControl', 'pull' | 'push' | 'manage'>; export type SourceControlScope = ResourceScope<'sourceControl', 'pull' | 'push' | 'manage'>;
export type TagScope = ResourceScope<'tag'>; export type TagScope = ResourceScope<'tag'>;
export type UserScope = ResourceScope<'user', DefaultOperations | 'resetPassword'>; export type UserScope = ResourceScope<'user', DefaultOperations | 'resetPassword' | 'changeRole'>;
export type VariableScope = ResourceScope<'variable'>; export type VariableScope = ResourceScope<'variable'>;
export type WorkflowScope = ResourceScope<'workflow', DefaultOperations | 'share'>; export type WorkflowScope = ResourceScope<'workflow', DefaultOperations | 'share' | 'execute'>;
export type Scope = export type Scope =
| AuditLogsScope | AuditLogsScope

View file

@ -68,6 +68,7 @@ import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.reposi
import { WorkflowRepository } from '@db/repositories/workflow.repository'; import { WorkflowRepository } from '@db/repositories/workflow.repository';
import { MultiMainSetup } from '@/services/orchestration/main/MultiMainSetup.ee'; import { MultiMainSetup } from '@/services/orchestration/main/MultiMainSetup.ee';
import { ActivationErrorsService } from '@/ActivationErrors.service'; import { ActivationErrorsService } from '@/ActivationErrors.service';
import type { Scope } from '@n8n/permissions';
import { NotFoundError } from './errors/response-errors/not-found.error'; import { NotFoundError } from './errors/response-errors/not-found.error';
const WEBHOOK_PROD_UNREGISTERED_HINT = const WEBHOOK_PROD_UNREGISTERED_HINT =
@ -270,8 +271,8 @@ export class ActiveWorkflowRunner implements IWebhookManager {
/** /**
* Get the IDs of active workflows from storage. * Get the IDs of active workflows from storage.
*/ */
async allActiveInStorage(user?: User) { async allActiveInStorage(options?: { user: User; scope: Scope | Scope[] }) {
const isFullAccess = !user || user.globalRole.name === 'owner'; const isFullAccess = !options?.user || (await options.user.hasGlobalScope(options.scope));
const activationErrors = await this.activationErrorsService.getAll(); const activationErrors = await this.activationErrorsService.getAll();
@ -286,8 +287,9 @@ export class ActiveWorkflowRunner implements IWebhookManager {
.filter((workflowId) => !activationErrors[workflowId]); .filter((workflowId) => !activationErrors[workflowId]);
} }
const where = whereClause({ const where = await whereClause({
user, user: options.user,
globalScope: 'workflow:list',
entityType: 'workflow', entityType: 'workflow',
}); });

View file

@ -579,7 +579,7 @@ export class CredentialsHelper extends ICredentialsHelper {
credentialType, credentialType,
'internal' as WorkflowExecuteMode, 'internal' as WorkflowExecuteMode,
undefined, undefined,
user.isOwner, await user.hasGlobalScope('externalSecret:use'),
); );
} catch (error) { } catch (error) {
this.logger.debug('Credential test failed', error); this.logger.debug('Credential test failed', error);

View file

@ -448,7 +448,10 @@ export class Server extends AbstractServer {
this.app.get( this.app.get(
`/${this.restEndpoint}/active`, `/${this.restEndpoint}/active`,
ResponseHelper.send(async (req: WorkflowRequest.GetAllActive) => { ResponseHelper.send(async (req: WorkflowRequest.GetAllActive) => {
return this.activeWorkflowRunner.allActiveInStorage(req.user); return this.activeWorkflowRunner.allActiveInStorage({
user: req.user,
scope: 'workflow:list',
});
}), }),
); );
@ -460,8 +463,9 @@ export class Server extends AbstractServer {
const shared = await Container.get(SharedWorkflowRepository).findOne({ const shared = await Container.get(SharedWorkflowRepository).findOne({
relations: ['workflow'], relations: ['workflow'],
where: whereClause({ where: await whereClause({
user: req.user, user: req.user,
globalScope: 'workflow:read',
entityType: 'workflow', entityType: 'workflow',
entityId: workflowId, entityId: workflowId,
}), }),

View file

@ -32,7 +32,7 @@ export class PermissionChecker {
relations: ['globalRole'], relations: ['globalRole'],
}); });
if (user.globalRole.name === 'owner') return; if (await user.hasGlobalScope('workflow:execute')) return;
// allow if all creds used in this workflow are a subset of // allow if all creds used in this workflow are a subset of
// all creds accessible to users who have access to this workflow // all creds accessible to users who have access to this workflow

View file

@ -10,6 +10,7 @@ import { License } from '@/License';
import { getWebhookBaseUrl } from '@/WebhookHelpers'; import { getWebhookBaseUrl } from '@/WebhookHelpers';
import { RoleService } from '@/services/role.service'; import { RoleService } from '@/services/role.service';
import { UserRepository } from '@db/repositories/user.repository'; import { UserRepository } from '@db/repositories/user.repository';
import type { Scope } from '@n8n/permissions';
import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { BadRequestError } from '@/errors/response-errors/bad-request.error';
import { ApplicationError } from 'n8n-workflow'; import { ApplicationError } from 'n8n-workflow';
@ -131,21 +132,22 @@ export function rightDiff<T1, T2>(
* Build a `where` clause for a TypeORM entity search, * Build a `where` clause for a TypeORM entity search,
* checking for member access if the user is not an owner. * checking for member access if the user is not an owner.
*/ */
export function whereClause({ export async function whereClause({
user, user,
entityType, entityType,
globalScope,
entityId = '', entityId = '',
roles = [], roles = [],
}: { }: {
user: User; user: User;
entityType: 'workflow' | 'credentials'; entityType: 'workflow' | 'credentials';
globalScope: Scope;
entityId?: string; entityId?: string;
roles?: string[]; roles?: string[];
}): WhereClause { }): Promise<WhereClause> {
const where: WhereClause = entityId ? { [entityType]: { id: entityId } } : {}; const where: WhereClause = entityId ? { [entityType]: { id: entityId } } : {};
// TODO: Decide if owner access should be restricted if (!(await user.hasGlobalScope(globalScope))) {
if (user.globalRole.name !== 'owner') {
where.user = { id: user.id }; where.user = { id: user.id };
if (roles?.length) { if (roles?.length) {
where.role = { name: In(roles) }; where.role = { name: In(roles) };

View file

@ -415,11 +415,11 @@ export async function replaceInvalidCredentials(workflow: WorkflowEntity): Promi
/** /**
* Get the IDs of the workflows that have been shared with the user. * Get the IDs of the workflows that have been shared with the user.
* Returns all IDs if user is global owner (see `whereClause`) * Returns all IDs if user has the 'workflow:read' scope (see `whereClause`)
*/ */
export async function getSharedWorkflowIds(user: User, roles?: RoleNames[]): Promise<string[]> { export async function getSharedWorkflowIds(user: User, roles?: RoleNames[]): Promise<string[]> {
const where: FindOptionsWhere<SharedWorkflow> = {}; const where: FindOptionsWhere<SharedWorkflow> = {};
if (user.globalRole?.name !== 'owner') { if (!(await user.hasGlobalScope('workflow:read'))) {
where.userId = user.id; where.userId = user.id;
} }
if (roles?.length) { if (roles?.length) {

View file

@ -97,7 +97,10 @@ export class PasswordResetController {
} }
if ( if (
isSamlCurrentAuthenticationMethod() && isSamlCurrentAuthenticationMethod() &&
!(user?.globalRole.name === 'owner' || user?.settings?.allowSSOManualLogin === true) !(
(user && (await user.hasGlobalScope('user:resetPassword'))) === true ||
user?.settings?.allowSSOManualLogin === true
)
) { ) {
this.logger.debug( this.logger.debug(
'Request to send password reset email failed because login is handled by SAML', 'Request to send password reset email failed because login is handled by SAML',

View file

@ -38,7 +38,6 @@ export class UsersController {
static ERROR_MESSAGES = { static ERROR_MESSAGES = {
CHANGE_ROLE: { CHANGE_ROLE: {
NO_MEMBER: 'Member cannot change role for any user',
MISSING_NEW_ROLE_KEY: 'Expected `newRole` to exist', MISSING_NEW_ROLE_KEY: 'Expected `newRole` to exist',
MISSING_NEW_ROLE_VALUE: 'Expected `newRole` to have `name` and `scope`', MISSING_NEW_ROLE_VALUE: 'Expected `newRole` to have `name` and `scope`',
NO_USER: 'Target user not found', NO_USER: 'Target user not found',
@ -326,13 +325,10 @@ export class UsersController {
return { success: true }; return { success: true };
} }
// @TODO: Add scope check `@RequireGlobalScope('user:changeRole')`
// once this has been merged: https://github.com/n8n-io/n8n/pull/7737
@Authorized('any')
@Patch('/:id/role') @Patch('/:id/role')
@RequireGlobalScope('user:changeRole')
async changeRole(req: UserRequest.ChangeRole) { async changeRole(req: UserRequest.ChangeRole) {
const { const {
NO_MEMBER,
MISSING_NEW_ROLE_KEY, MISSING_NEW_ROLE_KEY,
MISSING_NEW_ROLE_VALUE, MISSING_NEW_ROLE_VALUE,
NO_ADMIN_ON_OWNER, NO_ADMIN_ON_OWNER,
@ -342,10 +338,6 @@ export class UsersController {
NO_ADMIN_IF_UNLICENSED, NO_ADMIN_IF_UNLICENSED,
} = UsersController.ERROR_MESSAGES.CHANGE_ROLE; } = UsersController.ERROR_MESSAGES.CHANGE_ROLE;
if (req.user.globalRole.scope === 'global' && req.user.globalRole.name === 'member') {
throw new UnauthorizedError(NO_MEMBER);
}
const { newRole } = req.body; const { newRole } = req.body;
if (!newRole) { if (!newRole) {

View file

@ -37,8 +37,9 @@ export class WorkflowStatisticsController {
const workflowId = req.params.id; const workflowId = req.params.id;
const allowed = await this.sharedWorkflowRepository.exist({ const allowed = await this.sharedWorkflowRepository.exist({
relations: ['workflow'], relations: ['workflow'],
where: whereClause({ where: await whereClause({
user, user,
globalScope: 'workflow:read',
entityType: 'workflow', entityType: 'workflow',
entityId: workflowId, entityId: workflowId,
}), }),

View file

@ -50,7 +50,7 @@ EECredentialsController.get(
const userSharing = credential.shared?.find((shared) => shared.user.id === req.user.id); const userSharing = credential.shared?.find((shared) => shared.user.id === req.user.id);
if (!userSharing && req.user.globalRole.name !== 'owner') { if (!userSharing && !(await req.user.hasGlobalScope('credential:read'))) {
throw new UnauthorizedError('Forbidden.'); throw new UnauthorizedError('Forbidden.');
} }
@ -82,7 +82,10 @@ EECredentialsController.post(
const credentialId = credentials.id; const credentialId = credentials.id;
const { ownsCredential } = await EECredentials.isOwned(req.user, credentialId); const { ownsCredential } = await EECredentials.isOwned(req.user, credentialId);
const sharing = await EECredentials.getSharing(req.user, credentialId); const sharing = await EECredentials.getSharing(req.user, credentialId, {
allowGlobalScope: true,
globalScope: 'credential:read',
});
if (!ownsCredential) { if (!ownsCredential) {
if (!sharing) { if (!sharing) {
throw new UnauthorizedError('Forbidden'); throw new UnauthorizedError('Forbidden');

View file

@ -58,7 +58,12 @@ credentialsController.get(
const { id: credentialId } = req.params; const { id: credentialId } = req.params;
const includeDecryptedData = req.query.includeData === 'true'; const includeDecryptedData = req.query.includeData === 'true';
const sharing = await CredentialsService.getSharing(req.user, credentialId, ['credentials']); const sharing = await CredentialsService.getSharing(
req.user,
credentialId,
{ allowGlobalScope: true, globalScope: 'credential:read' },
['credentials'],
);
if (!sharing) { if (!sharing) {
throw new NotFoundError(`Credential with ID "${credentialId}" could not be found.`); throw new NotFoundError(`Credential with ID "${credentialId}" could not be found.`);
@ -91,7 +96,10 @@ credentialsController.post(
ResponseHelper.send(async (req: CredentialRequest.Test): Promise<INodeCredentialTestResult> => { ResponseHelper.send(async (req: CredentialRequest.Test): Promise<INodeCredentialTestResult> => {
const { credentials } = req.body; const { credentials } = req.body;
const sharing = await CredentialsService.getSharing(req.user, credentials.id); const sharing = await CredentialsService.getSharing(req.user, credentials.id, {
allowGlobalScope: true,
globalScope: 'credential:read',
});
const mergedCredentials = deepCopy(credentials); const mergedCredentials = deepCopy(credentials);
if (mergedCredentials.data && sharing?.credentials) { if (mergedCredentials.data && sharing?.credentials) {
@ -134,7 +142,10 @@ credentialsController.patch(
ResponseHelper.send(async (req: CredentialRequest.Update): Promise<ICredentialsDb> => { ResponseHelper.send(async (req: CredentialRequest.Update): Promise<ICredentialsDb> => {
const { id: credentialId } = req.params; const { id: credentialId } = req.params;
const sharing = await CredentialsService.getSharing(req.user, credentialId); const sharing = await CredentialsService.getSharing(req.user, credentialId, {
allowGlobalScope: true,
globalScope: 'credential:update',
});
if (!sharing) { if (!sharing) {
Container.get(Logger).info( Container.get(Logger).info(
@ -184,7 +195,10 @@ credentialsController.delete(
ResponseHelper.send(async (req: CredentialRequest.Delete) => { ResponseHelper.send(async (req: CredentialRequest.Delete) => {
const { id: credentialId } = req.params; const { id: credentialId } = req.params;
const sharing = await CredentialsService.getSharing(req.user, credentialId); const sharing = await CredentialsService.getSharing(req.user, credentialId, {
allowGlobalScope: true,
globalScope: 'credential:delete',
});
if (!sharing) { if (!sharing) {
Container.get(Logger).info( Container.get(Logger).info(

View file

@ -4,7 +4,7 @@ import { CredentialsEntity } from '@db/entities/CredentialsEntity';
import { SharedCredentials } from '@db/entities/SharedCredentials'; import { SharedCredentials } from '@db/entities/SharedCredentials';
import type { User } from '@db/entities/User'; import type { User } from '@db/entities/User';
import { UserService } from '@/services/user.service'; import { UserService } from '@/services/user.service';
import { CredentialsService } from './credentials.service'; import { CredentialsService, type CredentialsGetSharedOptions } from './credentials.service';
import { RoleService } from '@/services/role.service'; import { RoleService } from '@/services/role.service';
import Container from 'typedi'; import Container from 'typedi';
import { SharedCredentialsRepository } from '@db/repositories/sharedCredentials.repository'; import { SharedCredentialsRepository } from '@db/repositories/sharedCredentials.repository';
@ -14,9 +14,10 @@ export class EECredentialsService extends CredentialsService {
user: User, user: User,
credentialId: string, credentialId: string,
): Promise<{ ownsCredential: boolean; credential?: CredentialsEntity }> { ): Promise<{ ownsCredential: boolean; credential?: CredentialsEntity }> {
const sharing = await this.getSharing(user, credentialId, ['credentials', 'role'], { const sharing = await this.getSharing(user, credentialId, { allowGlobalScope: false }, [
allowGlobalOwner: false, 'credentials',
}); 'role',
]);
if (!sharing || sharing.role.name !== 'owner') return { ownsCredential: false }; if (!sharing || sharing.role.name !== 'owner') return { ownsCredential: false };
@ -31,15 +32,15 @@ export class EECredentialsService extends CredentialsService {
static async getSharing( static async getSharing(
user: User, user: User,
credentialId: string, credentialId: string,
options: CredentialsGetSharedOptions,
relations: string[] = ['credentials'], relations: string[] = ['credentials'],
{ allowGlobalOwner } = { allowGlobalOwner: true },
): Promise<SharedCredentials | null> { ): Promise<SharedCredentials | null> {
const where: FindOptionsWhere<SharedCredentials> = { credentialsId: credentialId }; const where: FindOptionsWhere<SharedCredentials> = { credentialsId: credentialId };
// Omit user from where if the requesting user is the global // Omit user from where if the requesting user has relevant
// owner. This allows the global owner to view and delete // global credential permissions. This allows the user to
// credentials they don't own. // access credentials they don't own.
if (!allowGlobalOwner || user.globalRole.name !== 'owner') { if (!options.allowGlobalScope || !(await user.hasGlobalScope(options.globalScope))) {
where.userId = user.id; where.userId = user.id;
} }

View file

@ -11,6 +11,8 @@ import { Container } from 'typedi';
import type { FindManyOptions, FindOptionsWhere } from 'typeorm'; import type { FindManyOptions, FindOptionsWhere } from 'typeorm';
import { In, Like } from 'typeorm'; import { In, Like } from 'typeorm';
import type { Scope } from '@n8n/permissions';
import * as Db from '@/Db'; import * as Db from '@/Db';
import type { ICredentialsDb } from '@/Interfaces'; import type { ICredentialsDb } from '@/Interfaces';
import { CredentialsHelper, createCredentialsFromCredentialsEntity } from '@/CredentialsHelper'; import { CredentialsHelper, createCredentialsFromCredentialsEntity } from '@/CredentialsHelper';
@ -28,6 +30,10 @@ import { Logger } from '@/Logger';
import { CredentialsRepository } from '@db/repositories/credentials.repository'; import { CredentialsRepository } from '@db/repositories/credentials.repository';
import { SharedCredentialsRepository } from '@db/repositories/sharedCredentials.repository'; import { SharedCredentialsRepository } from '@db/repositories/sharedCredentials.repository';
export type CredentialsGetSharedOptions =
| { allowGlobalScope: true; globalScope: Scope }
| { allowGlobalScope: false };
export class CredentialsService { export class CredentialsService {
static async get( static async get(
where: FindOptionsWhere<ICredentialsDb>, where: FindOptionsWhere<ICredentialsDb>,
@ -86,7 +92,7 @@ export class CredentialsService {
) { ) {
const findManyOptions = this.toFindManyOptions(options.listQueryOptions); const findManyOptions = this.toFindManyOptions(options.listQueryOptions);
const returnAll = user.globalRole.name === 'owner' && !options.onlyOwn; const returnAll = (await user.hasGlobalScope('credential:list')) && !options.onlyOwn;
const isDefaultSelect = !options.listQueryOptions?.select; const isDefaultSelect = !options.listQueryOptions?.select;
if (returnAll) { if (returnAll) {
@ -136,15 +142,15 @@ export class CredentialsService {
static async getSharing( static async getSharing(
user: User, user: User,
credentialId: string, credentialId: string,
options: CredentialsGetSharedOptions,
relations: string[] = ['credentials'], relations: string[] = ['credentials'],
{ allowGlobalOwner } = { allowGlobalOwner: true },
): Promise<SharedCredentials | null> { ): Promise<SharedCredentials | null> {
const where: FindOptionsWhere<SharedCredentials> = { credentialsId: credentialId }; const where: FindOptionsWhere<SharedCredentials> = { credentialsId: credentialId };
// Omit user from where if the requesting user is the global // Omit user from where if the requesting user has relevant
// owner. This allows the global owner to view and delete // global credential permissions. This allows the user to
// credentials they don't own. // access credentials they don't own.
if (!allowGlobalOwner || user.globalRole.name !== 'owner') { if (!options.allowGlobalScope || !(await user.hasGlobalScope(options.globalScope))) {
Object.assign(where, { Object.assign(where, {
userId: user.id, userId: user.id,
role: { name: 'owner' }, role: { name: 'owner' },

View file

@ -273,10 +273,10 @@ export class ExecutionRepository extends Repository<ExecutionEntity> {
filters: IGetExecutionsQueryFilter | undefined, filters: IGetExecutionsQueryFilter | undefined,
accessibleWorkflowIds: string[], accessibleWorkflowIds: string[],
currentlyRunningExecutions: string[], currentlyRunningExecutions: string[],
isOwner: boolean, hasGlobalRead: boolean,
): Promise<{ count: number; estimated: boolean }> { ): Promise<{ count: number; estimated: boolean }> {
const dbType = config.getEnv('database.type'); const dbType = config.getEnv('database.type');
if (dbType !== 'postgresdb' || (filters && Object.keys(filters).length > 0) || !isOwner) { if (dbType !== 'postgresdb' || (filters && Object.keys(filters).length > 0) || !hasGlobalRead) {
const query = this.createQueryBuilder('execution').andWhere( const query = this.createQueryBuilder('execution').andWhere(
'execution.workflowId IN (:...accessibleWorkflowIds)', 'execution.workflowId IN (:...accessibleWorkflowIds)',
{ accessibleWorkflowIds }, { accessibleWorkflowIds },

View file

@ -15,7 +15,7 @@ export class SharedCredentialsRepository extends Repository<SharedCredentials> {
relations: ['credentials'], relations: ['credentials'],
where: { where: {
credentialsId, credentialsId,
...(!user.isOwner ? { userId: user.id } : {}), ...(!(await user.hasGlobalScope('credential:read')) ? { userId: user.id } : {}),
}, },
}); });
if (!sharedCredential) return null; if (!sharedCredential) return null;

View file

@ -155,7 +155,7 @@ export class ExecutionsService {
filter, filter,
sharedWorkflowIds, sharedWorkflowIds,
executingWorkflowIds, executingWorkflowIds,
req.user.globalRole.name === 'owner', await req.user.hasGlobalScope('workflow:list'),
); );
const formattedExecutions = await Container.get(ExecutionRepository).searchExecutions( const formattedExecutions = await Container.get(ExecutionRepository).searchExecutions(

View file

@ -32,6 +32,7 @@ export const ownerPermissions: Scope[] = [
'externalSecretsProvider:list', 'externalSecretsProvider:list',
'externalSecretsProvider:sync', 'externalSecretsProvider:sync',
'externalSecret:list', 'externalSecret:list',
'externalSecret:use',
'ldap:manage', 'ldap:manage',
'ldap:sync', 'ldap:sync',
'logStreaming:manage', 'logStreaming:manage',
@ -52,6 +53,7 @@ export const ownerPermissions: Scope[] = [
'user:delete', 'user:delete',
'user:list', 'user:list',
'user:resetPassword', 'user:resetPassword',
'user:changeRole',
'variable:create', 'variable:create',
'variable:read', 'variable:read',
'variable:update', 'variable:update',
@ -63,6 +65,7 @@ export const ownerPermissions: Scope[] = [
'workflow:delete', 'workflow:delete',
'workflow:list', 'workflow:list',
'workflow:share', 'workflow:share',
'workflow:execute',
]; ];
export const adminPermissions: Scope[] = ownerPermissions.concat(); export const adminPermissions: Scope[] = ownerPermissions.concat();
export const memberPermissions: Scope[] = [ export const memberPermissions: Scope[] = [

View file

@ -21,7 +21,7 @@ export class WorkflowHistoryService {
private async getSharedWorkflow(user: User, workflowId: string): Promise<SharedWorkflow | null> { private async getSharedWorkflow(user: User, workflowId: string): Promise<SharedWorkflow | null> {
return this.sharedWorkflowRepository.findOne({ return this.sharedWorkflowRepository.findOne({
where: { where: {
...(!user.isOwner && { userId: user.id }), ...(!(await user.hasGlobalScope('workflow:read')) && { userId: user.id }),
workflowId, workflowId,
}, },
}); });

View file

@ -211,9 +211,10 @@ workflowsController.get(
const shared = await Container.get(SharedWorkflowRepository).findOne({ const shared = await Container.get(SharedWorkflowRepository).findOne({
relations, relations,
where: whereClause({ where: await whereClause({
user: req.user, user: req.user,
entityType: 'workflow', entityType: 'workflow',
globalScope: 'workflow:read',
entityId: workflowId, entityId: workflowId,
roles: ['owner'], roles: ['owner'],
}), }),

View file

@ -24,9 +24,10 @@ export class EEWorkflowsService extends WorkflowsService {
user: User, user: User,
workflowId: string, workflowId: string,
): Promise<{ ownsWorkflow: boolean; workflow?: WorkflowEntity }> { ): Promise<{ ownsWorkflow: boolean; workflow?: WorkflowEntity }> {
const sharing = await this.getSharing(user, workflowId, ['workflow', 'role'], { const sharing = await this.getSharing(user, workflowId, { allowGlobalScope: false }, [
allowGlobalOwner: false, 'workflow',
}); 'role',
]);
if (!sharing || sharing.role.name !== 'owner') return { ownsWorkflow: false }; if (!sharing || sharing.role.name !== 'owner') return { ownsWorkflow: false };

View file

@ -28,6 +28,7 @@ import { OwnershipService } from '@/services/ownership.service';
import { isStringArray, isWorkflowIdValid } from '@/utils'; import { isStringArray, isWorkflowIdValid } from '@/utils';
import { WorkflowHistoryService } from './workflowHistory/workflowHistory.service.ee'; import { WorkflowHistoryService } from './workflowHistory/workflowHistory.service.ee';
import { BinaryDataService } from 'n8n-core'; import { BinaryDataService } from 'n8n-core';
import type { Scope } from '@n8n/permissions';
import { Logger } from '@/Logger'; import { Logger } from '@/Logger';
import { MultiMainSetup } from '@/services/orchestration/main/MultiMainSetup.ee'; import { MultiMainSetup } from '@/services/orchestration/main/MultiMainSetup.ee';
import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository'; import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository';
@ -36,19 +37,23 @@ import { ExecutionRepository } from '@db/repositories/execution.repository';
import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { BadRequestError } from '@/errors/response-errors/bad-request.error';
import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error';
export type WorkflowsGetSharedOptions =
| { allowGlobalScope: true; globalScope: Scope }
| { allowGlobalScope: false };
export class WorkflowsService { export class WorkflowsService {
static async getSharing( static async getSharing(
user: User, user: User,
workflowId: string, workflowId: string,
options: WorkflowsGetSharedOptions,
relations: string[] = ['workflow'], relations: string[] = ['workflow'],
{ allowGlobalOwner } = { allowGlobalOwner: true },
): Promise<SharedWorkflow | null> { ): Promise<SharedWorkflow | null> {
const where: FindOptionsWhere<SharedWorkflow> = { workflowId }; const where: FindOptionsWhere<SharedWorkflow> = { workflowId };
// Omit user from where if the requesting user is the global // Omit user from where if the requesting user has relevant
// owner. This allows the global owner to view and delete // global workflow permissions. This allows the user to
// workflows they don't own. // access workflows they don't own.
if (!allowGlobalOwner || user.globalRole.name !== 'owner') { if (!options.allowGlobalScope || !(await user.hasGlobalScope(options.globalScope))) {
where.userId = user.id; where.userId = user.id;
} }
@ -195,8 +200,9 @@ export class WorkflowsService {
): Promise<WorkflowEntity> { ): Promise<WorkflowEntity> {
const shared = await Container.get(SharedWorkflowRepository).findOne({ const shared = await Container.get(SharedWorkflowRepository).findOne({
relations: ['workflow', 'role'], relations: ['workflow', 'role'],
where: whereClause({ where: await whereClause({
user, user,
globalScope: 'workflow:update',
entityType: 'workflow', entityType: 'workflow',
entityId: workflowId, entityId: workflowId,
roles, roles,
@ -476,8 +482,9 @@ export class WorkflowsService {
const sharedWorkflow = await Container.get(SharedWorkflowRepository).findOne({ const sharedWorkflow = await Container.get(SharedWorkflowRepository).findOne({
relations: ['workflow', 'role'], relations: ['workflow', 'role'],
where: whereClause({ where: await whereClause({
user, user,
globalScope: 'workflow:delete',
entityType: 'workflow', entityType: 'workflow',
entityId: workflowId, entityId: workflowId,
roles: ['owner'], roles: ['owner'],

View file

@ -97,7 +97,6 @@ describe('POST /login', () => {
const ownerUser = await createUser({ const ownerUser = await createUser({
password: randomValidPassword(), password: randomValidPassword(),
globalRole: globalOwnerRole, globalRole: globalOwnerRole,
isOwner: true,
}); });
const response = await testServer.authAgentFor(ownerUser).get('/login'); const response = await testServer.authAgentFor(ownerUser).get('/login');

View file

@ -16,7 +16,7 @@ import { getGlobalAdminRole, getGlobalMemberRole, getGlobalOwnerRole } from './r
*/ */
export async function createUser(attributes: Partial<User> = {}): Promise<User> { export async function createUser(attributes: Partial<User> = {}): Promise<User> {
const { email, password, firstName, lastName, globalRole, ...rest } = attributes; const { email, password, firstName, lastName, globalRole, ...rest } = attributes;
const user: Partial<User> = { const user = Container.get(UserRepository).create({
email: email ?? randomEmail(), email: email ?? randomEmail(),
password: await hash(password ?? randomValidPassword(), 10), password: await hash(password ?? randomValidPassword(), 10),
firstName: firstName ?? randomName(), firstName: firstName ?? randomName(),
@ -24,7 +24,8 @@ export async function createUser(attributes: Partial<User> = {}): Promise<User>
globalRoleId: (globalRole ?? (await getGlobalMemberRole())).id, globalRoleId: (globalRole ?? (await getGlobalMemberRole())).id,
globalRole, globalRole,
...rest, ...rest,
}; });
user.computeIsOwner();
return Container.get(UserRepository).save(user); return Container.get(UserRepository).save(user);
} }

View file

@ -362,7 +362,6 @@ describe('PATCH /users/:id/role', () => {
let authlessAgent: SuperAgentTest; let authlessAgent: SuperAgentTest;
const { const {
NO_MEMBER,
MISSING_NEW_ROLE_KEY, MISSING_NEW_ROLE_KEY,
MISSING_NEW_ROLE_VALUE, MISSING_NEW_ROLE_VALUE,
NO_ADMIN_ON_OWNER, NO_ADMIN_ON_OWNER,
@ -372,6 +371,8 @@ describe('PATCH /users/:id/role', () => {
NO_ADMIN_IF_UNLICENSED, NO_ADMIN_IF_UNLICENSED,
} = UsersController.ERROR_MESSAGES.CHANGE_ROLE; } = UsersController.ERROR_MESSAGES.CHANGE_ROLE;
const UNAUTHORIZED = 'Unauthorized';
beforeAll(async () => { beforeAll(async () => {
await testDb.truncate(['User']); await testDb.truncate(['User']);
@ -406,7 +407,7 @@ describe('PATCH /users/:id/role', () => {
}); });
expect(response.statusCode).toBe(403); expect(response.statusCode).toBe(403);
expect(response.body.message).toBe(NO_MEMBER); expect(response.body.message).toBe(UNAUTHORIZED);
}); });
test('should fail to demote owner to admin', async () => { test('should fail to demote owner to admin', async () => {
@ -415,7 +416,7 @@ describe('PATCH /users/:id/role', () => {
}); });
expect(response.statusCode).toBe(403); expect(response.statusCode).toBe(403);
expect(response.body.message).toBe(NO_MEMBER); expect(response.body.message).toBe(UNAUTHORIZED);
}); });
test('should fail to demote admin to member', async () => { test('should fail to demote admin to member', async () => {
@ -424,7 +425,7 @@ describe('PATCH /users/:id/role', () => {
}); });
expect(response.statusCode).toBe(403); expect(response.statusCode).toBe(403);
expect(response.body.message).toBe(NO_MEMBER); expect(response.body.message).toBe(UNAUTHORIZED);
}); });
test('should fail to promote other member to owner', async () => { test('should fail to promote other member to owner', async () => {
@ -433,7 +434,7 @@ describe('PATCH /users/:id/role', () => {
}); });
expect(response.statusCode).toBe(403); expect(response.statusCode).toBe(403);
expect(response.body.message).toBe(NO_MEMBER); expect(response.body.message).toBe(UNAUTHORIZED);
}); });
test('should fail to promote other member to admin', async () => { test('should fail to promote other member to admin', async () => {
@ -442,7 +443,7 @@ describe('PATCH /users/:id/role', () => {
}); });
expect(response.statusCode).toBe(403); expect(response.statusCode).toBe(403);
expect(response.body.message).toBe(NO_MEMBER); expect(response.body.message).toBe(UNAUTHORIZED);
}); });
test('should fail to promote self to admin', async () => { test('should fail to promote self to admin', async () => {
@ -451,7 +452,7 @@ describe('PATCH /users/:id/role', () => {
}); });
expect(response.statusCode).toBe(403); expect(response.statusCode).toBe(403);
expect(response.body.message).toBe(NO_MEMBER); expect(response.body.message).toBe(UNAUTHORIZED);
}); });
test('should fail to promote self to owner', async () => { test('should fail to promote self to owner', async () => {
@ -460,7 +461,7 @@ describe('PATCH /users/:id/role', () => {
}); });
expect(response.statusCode).toBe(403); expect(response.statusCode).toBe(403);
expect(response.body.message).toBe(NO_MEMBER); expect(response.body.message).toBe(UNAUTHORIZED);
}); });
}); });

View file

@ -6,6 +6,8 @@ import type { CredentialsEntity } from '@db/entities/CredentialsEntity';
import { SharedCredentials } from '@db/entities/SharedCredentials'; import { SharedCredentials } from '@db/entities/SharedCredentials';
import { SharedCredentialsRepository } from '@db/repositories/sharedCredentials.repository'; import { SharedCredentialsRepository } from '@db/repositories/sharedCredentials.repository';
import { mockInstance } from '../../shared/mocking'; import { mockInstance } from '../../shared/mocking';
import { memberPermissions, ownerPermissions } from '@/permissions/roles';
import { hasScope } from '@n8n/permissions';
describe('SharedCredentialsRepository', () => { describe('SharedCredentialsRepository', () => {
const entityManager = mockInstance(EntityManager); const entityManager = mockInstance(EntityManager);
@ -20,8 +22,23 @@ describe('SharedCredentialsRepository', () => {
const credentialsId = 'cred_123'; const credentialsId = 'cred_123';
const sharedCredential = mock<SharedCredentials>(); const sharedCredential = mock<SharedCredentials>();
sharedCredential.credentials = mock<CredentialsEntity>({ id: credentialsId }); sharedCredential.credentials = mock<CredentialsEntity>({ id: credentialsId });
const owner = mock<User>({ isOwner: true }); const owner = mock<User>({
const member = mock<User>({ isOwner: false, id: 'test' }); isOwner: true,
hasGlobalScope: async (scope) => {
return hasScope(scope, {
global: ownerPermissions,
});
},
});
const member = mock<User>({
isOwner: false,
id: 'test',
hasGlobalScope: async (scope) => {
return hasScope(scope, {
global: memberPermissions,
});
},
});
beforeEach(() => { beforeEach(() => {
jest.resetAllMocks(); jest.resetAllMocks();