Switch to Spring Security for authentication and allow local OAuth 2 log in

Instead of storing the current user in the Wicket session, let Spring Security handle it. The CurrentUser implementation has been changed to look it up from there instead of the Wicket session.

Also enable, in addition to Shibboleth (pre-authenticated remote user), OAuth 2 login which removes the need for the locally modified web.xml with a faked remote user. The Docker Compose file has been updated to run a OAuth 2 container for this type of login.

README.md

@@ -1,3 +1,7 @@
+## Working with the web GUI (Wicket)
+The web GUI is protected by OAuth 2 log in. Run the Docker Compose containers with
+`docker compose up` to start the authorization server to be able to log in.
+
 ## Working with the API
 The API is protected by OAuth 2 acting as a [resource server](https://www.oauth.com/oauth2-servers/the-resource-server/)
 verifying tokens using [token introspection](https://datatracker.ietf.org/doc/html/rfc7662).

core/src/main/java/se/su/dsv/scipro/system/AuthenticationContext.java

@@ -0,0 +1,15 @@
+package se.su.dsv.scipro.system;
+
+/**
+ * Information about the current authentication context.
+ * <p>
+ * The difference between this and {@link CurrentUser} is that a user can be
+ * authenticated without being a user in the system. This can happen when a
+ * user logs in for the first time via SSO. The {@link #set(User)} method can
+ * be used if the user can be imported based on the {@link #getPrincipalName()}.
+ */
+public interface AuthenticationContext extends CurrentUser {
+    void set(User user);
+
+    String getPrincipalName();
+}

daisy-integration/src/main/java/se/su/dsv/scipro/daisyExternal/impl/ImporterTransactionsImpl.java

@@ -157,6 +157,7 @@ public class ImporterTransactionsImpl implements ImporterTransactions {
                 username.setUsername(completeUsername);
                 username.setUser(local);
                 userNameService.save(username);
+                local.getUsernames().add(username);
             }
         }
     }

docker-compose.yml

@@ -13,3 +13,14 @@ services:
       - CLIENT_REDIRECT_URI=http://localhost:59732/
       - RESOURCE_SERVER_ID=scipro-api-client
       - RESOURCE_SERVER_SECRET=scipro-api-secret
+  oauth2-wicket:
+    build:
+      context: https://github.com/dsv-su/toker.git
+      dockerfile: embedded.Dockerfile
+    restart: on-failure
+    ports:
+      - '59734:8080'
+    environment:
+      - CLIENT_ID=scipro
+      - CLIENT_SECRET=s3cr3t
+      - CLIENT_REDIRECT_URI=http://localhost:8080/login/oauth2/code/scipro

view/src/main/java/se/su/dsv/scipro/CurrentUserFromWicketSession.java

@@ -1,13 +0,0 @@
-package se.su.dsv.scipro;
-
-import org.apache.wicket.Session;
-import se.su.dsv.scipro.session.SciProSession;
-import se.su.dsv.scipro.system.CurrentUser;
-import se.su.dsv.scipro.system.User;
-
-public class CurrentUserFromWicketSession implements CurrentUser {
-    @Override
-    public User get() {
-        return Session.exists() ? SciProSession.get().getUser() : null;
-    }
-}

view/src/main/java/se/su/dsv/scipro/loginlogout/pages/SSOPage.java

@@ -4,11 +4,12 @@ import se.su.dsv.scipro.basepages.PublicPage;
 import se.su.dsv.scipro.security.auth.Authorization;
 import se.su.dsv.scipro.session.SciProSession;
 import se.su.dsv.scipro.system.User;
+import se.su.dsv.scipro.system.AuthenticationContext;
 import se.su.dsv.scipro.system.UserImportService;
 import se.su.dsv.scipro.system.UserService;
 
 import jakarta.inject.Inject;
-import jakarta.servlet.http.HttpServletRequest;
+
 import java.util.Optional;
 import java.util.Set;
 
