diff --git a/src/lib/error/forbidden-error.ts b/src/lib/error/forbidden-error.ts new file mode 100644 index 0000000000..d8ca41dcfe --- /dev/null +++ b/src/lib/error/forbidden-error.ts @@ -0,0 +1,6 @@ +import { UnleashError } from './unleash-error'; + +class ForbiddenError extends UnleashError {} + +export default ForbiddenError; +module.exports = ForbiddenError; diff --git a/src/lib/error/index.ts b/src/lib/error/index.ts index 75e836b037..479c72ad95 100644 --- a/src/lib/error/index.ts +++ b/src/lib/error/index.ts @@ -7,13 +7,14 @@ import InvalidOperationError from './invalid-operation-error'; import InvalidTokenError from './invalid-token-error'; import MinimumOneEnvironmentError from './minimum-one-environment-error'; import NameExistsError from './name-exists-error'; -import NoAccessError from './no-access-error'; +import PermissionError from './permission-error'; import { OperationDeniedError } from './operation-denied-error'; import UserTokenError from './used-token-error'; import RoleInUseError from './role-in-use-error'; import ProjectWithoutOwnerError from './project-without-owner-error'; import PasswordUndefinedError from './password-undefined'; import PasswordMismatchError from './password-mismatch'; +import ForbiddenError from './forbidden-error'; export { BadDataError, @@ -26,7 +27,8 @@ export { InvalidTokenError, MinimumOneEnvironmentError, NameExistsError, - NoAccessError, + PermissionError, + ForbiddenError, OperationDeniedError, UserTokenError, RoleInUseError, diff --git a/src/lib/error/no-access-error.ts b/src/lib/error/permission-error.ts similarity index 65% rename from src/lib/error/no-access-error.ts rename to src/lib/error/permission-error.ts index 38fbdaa8c1..2cbf50bc32 100644 --- a/src/lib/error/no-access-error.ts +++ b/src/lib/error/permission-error.ts @@ -2,7 +2,7 @@ import { ApiErrorSchema, UnleashError } from './unleash-error'; type Permission = string | string[]; -class NoAccessError extends UnleashError { +class PermissionError extends UnleashError { permissions: Permission; constructor(permission: Permission = [], environment?: string) { @@ -12,11 +12,13 @@ class NoAccessError extends UnleashError { const permissionsMessage = permissions.length === 1 - ? `the ${permissions[0]} permission` - : `any of the following permissions: ${permissions.join(', ')}`; + ? `the "${permissions[0]}" permission` + : `all of the following permissions: ${permissions + .map((perm) => `"${perm}"`) + .join(', ')}`; const message = - `You don't have the required permissions to perform this operation. You need ${permissionsMessage}" to perform this action` + + `You don't have the required permissions to perform this operation. To perform this action, you need ${permissionsMessage}` + (environment ? ` in the "${environment}" environment.` : `.`); super(message); @@ -32,5 +34,5 @@ class NoAccessError extends UnleashError { } } -export default NoAccessError; -module.exports = NoAccessError; +export default PermissionError; +module.exports = PermissionError; diff --git a/src/lib/error/unleash-error.test.ts b/src/lib/error/unleash-error.test.ts index 9a6a2898b9..e3ba8f0766 100644 --- a/src/lib/error/unleash-error.test.ts +++ b/src/lib/error/unleash-error.test.ts @@ -6,7 +6,7 @@ import BadDataError, { fromOpenApiValidationError, fromOpenApiValidationErrors, } from './bad-data-error'; -import NoAccessError from './no-access-error'; +import PermissionError from './permission-error'; import OwaspValidationError from './owasp-validation-error'; import IncompatibleProjectError from './incompatible-project-error'; import PasswordUndefinedError from './password-undefined'; @@ -462,7 +462,7 @@ describe('Error serialization special cases', () => { it('NoAccessError: adds `permissions`', () => { const permission = 'x'; - const error = new NoAccessError(permission); + const error = new PermissionError(permission); const json = error.toJSON(); expect(json.permissions).toStrictEqual([permission]); @@ -470,7 +470,7 @@ describe('Error serialization special cases', () => { it('NoAccessError: supports multiple permissions', () => { const permission = ['x', 'y', 'z']; - const error = new NoAccessError(permission); + const error = new PermissionError(permission); const json = error.toJSON(); expect(json.permissions).toStrictEqual(permission); diff --git a/src/lib/error/unleash-error.ts b/src/lib/error/unleash-error.ts index 5673958ae3..331a37ec75 100644 --- a/src/lib/error/unleash-error.ts +++ b/src/lib/error/unleash-error.ts @@ -24,9 +24,10 @@ export const UnleashApiErrorTypes = [ 'ValidationError', 'AuthenticationRequired', 'UnauthorizedError', - 'NoAccessError', + 'PermissionError', 'InvalidTokenError', 'OwaspValidationError', + 'ForbiddenError', // server errors; not the end user's fault 'InternalError', @@ -86,6 +87,10 @@ const statusCode = (errorName: string): number => { return 403; case 'AuthenticationRequired': return 401; + case 'ForbiddenError': + return 403; + case 'PermissionError': + return 403; case 'BadRequestError': //thrown by express; do not remove return 400; default: diff --git a/src/lib/routes/admin-api/project/variants.ts b/src/lib/routes/admin-api/project/variants.ts index 48ec317d5f..f076fae2b1 100644 --- a/src/lib/routes/admin-api/project/variants.ts +++ b/src/lib/routes/admin-api/project/variants.ts @@ -17,7 +17,7 @@ import { FeatureVariantsSchema } from '../../../openapi/spec/feature-variants-sc import { createRequestSchema } from '../../../openapi/util/create-request-schema'; import { createResponseSchema } from '../../../openapi/util/create-response-schema'; import { AccessService } from '../../../services'; -import { BadDataError, NoAccessError } from '../../../../lib/error'; +import { BadDataError, PermissionError } from '../../../../lib/error'; import { User } from 'lib/server-impl'; import { PushVariantsSchema } from 'lib/openapi/spec/push-variants-schema'; @@ -309,7 +309,7 @@ export default class VariantsController extends Controller { environment, )) ) { - throw new NoAccessError( + throw new PermissionError( UPDATE_FEATURE_ENVIRONMENT_VARIANTS, environment, ); diff --git a/src/lib/routes/controller.ts b/src/lib/routes/controller.ts index e3d69afe68..8588042c8a 100644 --- a/src/lib/routes/controller.ts +++ b/src/lib/routes/controller.ts @@ -3,8 +3,8 @@ import { Logger } from 'lib/logger'; import { IUnleashConfig } from '../types/option'; import { NONE } from '../types/permissions'; import { handleErrors } from './util'; -import NoAccessError from '../error/no-access-error'; import requireContentType from '../middleware/content_type_checker'; +import { PermissionError } from '../error'; interface IRequestHandler< P = any, @@ -52,7 +52,7 @@ const checkPermission = if (req.checkRbac && (await req.checkRbac(permissions))) { return next(); } - return res.status(403).json(new NoAccessError(permissions)).end(); + return res.status(403).json(new PermissionError(permissions)).end(); }; /** diff --git a/src/lib/services/feature-toggle-service.ts b/src/lib/services/feature-toggle-service.ts index ce94261df8..65ec3fde46 100644 --- a/src/lib/services/feature-toggle-service.ts +++ b/src/lib/services/feature-toggle-service.ts @@ -44,7 +44,12 @@ import { Logger } from '../logger'; import BadDataError from '../error/bad-data-error'; import NameExistsError from '../error/name-exists-error'; import InvalidOperationError from '../error/invalid-operation-error'; -import { FOREIGN_KEY_VIOLATION, OperationDeniedError } from '../error'; +import { + FOREIGN_KEY_VIOLATION, + OperationDeniedError, + PermissionError, + ForbiddenError, +} from '../error'; import { constraintSchema, featureMetadataSchema, @@ -79,7 +84,6 @@ import { } from '../features/playground/feature-evaluator/helpers'; import { AccessService } from './access-service'; import { User } from '../server-impl'; -import NoAccessError from '../error/no-access-error'; import { IFeatureProjectUserParams } from '../routes/admin-api/project/project-features'; import { unique } from '../util/unique'; import { ISegmentService } from 'lib/segments/segment-service-interface'; @@ -976,7 +980,7 @@ class FeatureToggleService { projectId, ); if (changeRequestEnabled) { - throw new NoAccessError( + throw new ForbiddenError( `Cloning not allowed. Project ${projectId} has change requests enabled.`, ); } @@ -1517,7 +1521,7 @@ class FeatureToggleService { newProject, ); if (changeRequestEnabled) { - throw new NoAccessError( + throw new ForbiddenError( `Changing project not allowed. Project ${newProject} has change requests enabled.`, ); } @@ -1918,7 +1922,7 @@ class FeatureToggleService { user, ); if (!canBypass) { - throw new NoAccessError(SKIP_CHANGE_REQUEST); + throw new PermissionError(SKIP_CHANGE_REQUEST); } } @@ -1950,7 +1954,7 @@ class FeatureToggleService { environment, )); if (!canAddStrategies) { - throw new NoAccessError(CREATE_FEATURE_STRATEGY); + throw new PermissionError(CREATE_FEATURE_STRATEGY); } } } diff --git a/src/lib/services/project-service.ts b/src/lib/services/project-service.ts index 6d8cac7d3b..fbdecbdc0d 100644 --- a/src/lib/services/project-service.ts +++ b/src/lib/services/project-service.ts @@ -47,7 +47,6 @@ import { IRoleDescriptor, } from '../types/stores/access-store'; import FeatureToggleService from './feature-toggle-service'; -import NoAccessError from '../error/no-access-error'; import IncompatibleProjectError from '../error/incompatible-project-error'; import { IFeatureTagStore } from 'lib/types/stores/feature-tag-store'; import ProjectWithoutOwnerError from '../error/project-without-owner-error'; @@ -58,6 +57,7 @@ import { FavoritesService } from './favorites-service'; import { calculateAverageTimeToProd } from '../features/feature-toggle/time-to-production/time-to-production'; import { IProjectStatsStore } from 'lib/types/stores/project-stats-store-type'; import { uniqueByKey } from '../util/unique'; +import { PermissionError } from '../error'; const getCreatedBy = (user: IUser) => user.email || user.username || 'unknown'; @@ -259,7 +259,7 @@ export default class ProjectService { const feature = await this.featureToggleStore.get(featureName); if (feature.project !== currentProjectId) { - throw new NoAccessError(MOVE_FEATURE_TOGGLE); + throw new PermissionError(MOVE_FEATURE_TOGGLE); } const project = await this.getProject(newProjectId); @@ -274,7 +274,7 @@ export default class ProjectService { ); if (!authorized) { - throw new NoAccessError(MOVE_FEATURE_TOGGLE); + throw new PermissionError(MOVE_FEATURE_TOGGLE); } const isCompatibleWithTargetProject = diff --git a/src/test/e2e/services/access-service.e2e.test.ts b/src/test/e2e/services/access-service.e2e.test.ts index 316b86ab66..bf313aa45c 100644 --- a/src/test/e2e/services/access-service.e2e.test.ts +++ b/src/test/e2e/services/access-service.e2e.test.ts @@ -776,7 +776,7 @@ test('Should be denied move feature toggle to project where the user does not ha projectOrigin.id, ); } catch (e) { - expect(e.name).toContain('NoAccess'); + expect(e.name).toContain('Permission'); expect(e.message.includes('permission')).toBeTruthy(); expect( e.message.includes(permissions.MOVE_FEATURE_TOGGLE), diff --git a/src/test/e2e/services/feature-toggle-service-v2.e2e.test.ts b/src/test/e2e/services/feature-toggle-service-v2.e2e.test.ts index 52bd074b08..f47e0aba3d 100644 --- a/src/test/e2e/services/feature-toggle-service-v2.e2e.test.ts +++ b/src/test/e2e/services/feature-toggle-service-v2.e2e.test.ts @@ -11,7 +11,7 @@ import { FeatureStrategySchema } from '../../../lib/openapi'; import User from '../../../lib/types/user'; import { IConstraint, IVariant, SKIP_CHANGE_REQUEST } from '../../../lib/types'; import EnvironmentService from '../../../lib/services/environment-service'; -import { NoAccessError } from '../../../lib/error'; +import { ForbiddenError, PermissionError } from '../../../lib/error'; import { ISegmentService } from '../../../lib/segments/segment-service-interface'; import { ChangeRequestAccessReadModel } from '../../../lib/features/change-request-access-service/sql-change-request-access-read-model'; @@ -350,7 +350,7 @@ test('cloning a feature toggle not allowed for change requests enabled', async ( 'test-user', ), ).rejects.toEqual( - new NoAccessError( + new ForbiddenError( `Cloning not allowed. Project default has change requests enabled.`, ), ); @@ -364,7 +364,7 @@ test('changing to a project with change requests enabled should not be allowed', await expect( service.changeProject('newToggleName', 'default', 'user'), ).rejects.toEqual( - new NoAccessError( + new ForbiddenError( `Changing project not allowed. Project default has change requests enabled.`, ), ); @@ -484,7 +484,7 @@ test('If change requests are enabled, cannot change variants without going via C }, [], ), - ).rejects.toThrowError(new NoAccessError(SKIP_CHANGE_REQUEST)); + ).rejects.toThrowError(new PermissionError(SKIP_CHANGE_REQUEST)); }); test('If CRs are protected for any environment in the project stops bulk update of variants', async () => { @@ -590,7 +590,7 @@ test('If CRs are protected for any environment in the project stops bulk update isAPI: true, }, ), - ).rejects.toThrowError(new NoAccessError(SKIP_CHANGE_REQUEST)); + ).rejects.toThrowError(new PermissionError(SKIP_CHANGE_REQUEST)); }); test('getPlaygroundFeatures should return ids and titles (if they exist) on client strategies', async () => {