AMBARI-15552: Role selection in List view of Manage Ambari page does not work correctly (Keta Patel via rzang)

ambari-admin/src/main/resources/ui/admin-web/app/scripts/controllers/clusters/UserAccessListCtrl.js

@@ -72,7 +72,9 @@ function($scope, $location, Cluster, $modal, $rootScope, $routeParams, Permissio
         var privilege = $scope.pickEffectivePrivilege(user.privileges);
         // Redefine principal_name and principal type in case of None
         privilege.principal_name = user.Users? user.Users.user_name : user.Groups.group_name;
-        privilege.principal_type = user.Users? 'USER' : 'GROUP';
+        if (privilege.permission_label === "None") {
+          privilege.principal_type = user.Users ? 'USER' : 'GROUP';
+        }
         var name = encodeURIComponent(privilege.principal_name);
         privilege.encoded_name = name;
         privilege.original_perm = privilege.permission_name;
@@ -86,11 +88,10 @@ function($scope, $location, Cluster, $modal, $rootScope, $routeParams, Permissio
   };
 
   $scope.pickEffectivePrivilege = function(privileges) {
-    var orderedRoles = Cluster.orderedRoles.concat(['VIEW.USER']);
     if (privileges && privileges.length > 0) {
       return privileges.reduce(function(prev, cur) {
-        var prevIndex = orderedRoles.indexOf(prev.PrivilegeInfo.permission_name);
-        var curIndex = orderedRoles.indexOf(cur.PrivilegeInfo.permission_name)
+        var prevIndex = $scope.getRoleRank(prev.PrivilegeInfo.permission_name);
+        var curIndex = $scope.getRoleRank(cur.PrivilegeInfo.permission_name)
         return (prevIndex < curIndex) ? prev : cur;
       }).PrivilegeInfo;
     } else {
@@ -123,20 +124,111 @@ function($scope, $location, Cluster, $modal, $rootScope, $routeParams, Permissio
     });
   };
 
+  $scope.getRoleRank = function(permission_name) {
+    var orderedRoles = Cluster.orderedRoles.concat(['VIEW.USER','CLUSTER.NONE']);
+    var index = orderedRoles.indexOf(permission_name);
+    return index;
+  };
+
   $scope.save = function(user) {
-    var fromNone = user.original_perm == $scope.NONE_ROLE.permission_name;
+    var fromNone = (user.original_perm === $scope.NONE_ROLE.permission_name);
     if (fromNone) {
       $scope.addPrivilege(user);
       return;
     }
-    Cluster.deletePrivilege(
-      $routeParams.id,
-      user.privilege_id
-    ).then(
-      function() {
-        $scope.addPrivilege(user);
-      }
-    );
+
+    if ($scope.isUserActive) {
+      Cluster.getPrivilegesForResource({
+          nameFilter : user.user_name,
+          typeFilter : $scope.currentTypeFilter,
+      }).then(function(data) {
+        var arrayOfPrivileges = data.items[0].privileges;
+        var privilegesOfTypeUser = [];
+        var privilegesOfTypeGroup = [];
+        for (var i = 0; i < arrayOfPrivileges.length; i++) {
+          if(arrayOfPrivileges[i].PrivilegeInfo.principal_type === "GROUP"){
+            privilegesOfTypeGroup.push(arrayOfPrivileges[i]);
+          } else {
+            privilegesOfTypeUser.push(arrayOfPrivileges[i].PrivilegeInfo);
+          }
+        }
+
+        var effectivePrivilege = $scope.pickEffectivePrivilege(arrayOfPrivileges);
+        var effectivePrivilegeFromGroups = $scope.pickEffectivePrivilege(privilegesOfTypeGroup);
+        user.principal_type = 'USER';
+        user.original_perm = effectivePrivilege.permission_name;
+        user.editable = (Cluster.ineditableRoles.indexOf(effectivePrivilege.permission_name) === -1);
+
+        //add a new privilege of type USER only if it is also the effective privilege considering the user's Group privileges
+        var curIndex = $scope.getRoleRank(user.permission_name);
+        var prevIndex = -1;
+        if (privilegesOfTypeGroup.length !== 0) {
+          prevIndex = $scope.getRoleRank(effectivePrivilegeFromGroups.permission_name);
+        }
+        if ((curIndex === 6) || (curIndex <= prevIndex)) {
+          var privilege_ids = [];
+          privilegesOfTypeUser.forEach(function(privilegeOfTypeUser) {
+            privilege_ids.push(privilegeOfTypeUser.privilege_id);
+          });
+
+          //delete all privileges of type USER, if they exist
+          //then add the privilege for the user, after which the user displays the effective privilege
+          if(privilege_ids.length !== 0) {
+            Cluster.deleteMultiplePrivileges(
+                $routeParams.id,
+                privilege_ids
+            )
+            .then(function() {
+              $scope.addPrivilege(user);
+            });
+          } else {
+            $scope.addPrivilege(user);
+          }
+        } else {
+          Alert.error($t('common.alerts.cannotSavePermissions'),
+              $t('users.alerts.usersEffectivePrivilege', {user_name : user.user_name})
+          );
+          $scope.loadUsers();
+        }
+      });
+    } else {
+      Cluster.getPrivilegesForResource({
+          nameFilter : user.group_name,
+          typeFilter : $scope.currentTypeFilter
+      }).then(function(data) {
+        var arrayOfPrivileges = data.items[0].privileges;
+        var privilegesOfTypeGroup = [];
+        var privilege = $scope.pickEffectivePrivilege(arrayOfPrivileges);
+        user.principal_type = 'GROUP';
+        user.original_perm = privilege.permission_name;
+        user.editable = (Cluster.ineditableRoles.indexOf(privilege.permission_name) === -1);
+
+        arrayOfPrivileges.forEach(function(privilegeOfTypeGroup) {
+          if (privilegeOfTypeGroup.PrivilegeInfo.principal_type === "GROUP") {
+            privilegesOfTypeGroup.push(privilegeOfTypeGroup.PrivilegeInfo);
+          }
+        });
+
+        var privilege_ids = [];
+        privilegesOfTypeGroup.forEach(function(privilegeOfTypeGroup) {
+          privilege_ids.push(privilegeOfTypeGroup.privilege_id);
+        });
+
+        //delete all privileges of type GROUP, if they exist
+        //then add the privilege for the group, after which the group displays the effective privilege
+        if(privilege_ids.length !== 0) {
+          Cluster.deleteMultiplePrivileges(
+              $routeParams.id,
+              privilege_ids
+          )
+          .then(function() {
+            $scope.addPrivilege(user);
+          });
+        } else {
+          $scope.addPrivilege(user);
+        }
+      });
+    }
   };
 
   $scope.cancel = function(user) {
@@ -146,7 +238,13 @@ function($scope, $location, Cluster, $modal, $rootScope, $routeParams, Permissio
   $scope.addPrivilege = function(user) {
     var changeToNone = user.permission_name == $scope.NONE_ROLE.permission_name;
     if (changeToNone) {
-      $scope.showSuccess(user);
+      if ($scope.isUserActive) {
+        Alert.success($t('users.alerts.roleChangedToNone', {
+            user_name : user.user_name
+        }));
+      } else {
+        $scope.showSuccess(user);
+      }
       $scope.loadUsers();
       return;
     }

ambari-admin/src/main/resources/ui/admin-web/app/scripts/i18n.config.js

@@ -363,7 +363,9 @@ angular.module('ambariAdminConsole')
         'cannotAddUser': 'Cannot add user to group',
         'passwordChanged': 'Password changed.',
         'cannotChangePassword': 'Cannot change password',
-        'roleChanged': '{{name}} changed to {{role}}'
+        'roleChanged': '{{name}} changed to {{role}}',
+        'roleChangedToNone': '{{user_name}}\'s explicit privilege has been changed to \'NONE\'. Any privilege now seen for this user comes through its Group(s).',
+        'usersEffectivePrivilege': '{{user_name}}\'s effective privilege through its Group(s) is higher than your selected privilege.'
       }
     },
 

ambari-admin/src/main/resources/ui/admin-web/app/scripts/services/Cluster.js

@@ -230,6 +230,25 @@ angular.module('ambariAdminConsole')
 
       return deferred.promise;
     },
+    getPrivilegesForResource: function(params) {
+      var deferred = $q.defer();
+      var isUser = (params.typeFilter.value == 'USER');
+      var endpoint = isUser ? '/users' : '/groups';
+      var nameURL = isUser ? '&Users/user_name.matches(' : '&Groups/group_name.matches(';
+      var nameFilter = params.nameFilter ? (nameURL + params.nameFilter + ')') : '';
+      $http({
+        method : 'GET',
+        url : Settings.baseUrl + endpoint + '?' + 'fields=privileges/PrivilegeInfo/*' + nameFilter
+      })
+      .success(function(data) {
+        deferred.resolve(data);
+      })
+      .catch(function(data) {
+        deferred.reject(data);
+      });
+
+      return deferred.promise;
+    },
     createPrivileges: function(params, data) {
       return $http({
         method: 'POST',
@@ -244,6 +263,12 @@ angular.module('ambariAdminConsole')
         data: data
       });
     },
+    deleteMultiplePrivileges: function(clusterId, privilege_ids) {
+      return $http({
+        method: 'DELETE',
+        url: Settings.baseUrl + '/clusters/'+clusterId+'/privileges?PrivilegeInfo/privilege_id.in\('+privilege_ids+'\)'
+      });
+    },
     updatePrivileges: function(params, privileges) {
       return $http({
         method: 'PUT',

ambari-admin/src/main/resources/ui/admin-web/test/unit/controllers/clusters/UserAccessListCtrl_test.js

@@ -31,8 +31,11 @@ describe('#Cluster', function () {
       Cluster = _Cluster_;
       Alert = _Alert_;
       deferred = {
-        deletePrivilege: _$q_.defer(),
-        createPrivileges: _$q_.defer()
+        createPrivileges: _$q_.defer(),
+        getPrivilegesForResource: _$q_.defer(),
+        getPrivilegesWithFilters: _$q_.defer(),
+        deletePrivileges: _$q_.defer(),
+        deleteMultiplePrivileges: _$q_.defer()
       };
       ctrl = $controller('UserAccessListCtrl', {
         $scope: scope
@@ -42,11 +45,14 @@ describe('#Cluster', function () {
         Alert: Alert,
         scope: scope
       };
-      spyOn(Cluster, 'deletePrivilege').andReturn(deferred.deletePrivilege.promise);
       spyOn(Cluster, 'createPrivileges').andReturn(deferred.createPrivileges.promise);
+      spyOn(Cluster, 'deletePrivileges').andReturn(deferred.deletePrivileges.promise);
+      spyOn(Cluster, 'getPrivilegesForResource').andReturn(deferred.getPrivilegesForResource.promise);
+      spyOn(Cluster, 'getPrivilegesWithFilters').andReturn(deferred.getPrivilegesWithFilters.promise);
       spyOn(Alert, 'success').andCallFake(angular.noop);
       spyOn(Alert, 'error').andCallFake(angular.noop);
       spyOn(scope, 'loadRoles').andCallFake(angular.noop);
+
       $httpBackend.expectGET(/\/api\/v1\/permissions/).respond(200, {
         items: []
       });
@@ -242,6 +248,548 @@ describe('#Cluster', function () {
       });
 
     });
-  });
 
+    describe('#save() for Users', function(){
+      var user = {};
+
+      beforeEach(function() {
+        items1 = {
+            "href" : "http://abc.com:8080/api/v1/users/user1",
+            "Users" : { "user_name" : "user1" },
+            "privileges" : [
+              {
+                "href" : "http://abc.com:8080/api/v1/users/user1/privileges/222",
+                "PrivilegeInfo" : {
+                  "cluster_name" : "myCluster",
+                  "permission_label" : "Service Administrator",
+                  "permission_name" : "SERVICE.ADMINISTRATOR",
+                  "principal_name" : "mygroup2",
+                  "principal_type" : "GROUP",
+                  "privilege_id" : 222,
+                  "type" : "CLUSTER",
+                  "user_name" : "user1"
+                }
+              }, {
+                "href" : "http://abc.com:8080/api/v1/users/user1/privileges/111",
+                "PrivilegeInfo" : {
+                  "cluster_name" : "myCluster",
+                  "permission_label" : "Service Administrator",
+                  "permission_name" : "SERVICE.ADMINISTRATOR",
+                  "principal_name" : "mygroup",
+                  "principal_type" : "GROUP",
+                  "privilege_id" : 111,
+                  "type" : "CLUSTER",
+                  "user_name" : "user1"
+                }
+              }, {
+                "href" : "http://abc.com:8080/api/v1/users/user1/privileges/11",
+                "PrivilegeInfo" : {
+                  "cluster_name" : "myCluster",
+                  "permission_label" : "Cluster Administrator",
+                  "permission_name" : "CLUSTER.ADMINISTRATOR",
+                  "principal_name" : "user1",
+                  "principal_type" : "USER",
+                  "privilege_id" : 11,
+                  "type" : "CLUSTER",
+                  "user_name" : "user1"
+                }
+              }
+            ]
+          };
+
+        items2 =
+          {
+            "href" : "http://abc.com:8080/api/v1/users/user2",
+            "Users" : { "user_name" : "user2" },
+            "privileges" : [
+              {
+                "href" : "http://abc.com:8080/api/v1/users/user2/privileges/111",
+                "PrivilegeInfo" : {
+                  "cluster_name" : "myCluster",
+                  "permission_label" : "Service Administrator",
+                  "permission_name" : "SERVICE.ADMINISTRATOR",
+                  "principal_name" : "mygroup",
+                  "principal_type" : "GROUP",
+                  "privilege_id" : 111,
+                  "type" : "CLUSTER",
+                  "user_name" : "user2"
+                }
+              }, {
+                "href" : "http://abc.com:8080/api/v1/users/user2/privileges/22",
+                "PrivilegeInfo" : {
+                  "cluster_name" : "myCluster",
+                  "permission_label" : "Service Administrator",
+                  "permission_name" : "SERVICE.ADMINISTRATOR",
+                  "principal_name" : "user2",
+                  "principal_type" : "USER",
+                  "privilege_id" : 22,
+                  "type" : "CLUSTER",
+                  "user_name" : "user2"
+                }
+              }
+            ]
+          };
+
+        all_items = { "items": [items1, items2]};
+
+        scope.loadUsers();
+
+        spyOn(Cluster, 'deleteMultiplePrivileges').andCallFake(function(clusterId, privilege_ids) {
+          privilege_ids.forEach(function(privilege_id) {
+            items1.privileges.forEach(function(p, index) {
+              if (p.PrivilegeInfo.privilege_id === privilege_id) {
+                //Remove from array
+                items1.privileges.splice(index, 1);
+              }
+            });
+          });
+        });
+        spyOn(scope, 'addPrivilege').andCallFake(function(user) {
+          var p = {};
+          p.PrivilegeInfo = {};
+          p.PrivilegeInfo.privilege_id = user.privilege_id + 1;
+          p.PrivilegeInfo.permission_name = user.permission_name;
+          p.PrivilegeInfo.principal_type = 'USER';
+
+          items1.privileges.push(p);
+          scope.loadUsers();
+        });
+
+        deferred.getPrivilegesWithFilters.resolve(all_items);
+        deferred.getPrivilegesForResource.promise.then(function(data) {
+          var arrayOfPrivileges = data.items[0].privileges;
+          var privilegesOfTypeUser = [];
+          var privilegesOfTypeGroup = [];
+          for (var i = 0; i < arrayOfPrivileges.length; i++) {
+            if(arrayOfPrivileges[i].PrivilegeInfo.principal_type === "GROUP"){
+              privilegesOfTypeGroup.push(arrayOfPrivileges[i]);
+            } else {
+              privilegesOfTypeUser.push(arrayOfPrivileges[i].PrivilegeInfo);
+            }
+          }
+
+          var effectivePrivilege = scope.pickEffectivePrivilege(arrayOfPrivileges);
+          var effectivePrivilegeFromGroups = scope.pickEffectivePrivilege(privilegesOfTypeGroup);
+          user.principal_type = 'USER';
+          user.original_perm = effectivePrivilege.permission_name;
+          user.editable = (Cluster.ineditableRoles.indexOf(effectivePrivilege.permission_name) === -1);
+
+          //add a new privilege of type USER only if it is also the effective privilege considering the user's Group privileges
+          var curIndex = scope.getRoleRank(user.permission_name);
+          var prevIndex = -1;
+          if (privilegesOfTypeGroup.length !== 0) {
+            prevIndex = scope.getRoleRank(effectivePrivilegeFromGroups.permission_name);
+          }
+          if ((curIndex === 6) || (curIndex <= prevIndex)) {
+            var privilege_ids = [];
+            privilegesOfTypeUser.forEach(function(privilegeOfTypeUser) {
+              privilege_ids.push(privilegeOfTypeUser.privilege_id);
+            });
+
+            //delete all privileges of type USER, if they exist
+            //then add the privilege for the user, after which the user displays the effective privilege
+            if(privilege_ids.length !== 0) {
+              Cluster.deleteMultiplePrivileges(
+                123,
+                privilege_ids
+              );
+            }
+            scope.addPrivilege(user);
+          } else {
+            Alert.error($t('common.alerts.cannotSavePermissions'), "User's effective privilege through its Group(s) is higher than your selected privilege.");
+            scope.loadUsers();
+          }
+        });
+
+        scope.$apply();
+      });
+
+      it('Should save the Privilege equal to the user\'s group privileges. Should also remove any individual user privileges', function() {
+        //using 'user1' for updating the new privilege
+        user.principal_name = scope.users[0].principal_name;
+        user.principal_type = scope.users[0].principal_type;
+        user.privilege_id = scope.users[0].privilege_id;
+        user.permission_name = "SERVICE.ADMINISTRATOR";
+
+        deferred.getPrivilegesForResource.resolve({ "items" : [items1] });
+
+        scope.$apply();
+
+        expect(scope.users[0].permission_name).toEqual(user.permission_name);
+        expect(scope.users[0].privilege_id).toEqual(user.privilege_id + 1);
+
+        var oldPrivilege = {
+          "href" : "http://abc.com:8080/api/v1/users/user1/privileges/11",
+          "PrivilegeInfo" : {
+            "cluster_name" : "myCluster",
+            "permission_label" : "Cluster Administrator",
+            "permission_name" : "CLUSTER.ADMINISTRATOR",
+            "principal_name" : "user1",
+            "principal_type" : "USER",
+            "privilege_id" : 11,
+            "type" : "CLUSTER",
+            "user_name" : "user1"
+          }
+        };
+        var newPrivilege = {
+          "PrivilegeInfo" : {
+            "permission_name" : user.permission_name,
+            "privilege_id" : user.privilege_id+1,
+            "principal_type" : "USER",
+            "principal_name" : "user1",
+            "encoded_name" : "user1",
+            "original_perm" : user.permission_name,
+            "url" : "users/user1",
+            "editable" : true
+          }
+        };
+
+        //test if the individual user privilege CLUSTER.ADMINISTRATOR is removed from 'items1' by deletePrivilege()
+        expect(items1.privileges).toNotContain(oldPrivilege);
+
+        //test if the new privilege got added to 'items1' by addPrivilege()
+        expect(items1.privileges).toContain(newPrivilege);
+      });
+
+      it('Should save the Privilege greater than the user\'s group privileges. Should also remove any individual user privileges', function() {
+        //using 'user1' for updating the new privilege
+        user.principal_name = scope.users[0].principal_name;
+        user.principal_type = scope.users[0].principal_type;
+        user.privilege_id = scope.users[0].privilege_id;
+        user.permission_name = "CLUSTER.OPERATOR";
+
+        deferred.getPrivilegesForResource.resolve({ "items" : [items1] });
+        scope.$apply();
+
+        expect(scope.users[0].permission_name).toEqual(user.permission_name);
+        expect(scope.users[0].privilege_id).toEqual(user.privilege_id + 1);
+
+        var oldPrivilege = {
+          "href" : "http://abc.com:8080/api/v1/users/user1/privileges/11",
+          "PrivilegeInfo" : {
+            "cluster_name" : "myCluster",
+            "permission_label" : "Cluster Administrator",
+            "permission_name" : "CLUSTER.ADMINISTRATOR",
+            "principal_name" : "user1",
+            "principal_type" : "USER",
+            "privilege_id" : 11,
+            "type" : "CLUSTER",
+            "user_name" : "user1"
+          }
+        };
+        var newPrivilege = {
+          "PrivilegeInfo" : {
+            "permission_name" : user.permission_name,
+            "principal_name" : "user1",
+            "principal_type" : "USER",
+            "privilege_id" : user.privilege_id + 1,
+            "encoded_name" : "user1",
+            "original_perm" : user.permission_name,
+            "url" : "users/user1",
+            "editable" : true
+          }
+        };
+
+        //test if the individual user privilege CLUSTER.ADMINISTRATOR is removed from 'items1' by deletePrivilege()
+        expect(items1.privileges).toNotContain(oldPrivilege);
+
+        //test if the new privilege got added to 'items1' by addPrivilege()
+        expect(items1.privileges).toContain(newPrivilege);
+      });
+
+      it('Should NOT save the Privilege smaller than the user\'s group privileges. Should keep the user\'s original privileges intact', function() {
+        //using 'user1' for updating the new privilege
+        user.principal_name = scope.users[0].principal_name;
+        user.principal_type = scope.users[0].principal_type;
+        user.privilege_id = scope.users[0].privilege_id;
+        user.permission_name = "CLUSTER.USER";
+
+        deferred.getPrivilegesForResource.resolve({ "items" : [items1] });
+        scope.$apply();
+
+        expect(scope.users[0].permission_name).toEqual(user.original_perm);
+        expect(scope.users[0].privilege_id).toEqual(user.privilege_id);
+
+        var oldPrivilege = {
+          "href" : "http://abc.com:8080/api/v1/users/user1/privileges/11",
+          "PrivilegeInfo" : {
+            "cluster_name" : "myCluster",
+            "permission_label" : "Cluster Administrator",
+            "permission_name" : "CLUSTER.ADMINISTRATOR",
+            "principal_name" : "user1",
+            "principal_type" : "USER",
+            "privilege_id" : 11,
+            "type" : "CLUSTER",
+            "user_name" : "user1",
+            "encoded_name" : "user1",
+            "original_perm" : "CLUSTER.ADMINISTRATOR",
+            "url" : "users/user1",
+            "editable" : true
+          }
+        };
+        var newPrivilege = {
+          "PrivilegeInfo" : {
+            "permission_name" : user.permission_name,
+            "principal_name" : "user1",
+            "principal_type" : "USER",
+            "privilege_id" : user.privilege_id+1,
+            "encoded_name" : "user1",
+            "original_perm" : user.permission_name,
+            "url" : "users/user1",
+            "editable" : true
+          }
+        };
+
+        //test if the individual user privilege CLUSTER.ADMINISTRATOR is NOT removed from 'items1'
+        expect(items1.privileges).toContain(oldPrivilege);
+
+        //test if the new privilege is NOT added to 'items1'
+        expect(items1.privileges).toNotContain(newPrivilege);
+      });
+
+    });
+
+    describe('#save() for Groups', function() {
+      var user = {};
+
+      beforeEach(function() {
+        items1 = {
+          "href" : "http://abc.com:8080/api/v1/groups/mygroup",
+          "Groups" : { "group_name" : "mygroup" },
+          "privileges" : [
+            {
+              "href" : "http://abc.com:8080/api/v1/groups/mygroup/privileges/3359",
+              "PrivilegeInfo" : {
+                "cluster_name" : "myCluster",
+                "group_name" : "mygroup",
+                "permission_label" : "Service Administrator",
+                "permission_name" : "SERVICE.ADMINISTRATOR",
+                "principal_name" : "mygroup",
+                "principal_type" : "GROUP",
+                "privilege_id" : 3359,
+                "type" : "CLUSTER"
+              }
+            }
+          ]
+        };
+
+        items2 = {
+          "href" : "http://abc.com:8080/api/v1/groups/mygroup2",
+          "Groups" : { "group_name" : "mygroup2" },
+          "privileges" : [
+            {
+              "href" : "http://abc.com:8080/api/v1/groups/mygroup2/privileges/3356",
+              "PrivilegeInfo" : {
+                "cluster_name" : "myCluster",
+                "group_name" : "mygroup2",
+                "permission_label" : "Service Administrator",
+                "permission_name" : "SERVICE.ADMINISTRATOR",
+                "principal_name" : "mygroup2",
+                "principal_type" : "GROUP",
+                "privilege_id" : 3356,
+                "type" : "CLUSTER"
+              }
+            }
+          ]
+        };
+
+        all_items = { "items": [items1, items2]};
+
+        scope.loadUsers();
+
+        spyOn(Cluster, 'deleteMultiplePrivileges').andCallFake(function(clusterId, privilege_ids) {
+          privilege_ids.forEach(function(privilege_id) {
+            items1.privileges.forEach(function(p, index) {
+              if (p.PrivilegeInfo.privilege_id === privilege_id) {
+                //Remove from array
+                items1.privileges.splice(index, 1);
+              }
+            });
+          });
+        });
+        spyOn(scope, 'addPrivilege').andCallFake(function(user) {
+          var p = {};
+          p.PrivilegeInfo = {};
+          p.PrivilegeInfo.privilege_id = user.privilege_id + 1;
+          p.PrivilegeInfo.permission_name = user.permission_name;
+          p.PrivilegeInfo.principal_type = 'GROUP';
+
+          items1.privileges.push(p);
+          scope.loadUsers();
+        });
+
+        deferred.getPrivilegesWithFilters.resolve(all_items);
+        deferred.getPrivilegesForResource.promise.then(function(data) {
+          var arrayOfPrivileges = data.items[0].privileges;
+          var privilegesOfTypeGroup = [];
+          var privilege = scope.pickEffectivePrivilege(arrayOfPrivileges);
+          user.principal_type = 'GROUP';
+          user.original_perm = privilege.permission_name;
+          user.editable = (Cluster.ineditableRoles.indexOf(privilege.permission_name) === -1);
+
+          arrayOfPrivileges.forEach(function(privilegeOfTypeGroup){
+            if (privilegeOfTypeGroup.PrivilegeInfo.principal_type === "GROUP") {
+              privilegesOfTypeGroup.push(privilegeOfTypeGroup.PrivilegeInfo);
+            }
+          });
+
+          var privilege_ids = [];
+          privilegesOfTypeGroup.forEach(function(privilegeOfTypeGroup) {
+            privilege_ids.push(privilegeOfTypeGroup.privilege_id);
+          });
+
+          //delete all privileges of type GROUP, if they exist
+          //then add the privilege for the group, after which the group displays the effective privilege
+          if(privilege_ids.length !== 0) {
+            Cluster.deleteMultiplePrivileges(
+                123,
+                privilege_ids
+            );
+          }
+          scope.addPrivilege(user);
+        });
+
+        scope.$apply();
+      });
+
+      it('Should save the Privilege equal to the group\'s effective privilege. Should remove any other privileges of the group',function(){
+        //using 'mygroup' for updating the new privilege
+        user.principal_name = scope.users[0].principal_name;
+        user.principal_type = scope.users[0].principal_type;
+        user.privilege_id = scope.users[0].privilege_id;
+        user.permission_name = "SERVICE.ADMINISTRATOR";
+
+        deferred.getPrivilegesForResource.resolve({ "items" : [items1] });
+
+        scope.$apply();
+
+        expect(scope.users[0].permission_name).toEqual(user.permission_name);
+        expect(scope.users[0].privilege_id).toEqual(user.privilege_id + 1);
+
+        var oldPrivilege = {
+          "PrivilegeInfo" : {
+            "cluster_name" : "myCluster",
+            "group_name" : "mygroup",
+            "permission_label" : "Service Administrator",
+            "permission_name" : "SERVICE.ADMINISTRATOR",
+            "principal_name" : "mygroup",
+            "principal_type" : "GROUP",
+            "privilege_id" : 3359,
+            "type" : "CLUSTER"
+          }
+        };
+        var newPrivilege = {
+          "PrivilegeInfo" : {
+            "privilege_id" : user.privilege_id + 1,
+            "permission_name" : user.permission_name,
+            "principal_type" : "GROUP",
+            "principal_name" : "mygroup",
+            "encoded_name" : "mygroup",
+            "original_perm" : user.permission_name,
+            "url" : "groups/mygroup/edit",
+            "editable" : true
+          }
+        };
+
+        //test if the older privilege is no longer present in 'items1'
+        expect(items1.privileges).toNotContain(oldPrivilege);
+
+        //test if the new privilege is added to 'items1'
+        expect(items1.privileges).toContain(newPrivilege);
+      });
+
+      it('Should save the Privilege greater than the group\'s effective privilege. Should remove any other privileges of the group',function(){
+        //using 'mygroup' for updating the new privilege
+        user.principal_name = scope.users[0].principal_name;
+        user.principal_type = scope.users[0].principal_type;
+        user.privilege_id = scope.users[0].privilege_id;
+        user.permission_name = "CLUSTER.ADMINISTRATOR";
+
+        deferred.getPrivilegesForResource.resolve({ "items" : [items1] });
+
+        scope.$apply();
+
+        expect(scope.users[0].permission_name).toEqual(user.permission_name);
+        expect(scope.users[0].privilege_id).toEqual(user.privilege_id + 1);
+
+        var oldPrivilege = {
+          "PrivilegeInfo" : {
+            "cluster_name" : "myCluster",
+            "group_name" : "mygroup",
+            "permission_label" : "Service Administrator",
+            "permission_name" : "SERVICE.ADMINISTRATOR",
+            "principal_name" : "mygroup",
+            "principal_type" : "GROUP",
+            "privilege_id" : 3359,
+            "type" : "CLUSTER"
+          }
+        };
+        var newPrivilege = {
+          "PrivilegeInfo" : {
+            "privilege_id" : user.privilege_id + 1,
+            "permission_name" : user.permission_name,
+            "principal_type" : "GROUP",
+            "principal_name" : "mygroup",
+            "encoded_name" : "mygroup",
+            "original_perm" : user.permission_name,
+            "url" : "groups/mygroup/edit",
+            "editable" : true
+          }
+        };
+
+        //test if the older privilege is no longer present in 'items1'
+        expect(items1.privileges).toNotContain(oldPrivilege);
+
+        //test if the new privilege is added to 'items1'
+        expect(items1.privileges).toContain(newPrivilege);
+      });
+
+      it('Should save the Privilege lesser than the group\'s effective privilege. Should remove any other privileges of the group', function() {
+        //using 'mygroup' for updating the new privilege
+        user.principal_name = scope.users[0].principal_name;
+        user.principal_type = scope.users[0].principal_type;
+        user.privilege_id = scope.users[0].privilege_id;
+        user.permission_name = "CLUSTER.USER";
+
+        deferred.getPrivilegesForResource.resolve({ "items" : [items1] });
+
+        scope.$apply();
+
+        expect(scope.users[0].permission_name).toEqual(user.permission_name);
+        expect(scope.users[0].privilege_id).toEqual(user.privilege_id + 1);
+
+        var oldPrivilege = {
+          "PrivilegeInfo" : {
+            "cluster_name" : "myCluster",
+            "group_name" : "mygroup",
+            "permission_label" : "Service Administrator",
+            "permission_name" : "SERVICE.ADMINISTRATOR",
+            "principal_name" : "mygroup",
+            "principal_type" : "GROUP",
+            "privilege_id" : 3359,
+            "type" : "CLUSTER"
+          }
+        };
+        var newPrivilege = {
+          "PrivilegeInfo" : {
+            "privilege_id" : user.privilege_id + 1,
+            "permission_name" : user.permission_name,
+            "principal_type" : "GROUP",
+            "principal_name" : "mygroup",
+            "encoded_name" : "mygroup",
+            "original_perm" : user.permission_name,
+            "url" : "groups/mygroup/edit",
+            "editable" : true
+          }
+        };
+
+        //test if the older privilege is no longer present in 'items1'
+        expect(items1.privileges).toNotContain(oldPrivilege);
+
+        //test if the new privilege is added to 'items1'
+        expect(items1.privileges).toContain(newPrivilege);
+      });
+
+    });
+  });
 });