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

team-add-users: add app-authorized guards #2758

Merged
merged 1 commit into from
Jan 30, 2020
Merged

Conversation

bcmdarroch
Copy link
Contributor

🔩 Description: What code changed, and why?

Fixes a tiny issue I noticed while acceptance testing. As a project owner, I'm allowed to add users to teams in my project. However, I can't create users. On the team-add-users modal, once all users are added to a team, we show a button to create new users. But as a project owner, I get a 403 error when I try to create a user because I don't have permission to.

This PR adds an app-authorized guard to the Create User buttons on the team-add-users modal so a project owner (or any user without create user permissions) can't see them.

It also slightly changes the message the non-permissioned user will see.

👍 Definition of Done

  • any logged-in user who lacks permission to create users will not see a Create User button of the team-add-users modal

👟 How to Build and Test the Change

  • start Automate + UI
  • create a project
  • add some user to the auto-generated <project name> Project Owners policy
  • create a team inside that project
  • log in as the project-owner user
  • navigate to https://a2-dev.test/settings/teams/editors/add-users
  • you should see the screen below:

Screen Shot 2020-01-29 at 5 26 54 PM

✅ Checklist

@bcmdarroch bcmdarroch requested a review from a team January 30, 2020 01:29
@bcmdarroch bcmdarroch added auth-team anything that needs to be on the auth team board automate-auth automate-ui ui labels Jan 30, 2020
@bcmdarroch bcmdarroch self-assigned this Jan 30, 2020
if (this.usersNotFiltered().length === 0) {
return '';
} else if (this.addingUsers) {
if (this.addingUsers) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was a bonus fix. I noticed in the case of if (this.usersNotFiltered().length === 0), the Add Users button just became a shortened blank button which looked off to me. So now when there are no users left to add you'll see a disabled Add Users button

Copy link
Contributor

Choose a reason for hiding this comment

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

oh was ur chef-ui-library up to date? those buttons should be totally hidden if there are no more users left to add.

Copy link
Contributor

Choose a reason for hiding this comment

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

@susanev is correct; this change should not be needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh good catch thanks!

</div>
<div id="table-container" *ngIf="usersNotFiltered()?.length > 0">
<div id="table-controls">
<app-authorized [anyOf]="['/iam/v2/teams/{id}/users:add', 'post', team?.id]">
<app-authorized [allOf]="['/iam/v2/users', 'post']">
Copy link
Contributor

Choose a reason for hiding this comment

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

While I agree that having the users permission there is good, I would suggest also include the original teams users:add as well, because that is not being checked prior to getting here. (It should be, but that looks like a bit larger can of worms to fix that.)

Copy link
Contributor Author

@bcmdarroch bcmdarroch Jan 30, 2020

Choose a reason for hiding this comment

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

I switched it because the app-authorized is only hiding the Create User button.
Started looking into how we could start guarding the Add Users buttons on the team-details page and realized it would take some refactoring of the app-user-table. I'll make an issue for that fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

issue: #2762

if (this.usersNotFiltered().length === 0) {
return '';
} else if (this.addingUsers) {
if (this.addingUsers) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@susanev is correct; this change should not be needed.

to create user buttons

Signed-off-by: Brenna Hewer-Darroch <brenna@chef.io>
@bcmdarroch bcmdarroch merged commit 5de9cd7 into master Jan 30, 2020
@chef-expeditor chef-expeditor bot deleted the bhd/quick-add-users-fix branch January 30, 2020 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auth-team anything that needs to be on the auth team board automate-auth automate-ui ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants