From ad883094015bbc82e28780857142dce8512835f6 Mon Sep 17 00:00:00 2001 From: Adam Retter Date: Tue, 21 Nov 2017 13:53:57 +0000 Subject: [PATCH] Don't create a session for every request if not required when doing form authentication Signed-off-by: Adam Retter --- .../authentication/FormAuthenticator.java | 24 ++++--------------- .../jetty/security/ConstraintTest.java | 9 +++++++ 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/FormAuthenticator.java b/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/FormAuthenticator.java index 684724ec8f5..c203bcdadfa 100644 --- a/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/FormAuthenticator.java +++ b/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/FormAuthenticator.java @@ -257,22 +257,6 @@ public class FormAuthenticator extends LoginAuthenticator if (isLoginOrErrorPage(URIUtil.addPaths(request.getServletPath(),request.getPathInfo())) &&!DeferredAuthentication.isDeferred(response)) return new DeferredAuthentication(this); - HttpSession session = null; - try - { - session = request.getSession(true); - } - catch (Exception e) - { - if (LOG.isDebugEnabled()) - LOG.debug(e); - } - - //if unable to create a session, user must be - //unauthenticated - if (session == null) - return Authentication.UNAUTHENTICATED; - try { // Handle a request for authentication. @@ -283,7 +267,7 @@ public class FormAuthenticator extends LoginAuthenticator UserIdentity user = login(username, password, request); LOG.debug("jsecuritycheck {} {}",username,user); - session = request.getSession(true); + HttpSession session = request.getSession(false); if (user!=null) { // Redirect to original request @@ -337,7 +321,8 @@ public class FormAuthenticator extends LoginAuthenticator } // Look for cached authentication - Authentication authentication = (Authentication) session.getAttribute(SessionAuthentication.__J_AUTHENTICATED); + HttpSession session = request.getSession(false); + Authentication authentication = session == null ? null : (Authentication) session.getAttribute(SessionAuthentication.__J_AUTHENTICATED); if (authentication != null) { // Has authentication been revoked? @@ -384,11 +369,12 @@ public class FormAuthenticator extends LoginAuthenticator // if we can't send challenge if (DeferredAuthentication.isDeferred(response)) { - LOG.debug("auth deferred {}",session.getId()); + LOG.debug("auth deferred {}",session == null ? null : session.getId()); return Authentication.UNAUTHENTICATED; } // remember the current URI + session = (session != null ? session : request.getSession(true)); synchronized (session) { // But only if it is not set already, or we save every uri that leads to a login form redirect diff --git a/jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java b/jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java index 007ed7c1728..f3d26f0ca0b 100644 --- a/jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java +++ b/jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java @@ -749,13 +749,16 @@ public class ConstraintTest response = _connector.getResponse("GET /ctx/noauth/info HTTP/1.0\r\n\r\n"); Assert.assertThat(response, Matchers.startsWith("HTTP/1.1 200 OK")); + Assert.assertThat(response, Matchers.not(Matchers.containsString("JSESSIONID="))); response = _connector.getResponse("GET /ctx/forbid/info HTTP/1.0\r\n\r\n"); Assert.assertThat(response, Matchers.startsWith("HTTP/1.1 403 Forbidden")); + Assert.assertThat(response, Matchers.not(Matchers.containsString("JSESSIONID="))); response = _connector.getResponse("GET /ctx/auth/info HTTP/1.0\r\n\r\n"); Assert.assertThat(response, Matchers.containsString(" 302 Found")); Assert.assertThat(response, Matchers.containsString("/ctx/testLoginPage")); + Assert.assertThat(response, Matchers.containsString("JSESSIONID=")); String session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); response = _connector.getResponse("GET /ctx/testLoginPage HTTP/1.0\r\n"+ @@ -763,6 +766,7 @@ public class ConstraintTest "\r\n"); Assert.assertThat(response, Matchers.containsString(" 200 OK")); Assert.assertThat(response, Matchers.containsString("URI=/ctx/testLoginPage")); + Assert.assertThat(response, Matchers.not(Matchers.containsString("JSESSIONID=" + session))); response = _connector.getResponse("POST /ctx/j_security_check HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + @@ -771,6 +775,7 @@ public class ConstraintTest "\r\n" + "j_username=user&j_password=wrong"); Assert.assertThat(response, Matchers.containsString("Location")); + Assert.assertThat(response, Matchers.not(Matchers.containsString("JSESSIONID=" + session))); response = _connector.getResponse("POST /ctx/j_security_check HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + @@ -781,18 +786,22 @@ public class ConstraintTest Assert.assertThat(response, Matchers.startsWith("HTTP/1.1 302 ")); Assert.assertThat(response, Matchers.containsString("Location")); Assert.assertThat(response, Matchers.containsString("/ctx/auth/info")); + Assert.assertThat(response, Matchers.containsString("JSESSIONID=")); + Assert.assertThat(response, Matchers.not(Matchers.containsString("JSESSIONID=" + session))); session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); response = _connector.getResponse("GET /ctx/auth/info HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + "\r\n"); Assert.assertThat(response, Matchers.startsWith("HTTP/1.1 200 OK")); + Assert.assertThat(response, Matchers.containsString("JSESSIONID=" + session)); response = _connector.getResponse("GET /ctx/admin/info HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + "\r\n"); Assert.assertThat(response, Matchers.startsWith("HTTP/1.1 403")); Assert.assertThat(response, Matchers.containsString("!role")); + Assert.assertThat(response, Matchers.not(Matchers.containsString("JSESSIONID=" + session))); } @Test