only persist secrets if we ever hydrated them in workspace webhook co…

…nfig handling (#19352)
airbytehq · Nov 17, 2022 · 1a97a8d · 1a97a8d
1 parent 301ef37
commit 1a97a8d
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 9 deletions.
diff --git a/airbyte-server/src/main/java/io/airbyte/server/handlers/WorkspacesHandler.java b/airbyte-server/src/main/java/io/airbyte/server/handlers/WorkspacesHandler.java
@@ -186,7 +186,14 @@ public WorkspaceRead updateWorkspace(final WorkspaceUpdate workspacePatch) throw
 
     LOGGER.debug("Patched Workspace before persisting: {}", workspace);
 
-    persistStandardWorkspace(workspace);
+    if (workspacePatch.getWebhookConfigs() == null) {
+      // We aren't persisting any secrets. It's safe (and necessary) to use the NoSecrets variant because
+      // we never hydrated them in the first place.
+      configRepository.writeStandardWorkspaceNoSecrets(workspace);
+    } else {
+      // We're saving new webhook configs, so we need to persist the secrets.
+      persistStandardWorkspace(workspace);
+    }
 
     // after updating email or tracking info, we need to re-identify the instance.
     TrackingClientSingleton.get().identify(workspaceId);
@@ -204,7 +211,9 @@ public WorkspaceRead updateWorkspaceName(final WorkspaceUpdateName workspaceUpda
         .withName(workspaceUpdateName.getName())
         .withSlug(generateUniqueSlug(workspaceUpdateName.getName()));
 
-    persistStandardWorkspace(persistedWorkspace);
+    // NOTE: it's safe (and necessary) to use the NoSecrets variant because we never hydrated them in
+    // the first place.
+    configRepository.writeStandardWorkspaceNoSecrets(persistedWorkspace);
 
     return buildWorkspaceReadFromId(workspaceId);
   }

diff --git a/airbyte-server/src/test/java/io/airbyte/server/handlers/WorkspacesHandlerTest.java b/airbyte-server/src/test/java/io/airbyte/server/handlers/WorkspacesHandlerTest.java
@@ -58,13 +58,16 @@
 
 class WorkspacesHandlerTest {
 
-  public static final String FAILURE_NOTIFICATION_WEBHOOK = "http://airbyte.notifications/failure";
-  public static final String NEW_WORKSPACE = "new workspace";
-  public static final String TEST_NAME = "test-name";
+  private static final String FAILURE_NOTIFICATION_WEBHOOK = "http://airbyte.notifications/failure";
+  private static final String NEW_WORKSPACE = "new workspace";
+  private static final String TEST_NAME = "test-name";
+
+  private static final String TEST_AUTH_TOKEN = "test-auth-token";
   private static final UUID WEBHOOK_CONFIG_ID = UUID.randomUUID();
   private static final JsonNode PERSISTED_WEBHOOK_CONFIGS = Jsons.deserialize(
       String.format("{\"webhookConfigs\": [{\"id\": \"%s\", \"name\": \"%s\", \"authToken\": {\"_secret\": \"a-secret_v1\"}}]}",
           WEBHOOK_CONFIG_ID, TEST_NAME));
+  public static final String UPDATED = "updated";
   private ConfigRepository configRepository;
   private SecretsRepositoryWriter secretsRepositoryWriter;
   private ConnectionsHandler connectionsHandler;
@@ -149,7 +152,7 @@ void testCreateWorkspace() throws JsonValidationException, IOException, ConfigNo
         .securityUpdates(false)
         .notifications(List.of(generateApiNotification()))
         .defaultGeography(GEOGRAPHY_US)
-        .webhookConfigs(List.of(new WebhookConfigWrite().name(TEST_NAME).authToken("test-auth-token")));
+        .webhookConfigs(List.of(new WebhookConfigWrite().name(TEST_NAME).authToken(TEST_AUTH_TOKEN)));
 
     final WorkspaceRead actualRead = workspacesHandler.createWorkspace(workspaceCreate);
     final WorkspaceRead expectedRead = new WorkspaceRead()
@@ -359,7 +362,7 @@ void testGetWorkspaceByConnectionId() {
   @Test
   void testUpdateWorkspace() throws JsonValidationException, ConfigNotFoundException, IOException {
     final io.airbyte.api.model.generated.Notification apiNotification = generateApiNotification();
-    apiNotification.getSlackConfiguration().webhook("updated");
+    apiNotification.getSlackConfiguration().webhook(UPDATED);
     final WorkspaceUpdate workspaceUpdate = new WorkspaceUpdate()
         .workspaceId(workspace.getWorkspaceId())
         .anonymousDataCollection(true)
@@ -372,7 +375,7 @@ void testUpdateWorkspace() throws JsonValidationException, ConfigNotFoundExcepti
         .webhookConfigs(List.of(new WebhookConfigWrite().name(TEST_NAME).authToken("test-auth-token")));
 
     final Notification expectedNotification = generateNotification();
-    expectedNotification.getSlackConfiguration().withWebhook("updated");
+    expectedNotification.getSlackConfiguration().withWebhook(UPDATED);
     final StandardWorkspace expectedWorkspace = new StandardWorkspace()
         .withWorkspaceId(workspace.getWorkspaceId())
         .withCustomerId(workspace.getCustomerId())
@@ -398,7 +401,7 @@ void testUpdateWorkspace() throws JsonValidationException, ConfigNotFoundExcepti
     final WorkspaceRead actualWorkspaceRead = workspacesHandler.updateWorkspace(workspaceUpdate);
 
     final io.airbyte.api.model.generated.Notification expectedNotificationRead = generateApiNotification();
-    expectedNotificationRead.getSlackConfiguration().webhook("updated");
+    expectedNotificationRead.getSlackConfiguration().webhook(UPDATED);
     final WorkspaceRead expectedWorkspaceRead = new WorkspaceRead()
         .workspaceId(workspace.getWorkspaceId())
         .customerId(workspace.getCustomerId())
@@ -419,6 +422,43 @@ void testUpdateWorkspace() throws JsonValidationException, ConfigNotFoundExcepti
     assertEquals(expectedWorkspaceRead, actualWorkspaceRead);
   }
 
+  @Test
+  void testUpdateWorkspaceWithoutWebhookConfigs() throws JsonValidationException, ConfigNotFoundException, IOException {
+    final io.airbyte.api.model.generated.Notification apiNotification = generateApiNotification();
+    apiNotification.getSlackConfiguration().webhook(UPDATED);
+    final WorkspaceUpdate workspaceUpdate = new WorkspaceUpdate()
+        .workspaceId(workspace.getWorkspaceId())
+        .anonymousDataCollection(false);
+
+    final Notification expectedNotification = generateNotification();
+    expectedNotification.getSlackConfiguration().withWebhook(UPDATED);
+    final StandardWorkspace expectedWorkspace = new StandardWorkspace()
+        .withWorkspaceId(workspace.getWorkspaceId())
+        .withCustomerId(workspace.getCustomerId())
+        .withEmail(TEST_EMAIL)
+        .withName(TEST_WORKSPACE_NAME)
+        .withSlug(TEST_WORKSPACE_SLUG)
+        .withAnonymousDataCollection(true)
+        .withSecurityUpdates(false)
+        .withNews(false)
+        .withInitialSetupComplete(true)
+        .withDisplaySetupWizard(false)
+        .withTombstone(false)
+        .withNotifications(List.of(expectedNotification))
+        .withDefaultGeography(Geography.US)
+        .withWebhookOperationConfigs(PERSISTED_WEBHOOK_CONFIGS);
+
+    when(uuidSupplier.get()).thenReturn(WEBHOOK_CONFIG_ID);
+
+    when(configRepository.getStandardWorkspaceNoSecrets(workspace.getWorkspaceId(), false))
+        .thenReturn(expectedWorkspace)
+        .thenReturn(expectedWorkspace.withAnonymousDataCollection(false));
+
+    workspacesHandler.updateWorkspace(workspaceUpdate);
+
+    verify(configRepository).writeStandardWorkspaceNoSecrets(expectedWorkspace);
+  }
+
   @Test
   @DisplayName("Updating workspace name should update name and slug")
   void testUpdateWorkspaceNoNameUpdate() throws JsonValidationException, ConfigNotFoundException, IOException {