feat: Introduce advanced permissions (#7844)

This PR introduces the possibility of inviting new users with an `admin` role and changing the role of already invited users. Also using scoped permission checks where applicable instead of using user role checks. --------- Co-authored-by: Val <[email protected]> Co-authored-by: Alex Grozav <[email protected]> Co-authored-by: Iván Ovejero <[email protected]>
n8n-io · Dec 8, 2023 · dbd62a4 · dbd62a4
1 parent e00577b
commit dbd62a4
Show file tree

Hide file tree

Showing 26 changed files with 364 additions and 71 deletions.
diff --git a/cypress/e2e/17-sharing.cy.ts b/cypress/e2e/17-sharing.cy.ts
@@ -1,4 +1,4 @@
-import { INSTANCE_MEMBERS, INSTANCE_OWNER } from '../constants';
+import { INSTANCE_MEMBERS, INSTANCE_OWNER, INSTANCE_ADMIN } from '../constants';
 import {
 	CredentialsModal,
 	CredentialsPage,
@@ -7,6 +7,7 @@ import {
 	WorkflowSharingModal,
 	WorkflowsPage,
 } from '../pages';
+import { getVisibleSelect } from '../utils';
 
 /**
  * User U1 - Instance owner
@@ -129,4 +130,38 @@ describe('Sharing', { disableAutoLogin: true }, () => {
 		credentialsPage.getters.credentialCard('Credential C2').click();
 		credentialsModal.getters.testSuccessTag().should('be.visible');
 	});
+
+	it.only('should work for admin role on credentials created by others (also can share it with themselves)', () => {
+		cy.signin(INSTANCE_MEMBERS[0]);
+
+		cy.visit(credentialsPage.url);
+		credentialsPage.getters.createCredentialButton().click();
+		credentialsModal.getters.newCredentialTypeOption('Notion API').click();
+		credentialsModal.getters.newCredentialTypeButton().click({ force: true });
+		credentialsModal.getters.connectionParameter('Internal Integration Secret').type('1234567890');
+		credentialsModal.actions.setName('Credential C3');
+		credentialsModal.actions.save();
+		credentialsModal.actions.close();
+
+		cy.signout();
+		cy.signin(INSTANCE_ADMIN);
+		cy.visit(credentialsPage.url);
+		credentialsPage.getters.credentialCard('Credential C3').click();
+		credentialsModal.getters.testSuccessTag().should('be.visible');
+		cy.get('input').should('not.have.length');
+		credentialsModal.actions.changeTab('Sharing');
+
+		credentialsModal.getters.usersSelect().click();
+		cy.getByTestId('user-email')
+			.filter(':visible')
+			.should('have.length', 3)
+			.contains(INSTANCE_ADMIN.email)
+			.should('have.length', 1);
+		getVisibleSelect().contains(INSTANCE_OWNER.email.toLowerCase()).click();
+
+		credentialsModal.actions.addUser(INSTANCE_MEMBERS[1].email);
+		credentialsModal.actions.addUser(INSTANCE_ADMIN.email);
+		credentialsModal.actions.saveSharing();
+		credentialsModal.actions.close();
+	});
 });
diff --git a/cypress/e2e/18-user-management.cy.ts b/cypress/e2e/18-user-management.cy.ts
@@ -1,6 +1,7 @@
 import { INSTANCE_MEMBERS, INSTANCE_OWNER, INSTANCE_ADMIN } from '../constants';
 import { MainSidebar, SettingsSidebar, SettingsUsersPage, WorkflowPage } from '../pages';
 import { PersonalSettingsPage } from '../pages/settings-personal';
+import { getVisibleSelect } from '../utils';
 
 /**
  * User A - Instance owner
@@ -29,7 +30,9 @@ const settingsSidebar = new SettingsSidebar();
 const mainSidebar = new MainSidebar();
 
 describe('User Management', { disableAutoLogin: true }, () => {
-	before(() => cy.enableFeature('sharing'));
+	before(() => {
+		cy.enableFeature('sharing');
+	});
 
 	it('should prevent non-owners to access UM settings', () => {
 		usersSettingsPage.actions.loginAndVisit(
@@ -58,6 +61,67 @@ describe('User Management', { disableAutoLogin: true }, () => {
 		usersSettingsPage.getters.userActionsToggle(INSTANCE_ADMIN.email).should('exist');
 	});
 
+	it('should be able to change user role to Admin and back', () => {
+		cy.enableFeature('advancedPermissions');
+
+		usersSettingsPage.actions.loginAndVisit(INSTANCE_OWNER.email, INSTANCE_OWNER.password, true);
+
+		// Change role from Member to Admin
+		usersSettingsPage.getters
+			.userRoleSelect(INSTANCE_MEMBERS[0].email)
+			.find('input')
+			.should('contain.value', 'Member');
+		usersSettingsPage.getters.userRoleSelect(INSTANCE_MEMBERS[0].email).click();
+		getVisibleSelect().find('li').contains('Admin').click();
+		usersSettingsPage.getters
+			.userRoleSelect(INSTANCE_MEMBERS[0].email)
+			.find('input')
+			.should('contain.value', 'Admin');
+
+		usersSettingsPage.actions.loginAndVisit(
+			INSTANCE_MEMBERS[0].email,
+			INSTANCE_MEMBERS[0].password,
+			true,
+		);
+
+		// Change role from Admin to Member, then back to Admin
+		usersSettingsPage.getters
+			.userRoleSelect(INSTANCE_ADMIN.email)
+			.find('input')
+			.should('contain.value', 'Admin');
+
+		usersSettingsPage.getters.userRoleSelect(INSTANCE_ADMIN.email).click();
+		getVisibleSelect().find('li').contains('Member').click();
+		usersSettingsPage.getters
+			.userRoleSelect(INSTANCE_ADMIN.email)
+			.find('input')
+			.should('contain.value', 'Member');
+
+		usersSettingsPage.actions.loginAndVisit(INSTANCE_ADMIN.email, INSTANCE_ADMIN.password, false);
+		usersSettingsPage.actions.loginAndVisit(
+			INSTANCE_MEMBERS[0].email,
+			INSTANCE_MEMBERS[0].password,
+			true,
+		);
+
+		usersSettingsPage.getters.userRoleSelect(INSTANCE_ADMIN.email).click();
+		getVisibleSelect().find('li').contains('Admin').click();
+		usersSettingsPage.getters
+			.userRoleSelect(INSTANCE_ADMIN.email)
+			.find('input')
+			.should('contain.value', 'Admin');
+
+		usersSettingsPage.actions.loginAndVisit(INSTANCE_ADMIN.email, INSTANCE_ADMIN.password, true);
+		usersSettingsPage.getters.userRoleSelect(INSTANCE_MEMBERS[0].email).click();
+		getVisibleSelect().find('li').contains('Member').click();
+		usersSettingsPage.getters
+			.userRoleSelect(INSTANCE_MEMBERS[0].email)
+			.find('input')
+			.should('contain.value', 'Member');
+
+		cy.disableFeature('advancedPermissions');
+	});
+
 	it('should be able to change theme', () => {
 		personalSettingsPage.actions.loginAndVisit(INSTANCE_OWNER.email, INSTANCE_OWNER.password);
 

diff --git a/cypress/e2e/35-admin-user-smoke-test.cy.ts b/cypress/e2e/35-admin-user-smoke-test.cy.ts
@@ -0,0 +1,23 @@
+import { INSTANCE_ADMIN, INSTANCE_OWNER } from '../constants';
+import { SettingsPage } from '../pages/settings';
+
+const settingsPage = new SettingsPage();
+
+describe('Admin user', { disableAutoLogin: true }, () => {
+	it('should see same Settings sub menu items as instance owner', () => {
+		cy.signin(INSTANCE_OWNER);
+		cy.visit(settingsPage.url);
+
+		let ownerMenuItems = 0;
+
+		settingsPage.getters.menuItems().then(($el) => {
+			ownerMenuItems = $el.length;
+		});
+
+		cy.signout();
+		cy.signin(INSTANCE_ADMIN);
+		cy.visit(settingsPage.url);
+
+		settingsPage.getters.menuItems().should('have.length', ownerMenuItems);
+	});
+});
diff --git a/cypress/pages/modals/credentials-modal.ts b/cypress/pages/modals/credentials-modal.ts
@@ -1,4 +1,5 @@
 import { BasePage } from '../base';
+import { getVisibleSelect } from '../../utils';
 
 export class CredentialsModal extends BasePage {
 	getters = {
@@ -30,11 +31,7 @@ export class CredentialsModal extends BasePage {
 	actions = {
 		addUser: (email: string) => {
 			this.getters.usersSelect().click();
-			this.getters
-				.usersSelect()
-				.get('.el-select-dropdown__item')
-				.contains(email.toLowerCase())
-				.click();
+			getVisibleSelect().contains(email.toLowerCase()).click();
 		},
 		setName: (name: string) => {
 			this.getters.name().click();
@@ -48,6 +45,12 @@ export class CredentialsModal extends BasePage {
 			if (test) cy.wait('@testCredential');
 			this.getters.saveButton().should('contain.text', 'Saved');
 		},
+		saveSharing: (test = false) => {
+			cy.intercept('PUT', '/rest/credentials/*/share').as('shareCredential');
+			this.getters.saveButton().click({ force: true });
+			cy.wait('@shareCredential');
+			this.getters.saveButton().should('contain.text', 'Saved');
+		},
 		close: () => {
 			this.getters.closeButton().click();
 		},

diff --git a/cypress/pages/settings-users.ts b/cypress/pages/settings-users.ts
@@ -20,6 +20,8 @@ export class SettingsUsersPage extends BasePage {
 		userItem: (email: string) => cy.getByTestId(`user-list-item-${email.toLowerCase()}`),
 		userActionsToggle: (email: string) =>
 			this.getters.userItem(email).find('[data-test-id="action-toggle"]'),
+		userRoleSelect: (email: string) =>
+			this.getters.userItem(email).find('[data-test-id="user-role-select"]'),
 		deleteUserAction: () =>
 			cy.getByTestId('action-toggle-dropdown').find('li:contains("Delete"):visible'),
 		confirmDeleteModal: () => cy.getByTestId('deleteUser-modal').last(),

diff --git a/cypress/pages/settings.ts b/cypress/pages/settings.ts
@@ -0,0 +1,9 @@
+import { BasePage } from './base';
+
+export class SettingsPage extends BasePage {
+	url = '/settings';
+	getters = {
+		menuItems: () => cy.getByTestId('menu-item'),
+	};
+	actions = {};
+}
diff --git a/packages/cli/src/controllers/users.controller.ts b/packages/cli/src/controllers/users.controller.ts
@@ -170,7 +170,6 @@ export class UsersController {
 	/**
 	 * Delete a user. Optionally, designate a transferee for their workflows and credentials.
 	 */
-	@Authorized(['global', 'owner'])
 	@Delete('/:id')
 	@RequireGlobalScope('user:delete')
 	async deleteUser(req: UserRequest.Delete) {

diff --git a/packages/design-system/src/components/N8nFormInput/FormInput.vue b/packages/design-system/src/components/N8nFormInput/FormInput.vue
@@ -51,6 +51,7 @@
 					:key="option.value"
 					:value="option.value"
 					:label="option.label"
+					:disabled="!!option.disabled"
 					size="small"
 				/>
 			</n8n-select>
@@ -118,7 +119,7 @@ export interface Props {
 	validationRules?: Array<Rule | RuleGroup>;
 	validators?: { [key: string]: IValidator | RuleGroup };
 	maxlength?: number;
-	options?: Array<{ value: string | number; label: string }>;
+	options?: Array<{ value: string | number; label: string; disabled?: boolean }>;
 	autocomplete?: string;
 	name?: string;
 	focusInitially?: boolean;

diff --git a/packages/design-system/src/types/form.ts b/packages/design-system/src/types/form.ts
@@ -44,7 +44,7 @@ export type IFormInput = {
 		validateOnBlur?: boolean;
 		infoText?: string;
 		placeholder?: string;
-		options?: Array<{ label: string; value: string }>;
+		options?: Array<{ label: string; value: string; disabled?: boolean }>;
 		autocomplete?:
 			| 'off'
 			| 'new-password'

diff --git a/packages/editor-ui/src/Interface.ts b/packages/editor-ui/src/Interface.ts
@@ -1790,7 +1790,8 @@ export type UTMCampaign =
 	| 'upgrade-users'
 	| 'upgrade-variables'
 	| 'upgrade-community-nodes'
-	| 'upgrade-workflow-history';
+	| 'upgrade-workflow-history'
+	| 'upgrade-advanced-permissions';
 
 export type N8nBanners = {
 	[key in BannerName]: {

diff --git a/packages/editor-ui/src/__tests__/router.test.ts b/packages/editor-ui/src/__tests__/router.test.ts
@@ -4,6 +4,9 @@ import router from '@/router';
 import { VIEWS } from '@/constants';
 import { setupServer } from '@/__tests__/server';
 import { useSettingsStore } from '@/stores/settings.store';
+import { useRBACStore } from '@/stores/rbac.store';
+import type { Scope } from '@n8n/permissions';
+import type { RouteRecordName } from 'vue-router';
 
 const App = {
 	template: '<div />',
@@ -64,7 +67,7 @@ describe('router', () => {
 		'should resolve %s to %s if user has permissions',
 		async (path, name) => {
 			const settingsStore = useSettingsStore();
-			await settingsStore.getSettings();
+
 			settingsStore.settings.enterprise.debugInEditor = true;
 			settingsStore.settings.enterprise.workflowHistory = true;
 
@@ -73,4 +76,42 @@ describe('router', () => {
 		},
 		10000,
 	);
+
+	test.each<[string, RouteRecordName, Scope[]]>([
+		['/settings/users', VIEWS.WORKFLOWS, []],
+		['/settings/users', VIEWS.USERS_SETTINGS, ['user:create', 'user:update']],
+		['/settings/environments', VIEWS.WORKFLOWS, []],
+		['/settings/environments', VIEWS.SOURCE_CONTROL, ['sourceControl:manage']],
+		['/settings/external-secrets', VIEWS.WORKFLOWS, []],
+		[
+			'/settings/external-secrets',
+			VIEWS.EXTERNAL_SECRETS_SETTINGS,
+			['externalSecretsProvider:list', 'externalSecretsProvider:update'],
+		],
+		['/settings/sso', VIEWS.WORKFLOWS, []],
+		['/settings/sso', VIEWS.SSO_SETTINGS, ['saml:manage']],
+		['/settings/log-streaming', VIEWS.WORKFLOWS, []],
+		['/settings/log-streaming', VIEWS.LOG_STREAMING_SETTINGS, ['logStreaming:manage']],
+		['/settings/community-nodes', VIEWS.WORKFLOWS, []],
+		[
+			'/settings/community-nodes',
+			VIEWS.COMMUNITY_NODES,
+			['communityPackage:list', 'communityPackage:update'],
+		],
+		['/settings/ldap', VIEWS.WORKFLOWS, []],
+		['/settings/ldap', VIEWS.LDAP_SETTINGS, ['ldap:manage']],
+	])(
+		'should resolve %s to %s with %s user permissions',
+		async (path, name, scopes) => {
+			const settingsStore = useSettingsStore();
+			const rbacStore = useRBACStore();
+
+			settingsStore.settings.communityNodesEnabled = true;
+			rbacStore.setGlobalScopes(scopes);
+
+			await router.push(path);
+			expect(router.currentRoute.value.name).toBe(name);
+		},
+		10000,
+	);
 });
diff --git a/packages/editor-ui/src/api/invitation.ts b/packages/editor-ui/src/api/invitation.ts
@@ -1,4 +1,4 @@
-import type { CurrentUserResponse, IInviteResponse, IRestApiContext } from '@/Interface';
+import type { CurrentUserResponse, IInviteResponse, IRestApiContext, IRole } from '@/Interface';
 import type { IDataObject } from 'n8n-workflow';
 import { makeRestApiRequest } from '@/utils/apiUtils';
 
@@ -10,7 +10,10 @@ type AcceptInvitationParams = {
 	password: string;
 };
 
-export async function inviteUsers(context: IRestApiContext, params: Array<{ email: string }>) {
+export async function inviteUsers(
+	context: IRestApiContext,
+	params: Array<{ email: string; role: IRole }>,
+) {
 	return makeRestApiRequest<IInviteResponse[]>(context, 'POST', '/invitations', params);
 }
 

diff --git a/packages/editor-ui/src/components/CredentialEdit/CredentialConfig.vue b/packages/editor-ui/src/components/CredentialEdit/CredentialConfig.vue
@@ -61,7 +61,7 @@
 			data-test-id="credentials-config-container-test-success"
 		/>
 
-		<template v-if="credentialPermissions.updateConnection">
+		<template v-if="credentialPermissions.update">
 			<n8n-notice v-if="documentationUrl && credentialProperties.length" theme="warning">
 				{{ $locale.baseText('credentialEdit.credentialConfig.needHelpFillingOutTheseFields') }}
 				<span class="ml-4xs">
@@ -104,7 +104,7 @@
 		</enterprise-edition>
 
 		<CredentialInputs
-			v-if="credentialType && credentialPermissions.updateConnection"
+			v-if="credentialType && credentialPermissions.update"
 			:credentialData="credentialData"
 			:credentialProperties="credentialProperties"
 			:documentationUrl="documentationUrl"