From c30e1afc6f0b18a0100fa7ee4d4cf50ad0a856a3 Mon Sep 17 00:00:00 2001 From: chasegawa Date: Tue, 14 Sep 2021 08:28:48 -0700 Subject: [PATCH] SHIBUI-2063 wrapping up test adjustments --- .../ui/security/service/RolesServiceImpl.java | 4 +- backend/src/main/resources/application.yml | 1 + .../src/main/resources/application.yml | 3 +- .../pac4j/AddNewUserFilterMockTests.groovy | 55 +++++++------------ .../shibui/pac4j/AddNewUserFilterTests.groovy | 7 ++- .../shibui/pac4j/Pac4JTestingConfig.groovy | 9 +++ 6 files changed, 40 insertions(+), 39 deletions(-) diff --git a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/RolesServiceImpl.java b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/RolesServiceImpl.java index 373843781..939be59d8 100644 --- a/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/RolesServiceImpl.java +++ b/backend/src/main/java/edu/internet2/tier/shibboleth/admin/ui/security/service/RolesServiceImpl.java @@ -17,7 +17,7 @@ @Service public class RolesServiceImpl implements IRolesService { @Autowired - private RoleRepository roleRepository; + RoleRepository roleRepository; @Override public Role createRole(Role role) throws RoleExistsConflictException { @@ -61,7 +61,7 @@ public Role findByResourceId(String resourceId) throws EntityNotFoundException { @Override public Set getAndCreateAllRoles(Set roleNames) { HashSet result = new HashSet<>(); - if (roleNames.isEmpty()) { + if (roleNames == null || roleNames.isEmpty()) { Role r = getRoleNone(); result.add(r); return result; diff --git a/backend/src/main/resources/application.yml b/backend/src/main/resources/application.yml index 4035d8549..671000aa6 100644 --- a/backend/src/main/resources/application.yml +++ b/backend/src/main/resources/application.yml @@ -21,6 +21,7 @@ # lastname: urn:oid:2.5.4.4 # email: urn:oid:0.9.2342.19200300.100.1.3 # groups: urn:oid:1.3.6.1.4.1.5923.1.5.1.1 # attributeId - isMemberOf +# roles: --define name of the attribute containing the incoming user roles-- custom: attributes: diff --git a/pac4j-module/src/main/resources/application.yml b/pac4j-module/src/main/resources/application.yml index ef2d684d0..78a3c1644 100644 --- a/pac4j-module/src/main/resources/application.yml +++ b/pac4j-module/src/main/resources/application.yml @@ -31,4 +31,5 @@ shibui: # firstname: urn:oid:2.5.4.42 # lastname: urn:oid:2.5.4.4 # email: urn:oid:0.9.2342.19200300.100.1.3 -# groups: urn:oid:1.3.6.1.4.1.5923.1.5.1.1 # attributeId - isMemberOf \ No newline at end of file +# groups: urn:oid:1.3.6.1.4.1.5923.1.5.1.1 # attributeId - isMemberOf +# roles: --define name of the attribute containing the incoming user roles-- \ No newline at end of file diff --git a/pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/AddNewUserFilterMockTests.groovy b/pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/AddNewUserFilterMockTests.groovy index f39a8b988..5c1e7450a 100644 --- a/pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/AddNewUserFilterMockTests.groovy +++ b/pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/AddNewUserFilterMockTests.groovy @@ -4,6 +4,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.RoleRepository import edu.internet2.tier.shibboleth.admin.ui.security.service.IGroupService +import edu.internet2.tier.shibboleth.admin.ui.security.service.RolesServiceImpl import edu.internet2.tier.shibboleth.admin.ui.security.service.UserService import edu.internet2.tier.shibboleth.admin.ui.service.EmailService import org.pac4j.core.matching.PathMatcher @@ -28,58 +29,42 @@ import javax.servlet.http.HttpServletResponse @EnableConfigurationProperties([Pac4jConfigurationProperties]) class AddNewUserFilterMockTests extends Specification { - UserService userService = Mock() - RoleRepository roleRepository = Mock() + @Subject + AddNewUserFilter addNewUserFilter + + @Autowired + Pac4jConfigurationProperties pac4jConfigurationProperties + + Authentication authentication = Mock() + FilterChain chain = Mock() EmailService emailService = Mock() IGroupService groupService = Mock() - HttpServletRequest request = Mock() HttpServletResponse response = Mock() - FilterChain chain = Mock() - - SecurityContext securityContext = Mock() - Authentication authentication = Mock() + RoleRepository roleRepository = Mock() SAML2Profile saml2Profile = Mock() - - @Autowired - Pac4jConfigurationProperties pac4jConfigurationProperties + SecurityContext securityContext = Mock() + UserService userService = Mock() Pac4jConfigurationProperties.SimpleProfileMapping profileMapping - @Subject - AddNewUserFilter addNewUserFilter - def setup() { SecurityContextHolder.setContext(securityContext) securityContext.getAuthentication() >> authentication authentication.getPrincipal() >> saml2Profile - addNewUserFilter = new AddNewUserFilter(pac4jConfigurationProperties, userService, roleRepository, new PathMatcher(), groupService, Optional.of(emailService)) - profileMapping = pac4jConfigurationProperties.simpleProfileMapping - } - - def "new users are redirected"() { - given: - ['Username': 'newUser', - 'FirstName': 'New', - 'LastName': 'User', - 'Email': 'newuser@institution.edu'].each { key, value -> - saml2Profile.getAttribute(profileMapping."get${key}"()) >> [value] + RolesServiceImpl roleService = new RolesServiceImpl().with { + it.roleRepository = roleRepository + it } - saml2Profile.getUsername() >> "newUser" - userService.findByUsername('newUser') >> Optional.empty() - roleRepository.findByName('ROLE_NONE') >> Optional.of(new Role('ROLE_NONE')) - - when: - addNewUserFilter.doFilter(request, response, chain) - then: - 0 * roleRepository.save(_) - 1 * userService.save(_ as User) >> { User user -> user } - 1 * emailService.sendNewUserMail('newUser') - 1 * response.sendRedirect("/unsecured/error.html") + addNewUserFilter = new AddNewUserFilter(pac4jConfigurationProperties, userService, roleService, new PathMatcher(), groupService, Optional.of(emailService)) + profileMapping = pac4jConfigurationProperties.simpleProfileMapping } + // Checked in AddNewUserFilterTests + // def "new users are redirected"() + def "existing users are not redirected"() { given: saml2Profile.getUsername() >> "existingUser" 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 index a7c63e1a0..26fe665fd 100644 --- a/pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/AddNewUserFilterTests.groovy +++ b/pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/AddNewUserFilterTests.groovy @@ -6,6 +6,8 @@ import edu.internet2.tier.shibboleth.admin.ui.security.repository.OwnershipRepos 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.IRolesService +import edu.internet2.tier.shibboleth.admin.ui.security.service.RolesServiceImpl import edu.internet2.tier.shibboleth.admin.ui.security.service.UserService import org.pac4j.core.matching.PathMatcher import org.pac4j.saml.profile.SAML2Profile @@ -46,6 +48,9 @@ class AddNewUserFilterTests extends Specification { @Autowired RoleRepository roleRepository + @Autowired + RolesServiceImpl roleService + @Autowired Pac4jConfigurationProperties pac4jConfigurationProperties @@ -71,7 +76,7 @@ class AddNewUserFilterTests extends Specification { securityContext.getAuthentication() >> authentication authentication.getPrincipal() >> saml2Profile - addNewUserFilter = new AddNewUserFilter(pac4jConfigurationProperties, userService, roleRepository, new PathMatcher(), groupService, Optional.empty()) + addNewUserFilter = new AddNewUserFilter(pac4jConfigurationProperties, userService, roleService , new PathMatcher(), groupService, Optional.empty()) profileMapping = pac4jConfigurationProperties.simpleProfileMapping userRepository.findAll().forEach { 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 index 552aaca04..c2eb94ca7 100644 --- a/pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/Pac4JTestingConfig.groovy +++ b/pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/Pac4JTestingConfig.groovy @@ -9,6 +9,7 @@ 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.RolesServiceImpl import edu.internet2.tier.shibboleth.admin.ui.security.service.UserService import org.springframework.context.annotation.Bean import org.springframework.context.annotation.Configuration @@ -49,4 +50,12 @@ class Pac4JTestingConfig { UserService userService(IGroupService groupService, OwnershipRepository ownershipRepository, RoleRepository roleRepository, UserRepository userRepository) { return new UserService(groupService, ownershipRepository, roleRepository, userRepository) } + + @Bean + RolesServiceImpl rolesServiceImpl(RoleRepository roleRepository) { + new RolesServiceImpl().with { + it.roleRepository = roleRepository + it + } + } } \ No newline at end of file