revolution icon indicating copy to clipboard operation
revolution copied to clipboard

Fix Role Authority Editability

Open smg6511 opened this issue 1 year ago • 3 comments

What does it do?

Add new listener routine to restrict editing based on whether a Role is assigned to an ACL entry. Also:

  1. Adds new UI feedback, both a lock icon (instead of pencil) on hover of authority as well as an alert dialog when double-clicking on locked authority
  2. Ensures authority cannot be deleted in grid editor
  3. Remove pencil icon for cells of non-editable Roles

Why is it needed?

The ability to edit an assigned Role's authority leads to orphaned ACL rules that not longer show up in the manager, yet remain in the database. See the referenced issue below.

How to test

  1. Rebuild template (grunt build) and clear manager and browser caches
  2. Create a few Roles under the Access Control Lists section
  3. Assign at least one Role to any ACL entry of your choice
  4. Verify that the assigned Roles' authority is locked in the Roles grid
  5. Verify that unassigned Roles remain fully editable in the Roles grid (authority is unlocked)

Note

The initial commit contains all substantive changes, while the follow up is just code-style/optimization oriented.

Related issue(s)/PR(s)

Resolves #16565

smg6511 avatar May 08 '24 16:05 smg6511

As mentioned out of context in #16469, I think we need to consider just letting authority be presented/editable directly as an integer authority value when editing ACLs. Or don't allow ACLs to be edited at all, only allowing them to be created (setting the authority from a role) and deleted.

opengeek avatar Jun 13 '24 17:06 opengeek

Actually, if we make modUserGroupRole.authority a unique index, we can rely on the link between a role and an authority. If no one has objections, I'll make this change, add the appropriate upgrade scripts, and add an upgrade test to ensure there are not multiple modUserGroupRole objects with the same authority before the upgrade to 3.1 can be executed. Then this PR can stand as is.

opengeek avatar Jun 13 '24 19:06 opengeek

make modUserGroupRole.authority a unique index

Makes sense to me ;-)

smg6511 avatar Jun 13 '24 20:06 smg6511