[#2819] Prevent tenants from having empty CA array

The (partial) unique index defined on the tenant's trusted-ca array is
defined using an $exists expression on the trusted-ca property.
This causes tenants with an empty trusted-ca array to also be indexed.

The Tenant class' trustedCertificateAuthorities field has been annotated
to only be included in its JSON representation when the collection is
not empty.

Fixes #2819

Conflicts:
	site/homepage/content/release-notes.md

Signed-off-by: Kai Hudalla <[email protected]>

Author: sophokles73

services/device-registry-base/src/main/java/org/eclipse/hono/service/management/tenant/Tenant.java

@@ -78,11 +78,12 @@ public class Tenant {
     private ResourceLimits resourceLimits;
 
     @JsonProperty(RegistryManagementConstants.FIELD_TRACING)
-    @JsonInclude(Include.NON_EMPTY)
+    @JsonInclude(Include.NON_NULL)
     private TenantTracingConfig tracing;
 
     @JsonProperty(RegistryManagementConstants.FIELD_PAYLOAD_TRUSTED_CA)
-    private List<TrustedCertificateAuthority> trustedCertificateAuthorities;
+    @JsonInclude(Include.NON_EMPTY)
+    private List<TrustedCertificateAuthority> trustedCertificateAuthorities = List.of();
 
     @JsonProperty(RegistryManagementConstants.FIELD_REGISTRATION_LIMITS)
     @JsonInclude(Include.NON_DEFAULT)
@@ -118,7 +119,7 @@ public Tenant(final Tenant other) {
         this.registrationLimits = other.registrationLimits;
         this.tracing = other.tracing;
         if (Objects.nonNull(other.trustedCertificateAuthorities)) {
-            this.trustedCertificateAuthorities = new ArrayList<>(other.trustedCertificateAuthorities);
+            this.trustedCertificateAuthorities = List.copyOf(other.trustedCertificateAuthorities);
         }
     }
 
@@ -399,7 +400,7 @@ public List<TrustedCertificateAuthority> getTrustedCertificateAuthorities() {
      */
     public Tenant setTrustedCertificateAuthorities(final List<TrustedCertificateAuthority> trustedCertificateAuthorities) {
         if (trustedCertificateAuthorities != null) {
-            this.trustedCertificateAuthorities = Collections.unmodifiableList(trustedCertificateAuthorities);
+            this.trustedCertificateAuthorities = List.copyOf(trustedCertificateAuthorities);
         }
         return this;
     }

services/device-registry-base/src/test/java/org/eclipse/hono/service/tenant/AbstractTenantServiceTest.java

@@ -563,6 +563,52 @@ default void testUpdateTenantFailsForDuplicateCa(final VertxTestContext ctx) {
             }));
     }
 
+    /**
+     * Verifies that setting an empty list of trusted CAs on multiple tenants does not result in a unique key
+     * violation.
+     *
+     * @param ctx The vert.x test context.
+     */
+    @Test
+    default void testUpdateTenantPreventsEmptyCaArray(final VertxTestContext ctx) {
+
+        final var trustedCaOne = new TrustedCertificateAuthority();
+        trustedCaOne.setSubjectDn("CN=one");
+        trustedCaOne.setPublicKey("NOTAKEY".getBytes(StandardCharsets.UTF_8));
+
+        final var trustedCaTwo = new TrustedCertificateAuthority();
+        trustedCaTwo.setSubjectDn("CN=two");
+        trustedCaTwo.setPublicKey("NOTAKEY".getBytes(StandardCharsets.UTF_8));
+
+        // GIVEN two tenants with one CA configured each
+        addTenant("tenantOne", new Tenant().setTrustedCertificateAuthorities(List.of(trustedCaOne)))
+            .onFailure(ctx::failNow)
+            .compose(ok -> addTenant("tenantTwo", new Tenant().setTrustedCertificateAuthorities(List.of(trustedCaTwo))))
+            .onFailure(ctx::failNow)
+            .compose(ok -> {
+                // WHEN setting an empty list of trusted CAs on the first tenant
+                final var updatedTenantOne = new Tenant().setTrustedCertificateAuthorities(List.of());
+                return getTenantManagementService().updateTenant(
+                        "tenantOne",
+                        updatedTenantOne,
+                        Optional.empty(),
+                        NoopSpan.INSTANCE);
+            })
+            // THEN the update succeeds
+            .onFailure(ctx::failNow)
+            .compose(ok -> {
+                // and WHEN setting an empty list of trusted CAs on the second tenant
+                final var updatedTenantTwo = new Tenant().setTrustedCertificateAuthorities(List.of());
+                return getTenantManagementService().updateTenant(
+                        "tenantTwo",
+                        updatedTenantTwo,
+                        Optional.empty(),
+                        NoopSpan.INSTANCE);
+            })
+            // THEN the update succeeds as well
+            .onComplete(ctx.completing());
+    }
+
     /**
      * Verifies that a tenant is registered.
      *

site/homepage/content/release-notes.md

@@ -20,6 +20,8 @@ description = "Information about changes in recent Hono releases. Includes new f
 * The device registry implementations did not return a JSON object in a response to a failed request as specified
   in the Device Registry Management API. This has been fixed.
 * The tracing output in error scenarios has been improved in the Mongo DB based device registry.
+* The MongoDB based registry erroneously rejected requests that would result in multiple tenants having an empty
+  set of trusted CAs. This has been fixed.
 
 ## 1.9.0
 

tests/src/test/java/org/eclipse/hono/tests/registry/TenantManagementIT.java

@@ -426,6 +426,39 @@ public void testUpdateTenantSucceedsForConfigurationWithMissingTrustAnchorIds(fi
                 }));
     }
 
+    /**
+     * Verifies that setting an empty list of trusted CAs on multiple tenants does not result in a unique key
+     * violation.
+     *
+     * @param context The Vert.x test context.
+     */
+    @Test
+    public void testUpdateTenantPreventsEmptyCaArray(final VertxTestContext context) {
+
+        final var tenantTwoId = getHelper().getRandomTenantId();
+        final PublicKey publicKey = TenantApiTests.getRandomPublicKey();
+        final TrustedCertificateAuthority trustAnchor1 = Tenants
+                .createTrustAnchor("test-ca", "CN=test-dn-1", publicKey.getEncoded(), publicKey.getAlgorithm(),
+                        Instant.now(), Instant.now().plus(365, ChronoUnit.DAYS));
+        final TrustedCertificateAuthority trustAnchor2 = Tenants
+                .createTrustAnchor(null, "CN=test-dn-2", publicKey.getEncoded(), publicKey.getAlgorithm(),
+                        Instant.now(), Instant.now().plus(365, ChronoUnit.DAYS));
+
+        getHelper().registry.addTenant(tenantId, new Tenant().setTrustedCertificateAuthorities(List.of(trustAnchor1)))
+            .compose(ok -> getHelper().registry.addTenant(
+                    tenantTwoId,
+                    new Tenant().setTrustedCertificateAuthorities(List.of(trustAnchor2))))
+            .compose(ok -> getHelper().registry.updateTenant(
+                    tenantId,
+                    new Tenant().setTrustedCertificateAuthorities(List.of()),
+                    HttpURLConnection.HTTP_NO_CONTENT))
+            .compose(ok -> getHelper().registry.updateTenant(
+                    tenantTwoId,
+                    new Tenant().setTrustedCertificateAuthorities(List.of()),
+                    HttpURLConnection.HTTP_NO_CONTENT))
+            .onComplete(context.completing());
+    }
+
     /**
      * Verifies that the service rejects an update request for a non-existing tenant.
      *