Opened 5 years ago
Last modified 2 years ago
#13014 new enhancement
Deprecate and remove IPermissionStore.get_users_with_permission
|Reported by:||Ryan J Ollos||Owned by:|
This is a follow-on to comment:15:ticket:12915.
The proposed changes are:
IPermissionGroupProvidersare not searched.
DefaultPermissionStorewill no longer have any knowledge of
IPermissionGroupProviders that it does not implement.
IPermissionStore.get_users_with_permissionsand remove it in 1.5.1.
- The method was added in #4245.
- LdapPlugin is the only plugin on trac-hacks.org that implements
IPermissionStore, and it has copied the implementation of DefaultPermissionStore.get_users_with_permissions.
PermissionSystem.get_users_with_permissioncan do the equivalent work for all
PermissionSystem.get_users_with_permissionso that it does not call
Change History (10)
comment:1 by , 5 years ago
comment:2 by , 5 years ago
[52b42e0d/rjollos.git] proposes to make
DefaultPermissionStore.get_user_permissions unaware of other
DefaultPermissionStore.get_user_permissionsstill returns actions for a user that are inherited from
DefaultPermissionStoregroups that the user is a member of.
PermissionSystem.get_user_permissionsis responsible for determining group membership for groups defined by other
IPermissionStore.get_user_permissionsinterface is changed to require accepting a tuple of subjects (users and groups).
I've been debating a few points:
- I favor changing
DefaultPermissionStoreto be unaware of all
IPermissionGroupProviders because that work is better done by
PermissionSystemand doesn't have to be repeated by every
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_permissionsshould only return actions for the specified subjects and not include
DefaultPermissionStoregroups that the user is a member of. The
DefaultPermissionStoregroups 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
IPermissionStoredefines groups through an
get_user_permissionsshould return actions inherited from those groups for all subjects.
Last edited 5 years ago by (previous) (diff)
comment:3 by , 5 years ago
|Status:||new → assigned|
comment:4 by , 5 years ago
|Milestone:||1.3.4 → 1.5.1|
comment:5 by , 3 years ago
|Milestone:||1.5.1 → 1.5.3|
comment:6 by , 2 years ago
|Milestone:||1.5.3 → 1.7|
comment:7 by , 2 years ago
|Milestone:||1.7 → 1.7.1|
comment:8 by , 2 years ago
|Status:||assigned → new|
comment:9 by , 2 years ago
comment:10 by , 2 years ago
|Milestone:||1.7.1 → next-dev-1.7.x|
Note: See TracTickets for help on using tickets.
Changes in log:rjollos.git:t13014_remove_get_users_with_permission. Need to add more test coverage and check performance implications.