2023-07-06 09:30:31 +02:00
|
|
|
import { setupAppWithCustomConfig } from '../../helpers/test-helper';
|
2021-03-29 19:58:11 +02:00
|
|
|
import dbInit from '../../helpers/database-init';
|
|
|
|
import getLogger from '../../../fixtures/no-logger';
|
2021-09-15 20:28:10 +02:00
|
|
|
import { ALL, ApiTokenType } from '../../../../lib/types/models/api-token';
|
2023-03-29 09:33:14 +02:00
|
|
|
import { DEFAULT_ENV } from '../../../../lib/util';
|
feat: Separate api token roles (#4019)
## What
As part of the move to enable custom-root-roles, our permissions model
was found to not be granular enough to allow service accounts to only be
allowed to create read-only tokens (client, frontend), but not be
allowed to create admin tokens to avoid opening up a path for privilege
escalation.
## How
This PR adds 12 new roles, a CRUD set for each of the three token types
(admin, client, frontend). To access the `/api/admin/api-tokens`
endpoints you will still need the existing permission (CREATE_API_TOKEN,
DELETE_API_TOKEN, READ_API_TOKEN, UPDATE_API_TOKEN). Once this PR has
been merged the token type you're modifying will also be checked, so if
you're trying to create a CLIENT api-token, you will need
`CREATE_API_TOKEN` and `CREATE_CLIENT_API_TOKEN` permissions. If the
user performing the create call does not have these two permissions or
the `ADMIN` permission, the creation will be rejected with a `403 -
FORBIDDEN` status.
### Discussion points
The test suite tests all operations using a token with
operation_CLIENT_API_TOKEN permission and verifies that it fails trying
to do any of the operations against FRONTEND and ADMIN tokens. During
development the operation_FRONTEND_API_TOKEN and
operation_ADMIN_API_TOKEN permission has also been tested in the same
way. I wonder if it's worth it to re-add these tests in order to verify
that the permission checker works for all operations, or if this is
enough. Since we're running them using e2e tests, I've removed them for
now, to avoid hogging too much processing time.
2023-06-20 14:21:14 +02:00
|
|
|
import { addDays } from 'date-fns';
|
2021-03-29 19:58:11 +02:00
|
|
|
|
|
|
|
let db;
|
2021-05-28 11:10:24 +02:00
|
|
|
let app;
|
2021-03-29 19:58:11 +02:00
|
|
|
|
2021-05-28 11:10:24 +02:00
|
|
|
beforeAll(async () => {
|
2021-03-29 19:58:11 +02:00
|
|
|
db = await dbInit('token_api_serial', getLogger);
|
2023-07-06 09:30:31 +02:00
|
|
|
app = await setupAppWithCustomConfig(db.stores, {
|
|
|
|
experimental: {
|
|
|
|
flags: {
|
|
|
|
strictSchemaValidation: true,
|
|
|
|
},
|
|
|
|
},
|
|
|
|
});
|
2021-03-29 19:58:11 +02:00
|
|
|
});
|
|
|
|
|
2021-05-28 11:10:24 +02:00
|
|
|
afterAll(async () => {
|
|
|
|
if (db) {
|
|
|
|
await db.destroy();
|
|
|
|
}
|
|
|
|
await app.destroy();
|
2021-03-29 19:58:11 +02:00
|
|
|
});
|
|
|
|
|
2021-05-28 11:10:24 +02:00
|
|
|
afterEach(async () => {
|
|
|
|
await db.stores.apiTokenStore.deleteAll();
|
2021-03-29 19:58:11 +02:00
|
|
|
});
|
|
|
|
|
2021-05-28 11:10:24 +02:00
|
|
|
test('returns empty list of tokens', async () => {
|
|
|
|
return app.request
|
2021-03-29 19:58:11 +02:00
|
|
|
.get('/api/admin/api-tokens')
|
|
|
|
.expect('Content-Type', /json/)
|
|
|
|
.expect(200)
|
2021-08-12 15:04:37 +02:00
|
|
|
.expect((res) => {
|
2021-05-28 11:10:24 +02:00
|
|
|
expect(res.body.tokens.length).toBe(0);
|
2021-03-29 19:58:11 +02:00
|
|
|
});
|
|
|
|
});
|
|
|
|
|
2021-05-28 11:10:24 +02:00
|
|
|
test('creates new client token', async () => {
|
|
|
|
return app.request
|
2021-03-29 19:58:11 +02:00
|
|
|
.post('/api/admin/api-tokens')
|
|
|
|
.send({
|
|
|
|
username: 'default-client',
|
|
|
|
type: 'client',
|
|
|
|
})
|
|
|
|
.set('Content-Type', 'application/json')
|
|
|
|
.expect(201)
|
2021-08-12 15:04:37 +02:00
|
|
|
.expect((res) => {
|
2021-05-28 11:10:24 +02:00
|
|
|
expect(res.body.username).toBe('default-client');
|
2023-05-04 09:56:00 +02:00
|
|
|
expect(res.body.tokenName).toBe(res.body.username);
|
2021-05-28 11:10:24 +02:00
|
|
|
expect(res.body.type).toBe('client');
|
|
|
|
expect(res.body.createdAt).toBeTruthy();
|
|
|
|
expect(res.body.secret.length > 16).toBe(true);
|
2021-03-29 19:58:11 +02:00
|
|
|
});
|
|
|
|
});
|
|
|
|
|
2021-05-28 11:10:24 +02:00
|
|
|
test('creates new admin token', async () => {
|
|
|
|
return app.request
|
2021-03-29 19:58:11 +02:00
|
|
|
.post('/api/admin/api-tokens')
|
|
|
|
.send({
|
|
|
|
username: 'default-admin',
|
|
|
|
type: 'admin',
|
|
|
|
})
|
|
|
|
.set('Content-Type', 'application/json')
|
|
|
|
.expect(201)
|
2021-08-12 15:04:37 +02:00
|
|
|
.expect((res) => {
|
2021-05-28 11:10:24 +02:00
|
|
|
expect(res.body.username).toBe('default-admin');
|
2023-05-04 09:56:00 +02:00
|
|
|
expect(res.body.tokenName).toBe(res.body.username);
|
2021-05-28 11:10:24 +02:00
|
|
|
expect(res.body.type).toBe('admin');
|
2021-09-24 13:55:00 +02:00
|
|
|
expect(res.body.environment).toBe(ALL);
|
2021-05-28 11:10:24 +02:00
|
|
|
expect(res.body.createdAt).toBeTruthy();
|
|
|
|
expect(res.body.expiresAt).toBeFalsy();
|
|
|
|
expect(res.body.secret.length > 16).toBe(true);
|
2021-03-29 19:58:11 +02:00
|
|
|
});
|
|
|
|
});
|
|
|
|
|
2021-09-15 20:28:10 +02:00
|
|
|
test('creates new ADMIN token should fix casing', async () => {
|
|
|
|
return app.request
|
|
|
|
.post('/api/admin/api-tokens')
|
|
|
|
.send({
|
|
|
|
username: 'default-admin',
|
|
|
|
type: 'ADMIN',
|
|
|
|
})
|
|
|
|
.set('Content-Type', 'application/json')
|
|
|
|
.expect(201)
|
|
|
|
.expect((res) => {
|
|
|
|
expect(res.body.username).toBe('default-admin');
|
2023-05-04 09:56:00 +02:00
|
|
|
expect(res.body.tokenName).toBe(res.body.username);
|
2021-09-15 20:28:10 +02:00
|
|
|
expect(res.body.type).toBe('admin');
|
|
|
|
expect(res.body.createdAt).toBeTruthy();
|
|
|
|
expect(res.body.expiresAt).toBeFalsy();
|
|
|
|
expect(res.body.secret.length > 16).toBe(true);
|
|
|
|
});
|
|
|
|
});
|
|
|
|
|
2021-05-28 11:10:24 +02:00
|
|
|
test('creates new admin token with expiry', async () => {
|
2021-03-29 19:58:11 +02:00
|
|
|
const expiresAt = new Date();
|
|
|
|
const expiresAtAsISOStr = JSON.parse(JSON.stringify(expiresAt));
|
2021-05-28 11:10:24 +02:00
|
|
|
return app.request
|
2021-03-29 19:58:11 +02:00
|
|
|
.post('/api/admin/api-tokens')
|
|
|
|
.send({
|
|
|
|
username: 'default-admin',
|
|
|
|
type: 'admin',
|
|
|
|
expiresAt,
|
|
|
|
})
|
|
|
|
.set('Content-Type', 'application/json')
|
|
|
|
.expect(201)
|
2021-08-12 15:04:37 +02:00
|
|
|
.expect((res) => {
|
2021-05-28 11:10:24 +02:00
|
|
|
expect(res.body.expiresAt).toBe(expiresAtAsISOStr);
|
2021-03-29 19:58:11 +02:00
|
|
|
});
|
|
|
|
});
|
|
|
|
|
feat: Separate api token roles (#4019)
## What
As part of the move to enable custom-root-roles, our permissions model
was found to not be granular enough to allow service accounts to only be
allowed to create read-only tokens (client, frontend), but not be
allowed to create admin tokens to avoid opening up a path for privilege
escalation.
## How
This PR adds 12 new roles, a CRUD set for each of the three token types
(admin, client, frontend). To access the `/api/admin/api-tokens`
endpoints you will still need the existing permission (CREATE_API_TOKEN,
DELETE_API_TOKEN, READ_API_TOKEN, UPDATE_API_TOKEN). Once this PR has
been merged the token type you're modifying will also be checked, so if
you're trying to create a CLIENT api-token, you will need
`CREATE_API_TOKEN` and `CREATE_CLIENT_API_TOKEN` permissions. If the
user performing the create call does not have these two permissions or
the `ADMIN` permission, the creation will be rejected with a `403 -
FORBIDDEN` status.
### Discussion points
The test suite tests all operations using a token with
operation_CLIENT_API_TOKEN permission and verifies that it fails trying
to do any of the operations against FRONTEND and ADMIN tokens. During
development the operation_FRONTEND_API_TOKEN and
operation_ADMIN_API_TOKEN permission has also been tested in the same
way. I wonder if it's worth it to re-add these tests in order to verify
that the permission checker works for all operations, or if this is
enough. Since we're running them using e2e tests, I've removed them for
now, to avoid hogging too much processing time.
2023-06-20 14:21:14 +02:00
|
|
|
test('update client token with expiry', async () => {
|
2021-03-29 19:58:11 +02:00
|
|
|
const tokenSecret = 'random-secret-update';
|
|
|
|
|
2021-05-28 11:10:24 +02:00
|
|
|
await db.stores.apiTokenStore.insert({
|
2021-03-29 19:58:11 +02:00
|
|
|
username: 'test',
|
|
|
|
secret: tokenSecret,
|
|
|
|
type: ApiTokenType.CLIENT,
|
|
|
|
});
|
|
|
|
|
2021-05-28 11:10:24 +02:00
|
|
|
await app.request
|
2021-03-29 19:58:11 +02:00
|
|
|
.put(`/api/admin/api-tokens/${tokenSecret}`)
|
|
|
|
.send({
|
|
|
|
expiresAt: new Date(),
|
|
|
|
})
|
|
|
|
.set('Content-Type', 'application/json')
|
|
|
|
.expect(200);
|
|
|
|
|
2021-05-28 11:10:24 +02:00
|
|
|
return app.request
|
2021-03-29 19:58:11 +02:00
|
|
|
.get('/api/admin/api-tokens')
|
|
|
|
.expect('Content-Type', /json/)
|
|
|
|
.expect(200)
|
2021-08-12 15:04:37 +02:00
|
|
|
.expect((res) => {
|
2021-05-28 11:10:24 +02:00
|
|
|
expect(res.body.tokens.length).toBe(1);
|
|
|
|
expect(res.body.tokens[0].expiresAt).toBeTruthy();
|
2021-03-29 19:58:11 +02:00
|
|
|
});
|
|
|
|
});
|
|
|
|
|
2021-05-28 11:10:24 +02:00
|
|
|
test('creates a lot of client tokens', async () => {
|
2023-04-28 13:59:04 +02:00
|
|
|
const requests: any[] = [];
|
2021-03-29 19:58:11 +02:00
|
|
|
|
|
|
|
for (let i = 0; i < 10; i++) {
|
|
|
|
requests.push(
|
2021-05-28 11:10:24 +02:00
|
|
|
app.request
|
2021-03-29 19:58:11 +02:00
|
|
|
.post('/api/admin/api-tokens')
|
|
|
|
.send({
|
|
|
|
username: 'default-client',
|
|
|
|
type: 'client',
|
|
|
|
})
|
|
|
|
.set('Content-Type', 'application/json')
|
|
|
|
.expect(201),
|
|
|
|
);
|
|
|
|
}
|
|
|
|
await Promise.all(requests);
|
2021-05-28 11:10:24 +02:00
|
|
|
expect.assertions(2);
|
|
|
|
return app.request
|
2021-03-29 19:58:11 +02:00
|
|
|
.get('/api/admin/api-tokens')
|
|
|
|
.expect('Content-Type', /json/)
|
|
|
|
.expect(200)
|
2021-08-12 15:04:37 +02:00
|
|
|
.expect((res) => {
|
2021-05-28 11:10:24 +02:00
|
|
|
expect(res.body.tokens.length).toBe(10);
|
|
|
|
expect(res.body.tokens[2].type).toBe('client');
|
2021-03-29 19:58:11 +02:00
|
|
|
});
|
|
|
|
});
|
|
|
|
|
2021-05-28 11:10:24 +02:00
|
|
|
test('removes api token', async () => {
|
2021-03-29 19:58:11 +02:00
|
|
|
const tokenSecret = 'random-secret';
|
|
|
|
|
2021-05-28 11:10:24 +02:00
|
|
|
await db.stores.apiTokenStore.insert({
|
2021-03-29 19:58:11 +02:00
|
|
|
username: 'test',
|
|
|
|
secret: tokenSecret,
|
|
|
|
type: ApiTokenType.CLIENT,
|
|
|
|
});
|
|
|
|
|
2021-05-28 11:10:24 +02:00
|
|
|
await app.request
|
2021-03-29 19:58:11 +02:00
|
|
|
.delete(`/api/admin/api-tokens/${tokenSecret}`)
|
|
|
|
.set('Content-Type', 'application/json')
|
|
|
|
.expect(200);
|
|
|
|
|
2021-05-28 11:10:24 +02:00
|
|
|
return app.request
|
2021-03-29 19:58:11 +02:00
|
|
|
.get('/api/admin/api-tokens')
|
|
|
|
.expect('Content-Type', /json/)
|
|
|
|
.expect(200)
|
2021-08-12 15:04:37 +02:00
|
|
|
.expect((res) => {
|
2021-05-28 11:10:24 +02:00
|
|
|
expect(res.body.tokens.length).toBe(0);
|
2021-03-29 19:58:11 +02:00
|
|
|
});
|
|
|
|
});
|
2021-09-15 20:28:10 +02:00
|
|
|
|
|
|
|
test('creates new client token: project & environment defaults to "*"', async () => {
|
|
|
|
return app.request
|
|
|
|
.post('/api/admin/api-tokens')
|
|
|
|
.send({
|
|
|
|
username: 'default-client',
|
|
|
|
type: 'client',
|
|
|
|
})
|
|
|
|
.set('Content-Type', 'application/json')
|
|
|
|
.expect(201)
|
|
|
|
.expect((res) => {
|
|
|
|
expect(res.body.type).toBe('client');
|
|
|
|
expect(res.body.secret.length > 16).toBe(true);
|
2021-09-24 13:55:00 +02:00
|
|
|
expect(res.body.environment).toBe(DEFAULT_ENV);
|
2022-04-06 08:11:41 +02:00
|
|
|
expect(res.body.projects[0]).toBe(ALL);
|
2021-09-15 20:28:10 +02:00
|
|
|
});
|
|
|
|
});
|
|
|
|
|
|
|
|
test('creates new client token with project & environment set', async () => {
|
|
|
|
return app.request
|
|
|
|
.post('/api/admin/api-tokens')
|
|
|
|
.send({
|
|
|
|
username: 'default-client',
|
|
|
|
type: 'client',
|
|
|
|
project: 'default',
|
2021-09-24 13:55:00 +02:00
|
|
|
environment: DEFAULT_ENV,
|
2021-09-15 20:28:10 +02:00
|
|
|
})
|
|
|
|
.set('Content-Type', 'application/json')
|
|
|
|
.expect(201)
|
|
|
|
.expect((res) => {
|
|
|
|
expect(res.body.type).toBe('client');
|
|
|
|
expect(res.body.secret.length > 16).toBe(true);
|
2021-09-24 13:55:00 +02:00
|
|
|
expect(res.body.environment).toBe(DEFAULT_ENV);
|
2022-04-06 08:11:41 +02:00
|
|
|
expect(res.body.projects[0]).toBe('default');
|
2021-09-15 20:28:10 +02:00
|
|
|
});
|
|
|
|
});
|
|
|
|
|
|
|
|
test('should prefix default token with "*:*."', async () => {
|
|
|
|
return app.request
|
|
|
|
.post('/api/admin/api-tokens')
|
|
|
|
.send({
|
|
|
|
username: 'default-client',
|
|
|
|
type: 'client',
|
|
|
|
})
|
|
|
|
.set('Content-Type', 'application/json')
|
|
|
|
.expect(201)
|
|
|
|
.expect((res) => {
|
2021-09-24 13:55:00 +02:00
|
|
|
expect(res.body.secret).toMatch(/\*:default\..*/);
|
2021-09-15 20:28:10 +02:00
|
|
|
});
|
|
|
|
});
|
|
|
|
|
|
|
|
test('should prefix token with "project:environment."', async () => {
|
|
|
|
return app.request
|
|
|
|
.post('/api/admin/api-tokens')
|
|
|
|
.send({
|
|
|
|
username: 'default-client',
|
|
|
|
type: 'client',
|
|
|
|
project: 'default',
|
2021-09-24 13:55:00 +02:00
|
|
|
environment: DEFAULT_ENV,
|
2021-09-15 20:28:10 +02:00
|
|
|
})
|
|
|
|
.set('Content-Type', 'application/json')
|
|
|
|
.expect(201)
|
|
|
|
.expect((res) => {
|
2021-09-24 13:55:00 +02:00
|
|
|
expect(res.body.secret).toMatch(/default:default\..*/);
|
2021-09-15 20:28:10 +02:00
|
|
|
});
|
|
|
|
});
|
|
|
|
|
|
|
|
test('should not create token for invalid projectId', async () => {
|
|
|
|
return app.request
|
|
|
|
.post('/api/admin/api-tokens')
|
|
|
|
.send({
|
|
|
|
username: 'default-client',
|
|
|
|
type: 'client',
|
|
|
|
project: 'bogus-project-something',
|
|
|
|
})
|
|
|
|
.set('Content-Type', 'application/json')
|
|
|
|
.expect(400)
|
|
|
|
.expect((res) => {
|
feat: unify error responses (#3607)
This PR implements the first version of a suggested unification (and
documentation) of the errors that we return from the API today.
The goal is for this to be the first step towards the error type defined
in this internal [linear
task](https://linear.app/unleash/issue/1-629/define-the-error-type
'Define the new API error type').
## The state of things today
As things stand, we currently have no (or **very** little) documentation
of the errors that are returned from the API. We mention error codes,
but never what the errors may contain.
Second, there is no specified format for errors, so what they return is
arbitrary, and based on ... Who knows? As a result, we have multiple
different errors returned by the API depending on what operation you're
trying to do. What's more, with OpenAPI validation in the mix, it's
absolutely possible for you to get two completely different error
objects for operations to the same endpoint.
Third, the errors we do return are usually pretty vague and don't really
provide any real help to the user. "You don't have the right
permissions". Great. Well what permissions do I need? And how would I
know? "BadDataError". Sick. Why is it bad?
... You get it.
## What we want to achieve
The ultimate goal is for error messages to serve both humans and
machines. When the user provides bad data, we should tell them what
parts of the data are bad and what they can do to fix it. When they
don't have the right permissions, we should tell them what permissions
they need.
Additionally, it would be nice if we could provide an ID for each error
instance, so that you (or an admin) can look through the logs and locate
he incident.
## What's included in **this** PR?
This PR does not aim to implement everything above. It's not intended to
magically fix everything. Its goal is to implement the necessary
**breaking** changes, so that they can be included in v5. Changing error
messages is a slightly grayer area than changing APIs directly, but
changing the format is definitely something I'd consider breaking.
So this PR:
- defines a minimal version of the error type defined in the [API error
definition linear
task](https://linear.app/unleash/issue/1-629/define-the-error-type).
- aims to catch all errors we return today and wrap them in the error
type
- updates tests to match the new expectations.
An important point: because we are cutting v5 very soon and because work
for this wasn't started until last week, the code here isn't necessarily
very polished. But it doesn't need to be. The internals can be as messy
as we want, as long as the API surface is stable.
That said, I'm very open to feedback about design and code completeness,
etc, but this has intentionally been done quickly.
Please also see my inline comments on the changes for more specific
details.
### Proposed follow-ups
As mentioned, this is the first step to implementing the error type. The
public API error type only exposes `id`, `name`, and `message`. This is
barely any more than most of the previous messages, but they are now all
using the same format. Any additional properties, such as `suggestion`,
`help`, `documentationLink` etc can be added as features without
breaking the current format. This is an intentional limitation of this
PR.
Regarding additional properties: there are some error responses that
must contain extra properties. Some of these are documented in the types
of the new error constructor, but not all. This includes `path` and
`type` properties on 401 errors, `details` on validation errors, and
more.
Also, because it was put together quickly, I don't yet know exactly how
we (as developers) would **prefer** to use these new error messages
within the code, so the internal API (the new type, name, etc), is just
a suggestion. This can evolve naturally over time if (based on feedback
and experience) without changing the public API.
## Returning multiple errors
Most of the time when we return errors today, we only return a single
error (even if many things are wrong). AJV, the OpenAPI integration we
use does have a setting that allows it to return all errors in a request
instead of a single one. I suggest we turn that on, but that we do it in
a separate PR (because it updates a number of other snapshots).
When returning errors that point to `details`, the objects in the
`details` now contain a new `description` property. This "deprecates"
the `message` property. Due to our general deprecation policy, this
should be kept around for another full major and can be removed in v6.
```json
{
"name": "BadDataError",
"message": "Something went wrong. Check the `details` property for more information."
"details": [{
"message": "The .params property must be an object. You provided an array.",
"description": "The .params property must be an object. You provided an array.",
}]
}
```
2023-04-25 15:40:46 +02:00
|
|
|
expect(res.body.details[0].description).toMatch(
|
2021-09-15 20:28:10 +02:00
|
|
|
/bogus-project-something/,
|
|
|
|
);
|
|
|
|
});
|
|
|
|
});
|
|
|
|
|
|
|
|
test('should not create token for invalid environment', async () => {
|
|
|
|
return app.request
|
|
|
|
.post('/api/admin/api-tokens')
|
|
|
|
.send({
|
|
|
|
username: 'default-client',
|
|
|
|
type: 'client',
|
|
|
|
environment: 'bogus-environment-something',
|
|
|
|
})
|
|
|
|
.set('Content-Type', 'application/json')
|
|
|
|
.expect(400)
|
|
|
|
.expect((res) => {
|
feat: unify error responses (#3607)
This PR implements the first version of a suggested unification (and
documentation) of the errors that we return from the API today.
The goal is for this to be the first step towards the error type defined
in this internal [linear
task](https://linear.app/unleash/issue/1-629/define-the-error-type
'Define the new API error type').
## The state of things today
As things stand, we currently have no (or **very** little) documentation
of the errors that are returned from the API. We mention error codes,
but never what the errors may contain.
Second, there is no specified format for errors, so what they return is
arbitrary, and based on ... Who knows? As a result, we have multiple
different errors returned by the API depending on what operation you're
trying to do. What's more, with OpenAPI validation in the mix, it's
absolutely possible for you to get two completely different error
objects for operations to the same endpoint.
Third, the errors we do return are usually pretty vague and don't really
provide any real help to the user. "You don't have the right
permissions". Great. Well what permissions do I need? And how would I
know? "BadDataError". Sick. Why is it bad?
... You get it.
## What we want to achieve
The ultimate goal is for error messages to serve both humans and
machines. When the user provides bad data, we should tell them what
parts of the data are bad and what they can do to fix it. When they
don't have the right permissions, we should tell them what permissions
they need.
Additionally, it would be nice if we could provide an ID for each error
instance, so that you (or an admin) can look through the logs and locate
he incident.
## What's included in **this** PR?
This PR does not aim to implement everything above. It's not intended to
magically fix everything. Its goal is to implement the necessary
**breaking** changes, so that they can be included in v5. Changing error
messages is a slightly grayer area than changing APIs directly, but
changing the format is definitely something I'd consider breaking.
So this PR:
- defines a minimal version of the error type defined in the [API error
definition linear
task](https://linear.app/unleash/issue/1-629/define-the-error-type).
- aims to catch all errors we return today and wrap them in the error
type
- updates tests to match the new expectations.
An important point: because we are cutting v5 very soon and because work
for this wasn't started until last week, the code here isn't necessarily
very polished. But it doesn't need to be. The internals can be as messy
as we want, as long as the API surface is stable.
That said, I'm very open to feedback about design and code completeness,
etc, but this has intentionally been done quickly.
Please also see my inline comments on the changes for more specific
details.
### Proposed follow-ups
As mentioned, this is the first step to implementing the error type. The
public API error type only exposes `id`, `name`, and `message`. This is
barely any more than most of the previous messages, but they are now all
using the same format. Any additional properties, such as `suggestion`,
`help`, `documentationLink` etc can be added as features without
breaking the current format. This is an intentional limitation of this
PR.
Regarding additional properties: there are some error responses that
must contain extra properties. Some of these are documented in the types
of the new error constructor, but not all. This includes `path` and
`type` properties on 401 errors, `details` on validation errors, and
more.
Also, because it was put together quickly, I don't yet know exactly how
we (as developers) would **prefer** to use these new error messages
within the code, so the internal API (the new type, name, etc), is just
a suggestion. This can evolve naturally over time if (based on feedback
and experience) without changing the public API.
## Returning multiple errors
Most of the time when we return errors today, we only return a single
error (even if many things are wrong). AJV, the OpenAPI integration we
use does have a setting that allows it to return all errors in a request
instead of a single one. I suggest we turn that on, but that we do it in
a separate PR (because it updates a number of other snapshots).
When returning errors that point to `details`, the objects in the
`details` now contain a new `description` property. This "deprecates"
the `message` property. Due to our general deprecation policy, this
should be kept around for another full major and can be removed in v6.
```json
{
"name": "BadDataError",
"message": "Something went wrong. Check the `details` property for more information."
"details": [{
"message": "The .params property must be an object. You provided an array.",
"description": "The .params property must be an object. You provided an array.",
}]
}
```
2023-04-25 15:40:46 +02:00
|
|
|
expect(res.body.details[0].description).toMatch(
|
2021-09-15 20:28:10 +02:00
|
|
|
/bogus-environment-something/,
|
|
|
|
);
|
|
|
|
});
|
|
|
|
});
|
|
|
|
|
|
|
|
test('should not create token for invalid project & environment', async () => {
|
|
|
|
return app.request
|
|
|
|
.post('/api/admin/api-tokens')
|
|
|
|
.send({
|
|
|
|
username: 'default-admin',
|
|
|
|
type: 'admin',
|
|
|
|
project: 'bogus-project-something',
|
|
|
|
environment: 'bogus-environment-something',
|
|
|
|
})
|
|
|
|
.set('Content-Type', 'application/json')
|
|
|
|
.expect(400);
|
|
|
|
});
|
|
|
|
|
|
|
|
test('admin token only supports ALL projects', async () => {
|
|
|
|
return app.request
|
|
|
|
.post('/api/admin/api-tokens')
|
|
|
|
.send({
|
|
|
|
username: 'default-admin',
|
|
|
|
type: 'admin',
|
|
|
|
project: 'default',
|
|
|
|
environment: '*',
|
|
|
|
})
|
|
|
|
.set('Content-Type', 'application/json')
|
|
|
|
.expect(400);
|
|
|
|
});
|
|
|
|
|
2023-05-04 09:56:00 +02:00
|
|
|
test('needs one of the username and tokenName properties set', async () => {
|
|
|
|
return app.request
|
|
|
|
.post('/api/admin/api-tokens')
|
|
|
|
.send({
|
|
|
|
type: 'admin',
|
|
|
|
environment: '*',
|
|
|
|
})
|
|
|
|
.set('Content-Type', 'application/json')
|
|
|
|
.expect(400);
|
|
|
|
});
|
|
|
|
|
|
|
|
test('can create with tokenName only', async () => {
|
|
|
|
return app.request
|
|
|
|
.post('/api/admin/api-tokens')
|
|
|
|
.send({
|
|
|
|
tokenName: 'default-admin',
|
|
|
|
type: 'admin',
|
|
|
|
environment: '*',
|
|
|
|
})
|
|
|
|
.set('Content-Type', 'application/json')
|
|
|
|
.expect(201)
|
|
|
|
.expect((res) => {
|
|
|
|
expect(res.body.type).toBe('admin');
|
|
|
|
expect(res.body.secret.length > 16).toBe(true);
|
|
|
|
expect(res.body.username).toBe('default-admin');
|
|
|
|
expect(res.body.tokenName).toBe('default-admin');
|
|
|
|
});
|
|
|
|
});
|
|
|
|
|
|
|
|
test('only one of tokenName and username can be set', async () => {
|
|
|
|
return app.request
|
|
|
|
.post('/api/admin/api-tokens')
|
|
|
|
.send({
|
|
|
|
username: 'default-client-name',
|
|
|
|
tokenName: 'default-token-name',
|
|
|
|
type: 'admin',
|
|
|
|
environment: '*',
|
|
|
|
})
|
|
|
|
.set('Content-Type', 'application/json')
|
|
|
|
.expect(400);
|
|
|
|
});
|
|
|
|
|
2021-09-15 20:28:10 +02:00
|
|
|
test('admin token only supports ALL environments', async () => {
|
|
|
|
return app.request
|
|
|
|
.post('/api/admin/api-tokens')
|
|
|
|
.send({
|
|
|
|
username: 'default-admin',
|
|
|
|
type: 'admin',
|
|
|
|
project: '*',
|
2021-09-24 13:55:00 +02:00
|
|
|
environment: DEFAULT_ENV,
|
|
|
|
})
|
|
|
|
.set('Content-Type', 'application/json')
|
|
|
|
.expect(400);
|
|
|
|
});
|
|
|
|
|
|
|
|
test('client tokens cannot span all environments', async () => {
|
|
|
|
return app.request
|
|
|
|
.post('/api/admin/api-tokens')
|
|
|
|
.send({
|
|
|
|
username: 'default-client',
|
|
|
|
type: 'client',
|
|
|
|
environment: ALL,
|
2021-09-15 20:28:10 +02:00
|
|
|
})
|
|
|
|
.set('Content-Type', 'application/json')
|
|
|
|
.expect(400);
|
|
|
|
});
|
2022-03-24 11:26:00 +01:00
|
|
|
|
2023-03-29 09:33:14 +02:00
|
|
|
test('should create token for disabled environment', async () => {
|
2022-03-24 11:26:00 +01:00
|
|
|
await db.stores.environmentStore.create({
|
|
|
|
name: 'disabledEnvironment',
|
|
|
|
type: 'production',
|
|
|
|
enabled: false,
|
|
|
|
});
|
|
|
|
return app.request
|
|
|
|
.post('/api/admin/api-tokens')
|
|
|
|
.send({
|
|
|
|
username: 'default',
|
|
|
|
type: 'client',
|
|
|
|
project: 'default',
|
|
|
|
environment: 'disabledEnvironment',
|
|
|
|
})
|
|
|
|
.set('Content-Type', 'application/json')
|
2023-03-29 09:33:14 +02:00
|
|
|
.expect(201);
|
2022-03-24 11:26:00 +01:00
|
|
|
});
|
feat: Separate api token roles (#4019)
## What
As part of the move to enable custom-root-roles, our permissions model
was found to not be granular enough to allow service accounts to only be
allowed to create read-only tokens (client, frontend), but not be
allowed to create admin tokens to avoid opening up a path for privilege
escalation.
## How
This PR adds 12 new roles, a CRUD set for each of the three token types
(admin, client, frontend). To access the `/api/admin/api-tokens`
endpoints you will still need the existing permission (CREATE_API_TOKEN,
DELETE_API_TOKEN, READ_API_TOKEN, UPDATE_API_TOKEN). Once this PR has
been merged the token type you're modifying will also be checked, so if
you're trying to create a CLIENT api-token, you will need
`CREATE_API_TOKEN` and `CREATE_CLIENT_API_TOKEN` permissions. If the
user performing the create call does not have these two permissions or
the `ADMIN` permission, the creation will be rejected with a `403 -
FORBIDDEN` status.
### Discussion points
The test suite tests all operations using a token with
operation_CLIENT_API_TOKEN permission and verifies that it fails trying
to do any of the operations against FRONTEND and ADMIN tokens. During
development the operation_FRONTEND_API_TOKEN and
operation_ADMIN_API_TOKEN permission has also been tested in the same
way. I wonder if it's worth it to re-add these tests in order to verify
that the permission checker works for all operations, or if this is
enough. Since we're running them using e2e tests, I've removed them for
now, to avoid hogging too much processing time.
2023-06-20 14:21:14 +02:00
|
|
|
|
|
|
|
test('updating expiry of non existing token should yield 200', async () => {
|
|
|
|
return app.request
|
|
|
|
.put('/api/admin/api-tokens/randomnonexistingsecret')
|
|
|
|
.send({ expiresAt: addDays(new Date(), 14) })
|
|
|
|
.set('Content-Type', 'application/json')
|
|
|
|
.expect(200);
|
|
|
|
});
|
|
|
|
|
|
|
|
test('Deleting non-existing token should yield 200', async () => {
|
|
|
|
return app.request
|
|
|
|
.delete('/api/admin/api-tokens/random-non-existing-token')
|
|
|
|
.expect(200);
|
|
|
|
});
|