jackrabbit-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thomas Mueller" <thomas.tom.muel...@gmail.com>
Subject Re: public review of 283 - checkPermission
Date Thu, 26 Jul 2007 14:29:46 GMT
Hi,

I would keep Session.checkPermission(..). It find it useful. See also
java.lang.SecurityManager.

But exceptions shouldn't be used for flow control, so I would add:
boolean Session.isPermissable(..) or
boolean Session.hasAccess(..) or
boolean Session.getPermission(..)

The 'action' parameter is a comma separated list of action strings:
"add_node, set_property, remove, read". To avoid typos the predefined
actions should be constants. Bit masks could be used, int
Session.PERMISSION_ADD_NODE = 1, PERMISSION_SET_PROPERTY = 2, 4, 8.

Thomas


> "void checkPermission(String absPath, String actions)
> Determines whether this Session has permission to perform the
> specified actions at the specified absPath. This method quietly returns
> if the access request is permitted, or throws a suitable
> java.security.AccessControlException otherwise. "
>
> This method forces a checked exception approach to access control checking
> that I find awkward. Many debates have gone on re:checked vs unchecked
> exception, but this method looks strange for a number of reasons:
> 1) the method is called 'checkPermission', so during checking I wouldn't
> expect that an AccessControlException is thrown, I would only expect that
> exception to be thrown if the system was trying to actually *carry out* an
> action of some kind that wasn't allowed, but *not* while simply checking.
> 2) the method returns void. Why not just return a boolean? Its clearer,
> doesn't require as much coding (no try catch), and doesn't force an
> exception approach on the caller.
>
> Suggestion:
> Deprecate the current method and add in a new method called isPermissable
> (that returns a boolean, and no exception) which is a bit more standard
> through the use of the 'isXxxx' naming convention.
>
> Best,
>
> Mark
>

Mime
View raw message