From bd0d11b924a124721dd7b5faace242b1a5d95e72 Mon Sep 17 00:00:00 2001 From: chasegawa Date: Tue, 24 Aug 2021 11:10:30 -0700 Subject: [PATCH 1/2] SHIBUI-2015 parsing group from shib --- .../ui/security/service/UserService.java | 27 ++++--- .../unicon/shibui/pac4j/AddNewUserFilter.java | 80 ++++++++++++------- .../shibui/pac4j/BetterSAML2Profile.java | 10 ++- .../shibui/pac4j/Pac4jConfiguration.java | 16 ++-- .../pac4j/Pac4jConfigurationProperties.java | 6 +- .../net/unicon/shibui/pac4j/WebSecurity.java | 39 +++++---- .../src/main/resources/application.yml | 4 +- ...roovy => AddNewUserFilterMockTests.groovy} | 22 ++--- 8 files changed, 116 insertions(+), 88 deletions(-) rename pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/{AddNewUserFilterTests.groovy => AddNewUserFilterMockTests.groovy} (82%) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java index a04e2b574..cc579cb70 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/UserService.java @@ -50,20 +50,24 @@ public boolean currentUserIsAdmin() { User user = getCurrentUser(); return user != null && user.getRole().equals("ROLE_ADMIN"); } - + @Transactional public void delete(String username) throws EntityNotFoundException, OwnershipConflictException { Optional userToRemove = userRepository.findByUsername(username); if (userToRemove.isEmpty()) throw new EntityNotFoundException("User does not exist"); if (!ownershipRepository.findOwnedByUser(username).isEmpty()) throw new OwnershipConflictException("User ["+username+"] has ownership of entities in the system. Please remove all items before attemtping to delete the user."); - + // ok, user exists and doesn't own anything in the system, so delete them // If the user is owned by anything, clear that first ownershipRepository.clearUsersGroups(username); - User user = userToRemove.get(); + User user = userToRemove.get(); userRepository.delete(user); } - + + public Optional findByUsername(String username) { + return userRepository.findByUsername(username); + } + public User getCurrentUser() { //TODO: Consider returning an Optional here User user = null; @@ -82,7 +86,7 @@ public User getCurrentUser() { public UserAccess getCurrentUserAccess() { User user = getCurrentUser(); if (user == null) { - return UserAccess.NONE; + return UserAccess.NONE; } if (user.getRole().equals("ROLE_ADMIN")) { return UserAccess.ADMIN; @@ -92,7 +96,7 @@ public UserAccess getCurrentUserAccess() { } return UserAccess.NONE; } - + public Group getCurrentUserGroup() { switch (getCurrentUserAccess()) { case ADMIN: @@ -101,13 +105,11 @@ public Group getCurrentUserGroup() { return getCurrentUser().getGroup(); } } - + public Set getUserRoles(String username) { Optional user = userRepository.findByUsername(username); HashSet result = new HashSet<>(); - if (user.isPresent() ) { - user.get().getRoles().forEach(role -> result.add(role.getName())); - } + user.ifPresent(value -> value.getRoles().forEach(role -> result.add(role.getName()))); return result; } @@ -128,7 +130,7 @@ public boolean isAuthorizedFor(Ownable ownableObject) { return false; } } - + /** * Creating users should always have a group. If the user isn't assigned to a group, create one based on their name. * If the user has the ADMIN role, they are always solely assigned to the admin group. @@ -175,14 +177,13 @@ public User save(User user) { } return userRepository.saveAndFlush(user); } - + /** * Given a user with a defined User.role, update the User.roles collection with that role. * * This currently exists because users should only ever have one role in the system at this time. However, user * roles are persisted as a set of roles (for future-proofing). Once we start allowing a user to have multiple roles, * this method and User.role can go away. - * @param user */ public void updateUserRole(User user) { if (StringUtils.isNotBlank(user.getRole())) { diff --git a/pac4j-module/src/main/java/net/unicon/shibui/pac4j/AddNewUserFilter.java b/pac4j-module/src/main/java/net/unicon/shibui/pac4j/AddNewUserFilter.java index c379690c5..b0f92d3ae 100644 --- a/pac4j-module/src/main/java/net/unicon/shibui/pac4j/AddNewUserFilter.java +++ b/pac4j-module/src/main/java/net/unicon/shibui/pac4j/AddNewUserFilter.java @@ -1,69 +1,60 @@ package net.unicon.shibui.pac4j; -import com.fasterxml.jackson.databind.ObjectMapper; -import edu.internet2.tier.shibboleth.admin.ui.controller.ErrorResponse; +import edu.internet2.tier.shibboleth.admin.ui.security.exception.GroupExistsConflictException; +import edu.internet2.tier.shibboleth.admin.ui.security.model.Group; 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.RoleRepository; -import edu.internet2.tier.shibboleth.admin.ui.security.repository.UserRepository; +import edu.internet2.tier.shibboleth.admin.ui.security.service.IGroupService; +import edu.internet2.tier.shibboleth.admin.ui.security.service.UserService; import edu.internet2.tier.shibboleth.admin.ui.service.EmailService; - +import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.RandomStringUtils; import org.pac4j.core.context.JEEContext; import org.pac4j.core.context.session.JEESessionStore; import org.pac4j.core.matching.matcher.Matcher; import org.pac4j.core.profile.CommonProfile; -import org.pac4j.saml.profile.SAML2Profile; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.springframework.security.core.Authentication; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.crypto.bcrypt.BCrypt; import javax.mail.MessagingException; -import javax.servlet.Filter; -import javax.servlet.FilterChain; -import javax.servlet.FilterConfig; -import javax.servlet.ServletException; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; +import javax.servlet.*; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.transaction.Transactional; - import java.io.IOException; -import java.util.List; -import java.util.Optional; - -import lombok.extern.slf4j.Slf4j; +import java.util.*; @Slf4j public class AddNewUserFilter implements Filter { private static final String ROLE_NONE = "ROLE_NONE"; private Optional emailService; + private IGroupService groupService; + private Matcher matcher; private Pac4jConfigurationProperties pac4jConfigurationProperties; private RoleRepository roleRepository; private Pac4jConfigurationProperties.SimpleProfileMapping simpleProfileMapping; - private UserRepository userRepository; - private Matcher matcher; + private UserService userService; - public AddNewUserFilter(Pac4jConfigurationProperties pac4jConfigurationProperties, UserRepository userRepository, RoleRepository roleRepository, Matcher matcher, Optional emailService) { - this.userRepository = userRepository; + public AddNewUserFilter(Pac4jConfigurationProperties pac4jConfigurationProperties, UserService userService, RoleRepository roleRepository, Matcher matcher, IGroupService groupService, Optional emailService) { + this.userService = userService; this.roleRepository = roleRepository; this.emailService = emailService; this.pac4jConfigurationProperties = pac4jConfigurationProperties; this.matcher = matcher; + this.groupService = groupService; simpleProfileMapping = this.pac4jConfigurationProperties.getSimpleProfileMapping(); } @Transactional - private User buildAndPersistNewUserFromProfile(CommonProfile profile) { + User buildAndPersistNewUserFromProfile(CommonProfile profile) { Optional noRole = roleRepository.findByName(ROLE_NONE); Role newUserRole; if (noRole.isEmpty()) { newUserRole = new Role(ROLE_NONE); - newUserRole = roleRepository.save(newUserRole); + roleRepository.save(newUserRole); } newUserRole = noRole.get(); @@ -74,7 +65,23 @@ private User buildAndPersistNewUserFromProfile(CommonProfile profile) { user.setFirstName(profile.getFirstName()); user.setLastName(profile.getFamilyName()); user.setEmailAddress(profile.getEmail()); - User persistedUser = userRepository.save(user); + + // get profile attribute for groups + Object obj = profile.getAttribute(simpleProfileMapping.getGroupsName()); + if (obj != null) { + final ArrayList groupNames = new ArrayList<>(); + if (obj instanceof String) { + groupNames.add(obj.toString()); + } + if (obj instanceof List) { + ((List)obj).forEach(val -> groupNames.add(val.toString())); + } + if (!groupNames.isEmpty()) { + user.setUserGroups(findOrCreateGroups(groupNames)); + } + } + + User persistedUser = userService.save(user); if (log.isDebugEnabled()) { log.debug("Persisted new user:\n" + user); } @@ -97,7 +104,7 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha if (profile != null) { String username = profile.getUsername(); if (username != null) { - Optional persistedUser = userRepository.findByUsername(username); + Optional persistedUser = userService.findByUsername(username); User user; if (persistedUser.isEmpty()) { user = buildAndPersistNewUserFromProfile(profile); @@ -122,7 +129,26 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha } } + private Set findOrCreateGroups(ArrayList groupNames) { + final HashSet result = new HashSet<>(); + groupNames.forEach(name -> { + Group g = groupService.find(name); + if (g == null) { + g = new Group(); + g.setResourceId(name); + g.setName(name); + try { + groupService.createGroup(g); + } + catch (GroupExistsConflictException shouldntHappen) { + } + } + result.add(g); + }); + return result; + } + @Override public void init(FilterConfig filterConfig) throws ServletException { } -} +} \ 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 17a5edd64..7bef931f8 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 @@ -1,10 +1,10 @@ package net.unicon.shibui.pac4j; -import org.pac4j.saml.profile.SAML2Profile; - import net.unicon.shibui.pac4j.Pac4jConfigurationProperties.SimpleProfileMapping; +import org.pac4j.saml.profile.SAML2Profile; import java.util.Collection; +import java.util.List; public class BetterSAML2Profile extends SAML2Profile { private SimpleProfileMapping profileMapping; @@ -28,6 +28,10 @@ public String getFirstName() { return (String) getAttribute(profileMapping.getFirstName()); } + public List getGroups() { + return (List) getAttribute(profileMapping.getGroupsName()); + } + @Override public String getUsername() { Object username = getAttribute(profileMapping.getUsername()); @@ -38,4 +42,4 @@ public String getUsername() { } } -} +} \ No newline at end of file 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 5e173f151..39f12035b 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 @@ -1,5 +1,9 @@ package net.unicon.shibui.pac4j; +import com.google.common.collect.Lists; +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 org.pac4j.core.client.Clients; import org.pac4j.core.config.Config; @@ -17,7 +21,6 @@ import org.pac4j.saml.config.SAML2Configuration; import org.pac4j.saml.credentials.authenticator.SAML2Authenticator; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.boot.autoconfigure.AutoConfigureOrder; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.web.server.ErrorPage; import org.springframework.boot.web.server.ErrorPageRegistrar; @@ -26,12 +29,6 @@ import org.springframework.context.annotation.Configuration; import org.springframework.http.HttpStatus; -import com.google.common.collect.Lists; - -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; - /** * Configuration setup here following readme from - https://github.com/pac4j/spring-security-pac4j/tree/5.0.x * NOTE: matchers are now done as part of the config and have been moved over from the WebSecurity.java class of this package @@ -85,7 +82,6 @@ public Config config(final Pac4jConfigurationProperties pac4jConfigProps, //saml2Config.setSpLogoutRequestBindingType(pac4jConfigProps.getSpLogoutRequestBindingType()); final SAML2Client saml2Client = new SAML2Client(saml2Config); - saml2Client.setName("Saml2Client"); saml2Client.addAuthorizationGenerator(saml2ModelAuthorizationGenerator); SAML2Authenticator saml2Authenticator = new SAML2Authenticator(saml2Config.getAttributeAsId(), saml2Config.getMappedAttributes()); saml2Authenticator.setProfileDefinition(new CommonProfileDefinition(p -> new BetterSAML2Profile(pac4jConfigProps.getSimpleProfileMapping()))); @@ -110,7 +106,7 @@ public void validate(Credentials credentials, WebContext context, SessionStore s throw new CredentialsException("Invalid Credentials object generated by HeaderClient"); } final CommonProfile profile = new CommonProfile(); - String token = ((TokenCredentials)credentials).getToken(); + String token = ((TokenCredentials)credentials).getToken(); profile.setId(token); profile.addAttribute("username", token); profile.setRoles(userService.getUserRoles(token)); @@ -134,4 +130,4 @@ private void registerErrorPages(ErrorPageRegistry registry) { registry.addErrorPages(new ErrorPage(HttpStatus.UNAUTHORIZED, "/unsecured/error.html")); registry.addErrorPages(new ErrorPage(HttpStatus.FORBIDDEN, "/unsecured/error.html")); } -} +} \ No newline at end of file diff --git a/pac4j-module/src/main/java/net/unicon/shibui/pac4j/Pac4jConfigurationProperties.java b/pac4j-module/src/main/java/net/unicon/shibui/pac4j/Pac4jConfigurationProperties.java index 977164958..68b1925fe 100644 --- a/pac4j-module/src/main/java/net/unicon/shibui/pac4j/Pac4jConfigurationProperties.java +++ b/pac4j-module/src/main/java/net/unicon/shibui/pac4j/Pac4jConfigurationProperties.java @@ -40,8 +40,8 @@ public class Pac4jConfigurationProperties { public static class SimpleProfileMapping { private String email; private String firstName; + private String groupsName; private String lastName; private String username; - } -} - + } +} \ No newline at end of file diff --git a/pac4j-module/src/main/java/net/unicon/shibui/pac4j/WebSecurity.java b/pac4j-module/src/main/java/net/unicon/shibui/pac4j/WebSecurity.java index b16af86ba..ac7633afa 100644 --- a/pac4j-module/src/main/java/net/unicon/shibui/pac4j/WebSecurity.java +++ b/pac4j-module/src/main/java/net/unicon/shibui/pac4j/WebSecurity.java @@ -2,16 +2,13 @@ import edu.internet2.tier.shibboleth.admin.ui.configuration.auto.EmailConfiguration; import edu.internet2.tier.shibboleth.admin.ui.security.repository.RoleRepository; -import edu.internet2.tier.shibboleth.admin.ui.security.repository.UserRepository; +import edu.internet2.tier.shibboleth.admin.ui.security.service.IGroupService; +import edu.internet2.tier.shibboleth.admin.ui.security.service.UserService; import edu.internet2.tier.shibboleth.admin.ui.service.EmailService; - -import org.apache.commons.lang3.StringUtils; import org.pac4j.core.config.Config; import org.pac4j.core.matching.matcher.Matcher; -import org.pac4j.core.matching.matcher.PathMatcher; import org.pac4j.springframework.security.web.CallbackFilter; import org.pac4j.springframework.security.web.SecurityFilter; -import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.AutoConfigureAfter; import org.springframework.boot.autoconfigure.AutoConfigureOrder; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; @@ -24,11 +21,9 @@ import org.springframework.security.config.http.SessionCreationPolicy; import org.springframework.security.web.authentication.www.BasicAuthenticationFilter; import org.springframework.security.web.firewall.StrictHttpFirewall; -import org.springframework.security.web.util.matcher.AntPathRequestMatcher; - -import java.util.Optional; import javax.servlet.Filter; +import java.util.Optional; @Configuration @AutoConfigureOrder(-1) @@ -36,39 +31,43 @@ @AutoConfigureAfter(EmailConfiguration.class) public class WebSecurity { @Bean("webSecurityConfig") - public WebSecurityConfigurerAdapter webSecurityConfigurerAdapter(final Config config, UserRepository userRepository, + public WebSecurityConfigurerAdapter webSecurityConfigurerAdapter(final Config config, UserService userService, RoleRepository roleRepository, Optional emailService, - Pac4jConfigurationProperties pac4jConfigurationProperties) { - return new Pac4jWebSecurityConfigurerAdapter(config, userRepository, roleRepository, emailService, + Pac4jConfigurationProperties pac4jConfigurationProperties, IGroupService groupService) { + return new Pac4jWebSecurityConfigurerAdapter(config, userService, roleRepository, emailService, groupService, pac4jConfigurationProperties); } @Order(100) public static class Pac4jWebSecurityConfigurerAdapter extends WebSecurityConfigurerAdapter { private final Config config; - private UserRepository userRepository; - private RoleRepository roleRepository; private Optional emailService; + private IGroupService groupService; private Pac4jConfigurationProperties pac4jConfigurationProperties; + private RoleRepository roleRepository; + private UserService userService; - public Pac4jWebSecurityConfigurerAdapter(final Config config, UserRepository userRepository, RoleRepository roleRepository, - Optional emailService, Pac4jConfigurationProperties pac4jConfigurationProperties) { + public Pac4jWebSecurityConfigurerAdapter(final Config config, UserService userService, RoleRepository roleRepository, + Optional emailService, IGroupService groupService, Pac4jConfigurationProperties pac4jConfigurationProperties) { this.config = config; - this.userRepository = userRepository; + this.userService = userService; this.roleRepository = roleRepository; this.emailService = emailService; + this.groupService = groupService; this.pac4jConfigurationProperties = pac4jConfigurationProperties; } @Override protected void configure(HttpSecurity http) throws Exception { http.authorizeRequests().antMatchers("/unsecured/**/*").permitAll(); - + + final SecurityFilter securityFilter = new SecurityFilter(this.config, "Saml2Client"); + // add filter based on auth type http.antMatcher("/**").addFilterBefore(getFilter(config, pac4jConfigurationProperties.getTypeOfAuth()), BasicAuthenticationFilter.class); - + http.antMatcher("/**").addFilterBefore(securityFilter, BasicAuthenticationFilter.class); // add the new user filter - http.addFilterAfter(new AddNewUserFilter(pac4jConfigurationProperties, userRepository, roleRepository, getPathMatcher("exclude-paths-matcher") , emailService), SecurityFilter.class); + http.addFilterAfter(new AddNewUserFilter(pac4jConfigurationProperties, userService, roleRepository, getPathMatcher("exclude-paths-matcher"), groupService, emailService), SecurityFilter.class); http.exceptionHandling().accessDeniedHandler((request, response, accessDeniedException) -> response.sendRedirect("/unsecured/error.html")); http.sessionManagement().sessionCreationPolicy(SessionCreationPolicy.ALWAYS); @@ -110,4 +109,4 @@ public void configure(org.springframework.security.config.annotation.web.builder public AuditorAware pac4jAuditorAware() { return new Pac4jAuditorAware(); } -} +} \ No newline at end of file diff --git a/pac4j-module/src/main/resources/application.yml b/pac4j-module/src/main/resources/application.yml index ebba4fcd9..82c3e831d 100644 --- a/pac4j-module/src/main/resources/application.yml +++ b/pac4j-module/src/main/resources/application.yml @@ -1,7 +1,9 @@ shibui: + pac4j-enabled: true pac4j: simpleProfileMapping: username: urn:oid:0.9.2342.19200300.100.1.3 firstName: givenName lastName: sn - email: mail \ No newline at end of file + email: mail + groupsName: urn:oid:1.3.6.1.4.1.5923.1.5.1.1 # attributeId - isMemberOf \ No newline at end of file diff --git a/pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/AddNewUserFilterTests.groovy b/pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/AddNewUserFilterMockTests.groovy similarity index 82% rename from pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/AddNewUserFilterTests.groovy rename to pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/AddNewUserFilterMockTests.groovy index d15b70f0a..1f729beb7 100644 --- a/pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/AddNewUserFilterTests.groovy +++ b/pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/AddNewUserFilterMockTests.groovy @@ -3,11 +3,11 @@ package net.unicon.shibui.pac4j 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.RoleRepository -import edu.internet2.tier.shibboleth.admin.ui.security.repository.UserRepository +import edu.internet2.tier.shibboleth.admin.ui.security.service.IGroupService +import edu.internet2.tier.shibboleth.admin.ui.security.service.UserService import edu.internet2.tier.shibboleth.admin.ui.service.EmailService import org.pac4j.core.matching.matcher.PathMatcher -import org.pac4j.core.profile.CommonProfile import org.pac4j.saml.profile.SAML2Profile import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.context.properties.EnableConfigurationProperties @@ -19,7 +19,6 @@ import spock.lang.Specification import spock.lang.Subject import javax.servlet.FilterChain -import javax.servlet.ServletRequest import javax.servlet.http.HttpServletRequest import javax.servlet.http.HttpServletResponse @@ -28,11 +27,12 @@ import javax.servlet.http.HttpServletResponse */ @SpringBootTest(classes = [Pac4jConfigurationProperties]) @EnableConfigurationProperties([Pac4jConfigurationProperties]) -class AddNewUserFilterTests extends Specification { +class AddNewUserFilterMockTests extends Specification { - UserRepository userRepository = Mock() + UserService userService = Mock() RoleRepository roleRepository = Mock() EmailService emailService = Mock() + IGroupService groupService = Mock() HttpServletRequest request = Mock() HttpServletResponse response = Mock() @@ -55,7 +55,7 @@ class AddNewUserFilterTests extends Specification { securityContext.getAuthentication() >> authentication authentication.getPrincipal() >> saml2Profile - addNewUserFilter = new AddNewUserFilter(pac4jConfigurationProperties, userRepository, roleRepository, new PathMatcher(), Optional.of(emailService)) + addNewUserFilter = new AddNewUserFilter(pac4jConfigurationProperties, userService, roleRepository, new PathMatcher(), groupService, Optional.of(emailService)) profileMapping = pac4jConfigurationProperties.simpleProfileMapping } @@ -68,7 +68,7 @@ class AddNewUserFilterTests extends Specification { saml2Profile.getAttribute(profileMapping."get${key}"()) >> [value] } saml2Profile.getUsername() >> "newUser" - userRepository.findByUsername('newUser') >> Optional.empty() + userService.findByUsername('newUser') >> Optional.empty() roleRepository.findByName('ROLE_NONE') >> Optional.of(new Role('ROLE_NONE')) when: @@ -76,7 +76,7 @@ class AddNewUserFilterTests extends Specification { then: 0 * roleRepository.save(_) - 1 * userRepository.save(_ as User) >> { User user -> user } + 1 * userService.save(_ as User) >> { User user -> user } 1 * emailService.sendNewUserMail('newUser') 1 * response.sendRedirect("/unsecured/error.html") } @@ -84,7 +84,7 @@ class AddNewUserFilterTests extends Specification { def "existing users are not redirected"() { given: saml2Profile.getUsername() >> "existingUser" - userRepository.findByUsername('existingUser') >> Optional.of(new User().with { + userService.findByUsername('existingUser') >> Optional.of(new User().with { it.username = 'existingUser' it.roles = [new Role('ROLE_USER')] it @@ -95,7 +95,7 @@ class AddNewUserFilterTests extends Specification { then: 0 * roleRepository.save(_) - 0 * userRepository.save(_) + 0 * userService.save(_) 1 * chain.doFilter(_, _) } -} +} \ No newline at end of file From 65694f6d2214f2cac834ff5065fad149c0413dc0 Mon Sep 17 00:00:00 2001 From: chasegawa Date: Tue, 24 Aug 2021 11:43:45 -0700 Subject: [PATCH 2/2] SHIBUI-2015 parsing group from shib --- .../service/GroupServiceForTesting.groovy | 17 +++ .../shibui/pac4j/AddNewUserFilterTests.groovy | 140 ++++++++++++++++++ .../shibui/pac4j/Pac4JTestingConfig.groovy | 52 +++++++ 3 files changed, 209 insertions(+) create mode 100644 pac4j-module/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/service/GroupServiceForTesting.groovy create mode 100644 pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/AddNewUserFilterTests.groovy create mode 100644 pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/Pac4JTestingConfig.groovy diff --git a/pac4j-module/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/service/GroupServiceForTesting.groovy b/pac4j-module/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/service/GroupServiceForTesting.groovy new file mode 100644 index 000000000..7773c2d20 --- /dev/null +++ b/pac4j-module/src/test/groovy/edu/internet2/tier/shibboleth/admin/ui/security/service/GroupServiceForTesting.groovy @@ -0,0 +1,17 @@ +package edu.internet2.tier.shibboleth.admin.ui.security.service + +import org.springframework.transaction.annotation.Transactional + +class GroupServiceForTesting extends GroupServiceImpl { + public GroupServiceForTesting(GroupServiceImpl impl) { + this.groupRepository = impl.groupRepository + this.ownershipRepository = impl.ownershipRepository + } + + @Transactional + public void clearAllForTesting() { + groupRepository.deleteAll(); + ownershipRepository.clearAllOwnedByGroup() + ensureAdminGroupExists() + } +} \ No newline at end of file diff --git a/pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/AddNewUserFilterTests.groovy b/pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/AddNewUserFilterTests.groovy new file mode 100644 index 000000000..5d70c9386 --- /dev/null +++ b/pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/AddNewUserFilterTests.groovy @@ -0,0 +1,140 @@ +package net.unicon.shibui.pac4j + +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.OwnershipRepository +import edu.internet2.tier.shibboleth.admin.ui.security.repository.RoleRepository +import edu.internet2.tier.shibboleth.admin.ui.security.repository.UserRepository +import edu.internet2.tier.shibboleth.admin.ui.security.service.GroupServiceForTesting +import edu.internet2.tier.shibboleth.admin.ui.security.service.UserService +import org.pac4j.core.matching.matcher.PathMatcher +import org.pac4j.saml.profile.SAML2Profile +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.boot.autoconfigure.domain.EntityScan +import org.springframework.boot.context.properties.EnableConfigurationProperties +import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest +import org.springframework.data.jpa.repository.config.EnableJpaRepositories +import org.springframework.security.core.Authentication +import org.springframework.security.core.context.SecurityContext +import org.springframework.security.core.context.SecurityContextHolder +import org.springframework.test.annotation.DirtiesContext +import org.springframework.test.context.ContextConfiguration +import org.springframework.transaction.annotation.Transactional +import spock.lang.Specification +import spock.lang.Subject + +import javax.servlet.FilterChain +import javax.servlet.http.HttpServletRequest +import javax.servlet.http.HttpServletResponse + +@DataJpaTest +@ContextConfiguration(classes=[Pac4JTestingConfig]) +@EnableJpaRepositories(basePackages = ["edu.internet2.tier.shibboleth.admin.ui"]) +@EntityScan("edu.internet2.tier.shibboleth.admin.ui") +@DirtiesContext +@EnableConfigurationProperties([Pac4jConfigurationProperties]) +class AddNewUserFilterTests extends Specification { + @Subject + AddNewUserFilter addNewUserFilter + + @Autowired + GroupServiceForTesting groupService + + @Autowired + OwnershipRepository ownershipRepository + + @Autowired + RoleRepository roleRepository + + @Autowired + Pac4jConfigurationProperties pac4jConfigurationProperties + + @Autowired + UserRepository userRepository + + @Autowired + UserService userService + + HttpServletRequest request = Mock() + HttpServletResponse response = Mock() + FilterChain chain = Mock() + + SecurityContext securityContext = Mock() + Authentication authentication = Mock() + SAML2Profile saml2Profile = Mock() + + Pac4jConfigurationProperties.SimpleProfileMapping profileMapping + + @Transactional + def setup() { + SecurityContextHolder.setContext(securityContext) + securityContext.getAuthentication() >> authentication + authentication.getPrincipal() >> saml2Profile + + addNewUserFilter = new AddNewUserFilter(pac4jConfigurationProperties, userService, roleRepository, new PathMatcher(), groupService, Optional.empty()) + profileMapping = pac4jConfigurationProperties.simpleProfileMapping + + userRepository.findAll().forEach { + userService.delete(it.getUsername()) + } + userRepository.flush() + + roleRepository.deleteAll() + roleRepository.flush() + groupService.clearAllForTesting() //leaves us just the admingroup + + def roles = [new Role().with { + name = 'ROLE_ADMIN' + it + }, new Role().with { + name = 'ROLE_USER' + it + }, new Role().with { + name = 'ROLE_NONE' + it + }] + roles.each { + roleRepository.save(it) + } + } + + def "new user created"() { + given: + ['Username': 'newUser', + 'FirstName': 'New', + 'LastName': 'User', + 'Email': 'newuser@institution.edu'].each { key, value -> + saml2Profile.getAttribute(profileMapping."get${key}"()) >> [value] + } + saml2Profile.getUsername() >> "newUser" + + when: + addNewUserFilter.doFilter(request, response, chain) + + then: + 1 * response.sendRedirect("/unsecured/error.html") + User user = userRepository.findByUsername("newUser").get() + user.getGroupId() == "newUser" + } + + def "new user created with group - assumes saml2profile got property for groups"() { + given: + ['Username': 'newUser', + 'FirstName': 'New', + 'LastName': 'User', + 'Email': 'newuser@institution.edu', + 'GroupsName':'AAAGroup' + ].each { key, value -> + saml2Profile.getAttribute(profileMapping."get${key}"()) >> [value] + } + saml2Profile.getUsername() >> "newUser" + + when: + addNewUserFilter.doFilter(request, response, chain) + + then: + 1 * response.sendRedirect("/unsecured/error.html") + User user = userRepository.findByUsername("newUser").get() + user.getGroupId() == "AAAGroup" + } +} \ No newline at end of file diff --git a/pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/Pac4JTestingConfig.groovy b/pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/Pac4JTestingConfig.groovy new file mode 100644 index 000000000..552aaca04 --- /dev/null +++ b/pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/Pac4JTestingConfig.groovy @@ -0,0 +1,52 @@ +package net.unicon.shibui.pac4j + +import edu.internet2.tier.shibboleth.admin.ui.security.model.listener.GroupUpdatedEntityListener +import edu.internet2.tier.shibboleth.admin.ui.security.model.listener.UserUpdatedEntityListener +import edu.internet2.tier.shibboleth.admin.ui.security.repository.GroupsRepository +import edu.internet2.tier.shibboleth.admin.ui.security.repository.OwnershipRepository +import edu.internet2.tier.shibboleth.admin.ui.security.repository.RoleRepository +import edu.internet2.tier.shibboleth.admin.ui.security.repository.UserRepository +import edu.internet2.tier.shibboleth.admin.ui.security.service.GroupServiceForTesting +import edu.internet2.tier.shibboleth.admin.ui.security.service.GroupServiceImpl +import edu.internet2.tier.shibboleth.admin.ui.security.service.IGroupService +import edu.internet2.tier.shibboleth.admin.ui.security.service.UserService +import org.springframework.context.annotation.Bean +import org.springframework.context.annotation.Configuration +import org.springframework.context.annotation.Primary + +@Configuration +class Pac4JTestingConfig { + @Bean + @Primary + GroupServiceForTesting groupServiceForTesting(GroupsRepository repo, OwnershipRepository ownershipRepository) { + GroupServiceForTesting result = new GroupServiceForTesting(new GroupServiceImpl().with { + it.groupRepository = repo + it.ownershipRepository = ownershipRepository + return it + }) + result.ensureAdminGroupExists() + return result + } + + @Bean + @Primary + GroupUpdatedEntityListener groupUpdatedEntityListener(OwnershipRepository repo) { + GroupUpdatedEntityListener listener = new GroupUpdatedEntityListener() + listener.init(repo) + return listener + } + + @Bean + @Primary + UserUpdatedEntityListener userUpdatedEntityListener(OwnershipRepository repo, GroupsRepository groupRepo) { + UserUpdatedEntityListener listener = new UserUpdatedEntityListener() + listener.init(repo, groupRepo) + return listener + } + + @Bean + @Primary + UserService userService(IGroupService groupService, OwnershipRepository ownershipRepository, RoleRepository roleRepository, UserRepository userRepository) { + return new UserService(groupService, ownershipRepository, roleRepository, userRepository) + } +} \ No newline at end of file