SONAR-15349 Adding warning when missing SVN auth data

4 files changed, 131 insertions, 2 deletions

diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scm/svn/SvnBlameCommand.java b/sonar-scanner-engine/src/main/java/org/sonar/scm/svn/SvnBlameCommand.java
index c6e3fcbcd32..1a7a26c2d93 100644
--- a/sonar-scanner-engine/src/main/java/org/sonar/scm/svn/SvnBlameCommand.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/scm/svn/SvnBlameCommand.java
@@ -19,6 +19,7 @@
  */
 package org.sonar.scm.svn;
 
+import com.google.common.annotations.VisibleForTesting;
 import java.util.List;
 import org.sonar.api.batch.fs.FileSystem;
 import org.sonar.api.batch.fs.InputFile;
@@ -26,6 +27,7 @@ import org.sonar.api.batch.scm.BlameCommand;
 import org.sonar.api.batch.scm.BlameLine;
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
+import org.tmatesoft.svn.core.SVNAuthenticationException;
 import org.tmatesoft.svn.core.SVNErrorCode;
 import org.tmatesoft.svn.core.SVNException;
 import org.tmatesoft.svn.core.wc.SVNClientManager;
@@ -68,7 +70,8 @@ public class SvnBlameCommand extends BlameCommand {
     }
   }
 
