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