diff options
author | Greg Wilkins | 2012-08-19 23:22:03 +0000 |
---|---|---|
committer | Greg Wilkins | 2012-08-19 23:22:03 +0000 |
commit | 4ff7ae610650c503814556742f9ea700c5953b07 (patch) | |
tree | 413572ac29a6ec52fdf57a67d9c593afbc1ee346 /jetty-security | |
parent | 2900b4df02f9a13e87b6362c0058ee3c10c81c91 (diff) | |
download | org.eclipse.jetty.project-4ff7ae610650c503814556742f9ea700c5953b07.tar.gz org.eclipse.jetty.project-4ff7ae610650c503814556742f9ea700c5953b07.tar.xz org.eclipse.jetty.project-4ff7ae610650c503814556742f9ea700c5953b07.zip |
jetty-9 removed redirect race from form auth
Diffstat (limited to 'jetty-security')
4 files changed, 85 insertions, 43 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 c1493c897e..57587e12b4 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 @@ -43,6 +43,7 @@ import org.eclipse.jetty.server.Authentication.User; import org.eclipse.jetty.server.HttpChannel; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.UserIdentity; +import org.eclipse.jetty.server.session.HashedSession; import org.eclipse.jetty.util.MultiMap; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.URIUtil; @@ -200,6 +201,7 @@ public class FormAuthenticator extends LoginAuthenticator return _deferred; HttpSession session = request.getSession(true); + System.err.println(session.getId()+((HashedSession)session).getAttributeMap()); try { @@ -208,31 +210,36 @@ public class FormAuthenticator extends LoginAuthenticator { final String username = request.getParameter(__J_USERNAME); final String password = request.getParameter(__J_PASSWORD); - + UserIdentity user = _loginService.login(username,password); + LOG.debug("jsecuritycheck {} {}",username,user); if (user!=null) { session=renewSession(request,response); // Redirect to original request String nuri; + FormAuthentication form_auth; synchronized(session) { nuri = (String) session.getAttribute(__J_URI); - } - if (nuri == null || nuri.length() == 0) - { - nuri = request.getContextPath(); - if (nuri.length() == 0) - nuri = URIUtil.SLASH; + if (nuri == null || nuri.length() == 0) + { + nuri = request.getContextPath(); + if (nuri.length() == 0) + nuri = URIUtil.SLASH; + } + + Authentication cached=new SessionAuthentication(getAuthMethod(),user,password); + session.setAttribute(SessionAuthentication.__J_AUTHENTICATED, cached); + form_auth = new FormAuthentication(getAuthMethod(),user); } + LOG.debug("authenticated {}->{}",form_auth,nuri); + response.setContentLength(0); response.sendRedirect(response.encodeRedirectURL(nuri)); - - Authentication cached=new SessionAuthentication(getAuthMethod(),user,password); - session.setAttribute(SessionAuthentication.__J_AUTHENTICATED, cached); - return new FormAuthentication(getAuthMethod(),user); + return form_auth; } // not authenticated @@ -240,11 +247,13 @@ public class FormAuthenticator extends LoginAuthenticator LOG.debug("Form authentication FAILED for " + StringUtil.printable(username)); if (_formErrorPage == null) { + LOG.debug("auth failed {}->403",username); if (response != null) response.sendError(HttpServletResponse.SC_FORBIDDEN); } else if (_dispatch) { + LOG.debug("auth failed {}=={}",username,_formErrorPage); RequestDispatcher dispatcher = request.getRequestDispatcher(_formErrorPage); response.setHeader(HttpHeader.CACHE_CONTROL.asString(),HttpHeaderValue.NO_CACHE.asString()); response.setDateHeader(HttpHeader.EXPIRES.asString(),1); @@ -252,6 +261,7 @@ public class FormAuthenticator extends LoginAuthenticator } else { + LOG.debug("auth failed {}->{}",username,_formErrorPage); response.sendRedirect(response.encodeRedirectURL(URIUtil.addPaths(request.getContextPath(),_formErrorPage))); } @@ -267,43 +277,51 @@ public class FormAuthenticator extends LoginAuthenticator _loginService!=null && !_loginService.validate(((Authentication.User)authentication).getUserIdentity())) { - + LOG.debug("auth revoked {}",authentication); session.removeAttribute(SessionAuthentication.__J_AUTHENTICATED); } else { - String j_uri=(String)session.getAttribute(__J_URI); - if (j_uri!=null) + synchronized (session) { - MultiMap j_post = (MultiMap)session.getAttribute(__J_POST); - if (j_post!=null) + String j_uri=(String)session.getAttribute(__J_URI); + if (j_uri!=null) { - StringBuffer buf = request.getRequestURL(); - if (request.getQueryString() != null) - buf.append("?").append(request.getQueryString()); - - if (j_uri.equals(buf.toString())) + LOG.debug("auth retry {}->{}",authentication,j_uri); + MultiMap<String> j_post = (MultiMap<String>)session.getAttribute(__J_POST); + if (j_post!=null) { - // This is a retry of an original POST request - // so restore method and parameters - - session.removeAttribute(__J_POST); - Request base_request = HttpChannel.getCurrentHttpChannel().getRequest(); - base_request.setMethod(HttpMethod.POST,HttpMethod.POST.asString()); - base_request.setParameters(j_post); + LOG.debug("auth rePOST {}->{}",authentication,j_uri); + StringBuffer buf = request.getRequestURL(); + if (request.getQueryString() != null) + buf.append("?").append(request.getQueryString()); + + if (j_uri.equals(buf.toString())) + { + // This is a retry of an original POST request + // so restore method and parameters + + session.removeAttribute(__J_POST); + Request base_request = HttpChannel.getCurrentHttpChannel().getRequest(); + base_request.setMethod(HttpMethod.POST,HttpMethod.POST.asString()); + base_request.setParameters(j_post); + } } + else + session.removeAttribute(__J_URI); } - else - session.removeAttribute(__J_URI); - } + LOG.debug("auth {}",authentication); return authentication; } } // if we can't send challenge if (DeferredAuthentication.isDeferred(response)) + { + LOG.debug("auth deferred {}",session.getId()); return Authentication.UNAUTHENTICATED; + } // remember the current URI synchronized (session) @@ -320,7 +338,7 @@ public class FormAuthenticator extends LoginAuthenticator { Request base_request = (req instanceof Request)?(Request)req:HttpChannel.getCurrentHttpChannel().getRequest(); base_request.extractParameters(); - session.setAttribute(__J_POST, new MultiMap(base_request.getParameters())); + session.setAttribute(__J_POST, new MultiMap<String>(base_request.getParameters())); } } } @@ -328,6 +346,7 @@ public class FormAuthenticator extends LoginAuthenticator // send the the challenge if (_dispatch) { + LOG.debug("challenge {}=={}",session.getId(),_formLoginPage); RequestDispatcher dispatcher = request.getRequestDispatcher(_formLoginPage); response.setHeader(HttpHeader.CACHE_CONTROL.asString(),HttpHeaderValue.NO_CACHE.asString()); response.setDateHeader(HttpHeader.EXPIRES.asString(),1); @@ -335,11 +354,10 @@ public class FormAuthenticator extends LoginAuthenticator } else { + LOG.debug("challenge {}->{}",session.getId(),_formLoginPage); response.sendRedirect(response.encodeRedirectURL(URIUtil.addPaths(request.getContextPath(),_formLoginPage))); } return Authentication.SEND_CONTINUE; - - } catch (IOException | ServletException e) { diff --git a/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/LoginAuthenticator.java b/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/LoginAuthenticator.java index f7e30647a9..a1f0501f78 100644 --- a/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/LoginAuthenticator.java +++ b/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/LoginAuthenticator.java @@ -26,9 +26,15 @@ import org.eclipse.jetty.security.Authenticator; import org.eclipse.jetty.security.IdentityService; import org.eclipse.jetty.security.LoginService; import org.eclipse.jetty.server.session.AbstractSessionManager; +import org.eclipse.jetty.server.session.HashSessionManager; +import org.eclipse.jetty.server.session.HashedSession; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; public abstract class LoginAuthenticator implements Authenticator -{ +{ + private static final Logger LOG = Log.getLogger(LoginAuthenticator.class); + protected final DeferredAuthentication _deferred=new DeferredAuthentication(this); protected LoginService _loginService; protected IdentityService _identityService; @@ -67,16 +73,20 @@ public abstract class LoginAuthenticator implements Authenticator protected HttpSession renewSession(HttpServletRequest request, HttpServletResponse response) { HttpSession httpSession = request.getSession(false); - - //if we should renew sessions, and there is an existing session that may have been seen by non-authenticated users - //(indicated by SESSION_SECURED not being set on the session) then we should change id - if (_renewSession && httpSession!=null && httpSession.getAttribute(AbstractSessionManager.SESSION_KNOWN_ONLY_TO_AUTHENTICATED)!=Boolean.TRUE) + + synchronized (httpSession) { - synchronized (this) + //if we should renew sessions, and there is an existing session that may have been seen by non-authenticated users + //(indicated by SESSION_SECURED not being set on the session) then we should change id + if (_renewSession && httpSession!=null && httpSession.getAttribute(AbstractSessionManager.SESSION_KNOWN_ONLY_TO_AUTHENTICATED)!=Boolean.TRUE) { - httpSession = AbstractSessionManager.renewSession(request, httpSession,true); + System.err.println(((HashedSession)httpSession).getAttributeMap()); + HttpSession newSession = AbstractSessionManager.renewSession(request, httpSession,true); + LOG.debug("renew {}->{}",httpSession.getId(),newSession.getId()); + System.err.println(((HashedSession)newSession).getAttributeMap()); + httpSession=newSession; } + return httpSession; } - return httpSession; } } diff --git a/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/SessionAuthentication.java b/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/SessionAuthentication.java index 74f143b05f..4aa063f8a6 100644 --- a/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/SessionAuthentication.java +++ b/jetty-security/src/main/java/org/eclipse/jetty/security/authentication/SessionAuthentication.java @@ -114,23 +114,27 @@ public class SessionAuthentication implements Authentication.User, Serializable, @Override public String toString() { - return "Session"+super.toString(); + return String.format("%s@%x{%s,%s}",this.getClass().getSimpleName(),hashCode(),_session==null?"-":_session.getId(),_userIdentity); } + @Override public void sessionWillPassivate(HttpSessionEvent se) { } + @Override public void sessionDidActivate(HttpSessionEvent se) { if (_session==null) _session=se.getSession(); } + @Override public void valueBound(HttpSessionBindingEvent event) { } + @Override public void valueUnbound(HttpSessionBindingEvent event) { doLogout(); 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 019b8736e2..11eabb3779 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 @@ -34,6 +34,7 @@ import javax.servlet.http.HttpServletResponse; import org.eclipse.jetty.security.authentication.BasicAuthenticator; import org.eclipse.jetty.security.authentication.FormAuthenticator; +import org.eclipse.jetty.security.authentication.LoginAuthenticator; import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.Request; @@ -44,6 +45,7 @@ import org.eclipse.jetty.server.handler.ContextHandler; import org.eclipse.jetty.server.handler.HandlerWrapper; import org.eclipse.jetty.server.session.SessionHandler; import org.eclipse.jetty.util.B64Code; +import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.security.Constraint; import org.eclipse.jetty.util.security.Password; import org.junit.After; @@ -303,6 +305,10 @@ public class ConstraintTest @Test public void testFormRedirect() throws Exception { + Log.getLogger(SecurityHandler.class).setDebugEnabled(true); + Log.getLogger(LoginAuthenticator.class).setDebugEnabled(true); + Log.getLogger(FormAuthenticator.class).setDebugEnabled(true); + _security.setAuthenticator(new FormAuthenticator("/testLoginPage","/testErrorPage",false)); _security.setStrict(false); _server.start(); @@ -326,6 +332,7 @@ public class ConstraintTest assertThat(response,containsString(" 200 OK")); assertThat(response,containsString("URI=/ctx/testLoginPage")); + System.err.println("-- wrong password"); response = _connector.getResponses("POST /ctx/j_security_check HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + "Content-Type: application/x-www-form-urlencoded\r\n" + @@ -334,6 +341,7 @@ public class ConstraintTest "j_username=user&j_password=wrong"); assertThat(response,containsString("Location")); + System.err.println("-- right password"); response = _connector.getResponses("POST /ctx/j_security_check HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + "Content-Type: application/x-www-form-urlencoded\r\n" + @@ -345,9 +353,11 @@ public class ConstraintTest assertThat(response,containsString("/ctx/auth/info")); session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + System.err.println("--"); response = _connector.getResponses("GET /ctx/auth/info HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + "\r\n"); + System.err.println("=="); assertThat(response,startsWith("HTTP/1.1 200 OK")); response = _connector.getResponses("GET /ctx/admin/info HTTP/1.0\r\n" + |