Fix sonar issues

italiangrid · Sep 18, 2024 · 7c1fe2e · 7c1fe2e
1 parent 7611610
commit 7c1fe2e
Show file tree

Hide file tree

Showing 9 changed files with 115 additions and 89 deletions.
diff --git a/src/main/java/org/italiangrid/storm/webdav/oauth/utils/DefaultOidcConfigurationFetcher.java b/src/main/java/org/italiangrid/storm/webdav/oauth/utils/DefaultOidcConfigurationFetcher.java
@@ -35,6 +35,7 @@
 import org.springframework.web.client.RestTemplate;
 import org.springframework.web.util.UriComponentsBuilder;
 
+import com.nimbusds.jose.KeySourceException;
 import com.nimbusds.jose.RemoteKeySourceException;
 
 @Service
@@ -55,8 +56,8 @@ public class DefaultOidcConfigurationFetcher implements OidcConfigurationFetcher
 
   public DefaultOidcConfigurationFetcher(RestTemplateBuilder restBuilder,
       OAuthProperties oAuthProperties) {
-    final Duration TIMEOUT = Duration.ofSeconds(oAuthProperties.getRefreshTimeoutSeconds());
-    this.restTemplate = restBuilder.setConnectTimeout(TIMEOUT).setReadTimeout(TIMEOUT).build();
+    final Duration timeout = Duration.ofSeconds(oAuthProperties.getRefreshTimeoutSeconds());
+    this.restTemplate = restBuilder.setConnectTimeout(timeout).setReadTimeout(timeout).build();
   }
 
   private void metadataChecks(String issuer, Map<String, Object> oidcConfiguration) {
@@ -86,26 +87,24 @@ public Map<String, Object> loadConfigurationForIssuer(String issuer) {
     URI uri = UriComponentsBuilder.fromUriString(issuer + WELL_KNOWN_FRAGMENT).build().toUri();
     ResponseEntity<Map<String, Object>> response = null;
     try {
-
-      RequestEntity<Void> request = RequestEntity.get(uri).build();
-      response = restTemplate.exchange(request, typeReference);
-      if (response.getStatusCodeValue() != 200) {
-        throw new RuntimeException(
-            format("Received status code: %s", response.getStatusCodeValue()));
-      }
-      metadataChecks(issuer, response.getBody());
-      return response.getBody();
+      response = restTemplate.exchange(RequestEntity.get(uri).build(), typeReference);
     } catch (RuntimeException e) {
       final String errorMsg = format("Unable to resolve OpenID configuration from '%s'", uri);
       if (LOG.isDebugEnabled()) {
         LOG.error("{}: {}", errorMsg, e.getMessage());
       }
       throw new OidcConfigurationResolutionError(errorMsg, e);
     }
+    if (response.getStatusCodeValue() != 200) {
+      throw new OidcConfigurationResolutionError(
+          format("Received status code: %s", response.getStatusCodeValue()));
+    }
+    metadataChecks(issuer, response.getBody());
+    return response.getBody();
   }
 
   @Override
-  public String loadJWKSourceForURL(URI uri) throws RemoteKeySourceException {
+  public String loadJWKSourceForURL(URI uri) throws KeySourceException {
 
     LOG.debug("Fetching JWK from {}", uri);
 
@@ -115,18 +114,17 @@ public String loadJWKSourceForURL(URI uri) throws RemoteKeySourceException {
     try {
       RequestEntity<Void> request = RequestEntity.get(uri).headers(headers).build();
       response = restTemplate.exchange(request, String.class);
-      if (response.getStatusCodeValue() != 200) {
-        throw new RuntimeException(
-            format("Received status code: %s", response.getStatusCodeValue()));
-      }
     } catch (RuntimeException e) {
       final String errorMsg = format("Unable to get JWK from '%s'", uri);
       if (LOG.isDebugEnabled()) {
         LOG.error("{}: {}", errorMsg, e.getMessage());
       }
       throw new RemoteKeySourceException(errorMsg, e);
     }
-
+    if (response.getStatusCodeValue() != 200) {
+      throw new KeySourceException(
+          format("Received status code: %s", response.getStatusCodeValue()));
+    }
     return response.getBody();
   }
 }
diff --git a/src/main/java/org/italiangrid/storm/webdav/oauth/utils/NoExpirationStringCache.java b/src/main/java/org/italiangrid/storm/webdav/oauth/utils/NoExpirationStringCache.java
@@ -48,17 +48,14 @@ protected Object lookup(Object key) {
 
   @Override
   public void put(Object key, Object value) {
-    return;
   }
 
   @Override
   public void evict(Object key) {
-    return;
   }
 
   @Override
   public void clear() {
-    return;
   }
 
   @SuppressWarnings("unchecked")

diff --git a/src/main/java/org/italiangrid/storm/webdav/oauth/utils/OidcConfigurationFetcher.java b/src/main/java/org/italiangrid/storm/webdav/oauth/utils/OidcConfigurationFetcher.java
@@ -18,12 +18,12 @@
 import java.net.URI;
 import java.util.Map;
 
-import com.nimbusds.jose.RemoteKeySourceException;
+import com.nimbusds.jose.KeySourceException;
 
 public interface OidcConfigurationFetcher {
 
   Map<String, Object> loadConfigurationForIssuer(String issuer);
 
-  String loadJWKSourceForURL(URI uri) throws RemoteKeySourceException;
+  String loadJWKSourceForURL(URI uri) throws KeySourceException;
 
 }
diff --git a/src/main/java/org/italiangrid/storm/webdav/oauth/utils/TrustedJwtDecoderCacheLoader.java b/src/main/java/org/italiangrid/storm/webdav/oauth/utils/TrustedJwtDecoderCacheLoader.java
@@ -78,7 +78,7 @@ public JwtDecoder load(String issuer) throws Exception {
     Map<String, Object> oidcConfiguration = fetcher.loadConfigurationForIssuer(issuer);
     URI jwksUri = URI.create(oidcConfiguration.get("jwks_uri").toString());
     Cache noExpirationCache =
-        new NoExpirationStringCache(fetcher.loadJWKSourceForURL(jwksUri).toString());
+        new NoExpirationStringCache(fetcher.loadJWKSourceForURL(jwksUri));
 
     NimbusJwtDecoder decoder =
         NimbusJwtDecoder.withJwkSetUri((oidcConfiguration.get("jwks_uri").toString()))

diff --git a/src/main/java/org/italiangrid/storm/webdav/server/DefaultWebServerFactory.java b/src/main/java/org/italiangrid/storm/webdav/server/DefaultWebServerFactory.java
@@ -45,9 +45,10 @@ public DefaultWebServerFactory(ServiceConfiguration configuration,
   }
 
   private InstrumentedQueuedThreadPool getInstrumentedThreadPool() {
-    InstrumentedQueuedThreadPool tPool = new InstrumentedQueuedThreadPool(metricRegistry, configuration.getMaxConnections(),
-        configuration.getMinConnections(), configuration.getThreadPoolMaxIdleTimeInMsec(),
-        new ArrayBlockingQueue<Runnable>(configuration.getMaxQueueSize()), "storm.http");
+    InstrumentedQueuedThreadPool tPool =
+        new InstrumentedQueuedThreadPool(metricRegistry, configuration.getMaxConnections(),
+            configuration.getMinConnections(), configuration.getThreadPoolMaxIdleTimeInMsec(),
+            new ArrayBlockingQueue<>(configuration.getMaxQueueSize()), "storm.http");
     tPool.setName("thread-pool");
     return tPool;
   }

diff --git a/src/test/java/org/italiangrid/storm/webdav/server/TLSConnectorBuilderTest.java b/src/test/java/org/italiangrid/storm/webdav/server/TLSConnectorBuilderTest.java
@@ -15,6 +15,9 @@
  */
 package org.italiangrid.storm.webdav.server;
 
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.assertThrows;
 
 import java.util.concurrent.TimeUnit;
@@ -24,6 +27,7 @@
 import org.apache.http.conn.ssl.NoopHostnameVerifier;
 import org.eclipse.jetty.server.HttpConfiguration;
 import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.server.ServerConnector;
 import org.italiangrid.storm.webdav.server.util.CANLListener;
 import org.italiangrid.voms.util.CertificateValidatorBuilder;
 import org.junit.jupiter.api.Test;
@@ -40,15 +44,19 @@
 public class TLSConnectorBuilderTest {
 
   @Test
-  public void tlsConnectorBuilderErrorTests() {
-
-    new TLSConnectorBuilderError("This is an error!");
-    new TLSConnectorBuilderError("This is an error!", new RuntimeException());
-    new TLSConnectorBuilderError(new RuntimeException());
+  void tlsConnectorBuilderErrorTests() {
+
+    TLSConnectorBuilderError e = new TLSConnectorBuilderError("This is an error!");
+    assertThat(e.getMessage(), is("This is an error!"));
+    e = new TLSConnectorBuilderError("This is an error!", new RuntimeException());
+    assertThat(e.getMessage(), is("This is an error!"));
+    e = new TLSConnectorBuilderError(new RuntimeException("This is an error!"));
+    assertThat(e.getCause() instanceof RuntimeException, is(true));
+    assertThat(e.getMessage(), containsString("This is an error!"));
   }
 
   @Test
-  public void illegalArgumentExceptionThrown() {
+  void illegalArgumentExceptionThrown() {
 
     Server server = Mockito.mock(Server.class);
     X509CertChainValidatorExt validator = Mockito.mock(X509CertChainValidatorExt.class);
@@ -83,7 +91,7 @@ private X509CertChainValidatorExt getValidator() {
   }
 
   @Test
-  public void tlsConnectorBuilderTests() {
+  void tlsConnectorBuilderTests() {
 
     Server server = new Server();
     X509CertChainValidatorExt validator = getValidator();
@@ -103,6 +111,7 @@ public void tlsConnectorBuilderTests() {
       .withHostnameVerifier(new NoopHostnameVerifier())
       .withConscrypt(false);
 
-    builder.build();
+    ServerConnector c = builder.build();
+    assertThat(c.getPort(), is(1234));
   }
 }