Edgewall Software
Modify

Opened 6 years ago

Last modified 3 years ago

#13014 new enhancement

Deprecate and remove IPermissionStore.get_users_with_permission

Reported by: Ryan J Ollos Owned by:
Priority: normal Milestone: next-dev-1.7.x
Component: general Version:
Severity: normal Keywords: permissions
Cc: Branch:
Release Notes:
API Changes:
Internal Changes:

Description

This is a follow-on to comment:15:ticket:12915.

The proposed changes are:

  • Change DefaultPermissionStore.get_user_permissions so that IPermissionGroupProviders are not searched. PermissionSystem.get_user_permissions will search IPermissionGroupProviders. DefaultPermissionStore will no longer have any knowledge of IPermissionGroupProviders that it does not implement.
  • Deprecate IPermissionStore.get_users_with_permissions and remove it in 1.5.1.
  • Change PermissionSystem.get_users_with_permission so that it does not call IPermissionStore.get_users_with_permissions.

Attachments (0)

Change History (10)

comment:1 by Ryan J Ollos, 6 years ago

Changes in log:rjollos.git:t13014_remove_get_users_with_permission. Need to add more test coverage and check performance implications.

comment:2 by Ryan J Ollos, 6 years ago

[52b42e0d/rjollos.git] proposes to make DefaultPermissionStore.get_user_permissions unaware of other IPermissionGroupProviders.

  • DefaultPermissionStore.get_user_permissions still returns actions for a user that are inherited from DefaultPermissionStore groups that the user is a member of.
  • PermissionSystem.get_user_permissions is responsible for determining group membership for groups defined by other IPermissionGroupProviders.
  • The IPermissionStore.get_user_permissions interface is changed to require accepting a tuple of subjects (users and groups).

I've been debating a few points:

  • I favor changing DefaultPermissionStore to be unaware of all IPermissionGroupProviders because that work is better done by PermissionSystem and doesn't have to be repeated by every IPermissionStore implementation. The IPermissionStores have a more narrow and well-defined role. However, if this change leads to too many complications, the proposed change should be dropped.
  • With that in mind, maybe DefaultPermissionStore.get_user_permissions should only return actions for the specified subjects and not include DefaultPermissionStore groups that the user is a member of. The DefaultPermissionStore groups would be discovered by the query in PermissionSystem.get_user_permissions. The downside is that it results in two database queries, one to find the groups and one to find the actions for subjects.
  • Otherwise, we could explicitly document the rule. If the component implementing IPermissionStore defines groups through an IPermissionGroupProvider implementation, get_user_permissions should return actions inherited from those groups for all subjects.
Version 1, edited 6 years ago by Ryan J Ollos (previous) (next) (diff)

comment:3 by Ryan J Ollos, 6 years ago

Owner: set to Ryan J Ollos
Status: newassigned

comment:4 by Ryan J Ollos, 6 years ago

Milestone: 1.3.41.5.1

comment:5 by Ryan J Ollos, 4 years ago

Milestone: 1.5.11.5.3

comment:6 by Ryan J Ollos, 3 years ago

Milestone: 1.5.31.7

comment:7 by Ryan J Ollos, 3 years ago

Milestone: 1.71.7.1

Milestone renamed

comment:8 by Ryan J Ollos, 3 years ago

Milestone: 1.7.1
Owner: Ryan J Ollos removed
Status: assignednew

comment:9 by Ryan J Ollos, 3 years ago

Milestone: 1.7.1

comment:10 by Ryan J Ollos, 3 years ago

Milestone: 1.7.1next-dev-1.7.x

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The ticket will remain with no owner.
The ticket will be disowned.
as The resolution will be set. Next status will be 'closed'.
The owner will be changed from (none) to anonymous. Next status will be 'assigned'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.