+1 to this whole list.Inheritance on both sides would be good and I think is the logical assumption most people will make when trying to assign permissions.
Avoiding regex matching in high call-count code like permissions would be good and looking into meta targets would be a good compromise as well.
Feel free to commit the patch to trunk and we can make sure to do some focused testing after the 3.2 RC comes out.
-Eric On 12/10/09 7:09 AM, Jennifer Bourey wrote:
Hi all, In the course of continuing UP-2047 work, I've come across a few opportunities for improvement in uPortal's permissions code. In particular, I've found that while principals inherit permissions from the groups to which they belong, targets do not inherit permissions from their parent groups. As a concrete example, the VIEW permission is set on "All Categories" for the "Everyone" group. I inherit the VIEW permission on "All Categories" because I belong to the "Everyone" group, and the AnyUnblockedPermission policy walks up the group tree to find that permission. However, the permissions service would report that neither I nor the Everyone group have VIEW permissions on the "Demonstration" category. "Demonstration" is a first-level child of the "Everyone" group, but since the permissions code doesn't walk the group tree for permission targets, this relationship is never taken into consideration. As of 3.1, JHU contributed changes to allow the inheritance of permissions specifically for channel targets. This allowed the new MANAGE permission to be targeted to channel categories and then inherited by member channels, which made the new delegated channel management feature interesting and useful. I'd like to propose extending this behavior to all groupable entities. Instead of having special code to find hierarchical relationships for channels, it seems to me like we should be attempting to walk group trees for people, person groups, channel categories, etc. 3.1 also introduces the ability to match channel targets via a regex. This allows permissions to be targeted to all channels, even if the channel does not belong to any categories, by setting a regex of<literal>.*</literal>. This behavior is certainly useful, since it helps target permissions to channels that may be kept out of categories to prevent users from being able to subscribe to them. It does seem a little awkward though, since I can't think of any other useful regex values besides .* (the channel targets look like CHAN_ID.43). I think both clarity and performance would be improved by removing the regex behavior in favor of some new string literal target. If we want to create a target that represents all channels, perhaps we could use something like "ALL_CHANNELS". I've made changes locally to provide the functionality described above, and it all appears to work reasonably well. Before I check anything in, I'd like some feedback on my current approach and some of its implications. For the inherited target permission functionality I've currently taken the approach of simply gathering up the list of parent groups for an entity and adding their permissions. While this is the simplest, fastest approach, it doesn't allow the use of inherited blocks for targets. This of course doesn't affect the ability to block inheritance of permissions via the principal, so I don't think it will be the end of the world. I also don't know if we have a concrete need for providing an analogue for the ALL_CHANNELS feature for other entity types. - Jen
smime.p7s
Description: S/MIME Cryptographic Signature
