[ http://issues.apache.org/jira/browse/JS2-475?page=comments#action_12363747 ]
David Jencks commented on JS2-475: ---------------------------------- Latest version of these changes in JS2-444 geronimo-jetspeed10.zip > Proposed changes in portal permissions > -------------------------------------- > > Key: JS2-475 > URL: http://issues.apache.org/jira/browse/JS2-475 > Project: Jetspeed 2 > Type: Bug > Versions: 2.1-dev > Reporter: David Jencks > Attachments: permissions-fragment1.diff, permissions-fragment3.jar, > permissions1.diff, permissions1.jar > > (from email, with added comments) > I've been looking at the portal permissions and how they are used and think a > few things can be simplified and speeded up. If there are no objections to > this general direction I will prepare an initial patch. > 1. FolderPermission duplicates the parseActions method from > PortalResourcePermission, and in fact calls it's copy again. I think this > can be eliminated. > 2. PortalResourcePermission.parseActions seems to have some rather odd code: > if (token.equals(JetspeedActions.VIEW)) > mask |= JetspeedActions.MASK_VIEW; > else if (token.equals(JetspeedActions.VIEW) || > token.equals(JetspeedActions.RESTORE)) > mask |= JetspeedActions.MASK_VIEW; > I think this can be simplified. > 3. I may not have found all the constructor uses, but I think that subject > should be removed from all the portal permissions. I haven't found any uses > of the constructor including a non-null subject (although I might have missed > some). In addition to the resulting simplification, I believe the subject > has no place in the permissions. The JACC defined permissions for web and > ejb do not include a subject. JACC does allow for unchecked permissions, > which are difficult to imagine if the permissions involved may include a > subject. I think a generally more satisfactory approach is to rely on the > policy implementation to determine the subject itself. > --This requires converting several direct uses of the PermissionManager to > AccessController.checkPermission. This is more generatlly consistent with > use of Policy to check permissions. > 4. Currently each construction of a portal permission involves string > parsing to decipher an actions string. It looks to me as if this can occur > hundreds of times for a medium sized portal page. Futhermore, this action > string appears to be constructed using ad-hoc string manipulations in > AbstractBaseElement.checkPermissions(String actions). Similarly, the > constraints implementation seems to do an enormous amount of string > comparison to match actions. I think that this can be entirely converted to > integer masks with bitwise operations. I'd propose to do this in steps, > starting with the permissions and working backwards until I hit the > contraints implementation, then converting it. > -- initial patch completely converts permissions to use mask for runtime > evaluations. Constraints remain as before. > 5. Some of the constants are duplicated between SecuredResource and > JetspeedActions: moved to JetspeedActions only. > 6. There are lots of little bugs like wrongly implemented equals methods in > portal permissions > 7. I've fixed the javadoc in the classes in the patch. > 8. This includes the fixes for JS2-472 > I don't really know how to test my changes thoroughly. AFAICT they appear to > work with my geronimo/jacc integration (latest version will be posted soon). > I apologize for the number of white space changes in the diff. I pressed the > "reformat" button in idea. The patch is generated with svk and I have > sometimes had troubles applying these with patch so I'm also attaching copies > of the source files. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
