Repository: eagle Updated Branches: refs/heads/master d5dce2b34 -> 56c2b8a6f
[MINOR] Fix AuthFilter to support REST Stream Proxy * Support `Auth(required=false)` * Return `403` instead of `401` for `invalid access` other than `unauthorized access` Author: Hao Chen <[email protected]> Closes #866 from haoch/FixBasicAuthenticationTestCase. Project: http://git-wip-us.apache.org/repos/asf/eagle/repo Commit: http://git-wip-us.apache.org/repos/asf/eagle/commit/56c2b8a6 Tree: http://git-wip-us.apache.org/repos/asf/eagle/tree/56c2b8a6 Diff: http://git-wip-us.apache.org/repos/asf/eagle/diff/56c2b8a6 Branch: refs/heads/master Commit: 56c2b8a6f828e36eb399a780a7705000222475e6 Parents: d5dce2b Author: Hao Chen <[email protected]> Authored: Mon Mar 13 00:06:51 2017 +0800 Committer: Hao Chen <[email protected]> Committed: Mon Mar 13 00:06:51 2017 +0800 ---------------------------------------------------------------------- .../server/security/BasicAuthRequestFilter.java | 12 ++++++++---- .../resource/BasicAuthenticationTestCase.java | 16 ++++++++++++++-- 2 files changed, 22 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/eagle/blob/56c2b8a6/eagle-server/src/main/java/org/apache/eagle/server/security/BasicAuthRequestFilter.java ---------------------------------------------------------------------- diff --git a/eagle-server/src/main/java/org/apache/eagle/server/security/BasicAuthRequestFilter.java b/eagle-server/src/main/java/org/apache/eagle/server/security/BasicAuthRequestFilter.java index 81a10c8..ebdc93f 100644 --- a/eagle-server/src/main/java/org/apache/eagle/server/security/BasicAuthRequestFilter.java +++ b/eagle-server/src/main/java/org/apache/eagle/server/security/BasicAuthRequestFilter.java @@ -54,6 +54,7 @@ public class BasicAuthRequestFilter implements ContainerRequestFilter { private static final Logger LOG = LoggerFactory.getLogger(BasicAuthRequestFilter.class); private boolean isSecurityDefined = false; private boolean isAuthRequired = false; + private boolean isAuthDefined = false; private boolean hasPermitAllAnnotation = false; private boolean hasDenyAllAnnotation = false; private boolean hasRolesAllowedAnnotation = false; @@ -66,15 +67,16 @@ public class BasicAuthRequestFilter implements ContainerRequestFilter { this.hasRolesAllowedAnnotation = method.isAnnotationPresent(RolesAllowed.class); this.isSecurityDefined = this.hasPermitAllAnnotation || this.hasDenyAllAnnotation || this.hasRolesAllowedAnnotation; for (Parameter parameter : method.getMethod().getParameters()) { - if (isSecurityDefined && isAuthRequired) { + if (isAuthRequired && isAuthDefined) { break; } Auth[] authAnnotations = parameter.getAnnotationsByType(Auth.class); - this.isSecurityDefined = this.isSecurityDefined || authAnnotations.length > 0; + this.isAuthDefined = authAnnotations.length > 0 || this.isAuthDefined; for (Auth auth : authAnnotations) { - this.isAuthRequired = this.isAuthRequired || auth.required(); + this.isAuthRequired = auth.required() || this.isAuthRequired; } } + this.isSecurityDefined = this.isAuthDefined || this.isSecurityDefined; Preconditions.checkArgument(!(this.hasDenyAllAnnotation && this.hasPermitAllAnnotation), "Conflict @DenyAll and @PermitAll on method " + this.method.toString()); } @@ -182,7 +184,9 @@ public class BasicAuthRequestFilter implements ContainerRequestFilter { throw new WebApplicationException(INVALID_ACCESS_FORBIDDEN); } } else { - throw new WebApplicationException(UNAUTHORIZED_ACCESS_DENIED); + if (!this.isAuthDefined) { + throw new WebApplicationException(UNAUTHORIZED_ACCESS_DENIED); + } } } catch (WebApplicationException e) { throw e; http://git-wip-us.apache.org/repos/asf/eagle/blob/56c2b8a6/eagle-server/src/test/java/org/apache/eagle/server/security/resource/BasicAuthenticationTestCase.java ---------------------------------------------------------------------- diff --git a/eagle-server/src/test/java/org/apache/eagle/server/security/resource/BasicAuthenticationTestCase.java b/eagle-server/src/test/java/org/apache/eagle/server/security/resource/BasicAuthenticationTestCase.java index c848d19..b8f2a43 100644 --- a/eagle-server/src/test/java/org/apache/eagle/server/security/resource/BasicAuthenticationTestCase.java +++ b/eagle-server/src/test/java/org/apache/eagle/server/security/resource/BasicAuthenticationTestCase.java @@ -55,6 +55,18 @@ public class BasicAuthenticationTestCase { } @Test + public void testAuthUserOnlyWithoutKey() { + try { + Client client = new Client(); + client.resource(String.format("http://localhost:%d/rest/testAuth/userOnly";, RULE.getLocalPort())) + .get(User.class); + Assert.fail(); + } catch (UniformInterfaceException e) { + Assert.assertEquals(401, e.getResponse().getStatus()); + } + } + + @Test public void testAuthAdminOnly() { Client client = new Client(); client.resource(String.format("http://localhost:%d/rest/testAuth/adminOnly";, RULE.getLocalPort())) @@ -142,7 +154,7 @@ public class BasicAuthenticationTestCase { } @Test - public void testAuthPermitAllWithBadKeyShouldAccept401() { + public void testAuthPermitAllWithBadKeyShouldAccept403() { Client client = new Client(); try { client.resource(String.format("http://localhost:%d/rest/testAuth/permitAll";, RULE.getLocalPort())) @@ -150,7 +162,7 @@ public class BasicAuthenticationTestCase { .get(User.class); Assert.fail(); } catch (UniformInterfaceException e) { - Assert.assertEquals(401, e.getResponse().getStatus()); + Assert.assertEquals(403, e.getResponse().getStatus()); } }
