From cf504e59b6dd7ff1c2e6b23f63e8281bd18f4bef Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 10 Jan 2025 18:01:50 +1100 Subject: [PATCH] Issue #12697 - improve testing for FormAuthenticatorTest Signed-off-by: Lachlan Roberts --- .../security/FormAuthenticatorTest.java | 35 ++++++++++++++++--- .../security/FormAuthenticatorTest.java | 35 ++++++++++++++++--- 2 files changed, 60 insertions(+), 10 deletions(-) diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/security/FormAuthenticatorTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/security/FormAuthenticatorTest.java index 101b27c8e971..208865b5088c 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/security/FormAuthenticatorTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/security/FormAuthenticatorTest.java @@ -20,6 +20,7 @@ import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import org.eclipse.jetty.ee10.servlet.ErrorPageErrorHandler; import org.eclipse.jetty.ee10.servlet.ServletContextHandler; import org.eclipse.jetty.security.Constraint; import org.eclipse.jetty.security.EmptyLoginService; @@ -28,25 +29,28 @@ import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.Server; import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.startsWith; public class FormAuthenticatorTest { private Server _server; private LocalConnector _connector; - @BeforeEach - public void configureServer() throws Exception + public void configureServer(FormAuthenticator authenticator) throws Exception { _server = new Server(); _connector = new LocalConnector(_server); _server.addConnector(_connector); ServletContextHandler contextHandler = new ServletContextHandler("/ctx", ServletContextHandler.SESSIONS); + ErrorPageErrorHandler errorPageErrorHandler = new ErrorPageErrorHandler(); + errorPageErrorHandler.addErrorPage(403, "/servletErrorPage"); + contextHandler.setErrorHandler(errorPageErrorHandler); + _server.setHandler(contextHandler); contextHandler.addServlet(new AuthenticationTestServlet(), "/"); @@ -56,7 +60,7 @@ public void configureServer() throws Exception securityHandler.put("/any/*", Constraint.ANY_USER); securityHandler.put("/known/*", Constraint.KNOWN_ROLE); securityHandler.put("/admin/*", Constraint.from("admin")); - securityHandler.setAuthenticator(new FormAuthenticator("/login", "/error", true)); + securityHandler.setAuthenticator(authenticator); _server.start(); } @@ -76,16 +80,19 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) throws @AfterEach public void stopServer() throws Exception { - if (_server.isRunning()) + if (_server != null && _server.isRunning()) { _server.stop(); _server.join(); + _server = null; + _connector = null; } } @Test public void testLoginDispatch() throws Exception { + configureServer(new FormAuthenticator("/login", null, true)); String response = _connector.getResponse("GET /ctx/admin/user HTTP/1.0\r\nHost:host:8888\r\n\r\n"); assertThat(response, containsString("HTTP/1.1 200 OK")); assertThat(response, containsString("dispatcherType: REQUEST")); @@ -96,9 +103,27 @@ public void testLoginDispatch() throws Exception @Test public void testErrorDispatch() throws Exception { + // With dispatch enabled it does a AuthenticationState.serveAs to the error page with REQUEST dispatch type. + configureServer(new FormAuthenticator("/login", "/error", true)); String response = _connector.getResponse("GET /ctx/j_security_check?j_username=user&j_password=wrong HTTP/1.0\r\nHost:host:8888\r\n\r\n"); assertThat(response, containsString("dispatcherType: REQUEST")); assertThat(response, containsString("contextPath: /ctx")); assertThat(response, containsString("servletPath: /error")); + stopServer(); + + // With no dispatch it should do a redirect to the error page. + configureServer(new FormAuthenticator("/login", "/error", false)); + response = _connector.getResponse("GET /ctx/j_security_check?j_username=user&j_password=wrong HTTP/1.0\r\nHost:host:8888\r\n\r\n"); + assertThat(response, startsWith("HTTP/1.1 302 Found")); + assertThat(response, containsString("Location: /ctx/error")); + stopServer(); + + // With no FormAuthenticator error page, it will do an error dispatch to the servlet error page for that code. + configureServer(new FormAuthenticator("/login", null, true)); + response = _connector.getResponse("GET /ctx/j_security_check?j_username=user&j_password=wrong HTTP/1.0\r\nHost:host:8888\r\n\r\n"); + assertThat(response, containsString("dispatcherType: ERROR")); + assertThat(response, containsString("contextPath: /ctx")); + assertThat(response, containsString("servletPath: /servletErrorPage")); + stopServer(); } } diff --git a/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/security/FormAuthenticatorTest.java b/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/security/FormAuthenticatorTest.java index b3ec12e7c358..c5b1ccf11f00 100644 --- a/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/security/FormAuthenticatorTest.java +++ b/jetty-ee11/jetty-ee11-servlet/src/test/java/org/eclipse/jetty/ee11/servlet/security/FormAuthenticatorTest.java @@ -20,6 +20,7 @@ import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import org.eclipse.jetty.ee11.servlet.ErrorPageErrorHandler; import org.eclipse.jetty.ee11.servlet.ServletContextHandler; import org.eclipse.jetty.security.Constraint; import org.eclipse.jetty.security.EmptyLoginService; @@ -28,25 +29,28 @@ import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.Server; import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.startsWith; public class FormAuthenticatorTest { private Server _server; private LocalConnector _connector; - @BeforeEach - public void configureServer() throws Exception + public void configureServer(FormAuthenticator authenticator) throws Exception { _server = new Server(); _connector = new LocalConnector(_server); _server.addConnector(_connector); ServletContextHandler contextHandler = new ServletContextHandler("/ctx", ServletContextHandler.SESSIONS); + ErrorPageErrorHandler errorPageErrorHandler = new ErrorPageErrorHandler(); + errorPageErrorHandler.addErrorPage(403, "/servletErrorPage"); + contextHandler.setErrorHandler(errorPageErrorHandler); + _server.setHandler(contextHandler); contextHandler.addServlet(new AuthenticationTestServlet(), "/"); @@ -56,7 +60,7 @@ public void configureServer() throws Exception securityHandler.put("/any/*", Constraint.ANY_USER); securityHandler.put("/known/*", Constraint.KNOWN_ROLE); securityHandler.put("/admin/*", Constraint.from("admin")); - securityHandler.setAuthenticator(new FormAuthenticator("/login", "/error", true)); + securityHandler.setAuthenticator(authenticator); _server.start(); } @@ -76,16 +80,19 @@ protected void service(HttpServletRequest req, HttpServletResponse resp) throws @AfterEach public void stopServer() throws Exception { - if (_server.isRunning()) + if (_server != null && _server.isRunning()) { _server.stop(); _server.join(); + _server = null; + _connector = null; } } @Test public void testLoginDispatch() throws Exception { + configureServer(new FormAuthenticator("/login", null, true)); String response = _connector.getResponse("GET /ctx/admin/user HTTP/1.0\r\nHost:host:8888\r\n\r\n"); assertThat(response, containsString("HTTP/1.1 200 OK")); assertThat(response, containsString("dispatcherType: REQUEST")); @@ -96,9 +103,27 @@ public void testLoginDispatch() throws Exception @Test public void testErrorDispatch() throws Exception { + // With dispatch enabled it does a AuthenticationState.serveAs to the error page with REQUEST dispatch type. + configureServer(new FormAuthenticator("/login", "/error", true)); String response = _connector.getResponse("GET /ctx/j_security_check?j_username=user&j_password=wrong HTTP/1.0\r\nHost:host:8888\r\n\r\n"); assertThat(response, containsString("dispatcherType: REQUEST")); assertThat(response, containsString("contextPath: /ctx")); assertThat(response, containsString("servletPath: /error")); + stopServer(); + + // With no dispatch it should do a redirect to the error page. + configureServer(new FormAuthenticator("/login", "/error", false)); + response = _connector.getResponse("GET /ctx/j_security_check?j_username=user&j_password=wrong HTTP/1.0\r\nHost:host:8888\r\n\r\n"); + assertThat(response, startsWith("HTTP/1.1 302 Found")); + assertThat(response, containsString("Location: /ctx/error")); + stopServer(); + + // With no FormAuthenticator error page, it will do an error dispatch to the servlet error page for that code. + configureServer(new FormAuthenticator("/login", null, true)); + response = _connector.getResponse("GET /ctx/j_security_check?j_username=user&j_password=wrong HTTP/1.0\r\nHost:host:8888\r\n\r\n"); + assertThat(response, containsString("dispatcherType: ERROR")); + assertThat(response, containsString("contextPath: /ctx")); + assertThat(response, containsString("servletPath: /servletErrorPage")); + stopServer(); } }