From b1f97ada950ff4d1fdaac0dfe43c86b5a4ca8aa6 Mon Sep 17 00:00:00 2001 From: AFaust Date: Sun, 5 Dec 2021 12:48:02 +0100 Subject: [PATCH] Add couple of toString / simplify --- .../KeycloakAuthenticationComponent.java | 2 - .../KeycloakAuthenticationFilter.java | 127 +++++++++--------- .../KeycloakRemoteUserMapper.java | 3 - ...cloakWebScriptSSOAuthenticationFilter.java | 3 +- .../repo/roles/AggregateRoleNameMapper.java | 25 +++- .../repo/roles/PatternRoleNameMapper.java | 75 ++++++++--- .../roles/PrefixAttachingRoleNameMapper.java | 25 +++- .../repo/roles/StaticRoleNameFilter.java | 34 +++-- .../repo/roles/StaticRoleNameMapper.java | 54 +++++--- repository/src/test/docker/test-realm.json | 6 +- share/src/test/docker/test-realm.json | 6 +- 11 files changed, 232 insertions(+), 128 deletions(-) diff --git a/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakAuthenticationComponent.java b/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakAuthenticationComponent.java index 041c310..1dff46a 100644 --- a/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakAuthenticationComponent.java +++ b/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakAuthenticationComponent.java @@ -304,8 +304,6 @@ public class KeycloakAuthenticationComponent extends AbstractAuthenticationCompo throw new AuthenticationException("Failed to authenticate against Keycloak", atex); } - // TODO Override setCurrentUser to perform user existence validation and role retrieval for non-Keycloak logins - // (e.g. via public API setCurrentUser) this.setCurrentUser(realUserName); this.handleUserTokens(accessTokenHolder.getAccessToken(), accessTokenHolder.getIdToken(), true); } 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 8037d02..6865205 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 @@ -176,7 +176,7 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter /** * @param active - * the active to set + * the active to set */ public void setActive(final boolean active) { @@ -185,7 +185,7 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter /** * @param allowTicketLogon - * the allowTicketLogon to set + * the allowTicketLogon to set */ public void setAllowTicketLogon(final boolean allowTicketLogon) { @@ -194,7 +194,7 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter /** * @param allowHttpBasicLogon - * the allowHttpBasicLogon to set + * the allowHttpBasicLogon to set */ public void setAllowHttpBasicLogon(final boolean allowHttpBasicLogon) { @@ -203,7 +203,7 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter /** * @param handlePublicApi - * the handlePublicApi to set + * the handlePublicApi to set */ public void setHandlePublicApi(final boolean handlePublicApi) { @@ -212,7 +212,7 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter /** * @param loginPageUrl - * the loginPageUrl to set + * the loginPageUrl to set */ public void setLoginPageUrl(final String loginPageUrl) { @@ -221,7 +221,7 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter /** * @param originalRequestUrlHeaderName - * the originalRequestUrlHeaderName to set + * the originalRequestUrlHeaderName to set */ public void setOriginalRequestUrlHeaderName(final String originalRequestUrlHeaderName) { @@ -230,7 +230,7 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter /** * @param noKeycloakHandlingHeaderName - * the noKeycloakHandlingHeaderName to set + * the noKeycloakHandlingHeaderName to set */ public void setNoKeycloakHandlingHeaderName(final String noKeycloakHandlingHeaderName) { @@ -239,7 +239,7 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter /** * @param bodyBufferLimit - * the bodyBufferLimit to set + * the bodyBufferLimit to set */ public void setBodyBufferLimit(final int bodyBufferLimit) { @@ -248,7 +248,7 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter /** * @param keycloakDeployment - * the keycloakDeployment to set + * the keycloakDeployment to set */ public void setKeycloakDeployment(final KeycloakDeployment keycloakDeployment) { @@ -257,7 +257,7 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter /** * @param sessionIdMapper - * the sessionIdMapper to set + * the sessionIdMapper to set */ public void setSessionIdMapper(final SessionIdMapper sessionIdMapper) { @@ -266,7 +266,7 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter /** * @param keycloakAuthenticationComponent - * the keycloakAuthenticationComponent to set + * the keycloakAuthenticationComponent to set */ public void setKeycloakAuthenticationComponent(final KeycloakAuthenticationComponent keycloakAuthenticationComponent) { @@ -275,7 +275,7 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter /** * @param keycloakTicketTokenCache - * the keycloakTicketTokenCache to set + * the keycloakTicketTokenCache to set */ public void setKeycloakTicketTokenCache(final SimpleCache keycloakTicketTokenCache) { @@ -284,7 +284,7 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter /** * @param publicApiRuntimeContainer - * the publicApiRuntimeContainer to set + * the publicApiRuntimeContainer to set */ public void setPublicApiRuntimeContainer(final RuntimeContainer publicApiRuntimeContainer) { @@ -333,12 +333,12 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter * Checks and processes any HTTP Basic authentication if allowed. * * @param req - * the servlet request + * the servlet request * * @throws IOException - * if any error occurs during processing of HTTP Basic authentication + * if any error occurs during processing of HTTP Basic authentication * @throws ServletException - * if any error occurs during processing of HTTP Basic authentication + * if any error occurs during processing of HTTP Basic authentication * * @return {@code true} if an existing HTTP Basic authentication header was successfully processed, {@code false} otherwise */ @@ -415,17 +415,17 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter * will be terminated. Otherwise processing may continue with the filter chain (if still applicable). * * @param context - * the servlet context + * the servlet context * @param req - * the servlet request + * the servlet request * @param res - * the servlet response + * the servlet response * @param chain - * the filter chain + * the filter chain * @throws IOException - * if any error occurs during Keycloak authentication or processing of the filter chain + * if any error occurs during Keycloak authentication or processing of the filter chain * @throws ServletException - * if any error occurs during Keycloak authentication or processing of the filter chain + * if any error occurs during Keycloak authentication or processing of the filter chain */ protected void processKeycloakAuthenticationAndActions(final ServletContext context, final HttpServletRequest req, final HttpServletResponse res, final FilterChain chain) throws IOException, ServletException @@ -556,21 +556,21 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter * Processes a sucessfull authentication via Keycloak. * * @param context - * the servlet context + * the servlet context * @param req - * the servlet request + * the servlet request * @param res - * the servlet response + * the servlet response * @param chain - * the filter chain + * the filter chain * @param facade - * the Keycloak HTTP facade + * the Keycloak HTTP facade * @param tokenStore - * the Keycloak token store + * the Keycloak token store * @throws IOException - * if any error occurs during Keycloak authentication or processing of the filter chain + * if any error occurs during Keycloak authentication or processing of the filter chain * @throws ServletException - * if any error occurs during Keycloak authentication or processing of the filter chain + * if any error occurs during Keycloak authentication or processing of the filter chain */ protected void onKeycloakAuthenticationSuccess(final ServletContext context, final HttpServletRequest req, final HttpServletResponse res, final FilterChain chain, final OIDCServletHttpFacade facade, @@ -668,16 +668,16 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter * Processes a failed authentication via Keycloak. * * @param context - * the servlet context + * the servlet context * @param req - * the servlet request + * the servlet request * @param res - * the servlet response + * the servlet response * * @throws IOException - * if any error occurs during processing of the filter chain + * if any error occurs during processing of the filter chain * @throws ServletException - * if any error occurs during processing of the filter chain + * if any error occurs during processing of the filter chain */ protected void onKeycloakAuthenticationFailure(final ServletContext context, final HttpServletRequest req, final HttpServletResponse res) throws IOException, ServletException @@ -707,18 +707,18 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter * Checks if processing of the filter must be skipped for the specified request. * * @param context - * the servlet context + * the servlet context * @param req - * the servlet request to check for potential conditions to skip + * the servlet request to check for potential conditions to skip * @param res - * the servlet response on which potential updates of cookies / response headers need to be set + * the servlet response on which potential updates of cookies / response headers need to be set * @return {@code true} if processing of the {@link #doFilter(ServletContext, ServletRequest, ServletResponse, FilterChain) filter - * operation} must be skipped, {@code false} otherwise + * operation} must be skipped, {@code false} otherwise * * @throws IOException - * if any error occurs during inspection of the request + * if any error occurs during inspection of the request * @throws ServletException - * if any error occurs during inspection of the request + * if any error occurs during inspection of the request */ protected boolean checkForSkipCondition(final ServletContext context, final HttpServletRequest req, final HttpServletResponse res) throws IOException, ServletException @@ -772,11 +772,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()); + // even though we provide a remote user mapper, it may not be the first in the chain, so Bearer might not be processed (yet) and + // thus session not initialised + final AccessToken accessToken = session != null ? (AccessToken) session.getAttribute(KeycloakRemoteUserMapper.class.getName()) + : null; if (accessToken != null) { if (accessToken.isActive()) @@ -1011,13 +1010,13 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter * Checks whether a particular request is aimed at a Public v1 ReST API web script which does not require any authentication. * * @param req - * the request to check + * the request to check * @param servletPath - * the path to the servlet matching the request + * the path to the servlet matching the request * @param pathInfo - * the request path following the servlet path + * the request path following the servlet path * @return {@code true} if the request targets a Public v1 ReST API web script which does not require authentication, {@code false} - * otherwise + * otherwise */ protected boolean isNoAuthPublicRestApiWebScriptRequest(final HttpServletRequest req, final String servletPath, final String pathInfo) { @@ -1059,13 +1058,13 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter * necessary or configured. * * @param req - * the HTTP servlet request + * the HTTP servlet request * @param res - * the HTTP servlet response + * the HTTP servlet response * @param userId - * the ID of the authenticated user + * the ID of the authenticated user * @return {@code true} if processing of the {@link #doFilter(ServletContext, ServletRequest, ServletResponse, FilterChain) filter - * operation} can be skipped as the account represents a valid and still active authentication, {@code false} otherwise + * operation} can be skipped as the account represents a valid and still active authentication, {@code false} otherwise */ protected boolean validateAndRefreshKeycloakAuthentication(final HttpServletRequest req, final HttpServletResponse res, final String userId) @@ -1133,16 +1132,16 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter * Check if the request has specified a ticket parameter to bypass the standard authentication. * * @param context - * the servlet context + * the servlet context * @param req - * the request + * the request * @param resp - * the response + * the response * * @throws IOException - * if any error occurs during ticket processing + * if any error occurs during ticket processing * @throws ServletException - * if any error occurs during ticket processing + * if any error occurs during ticket processing * * @return boolean */ @@ -1202,7 +1201,7 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter * Checks if the HTTP request has set the Keycloak state cookie. * * @param req - * the HTTP request to check + * the HTTP request to check * @return {@code true} if the state cookie is set, {@code false} otherwise */ protected boolean hasStateCookie(final HttpServletRequest req) @@ -1219,11 +1218,11 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter * Resets any Keycloak-related state cookies present in the current request. * * @param context - * the servlet context + * the servlet context * @param req - * the servlet request + * the servlet request * @param res - * the servlet response + * the servlet response */ protected void resetStateCookies(final ServletContext context, final HttpServletRequest req, final HttpServletResponse res) { @@ -1247,7 +1246,7 @@ public class KeycloakAuthenticationFilter extends BaseAuthenticationFilter * technical default value in lieu of an explicitly configured value. * * @param req - * the incoming request + * the incoming request * @return the assumed SSL port to be used in redirects */ protected int determineLikelySslPort(final HttpServletRequest req) diff --git a/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakRemoteUserMapper.java b/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakRemoteUserMapper.java index 7842e3a..a4f525d 100644 --- a/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakRemoteUserMapper.java +++ b/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakRemoteUserMapper.java @@ -121,9 +121,6 @@ public class KeycloakRemoteUserMapper implements RemoteUserMapper, ActivateableB final BearerTokenRequestAuthenticator authenticator = new BearerTokenRequestAuthenticator(this.keycloakDeployment); final AuthOutcome authOutcome = authenticator.authenticate(httpFacade); - // TODO Check on how to enable / add client/audience validation - // currently, Share token seems to be valid here, which it shouldn't be - // also, Share token may not contain Alfresco client roles (e.g. admin) if (authOutcome == AuthOutcome.AUTHENTICATED) { final AccessToken token = authenticator.getToken(); diff --git a/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakWebScriptSSOAuthenticationFilter.java b/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakWebScriptSSOAuthenticationFilter.java index 5767154..a4ace4b 100644 --- a/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakWebScriptSSOAuthenticationFilter.java +++ b/repository/src/main/java/de/acosix/alfresco/keycloak/repo/authentication/KeycloakWebScriptSSOAuthenticationFilter.java @@ -34,7 +34,6 @@ import org.apache.commons.logging.LogFactory; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.InitializingBean; -import org.springframework.extensions.surf.util.URLDecoder; import org.springframework.extensions.webscripts.Description.RequiredAuthentication; import org.springframework.extensions.webscripts.Match; import org.springframework.extensions.webscripts.RuntimeContainer; @@ -116,7 +115,7 @@ public class KeycloakWebScriptSSOAuthenticationFilter extends BaseAuthentication LOGGER.debug("Processing request: {} SID: {}", pathInfo, req.getSession(false) != null ? req.getSession().getId() : null); - final Match match = this.container.getRegistry().findWebScript(req.getMethod(), URLDecoder.decode(pathInfo)); + final Match match = this.container.getRegistry().findWebScript(req.getMethod(), pathInfo); if (match != null && match.getWebScript() != null) { final RequiredAuthentication reqAuth = match.getWebScript().getDescription().getRequiredAuthentication(); diff --git a/repository/src/main/java/de/acosix/alfresco/keycloak/repo/roles/AggregateRoleNameMapper.java b/repository/src/main/java/de/acosix/alfresco/keycloak/repo/roles/AggregateRoleNameMapper.java index a889707..e9153b7 100644 --- a/repository/src/main/java/de/acosix/alfresco/keycloak/repo/roles/AggregateRoleNameMapper.java +++ b/repository/src/main/java/de/acosix/alfresco/keycloak/repo/roles/AggregateRoleNameMapper.java @@ -52,7 +52,7 @@ public class AggregateRoleNameMapper implements InitializingBean, RoleNameMapper /** * @param granularMappers - * the granularMappers to set + * the granularMappers to set */ public void setGranularMappers(final List granularMappers) { @@ -61,7 +61,7 @@ public class AggregateRoleNameMapper implements InitializingBean, RoleNameMapper /** * @param upperCaseRoles - * the upperCaseRoles to set + * the upperCaseRoles to set */ public void setUpperCaseRoles(final boolean upperCaseRoles) { @@ -109,4 +109,25 @@ public class AggregateRoleNameMapper implements InitializingBean, RoleNameMapper } return mappedName; } + + /** + * {@inheritDoc} + */ + @Override + public String toString() + { + final StringBuilder builder = new StringBuilder(); + builder.append("AggregateRoleNameMapper ["); + if (this.granularMappers != null) + { + builder.append("granularMappers="); + builder.append(this.granularMappers); + builder.append(", "); + } + builder.append("upperCaseRoles="); + builder.append(this.upperCaseRoles); + builder.append("]"); + return builder.toString(); + } + } diff --git a/repository/src/main/java/de/acosix/alfresco/keycloak/repo/roles/PatternRoleNameMapper.java b/repository/src/main/java/de/acosix/alfresco/keycloak/repo/roles/PatternRoleNameMapper.java index 41bf63e..ed9c489 100644 --- a/repository/src/main/java/de/acosix/alfresco/keycloak/repo/roles/PatternRoleNameMapper.java +++ b/repository/src/main/java/de/acosix/alfresco/keycloak/repo/roles/PatternRoleNameMapper.java @@ -15,6 +15,7 @@ */ package de.acosix.alfresco.keycloak.repo.roles; +import java.util.HashMap; import java.util.Locale; import java.util.Map; import java.util.Optional; @@ -34,33 +35,41 @@ public class PatternRoleNameMapper implements RoleNameMapper private static final Logger LOGGER = LoggerFactory.getLogger(PatternRoleNameMapper.class); - protected Map patternMappings; + protected final Map patternMappings = new HashMap<>(); - protected Map patternInverseMappings; + protected final Map patternInverseMappings = new HashMap<>(); protected boolean upperCaseRoles; /** * @param patternMappings - * the patternMappings to set + * the patternMappings to set */ public void setPatternMappings(final Map patternMappings) { - this.patternMappings = patternMappings; + this.patternMappings.clear(); + if (patternMappings != null) + { + this.patternMappings.putAll(patternMappings); + } } /** * @param patternInverseMappings - * the patternInverseMappings to set + * the patternInverseMappings to set */ public void setPatternInverseMappings(final Map patternInverseMappings) { - this.patternInverseMappings = patternInverseMappings; + this.patternInverseMappings.clear(); + if (patternInverseMappings != null) + { + this.patternInverseMappings.putAll(patternInverseMappings); + } } /** * @param upperCaseRoles - * the upperCaseRoles to set + * the upperCaseRoles to set */ public void setUpperCaseRoles(final boolean upperCaseRoles) { @@ -75,23 +84,18 @@ public class PatternRoleNameMapper implements RoleNameMapper { ParameterCheck.mandatoryString("roleName", roleName); - Optional result = Optional.empty(); + final Optional matchingPattern = this.patternMappings.keySet().stream().filter(roleName::matches).findFirst(); + final Optional result = matchingPattern.map(pattern -> { + final String replacement = this.patternMappings.get(pattern); + LOGGER.debug("Role {} matches mapping pattern {} - applying replacement pattern {}", roleName, pattern, replacement); + final String mappedName = roleName.replaceAll(pattern, replacement); + LOGGER.debug("Mapped role {} to {}", roleName, mappedName); + return mappedName; + }).map(name -> this.upperCaseRoles ? name.toUpperCase(Locale.ENGLISH) : name); - if (this.patternMappings != null) + if (!result.isPresent()) { - final Optional matchingPattern = this.patternMappings.keySet().stream().filter(roleName::matches).findFirst(); - result = matchingPattern.map(pattern -> { - final String replacement = this.patternMappings.get(pattern); - LOGGER.debug("Role {} matches mapping pattern {} - applying replacement pattern {}", roleName, pattern, replacement); - final String mappedName = roleName.replaceAll(pattern, replacement); - LOGGER.debug("Mapped role {} to {}", roleName, mappedName); - return mappedName; - }).map(name -> this.upperCaseRoles ? name.toUpperCase(Locale.ENGLISH) : name); - - if (!result.isPresent()) - { - LOGGER.debug("No matching pattern applies to role {}", roleName); - } + LOGGER.debug("No matching pattern applies to role {}", roleName); } return result; @@ -125,4 +129,31 @@ public class PatternRoleNameMapper implements RoleNameMapper return result; } + + /** + * {@inheritDoc} + */ + @Override + public String toString() + { + final StringBuilder builder = new StringBuilder(); + builder.append("PatternRoleNameMapper ["); + if (this.patternMappings != null) + { + builder.append("patternMappings="); + builder.append(this.patternMappings); + builder.append(", "); + } + if (this.patternInverseMappings != null) + { + builder.append("patternInverseMappings="); + builder.append(this.patternInverseMappings); + builder.append(", "); + } + builder.append("upperCaseRoles="); + builder.append(this.upperCaseRoles); + builder.append("]"); + return builder.toString(); + } + } diff --git a/repository/src/main/java/de/acosix/alfresco/keycloak/repo/roles/PrefixAttachingRoleNameMapper.java b/repository/src/main/java/de/acosix/alfresco/keycloak/repo/roles/PrefixAttachingRoleNameMapper.java index 8a448e8..4beb336 100644 --- a/repository/src/main/java/de/acosix/alfresco/keycloak/repo/roles/PrefixAttachingRoleNameMapper.java +++ b/repository/src/main/java/de/acosix/alfresco/keycloak/repo/roles/PrefixAttachingRoleNameMapper.java @@ -39,7 +39,7 @@ public class PrefixAttachingRoleNameMapper implements RoleNameMapper /** * @param prefix - * the prefix to set + * the prefix to set */ public void setPrefix(final String prefix) { @@ -48,7 +48,7 @@ public class PrefixAttachingRoleNameMapper implements RoleNameMapper /** * @param upperCaseRoles - * the upperCaseRoles to set + * the upperCaseRoles to set */ public void setUpperCaseRoles(final boolean upperCaseRoles) { @@ -96,4 +96,25 @@ public class PrefixAttachingRoleNameMapper implements RoleNameMapper return result; } + + /** + * {@inheritDoc} + */ + @Override + public String toString() + { + final StringBuilder builder = new StringBuilder(); + builder.append("PrefixAttachingRoleNameMapper ["); + if (this.prefix != null) + { + builder.append("prefix="); + builder.append(this.prefix); + builder.append(", "); + } + builder.append("upperCaseRoles="); + builder.append(this.upperCaseRoles); + builder.append("]"); + return builder.toString(); + } + } diff --git a/repository/src/main/java/de/acosix/alfresco/keycloak/repo/roles/StaticRoleNameFilter.java b/repository/src/main/java/de/acosix/alfresco/keycloak/repo/roles/StaticRoleNameFilter.java index ecd0f86..e92e8b7 100644 --- a/repository/src/main/java/de/acosix/alfresco/keycloak/repo/roles/StaticRoleNameFilter.java +++ b/repository/src/main/java/de/acosix/alfresco/keycloak/repo/roles/StaticRoleNameFilter.java @@ -15,6 +15,7 @@ */ package de.acosix.alfresco.keycloak.repo.roles; +import java.util.HashSet; import java.util.Set; import org.alfresco.util.ParameterCheck; @@ -31,15 +32,19 @@ public class StaticRoleNameFilter implements RoleNameFilter private static final Logger LOGGER = LoggerFactory.getLogger(StaticRoleNameFilter.class); - protected Set allowedRoles; + protected final Set allowedRoles = new HashSet<>(); /** * @param allowedRoles - * the allowedRoles to set + * the allowedRoles to set */ public void setAllowedRoles(final Set allowedRoles) { - this.allowedRoles = allowedRoles; + this.allowedRoles.clear(); + if (allowedRoles != null) + { + this.allowedRoles.addAll(allowedRoles); + } } /** @@ -50,15 +55,24 @@ public class StaticRoleNameFilter implements RoleNameFilter { ParameterCheck.mandatoryString("roleName", roleName); - boolean exposed = false; - - if (this.allowedRoles != null) - { - exposed = this.allowedRoles.contains(roleName); - LOGGER.debug("Determined exposure flag of {} for role {} using a static match set", exposed, roleName); - } + final boolean exposed = this.allowedRoles.contains(roleName); + LOGGER.debug("Determined exposure flag of {} for role {} using a static match set", exposed, roleName); return exposed; } + /** + * {@inheritDoc} + */ + @Override + public String toString() + { + final StringBuilder builder = new StringBuilder(); + builder.append("StaticRoleNameFilter ["); + builder.append("allowedRoles="); + builder.append(this.allowedRoles); + builder.append("]"); + return builder.toString(); + } + } diff --git a/repository/src/main/java/de/acosix/alfresco/keycloak/repo/roles/StaticRoleNameMapper.java b/repository/src/main/java/de/acosix/alfresco/keycloak/repo/roles/StaticRoleNameMapper.java index 8b8b69e..70c855f 100644 --- a/repository/src/main/java/de/acosix/alfresco/keycloak/repo/roles/StaticRoleNameMapper.java +++ b/repository/src/main/java/de/acosix/alfresco/keycloak/repo/roles/StaticRoleNameMapper.java @@ -15,6 +15,7 @@ */ package de.acosix.alfresco.keycloak.repo.roles; +import java.util.HashMap; import java.util.Locale; import java.util.Map; import java.util.Map.Entry; @@ -34,22 +35,26 @@ public class StaticRoleNameMapper implements RoleNameMapper private static final Logger LOGGER = LoggerFactory.getLogger(StaticRoleNameMapper.class); - protected Map nameMappings; + protected final Map nameMappings = new HashMap<>(); protected boolean upperCaseRoles; /** * @param nameMappings - * the nameMappings to set + * the nameMappings to set */ public void setNameMappings(final Map nameMappings) { - this.nameMappings = nameMappings; + this.nameMappings.clear(); + if (nameMappings != null) + { + this.nameMappings.putAll(nameMappings); + } } /** * @param upperCaseRoles - * the upperCaseRoles to set + * the upperCaseRoles to set */ public void setUpperCaseRoles(final boolean upperCaseRoles) { @@ -93,24 +98,39 @@ public class StaticRoleNameMapper implements RoleNameMapper Optional result = Optional.empty(); - if (this.nameMappings != null) + for (final Entry entry : this.nameMappings.entrySet()) { - for (final Entry entry : this.nameMappings.entrySet()) + if (entry.getValue().equals(authorityName) || (this.upperCaseRoles && entry.getValue().equalsIgnoreCase(authorityName))) { - if (entry.getValue().equals(authorityName) || (this.upperCaseRoles && entry.getValue().equalsIgnoreCase(authorityName))) - { - final String mappedName = entry.getKey(); - LOGGER.debug("Mapped authority name {} to {} using static mapping", authorityName, mappedName); - result = Optional.of(mappedName); - break; - } - } - if (!result.isPresent()) - { - LOGGER.debug("No static mapping applies to authority name {}", authorityName); + final String mappedName = entry.getKey(); + LOGGER.debug("Mapped authority name {} to {} using static mapping", authorityName, mappedName); + result = Optional.of(mappedName); + break; } } + if (!result.isPresent()) + { + LOGGER.debug("No static mapping applies to authority name {}", authorityName); + } + return result; } + + /** + * {@inheritDoc} + */ + @Override + public String toString() + { + final StringBuilder builder = new StringBuilder(); + builder.append("StaticRoleNameMapper ["); + builder.append("nameMappings="); + builder.append(this.nameMappings); + builder.append(", "); + builder.append("upperCaseRoles="); + builder.append(this.upperCaseRoles); + builder.append("]"); + return builder.toString(); + } } diff --git a/repository/src/test/docker/test-realm.json b/repository/src/test/docker/test-realm.json index 0561241..df72400 100644 --- a/repository/src/test/docker/test-realm.json +++ b/repository/src/test/docker/test-realm.json @@ -574,7 +574,8 @@ { "clientScope": "alfresco-role-service", "roles": [ - "view-clients" + "view-clients", + "view-realm" ] } ], @@ -1135,7 +1136,8 @@ "query-groups", "query-users", "view-users", - "view-clients" + "view-clients", + "view-realm" ] } }, diff --git a/share/src/test/docker/test-realm.json b/share/src/test/docker/test-realm.json index 1b18443..a97a9ef 100644 --- a/share/src/test/docker/test-realm.json +++ b/share/src/test/docker/test-realm.json @@ -609,7 +609,8 @@ { "clientScope": "alfresco-role-service", "roles": [ - "view-clients" + "view-clients", + "view-realm" ] } ], @@ -1205,7 +1206,8 @@ "query-groups", "query-users", "view-users", - "view-clients" + "view-clients", + "view-realm" ] } },