diff --git a/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakAuthenticationFilter.java b/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakAuthenticationFilter.java index 48c9066..5adf4bb 100644 --- a/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakAuthenticationFilter.java +++ b/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakAuthenticationFilter.java @@ -94,6 +94,12 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter // copied from WebScriptRequestImpl due to accessible constraints private static final String ARG_GUEST = "guest"; + // copied from BasicHttpAuthenticator (inline literal constant) + private static final String ARG_ALF_TICKET = "alf_ticket"; + + // copied from base class - inaccessible there + private static final String LOGIN_EXTERNAL_AUTH = "_alfExternalAuth"; + private static final Logger LOGGER = LoggerFactory.getLogger(KeycloakAuthenticationFilter.class); private static final String HEADER_AUTHORIZATION = "Authorization"; @@ -737,7 +743,7 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter final String servletRequestUri = servletPath + (pathInfo != null ? pathInfo : ""); SessionUser sessionUser = this.getSessionUser(context, req, res, true); - HttpSession session = req.getSession(); + HttpSession session = req.getSession(false); final boolean publicRestApi = API_SERVLET_PATH.equals(servletPath); final boolean noAuthPublicRestApiWebScript = publicRestApi @@ -776,6 +782,10 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter } else if (authHeader != null && authHeader.toLowerCase(Locale.ENGLISH).startsWith("bearer ")) { + if (session == null) + { + throw new IllegalStateException("Session should have been initialised by Bearer authentication in remote user mapper"); + } final AccessToken accessToken = (AccessToken) session.getAttribute(KeycloakRemoteUserMapper.class.getName()); if (accessToken != null) { @@ -928,6 +938,85 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter return skip; } + /** + * + * {@inheritDoc} + */ + // mostly copied from base class + // overridden / patched to avoid forced session initialisation + @Override + protected SessionUser getSessionUser(final ServletContext servletContext, final HttpServletRequest httpServletRequest, + final HttpServletResponse httpServletResponse, final boolean externalAuth) + { + String userId = null; + if (this.remoteUserMapper != null + && (!(this.remoteUserMapper instanceof ActivateableBean) || ((ActivateableBean) this.remoteUserMapper).isActive())) + { + userId = this.remoteUserMapper.getRemoteUser(httpServletRequest); + LOGGER.trace("Found a remote user: {}", AlfrescoCompatibilityUtil.maskUsername(userId)); + } + + final String sessionAttrib = this.getUserAttributeName(); + // deviation: don't force session + HttpSession session = httpServletRequest.getSession(false); + SessionUser sessionUser = session != null ? (SessionUser) session.getAttribute(sessionAttrib) : null; + + if (sessionUser != null) + { + try + { + LOGGER.trace("Found a session user: {}", AlfrescoCompatibilityUtil.maskUsername(sessionUser.getUserName())); + this.authenticationService.validate(sessionUser.getTicket()); + if (externalAuth) + { + session.setAttribute(LOGIN_EXTERNAL_AUTH, Boolean.TRUE); + } + else + { + session.removeAttribute(LOGIN_EXTERNAL_AUTH); + } + } + catch (final AuthenticationException e) + { + LOGGER.debug("The ticket may have expired or the person could have been removed, invalidating session.", e); + this.invalidateSession(httpServletRequest); + sessionUser = null; + } + } + + if (userId != null) + { + LOGGER.debug("We have a previously-cached user with the wrong identity - replace them."); + + if (sessionUser != null && !sessionUser.getUserName().equals(userId)) + { + LOGGER.debug("Removing the session user, invalidating session."); + session.removeAttribute(sessionAttrib); + session.invalidate(); + sessionUser = null; + } + + if (sessionUser == null) + { + LOGGER.debug("Propagating through the user identity: {}", AlfrescoCompatibilityUtil.maskUsername(userId)); + this.authenticationComponent.setCurrentUser(userId); + session = httpServletRequest.getSession(); + + try + { + sessionUser = this.createUserEnvironment(session, this.authenticationService.getCurrentUserName(), + this.authenticationService.getCurrentTicket(), true); + } + catch (final Throwable e) + { + LOGGER.debug("Error during ticket validation and user creation: {}", e.getMessage(), e); + } + } + } + + return sessionUser; + } + /** * Checks whether a particular request is aimed at a Public v1 ReST API web script which does not require any authentication. * @@ -1072,7 +1161,12 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter throws IOException, ServletException { boolean ticketValid = false; - final String ticket = req.getParameter(ARG_TICKET); + // prefer ticket over alf_ticket, as default Alfresco filters only handle ticket + String ticket = req.getParameter(ARG_TICKET); + if (ticket == null) + { + ticket = req.getParameter(ARG_ALF_TICKET); + } if (ticket != null && ticket.length() != 0) { @@ -1081,21 +1175,20 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter try { - final SessionUser user = this.getSessionUser(context, req, resp, true); + // implicitly validates the ticket of the session user is still valid + SessionUser user = this.getSessionUser(context, req, resp, true); if (user != null && !ticket.equals(user.getTicket())) { LOGGER.debug("Invalidating current session as URL-provided authentication ticket does not match"); this.invalidateSession(req); + user = null; } if (user == null) { this.authenticationService.validate(ticket); - this.createUserEnvironment(req.getSession(), this.authenticationService.getCurrentUserName(), - this.authenticationService.getCurrentTicket(), true); - LOGGER.debug("Authenticated user {} via URL-provided authentication ticket", AlfrescoCompatibilityUtil.maskUsername(this.authenticationService.getCurrentUserName())); diff --git a/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakWebScriptCookieAuthenticationFilter.java b/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakWebScriptCookieAuthenticationFilter.java index 4168a8d..2f8253f 100644 --- a/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakWebScriptCookieAuthenticationFilter.java +++ b/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakWebScriptCookieAuthenticationFilter.java @@ -17,23 +17,59 @@ package de.acosix.alfresco.keycloak.repo.authentication; import java.io.IOException; +import javax.servlet.FilterChain; +import javax.servlet.ServletContext; import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpSession; import org.alfresco.repo.SessionUser; -import org.alfresco.repo.webdav.auth.AuthenticationDriver; -import org.alfresco.repo.webdav.auth.BaseAuthenticationFilter; +import org.alfresco.repo.web.scripts.bean.LoginPost; import org.alfresco.web.app.servlet.WebscriptCookieAuthenticationFilter; /** - * This sub-class of the default web script cookie filter only exists to ensure the proper session attribute names are used for mapping the - * authenticated session user. + * This sub-class of the default web script cookie filter only exists to ensure that the filter does NOT completely intercept the call for + * the {@link LoginPost login web script}, otherwise clients relying on the response body may not function correctly, and that it ensures + * proper any previously existing session is invalidated when the login web script is called with explicit credentials. * * @author Axel Faust */ public class KeycloakWebScriptCookieAuthenticationFilter extends WebscriptCookieAuthenticationFilter { + // copied from base class - inaccessible otherwise + private static final String API_LOGIN = "/api/login"; + + /** + * + * {@inheritDoc} + */ + @Override + public void doFilter(final ServletContext context, final ServletRequest sreq, final ServletResponse sresp, final FilterChain chain) + throws IOException, ServletException + { + final HttpServletRequest req = (HttpServletRequest) sreq; + + if (API_LOGIN.equals(req.getPathInfo()) && req.getMethod().equalsIgnoreCase("POST")) + { + final HttpSession session = req.getSession(false); + if (session != null) + { + session.invalidate(); + } + + // from here on the default web script will handle things, including - most importantly - the response instead of 204 no content + // response by base class + chain.doFilter(req, sresp); + } + else + { + chain.doFilter(sreq, sresp); + } + } + /** * * {@inheritDoc} @@ -41,14 +77,7 @@ public class KeycloakWebScriptCookieAuthenticationFilter extends WebscriptCookie @Override protected SessionUser createUserEnvironment(final HttpSession session, final String userName) throws IOException, ServletException { - final SessionUser sessionUser = super.createUserEnvironment(session, userName); - - // ensure all common attribute names are mapped - // Alfresco is really inconsistent with these attribute names - session.setAttribute(AuthenticationDriver.AUTHENTICATION_USER, sessionUser); - session.setAttribute(BaseAuthenticationFilter.AUTHENTICATION_USER, sessionUser); - - return sessionUser; + throw new UnsupportedOperationException("Should never be called"); } /** @@ -59,13 +88,6 @@ public class KeycloakWebScriptCookieAuthenticationFilter extends WebscriptCookie protected SessionUser createUserEnvironment(final HttpSession session, final String userName, final String ticket, final boolean externalAuth) throws IOException, ServletException { - final SessionUser sessionUser = super.createUserEnvironment(session, userName, ticket, externalAuth); - - // ensure all common attribute names are mapped - // Alfresco is really inconsistent with these attribute names - session.setAttribute(AuthenticationDriver.AUTHENTICATION_USER, sessionUser); - session.setAttribute(BaseAuthenticationFilter.AUTHENTICATION_USER, sessionUser); - - return sessionUser; + throw new UnsupportedOperationException("Should never be called"); } }