Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable removal of org user role if user has others #2427

Merged
merged 7 commits into from
Jun 20, 2018

Conversation

irfanhabib
Copy link
Contributor

No description provided.

@cfdreddbot
Copy link

Hey irfanhabib!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

@codecov
Copy link

codecov bot commented Jun 19, 2018

Codecov Report

Merging #2427 into v2-master will decrease coverage by 0.06%.
The diff coverage is 63.88%.

@@              Coverage Diff              @@
##           v2-master    #2427      +/-   ##
=============================================
- Coverage      70.81%   70.74%   -0.07%     
=============================================
  Files            589      589              
  Lines          24760    24790      +30     
  Branches        5578     5593      +15     
=============================================
+ Hits           17534    17538       +4     
- Misses          7226     7252      +26

map( (entity: CfUser) => this.cfUserService.hasRoles(entity))
);
} else {
chipConfig.hideClearButton$ = this.canRemovePermission(perm.cfGuid, perm.orgGuid, perm.spaceGuid).pipe(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The permissions check still need to be applied. ATM a non-manager can remove an org user role if it's the only role that user has

// If there are other roles than Org User, disable clear button
chipConfig.hideClearButton$ = this.userEntity.pipe(
filter(p => !!p),
map( (entity: CfUser) => this.cfUserService.hasRoles(entity))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check needs to be specific per org. ATM a user cannot remove an org with a user role if there are other orgs with roles.

const orgRoles = this.getOrgRolesFromUser(user);
const spaceRoles = this.getSpaceRolesFromUser(user);

let hasRoles = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to return early if the hasRoles flag becomes true

@richard-cox richard-cox added needs attention This PR needs attention and removed in review labels Jun 20, 2018
@irfanhabib irfanhabib added comments-addressed and removed needs attention This PR needs attention labels Jun 20, 2018
@richard-cox richard-cox merged commit 46085ed into v2-master Jun 20, 2018
@richard-cox richard-cox deleted the fix-org-user-permissions branch June 20, 2018 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants