fix(core): Ensure all Roles always include the Authenticated permission

packages/core/e2e/__snapshots__/role.e2e-spec.ts.snap

@@ -1,26 +0,0 @@
-// Jest Snapshot v1, https://goo.gl/fbAQLP
-
-exports[`Role resolver createRole 1`] = `
-Object {
-  "code": "test",
-  "description": "test role",
-  "id": "T_3",
-  "permissions": Array [
-    "ReadCustomer",
-    "UpdateCustomer",
-  ],
-}
-`;
-
-exports[`Role resolver updateRole 1`] = `
-Object {
-  "code": "test-modified",
-  "description": "test role modified",
-  "id": "T_3",
-  "permissions": Array [
-    "ReadCustomer",
-    "UpdateCustomer",
-    "DeleteCustomer",
-  ],
-}
-`;

packages/core/e2e/auth.e2e-spec.ts

@@ -65,7 +65,7 @@ describe('Authorization & permissions', () => {
             });
         });
 
-        describe('ReadCatalog', () => {
+        describe('ReadCatalog permission', () => {
             beforeAll(async () => {
                 await client.asSuperAdmin();
                 const { identifier, password } = await createAdministratorWithPermissions('ReadCatalog', [
@@ -77,7 +77,10 @@ describe('Authorization & permissions', () => {
             it('me returns correct permissions', async () => {
                 const { me } = await client.query<Me.Query>(ME);
 
-                expect(me!.channels[0].permissions).toEqual(['ReadCatalog']);
+                expect(me!.channels[0].permissions).toEqual([
+                    Permission.Authenticated,
+                    Permission.ReadCatalog,
+                ]);
             });
 
             it('can read', async () => {
@@ -102,7 +105,7 @@ describe('Authorization & permissions', () => {
             });
         });
 
-        describe('CRUD on Customers', () => {
+        describe('CRUD on Customers permissions', () => {
             beforeAll(async () => {
                 await client.asSuperAdmin();
                 const { identifier, password } = await createAdministratorWithPermissions('CRUDCustomer', [
@@ -118,10 +121,11 @@ describe('Authorization & permissions', () => {
                 const { me } = await client.query<Me.Query>(ME);
 
                 expect(me!.channels[0].permissions).toEqual([
-                    'CreateCustomer',
-                    'ReadCustomer',
-                    'UpdateCustomer',
-                    'DeleteCustomer',
+                    Permission.Authenticated,
+                    Permission.CreateCustomer,
+                    Permission.ReadCustomer,
+                    Permission.UpdateCustomer,
+                    Permission.DeleteCustomer,
                 ]);
             });
 

packages/core/e2e/role.e2e-spec.ts

@@ -5,7 +5,14 @@ import path from 'path';
 
 import { TEST_SETUP_TIMEOUT_MS } from './config/test-config';
 import { ROLE_FRAGMENT } from './graphql/fragments';
-import { CreateRole, GetRole, GetRoles, Permission, Role, UpdateRole } from './graphql/generated-e2e-admin-types';
+import {
+    CreateRole,
+    GetRole,
+    GetRoles,
+    Permission,
+    Role,
+    UpdateRole,
+} from './graphql/generated-e2e-admin-types';
 import { CREATE_ROLE } from './graphql/shared-definitions';
 import { TestAdminClient } from './test-client';
 import { TestServer } from './test-server';
@@ -37,7 +44,54 @@ describe('Role resolver', () => {
         expect(result.roles.totalItems).toBe(2);
     });
 
-    it('createRole', async () => {
+    it(
+        'createRole with invalid permission',
+        assertThrowsWithMessage(async () => {
+            await client.query<CreateRole.Mutation, CreateRole.Variables>(CREATE_ROLE, {
+                input: {
+                    code: 'test',
+                    description: 'test role',
+                    permissions: ['bad permission' as any],
+                },
+            });
+        }, 'Variable "$input" got invalid value "bad permission" at "input.permissions[0]"; Expected type Permission.'),
+    );
+
+    it('createRole with no permissions includes Authenticated', async () => {
+        const { createRole } = await client.query<CreateRole.Mutation, CreateRole.Variables>(CREATE_ROLE, {
+            input: {
+                code: 'test',
+                description: 'test role',
+                permissions: [],
+            },
+        });
+
+        expect(omit(createRole, ['channels'])).toEqual({
+            code: 'test',
+            description: 'test role',
+            id: 'T_3',
+            permissions: [Permission.Authenticated],
+        });
+    });
+
+    it('createRole deduplicates permissions', async () => {
+        const { createRole } = await client.query<CreateRole.Mutation, CreateRole.Variables>(CREATE_ROLE, {
+            input: {
+                code: 'test2',
+                description: 'test role2',
+                permissions: [Permission.ReadSettings, Permission.ReadSettings],
+            },
+        });
+
+        expect(omit(createRole, ['channels'])).toEqual({
+            code: 'test2',
+            description: 'test role2',
+            id: 'T_4',
+            permissions: [Permission.Authenticated, Permission.ReadSettings],
+        });
+    });
+
+    it('createRole with permissions', async () => {
         const result = await client.query<CreateRole.Mutation, CreateRole.Variables>(CREATE_ROLE, {
             input: {
                 code: 'test',
@@ -47,7 +101,12 @@ describe('Role resolver', () => {
         });
 
         createdRole = result.createRole;
-        expect(omit(createdRole, ['channels'])).toMatchSnapshot();
+        expect(omit(createdRole, ['channels'])).toEqual({
+            code: 'test',
+            description: 'test role',
+            id: 'T_5',
+            permissions: [Permission.Authenticated, Permission.ReadCustomer, Permission.UpdateCustomer],
+        });
     });
 
     it('role', async () => {
@@ -65,7 +124,17 @@ describe('Role resolver', () => {
             },
         });
 
-        expect(omit(result.updateRole, ['channels'])).toMatchSnapshot();
+        expect(omit(result.updateRole, ['channels'])).toEqual({
+            code: 'test-modified',
+            description: 'test role modified',
+            id: 'T_5',
+            permissions: [
+                Permission.Authenticated,
+                Permission.ReadCustomer,
+                Permission.UpdateCustomer,
+                Permission.DeleteCustomer,
+            ],
+        });
     });
 
     it('updateRole works with partial input', async () => {
@@ -79,12 +148,29 @@ describe('Role resolver', () => {
         expect(result.updateRole.code).toBe('test-modified-again');
         expect(result.updateRole.description).toBe('test role modified');
         expect(result.updateRole.permissions).toEqual([
+            Permission.Authenticated,
             Permission.ReadCustomer,
             Permission.UpdateCustomer,
             Permission.DeleteCustomer,
         ]);
     });
 
+    it('updateRole deduplicates permissions', async () => {
+        const result = await client.query<UpdateRole.Mutation, UpdateRole.Variables>(UPDATE_ROLE, {
+            input: {
+                id: createdRole.id,
+                permissions: [
+                    Permission.Authenticated,
+                    Permission.Authenticated,
+                    Permission.ReadCustomer,
+                    Permission.ReadCustomer,
+                ],
+            },
+        });
+
+        expect(result.updateRole.permissions).toEqual([Permission.Authenticated, Permission.ReadCustomer]);
+    });
+
     it(
         'updateRole is not allowed for SuperAdmin role',
         assertThrowsWithMessage(async () => {

packages/core/src/i18n/messages/en.json

@@ -42,6 +42,7 @@
     "payment-may-only-be-added-in-arrangingpayment-state": "A Payment may only be added when Order is in \"ArrangingPayment\" state",
     "password-reset-token-has-expired": "Password reset token has expired.",
     "password-reset-token-not-recognized": "Password reset token not recognized",
+    "permission-invalid": "The permission \"{ permission }\" is not valid",
     "product-id-or-slug-must-be-provided": "Either the product id or slug must be provided",
     "product-id-slug-mismatch": "The provided id and slug refer to different Products",
     "product-variant-option-ids-not-compatible": "ProductVariant optionIds must include one optionId from each of the groups: {groupNames}",

packages/core/src/service/services/role.service.ts

@@ -8,9 +8,10 @@ import {
     SUPER_ADMIN_ROLE_DESCRIPTION,
 } from '@vendure/common/lib/shared-constants';
 import { ID, PaginatedList } from '@vendure/common/lib/shared-types';
+import { unique } from '@vendure/common/lib/unique';
 import { Connection } from 'typeorm';
 
-import { EntityNotFoundError, InternalServerError } from '../../common/error/errors';
+import { EntityNotFoundError, InternalServerError, UserInputError } from '../../common/error/errors';
 import { ListQueryOptions } from '../../common/types/common-types';
 import { assertFound } from '../../common/utils';
 import { Role } from '../../entity/role/role.entity';
@@ -66,13 +67,26 @@ export class RoleService {
         });
     }
 
+    /**
+     * Returns all the valid Permission values
+     */
+    getAllPermissions(): string[] {
+        return Object.values(Permission);
+    }
+
     async create(input: CreateRoleInput): Promise<Role> {
-        const role = new Role(input);
+        this.checkPermissionsAreValid(input.permissions);
+        const role = new Role({
+            code: input.code,
+            description: input.description,
+            permissions: unique([Permission.Authenticated, ...input.permissions]),
+        });
         role.channels = [this.channelService.getDefaultChannel()];
         return this.connection.manager.save(role);
     }
 
     async update(input: UpdateRoleInput): Promise<Role> {
+        this.checkPermissionsAreValid(input.permissions);
         const role = await this.findOne(input.id);
         if (!role) {
             throw new EntityNotFoundError('Role', input.id);
@@ -80,11 +94,27 @@ export class RoleService {
         if (role.code === SUPER_ADMIN_ROLE_CODE || role.code === CUSTOMER_ROLE_CODE) {
             throw new InternalServerError(`error.cannot-modify-role`, { roleCode: role.code });
         }
-        const updatedRole = patchEntity(role, input);
+        const updatedRole = patchEntity(role, {
+            code: input.code,
+            description: input.description,
+            permissions: input.permissions ? unique([Permission.Authenticated, ...input.permissions]) : null,
+        });
         await this.connection.manager.save(updatedRole);
         return assertFound(this.findOne(role.id));
     }
 
+    private checkPermissionsAreValid(permissions?: string[] | null) {
+        if (!permissions) {
+            return;
+        }
+        const allPermissions = this.getAllPermissions();
+        for (const permission of permissions) {
+            if (!allPermissions.includes(permission)) {
+                throw new UserInputError('error.permission-invalid', { permission });
+            }
+        }
+    }
+
     private getRoleByCode(code: string): Promise<Role | undefined> {
         return this.connection.getRepository(Role).findOne({
             where: { code },