fixing testsuite, also partially reverts previous cherry-picks

* Testsuite broken because sending a request with no Accept header (or "Accept: */*") will make spring sending a 401 instead of a 302 redirect to /login * restoring previous work on csrf/cors not present in DT's fork (#73, #59) * restoring some other works related to login/logout controllers/templates
georchestra · Dec 6, 2023 · ba74f14 · ba74f14
1 parent 283cbe2
commit ba74f14
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 7 deletions.
diff --git a/gateway/src/main/java/org/georchestra/gateway/app/GeorchestraGatewayApplication.java b/gateway/src/main/java/org/georchestra/gateway/app/GeorchestraGatewayApplication.java
@@ -29,7 +29,10 @@
 import org.springframework.boot.autoconfigure.security.oauth2.client.OAuth2ClientProperties;
 import org.springframework.boot.context.event.ApplicationReadyEvent;
 import org.springframework.cloud.gateway.route.RouteLocator;
+import org.springframework.context.MessageSource;
+import org.springframework.context.annotation.Bean;
 import org.springframework.context.event.EventListener;
+import org.springframework.context.support.ReloadableResourceBundleMessageSource;
 import org.springframework.core.env.Environment;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.oauth2.client.registration.ClientRegistration;
@@ -45,6 +48,7 @@
 
 import javax.annotation.PostConstruct;
 import java.io.File;
+import java.nio.charset.StandardCharsets;
 import java.util.*;
 
 @Controller
@@ -61,6 +65,7 @@ public class GeorchestraGatewayApplication {
     private @Value("${georchestra.gateway.headerUrl:/header/}") String georchestraHeaderUrl;
     private @Value("${georchestra.gateway.headerHeight:90}") String georchestraHeaderHeight;
     private @Value("${georchestra.gateway.footerUrl:#{null}}") String georchestraFooterUrl;
+    private @Value("${spring.messages.basename:}") String messagesBasename;
 
     @PostConstruct
     void initialize() {
@@ -137,4 +142,14 @@ public void onApplicationReady(ApplicationReadyEvent e) {
                 routeCount, instanceId, cpus, maxMem);
     }
 
+    @Bean
+    public MessageSource messageSource() {
+        ReloadableResourceBundleMessageSource messageSource = new ReloadableResourceBundleMessageSource();
+        messageSource.setBasenames(("classpath:messages/login," + messagesBasename).split(","));
+        messageSource.setCacheSeconds(600);
+        messageSource.setUseCodeAsDefaultMessage(true);
+        messageSource.setDefaultEncoding(StandardCharsets.UTF_8.name());
+        return messageSource;
+    }
+
 }
diff --git a/gateway/src/main/java/org/georchestra/gateway/security/GatewaySecurityConfiguration.java b/gateway/src/main/java/org/georchestra/gateway/security/GatewaySecurityConfiguration.java
@@ -20,13 +20,15 @@
 
 import lombok.extern.slf4j.Slf4j;
 import org.georchestra.gateway.model.GatewayConfigProperties;
+import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.Description;
 import org.springframework.security.config.annotation.web.reactive.EnableWebFluxSecurity;
 import org.springframework.security.config.web.server.ServerHttpSecurity;
 import org.springframework.security.web.server.SecurityWebFilterChain;
+import org.springframework.security.web.server.authentication.logout.ServerLogoutSuccessHandler;
 import org.springframework.web.reactive.result.view.UrlBasedViewResolver;
 import org.springframework.web.reactive.result.view.ViewResolver;
 import org.springframework.web.reactive.result.view.freemarker.FreeMarkerView;
@@ -55,6 +57,9 @@
 @Slf4j(topic = "org.georchestra.gateway.security")
 public class GatewaySecurityConfiguration {
 
+    @Autowired(required = false)
+    ServerLogoutSuccessHandler oidcLogoutSuccessHandler;
+
     /**
      * Relies on available {@link ServerHttpSecurityCustomizer} extensions to
      * configure the different aspects of the {@link ServerHttpSecurity} used to
@@ -65,20 +70,29 @@ public SecurityWebFilterChain securityWebFilterChain(ServerHttpSecurity http,
             List<ServerHttpSecurityCustomizer> customizers) throws Exception {
 
         log.info("Initializing security filter chain...");
-        // disable csrf and cors or the websocket connection gets a 403 Forbidden.
-        // Revisit.
-        log.info("CSRF and CORS disabled. Revisit how they interfer with Websockets proxying.");
-        http.csrf().disable().cors().disable();
+        // disable CSRF protection, considering it will be managed
+        // by proxified webapps, not the gateway.
+        http.csrf().disable();
 
-        http.formLogin().loginPage("/login");
+        http.formLogin()
+                .authenticationFailureHandler(new ExtendedRedirectServerAuthenticationFailureHandler("login?error"))
+                .loginPage("/login");
 
         sortedCustomizers(customizers).forEach(customizer -> {
             log.debug("Applying security customizer {}", customizer.getName());
             customizer.customize(http);
         });
 
         log.info("Security filter chain initialized");
-        return http.build();
+
+        // TODO: why do we need to recall formLogin().loginPage() ?
+        ServerHttpSecurity.LogoutSpec logoutUrl = http.formLogin().loginPage("/login").and().logout()
+                .logoutUrl("/logout");
+        if (oidcLogoutSuccessHandler != null) {
+            logoutUrl = logoutUrl.logoutSuccessHandler(oidcLogoutSuccessHandler);
+        }
+
+        return logoutUrl.and().build();
     }
 
     private Stream<ServerHttpSecurityCustomizer> sortedCustomizers(List<ServerHttpSecurityCustomizer> customizers) {

diff --git a/...y/src/test/java/org/georchestra/gateway/security/accessrules/AccessRulesCustomizerIT.java b/...y/src/test/java/org/georchestra/gateway/security/accessrules/AccessRulesCustomizerIT.java
@@ -158,10 +158,12 @@ private static void mockServiceTarget(DynamicPropertyRegistry registry, String s
         mockService.stubFor(get(urlMatching("/mapfishapp(/.*)?")).willReturn(ok()));
 
         testClient.get().uri("/mapfishapp/ogcproxy")//
+                .header("accept", "text/html")//
                 .exchange()//
                 .expectHeader().location("/login");
 
         testClient.get().uri("/mapfishapp/ogcproxy/somethingprivate")//
+                .header("accept", "text/html")//
                 .exchange()//
                 .expectHeader().location("/login");
 
@@ -212,11 +214,12 @@ private static void mockServiceTarget(DynamicPropertyRegistry registry, String s
         mockService.stubFor(get(urlMatching("/import(/.*)?")).willReturn(noContent()));
 
         testClient.get().uri("/import")//
+                .header("Accept", "text/html")//
                 .exchange()//
                 .expectHeader().location("/login");
 
         testClient.get().uri("/import/any/thing")//
-                .exchange()//
+                .header("Accept", "text/html").exchange()//
                 .expectHeader().location("/login");
     }
 
@@ -299,10 +302,12 @@ private static void mockServiceTarget(DynamicPropertyRegistry registry, String s
         mockService.stubFor(get(urlMatching("/analytics(/.*)?")).willReturn(ok()));
 
         testClient.get().uri("/analytics")//
+                .header("Accept", "text/html")//
                 .exchange()//
                 .expectHeader().location("/login");
 
         testClient.get().uri("/analytics/any/thing")//
+                .header("Accept", "text/html")//
                 .exchange()//
                 .expectHeader().location("/login");
     }