[#3608] Verify that auto-provisioned device ids match device id pattern

eclipse-hono · Jun 2, 2024 · d72c3d6 · d72c3d6
1 parent 6a05ee3
commit d72c3d6
Show file tree

Hide file tree

Showing 12 changed files with 115 additions and 23 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/...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,7 +11,6 @@
  * SPDX-License-Identifier: EPL-2.0
  */
 
-
 package org.eclipse.hono.deviceregistry.jdbc.app;
 
 import org.eclipse.hono.deviceregistry.jdbc.config.DeviceServiceOptions;
@@ -24,6 +23,7 @@
 import org.eclipse.hono.service.base.jdbc.store.device.TableManagementStore;
 import org.eclipse.hono.service.base.jdbc.store.tenant.AdapterStore;
 import org.eclipse.hono.service.base.jdbc.store.tenant.ManagementStore;
+import org.eclipse.hono.service.http.HttpServiceConfigProperties;
 import org.eclipse.hono.service.management.credentials.CredentialsManagementService;
 import org.eclipse.hono.service.management.device.DeviceManagementService;
 import org.eclipse.hono.service.management.tenant.TenantManagementService;
@@ -75,16 +75,22 @@ public TenantManagementService tenantManagementService(
      * @param devicesManagementStore The data store for accessing device data.
      * @param deviceServiceOptions The device management service configuration.
      * @param tenantInformationService The service for retrieving tenant information.
+     * @param httpServiceConfigProperties The Http service config properties to read server config from.
      * @return The service.
      */
     @Produces
     @Singleton
     public DeviceManagementService deviceManagementService(
             final TableManagementStore devicesManagementStore,
             final DeviceServiceOptions deviceServiceOptions,
-            final TenantInformationService tenantInformationService) {
+            final TenantInformationService tenantInformationService,
+            final HttpServiceConfigProperties httpServiceConfigProperties) {
 
-        final var service = new DeviceManagementServiceImpl(vertx, devicesManagementStore, deviceServiceOptions);
+        final var service = new DeviceManagementServiceImpl(
+                vertx,
+                devicesManagementStore,
+                deviceServiceOptions,
+                httpServiceConfigProperties);
         service.setTenantInformationService(tenantInformationService);
         return service;
     }

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();
+            }));
+    }
 }
diff --git a/...src/main/java/org/eclipse/hono/deviceregistry/mongodb/app/ManagementServicesProducer.java b/...src/main/java/org/eclipse/hono/deviceregistry/mongodb/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.
@@ -26,6 +26,7 @@
 import org.eclipse.hono.deviceregistry.mongodb.service.MongoDbBasedTenantManagementService;
 import org.eclipse.hono.deviceregistry.service.tenant.TenantInformationService;
 import org.eclipse.hono.service.auth.SpringBasedHonoPasswordEncoder;
+import org.eclipse.hono.service.http.HttpServiceConfigProperties;
 import org.eclipse.hono.service.management.credentials.CredentialsManagementService;
 import org.eclipse.hono.service.management.device.DeviceManagementService;
 import org.eclipse.hono.service.management.tenant.TenantManagementService;
@@ -89,20 +90,23 @@ public TenantManagementService tenantManagementService(final TenantDao tenantDao
      * @param deviceDao The DAO for accessing device data.
      * @param credentialsDao The DAO for accessing credentials data.
      * @param tenantInformationService The service for retrieving tenant information.
+     * @param httpServiceConfigProperties The Http service config properties to read server config from.
      * @return The service.
      */
     @Produces
     @Singleton
     public DeviceManagementService deviceManagementService(
             final DeviceDao deviceDao,
             final CredentialsDao credentialsDao,
-            final TenantInformationService tenantInformationService) {
+            final TenantInformationService tenantInformationService,
+            final HttpServiceConfigProperties httpServiceConfigProperties) {
 
         final var service = new MongoDbBasedDeviceManagementService(
                 vertx,
                 deviceDao,
                 credentialsDao,
-                registrationServiceProperties);
+                registrationServiceProperties,
+                httpServiceConfigProperties);
         service.setTenantInformationService(tenantInformationService);
         return service;
     }

diff --git a/.../org/eclipse/hono/deviceregistry/mongodb/service/MongoDbBasedDeviceManagementService.java b/.../org/eclipse/hono/deviceregistry/mongodb/service/MongoDbBasedDeviceManagementService.java
@@ -17,6 +17,7 @@
 import java.util.Objects;
 import java.util.Optional;
 
+import org.eclipse.hono.config.ServiceConfigProperties;
 import org.eclipse.hono.deviceregistry.mongodb.config.MongoDbBasedRegistrationConfigProperties;
 import org.eclipse.hono.deviceregistry.mongodb.model.CredentialsDao;
 import org.eclipse.hono.deviceregistry.mongodb.model.DeviceDao;
