You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shiro.apache.org by ka...@apache.org on 2010/07/27 05:42:15 UTC

svn commit: r979512 - in /incubator/shiro/trunk/core: ./ src/main/java/org/apache/shiro/aop/ src/main/java/org/apache/shiro/authz/annotation/ src/main/java/org/apache/shiro/authz/aop/ src/main/java/org/apache/shiro/subject/ src/test/java/org/apache/shi...

Author: kaosko
Date: Tue Jul 27 03:42:14 2010
New Revision: 979512

URL: http://svn.apache.org/viewvc?rev=979512&view=rev
Log:
Incomplete - issue SHIRO-175: Improve Set of permission and role checks 
https://issues.apache.org/jira/browse/SHIRO-175
- extend all core authz annotations to @Target(ElementType.TYPE) as well 
- change value parameter of RequiresPermissions and RequiresRoles annotations from String to String[]
- Change DefaultAnnotationResolver to consider classes as well
- remove obsolete imports and code
- add test for authz annotation attached to classes
- fix the ambiguity in the javadoc for RequiresRoles semantics
- add note to authz/annotation/package-info of the slight changes in the annotation syntax in 1.1

Added:
    incubator/shiro/trunk/core/src/test/java/org/apache/shiro/aop/
    incubator/shiro/trunk/core/src/test/java/org/apache/shiro/aop/AnnotationResolverTest.java
Modified:
    incubator/shiro/trunk/core/   (props changed)
    incubator/shiro/trunk/core/src/main/java/org/apache/shiro/aop/AnnotationResolver.java
    incubator/shiro/trunk/core/src/main/java/org/apache/shiro/aop/DefaultAnnotationResolver.java
    incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/RequiresAuthentication.java
    incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/RequiresGuest.java
    incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/RequiresPermissions.java
    incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/RequiresRoles.java
    incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/RequiresUser.java
    incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/package-info.java
    incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/aop/PermissionAnnotationHandler.java
    incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/aop/RoleAnnotationHandler.java
    incubator/shiro/trunk/core/src/main/java/org/apache/shiro/subject/Subject.java
    incubator/shiro/trunk/core/src/test/java/org/apache/shiro/authz/aop/PermissionAnnotationHandlerTest.java
    incubator/shiro/trunk/core/src/test/java/org/apache/shiro/authz/aop/RoleAnnotationHandlerTest.java

Propchange: incubator/shiro/trunk/core/
------------------------------------------------------------------------------
--- svn:ignore (original)
+++ svn:ignore Tue Jul 27 03:42:14 2010
@@ -1,5 +1,6 @@
 *.iml
-target
-.settings
-.project
 .classpath
+.externalToolBuilders
+.project
+.settings
+target

