You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by cm...@apache.org on 2015/04/22 00:04:46 UTC
wicket git commit: WICKET-5749 non-annotated resources should be
allowed, not denied
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 <cm...@wicketbuch.de>
Authored: Wed Apr 22 00:03:51 2015 +0200
Committer: Carl-Eric Menzel <cm...@wicketbuch.de>
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.
Re: wicket git commit: WICKET-5749 non-annotated resources should be
allowed, not denied
Posted by Martin Grigorov <mg...@apache.org>.
Hi,
On Wed, Apr 22, 2015 at 1:04 AM, <cm...@apache.org> 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 <cm...@wicketbuch.de>
> Authored: Wed Apr 22 00:03:51 2015 +0200
> Committer: Carl-Eric Menzel <cm...@wicketbuch.de>
> 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.
>
>