Skip to content

Commit 142b98a

Browse files
kaniyankaniyan
committed
Incorporate review comments and minor improvements
Signed-off-by: Kartheeswaran Kalidass <kartheeswaran.kalidass@bosch.io>
1 parent a9af944 commit 142b98a

7 files changed

Lines changed: 199 additions & 56 deletions

File tree

‎services/device-registry-base/src/main/java/org/eclipse/hono/deviceregistry/service/credentials/AbstractCredentialsManagementService.java‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.util.Objects;
2121
import java.util.Optional;
2222
import java.util.Set;
23+
import java.util.stream.Collectors;
2324

2425
import org.eclipse.hono.auth.HonoPasswordEncoder;
2526
import org.eclipse.hono.client.ClientErrorException;
@@ -285,10 +286,14 @@ protected List<CommonCredential> checkCredentials(final List<CommonCredential> c
285286
private static Future<List<CommonCredential>> applyAuthIdTemplateForX509CertificateCredentials(
286287
final Tenant tenant,
287288
final List<CommonCredential> credentials) {
288-
credentials.stream()
289-
.filter(X509CertificateCredential.class::isInstance)
290-
.map(X509CertificateCredential.class::cast)
291-
.forEach(c -> c.applyAuthIdTemplate(tenant));
292-
return Future.succeededFuture(credentials);
289+
final List<CommonCredential> creds = credentials.stream()
290+
.map(cred -> {
291+
if (cred instanceof X509CertificateCredential) {
292+
return ((X509CertificateCredential) cred).applyAuthIdTemplate(tenant);
293+
}
294+
return cred;
295+
})
296+
.collect(Collectors.toUnmodifiableList());
297+
return Future.succeededFuture(creds);
293298
}
294299
}

‎services/device-registry-base/src/main/java/org/eclipse/hono/deviceregistry/service/credentials/AbstractCredentialsService.java‎

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import org.eclipse.hono.util.CredentialsConstants;
3030
import org.eclipse.hono.util.CredentialsResult;
3131
import org.eclipse.hono.util.Lifecycle;
32-
import org.eclipse.hono.util.RegistryManagementConstants;
3332
import org.slf4j.Logger;
3433
import org.slf4j.LoggerFactory;
3534
import org.springframework.beans.factory.annotation.Autowired;
@@ -181,7 +180,6 @@ public final Future<CredentialsResult<JsonObject>> get(
181180
}
182181
});
183182
})
184-
.map(AbstractCredentialsService::overrideAuthIdWithGeneratedAuthIdIfExists)
185183
.recover(error -> {
186184
LOG.debug("error getting credentials [tenant: {}, type: {}, auth-id: {}]", tenantId, type, authId, error);
187185
TracingHelper.logError(span, error);
@@ -192,20 +190,8 @@ public final Future<CredentialsResult<JsonObject>> get(
192190
});
193191
}
194192

195-
private static CredentialsResult<JsonObject> overrideAuthIdWithGeneratedAuthIdIfExists(
196-
final CredentialsResult<JsonObject> credentialsResult) {
197-
if (!credentialsResult.isError()) {
198-
final JsonObject credential = credentialsResult.getPayload();
199-
Optional.ofNullable(credential)
200-
.map(c -> c.getString(RegistryManagementConstants.FIELD_TYPE))
201-
.filter(RegistryManagementConstants.SECRETS_TYPE_X509_CERT::equals)
202-
.map(ok -> credential.getString(RegistryManagementConstants.FIELD_GENERATED_AUTH_ID))
203-
.ifPresent(id -> credential.put(RegistryManagementConstants.FIELD_AUTH_ID, id));
204-
}
205-
return credentialsResult;
206-
}
207-
208193
private boolean isAutoProvisioningConfigured() {
209194
return this.deviceAndGatewayAutoProvisioner != null;
210195
}
196+
211197
}

‎services/device-registry-base/src/main/java/org/eclipse/hono/service/management/credentials/DelegatingCredentialsManagementHttpEndpoint.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.eclipse.hono.service.management.OperationResult;
2828
import org.eclipse.hono.tracing.TracingHelper;
2929
import org.eclipse.hono.util.RegistryManagementConstants;
30+
import org.eclipse.hono.util.Strings;
3031

