From 929f824a3ab8396e0dc8274cac24dc731e48074e Mon Sep 17 00:00:00 2001 From: Fredrik Strand Oseberg Date: Wed, 26 Oct 2022 13:00:49 +0200 Subject: [PATCH] fix: refactor conditional middleware (#2261) * fix: refactor conditional middleware * fix: update tests * test: update snapshot to hide things behing flag from openapi Co-authored-by: kwasniew --- src/lib/middleware/conditional-middleware.ts | 5 +- .../admin/conditional-middleware.e2e.test.ts | 93 ++++++++ .../__snapshots__/openapi.e2e.test.ts.snap | 200 ------------------ 3 files changed, 95 insertions(+), 203 deletions(-) create mode 100644 src/test/e2e/api/admin/conditional-middleware.e2e.test.ts diff --git a/src/lib/middleware/conditional-middleware.ts b/src/lib/middleware/conditional-middleware.ts index 7292b299ce..c548618c20 100644 --- a/src/lib/middleware/conditional-middleware.ts +++ b/src/lib/middleware/conditional-middleware.ts @@ -8,12 +8,11 @@ export const conditionalMiddleware = ( router.use((req, res, next) => { if (condition()) { - next(); + middleware(req, res, next); } else { - res.status(404).send({ message: 'Not found' }); + next(); } }); - router.use(middleware); return router; }; diff --git a/src/test/e2e/api/admin/conditional-middleware.e2e.test.ts b/src/test/e2e/api/admin/conditional-middleware.e2e.test.ts new file mode 100644 index 0000000000..cb0f291fb5 --- /dev/null +++ b/src/test/e2e/api/admin/conditional-middleware.e2e.test.ts @@ -0,0 +1,93 @@ +import express from 'express'; +import { conditionalMiddleware } from '../../../../lib/middleware/conditional-middleware'; +import supertest from 'supertest'; + +test('disabled middleware should not block paths that use the same path', async () => { + const app = express(); + const path = '/api/admin/projects'; + + app.use( + path, + conditionalMiddleware( + () => false, + (req, res) => { + res.send({ suggestChanges: 'hello' }); + }, + ), + ); + + app.get(path, (req, res) => { + res.json({ projects: [] }); + }); + + await supertest(app) + .get('/api/admin/projects') + .expect(200, { projects: [] }); +}); + +test('should return 404 when path is not enabled', async () => { + const app = express(); + const path = '/api/admin/projects'; + + app.use( + `${path}/suggest-changes`, + conditionalMiddleware( + () => false, + (req, res) => { + res.send({ suggestChanges: 'hello' }); + }, + ), + ); + + app.get(path, (req, res) => { + res.json({ projects: [] }); + }); + + await supertest(app).get('/api/admin/projects/suggest-changes').expect(404); +}); + +test('should respect ordering of endpoints', async () => { + const app = express(); + const path = '/api/admin/projects'; + + app.use( + path, + conditionalMiddleware( + () => true, + (req, res) => { + res.json({ name: 'Suggest changes' }); + }, + ), + ); + + app.get(path, (req, res) => { + res.json({ projects: [] }); + }); + + await supertest(app) + .get('/api/admin/projects') + .expect(200, { name: 'Suggest changes' }); +}); + +test('disabled middleware should not block paths that use the same basepath', async () => { + const app = express(); + const path = '/api/admin/projects'; + + app.use( + `${path}/suggest-changes`, + conditionalMiddleware( + () => false, + (req, res) => { + res.json({ name: 'Suggest changes' }); + }, + ), + ); + + app.get(path, (req, res) => { + res.json({ projects: [] }); + }); + + await supertest(app) + .get('/api/admin/projects') + .expect(200, { projects: [] }); +}); diff --git a/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap b/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap index 3adb52d20b..b001a7835a 100644 --- a/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap +++ b/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap @@ -4663,138 +4663,6 @@ If the provided project does not exist, the list of events will be empty.", ], }, }, - "/api/admin/invite-link/tokens": { - "get": { - "operationId": "getAllPublicSignupTokens", - "responses": { - "200": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/publicSignupTokensSchema", - }, - }, - }, - "description": "publicSignupTokensSchema", - }, - }, - "summary": "Retrieve all existing public signup tokens", - "tags": [ - "Public signup tokens", - ], - }, - "post": { - "operationId": "createPublicSignupToken", - "requestBody": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/publicSignupTokenCreateSchema", - }, - }, - }, - "description": "publicSignupTokenCreateSchema", - "required": true, - }, - "responses": { - "201": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/publicSignupTokenSchema", - }, - }, - }, - "description": "The resource was successfully created.", - "headers": { - "location": { - "description": "The location of the newly created resource.", - "schema": { - "format": "uri", - "type": "string", - }, - }, - }, - }, - }, - "summary": "Create a public signup token", - "tags": [ - "Public signup tokens", - ], - }, - }, - "/api/admin/invite-link/tokens/{token}": { - "get": { - "description": "Get information about a specific token. The \`:token\` part of the URL should be the token's secret.", - "operationId": "getPublicSignupToken", - "parameters": [ - { - "in": "path", - "name": "token", - "required": true, - "schema": { - "type": "string", - }, - }, - ], - "responses": { - "200": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/publicSignupTokenSchema", - }, - }, - }, - "description": "publicSignupTokenSchema", - }, - }, - "summary": "Retrieve a token", - "tags": [ - "Public signup tokens", - ], - }, - "put": { - "operationId": "updatePublicSignupToken", - "parameters": [ - { - "in": "path", - "name": "token", - "required": true, - "schema": { - "type": "string", - }, - }, - ], - "requestBody": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/publicSignupTokenUpdateSchema", - }, - }, - }, - "description": "publicSignupTokenUpdateSchema", - "required": true, - }, - "responses": { - "200": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/publicSignupTokenSchema", - }, - }, - }, - "description": "publicSignupTokenSchema", - }, - }, - "summary": "Update a public signup token", - "tags": [ - "Public signup tokens", - ], - }, - }, "/api/admin/metrics/applications": { "get": { "operationId": "getApplications", @@ -7282,74 +7150,6 @@ If the provided project does not exist, the list of events will be empty.", ], }, }, - "/api/frontend": { - "get": { - "operationId": "getFrontendFeatures", - "responses": { - "200": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/proxyFeaturesSchema", - }, - }, - }, - "description": "proxyFeaturesSchema", - }, - }, - "tags": [ - "Unstable", - ], - }, - }, - "/api/frontend/client/metrics": { - "post": { - "operationId": "registerFrontendMetrics", - "requestBody": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/proxyMetricsSchema", - }, - }, - }, - "description": "proxyMetricsSchema", - "required": true, - }, - "responses": { - "200": { - "description": "This response has no body.", - }, - }, - "tags": [ - "Unstable", - ], - }, - }, - "/api/frontend/client/register": { - "post": { - "operationId": "registerFrontendClient", - "requestBody": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/proxyClientSchema", - }, - }, - }, - "description": "proxyClientSchema", - "required": true, - }, - "responses": { - "200": { - "description": "This response has no body.", - }, - }, - "tags": [ - "Unstable", - ], - }, - }, "/auth/reset/password": { "post": { "operationId": "changePassword",