@@ -21,8 +22,11 @@ public class SSOPage extends PublicPage {
     @Inject
     private UserService userService;
 
+    @Inject
+    private AuthenticationContext authenticationContext;
+
     public SSOPage() {
-        String remoteUserName = ((HttpServletRequest) getRequest().getContainerRequest()).getRemoteUser();
+        String remoteUserName = authenticationContext.getPrincipalName();
         User user = userService.findByUsername(remoteUserName);
 
         if (user != null) {

view/src/main/java/se/su/dsv/scipro/security/auth/MockRemoteUserFilter.java

@@ -1,70 +0,0 @@
-package se.su.dsv.scipro.security.auth;
-
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import jakarta.servlet.*;
-import jakarta.servlet.http.HttpServletRequest;
-import jakarta.servlet.http.HttpServletRequestWrapper;
-import java.io.IOException;
-
-/**
- * Throw-away implementation of a servlet filter, main task is to fake the getRemoteUser() call for the request chain.
- */
-public final class MockRemoteUserFilter implements Filter {
-	private static final Logger LOGGER = LoggerFactory.getLogger(MockRemoteUserFilter.class);
-	//Default value unless supplied via init parameter
-	private String fakedUser = "SOME_GUY";
-	private FilterConfig cfg = null;
-    /**
-     * Default constructor. 
-     */
-    public MockRemoteUserFilter() {
-    }
-	/**
-	 * @see Filter#destroy()
-	 */
-	@Override
-    public void destroy() {
-		cfg = null;
-	}
-	/**
-	 * Wraps the passed request and alters the behavior of getRemoteUser() for later links of the chain.
-	 * @see Filter#doFilter(ServletRequest, ServletResponse, FilterChain)
-	 */
-	@Override
-    public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
-		LOGGER.debug("Faking external authentication user: " + fakedUser);
-		if(cfg != null){
-			HttpServletRequestWrapper wrapper = new ModifiedRemoteUserRequestWrapper((HttpServletRequest)request,fakedUser);
-			// pass the request along the filter chain
-			chain.doFilter(wrapper, response);
-			return;
-		}
-		chain.doFilter(request, response);
-	}
-	/**
-	 * @see Filter#init(FilterConfig)
-	 */
-	@Override
-    public void init(FilterConfig fConfig) {
-		cfg = fConfig;
-		if(cfg!=null){
-			fakedUser = cfg.getInitParameter("fakedUser");
-		}
-	}
-	/**
-	 * Private RequestWrapper, of no interest to anyone outside of this class.
-	 */
-	static class ModifiedRemoteUserRequestWrapper extends HttpServletRequestWrapper{
-		private final String fakedUser;
-		ModifiedRemoteUserRequestWrapper(final HttpServletRequest request,final String fakedUser){
-			super(request);
-			this.fakedUser = fakedUser; 
-		}
-		@Override
-		public String getRemoteUser(){
-			return fakedUser;
-		}
-	}
-}

view/src/main/java/se/su/dsv/scipro/session/SciProSession.java

@@ -1,18 +1,18 @@
 package se.su.dsv.scipro.session;
 
-import org.apache.wicket.MetaDataKey;
 import org.apache.wicket.Session;
 import org.apache.wicket.injection.Injector;
 import org.apache.wicket.protocol.http.WebSession;
 import org.apache.wicket.request.Request;
 import se.su.dsv.scipro.security.auth.roles.IRole;
 import se.su.dsv.scipro.security.auth.roles.Roles;
+import se.su.dsv.scipro.system.AuthenticationContext;
 import se.su.dsv.scipro.system.ProjectModule;
 import se.su.dsv.scipro.system.SystemModule;
 import se.su.dsv.scipro.system.User;
-import se.su.dsv.scipro.system.UserService;
 
 import jakarta.inject.Inject;
+
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Locale;
@@ -20,10 +20,8 @@ import java.util.Set;
 
 public class SciProSession extends WebSession {
 
-    private final static MetaDataKey<Long> LOGGED_IN_USER_ID = new MetaDataKey<>() {};
-
     @Inject
-    private UserService userService;
+    private AuthenticationContext authenticationContext;
 
     private Set<ProjectModule> projectModules = new HashSet<>();
     private Set<SystemModule> systemModules = new HashSet<>();
@@ -38,15 +36,15 @@ public class SciProSession extends WebSession {
     }
 
     public synchronized void setUser(User user) {
-        setMetaData(LOGGED_IN_USER_ID, user.getId());
+        authenticationContext.set(user);
     }
 
     public synchronized User getUser() {
-        return isLoggedIn() ? userService.findOne(getMetaData(LOGGED_IN_USER_ID)) : null;
+        return authenticationContext.get();
     }
 
     public synchronized boolean isLoggedIn() {
-        return getMetaData(LOGGED_IN_USER_ID) != null;
+        return authenticationContext.get() != null;
     }
 
     public synchronized boolean authorizedForRole(Roles role) {

view/src/test/java/se/su/dsv/scipro/SciProTest.java

@@ -111,12 +111,10 @@ import se.su.dsv.scipro.springdata.services.UnitService;
 import se.su.dsv.scipro.springdata.services.UserProfileService;
 import se.su.dsv.scipro.supervisor.pages.SupervisorStartPage;
 import se.su.dsv.scipro.survey.SurveyService;
-import se.su.dsv.scipro.system.CurrentUser;
 import se.su.dsv.scipro.system.ExternalResourceService;
 import se.su.dsv.scipro.system.FooterAddressRepo;
 import se.su.dsv.scipro.system.FooterLinkService;
 import se.su.dsv.scipro.system.GenericService;
-import se.su.dsv.scipro.system.Lifecycle;
 import se.su.dsv.scipro.system.PasswordRepo;
 import se.su.dsv.scipro.system.PasswordService;
 import se.su.dsv.scipro.system.ProjectModule;
@@ -124,6 +122,7 @@ import se.su.dsv.scipro.system.ProjectTypeService;
 import se.su.dsv.scipro.system.ResearchAreaService;
 import se.su.dsv.scipro.system.SystemModule;
 import se.su.dsv.scipro.system.User;
+import se.su.dsv.scipro.system.AuthenticationContext;
 import se.su.dsv.scipro.system.UserImportService;
 import se.su.dsv.scipro.system.UserNameService;
 import se.su.dsv.scipro.system.UserSearchService;
@@ -298,7 +297,7 @@ public abstract class SciProTest {
     @Mock
     protected ChecklistAnswerService checklistAnswerService;
     @Mock
-    protected CurrentUser currentUser;
+    protected AuthenticationContext authenticationContext;
     @Mock
     private Scheduler scheduler;
     @Mock

war/pom.xml

@@ -41,6 +41,10 @@
             <artifactId>spring-boot-starter-tomcat</artifactId>
             <scope>provided</scope>
         </dependency>
+        <dependency>
+            <groupId>org.springframework.boot</groupId>
+            <artifactId>spring-boot-starter-oauth2-client</artifactId>
+        </dependency>
         <dependency>
             <groupId>org.springframework</groupId>
             <artifactId>spring-orm</artifactId>

war/src/main/java/se/su/dsv/scipro/war/CurrentUserFromSpringSecurity.java

@@ -0,0 +1,98 @@
+package se.su.dsv.scipro.war;
+
+import jakarta.inject.Inject;
+import jakarta.inject.Provider;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.core.context.SecurityContext;
+import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.security.core.context.SecurityContextHolderStrategy;
+import org.springframework.security.web.context.HttpSessionSecurityContextRepository;
+import org.springframework.security.web.context.SecurityContextRepository;
+import se.su.dsv.scipro.system.User;
+import se.su.dsv.scipro.system.AuthenticationContext;
+import se.su.dsv.scipro.system.UserService;
+import se.su.dsv.scipro.system.Username;
+
+import java.security.Principal;
+import java.util.Collections;
+
+public class CurrentUserFromSpringSecurity implements AuthenticationContext {
+    private final UserService userService;
+
+    // injecting providers since this is a singleton and the request and response are not
+    private final Provider<HttpServletRequest> currentRequest;
+    private final Provider<HttpServletResponse> currentResponse;
+
+    // hardcoded since that is what Spring Security does (see SwitchUserFilter)
+    private final SecurityContextRepository securityContextRepository = new HttpSessionSecurityContextRepository();
+
+    @Inject
+    public CurrentUserFromSpringSecurity(
+            UserService userService,
+            Provider<HttpServletRequest> currentRequest,
+            Provider<HttpServletResponse> currentResponse)
+    {
+        this.userService = userService;
+        this.currentRequest = currentRequest;
+        this.currentResponse = currentResponse;
+    }
+
+    @Override
+    public User get() {
+        SecurityContext context = SecurityContextHolder.getContext();
+        Authentication authentication = context.getAuthentication();
+        if (authentication == null) {
+            return null;
+        }
+        String username = authentication.getName();
+        return userService.findByUsername(username);
+    }
+
+    // Implementing switch user manually rather than using the built-in Spring Security switch user feature
+    // due to compatibility with Wicket.
+    // Wicket does not supply a form with a username field since it has some JavaScript based auto-complete
+    // person finder.
+    // See Spring's SwitchUserFilter for the built-in switch user feature from where most of the code is copied.
+    @Override
+    public void set(User user) {
+        SecurityContextHolderStrategy strategy = SecurityContextHolder.getContextHolderStrategy();
+        SecurityContext context = strategy.createEmptyContext();
+        WicketControlledPrincipal principal = new WicketControlledPrincipal(user);
+        UsernamePasswordAuthenticationToken targetUser = UsernamePasswordAuthenticationToken.authenticated(
+                principal, null, Collections.emptyList());
+        context.setAuthentication(targetUser);
+        strategy.setContext(context);
+        this.securityContextRepository.saveContext(context, currentRequest.get(), currentResponse.get());
+    }
+
+    @Override
+    public String getPrincipalName() {
+        SecurityContext context = SecurityContextHolder.getContext();
+        Authentication authentication = context.getAuthentication();
+        if (authentication == null) {
+            return null;
+        }
+        return authentication.getName();
+    }
+
+    private static final class WicketControlledPrincipal implements Principal {
+        private final String username;
+
+        public WicketControlledPrincipal(User user) {
+            // extract any username so that we can look it up later
+            this.username = user.getUsernames()
+                    .stream()
+                    .findAny()
+                    .map(Username::getUsername)
+                    .orElse("<unknown>");
+        }
+
+        @Override
+        public String getName() {
+            return username;
+        }
+    }
+}

war/src/main/java/se/su/dsv/scipro/war/Main.java

@@ -18,7 +18,6 @@ import org.springframework.core.task.SimpleAsyncTaskExecutor;
 import org.springframework.orm.jpa.SharedEntityManagerCreator;
 import org.springframework.orm.jpa.support.OpenEntityManagerInViewFilter;
 import se.su.dsv.scipro.CoreConfig;
-import se.su.dsv.scipro.CurrentUserFromWicketSession;
 import se.su.dsv.scipro.FileSystemStore;
 import se.su.dsv.scipro.RepositoryConfiguration;
 import se.su.dsv.scipro.file.FileStore;
@@ -83,11 +82,6 @@ public class Main extends SpringBootServletInitializer implements ServletContain
         return currentProfile;
     }
 
-    @Bean
-    public CurrentUserFromWicketSession currentUserFromWicketSession() {
-        return new CurrentUserFromWicketSession();
-    }
-
     @Bean
     public FileStore fileStore() {
         return new FileSystemStore();

war/src/main/java/se/su/dsv/scipro/war/WicketConfiguration.java

@@ -1,6 +1,9 @@
 package se.su.dsv.scipro.war;
 
 import com.google.common.eventbus.EventBus;
+import jakarta.inject.Provider;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
 import org.apache.wicket.protocol.http.WebApplication;
 import org.apache.wicket.protocol.http.WicketFilter;
 import org.apache.wicket.spring.injection.annot.SpringComponentInjector;
@@ -8,6 +11,10 @@ import org.springframework.boot.web.servlet.FilterRegistrationBean;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
+import org.springframework.core.annotation.Order;
+import org.springframework.security.config.Customizer;
+import org.springframework.security.config.annotation.web.builders.HttpSecurity;
+import org.springframework.security.web.SecurityFilterChain;
 import se.su.dsv.scipro.SciProApplication;
 import se.su.dsv.scipro.crosscutting.ForwardPhase2Feedback;
 import se.su.dsv.scipro.crosscutting.NotifyFailedReflection;
@@ -21,6 +28,7 @@ import se.su.dsv.scipro.notifications.NotificationController;
 import se.su.dsv.scipro.profiles.CurrentProfile;
 import se.su.dsv.scipro.reviewing.FinalSeminarApprovalService;
 import se.su.dsv.scipro.reviewing.RoughDraftApprovalService;
+import se.su.dsv.scipro.system.UserService;
 
 @Configuration
 public class WicketConfiguration {
@@ -47,6 +55,28 @@ public class WicketConfiguration {
         return new SciProApplication(currentProfile);
     }
 
+    @Bean
+    @Order(3) // make sure it's after the API security filters
+    public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
+        http.authorizeHttpRequests((requests) -> requests.anyRequest().authenticated());
+        http.jee(Customizer.withDefaults()); // Shibboleth integration
+        http.oauth2Login(Customizer.withDefaults());
+        http.csrf(csrf -> csrf.disable()); // Wicket has its own CSRF protection
+        http.logout(logout -> logout
+                .logoutUrl("/logout")
+                .logoutSuccessUrl("/"));
+        return http.build();
+    }
+
+    @Bean
+    public CurrentUserFromSpringSecurity currentUserFromSpringSecurity(
+            UserService userService,
+            Provider<HttpServletRequest> httpServletRequestProvider,
+            Provider<HttpServletResponse> httpServletResponseProvider)
+    {
+        return new CurrentUserFromSpringSecurity(userService, httpServletRequestProvider, httpServletResponseProvider);
+    }
+
     @Bean
     public ReviewingNotifications reviewingNotifications(
             EventBus eventBus,

war/src/main/resources/application.properties

@@ -18,3 +18,14 @@ springdoc.swagger-ui.persist-authorization=true
 spring.security.oauth2.resourceserver.opaquetoken.client-id=scipro-api-client
 spring.security.oauth2.resourceserver.opaquetoken.client-secret=scipro-api-secret
 spring.security.oauth2.resourceserver.opaquetoken.introspection-uri=http://localhost:59733/introspect
+
+# Log in via local OAuth 2 authorization server
+spring.security.oauth2.client.provider.docker.user-info-uri=http://localhost:59734/verify
+spring.security.oauth2.client.provider.docker.user-name-attribute=sub
+spring.security.oauth2.client.provider.docker.token-uri=http://localhost:59734/exchange
+spring.security.oauth2.client.provider.docker.authorization-uri=http://localhost:59734/authorize
+spring.security.oauth2.client.registration.scipro.redirect-uri={baseUrl}/login/oauth2/code/{registrationId}
+spring.security.oauth2.client.registration.scipro.provider=docker
+spring.security.oauth2.client.registration.scipro.client-id=scipro
+spring.security.oauth2.client.registration.scipro.client-secret=s3cr3t
+spring.security.oauth2.client.registration.scipro.authorization-grant-type=authorization_code