refactor: add OpenAPI schema to simple-password-provider controller (#1734)

* refactor: add OpenAPI schema to simple-password-provider controller

* finish implementation after merge

* refactor: address PR comments

src/lib/openapi/index.ts

@@ -23,6 +23,7 @@ import { healthCheckSchema } from './spec/health-check-schema';
 import { healthOverviewSchema } from './spec/health-overview-schema';
 import { healthReportSchema } from './spec/health-report-schema';
 import { legalValueSchema } from './spec/legal-value-schema';
+import { loginSchema } from './spec/login-schema';
 import { idSchema } from './spec/id-schema';
 import { mapValues } from '../util/map-values';
 import { nameSchema } from './spec/name-schema';
@@ -116,6 +117,7 @@ export const schemas = {
     healthOverviewSchema,
     healthReportSchema,
     legalValueSchema,
+    loginSchema,
     nameSchema,
     idSchema,
     meSchema,
@@ -172,16 +174,12 @@ export interface JsonSchemaProps {
     components: object;
 }
 
-interface ApiOperation<Tag = 'client' | 'admin' | 'other'>
+export interface ApiOperation<Tag = 'admin' | 'client' | 'auth' | 'other'>
     extends Omit<OpenAPIV3.OperationObject, 'tags'> {
     operationId: string;
     tags: [Tag];
 }
 
-export type AdminApiOperation = ApiOperation<'admin'>;
-export type ClientApiOperation = ApiOperation<'client'>;
-export type OtherApiOperation = ApiOperation<'other'>;
-
 export const createRequestSchema = (
     schemaName: string,
 ): OpenAPIV3.RequestBodyObject => {

src/lib/openapi/spec/login-schema.ts

@@ -0,0 +1,19 @@
+import { FromSchema } from 'json-schema-to-ts';
+
+export const loginSchema = {
+    $id: '#/components/schemas/loginSchema',
+    type: 'object',
+    additionalProperties: false,
+    required: ['username', 'password'],
+    properties: {
+        username: {
+            type: 'string',
+        },
+        password: {
+            type: 'string',
+        },
+    },
+    components: {},
+} as const;
+
+export type LoginSchema = FromSchema<typeof loginSchema>;

src/lib/routes/auth/reset-password-controller.ts

@@ -55,7 +55,7 @@ class ResetPasswordController extends Controller {
             permission: NONE,
             middleware: [
                 openApiService.validPath({
-                    tags: ['other'],
+                    tags: ['auth'],
                     operationId: 'validateToken',
                     responses: { 200: createResponseSchema('tokenUserSchema') },
                 }),

src/lib/routes/auth/simple-password-provider.test.ts

@@ -1,18 +1,24 @@
 import request from 'supertest';
 import express from 'express';
 import User from '../../types/user';
-import PasswordProvider from './simple-password-provider';
+import { SimplePasswordProvider } from './simple-password-provider';
 import PasswordMismatchError from '../../error/password-mismatch';
-import getLogger from '../../../test/fixtures/no-logger';
+import { createTestConfig } from '../../../test/config/test-config';
+import { OpenApiService } from '../../services/openapi-service';
 
 test('Should require password', async () => {
+    const config = createTestConfig();
+    const openApiService = new OpenApiService(config);
     const app = express();
     app.use(express.json());
     const userService = () => {};
-    // @ts-ignore
-    const ctr = new PasswordProvider({ getLogger }, { userService });
 
-    //@ts-ignore
+    const ctr = new SimplePasswordProvider(config, {
+        // @ts-expect-error
+        userService,
+        openApiService,
+    });
+
     app.use('/auth/simple', ctr.router);
 
     const res = await request(app)
@@ -23,6 +29,8 @@ test('Should require password', async () => {
 });
 
 test('Should login user', async () => {
+    const config = createTestConfig();
+    const openApiService = new OpenApiService(config);
     const username = 'ola';
     const password = 'simplepass';
     const user = new User({ id: 123, username });
@@ -30,10 +38,11 @@ test('Should login user', async () => {
     const app = express();
     app.use(express.json());
     app.use((req, res, next) => {
-        //@ts-ignore
+        // @ts-expect-error
         req.session = {};
         next();
     });
+
     const userService = {
         loginUser: (u, p) => {
             if (u === username && p === password) {
@@ -42,10 +51,13 @@ test('Should login user', async () => {
             throw new Error('Wrong password');
         },
     };
-    // @ts-ignore
-    const ctr = new PasswordProvider({ getLogger }, { userService });
 
-    //@ts-ignore
+    const ctr = new SimplePasswordProvider(config, {
+        // @ts-expect-error
+        userService,
+        openApiService,
+    });
+
     app.use('/auth/simple', ctr.router);
 
     const res = await request(app)
@@ -57,6 +69,8 @@ test('Should login user', async () => {
 });
 
 test('Should not login user with wrong password', async () => {
+    const config = createTestConfig();
+    const openApiService = new OpenApiService(config);
     const username = 'ola';
     const password = 'simplepass';
     const user = new User({ id: 133, username });
@@ -64,10 +78,11 @@ test('Should not login user with wrong password', async () => {
     const app = express();
     app.use(express.json());
     app.use((req, res, next) => {
-        //@ts-ignore
+        // @ts-expect-error
         req.session = {};
         next();
     });
+
     const userService = {
         loginUser: (u, p) => {
             if (u === username && p === password) {
@@ -76,10 +91,13 @@ test('Should not login user with wrong password', async () => {
             throw new PasswordMismatchError();
         },
     };
-    // @ts-ignore
-    const ctr = new PasswordProvider({ getLogger }, { userService });
 
-    //@ts-ignore
+    const ctr = new SimplePasswordProvider(config, {
+        // @ts-expect-error
+        userService,
+        openApiService,
+    });
+
     app.use('/auth/simple', ctr.router);
 
     const res = await request(app)

src/lib/routes/auth/simple-password-provider.ts

@@ -1,4 +1,5 @@
 import { Response } from 'express';
+import { OpenApiService } from '../../services/openapi-service';
 import { Logger } from '../../logger';
 import { IUnleashConfig } from '../../server-impl';
 import UserService from '../../services/user-service';
@@ -6,37 +7,61 @@ import { IUnleashServices } from '../../types';
 import { NONE } from '../../types/permissions';
 import Controller from '../controller';
 import { IAuthRequest } from '../unleash-types';
+import { createRequestSchema, createResponseSchema } from '../../openapi';
+import { userSchema, UserSchema } from '../../openapi/spec/user-schema';
+import { LoginSchema } from '../../openapi/spec/login-schema';
+import { serializeDates } from '../../types/serialize-dates';
 
-class PasswordProvider extends Controller {
+export class SimplePasswordProvider extends Controller {
-    private userService: UserService;
-
     private logger: Logger;
 
+    private openApiService: OpenApiService;
+
+    private userService: UserService;
+
     constructor(
         config: IUnleashConfig,
-        { userService }: Pick<IUnleashServices, 'userService'>,
+        {
+            userService,
+            openApiService,
+        }: Pick<IUnleashServices, 'userService' | 'openApiService'>,
     ) {
         super(config);
         this.logger = config.getLogger('/auth/password-provider.js');
+        this.openApiService = openApiService;
         this.userService = userService;
 
-        this.post('/login', this.login, NONE);
+        this.route({
-    }
+            method: 'post',
-
+            path: '/login',
-    async login(req: IAuthRequest, res: Response): Promise<void> {
+            handler: this.login,
-        const { username, password } = req.body;
+            permission: NONE,
-
+            middleware: [
-        if (!username || !password) {
+                openApiService.validPath({
-            res.status(400).json({
+                    tags: ['auth'],
-                message: 'You must provide username and password',
+                    operationId: 'login',
+                    requestBody: createRequestSchema('loginSchema'),
+                    responses: {
+                        200: createResponseSchema('userSchema'),
+                    },
+                }),
+            ],
         });
-            return;
     }
 
+    async login(
+        req: IAuthRequest<void, void, LoginSchema>,
+        res: Response<UserSchema>,
+    ): Promise<void> {
+        const { username, password } = req.body;
+
         const user = await this.userService.loginUser(username, password);
         req.session.user = user;
-        res.status(200).json(user);
+        this.openApiService.respondWithValidation(
+            200,
+            res,
+            userSchema.$id,
+            serializeDates(user),
+        );
     }
 }
-
-export default PasswordProvider;

src/lib/routes/index.ts

@@ -1,7 +1,7 @@
 import { Request, Response } from 'express';
 import { BackstageController } from './backstage';
 import ResetPasswordController from './auth/reset-password-controller';
-import SimplePasswordProvider from './auth/simple-password-provider';
+import { SimplePasswordProvider } from './auth/simple-password-provider';
 import { IUnleashConfig } from '../types/option';
 import { IUnleashServices } from '../types/services';
 import { api } from './api-def';

src/lib/services/openapi-service.ts

@@ -2,9 +2,7 @@ import openapi, { IExpressOpenApi } from '@unleash/express-openapi';
 import { Express, RequestHandler, Response } from 'express';
 import { IUnleashConfig } from '../types/option';
 import {
-    AdminApiOperation,
+    ApiOperation,
-    ClientApiOperation,
-    OtherApiOperation,
     createOpenApiSchema,
     JsonSchemaProps,
     removeJsonSchemaProps,
@@ -31,9 +29,7 @@ export class OpenApiService {
         );
     }
 
-    validPath(
+    validPath(op: ApiOperation): RequestHandler {
-        op: AdminApiOperation | ClientApiOperation | OtherApiOperation,
-    ): RequestHandler {
         return this.api.validPath(op);
     }
 

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

@@ -1041,6 +1041,22 @@ Object {
         ],
         "type": "object",
       },
+      "loginSchema": Object {
+        "additionalProperties": false,
+        "properties": Object {
+          "password": Object {
+            "type": "string",
+          },
+          "username": Object {
+            "type": "string",
+          },
+        },
+        "required": Array [
+          "username",
+          "password",
+        ],
+        "type": "object",
+      },
       "meSchema": Object {
         "additionalProperties": false,
         "properties": Object {
@@ -4950,7 +4966,7 @@ Object {
           },
         },
         "tags": Array [
-          "other",
+          "auth",
         ],
       },
     },
@@ -4978,6 +4994,37 @@ Object {
         ],
       },
     },
+    "/auth/simple/login": Object {
+      "post": Object {
+        "operationId": "login",
+        "requestBody": Object {
+          "content": Object {
+            "application/json": Object {
+              "schema": Object {
+                "$ref": "#/components/schemas/loginSchema",
+              },
+            },
+          },
+          "description": "loginSchema",
+          "required": true,
+        },
+        "responses": Object {
+          "200": Object {
+            "content": Object {
+              "application/json": Object {
+                "schema": Object {
+                  "$ref": "#/components/schemas/userSchema",
+                },
+              },
+            },
+            "description": "userSchema",
+          },
+        },
+        "tags": Array [
+          "auth",
+        ],
+      },
+    },
     "/health": Object {
       "get": Object {
         "operationId": "getHealth",