diff --git a/app/scripts/controllers/membership.js b/app/scripts/controllers/membership.js index d21e9fed35..6ea6114b2c 100644 --- a/app/scripts/controllers/membership.js +++ b/app/scripts/controllers/membership.js @@ -274,7 +274,8 @@ angular project: project, subjectKinds: subjectKinds, canUpdateRolebindings: canI('rolebindings', 'update', projectName), - confirmRemove: function(subjectName, kindName, roleName) { + confirmRemove: function(subjectName, kindName, roleName, namespace) { + var redirectToProjectList = null; var modalScope = createModalScope(subjectName, kindName, roleName, $scope.user.metadata.name); if(_.isEqual(subjectName, $scope.user.metadata.name)) { @@ -294,7 +295,7 @@ angular }) .result.then(function() { RoleBindingsService - .removeSubject(subjectName, roleName, $scope.roleBindings, requestContext) + .removeSubject(subjectName, roleName, namespace, $scope.roleBindings, requestContext) .then(function(updateRolebinding) { if(redirectToProjectList) { $location.url("./"); diff --git a/app/scripts/services/membership/roleBindings.js b/app/scripts/services/membership/roleBindings.js index 28bdd2e417..d34dad3f63 100644 --- a/app/scripts/services/membership/roleBindings.js +++ b/app/scripts/services/membership/roleBindings.js @@ -87,20 +87,27 @@ angular }; // has to handle multiple bindings or multiple reference to a subject within a single binding - var removeSubject = function(subjectName, role, roleBindings, context) { - var matches = _.filter(roleBindings, {roleRef: {name: role}}); + var removeSubject = function(subjectName, role, namespace, roleBindings, context) { + var matchingBindings = _.filter(roleBindings, {roleRef: {name: role}}); + return $q.all( - _.map(matches, function(binding) { + _.map(matchingBindings, function(binding) { var tpl = bindingTPL(); binding = _.extend(tpl, binding); cleanBinding(binding); - binding.subjects = _.reject(binding.subjects, {name: subjectName}); + var toMatch = { name: subjectName }; + if(namespace) { + toMatch.namespace = namespace; + } + + binding.subjects = _.reject(binding.subjects, toMatch); + return binding.subjects.length ? DataService.update('rolebindings', binding.metadata.name, binding, context) : DataService.delete('rolebindings', binding.metadata.name, context) // For a delete, resp is simply a 201 or less useful object. // Instead, this intercepts the response & returns the binding object - // with the empty .subjects[] list. + // with the empty .subjects[] list. .then(function() { return binding; }); diff --git a/app/views/membership.html b/app/views/membership.html index da29e1475c..d47a31fbb2 100644 --- a/app/views/membership.html +++ b/app/views/membership.html @@ -113,7 +113,7 @@

key="role.metadata.name" key-help="roleHelp(role)" show-action="mode.edit" - action="confirmRemove(subject.name, subjectKind.name, role.metadata.name)" + action="confirmRemove(subject.name, subjectKind.name, role.metadata.name, subject.namespace)" action-title="Remove role {{role.metadata.name}} from {{subject.name}}">
\n" + "
\n" + "
\n" + - "\n" + + "\n" + "
\n" + "
\n" + "
\n" + diff --git a/test/spec/services/membership/roleBindingsSpec.js b/test/spec/services/membership/roleBindingsSpec.js index 823bd4b774..bfb316090d 100644 --- a/test/spec/services/membership/roleBindingsSpec.js +++ b/test/spec/services/membership/roleBindingsSpec.js @@ -88,10 +88,13 @@ describe('RoleBindingsService', function() { it('should update the rolebinding by removing the subject from the subject list', function() { var bindings = [{ roleRef: { name: 'admin'}, - subjects: [{name: 'jane', kind: 'user'}, {name: 'jack', kind: 'user'}] + subjects: [ + {name: 'jane', kind: 'user'}, + {name: 'jack', kind: 'user'} + ] }]; RoleBindingsService - .removeSubject('jane', 'admin', bindings) + .removeSubject('jane', 'admin', null, bindings) .then(function(resp) { expect(resp[0].subjects).toEqual([{ name: 'jack', @@ -102,13 +105,62 @@ describe('RoleBindingsService', function() { $rootScope.$digest(); }); + // TODO: cover this case + it('should honor the namespace of a subject if provided', function() { + var bindings = [{ + roleRef: { name: 'admin'}, + subjects: [ + // multiple jim, only one should be removed in test 1 + {name: 'jim', kind: 'user'}, + {name: 'jim', kind: 'user', namespace: 'yourproject'}, + {name: 'jim', kind: 'user', namespace: 'myproject'}, + {name: 'jack', kind: 'user'} + ] + }, { + roleRef: { name: 'view'}, + subjects: [ + // multiple jane, only one should be removed in test 2 + {name: 'jane', kind: 'user', namespace: 'myproject'}, + {name: 'jack', kind: 'user', namespace: 'yourproject'}, + {name: 'jane', kind: 'user', namespace: 'herproject'}, + {name: 'jack', kind: 'user', namespace: 'hisproject'} + ] + }]; + + // test 1 + RoleBindingsService + .removeSubject('jim', 'admin', 'myproject', bindings) + .then(function(resp) { + expect(resp[0].subjects).toEqual([ + {name: 'jim', kind: 'user'}, + {name: 'jim', kind: 'user', namespace: 'yourproject'}, + {name: 'jack', kind: 'user'} + ]); + + }); + + // test 2 + RoleBindingsService + .removeSubject('jane', 'view', 'herproject', bindings) + .then(function(resp) { + expect(resp[0].subjects).toEqual([ + {name: 'jane', kind: 'user', namespace: 'myproject'}, + {name: 'jack', kind: 'user', namespace: 'yourproject'}, + {name: 'jack', kind: 'user', namespace: 'hisproject'} + ]); + }); + // resolve $q.all() via digest loop + $rootScope.$digest(); + + }); + it('should delete the rolebinding if the removed subject was the only subject', function() { var bindings = [{ roleRef: { name: 'admin'}, subjects: [{name: 'jane', kind: 'user'}] }]; RoleBindingsService - .removeSubject('jane', 'admin', bindings) + .removeSubject('jane', 'admin', null, bindings) .then(function(resp) { expect(resp[0]).toBe(undefined); }); @@ -130,7 +182,7 @@ describe('RoleBindingsService', function() { subjects: [{name: 'jane', kind: 'user'}, {name: 'jack', kind: 'user'}] }]; RoleBindingsService - .removeSubject('jane', 'admin', bindings) + .removeSubject('jane', 'admin', null, bindings) .then(function(resp) { _.each(resp, function(roleBinding) { expect(roleBinding.subjects).toEqual([ {name: 'jack', kind: 'user'}]); @@ -154,7 +206,7 @@ describe('RoleBindingsService', function() { subjects: [{name: 'jane', kind: 'user'}] }]; RoleBindingsService - .removeSubject('jane', 'admin', bindings) + .removeSubject('jane', 'admin', null, bindings) .then(function(resp) { expect(resp[0]).toBe(undefined); expect(resp[1].subjects).toEqual([ {name: 'jack', kind: 'user'}]);