Hi, On Wed, Apr 22, 2015 at 1:04 AM, <[email protected]> wrote:
> 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; > Maybe this should use org.apache.wicket.util.collections.ClassMetaCache to prevent introspecting the classes again and again for each request ? > } > > - 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. > >
