[ http://issues.apache.org/jira/browse/JS2-475?page=comments#action_12363624 ]
David Jencks commented on JS2-475: ---------------------------------- >3. We have proposed removing the Subject from the system and using it only in >the PermissionManager if needed. This implies removing all of the >"AccessController" code from the PageManager and accessing the >PermissionManager instead. +1. I'm a bit confused. IIUC the PermissionManager is mostly"hidden" behind the jetspeed Policy implementation. I would think that it would be a good idea to make J2 run against any Policy, in particular a JACC implementation that has the Jetspeed permission installed in it. I think this involves replacing direct reference to the PermissionManager with use of AccessController, which will use the PermissionManager if the Jetspeed policy is installed. This is certainly the direction I have been heading in in the Geronimo integration, which now runs without use of the PermissionManager but with the Jetspeed permissions installed. ( I don't consider this a final solution by any means, but a demonstration that J2 can run with JACC. I think the next step is to push the permissions from the PermissionManager into the JACC PolicyConfiguration) >4. I can take responsibility for the "enormous" number of string operations >involved in the constraints implementation. I am not a huge fan of binary bit >tests when it comes to readability, but I have to agree that this code is not >much better from a readability standpoint. Let me know if you'd like me to fix >this or if you are anxious to get to it. I haven't studied the constraints implementation very much yet. I would prefer to see what the response is to this set of changes before looking at it in detail. I wonder if it would be possible to use the jetspeed permissions within the constraints to decide authorization without dealing with AccessController etc. If possible, this might avoid duplicating the essence of the authorization logic. Anyway, thanks for reviewing this patch :-) > 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]
