refactor: token permissions, drop admin-like permissions (#4050)

https://linear.app/unleash/issue/2-1155/refactor-permissions

- Our `rbac-middleware` now supports multiple OR permissions;
- Drops non-specific permissions (e.g. CRUD API token permissions
without specifying the token type);
- Makes our permission descriptions consistent;
- Drops our higher-level permissions that basically mean ADMIN (e.g.
ADMIN token permissions) in favor of `ADMIN` permission in order to
avoid privilege escalations;

This PR may help with
https://linear.app/unleash/issue/2-1144/discover-potential-privilege-escalations
as it may prevent privilege escalations altogether.

There's some UI permission logic around this, but in the future
https://linear.app/unleash/issue/2-1156/adapt-api-tokens-creation-ui-to-new-permissions
could take it a bit further by adapting the creation of tokens as well.

---------

Co-authored-by: Gastón Fournier <gaston@getunleash.io>

frontend/src/component/admin/apiToken/ApiTokenPage/ApiTokenPage.tsx

@@ -1,11 +1,6 @@
 import { useContext } from 'react';
 import AccessContext from 'contexts/AccessContext';
 import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender';
-import {
-    CREATE_API_TOKEN,
-    DELETE_API_TOKEN,
-    READ_API_TOKEN,
-} from 'component/providers/AccessProvider/permissions';
 import { AdminAlert } from 'component/common/AdminAlert/AdminAlert';
 import { ApiTokenTable } from 'component/common/ApiTokenTable/ApiTokenTable';
 import { PageContent } from 'component/common/PageContent/PageContent';
@@ -18,6 +13,13 @@ import { ActionCell } from 'component/common/Table/cells/ActionCell/ActionCell';
 import { CopyApiTokenButton } from 'component/common/ApiTokenTable/CopyApiTokenButton/CopyApiTokenButton';
 import { RemoveApiTokenButton } from 'component/common/ApiTokenTable/RemoveApiTokenButton/RemoveApiTokenButton';
 import useApiTokensApi from 'hooks/api/actions/useApiTokensApi/useApiTokensApi';
+import {
+    ADMIN,
+    DELETE_CLIENT_API_TOKEN,
+    DELETE_FRONTEND_API_TOKEN,
+    READ_CLIENT_API_TOKEN,
+    READ_FRONTEND_API_TOKEN,
+} from '@server/types/permissions';
 
 export const ApiTokenPage = () => {
     const { hasAccess } = useContext(AccessContext);
@@ -34,26 +36,45 @@ export const ApiTokenPage = () => {
         setGlobalFilter,
         setHiddenColumns,
         columns,
-    } = useApiTokenTable(tokens, props => (
+    } = useApiTokenTable(tokens, props => {
-        <ActionCell>
+        const READ_PERMISSION =
-            <CopyApiTokenButton
+            props.row.original.type === 'client'
-                token={props.row.original}
+                ? READ_CLIENT_API_TOKEN
-                permission={READ_API_TOKEN}
+                : props.row.original.type === 'frontend'
-            />
+                ? READ_FRONTEND_API_TOKEN
-            <RemoveApiTokenButton
+                : ADMIN;
-                token={props.row.original}
+        const DELETE_PERMISSION =
-                permission={DELETE_API_TOKEN}
+            props.row.original.type === 'client'
-                onRemove={async () => {
+                ? DELETE_CLIENT_API_TOKEN
-                    await deleteToken(props.row.original.secret);
+                : props.row.original.type === 'frontend'
-                    refetch();
+                ? DELETE_FRONTEND_API_TOKEN
-                }}
+                : ADMIN;
-            />
+
-        </ActionCell>
+        return (
-    ));
+            <ActionCell>
+                <CopyApiTokenButton
+                    token={props.row.original}
+                    permission={READ_PERMISSION}
+                />
+                <RemoveApiTokenButton
+                    token={props.row.original}
+                    permission={DELETE_PERMISSION}
+                    onRemove={async () => {
+                        await deleteToken(props.row.original.secret);
+                        refetch();
+                    }}
+                />
+            </ActionCell>
+        );
+    });
 
     return (
         <ConditionallyRender
-            condition={hasAccess(READ_API_TOKEN)}
+            condition={hasAccess([
+                READ_CLIENT_API_TOKEN,
+                READ_FRONTEND_API_TOKEN,
+                ADMIN,
+            ])}
             show={() => (
                 <PageContent
                     header={
@@ -67,7 +88,7 @@ export const ApiTokenPage = () => {
                                     />
                                     <PageHeader.Divider />
                                     <CreateApiTokenButton
-                                        permission={CREATE_API_TOKEN}
+                                        permission={ADMIN}
                                         path="/admin/api/create-token"
                                     />
                                 </>

frontend/src/component/admin/apiToken/CreateApiToken/CreateApiToken.tsx

@@ -7,7 +7,6 @@ import useApiTokensApi from 'hooks/api/actions/useApiTokensApi/useApiTokensApi';
 import useUiConfig from 'hooks/api/getters/useUiConfig/useUiConfig';
 import useToast from 'hooks/useToast';
 import { useApiTokenForm } from 'component/admin/apiToken/ApiTokenForm/useApiTokenForm';
-import { CREATE_API_TOKEN } from 'component/providers/AccessProvider/permissions';
 import { ConfirmToken } from '../ConfirmToken/ConfirmToken';
 import { scrollToTop } from 'component/common/util';
 import { formatUnknownError } from 'utils/formatUnknownError';
@@ -18,6 +17,7 @@ import { TokenInfo } from '../ApiTokenForm/TokenInfo/TokenInfo';
 import { TokenTypeSelector } from '../ApiTokenForm/TokenTypeSelector/TokenTypeSelector';
 import { ProjectSelector } from '../ApiTokenForm/ProjectSelector/ProjectSelector';
 import { EnvironmentSelector } from '../ApiTokenForm/EnvironmentSelector/EnvironmentSelector';
+import { ADMIN } from '@server/types/permissions';
 
 const pageTitle = 'Create API token';
 interface ICreateApiTokenProps {
@@ -52,8 +52,6 @@ export const CreateApiToken = ({ modal = false }: ICreateApiTokenProps) => {
 
     const PATH = `api/admin/api-tokens`;
 
-    const permission = CREATE_API_TOKEN;
-
     const handleSubmit = async (e: Event) => {
         e.preventDefault();
         if (!isValid()) {
@@ -107,7 +105,7 @@ export const CreateApiToken = ({ modal = false }: ICreateApiTokenProps) => {
                 handleSubmit={handleSubmit}
                 handleCancel={handleCancel}
                 mode="Create"
-                actions={<CreateButton name="token" permission={permission} />}
+                actions={<CreateButton name="token" permission={ADMIN} />}
             >
                 <TokenInfo
                     username={username}

frontend/src/component/admin/roles/Roles.tsx

@@ -15,7 +15,6 @@ import { Search } from 'component/common/Search/Search';
 import theme from 'themes/theme';
 import { PageHeader } from 'component/common/PageHeader/PageHeader';
 import { Add } from '@mui/icons-material';
-import { UPDATE_ROLE } from '@server/types/permissions';
 import ResponsiveButton from 'component/common/ResponsiveButton/ResponsiveButton';
 import { IRole } from 'interfaces/role';
 
@@ -146,7 +145,7 @@ export const Roles = () => {
                                             }}
                                             maxWidth={`${theme.breakpoints.values['sm']}px`}
                                             Icon={Add}
-                                            permission={UPDATE_ROLE}
+                                            permission={ADMIN}
                                         >
                                             New {type} role
                                         </ResponsiveButton>

frontend/src/component/providers/AccessProvider/AccessProvider.tsx

@@ -40,16 +40,22 @@ export const checkAdmin = (permissions: IPermission[] | undefined): boolean => {
 
 export const hasAccess = (
     permissions: IPermission[] | undefined,
-    permission: string,
+    permission: string | string[],
     project?: string,
     environment?: string
 ): boolean => {
     if (!permissions) {
         return false;
     }
-    return permissions.some(p => {
+    const permissionsToCheck = Array.isArray(permission)
-        return checkPermission(p, permission, project, environment);
+        ? permission
-    });
+        : [permission];
+
+    return permissions.some(p =>
+        permissionsToCheck.some(permissionToCheck =>
+            checkPermission(p, permissionToCheck, project, environment)
+        )
+    );
 };
 
 const checkPermission = (

frontend/src/component/providers/AccessProvider/permissions.ts

@@ -17,10 +17,6 @@ export const UPDATE_TAG_TYPE = 'UPDATE_TAG_TYPE';
 export const CREATE_ADDON = 'CREATE_ADDON';
 export const UPDATE_ADDON = 'UPDATE_ADDON';
 export const DELETE_ADDON = 'DELETE_ADDON';
-export const CREATE_API_TOKEN = 'CREATE_API_TOKEN';
-export const UPDATE_API_TOKEN = 'UPDATE_API_TOKEN';
-export const DELETE_API_TOKEN = 'DELETE_API_TOKEN';
-export const READ_API_TOKEN = 'READ_API_TOKEN';
 export const DELETE_ENVIRONMENT = 'DELETE_ENVIRONMENT';
 export const UPDATE_ENVIRONMENT = 'UPDATE_ENVIRONMENT';
 export const CREATE_FEATURE_STRATEGY = 'CREATE_FEATURE_STRATEGY';

frontend/src/contexts/AccessContext.ts

@@ -3,7 +3,7 @@ import React from 'react';
 export interface IAccessContext {
     isAdmin: boolean;
     hasAccess: (
-        permission: string,
+        permission: string | string[],
         project?: string,
         environment?: string
     ) => boolean;

src/lib/error/no-access-error.ts

@@ -1,22 +1,33 @@
 import { ApiErrorSchema, UnleashError } from './unleash-error';
 
-class NoAccessError extends UnleashError {
+type Permission = string | string[];
-    permission: string;
+
+class NoAccessError extends UnleashError {
+    permissions: Permission;
+
+    constructor(permission: Permission = [], environment?: string) {
+        const permissions = Array.isArray(permission)
+            ? permission
+            : [permission];
+
+        const permissionsMessage =
+            permissions.length === 1
+                ? `the ${permissions[0]} permission`
+                : `any of the following permissions: ${permissions.join(', ')}`;
 
-    constructor(permission: string, environment?: string) {
         const message =
-            `You don't have the required permissions to perform this operation. You need the "${permission}" permission to perform this action` +
+            `You don't have the required permissions to perform this operation. You need ${permissionsMessage}" to perform this action` +
             (environment ? ` in the "${environment}" environment.` : `.`);
 
         super(message);
 
-        this.permission = permission;
+        this.permissions = permissions;
     }
 
     toJSON(): ApiErrorSchema {
         return {
             ...super.toJSON(),
-            permission: this.permission,
+            permissions: this.permissions,
         };
     }
 }

src/lib/error/unleash-error.test.ts

@@ -417,12 +417,20 @@ describe('Error serialization special cases', () => {
         expect(json).toMatchObject(config);
     });
 
-    it('NoAccessError: adds `permission`', () => {
+    it('NoAccessError: adds `permissions`', () => {
         const permission = 'x';
         const error = new NoAccessError(permission);
         const json = error.toJSON();
 
-        expect(json.permission).toBe(permission);
+        expect(json.permissions).toStrictEqual([permission]);
+    });
+
+    it('NoAccessError: supports multiple permissions', () => {
+        const permission = ['x', 'y', 'z'];
+        const error = new NoAccessError(permission);
+        const json = error.toJSON();
+
+        expect(json.permissions).toStrictEqual(permission);
     });
 
     it('BadDataError: adds `details` with error details', () => {

src/lib/middleware/rbac-middleware.test.ts

@@ -152,7 +152,7 @@ test('should verify permission for root resource', async () => {
     expect(accessService.hasPermission).toHaveBeenCalledTimes(1);
     expect(accessService.hasPermission).toHaveBeenCalledWith(
         req.user,
-        perms.ADMIN,
+        [perms.ADMIN],
         undefined,
         undefined,
     );
@@ -182,7 +182,7 @@ test('should lookup projectId from params', async () => {
 
     expect(accessService.hasPermission).toHaveBeenCalledWith(
         req.user,
-        perms.UPDATE_PROJECT,
+        [perms.UPDATE_PROJECT],
         req.params.projectId,
         undefined,
     );
@@ -217,7 +217,7 @@ test('should lookup projectId from feature toggle', async () => {
 
     expect(accessService.hasPermission).toHaveBeenCalledWith(
         req.user,
-        perms.UPDATE_FEATURE,
+        [perms.UPDATE_FEATURE],
         projectId,
         undefined,
     );
@@ -252,7 +252,7 @@ test('should lookup projectId from data', async () => {
 
     expect(accessService.hasPermission).toHaveBeenCalledWith(
         req.user,
-        perms.CREATE_FEATURE,
+        [perms.CREATE_FEATURE],
         projectId,
         undefined,
     );
@@ -279,7 +279,7 @@ test('Does not double check permission if not changing project when updating tog
     expect(accessService.hasPermission).toHaveBeenCalledTimes(1);
     expect(accessService.hasPermission).toHaveBeenCalledWith(
         req.user,
-        perms.UPDATE_FEATURE,
+        [perms.UPDATE_FEATURE],
         oldProjectId,
         undefined,
     );
@@ -303,7 +303,7 @@ test('UPDATE_TAG_TYPE does not need projectId', async () => {
     expect(accessService.hasPermission).toHaveBeenCalledTimes(1);
     expect(accessService.hasPermission).toHaveBeenCalledWith(
         req.user,
-        perms.UPDATE_TAG_TYPE,
+        [perms.UPDATE_TAG_TYPE],
         undefined,
         undefined,
     );
@@ -327,7 +327,7 @@ test('DELETE_TAG_TYPE does not need projectId', async () => {
     expect(accessService.hasPermission).toHaveBeenCalledTimes(1);
     expect(accessService.hasPermission).toHaveBeenCalledWith(
         req.user,
-        perms.DELETE_TAG_TYPE,
+        [perms.DELETE_TAG_TYPE],
         undefined,
         undefined,
     );
@@ -360,7 +360,7 @@ test('should not expect featureName for UPDATE_FEATURE when projectId specified'
 
     expect(accessService.hasPermission).toHaveBeenCalledWith(
         req.user,
-        perms.UPDATE_FEATURE,
+        [perms.UPDATE_FEATURE],
         projectId,
         undefined,
     );

src/lib/middleware/rbac-middleware.ts

@@ -11,7 +11,7 @@ import User from '../types/user';
 interface PermissionChecker {
     hasPermission(
         user: User,
-        permission: string,
+        permissions: string[],
         projectId?: string,
         environment?: string,
     ): Promise<boolean>;
@@ -38,7 +38,11 @@ const rbacMiddleware = (
     logger.debug('Enabling RBAC middleware');
 
     return (req, res, next) => {
-        req.checkRbac = async (permission: string) => {
+        req.checkRbac = async (permissions: string | string[]) => {
+            const permissionsArray = Array.isArray(permissions)
+                ? permissions
+                : [permissions];
+
             const { user, params } = req;
 
             if (!user) {
@@ -65,21 +69,26 @@ const rbacMiddleware = (
             // will be removed in Unleash v5.0
             if (
                 !projectId &&
-                [DELETE_FEATURE, UPDATE_FEATURE].includes(permission)
+                permissionsArray.some((permission) =>
+                    [DELETE_FEATURE, UPDATE_FEATURE].includes(permission),
+                )
             ) {
                 const { featureName } = params;
                 projectId = await featureToggleStore.getProjectId(featureName);
             } else if (
                 projectId === undefined &&
-                (permission == CREATE_FEATURE ||
+                permissionsArray.some(
-                    permission.endsWith('FEATURE_STRATEGY'))
+                    (permission) =>
+                        permission == CREATE_FEATURE ||
+                        permission.endsWith('FEATURE_STRATEGY'),
+                )
             ) {
                 projectId = 'default';
             }
 
             return accessService.hasPermission(
                 user,
-                permission,
+                permissionsArray,
                 projectId,
                 environment,
             );

src/lib/routes/admin-api/api-token.ts

@@ -3,20 +3,12 @@ import { Response } from 'express';
 import Controller from '../controller';
 import {
     ADMIN,
-    CREATE_ADMIN_API_TOKEN,
-    CREATE_API_TOKEN,
     CREATE_CLIENT_API_TOKEN,
     CREATE_FRONTEND_API_TOKEN,
-    DELETE_ADMIN_API_TOKEN,
-    DELETE_API_TOKEN,
     DELETE_CLIENT_API_TOKEN,
     DELETE_FRONTEND_API_TOKEN,
-    READ_ADMIN_API_TOKEN,
-    READ_API_TOKEN,
     READ_CLIENT_API_TOKEN,
     READ_FRONTEND_API_TOKEN,
-    UPDATE_ADMIN_API_TOKEN,
-    UPDATE_API_TOKEN,
     UPDATE_CLIENT_API_TOKEN,
     UPDATE_FRONTEND_API_TOKEN,
 } from '../../types/permissions';
@@ -58,7 +50,7 @@ const tokenTypeToCreatePermission: (tokenType: ApiTokenType) => string = (
 ) => {
     switch (tokenType) {
         case ApiTokenType.ADMIN:
-            return CREATE_ADMIN_API_TOKEN;
+            return ADMIN;
         case ApiTokenType.CLIENT:
             return CREATE_CLIENT_API_TOKEN;
         case ApiTokenType.FRONTEND:
@@ -87,14 +79,7 @@ const permissionToTokenType: (
         ].includes(permission)
     ) {
         return ApiTokenType.CLIENT;
-    } else if (
+    } else if (ADMIN === permission) {
-        [
-            READ_ADMIN_API_TOKEN,
-            CREATE_ADMIN_API_TOKEN,
-            DELETE_ADMIN_API_TOKEN,
-            UPDATE_ADMIN_API_TOKEN,
-        ].includes(permission)
-    ) {
         return ApiTokenType.ADMIN;
     } else {
         return undefined;
@@ -106,7 +91,7 @@ const tokenTypeToUpdatePermission: (tokenType: ApiTokenType) => string = (
 ) => {
     switch (tokenType) {
         case ApiTokenType.ADMIN:
-            return UPDATE_ADMIN_API_TOKEN;
+            return ADMIN;
         case ApiTokenType.CLIENT:
             return UPDATE_CLIENT_API_TOKEN;
         case ApiTokenType.FRONTEND:
@@ -119,7 +104,7 @@ const tokenTypeToDeletePermission: (tokenType: ApiTokenType) => string = (
 ) => {
     switch (tokenType) {
         case ApiTokenType.ADMIN:
-            return DELETE_ADMIN_API_TOKEN;
+            return ADMIN;
         case ApiTokenType.CLIENT:
             return DELETE_CLIENT_API_TOKEN;
         case ApiTokenType.FRONTEND:
@@ -164,7 +149,7 @@ export class ApiTokenController extends Controller {
             method: 'get',
             path: '',
             handler: this.getAllApiTokens,
-            permission: READ_API_TOKEN,
+            permission: [ADMIN, READ_CLIENT_API_TOKEN, READ_FRONTEND_API_TOKEN],
             middleware: [
                 openApiService.validPath({
                     tags: ['API tokens'],
@@ -180,7 +165,11 @@ export class ApiTokenController extends Controller {
             method: 'post',
             path: '',
             handler: this.createApiToken,
-            permission: CREATE_API_TOKEN,
+            permission: [
+                ADMIN,
+                CREATE_CLIENT_API_TOKEN,
+                CREATE_FRONTEND_API_TOKEN,
+            ],
             middleware: [
                 openApiService.validPath({
                     tags: ['API tokens'],
@@ -197,7 +186,11 @@ export class ApiTokenController extends Controller {
             method: 'put',
             path: '/:token',
             handler: this.updateApiToken,
-            permission: UPDATE_API_TOKEN,
+            permission: [
+                ADMIN,
+                UPDATE_CLIENT_API_TOKEN,
+                UPDATE_FRONTEND_API_TOKEN,
+            ],
             middleware: [
                 openApiService.validPath({
                     tags: ['API tokens'],
@@ -215,7 +208,11 @@ export class ApiTokenController extends Controller {
             path: '/:token',
             handler: this.deleteApiToken,
             acceptAnyContentType: true,
-            permission: DELETE_API_TOKEN,
+            permission: [
+                ADMIN,
+                DELETE_CLIENT_API_TOKEN,
+                DELETE_FRONTEND_API_TOKEN,
+            ],
             middleware: [
                 openApiService.validPath({
                     tags: ['API tokens'],
@@ -355,7 +352,7 @@ export class ApiTokenController extends Controller {
         );
 
         const allowedTokenTypes = [
-            READ_ADMIN_API_TOKEN,
+            ADMIN,
             READ_CLIENT_API_TOKEN,
             READ_FRONTEND_API_TOKEN,
         ]

src/lib/routes/controller.ts

@@ -18,9 +18,11 @@ interface IRequestHandler<
     ): Promise<void> | void;
 }
 
+type Permission = string | string[];
+
 interface IRouteOptionsBase {
     path: string;
-    permission: string;
+    permission: Permission;
     middleware?: RequestHandler[];
     handler: IRequestHandler;
     acceptedContentTypes?: string[];
@@ -37,15 +39,21 @@ interface IRouteOptionsNonGet extends IRouteOptionsBase {
 
 type IRouteOptions = IRouteOptionsNonGet | IRouteOptionsGet;
 
-const checkPermission = (permission) => async (req, res, next) => {
+const checkPermission =
-    if (!permission || permission === NONE) {
+    (permission: Permission = []) =>
-        return next();
+    async (req, res, next) => {
-    }
+        const permissions = (
-    if (req.checkRbac && (await req.checkRbac(permission))) {
+            Array.isArray(permission) ? permission : [permission]
-        return next();
+        ).filter((p) => p !== NONE);
-    }
+
-    return res.status(403).json(new NoAccessError(permission)).end();
+        if (!permissions.length) {
-};
+            return next();
+        }
+        if (req.checkRbac && (await req.checkRbac(permissions))) {
+            return next();
+        }
+        return res.status(403).json(new NoAccessError(permissions)).end();
+    };
 
 /**
  * Base class for Controllers to standardize binding to express Router.
@@ -97,7 +105,11 @@ export default class Controller {
         );
     }
 
-    get(path: string, handler: IRequestHandler, permission?: string): void {
+    get(
+        path: string,
+        handler: IRequestHandler,
+        permission: Permission = NONE,
+    ): void {
         this.route({
             method: 'get',
             path,
@@ -109,7 +121,7 @@ export default class Controller {
     post(
         path: string,
         handler: IRequestHandler,
-        permission: string,
+        permission: Permission = NONE,
         ...acceptedContentTypes: string[]
     ): void {
         this.route({
@@ -124,7 +136,7 @@ export default class Controller {
     put(
         path: string,
         handler: IRequestHandler,
-        permission: string,
+        permission: Permission = NONE,
         ...acceptedContentTypes: string[]
     ): void {
         this.route({
@@ -139,7 +151,7 @@ export default class Controller {
     patch(
         path: string,
         handler: IRequestHandler,
-        permission: string,
+        permission: Permission = NONE,
         ...acceptedContentTypes: string[]
     ): void {
         this.route({
@@ -151,7 +163,11 @@ export default class Controller {
         });
     }
 
-    delete(path: string, handler: IRequestHandler, permission: string): void {
+    delete(
+        path: string,
+        handler: IRequestHandler,
+        permission: Permission = NONE,
+    ): void {
         this.route({
             method: 'delete',
             path,
@@ -165,7 +181,7 @@ export default class Controller {
         path: string,
         filehandler: IRequestHandler,
         handler: Function,
-        permission: string,
+        permission: Permission = NONE,
     ): void {
         this.app.post(
             path,

src/lib/services/access-service.ts

@@ -120,12 +120,21 @@ export class AccessService {
      */
     async hasPermission(
         user: Pick<IUser, 'id' | 'permissions' | 'isAPI'>,
-        permission: string,
+        permission: string | string[],
         projectId?: string,
         environment?: string,
     ): Promise<boolean> {
+        const permissionsArray = Array.isArray(permission)
+            ? permission
+            : [permission];
+
+        const permissionLogInfo =
+            permissionsArray.length === 1
+                ? `permission=${permissionsArray[0]}`
+                : `permissions=[${permissionsArray.join(',')}]`;
+
         this.logger.info(
-            `Checking permission=${permission}, userId=${user.id}, projectId=${projectId}, environment=${environment}`,
+            `Checking ${permissionLogInfo}, userId=${user.id}, projectId=${projectId}, environment=${environment}`,
         );
 
         try {
@@ -145,11 +154,12 @@ export class AccessService {
                 )
                 .some(
                     (p) =>
-                        p.permission === permission || p.permission === ADMIN,
+                        permissionsArray.includes(p.permission) ||
+                        p.permission === ADMIN,
                 );
         } catch (e) {
             this.logger.error(
-                `Error checking permission=${permission}, userId=${user.id} projectId=${projectId}`,
+                `Error checking ${permissionLogInfo}, userId=${user.id} projectId=${projectId}`,
                 e,
             );
             return Promise.resolve(false);

src/lib/types/permissions.ts

@@ -1,40 +1,13 @@
-//Special
+// Special
 export const ADMIN = 'ADMIN';
 export const CLIENT = 'CLIENT';
 export const FRONTEND = 'FRONTEND';
 export const NONE = 'NONE';
 
-export const CREATE_FEATURE = 'CREATE_FEATURE';
+// Root
-export const UPDATE_FEATURE = 'UPDATE_FEATURE';
-export const DELETE_FEATURE = 'DELETE_FEATURE';
-export const CREATE_FEATURE_STRATEGY = 'CREATE_FEATURE_STRATEGY';
-export const UPDATE_FEATURE_STRATEGY = 'UPDATE_FEATURE_STRATEGY';
-export const DELETE_FEATURE_STRATEGY = 'DELETE_FEATURE_STRATEGY';
-export const UPDATE_FEATURE_ENVIRONMENT = 'UPDATE_FEATURE_ENVIRONMENT';
-export const CREATE_STRATEGY = 'CREATE_STRATEGY';
-export const UPDATE_STRATEGY = 'UPDATE_STRATEGY';
-export const DELETE_STRATEGY = 'DELETE_STRATEGY';
-export const UPDATE_APPLICATION = 'UPDATE_APPLICATION';
-export const CREATE_CONTEXT_FIELD = 'CREATE_CONTEXT_FIELD';
-export const UPDATE_CONTEXT_FIELD = 'UPDATE_CONTEXT_FIELD';
-export const DELETE_CONTEXT_FIELD = 'DELETE_CONTEXT_FIELD';
-export const CREATE_PROJECT = 'CREATE_PROJECT';
-export const UPDATE_PROJECT = 'UPDATE_PROJECT';
-export const DELETE_PROJECT = 'DELETE_PROJECT';
 export const CREATE_ADDON = 'CREATE_ADDON';
 export const UPDATE_ADDON = 'UPDATE_ADDON';
 export const DELETE_ADDON = 'DELETE_ADDON';
-export const READ_ROLE = 'READ_ROLE';
-export const UPDATE_ROLE = 'UPDATE_ROLE';
-export const UPDATE_API_TOKEN = 'UPDATE_API_TOKEN';
-export const CREATE_API_TOKEN = 'CREATE_API_TOKEN';
-export const DELETE_API_TOKEN = 'DELETE_API_TOKEN';
-export const READ_API_TOKEN = 'READ_API_TOKEN';
-
-export const UPDATE_ADMIN_API_TOKEN = 'UPDATE_ADMIN_API_TOKEN';
-export const CREATE_ADMIN_API_TOKEN = 'CREATE_ADMIN_API_TOKEN';
-export const DELETE_ADMIN_API_TOKEN = 'DELETE_ADMIN_API_TOKEN';
-export const READ_ADMIN_API_TOKEN = 'READ_ADMIN_API_TOKEN';
 
 export const UPDATE_CLIENT_API_TOKEN = 'UPDATE_CLIENT_API_TOKEN';
 export const CREATE_CLIENT_API_TOKEN = 'CREATE_CLIENT_API_TOKEN';
@@ -46,22 +19,49 @@ export const CREATE_FRONTEND_API_TOKEN = 'CREATE_FRONTEND_API_TOKEN';
 export const DELETE_FRONTEND_API_TOKEN = 'DELETE_FRONTEND_API_TOKEN';
 export const READ_FRONTEND_API_TOKEN = 'READ_FRONTEND_API_TOKEN';
 
-export const UPDATE_TAG_TYPE = 'UPDATE_TAG_TYPE';
+export const UPDATE_APPLICATION = 'UPDATE_APPLICATION';
-export const DELETE_TAG_TYPE = 'DELETE_TAG_TYPE';
+
-export const UPDATE_FEATURE_VARIANTS = 'UPDATE_FEATURE_VARIANTS';
+export const CREATE_CONTEXT_FIELD = 'CREATE_CONTEXT_FIELD';
-export const UPDATE_FEATURE_ENVIRONMENT_VARIANTS =
+export const UPDATE_CONTEXT_FIELD = 'UPDATE_CONTEXT_FIELD';
-    'UPDATE_FEATURE_ENVIRONMENT_VARIANTS';
+export const DELETE_CONTEXT_FIELD = 'DELETE_CONTEXT_FIELD';
-export const MOVE_FEATURE_TOGGLE = 'MOVE_FEATURE_TOGGLE';
+
+export const CREATE_PROJECT = 'CREATE_PROJECT';
+
+export const READ_ROLE = 'READ_ROLE';
+
 export const CREATE_SEGMENT = 'CREATE_SEGMENT';
 export const UPDATE_SEGMENT = 'UPDATE_SEGMENT';
 export const DELETE_SEGMENT = 'DELETE_SEGMENT';
-export const UPDATE_PROJECT_SEGMENT = 'UPDATE_PROJECT_SEGMENT';
+
-export const APPROVE_CHANGE_REQUEST = 'APPROVE_CHANGE_REQUEST';
+export const CREATE_STRATEGY = 'CREATE_STRATEGY';
-export const APPLY_CHANGE_REQUEST = 'APPLY_CHANGE_REQUEST';
+export const UPDATE_STRATEGY = 'UPDATE_STRATEGY';
-export const SKIP_CHANGE_REQUEST = 'SKIP_CHANGE_REQUEST';
+export const DELETE_STRATEGY = 'DELETE_STRATEGY';
+
+export const UPDATE_TAG_TYPE = 'UPDATE_TAG_TYPE';
+export const DELETE_TAG_TYPE = 'DELETE_TAG_TYPE';
+
+// Project
+export const CREATE_FEATURE = 'CREATE_FEATURE';
+export const UPDATE_FEATURE = 'UPDATE_FEATURE';
+export const DELETE_FEATURE = 'DELETE_FEATURE';
+export const UPDATE_PROJECT = 'UPDATE_PROJECT';
+export const DELETE_PROJECT = 'DELETE_PROJECT';
+export const UPDATE_FEATURE_VARIANTS = 'UPDATE_FEATURE_VARIANTS';
+export const MOVE_FEATURE_TOGGLE = 'MOVE_FEATURE_TOGGLE';
 export const READ_PROJECT_API_TOKEN = 'READ_PROJECT_API_TOKEN';
 export const CREATE_PROJECT_API_TOKEN = 'CREATE_PROJECT_API_TOKEN';
 export const DELETE_PROJECT_API_TOKEN = 'DELETE_PROJECT_API_TOKEN';
+export const UPDATE_PROJECT_SEGMENT = 'UPDATE_PROJECT_SEGMENT';
+
+export const CREATE_FEATURE_STRATEGY = 'CREATE_FEATURE_STRATEGY';
+export const UPDATE_FEATURE_STRATEGY = 'UPDATE_FEATURE_STRATEGY';
+export const DELETE_FEATURE_STRATEGY = 'DELETE_FEATURE_STRATEGY';
+export const UPDATE_FEATURE_ENVIRONMENT_VARIANTS =
+    'UPDATE_FEATURE_ENVIRONMENT_VARIANTS';
+export const UPDATE_FEATURE_ENVIRONMENT = 'UPDATE_FEATURE_ENVIRONMENT';
+export const APPROVE_CHANGE_REQUEST = 'APPROVE_CHANGE_REQUEST';
+export const APPLY_CHANGE_REQUEST = 'APPLY_CHANGE_REQUEST';
+export const SKIP_CHANGE_REQUEST = 'SKIP_CHANGE_REQUEST';
 
 export const ROOT_PERMISSION_CATEGORIES = [
     {
@@ -71,10 +71,14 @@ export const ROOT_PERMISSION_CATEGORIES = [
     {
         label: 'API token',
         permissions: [
-            READ_API_TOKEN,
+            UPDATE_CLIENT_API_TOKEN,
-            CREATE_API_TOKEN,
+            CREATE_CLIENT_API_TOKEN,
-            UPDATE_API_TOKEN,
+            DELETE_CLIENT_API_TOKEN,
-            DELETE_API_TOKEN,
+            READ_CLIENT_API_TOKEN,
+            UPDATE_FRONTEND_API_TOKEN,
+            CREATE_FRONTEND_API_TOKEN,
+            DELETE_FRONTEND_API_TOKEN,
+            READ_FRONTEND_API_TOKEN,
         ],
     },
     {
@@ -95,7 +99,7 @@ export const ROOT_PERMISSION_CATEGORIES = [
     },
     {
         label: 'Role',
-        permissions: [READ_ROLE, UPDATE_ROLE],
+        permissions: [READ_ROLE],
     },
     {
         label: 'Segment',

src/migrations/20230621141239-refactor-api-token-permissions.js

@@ -0,0 +1,24 @@
+exports.up = function (db, cb) {
+    db.runSql(
+        `
+    UPDATE permissions SET display_name = 'Create CLIENT API tokens' WHERE permission = 'CREATE_CLIENT_API_TOKEN';
+    UPDATE permissions SET display_name = 'Update CLIENT API tokens' WHERE permission = 'UPDATE_CLIENT_API_TOKEN';
+    UPDATE permissions SET display_name = 'Delete CLIENT API tokens' WHERE permission = 'DELETE_CLIENT_API_TOKEN';
+    UPDATE permissions SET display_name = 'Read CLIENT API tokens' WHERE permission = 'READ_CLIENT_API_TOKEN';
+
+    UPDATE permissions SET display_name = 'Create FRONTEND API tokens' WHERE permission = 'CREATE_FRONTEND_API_TOKEN';
+    UPDATE permissions SET display_name = 'Update FRONTEND API tokens' WHERE permission = 'UPDATE_FRONTEND_API_TOKEN';
+    UPDATE permissions SET display_name = 'Delete FRONTEND API tokens' WHERE permission = 'DELETE_FRONTEND_API_TOKEN';
+    UPDATE permissions SET display_name = 'Read FRONTEND API tokens' WHERE permission = 'READ_FRONTEND_API_TOKEN';
+  `,
+        cb,
+    );
+};
+
+exports.down = function (db, cb) {
+    db.runSql(
+        `
+        `,
+        cb,
+    );
+};

src/test/e2e/api/admin/api-token.auth.e2e.test.ts

@@ -4,15 +4,10 @@ import getLogger from '../../../fixtures/no-logger';
 import { ApiTokenType } from '../../../../lib/types/models/api-token';
 import { RoleName } from '../../../../lib/types/model';
 import {
-    CREATE_API_TOKEN,
     CREATE_CLIENT_API_TOKEN,
-    DELETE_API_TOKEN,
     DELETE_CLIENT_API_TOKEN,
-    READ_ADMIN_API_TOKEN,
-    READ_API_TOKEN,
     READ_CLIENT_API_TOKEN,
     READ_FRONTEND_API_TOKEN,
-    UPDATE_API_TOKEN,
     UPDATE_CLIENT_API_TOKEN,
 } from '../../../../lib/types';
 import { addDays } from 'date-fns';
@@ -196,10 +191,6 @@ describe('Fine grained API token permissions', () => {
                             permissions: [],
                             type: 'root-custom',
                         });
-                    await accessService.addPermissionToRole(
-                        role.id,
-                        CREATE_API_TOKEN,
-                    );
                     await accessService.addPermissionToRole(
                         role.id,
                         CREATE_CLIENT_API_TOKEN,
@@ -251,10 +242,6 @@ describe('Fine grained API token permissions', () => {
                             permissions: [],
                             type: 'root-custom',
                         });
-                    await accessService.addPermissionToRole(
-                        role.id,
-                        CREATE_API_TOKEN,
-                    );
                     await accessService.addPermissionToRole(
                         role.id,
                         CREATE_CLIENT_API_TOKEN,
@@ -306,10 +293,6 @@ describe('Fine grained API token permissions', () => {
                             permissions: [],
                             type: 'root-custom',
                         });
-                    await accessService.addPermissionToRole(
-                        role.id,
-                        CREATE_API_TOKEN,
-                    );
                     await accessService.addPermissionToRole(
                         role.id,
                         CREATE_CLIENT_API_TOKEN,
@@ -364,10 +347,6 @@ describe('Fine grained API token permissions', () => {
                             type: 'root-custom',
                         },
                     );
-                    await accessService.addPermissionToRole(
-                        readFrontendApiToken.id,
-                        READ_API_TOKEN,
-                    );
                     await accessService.addPermissionToRole(
                         readFrontendApiToken.id,
                         READ_FRONTEND_API_TOKEN,
@@ -437,10 +416,6 @@ describe('Fine grained API token permissions', () => {
                         permissions: [],
                         type: 'root-custom',
                     });
-                    await accessService.addPermissionToRole(
-                        readClientTokenRole.id,
-                        READ_API_TOKEN,
-                    );
                     await accessService.addPermissionToRole(
                         readClientTokenRole.id,
                         READ_CLIENT_API_TOKEN,
@@ -490,49 +465,23 @@ describe('Fine grained API token permissions', () => {
                 });
             await destroy();
         });
-        test('READ_ADMIN_API_TOKEN should be able to see ADMIN tokens', async () => {
+        test('Admin users should be able to see all tokens', async () => {
             const preHook = (app, config, { userService, accessService }) => {
                 app.use('/api/admin/', async (req, res, next) => {
                     const role = await accessService.getRootRole(
-                        RoleName.VIEWER,
+                        RoleName.ADMIN,
                     );
                     const user = await userService.createUser({
                         email: 'read_admin_token@example.com',
                         rootRole: role.id,
                     });
                     req.user = user;
-                    const readAdminApiToken = await accessService.createRole({
-                        name: 'admin_token_reader',
-                        description: 'Can read admin tokens',
-                        permissions: [],
-                        type: 'root-custom',
-                    });
-                    await accessService.addPermissionToRole(
-                        readAdminApiToken.id,
-                        READ_API_TOKEN,
-                    );
-                    await accessService.addPermissionToRole(
-                        readAdminApiToken.id,
-                        READ_ADMIN_API_TOKEN,
-                    );
-                    await accessService.addUserToRole(
-                        user.id,
-                        readAdminApiToken.id,
-                        'default',
-                    );
                     next();
                 });
             };
             const { request, destroy } = await setupAppWithCustomAuth(
                 stores,
                 preHook,
-                {
-                    experimental: {
-                        flags: {
-                            customRootRoles: true,
-                        },
-                    },
-                },
             );
             await stores.apiTokenStore.insert({
                 username: 'client',
@@ -555,8 +504,54 @@ describe('Fine grained API token permissions', () => {
                 .set('Content-Type', 'application/json')
                 .expect(200)
                 .expect((res) => {
-                    expect(res.body.tokens).toHaveLength(1);
+                    expect(res.body.tokens).toHaveLength(3);
-                    expect(res.body.tokens[0].type).toBe(ApiTokenType.ADMIN);
+                });
+            await destroy();
+        });
+        test('Editor users should be able to see all tokens except ADMIN tokens', async () => {
+            const preHook = (app, config, { userService, accessService }) => {
+                app.use('/api/admin/', async (req, res, next) => {
+                    const role = await accessService.getRootRole(
+                        RoleName.EDITOR,
+                    );
+                    const user = await userService.createUser({
+                        email: 'standard-editor-reads-tokens@example.com',
+                        rootRole: role.id,
+                    });
+                    req.user = user;
+                    next();
+                });
+            };
+            const { request, destroy } = await setupAppWithCustomAuth(
+                stores,
+                preHook,
+            );
+            await stores.apiTokenStore.insert({
+                username: 'client',
+                secret: 'client_secret_4321',
+                type: ApiTokenType.CLIENT,
+            });
+            await stores.apiTokenStore.insert({
+                username: 'admin',
+                secret: 'admin_secret_4321',
+                type: ApiTokenType.ADMIN,
+            });
+            await stores.apiTokenStore.insert({
+                username: 'frontender',
+                secret: 'frontend_secret_4321',
+                type: ApiTokenType.FRONTEND,
+            });
+            await request
+                .get('/api/admin/api-tokens')
+                .set('Content-Type', 'application/json')
+                .expect(200)
+                .expect((res) => {
+                    expect(res.body.tokens).toHaveLength(2);
+                    expect(
+                        res.body.tokens.filter(
+                            ({ type }) => type === ApiTokenType.ADMIN,
+                        ),
+                    ).toHaveLength(0);
                 });
             await destroy();
         });
@@ -585,10 +580,6 @@ describe('Fine grained API token permissions', () => {
                                 permissions: [],
                                 type: 'root-custom',
                             });
-                        await accessService.addPermissionToRole(
-                            updateClientApiExpiry.id,
-                            UPDATE_API_TOKEN,
-                        );
                         await accessService.addPermissionToRole(
                             updateClientApiExpiry.id,
                             UPDATE_CLIENT_API_TOKEN,
@@ -645,10 +636,6 @@ describe('Fine grained API token permissions', () => {
                                 permissions: [],
                                 type: 'root-custom',
                             });
-                        await accessService.addPermissionToRole(
-                            updateClientApiExpiry.id,
-                            UPDATE_API_TOKEN,
-                        );
                         await accessService.addPermissionToRole(
                             updateClientApiExpiry.id,
                             UPDATE_CLIENT_API_TOKEN,
@@ -706,10 +693,6 @@ describe('Fine grained API token permissions', () => {
                                 permissions: [],
                                 type: 'root-custom',
                             });
-                        await accessService.addPermissionToRole(
-                            updateClientApiExpiry.id,
-                            UPDATE_API_TOKEN,
-                        );
                         await accessService.addPermissionToRole(
                             updateClientApiExpiry.id,
                             UPDATE_CLIENT_API_TOKEN,
@@ -770,10 +753,6 @@ describe('Fine grained API token permissions', () => {
                                 permissions: [],
                                 type: 'root-custom',
                             });
-                        await accessService.addPermissionToRole(
-                            updateClientApiExpiry.id,
-                            DELETE_API_TOKEN,
-                        );
                         await accessService.addPermissionToRole(
                             updateClientApiExpiry.id,
                             DELETE_CLIENT_API_TOKEN,
@@ -830,10 +809,6 @@ describe('Fine grained API token permissions', () => {
                                 permissions: [],
                                 type: 'root-custom',
                             });
-                        await accessService.addPermissionToRole(
-                            updateClientApiExpiry.id,
-                            DELETE_API_TOKEN,
-                        );
                         await accessService.addPermissionToRole(
                             updateClientApiExpiry.id,
                             DELETE_CLIENT_API_TOKEN,
@@ -890,10 +865,6 @@ describe('Fine grained API token permissions', () => {
                                 permissions: [],
                                 type: 'root-custom',
                             });
-                        await accessService.addPermissionToRole(
-                            updateClientApiExpiry.id,
-                            DELETE_API_TOKEN,
-                        );
                         await accessService.addPermissionToRole(
                             updateClientApiExpiry.id,
                             DELETE_CLIENT_API_TOKEN,