From e85069010fa7bf1ecf9d01c9f597124f7e3b4700 Mon Sep 17 00:00:00 2001 From: Marc Bernard <59966492+mbtools@users.noreply.github.com> Date: Wed, 25 Sep 2024 17:13:31 -0400 Subject: [PATCH] fix(middleware): pass version to allow check (#4846) * fix(middleware): pass version to allow check * add tests --- .changeset/angry-doors-tan.md | 5 ++ packages/middleware/src/middlewares/allow.ts | 25 ++++-- packages/middleware/test/allow.spec.ts | 88 +++++++++++--------- 3 files changed, 75 insertions(+), 43 deletions(-) create mode 100644 .changeset/angry-doors-tan.md diff --git a/.changeset/angry-doors-tan.md b/.changeset/angry-doors-tan.md new file mode 100644 index 000000000..89f5697a5 --- /dev/null +++ b/.changeset/angry-doors-tan.md @@ -0,0 +1,5 @@ +--- +'@verdaccio/middleware': patch +--- + +fix(middleware): pass version to allow check diff --git a/packages/middleware/src/middlewares/allow.ts b/packages/middleware/src/middlewares/allow.ts index 014c99f72..94720afc9 100644 --- a/packages/middleware/src/middlewares/allow.ts +++ b/packages/middleware/src/middlewares/allow.ts @@ -1,8 +1,12 @@ +import buildDebug from 'debug'; + import { API_ERROR, errorUtils } from '@verdaccio/core'; import { getVersionFromTarball } from '@verdaccio/utils'; import { $NextFunctionVer, $RequestExtend, $ResponseExtend } from '../types'; +const debug = buildDebug('verdaccio:middleware:allow'); + export function allow( auth: T, options = { @@ -21,22 +25,33 @@ export function allow( : req.params.package; const packageVersion = req.params.filename ? getVersionFromTarball(req.params.filename) - : undefined; - const remote = req.remote_user; + : req.params.version + ? req.params.version + : undefined; + const remote_user = req.remote_user; + debug( + 'check if user %o can %o package %o version %o', + remote_user?.name, + action, + packageName, + packageVersion + ); beforeAll?.( - { action, user: remote?.name }, + { action, user: remote_user?.name }, `[middleware/allow][@{action}] allow for @{user}` ); auth['allow_' + action]( { packageName, packageVersion }, - remote, + remote_user, function (error, allowed): void { req.resume(); if (error) { + debug('user is NOT allowed to %o', action); next(error); } else if (allowed) { + debug('user is allowed to %o', action); afterAll?.( - { action, user: remote?.name }, + { action, user: remote_user?.name }, `[middleware/allow][@{action}] allowed for @{user}` ); next(); diff --git a/packages/middleware/test/allow.spec.ts b/packages/middleware/test/allow.spec.ts index 48330d6bf..fda8fb15f 100644 --- a/packages/middleware/test/allow.spec.ts +++ b/packages/middleware/test/allow.spec.ts @@ -1,22 +1,16 @@ import request from 'supertest'; import { HTTP_STATUS } from '@verdaccio/core'; -import { logger, setup } from '@verdaccio/logger'; import { allow } from '../src'; import { getApp } from './helper'; -setup({}); - test('should allow request', async () => { - const can = allow( - { - allow_publish: (params, remove, cb) => { - return cb(null, true); - }, + const can = allow({ + allow_publish: (params, remove, cb) => { + return cb(null, true); }, - logger - ); + }); const app = getApp([]); // @ts-ignore app.get('/:package', can('publish'), (req, res) => { @@ -27,14 +21,11 @@ test('should allow request', async () => { }); test('should allow scope request', async () => { - const can = allow( - { - allow_publish: (params, remove, cb) => { - return cb(null, true); - }, + const can = allow({ + allow_publish: (params, remove, cb) => { + return cb(null, true); }, - logger - ); + }); const app = getApp([]); // @ts-ignore app.get('/:package/:scope', can('publish'), (req, res) => { @@ -45,14 +36,11 @@ test('should allow scope request', async () => { }); test('should allow filename request', async () => { - const can = allow( - { - allow_publish: (params, remove, cb) => { - return cb(null, true); - }, + const can = allow({ + allow_publish: (params, remove, cb) => { + return cb(null, true); }, - logger - ); + }); const app = getApp([]); // @ts-ignore app.get('/:filename', can('publish'), (req, res) => { @@ -63,14 +51,11 @@ test('should allow filename request', async () => { }); test('should not allow request', async () => { - const can = allow( - { - allow_publish: (params, remove, cb) => { - return cb(null, false); - }, + const can = allow({ + allow_publish: (params, remove, cb) => { + return cb(null, false); }, - logger - ); + }); const app = getApp([]); // @ts-ignore app.get('/sec', can('publish'), (req, res) => { @@ -81,17 +66,44 @@ test('should not allow request', async () => { }); test('should handle error request', async () => { - const can = allow( - { - allow_publish: (params, remove, cb) => { - return cb(Error('foo error')); - }, + const can = allow({ + allow_publish: (params, remove, cb) => { + return cb(Error('foo error')); }, - logger - ); + }); const app = getApp([]); // @ts-ignore app.get('/err', can('publish')); return request(app).get('/err').expect(HTTP_STATUS.INTERNAL_ERROR); }); + +test('should allow request with version', async () => { + const can = allow({ + allow_publish: (params, remove, cb) => { + return params.packageVersion === '1.0.0' ? cb(null, true) : cb(null, false); + }, + }); + const app = getApp([]); + // @ts-ignore + app.get('/:package/:version', can('publish'), (req, res) => { + res.status(HTTP_STATUS.OK).json({}); + }); + + return request(app).get('/pacman/1.0.0').expect(HTTP_STATUS.OK); +}); + +test('should not allow request with version', async () => { + const can = allow({ + allow_publish: (params, remove, cb) => { + return params.packageVersion === '1.0.0' ? cb(null, true) : cb(null, false); + }, + }); + const app = getApp([]); + // @ts-ignore + app.get('/:package/:version', can('publish'), (req, res) => { + res.status(HTTP_STATUS.OK).json({}); + }); + + return request(app).get('/pacman/2.0.0').expect(HTTP_STATUS.INTERNAL_ERROR); +});