3132
import io.opentracing.Span;
3233
import io.vertx.core.CompositeFuture;
@@ -207,7 +208,19 @@ private static List<CommonCredential> decodeCredentials(final JsonArray array) {
207208
return array.stream()
208209
.filter(JsonObject.class::isInstance)
209210
.map(JsonObject.class::cast)
211+
.map(DelegatingCredentialsManagementHttpEndpoint::checkForGeneratedAuthId)
210212
.map(json -> json.mapTo(CommonCredential.class))
211213
.collect(Collectors.toList());
212214
}
215+
216+
private static JsonObject checkForGeneratedAuthId(final JsonObject credential) {
217+
final String type = credential.getString(RegistryManagementConstants.FIELD_TYPE);
218+
if (!Strings.isNullOrEmpty(type) && type.equals(RegistryManagementConstants.SECRETS_TYPE_X509_CERT)) {
219+
if (credential.containsKey(RegistryManagementConstants.FIELD_GENERATED_AUTH_ID)) {
220+
throw new IllegalArgumentException(
221+
"credentials object contains an invalid attribute [generated-auth-id]");
222+
}
223+
}
224+
return credential;
225+
}
213226
}

‎services/device-registry-base/src/main/java/org/eclipse/hono/service/management/credentials/X509CertificateCredential.java‎

Lines changed: 58 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import java.util.List;
2222
import java.util.Objects;
2323
import java.util.Optional;
24-
import java.util.function.Predicate;
2524

2625
import javax.security.auth.x500.X500Principal;
2726

@@ -32,6 +31,7 @@
3231

