SHIRO-591 - Allow BasicHttpAuthenticationFilter to be configured in permissive mode
Project: http://git-wip-us.apache.org/repos/asf/shiro/repo Commit: http://git-wip-us.apache.org/repos/asf/shiro/commit/95131835 Tree: http://git-wip-us.apache.org/repos/asf/shiro/tree/95131835 Diff: http://git-wip-us.apache.org/repos/asf/shiro/diff/95131835 Branch: refs/heads/1.4.x Commit: 951318354a9005a7ccaba18effc3d2ea494e1e31 Parents: db46b11 Author: Brian Demers <bdem...@apache.org> Authored: Mon Oct 10 15:24:53 2016 -0400 Committer: Brian Demers <bdem...@apache.org> Committed: Fri Oct 14 15:15:51 2016 -0400 ---------------------------------------------------------------------- .../authc/BasicHttpAuthenticationFilter.java | 24 ++++++- .../BasicHttpFilterAuthenticationTest.java | 66 +++++++++++++++++++- 2 files changed, 85 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/shiro/blob/95131835/web/src/main/java/org/apache/shiro/web/filter/authc/BasicHttpAuthenticationFilter.java ---------------------------------------------------------------------- diff --git a/web/src/main/java/org/apache/shiro/web/filter/authc/BasicHttpAuthenticationFilter.java b/web/src/main/java/org/apache/shiro/web/filter/authc/BasicHttpAuthenticationFilter.java index 9f9a7e3..8598c2e 100644 --- a/web/src/main/java/org/apache/shiro/web/filter/authc/BasicHttpAuthenticationFilter.java +++ b/web/src/main/java/org/apache/shiro/web/filter/authc/BasicHttpAuthenticationFilter.java @@ -28,7 +28,9 @@ import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import java.util.HashSet; import java.util.Locale; +import java.util.Set; /** @@ -197,10 +199,11 @@ public class BasicHttpAuthenticationFilter extends AuthenticatingFilter { // Check whether the current request's method requires authentication. // If no methods have been configured, then all of them require auth, // otherwise only the declared ones need authentication. - String[] methods = (String[]) (mappedValue == null ? new String[0] : mappedValue); - boolean authcRequired = methods.length == 0; + + Set<String> methods = httpMethodsFromOptions((String[])mappedValue); + boolean authcRequired = methods.size() == 0; for (String m : methods) { - if (httpMethod.equalsIgnoreCase(m)) { + if (httpMethod.toUpperCase(Locale.ENGLISH).equals(m)) { // list of methods is in upper case authcRequired = true; break; } @@ -214,6 +217,21 @@ public class BasicHttpAuthenticationFilter extends AuthenticatingFilter { } } + private Set<String> httpMethodsFromOptions(String[] options) { + Set<String> methods = new HashSet<String>(); + + if (options != null) { + for (String option : options) { + // to be backwards compatible with 1.3, we can ONLY check for known args + // ideally we would just validate HTTP methods, but someone could already be using this for webdav + if (!option.equalsIgnoreCase(PERMISSIVE)) { + methods.add(option.toUpperCase(Locale.ENGLISH)); + } + } + } + return methods; + } + /** * Processes unauthenticated requests. It handles the two-stage request/challenge authentication protocol. * http://git-wip-us.apache.org/repos/asf/shiro/blob/95131835/web/src/test/java/org/apache/shiro/web/filter/authc/BasicHttpFilterAuthenticationTest.java ---------------------------------------------------------------------- diff --git a/web/src/test/java/org/apache/shiro/web/filter/authc/BasicHttpFilterAuthenticationTest.java b/web/src/test/java/org/apache/shiro/web/filter/authc/BasicHttpFilterAuthenticationTest.java index 3c32002..afdcbd6 100644 --- a/web/src/test/java/org/apache/shiro/web/filter/authc/BasicHttpFilterAuthenticationTest.java +++ b/web/src/test/java/org/apache/shiro/web/filter/authc/BasicHttpFilterAuthenticationTest.java @@ -177,7 +177,7 @@ public class BasicHttpFilterAuthenticationTest extends SecurityManagerTestSuppor HttpServletRequest request = createMock(HttpServletRequest.class); expect(request.getMethod()).andReturn("GET"); - expect(request.getMethod()).andReturn("POST"); + expect(request.getMethod()).andReturn("post"); expect(request.getHeader("Authorization")).andReturn(createAuthorizationHeader("pedro", "")).anyTimes(); expect(request.getRemoteHost()).andReturn("localhost").anyTimes(); replay(request); @@ -185,7 +185,7 @@ public class BasicHttpFilterAuthenticationTest extends SecurityManagerTestSuppor HttpServletResponse response = createMock(HttpServletResponse.class); replay(response); - boolean accessAllowed = testFilter.isAccessAllowed(request, response, new String[] { "post", "put", "delete" }); + boolean accessAllowed = testFilter.isAccessAllowed(request, response, new String[] { "POST", "put", "delete" }); assertTrue("Access not allowed for GET", accessAllowed); accessAllowed = testFilter.isAccessAllowed(request, response, new String[] { "post", "put", "delete" }); @@ -234,6 +234,68 @@ public class BasicHttpFilterAuthenticationTest extends SecurityManagerTestSuppor assertTrue("Access allowed for POST", !accessAllowed); } + /** + * @since 1.4 + */ + @Test + public void permissiveEnabledWithLoginTest() { + testFilter = new BasicHttpAuthenticationFilter(); + + HttpServletRequest request = createMock(HttpServletRequest.class); + expect(request.getHeader("Authorization")).andReturn(createAuthorizationHeader("pedro", "")).anyTimes(); + expect(request.getRemoteHost()).andReturn("localhost").anyTimes(); + expect(request.getMethod()).andReturn("GET"); + replay(request); + + HttpServletResponse response = createMock(HttpServletResponse.class); + replay(response); + + String[] mappedValue = {"permissive"}; + boolean accessAllowed = testFilter.isAccessAllowed(request, response, mappedValue); + assertTrue("Access allowed for GET", !accessAllowed); // login attempt should always be false + } + + /** + * @since 1.4 + */ + @Test + public void permissiveEnabledTest() { + testFilter = new BasicHttpAuthenticationFilter(); + + HttpServletRequest request = createMock(HttpServletRequest.class); + expect(request.getHeader("Authorization")).andReturn(null).anyTimes(); + expect(request.getRemoteHost()).andReturn("localhost").anyTimes(); + expect(request.getMethod()).andReturn("GET"); + replay(request); + + HttpServletResponse response = createMock(HttpServletResponse.class); + replay(response); + + String[] mappedValue = {"permissive"}; + boolean accessAllowed = testFilter.isAccessAllowed(request, response, mappedValue); + assertTrue("Access should be allowed for GET", accessAllowed); // non-login attempt, return true + } + + /** + * @since 1.4 + */ + @Test + public void httpMethodRequiresAuthenticationWithPermissive() throws Exception { + testFilter = new BasicHttpAuthenticationFilter(); + + HttpServletRequest request = createMock(HttpServletRequest.class); + expect(request.getHeader("Authorization")).andReturn(createAuthorizationHeader("pedro", "")); + expect(request.getRemoteHost()).andReturn("localhost"); + expect(request.getMethod()).andReturn("POST"); + replay(request); + + HttpServletResponse response = createMock(HttpServletResponse.class); + replay(response); + + boolean accessAllowed = testFilter.isAccessAllowed(request, response, new String[] {"permissive", "POST", "PUT", "DELETE" }); + assertTrue("Access allowed for POST", !accessAllowed); + } + private String createAuthorizationHeader(String username, String password) { return "Basic " + new String(Base64.encode((username + ":" + password).getBytes())); }
