conflicts with develop

pom.xml

@@ -143,7 +143,6 @@ knowledge of the CeCILL-B license and that you accept its terms.
                 <artifactId>logback-classic</artifactId>
                 <version>1.2.3</version>
             </dependency>
-
             <!-- make sure we pick a version of commons-io recent enough for
               commons-fileupload2-jakarta-servlet6 in vip-datamanagement,
               otherwise commons-io 2.4 is pulled by com.github.h-thurow -->
@@ -217,6 +216,8 @@ knowledge of the CeCILL-B license and that you accept its terms.
             <version>2.2.0</version>
             <scope>test</scope>
         </dependency>
+
+
     </dependencies>
 
     <repositories>

vip-api/pom.xml

@@ -98,6 +98,11 @@ knowledge of the CeCILL-B license and that you accept its terms.
             <artifactId>spring-security-web</artifactId>
             <version>${springsecurity.version}</version>
         </dependency>
+        <dependency>
+            <groupId>org.springframework.security</groupId>
+            <artifactId>spring-security-oauth2-resource-server</artifactId>
+            <version>${springoauth.version}</version>
+        </dependency>
 
         <!-- Validation implementation -->
         <dependency>

vip-api/src/main/java/fr/insalyon/creatis/vip/api/ApiPropertiesInitializer.java

