From 0cec42b5288f74952d79b5ca483ba5c2ef9170c2 Mon Sep 17 00:00:00 2001 From: chasegawa Date: Wed, 29 Sep 2021 12:43:00 -0700 Subject: [PATCH] SHIBUI-2111 fixes to role issues when using IDP logins --- .../springsecurity/AdminUserService.java | 9 +++-- .../shibui/pac4j/BetterSAML2Profile.java | 3 +- .../shibui/pac4j/Pac4jConfiguration.java | 35 +++---------------- testbed/authentication/docker-compose.yml | 6 +++- 4 files changed, 14 insertions(+), 39 deletions(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/springsecurity/AdminUserService.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/springsecurity/AdminUserService.java index a2cab06e2..8782362a4 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/springsecurity/AdminUserService.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/springsecurity/AdminUserService.java @@ -2,7 +2,7 @@ import edu.internet2.tier.shibboleth.admin.ui.security.model.Role; import edu.internet2.tier.shibboleth.admin.ui.security.model.User; -import edu.internet2.tier.shibboleth.admin.ui.security.repository.UserRepository; +import edu.internet2.tier.shibboleth.admin.ui.security.service.UserService; import lombok.RequiredArgsConstructor; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.authority.SimpleGrantedAuthority; @@ -22,12 +22,12 @@ @RequiredArgsConstructor public class AdminUserService implements UserDetailsService { - private final UserRepository userRepository; + private final UserService userService; @Override @Transactional(readOnly = true) public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException { - User user = userRepository + User user = userService .findByUsername(username) .orElseThrow(() -> new UsernameNotFoundException(String.format("User [%s] is not found", username))); @@ -43,5 +43,4 @@ public UserDetails loadUserByUsername(String username) throws UsernameNotFoundEx return new org.springframework.security.core.userdetails.User(user.getUsername(), user.getPassword(), grantedAuthorities); } -} - +} \ No newline at end of file diff --git a/pac4j-module/src/main/java/net/unicon/shibui/pac4j/BetterSAML2Profile.java b/pac4j-module/src/main/java/net/unicon/shibui/pac4j/BetterSAML2Profile.java index 1411c2997..1036781e0 100644 --- a/pac4j-module/src/main/java/net/unicon/shibui/pac4j/BetterSAML2Profile.java +++ b/pac4j-module/src/main/java/net/unicon/shibui/pac4j/BetterSAML2Profile.java @@ -4,7 +4,6 @@ import org.pac4j.saml.profile.SAML2Profile; import java.util.Collection; -import java.util.HashSet; import java.util.List; import java.util.Set; @@ -42,7 +41,7 @@ public List getGroups() { } public Set getRoles() { - Set result = new HashSet<>(); + Set result = super.getRoles(); List assertedRoles = (List) getAttribute(profileMapping.getRoles()); if (assertedRoles != null) { result.addAll(assertedRoles); diff --git a/pac4j-module/src/main/java/net/unicon/shibui/pac4j/Pac4jConfiguration.java b/pac4j-module/src/main/java/net/unicon/shibui/pac4j/Pac4jConfiguration.java index 37a33b5ae..26d32708f 100644 --- a/pac4j-module/src/main/java/net/unicon/shibui/pac4j/Pac4jConfiguration.java +++ b/pac4j-module/src/main/java/net/unicon/shibui/pac4j/Pac4jConfiguration.java @@ -4,16 +4,11 @@ import edu.internet2.tier.shibboleth.admin.ui.security.repository.UserRepository; import edu.internet2.tier.shibboleth.admin.ui.security.service.UserService; import lombok.extern.slf4j.Slf4j; -import org.apache.commons.lang3.StringUtils; +import net.unicon.shibui.pac4j.authenticator.ShibuiPac4JHeaderClientAuthenticator; +import net.unicon.shibui.pac4j.authenticator.ShibuiSAML2Authenticator; import org.pac4j.core.client.Clients; import org.pac4j.core.config.Config; -import org.pac4j.core.context.WebContext; -import org.pac4j.core.credentials.Credentials; -import org.pac4j.core.credentials.TokenCredentials; -import org.pac4j.core.credentials.authenticator.Authenticator; -import org.pac4j.core.exception.CredentialsException; import org.pac4j.core.matching.PathMatcher; -import org.pac4j.core.profile.CommonProfile; import org.pac4j.core.profile.definition.CommonProfileDefinition; import org.pac4j.http.client.direct.HeaderClient; import org.pac4j.saml.client.SAML2Client; @@ -66,29 +61,7 @@ public Config config(final Pac4jConfigurationProperties pac4jConfigProps, switch (pac4jConfigProps.getTypeOfAuth()) { case "HEADER": { log.info("**** Configuring PAC4J Header Client"); - HeaderClient headerClient = new HeaderClient(pac4jConfigProps.getAuthenticationHeader(), - new Authenticator() { - @Override - public void validate(Credentials credentials, WebContext context) { - { - if (credentials instanceof TokenCredentials) { - TokenCredentials creds = (TokenCredentials) credentials; - String token = creds.getToken(); - if (StringUtils.isAllBlank(token)) { - throw new CredentialsException("Supplied token value in header was missing or blank"); - } - } else { - throw new CredentialsException("Invalid Credentials object generated by HeaderClient"); - } - final CommonProfile profile = new CommonProfile(); - String token = ((TokenCredentials)credentials).getToken(); - profile.setId(token); - profile.addAttribute("username", token); - profile.setRoles(userService.getUserRoles(token)); - credentials.setUserProfile(profile); - } - } - }); + HeaderClient headerClient = new HeaderClient(pac4jConfigProps.getAuthenticationHeader(), new ShibuiPac4JHeaderClientAuthenticator(userService)); headerClient.setName(PAC4J_CLIENT_NAME); clients.setClients(headerClient); break; @@ -110,7 +83,7 @@ public void validate(Credentials credentials, WebContext context) { final SAML2Client saml2Client = new SAML2Client(saml2Config); saml2Client.addAuthorizationGenerator(saml2ModelAuthorizationGenerator); - SAML2Authenticator saml2Authenticator = new SAML2Authenticator(saml2Config.getAttributeAsId(), saml2Config.getMappedAttributes()); + SAML2Authenticator saml2Authenticator = new ShibuiSAML2Authenticator(saml2Config.getAttributeAsId(), saml2Config.getMappedAttributes(), userService); saml2Authenticator.setProfileDefinition(new CommonProfileDefinition(p -> new BetterSAML2Profile(pac4jConfigProps.getSimpleProfileMapping()))); saml2Client.setAuthenticator(saml2Authenticator); diff --git a/testbed/authentication/docker-compose.yml b/testbed/authentication/docker-compose.yml index 1ed95975b..884042c4a 100644 --- a/testbed/authentication/docker-compose.yml +++ b/testbed/authentication/docker-compose.yml @@ -20,6 +20,7 @@ services: - "8080:8080" - "443:443" - "8443:8443" +# - "8000:8000" volumes: - /var/run/docker.sock:/var/run/docker.sock - ../reverse-proxy/:/configuration/ @@ -69,6 +70,9 @@ services: volumes: - ./shibui:/conf - ./shibui/application.yml:/application.yml + ports: + - "8000:8000" + entrypoint: ["/usr/bin/java", "-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=*:8000", "-jar", "app.war"] networks: reverse-proxy: idp: @@ -76,4 +80,4 @@ volumes: directory_data: driver: local directory_config: - driver: local + driver: local \ No newline at end of file