Deprecate and remove IPermissionStore.get_users_with_permission

[Ryan J Ollos]

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.
  • 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_permission can do the equivalent work for all IPermissionStores.
• Change PermissionSystem.get_users_with_permission so that it does not call IPermissionStore.get_users_with_permissions.

[Ryan J Ollos]

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

[Ryan J Ollos]

[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.

[Ryan J Ollos]

Milestone renamed