Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #1985 from adamretter/hotfix/FormAuthenticator-session 

Remove unnecessary session creation in FormAuthenticator
This commit is contained in:
Jan Bartel 2017-11-23 12:58:07 +01:00 committed by GitHub
parent 9e891a4ceb ad88309401
commit 375d7aa8da
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 14 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
jetty-security/src/main/java/org/eclipse/jetty/security/authentication/FormAuthenticator.java
View File

@ -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

9
jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java
View File

@ -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