From a4a96eb39fe463b8fff9cdc885664be5c25dbf9f Mon Sep 17 00:00:00 2001
From: Eric <eric.barault@gmail.com>
Date: Wed, 23 Nov 2016 08:51:23 +0100
Subject: [PATCH] Add "returnOnlyRoleNames" option to Role.getRoles

Currently the return type of Role.getRoles() method is inconsistent:
role names are returned for smart roles and role ids are returned for
static roles (configured through user-role mapping).

This commit adds a new option to Role.getRoles() allowing the caller
to request role names to be returned for all types of roles.
---
 common/models/role.js | 22 ++++++++++++++++++----
 test/role.test.js     | 14 ++++++++++++++
 2 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/common/models/role.js b/common/models/role.js
index 680297967..e7e93c51c 100644
--- a/common/models/role.js
+++ b/common/models/role.js
@@ -383,7 +383,12 @@ module.exports = function(Role) {
    * @param {Error} err Error object.
    * @param {String[]} roles An array of role IDs
    */
-  Role.getRoles = function(context, callback) {
+  Role.getRoles = function(context, options, callback) {
+    if (!callback && typeof options === 'function') {
+      callback = options;
+      options = {};
+    }
+
     if (!(context instanceof AccessContext)) {
       context = new AccessContext(context);
     }
@@ -433,15 +438,24 @@ module.exports = function(Role) {
       if (principalType && principalId) {
         // Please find() treat undefined matches all values
         inRoleTasks.push(function(done) {
-          roleMappingModel.find({where: {principalType: principalType,
-            principalId: principalId}}, function(err, mappings) {
+          var filter = {where: {principalType: principalType, principalId: principalId}};
+          if (options.returnOnlyRoleNames === true) {
+            filter.include = ['role'];
+          }
+          roleMappingModel.find(filter, function(err, mappings) {
             debug('Role mappings found: %s %j', err, mappings);
             if (err) {
               if (done) done(err);
               return;
             }
             mappings.forEach(function(m) {
-              addRole(m.roleId);
+              var role;
+              if (options.returnOnlyRoleNames === true) {
+                role = m.toJSON().role.name;
+              } else {
+                role = m.roleId;
+              }
+              addRole(role);
             });
             if (done) done();
           });
diff --git a/test/role.test.js b/test/role.test.js
index 5362135a0..a7bbf8b7b 100644
--- a/test/role.test.js
+++ b/test/role.test.js
@@ -249,6 +249,20 @@ describe('role model', function() {
                   next();
                 });
             },
+            function(next) {
+              Role.getRoles(
+                { principalType: RoleMapping.USER, principalId: user.id },
+                { returnOnlyRoleNames: true },
+                function(err, roles) {
+                  if (err) return next(err);
+                  expect(roles).to.eql([
+                    Role.AUTHENTICATED,
+                    Role.EVERYONE,
+                    role.name,
+                  ]);
+                  next();
+                });
+            },
             function(next) {
               Role.getRoles(
                 { principalType: RoleMapping.APP, principalId: user.id },