Skip to content

Commit

Permalink
Incorporate review comments and minor improvements
Browse files Browse the repository at this point in the history
Signed-off-by: Kartheeswaran Kalidass <[email protected]>
  • Loading branch information
kaniyan committed Dec 6, 2021
1 parent a9af944 commit 142b98a
Show file tree
Hide file tree
Showing 7 changed files with 199 additions and 56 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;

import org.eclipse.hono.auth.HonoPasswordEncoder;
import org.eclipse.hono.client.ClientErrorException;
Expand Down Expand Up @@ -285,10 +286,14 @@ protected List<CommonCredential> checkCredentials(final List<CommonCredential> c
private static Future<List<CommonCredential>> applyAuthIdTemplateForX509CertificateCredentials(
final Tenant tenant,
final List<CommonCredential> credentials) {
credentials.stream()
.filter(X509CertificateCredential.class::isInstance)
.map(X509CertificateCredential.class::cast)
.forEach(c -> c.applyAuthIdTemplate(tenant));
return Future.succeededFuture(credentials);
final List<CommonCredential> creds = credentials.stream()
.map(cred -> {
if (cred instanceof X509CertificateCredential) {
return ((X509CertificateCredential) cred).applyAuthIdTemplate(tenant);
}
return cred;
})
.collect(Collectors.toUnmodifiableList());
return Future.succeededFuture(creds);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import org.eclipse.hono.util.CredentialsConstants;
import org.eclipse.hono.util.CredentialsResult;
import org.eclipse.hono.util.Lifecycle;
import org.eclipse.hono.util.RegistryManagementConstants;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
Expand Down Expand Up @@ -181,7 +180,6 @@ public final Future<CredentialsResult<JsonObject>> get(
}
});
})
.map(AbstractCredentialsService::overrideAuthIdWithGeneratedAuthIdIfExists)
.recover(error -> {
LOG.debug("error getting credentials [tenant: {}, type: {}, auth-id: {}]", tenantId, type, authId, error);
TracingHelper.logError(span, error);
Expand All @@ -192,20 +190,8 @@ public final Future<CredentialsResult<JsonObject>> get(
});
}

private static CredentialsResult<JsonObject> overrideAuthIdWithGeneratedAuthIdIfExists(
final CredentialsResult<JsonObject> credentialsResult) {
if (!credentialsResult.isError()) {
final JsonObject credential = credentialsResult.getPayload();
Optional.ofNullable(credential)
.map(c -> c.getString(RegistryManagementConstants.FIELD_TYPE))
.filter(RegistryManagementConstants.SECRETS_TYPE_X509_CERT::equals)
.map(ok -> credential.getString(RegistryManagementConstants.FIELD_GENERATED_AUTH_ID))
.ifPresent(id -> credential.put(RegistryManagementConstants.FIELD_AUTH_ID, id));
}
return credentialsResult;
}

