From 853c9a249eef2685894c56041b838c05991cc5d2 Mon Sep 17 00:00:00 2001
From: Robert Manning <robcmann@amazon.com>
Date: Tue, 20 Feb 2024 15:36:59 -0500
Subject: [PATCH] feat: test config validation in config change handler

---
 .../integrationtests/policy/PolicyTest.java   | 25 +++++++++++++++++++
 .../auth/ClientDevicesAuthService.java        |  8 +++---
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/src/integrationtests/java/com/aws/greengrass/integrationtests/policy/PolicyTest.java b/src/integrationtests/java/com/aws/greengrass/integrationtests/policy/PolicyTest.java
index acfc7c47a..aaddb6867 100644
--- a/src/integrationtests/java/com/aws/greengrass/integrationtests/policy/PolicyTest.java
+++ b/src/integrationtests/java/com/aws/greengrass/integrationtests/policy/PolicyTest.java
@@ -141,6 +141,31 @@ void GIVEN_cda_with_client_policy_WHEN_resource_removed_from_policy_THEN_resourc
                 .build()));
     }
 
+    @Test
+    void GIVEN_cda_running_WHEN_policy_updated_with_invalid_policy_variable_THEN_cda_broken(ExtensionContext context) throws Exception {
+        ignoreExceptionOfType(context, PolicyException.class);
+        startNucleus("empty-config.yaml");
+        //CDA is RUNNING
+
+        Runnable mainRunning = createServiceStateChangeWaiter(kernel,
+                ClientDevicesAuthService.CLIENT_DEVICES_AUTH_SERVICE_NAME, 30, State.BROKEN);
+
+        // merge bad policy config
+        replacePolicy(GroupConfiguration.builder()
+                .definitions(Utils.immutableMap("group1", GroupDefinition.builder()
+                        .policyName("policyA")
+                        .selectionRule("thingName: myThing")
+                        .build()))
+                .policies(Utils.immutableMap("policyA", Utils.immutableMap("statement1", AuthorizationPolicyStatement.builder()
+                        .statementDescription("invalid policy variable")
+                        .operations(Stream.of("mqtt:publish").collect(Collectors.toSet()))
+                        .resources(Stream.of("mqtt:topic:${iot:Connection.Thing.Unknown}").collect(Collectors.toSet()))
+                        .effect(AuthorizationPolicyStatement.Effect.ALLOW)
+                        .build())))
+                .build());
+        mainRunning.run();
+    }
+
     @ParameterizedTest
     @ValueSource(strings = {
             "malformed-variable.yaml",
diff --git a/src/main/java/com/aws/greengrass/clientdevices/auth/ClientDevicesAuthService.java b/src/main/java/com/aws/greengrass/clientdevices/auth/ClientDevicesAuthService.java
index 3b6de30bb..8d924432e 100644
--- a/src/main/java/com/aws/greengrass/clientdevices/auth/ClientDevicesAuthService.java
+++ b/src/main/java/com/aws/greengrass/clientdevices/auth/ClientDevicesAuthService.java
@@ -216,7 +216,8 @@ protected void startup() throws InterruptedException {
         context.get(CertificateManager.class).startMonitors();
         try {
             subscribeToConfigChanges();
-            validateConfig();
+            // Validate CDA policy to force CDA to break on bad config policies before CDA reaches RUNNING
+            lookupAndValidateDeviceGroups();
         } catch (IllegalArgumentException | PolicyException e) {
             serviceErrored(e);
             return;
@@ -270,7 +271,8 @@ private void updateDeviceGroups() {
         GroupConfiguration groupConfiguration;
 
         try {
-            groupConfiguration = validateConfig();
+            // Lookup and validate DeviceGroups to ensure CDA errors on bad CDA policy changes
+            groupConfiguration = lookupAndValidateDeviceGroups();
         } catch (IllegalArgumentException | PolicyException e) {
             serviceErrored(e);
             return;
@@ -279,7 +281,7 @@ private void updateDeviceGroups() {
         context.get(GroupManager.class).setGroupConfiguration(groupConfiguration);
     }
 
-    private GroupConfiguration validateConfig() throws IllegalArgumentException, PolicyException {
+    private GroupConfiguration lookupAndValidateDeviceGroups() throws IllegalArgumentException, PolicyException {
         GroupConfiguration groupConfiguration;
         Topics deviceGroupTopics = this.config.lookupTopics(CONFIGURATION_CONFIG_KEY, DEVICE_GROUPS_TOPICS);
         groupConfiguration = MAPPER.convertValue(deviceGroupTopics.toPOJO(), GroupConfiguration.class);