fix(middleware): pass version to allow check (#4846)

* fix(middleware): pass version to allow check

* add tests

.changeset/angry-doors-tan.md

@@ -0,0 +1,5 @@
+---
+'@verdaccio/middleware': patch
+---
+
+fix(middleware): pass version to allow check

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<T>(
   auth: T,
   options = {
@@ -21,22 +25,33 @@ export function allow<T>(
         : 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();

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);
+});