diff --git a/repository/src/main/globalConfig/subsystems/Authentication/keycloak/keycloak-authentication-context.xml b/repository/src/main/globalConfig/subsystems/Authentication/keycloak/keycloak-authentication-context.xml index cd127b8..70085ba 100644 --- a/repository/src/main/globalConfig/subsystems/Authentication/keycloak/keycloak-authentication-context.xml +++ b/repository/src/main/globalConfig/subsystems/Authentication/keycloak/keycloak-authentication-context.xml @@ -133,6 +133,7 @@ + diff --git a/repository/src/main/globalConfig/subsystems/Authentication/keycloak/keycloak-authentication.properties b/repository/src/main/globalConfig/subsystems/Authentication/keycloak/keycloak-authentication.properties index 27c1dc1..96f0965 100644 --- a/repository/src/main/globalConfig/subsystems/Authentication/keycloak/keycloak-authentication.properties +++ b/repository/src/main/globalConfig/subsystems/Authentication/keycloak/keycloak-authentication.properties @@ -19,7 +19,7 @@ keycloak.authentication.sslRedirectPort=8443 keycloak.authentication.bodyBufferLimit=10485760 # override for a direct route to the auth server host -# useful primarily for Dockerized deployments where container running Alfresco cannot resolve the auth server via the public DNS name +# useful primarily for Docker-ized deployments where container running Alfresco cannot resolve the auth server via the public DNS name keycloak.authentication.directAuthHost= keycloak.adapter.auth-server-url=http://localhost:8180/auth @@ -29,6 +29,8 @@ keycloak.adapter.ssl-required=none keycloak.adapter.public-client=false keycloak.adapter.credentials.provider=secret keycloak.adapter.credentials.secret= +# for some reason, this is not a sane default in Keycloak Adapter config +keycloak.adapter.verify-token-audience=true # TODO default settings (identical to AdapterConfig defaults) to better align with default Alfresco subsystem property handling 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 6bdb47d..5c774c0 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 @@ -116,6 +116,8 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter protected String originalRequestUrlHeaderName; + protected String noKeycloakHandlingHeaderName; + protected int bodyBufferLimit = DEFAULT_BODY_BUFFER_LIMIT; // use 8443 as default SSL redirect based on Tomcat default server.xml configuration @@ -146,6 +148,8 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter PropertyCheck.mandatory(this, "keycloakTicketTokenCache", this.keycloakTicketTokenCache); PropertyCheck.mandatory(this, "publicApiRuntimeContainer", this.publicApiRuntimeContainer); + PropertyCheck.mandatory(this, "noKeycloakHandlingHeaderName", this.noKeycloakHandlingHeaderName); + // parent class does not check, so we do PropertyCheck.mandatory(this, "authenticationService", this.authenticationService); PropertyCheck.mandatory(this, "authenticationComponent", this.authenticationComponent); @@ -222,6 +226,15 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter this.originalRequestUrlHeaderName = originalRequestUrlHeaderName; } + /** + * @param noKeycloakHandlingHeaderName + * the noKeycloakHandlingHeaderName to set + */ + public void setNoKeycloakHandlingHeaderName(final String noKeycloakHandlingHeaderName) + { + this.noKeycloakHandlingHeaderName = noKeycloakHandlingHeaderName; + } + /** * @param bodyBufferLimit * the bodyBufferLimit to set @@ -710,6 +723,7 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter boolean skip = false; final String authHeader = req.getHeader(HEADER_AUTHORIZATION); + final String noKeycloakLoginRedirectHeader = req.getHeader(this.noKeycloakHandlingHeaderName); final String servletPath = req.getServletPath(); final String pathInfo = req.getPathInfo(); @@ -765,8 +779,7 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter // cannot rely on session.isNew() to determine if this is a fresh login // consider "fresh" login if issued within age limit (implicitly include any token refreshes performed client-side) - final boolean isFreshLogin = accessToken.getIssuedAt() - * 1000l > (System.currentTimeMillis() - FRESH_TOKEN_AGE_LIMIT_MS); + final boolean isFreshLogin = accessToken.getIat() * 1000l > (System.currentTimeMillis() - FRESH_TOKEN_AGE_LIMIT_MS); this.keycloakAuthenticationComponent.handleUserTokens(accessToken, accessToken, isFreshLogin); // sessionUser should be guaranteed here, but still check - we need it for the cache key @@ -857,9 +870,9 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter "Skipping processKeycloakAuthenticationAndActions as request is aimed at a Public v1 ReST API which does not require authentication"); skip = true; } - // check no-auth flag (derived e.g. from checking if target web script requires authentication) only after all - // pre-emptive auth - // request details have been checked + + // check no-auth flag (derived e.g. from checking if target web script requires authentication) as last resort to see if + // we need to force authentication after invalidating session else if (Boolean.TRUE.equals(req.getAttribute(NO_AUTH_REQUIRED))) { LOGGER.trace( @@ -895,6 +908,13 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter "Skipping processKeycloakAuthenticationAndActions as filter higher up in chain determined authentication as not required"); skip = true; } + else if (Boolean.parseBoolean(noKeycloakLoginRedirectHeader)) + { + LOGGER.trace( + "Skipping processKeycloakAuthenticationAndActions as client provided custom 'no Keycloak handling' header {} with value that resolves to 'true'", + this.noKeycloakHandlingHeaderName); + skip = true; + } // TODO Check for login page URL (rarely configured since Repository by default has no login page since 5.0) return skip; diff --git a/share/src/main/config/default-config.xml b/share/src/main/config/default-config.xml index eabf189..20a06eb 100644 --- a/share/src/main/config/default-config.xml +++ b/share/src/main/config/default-config.xml @@ -50,6 +50,8 @@ secret + + true diff --git a/share/src/main/config/module-context.xml b/share/src/main/config/module-context.xml index c007266..7b4e970 100644 --- a/share/src/main/config/module-context.xml +++ b/share/src/main/config/module-context.xml @@ -47,12 +47,17 @@ - - - - + + + + + + + + diff --git a/share/src/main/config/share-global.properties b/share/src/main/config/share-global.properties index 8035528..8a64e81 100644 --- a/share/src/main/config/share-global.properties +++ b/share/src/main/config/share-global.properties @@ -1 +1 @@ -${moduleId}.surf.onlyOneRedirect.enabled=true \ No newline at end of file +# no configuration properties necessary - empty file to mark this as a Share module \ No newline at end of file diff --git a/share/src/main/java/de/acosix/alfresco/keycloak/share/remote/AccessTokenAwareSlingshotAlfrescoConnector.java b/share/src/main/java/de/acosix/alfresco/keycloak/share/remote/AccessTokenAwareSlingshotAlfrescoConnector.java index 7a1b753..590333d 100644 --- a/share/src/main/java/de/acosix/alfresco/keycloak/share/remote/AccessTokenAwareSlingshotAlfrescoConnector.java +++ b/share/src/main/java/de/acosix/alfresco/keycloak/share/remote/AccessTokenAwareSlingshotAlfrescoConnector.java @@ -71,6 +71,7 @@ public class AccessTokenAwareSlingshotAlfrescoConnector extends SlingshotAlfresc final RefreshableAccessTokenHolder endpointSpecificAccessToken = (RefreshableAccessTokenHolder) (session != null ? session.getAttribute(KeycloakAuthenticationFilter.BACKEND_ACCESS_TOKEN_SESSION_KEY) : null); + if (endpointSpecificAccessToken != null) { if (endpointSpecificAccessToken.isActive()) diff --git a/share/src/main/java/de/acosix/alfresco/keycloak/share/web/KeycloakAuthenticationFilter.java b/share/src/main/java/de/acosix/alfresco/keycloak/share/web/KeycloakAuthenticationFilter.java index 045bd63..3b847ff 100644 --- a/share/src/main/java/de/acosix/alfresco/keycloak/share/web/KeycloakAuthenticationFilter.java +++ b/share/src/main/java/de/acosix/alfresco/keycloak/share/web/KeycloakAuthenticationFilter.java @@ -83,6 +83,7 @@ import de.acosix.alfresco.keycloak.share.config.KeycloakAuthenticationConfigElem import de.acosix.alfresco.keycloak.share.config.KeycloakConfigConstants; import de.acosix.alfresco.keycloak.share.deps.keycloak.KeycloakSecurityContext; import de.acosix.alfresco.keycloak.share.deps.keycloak.OAuth2Constants; +import de.acosix.alfresco.keycloak.share.deps.keycloak.TokenVerifier; import de.acosix.alfresco.keycloak.share.deps.keycloak.adapters.AdapterDeploymentContext; import de.acosix.alfresco.keycloak.share.deps.keycloak.adapters.AuthenticatedActionsHandler; import de.acosix.alfresco.keycloak.share.deps.keycloak.adapters.BearerTokenRequestAuthenticator; @@ -95,6 +96,7 @@ import de.acosix.alfresco.keycloak.share.deps.keycloak.adapters.PreAuthActionsHa import de.acosix.alfresco.keycloak.share.deps.keycloak.adapters.ServerRequest; import de.acosix.alfresco.keycloak.share.deps.keycloak.adapters.authentication.ClientCredentialsProviderUtils; import de.acosix.alfresco.keycloak.share.deps.keycloak.adapters.rotation.AdapterTokenVerifier; +import de.acosix.alfresco.keycloak.share.deps.keycloak.adapters.rotation.AdapterTokenVerifier.VerifiedTokens; import de.acosix.alfresco.keycloak.share.deps.keycloak.adapters.servlet.FilterRequestAuthenticator; import de.acosix.alfresco.keycloak.share.deps.keycloak.adapters.servlet.OIDCFilterSessionStore; import de.acosix.alfresco.keycloak.share.deps.keycloak.adapters.servlet.OIDCServletHttpFacade; @@ -832,7 +834,7 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I session.setAttribute(UserFactory.SESSION_ATTRIBUTE_EXTERNAL_AUTH, Boolean.TRUE); session.setAttribute(UserFactory.SESSION_ATTRIBUTE_KEY_USER_ID, userId); - this.handleAlfrescoResourceAccessToken(session); + this.handleAlfrescoResourceAccessToken(session, false); } if (facade.isEnded()) @@ -937,13 +939,17 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I protected void onKeycloakAuthenticationFailure(final ServletContext context, final HttpServletRequest req, final HttpServletResponse res, final FilterChain chain) throws IOException, ServletException { - LOGGER.warn("Keycloak authentication failed due to {}", req.getAttribute(AuthenticationError.class.getName())); + final Object authError = req.getAttribute(AuthenticationError.class.getName()); + LOGGER.warn("Keycloak authentication failed due to {}", + authError != null ? authError : ""); LOGGER.debug("Resetting session and state cookie before continueing with filter chain"); req.getSession().invalidate(); this.resetStateCookies(context, req, res); + // TODO If error occurred as part of redirect back from Keycloak, strip state / code params from URL query + // TODO If login page may follow, see about providing error message this.continueFilterChain(context, req, res, chain); } @@ -1091,9 +1097,11 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I } else { - // TODO Validate via custom /touch to check if session is still valid - // custom => handle potential 302 instead of 401 response from Keycloak-enabled backend - // custom => deal with redirect host being unknown (similar to our auth-server-url vs. directAuthHost case) + /* + * Note: We could validate session with a custom call to /touch but we leave that to any remaining SSO filters. We patch + * remoteClient to submit a custom HTTP header to backend to avoid 302 redirects to Keycloak which other SSO filters cannot + * handle, and this also avoids any issues with (public) Keycloak auth server URL being unknown, e.g. in a Docker scenario + */ LOGGER.debug( "Skipping processKeycloakAuthenticationAndActions as non-Keycloak-authenticated session is already established"); skip = true; @@ -1174,7 +1182,7 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I if (currentSession != null) { LOGGER.debug("Skipping processKeycloakAuthenticationAndActions as Keycloak-authentication session is still valid"); - this.handleAlfrescoResourceAccessToken(currentSession); + this.handleAlfrescoResourceAccessToken(currentSession, false); skip = true; } else @@ -1333,8 +1341,11 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I * * @param session * the active session managing any persistent access token state + * @param retry + * {@code true} if the invocation of the operation is a retry as the result of a (hopefully temporary) verification failure + * in the current thread */ - protected void handleAlfrescoResourceAccessToken(final HttpSession session) + protected void handleAlfrescoResourceAccessToken(final HttpSession session, final boolean retry) { final KeycloakAuthenticationConfigElement keycloakAuthConfig = (KeycloakAuthenticationConfigElement) this.configService .getConfig(KeycloakConfigConstants.KEYCLOAK_CONFIG_SECTION_NAME).getConfigElement(KeycloakAuthenticationConfigElement.NAME); @@ -1390,28 +1401,44 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I } final String tokenString = response.getToken(); - final AdapterTokenVerifier.VerifiedTokens tokens; try { - tokens = AdapterTokenVerifier.verifyTokens(tokenString, response.getIdToken(), this.keycloakDeployment); + // cannot use simple AdapterTokenVerifier.verifyTokens as it checks for wrong audience + // we also do not care about any IDToken retrieved (implicitly) with token exchange + final TokenVerifier tokenVerifier = AdapterTokenVerifier.createVerifier(tokenString, + this.keycloakDeployment, true, AccessToken.class); + tokenVerifier.audience(alfrescoResourceName); + tokenVerifier.issuedFor(this.keycloakDeployment.getResourceName()); + + final AccessToken accessToken = tokenVerifier.verify().getToken(); + + if ((accessToken.getExp() - this.keycloakDeployment.getTokenMinimumTimeToLive()) <= Time.currentTime()) + { + throw new AlfrescoRuntimeException( + "Failed to retrieve / refresh the access token for the Alfresco backend with a longer time-to-live than the minimum"); + } + + token = new RefreshableAccessTokenHolder(response, new VerifiedTokens(accessToken, null)); + session.setAttribute(BACKEND_ACCESS_TOKEN_SESSION_KEY, token); + LOGGER.debug("Successfully retrieved / refresh access token for Alfresco backend"); } catch (final VerificationException vex) { - LOGGER.error("Verification of access token for Alfresco backend failed", vex); - throw new AlfrescoRuntimeException("Failed to verify access token for Alfresco backend", vex); + session.removeAttribute(BACKEND_ACCESS_TOKEN_SESSION_KEY); + if (!retry && token != null && token.canRefresh()) + { + LOGGER.warn( + "Verification of refreshed access token for Alfresco backend failed - removed previous token from the session before retrying token exchange from scratch"); + + this.handleAlfrescoResourceAccessToken(session, true); + } + else + { + LOGGER.error("Verification of access token for Alfresco backend failed in retry", vex); + throw new AlfrescoRuntimeException("Keycloak token exchange for access to backend yielded invalid access token", + vex); + } } - - final AccessToken accessToken = tokens.getAccessToken(); - - if ((accessToken.getExpiration() - this.keycloakDeployment.getTokenMinimumTimeToLive()) <= Time.currentTime()) - { - throw new AlfrescoRuntimeException( - "Failed to retrieve / refresh the access token for the Alfresco backend with a longer time-to-live than the minimum"); - } - - token = new RefreshableAccessTokenHolder(response, tokens); - session.setAttribute(BACKEND_ACCESS_TOKEN_SESSION_KEY, token); - LOGGER.debug("Successfully retrieved / refresh access token for Alfresco backend"); } } else if (alfrescoResourceName == null) @@ -1455,8 +1482,21 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I formParams.add(new BasicNameValuePair(OAuth2Constants.REQUESTED_TOKEN_TYPE, OAuth2Constants.REFRESH_TOKEN_TYPE)); final OidcKeycloakAccount keycloakAccount = (OidcKeycloakAccount) session.getAttribute(KEYCLOAK_ACCOUNT_SESSION_KEY); - final String tokenString = keycloakAccount.getKeycloakSecurityContext().getTokenString(); - formParams.add(new BasicNameValuePair(OAuth2Constants.SUBJECT_TOKEN, tokenString)); + final RefreshableAccessTokenHolder accessToken = (RefreshableAccessTokenHolder) session.getAttribute(ACCESS_TOKEN_SESSION_KEY); + if (keycloakAccount != null) + { + final String tokenString = keycloakAccount.getKeycloakSecurityContext().getTokenString(); + formParams.add(new BasicNameValuePair(OAuth2Constants.SUBJECT_TOKEN, tokenString)); + } + else if (accessToken != null && accessToken.isActive()) + { + formParams.add(new BasicNameValuePair(OAuth2Constants.SUBJECT_TOKEN, accessToken.getToken())); + } + else + { + throw new IllegalStateException( + "Either an active security context or access token should be present in the session, or previous validations have caught their non-existence and prevented this operation form being called"); + } ClientCredentialsProviderUtils.setClientCredentials(this.keycloakDeployment, post, formParams); diff --git a/share/src/main/java/de/acosix/alfresco/keycloak/share/web/UserGroupsLoadFilter.java b/share/src/main/java/de/acosix/alfresco/keycloak/share/web/UserGroupsLoadFilter.java index c72231f..d71d61c 100644 --- a/share/src/main/java/de/acosix/alfresco/keycloak/share/web/UserGroupsLoadFilter.java +++ b/share/src/main/java/de/acosix/alfresco/keycloak/share/web/UserGroupsLoadFilter.java @@ -245,7 +245,9 @@ public class UserGroupsLoadFilter implements DependencyInjectedFilter, Initializ } else { - LOGGER.warn("Failed to load user groups for {} with backend call resulting in HTTP {} response and message {}", userId, + // TODO Specific handling for expectable error codes (401 / 302) + LOGGER.warn("Failed to load user groups for {} with backend call resulting in HTTP response with status {} {}", + userId, res.getStatus().getCode(), res.getStatus().getMessage()); userGroupsCSVList = ""; }