mirror of
https://github.com/Unleash/unleash.git
synced 2025-04-24 01:18:01 +02:00
fix: reject API admin tokens when importing features (#4016)
This PR fixes an issue where trying to use an admin token to import features via the API resulted in a 500 (due to missing properties). The solution is to catch when the user is using and admin token in the controller, throw a 400, and tell them to use personal access tokens or service accounts. The PR includes a new test file for this specific use case. We don't really test these cases many other places, so it seemed the logical choice.
This commit is contained in:
parent
a0862cfc10
commit
80a2e1b93f
@ -0,0 +1,76 @@
|
|||||||
|
import { setupAppWithCustomAuth } from '../../../test/e2e/helpers/test-helper';
|
||||||
|
import dbInit, { ITestDb } from '../../../test/e2e/helpers/database-init';
|
||||||
|
import getLogger from '../../../test/fixtures/no-logger';
|
||||||
|
import { DEFAULT_PROJECT } from '../../types';
|
||||||
|
import { DEFAULT_ENV } from '../../util';
|
||||||
|
import { ImportTogglesSchema } from '../../openapi';
|
||||||
|
import { ApiTokenType } from '../../types/models/api-token';
|
||||||
|
import { ApiUser } from '../../server-impl';
|
||||||
|
|
||||||
|
let db: ITestDb;
|
||||||
|
|
||||||
|
beforeAll(async () => {
|
||||||
|
db = await dbInit('export_import_api_admin', getLogger);
|
||||||
|
});
|
||||||
|
|
||||||
|
afterAll(async () => {
|
||||||
|
await db.destroy();
|
||||||
|
});
|
||||||
|
|
||||||
|
const defaultImportPayload: ImportTogglesSchema = {
|
||||||
|
data: {
|
||||||
|
features: [
|
||||||
|
{
|
||||||
|
project: 'old_project',
|
||||||
|
name: 'first_feature',
|
||||||
|
},
|
||||||
|
],
|
||||||
|
featureStrategies: [],
|
||||||
|
featureEnvironments: [],
|
||||||
|
featureTags: [],
|
||||||
|
tagTypes: [],
|
||||||
|
contextFields: [],
|
||||||
|
segments: [],
|
||||||
|
},
|
||||||
|
project: DEFAULT_PROJECT,
|
||||||
|
environment: DEFAULT_ENV,
|
||||||
|
};
|
||||||
|
|
||||||
|
test('reject API imports with admin tokens', async () => {
|
||||||
|
const preHook = (app: any) => {
|
||||||
|
app.use('/api/admin/', async (req, res, next) => {
|
||||||
|
const user = new ApiUser({
|
||||||
|
permissions: ['ADMIN'],
|
||||||
|
environment: '*',
|
||||||
|
type: ApiTokenType.ADMIN,
|
||||||
|
tokenName: 'tokenName',
|
||||||
|
secret: 'secret',
|
||||||
|
projects: ['*'],
|
||||||
|
});
|
||||||
|
req.user = user;
|
||||||
|
next();
|
||||||
|
});
|
||||||
|
};
|
||||||
|
|
||||||
|
const { request, destroy } = await setupAppWithCustomAuth(
|
||||||
|
db.stores,
|
||||||
|
preHook,
|
||||||
|
);
|
||||||
|
|
||||||
|
const { body } = await request
|
||||||
|
.post('/api/admin/features-batch/import')
|
||||||
|
.send(defaultImportPayload)
|
||||||
|
.expect(400);
|
||||||
|
|
||||||
|
expect(body).toMatchObject({
|
||||||
|
message:
|
||||||
|
// it tells the user that they used an admin token
|
||||||
|
expect.stringContaining('admin') &&
|
||||||
|
// it tells the user to use a personal access token
|
||||||
|
expect.stringContaining('personal access token') &&
|
||||||
|
// it tells the user to use a service account
|
||||||
|
expect.stringContaining('service account'),
|
||||||
|
});
|
||||||
|
|
||||||
|
await destroy();
|
||||||
|
});
|
@ -21,7 +21,8 @@ import {
|
|||||||
} from '../../openapi';
|
} from '../../openapi';
|
||||||
import { IAuthRequest } from '../../routes/unleash-types';
|
import { IAuthRequest } from '../../routes/unleash-types';
|
||||||
import { extractUsername } from '../../util';
|
import { extractUsername } from '../../util';
|
||||||
import { InvalidOperationError } from '../../error';
|
import { BadDataError, InvalidOperationError } from '../../error';
|
||||||
|
import ApiUser from '../../types/api-user';
|
||||||
|
|
||||||
class ExportImportController extends Controller {
|
class ExportImportController extends Controller {
|
||||||
private logger: Logger;
|
private logger: Logger;
|
||||||
@ -156,8 +157,16 @@ class ExportImportController extends Controller {
|
|||||||
res: Response,
|
res: Response,
|
||||||
): Promise<void> {
|
): Promise<void> {
|
||||||
this.verifyExportImportEnabled();
|
this.verifyExportImportEnabled();
|
||||||
const dto = req.body;
|
|
||||||
const { user } = req;
|
const { user } = req;
|
||||||
|
|
||||||
|
if (user instanceof ApiUser && user.type === 'admin') {
|
||||||
|
throw new BadDataError(
|
||||||
|
`You can't use an admin token to import features. Please use either a personal access token (https://docs.getunleash.io/reference/api-tokens-and-client-keys#personal-access-tokens) or a service account (https://docs.getunleash.io/reference/service-accounts).`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
const dto = req.body;
|
||||||
|
|
||||||
await this.startTransaction(async (tx) =>
|
await this.startTransaction(async (tx) =>
|
||||||
this.transactionalExportImportService(tx).import(dto, user),
|
this.transactionalExportImportService(tx).import(dto, user),
|
||||||
);
|
);
|
||||||
|
Loading…
Reference in New Issue
Block a user