Skip to content

Commit

Permalink
Jetty 12.1.x 12088 core requested session ID source (#12145)
Browse files Browse the repository at this point in the history 
add isRequestedSessionIdFromCookie/URL for core request

Co-authored-by: Jan Bartel <janb@webtide.com>
  • Loading branch information
@gregw @janbartel
gregw and janbartel authored Aug 20, 2024
1 parent e2753e6 commit 8b4e13d
Show file tree
Hide file tree
Showing 17 changed files with 205 additions and 95 deletions.
2 changes: 1 addition & 1 deletion ...examples/src/main/java/org/eclipse/jetty/docs/programming/server/session/SessionDocs.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public void coreSessionHandler()
org.eclipse.jetty.session.SessionHandler sessionHandler = new org.eclipse.jetty.session.SessionHandler();
sessionHandler.setSessionCookie("SIMPLE");
sessionHandler.setUsingCookies(true);
sessionHandler.setUsingURLs(false);
sessionHandler.setUsingUriParameters(false);
sessionHandler.setSessionPath("/");
server.setHandler(sessionHandler);
sessionHandler.setHandler(new Handler.Abstract()
Expand Down
10 changes: 4 additions & 6 deletions ...ore/jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -444,14 +444,12 @@ public AuthenticationState validateRequest(Request request, Response response, C
return AuthenticationState.SEND_FAILURE;
}

// TODO: No session API to work this out?
/*
if (request.isRequestedSessionIdFromURL())
String sessionIdFrom = (String)request.getAttribute("org.eclipse.jetty.session.RequestedSession.sessionIdFrom");
if (sessionIdFrom != null && !sessionIdFrom.startsWith("cookie"))
{
sendError(req, res, cb, "Session ID must be a cookie to support OpenID authentication");
return Authentication.SEND_FAILURE;
sendError(request, response, cb, "Session ID must be a cookie to support OpenID authentication");
return AuthenticationState.SEND_FAILURE;
}
*/

// Handle a request for authentication.
if (isJSecurityCheck(uri))
Expand Down
1 change: 0 additions & 1 deletion ...etty-security/src/test/java/org/eclipse/jetty/security/jaas/JAASLdapLoginServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -396,5 +396,4 @@ public Session getSession(boolean create)
return null;
}
}

}
126 changes: 93 additions & 33 deletions jetty-core/jetty-session/src/main/java/org/eclipse/jetty/session/AbstractSessionManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.Session;
import org.eclipse.jetty.server.handler.ContextHandler;
import org.eclipse.jetty.util.Attributes;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.URIUtil;
Expand Down Expand Up @@ -981,24 +982,6 @@ public void setUsingUriParameters(boolean usingUriParameters)
_usingUriParameters = usingUriParameters;
}

/**
* @deprecated use {@link #isUsingUriParameters()} instead, will be removed in Jetty 12.1.0
*/
@Deprecated(since = "12.0.1", forRemoval = true)
public boolean isUsingURLs()
{
return isUsingUriParameters();
}

/**
* @deprecated use {@link #setUsingUriParameters(boolean)} instead, will be removed in Jetty 12.1.0
*/
@Deprecated(since = "12.0.1", forRemoval = true)
public void setUsingURLs(boolean usingURLs)
{
setUsingUriParameters(usingURLs);
}

