[ http://issues.apache.org/jira/browse/JS2-475?page=all ]

David Jencks updated JS2-475:
-----------------------------

    Attachment: permissions-fragment1.diff
                permissions-fragment3.jar

I'm attaching a diff and jar of the combined modifications for JS2-473 and this 
issue since there are many files modified in both and I did not succeed in 
producing a diff for the JS2-473 changes alone.  If both patches are accepted 
they can be applied at once with this patch/fileset.  

> 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]

Reply via email to