Merge pull request #36 from gtudan/master

fix sonar issues and improved test coverage

.gitignore

@@ -1,3 +1,4 @@
 target/
 *.iml
-.idea
+.idea
+.DS_Store

sonar-dependency-check-plugin/src/main/java/org/sonar/dependencycheck/DependencyCheckSensor.java

@@ -43,7 +43,10 @@
 import org.xml.sax.SAXException;
 
 import javax.xml.parsers.ParserConfigurationException;
-import java.io.*;
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.Serializable;
 
 public class DependencyCheckSensor implements Sensor {
 
@@ -106,6 +109,8 @@ private void incrementCount(Severity severity) {
             case MINOR:
                 this.minorIssuesCount++;
                 break;
+            default:
+                LOGGER.debug("Unknown severity {}", severity);
         }
     }
 
@@ -114,7 +119,7 @@ private void addIssues(SensorContext context, Analysis analysis) {
             return;
         }
         for (Dependency dependency : analysis.getDependencies()) {
-            InputFile testFile = fileSystem.inputFile(fileSystem.predicates().is(new File(dependency.getFilePath())));
+            InputFile testFile = fileSystem.inputFile(fileSystem.predicates().hasPath(dependency.getFilePath()));
 
             int depVulnCount = dependency.getVulnerabilities().size();
 
@@ -174,7 +179,7 @@ public void execute(SensorContext sensorContext) {
         profiler.startInfo("Process Dependency-Check report");
         try {
             Analysis analysis = parseAnalysis(sensorContext);
-            totalDependencies = analysis.getDependencies().size();
+            this.totalDependencies = analysis.getDependencies().size();
             addIssues(sensorContext, analysis);
         } catch (FileNotFoundException e) {
             LOGGER.debug("Analysis aborted due to missing report file", e);

sonar-dependency-check-plugin/src/main/java/org/sonar/dependencycheck/base/DependencyCheckMetrics.java

@@ -28,7 +28,7 @@
 
 public final class DependencyCheckMetrics implements Metrics {
 
-    public static final String DOMAIN = "OWASP-Dependency-Check";
+    private static final String DOMAIN = "OWASP-Dependency-Check";
 
     private static final String INHERITED_RISK_SCORE_KEY = "inherited_risk_score";
     private static final String VULNERABLE_COMPONENT_RATIO_KEY = "vulnerable_component_ratio";
@@ -40,19 +40,6 @@ public final class DependencyCheckMetrics implements Metrics {
     private static final String MEDIUM_SEVERITY_VULNS_KEY = "medium_severity_vulns";
     private static final String LOW_SEVERITY_VULNS_KEY = "low_severity_vulns";
 
-
-    public static int inheritedRiskScore(int high, int medium, int low) {
-        return (high * 5) + (medium * 3) + (low);
-    }
-
-    public static double vulnerableComponentRatio(int vulnerabilities, int vulnerableComponents) {
-        double ratio = 0.0;
-        if(vulnerableComponents > 0) {
-            ratio = (double) vulnerabilities / vulnerableComponents;
-        }
-        return ratio;
-    }
-
     public static final Metric<Serializable> INHERITED_RISK_SCORE = new Metric.Builder(DependencyCheckMetrics.INHERITED_RISK_SCORE_KEY, "Inherited Risk Score", Metric.ValueType.INT)
             .setDescription("Inherited Risk Score")
             .setDirection(Metric.DIRECTION_BETTER)
@@ -122,6 +109,19 @@ public static double vulnerableComponentRatio(int vulnerabilities, int vulnerabl
             .setHidden(false)
             .create();
 
+    public static double vulnerableComponentRatio(int vulnerabilities, int vulnerableComponents) {
+        double ratio = 0.0;
+        if(vulnerableComponents > 0) {
+            ratio = (double) vulnerabilities / vulnerableComponents;
+        }
+        return ratio;
+    }
+
+    public static int inheritedRiskScore(int high, int medium, int low) {
+        return (high * 5) + (medium * 3) + (low);
+    }
+
+
     @Override
     public List<Metric> getMetrics() {
         return Arrays.asList(

sonar-dependency-check-plugin/src/main/java/org/sonar/dependencycheck/rule/KnownCveRuleDefinition.java

@@ -29,6 +29,7 @@
 public class KnownCveRuleDefinition implements RulesDefinition {
 
 
+    @Override
     @ParametersAreNonnullByDefault
     public void define(Context context) {
         NewRepository repo = context.createRepository(DependencyCheckPlugin.REPOSITORY_KEY, DependencyCheckPlugin.LANGUAGE_KEY);
@@ -52,8 +53,6 @@ public void define(Context context) {
         rule.setHtmlDescription(description);
 
         // There's simply no way to know how much effort will be involved in updating/replacing a vulnerable component
-        //rule.setDebtSubCharacteristic(RulesDefinition.SubCharacteristics.SECURITY_COMPLIANCE);
-        //rule.setDebtRemediationFunction(rule.debtRemediationFunctions().linearWithOffset("1h", "30min"));
 
         repo.done();
     }

sonar-dependency-check-plugin/src/main/java/org/sonar/dependencycheck/rule/NeutralLanguage.java

@@ -36,6 +36,7 @@ public NeutralLanguage() {
         super(DependencyCheckPlugin.LANGUAGE_KEY, "Neutral");
     }
 
+    @Override
     public String[] getFileSuffixes() {
         return new String[0];
     }

sonar-dependency-check-plugin/src/test/java/org/sonar/dependencycheck/parser/DependencyCheckSensorTest.java

@@ -22,33 +22,102 @@
 import org.junit.Before;
 import org.junit.Test;
 import org.sonar.api.batch.fs.FileSystem;
+import org.sonar.api.batch.fs.InputComponent;
+import org.sonar.api.batch.measure.Metric;
+import org.sonar.api.batch.sensor.SensorContext;
+import org.sonar.api.batch.sensor.SensorDescriptor;
 import org.sonar.api.scan.filesystem.PathResolver;
 import org.sonar.dependencycheck.DependencyCheckSensor;
+import org.sonar.dependencycheck.base.DependencyCheckConstants;
 
+import java.io.File;
 import java.net.URISyntaxException;
+import java.net.URL;
+import java.nio.file.Paths;
 
 import static org.fest.assertions.Assertions.assertThat;
-import static org.mockito.Mockito.mock;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Mockito.*;
 
 public class DependencyCheckSensorTest {
     private FileSystem fileSystem;
     private PathResolver pathResolver;
     private DependencyCheckSensor sensor;
 
+    private File sampleReport;
+
     @Before
-    public void init() {
-        this.fileSystem = mock(FileSystem.class);
+    public void init() throws URISyntaxException {
+        this.fileSystem = mock(FileSystem.class, RETURNS_DEEP_STUBS);
         this.pathResolver = mock(PathResolver.class);
         this.sensor = new DependencyCheckSensor(this.fileSystem, this.pathResolver);
+
+        // mock a sample report
+        final URL sampleResourceURI = getClass().getClassLoader().getResource("report/dependency-check-report.xml");
+        assert sampleResourceURI != null;
+        this.sampleReport = Paths.get(sampleResourceURI.toURI()).toFile();
     }
 
     @Test
     public void toStringTest() {
         assertThat(this.sensor.toString()).isEqualTo("OWASP Dependency-Check");
     }
 
+    @Test
+    public void testDescribe() {
+        final SensorDescriptor descriptor = mock(SensorDescriptor.class);
+        sensor.describe(descriptor);
+        verify(descriptor).name("OWASP Dependency-Check");
+    }
     @Test
     public void shouldAnalyse() throws URISyntaxException {
-        //todo: Once the Sensor is capable of working properly, populate this unit test.
+        final SensorContext context = mock(SensorContext.class, RETURNS_DEEP_STUBS);
+
+        when(context.settings().getString(DependencyCheckConstants.REPORT_PATH_PROPERTY)).thenReturn("dependency-check-report.xml");
+        when(pathResolver.relativeFile(any(File.class), anyString())).thenReturn(sampleReport);
+        sensor.execute(context);
+    }
+
+    @Test
+    public void shouldSkipIfReportWasNotFound() throws URISyntaxException {
+        final SensorContext context = mock(SensorContext.class, RETURNS_DEEP_STUBS);
+
+        when(pathResolver.relativeFile(any(File.class), anyString())).thenReturn(null);
+        sensor.execute(context);
+        verify(context, never()).newIssue();
+    }
+
+    @Test
+    public void shouldAddAnIssueForAVulnerability() throws URISyntaxException {
+        final SensorContext context = mock(SensorContext.class, RETURNS_DEEP_STUBS);
+
+        when(context.settings().getString(DependencyCheckConstants.REPORT_PATH_PROPERTY)).thenReturn("dependency-check-report.xml");
+        when(pathResolver.relativeFile(any(File.class), anyString())).thenReturn(sampleReport);
+        sensor.execute(context);
+
+        verify(context, times(3)).newIssue();
+    }
+
+    @Test
+    public void shouldPersistTotalMetrics() throws URISyntaxException {
+        final SensorContext context = mock(SensorContext.class, RETURNS_DEEP_STUBS);
+
+        when(context.settings().getString(DependencyCheckConstants.REPORT_PATH_PROPERTY)).thenReturn("dependency-check-report.xml");
+        when(pathResolver.relativeFile(any(File.class), anyString())).thenReturn(sampleReport);
+        sensor.execute(context);
+
+        verify(context.newMeasure(), times(8)).forMetric(any(Metric.class));
+    }
+
+    @Test
+    public void shouldPersistMetricsOnReport() throws URISyntaxException {
+        final SensorContext context = mock(SensorContext.class, RETURNS_DEEP_STUBS);
+
+        when(context.settings().getString(DependencyCheckConstants.REPORT_PATH_PROPERTY)).thenReturn("dependency-check-report.xml");
+        when(pathResolver.relativeFile(any(File.class), anyString())).thenReturn(sampleReport);
+        sensor.execute(context);
+
+        verify(context.newMeasure(), atLeastOnce()).on(any(InputComponent.class));
     }
 }

sonar-dependency-check-plugin/src/test/java/org/sonar/dependencycheck/parser/ReportParserTest.java

@@ -44,7 +44,7 @@ public void parseReport() throws Exception {
         assertThat(analysis.getProjectInfo().getCredits()).isEqualTo("This report contains data retrieved from the National Vulnerability Database: http://nvd.nist.gov");
 
         Collection<Dependency> dependencies = analysis.getDependencies();
-        assertThat(dependencies.size()).isEqualTo(5);
+        assertThat(dependencies).hasSize(5);
         Iterator iterator = dependencies.iterator();
         Dependency dependency = (Dependency) iterator.next();
 
@@ -68,31 +68,31 @@ public void parseReport() throws Exception {
         assertThat(vulnerability.getName()).isEqualTo("CVE-2014-3596");
         assertThat(vulnerability.getCvssScore()).isEqualTo("5.8");
         assertThat(vulnerability.getSeverity()).isEqualTo("Medium");
-        assertThat(vulnerability.getCwe() == null);
+        assertThat(vulnerability.getCwe()).isNull();
         assertThat(vulnerability.getDescription()).isEqualTo("The getCN function in Apache Axis 1.4 and earlier does not properly verify that the server hostname matches a domain name in the subject's Common Name (CN) or subjectAltName field of the X.509 certificate, which allows man-in-the-middle attackers to spoof SSL servers via a certificate with a subject that specifies a common name in a field that is not the CN field.  NOTE: this issue exists because of an incomplete fix for CVE-2012-5784.");
 
         vulnerability = (Vulnerability) vulnIterator.next();
         assertThat(vulnerability.getName()).isEqualTo("CVE-2012-5784");
         assertThat(vulnerability.getCvssScore()).isEqualTo("5.8");
         assertThat(vulnerability.getSeverity()).isEqualTo("Medium");
-        assertThat(vulnerability.getCwe().equals("CWE-20 Improper Input Validation"));
+        assertThat(vulnerability.getCwe()).isEqualTo("CWE-20 Improper Input Validation");
         assertThat(vulnerability.getDescription()).isEqualTo("Apache Axis 1.4 and earlier, as used in PayPal Payments Pro, PayPal Mass Pay, PayPal Transactional Information SOAP, the Java Message Service implementation in Apache ActiveMQ, and other products, does not verify that the server hostname matches a domain name in the subject's Common Name (CN) or subjectAltName field of the X.509 certificate, which allows man-in-the-middle attackers to spoof SSL servers via an arbitrary valid certificate.");
 
         dependency = (Dependency) iterator.next();
         assertThat(dependency.getEvidenceCollected().size()).isEqualTo(13);
         assertThat(dependency.getVulnerabilities().size()).isEqualTo(0);
 
         dependency = (Dependency) iterator.next();
-        assertThat(dependency.getEvidenceCollected().size()).isEqualTo(12);
-        assertThat(dependency.getVulnerabilities().size()).isEqualTo(0);
+        assertThat(dependency.getEvidenceCollected()).hasSize(12);
+        assertThat(dependency.getVulnerabilities()).isEmpty();
 
         dependency = (Dependency) iterator.next();
-        assertThat(dependency.getEvidenceCollected().size()).isEqualTo(32);
-        assertThat(dependency.getVulnerabilities().size()).isEqualTo(1);
+        assertThat(dependency.getEvidenceCollected()).hasSize(32);
+        assertThat(dependency.getVulnerabilities()).hasSize(1);
 
         dependency = (Dependency) iterator.next();
-        assertThat(dependency.getEvidenceCollected().size()).isEqualTo(9);
-        assertThat(dependency.getVulnerabilities().size()).isEqualTo(0);
+        assertThat(dependency.getEvidenceCollected()).hasSize(9);
+        assertThat(dependency.getVulnerabilities()).isEmpty();
     }
 
 }

sonar-dependency-check-plugin/src/test/java/org/sonar/dependencycheck/rule/KnownCveRuleDefinitionTest.java

@@ -0,0 +1,57 @@
+/*
+ * Dependency-Check Plugin for SonarQube
+ * Copyright (C) 2015-2017 Steve Springett
+ * steve.springett@owasp.org
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.dependencycheck.rule;
+
+import org.junit.Test;
+import org.mockito.InOrder;
+import org.sonar.api.server.rule.RulesDefinition;
+import org.sonar.dependencycheck.DependencyCheckPlugin;
+
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Mockito.*;
+
+/**
+ * @author Gregor Tudan, Cofinpro AG
+ */
+public class KnownCveRuleDefinitionTest {
+
+    private KnownCveRuleDefinition rule = new KnownCveRuleDefinition();
+
+    @Test
+    public void define() throws Exception {
+        final RulesDefinition.Context context = mock(RulesDefinition.Context.class);
+        final RulesDefinition.NewRepository repo = mock(RulesDefinition.NewRepository.class);
+        final RulesDefinition.NewRule rule = mock(RulesDefinition.NewRule.class, RETURNS_SMART_NULLS);
+
+        when(repo.createRule(DependencyCheckPlugin.RULE_KEY)).thenReturn(rule);
+        when(context.createRepository(anyString(), anyString())).thenReturn(repo);
+
+        this.rule.define(context);
+
+        InOrder inOrder = inOrder(context, repo);
+
+        inOrder.verify(context).createRepository("OWASP","neutral");
+        inOrder.verify(repo).createRule(DependencyCheckPlugin.RULE_KEY);
+
+        inOrder.verify(repo).done();
+        inOrder.verifyNoMoreInteractions();
+    }
+
+}

sonar-dependency-check-plugin/src/test/java/org/sonar/dependencycheck/rule/NeutralLanguageTest.java

@@ -0,0 +1,36 @@
+/*
+ * Dependency-Check Plugin for SonarQube
+ * Copyright (C) 2015-2017 Steve Springett
+ * steve.springett@owasp.org
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.dependencycheck.rule;
+
+import org.junit.Test;
+
+import static org.fest.assertions.Assertions.assertThat;
+
+
+/**
+ * @author Gregor Tudan, Cofinpro AG
+ */
+public class NeutralLanguageTest {
+    @Test
+    public void getFileSuffixes() throws Exception {
+        assertThat(new NeutralLanguage().getFileSuffixes()).isEmpty();
+    }
+
+}