Skip to content

Commit

Permalink
[eclipse-hono#3608] Verify that auto-provisioned device ids match dev…
Browse files Browse the repository at this point in the history
…ice id pattern
  • Loading branch information
harism committed Jun 2, 2024
1 parent 6a05ee3 commit ed5841c
Show file tree
Hide file tree
Showing 13 changed files with 120 additions and 22 deletions.
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -26,7 +26,9 @@

import io.opentracing.Tracer;
import io.vertx.core.Vertx;
import jakarta.enterprise.inject.Produces;
import jakarta.inject.Inject;
import jakarta.inject.Singleton;

/**
* A factory base class for creating Device Registry Management API endpoints.
Expand Down Expand Up @@ -57,6 +59,8 @@ public abstract class AbstractHttpServerFactory {
*
* @return The properties.
*/
@Produces
@Singleton
protected abstract HttpServiceConfigProperties getHttpServerProperties();

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
*
* SPDX-License-Identifier: EPL-2.0
*******************************************************************************/

package org.eclipse.hono.deviceregistry.service.device;

import java.net.HttpURLConnection;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -10,6 +10,7 @@
*
* SPDX-License-Identifier: EPL-2.0
*******************************************************************************/

package org.eclipse.hono.service.registration;

import static com.google.common.truth.Truth.assertThat;
Expand Down Expand Up @@ -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.
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -44,6 +44,9 @@ public class ManagementServicesProducer {
@Inject
Vertx vertx;

@Inject
HttpServiceConfigProperties httpServiceConfigProperties;

/**
* Creates a service for retrieving tenant information.
*
Expand Down Expand Up @@ -84,7 +87,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,
httpServiceConfigProperties);
service.setTenantInformationService(tenantInformationService);
return service;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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();
}));
}
}
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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;
Expand Down Expand Up @@ -55,6 +56,9 @@ public class ManagementServicesProducer {
@Inject
MongoDbBasedCredentialsConfigProperties credentialsServiceProperties;

@Inject
HttpServiceConfigProperties httpServiceConfigProperties;

/**
* Creates a service for retrieving tenant information.
*
Expand Down Expand Up @@ -102,7 +106,8 @@ public DeviceManagementService deviceManagementService(
vertx,
deviceDao,
credentialsDao,
registrationServiceProperties);
registrationServiceProperties,
httpServiceConfigProperties);
service.setTenantInformationService(tenantInformationService);
return service;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand Down
Loading

0 comments on commit ed5841c

Please sign in to comment.