SONARIAC-1104 S5332 should raise if isHttpAllowed is set to true (#1512)

SonarSource · Sep 6, 2024 · 3b90b5c · 3b90b5c
1 parent 11c5d42
commit 3b90b5c
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 28 deletions.
diff --git a/iac-extensions/arm/src/main/java/org/sonar/iac/arm/checks/ClearTextProtocolsCheck.java b/iac-extensions/arm/src/main/java/org/sonar/iac/arm/checks/ClearTextProtocolsCheck.java
@@ -26,6 +26,7 @@
 
 import static org.sonar.iac.arm.checks.utils.CheckUtils.isEqual;
 import static org.sonar.iac.arm.checks.utils.CheckUtils.isFalse;
+import static org.sonar.iac.arm.checks.utils.CheckUtils.isTrue;
 
 @Rule(key = "S5332")
 public class ClearTextProtocolsCheck extends AbstractArmResourceCheck {
@@ -40,27 +41,27 @@ public class ClearTextProtocolsCheck extends AbstractArmResourceCheck {
 
   @Override
   protected void registerResourceConsumer() {
-    register("Microsoft.Web/sites", checkPropertyIsNotSetOrFalse("httpsOnly"));
+    register("Microsoft.Web/sites", ClearTextProtocolsCheck::checkHttpsOnly);
     register("Microsoft.Web/sites/config", checkPropertyHasValue("ftpsState", "AllAllowed"));
-    register("Microsoft.Storage/storageAccounts", ClearTextProtocolsCheck::checkHttpsTraffic);
+    register("Microsoft.Storage/storageAccounts", ClearTextProtocolsCheck::checkHttpsTrafficOnly);
     register("Microsoft.ApiManagement/service/apis", ClearTextProtocolsCheck::checkProtocols);
-    register("Microsoft.Cdn/profiles/endpoints", checkPropertyIsNotSetOrFalse("isHttpAllowed"));
+    register("Microsoft.Cdn/profiles/endpoints", ClearTextProtocolsCheck::checkHttpAllowed);
     register("Microsoft.Cache/redisEnterprise/databases", checkPropertyHasValue("clientProtocol", "Plaintext"));
     register(DATABASE_SERVER_TYPES, checkPropertyHasValue("sslEnforcement", "Disabled"));
   }
 
-  private static Consumer<ContextualResource> checkPropertyIsNotSetOrFalse(String propertyName) {
-    return resource -> resource.property(propertyName)
-      .reportIfAbsent(ISSUE_MESSAGE_ON_MISSING_PROPERTY)
-      .reportIf(isFalse(), GENERAL_ISSUE_MESSAGE);
-  }
-
   private static Consumer<ContextualResource> checkPropertyHasValue(String propertyName, String value) {
     return resource -> resource.property(propertyName)
       .reportIf(isEqual(value), GENERAL_ISSUE_MESSAGE);
   }
 
-  private static void checkHttpsTraffic(ContextualResource resource) {
+  private static void checkHttpsOnly(ContextualResource resource) {
+    resource.property("httpsOnly")
+      .reportIfAbsent(ISSUE_MESSAGE_ON_MISSING_PROPERTY)
+      .reportIf(isFalse(), GENERAL_ISSUE_MESSAGE);
+  }
+
+  private static void checkHttpsTrafficOnly(ContextualResource resource) {
     resource.property("supportsHttpsTrafficOnly")
       .reportIf(isFalse(), GENERAL_ISSUE_MESSAGE);
   }
@@ -69,4 +70,10 @@ private static void checkProtocols(ContextualResource resource) {
     resource.list("protocols")
       .reportItemIf(isEqual("http"), GENERAL_ISSUE_MESSAGE);
   }
+
+  private static void checkHttpAllowed(ContextualResource resource) {
+    resource.property("isHttpAllowed")
+      .reportIfAbsent(ISSUE_MESSAGE_ON_MISSING_PROPERTY)
+      .reportIf(isTrue(), GENERAL_ISSUE_MESSAGE);
+  }
 }
diff --git a/iac-extensions/arm/src/test/java/org/sonar/iac/arm/checks/ClearTextProtocolsCheckTest.java b/iac-extensions/arm/src/test/java/org/sonar/iac/arm/checks/ClearTextProtocolsCheckTest.java
@@ -22,7 +22,6 @@
 import java.util.stream.Stream;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.CsvSource;
 import org.junit.jupiter.params.provider.MethodSource;
 import org.sonar.iac.common.api.checks.IacCheck;
 
@@ -34,19 +33,27 @@ class ClearTextProtocolsCheckTest {
 
   IacCheck check = new ClearTextProtocolsCheck();
 
-  @ParameterizedTest
-  @CsvSource({"Microsoft.Web_sites.json,httpsOnly", "Microsoft.Cdn_profiles_endpoints.json,isHttpAllowed"})
-  void testClearTextProtocolWithHttpsFlagJson(String fileName, String propertyName) {
-    int endColumnForProperty = 17 + propertyName.length();
-    int endColumnForType = 11 + fileName.length();
-    ArmVerifier.verify("ClearTextProtocolsCheck/" + fileName, check,
-      issue(range(9, 8, 9, endColumnForProperty), "Make sure that using clear-text protocols is safe here."),
-      issue(range(14, 14, 14, endColumnForType), "Omitting \"" + propertyName + "\" allows the use of clear-text protocols. Make sure it is safe here."));
+  @Test
+  void testClearTextProtocolWithHttpsOnlyJson() {
+    ArmVerifier.verify("ClearTextProtocolsCheck/Microsoft.Web_sites.json", check,
+      issue(range(9, 8, 9, 26), "Make sure that using clear-text protocols is safe here."),
+      issue(range(14, 14, 14, 35), "Omitting \"httpsOnly\" allows the use of clear-text protocols. Make sure it is safe here."));
   }
 
   @Test
-  void testClearTextProtocolWithHttpsFlagBicep() {
+  void testClearTextProtocolWithHttpsOnlyBicep() {
     BicepVerifier.verify("ClearTextProtocolsCheck/Microsoft.Web_sites.bicep", check);
+  }
+
+  @Test
+  void testClearTextProtocolWithHttpAllowedJson() {
+    ArmVerifier.verify("ClearTextProtocolsCheck/Microsoft.Cdn_profiles_endpoints.json", check,
+      issue(range(9, 8, 9, 29), "Make sure that using clear-text protocols is safe here."),
+      issue(range(14, 14, 14, 48), "Omitting \"isHttpAllowed\" allows the use of clear-text protocols. Make sure it is safe here."));
+  }
+
+  @Test
+  void testClearTextProtocolWithHttpAllowedBicep() {
     BicepVerifier.verify("ClearTextProtocolsCheck/Microsoft.Cdn_profiles_endpoints.bicep", check);
   }
 

diff --git a/.../src/test/resources/checks/ClearTextProtocolsCheck/Microsoft.Cdn_profiles_endpoints.bicep b/.../src/test/resources/checks/ClearTextProtocolsCheck/Microsoft.Cdn_profiles_endpoints.bicep
@@ -1,8 +1,8 @@
-resource Raise_issue_as_httpsOnly_is_set_to_false 'Microsoft.Cdn/profiles/endpoints@2022-07-01' = {
-  name: 'Raise issue as httpsOnly is set to false'
+resource Raise_issue_as_isHttpAllowed_is_set_to_true 'Microsoft.Cdn/profiles/endpoints@2022-07-01' = {
+  name: 'Raise issue as isHttpAllowed is set to true'
   properties: {
-    isHttpAllowed: false // Noncompliant{{Make sure that using clear-text protocols is safe here.}}
-//  ^^^^^^^^^^^^^^^^^^^^
+    isHttpAllowed: true // Noncompliant{{Make sure that using clear-text protocols is safe here.}}
+//  ^^^^^^^^^^^^^^^^^^^
   }
 }
 
@@ -15,7 +15,7 @@ resource Raise_issue_as_isHttpAllowed_is_missing 'Microsoft.Cdn/profiles/endpoin
 resource Microsoft_Cdn_profiles_endpoints_Compliant_1 'Microsoft.Cdn/profiles/endpoints@2022-07-01' = {
   name: 'Compliant_1'
   properties: {
-    isHttpAllowed: true
+    isHttpAllowed: false
   }
 }
 

diff --git a/...m/src/test/resources/checks/ClearTextProtocolsCheck/Microsoft.Cdn_profiles_endpoints.json b/...m/src/test/resources/checks/ClearTextProtocolsCheck/Microsoft.Cdn_profiles_endpoints.json
@@ -2,11 +2,11 @@
   "contentVersion": "1.0.0.0",
   "resources": [
     {
-      "name": "Raise issue as httpsOnly is set to false",
+      "name": "Raise issue as isHttpAllowed is set to true",
       "type": "Microsoft.Cdn/profiles/endpoints",
       "apiVersion": "2022-07-01",
       "properties": {
-        "isHttpAllowed": false
+        "isHttpAllowed": true
       }
     },
     {
@@ -21,7 +21,7 @@
       "type": "Microsoft.Cdn/profiles/endpoints",
       "apiVersion": "2022-07-01",
       "properties": {
-        "isHttpAllowed": true
+        "isHttpAllowed": false
       }
     },
     {