openapi: update API tokens tag (#4137)

This PR updates endpoints and schemas for the API tokens tag.

As part of that, they also handle oneOf openapi validation errors and improve the console output for the enforcer tests.

src/lib/error/bad-data-error.ts

@@ -83,27 +83,46 @@ const genericErrorMessage = (
     };
 };
 
+const oneOfMessage = (
+    propertyName: string,
+    errorMessage: string = 'is invalid',
+) => {
+    const errorPosition =
+        propertyName === '' ? 'root object' : `${propertyName} property`;
+
+    const description = `The ${errorPosition} ${errorMessage}. The data you provided matches more than one option in the schema. These options are mutually exclusive. Please refer back to the schema and remove any excess properties.`;
+
+    return {
+        description,
+        message: description,
+        path: propertyName,
+    };
+};
+
 export const fromOpenApiValidationError =
     (requestBody: object) =>
     (validationError: ErrorObject): ValidationErrorDescription => {
         // @ts-expect-error Unsure why, but the `dataPath` isn't listed on the type definition for error objects. However, it's always there. Suspect this is a bug in the library.
-        const dataPath = validationError.dataPath.substring('.body.'.length);
+        const dataPath = validationError.dataPath;
+        const propertyName = dataPath.substring('.body.'.length);
 
         switch (validationError.keyword) {
             case 'required':
                 return missingRequiredPropertyMessage(
-                    dataPath,
+                    propertyName,
                     validationError.params.missingProperty,
                 );
             case 'additionalProperties':
                 return additionalPropertiesMessage(
-                    dataPath,
+                    propertyName,
                     validationError.params.additionalProperty,
                 );
+            case 'oneOf':
+                return oneOfMessage(propertyName, validationError.message);
             default:
                 return genericErrorMessage(
                     requestBody,
-                    dataPath,
+                    propertyName,
                     validationError.message,
                 );
         }

src/lib/error/unleash-error.test.ts

@@ -99,6 +99,46 @@ describe('OpenAPI error conversion', () => {
         });
     });
 
