On 25/06/2010 23:18, Les Hazlewood wrote:
Definitely some good ideas here.
My initial concern is that 1.0.1 should be backwards and forwards compatible
w/ 1.0.0 and 1.0.3 respectively per traditional APR compatibility
guidelines. If we as a project want to retain that compatibility policy,
shouldn't these changes go into 1.1? (I personally think the compatibility
guidelines are a sound way to go about this kind of stuff, barring some
other reasonable alternative.)
As to the Annotations, Jeremy and I have talked a number of times (long ago)
about adding expression support to annotations using a proper grammar.
Something like:
@SecurityCheck("perm(user:read:{0}.id)&& (role[admin] || role[developer])")
public void someSecureMethod(User user) { ... }
where {0} represents the first method argument, {1} the second, etc.
I don't know if we'll get this in the codebase any time soon, but I wanted
to surface it again to get the creative juices flowing again. Food for
thought.
As for the existing annotations, I believe they're both supposed to be
aggregating - i.e. 'ALL' for both of them. My personal preference is that
we keep the existing annotations and have retain 'ALL' semantics.
@RequiresRoles(new String[] { "dev", "teamLead"}) to me definitely reads as
Correct we if i am wrong but i think you can omit the new String[] stuff
like this:
@RequiresRoles({"dev", "teamLead"})
which would look/read so much better.
If there is only one role it should be possible to write:
@RequiresRoles("dev")
I should have both the 'dev' and 'teamLead' roles in order for the method to
execute. @RequiresAnyRoles (again, to me) sounds like a good alternative
for 'OR' semantics.
Thoughts?
Les
On Fri, Jun 25, 2010 at 1:24 PM, Kalle Korhonen
<[email protected]>wrote:
Related to https://issues.apache.org/jira/browse/SHIRO-175 and some of
the Shiro extensions we (Tynamo.org) have made, I'd like to change
@Target(ElementType.METHOD) of all annotations to
@Target({ElementType.TYPE, ElementType.METHOD}). The obvious semantics
of attaching a security annotation to a class is that the same
permission would apply to all method invocations in that class.
SHIRO-175 suggests that we'd deprecate the existing annotations in
favor of RequiresAnyXXX and RequiresAllXXX annotations, forcing to
target 1.1 release for the issue resolution. We could also keep the
existing annotations and pick either ANY or ALL semantics in order to
avoid deprecating anything - not sure which one is better. The current
semantics are unclear, the Javadoc implies ANY for RequiresRoles but
ALL for RequiresPermissions.
Here's what I'd suggest:
- 1.0.1 change @Target to apply to TYPEs as well, fix ambiguity of
current annotations (especially if we decide to keep them)
- 1.1.0 resolve SHIRO-175 and add RequiresAnyXXX and RequiresAllXXX
annotations (and deprecate existing annotations if we choose to do so)
I can handle implementing the changes. Any comments?
Kalle
