Edgewall Software
Modify

Opened 17 months ago

Last modified 14 months ago

#13014 assigned enhancement

Deprecate and remove IPermissionStore.get_users_with_permission

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Milestone: 1.5.1
Component: general Version:
Severity: normal Keywords: permissions
Cc: Branch:
Release Notes:
API 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 (4)

comment:1 by Ryan J Ollos, 17 months 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, 17 months ago

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

  • However, 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.
Last edited 17 months ago by Ryan J Ollos (previous) (diff)

comment:3 by Ryan J Ollos, 14 months ago

Owner: set to Ryan J Ollos
Status: newassigned

comment:4 by Ryan J Ollos, 14 months ago

Milestone: 1.3.41.5.1

Modify Ticket

Change Properties
Set your email in Preferences
Action
as assigned The owner will remain Ryan J Ollos.
The ticket will be disowned. Next status will be 'new'.
as The resolution will be set. Next status will be 'closed'.
to as assigned The owner will be changed from Ryan J Ollos to the specified user.

Add Comment


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