private boolean isAutoProvisioningConfigured() {
return this.deviceAndGatewayAutoProvisioner != null;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.eclipse.hono.service.management.OperationResult;
import org.eclipse.hono.tracing.TracingHelper;
import org.eclipse.hono.util.RegistryManagementConstants;
import org.eclipse.hono.util.Strings;

import io.opentracing.Span;
import io.vertx.core.CompositeFuture;
Expand Down Expand Up @@ -207,7 +208,19 @@ private static List<CommonCredential> decodeCredentials(final JsonArray array) {
return array.stream()
.filter(JsonObject.class::isInstance)
.map(JsonObject.class::cast)
.map(DelegatingCredentialsManagementHttpEndpoint::checkForGeneratedAuthId)
.map(json -> json.mapTo(CommonCredential.class))
.collect(Collectors.toList());
}

private static JsonObject checkForGeneratedAuthId(final JsonObject credential) {
final String type = credential.getString(RegistryManagementConstants.FIELD_TYPE);
if (!Strings.isNullOrEmpty(type) && type.equals(RegistryManagementConstants.SECRETS_TYPE_X509_CERT)) {
if (credential.containsKey(RegistryManagementConstants.FIELD_GENERATED_AUTH_ID)) {
throw new IllegalArgumentException(
"credentials object contains an invalid attribute [generated-auth-id]");
}
}
return credential;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Predicate;

import javax.security.auth.x500.X500Principal;

Expand All @@ -32,6 +31,7 @@

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonCreator.Mode;
import com.fasterxml.jackson.annotation.JsonGetter;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.annotation.JsonProperty;
Expand All @@ -50,7 +50,24 @@ public class X509CertificateCredential extends CommonCredential {

private final List<X509CertificateSecret> secrets = new LinkedList<>();
private final String issuerDN;
private String generatedAuthId;
private final String generatedAuthId;

/**
* Creates a new credentials object from the given authentication identifier and secrets.
*
* @param authId The authentication identifier.
* @param generatedAuthId The authentication identifier generated by applying the <em>auth-id-template</em>
* from the tenant's trust anchor to the client certificate's subject DN.
* @param secrets The credential's secret(s).
* @throws NullPointerException if authentication identifier and secrets are {@code null}.
*/
protected X509CertificateCredential(final String authId, final String generatedAuthId,
final List<X509CertificateSecret> secrets) {
super(authId);
setSecrets(secrets);
this.generatedAuthId = generatedAuthId;
this.issuerDN = null;
}

/**
* Creates a new credentials object for an X.500 Distinguished Name.
Expand Down Expand Up @@ -205,20 +222,6 @@ private static X509Certificate deserialize(final byte[] base64EncodedX509Certifi
}
}

/**
* {@inheritDoc}
*/
@Override
protected Predicate<String> getAuthIdValidator() {
return authId -> {
if (Strings.isNullOrEmpty(authId)) {
return false;
}
final X500Principal distinguishedName = new X500Principal(authId);
return distinguishedName.getName(X500Principal.RFC2253).equals(authId);
};
}

/**
* {@inheritDoc}
*/
Expand Down Expand Up @@ -260,43 +263,66 @@ public final X509CertificateCredential setSecrets(final List<X509CertificateSecr
}

/**
* Sets the <em>generated-auth-id</em> by applying the <em>auth-id-template</em> from the tenant's
* trust anchor to the client certificate's subject DN.
* Applies the <em>auth-id-template</em> from the tenant's trust anchor to the client certificate's subject DN.
* <p>
* It is only applicable, if a template is configured.
*
* @param tenant The tenant information.
* @return the credential with the generated authentication identifier.
* @throws NullPointerException if the tenant is {@code null}.
*/
@JsonIgnore
public final void applyAuthIdTemplate(final Tenant tenant) {
public final X509CertificateCredential applyAuthIdTemplate(final Tenant tenant) {
Objects.requireNonNull(tenant, "tenant information must not be null");

Optional.ofNullable(issuerDN)
final String generatedAuthId = Optional.ofNullable(issuerDN)
.flatMap(tenant::getAuthIdTemplate)
.map(IdentityTemplate::new)
.map(t -> t.apply(getAuthId()))
.ifPresent(this::setGeneratedAuthId);
.orElse(null);
final X509CertificateCredential credential = new X509CertificateCredential(getAuthId(),
generatedAuthId, secrets) {

@Override
@JsonGetter(value = RegistryManagementConstants.FIELD_GENERATED_AUTH_ID)
protected String getGeneratedAuthId() {
return super.getGeneratedAuthId();
}
};

copyCommonAttributesTo(credential);
return credential;
}

/**
* Gets the authentication identifier generated by applying the <em>auth-id-template</em> from the tenant's trust
* anchor to the client certificate's subject DN.
* Overrides the authentication identifier with the generated authentication identifier,
* if the generated one is not {@code null}.
*
* @return The generated authentication identifier.
* @return The credential with the overridden authentication identifier.
*/
@JsonProperty(value = RegistryManagementConstants.FIELD_GENERATED_AUTH_ID)
public final String getGeneratedAuthId() {
return generatedAuthId;
@JsonIgnore
public final X509CertificateCredential overrideAuthIdWithGeneratedAuthId() {
final String authId = Strings.isNullOrEmpty(generatedAuthId) ? getAuthId() : generatedAuthId;
final X509CertificateCredential credential = new X509CertificateCredential(authId, null, secrets);

copyCommonAttributesTo(credential);

return credential;
}

/**
* Sets the authentication identifier generated by applying the <em>auth-id-template</em> from the tenant's trust
* anchor to the client certificate's subject DN.
* Gets the authentication identifier generated by applying the <em>auth-id-template</em>
* from the tenant's trust anchor to the client certificate's subject DN.
*
* @param generatedAuthId The generated authentication identifier.
* @return The generated authentication identifier or {@code null}.
*/
public final void setGeneratedAuthId(final String generatedAuthId) {
this.generatedAuthId = generatedAuthId;
protected String getGeneratedAuthId() {
return generatedAuthId;
}

private void copyCommonAttributesTo(final CommonCredential credential) {
credential.setComment(getComment());
credential.setEnabled(isEnabled());
credential.setExtensions(getExtensions());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/*******************************************************************************
* Copyright (c) 2021 Contributors to the Eclipse Foundation
*
* See the NOTICE file(s) distributed with this work for additional
* information regarding copyright ownership.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0
*
* SPDX-License-Identifier: EPL-2.0
*******************************************************************************/
package org.eclipse.hono.service.management.credentials;

import static com.google.common.truth.Truth.assertThat;

import java.nio.charset.StandardCharsets;
import java.time.Instant;
import java.time.temporal.ChronoUnit;
import java.util.List;
import java.util.Map;
import java.util.UUID;

import org.eclipse.hono.service.management.tenant.Tenant;
import org.eclipse.hono.service.management.tenant.TrustedCertificateAuthority;
import org.eclipse.hono.util.RegistryManagementConstants;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import io.vertx.core.json.JsonObject;

/**
* Verifies {@link X509CertificateCredential}.
*/
public class X509CertificateCredentialTest {

private String commonName;

/**
* Sets up the fixture.
*/
@BeforeEach
void setup() {
commonName = UUID.randomUUID().toString();
}

/**
* Verifies the credentials object generated while applying the given auth-id-template
* to the client certificate's subject DN.
*/
@Test
void testCredentialObtainedByApplyingAuthIdTemplate() {
final String authIdTemplate = "auth-{{subject-cn}}-{{subject-ou}}-{{subject-o}}";
final String expectedAuthId = String.format("auth-%s-Hono-Eclipse", commonName);
final X509CertificateCredential result = getCertificateByApplyingTemplate(authIdTemplate);

assertThat(result.getGeneratedAuthId()).isEqualTo(expectedAuthId);
assertThat(result.isEnabled()).isEqualTo(true);
assertThat(result.getComment()).isEqualTo("comment");
assertThat(result.getExtensions().size()).isEqualTo(1);
assertThat(result.getExtensions().get("test")).isEqualTo("value");
}

/**
* Verify the credential obtained by overriding the authId with the generated authId.
*/
@Test
void testCredentialObtainedByOverridingAuthId() {
final String authIdTemplate = "auth-{{subject-cn}}-{{subject-ou}}";
final String expectedAuthId = String.format("auth-%s-Hono", commonName);
final X509CertificateCredential result = getCertificateByApplyingTemplate(authIdTemplate)
.overrideAuthIdWithGeneratedAuthId();
final JsonObject json = JsonObject.mapFrom(result);

assertThat(json.getString(RegistryManagementConstants.FIELD_AUTH_ID)).isEqualTo(expectedAuthId);
assertThat(json.getBoolean(RegistryManagementConstants.FIELD_ENABLED)).isEqualTo(true);
assertThat(json.getString(RegistryManagementConstants.FIELD_COMMENT)).isEqualTo("comment");
final JsonObject extensions = json.getJsonObject(RegistryManagementConstants.FIELD_EXT);
assertThat(extensions.getString("test")).isEqualTo("value");
}

private X509CertificateCredential getCertificateByApplyingTemplate(final String authIdTemplate) {
final String issuerDN = "CN=testBase,OU=Hono,O=Eclipse";
final String subjectDN = String.format("CN=%s,OU=Hono,O=Eclipse", commonName);
final var credential = X509CertificateCredential.fromSubjectDn(subjectDN, issuerDN, null,
List.of(new X509CertificateSecret()));
credential.setEnabled(true)
.setComment("comment")
.setExtensions(Map.of("test", "value"));
final var trustedCa = new TrustedCertificateAuthority()
.setSubjectDn(issuerDN)
.setPublicKey("NOTAKEY".getBytes(StandardCharsets.UTF_8))
.setAuthIdTemplate(authIdTemplate)
.setNotBefore(Instant.now().minus(1, ChronoUnit.DAYS))
.setNotAfter(Instant.now().plus(2, ChronoUnit.DAYS));
final var tenant = new Tenant().setTrustedCertificateAuthorities(List.of(trustedCa));

return credential.applyAuthIdTemplate(tenant);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.eclipse.hono.deviceregistry.service.tenant.TenantKey;
import org.eclipse.hono.deviceregistry.util.DeviceRegistryUtils;
import org.eclipse.hono.service.base.jdbc.store.device.TableAdapterStore;
import org.eclipse.hono.service.management.credentials.X509CertificateCredential;
import org.eclipse.hono.util.Constants;
import org.eclipse.hono.util.CredentialsConstants;
import org.eclipse.hono.util.CredentialsResult;
Expand Down Expand Up @@ -77,6 +78,12 @@ protected Future<CredentialsResult<JsonObject>> processGet(

final var secrets = result.getCredentials()
.stream()
.map(credential -> {
if (credential instanceof X509CertificateCredential) {
return ((X509CertificateCredential) credential).overrideAuthIdWithGeneratedAuthId();
}
return credential;
})
.map(JsonObject::mapFrom)
.filter(filter(key.getType(), key.getAuthId()))
.filter(credential -> DeviceRegistryUtils.matchesWithClientContext(credential, clientContext))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import org.eclipse.hono.deviceregistry.service.credentials.CredentialKey;
import org.eclipse.hono.deviceregistry.service.tenant.TenantKey;
import org.eclipse.hono.deviceregistry.util.DeviceRegistryUtils;
import org.eclipse.hono.service.management.credentials.CommonCredential;
import org.eclipse.hono.service.management.credentials.X509CertificateCredential;
import org.eclipse.hono.util.CacheDirective;
import org.eclipse.hono.util.CredentialsConstants;
import org.eclipse.hono.util.CredentialsResult;
Expand Down Expand Up @@ -99,14 +101,18 @@ protected Future<CredentialsResult<JsonObject>> processGet(
return dao.getByAuthIdAndType(tenant.getTenantId(), key.getAuthId(), key.getType(), span.context())
.map(dto -> {
LOG.trace("found credentials matching criteria");
final var json = JsonObject.mapFrom(dto.getCredentials().get(0));
CommonCredential credential = dto.getCredentials().get(0);
if (credential instanceof X509CertificateCredential) {
credential = ((X509CertificateCredential) credential).overrideAuthIdWithGeneratedAuthId();
}
final var json = JsonObject.mapFrom(credential);
json.put(CredentialsConstants.FIELD_PAYLOAD_DEVICE_ID, dto.getDeviceId());
return Optional.of(json)
.filter(MongoDbBasedCredentialsService::isCredentialEnabled)
.filter(credential -> DeviceRegistryUtils.matchesWithClientContext(credential, clientContext))
.map(credential -> CredentialsResult.from(
.filter(cred -> DeviceRegistryUtils.matchesWithClientContext(cred, clientContext))
.map(cred -> CredentialsResult.from(
HttpURLConnection.HTTP_OK,
credential,
cred,
getCacheDirective(key.getType())))
.orElseThrow(() -> new ClientErrorException(
tenant.getTenantId(),
Expand Down

0 comments on commit 142b98a

Please sign in to comment.