@@ -96,7 +96,7 @@ private void verifyProperties() {
 
 
         if (env.getProperty(KEYCLOAK_ACTIVATED, Boolean.class, Boolean.FALSE)) {
-            logger.info("Keycloak/OIDC activated, but this has no effect yet");
+            logger.info("Keycloak/OIDC activated");
         } else {
             logger.info("Keycloak/OIDC NOT active");
         }

vip-api/src/main/java/fr/insalyon/creatis/vip/api/SpringWebConfig.java

@@ -63,15 +63,6 @@ public SpringWebConfig(Environment env, VipConfigurer vipConfigurer) {
         this.vipConfigurer = vipConfigurer;
     }
 
-    @Override
-    public void configurePathMatch(PathMatchConfigurer configurer) {
-        // Otherwise all that follow a dot in an URL is considered an extension and removed
-        // It's a problem for URL like "/pipelines/gate/3.2
-        // The below will become the default values in Spring 5.3
-        // Safe to use in 5.2 as long as disabling pattern match
-        configurer.setUseSuffixPatternMatch(false);
-    }
-
     @Override
     public void configureContentNegotiation(ContentNegotiationConfigurer configurer) {
         // necessary in the content negotiation stuff of carmin data

vip-api/src/main/java/fr/insalyon/creatis/vip/api/business/DataApiBusiness.java

@@ -52,6 +52,7 @@
 import fr.insalyon.creatis.vip.datamanager.server.business.TransferPoolBusiness;
 import org.apache.commons.io.FilenameUtils;
 import org.apache.commons.io.input.ReaderInputStream;
+import org.apache.commons.io.input.ReaderInputStream.Builder;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -391,8 +392,12 @@ private boolean isOperationOver(String operationId, User user)
     private void writeFileFromBase64(String base64Content, String localFilePath) throws ApiException {
         Base64.Decoder decoder = Base64.getDecoder();
         StringReader stringReader = new StringReader(base64Content);
-        InputStream inputStream = new ReaderInputStream(stringReader, StandardCharsets.UTF_8);
-        try (InputStream base64InputStream = decoder.wrap(inputStream)) {
+        try {
+            InputStream inputStream = ReaderInputStream.builder()
+                    .setReader(new StringReader(base64Content))
+                    .setCharset(StandardCharsets.UTF_8)
+                    .get();
+            InputStream base64InputStream = decoder.wrap(inputStream);
             Files.copy(base64InputStream, Paths.get(localFilePath));
         } catch (IOException e) {
             logger.error("Error writing base64 file in {}", localFilePath, e);

vip-api/src/main/java/fr/insalyon/creatis/vip/api/security/ApiSecurityConfig.java

@@ -38,6 +38,9 @@
 import fr.insalyon.creatis.vip.api.security.apikey.ApikeyAuthenticationProvider;
 import fr.insalyon.creatis.vip.core.client.bean.User;
 
+import fr.insalyon.creatis.vip.api.security.oidc.OidcConfig;
+import fr.insalyon.creatis.vip.api.security.oidc.OidcResolver;
+
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -47,9 +50,6 @@
 import org.springframework.core.env.Environment;
 import org.springframework.stereotype.Service;
 
-import org.springframework.security.authentication.AuthenticationManager;
-import org.springframework.security.authentication.AuthenticationProvider;
-import org.springframework.security.authentication.ProviderManager;
 import org.springframework.security.config.annotation.web.builders.HttpSecurity;
 import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
 import org.springframework.security.config.Customizer;
@@ -60,21 +60,17 @@
 import org.springframework.security.web.SecurityFilterChain;
 import org.springframework.security.web.authentication.www.BasicAuthenticationFilter;
 import org.springframework.security.web.firewall.DefaultHttpFirewall;
-import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
+import static org.springframework.security.web.util.matcher.AntPathRequestMatcher.antMatcher;
 import org.springframework.security.web.util.matcher.RegexRequestMatcher;
 
 import java.util.function.Supplier;
-import java.util.ArrayList;
-
-import static fr.insalyon.creatis.vip.api.CarminProperties.KEYCLOAK_ACTIVATED;
 
 /**
  * VIP API configuration for API key and OIDC authentications.
  *
  * Authenticates /rest requests with either:
- * - a static per-user API key (ApikeyAuthenticationFilter)
- * - or an OIDC Bearer token (ex-Keycloak). This part is currently work-in-progress,
- *   with org.keycloak currently removed, and proper OIDC connector not implemented yet.
+ * - a static per-user API key, in apikeyAuthenticationFilter()
+ * - or an OIDC Bearer token, in oauth2ResourceServer()
  */
 @Configuration
 @EnableWebSecurity
@@ -84,90 +80,96 @@ public class ApiSecurityConfig {
 
     private final Environment env;
     private final VipAuthenticationEntryPoint vipAuthenticationEntryPoint;
-    private final AuthenticationManager vipAuthenticationManager;
+    private final ApikeyAuthenticationProvider apikeyAuthenticationProvider;
+    private final OidcConfig oidcConfig;
+    private final OidcResolver oidcResolver;
 
     @Autowired
     public ApiSecurityConfig(
             Environment env, ApikeyAuthenticationProvider apikeyAuthenticationProvider,
-            VipAuthenticationEntryPoint vipAuthenticationEntryPoint) {
+            VipAuthenticationEntryPoint vipAuthenticationEntryPoint,
+            OidcConfig oidcConfig, OidcResolver oidcResolver) {
         this.env = env;
         this.vipAuthenticationEntryPoint = vipAuthenticationEntryPoint;
-        // Build our AuthenticationManager instance, with one provider for each authentication method
-        ArrayList<AuthenticationProvider> providers = new ArrayList<>();
-        providers.add(apikeyAuthenticationProvider);
-        // providers.add(oidcAuthenticationProvider);
-        this.vipAuthenticationManager = new ProviderManager(providers);
-    }
-
-    protected boolean isOIDCActive() {
-        return env.getProperty(KEYCLOAK_ACTIVATED, Boolean.class, Boolean.FALSE);
+        this.apikeyAuthenticationProvider = apikeyAuthenticationProvider;
+        this.oidcConfig = oidcConfig;
+        this.oidcResolver = oidcResolver;
     }
 
     @Bean
     @Order(1)
     public SecurityFilterChain apiFilterChain(HttpSecurity http) throws Exception {
-        // It is required to used AntPathRequestMatcher.antMatcher() everywhere below,
-        // otherwise Spring users MvcRequestMatcher as the default requestMatchers implementation.
+        // Spring Security configuration for /rest API endpoints, common to both API key and OIDC authentications.
+        // Note that it is required to used AntPathRequestMatcher.antMatcher() everywhere below,
+        // otherwise Spring uses MvcRequestMatcher as the default requestMatchers implementation.
         http
-                .securityMatcher(AntPathRequestMatcher.antMatcher("/rest/**"))
+                .securityMatcher(antMatcher("/rest/**"))
                 .authorizeHttpRequests((authorize) -> authorize
-                        .requestMatchers(AntPathRequestMatcher.antMatcher("/rest/platform")).permitAll()
-                        .requestMatchers(AntPathRequestMatcher.antMatcher("/rest/authenticate")).permitAll()
-                        .requestMatchers(AntPathRequestMatcher.antMatcher("/rest/session")).permitAll()
+                        .requestMatchers(antMatcher("/rest/platform")).permitAll()
+                        .requestMatchers(antMatcher("/rest/authenticate")).permitAll()
+                        .requestMatchers(antMatcher("/rest/session")).permitAll()
                         .requestMatchers(new RegexRequestMatcher("/rest/pipelines\\?public", "GET")).permitAll()
-                        .requestMatchers(AntPathRequestMatcher.antMatcher("/rest/publications")).permitAll()
-                        .requestMatchers(AntPathRequestMatcher.antMatcher("/rest/reset-password")).permitAll()
-                        .requestMatchers(AntPathRequestMatcher.antMatcher("/rest/register")).permitAll()
-                        .requestMatchers(AntPathRequestMatcher.antMatcher("/rest/executions/{executionId}/summary")).hasAnyRole("SERVICE")
-                        .requestMatchers(AntPathRequestMatcher.antMatcher("/rest/statistics/**")).hasAnyRole("ADVANCED", "ADMINISTRATOR")
-                        .requestMatchers(AntPathRequestMatcher.antMatcher("/rest/**")).authenticated()
+                        .requestMatchers(antMatcher("/rest/publications")).permitAll()
+                        .requestMatchers(antMatcher("/rest/reset-password")).permitAll()
+                        .requestMatchers(antMatcher("/rest/register")).permitAll()
+                        .requestMatchers(antMatcher("/rest/executions/{executionId}/summary")).hasAnyRole("SERVICE")
+                        .requestMatchers(antMatcher("/rest/statistics/**")).hasAnyRole("ADVANCED", "ADMINISTRATOR")
+                        .requestMatchers(antMatcher("/rest/**")).authenticated()
                         .anyRequest().permitAll()
                 )
-                .addFilterBefore(apikeyAuthenticationFilter(), BasicAuthenticationFilter.class)
-                //.addFilterBefore(oidcAuthenticationFilter(), BasicAuthenticationFilter.class)
                 .exceptionHandling((exceptionHandling) -> exceptionHandling.authenticationEntryPoint(vipAuthenticationEntryPoint))
                 // session must be activated otherwise OIDC auth info will be lost when accessing /loginEgi
                 // .sessionManagement((sessionManagement) -> sessionManagement.sessionCreationPolicy(SessionCreationPolicy.STATELESS))
                 .cors(Customizer.withDefaults())
                 .headers((headers) -> headers.frameOptions((frameOptions) -> frameOptions.sameOrigin()))
                 .csrf((csrf) -> csrf.disable());
+        // API key authentication always active
+        http.addFilterBefore(apikeyAuthenticationFilter(), BasicAuthenticationFilter.class);
+        // OIDC Bearer token authentication, if enabled
+        if (oidcConfig.isOIDCActive()) {
+            // We configure each OIDC server with issuerLocation instead of jwks_uri: on first token verification,
+            // this does two requests to the relevant OIDC server (obtained from the JWT "iss" field):
+            // - a GET .well-known/openid-configuration request to the OIDC server, to get this server jwks_uri
+            // - then a GET on the jwks_uri, to get the public key which is then used to verify the token
+            // Note that these two requests are done just once per OIDC server (not once per inbound API request).
+            // We also use a customized authenticationManagerResolver instead of simpler JwtDecoder bean, so that:
+            // - requests to the OIDC server happen at inbound-request-time instead of boot, and can be retried on failure
+            // - multiple servers can be supported
+            // - on successful authentication, Jwt principal is converted to a User principal, so DB lookup happens only once
+            http.oauth2ResourceServer((oauth2) -> oauth2
+                    .authenticationManagerResolver(oidcResolver.getAuthenticationManagerResolver()));
+        }
         return http.build();
     }
 
     @Bean
     public ApikeyAuthenticationFilter apikeyAuthenticationFilter() throws Exception {
         return new ApikeyAuthenticationFilter(
                 env.getRequiredProperty(CarminProperties.APIKEY_HEADER_NAME),
-                vipAuthenticationEntryPoint, vipAuthenticationManager);
+                vipAuthenticationEntryPoint, apikeyAuthenticationProvider);
     }
 
+    // Provide authenticated user after a successful API key or OIDC token authentication
     @Service
     public static class CurrentUserProvider implements Supplier<User> {
 
         private final Logger logger = LoggerFactory.getLogger(getClass());
 
         @Override
         public User get() {
-            Authentication authentication =
-                    SecurityContextHolder.getContext().getAuthentication();
+            Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
             if (authentication == null) {
                 return null;
             }
-            User user = getApikeyUser(authentication);
-            if (user != null) {
-                return user;
-            }
-            // user = getOidcUser(authentication);
-            return null;
-        }
-
-        private User getApikeyUser(Authentication authentication) {
-            if ( ! (authentication.getPrincipal() instanceof SpringApiPrincipal)) {
+            Object principal = authentication.getPrincipal();
+            if (principal instanceof SpringApiPrincipal) { // API key authentication
+                return ((SpringApiPrincipal) principal).getVipUser();
+            } else if (principal instanceof User) { // OIDC authentication
+                return (User) principal;
+            } else { // no resolvable user found (shouldn't happen)
+                logger.error("CurrentUserProvider: unknown principal class {}", principal.getClass());
                 return null;
             }
-            SpringApiPrincipal springCompatibleUser =
-                    (SpringApiPrincipal) authentication.getPrincipal();
-            return springCompatibleUser.getVipUser();
         }
     }
 

vip-api/src/main/java/fr/insalyon/creatis/vip/api/security/VipAuthenticationEntryPoint.java

@@ -72,7 +72,7 @@ public void commence(HttpServletRequest request, HttpServletResponse response, A
 
     @Override
     public void onAuthenticationFailure(HttpServletRequest request, HttpServletResponse response, AuthenticationException authException) throws IOException, ServletException {
-        // keycloak may already have set it up
+        // OIDC resource server handler may already have set this header
         if ( ! response.containsHeader("WWW-Authenticate")) {
             response.addHeader("WWW-Authenticate", "API-key");
         }

vip-api/src/main/java/fr/insalyon/creatis/vip/api/security/apikey/ApikeyAuthenticationFilter.java

@@ -33,8 +33,8 @@
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.springframework.security.authentication.AuthenticationManager;
 import org.springframework.security.authentication.AuthenticationServiceException;
+import org.springframework.security.authentication.AuthenticationProvider;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.AuthenticationException;
 import org.springframework.security.core.context.SecurityContextHolder;
@@ -59,21 +59,21 @@ public class ApikeyAuthenticationFilter extends OncePerRequestFilter {
 
     private final String apikeyHeader;
     private final AuthenticationEntryPoint authenticationEntryPoint;
-    private final AuthenticationManager authenticationManager;
+    private final AuthenticationProvider authenticationProvider;
 
     public ApikeyAuthenticationFilter(
             String apikeyHeader,
             AuthenticationEntryPoint authenticationEntryPoint,
-            AuthenticationManager authenticationManager) {
+            AuthenticationProvider authenticationProvider) {
         this.apikeyHeader = apikeyHeader;
         this.authenticationEntryPoint = authenticationEntryPoint;
-        this.authenticationManager = authenticationManager;
+        this.authenticationProvider = authenticationProvider;
     }
 
     @Override
     public void afterPropertiesSet() {
-        Assert.notNull(this.authenticationManager,
-                "An AuthenticationManager is required");
+        Assert.notNull(this.authenticationProvider,
+                "An AuthenticationProvider is required");
 
         Assert.notNull(this.authenticationEntryPoint,
                 "An AuthenticationEntryPoint is required");
@@ -96,8 +96,7 @@ protected void doFilterInternal(
             logger.debug("apikey header found.");
 
             ApikeyAuthenticationToken authRequest = new ApikeyAuthenticationToken(apikey);
-            Authentication authResult = this.authenticationManager
-                    .authenticate(authRequest);
+            Authentication authResult = this.authenticationProvider.authenticate(authRequest);
 
             logger.debug("Authentication success for : " + authResult);