diff --git a/core/src/main/java/org/eclipse/hono/util/CredentialsConstants.java b/core/src/main/java/org/eclipse/hono/util/CredentialsConstants.java index 14dd7b7c79..6d383a9b14 100644 --- a/core/src/main/java/org/eclipse/hono/util/CredentialsConstants.java +++ b/core/src/main/java/org/eclipse/hono/util/CredentialsConstants.java @@ -14,6 +14,7 @@ import java.util.Objects; import java.util.Optional; +import java.util.regex.Pattern; import io.vertx.core.json.JsonObject; @@ -119,11 +120,11 @@ public final class CredentialsConstants extends RequestResponseApiConstants { /** * The regular expression to validate that the auth-id field supplied in credentials is legal. */ - public static final String AUTH_ID_VALUE_REGEX = "^[a-zA-Z0-9-=.]+$"; + public static final Pattern PATTERN_AUTH_ID_VALUE = Pattern.compile("^[a-zA-Z0-9-=.]+$"); /** * The regular expression to validate that the type field supplied in credentials is legal. */ - public static final String TYPE_VALUE_REGEX = "^[a-z0-9-]+$"; + public static final Pattern PATTERN_TYPE_VALUE = Pattern.compile("^[a-z0-9-]+$"); /** * Request actions that belong to the Credentials API. diff --git a/service-base/src/main/java/org/eclipse/hono/service/http/AbstractHttpEndpoint.java b/service-base/src/main/java/org/eclipse/hono/service/http/AbstractHttpEndpoint.java index c8d29e9a38..0b4706cf46 100644 --- a/service-base/src/main/java/org/eclipse/hono/service/http/AbstractHttpEndpoint.java +++ b/service-base/src/main/java/org/eclipse/hono/service/http/AbstractHttpEndpoint.java @@ -19,7 +19,6 @@ import java.util.Set; import java.util.function.Function; import java.util.function.Predicate; -import java.util.regex.Pattern; import org.eclipse.hono.client.ClientErrorException; import org.eclipse.hono.client.ServiceInvocationException; @@ -39,7 +38,6 @@ import io.vertx.core.http.HttpHeaders; import io.vertx.core.http.HttpMethod; import io.vertx.core.json.DecodeException; -import io.vertx.core.json.JsonObject; import io.vertx.ext.web.MIMEHeader; import io.vertx.ext.web.RoutingContext; import io.vertx.ext.web.handler.CorsHandler; @@ -70,8 +68,6 @@ public abstract class AbstractHttpEndpoint ex */ protected static final String KEY_RESOURCE_VERSION = "KEY_RESOURCE_VERSION"; - private static final Pattern PATTERN_ANY_STRING = Pattern.compile(".*"); - /** * The configuration properties for this endpoint. */ @@ -152,9 +148,14 @@ protected final void extractRequiredJsonPayload(final RoutingContext ctx) { } /** - * Check if the payload is not null and call \ - * {@link #extractRequiredJson(RoutingContext, Function)}} to extract it accordingly. + * Extracts JSON payload from a request body, if not empty. + *

+ * This method tries to de-serialize the content of the request body + * into a {@code JsonObject} and put it to the request context using key + * {@value #KEY_REQUEST_BODY}. *

+ * The request is failed with a 400 status code if de-serialization fails, for example + * because the content is not a valid JSON string. * * @param ctx The routing context to retrieve the JSON request body from. */ @@ -163,7 +164,6 @@ protected final void extractOptionalJsonPayload(final RoutingContext ctx) { if (ctx.getBody().length() != 0) { extractRequiredJson(ctx, RoutingContext::getBodyAsJson); } else { - ctx.put(KEY_REQUEST_BODY, new JsonObject()); ctx.next(); } } @@ -256,94 +256,6 @@ protected final Future getRequestParameter( return result.future(); } - /** - * Get request parameter value and check if it has been set. If it's not set, fail the request. - * - * @param paramName The name of the parameter to get. - * @param ctx The routing context of the request. - * @param span The active OpenTracing span for this operation. In case of the missing mandatory parameter, the error is logged and the span is finished. - * Otherwise, the parameter is set as a span tag. - * @return The value of the parameter if it's set or {@code null} otherwise. - * @throws NullPointerException If ctx or paramName are {@code null}. - * @deprecated Use {@link #getRequestIdParam(String, RoutingContext, Span, boolean)} instead. - */ - @Deprecated - protected final String getMandatoryIdRequestParam(final String paramName, final RoutingContext ctx, final Span span) { - return getRequestIdParam(paramName, ctx, span, false); - } - - /** - * Get request parameter value. Optionally, if parameter has not been set, fail the request. - * - * @param paramName The name of the parameter to get. - * @param ctx The routing context of the request. - * @param span The active OpenTracing span for this operation. In case of the missing mandatory parameter, the error is logged and the span is finished. - * Otherwise, the parameter is set as a span tag. - * @param optional Whether to check if parameter has been set or not. - * @return The value of the parameter if it's set or {@code null} otherwise. - * @throws NullPointerException If ctx or paramName are {@code null}. - * @deprecated Use {@link #getRequestIdParam(String, RoutingContext, Span, boolean)} instead. - */ - @Deprecated - protected final String getRequestIdParam(final String paramName, final RoutingContext ctx, final Span span, final boolean optional) { - - final String value = ctx.request().getParam(paramName); - - if (value != null) { - - final Pattern pattern; - final boolean matches; - switch (paramName) { - case PARAM_TENANT_ID: - pattern = config.getTenantIdPattern(); - matches = pattern.matcher(value).matches(); - break; - case PARAM_DEVICE_ID: - pattern = config.getDeviceIdPattern(); - matches = pattern.matcher(value).matches(); - break; - default: - pattern = PATTERN_ANY_STRING; - matches = true; - } - - if (matches) { - span.setTag(paramName, value); - return value; - } else { - final String msg = String.format("parameter [name: %s, value: %s] does not match allowed pattern: %s", - paramName, value, pattern.pattern()); - HttpUtils.badRequest(ctx, msg); - finishSpanWithError(span, HttpURLConnection.HTTP_BAD_REQUEST, msg); - return null; - } - - } else if (!optional) { - final String msg = String.format("Missing request parameter: %s", paramName); - HttpUtils.badRequest(ctx, msg); - finishSpanWithError(span, HttpURLConnection.HTTP_BAD_REQUEST, msg); - return null; - } else { - span.setTag(paramName, value); - return value; - } - } - - /** - * Finish the given span with the error code and logs the error message. - * - * @param span The nspan to finish. - * @param httpErrorCode The HTTP Error code to use. - * @param errorMessage A string containing a message describing the error. - * @deprecated Use {@link #failRequest(RoutingContext, Throwable, Span)} instead. - */ - @Deprecated - protected final void finishSpanWithError(final Span span, final int httpErrorCode, final String errorMessage) { - TracingHelper.logError(span, errorMessage); - Tags.HTTP_STATUS.set(span, httpErrorCode); - span.finish(); - } - /** * Fails a request with a given error. *

diff --git a/services/base-jdbc/src/test/java/org/eclipse/hono/service/base/jdbc/store/device/JsonStoreTest.java b/services/base-jdbc/src/test/java/org/eclipse/hono/service/base/jdbc/store/device/JsonStoreTest.java index ef9dc642aa..740d1936b1 100644 --- a/services/base-jdbc/src/test/java/org/eclipse/hono/service/base/jdbc/store/device/JsonStoreTest.java +++ b/services/base-jdbc/src/test/java/org/eclipse/hono/service/base/jdbc/store/device/JsonStoreTest.java @@ -36,8 +36,7 @@ private static PskCredential newPskCredential(final String authId) { final PskSecret psk1 = new PskSecret(); psk1.setKey("foo".getBytes(StandardCharsets.UTF_8)); - final PskCredential result = new PskCredential(); - result.setAuthId(authId); + final PskCredential result = new PskCredential(authId); result.setSecrets(Arrays.asList(psk1)); return result; diff --git a/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/AbstractDelegatingRegistryHttpEndpoint.java b/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/AbstractDelegatingRegistryHttpEndpoint.java index c4c6f73c58..a6d49b495f 100644 --- a/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/AbstractDelegatingRegistryHttpEndpoint.java +++ b/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/AbstractDelegatingRegistryHttpEndpoint.java @@ -14,7 +14,6 @@ package org.eclipse.hono.service.management; -import java.net.HttpURLConnection; import java.util.Objects; import java.util.Optional; import java.util.function.BiConsumer; @@ -28,12 +27,12 @@ import io.opentracing.Span; import io.opentracing.tag.Tags; -import io.vertx.core.Handler; import io.vertx.core.MultiMap; import io.vertx.core.Vertx; +import io.vertx.core.buffer.Buffer; import io.vertx.core.http.HttpHeaders; import io.vertx.core.http.HttpServerResponse; -import io.vertx.core.json.JsonObject; +import io.vertx.core.json.Json; import io.vertx.ext.web.RoutingContext; @@ -134,74 +133,17 @@ protected final void writeResponse(final RoutingContext ctx, final Result res customHandler.accept(response.headers(), status); } // once the body has been written, the response headers can no longer be changed - HttpUtils.setResponseBody(response, asJson(result.getPayload())); + HttpUtils.setResponseBody(response, asJson(result.getPayload()), HttpUtils.CONTENT_TYPE_JSON_UTF8); Tags.HTTP_STATUS.set(span, status); response.end(); } - private JsonObject asJson(final Object obj) { + private Buffer asJson(final Object obj) { try { - return JsonObject.mapFrom(obj); + return Json.encodeToBuffer(obj); } catch (final IllegalArgumentException e) { logger.debug("error serializing result object [type: {}] to JSON", obj.getClass().getName(), e); return null; } } - - /** - * Writes a response based on generic result. - *

- * The behavior is as follows: - *

    - *
  1. Set the status code on the response.
    2. - *
  2. If the status code represents an error condition (i.e. the code is >= 400), - * then the JSON object passed in the result is written to the response body.
    3. - *
  3. If the result is created (the code is = 201), the JSON object is written to the response body and the given custom handler is - * invoked (if not {@code null}).
    4. - *
  4. Sets the status of the tracing span and finishes it.
    5. - *
  5. Ends a response.
    6. - *
- * - * @param ctx The routing context of the request. - * @param result The generic result of the operation. - * @param customHandler An (optional) handler for post processing successful HTTP response, e.g. to set any additional HTTP - * headers. The handler must not write to response body. May be {@code null}. - * @param span The active OpenTracing span for this operation. The status of the response is logged and span is finished. - * @deprecated Use {@link #writeResponse(RoutingContext, Result, BiConsumer, Span)} instead. - */ - @Deprecated - protected final void writeResponse(final RoutingContext ctx, final Result result, final Handler customHandler, final Span span) { - final int status = result.getStatus(); - final HttpServerResponse response = ctx.response(); - response.setStatusCode(status); - if (status >= 400) { - HttpUtils.setResponseBody(response, JsonObject.mapFrom(result.getPayload())); - } else if (status == HttpURLConnection.HTTP_CREATED) { - if (customHandler != null) { - customHandler.handle(response); - } - HttpUtils.setResponseBody(response, JsonObject.mapFrom(result.getPayload())); - } - Tags.HTTP_STATUS.set(span, status); - span.finish(); - response.end(); - } - - /** - * Writes a response based on operation result (including the resource version). - *

- * Sets ETAG header and then calls {@link #writeResponse} - * - * @param ctx The routing context of the request. - * @param result The operation result of the operation. - * @param customHandler An (optional) handler for post processing successful HTTP response, e.g. to set any additional HTTP - * headers. The handler must not write to response body. May be {@code null}. - * @param span The active OpenTracing span for this operation. The status of the response is logged and span is finished. - * @deprecated Use {@link #writeResponse(RoutingContext, Result, BiConsumer, Span)} instead. - */ - @Deprecated - protected final void writeOperationResponse(final RoutingContext ctx, final OperationResult result, final Handler customHandler, final Span span) { - result.getResourceVersion().ifPresent(v -> ctx.response().putHeader(HttpHeaders.ETAG, v)); - writeResponse(ctx, result, customHandler, span); - } } diff --git a/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/credentials/CommonCredential.java b/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/credentials/CommonCredential.java index 24d81a830a..b164c61ff6 100644 --- a/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/credentials/CommonCredential.java +++ b/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/credentials/CommonCredential.java @@ -17,11 +17,13 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.function.Predicate; +import java.util.regex.Matcher; +import org.eclipse.hono.util.CredentialsConstants; import org.eclipse.hono.util.RegistryManagementConstants; import com.fasterxml.jackson.annotation.JsonIgnore; -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonTypeInfo; @@ -34,10 +36,23 @@ @JsonInclude(value = JsonInclude.Include.NON_NULL) @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "type", visible = true) @JsonTypeIdResolver(CredentialTypeResolver.class) -@JsonIgnoreProperties(ignoreUnknown = true) public abstract class CommonCredential { - @JsonProperty(RegistryManagementConstants.FIELD_AUTH_ID) + /** + * A predicate for matching authentication identifiers against the + * {@linkplain CredentialsConstants#PATTERN_AUTH_ID_VALUE default pattern}. + */ + protected static final Predicate AUTH_ID_VALIDATOR_DEFAULT = authId -> { + final Matcher matcher = CredentialsConstants.PATTERN_AUTH_ID_VALUE.matcher(authId); + if (matcher.matches()) { + return true; + } else { + throw new IllegalArgumentException("authentication identifier must match pattern " + + CredentialsConstants.PATTERN_AUTH_ID_VALUE.pattern()); + } + }; + + @JsonProperty(value = RegistryManagementConstants.FIELD_AUTH_ID) private String authId; @JsonProperty(RegistryManagementConstants.FIELD_ENABLED) private Boolean enabled; @@ -48,6 +63,22 @@ public abstract class CommonCredential { @JsonInclude(value = JsonInclude.Include.NON_EMPTY) private Map extensions = new HashMap<>(); + /** + * Creates a new credentials object for an authentication identifier. + * + * @param authId The authentication identifier. + * @throws NullPointerException if the auth ID is {@code null}. + * @throws IllegalArgumentException if auth ID does not match {@link CredentialsConstants#PATTERN_AUTH_ID_VALUE}. + */ + protected CommonCredential(final String authId) { + Objects.requireNonNull(authId); + if (getAuthIdValidator().test(authId)) { + this.authId = authId; + } else { + throw new IllegalArgumentException("validation of authentication identifier failed"); + } + } + /** * Get a list of secrets for this credential. * @@ -62,21 +93,43 @@ public abstract class CommonCredential { */ public abstract String getType(); - public String getAuthId() { - return authId; + /** + * Gets a predicate to use for validating authentication identifiers. + *

+ * The predicate is invoked by the constructor before setting the authId property. + * This default implementation returns a predicate that matches the identifier + * against the {@linkplain CredentialsConstants#PATTERN_AUTH_ID_VALUE default pattern}. + *

+ * The constructor will re-throw an {@code IllegalArgumentException}y thrown by the + * predicate's test method. This might be used to convey additional information + * about the failed validation. + *

+ * Subclasses should override this method in order to use other means of validation. + * + * @return The predicate. + */ + protected Predicate getAuthIdValidator() { + return AUTH_ID_VALIDATOR_DEFAULT; } /** - * Sets the authentication identifier that the device uses for authenticating to protocol adapters. + * Sets the authentication identifier used by the device. * - * @param authId The authentication identifier use for authentication. + * @param authId The identifier. * @return A reference to this for fluent use. + * @throws NullPointerException if ID is {@code null}. */ - public CommonCredential setAuthId(final String authId) { + @JsonIgnore + protected final CommonCredential setAuthId(final String authId) { + Objects.requireNonNull(authId); this.authId = authId; return this; } + public final String getAuthId() { + return authId; + } + @JsonIgnore public boolean isEnabled() { return Optional.ofNullable(enabled).orElse(true); diff --git a/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/credentials/DelegatingCredentialsManagementHttpEndpoint.java b/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/credentials/DelegatingCredentialsManagementHttpEndpoint.java index 863c439100..a7697517dc 100644 --- a/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/credentials/DelegatingCredentialsManagementHttpEndpoint.java +++ b/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/credentials/DelegatingCredentialsManagementHttpEndpoint.java @@ -20,21 +20,21 @@ import java.util.Optional; import java.util.stream.Collectors; +import org.eclipse.hono.client.ClientErrorException; import org.eclipse.hono.config.ServiceConfigProperties; -import org.eclipse.hono.service.http.HttpUtils; import org.eclipse.hono.service.http.TracingHandler; import org.eclipse.hono.service.management.AbstractDelegatingRegistryHttpEndpoint; import org.eclipse.hono.service.management.OperationResult; import org.eclipse.hono.tracing.TracingHelper; -import org.eclipse.hono.util.CredentialsConstants; import org.eclipse.hono.util.RegistryManagementConstants; import io.opentracing.Span; -import io.opentracing.tag.Tags; +import io.vertx.core.CompositeFuture; +import io.vertx.core.Future; +import io.vertx.core.Promise; import io.vertx.core.Vertx; -import io.vertx.core.http.HttpHeaders; import io.vertx.core.http.HttpMethod; -import io.vertx.core.http.HttpServerResponse; +import io.vertx.core.json.DecodeException; import io.vertx.core.json.JsonArray; import io.vertx.core.json.JsonObject; import io.vertx.ext.web.Router; @@ -101,163 +101,111 @@ public void addRoutes(final Router router) { router.put(pathWithTenantAndDeviceId).handler(this::updateCredentials); } - private void updateCredentials(final RoutingContext ctx) { + private void getCredentialsForDevice(final RoutingContext ctx) { final Span span = TracingHelper.buildServerChildSpan( tracer, TracingHandler.serverSpanContext(ctx), - SPAN_NAME_UPDATE_CREDENTIALS, + SPAN_NAME_GET_CREDENTIALS, getClass().getSimpleName() ).start(); - final JsonArray credentials = ctx.get(KEY_REQUEST_BODY); - - final String tenantId = getMandatoryIdRequestParam(PARAM_TENANT_ID, ctx, span); - final String deviceId = getMandatoryIdRequestParam(PARAM_DEVICE_ID, ctx, span); - final Optional resourceVersion = Optional.ofNullable(ctx.get(KEY_RESOURCE_VERSION)); - - final List commonCredentials; - try { - commonCredentials = decodeCredentials(credentials); - } catch (final IllegalArgumentException e) { - final String msg = "Error parsing credentials"; - logger.debug(msg); - TracingHelper.logError(span, msg); - Tags.HTTP_STATUS.set(span, HttpURLConnection.HTTP_BAD_REQUEST); - HttpUtils.badRequest(ctx, msg); - span.finish(); - return; - } - - logger.debug("updating credentials [tenant: {}, device-id: {}] - {}", tenantId, deviceId, credentials); - - getService().updateCredentials(tenantId, deviceId, commonCredentials, resourceVersion, span) - .onComplete(handler -> { - final OperationResult operationResult = handler.result(); - writeOperationResponse( - ctx, - operationResult, - null, - span); - }); + final Future tenantId = getRequestParameter(ctx, PARAM_TENANT_ID, getPredicate(config.getTenantIdPattern(), false)); + final Future deviceId = getRequestParameter(ctx, PARAM_DEVICE_ID, getPredicate(config.getDeviceIdPattern(), false)); + + CompositeFuture.all(tenantId, deviceId) + .compose(ok -> { + logger.debug("getting credentials [tenant: {}, device-id: {}]]", + tenantId.result(), deviceId.result()); + return getService().readCredentials(tenantId.result(), deviceId.result(), span); + }) + .map(operationResult -> { + if (operationResult.isOk()) { + // we need to map the payload from List to JsonArray because + // Jackson does not include the credential's type property in the + // JSON when serializing a plain java List object. + final JsonArray credentialsArray = operationResult.getPayload().stream() + .map(credentials -> JsonObject.mapFrom(credentials)) + .collect(JsonArray::new, JsonArray::add, JsonArray::addAll); + return OperationResult.ok( + operationResult.getStatus(), + credentialsArray, + operationResult.getCacheDirective(), + operationResult.getResourceVersion()); + } else { + return operationResult; + } + }) + .onSuccess(operationResult -> writeResponse(ctx, operationResult, span)) + .onFailure(t -> failRequest(ctx, t, span)) + .onComplete(s -> span.finish()); } - private void getCredentialsForDevice(final RoutingContext ctx) { + private void updateCredentials(final RoutingContext ctx) { final Span span = TracingHelper.buildServerChildSpan( tracer, TracingHandler.serverSpanContext(ctx), - SPAN_NAME_GET_CREDENTIALS, + SPAN_NAME_UPDATE_CREDENTIALS, getClass().getSimpleName() ).start(); - // mandatory params - final String tenantId = getMandatoryIdRequestParam(PARAM_TENANT_ID, ctx, span); - final String deviceId = getMandatoryIdRequestParam(PARAM_DEVICE_ID, ctx, span); - - final HttpServerResponse response = ctx.response(); - - logger.debug("getCredentialsForDevice [tenant: {}, device-id: {}]]", tenantId, deviceId); - - getService().readCredentials(tenantId, deviceId, span) - .onComplete(handler -> { - final OperationResult> operationResult = handler.result(); - final int status = operationResult.getStatus(); - response.setStatusCode(status); - switch (status) { - case HttpURLConnection.HTTP_OK: - final JsonArray credentialsArray = new JsonArray(); - for (final CommonCredential credential : operationResult.getPayload()) { - credentialsArray.add(JsonObject.mapFrom(credential)); - } - operationResult.getResourceVersion().ifPresent(v -> response.putHeader(HttpHeaders.ETAG, v)); - HttpUtils.setResponseBody(response, credentialsArray); - - // falls through intentionally - default: - Tags.HTTP_STATUS.set(span, status); - span.finish(); - response.end(); - } - }); + final Future tenantId = getRequestParameter(ctx, PARAM_TENANT_ID, getPredicate(config.getTenantIdPattern(), false)); + final Future deviceId = getRequestParameter(ctx, PARAM_DEVICE_ID, getPredicate(config.getDeviceIdPattern(), false)); + final Future> updatedCredentials = fromPayload(ctx); + + CompositeFuture.all(tenantId, deviceId, updatedCredentials) + .compose(ok -> { + logger.debug("updating {} credentials [tenant: {}, device-id: {}]", + updatedCredentials.result().size(), tenantId.result(), deviceId.result()); + final Optional resourceVersion = Optional.ofNullable(ctx.get(KEY_RESOURCE_VERSION)); + return getService().updateCredentials( + tenantId.result(), + deviceId.result(), + updatedCredentials.result(), + resourceVersion, + span); + }) + .onSuccess(operationResult -> writeResponse(ctx, operationResult, span)) + .onFailure(t -> failRequest(ctx, t, span)) + .onComplete(s -> span.finish()); } /** - * Decode a list of secrets from a JSON array. - *

- * This is a convenience method, decoding a list of secrets from a JSON array. + * Decodes a list of secrets from a request body. * - * @param objects The JSON array. + * @param ctx The request to retrieve the updated credentials from. * @return The list of decoded secrets. - * @throws NullPointerException in the case the {@code objects} parameter is {@code null}. + * @throws NullPointerException if context is {@code null}. * @throws IllegalArgumentException If a credentials object is invalid. */ - protected List decodeCredentials(final JsonArray objects) { - return objects - .stream() - .filter(JsonObject.class::isInstance) - .map(JsonObject.class::cast) - .map(this::decodeCredential) - .filter(Objects::nonNull) - .collect(Collectors.toList()); - } - - /** - * Decode a credential from a JSON object. - * - * @param object The object to device from. - * @return The decoded secret. Or {@code null} if the provided JSON object was {@code null}. - * @throws IllegalArgumentException If the credential object is invalid. - */ - protected CommonCredential decodeCredential(final JsonObject object) { - - if (object == null) { - return null; - } - - verifyRegex(object, CredentialsConstants.FIELD_TYPE, CredentialsConstants.TYPE_VALUE_REGEX); - verifyRegex(object, CredentialsConstants.FIELD_AUTH_ID, CredentialsConstants.AUTH_ID_VALUE_REGEX); - - final String type = object.getString(CredentialsConstants.FIELD_TYPE); - return decodeCredential(type, object); - } - - /** - * Verify a fields exists in a JSON object and match a regular expression. - * - * @throws IllegalArgumentException If field is not set or does not match the regular expression. - */ - private void verifyRegex(final JsonObject object, final String name, final String regexp) { - final String value = object.getString(name); - - if (value == null || value.isEmpty()) { - throw new IllegalArgumentException(String.format("'%s' field must be set", name)); - } + private static Future> fromPayload(final RoutingContext ctx) { + + Objects.requireNonNull(ctx); + final Promise> result = Promise.promise(); + Optional.ofNullable(ctx.get(KEY_REQUEST_BODY)) + .map(JsonArray.class::cast) + .ifPresentOrElse( + // deserialize & validate payload + array -> { + try { + result.complete(decodeCredentials(array)); + } catch (final IllegalArgumentException | DecodeException e) { + result.fail(new ClientErrorException(HttpURLConnection.HTTP_BAD_REQUEST, + "request body does not contain valid Credentials objects")); + } + }, + () -> result.fail(new ClientErrorException(HttpURLConnection.HTTP_BAD_REQUEST, + "request body does not contain JSON array"))); + return result.future(); - if (! value.matches(regexp)) { - throw new IllegalArgumentException(String.format("'%s' value : '%s' does not match allowed pattern: %s", - name, value, regexp)); - } } - /** - * Decode a credential, based on the provided type. - * - * @param type The type of the secret. Will never be {@code null}. - * @param object The JSON object to decode. Will never be {@code null}. - * @return The decoded secret. - * @throws IllegalArgumentException If the credential object is invalid. - */ - protected CommonCredential decodeCredential(final String type, final JsonObject object) { - switch (type) { - case RegistryManagementConstants.SECRETS_TYPE_HASHED_PASSWORD: - return object.mapTo(PasswordCredential.class); - case RegistryManagementConstants.SECRETS_TYPE_PRESHARED_KEY: - return object.mapTo(PskCredential.class); - case RegistryManagementConstants.SECRETS_TYPE_X509_CERT: - return object.mapTo(X509CertificateCredential.class); - default: - return object.mapTo(GenericCredential.class); - } + private static List decodeCredentials(final JsonArray array) { + return array.stream() + .filter(JsonObject.class::isInstance) + .map(JsonObject.class::cast) + .map(json -> json.mapTo(CommonCredential.class)) + .collect(Collectors.toList()); } } diff --git a/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/credentials/GenericCredential.java b/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/credentials/GenericCredential.java index 2fe846371e..e9c4f1184b 100644 --- a/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/credentials/GenericCredential.java +++ b/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/credentials/GenericCredential.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2019 Contributors to the Eclipse Foundation + * Copyright (c) 2019, 2020 Contributors to the Eclipse Foundation * * See the NOTICE file(s) distributed with this work for additional * information regarding copyright ownership. @@ -17,16 +17,25 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.function.Predicate; + +import org.eclipse.hono.util.CredentialsConstants; +import org.eclipse.hono.util.RegistryManagementConstants; import com.fasterxml.jackson.annotation.JsonAnyGetter; import com.fasterxml.jackson.annotation.JsonAnySetter; import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; /** * A generic credentials implementation. */ public class GenericCredential extends CommonCredential { + private static final Predicate typeValidator = CredentialsConstants.PATTERN_TYPE_VALUE.asMatchPredicate(); + + @JsonProperty(value = RegistryManagementConstants.FIELD_TYPE) private String type; @JsonAnySetter @@ -36,26 +45,34 @@ public class GenericCredential extends CommonCredential { private List secrets = new LinkedList<>(); /** - * Sets the credentials type name reflecting the type of authentication mechanism a device will use - * when authenticating to protocol adapters. - * - * @param type The credential name type to set. - * @return a reference to this for fluent use. + * Creates a new credentials object for a type and authentication identifier. * - * @see Standard Credential Types + * @param type The credentials type to set. + * @param authId The authentication identifier. + * @throws NullPointerException if type or auth ID are {@code null}. + * @throws IllegalArgumentException if type does not match {@link CredentialsConstants#PATTERN_TYPE_VALUE} + * or if auth ID does not match {@link CredentialsConstants#PATTERN_AUTH_ID_VALUE}. */ - public GenericCredential setType(final String type) { - this.type = type; - return this; + public GenericCredential( + @JsonProperty(value = RegistryManagementConstants.FIELD_TYPE, required = true) final String type, + @JsonProperty(value = RegistryManagementConstants.FIELD_AUTH_ID, required = true) final String authId) { + super(authId); + Objects.requireNonNull(type); + if (typeValidator.test(type)) { + this.type = type; + } else { + throw new IllegalArgumentException("type name must match pattern " + + CredentialsConstants.PATTERN_TYPE_VALUE.pattern()); + } } @Override - public String getType() { + public final String getType() { return type; } @Override - public List getSecrets() { + public final List getSecrets() { return this.secrets; } @@ -68,7 +85,7 @@ public List getSecrets() { * @return a reference to this for fluent use. * @throws IllegalArgumentException if the list of secrets is empty. */ - public GenericCredential setSecrets(final List secrets) { + public final GenericCredential setSecrets(final List secrets) { if (secrets != null && secrets.isEmpty()) { throw new IllegalArgumentException("Secrets cannot be empty"); } @@ -82,13 +99,13 @@ public GenericCredential setSecrets(final List secrets) { * @param additionalProperties The additional properties for this credential. * @return a reference to this for fluent use. */ - public GenericCredential setAdditionalProperties(final Map additionalProperties) { + public final GenericCredential setAdditionalProperties(final Map additionalProperties) { this.additionalProperties = additionalProperties; return this; } @JsonAnyGetter - public Map getAdditionalProperties() { + public final Map getAdditionalProperties() { return this.additionalProperties; } diff --git a/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/credentials/PasswordCredential.java b/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/credentials/PasswordCredential.java index ee29375a8d..28e5aaeda5 100644 --- a/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/credentials/PasswordCredential.java +++ b/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/credentials/PasswordCredential.java @@ -36,6 +36,17 @@ public class PasswordCredential extends CommonCredential { @JsonInclude(value = JsonInclude.Include.NON_EMPTY) private List secrets = new LinkedList<>(); + /** + * Creates a new credentials object for an authentication identifier. + * + * @param authId The authentication identifier. + * @throws NullPointerException if the auth ID is {@code null}. + * @throws IllegalArgumentException if auth ID does not match {@link org.eclipse.hono.util.CredentialsConstants#PATTERN_AUTH_ID_VALUE}. + */ + public PasswordCredential(@JsonProperty(value = RegistryManagementConstants.FIELD_AUTH_ID, required = true) final String authId) { + super(authId); + } + /** * {@inheritDoc} */ diff --git a/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/credentials/PskCredential.java b/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/credentials/PskCredential.java index d32f85b116..9ddb4295a3 100644 --- a/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/credentials/PskCredential.java +++ b/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/credentials/PskCredential.java @@ -32,10 +32,21 @@ public class PskCredential extends CommonCredential { static final String TYPE = RegistryManagementConstants.SECRETS_TYPE_PRESHARED_KEY; - @JsonProperty + @JsonProperty(value = RegistryManagementConstants.FIELD_SECRETS) @JsonInclude(value = JsonInclude.Include.NON_EMPTY) private List secrets = new LinkedList<>(); + /** + * Creates a new credentials object for an authentication identifier. + * + * @param authId The authentication identifier. + * @throws NullPointerException if the auth ID is {@code null}. + * @throws IllegalArgumentException if auth ID does not match {@link org.eclipse.hono.util.CredentialsConstants#PATTERN_AUTH_ID_VALUE}. + */ + public PskCredential(@JsonProperty(value = RegistryManagementConstants.FIELD_AUTH_ID, required = true) final String authId) { + super(authId); + } + /** * {@inheritDoc} */ @@ -46,7 +57,7 @@ public final String getType() { } @Override - public List getSecrets() { + public final List getSecrets() { return secrets; } @@ -59,7 +70,7 @@ public List getSecrets() { * @return a reference to this for fluent use. * @throws IllegalArgumentException if the list of secrets is empty. */ - public PskCredential setSecrets(final List secrets) { + public final PskCredential setSecrets(final List secrets) { if (secrets != null && secrets.isEmpty()) { throw new IllegalArgumentException("secrets cannot be empty"); } diff --git a/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/credentials/X509CertificateCredential.java b/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/credentials/X509CertificateCredential.java index 10a1e96257..752fc9a37f 100644 --- a/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/credentials/X509CertificateCredential.java +++ b/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/credentials/X509CertificateCredential.java @@ -14,6 +14,9 @@ import java.util.LinkedList; import java.util.List; +import java.util.function.Predicate; + +import javax.security.auth.x500.X500Principal; import org.eclipse.hono.util.RegistryManagementConstants; @@ -37,6 +40,30 @@ public class X509CertificateCredential extends CommonCredential { @JsonInclude(value = JsonInclude.Include.NON_EMPTY) private List secrets = new LinkedList<>(); + /** + * Creates a new credentials object for a an X.500 Distinguished Name. + *

+ * The given distinguished name will be normalized to RFC 2253 format. + * + * @param distinguishedName The DN to use as the authentication identifier. + * @throws NullPointerException if the DN is {@code null}. + * @throws IllegalArgumentException if the given string is not a valid X.500 distinguished name. + */ + public X509CertificateCredential(@JsonProperty(value = RegistryManagementConstants.FIELD_AUTH_ID, required = true) final String distinguishedName) { + super(new X500Principal(distinguishedName).getName(X500Principal.RFC2253)); + } + + /** + * {@inheritDoc} + */ + @Override + protected Predicate getAuthIdValidator() { + return authId -> { + final X500Principal distinguishedName = new X500Principal(authId); + return distinguishedName.getName(X500Principal.RFC2253).equals(authId); + }; + } + /** * {@inheritDoc} */ @@ -47,7 +74,7 @@ public final String getType() { } @Override - public List getSecrets() { + public final List getSecrets() { return secrets; } @@ -60,7 +87,7 @@ public List getSecrets() { * @return a reference to this for fluent use. * @throws IllegalArgumentException if the list of secrets is empty. */ - public X509CertificateCredential setSecrets(final List secrets) { + public final X509CertificateCredential setSecrets(final List secrets) { if (secrets != null && secrets.isEmpty()) { throw new IllegalArgumentException("secrets cannot be empty"); } diff --git a/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/device/AutoProvisioningEnabledDeviceBackend.java b/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/device/AutoProvisioningEnabledDeviceBackend.java index 1a705ec005..2074f41bf3 100644 --- a/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/device/AutoProvisioningEnabledDeviceBackend.java +++ b/services/device-registry-base/src/main/java/org/eclipse/hono/service/management/device/AutoProvisioningEnabledDeviceBackend.java @@ -76,10 +76,10 @@ default Future> provisionDevice( } // 2. set the certificate credential - final X509CertificateCredential certCredential = new X509CertificateCredential() + final String authId = clientCertificate.getSubjectX500Principal().getName(X500Principal.RFC2253); + final X509CertificateCredential certCredential = new X509CertificateCredential(authId) .setSecrets(List.of(new X509CertificateSecret())); - certCredential.setEnabled(true).setComment(comment) - .setAuthId(clientCertificate.getSubjectX500Principal().getName(X500Principal.RFC2253)); + certCredential.setEnabled(true).setComment(comment); final String deviceId = r.getPayload().getId(); diff --git a/services/device-registry-base/src/test/java/org/eclipse/hono/service/credentials/AbstractCredentialsServiceTest.java b/services/device-registry-base/src/test/java/org/eclipse/hono/service/credentials/AbstractCredentialsServiceTest.java index b290920014..97f80db249 100644 --- a/services/device-registry-base/src/test/java/org/eclipse/hono/service/credentials/AbstractCredentialsServiceTest.java +++ b/services/device-registry-base/src/test/java/org/eclipse/hono/service/credentials/AbstractCredentialsServiceTest.java @@ -164,8 +164,7 @@ public void testGetCredentialsSucceedsForExistingDevice(final VertxTestContext c * @return The fully populated secret. */ public static PskCredential createPSKCredential(final String authId, final String psk) { - final PskCredential p = new PskCredential(); - p.setAuthId(authId); + final PskCredential p = new PskCredential(authId); final PskSecret s = new PskSecret(); s.setKey(psk.getBytes()); @@ -185,8 +184,7 @@ public static PskCredential createPSKCredential(final String authId, final Strin */ public static PasswordCredential createPasswordCredential(final String authId, final String password, final OptionalInt maxBcryptIterations) { - final PasswordCredential p = new PasswordCredential(); - p.setAuthId(authId); + final PasswordCredential p = new PasswordCredential(authId); p.setSecrets(Collections.singletonList(createPasswordSecret(password, maxBcryptIterations))); @@ -201,8 +199,7 @@ public static PasswordCredential createPasswordCredential(final String authId, f * @return The fully populated credential. */ public static PasswordCredential createPlainPasswordCredential(final String authId, final String password) { - final PasswordCredential p = new PasswordCredential(); - p.setAuthId(authId); + final PasswordCredential p = new PasswordCredential(authId); final PasswordSecret secret = new PasswordSecret(); secret.setPasswordPlain(password); @@ -892,8 +889,7 @@ public void testSecretsWithMissingIDsAreRemoved(final VertxTestContext ctx) { final String deviceId = UUID.randomUUID().toString(); final String authId = UUID.randomUUID().toString(); - final PasswordCredential credential = new PasswordCredential(); - credential.setAuthId(authId); + final PasswordCredential credential = new PasswordCredential(authId); final PasswordSecret sec1 = new PasswordSecret().setPasswordPlain("bar"); final PasswordSecret sec2 = new PasswordSecret().setPasswordPlain("foo"); @@ -943,8 +939,7 @@ public void testSecretsWithMissingIDsAreRemoved(final VertxTestContext ctx) { phase2.future().onComplete(ctx.succeeding(n -> { // create a credential object with only one of the ID. - final PasswordCredential credentialWithOnlyId = new PasswordCredential(); - credentialWithOnlyId.setAuthId(authId); + final PasswordCredential credentialWithOnlyId = new PasswordCredential(authId); final PasswordSecret secretWithOnlyId = new PasswordSecret(); secretWithOnlyId.setId(secretIDs.get(0)); @@ -1137,8 +1132,7 @@ public void testSecretMetadataUpdateDoesntChangeSecretID(final VertxTestContext phase2.future().onComplete(ctx.succeeding(n -> { // Add some metadata to the secret - final PasswordCredential credentialWithMetadataUpdate = new PasswordCredential(); - credentialWithMetadataUpdate.setAuthId(authId); + final PasswordCredential credentialWithMetadataUpdate = new PasswordCredential(authId); final PasswordSecret secretWithOnlyIdAndMetadata = new PasswordSecret(); secretWithOnlyIdAndMetadata.setId(secretIDs.get(0)); @@ -1248,8 +1242,7 @@ public void testSecretMetadataDeletion(final VertxTestContext ctx) { phase2.future().onComplete(ctx.succeeding(n -> { // Add some other metadata to the secret - final PasswordCredential credentialWithMetadataUpdate = new PasswordCredential(); - credentialWithMetadataUpdate.setAuthId(authId); + final PasswordCredential credentialWithMetadataUpdate = new PasswordCredential(authId); final PasswordSecret secretWithOnlyIdAndMetadata = new PasswordSecret(); secretWithOnlyIdAndMetadata.setId(secretIDs.get(0)); diff --git a/services/device-registry-base/src/test/java/org/eclipse/hono/service/management/credentials/CredentialsTest.java b/services/device-registry-base/src/test/java/org/eclipse/hono/service/management/credentials/CredentialsTest.java index 34801f2324..ad3d8cf50b 100644 --- a/services/device-registry-base/src/test/java/org/eclipse/hono/service/management/credentials/CredentialsTest.java +++ b/services/device-registry-base/src/test/java/org/eclipse/hono/service/management/credentials/CredentialsTest.java @@ -35,6 +35,7 @@ import org.eclipse.hono.util.RegistryManagementConstants; import org.junit.jupiter.api.Test; +import io.vertx.core.json.DecodeException; import io.vertx.core.json.Json; import io.vertx.core.json.JsonArray; import io.vertx.core.json.JsonObject; @@ -60,8 +61,7 @@ public void testEncodePasswordCredential() { secret.setHashFunction(RegistryManagementConstants.HASH_FUNCTION_SHA256); - final PasswordCredential credential = new PasswordCredential(); - credential.setAuthId("foo"); + final PasswordCredential credential = new PasswordCredential("foo"); credential.setComment("setec astronomy"); credential.setSecrets(Collections.singletonList(secret)); @@ -85,9 +85,8 @@ public void testEncodePasswordCredential() { */ @Test public void testEncodePskCredential() { - final PskCredential credential = new PskCredential(); - credential.setAuthId("psk-device"); - credential.setSecrets(List.of(new PskSecret().setKey(new byte[] { 0x00, 0x01 }))); + final PskCredential credential = new PskCredential("foo"); + credential.setSecrets(List.of(new PskSecret().setKey(new byte[] { 0x00, 0x01 }))); final JsonObject json = JsonObject.mapFrom(credential); assertNotNull(json); @@ -103,13 +102,14 @@ public void testEncodePskCredential() { */ @Test public void testEncodeX509Credential() { - final X509CertificateCredential credential = new X509CertificateCredential(); + final X509CertificateCredential credential = new X509CertificateCredential("CN=foo, O=bar"); final JsonObject json = JsonObject.mapFrom(credential); assertNotNull(json); assertNull(json.getJsonArray(RegistryManagementConstants.FIELD_SECRETS)); assertEquals("x509-cert", json.getString(RegistryManagementConstants.FIELD_TYPE)); + assertEquals("CN=foo,O=bar", json.getString(RegistryManagementConstants.FIELD_AUTH_ID)); } /** @@ -117,14 +117,13 @@ public void testEncodeX509Credential() { */ @Test public void testEncodeGenericCredential() { - final GenericCredential credential = new GenericCredential(); - credential.setType("custom"); + final GenericCredential credential = new GenericCredential("custom-type", "foo"); final JsonObject json = JsonObject.mapFrom(credential); assertThat(json).isNotNull(); assertThat(json.getJsonArray(RegistryManagementConstants.FIELD_SECRETS)).isNull(); - assertThat(json.getString(RegistryManagementConstants.FIELD_TYPE)).isEqualTo("custom"); + assertThat(json.getString(RegistryManagementConstants.FIELD_TYPE)).isEqualTo("custom-type"); final CommonCredential decodedCredential = json.mapTo(CommonCredential.class); assertThat(decodedCredential).isInstanceOf(GenericCredential.class); @@ -146,8 +145,7 @@ public void testEncodeArray() { protected void testEncodeMany(final Function, Object> provider) { final List credentials = new ArrayList<>(); - final PskCredential credential = new PskCredential(); - credential.setAuthId(RegistryManagementConstants.FIELD_AUTH_ID); + final PskCredential credential = new PskCredential("device"); final PskSecret secret = new PskSecret(); secret.setKey("foo".getBytes(StandardCharsets.UTF_8)); credential.setSecrets(Collections.singletonList(secret)); @@ -164,6 +162,25 @@ protected void testEncodeMany(final Function, Object> pro } } + /** + * Decode credentials with unknown property fails. + */ + @Test + public void testDecodeFailsForUnknownProperties() { + assertThatThrownBy(() -> Json.decodeValue( + "{\"type\": \"psk\", \"auth-id\": \"device1\", \"unexpected\": \"property\"}", + CommonCredential.class)) + .isInstanceOf(DecodeException.class); + assertThatThrownBy(() -> Json.decodeValue( + "{\"type\": \"hashed-password\", \"auth-id\": \"device1\", \"unexpected\": \"property\"}", + CommonCredential.class)) + .isInstanceOf(DecodeException.class); + assertThatThrownBy(() -> Json.decodeValue( + "{\"type\": \"x509-cert\", \"auth-id\": \"CN=foo\", \"unexpected\": \"property\"}", + CommonCredential.class)) + .isInstanceOf(DecodeException.class); + } + /** * Test the decoding of a Json Object to a simple password credential. */ @@ -208,7 +225,7 @@ public void testDecodePasswordCredential() { @Test public void testEncodeDecode1() { - final String authId = "abcd+#:,%\""; + final String authId = "abcd-=."; final Instant notAfter = Instant.parse("1992-09-11T11:38:00.123456Z"); // create credentials to encode @@ -218,20 +235,13 @@ public void testEncodeDecode1() { secret.setSalt("abc"); secret.setNotAfter(notAfter); - final PasswordCredential cred = new PasswordCredential(); - cred.setAuthId(authId); + final PasswordCredential cred = new PasswordCredential(authId); cred.setSecrets(Arrays.asList(secret)); // encode final String encode = Json.encode(new CommonCredential[] { cred }); - // Test for the exact format - - assertEquals( - "[{\"type\":\"hashed-password\",\"secrets\":[{\"not-after\":\"1992-09-11T11:38:00Z\",\"pwd-hash\":\"setec astronomy\",\"salt\":\"abc\"}],\"auth-id\":\"abcd+#:,%\\\"\"}]", - encode); - // now decode final CommonCredential[] decode = Json.decodeValue(encode, CommonCredential[].class); @@ -254,8 +264,8 @@ public void testEncodeDecode1() { @Test public void testMergeFailsForDifferentType() { - final PasswordCredential pwdCredentials = new PasswordCredential(); - assertThatThrownBy(() -> pwdCredentials.merge(new PskCredential())) + final PasswordCredential pwdCredentials = new PasswordCredential("foo"); + assertThatThrownBy(() -> pwdCredentials.merge(new PskCredential("bar"))) .isInstanceOf(IllegalArgumentException.class); } @@ -268,12 +278,12 @@ public void testMergeFailsForNonMatchingSecretId() { final PasswordSecret existingSecret = spy(PasswordSecret.class); existingSecret.setId("two"); - final PasswordCredential existingCredentials = new PasswordCredential(); + final PasswordCredential existingCredentials = new PasswordCredential("foo"); existingCredentials.setSecrets(List.of(existingSecret)); final PasswordSecret newSecret = spy(PasswordSecret.class); newSecret.setId("one"); - final PasswordCredential newCredentials = new PasswordCredential(); + final PasswordCredential newCredentials = new PasswordCredential("foo"); newCredentials.setSecrets(List.of(newSecret)); assertThatThrownBy(() -> newCredentials.merge(existingCredentials)) @@ -291,13 +301,13 @@ public void testMergeSucceedsForMatchingSecretIds() { final PskSecret existingSecret = spy(PskSecret.class); existingSecret.setId("one"); - final PskCredential existingCredentials = new PskCredential(); + final PskCredential existingCredentials = new PskCredential("foo"); existingCredentials.setSecrets(List.of(existingSecret)); final PskSecret updatedSecret = spy(PskSecret.class); updatedSecret.setId("one"); final PskSecret newSecret = spy(PskSecret.class); - final PskCredential updatedCredentials = new PskCredential(); + final PskCredential updatedCredentials = new PskCredential("foo"); updatedCredentials.setSecrets(List.of(updatedSecret, newSecret)); updatedCredentials.merge(existingCredentials); @@ -306,4 +316,82 @@ public void testMergeSucceedsForMatchingSecretIds() { verify(newSecret, never()).merge(any(CommonSecret.class)); assertThat(updatedCredentials.getSecrets()).hasSize(2); } + + /** + * Verifies that a credentials object requires the authentication identifier to match + * the auth id regex. + */ + @Test + public void testSetAuthIdFailsForIllegalIdentifier() { + assertThatThrownBy(() -> new PskCredential("_illegal")) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> new PskCredential("#illegal")) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> new PskCredential("")) + .isInstanceOf(IllegalArgumentException.class); + } + + /** + * Verifies that decoding of a JSON object to a PSK credential + * fails if the authentication identifier does not match the auth ID regex. + */ + @Test + public void testDecodePskCredentialFailsForIllegalAuthId() { + final JsonObject jsonCredential = new JsonObject() + .put(RegistryManagementConstants.FIELD_TYPE, RegistryManagementConstants.SECRETS_TYPE_PRESHARED_KEY) + .put(RegistryManagementConstants.FIELD_AUTH_ID, "#illegal"); + assertThatThrownBy(() -> jsonCredential.mapTo(PskCredential.class)) + .isInstanceOf(IllegalArgumentException.class); + } + + /** + * Verifies that decoding of a JSON object to a PSK credential + * fails if the JSON object does not contain an auth-id property. + */ + @Test + public void testDecodePskCredentialFailsForMissingAuthId() { + final JsonObject jsonCredential = new JsonObject() + .put(RegistryManagementConstants.FIELD_TYPE, RegistryManagementConstants.SECRETS_TYPE_PRESHARED_KEY); + assertThatThrownBy(() -> jsonCredential.mapTo(PskCredential.class)) + .isInstanceOf(IllegalArgumentException.class); + } + + /** + * Verifies that a generic credentials object requires the type to match + * the type name regex. + */ + @Test + public void testSetTypeFailsForIllegalName() { + assertThatThrownBy(() -> new GenericCredential("_illegal", "device")) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> new GenericCredential("#illegal", "device")) + .isInstanceOf(IllegalArgumentException.class); + assertThatThrownBy(() -> new GenericCredential("", "device")) + .isInstanceOf(IllegalArgumentException.class); + } + + /** + * Verifies that decoding of a JSON object to a generic credentials object + * fails if the type does not match the type name regex. + */ + @Test + public void testDecodeGenericCredentialFailsForIllegalType() { + final JsonObject jsonCredential = new JsonObject() + .put(RegistryManagementConstants.FIELD_TYPE, "#illegal") + .put(RegistryManagementConstants.FIELD_AUTH_ID, "device1"); + assertThatThrownBy(() -> jsonCredential.mapTo(GenericCredential.class)) + .isInstanceOf(IllegalArgumentException.class); + } + + /** + * Verifies that decoding of a JSON object to a credentials object + * fails if the JSON does not contain a type property. + */ + @Test + public void testDecodeCredentialFailsForMissingType() { + final JsonObject jsonCredential = new JsonObject() + .put(RegistryManagementConstants.FIELD_AUTH_ID, "device1"); + assertThatThrownBy(() -> jsonCredential.mapTo(CommonCredential.class)) + .isInstanceOf(IllegalArgumentException.class); + } } diff --git a/services/device-registry-file/src/test/java/org/eclipse/hono/deviceregistry/file/FileBasedCredentialsServiceTest.java b/services/device-registry-file/src/test/java/org/eclipse/hono/deviceregistry/file/FileBasedCredentialsServiceTest.java index 59cf66b5b5..39046c534c 100644 --- a/services/device-registry-file/src/test/java/org/eclipse/hono/deviceregistry/file/FileBasedCredentialsServiceTest.java +++ b/services/device-registry-file/src/test/java/org/eclipse/hono/deviceregistry/file/FileBasedCredentialsServiceTest.java @@ -370,8 +370,7 @@ public void testLoadCredentialsCanReadOutputOfSaveToFile(final VertxTestContext when(fileSystem.existsBlocking(credentialsConfig.getFilename())).thenReturn(Boolean.TRUE); // 4700 - final PasswordCredential passwordCredential = new PasswordCredential(); - passwordCredential.setAuthId("bumlux"); + final PasswordCredential passwordCredential = new PasswordCredential("bumlux"); final PasswordSecret hashedPassword = new PasswordSecret(); hashedPassword.setPasswordHash("$2a$10$UK9lmSMlYmeXqABkTrDRsu1nlZRnAmGnBdPIWZoDajtjyxX18Dry."); @@ -379,8 +378,7 @@ public void testLoadCredentialsCanReadOutputOfSaveToFile(final VertxTestContext passwordCredential.setSecrets(Collections.singletonList(hashedPassword)); // 4711RegistryManagementConstants.FIELD_ID - final PskCredential pskCredential = new PskCredential(); - pskCredential.setAuthId("sensor1"); + final PskCredential pskCredential = new PskCredential("sensor1"); final PskSecret pskSecret = new PskSecret(); pskSecret.setKey("sharedkey".getBytes(StandardCharsets.UTF_8)); @@ -525,8 +523,7 @@ public void testSetCredentialsWithUnauthorisedHashingAlgorithmFails(final VertxT credentialsConfig.setHashAlgorithmsWhitelist(whitelist); // 4700 - final PasswordCredential passwordCredential = new PasswordCredential(); - passwordCredential.setAuthId("bumlux"); + final PasswordCredential passwordCredential = new PasswordCredential("bumlux"); final PasswordSecret hashedPassword = new PasswordSecret(); hashedPassword.setPasswordHash("$2a$10$UK9lmSMlYmeXqABkTrDRsu1nlZRnAmGnBdPIWZoDajtjyxX18Dry."); @@ -564,8 +561,7 @@ private void testGetCredentialsWithClientContext( final PskSecret pskSecret = new PskSecret(); pskSecret.setKey("sharedkey".getBytes(StandardCharsets.UTF_8)); - final PskCredential pskCredential = new PskCredential(); - pskCredential.setAuthId(authId); + final PskCredential pskCredential = new PskCredential(authId); pskCredential.setExtensions(Map.of("property-to-match", expectedContextValue)); pskCredential.setSecrets(Collections.singletonList(pskSecret)); diff --git a/tests/src/test/java/org/eclipse/hono/tests/DeviceRegistryHttpClient.java b/tests/src/test/java/org/eclipse/hono/tests/DeviceRegistryHttpClient.java index 9b6c25bd13..cf010c8de9 100644 --- a/tests/src/test/java/org/eclipse/hono/tests/DeviceRegistryHttpClient.java +++ b/tests/src/test/java/org/eclipse/hono/tests/DeviceRegistryHttpClient.java @@ -687,7 +687,7 @@ public Future updateCredentials(final String tenantId, final String de * Otherwise the future will fail with a {@link org.eclipse.hono.client.ServiceInvocationException}. * @throws NullPointerException if the tenant is {@code null}. */ - public Future updateCredentialsWithVersion( + public Future updateCredentialsWithVersion( final String tenantId, final String deviceId, final Collection credentialsSpec, @@ -704,9 +704,7 @@ public Future updateCredentialsWithVersion( // encode array not list, workaround for vert.x issue final var payload = Json.encodeToBuffer(credentialsSpec.toArray(CommonCredential[]::new)); - return httpClient - .update(uri, payload, headers, status -> status == expectedStatusCode, true) - .compose(ok -> Future.succeededFuture()); + return httpClient.update(uri, payload, headers, status -> status == expectedStatusCode, true); } /** @@ -904,8 +902,8 @@ public Future addDeviceForTenant(final String tenantId, final Tenant tenan .compose(ok -> registerDevice(tenantId, deviceId)) .compose(ok -> { - final X509CertificateCredential credential = new X509CertificateCredential(); - credential.setAuthId(deviceCert.getSubjectDN().getName()); + final String authId = deviceCert.getSubjectDN().getName(); + final X509CertificateCredential credential = new X509CertificateCredential(authId); credential.getSecrets().add(new X509CertificateSecret()); return addCredentials(tenantId, deviceId, Collections.singleton(credential)); @@ -961,8 +959,7 @@ public Future addPskDeviceForTenant( Objects.requireNonNull(deviceData); Objects.requireNonNull(key); - final PskCredential credential = new PskCredential(); - credential.setAuthId(deviceId); + final PskCredential credential = new PskCredential(deviceId); final PskSecret secret = new PskSecret(); secret.setKey(key.getBytes(StandardCharsets.UTF_8)); @@ -988,8 +985,7 @@ public Future addPskDeviceForTenant( */ public Future addPskDeviceToTenant(final String tenantId, final String deviceId, final String key) { - final PskCredential credential = new PskCredential(); - credential.setAuthId(deviceId); + final PskCredential credential = new PskCredential(deviceId); final PskSecret secret = new PskSecret(); secret.setKey(key.getBytes(StandardCharsets.UTF_8)); diff --git a/tests/src/test/java/org/eclipse/hono/tests/mqtt/MqttConnectionIT.java b/tests/src/test/java/org/eclipse/hono/tests/mqtt/MqttConnectionIT.java index 93f496c1e8..4721acd9b5 100644 --- a/tests/src/test/java/org/eclipse/hono/tests/mqtt/MqttConnectionIT.java +++ b/tests/src/test/java/org/eclipse/hono/tests/mqtt/MqttConnectionIT.java @@ -237,8 +237,8 @@ public void testConnectX509FailsForUnknownSubjectDN(final VertxTestContext ctx) return helper.registry.addTenant(tenantId, tenant); }).compose(ok -> helper.registry.registerDevice(tenantId, deviceId)) .compose(ok -> { - final X509CertificateCredential credential = new X509CertificateCredential(); - credential.setAuthId(new X500Principal("CN=4711").getName(X500Principal.RFC2253)); + final String authId = new X500Principal("CN=4711").getName(X500Principal.RFC2253); + final X509CertificateCredential credential = new X509CertificateCredential(authId); credential.getSecrets().add(new X509CertificateSecret()); return helper.registry.addCredentials(tenantId, deviceId, Collections.singleton(credential)); }) diff --git a/tests/src/test/java/org/eclipse/hono/tests/registry/CredentialsApiTests.java b/tests/src/test/java/org/eclipse/hono/tests/registry/CredentialsApiTests.java index efcbd90013..e329d8b5d0 100644 --- a/tests/src/test/java/org/eclipse/hono/tests/registry/CredentialsApiTests.java +++ b/tests/src/test/java/org/eclipse/hono/tests/registry/CredentialsApiTests.java @@ -299,8 +299,7 @@ private CommonCredential getRandomHashedPasswordCredential(final String authId) final var secret2 = AbstractCredentialsServiceTest.createPasswordSecret("hono-password", OptionalInt.of(IntegrationTestSupport.MAX_BCRYPT_ITERATIONS)); - final var credential = new PasswordCredential(); - credential.setAuthId(authId); + final var credential = new PasswordCredential(authId); credential.setSecrets(Arrays.asList(secret1, secret2)); return credential; diff --git a/tests/src/test/java/org/eclipse/hono/tests/registry/CredentialsManagementIT.java b/tests/src/test/java/org/eclipse/hono/tests/registry/CredentialsManagementIT.java index 2543ee9b57..e770c3ce11 100644 --- a/tests/src/test/java/org/eclipse/hono/tests/registry/CredentialsManagementIT.java +++ b/tests/src/test/java/org/eclipse/hono/tests/registry/CredentialsManagementIT.java @@ -17,11 +17,13 @@ import java.net.HttpURLConnection; import java.nio.charset.StandardCharsets; import java.util.ArrayList; -import java.util.Collections; import java.util.List; +import java.util.Optional; import java.util.UUID; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import org.eclipse.hono.service.http.HttpUtils; import org.eclipse.hono.service.management.credentials.CommonCredential; import org.eclipse.hono.service.management.credentials.GenericCredential; import org.eclipse.hono.service.management.credentials.GenericSecret; @@ -32,7 +34,6 @@ import org.eclipse.hono.tests.CrudHttpClient; import org.eclipse.hono.tests.DeviceRegistryHttpClient; import org.eclipse.hono.tests.IntegrationTestSupport; -import org.eclipse.hono.util.Constants; import org.eclipse.hono.util.CredentialsConstants; import org.eclipse.hono.util.CredentialsObject; import org.eclipse.hono.util.RegistryManagementConstants; @@ -48,6 +49,7 @@ import org.slf4j.LoggerFactory; import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; +import io.vertx.core.MultiMap; import io.vertx.core.Vertx; import io.vertx.core.http.HttpHeaders; import io.vertx.core.json.Json; @@ -66,13 +68,12 @@ public class CredentialsManagementIT { private static final Logger LOG = LoggerFactory.getLogger(CredentialsManagementIT.class); - private static final String HTTP_HEADER_ETAG = HttpHeaders.ETAG.toString(); - - private static final String TENANT = Constants.DEFAULT_TENANT; - private static final String TEST_AUTH_ID = "sensor20"; - private static final Vertx vertx = Vertx.vertx(); + private static final String PREFIX_AUTH_ID = "sensor20"; + private static final Vertx VERTX = Vertx.vertx(); private static final String ORIG_BCRYPT_PWD; + + private static IntegrationTestSupport helper; private static DeviceRegistryHttpClient registry; static { @@ -80,8 +81,10 @@ public class CredentialsManagementIT { ORIG_BCRYPT_PWD = encoder.encode("thePassword"); } + private String tenantId; private String deviceId; private String authId; + private AtomicReference resourceVersion = new AtomicReference<>(); private PasswordCredential hashedPasswordCredential; private PskCredential pskCredentials; @@ -91,11 +94,9 @@ public class CredentialsManagementIT { @BeforeAll public static void setUpClient() { - registry = new DeviceRegistryHttpClient( - vertx, - IntegrationTestSupport.HONO_DEVICEREGISTRY_HOST, - IntegrationTestSupport.HONO_DEVICEREGISTRY_HTTP_PORT); - + helper = new IntegrationTestSupport(VERTX); + helper.initRegistryClient(); + registry = helper.registry; } /** @@ -108,12 +109,13 @@ public static void setUpClient() { public void setUp(final VertxTestContext ctx, final TestInfo testInfo) { LOG.info("running test: {}", testInfo.getDisplayName()); - deviceId = UUID.randomUUID().toString(); - authId = getRandomAuthId(TEST_AUTH_ID); + tenantId = helper.getRandomTenantId(); + deviceId = helper.getRandomDeviceId(tenantId); + authId = getRandomAuthId(PREFIX_AUTH_ID); hashedPasswordCredential = IntegrationTestSupport.createPasswordCredential(authId, ORIG_BCRYPT_PWD); pskCredentials = newPskCredentials(authId); - registry.registerDevice(Constants.DEFAULT_TENANT, deviceId) - .onComplete(ctx.completing()); + registry.registerDevice(tenantId, deviceId) + .onComplete(ctx.completing()); } /** @@ -123,8 +125,7 @@ public void setUp(final VertxTestContext ctx, final TestInfo testInfo) { */ @AfterEach public void removeCredentials(final VertxTestContext ctx) { - - registry.deregisterDevice(TENANT, deviceId).onComplete(ctx.completing()); + helper.deleteObjects(ctx); } /** @@ -134,42 +135,55 @@ public void removeCredentials(final VertxTestContext ctx) { */ @AfterAll public static void tearDown(final VertxTestContext context) { - vertx.close(context.completing()); + VERTX.close(context.completing()); + } + + private static void assertResourceVersionHasChanged(final AtomicReference originalVersion, final MultiMap responseHeaders) { + final String resourceVersion = responseHeaders.get(HttpHeaders.ETAG); + assertThat(resourceVersion).isNotNull(); + assertThat(resourceVersion).isNotEqualTo(originalVersion.get()); + originalVersion.set(resourceVersion); } /** - * Verifies that the service accepts an add credentials request containing valid credentials. + * Verifies that when a device is created, an associated entry is created in the credential Service. * * @param context The vert.x test context. */ @Test - public void testAddCredentialsSucceeds(final VertxTestContext context) { + public void testNewDeviceReturnsEmptyCredentials(final VertxTestContext context) { + + registry.getCredentials(tenantId, deviceId) + .onComplete(context.succeeding(responseBody -> { + context.verify(() -> { + final CommonCredential[] credentials = Json.decodeValue(responseBody, + CommonCredential[].class); + assertThat(credentials).isEmpty(); + }); + context.completeNow(); + })); - registry - .updateCredentials(TENANT, deviceId, Collections.singleton(hashedPasswordCredential), - HttpURLConnection.HTTP_NO_CONTENT) - .onComplete(context.completing()); } /** - * Verifies that when a device is created, an associated entry is created in the credential Service. + * Verifies that the service accepts a request to add credentials request containing valid credentials. * * @param context The vert.x test context. */ @Test - public void testNewDeviceReturnsEmptyCredentials(final VertxTestContext context) { - - registry - .getCredentials(TENANT, deviceId) - .onComplete(context.succeeding(ok2 -> { - context.verify(() -> { - final CommonCredential[] credentials = Json.decodeValue(ok2, - CommonCredential[].class); - assertThat(credentials).hasSize(0); - }); - context.completeNow(); - })); + public void testAddCredentialsSucceeds(final VertxTestContext context) { + registry.updateCredentials( + tenantId, + deviceId, + List.of(hashedPasswordCredential), + HttpURLConnection.HTTP_NO_CONTENT) + .onComplete(context.succeeding(responseHeaders -> { + context.verify(() -> { + assertResourceVersionHasChanged(resourceVersion, responseHeaders); + }); + context.completeNow(); + })); } /** @@ -183,8 +197,11 @@ public void testAddCredentialsSucceedsForAdditionalProperties(final VertxTestCon final PasswordCredential credential = IntegrationTestSupport.createPasswordCredential(authId, "thePassword"); credential.getExtensions().put("client-id", "MQTT-client-2384236854"); - registry.addCredentials(TENANT, deviceId, Collections.singleton(credential)) - .compose(createAttempt -> registry.getCredentials(TENANT, deviceId)) + registry.addCredentials(tenantId, deviceId, List.of(credential)) + .compose(responseHeaders -> { + context.verify(() -> assertResourceVersionHasChanged(resourceVersion, responseHeaders)); + return registry.getCredentials(tenantId, deviceId); + }) .onComplete(context.succeeding(b -> { context.verify(() -> { final JsonArray response = b.toJsonArray(); @@ -210,38 +227,13 @@ public void testAddCredentialsSucceedsForAdditionalProperties(final VertxTestCon @Test public void testAddCredentialsFailsForWrongContentType(final VertxTestContext context) { - registry - .updateCredentials( - TENANT, - deviceId, - Collections.singleton(hashedPasswordCredential), - "application/x-www-form-urlencoded", - HttpURLConnection.HTTP_BAD_REQUEST) - .onComplete(context.completing()); - - } - - /** - * Verifies that the service rejects a request to update a credentials set if the resource Version value is - * outdated. - * - * @param context The vert.x test context. - */ - @Test - public void testUpdateCredentialsWithOutdatedResourceVersionFails(final VertxTestContext context) { - - registry - .updateCredentials( - TENANT, deviceId, Collections.singleton(hashedPasswordCredential), - HttpURLConnection.HTTP_NO_CONTENT) - .compose(ar -> { - final var etag = ar.get(HTTP_HEADER_ETAG); - context.verify(() -> assertThat(etag).as("missing etag header").isNotNull()); - // now try to update credentials with other version - return registry.updateCredentialsWithVersion(TENANT, deviceId, Collections.singleton(hashedPasswordCredential), - etag + 10, HttpURLConnection.HTTP_PRECON_FAILED); - }) - .onComplete(context.completing()); + registry.updateCredentials( + tenantId, + deviceId, + List.of(hashedPasswordCredential), + "application/x-www-form-urlencoded", + HttpURLConnection.HTTP_BAD_REQUEST) + .onComplete(context.completing()); } @@ -257,23 +249,41 @@ public void testAddCredentialsFailsForBCryptWithTooManyIterations(final VertxTes // GIVEN a hashed password using bcrypt with more than the configured max iterations final BCryptPasswordEncoder encoder = new BCryptPasswordEncoder(IntegrationTestSupport.MAX_BCRYPT_ITERATIONS + 1); - final PasswordCredential credential = new PasswordCredential(); - credential.setAuthId(authId); + final PasswordCredential credential = new PasswordCredential(authId); final PasswordSecret secret = new PasswordSecret(); secret.setHashFunction(CredentialsConstants.HASH_FUNCTION_BCRYPT); secret.setPasswordHash(encoder.encode("thePassword")); - credential.setSecrets(Collections.singletonList(secret)); + credential.setSecrets(List.of(secret)); // WHEN adding the credentials - registry - .updateCredentials( - TENANT, - deviceId, - Collections.singleton(credential), - // THEN the request fails with 400 - HttpURLConnection.HTTP_BAD_REQUEST) - .onComplete(context.completing()); + testAddCredentialsWithErroneousPayload( + context, + new JsonArray().add(JsonObject.mapFrom(credential)), + // THEN the request fails with 400 + HttpURLConnection.HTTP_BAD_REQUEST); + } + + /** + * Verifies that a request to add credentials that contain unsupported properties + * fails with a 400 status code. + * + * @param ctx The vert.x test context + */ + @Test + public void testAddCredentialsFailsForUnknownProperties(final VertxTestContext ctx) { + + final PskCredential credentials = newPskCredentials("device1"); + final JsonArray requestBody = new JsonArray() + .add(JsonObject.mapFrom(credentials).put("unexpected", "property")); + + registry.updateCredentialsRaw( + tenantId, + deviceId, + requestBody.toBuffer(), + HttpUtils.CONTENT_TYPE_JSON_UTF8, + HttpURLConnection.HTTP_BAD_REQUEST) + .onComplete(ctx.completing()); } /** @@ -284,10 +294,10 @@ public void testAddCredentialsFailsForBCryptWithTooManyIterations(final VertxTes @Test public void testAddCredentialsFailsForEmptyBody(final VertxTestContext context) { - registry.updateCredentialsRaw(TENANT, deviceId, null, CrudHttpClient.CONTENT_TYPE_JSON, - HttpURLConnection.HTTP_BAD_REQUEST) - .onComplete(context.completing()); - + testAddCredentialsWithErroneousPayload( + context, + null, + HttpURLConnection.HTTP_BAD_REQUEST); } /** @@ -299,7 +309,33 @@ public void testAddCredentialsFailsForEmptyBody(final VertxTestContext context) */ @Test public void testAddCredentialsFailsForMissingType(final VertxTestContext context) { - testAddCredentialsWithMissingPayloadParts(context, CredentialsConstants.FIELD_TYPE); + + final JsonObject credentials = JsonObject.mapFrom(hashedPasswordCredential); + credentials.remove(CredentialsConstants.FIELD_TYPE); + + testAddCredentialsWithErroneousPayload( + context, + new JsonArray().add(credentials), + HttpURLConnection.HTTP_BAD_REQUEST); + } + + /** + * Verifies that a JSON payload to add generic credentials that contain a type name that + * does not match the type name regex is rejected with a 400. + * + * @param context The vert.x test context. + */ + @Test + public void testAddCredentialsFailsForIllegalTypeName(final VertxTestContext context) { + + final JsonObject credentials = new JsonObject() + .put(RegistryManagementConstants.FIELD_AUTH_ID, "deviceId") + .put(RegistryManagementConstants.FIELD_TYPE, "#illegal"); + + testAddCredentialsWithErroneousPayload( + context, + new JsonArray().add(credentials), + HttpURLConnection.HTTP_BAD_REQUEST); } /** @@ -311,20 +347,49 @@ public void testAddCredentialsFailsForMissingType(final VertxTestContext context */ @Test public void testAddCredentialsFailsForMissingAuthId(final VertxTestContext context) { - testAddCredentialsWithMissingPayloadParts(context, CredentialsConstants.FIELD_AUTH_ID); + + final JsonObject credentials = JsonObject.mapFrom(hashedPasswordCredential); + credentials.remove(CredentialsConstants.FIELD_AUTH_ID); + + testAddCredentialsWithErroneousPayload( + context, + new JsonArray().add(credentials), + HttpURLConnection.HTTP_BAD_REQUEST); + } + + /** + * Verifies that a JSON payload to add credentials that contains an authentication identifier that + * does not match the auth-id regex is rejected with a 400. + * + * @param context The vert.x test context. + */ + @Test + public void testAddCredentialsFailsForIllegalAuthId(final VertxTestContext context) { + + final JsonObject credentials = JsonObject.mapFrom(hashedPasswordCredential) + .put(RegistryManagementConstants.FIELD_AUTH_ID, "#illegal"); + + testAddCredentialsWithErroneousPayload( + context, + new JsonArray().add(credentials), + HttpURLConnection.HTTP_BAD_REQUEST); } - private void testAddCredentialsWithMissingPayloadParts(final VertxTestContext context, final String fieldMissing) { + private void testAddCredentialsWithErroneousPayload( + final VertxTestContext context, + final JsonArray payload, + final int expectedStatus) { - final JsonObject json = JsonObject.mapFrom(hashedPasswordCredential); - json.remove(fieldMissing); - final JsonArray payload = new JsonArray() - .add(json); + LOG.debug("updating credentials with request body: {}", + Optional.ofNullable(payload).map(p -> p.encodePrettily()).orElse(null)); - registry - .updateCredentialsRaw(TENANT, deviceId, payload.toBuffer(), - CrudHttpClient.CONTENT_TYPE_JSON, HttpURLConnection.HTTP_BAD_REQUEST) - .onComplete(context.completing()); + registry.updateCredentialsRaw( + tenantId, + deviceId, + Optional.ofNullable(payload).map(JsonArray::toBuffer).orElse(null), + CrudHttpClient.CONTENT_TYPE_JSON, + HttpURLConnection.HTTP_BAD_REQUEST) + .onComplete(context.completing()); } /** @@ -340,11 +405,24 @@ public void testUpdateCredentialsSucceeds(final VertxTestContext context) { .mapTo(PasswordCredential.class); altered.getSecrets().get(0).setComment("test"); - registry.updateCredentials(TENANT, deviceId, Collections.singleton(hashedPasswordCredential), + registry.updateCredentials( + tenantId, + deviceId, + List.of(hashedPasswordCredential), HttpURLConnection.HTTP_NO_CONTENT) - .compose(ar -> registry.updateCredentials(TENANT, deviceId, Collections.singleton(altered), - HttpURLConnection.HTTP_NO_CONTENT)) - .compose(ur -> registry.getCredentials(TENANT, deviceId)) + .compose(responseHeaders -> { + context.verify(() -> assertResourceVersionHasChanged(resourceVersion, responseHeaders)); + return registry.updateCredentialsWithVersion( + tenantId, + deviceId, + List.of(altered), + resourceVersion.get(), + HttpURLConnection.HTTP_NO_CONTENT); + }) + .compose(responseHeaders -> { + context.verify(() -> assertResourceVersionHasChanged(resourceVersion, responseHeaders)); + return registry.getCredentials(tenantId, deviceId); + }) .onComplete(context.succeeding(gr -> { final JsonObject retrievedSecret = gr.toJsonArray().getJsonObject(0).getJsonArray("secrets").getJsonObject(0); context.verify(() -> assertThat(retrievedSecret.getString("comment")).isEqualTo("test")); @@ -363,9 +441,9 @@ public void testUpdateCredentialsSucceedsForClearTextPassword(final VertxTestCon final PasswordCredential secret = IntegrationTestSupport.createPasswordCredential(authId, "newPassword"); - registry.addCredentials(TENANT, deviceId, Collections. singleton(hashedPasswordCredential)) - .compose(ar -> registry.updateCredentials(TENANT, deviceId, secret)) - .compose(ur -> registry.getCredentials(TENANT, deviceId)) + registry.addCredentials(tenantId, deviceId, List.of(hashedPasswordCredential)) + .compose(ar -> registry.updateCredentials(tenantId, deviceId, secret)) + .compose(ur -> registry.getCredentials(tenantId, deviceId)) .onComplete(context.succeeding(gr -> { final CredentialsObject o = extractFirstCredential(gr.toJsonObject()) .mapTo(CredentialsObject.class); @@ -380,22 +458,33 @@ public void testUpdateCredentialsSucceedsForClearTextPassword(final VertxTestCon } /** - * Verifies that the service rejects an update request for non-existing credentials. + * Verifies that the service rejects a request to update a credentials set if the resource Version value is + * outdated. * * @param context The vert.x test context. */ @Test - public void testUpdateCredentialsFailsForNonExistingCredentials(final VertxTestContext context) { - registry - .updateCredentialsWithVersion( - TENANT, + public void testUpdateCredentialsFailsForWrongResourceVersion(final VertxTestContext context) { + + registry.updateCredentials( + tenantId, + deviceId, + List.of(hashedPasswordCredential), + HttpURLConnection.HTTP_NO_CONTENT) + .compose(responseHeaders -> { + context.verify(() -> assertResourceVersionHasChanged(resourceVersion, responseHeaders)); + // now try to update credentials with other version + return registry.updateCredentialsWithVersion( + tenantId, deviceId, - Collections.singleton(hashedPasswordCredential), - "3", - HttpURLConnection.HTTP_PRECON_FAILED) - .onComplete(context.completing()); + List.of(hashedPasswordCredential), + resourceVersion.get() + "_other", + HttpURLConnection.HTTP_PRECON_FAILED); + }) + .onComplete(context.completing()); } + /** * Verify that a correctly added credentials record can be successfully looked up again by using the type and * authId. @@ -405,13 +494,15 @@ public void testUpdateCredentialsFailsForNonExistingCredentials(final VertxTestC @Test public void testGetAddedCredentials(final VertxTestContext context) { - registry.updateCredentials(TENANT, deviceId, Collections.singleton(hashedPasswordCredential), - HttpURLConnection.HTTP_NO_CONTENT) - .compose(ar -> registry.getCredentials(TENANT, deviceId)) + registry.updateCredentials(tenantId, deviceId, List.of(hashedPasswordCredential), HttpURLConnection.HTTP_NO_CONTENT) + .compose(ar -> registry.getCredentials(tenantId, deviceId)) .onComplete(context.succeeding(b -> { - final PasswordCredential cred = b.toJsonArray().getJsonObject(0).mapTo(PasswordCredential.class); - cred.getSecrets().forEach(secret -> { - context.verify(() -> assertThat(secret.getId()).isNotNull()); + context.verify(() -> { + final JsonArray credentials = b.toJsonArray(); + LOG.trace("retrieved credentials [tenant-id: {}, device-id: {}]: {}", + tenantId, deviceId, credentials); + final PasswordCredential cred = credentials.getJsonObject(0).mapTo(PasswordCredential.class); + cred.getSecrets().forEach(secret -> assertThat(secret.getId()).isNotNull()); }); context.completeNow();; })); @@ -424,15 +515,16 @@ public void testGetAddedCredentials(final VertxTestContext context) { * @param context The vert.x test context. */ @Test - public void testAddedCredentialsContainsEtag(final VertxTestContext context) { + public void testUpdateCredentialsChangesResourceVersion(final VertxTestContext context) { - registry - .updateCredentials(TENANT, deviceId, Collections.singleton(hashedPasswordCredential), - HttpURLConnection.HTTP_NO_CONTENT) - .onComplete(context.succeeding(res -> { - context.verify(() -> assertThat(res.get(HTTP_HEADER_ETAG)).as("etag header missing").isNotNull()); - context.completeNow(); - })); + registry.updateCredentials( + tenantId, + deviceId, List.of(hashedPasswordCredential), + HttpURLConnection.HTTP_NO_CONTENT) + .onComplete(context.succeeding(responseHeaders -> { + context.verify(() -> assertResourceVersionHasChanged(resourceVersion, responseHeaders)); + context.completeNow(); + })); } /** @@ -448,10 +540,10 @@ public void testGetAddedCredentialsMultipleTypesSingleRequests(final VertxTestCo credentialsListToAdd.add(hashedPasswordCredential); credentialsListToAdd.add(pskCredentials); - registry.addCredentials(TENANT, deviceId, credentialsListToAdd) - .compose(ar -> registry.getCredentials(TENANT, deviceId)) + registry.addCredentials(tenantId, deviceId, credentialsListToAdd) + .compose(ar -> registry.getCredentials(tenantId, deviceId)) .onComplete(context.succeeding(b -> { - assertResponseBodyContainsAllCredentials(context, b.toJsonArray(), credentialsListToAdd); + context.verify(() -> assertResponseBodyContainsAllCredentials(b.toJsonArray(), credentialsListToAdd)); context.completeNow(); })); } @@ -471,10 +563,10 @@ public void testGetAllCredentialsForDeviceSucceeds(final VertxTestContext contex credentialsListToAdd.add(newPskCredentials("auth")); credentialsListToAdd.add(newPskCredentials("other-auth")); - registry.addCredentials(TENANT, deviceId, credentialsListToAdd) - .compose(ar -> registry.getCredentials(TENANT, deviceId)) + registry.addCredentials(tenantId, deviceId, credentialsListToAdd) + .compose(ar -> registry.getCredentials(tenantId, deviceId)) .onComplete(context.succeeding(b -> { - assertResponseBodyContainsAllCredentials(context, b.toJsonArray(), credentialsListToAdd); + context.verify(() -> assertResponseBodyContainsAllCredentials(b.toJsonArray(), credentialsListToAdd)); context.completeNow(); })); } @@ -490,12 +582,10 @@ public void testGetAllCredentialsForDeviceSucceeds(final VertxTestContext contex @Test public void testGetCredentialsForDeviceRegardlessOfType(final VertxTestContext context) throws InterruptedException { - final String pskAuthId = getRandomAuthId(TEST_AUTH_ID); + final String pskAuthId = getRandomAuthId(PREFIX_AUTH_ID); final List credentialsToAdd = new ArrayList<>(); for (int i = 0; i < 5; i++) { - final GenericCredential credential = new GenericCredential(); - credential.setAuthId(pskAuthId); - credential.setType("type" + i); + final GenericCredential credential = new GenericCredential("type" + i, pskAuthId); final GenericSecret secret = new GenericSecret(); secret.getAdditionalProperties().put("field" + i, "setec astronomy"); @@ -504,10 +594,10 @@ public void testGetCredentialsForDeviceRegardlessOfType(final VertxTestContext c credentialsToAdd.add(credential); } - registry.addCredentials(TENANT, deviceId, credentialsToAdd) - .compose(ar -> registry.getCredentials(TENANT, deviceId)) + registry.addCredentials(tenantId, deviceId, credentialsToAdd) + .compose(ar -> registry.getCredentials(tenantId, deviceId)) .onComplete(context.succeeding(b -> { - assertResponseBodyContainsAllCredentials(context, b.toJsonArray(), credentialsToAdd); + context.verify(() -> assertResponseBodyContainsAllCredentials(b.toJsonArray(), credentialsToAdd)); context.completeNow(); })); } @@ -520,9 +610,9 @@ public void testGetCredentialsForDeviceRegardlessOfType(final VertxTestContext c @Test public void testGetAddedCredentialsButWithWrongType(final VertxTestContext context) { - registry.updateCredentials(TENANT, deviceId, Collections.singleton(hashedPasswordCredential), + registry.updateCredentials(tenantId, deviceId, List.of(hashedPasswordCredential), HttpURLConnection.HTTP_NO_CONTENT) - .compose(ar -> registry.getCredentials(TENANT, authId, "wrong-type", HttpURLConnection.HTTP_NOT_FOUND)) + .compose(ar -> registry.getCredentials(tenantId, authId, "wrong-type", HttpURLConnection.HTTP_NOT_FOUND)) .onComplete(context.completing()); } @@ -534,18 +624,17 @@ public void testGetAddedCredentialsButWithWrongType(final VertxTestContext conte @Test public void testGetAddedCredentialsButWithWrongAuthId(final VertxTestContext context) { - registry.updateCredentials(TENANT, deviceId, Collections.singleton(hashedPasswordCredential), + registry.updateCredentials(tenantId, deviceId, List.of(hashedPasswordCredential), HttpURLConnection.HTTP_NO_CONTENT) .compose(ar -> registry.getCredentials( - TENANT, + tenantId, "wrong-auth-id", CredentialsConstants.SECRETS_TYPE_HASHED_PASSWORD, HttpURLConnection.HTTP_NOT_FOUND)) .onComplete(context.completing()); } - private static void assertResponseBodyContainsAllCredentials(final VertxTestContext context, final JsonArray responseBody, - final List expected) { + private static void assertResponseBodyContainsAllCredentials(final JsonArray responseBody, final List expected) { assertThat(expected.size()).isEqualTo(responseBody.size()); @@ -574,7 +663,7 @@ private static void assertResponseBodyContainsAllCredentials(final VertxTestCont }); // now compare - context.verify(() -> assertThat(responseBody).isEqualTo(expectedArray)); + assertThat(responseBody).isEqualTo(expectedArray); } private static String getRandomAuthId(final String authIdPrefix) { @@ -583,12 +672,11 @@ private static String getRandomAuthId(final String authIdPrefix) { private static PskCredential newPskCredentials(final String authId) { - final PskCredential credential = new PskCredential(); - credential.setAuthId(authId); + final PskCredential credential = new PskCredential(authId); final PskSecret secret = new PskSecret(); secret.setKey("secret".getBytes(StandardCharsets.UTF_8)); - credential.setSecrets(Collections.singletonList(secret)); + credential.setSecrets(List.of(secret)); return credential;