-  private static void blame(SVNClientManager clientManager, InputFile inputFile, BlameOutput output) {
+  @VisibleForTesting
+  void blame(SVNClientManager clientManager, InputFile inputFile, BlameOutput output) {
     String filename = inputFile.relativePath();
 
     LOG.debug("Process file {}", filename);
@@ -81,6 +84,11 @@ public class SvnBlameCommand extends BlameCommand {
       SVNLogClient logClient = clientManager.getLogClient();
       logClient.setDiffOptions(new SVNDiffOptions(true, true, true));
       logClient.doAnnotate(inputFile.file(), SVNRevision.UNDEFINED, SVNRevision.create(1), SVNRevision.BASE, true, true, handler, null);
+    } catch (SVNAuthenticationException e) {
+      if(configuration.isEmpty()) {
+        LOG.warn("Authentication to SVN server is required but no authentication data was passed to the scanner");
+      }
+      throw new IllegalStateException("Authentication error when executing blame for file " + filename, e);
     } catch (SVNException e) {
       throw new IllegalStateException("Error when executing blame for file " + filename, e);
     }
diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scm/svn/SvnConfiguration.java b/sonar-scanner-engine/src/main/java/org/sonar/scm/svn/SvnConfiguration.java
index 9e0e49c63ba..c7efa4266d5 100644
--- a/sonar-scanner-engine/src/main/java/org/sonar/scm/svn/SvnConfiguration.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/scm/svn/SvnConfiguration.java
@@ -64,4 +64,8 @@ public class SvnConfiguration {
   public String passPhrase() {
     return config.get(PASSPHRASE_PROP_KEY).orElse(null);
   }
+
+  public boolean isEmpty() {
+    return username() == null && password() == null && privateKey() == null && passPhrase() == null;
+  }
 }
diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scm/svn/SvnBlameCommandTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scm/svn/SvnBlameCommandTest.java
index 9189319cda6..2e14e099827 100644
--- a/sonar-scanner-engine/src/test/java/org/sonar/scm/svn/SvnBlameCommandTest.java
+++ b/sonar-scanner-engine/src/test/java/org/sonar/scm/svn/SvnBlameCommandTest.java
@@ -43,18 +43,26 @@ import org.junit.runners.Parameterized;
 import org.junit.runners.Parameterized.Parameters;
 import org.mockito.ArgumentCaptor;
 import org.sonar.api.batch.fs.FileSystem;
+import org.sonar.api.batch.fs.InputFile;
 import org.sonar.api.batch.fs.internal.DefaultInputFile;
 import org.sonar.api.batch.fs.internal.TestInputFileBuilder;
 import org.sonar.api.batch.scm.BlameCommand.BlameInput;
 import org.sonar.api.batch.scm.BlameCommand.BlameOutput;
 import org.sonar.api.batch.scm.BlameLine;
+import org.sonar.api.utils.log.LogTester;
+import org.sonar.api.utils.log.LoggerLevel;
+import org.tmatesoft.svn.core.SVNAuthenticationException;
 import org.tmatesoft.svn.core.SVNDepth;
 import org.tmatesoft.svn.core.SVNURL;
 import org.tmatesoft.svn.core.auth.ISVNAuthenticationManager;
 import org.tmatesoft.svn.core.internal.wc2.compat.SvnCodec;
 import org.tmatesoft.svn.core.wc.ISVNOptions;
 import org.tmatesoft.svn.core.wc.SVNClientManager;
+import org.tmatesoft.svn.core.wc.SVNLogClient;
 import org.tmatesoft.svn.core.wc.SVNRevision;
+import org.tmatesoft.svn.core.wc.SVNStatus;
+import org.tmatesoft.svn.core.wc.SVNStatusClient;
+import org.tmatesoft.svn.core.wc.SVNStatusType;
 import org.tmatesoft.svn.core.wc.SVNUpdateClient;
 import org.tmatesoft.svn.core.wc.SVNWCUtil;
 import org.tmatesoft.svn.core.wc2.SvnCheckout;
@@ -62,7 +70,11 @@ import org.tmatesoft.svn.core.wc2.SvnTarget;
 
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Matchers.eq;
+import static org.junit.Assert.assertThrows;
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.anyBoolean;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoInteractions;
@@ -83,6 +95,9 @@ public class SvnBlameCommandTest {
   @Rule
   public ExpectedException thrown = ExpectedException.none();
 
+  @Rule
+  public LogTester logTester = new LogTester();
+
   private FileSystem fs;
   private BlameInput input;
   private String serverVersion;
@@ -287,6 +302,65 @@ public class SvnBlameCommandTest {
     verifyNoInteractions(blameResult);
   }
 
+  @Test
+  public void blame_givenNoCredentials_logWarning() throws Exception {
+    BlameOutput output = mock(BlameOutput.class);
+    InputFile inputFile = mock(InputFile.class);
+    SvnBlameCommand svnBlameCommand = newSvnBlameCommand();
+
+    SVNClientManager clientManager = mock(SVNClientManager.class);
+    SVNLogClient logClient = mock(SVNLogClient.class);
+    SVNStatusClient statusClient = mock(SVNStatusClient.class);
+    SVNStatus status = mock(SVNStatus.class);
+
+    when(clientManager.getLogClient()).thenReturn(logClient);
+    when(clientManager.getStatusClient()).thenReturn(statusClient);
+    when(status.getContentsStatus()).thenReturn(SVNStatusType.STATUS_NORMAL);
+    when(inputFile.file()).thenReturn(mock(File.class));
+    when(statusClient.doStatus(any(File.class), anyBoolean())).thenReturn(status);
+
+    doThrow(SVNAuthenticationException.class).when(logClient).doAnnotate(any(File.class), any(SVNRevision.class),
+      any(SVNRevision.class), any(SVNRevision.class), anyBoolean(), anyBoolean(), any(AnnotationHandler.class),
+      eq(null));
+
+    assertThrows(IllegalStateException.class, () -> {
+      svnBlameCommand.blame(clientManager, inputFile, output);
+      assertThat(logTester.logs(LoggerLevel.WARN)).contains("Authentication to SVN server is required but no " +
+        "authentication data was passed to the scanner");
+    });
+
+  }
+
+  @Test
+  public void blame_givenCredentialsSupplied_doNotlogWarning() throws Exception {
+    BlameOutput output = mock(BlameOutput.class);
+    InputFile inputFile = mock(InputFile.class);
+    SvnConfiguration properties = mock(SvnConfiguration.class);
+    SvnBlameCommand svnBlameCommand = new SvnBlameCommand(properties);
+
+    SVNClientManager clientManager = mock(SVNClientManager.class);
+    SVNLogClient logClient = mock(SVNLogClient.class);
+    SVNStatusClient statusClient = mock(SVNStatusClient.class);
+    SVNStatus status = mock(SVNStatus.class);
+
+    when(properties.isEmpty()).thenReturn(true);
+    when(clientManager.getLogClient()).thenReturn(logClient);
+    when(clientManager.getStatusClient()).thenReturn(statusClient);
+    when(status.getContentsStatus()).thenReturn(SVNStatusType.STATUS_NORMAL);
+    when(inputFile.file()).thenReturn(mock(File.class));
+    when(statusClient.doStatus(any(File.class), anyBoolean())).thenReturn(status);
+
+    doThrow(SVNAuthenticationException.class).when(logClient).doAnnotate(any(File.class), any(SVNRevision.class),
+      any(SVNRevision.class), any(SVNRevision.class), anyBoolean(), anyBoolean(), any(AnnotationHandler.class),
+      eq(null));
+
+    assertThrows(IllegalStateException.class, () -> {
+      svnBlameCommand.blame(clientManager, inputFile, output);
+      assertThat(logTester.logs(LoggerLevel.WARN)).isEmpty();
+    });
+
+  }
+
   private static void javaUnzip(File zip, File toDir) {
     try {
       try (ZipFile zipFile = new ZipFile(zip)) {
diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scm/svn/SvnConfigurationTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scm/svn/SvnConfigurationTest.java
index 3a9fbbe90fc..0d24ba57771 100644
--- a/sonar-scanner-engine/src/test/java/org/sonar/scm/svn/SvnConfigurationTest.java
+++ b/sonar-scanner-engine/src/test/java/org/sonar/scm/svn/SvnConfigurationTest.java
@@ -32,6 +32,7 @@ import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.fail;
 
 public class SvnConfigurationTest {
+
   @Rule
   public TemporaryFolder temp = new TemporaryFolder();
 
@@ -65,4 +66,46 @@ public class SvnConfigurationTest {
       assertThat(e).hasMessageContaining("Unable to read private key from ");
     }
   }
+
+  @Test
+  public void isEmpty_givenNullProperties_returnTrue() {
+    MapSettings settings = new MapSettings(new PropertyDefinitions(System2.INSTANCE));
+
+    SvnConfiguration svnConfiguration = new SvnConfiguration(settings.asConfig());
+
+    assertThat(svnConfiguration.isEmpty()).isTrue();
+  }
+
+  @Test
+  public void isEmpty_givenNotNullProperties_returnFalse() {
+    MapSettings settings = new MapSettings(new PropertyDefinitions(System2.INSTANCE));
+    settings.setProperty("sonar.svn.username", "bob");
+
+    SvnConfiguration svnConfiguration = new SvnConfiguration(settings.asConfig());
+
+    assertThat(svnConfiguration.isEmpty()).isFalse();
+  }
+
+  @Test
+  public void isEmpty_givenAllNotNullProperties_returnFalse() {
+    MapSettings settings = new MapSettings(new PropertyDefinitions(System2.INSTANCE));
+    settings.setProperty("sonar.svn.username", "bob");
+    settings.setProperty("sonar.svn.privateKeyPath", "bob");
+    settings.setProperty("sonar.svn.passphrase.secured", "bob");
+
+    SvnConfiguration svnConfiguration = new SvnConfiguration(settings.asConfig());
+
+    assertThat(svnConfiguration.isEmpty()).isFalse();
+  }
+
+  @Test
+  public void isEmpty_givenHalfNotNullProperties_returnFalse() {
+    MapSettings settings = new MapSettings(new PropertyDefinitions(System2.INSTANCE));
+    settings.setProperty("sonar.svn.password.secured", "bob");
+    settings.setProperty("sonar.svn.passphrase.secured", "bob");
+
+    SvnConfiguration svnConfiguration = new SvnConfiguration(settings.asConfig());
+
+    assertThat(svnConfiguration.isEmpty()).isFalse();
+  }
 }