/**
* Create a new Session, using the requested session id if possible.
* @param request the inbound request
Expand Down Expand Up @@ -1229,7 +1212,7 @@ protected RequestedSession resolveRequestedSessionId(Request request)
{
//Cookie[] cookies = request.getCookies();
List<HttpCookie> cookies = Request.getCookies(request);
if (cookies != null && cookies.size() > 0)
if (!cookies.isEmpty())
{
final String sessionCookie = getSessionCookie();
for (HttpCookie cookie : cookies)
Expand Down Expand Up @@ -1279,7 +1262,7 @@ protected RequestedSession resolveRequestedSessionId(Request request)
}

if (ids == null)
return NO_REQUESTED_SESSION;
return RequestedSession.NO_REQUESTED_SESSION;

if (LOG.isDebugEnabled())
LOG.debug("Got Session IDs {} from cookies {}", ids, cookieIds);
Expand Down Expand Up @@ -1319,8 +1302,7 @@ else if (session.getId().equals(getSessionIdManager().getId(id)))
{
//we already have a valid session and now have a duplicate ID for it
if (LOG.isDebugEnabled())
LOG.debug(duplicateSession(
requestedSessionId, true, requestedSessionIdFromCookie,
LOG.debug(duplicateSession(requestedSessionId, requestedSessionIdFromCookie,
id, false, i < cookieIds));
}
else
Expand Down Expand Up @@ -1350,26 +1332,27 @@ else if (session.getId().equals(getSessionIdManager().getId(id)))
}

throw new BadMessageException(duplicateSession(
requestedSessionId, true, requestedSessionIdFromCookie,
requestedSessionId, requestedSessionIdFromCookie,
id, true, i < cookieIds));
}
else if (LOG.isDebugEnabled())
{
LOG.debug(duplicateSession(
requestedSessionId, true, requestedSessionIdFromCookie,
requestedSessionId, requestedSessionIdFromCookie,
id, false, i < cookieIds));
}
}
}

return new RequestedSession((session != null && session.isValid()) ? session : null, requestedSessionId, requestedSessionIdFromCookie);
return new RequestedSession((session != null && session.isValid()) ? session : null, requestedSessionId,
requestedSessionIdFromCookie ? RequestedSession.ID_FROM_COOKIE : RequestedSession.ID_FROM_URI_PARAMETER);
}

private static String duplicateSession(String id0, boolean valid0, boolean cookie0, String id1, boolean valid1, boolean cookie1)
private static String duplicateSession(String id0, boolean fromCookie0, String id1, boolean valid1, boolean fromCookie1)
{
return "Duplicate sessions: %s[%s,%s] & %s[%s,%s]".formatted(
id0, valid0 ? "valid" : "unknown", cookie0 ? "cookie" : "param",
id1, valid1 ? "valid" : "unknown", cookie1 ? "cookie" : "param");
id0, "valid", fromCookie0 ? RequestedSession.ID_FROM_COOKIE : RequestedSession.ID_FROM_URI_PARAMETER,
id1, valid1 ? "valid" : "unknown", fromCookie1 ? RequestedSession.ID_FROM_COOKIE : RequestedSession.ID_FROM_URI_PARAMETER);
}

/**
Expand All @@ -1379,12 +1362,89 @@ private void shutdownSessions()
{
_sessionCache.shutdown();
}

public record RequestedSession(ManagedSession session, String sessionId, boolean sessionIdFromCookie)
{
}

private static final RequestedSession NO_REQUESTED_SESSION = new RequestedSession(null, null, false);
/**
* Details of the requested session.
* Session implementations should make an instance of this record available as a hidden (not in name set) request
* attribute for the name "org.eclipse.jetty.session.AbstractSessionManager$RequestedSession"
* @param session The {@link Session} associated with the ID, which may have been invalidated or changed ID since the
* request was received; or {@code null} if no session existed matching the requested ID.
* @param sessionId The requested session ID.
* @param sessionIdFrom A {@link String} representing the source of the session ID. Common values include:
* {@link #ID_FROM_COOKIE} or {@link #ID_FROM_URI_PARAMETER} if there is no ID.
*/
public record RequestedSession(ManagedSession session, String sessionId, String sessionIdFrom)
{
public static final RequestedSession NO_REQUESTED_SESSION = new RequestedSession(null, null, null);
public static final String ATTRIBUTE = "org.eclipse.jetty.session.RequestedSession";
public static final String ID_FROM_COOKIE = "cookie";
public static final String ID_FROM_URI_PARAMETER = "uri";

/**
* Get the {@code RequestedSession} by attribute
* @param request The attributes to query
* @return The found {@code RequestedSession} or {@link #NO_REQUESTED_SESSION} if none found. Never {@code null}.
*/
public static RequestedSession byAttribute(Attributes request)
{
RequestedSession requestedSession = (RequestedSession)request.getAttribute(ATTRIBUTE);
return requestedSession == null ? NO_REQUESTED_SESSION : requestedSession;
}

/**
* @param name An attribute name
* @return {@code true} if the attribute name is applicable to a requested session.
* @see #getAttribute(String)
*/
public static boolean isApplicableAttribute(String name)
{
return name != null && name.startsWith(ATTRIBUTE);
}

/**
* Get attributes asssociated with this requested session:
* <ul>
* <li>`org.eclipse.jetty.session.RequestedSession` this instance.</li>
* <li>`org.eclipse.jetty.session.RequestedSession.session` the {@link #session()}.</li>
* <li>`org.eclipse.jetty.session.RequestedSession.sessionId` the {@link #sessionId()}.</li>
* <li>`org.eclipse.jetty.session.RequestedSession.sessionIdFrom` the {@link #sessionIdFrom()}.</li>
* </ul>
* @param name An attributed name
* @return the attribute value or {@code null}
*/
public Object getAttribute(String name)
{
if (name == null || name.length() < ATTRIBUTE.length())
return null;

if (ATTRIBUTE.equals(name))
return this;

if (name.startsWith(ATTRIBUTE) && name.charAt(ATTRIBUTE.length()) == '.')
{
return switch (name.substring(ATTRIBUTE.length() + 1))
{
case "session" -> session();
case "sessionId" -> sessionId();
case "sessionIdFrom" -> sessionIdFrom();
default -> null;
};
}

return null;
}

/**
* Test if this {@code RequestedSession} ID is from a particular session source
* @param source A {@link String} representing the source of the session ID. Common values include:
* {@link #ID_FROM_COOKIE} or {@link #ID_FROM_URI_PARAMETER} if there is no ID.
* @return {@code True} iff this {@code RequestedSession} ID is from the source.
*/
public boolean isSessionIdFrom(String source)
{
return source != null && source.equals(sessionIdFrom);
}
}

