diff --git a/src/lib/routes/admin-api/context.ts b/src/lib/routes/admin-api/context.ts index 4d111085e4..328eb71684 100644 --- a/src/lib/routes/admin-api/context.ts +++ b/src/lib/routes/admin-api/context.ts @@ -41,7 +41,7 @@ class ContextController extends Controller { this.deleteContextField, DELETE_CONTEXT_FIELD, ); - this.post('/validate', this.validate); + this.post('/validate', this.validate, UPDATE_CONTEXT_FIELD); } async getContextFields(req: Request, res: Response): Promise { diff --git a/src/lib/routes/admin-api/feature.ts b/src/lib/routes/admin-api/feature.ts index 24ab695f9e..bc3b00ad7b 100644 --- a/src/lib/routes/admin-api/feature.ts +++ b/src/lib/routes/admin-api/feature.ts @@ -44,7 +44,7 @@ class FeatureController extends Controller { this.get('/:featureName', this.getToggle); this.put('/:featureName', this.updateToggle, UPDATE_FEATURE); this.delete('/:featureName', this.archiveToggle, DELETE_FEATURE); - this.post('/validate', this.validate); + this.post('/validate', this.validate, UPDATE_FEATURE); this.post('/:featureName/toggle', this.toggle, UPDATE_FEATURE); this.post('/:featureName/toggle/on', this.toggleOn, UPDATE_FEATURE); this.post('/:featureName/toggle/off', this.toggleOff, UPDATE_FEATURE); diff --git a/src/lib/routes/admin-api/project/features.ts b/src/lib/routes/admin-api/project/features.ts index bf8c4c8a9e..33ad91e10a 100644 --- a/src/lib/routes/admin-api/project/features.ts +++ b/src/lib/routes/admin-api/project/features.ts @@ -5,7 +5,11 @@ import { IUnleashConfig } from '../../../types/option'; import { IUnleashServices } from '../../../types/services'; import FeatureToggleService from '../../../services/feature-toggle-service'; import { Logger } from '../../../logger'; -import { CREATE_FEATURE, UPDATE_FEATURE } from '../../../types/permissions'; +import { + CREATE_FEATURE, + DELETE_FEATURE, + UPDATE_FEATURE, +} from '../../../types/permissions'; import { FeatureToggleDTO, IConstraint, @@ -84,9 +88,9 @@ export default class ProjectFeaturesController extends Controller { this.post(PATH_FEATURE_CLONE, this.cloneFeature, CREATE_FEATURE); this.get(PATH_FEATURE, this.getFeature); - this.put(PATH_FEATURE, this.updateFeature); - this.patch(PATH_FEATURE, this.patchFeature); - this.delete(PATH_FEATURE, this.archiveFeature); + this.put(PATH_FEATURE, this.updateFeature, UPDATE_FEATURE); + this.patch(PATH_FEATURE, this.patchFeature, UPDATE_FEATURE); + this.delete(PATH_FEATURE, this.archiveFeature, DELETE_FEATURE); } async getFeatures( diff --git a/src/lib/routes/admin-api/tag-type.ts b/src/lib/routes/admin-api/tag-type.ts index cbe7b15a87..f616cdc829 100644 --- a/src/lib/routes/admin-api/tag-type.ts +++ b/src/lib/routes/admin-api/tag-type.ts @@ -25,7 +25,7 @@ class TagTypeController extends Controller { this.tagTypeService = tagTypeService; this.get('/', this.getTagTypes); this.post('/', this.createTagType, UPDATE_TAG_TYPE); - this.post('/validate', this.validate); + this.post('/validate', this.validate, UPDATE_TAG_TYPE); this.get('/:name', this.getTagType); this.put('/:name', this.updateTagType, UPDATE_TAG_TYPE); this.delete('/:name', this.deleteTagType, DELETE_TAG_TYPE); diff --git a/src/lib/routes/admin-api/user-admin.ts b/src/lib/routes/admin-api/user-admin.ts index 590e506c78..147253e200 100644 --- a/src/lib/routes/admin-api/user-admin.ts +++ b/src/lib/routes/admin-api/user-admin.ts @@ -1,6 +1,6 @@ import { Request, Response } from 'express'; import Controller from '../controller'; -import { ADMIN } from '../../types/permissions'; +import { ADMIN, NONE } from '../../types/permissions'; import UserService from '../../services/user-service'; import { AccessService } from '../../services/access-service'; import { Logger } from '../../logger'; @@ -60,12 +60,12 @@ export default class UserAdminController extends Controller { this.get('/', this.getUsers, ADMIN); this.get('/search', this.search); this.post('/', this.createUser, ADMIN); - this.post('/validate-password', this.validatePassword); + this.post('/validate-password', this.validatePassword, NONE); this.get('/:id', this.getUser, ADMIN); this.put('/:id', this.updateUser, ADMIN); this.post('/:id/change-password', this.changePassword, ADMIN); this.delete('/:id', this.deleteUser, ADMIN); - this.post('/reset-password', this.resetPassword); + this.post('/reset-password', this.resetPassword, ADMIN); this.get('/active-sessions', this.getActiveSessions, ADMIN); } diff --git a/src/lib/routes/admin-api/user-feedback-controller.ts b/src/lib/routes/admin-api/user-feedback-controller.ts index d8c21e04c1..46dcbe2c5f 100644 --- a/src/lib/routes/admin-api/user-feedback-controller.ts +++ b/src/lib/routes/admin-api/user-feedback-controller.ts @@ -6,6 +6,7 @@ import { IUnleashConfig } from '../../types/option'; import { IUnleashServices } from '../../types/services'; import UserFeedbackService from '../../services/user-feedback-service'; import { IAuthRequest } from '../unleash-types'; +import { NONE } from '../../types/permissions'; interface IFeedbackBody { neverShow?: boolean; @@ -26,8 +27,8 @@ class UserFeedbackController extends Controller { this.logger = config.getLogger('feedback-controller.ts'); this.userFeedbackService = userFeedbackService; - this.post('/', this.recordFeedback); - this.put('/:id', this.updateFeedbackSettings); + this.post('/', this.recordFeedback, NONE); + this.put('/:id', this.updateFeedbackSettings, NONE); } private async recordFeedback( diff --git a/src/lib/routes/admin-api/user-splash-controller.ts b/src/lib/routes/admin-api/user-splash-controller.ts index 07f3b7a049..d0f21c8fe8 100644 --- a/src/lib/routes/admin-api/user-splash-controller.ts +++ b/src/lib/routes/admin-api/user-splash-controller.ts @@ -6,6 +6,7 @@ import { IUnleashConfig } from '../../types/option'; import { IUnleashServices } from '../../types/services'; import UserSplashService from '../../services/user-splash-service'; import { IAuthRequest } from '../unleash-types'; +import { NONE } from '../../types/permissions'; interface ISplashBody { seen: boolean; @@ -25,7 +26,7 @@ class UserSplashController extends Controller { this.logger = config.getLogger('splash-controller.ts'); this.userSplashService = userSplashService; - this.post('/:id', this.updateSplashSettings); + this.post('/:id', this.updateSplashSettings, NONE); } private async updateSplashSettings( diff --git a/src/lib/routes/admin-api/user.ts b/src/lib/routes/admin-api/user.ts index 02d162484e..ef98e01e53 100644 --- a/src/lib/routes/admin-api/user.ts +++ b/src/lib/routes/admin-api/user.ts @@ -8,6 +8,7 @@ import UserService from '../../services/user-service'; import SessionService from '../../services/session-service'; import UserFeedbackService from '../../services/user-feedback-service'; import UserSplashService from '../../services/user-splash-service'; +import { NONE } from '../../types/permissions'; interface IChangeUserRequest { password: string; @@ -50,7 +51,7 @@ class UserController extends Controller { this.userSplashService = userSplashService; this.get('/', this.getUser); - this.post('/change-password', this.updateUserPass); + this.post('/change-password', this.updateUserPass, NONE); this.get('/my-sessions', this.mySessions); } diff --git a/src/lib/routes/auth/reset-password-controller.ts b/src/lib/routes/auth/reset-password-controller.ts index 5445cb0883..ae2f72d0c8 100644 --- a/src/lib/routes/auth/reset-password-controller.ts +++ b/src/lib/routes/auth/reset-password-controller.ts @@ -4,6 +4,7 @@ import UserService from '../../services/user-service'; import { Logger } from '../../logger'; import { IUnleashConfig } from '../../types/option'; import { IUnleashServices } from '../../types/services'; +import { NONE } from '../../types/permissions'; interface IValidateQuery { token: string; @@ -31,9 +32,9 @@ class ResetPasswordController extends Controller { ); this.userService = userService; this.get('/validate', this.validateToken); - this.post('/password', this.changePassword); - this.post('/validate-password', this.validatePassword); - this.post('/password-email', this.sendResetPasswordEmail); + this.post('/password', this.changePassword, NONE); + this.post('/validate-password', this.validatePassword, NONE); + this.post('/password-email', this.sendResetPasswordEmail, NONE); } async sendResetPasswordEmail(req: Request, res: Response): Promise { diff --git a/src/lib/routes/client-api/metrics.ts b/src/lib/routes/client-api/metrics.ts index 9cbc3a7175..555fee530c 100644 --- a/src/lib/routes/client-api/metrics.ts +++ b/src/lib/routes/client-api/metrics.ts @@ -10,6 +10,7 @@ import { ALL } from '../../types/models/api-token'; import ClientMetricsServiceV2 from '../../services/client-metrics/client-metrics-service-v2'; import { User } from '../../server-impl'; import { IClientApp } from '../../types/model'; +import { NONE } from '../../types/permissions'; export default class ClientMetricsController extends Controller { logger: Logger; @@ -35,7 +36,7 @@ export default class ClientMetricsController extends Controller { this.metrics = clientMetricsService; this.metricsV2 = clientMetricsServiceV2; - this.post('/', this.registerMetrics); + this.post('/', this.registerMetrics, NONE); } private resolveEnvironment(user: User, data: IClientApp) { diff --git a/src/lib/routes/client-api/register.ts b/src/lib/routes/client-api/register.ts index 86f3b84873..189ad81adf 100644 --- a/src/lib/routes/client-api/register.ts +++ b/src/lib/routes/client-api/register.ts @@ -8,6 +8,7 @@ import { IAuthRequest, User } from '../../server-impl'; import { IClientApp } from '../../types/model'; import ApiUser from '../../types/api-user'; import { ALL } from '../../types/models/api-token'; +import { NONE } from '../../types/permissions'; export default class RegisterController extends Controller { logger: Logger; @@ -23,7 +24,9 @@ export default class RegisterController extends Controller { super(config); this.logger = config.getLogger('/api/client/register'); this.metrics = clientMetricsService; - this.post('/', this.handleRegister); + + // NONE permission is not optimal here in terms of readability. + this.post('/', this.handleRegister, NONE); } private resolveEnvironment(user: User, data: IClientApp) { diff --git a/src/lib/routes/controller.ts b/src/lib/routes/controller.ts index d18d977cb8..1caef74ce0 100644 --- a/src/lib/routes/controller.ts +++ b/src/lib/routes/controller.ts @@ -1,11 +1,10 @@ -import { IRouter, Request, Response } from 'express'; +import { IRouter, Router, Request, Response } from 'express'; import { Logger } from 'lib/logger'; import { IUnleashConfig } from '../types/option'; +import { NONE } from '../types/permissions'; import { handleErrors } from './util'; - -const { Router } = require('express'); -const NoAccessError = require('../error/no-access-error'); -const requireContentType = require('../middleware/content_type_checker'); +import NoAccessError from '../error/no-access-error'; +import requireContentType from '../middleware/content_type_checker'; interface IRequestHandler< P = any, @@ -20,7 +19,7 @@ interface IRequestHandler< } const checkPermission = (permission) => async (req, res, next) => { - if (!permission) { + if (!permission || permission === NONE) { return next(); } if (req.checkRbac && (await req.checkRbac(permission))) { @@ -73,7 +72,7 @@ export default class Controller { post( path: string, handler: IRequestHandler, - permission?: string, + permission: string, ...acceptedContentTypes: string[] ): void { this.app.post( @@ -87,7 +86,7 @@ export default class Controller { put( path: string, handler: IRequestHandler, - permission?: string, + permission: string, ...acceptedContentTypes: string[] ): void { this.app.put( @@ -101,7 +100,7 @@ export default class Controller { patch( path: string, handler: IRequestHandler, - permission?: string, + permission: string, ...acceptedContentTypes: string[] ): void { this.app.patch( @@ -112,7 +111,7 @@ export default class Controller { ); } - delete(path: string, handler: IRequestHandler, permission?: string): void { + delete(path: string, handler: IRequestHandler, permission: string): void { this.app.delete( path, checkPermission(permission), @@ -124,7 +123,7 @@ export default class Controller { path: string, filehandler: IRequestHandler, handler: Function, - permission?: string, + permission: string, ): void { this.app.post( path, diff --git a/src/lib/types/permissions.ts b/src/lib/types/permissions.ts index 46925e5f9e..0e0efe92f7 100644 --- a/src/lib/types/permissions.ts +++ b/src/lib/types/permissions.ts @@ -1,5 +1,8 @@ +// Special export const ADMIN = 'ADMIN'; export const CLIENT = 'CLIENT'; +export const NONE = 'NONE'; + export const CREATE_FEATURE = 'CREATE_FEATURE'; export const UPDATE_FEATURE = 'UPDATE_FEATURE'; export const DELETE_FEATURE = 'DELETE_FEATURE'; diff --git a/src/test/e2e/api/admin/project/features.auth.e2e.test.ts b/src/test/e2e/api/admin/project/features.auth.e2e.test.ts new file mode 100644 index 0000000000..a60442d7bc --- /dev/null +++ b/src/test/e2e/api/admin/project/features.auth.e2e.test.ts @@ -0,0 +1,78 @@ +import dbInit, { ITestDb } from '../../../helpers/database-init'; +import { IUnleashTest, setupAppWithAuth } from '../../../helpers/test-helper'; +import getLogger from '../../../../fixtures/no-logger'; +import { DEFAULT_ENV } from '../../../../../lib/util/constants'; +import { RoleName } from '../../../../../lib/server-impl'; + +let app: IUnleashTest; +let db: ITestDb; + +beforeAll(async () => { + db = await dbInit('feature_strategy_auth_api_serial', getLogger); + app = await setupAppWithAuth(db.stores); +}); + +afterEach(async () => { + const all = await db.stores.projectStore.getEnvironmentsForProject( + 'default', + ); + await Promise.all( + all + .filter((env) => env !== DEFAULT_ENV) + .map(async (env) => + db.stores.projectStore.deleteEnvironmentForProject( + 'default', + env, + ), + ), + ); +}); + +afterAll(async () => { + await app.destroy(); + await db.destroy(); +}); + +test('Should not be possible to update feature toggle without permission', async () => { + const email = 'user@mail.com'; + const url = '/api/admin/projects/default/features'; + const name = 'auth.toggle.update'; + + await db.stores.featureToggleStore.create('default', { name }); + + await app.services.userService.createUser({ + email, + rootRole: RoleName.VIEWER, + }); + + await app.request.post('/auth/demo/login').send({ + email, + }); + + await app.request + .put(`${url}/${name}`) + .send({ name, description: 'updated', type: 'kill-switch' }) + .expect(403); +}); + +test('Should be possible to update feature toggle with permission', async () => { + const email = 'user2@mail.com'; + const url = '/api/admin/projects/default/features'; + const name = 'auth.toggle.update2'; + + await db.stores.featureToggleStore.create('default', { name }); + + await app.services.userService.createUser({ + email, + rootRole: RoleName.EDITOR, + }); + + await app.request.post('/auth/demo/login').send({ + email, + }); + + await app.request + .put(`${url}/${name}`) + .send({ name, description: 'updated', type: 'kill-switch' }) + .expect(200); +});