Merge pull request #1527 from steve-community/1526-validate-chargebox…

…id-for-ws-connections

validate chargeBoxId for WS connections (#1526)

pom.xml

@@ -815,8 +815,8 @@
 
         <dependency>
             <groupId>org.owasp.encoder</groupId>
-            <artifactId>encoder</artifactId>
-            <version>1.2.3</version>
+            <artifactId>encoder-jakarta-jsp</artifactId>
+            <version>1.3.0</version>
         </dependency>
     </dependencies>
 </project>

src/main/java/de/rwth/idsg/steve/SteveConfiguration.java

@@ -104,6 +104,7 @@ public enum SteveConfiguration {
 
         ocpp = Ocpp.builder()
                    .autoRegisterUnknownStations(p.getOptionalBoolean("auto.register.unknown.stations"))
+                   .chargeBoxIdValidationRegex(p.getOptionalString("charge-box-id.validation.regex"))
                    .wsSessionSelectStrategy(
                            WsSessionSelectStrategyEnum.fromName(p.getString("ws.session.select.strategy")))
                    .build();
@@ -200,6 +201,7 @@ public static class WebApi {
     @Builder @Getter
     public static class Ocpp {
         private final boolean autoRegisterUnknownStations;
+        private final String chargeBoxIdValidationRegex;
         private final WsSessionSelectStrategy wsSessionSelectStrategy;
     }
 

src/main/java/de/rwth/idsg/steve/ocpp/ws/OcppWebSocketHandshakeHandler.java

@@ -20,6 +20,7 @@
 
 import de.rwth.idsg.steve.config.WebSocketConfiguration;
 import de.rwth.idsg.steve.service.ChargePointHelperService;
+import de.rwth.idsg.steve.web.validation.ChargeBoxIdValidator;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import ocpp.cs._2015._10.RegistrationStatus;
@@ -48,6 +49,8 @@
 @RequiredArgsConstructor
 public class OcppWebSocketHandshakeHandler implements HandshakeHandler {
 
+    private static final ChargeBoxIdValidator CHARGE_BOX_ID_VALIDATOR = new ChargeBoxIdValidator();
+
     private final DefaultHandshakeHandler delegate;
     private final List<AbstractWebSocketEndpoint> endpoints;
     private final ChargePointHelperService chargePointHelperService;
@@ -70,6 +73,13 @@ public boolean doHandshake(ServerHttpRequest request, ServerHttpResponse respons
         // -------------------------------------------------------------------------
 
         String chargeBoxId = getLastBitFromUrl(request.getURI().getPath());
+        boolean isValid = CHARGE_BOX_ID_VALIDATOR.isValid(chargeBoxId);
+        if (!isValid) {
+            log.error("ChargeBoxId '{}' violates the configured pattern.", chargeBoxId);
+            response.setStatusCode(HttpStatus.BAD_REQUEST);
+            return false;
+        }
+
         Optional<RegistrationStatus> status = chargePointHelperService.getRegistrationStatus(chargeBoxId);
 
         // Allow connections, if station is in db (registration_status field from db does not matter)

src/main/java/de/rwth/idsg/steve/web/validation/ChargeBoxId.java

@@ -36,7 +36,7 @@
 @Constraint(validatedBy = {ChargeBoxIdValidator.class, ChargeBoxIdListValidator.class})
 public @interface ChargeBoxId {
 
-    String message() default "ChargeBox ID cannot contain any whitespace";
+    String message() default "ChargeBox ID violates the configured pattern";
 
     // Required by validation runtime
     Class<?>[] groups() default {};

src/main/java/de/rwth/idsg/steve/web/validation/ChargeBoxIdValidator.java

@@ -18,8 +18,11 @@
  */
 package de.rwth.idsg.steve.web.validation;
 
+import com.google.common.base.Strings;
+import de.rwth.idsg.steve.SteveConfiguration;
 import jakarta.validation.ConstraintValidator;
 import jakarta.validation.ConstraintValidatorContext;
+
 import java.util.regex.Pattern;
 
 /**
@@ -28,8 +31,8 @@
  */
 public class ChargeBoxIdValidator implements ConstraintValidator<ChargeBoxId, String> {
 
-    private static final String REGEX = "\\S+";
-    private static final Pattern PATTERN = Pattern.compile(REGEX);
+    private static final String REGEX = "[^=/()<>]*";
+    private static final Pattern PATTERN = Pattern.compile(getRegexToUse());
 
     @Override
     public void initialize(ChargeBoxId idTag) {
@@ -38,6 +41,27 @@ public void initialize(ChargeBoxId idTag) {
 
     @Override
     public boolean isValid(String string, ConstraintValidatorContext constraintValidatorContext) {
-        return string == null || PATTERN.matcher(string).matches();
+        if (string == null) {
+            return true; // null is valid, because it is another constraint's responsibility
+        }
+        return isValid(string);
+    }
+
+    public boolean isValid(String str) {
+        if (Strings.isNullOrEmpty(str)) {
+            return false;
+        }
+
+        String str1 = str.strip();
+        if (!str1.equals(str)) {
+            return false;
+        }
+
+        return PATTERN.matcher(str).matches();
+    }
+
+    private static String getRegexToUse() {
+        String regexFromConfig = SteveConfiguration.CONFIG.getOcpp().getChargeBoxIdValidationRegex();
+        return Strings.isNullOrEmpty(regexFromConfig) ? REGEX : regexFromConfig;
     }
 }

src/main/resources/config/dev/main.properties

@@ -56,6 +56,11 @@ ws.session.select.strategy = ALWAYS_LAST
 #
 auto.register.unknown.stations = false
 
+# if this field is set, it will take precedence over the default regex we are using in
+# de.rwth.idsg.steve.web.validation.ChargeBoxIdValidator.REGEX to validate the format of the chargeBoxId values
+#
+charge-box-id.validation.regex =
+
 ### DO NOT MODIFY ###
 steve.version = ${project.version}
 git.describe = ${git.commit.id.describe}

src/main/resources/config/docker/main.properties

@@ -56,6 +56,11 @@ ws.session.select.strategy = ALWAYS_LAST
 #
 auto.register.unknown.stations = false
 
+# if this field is set, it will take precedence over the default regex we are using in
+# de.rwth.idsg.steve.web.validation.ChargeBoxIdValidator.REGEX to validate the format of the chargeBoxId values
+#
+charge-box-id.validation.regex =
+
 ### DO NOT MODIFY ###
 steve.version = ${project.version}
 git.describe = ${git.commit.id.describe}

src/main/resources/config/kubernetes/main.properties

@@ -56,6 +56,11 @@ ws.session.select.strategy = ALWAYS_LAST
 #
 auto.register.unknown.stations = false
 
+# if this field is set, it will take precedence over the default regex we are using in
+# de.rwth.idsg.steve.web.validation.ChargeBoxIdValidator.REGEX to validate the format of the chargeBoxId values
+#
+charge-box-id.validation.regex =
+
 ### DO NOT MODIFY ###
 steve.version = ${project.version}
 git.describe = ${git.commit.id.describe}

src/main/resources/config/prod/main.properties

@@ -56,6 +56,11 @@ ws.session.select.strategy = ALWAYS_LAST
 #
 auto.register.unknown.stations = false
 
+# if this field is set, it will take precedence over the default regex we are using in
+# de.rwth.idsg.steve.web.validation.ChargeBoxIdValidator.REGEX to validate the format of the chargeBoxId values
+#
+charge-box-id.validation.regex =
+
 ### DO NOT MODIFY ###
 steve.version = ${project.version}
 git.describe = ${git.commit.id.describe}

src/main/resources/config/test/main.properties

@@ -56,6 +56,11 @@ ws.session.select.strategy = ALWAYS_LAST
 #
 auto.register.unknown.stations = false
 
+# if this field is set, it will take precedence over the default regex we are using in
+# de.rwth.idsg.steve.web.validation.ChargeBoxIdValidator.REGEX to validate the format of the chargeBoxId values
+#
+charge-box-id.validation.regex =
+
 ### DO NOT MODIFY ###
 steve.version = ${project.version}
 git.describe = ${git.commit.id.describe}

src/main/resources/webapp/WEB-INF/views/00-header.jsp

@@ -22,6 +22,7 @@
 <%@ taglib uri="jakarta.tags.core" prefix="c" %>
 <%@ taglib uri="http://www.springframework.org/tags" prefix="spring" %>
 <%@ taglib uri="http://www.springframework.org/tags/form" prefix="form" %>
+<%@ taglib uri="owasp.encoder.jakarta" prefix="encode" %>
 
 <%@ include file="00-context.jsp" %>
 

src/main/resources/webapp/WEB-INF/views/data-man/chargepoints.jsp

@@ -48,7 +48,7 @@
             <tbody>
             <c:forEach items="${unknownList}" var="item">
                 <tr>
-                    <td>${item.key}</td>
+                    <td><encode:forHtml value="${item.key}" /></td>
                     <td>${item.numberOfAttempts}</td>
                     <td data-sort-value="${item.lastAttemptTimestamp.millis}">${item.lastAttemptTimestamp}</td>
                     <td>

src/test/java/de/rwth/idsg/steve/web/validation/ChargeBoxIdValidatorTest.java

@@ -0,0 +1,161 @@
+/*
+ * SteVe - SteckdosenVerwaltung - https://github.com/steve-community/steve
+ * Copyright (C) 2013-2024 SteVe Community Team
+ * All Rights Reserved.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ */
+package de.rwth.idsg.steve.web.validation;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+/**
+ * @author Sevket Goekay <sevketgokay@gmail.com>
+ * @since 01.08.2024
+ */
+public class ChargeBoxIdValidatorTest {
+
+    ChargeBoxIdValidator validator = new ChargeBoxIdValidator();
+
+    @Test
+    public void testNull() {
+        Assertions.assertFalse(validator.isValid(null));
+    }
+
+    @Test
+    public void testEmpty() {
+        Assertions.assertFalse(validator.isValid(""));
+    }
+
+    @Test
+    public void testSpace() {
+        Assertions.assertFalse(validator.isValid("  "));
+    }
+
+    @Test
+    public void testAllLowercaseLetters() {
+        Assertions.assertTrue(validator.isValid("test"));
+    }
+
+    @Test
+    public void testAllUppercaseLetters() {
+        Assertions.assertTrue(validator.isValid("TEST"));
+    }
+
+    @Test
+    public void testMixedCaseLetters() {
+        Assertions.assertTrue(validator.isValid("TesT"));
+        Assertions.assertTrue(validator.isValid("tEst"));
+    }
+
+    @Test
+    public void testLettersAndNumbers() {
+        Assertions.assertTrue(validator.isValid("test12"));
+        Assertions.assertTrue(validator.isValid("89test"));
+        Assertions.assertTrue(validator.isValid("te9s0t"));
+    }
+
+    @Test
+    public void testDot() {
+        Assertions.assertTrue(validator.isValid(".test"));
+        Assertions.assertTrue(validator.isValid("test."));
+        Assertions.assertTrue(validator.isValid("te..st"));
+    }
+
+    @Test
+    public void testDash() {
+        Assertions.assertTrue(validator.isValid("-test"));
+        Assertions.assertTrue(validator.isValid("test-"));
+        Assertions.assertTrue(validator.isValid("te--st"));
+    }
+
+    @Test
+    public void testUnderscore() {
+        Assertions.assertTrue(validator.isValid("_test"));
+        Assertions.assertTrue(validator.isValid("test_"));
+        Assertions.assertTrue(validator.isValid("te__st"));
+    }
+
+    @Test
+    public void testColon() {
+        Assertions.assertTrue(validator.isValid(":test"));
+        Assertions.assertTrue(validator.isValid("test:"));
+        Assertions.assertTrue(validator.isValid("te::st"));
+        Assertions.assertTrue(validator.isValid("VID:00XXXXXXXXXX"));
+    }
+
+    @Test
+    public void testPoundSign() {
+        Assertions.assertTrue(validator.isValid("#test"));
+        Assertions.assertTrue(validator.isValid("test#"));
+        Assertions.assertTrue(validator.isValid("te##st"));
+        Assertions.assertTrue(validator.isValid("#FreeCharging"));
+    }
+
+    @Test
+    public void testCombined() {
+        Assertions.assertTrue(validator.isValid("1t.E-S_:t20#"));
+    }
+
+    @Test
+    public void testSpaceAtBeginning() {
+        Assertions.assertFalse(validator.isValid(" test"));
+    }
+
+    @Test
+    public void testSpaceAtEnd() {
+        Assertions.assertFalse(validator.isValid("test "));
+    }
+
+    @Test
+    public void testSpaceInMiddle() {
+        Assertions.assertTrue(validator.isValid("test1 test2"));
+    }
+
+    @Test
+    public void testOpeningParenthesis() {
+        Assertions.assertFalse(validator.isValid("te(st"));
+    }
+
+    @Test
+    public void testClosingParenthesis() {
+        Assertions.assertFalse(validator.isValid("te)st"));
+    }
+
+    @Test
+    public void testBiggerSymbol() {
+        Assertions.assertFalse(validator.isValid("te>st"));
+    }
+
+    @Test
+    public void testSmallerSymbol() {
+        Assertions.assertFalse(validator.isValid("te<st"));
+    }
+
+    @Test
+    public void testSlash() {
+        Assertions.assertFalse(validator.isValid("te/st"));
+    }
+
+    @Test
+    public void testEquals() {
+        Assertions.assertFalse(validator.isValid("te=st"));
+    }
+
+    @Test
+    public void testSpecialCharAsExample() {
+        Assertions.assertTrue(validator.isValid("ÂtestÂ"));
+    }
+}