/**
* A session cookie is marked as secure IFF any of the following conditions are true:
Expand Down
20 changes: 13 additions & 7 deletions jetty-core/jetty-session/src/main/java/org/eclipse/jetty/session/SessionHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,10 @@ public Session.API newSessionAPIWrapper(ManagedSession session)
return null;
}

private class SessionRequest extends Request.Wrapper
public class SessionRequest extends Request.Wrapper
{
private final AtomicReference<ManagedSession> _session = new AtomicReference<>();
private String _requestedSessionId;
RequestedSession _requestedSession;
private Response _response;

public SessionRequest(Request request)
Expand All @@ -103,6 +103,14 @@ ManagedSession getManagedSession()
return _session.get();
}

@Override
public Object getAttribute(String name)
{
if (RequestedSession.isApplicableAttribute(name))
return _requestedSession.getAttribute(name);
return super.getAttribute(name);
}

@Override
public Session getSession(boolean create)
{
Expand All @@ -113,7 +121,7 @@ public Session getSession(boolean create)

if (session == null && create)
{
newSession(this, _requestedSessionId, this::setManagedSession);
newSession(this, _requestedSession.sessionId(), this::setManagedSession);
session = _session.get();
HttpCookie cookie = getSessionCookie(session, getConnectionMetaData().isSecure());
if (cookie != null)
Expand All @@ -126,10 +134,8 @@ public Session getSession(boolean create)
public boolean process(Handler handler, Response response, Callback callback) throws Exception
{
_response = response;

RequestedSession requestedSession = resolveRequestedSessionId(this);
_requestedSessionId = requestedSession.sessionId();
ManagedSession session = requestedSession.session();
_requestedSession = resolveRequestedSessionId(this);
ManagedSession session = _requestedSession.session();

if (session != null)
{
Expand Down
20 changes: 19 additions & 1 deletion jetty-core/jetty-session/src/test/java/org/eclipse/jetty/session/SessionHandlerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.eclipse.jetty.server.Response;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.Session;
import org.eclipse.jetty.session.AbstractSessionManager.RequestedSession;
import org.eclipse.jetty.util.Callback;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
Expand Down Expand Up @@ -148,8 +149,21 @@ public boolean handle(Request request, Response response, Callback callback)
{
if (session.isNew())
out.append("New\n");

RequestedSession requestedSession = RequestedSession.byAttribute(request);

out.append("RequestedSessionIdFromCookie: ")
.append(requestedSession.isSessionIdFrom(RequestedSession.ID_FROM_COOKIE))
.append('\n');
out.append("RequestedSessionIdFromURL: ")
.append(requestedSession.isSessionIdFrom(RequestedSession.ID_FROM_URI_PARAMETER))
.append('\n');
for (String name : session.getAttributeNameSet())
out.append("Attribute ").append(name).append(" = ").append(session.getAttribute(name)).append('\n');
out.append("Attribute ")
.append(name)
.append(" = ")
.append(session.getAttribute(name))
.append('\n');
out.append("URI [")
.append(session.encodeURI(request, "/some/path", request.getHeaders().contains(HttpHeader.COOKIE)))
.append("]");
Expand Down Expand Up @@ -499,6 +513,8 @@ public void testCookieAndURI() throws Exception
assertThat(response.getStatus(), equalTo(200));
content = response.getContent();
assertThat(content, containsString("Session=" + id.substring(0, id.indexOf(".node0"))));
assertThat(content, containsString("RequestedSessionIdFromCookie: true"));
assertThat(content, containsString("RequestedSessionIdFromURL: false"));
assertThat(content, containsString("URI [/some/path]")); // Cookies known to be in use

// Get with parameter
Expand All @@ -513,6 +529,8 @@ public void testCookieAndURI() throws Exception
assertThat(response.getStatus(), equalTo(200));
content = response.getContent();
assertThat(content, containsString("Session=" + id.substring(0, id.indexOf(".node0"))));
assertThat(content, containsString("RequestedSessionIdFromCookie: false"));
assertThat(content, containsString("RequestedSessionIdFromURL: true"));
assertThat(content, containsString("URI [/some/path;session_id=%s]".formatted(id))); // Cookies not in use

// Get with both, but param wrong
Expand Down
9 changes: 8 additions & 1 deletion jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/Attributes.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@
/**
* Attributes.
* Interface commonly used for storing attributes.
* <p>
* Some attributes may be "hidden" attributes, in that they are only found by an explicit call to
* {@link #getAttribute(String)} and they do not otherwise appear in {@link #getAttributeNameSet()}
* or {@link #asAttributeMap()}.
*/
public interface Attributes
{
Expand All @@ -51,7 +55,10 @@ public interface Attributes
Object setAttribute(String name, Object attribute);

/**
* Get an attribute
* Get an attribute by name.
* Some attributes may be "hidden" attributes, in that they are only found by an explicit call to
* {@code getAttribute(String)} and they do not otherwise appear in {@link #getAttributeNameSet()}
* or {@link #asAttributeMap()}.
* @param name the attribute to get
* @return the value of the attribute, or {@code null} if no such attribute exists
*/
Expand Down
14 changes: 7 additions & 7 deletions ...10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.Response;
import org.eclipse.jetty.server.Session;
import org.eclipse.jetty.session.AbstractSessionManager;
import org.eclipse.jetty.session.AbstractSessionManager.RequestedSession;
import org.eclipse.jetty.session.ManagedSession;
import org.eclipse.jetty.session.SessionManager;
import org.eclipse.jetty.util.Callback;
Expand Down Expand Up @@ -492,7 +492,7 @@ public Principal getUserPrincipal()
@Override
public String getRequestedSessionId()
{
AbstractSessionManager.RequestedSession requestedSession = getServletRequestInfo().getRequestedSession();
RequestedSession requestedSession = getServletRequestInfo().getRequestedSession();
return requestedSession == null ? null : requestedSession.sessionId();
}

Expand Down Expand Up @@ -551,7 +551,7 @@ public String changeSessionId()
@Override
public boolean isRequestedSessionIdValid()
{
AbstractSessionManager.RequestedSession requestedSession = getServletRequestInfo().getRequestedSession();
RequestedSession requestedSession = getServletRequestInfo().getRequestedSession();
HttpSession session = getSession(false);
SessionManager manager = getServletRequestInfo().getSessionManager();
return requestedSession != null &&
Expand All @@ -565,15 +565,15 @@ public boolean isRequestedSessionIdValid()
@Override
public boolean isRequestedSessionIdFromCookie()
{
AbstractSessionManager.RequestedSession requestedSession = getServletRequestInfo().getRequestedSession();
return requestedSession != null && requestedSession.sessionId() != null && requestedSession.sessionIdFromCookie();
RequestedSession requestedSession = getServletRequestInfo().getRequestedSession();
return requestedSession != null && requestedSession.sessionId() != null && requestedSession.isSessionIdFrom(RequestedSession.ID_FROM_COOKIE);
}

@Override
public boolean isRequestedSessionIdFromURL()
{
AbstractSessionManager.RequestedSession requestedSession = getServletRequestInfo().getRequestedSession();
return requestedSession != null && requestedSession.sessionId() != null && !requestedSession.sessionIdFromCookie();
RequestedSession requestedSession = getServletRequestInfo().getRequestedSession();
return requestedSession != null && requestedSession.sessionId() != null && requestedSession.isSessionIdFrom(RequestedSession.ID_FROM_URI_PARAMETER);
}

@Override
Expand Down
Loading

0 comments on commit 8b4e13d

Please sign in to comment.