Merged in hotfix/shibui-2081 (pull request #524)

SHIBUI-2081
TIER · Sep 10, 2021 · b06c32d · b06c32d
2 parents dca464f + 1609b6c
commit b06c32d
Show file tree

Hide file tree

Showing 51 changed files with 22,742 additions and 67 deletions.
diff --git 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
-}
+}
diff --git 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"
-}
+}
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
@@ -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/BetterSAML2Profile.java b/pac4j-module/src/main/java/net/unicon/shibui/pac4j/BetterSAML2Profile.java
@@ -13,19 +13,26 @@ public BetterSAML2Profile(final SimpleProfileMapping simpleProfileMapping) {
         this.profileMapping = simpleProfileMapping;
     }
 
+    private String getSingleAttributeValue(Object attribute) {
+        if (attribute instanceof Collection) {
+            return (String) ((Collection)attribute).toArray()[0];
+        }
+        return (String) attribute;
+    }
+
     @Override
     public String getEmail() {
-        return (String) getAttribute(profileMapping.getEmail());
+        return getSingleAttributeValue(getAttribute(profileMapping.getEmail()));
     }
 
     @Override
     public String getFamilyName() {
-        return (String) getAttribute(profileMapping.getLastName());
+        return getSingleAttributeValue(getAttribute(profileMapping.getLastName()));
     }
 
     @Override
     public String getFirstName() {
-        return (String) getAttribute(profileMapping.getFirstName());
+        return getSingleAttributeValue(getAttribute(profileMapping.getFirstName()));
     }
 
     public List<String> getGroups() {
@@ -42,4 +49,4 @@ public String getUsername() {
         }
     }
 
-}
+}
diff --git a/...-module/src/main/java/net/unicon/shibui/pac4j/LocalUserProfileAuthorizationGenerator.java b/...-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<UserProfile> generate(WebContext context, SessionStore sessionStore, UserProfile profile) {
+    public CommonProfile generate(WebContext context, CommonProfile profile) {
         Optional<User> 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;
     }
-}
+}
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
@@ -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
@@ -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
@@ -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
@@ -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