Skip to content

Only persist secrets if we hydrated them in workspace webhook config handling #19352

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 14, 2022
Merged

Only persist secrets if we hydrated them in workspace webhook config handling #19352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c2968c5
only persist secrets if we ever hydrated them in workspace webhook co…
mfsiega-airbyte Nov 11, 2022
841c8d3
Merge branch 'master' into msiega/fix-workspace-secrets-handling
mfsiega-airbyte Nov 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions airbyte-server/src/main/java/io/airbyte/server/handlers/WorkspacesHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
}
Expand Down
54 changes: 47 additions & 7 deletions airbyte-server/src/test/java/io/airbyte/server/handlers/WorkspacesHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Expand All @@ -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())
Expand All @@ -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())
Expand All @@ -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 {
Expand Down