From: Lukasz Jarocki Date: Fri, 10 Sep 2021 07:15:59 +0000 (+0200) Subject: SONAR-15349 Adding warning when missing SVN auth data X-Git-Tag: 9.1.0.47736~27 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=168e65d3e743a2bebb0f4ac496942e217501b1bb;p=sonarqube.git SONAR-15349 Adding warning when missing SVN auth data --- 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(); + } }