[SHIBUI-1029]

Consolidated properties into Pac4jConfigurationProperties. Refactored
tests and filter to use new properties.

pac4j-module/src/main/java/net/unicon/shibui/pac4j/AddNewUserFilter.java

@@ -41,16 +41,16 @@ public class AddNewUserFilter implements Filter {
     private RoleRepository roleRepository;
     private EmailService emailService;
 
-    private CustomPropertiesConfiguration customPropertiesConfiguration;
+    private Pac4jConfigurationProperties pac4jConfigurationProperties;
 
-    private Map<String, String> saml2ProfileMapping;
+    private Pac4jConfigurationProperties.SAML2ProfileMapping saml2ProfileMapping;
 
-    public AddNewUserFilter(CustomPropertiesConfiguration customPropertiesConfiguration, UserRepository userRepository, RoleRepository roleRepository, EmailService emailService) {
+    public AddNewUserFilter(Pac4jConfigurationProperties pac4jConfigurationProperties, UserRepository userRepository, RoleRepository roleRepository, EmailService emailService) {
         this.userRepository = userRepository;
         this.roleRepository = roleRepository;
         this.emailService = emailService;
-        this.customPropertiesConfiguration = customPropertiesConfiguration;
-        saml2ProfileMapping = this.customPropertiesConfiguration.getSaml2ProfileMapping();
+        this.pac4jConfigurationProperties = pac4jConfigurationProperties;
+        saml2ProfileMapping = this.pac4jConfigurationProperties.getSaml2ProfileMapping();
     }
 
     @Override
@@ -108,17 +108,27 @@ public void destroy() {
     }
 
     private String getAttributeFromProfile(SAML2Profile profile, String stringKey) {
-        String mappingKey = saml2ProfileMapping.get(stringKey);
-        List<String> attributeList = (List<String>) profile.getAttribute(mappingKey);
         String attribute = null;
-        if (attributeList.size() > 0) {
-            if (attributeList.size() != 1) {
-                logger.warn(String.format("More than one attribute was found for key [%s]", stringKey));
-            }
-            attribute = attributeList.get(0);
+        switch (stringKey) {
+            case "username":
+                attribute = saml2ProfileMapping.getUsername();
+                break;
+            case "firstName":
+                attribute = saml2ProfileMapping.getFirstName();
+                break;
+            case "lastName":
+                attribute = saml2ProfileMapping.getLastName();
+                break;
+            case "email":
+                attribute = saml2ProfileMapping.getEmail();
+                break;
+            default:
+                // do we care? Not yet.
         }
-        return attribute;
+        List<String> attributeList = (List<String>) profile.getAttribute(attribute);
+        return attributeList.size() < 1 ? null : attributeList.get(0);
     }
+
     private byte[] getJsonResponseBytes(ErrorResponse eErrorResponse) throws IOException {
         String errorResponseJson = new ObjectMapper().writeValueAsString(eErrorResponse);
         return errorResponseJson.getBytes();

pac4j-module/src/main/java/net/unicon/shibui/pac4j/Pac4jConfiguration.java

@@ -22,6 +22,7 @@ public Config config(final Pac4jConfigurationProperties pac4jConfigurationProper
         saml2ClientConfiguration.setServiceProviderMetadataPath(pac4jConfigurationProperties.getServiceProviderMetadataPath());
         saml2ClientConfiguration.setForceServiceProviderMetadataGeneration(pac4jConfigurationProperties.isForceServiceProviderMetadataGeneration());
         saml2ClientConfiguration.setWantsAssertionsSigned(pac4jConfigurationProperties.isWantAssertionsSigned());
+        saml2ClientConfiguration.setAttributeAsId(pac4jConfigurationProperties.getSaml2ProfileMapping().getUsername());
 
         final SAML2Client saml2Client = new SAML2Client(saml2ClientConfiguration);
         saml2Client.setName("Saml2Client");

pac4j-module/src/main/java/net/unicon/shibui/pac4j/Pac4jConfigurationProperties.java

@@ -1,10 +1,12 @@
 package net.unicon.shibui.pac4j;
 
 import org.springframework.boot.context.properties.ConfigurationProperties;
+import org.springframework.boot.context.properties.EnableConfigurationProperties;
 import org.springframework.stereotype.Component;
 
 @Component
 @ConfigurationProperties(prefix = "shibui.pac4j")
+@EnableConfigurationProperties
 public class Pac4jConfigurationProperties {
     private String keystorePath = "/tmp/samlKeystore.jks";
     private String keystorePassword = "changeit";
@@ -16,6 +18,46 @@ public class Pac4jConfigurationProperties {
     private boolean forceServiceProviderMetadataGeneration = false;
     private String callbackUrl;
     private boolean wantAssertionsSigned = true;
+    private SAML2ProfileMapping saml2ProfileMapping;
+
+    public static class SAML2ProfileMapping {
+        private String username;
+        private String email;
+        private String firstName;
+        private String lastName;
+
+        public String getUsername() {
+            return username;
+        }
+
+        public void setUsername(String username) {
+            this.username = username;
+        }
+
+        public String getEmail() {
+            return email;
+        }
+
+        public void setEmail(String email) {
+            this.email = email;
+        }
+
+        public String getFirstName() {
+            return firstName;
+        }
+
+        public void setFirstName(String firstName) {
+            this.firstName = firstName;
+        }
+
+        public String getLastName() {
+            return lastName;
+        }
+
+        public void setLastName(String lastName) {
+            this.lastName = lastName;
+        }
+    }
 
     public String getKeystorePath() {
         return keystorePath;
@@ -96,4 +138,12 @@ public boolean isWantAssertionsSigned() {
     public void setWantAssertionsSigned(boolean wantAssertionsSigned) {
         this.wantAssertionsSigned = wantAssertionsSigned;
     }
+
+    public SAML2ProfileMapping getSaml2ProfileMapping() {
+        return saml2ProfileMapping;
+    }
+
+    public void setSaml2ProfileMapping(SAML2ProfileMapping saml2ProfileMapping) {
+        this.saml2ProfileMapping = saml2ProfileMapping;
+    }
 }

pac4j-module/src/main/java/net/unicon/shibui/pac4j/WebSecurity.java

@@ -20,8 +20,8 @@
 @AutoConfigureOrder(-1)
 public class WebSecurity {
     @Bean("webSecurityConfig")
-    public WebSecurityConfigurerAdapter webSecurityConfigurerAdapter(final Config config, UserRepository userRepository, RoleRepository roleRepository, EmailService emailService, CustomPropertiesConfiguration customPropertiesConfiguration) {
-        return new Pac4jWebSecurityConfigurerAdapter(config, userRepository, roleRepository, emailService, customPropertiesConfiguration);
+    public WebSecurityConfigurerAdapter webSecurityConfigurerAdapter(final Config config, UserRepository userRepository, RoleRepository roleRepository, EmailService emailService, Pac4jConfigurationProperties pac4jConfigurationProperties) {
+        return new Pac4jWebSecurityConfigurerAdapter(config, userRepository, roleRepository, emailService, pac4jConfigurationProperties);
     }
 
     @Configuration
@@ -57,14 +57,14 @@ public static class Pac4jWebSecurityConfigurerAdapter extends WebSecurityConfigu
         private UserRepository userRepository;
         private RoleRepository roleRepository;
         private EmailService emailService;
-        private CustomPropertiesConfiguration customPropertiesConfiguration;
+        private Pac4jConfigurationProperties pac4jConfigurationProperties;
 
-        public Pac4jWebSecurityConfigurerAdapter(final Config config, UserRepository userRepository, RoleRepository roleRepository, EmailService emailService, CustomPropertiesConfiguration customPropertiesConfiguration) {
+        public Pac4jWebSecurityConfigurerAdapter(final Config config, UserRepository userRepository, RoleRepository roleRepository, EmailService emailService, Pac4jConfigurationProperties pac4jConfigurationProperties) {
             this.config = config;
             this.userRepository = userRepository;
             this.roleRepository = roleRepository;
             this.emailService = emailService;
-            this.customPropertiesConfiguration = customPropertiesConfiguration;
+            this.pac4jConfigurationProperties = pac4jConfigurationProperties;
         }
 
         @Override
@@ -74,7 +74,7 @@ protected void configure(HttpSecurity http) throws Exception {
             final CallbackFilter callbackFilter = new CallbackFilter(this.config);
             http.antMatcher("/**").addFilterBefore(callbackFilter, BasicAuthenticationFilter.class)
                     .addFilterBefore(securityFilter, BasicAuthenticationFilter.class)
-                    .addFilterAfter(new AddNewUserFilter(customPropertiesConfiguration, userRepository, roleRepository, emailService), SecurityFilter.class);
+                    .addFilterAfter(new AddNewUserFilter(pac4jConfigurationProperties, userRepository, roleRepository, emailService), SecurityFilter.class);
 
             http.authorizeRequests().anyRequest().fullyAuthenticated();
 

pac4j-module/src/main/resources/application.yml

@@ -1,6 +1,7 @@
-custom:
-  saml2ProfileMapping:
-    username: urn:oid:0.9.2342.19200300.100.1.3
-    firstName: givenName
-    lastName: sn
-    email: mail
+shibui:
+  pac4j:
+    saml2ProfileMapping:
+      username: urn:oid:0.9.2342.19200300.100.1.3
+      firstName: givenName
+      lastName: sn
+      email: mail

pac4j-module/src/test/groovy/net/unicon/shibui/pac4j/AddNewUserFilterTests.groovy

@@ -22,8 +22,8 @@ import javax.servlet.http.HttpServletResponse
 /**
  * @author Bill Smith (wsmith@unicon.net)
  */
-@SpringBootTest(classes = [CustomPropertiesConfiguration])
-@EnableConfigurationProperties([CustomPropertiesConfiguration])
+@SpringBootTest(classes = [Pac4jConfigurationProperties])
+@EnableConfigurationProperties([Pac4jConfigurationProperties])
 class AddNewUserFilterTests extends Specification {
 
     UserRepository userRepository = Mock()
@@ -39,9 +39,9 @@ class AddNewUserFilterTests extends Specification {
     SAML2Profile saml2Profile = Mock()
 
     @Autowired
-    CustomPropertiesConfiguration customPropertiesConfiguration
+    Pac4jConfigurationProperties pac4jConfigurationProperties
 
-    Map<String, String> userAttributeMapping
+    Pac4jConfigurationProperties.SAML2ProfileMapping saml2ProfileMapping
 
     @Subject
     AddNewUserFilter addNewUserFilter
@@ -51,16 +51,18 @@ class AddNewUserFilterTests extends Specification {
         securityContext.getAuthentication() >> authentication
         authentication.getPrincipal() >> saml2Profile
 
-        addNewUserFilter = new AddNewUserFilter(customPropertiesConfiguration, userRepository, roleRepository, emailService)
-        userAttributeMapping = customPropertiesConfiguration.saml2ProfileMapping
+        addNewUserFilter = new AddNewUserFilter(pac4jConfigurationProperties, userRepository, roleRepository, emailService)
+        saml2ProfileMapping = pac4jConfigurationProperties.saml2ProfileMapping
     }
 
     def "new users are redirected"() {
         given:
-        saml2Profile.getAttribute(userAttributeMapping.get('username')) >> ['newUser']
-        saml2Profile.getAttribute(userAttributeMapping.get('firstName')) >> ['New']
-        saml2Profile.getAttribute(userAttributeMapping.get('lastName')) >> ['User']
-        saml2Profile.getAttribute(userAttributeMapping.get('email')) >> ['newuser@institution.edu']
+        ['Username': 'newUser',
+         'FirstName': 'New',
+         'LastName': 'User',
+         'Email': 'newuser@institution.edu'].each { key, value ->
+            saml2Profile.getAttribute(saml2ProfileMapping."get${key}"()) >> [value]
+        }
         userRepository.findByUsername('newUser') >> Optional.empty()
         roleRepository.findByName('ROLE_NONE') >> Optional.of(new Role('ROLE_NONE'))
 
@@ -76,7 +78,7 @@ class AddNewUserFilterTests extends Specification {
 
     def "existing users are not redirected"() {
         given:
-        saml2Profile.getAttribute(userAttributeMapping.get('username')) >> ['existingUser']
+        saml2Profile.getAttribute(saml2ProfileMapping.getUsername()) >> ['existingUser']
         userRepository.findByUsername('existingUser') >> Optional.of(new User().with {
             it.username = 'existingUser'
             it.roles = [new Role('ROLE_USER')]