You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jetspeed-dev@portals.apache.org by "David Le Strat (JIRA)" <je...@portals.apache.org> on 2006/01/28 20:10:33 UTC

[jira] Resolved: (JS2-475) Proposed changes in portal permissions

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

    Fix Version: 2.1-dev
     Resolution: Fixed

Patch has been committed.  I will update JS2-444 to account for the commit.

> 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
>     Assignee: David Le Strat
>      Fix For: 2.1-dev
>  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: jetspeed-dev-unsubscribe@portals.apache.org
For additional commands, e-mail: jetspeed-dev-help@portals.apache.org