[eclipse-hono#3608] Verify that auto-provisioned device ids match dev…

…ice id pattern
harism · Jun 2, 2024 · c76d2ad · c76d2ad
1 parent 6a05ee3
commit c76d2ad
Show file tree

Hide file tree

Showing 15 changed files with 201 additions and 31 deletions.
diff --git a/.../java/org/eclipse/hono/deviceregistry/service/device/AbstractDeviceManagementService.java b/.../java/org/eclipse/hono/deviceregistry/service/device/AbstractDeviceManagementService.java
@@ -10,6 +10,7 @@
  *
  * SPDX-License-Identifier: EPL-2.0
  *******************************************************************************/
+
 package org.eclipse.hono.deviceregistry.service.device;
 
 import java.net.HttpURLConnection;
@@ -19,10 +20,13 @@
 import java.util.Objects;
 import java.util.Optional;
 import java.util.UUID;
+import java.util.regex.Matcher;
 
+import org.eclipse.hono.client.ClientErrorException;
 import org.eclipse.hono.client.ServerErrorException;
 import org.eclipse.hono.client.ServiceInvocationException;
 import org.eclipse.hono.client.util.StatusCodeMapper;
+import org.eclipse.hono.config.ServiceConfigProperties;
 import org.eclipse.hono.deviceregistry.service.tenant.NoopTenantInformationService;
 import org.eclipse.hono.deviceregistry.service.tenant.TenantInformationService;
 import org.eclipse.hono.deviceregistry.util.DeviceRegistryUtils;
@@ -64,15 +68,19 @@ public abstract class AbstractDeviceManagementService implements DeviceManagemen
 
     private final Handler<AbstractNotification> notificationSender;
 
+    private final ServiceConfigProperties serviceConfig;
+
     /**
      * Creates a new AbstractDeviceManagementService.
      *
      * @param vertx The vert.x instance to use.
+     * @param serviceConfig The service config instance to use for device id validation.
      * @throws NullPointerException if vertx is {@code null}.
      */
-    protected AbstractDeviceManagementService(final Vertx vertx) {
+    protected AbstractDeviceManagementService(final Vertx vertx, final ServiceConfigProperties serviceConfig) {
         this.vertx = Objects.requireNonNull(vertx);
         this.notificationSender = NotificationEventBusSupport.getNotificationSender(vertx);
+        this.serviceConfig = serviceConfig;
     }
 
     /**
@@ -276,6 +284,15 @@ public final Future<OperationResult<Id>> createDevice(
         Objects.requireNonNull(span);
 
         final String deviceIdValue = deviceId.orElseGet(() -> generateDeviceId(tenantId));
+        final Matcher matcher = Objects.requireNonNull(serviceConfig.getDeviceIdPattern()).matcher(deviceIdValue);
+
+        if (!matcher.matches()) {
+            return Future.failedFuture(new ClientErrorException(
+                    tenantId,
+                    HttpURLConnection.HTTP_BAD_REQUEST,
+                    "invalid device id \"" + deviceIdValue + "\""
+            ));
+        }
 
         return this.tenantInformationService
                 .tenantExists(tenantId, span)

diff --git a/...a/org/eclipse/hono/deviceregistry/service/device/AbstractDeviceManagementServiceTest.java b/...a/org/eclipse/hono/deviceregistry/service/device/AbstractDeviceManagementServiceTest.java
@@ -27,7 +27,9 @@
 import java.net.HttpURLConnection;
 import java.util.List;
 import java.util.Optional;
+import java.util.regex.Pattern;
 
+import org.eclipse.hono.config.ServiceConfigProperties;
 import org.eclipse.hono.notification.NotificationEventBusSupport;
 import org.eclipse.hono.notification.deviceregistry.AllDevicesOfTenantDeletedNotification;
 import org.eclipse.hono.notification.deviceregistry.DeviceChangeNotification;
@@ -36,6 +38,7 @@
 import org.eclipse.hono.service.management.OperationResult;
 import org.eclipse.hono.service.management.Result;
 import org.eclipse.hono.service.management.device.Device;
+import org.eclipse.hono.util.RegistryManagementConstants;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -67,7 +70,10 @@ void setUp() {
         eventBus = mock(EventBus.class);
         final Vertx vertx = mock(Vertx.class);
         when(vertx.eventBus()).thenReturn(eventBus);
-        deviceManagementService = new TestDeviceManagementService(vertx);
+        final ServiceConfigProperties serviceConfig = mock(ServiceConfigProperties.class);
+        when(serviceConfig.getDeviceIdPattern())
+                .thenReturn(Pattern.compile(RegistryManagementConstants.DEFAULT_REGEX_DEVICE_ID));
+        deviceManagementService = new TestDeviceManagementService(vertx, serviceConfig);
     }
 
     /**
@@ -223,8 +229,8 @@ public void testNotificationOnDeleteDevicesOfTenant(final VertxTestContext conte
 
     private static class TestDeviceManagementService extends AbstractDeviceManagementService {
 
-        TestDeviceManagementService(final Vertx vertx) {
-            super(vertx);
+        TestDeviceManagementService(final Vertx vertx, final ServiceConfigProperties serviceConfig) {
+            super(vertx, serviceConfig);
         }
 
         @Override

diff --git a/.../src/test/java/org/eclipse/hono/service/registration/AbstractRegistrationServiceTest.java b/.../src/test/java/org/eclipse/hono/service/registration/AbstractRegistrationServiceTest.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2016, 2023 Contributors to the Eclipse Foundation
+ * Copyright (c) 2016 Contributors to the Eclipse Foundation
  *
  * See the NOTICE file(s) distributed with this work for additional
  * information regarding copyright ownership.
@@ -10,6 +10,7 @@
  *
  * SPDX-License-Identifier: EPL-2.0
  *******************************************************************************/
+
 package org.eclipse.hono.service.registration;
 
 import static com.google.common.truth.Truth.assertThat;
@@ -61,6 +62,10 @@ public interface AbstractRegistrationServiceTest {
      * The device identifier used in tests.
      */
     String DEVICE = "4711";
+    /**
+     * Invalid device identifier used in tests.
+     */
+    String INVALID_DEVICE = "**4711++";
     /**
      * The gateway identifier used in the tests.
      */

diff --git a/...gistry-jdbc/src/main/java/org/eclipse/hono/deviceregistry/jdbc/app/HttpServerFactory.java b/...gistry-jdbc/src/main/java/org/eclipse/hono/deviceregistry/jdbc/app/HttpServerFactory.java
@@ -1,5 +1,5 @@
 /**
- * Copyright (c) 2022, 2023 Contributors to the Eclipse Foundation
+ * Copyright (c) 2022 Contributors to the Eclipse Foundation
  *
  * See the NOTICE file(s) distributed with this work for additional
  * information regarding copyright ownership.
@@ -14,10 +14,10 @@
 package org.eclipse.hono.deviceregistry.jdbc.app;
 
 import org.eclipse.hono.deviceregistry.app.AbstractHttpServerFactory;
-import org.eclipse.hono.service.http.HttpServiceConfigOptions;
+import org.eclipse.hono.deviceregistry.jdbc.config.JdbcBasedHttpServiceConfigOptions;
+import org.eclipse.hono.deviceregistry.jdbc.config.JdbcBasedHttpServiceConfigProperties;
 import org.eclipse.hono.service.http.HttpServiceConfigProperties;
 
-import io.smallrye.config.ConfigMapping;
 import jakarta.enterprise.context.ApplicationScoped;
 import jakarta.inject.Inject;
 
@@ -31,10 +31,8 @@ public class HttpServerFactory extends AbstractHttpServerFactory {
     private HttpServiceConfigProperties httpServerProperties;
 
     @Inject
-    void setHttpServerProperties(
-            @ConfigMapping(prefix = "hono.registry.http")
-            final HttpServiceConfigOptions endpointOptions) {
-        this.httpServerProperties = new HttpServiceConfigProperties(endpointOptions);
+    void setHttpServerProperties(final JdbcBasedHttpServiceConfigOptions endpointOptions) {
+        this.httpServerProperties = new JdbcBasedHttpServiceConfigProperties(endpointOptions);
     }
 
     @Override

diff --git a/...bc/src/main/java/org/eclipse/hono/deviceregistry/jdbc/app/ManagementServicesProducer.java b/...bc/src/main/java/org/eclipse/hono/deviceregistry/jdbc/app/ManagementServicesProducer.java
@@ -1,5 +1,5 @@
 /**
- * Copyright (c) 2022, 2023 Contributors to the Eclipse Foundation
+ * Copyright (c) 2022 Contributors to the Eclipse Foundation
  *
  * See the NOTICE file(s) distributed with this work for additional
  * information regarding copyright ownership.
@@ -11,10 +11,11 @@
  * SPDX-License-Identifier: EPL-2.0
  */
 
-
 package org.eclipse.hono.deviceregistry.jdbc.app;
 
 import org.eclipse.hono.deviceregistry.jdbc.config.DeviceServiceOptions;
+import org.eclipse.hono.deviceregistry.jdbc.config.JdbcBasedHttpServiceConfigOptions;
+import org.eclipse.hono.deviceregistry.jdbc.config.JdbcBasedHttpServiceConfigProperties;
 import org.eclipse.hono.deviceregistry.jdbc.impl.CredentialsManagementServiceImpl;
 import org.eclipse.hono.deviceregistry.jdbc.impl.DeviceManagementServiceImpl;
 import org.eclipse.hono.deviceregistry.jdbc.impl.StoreBasedTenantInformationService;
@@ -44,6 +45,9 @@ public class ManagementServicesProducer {
     @Inject
     Vertx vertx;
 
+    @Inject
+    JdbcBasedHttpServiceConfigOptions jdbcBasedHttpServiceConfigOptions;
+
     /**
      * Creates a service for retrieving tenant information.
      *
@@ -84,7 +88,11 @@ public DeviceManagementService deviceManagementService(
             final DeviceServiceOptions deviceServiceOptions,
             final TenantInformationService tenantInformationService) {
 
-        final var service = new DeviceManagementServiceImpl(vertx, devicesManagementStore, deviceServiceOptions);
+        final var service = new DeviceManagementServiceImpl(
+                vertx,
+                devicesManagementStore,
+                deviceServiceOptions,
+                new JdbcBasedHttpServiceConfigProperties(jdbcBasedHttpServiceConfigOptions));
         service.setTenantInformationService(tenantInformationService);
         return service;
     }

diff --git a/...n/java/org/eclipse/hono/deviceregistry/jdbc/config/JdbcBasedHttpServiceConfigOptions.java b/...n/java/org/eclipse/hono/deviceregistry/jdbc/config/JdbcBasedHttpServiceConfigOptions.java
@@ -0,0 +1,35 @@
+/**
+ * Copyright (c) 2024 Contributors to the Eclipse Foundation
+ *
+ * See the NOTICE file(s) distributed with this work for additional
+ * information regarding copyright ownership.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ */
+
+package org.eclipse.hono.deviceregistry.jdbc.config;
+
+import org.eclipse.hono.service.http.HttpServiceConfigOptions;
+
+import io.smallrye.config.ConfigMapping;
+import io.smallrye.config.WithParentName;
+
+/**
+ * Configuration properties for the JDBC based registry's HTTP endpoint.
+ */
+@ConfigMapping(prefix = "hono.registry.http", namingStrategy = ConfigMapping.NamingStrategy.VERBATIM)
+public interface JdbcBasedHttpServiceConfigOptions {
+
+    /**
+     * Gets the HTTP service configuration.
+     *
+     * @return The options.
+     */
+    @WithParentName
+    HttpServiceConfigOptions commonOptions();
+
+}
diff --git a/...ava/org/eclipse/hono/deviceregistry/jdbc/config/JdbcBasedHttpServiceConfigProperties.java b/...ava/org/eclipse/hono/deviceregistry/jdbc/config/JdbcBasedHttpServiceConfigProperties.java
@@ -0,0 +1,40 @@
+/**
+ * Copyright (c) 2024 Contributors to the Eclipse Foundation
+ *
+ * See the NOTICE file(s) distributed with this work for additional
+ * information regarding copyright ownership.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ */
+
+package org.eclipse.hono.deviceregistry.jdbc.config;
+
+import org.eclipse.hono.service.http.HttpServiceConfigProperties;
+
+/**
+ * Configuration properties for the registry's HTTP endpoint.
+ */
+public class JdbcBasedHttpServiceConfigProperties extends HttpServiceConfigProperties {
+
+    /**
+     * Creates default properties.
+     */
+    public JdbcBasedHttpServiceConfigProperties() {
+        super();
+    }
+
+    /**
+     * Creates properties from existing options.
+     *
+     * @param options The options.
+     * @throws NullPointerException if options are {@code null}.
+     */
+    public JdbcBasedHttpServiceConfigProperties(final JdbcBasedHttpServiceConfigOptions options) {
+        super(options.commonOptions());
+    }
+
+}
diff --git a/.../src/main/java/org/eclipse/hono/deviceregistry/jdbc/impl/DeviceManagementServiceImpl.java b/.../src/main/java/org/eclipse/hono/deviceregistry/jdbc/impl/DeviceManagementServiceImpl.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2020, 2023 Contributors to the Eclipse Foundation
+ * Copyright (c) 2020 Contributors to the Eclipse Foundation
  *
  * See the NOTICE file(s) distributed with this work for additional
  * information regarding copyright ownership.
@@ -20,6 +20,7 @@
 import java.util.UUID;
 
 import org.eclipse.hono.client.ClientErrorException;
+import org.eclipse.hono.config.ServiceConfigProperties;
 import org.eclipse.hono.deviceregistry.jdbc.config.DeviceServiceOptions;
 import org.eclipse.hono.deviceregistry.service.device.AbstractDeviceManagementService;
 import org.eclipse.hono.deviceregistry.service.device.DeviceKey;
@@ -53,9 +54,14 @@ public class DeviceManagementServiceImpl extends AbstractDeviceManagementService
      * @param vertx The vert.x instance to use.
      * @param store The backing store to use.
      * @param properties The service properties.
+     * @param serviceConfig The service config to use.
      */
-    public DeviceManagementServiceImpl(final Vertx vertx, final TableManagementStore store, final DeviceServiceOptions properties) {
-        super(vertx);
+    public DeviceManagementServiceImpl(
+            final Vertx vertx,
+            final TableManagementStore store,
+            final DeviceServiceOptions properties,
+            final ServiceConfigProperties serviceConfig) {
+        super(vertx, serviceConfig);
         this.store = store;
         this.ttl = Optional.of(CacheDirective.maxAgeDirective(properties.registrationTtl()));
         this.config = properties;

diff --git a/...dbc/src/test/java/org/eclipse/hono/deviceregistry/jdbc/impl/AbstractJdbcRegistryTest.java b/...dbc/src/test/java/org/eclipse/hono/deviceregistry/jdbc/impl/AbstractJdbcRegistryTest.java
@@ -36,6 +36,7 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicLong;
 
+import org.eclipse.hono.config.ServiceConfigProperties;
 import org.eclipse.hono.deviceregistry.jdbc.config.DeviceServiceOptions;
 import org.eclipse.hono.deviceregistry.jdbc.config.TenantServiceOptions;
 import org.eclipse.hono.deviceregistry.service.tenant.TenantInformationService;
@@ -129,6 +130,8 @@ void startDevices(final Vertx vertx) throws IOException, SQLException {
         when(properties.credentialsTtl()).thenReturn(Duration.ofMinutes(1));
         when(properties.registrationTtl()).thenReturn(Duration.ofMinutes(1));
 
+        final ServiceConfigProperties serviceConfig = new ServiceConfigProperties();
+
         this.credentialsAdapter = new CredentialsServiceImpl(
                 DeviceStores.adapterStoreFactory().createTable(vertx, TRACER, jdbc, Optional.empty(), Optional.empty(), Optional.empty()),
                 properties
@@ -147,7 +150,8 @@ void startDevices(final Vertx vertx) throws IOException, SQLException {
         this.registrationManagement = new DeviceManagementServiceImpl(
                 vertx,
                 DeviceStores.managementStoreFactory().createTable(vertx, TRACER, jdbc, Optional.empty(), Optional.empty(), Optional.empty()),
-                properties
+                properties,
+                serviceConfig
         );
 
         tenantInformationService = mock(TenantInformationService.class);

diff --git a/...test/java/org/eclipse/hono/deviceregistry/jdbc/impl/JdbcBasedRegistrationServiceTest.java b/...test/java/org/eclipse/hono/deviceregistry/jdbc/impl/JdbcBasedRegistrationServiceTest.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2020, 2021 Contributors to the Eclipse Foundation
+ * Copyright (c) 2020 Contributors to the Eclipse Foundation
  *
  * See the NOTICE file(s) distributed with this work for additional
  * information regarding copyright ownership.
@@ -73,4 +73,18 @@ public void testCreateDeviceFailsIfTenantLevelDeviceLimitHasBeenReached(final Ve
                 ctx.completeNow();
             }));
     }
+
+    /**
+     * Verifies that a request to create device with device id which does not match the device id pattern fails with a 400.
+     *
+     * @param ctx The vert.x test context.
+     */
+    @Test
+    public void testCreateDeviceFailsIfDeviceIdDoesNotMatchDeviceIdPattern(final VertxTestContext ctx) {
+        getDeviceManagementService().createDevice(TENANT, Optional.of(INVALID_DEVICE), new Device(), NoopSpan.INSTANCE)
+            .onComplete(ctx.failing(t -> {
+                ctx.verify(() -> Assertions.assertServiceInvocationException(t, HttpURLConnection.HTTP_BAD_REQUEST));
+                ctx.completeNow();
+            }));
+    }
 }