diff --git a/src/lib/routes/admin-api/api-token-controller.ts b/src/lib/routes/admin-api/api-token-controller.ts index bc212bb561..9cdc3f8577 100644 --- a/src/lib/routes/admin-api/api-token-controller.ts +++ b/src/lib/routes/admin-api/api-token-controller.ts @@ -5,6 +5,7 @@ import { ADMIN, CREATE_API_TOKEN, DELETE_API_TOKEN, + READ_API_TOKEN, UPDATE_API_TOKEN, } from '../../types/permissions'; import { ApiTokenService } from '../../services/api-token-service'; @@ -13,7 +14,7 @@ import { AccessService } from '../../services/access-service'; import { IAuthRequest } from '../unleash-types'; import User from '../../types/user'; import { IUnleashConfig } from '../../types/option'; -import { ApiTokenType } from '../../types/models/api-token'; +import { ApiTokenType, IApiToken } from '../../types/models/api-token'; import { createApiToken } from '../../schema/api-token-schema'; interface IServices { @@ -34,34 +35,16 @@ class ApiTokenController extends Controller { this.accessService = services.accessService; this.logger = config.getLogger('api-token-controller.js'); - this.get('/', this.getAllApiTokens); + this.get('/', this.getAllApiTokens, READ_API_TOKEN); this.post('/', this.createApiToken, CREATE_API_TOKEN); this.put('/:token', this.updateApiToken, UPDATE_API_TOKEN); this.delete('/:token', this.deleteApiToken, DELETE_API_TOKEN); } - private async isTokenAdmin(user: User) { - if (user.isAPI) { - return user.permissions.includes(ADMIN); - } - - return this.accessService.hasPermission(user, UPDATE_API_TOKEN); - } - async getAllApiTokens(req: IAuthRequest, res: Response): Promise { const { user } = req; - const isAdmin = await this.isTokenAdmin(user); - - const tokens = await this.apiTokenService.getAllTokens(); - - if (isAdmin) { - res.json({ tokens }); - } else { - const filteredTokens = tokens.filter( - (t) => !(t.type === ApiTokenType.ADMIN), - ); - res.json({ tokens: filteredTokens }); - } + const tokens = await this.accessibleTokens(user); + res.json({ tokens }); } async createApiToken(req: IAuthRequest, res: Response): Promise { @@ -89,6 +72,20 @@ class ApiTokenController extends Controller { await this.apiTokenService.updateExpiry(token, expiresAt); return res.status(200).end(); } + + private async accessibleTokens(user: User): Promise { + const allTokens = await this.apiTokenService.getAllTokens(); + + if (user.isAPI && user.permissions.includes(ADMIN)) { + return allTokens; + } + + if (await this.accessService.hasPermission(user, UPDATE_API_TOKEN)) { + return allTokens; + } + + return allTokens.filter((t) => t.type !== ApiTokenType.ADMIN); + } } module.exports = ApiTokenController; diff --git a/src/lib/types/permissions.ts b/src/lib/types/permissions.ts index ca6bb9744b..10486656a2 100644 --- a/src/lib/types/permissions.ts +++ b/src/lib/types/permissions.ts @@ -28,6 +28,7 @@ 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_TAG_TYPE = 'UPDATE_TAG_TYPE'; export const DELETE_TAG_TYPE = 'DELETE_TAG_TYPE'; export const UPDATE_FEATURE_VARIANTS = 'UPDATE_FEATURE_VARIANTS'; diff --git a/src/migrations/20220425090847-add-token-permission.js b/src/migrations/20220425090847-add-token-permission.js new file mode 100644 index 0000000000..26ed03df04 --- /dev/null +++ b/src/migrations/20220425090847-add-token-permission.js @@ -0,0 +1,38 @@ +'use strict'; + +exports.up = function (db, cb) { + db.runSql( + ` + INSERT INTO permissions (permission, display_name, type) VALUES + ('READ_API_TOKEN', 'Read API token', 'root'); + + INSERT INTO role_permission (role_id, permission_id) + SELECT + r.id AS role_id, + p.id AS permission_id + FROM roles r + JOIN permissions p ON p.permission = 'READ_API_TOKEN' + WHERE r.name IN ('Admin', 'Editor'); + + UPDATE roles SET description = 'Users with this role can only read root resources in Unleash. The viewer can be added to specific projects as project member. Viewers may not view API tokens.' + WHERE name = 'Viewer' + `, + cb, + ); +}; + +exports.down = function (db, cb) { + db.runSql( + ` + DELETE FROM role_permission WHERE permission_id = ( + SELECT id FROM permissions WHERE permission = 'READ_API_TOKEN' + ); + + DELETE FROM permissions WHERE permission = 'READ_API_TOKEN'; + + UPDATE roles SET description = 'Users with this role can only read root resources in Unleash. The viewer can be added to specific projects as project member.' + WHERE name = 'Viewer' + `, + cb, + ); +}; diff --git a/src/test/e2e/api/admin/api-token.auth.e2e.test.ts b/src/test/e2e/api/admin/api-token.auth.e2e.test.ts index a48117c040..0c1803e019 100644 --- a/src/test/e2e/api/admin/api-token.auth.e2e.test.ts +++ b/src/test/e2e/api/admin/api-token.auth.e2e.test.ts @@ -22,16 +22,14 @@ afterEach(async () => { await stores.apiTokenStore.deleteAll(); }); -test('none-admins should only get client tokens', async () => { +test('editor users should only get client tokens', async () => { expect.assertions(2); - const email = 'custom-user@mail.com'; - 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, + email: 'editor@example.com', rootRole: role.id, }); req.user = user; @@ -65,16 +63,51 @@ test('none-admins should only get client tokens', async () => { await destroy(); }); -test('Only token-admins should be allowed to create token', async () => { +test('viewer users should not be allowed to fetch tokens', async () => { expect.assertions(0); - const email = 'custom-user2@mail.com'; + const preHook = (app, config, { userService, accessService }) => { + app.use('/api/admin/', async (req, res, next) => { + const role = await accessService.getRootRole(RoleName.VIEWER); + const user = await userService.createUser({ + email: 'viewer@example.com', + rootRole: role.id, + }); + req.user = user; + next(); + }); + }; + + const { request, destroy } = await setupAppWithCustomAuth(stores, preHook); + + await stores.apiTokenStore.insert({ + username: 'test', + secret: '1234', + type: ApiTokenType.CLIENT, + }); + + await stores.apiTokenStore.insert({ + username: 'test', + secret: 'sdfsdf2d', + type: ApiTokenType.ADMIN, + }); + + await request + .get('/api/admin/api-tokens') + .expect('Content-Type', /json/) + .expect(403); + + await destroy(); +}); + +test('Only token-admins should be allowed to create token', async () => { + expect.assertions(0); const preHook = (app, config, { userService, accessService }) => { app.use('/api/admin/', async (req, res, next) => { const role = await accessService.getRootRole(RoleName.EDITOR); req.user = await userService.createUser({ - email, + email: 'editor2@example.com', rootRole: role.id, }); next(); @@ -97,13 +130,12 @@ test('Only token-admins should be allowed to create token', async () => { test('Token-admin should be allowed to create token', async () => { expect.assertions(0); - const email = 'custom-user3@mail.com'; const preHook = (app, config, { userService, accessService }) => { app.use('/api/admin/', async (req, res, next) => { const role = await accessService.getRootRole(RoleName.ADMIN); req.user = await userService.createUser({ - email, + email: 'admin@example.com', rootRole: role.id, }); next();