diff --git a/src/lib/app.ts b/src/lib/app.ts index 346c952641..6c1308b8db 100644 --- a/src/lib/app.ts +++ b/src/lib/app.ts @@ -29,7 +29,6 @@ import maintenanceMiddleware from './middleware/maintenance-middleware'; import { unless } from './middleware/unless-middleware'; import { catchAllErrorHandler } from './middleware/catch-all-error-handler'; import NotFoundError from './error/notfound-error'; -import { rejectDoubleSlashesInPath } from './middleware/reject-double-slashes-in-path'; export default async function getApp( config: IUnleashConfig, @@ -93,7 +92,6 @@ export default async function getApp( if (config.enableOAS && services.openApiService) { services.openApiService.useDocs(app); } - app.use(`${baseUriPath}/`, rejectDoubleSlashesInPath); // Support CORS preflight requests for the frontend endpoints. // Preflight requests should not have Authorization headers, // so this must be handled before the API token middleware. diff --git a/src/lib/middleware/api-token-middleware.ts b/src/lib/middleware/api-token-middleware.ts index 0fd689f98e..3c1b3046ad 100644 --- a/src/lib/middleware/api-token-middleware.ts +++ b/src/lib/middleware/api-token-middleware.ts @@ -4,7 +4,7 @@ import { IUnleashConfig } from '../types/option'; import { IAuthRequest } from '../routes/unleash-types'; const isClientApi = ({ path }) => { - return path && path.startsWith('/api/client'); + return path && path.indexOf('/api/client') > -1; }; const isProxyApi = ({ path }) => { @@ -15,16 +15,18 @@ const isProxyApi = ({ path }) => { // Handle all our current proxy paths which will redirect to the new // embedded proxy endpoint return ( - path.startsWith('/api/proxy') || - path.startsWith('/api/development/proxy') || - path.startsWith('/api/production/proxy') || - path.startsWith('/api/frontend') + path.indexOf('/api/proxy') > -1 || + path.indexOf('/api/development/proxy') > -1 || + path.indexOf('/api/production/proxy') > -1 || + path.indexOf('/api/frontend') > -1 ); }; export const TOKEN_TYPE_ERROR_MESSAGE = 'invalid token: expected a different token type for this endpoint'; +export const NO_TOKEN_WHERE_TOKEN_WAS_REQUIRED = + 'This endpoint requires an API token. Please add an authorization header to your request with a valid token'; const apiAccessMiddleware = ( { getLogger, @@ -64,6 +66,14 @@ const apiAccessMiddleware = ( return; } req.user = apiUser; + } else if (isClientApi(req) || isProxyApi(req)) { + // If we're here, we know that api token middleware was enabled, otherwise we'd returned a no-op middleware + // We explicitly only protect client and proxy apis, since admin apis are protected by our permission checker + // Reject with 401 + res.status(401).send({ + message: NO_TOKEN_WHERE_TOKEN_WAS_REQUIRED, + }); + return; } } } catch (error) { diff --git a/src/lib/middleware/reject-double-slashes-in-path.ts b/src/lib/middleware/reject-double-slashes-in-path.ts deleted file mode 100644 index 5d5388ddb0..0000000000 --- a/src/lib/middleware/reject-double-slashes-in-path.ts +++ /dev/null @@ -1,11 +0,0 @@ -import { RequestHandler } from 'express'; - -const MULTIPLE_SLASHES = /\/\/+/; - -export const rejectDoubleSlashesInPath: RequestHandler = (req, res, next) => { - if (req.path.match(MULTIPLE_SLASHES)) { - res.status(404).send(); - } else { - next(); - } -}; diff --git a/src/test/e2e/api/auth/leading-slashes-are-stripped.e2e.test.ts b/src/test/e2e/api/auth/leading-slashes-are-stripped.e2e.test.ts index 0feb8533e0..82646ea790 100644 --- a/src/test/e2e/api/auth/leading-slashes-are-stripped.e2e.test.ts +++ b/src/test/e2e/api/auth/leading-slashes-are-stripped.e2e.test.ts @@ -19,7 +19,7 @@ beforeAll(async () => { authentication: { enableApiToken: true, type: IAuthType.DEMO }, }); appWithBaseUrl = await setupAppWithAuth(stores, { - server: { baseUriPath: '/demo' }, + server: { unleashUrl: 'http://localhost:4242', basePathUri: '/demo' }, authentication: { enableApiToken: true, type: IAuthType.DEMO }, }); }); @@ -31,17 +31,13 @@ afterAll(async () => { test('Access to /api/client/features are refused no matter how many leading slashes', async () => { await app.request.get('/api/client/features').expect(401); - await app.request.get('/////api/client/features').expect(404); - await app.request.get('//api/client/features').expect(404); -}); - -test('Multiple slashes anywhere in the path is not a URL that exists', async () => { - await app.request.get('/api/admin///projects/default/features').expect(404); - await app.request.get('/api/client///features').expect(404); + await app.request.get('/////api/client/features').expect(401); + await app.request.get('//api/client/features').expect(401); }); test('multiple slashes after base path is also rejected with 404', async () => { - await appWithBaseUrl.request.get('/demo///api/client/features').expect(404); + await appWithBaseUrl.request.get('/demo///api/client/features').expect(401); + await appWithBaseUrl.request.get('/demo//api/client/features').expect(401); await appWithBaseUrl.request.get('/demo/api/client/features').expect(401); });