From db5209e97abeb23eca98faa3164a79a6b1b7b806 Mon Sep 17 00:00:00 2001 From: gregw Date: Thu, 17 Aug 2023 11:18:08 +1000 Subject: [PATCH] Implement ServeAs in core SecurityHandler --- .../jetty/security/AuthenticationState.java | 30 ++++++--- .../jetty/security/SecurityHandler.java | 62 +++++++++++++++---- .../jetty/ee10/servlet/ServletChannel.java | 30 --------- .../ee10/servlet/ServletChannelState.java | 24 ------- .../ee10/servlet/ServletContextRequest.java | 34 ++++++++++ .../servlet/security/FormAuthenticator.java | 21 ++++--- .../security/FormAuthenticatorTest.java | 4 +- 7 files changed, 120 insertions(+), 85 deletions(-) diff --git a/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/AuthenticationState.java b/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/AuthenticationState.java index 587294b553a..c96a8785850 100644 --- a/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/AuthenticationState.java +++ b/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/AuthenticationState.java @@ -17,6 +17,7 @@ import java.security.Principal; import org.eclipse.jetty.http.HttpException; import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.security.IdentityService.RunAsToken; import org.eclipse.jetty.security.authentication.LoginAuthenticator; import org.eclipse.jetty.security.internal.DeferredAuthenticationState; @@ -270,17 +271,30 @@ public interface AuthenticationState extends Request.AuthenticationState } }; - /** - * Authentication should be deferred, and request allowed to bypass security constraint. - */ - AuthenticationState DEFER = new AuthenticationState() + class ServeAs implements AuthenticationState { - @Override - public String toString() + private final HttpURI _uri; + + public ServeAs(HttpURI uri) { - return "DEFER"; + _uri = uri; } - }; + + public Request wrap(Request request) + { + if (request.getHttpURI().equals(_uri)) + return request; + + return new Request.Wrapper(request) + { + @Override + public HttpURI getHttpURI() + { + return _uri; + } + }; + } + } static Deferred defer(LoginAuthenticator loginAuthenticator) { diff --git a/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/SecurityHandler.java b/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/SecurityHandler.java index cc7a754003e..e66a2bb1620 100644 --- a/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/SecurityHandler.java +++ b/jetty-core/jetty-security/src/main/java/org/eclipse/jetty/security/SecurityHandler.java @@ -24,8 +24,12 @@ import java.util.Map; import java.util.ServiceLoader; import java.util.Set; +import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; +import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpHeaderValue; import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.http.pathmap.MappedResource; import org.eclipse.jetty.http.pathmap.PathMappings; import org.eclipse.jetty.http.pathmap.PathSpec; @@ -489,14 +493,51 @@ public abstract class SecurityHandler extends Handler.Wrapper implements Configu if (authenticationState instanceof AuthenticationState.ResponseSent) return true; - if (mustValidate && isNotAuthorized(constraint, authenticationState)) + if (authenticationState instanceof AuthenticationState.ServeAs serveAs) + { + HttpURI uri = request.getHttpURI(); + request = serveAs.wrap(request); + if (!uri.equals(request.getHttpURI())) + { + // URI is replaced, so filter out all metadata for the old URI + response.getHeaders().put(HttpHeader.CACHE_CONTROL.asString(), HttpHeaderValue.NO_CACHE.asString()); + response.getHeaders().putDate(HttpHeader.EXPIRES.asString(), 1); + HttpFields.Mutable headers = new HttpFields.Mutable.Wrapper(response.getHeaders()) + { + @Override + public HttpField onAddField(HttpField field) + { + if (field.getHeader() == null) + return field; + return switch (field.getHeader()) + { + case CACHE_CONTROL, PRAGMA, ETAG, EXPIRES, LAST_MODIFIED, AGE -> null; + default -> field; + }; + } + }; + + response = new Response.Wrapper(request, response) + { + @Override + public HttpFields.Mutable getHeaders() + { + return headers; + } + }; + } + + authenticationState = _deferred; + } + else if (mustValidate && !isAuthorized(constraint, authenticationState)) { Response.writeError(request, response, callback, HttpStatus.FORBIDDEN_403, "!authorized"); return true; } - - if (authenticationState == null || authenticationState == AuthenticationState.DEFER) + else if (authenticationState == null) + { authenticationState = _deferred; + } AuthenticationState.setAuthenticationState(request, authenticationState); IdentityService.Association association = @@ -553,23 +594,20 @@ public abstract class SecurityHandler extends Handler.Wrapper implements Configu } } - protected boolean isNotAuthorized(Constraint constraint, AuthenticationState authenticationState) + protected boolean isAuthorized(Constraint constraint, AuthenticationState authenticationState) { - if (authenticationState == AuthenticationState.DEFER) - return false; - UserIdentity userIdentity = authenticationState instanceof AuthenticationState.Succeeded user ? user.getUserIdentity() : null; return switch (constraint.getAuthorization()) { - case FORBIDDEN, ALLOWED, INHERIT -> false; + case FORBIDDEN, ALLOWED, INHERIT -> true; case ANY_USER -> userIdentity == null || userIdentity.getUserPrincipal() == null; case KNOWN_ROLE -> { if (userIdentity != null && userIdentity.getUserPrincipal() != null) for (String role : getKnownRoles()) if (userIdentity.isUserInRole(role)) - yield false; - yield true; + yield true; + yield false; } case SPECIFIC_ROLE -> @@ -577,8 +615,8 @@ public abstract class SecurityHandler extends Handler.Wrapper implements Configu if (userIdentity != null && userIdentity.getUserPrincipal() != null) for (String role : constraint.getRoles()) if (userIdentity.isUserInRole(role)) - yield false; - yield true; + yield true; + yield false; } }; } diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java index 9beac392089..b0c1b4b02ad 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannel.java @@ -197,11 +197,6 @@ public class ServletChannel return _httpInput; } - public ServletContextHandler.ServletContextApi getServletContextContext() - { - return _servletContextApi; - } - public boolean isSendError() { return _state.isSendError(); @@ -458,24 +453,6 @@ public class ServletChannel _expects100Continue = false; } - /** - *

When this is called the initial dispatch will use the {@link ServletChannel#FORWARD_PATH}, - * {@link ServletChannel#FORWARD_REQUEST} and {@link ServletChannel#FORWARD_RESPONSE} attributes - * to do a {@link jakarta.servlet.DispatcherType#FORWARD} dispatch instead of the initial - * {@link jakarta.servlet.DispatcherType#REQUEST} dispatch.

- * - *

This must only be called before {@link ServletChannel#handle()} is first invoked. - * This can be used to dispatch to a different target before the initial request has been dispatched.

- */ - public void forward(String path, HttpServletRequest request, HttpServletResponse response) - { - ServletContextRequest contextRequest = getServletContextRequest(); - contextRequest.setAttribute(FORWARD_PATH, path); - contextRequest.setAttribute(FORWARD_REQUEST, request); - contextRequest.setAttribute(FORWARD_RESPONSE, response); - _state.initialDispatch(); - } - /** * Handle the servlet request. This is called on the initial dispatch and then again on any asynchronous events. * @return True if the channel is ready to continue handling (ie it is not suspended) @@ -516,13 +493,6 @@ public class ServletChannel break; } - case FORWARD: - { - reopen(); - initialForward(); - break; - } - case ASYNC_DISPATCH: { reopen(); diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannelState.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannelState.java index e9994eec1fc..0045a24566c 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannelState.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletChannelState.java @@ -62,7 +62,6 @@ public class ServletChannelState public enum State { IDLE, // Idle request - FORWARD, // forward() has been called. HANDLING, // Request dispatched to filter/servlet or Async IO callback WAITING, // Suspended and waiting WOKEN, // Dispatch to handle from ASYNC_WAIT @@ -125,7 +124,6 @@ public class ServletChannelState public enum Action { DISPATCH, // handle a normal request dispatch - FORWARD, // initial request will be forwarded ASYNC_DISPATCH, // handle an async request dispatch SEND_ERROR, // Generate an error page or error dispatch ASYNC_ERROR, // handle an async error @@ -324,21 +322,6 @@ public class ServletChannelState } } - public void initialDispatch() - { - try (AutoLock ignored = lock()) - { - switch (_state) - { - case IDLE: - _state = State.FORWARD; - break; - default: - throw new IllegalStateException(getStatusStringLocked()); - } - } - } - /** * @return Next handling of the request should proceed */ @@ -358,13 +341,6 @@ public class ServletChannelState _state = State.HANDLING; return Action.DISPATCH; - case FORWARD: - if (_requestState != RequestState.BLOCKING) - throw new IllegalStateException(getStatusStringLocked()); - _initial = true; - _state = State.HANDLING; - return Action.FORWARD; - case WOKEN: if (_event != null && _event.getThrowable() != null && !_sendError) { diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java index 47d04608e0f..166250dae09 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java @@ -29,6 +29,7 @@ import jakarta.servlet.http.HttpServletResponse; import org.eclipse.jetty.http.HttpCookie; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpMethod; +import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.http.UriCompliance; import org.eclipse.jetty.http.pathmap.MatchedResource; import org.eclipse.jetty.server.FormFields; @@ -116,6 +117,39 @@ public class ServletContextRequest extends ContextRequest implements ServletCont addIdleTimeoutListener(_servletChannel.getServletRequestState()::onIdleTimeout); } + public Request serveAs(Request request, String path) + { + MatchedResource matchedResource = getServletContextHandler().getServletHandler().getMatchedServlet(path); + + if (matchedResource == null) + return null; + + ServletHandler.MappedServlet mappedServlet = matchedResource.getResource(); + if (mappedServlet == null) + return null; + + ServletChannel servletChannel = getServletChannel(); + + HttpURI uri = HttpURI.build(request.getHttpURI()).path(path).asImmutable(); + + ServletContextRequest servletContextRequest = getServletContextHandler().newServletContextRequest( + servletChannel, + new Request.Wrapper(request) + { + @Override + public HttpURI getHttpURI() + { + return uri; + } + }, + _response, + path, + matchedResource + ); + servletChannel.associate(servletContextRequest); + return servletContextRequest; + } + protected ServletApiRequest newServletApiRequest() { if (getHttpURI().hasViolations() && !getServletChannel().getServletContextHandler().getServletHandler().isDecodeAmbiguousURIs()) diff --git a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/security/FormAuthenticator.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/security/FormAuthenticator.java index 92bfdf53f43..4be86eeb2e3 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/security/FormAuthenticator.java +++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/security/FormAuthenticator.java @@ -23,7 +23,6 @@ import jakarta.servlet.http.HttpServletResponse; import jakarta.servlet.http.HttpServletResponseWrapper; import org.eclipse.jetty.ee10.servlet.ServletContextRequest; import org.eclipse.jetty.http.HttpHeader; -import org.eclipse.jetty.http.HttpHeaderValue; import org.eclipse.jetty.security.AuthenticationState; import org.eclipse.jetty.security.authentication.SessionAuthentication; import org.eclipse.jetty.server.Request; @@ -90,15 +89,19 @@ public class FormAuthenticator extends org.eclipse.jetty.security.authentication { try { - response.getHeaders().put(HttpHeader.CACHE_CONTROL.asString(), HttpHeaderValue.NO_CACHE.asString()); - response.getHeaders().putDate(HttpHeader.EXPIRES.asString(), 1); + // Currently we do not attempt to wrap the request + return new AuthenticationState.ServeAs(request.getHttpURI()) + { + @Override + public Request wrap(Request request) + { + ServletContextRequest servletContextRequest = Request.as(request, ServletContextRequest.class); + if (servletContextRequest == null) + return super.wrap(request); - ServletContextRequest contextRequest = Request.as(request, ServletContextRequest.class); - FormRequest formRequest = new FormRequest(contextRequest.getServletApiRequest()); - FormResponse formResponse = new FormResponse(contextRequest.getHttpServletResponse()); - contextRequest.getServletChannel().forward(path, formRequest, formResponse); - - return AuthenticationState.DEFER; + return servletContextRequest.serveAs(request, path); + } + }; } catch (Throwable t) { diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/security/FormAuthenticatorTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/security/FormAuthenticatorTest.java index 10067e1e5a3..0d669c903ec 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/security/FormAuthenticatorTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/security/FormAuthenticatorTest.java @@ -87,7 +87,7 @@ public class FormAuthenticatorTest { String response = _connector.getResponse("GET /ctx/admin/user HTTP/1.0\r\nHost:host:8888\r\n\r\n"); assertThat(response, containsString("HTTP/1.1 200 OK")); - assertThat(response, containsString("dispatcherType: FORWARD")); + assertThat(response, containsString("dispatcherType: REQUEST")); } @Test @@ -95,6 +95,6 @@ public class FormAuthenticatorTest { String response = _connector.getResponse("GET /ctx/j_security_check?j_username=user&j_password=wrong HTTP/1.0\r\nHost:host:8888\r\n\r\n"); assertThat(response, containsString("path: /ctx/error")); - assertThat(response, containsString("dispatcherType: FORWARD")); + assertThat(response, containsString("dispatcherType: REQUEST")); } }