Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(editor): Replace middleware for Role checks with Scope checks #7847

Merged
merged 12 commits into from
Nov 29, 2023
14 changes: 12 additions & 2 deletions cypress/e2e/18-user-management.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,23 @@ describe('User Management', { disableAutoLogin: true }, () => {
personalSettingsPage.actions.changeTheme('Dark');
cy.get('body').should('have.attr', 'data-theme', 'dark');
settingsSidebar.actions.back();
mainSidebar.getters.logo().should('have.attr', 'src', '/n8n-dev-logo-dark-mode.svg');
mainSidebar.getters
.logo()
.should('have.attr', 'src')
.then((src) => {
expect(src).to.include('/n8n-dev-logo-dark-mode.svg');
});

cy.visit(personalSettingsPage.url);
personalSettingsPage.actions.changeTheme('Light');
cy.get('body').should('have.attr', 'data-theme', 'light');
settingsSidebar.actions.back();
mainSidebar.getters.logo().should('have.attr', 'src', '/n8n-dev-logo.svg');
mainSidebar.getters
.logo()
.should('have.attr', 'src')
.then((src) => {
expect(src).to.include('/n8n-dev-logo.svg');
});
});

it('should delete user and their data', () => {
Expand Down
62 changes: 34 additions & 28 deletions packages/@n8n/permissions/src/types.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,21 @@
export type DefaultOperations = 'create' | 'read' | 'update' | 'delete' | 'list';
export type Resource =
| 'workflow'
| 'tag'
| 'user'
| 'auditLogs'
| 'communityPackage'
| 'credential'
| 'variable'
| 'sourceControl'
| 'externalSecretsProvider'
| 'externalSecret'
| 'eventBusEvent'
| 'eventBusDestination'
| 'orchestration'
| 'communityPackage'
| 'ldap'
| 'saml';
| 'logStreaming'
| 'orchestration'
| 'sourceControl'
| 'saml'
| 'tag'
| 'user'
| 'variable'
| 'workflow';

export type ResourceScope<
R extends Resource,
Expand All @@ -22,45 +24,49 @@ export type ResourceScope<

export type WildcardScope = `${Resource}:*` | '*';

export type WorkflowScope = ResourceScope<'workflow', DefaultOperations | 'share'>;
export type TagScope = ResourceScope<'tag'>;
export type UserScope = ResourceScope<'user', DefaultOperations | 'resetPassword'>;
export type AuditLogsScope = ResourceScope<'auditLogs', 'manage'>;
export type CommunityPackageScope = ResourceScope<
'communityPackage',
'install' | 'uninstall' | 'update' | 'list' | 'manage'
>;
export type CredentialScope = ResourceScope<'credential', DefaultOperations | 'share'>;
export type VariableScope = ResourceScope<'variable'>;
export type SourceControlScope = ResourceScope<'sourceControl', 'pull' | 'push' | 'manage'>;
export type ExternalSecretScope = ResourceScope<'externalSecret', 'list'>;
export type ExternalSecretProviderScope = ResourceScope<
'externalSecretsProvider',
DefaultOperations | 'sync'
>;
export type ExternalSecretScope = ResourceScope<'externalSecret', 'list'>;
export type EventBusEventScope = ResourceScope<'eventBusEvent', DefaultOperations | 'query'>;
export type EventBusDestinationScope = ResourceScope<
'eventBusDestination',
DefaultOperations | 'test'
>;
export type OrchestrationScope = ResourceScope<'orchestration', 'read' | 'list'>;
export type CommunityPackageScope = ResourceScope<
'communityPackage',
'install' | 'uninstall' | 'update' | 'list'
>;
export type EventBusEventScope = ResourceScope<'eventBusEvent', DefaultOperations | 'query'>;
export type LdapScope = ResourceScope<'ldap', 'manage' | 'sync'>;
export type LogStreamingScope = ResourceScope<'logStreaming', 'manage'>;
export type OrchestrationScope = ResourceScope<'orchestration', 'read' | 'list'>;
export type SamlScope = ResourceScope<'saml', 'manage'>;
export type SourceControlScope = ResourceScope<'sourceControl', 'pull' | 'push' | 'manage'>;
export type TagScope = ResourceScope<'tag'>;
export type UserScope = ResourceScope<'user', DefaultOperations | 'resetPassword'>;
export type VariableScope = ResourceScope<'variable'>;
export type WorkflowScope = ResourceScope<'workflow', DefaultOperations | 'share'>;

export type Scope =
| WorkflowScope
| TagScope
| UserScope
| AuditLogsScope
| CommunityPackageScope
| CredentialScope
| VariableScope
| SourceControlScope
| ExternalSecretProviderScope
| ExternalSecretScope
| EventBusEventScope
| EventBusDestinationScope
| OrchestrationScope
| CommunityPackageScope
| LdapScope
| SamlScope;
| LogStreamingScope
| OrchestrationScope
| SamlScope
| SourceControlScope
| TagScope
| UserScope
| VariableScope
| WorkflowScope;

export type ScopeLevel = 'global' | 'project' | 'resource';
export type GetScopeLevel<T extends ScopeLevel> = Record<T, Scope[]>;
Expand Down
88 changes: 45 additions & 43 deletions packages/cli/src/permissions/roles.ts
Original file line number Diff line number Diff line change
@@ -1,53 +1,17 @@
import type { Scope } from '@n8n/permissions';

export const ownerPermissions: Scope[] = [
'workflow:create',
'workflow:read',
'workflow:update',
'workflow:delete',
'workflow:list',
'workflow:share',
'tag:create',
'tag:read',
'tag:update',
'tag:delete',
'tag:list',
'user:create',
'user:read',
'user:update',
'user:delete',
'user:list',
'user:resetPassword',
'auditLogs:manage',
'credential:create',
'credential:read',
'credential:update',
'credential:delete',
'credential:list',
'credential:share',
'variable:create',
'variable:read',
'variable:update',
'variable:delete',
'variable:list',
'sourceControl:pull',
'sourceControl:push',
'sourceControl:manage',
'externalSecretsProvider:create',
'externalSecretsProvider:read',
'externalSecretsProvider:update',
'externalSecretsProvider:delete',
'externalSecretsProvider:list',
'externalSecretsProvider:sync',
'externalSecret:list',
'orchestration:read',
'orchestration:list',
'communityPackage:install',
'communityPackage:uninstall',
'communityPackage:update',
'communityPackage:list',
'ldap:manage',
'ldap:sync',
'saml:manage',
'eventBusEvent:create',
'eventBusEvent:read',
'eventBusEvent:update',
Expand All @@ -61,18 +25,56 @@ export const ownerPermissions: Scope[] = [
'eventBusDestination:delete',
'eventBusDestination:list',
'eventBusDestination:test',
];
export const adminPermissions: Scope[] = ownerPermissions.concat();
export const memberPermissions: Scope[] = [
'user:list',
'variable:list',
'variable:read',
Comment on lines -67 to -69
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are required on the backend for the user to be able to get the user list for sharing and to get the variables they can use.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My mistake! Got messed up from the conflicts.

'externalSecretsProvider:create',
'externalSecretsProvider:read',
'externalSecretsProvider:update',
'externalSecretsProvider:delete',
'externalSecretsProvider:list',
'externalSecretsProvider:sync',
'externalSecret:list',
'ldap:manage',
'ldap:sync',
'logStreaming:manage',
'orchestration:read',
'orchestration:list',
'saml:manage',
'sourceControl:pull',
'sourceControl:push',
'sourceControl:manage',
'tag:create',
'tag:read',
'tag:update',
'tag:delete',
'tag:list',
'user:create',
'user:read',
'user:update',
'user:delete',
'user:list',
'user:resetPassword',
'variable:create',
'variable:read',
'variable:update',
'variable:delete',
'variable:list',
'workflow:create',
'workflow:read',
'workflow:update',
'workflow:delete',
'workflow:list',
'workflow:share',
];
export const adminPermissions: Scope[] = ownerPermissions.concat();
export const memberPermissions: Scope[] = [
'eventBusEvent:list',
'eventBusEvent:read',
'eventBusDestination:list',
'eventBusDestination:test',
'tag:create',
'tag:read',
'tag:update',
'tag:list',
'user:list',
'variable:list',
'variable:read',
];
5 changes: 1 addition & 4 deletions packages/editor-ui/src/components/MainSidebar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ import { useWorkflowsStore } from '@/stores/workflows.store';
import { isNavigationFailure } from 'vue-router';
import ExecutionsUsage from '@/components/ExecutionsUsage.vue';
import MainSidebarSourceControl from '@/components/MainSidebarSourceControl.vue';
import { ROLE } from '@/utils/userUtils';
import { hasPermission } from '@/rbac/permissions';

export default defineComponent({
Expand Down Expand Up @@ -177,9 +176,7 @@ export default defineComponent({
return accessibleRoute !== null;
},
showUserArea(): boolean {
return hasPermission(['role'], {
role: [ROLE.Member, ROLE.Owner],
});
return hasPermission(['authenticated']);
},
workflowExecution(): IExecutionResponse | null {
return this.workflowsStore.getWorkflowExecution;
Expand Down
16 changes: 13 additions & 3 deletions packages/editor-ui/src/rbac/__tests__/permissions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ vi.mock('@/rbac/checks', () => ({
hasRole: vi.fn(),
hasScope: vi.fn(),
isGuest: vi.fn(),
isDefaultUser: vi.fn(),
isAuthenticated: vi.fn(),
isEnterpriseFeatureEnabled: vi.fn(),
isValid: vi.fn(),
Expand All @@ -15,13 +16,22 @@ describe('hasPermission()', () => {
vi.mocked(checks.hasRole).mockReturnValue(true);
vi.mocked(checks.hasScope).mockReturnValue(true);
vi.mocked(checks.isGuest).mockReturnValue(true);
vi.mocked(checks.isDefaultUser).mockReturnValue(true);
vi.mocked(checks.isAuthenticated).mockReturnValue(true);
vi.mocked(checks.isEnterpriseFeatureEnabled).mockReturnValue(true);
vi.mocked(checks.isValid).mockReturnValue(true);

expect(hasPermission(['authenticated', 'custom', 'enterprise', 'guest', 'rbac', 'role'])).toBe(
true,
);
expect(
hasPermission([
'authenticated',
'custom',
'enterprise',
'guest',
'rbac',
'role',
'defaultUser',
]),
).toBe(true);
});

it('should return false if any permission is invalid', () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/editor-ui/src/rbac/checks/__tests__/hasScope.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { useRBACStore } from '@/stores/rbac.store';
import { hasScope } from '@/rbac/checks/hasScope';
import type { HasScopeOptions } from '@n8n/permissions';
import type { ScopeOptions } from '@n8n/permissions';

vi.mock('@/stores/rbac.store', () => ({
useRBACStore: vi.fn(),
Expand All @@ -19,7 +19,7 @@ describe('Checks', () => {
} as unknown as ReturnType<typeof useRBACStore>);

const scope = 'workflow:read';
const options: HasScopeOptions = { mode: 'allOf' };
const options: ScopeOptions = { mode: 'allOf' };
const projectId = 'proj123';
const resourceType = 'workflow';
const resourceId = 'res123';
Expand Down
27 changes: 27 additions & 0 deletions packages/editor-ui/src/rbac/checks/__tests__/isDefaultUser.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { useUsersStore } from '@/stores/users.store';
import { isDefaultUser } from '@/rbac/checks/isDefaultUser';

vi.mock('@/stores/users.store', () => ({
useUsersStore: vi.fn(),
}));

describe('Checks', () => {
describe('isDefaultUser()', () => {
it('should return false if user not logged in', () => {
vi.mocked(useUsersStore).mockReturnValue({ currentUser: null } as ReturnType<
typeof useUsersStore
>);

expect(isDefaultUser()).toBe(false);
});

it('should return true if user is default user', () => {
const mockUser = { id: 'user123', name: 'Test User', isDefaultUser: true };
vi.mocked(useUsersStore).mockReturnValue({ currentUser: mockUser } as unknown as ReturnType<
typeof useUsersStore
>);

expect(isDefaultUser()).toBe(mockUser.isDefaultUser);
});
});
});
1 change: 1 addition & 0 deletions packages/editor-ui/src/rbac/checks/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
export * from './hasRole';
export * from './hasScope';
export * from './isAuthenticated';
export * from './isDefaultUser';
export * from './isEnterpriseFeatureEnabled';
export * from './isGuest';
export * from './isValid';
12 changes: 12 additions & 0 deletions packages/editor-ui/src/rbac/checks/isDefaultUser.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { useUsersStore } from '@/stores/users.store';
import type { DefaultUserMiddlewareOptions, RBACPermissionCheck } from '@/types/rbac';

export const isDefaultUser: RBACPermissionCheck<DefaultUserMiddlewareOptions> = () => {
const usersStore = useUsersStore();
const currentUser = usersStore.currentUser;

if (currentUser) {
return currentUser.isDefaultUser;
}
return false;
};
2 changes: 2 additions & 0 deletions packages/editor-ui/src/rbac/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { guestMiddleware } from '@/rbac/middleware/guest';
import { rbacMiddleware } from '@/rbac/middleware/rbac';
import { roleMiddleware } from '@/rbac/middleware/role';
import { customMiddleware } from '@/rbac/middleware/custom';
import { defaultUserMiddleware } from '@/rbac/middleware/defaultUser';

type Middleware = {
[key in RouterMiddlewareType]: RouterMiddleware<MiddlewareOptions[key]>;
Expand All @@ -13,6 +14,7 @@ type Middleware = {
export const middleware: Middleware = {
authenticated: authenticatedMiddleware,
custom: customMiddleware,
defaultUser: defaultUserMiddleware,
enterprise: enterpriseMiddleware,
guest: guestMiddleware,
rbac: rbacMiddleware,
Expand Down
Loading
Loading