Modified: incubator/shiro/trunk/core/src/main/java/org/apache/shiro/aop/AnnotationResolver.java
URL: http://svn.apache.org/viewvc/incubator/shiro/trunk/core/src/main/java/org/apache/shiro/aop/AnnotationResolver.java?rev=979512&r1=979511&r2=979512&view=diff
==============================================================================
--- incubator/shiro/trunk/core/src/main/java/org/apache/shiro/aop/AnnotationResolver.java (original)
+++ incubator/shiro/trunk/core/src/main/java/org/apache/shiro/aop/AnnotationResolver.java Tue Jul 27 03:42:14 2010
@@ -30,7 +30,8 @@ public interface AnnotationResolver {
     /**
      * Returns an {@link Annotation} instance of the specified type based on the given
      * {@link MethodInvocation MethodInvocation} argument, or {@code null} if no annotation
-     * of that type could be found.
+     * of that type could be found. First checks the invoked method itself and if not found, 
+     * then the class for the existence of the same annotation. 
      *
      * @param mi the intercepted method to be invoked.
      * @param clazz the annotation class of the annotation to find.

Modified: incubator/shiro/trunk/core/src/main/java/org/apache/shiro/aop/DefaultAnnotationResolver.java
URL: http://svn.apache.org/viewvc/incubator/shiro/trunk/core/src/main/java/org/apache/shiro/aop/DefaultAnnotationResolver.java?rev=979512&r1=979511&r2=979512&view=diff
==============================================================================
--- incubator/shiro/trunk/core/src/main/java/org/apache/shiro/aop/DefaultAnnotationResolver.java (original)
+++ incubator/shiro/trunk/core/src/main/java/org/apache/shiro/aop/DefaultAnnotationResolver.java Tue Jul 27 03:42:14 2010
@@ -58,6 +58,7 @@ public class DefaultAnnotationResolver i
             throw new IllegalArgumentException(msg);
 
         }
-        return m.getAnnotation(clazz);
+        Annotation annotation = m.getAnnotation(clazz);
+        return annotation == null ? mi.getThis().getClass().getAnnotation(clazz) : annotation;
     }
 }

Modified: incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/RequiresAuthentication.java
URL: http://svn.apache.org/viewvc/incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/RequiresAuthentication.java?rev=979512&r1=979511&r2=979512&view=diff
==============================================================================
--- incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/RequiresAuthentication.java (original)
+++ incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/RequiresAuthentication.java Tue Jul 27 03:42:14 2010
@@ -40,7 +40,7 @@ import java.lang.annotation.Target;
  *
  * @since 0.9.0
  */
-@Target(ElementType.METHOD)
+@Target({ElementType.TYPE, ElementType.METHOD})
 @Retention(RetentionPolicy.RUNTIME)
 public @interface RequiresAuthentication {
 }

Modified: incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/RequiresGuest.java
URL: http://svn.apache.org/viewvc/incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/RequiresGuest.java?rev=979512&r1=979511&r2=979512&view=diff
==============================================================================
--- incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/RequiresGuest.java (original)
+++ incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/RequiresGuest.java Tue Jul 27 03:42:14 2010
@@ -37,7 +37,7 @@ import java.lang.annotation.Target;
  *
  * @since 0.9.0
  */
-@Target(ElementType.METHOD)
+@Target({ElementType.TYPE, ElementType.METHOD})
 @Retention(RetentionPolicy.RUNTIME)
 public @interface RequiresGuest {
 }

Modified: incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/RequiresPermissions.java
URL: http://svn.apache.org/viewvc/incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/RequiresPermissions.java?rev=979512&r1=979511&r2=979512&view=diff
==============================================================================
--- incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/RequiresPermissions.java (original)
+++ incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/RequiresPermissions.java Tue Jul 27 03:42:14 2010
@@ -43,7 +43,7 @@ import java.lang.annotation.Target;
  * @see org.apache.shiro.subject.Subject#checkPermission
  * @since 0.1
  */
-@Target(ElementType.METHOD)
+@Target({ElementType.TYPE, ElementType.METHOD})
 @Retention(RetentionPolicy.RUNTIME)
 public @interface RequiresPermissions {
 
@@ -51,7 +51,7 @@ public @interface RequiresPermissions {
      * The permission string which will be passed to {@link org.apache.shiro.subject.Subject#isPermitted(String)}
      * to determine if the user is allowed to invoke the code protected by this annotation.
      */
-    String value();
+    String[] value();
 
 }
 

Modified: incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/RequiresRoles.java
URL: http://svn.apache.org/viewvc/incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/RequiresRoles.java?rev=979512&r1=979511&r2=979512&view=diff
==============================================================================
--- incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/RequiresRoles.java (original)
+++ incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/RequiresRoles.java Tue Jul 27 03:42:14 2010
@@ -24,8 +24,8 @@ import java.lang.annotation.RetentionPol
 import java.lang.annotation.Target;
 
 /**
- * Requires the currently executing {@link org.apache.shiro.subject.Subject Subject} to have one or more specified roles
- * in order to execute the annotated method. If they do not have the role(s), the method will not be executed and
+ * Requires the currently executing {@link org.apache.shiro.subject.Subject Subject} to have all of the 
+ * specified roles. If they do not have the role(s), the method will not be executed and
  * an {@link org.apache.shiro.authz.AuthorizationException AuthorizationException} is thrown.
  * <p/>
  * For example,
@@ -50,7 +50,7 @@ import java.lang.annotation.Target;
  * @see org.apache.shiro.subject.Subject#hasRole(String)
  * @since 0.1
  */
-@Target(ElementType.METHOD)
+@Target({ElementType.TYPE, ElementType.METHOD})
 @Retention(RetentionPolicy.RUNTIME)
 public @interface RequiresRoles {
 
@@ -58,6 +58,6 @@ public @interface RequiresRoles {
      * A single String role name or multiple comma-delimitted role names required in order for the method
      * invocation to be allowed.
      */
-    String value();
+    String[] value();
 
 }

Modified: incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/RequiresUser.java
URL: http://svn.apache.org/viewvc/incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/RequiresUser.java?rev=979512&r1=979511&r2=979512&view=diff
==============================================================================
--- incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/RequiresUser.java (original)
+++ incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/RequiresUser.java Tue Jul 27 03:42:14 2010
@@ -45,7 +45,7 @@ import java.lang.annotation.Target;
  *
  * @since 0.9.0
  */
-@Target(ElementType.METHOD)
+@Target({ElementType.TYPE, ElementType.METHOD})
 @Retention(RetentionPolicy.RUNTIME)
 public @interface RequiresUser {
 }

Modified: incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/package-info.java
URL: http://svn.apache.org/viewvc/incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/package-info.java?rev=979512&r1=979511&r2=979512&view=diff
==============================================================================
--- incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/package-info.java (original)
+++ incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/annotation/package-info.java Tue Jul 27 03:42:14 2010
@@ -19,5 +19,10 @@
 /**
  * Annotations used to restrict which classes, instances, or methods may be accessed or invoked depending on the
  * caller's access abilities or authentication state.
+ * 
+ * Since 1.1, all core annotations were extends to accept Target ElementType.TYPE in additon to ElementType.METHOD 
  */
 package org.apache.shiro.authz.annotation;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Target;
+

Modified: incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/aop/PermissionAnnotationHandler.java
URL: http://svn.apache.org/viewvc/incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/aop/PermissionAnnotationHandler.java?rev=979512&r1=979511&r2=979512&view=diff
==============================================================================
--- incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/aop/PermissionAnnotationHandler.java (original)
+++ incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/aop/PermissionAnnotationHandler.java Tue Jul 27 03:42:14 2010
@@ -24,6 +24,7 @@ import org.apache.shiro.subject.Subject;
 import org.apache.shiro.util.PermissionUtils;
 
 import java.lang.annotation.Annotation;
+import java.util.Arrays;
 import java.util.Set;
 
 
@@ -50,7 +51,7 @@ public class PermissionAnnotationHandler
      * @param a the RequiresPermissions annotation being inspected.
      * @return the annotation's <code>value</code>, from which the Permission will be constructed.
      */
-    protected String getAnnotationValue(Annotation a) {
+    protected String[] getAnnotationValue(Annotation a) {
         RequiresPermissions rpAnnotation = (RequiresPermissions) a;
         return rpAnnotation.value();
     }
@@ -65,20 +66,12 @@ public class PermissionAnnotationHandler
      *          continue access or execution.
      */
     public void assertAuthorized(Annotation a) throws AuthorizationException {
-        if (!(a instanceof RequiresPermissions)) {
-            return;
-        }
-        String p = getAnnotationValue(a);
-        Set<String> perms = PermissionUtils.toPermissionStrings(p);
+        if (!(a instanceof RequiresPermissions)) return;
 
+        String[] perms = getAnnotationValue(a);
         Subject subject = getSubject();
 
-        if (perms.size() == 1) {
-            subject.checkPermission(perms.iterator().next());
-        } else {
-            String[] permStrings = new String[perms.size()];
-            permStrings = perms.toArray(permStrings);
-            subject.checkPermissions(permStrings);
-        }
+        if (perms.length == 1) subject.checkPermission(perms[0]);
+        else subject.checkPermissions(perms);
     }
 }

Modified: incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/aop/RoleAnnotationHandler.java
URL: http://svn.apache.org/viewvc/incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/aop/RoleAnnotationHandler.java?rev=979512&r1=979511&r2=979512&view=diff
==============================================================================
--- incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/aop/RoleAnnotationHandler.java (original)
+++ incubator/shiro/trunk/core/src/main/java/org/apache/shiro/authz/aop/RoleAnnotationHandler.java Tue Jul 27 03:42:14 2010
@@ -23,9 +23,6 @@ import org.apache.shiro.authz.annotation
 
 import java.lang.annotation.Annotation;
 import java.util.Arrays;
-import java.util.LinkedHashSet;
-import java.util.Set;
-
 
 /**
  * Checks to see if a @{@link org.apache.shiro.authz.annotation.RequiresRoles RequiresRoles} annotation is declared, and if so, performs
@@ -53,21 +50,13 @@ public class RoleAnnotationHandler exten
      *          proceed.
      */
     public void assertAuthorized(Annotation a) throws AuthorizationException {
-        if (!(a instanceof RequiresRoles)) {
-            return;
-        }
-        RequiresRoles rrAnnotation = (RequiresRoles) a;
+        if (!(a instanceof RequiresRoles)) return;
 
-        String roleId = rrAnnotation.value();
-
-        String[] roles = roleId.split(",");
+        RequiresRoles rrAnnotation = (RequiresRoles) a;
+        String[] roles = rrAnnotation.value();
 
-        if (roles.length == 1) {
-            getSubject().checkRole(roles[0]);
-        } else {
-            Set<String> rolesSet = new LinkedHashSet<String>(Arrays.asList(roles));
-            getSubject().checkRoles(rolesSet);
-        }
+        if (roles.length == 1) getSubject().checkRole(roles[0]);
+        else getSubject().checkRoles(Arrays.asList(roles));
     }
 
 }

Modified: incubator/shiro/trunk/core/src/main/java/org/apache/shiro/subject/Subject.java
URL: http://svn.apache.org/viewvc/incubator/shiro/trunk/core/src/main/java/org/apache/shiro/subject/Subject.java?rev=979512&r1=979511&r2=979512&view=diff
==============================================================================
--- incubator/shiro/trunk/core/src/main/java/org/apache/shiro/subject/Subject.java (original)
+++ incubator/shiro/trunk/core/src/main/java/org/apache/shiro/subject/Subject.java Tue Jul 27 03:42:14 2010
@@ -781,7 +781,6 @@ public interface Subject {
          * @throws IllegalArgumentException if the {@code attributeKey} is {@code null}.
          * @see SubjectFactory#createSubject(SubjectContext)
          */
-        @SuppressWarnings({"UnusedDeclaration"})
         public Builder contextAttribute(String attributeKey, Object attributeValue) {
             if (attributeKey == null) {
                 String msg = "Subject context map key cannot be null.";

Added: incubator/shiro/trunk/core/src/test/java/org/apache/shiro/aop/AnnotationResolverTest.java
URL: http://svn.apache.org/viewvc/incubator/shiro/trunk/core/src/test/java/org/apache/shiro/aop/AnnotationResolverTest.java?rev=979512&view=auto
==============================================================================
--- incubator/shiro/trunk/core/src/test/java/org/apache/shiro/aop/AnnotationResolverTest.java (added)
+++ incubator/shiro/trunk/core/src/test/java/org/apache/shiro/aop/AnnotationResolverTest.java Tue Jul 27 03:42:14 2010
@@ -0,0 +1,46 @@
+package org.apache.shiro.aop;
+
+import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.replay;
+
+import java.lang.reflect.Method;
+
+import org.apache.shiro.authz.annotation.RequiresRoles;
+import org.apache.shiro.authz.annotation.RequiresUser;
+import org.junit.Test;
+import static org.junit.Assert.*;
+
+
+public class AnnotationResolverTest {
+    @SuppressWarnings("unused")
+    @RequiresRoles("root")
+    private class MyFixture {
+	public void operateThis() {}
+        @RequiresUser()
+        public void operateThat() {}
+    }
+    
+    DefaultAnnotationResolver annotationResolver = new DefaultAnnotationResolver();
+
+    @Test
+    public void testAnnotationFoundFromClass() throws SecurityException, NoSuchMethodException {
+	MyFixture myFixture = new MyFixture();
+	MethodInvocation methodInvocation = createMock(MethodInvocation.class);
+	Method method = MyFixture.class.getDeclaredMethod("operateThis");
+        expect(methodInvocation.getMethod()).andReturn(method);
+        expect(methodInvocation.getThis()).andReturn(myFixture);
+        replay(methodInvocation);
+	assertNotNull(annotationResolver.getAnnotation(methodInvocation, RequiresRoles.class));
+    }
+    
+    @Test
+    public void testAnnotationFoundFromMethod() throws SecurityException, NoSuchMethodException {
+	MethodInvocation methodInvocation = createMock(MethodInvocation.class);
+	Method method = MyFixture.class.getDeclaredMethod("operateThat");
+        expect(methodInvocation.getMethod()).andReturn(method);
+        replay(methodInvocation);
+	assertNotNull(annotationResolver.getAnnotation(methodInvocation, RequiresUser.class));
+    }
+}
+

Modified: incubator/shiro/trunk/core/src/test/java/org/apache/shiro/authz/aop/PermissionAnnotationHandlerTest.java
URL: http://svn.apache.org/viewvc/incubator/shiro/trunk/core/src/test/java/org/apache/shiro/authz/aop/PermissionAnnotationHandlerTest.java?rev=979512&r1=979511&r2=979512&view=diff
==============================================================================
--- incubator/shiro/trunk/core/src/test/java/org/apache/shiro/authz/aop/PermissionAnnotationHandlerTest.java (original)
+++ incubator/shiro/trunk/core/src/test/java/org/apache/shiro/authz/aop/PermissionAnnotationHandlerTest.java Tue Jul 27 03:42:14 2010
@@ -37,8 +37,8 @@ public class PermissionAnnotationHandler
         PermissionAnnotationHandler handler = new PermissionAnnotationHandler();
 
         Annotation requiresPermissionAnnotation = new RequiresPermissions() {
-            public String value() {
-                return "test:test";
+            public String[] value() {
+                return new String[]{"test:test"};
             }
 
             public Class<? extends Annotation> annotationType() {
@@ -56,8 +56,8 @@ public class PermissionAnnotationHandler
         PermissionAnnotationHandler handler = new PermissionAnnotationHandler();
 
         Annotation requiresPermissionAnnotation = new RequiresPermissions() {
-            public String value() {
-                return "test:test, test2:test2";
+            public String[] value() {
+                return new String[]{"test:test", "test2:test2"};
             }
 
             public Class<? extends Annotation> annotationType() {

Modified: incubator/shiro/trunk/core/src/test/java/org/apache/shiro/authz/aop/RoleAnnotationHandlerTest.java
URL: http://svn.apache.org/viewvc/incubator/shiro/trunk/core/src/test/java/org/apache/shiro/authz/aop/RoleAnnotationHandlerTest.java?rev=979512&r1=979511&r2=979512&view=diff
==============================================================================
--- incubator/shiro/trunk/core/src/test/java/org/apache/shiro/authz/aop/RoleAnnotationHandlerTest.java (original)
+++ incubator/shiro/trunk/core/src/test/java/org/apache/shiro/authz/aop/RoleAnnotationHandlerTest.java Tue Jul 27 03:42:14 2010
@@ -37,8 +37,8 @@ public class RoleAnnotationHandlerTest e
         RoleAnnotationHandler handler = new RoleAnnotationHandler();
 
         Annotation requiresRolesAnnotation = new RequiresRoles() {
-            public String value() {
-                return "blah";
+            public String[] value() {
+                return new String[]{"blah"};
             }
 
             public Class<? extends Annotation> annotationType() {
@@ -56,8 +56,8 @@ public class RoleAnnotationHandlerTest e
         RoleAnnotationHandler handler = new RoleAnnotationHandler();
 
         Annotation requiresRolesAnnotation = new RequiresRoles() {
-            public String value() {
-                return "blah, blah2";
+            public String[] value() {
+                return new String[]{"blah", "blah2"};
             }
 
             public Class<? extends Annotation> annotationType() {