Repository: wicket
Updated Branches:
  refs/heads/master 849cdc2ba -> 822a1693c


WICKET-5749 non-annotated resources should be allowed, not denied


Project: http://git-wip-us.apache.org/repos/asf/wicket/repo
Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/822a1693
Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/822a1693
Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/822a1693

Branch: refs/heads/master
Commit: 822a1693c2d017478613321ae6fce40d519b24fa
Parents: 849cdc2
Author: Carl-Eric Menzel <[email protected]>
Authored: Wed Apr 22 00:03:51 2015 +0200
Committer: Carl-Eric Menzel <[email protected]>
Committed: Wed Apr 22 00:03:51 2015 +0200

----------------------------------------------------------------------
 .../AnnotationsRoleAuthorizationStrategy.java   | 11 +++++--
 ...nnotationsRoleAuthorizationStrategyTest.java | 34 ++++++++++++++++----
 2 files changed, 36 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/wicket/blob/822a1693/wicket-auth-roles/src/main/java/org/apache/wicket/authroles/authorization/strategies/role/annotations/AnnotationsRoleAuthorizationStrategy.java
----------------------------------------------------------------------
diff --git 
a/wicket-auth-roles/src/main/java/org/apache/wicket/authroles/authorization/strategies/role/annotations/AnnotationsRoleAuthorizationStrategy.java
 
b/wicket-auth-roles/src/main/java/org/apache/wicket/authroles/authorization/strategies/role/annotations/AnnotationsRoleAuthorizationStrategy.java
index 9b1f95e..3a72d38 100644
--- 
a/wicket-auth-roles/src/main/java/org/apache/wicket/authroles/authorization/strategies/role/annotations/AnnotationsRoleAuthorizationStrategy.java
+++ 
b/wicket-auth-roles/src/main/java/org/apache/wicket/authroles/authorization/strategies/role/annotations/AnnotationsRoleAuthorizationStrategy.java
@@ -147,19 +147,24 @@ public class AnnotationsRoleAuthorizationStrategy extends 
AbstractRoleAuthorizat
        public boolean isResourceAuthorized(IResource resource, PageParameters 
pageParameters)
        {
                Class<? extends IResource> resourceClass = resource.getClass();
-               return 
checkResource(resourceClass.getAnnotation(AuthorizeResource.class)) || 
checkResource(
+               boolean allowedByResourceItself = isResourceAnnotationSatisfied(
+                               
resourceClass.getAnnotation(AuthorizeResource.class));
+               boolean allowedByPackage = isResourceAnnotationSatisfied(
                                
resourceClass.getPackage().getAnnotation(AuthorizeResource.class));
+               return allowedByResourceItself && allowedByPackage;
        }
 
-       private boolean checkResource(AuthorizeResource annotation)
+       private boolean isResourceAnnotationSatisfied(AuthorizeResource 
annotation)
        {
                if (annotation != null)
                {
+                       // we have an annotation => we must check for the 
required roles
                        return hasAny(new Roles(annotation.value()));
                }
                else
                {
-                       return false;
+                       // no annotation => no required roles => this resource 
can be accessed
+                       return true;
                }
        }
 }

http://git-wip-us.apache.org/repos/asf/wicket/blob/822a1693/wicket-auth-roles/src/test/java/org/apache/wicket/authroles/authorization/strategies/role/annotations/AnnotationsRoleAuthorizationStrategyTest.java
----------------------------------------------------------------------
diff --git 
a/wicket-auth-roles/src/test/java/org/apache/wicket/authroles/authorization/strategies/role/annotations/AnnotationsRoleAuthorizationStrategyTest.java
 
b/wicket-auth-roles/src/test/java/org/apache/wicket/authroles/authorization/strategies/role/annotations/AnnotationsRoleAuthorizationStrategyTest.java
index d5eecbf..90b3d66 100644
--- 
a/wicket-auth-roles/src/test/java/org/apache/wicket/authroles/authorization/strategies/role/annotations/AnnotationsRoleAuthorizationStrategyTest.java
+++ 
b/wicket-auth-roles/src/test/java/org/apache/wicket/authroles/authorization/strategies/role/annotations/AnnotationsRoleAuthorizationStrategyTest.java
@@ -16,9 +16,6 @@
  */
 package org.apache.wicket.authroles.authorization.strategies.role.annotations;
 
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-
 import org.apache.wicket.Component;
 import 
org.apache.wicket.authroles.authorization.strategies.role.IRoleCheckingStrategy;
 import org.apache.wicket.authroles.authorization.strategies.role.Roles;
@@ -27,6 +24,9 @@ import org.apache.wicket.request.resource.IResource;
 import org.junit.Test;
 import org.mockito.Mockito;
 
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
 /**
  * Tests for {@link AnnotationsRoleAuthorizationStrategy}
  */
@@ -139,7 +139,7 @@ public class AnnotationsRoleAuthorizationStrategyTest
        {
                AnnotationsRoleAuthorizationStrategy strategy = new 
AnnotationsRoleAuthorizationStrategy(
                                roles("role1"));
-               IResource resource = Mockito.mock(TestResource.class);
+               IResource resource = Mockito.mock(RestrictedResource.class);
                assertTrue(strategy.isResourceAuthorized(resource, null));
        }
 
@@ -148,10 +148,25 @@ public class AnnotationsRoleAuthorizationStrategyTest
        {
                AnnotationsRoleAuthorizationStrategy strategy = new 
AnnotationsRoleAuthorizationStrategy(
                                roles("role2"));
-               IResource resource = Mockito.mock(TestResource.class);
+               IResource resource = Mockito.mock(RestrictedResource.class);
                assertFalse(strategy.isResourceAuthorized(resource, null));
        }
 
+       @Test
+       public void allowsUnprotectedResourceWithRole() throws Exception {
+               AnnotationsRoleAuthorizationStrategy strategy = new 
AnnotationsRoleAuthorizationStrategy(
+                               roles("role2"));
+               IResource resource = Mockito.mock(UnrestrictedResource.class);
+               assertTrue(strategy.isResourceAuthorized(resource, null));
+       }
+       
+       @Test
+       public void allowsUnprotectedResourceWithoutRole() throws Exception {
+               AnnotationsRoleAuthorizationStrategy strategy = new 
AnnotationsRoleAuthorizationStrategy(roles());
+               IResource resource = Mockito.mock(UnrestrictedResource.class);
+               assertTrue(strategy.isResourceAuthorized(resource, null));
+       }
+
        @AuthorizeInstantiation({"role1"})
        private static class TestComponent_Instantiate extends WebComponent
        {
@@ -191,7 +206,7 @@ public class AnnotationsRoleAuthorizationStrategyTest
        }
 
        @AuthorizeResource("role1")
-       private static class TestResource implements IResource
+       private static class RestrictedResource implements IResource
        {
                /**
                 * Renders this resource to response using the provided 
attributes.
@@ -205,6 +220,13 @@ public class AnnotationsRoleAuthorizationStrategyTest
                }
        }
 
+       private static class UnrestrictedResource implements IResource {
+               @Override
+               public void respond(Attributes attributes) {
+                       ; // NOOP
+               }
+       }
+
        /**
         * Create a test role checking strategy that is simply given a list of 
roles
         * and returns true if that list contains any of the asked-for roles.

Reply via email to