+    it.each(['.body', '.body.subObject'])(
+        'Gives useful error messages for oneOf errors in %s',
+        (dataPath) => {
+            const error = {
+                keyword: 'oneOf',
+                instancePath: '',
+                dataPath,
+                schemaPath: '#/components/schemas/createApiTokenSchema/oneOf',
+                params: {
+                    passingSchemas: null,
+                },
+                message: 'should match exactly one schema in oneOf',
+            };
+
+            const result = fromOpenApiValidationError({
+                secret: 'blah',
+                username: 'string2',
+                type: 'admin',
+            })(error);
+
+            expect(result).toMatchObject({
+                description:
+                    // it provides the message
+                    expect.stringContaining(error.message),
+                path: dataPath.substring('.body.'.length),
+            });
+
+            // it tells the user what happened
+            expect(result.description).toContain(
+                'matches more than one option',
+            );
+            // it tells the user what part of the request body this pertains to
+            expect(result.description).toContain(
+                dataPath === '.body'
+                    ? 'root object'
+                    : `${dataPath.substring('.body.'.length)} property`,
+            );
+        },
+    );
+
     it('Gives useful pattern error messages', () => {
         const error = {
             instancePath: '',

src/lib/openapi/meta-schema-rules.test.ts

@@ -93,7 +93,6 @@ const metaRules: Rule[] = [
             'batchFeaturesSchema',
             'batchStaleSchema',
             'cloneFeatureSchema',
-            'createApiTokenSchema',
             'createFeatureSchema',
             'createInvitedUserSchema',
             'environmentsSchema',
@@ -136,7 +135,6 @@ const metaRules: Rule[] = [
             'tagTypesSchema',
             'tagWithVersionSchema',
             'uiConfigSchema',
-            'updateApiTokenSchema',
             'updateFeatureSchema',
             'updateFeatureStrategySchema',
             'updateTagTypeSchema',
@@ -166,7 +164,6 @@ const metaRules: Rule[] = [
             'batchFeaturesSchema',
             'batchStaleSchema',
             'cloneFeatureSchema',
-            'createApiTokenSchema',
             'createFeatureSchema',
             'createFeatureStrategySchema',
             'createInvitedUserSchema',
@@ -211,7 +208,6 @@ const metaRules: Rule[] = [
             'tagTypesSchema',
             'tagWithVersionSchema',
             'uiConfigSchema',
-            'updateApiTokenSchema',
             'updateFeatureSchema',
             'updateFeatureStrategySchema',
             'updateTagTypeSchema',

src/lib/openapi/spec/api-tokens-schema.ts

@@ -4,16 +4,17 @@ import { apiTokenSchema } from './api-token-schema';
 export const apiTokensSchema = {
     $id: '#/components/schemas/apiTokensSchema',
     type: 'object',
+    description:
+        'An object with [Unleash API tokens](https://docs.getunleash.io/reference/api-tokens-and-client-keys)',
     additionalProperties: false,
     required: ['tokens'],
-    description: 'Contains a list of API tokens.',
     properties: {
         tokens: {
             type: 'array',
+            description: 'A list of Unleash API tokens.',
             items: {
                 $ref: '#/components/schemas/apiTokenSchema',
             },
-            description: 'A list of API tokens.',
         },
     },
     components: {

src/lib/openapi/spec/create-api-token-schema.ts

@@ -1,5 +1,78 @@
 import { FromSchema } from 'json-schema-to-ts';
-import { ApiTokenType } from '../../types/models/api-token';
+
+const adminSchema = {
+    required: ['type'],
+    type: 'object',
+    properties: {
+        type: {
+            type: 'string',
+            pattern: '^[Aa][Dd][Mm][Ii][Nn]$',
+            description: `An admin token. Must be the string "admin" (not case sensitive).`,
+            example: 'admin',
+        },
+    },
+} as const;
+
+const tokenNameSchema = {
+    type: 'object',
+    required: ['tokenName'],
+    properties: {
+        tokenName: {
+            type: 'string',
+            description: 'The name of the token.',
+            example: 'token-64522',
+        },
+    },
+} as const;
+
+const usernameSchema = {
+    type: 'object',
+    required: ['username'],
+    properties: {
+        username: {
+            deprecated: true,
+            type: 'string',
+            description:
+                'The name of the token. This property is deprecated. Use `tokenName` instead.',
+            example: 'token-64523',
+        },
+    },
+} as const;
+
+const clientFrontendSchema = {
+    required: ['type'],
+    type: 'object',
+    properties: {
+        type: {
+            type: 'string',
+            pattern:
+                '^([Cc][Ll][Ii][Ee][Nn][Tt]|[Ff][Rr][Oo][Nn][Tt][Ee][Nn][Dd])$',
+            description: `A client or frontend token. Must be one of the strings "client" or "frontend" (not case sensitive).`,
+            example: 'frontend',
+        },
+        environment: {
+            type: 'string',
+            description:
+                'The environment that the token should be valid for. Defaults to "default"',
+            example: 'development',
+        },
+        project: {
+            type: 'string',
+            description:
+                'The project that the token should be valid for. Defaults to "*" meaning every project. This property is mutually incompatible with the `projects` property. If you specify one, you cannot specify the other.',
+            example: 'project-851',
+        },
+        projects: {
+            type: 'array',
+            description:
+                'A list of projects that the token should be valid for. This property is mutually incompatible with the `project` property. If you specify one, you cannot specify the other.',
+            example: ['project-851', 'project-852'],
+            items: {
+                type: 'string',
+            },
+        },
+    },
+} as const;
 
 // TODO: (openapi) this schema isn't entirely correct: `project` and `projects`
 // are mutually exclusive.
@@ -7,62 +80,34 @@ import { ApiTokenType } from '../../types/models/api-token';
 // That is, when creating a token, you can provide either `project` _or_
 // `projects`, but *not* both.
 //
-// We should be able to annotate this using `oneOf` and `allOf`, but making
-// `oneOf` only valid for _either_ `project` _or_ `projects` is tricky.
-//
-// I've opened an issue to get some help (thought it was a bug initially).
-// There's more info available at:
-//
-// https://github.com/ajv-validator/ajv/issues/2096
-//
-// This also applies to apiTokenSchema and potentially other related schemas.
-
+// Because we allow additional properties, we cannot express the mutual
+// exclusiveness in the schema (with OpenAPI 3.0). As such, it's mentioned in
+// the description for now.
 export const createApiTokenSchema = {
     $id: '#/components/schemas/createApiTokenSchema',
     type: 'object',
-    required: ['type'],
+    description:
+        'The data required to create an [Unleash API token](https://docs.getunleash.io/reference/api-tokens-and-client-keys).',
     properties: {
-        secret: {
-            type: 'string',
-        },
-        type: {
-            type: 'string',
-            description: `One of ${Object.values(ApiTokenType).join(', ')}`,
-        },
-        environment: {
-            type: 'string',
-        },
-        project: {
-            type: 'string',
-        },
-        projects: {
-            type: 'array',
-            items: {
-                type: 'string',
-            },
-        },
         expiresAt: {
             type: 'string',
             format: 'date-time',
-            nullable: true,
+            description: 'The time when this token should expire.',
+            example: '2023-07-04T11:26:24+02:00',
         },
     },
-    anyOf: [
+    oneOf: [
         {
-            properties: {
-                username: {
-                    type: 'string',
-                },
-            },
-            required: ['username'],
+            allOf: [adminSchema, tokenNameSchema],
         },
         {
-            properties: {
-                tokenName: {
-                    type: 'string',
-                },
-            },
-            required: ['tokenName'],
+            allOf: [adminSchema, usernameSchema],
+        },
+        {
+            allOf: [clientFrontendSchema, tokenNameSchema],
+        },
+        {
+            allOf: [clientFrontendSchema, usernameSchema],
         },
     ],
     components: {},

src/lib/openapi/spec/update-api-token-schema.ts

@@ -4,8 +4,11 @@ export const updateApiTokenSchema = {
     $id: '#/components/schemas/updateApiTokenSchema',
     type: 'object',
     required: ['expiresAt'],
+    description: 'An object with fields to updated for a given API token.',
     properties: {
         expiresAt: {
+            description: 'The new time when this token should expire.',
+            example: '2023-09-04T11:26:24+02:00',
             type: 'string',
             format: 'date-time',
         },

src/lib/openapi/util/standard-responses.ts

@@ -34,7 +34,7 @@ const unauthorizedResponse = {
 
 const forbiddenResponse = {
     description:
-        'User credentials are valid but does not have enough privileges to execute this operation',
+        'The provided user credentials are valid, but the user does not have the necessary permissions to perform this operation',
     content: {
         'application/json': {
             schema: {

src/lib/routes/admin-api/api-token.ts

@@ -37,7 +37,10 @@ import {
     ApiTokenSchema,
 } from '../../openapi/spec/api-token-schema';
 import { UpdateApiTokenSchema } from '../../openapi/spec/update-api-token-schema';
-import { emptyResponse } from '../../openapi/util/standard-responses';
+import {
+    emptyResponse,
+    getStandardResponses,
+} from '../../openapi/util/standard-responses';
 import { ProxyService } from '../../services/proxy-service';
 import { extractUsername } from '../../util';
 import { OperationDeniedError } from '../../error';
@@ -154,8 +157,12 @@ export class ApiTokenController extends Controller {
                 openApiService.validPath({
                     tags: ['API tokens'],
                     operationId: 'getAllApiTokens',
+                    summary: 'Get API tokens',
+                    description:
+                        'Retrieves all API tokens that exist in the Unleash instance.',
                     responses: {
                         200: createResponseSchema('apiTokensSchema'),
+                        ...getStandardResponses(401, 403),
                     },
                 }),
             ],
@@ -175,8 +182,13 @@ export class ApiTokenController extends Controller {
                     tags: ['API tokens'],
                     operationId: 'createApiToken',
                     requestBody: createRequestSchema('createApiTokenSchema'),
+                    summary: 'Create API token',
+                    description: `Create an API token of a specific type: one of ${Object.values(
+                        ApiTokenType,
+                    ).join(', ')}.`,
                     responses: {
                         201: resourceCreatedResponseSchema('apiTokenSchema'),
+                        ...getStandardResponses(401, 403, 415),
                     },
                 }),
             ],
@@ -195,9 +207,13 @@ export class ApiTokenController extends Controller {
                 openApiService.validPath({
                     tags: ['API tokens'],
                     operationId: 'updateApiToken',
+                    summary: 'Update API token',
+                    description:
+                        "Updates an existing API token with a new expiry date. The `token` path parameter is the token's `secret`. If the token does not exist, this endpoint returns a 200 OK, but does nothing.",
                     requestBody: createRequestSchema('updateApiTokenSchema'),
                     responses: {
                         200: emptyResponse,
+                        ...getStandardResponses(401, 403, 415),
                     },
                 }),
             ],
@@ -216,9 +232,13 @@ export class ApiTokenController extends Controller {
             middleware: [
                 openApiService.validPath({
                     tags: ['API tokens'],
+                    summary: 'Delete API token',
+                    description:
+                        "Deletes an existing API token. The `token` path parameter is the token's `secret`. If the token does not exist, this endpoint returns a 200 OK, but does nothing.",
                     operationId: 'deleteApiToken',
                     responses: {
                         200: emptyResponse,
+                        ...getStandardResponses(401, 403),
                     },
                 }),
             ],

src/test/e2e/api/admin/api-token.e2e.test.ts

@@ -1,4 +1,4 @@
-import { setupApp } from '../../helpers/test-helper';
+import { setupAppWithCustomConfig } from '../../helpers/test-helper';
 import dbInit from '../../helpers/database-init';
 import getLogger from '../../../fixtures/no-logger';
 import { ALL, ApiTokenType } from '../../../../lib/types/models/api-token';
@@ -10,7 +10,13 @@ let app;
 
 beforeAll(async () => {
     db = await dbInit('token_api_serial', getLogger);
-    app = await setupApp(db.stores);
+    app = await setupAppWithCustomConfig(db.stores, {
+        experimental: {
+            flags: {
+                strictSchemaValidation: true,
+            },
+        },
+    });
 });
 
 afterAll(async () => {

src/test/e2e/api/openapi/openapi.e2e.test.ts

@@ -94,7 +94,15 @@ test('the generated OpenAPI spec is valid', async () => {
         console.error(enforcerError);
     }
 
-    expect(enforcerWarning ?? enforcerError).toBe(undefined);
+    const enforcerResults = {
+        warnings: enforcerWarning?.toString(),
+        errors: enforcerError?.toString(),
+    };
+
+    expect(enforcerResults).toMatchObject({
+        warnings: undefined,
+        errors: undefined,
+    });
 });
 
 test('all root-level tags are "approved tags"', async () => {