3332
import com.fasterxml.jackson.annotation.JsonCreator;
3433
import com.fasterxml.jackson.annotation.JsonCreator.Mode;
34+
import com.fasterxml.jackson.annotation.JsonGetter;
3535
import com.fasterxml.jackson.annotation.JsonIgnore;
3636
import com.fasterxml.jackson.annotation.JsonInclude;
3737
import com.fasterxml.jackson.annotation.JsonProperty;
@@ -50,7 +50,24 @@ public class X509CertificateCredential extends CommonCredential {
5050

5151
private final List<X509CertificateSecret> secrets = new LinkedList<>();
5252
private final String issuerDN;
53-
private String generatedAuthId;
53+
private final String generatedAuthId;
54+
55+
/**
56+
* Creates a new credentials object from the given authentication identifier and secrets.
57+
*
58+
* @param authId The authentication identifier.
59+
* @param generatedAuthId The authentication identifier generated by applying the <em>auth-id-template</em>
60+
* from the tenant's trust anchor to the client certificate's subject DN.
61+
* @param secrets The credential's secret(s).
62+
* @throws NullPointerException if authentication identifier and secrets are {@code null}.
63+
*/
64+
protected X509CertificateCredential(final String authId, final String generatedAuthId,
65+
final List<X509CertificateSecret> secrets) {
66+
super(authId);
67+
setSecrets(secrets);
68+
this.generatedAuthId = generatedAuthId;
69+
this.issuerDN = null;
70+
}
5471

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

208-
/**
209-
* {@inheritDoc}
210-
*/
211-
@Override
212-
protected Predicate<String> getAuthIdValidator() {
213-
return authId -> {
214-
if (Strings.isNullOrEmpty(authId)) {
215-
return false;
216-
}
217-
final X500Principal distinguishedName = new X500Principal(authId);
218-
return distinguishedName.getName(X500Principal.RFC2253).equals(authId);
219-
};
220-
}
221-
222225
/**
223226
* {@inheritDoc}
224227
*/
@@ -260,43 +263,66 @@ public final X509CertificateCredential setSecrets(final List<X509CertificateSecr
260263
}
261264

262265
/**
263-
* Sets the <em>generated-auth-id</em> by applying the <em>auth-id-template</em> from the tenant's
264-
* trust anchor to the client certificate's subject DN.
266+
* Applies the <em>auth-id-template</em> from the tenant's trust anchor to the client certificate's subject DN.
265267
* <p>
266268
* It is only applicable, if a template is configured.
267269
*
268270
* @param tenant The tenant information.
271+
* @return the credential with the generated authentication identifier.
269272
* @throws NullPointerException if the tenant is {@code null}.
270273
*/
271274
@JsonIgnore
272-
public final void applyAuthIdTemplate(final Tenant tenant) {
275+
public final X509CertificateCredential applyAuthIdTemplate(final Tenant tenant) {
273276
Objects.requireNonNull(tenant, "tenant information must not be null");
274277

275-
Optional.ofNullable(issuerDN)
278+
final String generatedAuthId = Optional.ofNullable(issuerDN)
276279
.flatMap(tenant::getAuthIdTemplate)
277280
.map(IdentityTemplate::new)
278281
.map(t -> t.apply(getAuthId()))
279-
.ifPresent(this::setGeneratedAuthId);
282+
.orElse(null);
283+
final X509CertificateCredential credential = new X509CertificateCredential(getAuthId(),
284+
generatedAuthId, secrets) {
285+
286+
@Override
287+
@JsonGetter(value = RegistryManagementConstants.FIELD_GENERATED_AUTH_ID)
288+
protected String getGeneratedAuthId() {
289+
return super.getGeneratedAuthId();
290+
}
291+
};
292+
293+
copyCommonAttributesTo(credential);
294+
return credential;
280295
}
281296

282297
/**
283-
* Gets the authentication identifier generated by applying the <em>auth-id-template</em> from the tenant's trust
284-
* anchor to the client certificate's subject DN.
298+
* Overrides the authentication identifier with the generated authentication identifier,
299+
* if the generated one is not {@code null}.
285300
*
286-
* @return The generated authentication identifier.
301+
* @return The credential with the overridden authentication identifier.
287302
*/
288-
@JsonProperty(value = RegistryManagementConstants.FIELD_GENERATED_AUTH_ID)
289-
public final String getGeneratedAuthId() {
290-
return generatedAuthId;
303+
@JsonIgnore
304+
public final X509CertificateCredential overrideAuthIdWithGeneratedAuthId() {
305+
final String authId = Strings.isNullOrEmpty(generatedAuthId) ? getAuthId() : generatedAuthId;
306+
final X509CertificateCredential credential = new X509CertificateCredential(authId, null, secrets);
307+
308+
copyCommonAttributesTo(credential);
309+
310+
return credential;
291311
}
292312

293313
/**
294-
* Sets the authentication identifier generated by applying the <em>auth-id-template</em> from the tenant's trust
295-
* anchor to the client certificate's subject DN.
314+
* Gets the authentication identifier generated by applying the <em>auth-id-template</em>
315+
* from the tenant's trust anchor to the client certificate's subject DN.
296316
*
297-
* @param generatedAuthId The generated authentication identifier.
317+
* @return The generated authentication identifier or {@code null}.
298318
*/
299-
public final void setGeneratedAuthId(final String generatedAuthId) {
300-
this.generatedAuthId = generatedAuthId;
319+
protected String getGeneratedAuthId() {
320+
return generatedAuthId;
321+
}
322+
323+
private void copyCommonAttributesTo(final CommonCredential credential) {
324+
credential.setComment(getComment());
325+
credential.setEnabled(isEnabled());
326+
credential.setExtensions(getExtensions());
301327
}
302328
}

‎services/device-registry-base/src/test/java/org/eclipse/hono/service/management/credentials/X509CertificateCredentialTest.java‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2021 Contributors to the Eclipse Foundation
3+
*
4+
* See the NOTICE file(s) distributed with this work for additional
5+
* information regarding copyright ownership.
6+
*
7+
* This program and the accompanying materials are made available under the
8+
* terms of the Eclipse Public License 2.0 which is available at
9+
* http://www.eclipse.org/legal/epl-2.0
10+
*
11+
* SPDX-License-Identifier: EPL-2.0
12+
*******************************************************************************/
13+
package org.eclipse.hono.service.management.credentials;
14+
15+
import static com.google.common.truth.Truth.assertThat;
16+
17+
import java.nio.charset.StandardCharsets;
18+
import java.time.Instant;
19+
import java.time.temporal.ChronoUnit;
20+
import java.util.List;
21+
import java.util.Map;
22+
import java.util.UUID;
23+
24+
import org.eclipse.hono.service.management.tenant.Tenant;
25+
import org.eclipse.hono.service.management.tenant.TrustedCertificateAuthority;
26+
import org.eclipse.hono.util.RegistryManagementConstants;
27+
import org.junit.jupiter.api.BeforeEach;
28+
import org.junit.jupiter.api.Test;
29+
30+
import io.vertx.core.json.JsonObject;
31+
32+
/**
33+
* Verifies {@link X509CertificateCredential}.
34+
*/
35+
public class X509CertificateCredentialTest {
36+
37+
private String commonName;
38+
39+
/**
40+
* Sets up the fixture.
41+
*/
42+
@BeforeEach
43+
void setup() {
44+
commonName = UUID.randomUUID().toString();
45+
}
46+
47+
/**
48+
* Verifies the credentials object generated while applying the given auth-id-template
49+
* to the client certificate's subject DN.
50+
*/
51+
@Test
52+
void testCredentialObtainedByApplyingAuthIdTemplate() {
53+
final String authIdTemplate = "auth-{{subject-cn}}-{{subject-ou}}-{{subject-o}}";
54+
final String expectedAuthId = String.format("auth-%s-Hono-Eclipse", commonName);
55+
final X509CertificateCredential result = getCertificateByApplyingTemplate(authIdTemplate);
56+
57+
assertThat(result.getGeneratedAuthId()).isEqualTo(expectedAuthId);
58+
assertThat(result.isEnabled()).isEqualTo(true);
59+
assertThat(result.getComment()).isEqualTo("comment");
60+
assertThat(result.getExtensions().size()).isEqualTo(1);
61+
assertThat(result.getExtensions().get("test")).isEqualTo("value");
62+
}
63+
64+
/**
65+
* Verify the credential obtained by overriding the authId with the generated authId.
66+
*/
67+
@Test
68+
void testCredentialObtainedByOverridingAuthId() {
69+
final String authIdTemplate = "auth-{{subject-cn}}-{{subject-ou}}";
70+
final String expectedAuthId = String.format("auth-%s-Hono", commonName);
71+
final X509CertificateCredential result = getCertificateByApplyingTemplate(authIdTemplate)
72+
.overrideAuthIdWithGeneratedAuthId();
73+
final JsonObject json = JsonObject.mapFrom(result);
74+
75+
assertThat(json.getString(RegistryManagementConstants.FIELD_AUTH_ID)).isEqualTo(expectedAuthId);
76+
assertThat(json.getBoolean(RegistryManagementConstants.FIELD_ENABLED)).isEqualTo(true);
77+
assertThat(json.getString(RegistryManagementConstants.FIELD_COMMENT)).isEqualTo("comment");
78+
final JsonObject extensions = json.getJsonObject(RegistryManagementConstants.FIELD_EXT);
79+
assertThat(extensions.getString("test")).isEqualTo("value");
80+
}
81+
82+
private X509CertificateCredential getCertificateByApplyingTemplate(final String authIdTemplate) {
83+
final String issuerDN = "CN=testBase,OU=Hono,O=Eclipse";
84+
final String subjectDN = String.format("CN=%s,OU=Hono,O=Eclipse", commonName);
85+
final var credential = X509CertificateCredential.fromSubjectDn(subjectDN, issuerDN, null,
86+
List.of(new X509CertificateSecret()));
87+
credential.setEnabled(true)
88+
.setComment("comment")
89+
.setExtensions(Map.of("test", "value"));
90+
final var trustedCa = new TrustedCertificateAuthority()
91+
.setSubjectDn(issuerDN)
92+
.setPublicKey("NOTAKEY".getBytes(StandardCharsets.UTF_8))
93+
.setAuthIdTemplate(authIdTemplate)
94+
.setNotBefore(Instant.now().minus(1, ChronoUnit.DAYS))
95+
.setNotAfter(Instant.now().plus(2, ChronoUnit.DAYS));
96+
final var tenant = new Tenant().setTrustedCertificateAuthorities(List.of(trustedCa));
97+
98+
return credential.applyAuthIdTemplate(tenant);
99+
}
100+
}

‎services/device-registry-jdbc/src/main/java/org/eclipse/hono/deviceregistry/jdbc/impl/CredentialsServiceImpl.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.eclipse.hono.deviceregistry.service.tenant.TenantKey;
3030
import org.eclipse.hono.deviceregistry.util.DeviceRegistryUtils;
3131
import org.eclipse.hono.service.base.jdbc.store.device.TableAdapterStore;
32+
import org.eclipse.hono.service.management.credentials.X509CertificateCredential;
3233
import org.eclipse.hono.util.Constants;
3334
import org.eclipse.hono.util.CredentialsConstants;
3435
import org.eclipse.hono.util.CredentialsResult;
@@ -77,6 +78,12 @@ protected Future<CredentialsResult<JsonObject>> processGet(
7778

7879
final var secrets = result.getCredentials()
7980
.stream()
81+
.map(credential -> {
82+
if (credential instanceof X509CertificateCredential) {
83+
return ((X509CertificateCredential) credential).overrideAuthIdWithGeneratedAuthId();
84+
}
85+
return credential;
86+
})
8087
.map(JsonObject::mapFrom)
8188
.filter(filter(key.getType(), key.getAuthId()))
8289
.filter(credential -> DeviceRegistryUtils.matchesWithClientContext(credential, clientContext))

‎services/device-registry-mongodb/src/main/java/org/eclipse/hono/deviceregistry/mongodb/service/MongoDbBasedCredentialsService.java‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
import org.eclipse.hono.deviceregistry.service.credentials.CredentialKey;
2525
import org.eclipse.hono.deviceregistry.service.tenant.TenantKey;
2626
import org.eclipse.hono.deviceregistry.util.DeviceRegistryUtils;
27+
import org.eclipse.hono.service.management.credentials.CommonCredential;
28+
import org.eclipse.hono.service.management.credentials.X509CertificateCredential;
2729
import org.eclipse.hono.util.CacheDirective;
2830
import org.eclipse.hono.util.CredentialsConstants;
2931
import org.eclipse.hono.util.CredentialsResult;
@@ -99,14 +101,18 @@ protected Future<CredentialsResult<JsonObject>> processGet(
99101
return dao.getByAuthIdAndType(tenant.getTenantId(), key.getAuthId(), key.getType(), span.context())
100102
.map(dto -> {
101103
LOG.trace("found credentials matching criteria");
102-
final var json = JsonObject.mapFrom(dto.getCredentials().get(0));
104+
CommonCredential credential = dto.getCredentials().get(0);
105+
if (credential instanceof X509CertificateCredential) {
106+
credential = ((X509CertificateCredential) credential).overrideAuthIdWithGeneratedAuthId();
107+
}
108+
final var json = JsonObject.mapFrom(credential);
103109
json.put(CredentialsConstants.FIELD_PAYLOAD_DEVICE_ID, dto.getDeviceId());
104110
return Optional.of(json)
105111
.filter(MongoDbBasedCredentialsService::isCredentialEnabled)
106-
.filter(credential -> DeviceRegistryUtils.matchesWithClientContext(credential, clientContext))
107-
.map(credential -> CredentialsResult.from(
112+
.filter(cred -> DeviceRegistryUtils.matchesWithClientContext(cred, clientContext))
113+
.map(cred -> CredentialsResult.from(
108114
HttpURLConnection.HTTP_OK,
109-
credential,
115+
cred,
110116
getCacheDirective(key.getType())))
111117
.orElseThrow(() -> new ClientErrorException(
112118
tenant.getTenantId(),

0 commit comments

Comments
 (0)