Merge pull request #4 from kozzztya/owner-rule-request

Owner rule request

.travis.yml

@@ -1,6 +1,6 @@
 language: node_js
 node_js:
-  - '4'
+  - '6'
 services:
   - mongodb
 cache: yarn

README.md

@@ -21,12 +21,13 @@ const app = feathers();
 app.configure(rest())
   .configure(services)
   .configure(acl(aclConfig, {
-    denyNotAllowed: true,   // deny all routes without "allow" rules
-    mongooseConnection: db, // need for owner rule
+    denyNotAllowed: true,             // deny all routes without "allow" rules
+    adminRoles: ['admin'],            // allow all listed in config routes for this role
+    baseUrl: 'http://localhost:8080', // need for owner rule
     jwt: {
       secret: 'blab',
-      header: 'x-auth'      // Default is 'Authorization'
-      options: {}           // options for 'jsonwebtoken' lib
+      header: 'x-auth'                // Default is 'Authorization'
+      options: {}                     // options for 'jsonwebtoken' lib
     }
   }));
 
@@ -72,7 +73,7 @@ It gets user's role from `req.payload.roles` array.
 
 ### Owner
 
-Give access only for MongoDB document creator. First of all set:
+Give access only for owner. Makes request to GET route and checks `ownerField`. First of all set:
 
 ```
 app.configure(acl(config, { mongooseConnection: db }));
@@ -82,18 +83,11 @@ Then in config declare:
 
 ```
 allow: {
-  owner: {
-    where: { _id: '{params.id}' },
-    model: 'posts',
-    ownerField: 'author'
-  }
+  owner: { ownerField: 'author' }
 }
 ```
 
-`where` - how to find needed document. Set in {} path to needed values in `req` object. Default is `{ _id: '{params.id}' }`.
-`model` - mongoose model. By default can be got from route url. For example `posts` on `/posts`.
-`ownerField` - where you store user id?
-
+`ownerField` - where do you store user id?
 It gets user's id from `req.payload.userId`.
 
 ### Authenticated

package.json

@@ -1,10 +1,11 @@
 {
   "name": "feathers-acl",
-  "version": "0.7.0",
+  "version": "1.0.0",
   "description": "Declarative ACL for FeathersJS and Express apps",
   "main": "dist/index.js",
   "scripts": {
     "lint": "eslint --ignore-path .gitignore ./",
+    "prebuild": "rm -rf ./dist",
     "build": "babel -d dist/ src/",
     "prepublish": "npm run build",
     "ava": "ava -vs test/**/*.test.js",
@@ -15,6 +16,7 @@
     "http-errors": "1.6.1",
     "jsonwebtoken": "7.4.0",
     "lodash": "4.17.4",
+    "node-fetch": "1.6.3",
     "promdash": "1.2.0"
   },
   "devDependencies": {
@@ -34,8 +36,10 @@
     "feathers-hooks": "2.0.1",
     "feathers-mongoose": "5.1.0",
     "feathers-rest": "1.7.2",
+    "get-port-sync": "1.0.0",
     "mongoose": "4.9.8",
     "nyc": "10.3.2",
+    "random-port": "0.1.0",
     "supertest": "3.0.0"
   },
   "ava": {

src/index.js

@@ -3,8 +3,7 @@ const ruleChecker = require('./rule-checker');
 const jwtDecode = require('./jwt-decode');
 const denyNotAllowed = require('./deny-not-allowed');
 
-module.exports = (configs, options = {}) => function () {
-  const app = this;
+module.exports = (configs, options = {}) => function (app = this) {
   const check = ruleChecker(options);
 
   if (options.jwt) app.use(jwtDecode(options.jwt));

src/rule-checker/has-role.js

@@ -0,0 +1,5 @@
+const _ = require('lodash');
+
+module.exports = (adminRoles, payload) => {
+  return _.some(adminRoles, (role) => _.includes(_.get(payload, 'roles'), role));
+};

src/rule-checker/index.js

@@ -2,12 +2,15 @@ const promdash = require('promdash').default;
 const rolesRule = require('./rules/roles');
 const ownerRule = require('./rules/owner');
 const authenticatedRule = require('./rules/authenticated');
+const hasRole = require('./has-role');
+
+module.exports = ({ customRules, baseUrl, adminRoles }) => (payload, allow, req) => {
+  if (hasRole(adminRoles, payload)) return Promise.resolve(true);
 
-module.exports = ({ customRules, mongooseConnection }) => (payload, allow, req) => {
   const rules = Object.assign({
     roles: rolesRule(),
     authenticated: authenticatedRule(),
-    owner: ownerRule(mongooseConnection),
+    owner: ownerRule(baseUrl),
   }, customRules);
 
   return promdash.map(rules, rule => rule(payload, allow, req));

src/rule-checker/rules/owner.js

@@ -1,27 +1,28 @@
 const _ = require('lodash');
 const httpError = require('http-errors');
-const buildWhere = require('./build-where');
+const fetch = require('node-fetch');
 
-module.exports = (mongooseConnection) => (payload, allow, req) => {
+module.exports = (baseUrl) => (payload, allow, req) => {
   return new Promise((resolve, reject) => {
     if (!allow.owner) return resolve(true);
-    if (!mongooseConnection) return reject(httpError(500, 'No mongoose connection.'));
+    if (_.has(req, 'headers[x-owner-rule]')) return resolve(true);
 
     const userId = _.get(payload, 'userId');
-    const model = _.get(allow, 'owner.model') || _(req.url).split('/').get('[1]');
-    const whereTemplate = _.get(allow, 'owner.where') || { _id: '{params.id}' };
-    const where = buildWhere(whereTemplate, req);
+    const url = _.get(allow, 'owner.url') || req.url;
     const ownerField = _.get(allow, 'owner.ownerField');
 
     if (!userId) return reject(httpError(403, 'No user id.'));
 
-    const Model = mongooseConnection.model(model);
-    Model.findOne(where).then((doc) => {
-      if (!doc) return reject(httpError(500, 'Wrong where ' + JSON.stringify(where)));
-      if (!isOwner(doc[ownerField], userId)) return reject(httpError(403, 'No permissions.'));
+    fetch(baseUrl + url, {
+      method: 'GET', headers: Object.assign({ 'x-owner-rule': 'true' }, req.headers)
+    })
+      .then((res) => res.json())
+      .then((doc) => {
+        if (!isOwner(doc[ownerField], userId)) return reject(httpError(403, 'No permissions.'));
 
-      resolve(true);
-    }).catch(reject);
+        resolve(true);
+      })
+      .catch(reject);
   });
 };
 

src/rule-checker/rules/roles.js

@@ -1,13 +1,16 @@
 const _ = require('lodash');
 const httpError = require('http-errors');
+const hasRole = require('../has-role');
 
 module.exports = () => (payload, allow) => {
   const allowRolesList = _.get(allow, 'roles');
   if (!allowRolesList) return Promise.resolve(true);
 
   const roles = _.get(payload, 'roles');
-  const isAllowed = _.some(roles, (role) => _.includes(allowRolesList, role));
+  if (!roles) return Promise.reject(httpError(403, 'No role.'));
 
+  const isAllowed = hasRole(allowRolesList, payload);
   if (!isAllowed) return Promise.reject(httpError(403, 'Wrong roles: ' + roles));
+
   return Promise.resolve(isAllowed);
 };

test/env/app.js

@@ -7,9 +7,11 @@ const supertest = require('supertest');
 const lib = require('../../src');
 const db = require('./db');
 const jwt = require('./jwt');
+const randomPort = require('get-port-sync');
 
 module.exports = (config, options, payload) => {
   const app = feathers();
+  const port = randomPort();
 
   app.use(bodyParser.json());
   app.use(bodyParser.urlencoded({ extended: true }));
@@ -22,11 +24,11 @@ module.exports = (config, options, payload) => {
   });
   app.configure(lib(config, options &&
     Object.assign({
-      mongooseConnection: db, jwt: { secret: jwt.secret }, denyNotAllowed: true
+      jwt: { secret: jwt.secret }, denyNotAllowed: true, baseUrl: 'http://localhost:' + port
     }, options)));
   app.service('/posts', new Service({
     Model: db.model('posts')
   }));
 
-  return supertest(app);
+  return supertest(app.listen(port));
 };

test/env/db.js

@@ -3,7 +3,7 @@ const mongoose = require('mongoose');
 mongoose.Promise = Promise;
 const db = mongoose.createConnection('mongodb://localhost:27017/acl');
 
-const schema = new mongoose.Schema({ name: String, userId: String });
+const schema = new mongoose.Schema({ name: String, userId: String, usersIds: [String] });
 db.model('posts', schema);
 
 module.exports = db;

test/integration/owner.test.js

@@ -0,0 +1,63 @@
+const { test, App } = require('../env');
+
+const userId = '5901af327b35960019ee8b2e';
+const config = [{
+  url: '/posts', method: 'POST',
+  allow: {}
+}, {
+  url: '/posts/:id', method: 'GET',
+  allow: { owner: { ownerField: 'userId' } }
+}];
+
+test('should be no error for owner', async (t) => {
+  const app = App(config, {}, { userId });
+  const post = await app.post('/posts').send({ userId });
+  const { error } = await app.get('/posts/' + post.body._id);
+
+  t.falsy(error);
+});
+
+test('should be resolved for one of many owners', async (t) => {
+  const config = [{
+    url: '/posts', method: 'POST',
+    allow: {}
+  }, {
+    url: '/posts/:id', method: 'GET',
+    allow: { owner: { ownerField: 'usersIds' } }
+  }];
+  const app = App(config, {}, { userId });
+  const post = await app.post('/posts').send({ usersIds: [userId, '2', '3'] });
+  const { error } = await app.get('/posts/' + post.body._id);
+
+  t.falsy(error);
+});
+
+test('should be rejected for not owner', async (t) => {
+  const app = App(config, {}, { userId: '2' });
+  const post = await app.post('/posts').send({ userId });
+  const { error } = await app.get('/posts/' + post.body._id);
+
+  t.is(error.status, 403);
+  t.is(error.text, 'No permissions.');
+});
+
+test('should be rejected if no userId field', async (t) => {
+  const app = App(config, {}, {});
+  const post = await app.post('/posts').send({ userId });
+  const { error } = await app.get('/posts/' + post.body._id);
+
+  t.is(error.status, 403);
+  t.is(error.text, 'No user id.');
+});
+
+test('should be resolved if no rule', async (t) => {
+  const config = [
+    { url: '/posts', method: 'POST', allow: {} },
+    { url: '/posts/:id', method: 'GET', allow: {} }
+  ];
+  const app = App(config, {}, {});
+  const post = await app.post('/posts').send({ userId });
+  const { error } = await app.get('/posts/' + post.body._id);
+
+  t.falsy(error);
+});

test/integration/rules.test.js

@@ -2,18 +2,18 @@ const { test, App } = require('../env');
 
 const config = [{
   url: '/posts', method: 'POST',
-  allow: { roles: ['admin'] }
+  allow: { roles: ['admin', 'client'] }
 }, {
   url: '/posts/:id', method: 'GET',
   allow: {
     owner: { where: { _id: '{params.id}' }, ownerField: 'userId' },
-    roles: ['admin']
+    roles: ['admin', 'client']
   }
 }];
 const userId = '5901af327b35960019ee8b2e';
 
 test('should be allowed for all rules', async (t) => {
-  const app = App(config, {}, { roles: ['admin'], userId });
+  const app = App(config, {}, { roles: ['client'], userId });
   const post = await app.post('/posts').send({ userId });
   const { error } = await app.get('/posts/' + post.body._id);
 
@@ -30,10 +30,18 @@ test('should be denied because of role', async (t) => {
 });
 
 test('should be denied because of owner', async (t) => {
-  const app = App(config, {}, { roles: ['admin'], userId });
+  const app = App(config, {}, { roles: ['client'], userId });
   const post = await app.post('/posts').send();
   const { error } = await app.get('/posts/' + post.body._id);
 
   t.is(error.text, 'No permissions.');
   t.is(error.status, 403);
 });
+
+test('should be denied because of admin role', async (t) => {
+  const app = App(config, { adminRoles: ['admin'] }, { roles: ['admin'] });
+  const post = await app.post('/posts').send();
+  const { error } = await app.get('/posts/' + post.body._id);
+
+  t.falsy(error);
+});

test/unit/express.test.js

@@ -0,0 +1,28 @@
+const test = require('ava');
+const lib = require('../../src');
+const Supertest = require('supertest');
+const express = require('express');
+
+const app = express();
+
+test('should work for express', async (t) => {
+  const config = [{ url: '/blog', method: 'GET', allow: {} }];
+  const options = { denyNotAllowed: true };
+
+  lib(config, options)(app);
+  app.get('/blog', (req, res) => {
+    res.send({ message: 'hi' });
+  });
+
+  app.get('/secret', (req, res) => {
+    res.send({ message: 'secret' });
+  });
+
+  const supertest = Supertest(app);
+
+  const { error: getBlogError } = await supertest.get('/blog');
+  t.falsy(getBlogError);
+
+  const { error: getSecretError } = await supertest.get('/secret');
+  t.truthy(getSecretError);
+});