SHIBUI-2063

wrapping up test adjustments

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<Role> getAndCreateAllRoles(Set<String> roleNames) {
         HashSet<Role> result = new HashSet<>();
-        if (roleNames.isEmpty()) {
+        if (roleNames == null || roleNames.isEmpty()) {
             Role r = getRoleNone();
             result.add(r);
             return result;

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:

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
+#      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--

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"

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 {

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
+        }
+    }
 }