show maintainers when publish 403. fixed #430

controllers/registry/module.js

@@ -442,12 +442,13 @@ exports.addPackageAndDist = function *(next) {
   }
 
   // check maintainers
-  var isMaintainer = yield* packageService.isMaintainer(name, username);
-  if (!isMaintainer) {
+  var result = yield* packageService.authMaintainer(name, username);
+  if (!result.isMaintainer) {
     this.status = 403;
     this.body = {
       error: 'forbidden user',
-      reason: username + ' not authorized to modify ' + name
+      reason: username + ' not authorized to modify ' + name +
+        ', please contact maintainers: ' + result.maintainers.join(', ')
     };
     return;
   }

services/package.js

@@ -52,20 +52,13 @@ exports.removeAllMaintainers = function* (name) {
   return yield* ModuleMaintainer.removeAll(name);
 };
 
-exports.isMaintainer = function* (name, username) {
+exports.authMaintainer = function* (packageName, username) {
   var rs = yield [
-    ModuleMaintainer.get(name),
-    Module.getLatest(name)
+    ModuleMaintainer.get(packageName),
+    Module.getLatest(packageName)
   ];
   var maintainers = rs[0];
   var latestMod = rs[1];
-
-  if (latestMod && !latestMod.package._publish_on_cnpm) {
-    // no one can update public package maintainers
-    // public package only sync from source npm registry
-    return false;
-  }
-
   if (maintainers.length === 0) {
     // if not found maintainers, try to get from latest module package info
     var ms = latestMod && latestMod.package && latestMod.package.maintainers;
@@ -75,9 +68,27 @@ exports.isMaintainer = function* (name, username) {
       });
     }
   }
-  if (maintainers.length === 0) {
+
+  var isMaintainer = false;
+
+  if (latestMod && !latestMod.package._publish_on_cnpm) {
+    // no one can update public package maintainers
+    // public package only sync from source npm registry
+    isMaintainer = false;
+  } else if (maintainers.length === 0) {
     // no maintainers, meaning this module is free for everyone
-    return true;
+    isMaintainer = true;
+  } else if (maintainers.indexOf(username) >= 0) {
+    isMaintainer = true;
   }
-  return maintainers.indexOf(username) >= 0;
+
+  return {
+    isMaintainer: isMaintainer,
+    maintainers: maintainers
+  };
+};
+
+exports.isMaintainer = function* (name, username) {
+  var result = yield* exports.authMaintainer(name, username);
+  return result.isMaintainer;
 };

test/controllers/registry/module.test.js

@@ -569,6 +569,34 @@ describe('controllers/registry/module.test.js', function () {
       });
     });
 
+    it('should 403 when user is not maintainer', function (done) {
+      mm(config, 'enablePrivate', false);
+      var pkg = utils.getPackage('@cnpmtest/testpublishmodule-not-maintainer', '0.0.1');
+      request(app)
+      .put('/' + pkg.name)
+      .set('authorization', utils.adminAuth)
+      .send(pkg)
+      .expect(201, function (err, res) {
+        should.not.exist(err);
+        res.body.should.have.keys('ok', 'rev');
+        res.body.ok.should.equal(true);
+
+        // upload again should 403
+        request(app)
+        .put('/' + pkg.name)
+        .set('authorization', utils.otherUserAuth)
+        .send(pkg)
+        .expect(403, function (err, res) {
+          should.not.exist(err);
+          res.body.should.eql({
+            error: 'forbidden user',
+            reason: 'cnpmjstest101 not authorized to modify @cnpmtest/testpublishmodule-not-maintainer, please contact maintainers: cnpmjstest10'
+          });
+          done();
+        });
+      });
+    });
+
     it('should version_error when versions missing', function (done) {
       var pkg = utils.getPackage('version_missing_module');
       delete pkg.versions;