Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 = 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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -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
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 @@ -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;
Expand Down Expand Up @@ -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);
}
}
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 @@ -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;

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