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

Split out permissions for assigning roles/groups and editing them #6614

Merged

Conversation

ryanmitchell
Copy link
Contributor

This PR adds new permissions for assigning roles and groups, seperate from editing them. This allows you to have users that can modify other users groups and roles, but not be able to change the groups or roles themselves.

As editing groups/roles is not dependent on viewing users, I moved it out of the view users permissions block.

I also added an update script so any roles that could previously edit roles/groups can now also assign them.

Fixes: statamic/ideas#474

@ryanmitchell ryanmitchell changed the title Split out assigning roles/groups from editing them Split out permissions for assigning roles/groups and editing them Sep 9, 2022
Copy link
Member

@jasonvarga jasonvarga left a comment

Choose a reason for hiding this comment

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

When editing a group, we should give the "roles" field the same treatment. At the moment it doesn't. You should only be able to assign the roles to the group if you have the assign_roles permission.

@ryanmitchell
Copy link
Contributor Author

ryanmitchell commented Oct 14, 2022

Yeah that makes sense, however the approach we'd take now would need to change if/once #6506 merges (you could use ensureFields on the blueprint at that point).

So happy to work on it now, but just highlighting it would need be changed later... what do you think?

@jasonvarga
Copy link
Member

It would need to be updated in both places either way. Let's make the change here, and we can add a note in the other PR to update it.

@ryanmitchell
Copy link
Contributor Author

@jasonvarga What about this ?

@ryanmitchell
Copy link
Contributor Author

Just checking in to see if this change is sufficient, or if you need more?

@robdekort
Copy link
Contributor

Just expressing my gratitude here for this PR! No more creating users for clients in the near future!

@jasonbradberry
Copy link

Thumbs up for this.

@what-the-diff
Copy link

what-the-diff bot commented Nov 15, 2022

  • Added canAssignSuper prop to the role publish form.
  • Hid roles and groups fields in user group publish forms if permissions are not present (assign roles/groups).
  • Added canAssignRoles and canAssignGroups props to the users wizard component, which is used for creating new users as well as editing existing ones from within a modal window on other pages like entries or taxonomies where you can assign authorship of an item to another user by clicking their avatar image next to it's name in the sidebar list view, etc.. This will hide those fields when appropriate based on whether or not they have permission(s) that allow them access there (edit roles / edit user groups).
  • Added tests for the store and update methods of roles
  • Added tests for the store and update methods of user groups
  • Added a new test case to the AddPerEntryPermissionsTest.php file
  • The new test case checks that all roles are deleted after running tests

@jasonvarga
Copy link
Member

Alrighty, I made some additions here:

  • Added some server side checks instead of just relying on js
  • Added tests around creating/editing groups/roles.
  • Only super users can assign super permissions.
  • Adjusted visibility of role/group fields in the user creation wizard
  • A couple other little bits

Thanks for this PR, sorry about the delay.

@jasonvarga jasonvarga merged commit 374f0c5 into statamic:3.3 Nov 15, 2022
@ryanmitchell
Copy link
Contributor Author

Thanks for the changes and cleanup!

@ryanmitchell ryanmitchell deleted the feature/add-assign-roles-groups-permission branch November 15, 2022 22:15
@robdekort
Copy link
Contributor

Thanks you both!

@jasonvarga jasonvarga mentioned this pull request Nov 15, 2022
@jonatanalvarsson
Copy link

This is great! Thanks! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Split ability to assign roles vs. edit the permissions of roles
5 participants