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 9b85c1d
Show file tree
Hide file tree
Showing 15 changed files with 196 additions and 28 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
Expand Up @@ -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;

Expand All @@ -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
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,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;
Expand Down Expand Up @@ -44,6 +45,9 @@ public class ManagementServicesProducer {
@Inject
Vertx vertx;

@Inject
JdbcBasedHttpServiceConfigOptions jdbcBasedHttpServiceConfigOptions;

/**
* Creates a service for retrieving tenant information.
*
Expand Down Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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();

}
Original file line number Diff line number Diff line change
@@ -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());
}

}
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 @@ -11,10 +11,11 @@
* SPDX-License-Identifier: EPL-2.0
*/


package org.eclipse.hono.deviceregistry.mongodb.app;

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.MongoDbBasedRegistrationConfigProperties;
import org.eclipse.hono.deviceregistry.mongodb.config.MongoDbBasedTenantsConfigProperties;
import org.eclipse.hono.deviceregistry.mongodb.model.CredentialsDao;
Expand Down Expand Up @@ -55,6 +56,9 @@ public class ManagementServicesProducer {
@Inject
MongoDbBasedCredentialsConfigProperties credentialsServiceProperties;

@Inject
MongoDbBasedHttpServiceConfigOptions mongoDbBasedHttpServiceConfigOptions;

/**
* Creates a service for retrieving tenant information.
*
Expand Down Expand Up @@ -102,7 +106,8 @@ public DeviceManagementService deviceManagementService(
vertx,
deviceDao,
credentialsDao,
registrationServiceProperties);
registrationServiceProperties,
new MongoDbBasedHttpServiceConfigProperties(mongoDbBasedHttpServiceConfigOptions));
service.setTenantInformationService(tenantInformationService);
return service;
}
Expand Down
Loading

0 comments on commit 9b85c1d

Please sign in to comment.