From aabe5fb1c41033f6b4c8804be2470a9a3ee82e3e Mon Sep 17 00:00:00 2001
From: Raymond Feng <raymond@strongloop.com>
Date: Mon, 16 Dec 2013 17:14:56 -0800
Subject: [PATCH] Fix a bug in merging ACLs

---
 lib/utils.js      | 12 +++-----
 test/util.test.js | 73 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+), 8 deletions(-)

diff --git a/lib/utils.js b/lib/utils.js
index 2dd6da47..806f9c2d 100644
--- a/lib/utils.js
+++ b/lib/utils.js
@@ -147,13 +147,9 @@ function mergeSettings(target, src) {
   if (array) {
     target = target || [];
     dst = dst.concat(target);
-    src.forEach(function (e, i) {
-      if (typeof target[i] === 'undefined') {
-        dst[i] = e;
-      } else {
-        if (target.indexOf(e) === -1) {
-          dst.push(e);
-        }
+    src.forEach(function (e) {
+      if (dst.indexOf(e) === -1) {
+        dst.push(e);
       }
     });
   } else {
@@ -170,7 +166,7 @@ function mergeSettings(target, src) {
         if (!target[key]) {
           dst[key] = src[key]
         } else {
-          dst[key] = mergeSettings(target[key], src[key])
+          dst[key] = mergeSettings(target[key], src[key]);
         }
       }
     });
diff --git a/test/util.test.js b/test/util.test.js
index c8234f16..815a01e6 100644
--- a/test/util.test.js
+++ b/test/util.test.js
@@ -2,6 +2,7 @@ var should = require('./init.js');
 var utils = require('../lib/utils');
 var fieldsToArray = utils.fieldsToArray;
 var removeUndefined = utils.removeUndefined;
+var mergeSettings = utils.mergeSettings;
 
 
 describe('util.fieldsToArray', function(){
@@ -114,4 +115,76 @@ describe('util.parseSettings', function(){
 
     });
 
+});
+
+describe('mergeSettings', function () {
+  it('should merge settings correctly', function () {
+    var src = { base: 'User',
+      relations: { accessTokens: { model: 'accessToken', type: 'hasMany',
+        foreignKey: 'userId' },
+        account: { model: 'account', type: 'belongsTo' } },
+      acls: [
+        { accessType: '*',
+          permission: 'DENY',
+          principalType: 'ROLE',
+          principalId: '$everyone' },
+        { accessType: '*',
+          permission: 'ALLOW',
+          principalType: 'ROLE',
+          property: 'login',
+          principalId: '$everyone' },
+        { permission: 'ALLOW',
+          property: 'findById',
+          principalType: 'ROLE',
+          principalId: '$owner' }
+      ] };
+    var tgt = { strict: false,
+      acls: [
+        { principalType: 'ROLE',
+          principalId: '$everyone',
+          permission: 'ALLOW',
+          property: 'create' },
+        { principalType: 'ROLE',
+          principalId: '$owner',
+          permission: 'ALLOW',
+          property: 'removeById' }
+      ],
+      maxTTL: 31556926,
+      ttl: 1209600 };
+
+    var dst = mergeSettings(tgt, src);
+
+    var expected = { strict: false,
+      acls: [
+        { principalType: 'ROLE',
+          principalId: '$everyone',
+          permission: 'ALLOW',
+          property: 'create' },
+        { principalType: 'ROLE',
+          principalId: '$owner',
+          permission: 'ALLOW',
+          property: 'removeById' },
+        { accessType: '*',
+          permission: 'DENY',
+          principalType: 'ROLE',
+          principalId: '$everyone' },
+        { accessType: '*',
+          permission: 'ALLOW',
+          principalType: 'ROLE',
+          property: 'login',
+          principalId: '$everyone' },
+        { permission: 'ALLOW',
+          property: 'findById',
+          principalType: 'ROLE',
+          principalId: '$owner' }
+      ],
+      maxTTL: 31556926,
+      ttl: 1209600,
+      base: 'User',
+      relations: { accessTokens: { model: 'accessToken', type: 'hasMany',
+        foreignKey: 'userId' },
+        account: { model: 'account', type: 'belongsTo' } } };
+
+    should.deepEqual(dst.acls, expected.acls, 'Merged settings should match the expectation');
+  });
 });
\ No newline at end of file