Breaking change: throw when attempting to re-define actions, roles or…

… entities.

This prevents accidentally overwriting authorization rules with weaker
rules. Such an accident becomes more likely as apps grow in size and
could lead to an security vulnerability.

Fixes tschaub#11

Gruntfile.js

@@ -11,6 +11,7 @@ module.exports = function(grunt) {
   var testSrc = 'test/**/*.js';
 
   var lintOptions = {
+    esversion: 6,
     curly: true,
     eqeqeq: true,
     indent: 2,

README.md

@@ -172,7 +172,7 @@ Register a getter for a role.  If the role is a string of the form
 `entity.relation`, a getter for the entity must be registered with the
 [`entity`](#manager.entity) method.  Roles without `.` are "simple" roles (e.g.
 `"admin"`) and no entity is looked up.  Throws [`ConfigError`](#configerror) if
-called with an invalid role name.
+called with an invalid or duplicate role name.
 
 #### <a id='manager.entity'>`entity(type, getter)`</a>
 
@@ -183,7 +183,7 @@ called with an invalid role name.
    generated while getting the entity and the second is the target entity.
 
 Register a getter for an entity.  Throws [`ConfigError`](#configerror) if called
-with invalid arguments.
+with invalid arguments or duplicate entity name.
 
 #### <a id='manager.action'>`action(name, roles)`</a>
 
@@ -194,7 +194,7 @@ with invalid arguments.
 
 Specify the roles that a user must have to perform the named action.  Throws
 [`ConfigError`](#configerror) if the provided roles have not yet been registered
-with the [`role`](#manager.role) method.
+with the [`role`](#manager.role) method or if the action name is a duplicate.
 
 #### <a id='manager.can'>`can(action)`</a>
 

lib/manager.js

@@ -73,6 +73,11 @@ Manager.prototype.action = function(name, roles) {
     }
     return role;
   });
+
+  if (name in this.actionDefs_) {
+    throw new errors.ConfigError(`action already registered: ${name}`);
+  }
+
   this.actionDefs_[name] = roles;
 };
 
@@ -181,6 +186,11 @@ Manager.prototype.entity = function(type, getter) {
     throw new errors.ConfigError(
         'Entity getter must be a function that takes two arguments');
   }
+
+  if (type in this.entityGetters_) {
+    throw new errors.ConfigError(`entity already registered: ${type}`);
+  }
+
   this.entityGetters_[type] = getter;
 };
 
@@ -294,6 +304,11 @@ Manager.prototype.role = function(role, getter) {
     throw new errors.ConfigError(
         'Getters for simple roles (without entities) take two arguments');
   }
+
+  if (role.name in this.roleGetters_) {
+    throw new errors.ConfigError(`role already registered: ${role.name}`);
+  }
+
   this.roleGetters_[role.name] = getter;
 };
 

package.json

@@ -47,5 +47,8 @@
   "dependencies": {
     "async": "^0.9.0",
     "pause": "0.0.1"
+  },
+  "engines": {
+    "node": ">=10"
   }
 }

test/lib/manager.spec.js

@@ -107,16 +107,25 @@ describe('Manager', function() {
         // pretend nobody is admin
         done(null, false);
       });
-      auth.role('page.author', function(page, req, done) {
-        // pretend everybody is author
-        done(null, true);
-      });
 
       assert.throws(function() {
         auth.action('can edit page', ['admin', 'page.author']);
       });
     });
 
+    it('throws when attempting to define action twice', function() {
+      auth.role('admin', function(req, done) {
+        done(null, false);
+      });
+
+      auth.action('can edit page', ['admin']);
+
+      assert.throws(function() {
+        auth.action('can edit page', ['admin']);
+      });
+    });
+
+
   });
 
   describe('#can()', function() {
@@ -318,6 +327,16 @@ describe('Manager', function() {
       }, ConfigError);
     });
 
+    it('throws when attempting to define entity twice', function() {
+      assert.doesNotThrow(function() {
+        auth.entity('page', (req, done) => done(null, {}));
+      });
+      assert.throws(() => {
+        auth.entity('page', (req, done) => done(null, {}));
+      });
+    });
+
+
   });
 
   describe('#role()', function() {
@@ -405,7 +424,15 @@ describe('Manager', function() {
       }, ConfigError);
     });
 
-  });
-
+    it('throws when attempting to define a role twice', function() {
+      auth.role('admin', function(req, done) {
+        // pretend nobody is admin
+        done(null, false);
+      });
 
+      assert.throws(() => {
+        auth.role('admin', (req, done) => done(null, false));
+      });
+    });
+  });
 });