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

eclipse-hono · Jun 24, 2024 · 778a8e3 · 778a8e3
1 parent 34383d5
commit 778a8e3
Show file tree

Hide file tree

Showing 17 changed files with 179 additions and 46 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 = serviceConfig.getDeviceIdPattern().matcher(deviceIdValue);
+
+        if (!matcher.matches()) {
+            return Future.failedFuture(new ClientErrorException(
+                    tenantId,
+                    HttpURLConnection.HTTP_BAD_REQUEST,
+                    "invalid device ID: %s".formatted(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
@@ -28,6 +28,7 @@
 import java.util.List;
 import java.util.Optional;
 
+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;
@@ -67,7 +68,8 @@ void setUp() {
         eventBus = mock(EventBus.class);
         final Vertx vertx = mock(Vertx.class);
         when(vertx.eventBus()).thenReturn(eventBus);
-        deviceManagementService = new TestDeviceManagementService(vertx);
+        final ServiceConfigProperties serviceConfig = new ServiceConfigProperties();
+        deviceManagementService = new TestDeviceManagementService(vertx, serviceConfig);
     }
 
     /**
@@ -223,8 +225,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/...jdbc/src/main/java/org/eclipse/hono/deviceregistry/jdbc/app/ConfigPropertiesProducer.java b/...jdbc/src/main/java/org/eclipse/hono/deviceregistry/jdbc/app/ConfigPropertiesProducer.java
@@ -11,14 +11,16 @@
  * SPDX-License-Identifier: EPL-2.0
  */
 
-
 package org.eclipse.hono.deviceregistry.jdbc.app;
 
 import org.eclipse.hono.service.base.jdbc.config.JdbcDeviceStoreOptions;
 import org.eclipse.hono.service.base.jdbc.config.JdbcDeviceStoreProperties;
 import org.eclipse.hono.service.base.jdbc.config.JdbcTenantStoreOptions;
 import org.eclipse.hono.service.base.jdbc.config.JdbcTenantStoreProperties;
+import org.eclipse.hono.service.http.HttpServiceConfigOptions;
+import org.eclipse.hono.service.http.HttpServiceConfigProperties;
 
+import io.smallrye.config.ConfigMapping;
 import jakarta.enterprise.context.ApplicationScoped;
 import jakarta.enterprise.inject.Produces;
 import jakarta.inject.Singleton;
@@ -53,4 +55,18 @@ public JdbcTenantStoreProperties tenantsProperties(final JdbcTenantStoreOptions
     public JdbcDeviceStoreProperties devicesProperties(final JdbcDeviceStoreOptions options) {
         return new JdbcDeviceStoreProperties(options);
     }
+
+    /**
+     * Creates HTTP service configuration properties from existing options.
+     *
+     * @param options The options.
+     * @return The properties.
+     */
+    @Produces
+    @Singleton
+    public HttpServiceConfigProperties httpServiceConfigProperties(
+            @ConfigMapping(prefix = "hono.registry.http")
+            final HttpServiceConfigOptions options) {
+        return new HttpServiceConfigProperties(options);
+    }
 }
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,8 @@
 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.service.http.HttpServiceConfigProperties;
 
-import io.smallrye.config.ConfigMapping;
 import jakarta.enterprise.context.ApplicationScoped;
 import jakarta.inject.Inject;
 
@@ -28,17 +26,11 @@
 @ApplicationScoped
 public class HttpServerFactory extends AbstractHttpServerFactory {
 
-    private HttpServiceConfigProperties httpServerProperties;
-
     @Inject
-    void setHttpServerProperties(
-            @ConfigMapping(prefix = "hono.registry.http")
-            final HttpServiceConfigOptions endpointOptions) {
-        this.httpServerProperties = new HttpServiceConfigProperties(endpointOptions);
-    }
+    HttpServiceConfigProperties httpServerConfigProperties;
 
     @Override
     protected final HttpServiceConfigProperties getHttpServerProperties() {
-        return httpServerProperties;
+        return httpServerConfigProperties;
     }
 }
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 configuration.
      * @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/...b/src/main/java/org/eclipse/hono/deviceregistry/mongodb/app/ConfigPropertiesProducer.java b/...b/src/main/java/org/eclipse/hono/deviceregistry/mongodb/app/ConfigPropertiesProducer.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.
@@ -15,6 +15,8 @@
 
 import org.eclipse.hono.deviceregistry.mongodb.config.MongoDbBasedCredentialsConfigOptions;
 import org.eclipse.hono.deviceregistry.mongodb.config.MongoDbBasedCredentialsConfigProperties;
+import org.eclipse.hono.deviceregistry.mongodb.config.MongoDbBasedHttpServiceConfigOptions;
+import org.eclipse.hono.deviceregistry.mongodb.config.MongoDbBasedHttpServiceConfigProperties;
 import org.eclipse.hono.deviceregistry.mongodb.config.MongoDbBasedRegistrationConfigOptions;
 import org.eclipse.hono.deviceregistry.mongodb.config.MongoDbBasedRegistrationConfigProperties;
 import org.eclipse.hono.deviceregistry.mongodb.config.MongoDbBasedTenantsConfigOptions;
@@ -69,4 +71,17 @@ public MongoDbBasedCredentialsConfigProperties credentialsServiceProperties(
             final MongoDbBasedCredentialsConfigOptions options) {
         return new MongoDbBasedCredentialsConfigProperties(options);
     }
+
+    /**
+     * Creates HTTP service configuration properties from existing options.
+     *
+     * @param options The options.
+     * @return The properties.
+     */
+    @Produces
+    @Singleton
+    public MongoDbBasedHttpServiceConfigProperties httpServiceProperties(
+            final MongoDbBasedHttpServiceConfigOptions options) {
+        return new MongoDbBasedHttpServiceConfigProperties(options);
+    }
 }
diff --git a/...-mongodb/src/main/java/org/eclipse/hono/deviceregistry/mongodb/app/HttpServerFactory.java b/...-mongodb/src/main/java/org/eclipse/hono/deviceregistry/mongodb/app/HttpServerFactory.java
@@ -17,7 +17,6 @@
 import java.util.Optional;
 
 import org.eclipse.hono.deviceregistry.app.AbstractHttpServerFactory;
-import org.eclipse.hono.deviceregistry.mongodb.config.MongoDbBasedHttpServiceConfigOptions;
 import org.eclipse.hono.deviceregistry.mongodb.config.MongoDbBasedHttpServiceConfigProperties;
 import org.eclipse.hono.deviceregistry.server.DeviceRegistryHttpServer;
 import org.eclipse.hono.service.http.HttpServiceConfigProperties;
@@ -44,12 +43,8 @@ public class HttpServerFactory extends AbstractHttpServerFactory {
     @Inject
     MongoClient mongoClient;
 
-    private MongoDbBasedHttpServiceConfigProperties httpServerProperties;
-
     @Inject
-    void setHttpServerProperties(final MongoDbBasedHttpServiceConfigOptions endpointOptions) {
-        this.httpServerProperties = new MongoDbBasedHttpServiceConfigProperties(endpointOptions);
-    }
+    MongoDbBasedHttpServiceConfigProperties httpServerProperties;
 
     @Override
     protected final HttpServiceConfigProperties getHttpServerProperties() {