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
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
<kalle.o.korho...@gmail.com>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
>
