From 67a686117b2fbf59bee6b57085c03c3e927c9cfc Mon Sep 17 00:00:00 2001 From: chasegawa Date: Tue, 7 Sep 2021 16:02:35 -0700 Subject: [PATCH] SHIBUI-2081 Merge master to 1743 branch --- backend/build.gradle | 9 ++- pac4j-module/build.gradle | 19 ++++-- .../unicon/shibui/pac4j/AddNewUserFilter.java | 23 +++++-- ...ocalUserProfileAuthorizationGenerator.java | 18 +++-- .../shibui/pac4j/Pac4jConfiguration.java | 67 ++++++++++--------- .../net/unicon/shibui/pac4j/WebSecurity.java | 13 ++-- .../pac4j/AddNewUserFilterMockTests.groovy | 3 +- .../shibui/pac4j/AddNewUserFilterTests.groovy | 2 +- 8 files changed, 91 insertions(+), 63 deletions(-) diff --git a/backend/build.gradle b/backend/build.gradle index 92ea35284..f20edf7d7 100644 --- a/backend/build.gradle +++ b/backend/build.gradle @@ -117,6 +117,13 @@ dependencies { compile "org.opensaml:${it}:${project.'opensaml.version'}" } +// Left here to save time later - when pac4j is/was updated, I needed all of these to get the runtime right with the SAML2 client +// runtimeOnly "org.bouncycastle:bcprov-jdk15on:1.69" +// runtimeOnly "org.bouncycastle:bcprov-ext-jdk15on:1.69" +// runtimeOnly "org.bouncycastle:bcutil-jdk15on:1.69" +// runtimeOnly "org.bouncycastle:bcpkix-jdk15on:1.69" + + // shibboleth idp deps ['idp-profile-spring', 'idp-profile-api'].each { compile "net.shibboleth.idp:${it}:${project.'shibboleth.version'}" @@ -353,4 +360,4 @@ dockerRun { daemonize true command '--spring.profiles.include=very-dangerous,dev', '--shibui.default-password={noop}password' clean true -} +} \ No newline at end of file diff --git a/pac4j-module/build.gradle b/pac4j-module/build.gradle index a84d2fe11..8803d22c9 100644 --- a/pac4j-module/build.gradle +++ b/pac4j-module/build.gradle @@ -28,18 +28,27 @@ generateLombokConfig.enabled = false dependencies { compileOnly project(':backend') - compile "org.pac4j:spring-security-pac4j:6.0.0" // pac4j is "off" - spring 6.0.0 here uses 5.1 core, thus differences in versions - compile "org.pac4j:pac4j-core:5.1.0" - compile "org.pac4j:pac4j-http:5.1.0" - compile "org.pac4j:pac4j-saml:5.1.0", { + compile "org.pac4j:spring-security-pac4j:4.0.0" + compile "org.pac4j:pac4j-core:3.3.0" + compile "org.pac4j:pac4j-http:3.3.0" + compile "org.pac4j:pac4j-saml:3.3.0", { // opensaml libraries are provided exclude group: 'org.opensaml' } +// These updated versions don't play well with the opensaml 3.4.6 version - until we can update openSAML.... +// compile "org.pac4j:spring-security-pac4j:6.0.0" // pac4j is "off" - spring 6.0.0 here uses 5.1 core, thus differences in versions +// compile "org.pac4j:pac4j-core:5.1.0" +// compile "org.pac4j:pac4j-http:5.1.0" +// compile "org.pac4j:pac4j-saml:5.1.0", { +// // opensaml libraries are provided +// exclude group: 'org.opensaml' +// } + testCompile project(':backend') testCompile "org.springframework.boot:spring-boot-starter-test" testCompile "org.spockframework:spock-core:1.3-groovy-2.5" testCompile "org.spockframework:spock-spring:1.3-groovy-2.5" annotationProcessor "org.springframework.boot:spring-boot-configuration-processor" -} +} \ No newline at end of file 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 ee440c13f..548380712 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 @@ -10,21 +10,30 @@ 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.context.J2EContext; +import org.pac4j.core.context.WebContext; +import org.pac4j.core.matching.Matcher; import org.pac4j.core.profile.CommonProfile; 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.*; +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.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.transaction.Transactional; import java.io.IOException; -import java.util.*; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Optional; +import java.util.Set; @Slf4j public class AddNewUserFilter implements Filter { @@ -94,8 +103,8 @@ public void destroy() { @Override public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { - JEEContext context = new JEEContext((HttpServletRequest)request, (HttpServletResponse)response); - if (!matcher.matches(context, JEESessionStore.INSTANCE)) { + WebContext context = new J2EContext((HttpServletRequest)request, (HttpServletResponse)response); + if (!matcher.matches(context)) { return; } Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); diff --git a/pac4j-module/src/main/java/net/unicon/shibui/pac4j/LocalUserProfileAuthorizationGenerator.java b/pac4j-module/src/main/java/net/unicon/shibui/pac4j/LocalUserProfileAuthorizationGenerator.java index 1f0f0cba1..5d9642409 100644 --- a/pac4j-module/src/main/java/net/unicon/shibui/pac4j/LocalUserProfileAuthorizationGenerator.java +++ b/pac4j-module/src/main/java/net/unicon/shibui/pac4j/LocalUserProfileAuthorizationGenerator.java @@ -1,14 +1,12 @@ package net.unicon.shibui.pac4j; -import java.util.Optional; - +import edu.internet2.tier.shibboleth.admin.ui.security.model.User; +import edu.internet2.tier.shibboleth.admin.ui.security.repository.UserRepository; import org.pac4j.core.authorization.generator.AuthorizationGenerator; import org.pac4j.core.context.WebContext; -import org.pac4j.core.context.session.SessionStore; -import org.pac4j.core.profile.UserProfile; +import org.pac4j.core.profile.CommonProfile; -import edu.internet2.tier.shibboleth.admin.ui.security.model.User; -import edu.internet2.tier.shibboleth.admin.ui.security.repository.UserRepository; +import java.util.Optional; public class LocalUserProfileAuthorizationGenerator implements AuthorizationGenerator { private final UserRepository userRepository; @@ -18,9 +16,9 @@ public LocalUserProfileAuthorizationGenerator(UserRepository userRepository) { } @Override - public Optional generate(WebContext context, SessionStore sessionStore, UserProfile profile) { + public CommonProfile generate(WebContext context, CommonProfile profile) { Optional user = userRepository.findByUsername(profile.getUsername()); - user.ifPresent( u -> profile.addRole(u.getRole()) ); - return Optional.of(profile); + user.ifPresent(u -> profile.addRole(u.getRole())); + return profile; } -} +} \ 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 39f12035b..cda47108b 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 @@ -8,17 +8,16 @@ import org.pac4j.core.client.Clients; import org.pac4j.core.config.Config; import org.pac4j.core.context.WebContext; -import org.pac4j.core.context.session.SessionStore; import org.pac4j.core.credentials.Credentials; import org.pac4j.core.credentials.TokenCredentials; import org.pac4j.core.credentials.authenticator.Authenticator; import org.pac4j.core.exception.CredentialsException; -import org.pac4j.core.matching.matcher.PathMatcher; +import org.pac4j.core.matching.PathMatcher; import org.pac4j.core.profile.CommonProfile; import org.pac4j.core.profile.definition.CommonProfileDefinition; import org.pac4j.http.client.direct.HeaderClient; import org.pac4j.saml.client.SAML2Client; -import org.pac4j.saml.config.SAML2Configuration; +import org.pac4j.saml.client.SAML2ClientConfiguration; import org.pac4j.saml.credentials.authenticator.SAML2Authenticator; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; @@ -65,9 +64,39 @@ public Config config(final Pac4jConfigurationProperties pac4jConfigProps, // Configure the client switch (pac4jConfigProps.getTypeOfAuth()) { - case "SAML2": { + case "HEADER": { + log.info("**** Configuring PAC4J Header Client"); + HeaderClient headerClient = new HeaderClient(pac4jConfigProps.getAuthenticationHeader(), + new Authenticator() { + @Override + public void validate(Credentials credentials, WebContext context) { + { + if (credentials instanceof TokenCredentials) { + TokenCredentials creds = (TokenCredentials) credentials; + String token = creds.getToken(); + if (StringUtils.isAllBlank(token)) { + throw new CredentialsException("Supplied token value in header was missing or blank"); + } + } else { + throw new CredentialsException("Invalid Credentials object generated by HeaderClient"); + } + final CommonProfile profile = new CommonProfile(); + String token = ((TokenCredentials)credentials).getToken(); + profile.setId(token); + profile.addAttribute("username", token); + profile.setRoles(userService.getUserRoles(token)); + credentials.setUserProfile(profile); + } + } + }); + headerClient.setName(PAC4J_CLIENT_NAME); + clients.setClients(headerClient); + break; + } + case "SAML2": + default: log.info("**** Configuring PAC4J SAML2"); - final SAML2Configuration saml2Config = new SAML2Configuration(); + final SAML2ClientConfiguration saml2Config = new SAML2ClientConfiguration(); saml2Config.setKeystorePath(pac4jConfigProps.getKeystorePath()); saml2Config.setKeystorePassword(pac4jConfigProps.getKeystorePassword()); saml2Config.setPrivateKeyPassword(pac4jConfigProps.getPrivateKeyPassword()); @@ -89,34 +118,8 @@ public Config config(final Pac4jConfigurationProperties pac4jConfigProps, saml2Client.setName(PAC4J_CLIENT_NAME); clients.setClients(saml2Client); + break; } - case "HEADER": { - log.info("**** Configuring PAC4J Header Client"); - HeaderClient headerClient = new HeaderClient(pac4jConfigProps.getAuthenticationHeader(), - new Authenticator() { - @Override - public void validate(Credentials credentials, WebContext context, SessionStore sessionStore) { - if (credentials instanceof TokenCredentials) { - TokenCredentials creds = (TokenCredentials) credentials; - String token = creds.getToken(); - if (StringUtils.isAllBlank(token)) { - throw new CredentialsException("Supplied token value in header was missing or blank"); - } - } else { - throw new CredentialsException("Invalid Credentials object generated by HeaderClient"); - } - final CommonProfile profile = new CommonProfile(); - String token = ((TokenCredentials)credentials).getToken(); - profile.setId(token); - profile.addAttribute("username", token); - profile.setRoles(userService.getUserRoles(token)); - credentials.setUserProfile(profile); - } - }); - headerClient.setName(PAC4J_CLIENT_NAME); - clients.setClients(headerClient); - } - } config.setClients(clients); return config; } 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 ac7633afa..b9b14a168 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 @@ -5,8 +5,10 @@ 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 static net.unicon.shibui.pac4j.Pac4jConfiguration.PAC4J_CLIENT_NAME; import org.pac4j.core.config.Config; -import org.pac4j.core.matching.matcher.Matcher; + +import org.pac4j.core.matching.Matcher; import org.pac4j.springframework.security.web.CallbackFilter; import org.pac4j.springframework.security.web.SecurityFilter; import org.springframework.boot.autoconfigure.AutoConfigureAfter; @@ -61,15 +63,16 @@ public Pac4jWebSecurityConfigurerAdapter(final Config config, UserService userSe protected void configure(HttpSecurity http) throws Exception { http.authorizeRequests().antMatchers("/unsecured/**/*").permitAll(); - final SecurityFilter securityFilter = new SecurityFilter(this.config, "Saml2Client"); + final SecurityFilter securityFilter = new SecurityFilter(this.config, PAC4J_CLIENT_NAME); // 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, userService, roleRepository, getPathMatcher("exclude-paths-matcher"), groupService, emailService), SecurityFilter.class); - - http.exceptionHandling().accessDeniedHandler((request, response, accessDeniedException) -> response.sendRedirect("/unsecured/error.html")); + + http.authorizeRequests().anyRequest().fullyAuthenticated(); + http.sessionManagement().sessionCreationPolicy(SessionCreationPolicy.ALWAYS); http.csrf().disable(); http.headers().frameOptions().disable(); @@ -84,7 +87,7 @@ private Filter getFilter(Config config2, String typeOfAuth) { case "SAML2": return new CallbackFilter(this.config); case "HEADER": - final SecurityFilter securityFilterForHeader = new SecurityFilter(this.config, Pac4jConfiguration.PAC4J_CLIENT_NAME); + final SecurityFilter securityFilterForHeader = new SecurityFilter(this.config, PAC4J_CLIENT_NAME); securityFilterForHeader.setMatchers("exclude-paths-matcher"); return securityFilterForHeader; } 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 1f729beb7..f39a8b988 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 @@ -6,8 +6,7 @@ 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.UserService import edu.internet2.tier.shibboleth.admin.ui.service.EmailService - -import org.pac4j.core.matching.matcher.PathMatcher +import org.pac4j.core.matching.PathMatcher import org.pac4j.saml.profile.SAML2Profile import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.context.properties.EnableConfigurationProperties 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 9b8f6131d..a7c63e1a0 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 @@ -7,7 +7,7 @@ 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.core.matching.PathMatcher import org.pac4j.saml.profile.SAML2Profile import org.springframework.beans.factory.annotation.Autowired import org.springframework.boot.autoconfigure.domain.EntityScan