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 177882e
Show file tree
Hide file tree
Showing 12 changed files with 113 additions and 23 deletions.
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 Down Expand Up @@ -75,16 +74,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 httpServerFactory The Http server factory 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 HttpServerFactory httpServerFactory) {

final var service = new DeviceManagementServiceImpl(vertx, devicesManagementStore, deviceServiceOptions);
final var service = new DeviceManagementServiceImpl(
vertx,
devicesManagementStore,
deviceServiceOptions,
httpServerFactory.getHttpServerProperties());
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 Down Expand Up @@ -89,20 +89,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 httpServerFactory The Http server factory 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 HttpServerFactory httpServerFactory) {

final var service = new MongoDbBasedDeviceManagementService(
vertx,
deviceDao,
credentialsDao,
registrationServiceProperties);
registrationServiceProperties,
httpServerFactory.getHttpServerProperties());
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 177882e

Please sign in to comment.