Merge pull request #334 from cnpm/fix-permission

add permission check to /:name/:tag
cnpm · Apr 18, 2014 · e5a77a4 · e5a77a4
2 parents 6aae538 + b74cccd
commit e5a77a4
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 1 deletion.
diff --git a/controllers/registry/module.js b/controllers/registry/module.js
@@ -977,6 +977,16 @@ exports.updateTag = function *() {
     return;
   }
 
+  // check permission
+  if (!common.isMaintainer(this.user, mod.package.maintainers)) {
+    this.status = 403;
+    this.body = {
+      error: 'forbidden',
+      reason: 'no permission to modify ' + name
+    };
+    return;
+  }
+
   yield Module.addTag(name, tag, version);
   this.status = 201;
   this.body = {

diff --git a/routes/registry.js b/routes/registry.js
@@ -45,7 +45,7 @@ function routes(app) {
   app.put('/:name/sync', sync.sync);
   app.get('/:name/sync/log/:id', sync.getSyncLog);
 
-  app.put('/:name/:tag', mod.updateTag);
+  app.put('/:name/:tag', login, mod.updateTag);
 
   // need limit by ip
   app.get('/:name/download/:filename', limit, mod.download);

diff --git a/test/controllers/registry/module.test.js b/test/controllers/registry/module.test.js
@@ -690,6 +690,7 @@ describe('controllers/registry/module.test.js', function () {
       request(app)
       .put('/testputmodule/newtag')
       .set('content-type', 'application/json')
+      .set('authorization', baseauth)
       .send('"0.1.9"')
       .expect(201, done);
     });
@@ -698,6 +699,7 @@ describe('controllers/registry/module.test.js', function () {
       request(app)
       .put('/testputmodule/newtag')
       .set('content-type', 'application/json')
+      .set('authorization', baseauth)
       .send('"0.1.9"')
       .expect(201, done);
     });
@@ -706,6 +708,7 @@ describe('controllers/registry/module.test.js', function () {
       request(app)
       .put('/testputmodule/newtag')
       .set('content-type', 'application/json')
+      .set('authorization', baseauth)
       .send('"hello"')
       .expect(403)
       .expect({
@@ -718,13 +721,27 @@ describe('controllers/registry/module.test.js', function () {
       request(app)
       .put('/testputmodule/newtag')
       .set('content-type', 'application/json')
+      .set('authorization', baseauth)
       .send('"5.0.0"')
       .expect(403)
       .expect({
         error: 'forbidden',
         reason: 'setting tag newtag to unknown version: 5.0.0: testputmodule/newtag'
       }, done);
     });
+
+    it('should tag permission 403', function (done) {
+      request(app)
+      .put('/testputmodule/newtag')
+      .set('content-type', 'application/json')
+      .set('authorization', baseauthOther)
+      .send('"0.1.9"')
+      .expect(403)
+      .expect({
+        error: 'forbidden',
+        reason: 'no permission to modify testputmodule'
+      }, done);
+    });
   });
 
   describe('DELETE /:name/-rev/:rev', function () {