From a3d31d686049315106ef2337be5531241dc10532 Mon Sep 17 00:00:00 2001 From: Brian Demers Date: Fri, 10 Jun 2022 14:35:06 -0400 Subject: [PATCH 1/2] Add branch 1.9.x to GH Actions build --- .github/workflows/maven.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index aa2782227a..eca9719c27 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -20,7 +20,7 @@ name: Maven CI on: workflow_dispatch: {} push: - branches: [ main ] + branches: [ main, 1.9.x ] pull_request: branches: [ ] From 37e2191e49cdc3bc6d36d677845b05a62ab30aba Mon Sep 17 00:00:00 2001 From: Brian Demers Date: Wed, 15 Jun 2022 17:32:10 -0400 Subject: [PATCH 2/2] Fix sporadic failures of AuthorizationFilterTest These failures seemed to be caused by a test threading issue. This change scopes the use of the Subject to a runnable. --- .../filter/authz/AuthorizationFilterTest.java | 132 ++++++++++++------ 1 file changed, 90 insertions(+), 42 deletions(-) diff --git a/web/src/test/java/org/apache/shiro/web/filter/authz/AuthorizationFilterTest.java b/web/src/test/java/org/apache/shiro/web/filter/authz/AuthorizationFilterTest.java index dba7cb530e..dd6a86f1cd 100644 --- a/web/src/test/java/org/apache/shiro/web/filter/authz/AuthorizationFilterTest.java +++ b/web/src/test/java/org/apache/shiro/web/filter/authz/AuthorizationFilterTest.java @@ -18,8 +18,9 @@ */ package org.apache.shiro.web.filter.authz; -import org.apache.shiro.SecurityUtils; import org.apache.shiro.authc.UsernamePasswordToken; +import org.apache.shiro.mgt.SecurityManager; +import org.apache.shiro.subject.Subject; import org.apache.shiro.test.SecurityManagerTestSupport; import org.junit.Test; @@ -27,9 +28,12 @@ import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import java.io.IOException; +import java.util.Objects; -import static org.easymock.EasyMock.*; +import static org.easymock.EasyMock.createNiceMock; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.verify; /** * Test cases for the {@link AuthorizationFilter} class. @@ -37,63 +41,107 @@ public class AuthorizationFilterTest extends SecurityManagerTestSupport { @Test - public void testUserOnAccessDeniedWithResponseError() throws IOException { + public void testUserOnAccessDeniedWithResponseError() throws Exception { // Tests when a user (known identity) is denied access and no unauthorizedUrl has been configured. // This should trigger an HTTP response error code. //log in the user using the account provided by the superclass for tests: - SecurityUtils.getSubject().login(new UsernamePasswordToken("test", "test")); - - AuthorizationFilter filter = new AuthorizationFilter() { - @Override - protected boolean isAccessAllowed(ServletRequest request, ServletResponse response, Object mappedValue) - throws Exception { - return false; //for this test case - } - }; - - HttpServletRequest request = createNiceMock(HttpServletRequest.class); - HttpServletResponse response = createNiceMock(HttpServletResponse.class); - - response.sendError(HttpServletResponse.SC_UNAUTHORIZED); - replay(response); - filter.onAccessDenied(request, response); - verify(response); + runWithSubject(subject -> { + subject.login(new UsernamePasswordToken("test", "test")); + AuthorizationFilter filter = new AuthorizationFilter() { + @Override + protected boolean isAccessAllowed(ServletRequest request, ServletResponse response, Object mappedValue) + throws Exception { + return false; //for this test case + } + }; + + HttpServletRequest request = createNiceMock(HttpServletRequest.class); + HttpServletResponse response = createNiceMock(HttpServletResponse.class); + + response.sendError(HttpServletResponse.SC_UNAUTHORIZED); + replay(response); + filter.onAccessDenied(request, response); + verify(response); + }); } @Test - public void testUserOnAccessDeniedWithRedirect() throws IOException { + public void testUserOnAccessDeniedWithRedirect() throws Exception { // Tests when a user (known identity) is denied access and an unauthorizedUrl *has* been configured. // This should trigger an HTTP redirect //log in the user using the account provided by the superclass for tests: - SecurityUtils.getSubject().login(new UsernamePasswordToken("test", "test")); + runWithSubject(subject -> { + subject.login(new UsernamePasswordToken("test", "test")); + + String unauthorizedUrl = "unauthorized.jsp"; + + AuthorizationFilter filter = new AuthorizationFilter() { + @Override + protected boolean isAccessAllowed(ServletRequest request, ServletResponse response, Object mappedValue) + throws Exception { + return false; //for this test case + } + }; + filter.setUnauthorizedUrl(unauthorizedUrl); + + HttpServletRequest request = createNiceMock(HttpServletRequest.class); + HttpServletResponse response = createNiceMock(HttpServletResponse.class); + + expect(request.getContextPath()).andReturn("/").anyTimes(); - String unauthorizedUrl = "unauthorized.jsp"; + String encoded = "/" + unauthorizedUrl; + expect(response.encodeRedirectURL(unauthorizedUrl)).andReturn(encoded); + response.sendRedirect(encoded); + replay(request); + replay(response); - AuthorizationFilter filter = new AuthorizationFilter() { - @Override - protected boolean isAccessAllowed(ServletRequest request, ServletResponse response, Object mappedValue) - throws Exception { - return false; //for this test case + filter.onAccessDenied(request, response); + + verify(request); + verify(response); + }); + } + + /** + * Associates the {@code consumer} with the {@code subject} and executes. If an exeception was thrown by the + * consumer, it is re-thrown by this method. + * @param subject The subject to bind to the current thread. + * @param consumer The block of code to run under the context of the subject. + * @throws Exception propagates any exception thrown by the consumer. + */ + private static void runWithSubject(Subject subject, SubjectConsumer consumer) throws Exception { + Exception exception = subject.execute(() -> { + try { + consumer.accept(subject); + return null; + } catch (Exception e) { + return e; } - }; - filter.setUnauthorizedUrl(unauthorizedUrl); + }); + if (Objects.nonNull(exception)) { + throw exception; + } + } - HttpServletRequest request = createNiceMock(HttpServletRequest.class); - HttpServletResponse response = createNiceMock(HttpServletResponse.class); + private static void runWithSubject(SubjectConsumer consumer) throws Exception { + runWithSubject(createSubject(), consumer); + } - expect(request.getContextPath()).andReturn("/").anyTimes(); + private static Subject createSubject() { + SecurityManager securityManager = createTestSecurityManager(); + return new Subject.Builder(securityManager).buildSubject(); + } - String encoded = "/" + unauthorizedUrl; - expect(response.encodeRedirectURL(unauthorizedUrl)).andReturn(encoded); - response.sendRedirect(encoded); - replay(request); - replay(response); + // NOOP for the previous before/after test (the parent class is annotated) + public void setup() {} - filter.onAccessDenied(request, response); + public void teardown() {} - verify(request); - verify(response); + // A simple consumer that allows exceptions to be thrown + @FunctionalInterface + private interface SubjectConsumer { + void accept(Subject subject) throws Exception; } }