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

jetty-9 removed redirect race from form auth

This commit is contained in:
Greg Wilkins 2012-08-20 09:22:03 +10:00
parent 2900b4df02
commit 4ff7ae6106
5 changed files with 108 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -43,6 +43,7 @@ import org.eclipse.jetty.server.Authentication.User;
import org.eclipse.jetty.server.HttpChannel; import org.eclipse.jetty.server.HttpChannel;
import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.UserIdentity; import org.eclipse.jetty.server.UserIdentity;
import org.eclipse.jetty.server.session.HashedSession;
import org.eclipse.jetty.util.MultiMap; import org.eclipse.jetty.util.MultiMap;
import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.URIUtil; import org.eclipse.jetty.util.URIUtil;
@ -200,6 +201,7 @@ public class FormAuthenticator extends LoginAuthenticator
return _deferred; return _deferred;
HttpSession session = request.getSession(true); HttpSession session = request.getSession(true);
System.err.println(session.getId()+((HashedSession)session).getAttributeMap());
try try
{ {
@ -208,31 +210,36 @@ public class FormAuthenticator extends LoginAuthenticator
{ {
final String username = request.getParameter(__J_USERNAME); final String username = request.getParameter(__J_USERNAME);
final String password = request.getParameter(__J_PASSWORD); final String password = request.getParameter(__J_PASSWORD);
UserIdentity user = _loginService.login(username,password); UserIdentity user = _loginService.login(username,password);
LOG.debug("jsecuritycheck {} {}",username,user);
if (user!=null) if (user!=null)
{ {
session=renewSession(request,response); session=renewSession(request,response);
// Redirect to original request // Redirect to original request
String nuri; String nuri;
FormAuthentication form_auth;
synchronized(session) synchronized(session)
{ {
nuri = (String) session.getAttribute(__J_URI); nuri = (String) session.getAttribute(__J_URI);
}
if (nuri == null || nuri.length() == 0) if (nuri == null || nuri.length() == 0)
{ {
nuri = request.getContextPath(); nuri = request.getContextPath();
if (nuri.length() == 0) if (nuri.length() == 0)
nuri = URIUtil.SLASH; 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.setContentLength(0);
response.sendRedirect(response.encodeRedirectURL(nuri)); response.sendRedirect(response.encodeRedirectURL(nuri));
return form_auth;
Authentication cached=new SessionAuthentication(getAuthMethod(),user,password);
session.setAttribute(SessionAuthentication.__J_AUTHENTICATED, cached);
return new FormAuthentication(getAuthMethod(),user);
} }
// not authenticated // not authenticated
@ -240,11 +247,13 @@ public class FormAuthenticator extends LoginAuthenticator
LOG.debug("Form authentication FAILED for " + StringUtil.printable(username)); LOG.debug("Form authentication FAILED for " + StringUtil.printable(username));
if (_formErrorPage == null) if (_formErrorPage == null)
{ {
LOG.debug("auth failed {}->403",username);
if (response != null) if (response != null)
response.sendError(HttpServletResponse.SC_FORBIDDEN); response.sendError(HttpServletResponse.SC_FORBIDDEN);
} }
else if (_dispatch) else if (_dispatch)
{ {
LOG.debug("auth failed {}=={}",username,_formErrorPage);
RequestDispatcher dispatcher = request.getRequestDispatcher(_formErrorPage); RequestDispatcher dispatcher = request.getRequestDispatcher(_formErrorPage);
response.setHeader(HttpHeader.CACHE_CONTROL.asString(),HttpHeaderValue.NO_CACHE.asString()); response.setHeader(HttpHeader.CACHE_CONTROL.asString(),HttpHeaderValue.NO_CACHE.asString());
response.setDateHeader(HttpHeader.EXPIRES.asString(),1); response.setDateHeader(HttpHeader.EXPIRES.asString(),1);
@ -252,6 +261,7 @@ public class FormAuthenticator extends LoginAuthenticator
} }
else else
{ {
LOG.debug("auth failed {}->{}",username,_formErrorPage);
response.sendRedirect(response.encodeRedirectURL(URIUtil.addPaths(request.getContextPath(),_formErrorPage))); response.sendRedirect(response.encodeRedirectURL(URIUtil.addPaths(request.getContextPath(),_formErrorPage)));
} }
@ -267,43 +277,51 @@ public class FormAuthenticator extends LoginAuthenticator
_loginService!=null && _loginService!=null &&
!_loginService.validate(((Authentication.User)authentication).getUserIdentity())) !_loginService.validate(((Authentication.User)authentication).getUserIdentity()))
{ {
LOG.debug("auth revoked {}",authentication);
session.removeAttribute(SessionAuthentication.__J_AUTHENTICATED); session.removeAttribute(SessionAuthentication.__J_AUTHENTICATED);
} }
else else
{ {
String j_uri=(String)session.getAttribute(__J_URI); synchronized (session)
if (j_uri!=null)
{ {
MultiMap j_post = (MultiMap)session.getAttribute(__J_POST); String j_uri=(String)session.getAttribute(__J_URI);
if (j_post!=null) if (j_uri!=null)
{ {
StringBuffer buf = request.getRequestURL(); LOG.debug("auth retry {}->{}",authentication,j_uri);
if (request.getQueryString() != null) MultiMap<String> j_post = (MultiMap<String>)session.getAttribute(__J_POST);
buf.append("?").append(request.getQueryString()); if (j_post!=null)
if (j_uri.equals(buf.toString()))
{ {
// This is a retry of an original POST request LOG.debug("auth rePOST {}->{}",authentication,j_uri);
// so restore method and parameters StringBuffer buf = request.getRequestURL();
if (request.getQueryString() != null)
buf.append("?").append(request.getQueryString());
session.removeAttribute(__J_POST); if (j_uri.equals(buf.toString()))
Request base_request = HttpChannel.getCurrentHttpChannel().getRequest(); {
base_request.setMethod(HttpMethod.POST,HttpMethod.POST.asString()); // This is a retry of an original POST request
base_request.setParameters(j_post); // 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; return authentication;
} }
} }
// if we can't send challenge // if we can't send challenge
if (DeferredAuthentication.isDeferred(response)) if (DeferredAuthentication.isDeferred(response))
{
LOG.debug("auth deferred {}",session.getId());
return Authentication.UNAUTHENTICATED; return Authentication.UNAUTHENTICATED;
}
// remember the current URI // remember the current URI
synchronized (session) synchronized (session)
@ -320,7 +338,7 @@ public class FormAuthenticator extends LoginAuthenticator
{ {
Request base_request = (req instanceof Request)?(Request)req:HttpChannel.getCurrentHttpChannel().getRequest(); Request base_request = (req instanceof Request)?(Request)req:HttpChannel.getCurrentHttpChannel().getRequest();
base_request.extractParameters(); 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 // send the the challenge
if (_dispatch) if (_dispatch)
{ {
LOG.debug("challenge {}=={}",session.getId(),_formLoginPage);
RequestDispatcher dispatcher = request.getRequestDispatcher(_formLoginPage); RequestDispatcher dispatcher = request.getRequestDispatcher(_formLoginPage);
response.setHeader(HttpHeader.CACHE_CONTROL.asString(),HttpHeaderValue.NO_CACHE.asString()); response.setHeader(HttpHeader.CACHE_CONTROL.asString(),HttpHeaderValue.NO_CACHE.asString());
response.setDateHeader(HttpHeader.EXPIRES.asString(),1); response.setDateHeader(HttpHeader.EXPIRES.asString(),1);
@ -335,11 +354,10 @@ public class FormAuthenticator extends LoginAuthenticator
} }
else else
{ {
LOG.debug("challenge {}->{}",session.getId(),_formLoginPage);
response.sendRedirect(response.encodeRedirectURL(URIUtil.addPaths(request.getContextPath(),_formLoginPage))); response.sendRedirect(response.encodeRedirectURL(URIUtil.addPaths(request.getContextPath(),_formLoginPage)));
} }
return Authentication.SEND_CONTINUE; return Authentication.SEND_CONTINUE;
} }
catch (IOException | ServletException e) catch (IOException | ServletException e)
{ {

26
jetty-security/src/main/java/org/eclipse/jetty/security/authentication/LoginAuthenticator.java
View File

@ -26,9 +26,15 @@ import org.eclipse.jetty.security.Authenticator;
import org.eclipse.jetty.security.IdentityService; import org.eclipse.jetty.security.IdentityService;
import org.eclipse.jetty.security.LoginService; import org.eclipse.jetty.security.LoginService;
import org.eclipse.jetty.server.session.AbstractSessionManager; 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 public abstract class LoginAuthenticator implements Authenticator
{ {
private static final Logger LOG = Log.getLogger(LoginAuthenticator.class);
protected final DeferredAuthentication _deferred=new DeferredAuthentication(this); protected final DeferredAuthentication _deferred=new DeferredAuthentication(this);
protected LoginService _loginService; protected LoginService _loginService;
protected IdentityService _identityService; protected IdentityService _identityService;
@ -67,16 +73,20 @@ public abstract class LoginAuthenticator implements Authenticator
protected HttpSession renewSession(HttpServletRequest request, HttpServletResponse response) protected HttpSession renewSession(HttpServletRequest request, HttpServletResponse response)
{ {
HttpSession httpSession = request.getSession(false); 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 synchronized (httpSession)
//(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 (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;
} }
} }

6
jetty-security/src/main/java/org/eclipse/jetty/security/authentication/SessionAuthentication.java
View File

@ -114,23 +114,27 @@ public class SessionAuthentication implements Authentication.User, Serializable,
@Override @Override
public String toString() 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) public void sessionWillPassivate(HttpSessionEvent se)
{ {
} }
@Override
public void sessionDidActivate(HttpSessionEvent se) public void sessionDidActivate(HttpSessionEvent se)
{ {
if (_session==null) if (_session==null)
_session=se.getSession(); _session=se.getSession();
} }
@Override
public void valueBound(HttpSessionBindingEvent event) public void valueBound(HttpSessionBindingEvent event)
{ {
} }
@Override
public void valueUnbound(HttpSessionBindingEvent event) public void valueUnbound(HttpSessionBindingEvent event)
{ {
doLogout(); doLogout();

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

@ -34,6 +34,7 @@ import javax.servlet.http.HttpServletResponse;
import org.eclipse.jetty.security.authentication.BasicAuthenticator; import org.eclipse.jetty.security.authentication.BasicAuthenticator;
import org.eclipse.jetty.security.authentication.FormAuthenticator; 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.Connector;
import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.LocalConnector;
import org.eclipse.jetty.server.Request; 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.handler.HandlerWrapper;
import org.eclipse.jetty.server.session.SessionHandler; import org.eclipse.jetty.server.session.SessionHandler;
import org.eclipse.jetty.util.B64Code; 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.Constraint;
import org.eclipse.jetty.util.security.Password; import org.eclipse.jetty.util.security.Password;
import org.junit.After; import org.junit.After;
@ -303,6 +305,10 @@ public class ConstraintTest
@Test @Test
public void testFormRedirect() throws Exception 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.setAuthenticator(new FormAuthenticator("/testLoginPage","/testErrorPage",false));
_security.setStrict(false); _security.setStrict(false);
_server.start(); _server.start();
@ -326,6 +332,7 @@ public class ConstraintTest
assertThat(response,containsString(" 200 OK")); assertThat(response,containsString(" 200 OK"));
assertThat(response,containsString("URI=/ctx/testLoginPage")); assertThat(response,containsString("URI=/ctx/testLoginPage"));
System.err.println("-- wrong password");
response = _connector.getResponses("POST /ctx/j_security_check HTTP/1.0\r\n" + response = _connector.getResponses("POST /ctx/j_security_check HTTP/1.0\r\n" +
"Cookie: JSESSIONID=" + session + "\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" +
"Content-Type: application/x-www-form-urlencoded\r\n" + "Content-Type: application/x-www-form-urlencoded\r\n" +
@ -334,6 +341,7 @@ public class ConstraintTest
"j_username=user&j_password=wrong"); "j_username=user&j_password=wrong");
assertThat(response,containsString("Location")); assertThat(response,containsString("Location"));
System.err.println("-- right password");
response = _connector.getResponses("POST /ctx/j_security_check HTTP/1.0\r\n" + response = _connector.getResponses("POST /ctx/j_security_check HTTP/1.0\r\n" +
"Cookie: JSESSIONID=" + session + "\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" +
"Content-Type: application/x-www-form-urlencoded\r\n" + "Content-Type: application/x-www-form-urlencoded\r\n" +
@ -345,9 +353,11 @@ public class ConstraintTest
assertThat(response,containsString("/ctx/auth/info")); assertThat(response,containsString("/ctx/auth/info"));
session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); 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" + response = _connector.getResponses("GET /ctx/auth/info HTTP/1.0\r\n" +
"Cookie: JSESSIONID=" + session + "\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" +
"\r\n"); "\r\n");
System.err.println("==");
assertThat(response,startsWith("HTTP/1.1 200 OK")); assertThat(response,startsWith("HTTP/1.1 200 OK"));
response = _connector.getResponses("GET /ctx/admin/info HTTP/1.0\r\n" + response = _connector.getResponses("GET /ctx/admin/info HTTP/1.0\r\n" +

24
jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSession.java
View File

@ -111,6 +111,7 @@ public abstract class AbstractSession implements AbstractSessionManager.SessionI
} }
/* ------------------------------------------------------------- */ /* ------------------------------------------------------------- */
@Override
public AbstractSession getSession() public AbstractSession getSession()
{ {
return this; return this;
@ -124,8 +125,15 @@ public abstract class AbstractSession implements AbstractSessionManager.SessionI
return _accessed; return _accessed;
} }
} }
/* ------------------------------------------------------------- */
public Map<String,Object> getAttributeMap()
{
return _attributes;
}
/* ------------------------------------------------------------ */ /* ------------------------------------------------------------ */
@Override
public Object getAttribute(String name) public Object getAttribute(String name)
{ {
synchronized (this) synchronized (this)
@ -147,6 +155,7 @@ public abstract class AbstractSession implements AbstractSessionManager.SessionI
/* ------------------------------------------------------------ */ /* ------------------------------------------------------------ */
@SuppressWarnings({ "unchecked" }) @SuppressWarnings({ "unchecked" })
@Override
public Enumeration<String> getAttributeNames() public Enumeration<String> getAttributeNames()
{ {
synchronized (this) synchronized (this)
@ -173,12 +182,14 @@ public abstract class AbstractSession implements AbstractSessionManager.SessionI
} }
/* ------------------------------------------------------------- */ /* ------------------------------------------------------------- */
@Override
public long getCreationTime() throws IllegalStateException public long getCreationTime() throws IllegalStateException
{ {
return _created; return _created;
} }
/* ------------------------------------------------------------ */ /* ------------------------------------------------------------ */
@Override
public String getId() throws IllegalStateException public String getId() throws IllegalStateException
{ {
return _manager._nodeIdInSessionId?_nodeId:_clusterId; return _manager._nodeIdInSessionId?_nodeId:_clusterId;
@ -197,6 +208,7 @@ public abstract class AbstractSession implements AbstractSessionManager.SessionI
} }
/* ------------------------------------------------------------- */ /* ------------------------------------------------------------- */
@Override
public long getLastAccessedTime() throws IllegalStateException public long getLastAccessedTime() throws IllegalStateException
{ {
checkValid(); checkValid();
@ -204,6 +216,7 @@ public abstract class AbstractSession implements AbstractSessionManager.SessionI
} }
/* ------------------------------------------------------------- */ /* ------------------------------------------------------------- */
@Override
public int getMaxInactiveInterval() public int getMaxInactiveInterval()
{ {
checkValid(); checkValid();
@ -214,6 +227,7 @@ public abstract class AbstractSession implements AbstractSessionManager.SessionI
/* /*
* @see javax.servlet.http.HttpSession#getServletContext() * @see javax.servlet.http.HttpSession#getServletContext()
*/ */
@Override
public ServletContext getServletContext() public ServletContext getServletContext()
{ {
return _manager._context; return _manager._context;
@ -221,6 +235,7 @@ public abstract class AbstractSession implements AbstractSessionManager.SessionI
/* ------------------------------------------------------------- */ /* ------------------------------------------------------------- */
@Deprecated @Deprecated
@Override
public HttpSessionContext getSessionContext() throws IllegalStateException public HttpSessionContext getSessionContext() throws IllegalStateException
{ {
checkValid(); checkValid();
@ -233,6 +248,7 @@ public abstract class AbstractSession implements AbstractSessionManager.SessionI
* {@link #getAttribute} * {@link #getAttribute}
*/ */
@Deprecated @Deprecated
@Override
public Object getValue(String name) throws IllegalStateException public Object getValue(String name) throws IllegalStateException
{ {
return getAttribute(name); return getAttribute(name);
@ -244,6 +260,7 @@ public abstract class AbstractSession implements AbstractSessionManager.SessionI
* {@link #getAttributeNames} * {@link #getAttributeNames}
*/ */
@Deprecated @Deprecated
@Override
public String[] getValueNames() throws IllegalStateException public String[] getValueNames() throws IllegalStateException
{ {
synchronized(this) synchronized(this)
@ -309,6 +326,7 @@ public abstract class AbstractSession implements AbstractSessionManager.SessionI
} }
/* ------------------------------------------------------------- */ /* ------------------------------------------------------------- */
@Override
public void invalidate() throws IllegalStateException public void invalidate() throws IllegalStateException
{ {
// remove session from context and invalidate other sessions with same ID. // remove session from context and invalidate other sessions with same ID.
@ -372,6 +390,7 @@ public abstract class AbstractSession implements AbstractSessionManager.SessionI
} }
/* ------------------------------------------------------------- */ /* ------------------------------------------------------------- */
@Override
public boolean isNew() throws IllegalStateException public boolean isNew() throws IllegalStateException
{ {
checkValid(); checkValid();
@ -384,12 +403,14 @@ public abstract class AbstractSession implements AbstractSessionManager.SessionI
* {@link #setAttribute} * {@link #setAttribute}
*/ */
@Deprecated @Deprecated
@Override
public void putValue(java.lang.String name, java.lang.Object value) throws IllegalStateException public void putValue(java.lang.String name, java.lang.Object value) throws IllegalStateException
{ {
setAttribute(name,value); setAttribute(name,value);
} }
/* ------------------------------------------------------------ */ /* ------------------------------------------------------------ */
@Override
public void removeAttribute(String name) public void removeAttribute(String name)
{ {
setAttribute(name,null); setAttribute(name,null);
@ -401,6 +422,7 @@ public abstract class AbstractSession implements AbstractSessionManager.SessionI
* {@link #removeAttribute} * {@link #removeAttribute}
*/ */
@Deprecated @Deprecated
@Override
public void removeValue(java.lang.String name) throws IllegalStateException public void removeValue(java.lang.String name) throws IllegalStateException
{ {
removeAttribute(name); removeAttribute(name);
@ -419,6 +441,7 @@ public abstract class AbstractSession implements AbstractSessionManager.SessionI
} }
/* ------------------------------------------------------------ */ /* ------------------------------------------------------------ */
@Override
public void setAttribute(String name, Object value) public void setAttribute(String name, Object value)
{ {
Object old=null; Object old=null;
@ -447,6 +470,7 @@ public abstract class AbstractSession implements AbstractSessionManager.SessionI
} }
/* ------------------------------------------------------------- */ /* ------------------------------------------------------------- */
@Override
public void setMaxInactiveInterval(int secs) public void setMaxInactiveInterval(int secs)
{ {
_maxIdleMs=(long)secs*1000L; _maxIdleMs=(long)secs*1000L;