From 7856be12667b1c2967d2324d6d06db595c63bdce Mon Sep 17 00:00:00 2001 From: Bill Smith Date: Fri, 25 Jan 2019 15:35:14 -0700 Subject: [PATCH] [SHIBUI-1029] Consolidated properties into Pac4jConfigurationProperties. Refactored tests and filter to use new properties. --- .../unicon/shibui/pac4j/AddNewUserFilter.java | 36 ++++++++----- .../pac4j/CustomPropertiesConfiguration.java | 26 ---------- .../shibui/pac4j/Pac4jConfiguration.java | 1 + .../pac4j/Pac4jConfigurationProperties.java | 50 +++++++++++++++++++ .../net/unicon/shibui/pac4j/WebSecurity.java | 12 ++--- .../src/main/resources/application.yml | 13 ++--- .../shibui/pac4j/AddNewUserFilterTests.groovy | 24 +++++---- 7 files changed, 100 insertions(+), 62 deletions(-) delete mode 100644 pac4j-module/src/main/java/net/unicon/shibui/pac4j/CustomPropertiesConfiguration.java 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 af369937f..27275870d 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 @@ -41,16 +41,16 @@ public class AddNewUserFilter implements Filter { private RoleRepository roleRepository; private EmailService emailService; - private CustomPropertiesConfiguration customPropertiesConfiguration; + private Pac4jConfigurationProperties pac4jConfigurationProperties; - private Map 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 attributeList = (List) 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 attributeList = (List) 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(); diff --git a/pac4j-module/src/main/java/net/unicon/shibui/pac4j/CustomPropertiesConfiguration.java b/pac4j-module/src/main/java/net/unicon/shibui/pac4j/CustomPropertiesConfiguration.java deleted file mode 100644 index 793b43793..000000000 --- a/pac4j-module/src/main/java/net/unicon/shibui/pac4j/CustomPropertiesConfiguration.java +++ /dev/null @@ -1,26 +0,0 @@ -package net.unicon.shibui.pac4j; - -import org.springframework.boot.context.properties.ConfigurationProperties; -import org.springframework.boot.context.properties.EnableConfigurationProperties; -import org.springframework.stereotype.Component; - -import java.util.Map; - -/** - * @author Bill Smith (wsmith@unicon.net) - */ -@Component -@ConfigurationProperties(prefix="custom") -@EnableConfigurationProperties -public class CustomPropertiesConfiguration { - - private Map saml2ProfileMapping; - - public Map getSaml2ProfileMapping() { - return saml2ProfileMapping; - } - - public void setSaml2ProfileMapping(Map saml2ProfileMapping) { - this.saml2ProfileMapping = saml2ProfileMapping; - } -} 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 1c28b1ee1..5ee638178 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 @@ -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"); 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 afb1369a1..cf36d8e65 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 @@ -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; + } } 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 421dffe63..f0c183175 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 @@ -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(); diff --git a/pac4j-module/src/main/resources/application.yml b/pac4j-module/src/main/resources/application.yml index ae30333c7..4042fd939 100644 --- a/pac4j-module/src/main/resources/application.yml +++ b/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 \ No newline at end of file +shibui: + pac4j: + saml2ProfileMapping: + username: urn:oid:0.9.2342.19200300.100.1.3 + firstName: givenName + lastName: sn + email: mail \ 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 index 652b38783..ac470ea25 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 @@ -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 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')]