@@ -62,14 +63,16 @@ public final class MongoDbBasedDeviceManagementService extends AbstractDeviceMan
      * @param deviceDao The data access object to use for accessing device data in the MongoDB.
      * @param credentialsDao The data access object to use for accessing credentials data in the MongoDB.
      * @param config The properties for configuring this service.
+     * @param serviceConfig The service config to use.
      * @throws NullPointerException if any of the parameters are {@code null}.
      */
     public MongoDbBasedDeviceManagementService(
             final Vertx vertx,
             final DeviceDao deviceDao,
             final CredentialsDao credentialsDao,
-            final MongoDbBasedRegistrationConfigProperties config) {
-        super(vertx);
+            final MongoDbBasedRegistrationConfigProperties config,
+            final ServiceConfigProperties serviceConfig) {
+        super(vertx, serviceConfig);
         Objects.requireNonNull(deviceDao);
         Objects.requireNonNull(credentialsDao);
         Objects.requireNonNull(config);

diff --git a/...se/hono/deviceregistry/mongodb/service/MongoDBBasedDeviceManagementSearchDevicesTest.java b/...se/hono/deviceregistry/mongodb/service/MongoDBBasedDeviceManagementSearchDevicesTest.java
@@ -15,6 +15,7 @@
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 
+import org.eclipse.hono.config.ServiceConfigProperties;
 import org.eclipse.hono.deviceregistry.mongodb.config.MongoDbBasedRegistrationConfigProperties;
 import org.eclipse.hono.deviceregistry.mongodb.model.MongoDbBasedCredentialsDao;
 import org.eclipse.hono.deviceregistry.mongodb.model.MongoDbBasedDeviceDao;
@@ -49,6 +50,7 @@ public final class MongoDBBasedDeviceManagementSearchDevicesTest implements Abst
     private static final String DB_NAME = "hono-search-devices-test";
     private static final Logger LOG = LoggerFactory.getLogger(MongoDBBasedDeviceManagementSearchDevicesTest.class);
     private final MongoDbBasedRegistrationConfigProperties config = new MongoDbBasedRegistrationConfigProperties();
+    private final ServiceConfigProperties serviceConfig = new ServiceConfigProperties();
     private MongoDbBasedDeviceDao deviceDao;
     private MongoDbBasedCredentialsDao credentialsDao;
     private MongoDbBasedDeviceManagementService service;
@@ -65,7 +67,7 @@ public void setup(final VertxTestContext testContext) {
         vertx = Vertx.vertx();
         deviceDao = MongoDbTestUtils.getDeviceDao(vertx, DB_NAME);
         credentialsDao = MongoDbTestUtils.getCredentialsDao(vertx, DB_NAME);
-        service = new MongoDbBasedDeviceManagementService(vertx, deviceDao, credentialsDao, config);
+        service = new MongoDbBasedDeviceManagementService(vertx, deviceDao, credentialsDao, config, serviceConfig);
         Future.all(deviceDao.createIndices(), credentialsDao.createIndices()).onComplete(testContext.succeedingThenComplete());
     }
 

diff --git a/...va/org/eclipse/hono/deviceregistry/mongodb/service/MongoDbBasedCredentialServiceTest.java b/...va/org/eclipse/hono/deviceregistry/mongodb/service/MongoDbBasedCredentialServiceTest.java
@@ -25,6 +25,7 @@
 import java.util.UUID;
 import java.util.concurrent.TimeUnit;
 
+import org.eclipse.hono.config.ServiceConfigProperties;
 import org.eclipse.hono.deviceregistry.mongodb.config.MongoDbBasedCredentialsConfigProperties;
 import org.eclipse.hono.deviceregistry.mongodb.config.MongoDbBasedRegistrationConfigProperties;
 import org.eclipse.hono.deviceregistry.mongodb.model.MongoDbBasedCredentialsDao;
@@ -80,6 +81,7 @@ public class MongoDbBasedCredentialServiceTest implements CredentialsServiceTest
 
     private final MongoDbBasedCredentialsConfigProperties credentialsServiceConfig = new MongoDbBasedCredentialsConfigProperties();
     private final MongoDbBasedRegistrationConfigProperties registrationServiceConfig = new MongoDbBasedRegistrationConfigProperties();
+    private final ServiceConfigProperties serviceConfig = new ServiceConfigProperties();
 
     private MongoDbBasedCredentialsDao credentialsDao;
     private MongoDbBasedDeviceDao deviceDao;
@@ -112,7 +114,8 @@ public void createServices(final VertxTestContext ctx) {
                 vertx,
                 deviceDao,
                 credentialsDao,
-                registrationServiceConfig);
+                registrationServiceConfig,
+                serviceConfig);
 
         Future.all(deviceDao.createIndices(), credentialsDao.createIndices())
             .onComplete(ctx.succeedingThenComplete());