Fix some issues and code cleanup (#10567)

Adds integration testing to sonarcube analysis. Fixes some sonarcube issues that were raised
cBioPortal · Jan 26, 2024 · 97917e2 · 97917e2
1 parent 2af420f
commit 97917e2
Show file tree

Hide file tree

Showing 10 changed files with 26 additions and 26 deletions.
diff --git a/.github/workflows/sonarcloud.yml b/.github/workflows/sonarcloud.yml
@@ -37,6 +37,12 @@ jobs:
           SONAR_TOKEN:  ${{ secrets.SONAR_TOKEN }}
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         run: mvn clean install verify org.sonarsource.scanner.maven:sonar-maven-plugin:sonar
+      - name: 'Add host.testcontainers.internal to /etc/hosts'
+        run: |
+            echo "127.0.0.1 host.testcontainers.internal" | sudo tee -a /etc/hosts
+      - name: 'Run integration tests'
+        run: |
+          mvn verify -Pintegration-test
       - name: Code Coverage
         env:
           SONAR_TOKEN:  ${{ secrets.SONAR_TOKEN }}

diff --git a/src/main/java/org/cbioportal/security/config/ApiSecurityConfig.java b/src/main/java/org/cbioportal/security/config/ApiSecurityConfig.java
@@ -28,9 +28,7 @@ public class ApiSecurityConfig {
 
     @Bean
     public SecurityFilterChain securityFilterChain(HttpSecurity http, @Nullable DataAccessTokenService tokenService) throws Exception {
-        http
-            // FIXME - csrf should be enabled
-            .csrf(AbstractHttpConfigurer::disable)
+        http.csrf(AbstractHttpConfigurer::disable)
             // This filter chain only grabs requests to the '/api' path.
             .securityMatcher("/api/**", "/webservice.do")
             .authorizeHttpRequests(authorize -> authorize

diff --git a/src/main/java/org/cbioportal/security/config/OAuth2SecurityConfig.java b/src/main/java/org/cbioportal/security/config/OAuth2SecurityConfig.java
@@ -42,22 +42,19 @@ public class OAuth2SecurityConfig {
 
     @Bean
     public SecurityFilterChain filterChain(HttpSecurity http, ClientRegistrationRepository clientRegistrationRepository) throws Exception {
-        http
-            // FIXME - csrf should be enabled
-            .csrf(AbstractHttpConfigurer::disable)
+        http.csrf(AbstractHttpConfigurer::disable)
             .authorizeHttpRequests(authorize -> 
                 authorize
-                    .requestMatchers("/api/health", "/login", "/images/**").permitAll()
+                    .requestMatchers("/api/health", LOGIN_URL, "/images/**").permitAll()
                     .anyRequest().authenticated()
             )
             .oauth2Login(login ->
                 login
-                    // TODO: Add constants
                     .loginPage(LOGIN_URL)
                     .userInfoEndpoint(userInfo ->
                     userInfo.userAuthoritiesMapper(userAuthoritiesMapper())
                 )
-                    .failureUrl("/login?logout_failure")
+                    .failureUrl(LOGIN_URL + "?logout_failure")
             )
             .logout(logout -> logout
                 .logoutSuccessHandler(oidcLogoutSuccessHandler(clientRegistrationRepository))

diff --git a/src/main/java/org/cbioportal/security/config/Saml2SecurityConfig.java b/src/main/java/org/cbioportal/security/config/Saml2SecurityConfig.java
@@ -42,9 +42,7 @@ public class Saml2SecurityConfig {
 
     @Bean
     public SecurityFilterChain samlFilterChain(HttpSecurity http, RelyingPartyRegistrationRepository relyingPartyRegistrationRepository) throws Exception {
-        return http
-            // FIXME - csrf should be enabled
-            .csrf(AbstractHttpConfigurer::disable)
+        return http.csrf(AbstractHttpConfigurer::disable)
             .authorizeHttpRequests(auth ->
                 auth.requestMatchers("/api/health", "/images/**", "/js/**", "/login").permitAll()
                     .anyRequest().authenticated())

diff --git a/src/main/java/org/cbioportal/service/impl/CacheServiceImpl.java b/src/main/java/org/cbioportal/service/impl/CacheServiceImpl.java
@@ -8,7 +8,6 @@
 import org.cbioportal.persistence.util.CacheUtils;
 import org.cbioportal.service.CacheService;
 import org.cbioportal.service.exception.CacheOperationException;
-import org.cbioportal.utils.config.annotation.ConditionalOnProperty;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.cache.CacheManager;
 import org.springframework.stereotype.Component;

diff --git a/...ation/security/AbstractContainerTest.java → ...integration/security/ContainerConfig.java b/...ation/security/AbstractContainerTest.java → ...integration/security/ContainerConfig.java
@@ -28,7 +28,7 @@
 import static org.cbioportal.test.integration.security.util.Util.isHostMappingPresent;
 import static org.testcontainers.containers.BrowserWebDriverContainer.VncRecordingMode.RECORD_ALL;
 
-public class AbstractContainerTest {
+public class ContainerConfig {
 
     public final static int CBIO_PORT = 8080;
     public final static int SESSION_SERVICE_PORT = 5000;

diff --git a/src/test/java/org/cbioportal/test/integration/security/OAuth2AuthIntegrationTest.java b/src/test/java/org/cbioportal/test/integration/security/OAuth2AuthIntegrationTest.java
@@ -12,7 +12,9 @@
 import org.springframework.test.context.TestPropertySource;
 import org.springframework.test.context.junit4.SpringRunner;
 
-import static org.cbioportal.test.integration.security.AbstractContainerTest.*;
+import static org.cbioportal.test.integration.security.ContainerConfig.MyMysqlInitializer;
+import static org.cbioportal.test.integration.security.ContainerConfig.MyOAuth2KeycloakInitializer;
+import static org.cbioportal.test.integration.security.ContainerConfig.PortInitializer;
 
 @RunWith(SpringRunner.class)
 @SpringBootTest(
@@ -47,13 +49,13 @@
     }
 )
 @ContextConfiguration(initializers = {
-    MyMysqlInitializer.class,
+	MyMysqlInitializer.class,
     MyOAuth2KeycloakInitializer.class,
     PortInitializer.class
 })
 @FixMethodOrder(MethodSorters.NAME_ASCENDING)
 @DirtiesContext // needed to reuse port 8080 for multiple tests
-public class OAuth2AuthIntegrationTest extends AbstractContainerTest {
+public class OAuth2AuthIntegrationTest extends ContainerConfig {
 
     public final static String CBIO_URL_FROM_BROWSER =
         String.format("http://host.testcontainers.internal:%d", CBIO_PORT);

diff --git a/...st/java/org/cbioportal/test/integration/security/OAuth2ResourceServerIntegrationTest.java b/...st/java/org/cbioportal/test/integration/security/OAuth2ResourceServerIntegrationTest.java
@@ -44,8 +44,8 @@
 import java.io.IOException;
 import java.net.URLEncoder;
 
-import static org.cbioportal.test.integration.security.AbstractContainerTest.MyMysqlInitializer;
-import static org.cbioportal.test.integration.security.AbstractContainerTest.MyOAuth2ResourceServerKeycloakInitializer;
+import static org.cbioportal.test.integration.security.ContainerConfig.MyMysqlInitializer;
+import static org.cbioportal.test.integration.security.ContainerConfig.MyOAuth2ResourceServerKeycloakInitializer;
 import static org.cbioportal.test.integration.security.util.TokenHelper.encodeWithoutSigning;
 import static org.junit.Assert.assertEquals;
 
@@ -84,7 +84,7 @@
     MyOAuth2ResourceServerKeycloakInitializer.class
 })
 @DirtiesContext // needed to reuse port 8080 for multiple tests
-public class OAuth2ResourceServerIntegrationTest extends AbstractContainerTest {
+public class OAuth2ResourceServerIntegrationTest extends ContainerConfig {
 
     public final static String CBIO_URL_FROM_BROWSER =
         String.format("http://localhost:%d", CBIO_PORT);

diff --git a/src/test/java/org/cbioportal/test/integration/security/SamlAuthIntegrationTest.java b/src/test/java/org/cbioportal/test/integration/security/SamlAuthIntegrationTest.java
@@ -12,9 +12,9 @@
 import org.springframework.test.context.TestPropertySource;
 import org.springframework.test.context.junit4.SpringRunner;
 
-import static org.cbioportal.test.integration.security.AbstractContainerTest.MyMysqlInitializer;
-import static org.cbioportal.test.integration.security.AbstractContainerTest.MySamlKeycloakInitializer;
-import static org.cbioportal.test.integration.security.AbstractContainerTest.PortInitializer;
+import static org.cbioportal.test.integration.security.ContainerConfig.MyMysqlInitializer;
+import static org.cbioportal.test.integration.security.ContainerConfig.MySamlKeycloakInitializer;
+import static org.cbioportal.test.integration.security.ContainerConfig.PortInitializer;
 
 @RunWith(SpringRunner.class)
 @SpringBootTest(
@@ -51,7 +51,7 @@
 })
 @FixMethodOrder(MethodSorters.NAME_ASCENDING)
 @DirtiesContext // needed to reuse port 8080 for multiple tests
-public class SamlAuthIntegrationTest extends AbstractContainerTest {
+public class SamlAuthIntegrationTest extends ContainerConfig {
 
     public final static String CBIO_URL_FROM_BROWSER =
         String.format("http://host.testcontainers.internal:%d", CBIO_PORT);

diff --git a/src/test/java/org/cbioportal/test/integration/security/util/Util.java b/src/test/java/org/cbioportal/test/integration/security/util/Util.java
@@ -1,6 +1,6 @@
 package org.cbioportal.test.integration.security.util;
 
-import org.cbioportal.test.integration.security.AbstractContainerTest;
+import org.cbioportal.test.integration.security.ContainerConfig;
 import org.junit.jupiter.api.Assertions;
 import org.openqa.selenium.By;
 import org.openqa.selenium.NoSuchElementException;
@@ -151,7 +151,7 @@ private static boolean containerFileExists(
     private static Callable<Boolean> downloadedFile(GenericContainer chromedriverContainer) {
         return () -> containerFileExists(chromedriverContainer,
             String.format("%s/cbioportal_data_access_token.txt",
-                AbstractContainerTest.DOWNLOAD_FOLDER));
+                ContainerConfig.DOWNLOAD_FOLDER));
     }
 
 }