From f0f64a7b21c91ffbea75893fe1182a0628ab22a7 Mon Sep 17 00:00:00 2001 From: Suresh Srinivas Date: Thu, 25 May 2023 12:35:25 -0700 Subject: [PATCH] Change combined EncryptDecrypt methods to separate Encrypt and Decrpyt methods (#11755) --- .../jdbi3/IngestionPipelineRepository.java | 7 +- .../jdbi3/ServiceEntityRepository.java | 21 +- .../service/jdbi3/UserRepository.java | 2 +- .../service/jdbi3/WorkflowRepository.java | 2 +- .../automations/WorkflowResource.java | 4 +- .../services/ServiceEntityResource.java | 4 +- .../IngestionPipelineResource.java | 4 +- .../service/resources/teams/UserResource.java | 2 +- .../service/secrets/SecretsManager.java | 121 ++++++-- .../secrets/SecretsManagerUpdateService.java | 26 +- .../service/resources/EntityResourceTest.java | 29 +- .../DashboardServiceResourceTest.java | 4 +- .../services/DatabaseServiceResourceTest.java | 9 +- .../services/MetadataServiceResourceTest.java | 2 +- .../secrets/ExternalSecretsManagerTest.java | 279 +++++++----------- .../secrets/NoopSecretsManagerTest.java | 49 ++- 16 files changed, 265 insertions(+), 300 deletions(-) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/IngestionPipelineRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/IngestionPipelineRepository.java index 5473a58c8b3..5bced7b35bf 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/IngestionPipelineRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/IngestionPipelineRepository.java @@ -105,10 +105,9 @@ public class IngestionPipelineRepository extends EntityRepository { SecretsManager secretsManager = SecretsManagerFactory.getSecretsManager(); if (secretsManager != null && Boolean.TRUE.equals(user.getIsBot())) { - secretsManager.encryptOrDecryptAuthenticationMechanism(user.getName(), user.getAuthenticationMechanism(), true); + secretsManager.encryptAuthenticationMechanism(user.getName(), user.getAuthenticationMechanism()); } store(user, update); diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/WorkflowRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/WorkflowRepository.java index 38cf24099f0..5002342d81f 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/WorkflowRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/WorkflowRepository.java @@ -49,7 +49,7 @@ public class WorkflowRepository extends EntityRepository { SecretsManager secretsManager = SecretsManagerFactory.getSecretsManager(); if (secretsManager != null) { - entity = secretsManager.encryptOrDecryptWorkflow(entity, true); + entity = secretsManager.encryptWorkflow(entity); } // Don't store owner, database, href and tags as JSON. Build it on the fly based on relationships diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/resources/automations/WorkflowResource.java b/openmetadata-service/src/main/java/org/openmetadata/service/resources/automations/WorkflowResource.java index 184ed0756b6..c277b87363c 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/resources/automations/WorkflowResource.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/resources/automations/WorkflowResource.java @@ -511,11 +511,11 @@ public class WorkflowResource extends EntityResource { } catch (AuthorizationException | IOException e) { user.getAuthenticationMechanism().setConfig(null); } - secretsManager.encryptOrDecryptAuthenticationMechanism(user.getName(), user.getAuthenticationMechanism(), false); + secretsManager.decryptAuthenticationMechanism(user.getName(), user.getAuthenticationMechanism()); if (authorizer.shouldMaskPasswords(securityContext)) { EntityMaskerFactory.getEntityMasker() .maskAuthenticationMechanism(user.getName(), user.getAuthenticationMechanism()); diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/secrets/SecretsManager.java b/openmetadata-service/src/main/java/org/openmetadata/service/secrets/SecretsManager.java index 969a01b41dd..52f93d9337c 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/secrets/SecretsManager.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/secrets/SecretsManager.java @@ -54,15 +54,14 @@ public abstract class SecretsManager { this.fernet = Fernet.getInstance(); } - public Object encryptOrDecryptServiceConnectionConfig( - Object connectionConfig, String connectionType, String connectionName, ServiceType serviceType, boolean encrypt) { + public Object encryptServiceConnectionConfig( + Object connectionConfig, String connectionType, String connectionName, ServiceType serviceType) { try { Class clazz = ReflectionUtil.createConnectionConfigClass(connectionType, serviceType); Object newConnectionConfig = ClassConverterFactory.getConverter(clazz).convert(connectionConfig); - return encryptOrDecryptPasswordFields( - newConnectionConfig, buildSecretId(true, serviceType.value(), connectionName), encrypt, true); + return encryptPasswordFields(newConnectionConfig, buildSecretId(true, serviceType.value(), connectionName), true); } catch (Exception e) { - String message = SecretsUtil.buildExceptionMessageConnection(e.getMessage(), connectionType, encrypt); + String message = SecretsUtil.buildExceptionMessageConnection(e.getMessage(), connectionType, true); if (message != null) { throw new InvalidServiceConnectionException(message); } @@ -71,12 +70,27 @@ public abstract class SecretsManager { } } - public void encryptOrDecryptAuthenticationMechanism( - String name, AuthenticationMechanism authenticationMechanism, boolean encrypt) { + public Object decryptServiceConnectionConfig( + Object connectionConfig, String connectionType, ServiceType serviceType) { + try { + Class clazz = ReflectionUtil.createConnectionConfigClass(connectionType, serviceType); + Object newConnectionConfig = ClassConverterFactory.getConverter(clazz).convert(connectionConfig); + return decryptPasswordFields(newConnectionConfig); + } catch (Exception e) { + String message = SecretsUtil.buildExceptionMessageConnection(e.getMessage(), connectionType, false); + if (message != null) { + throw new InvalidServiceConnectionException(message); + } + throw InvalidServiceConnectionException.byMessage( + connectionType, String.format("Failed to encrypt connection instance of %s", connectionType)); + } + } + + public void encryptAuthenticationMechanism(String name, AuthenticationMechanism authenticationMechanism) { if (authenticationMechanism != null) { AuthenticationMechanismBuilder.addDefinedConfig(authenticationMechanism); try { - encryptOrDecryptPasswordFields(authenticationMechanism, buildSecretId(true, "bot", name), encrypt, true); + encryptPasswordFields(authenticationMechanism, buildSecretId(true, "bot", name), true); } catch (Exception e) { throw new CustomExceptionMessage( Response.Status.BAD_REQUEST, String.format("Failed to encrypt user bot instance [%s]", name)); @@ -84,15 +98,26 @@ public abstract class SecretsManager { } } - public void encryptOrDecryptIngestionPipeline(IngestionPipeline ingestionPipeline, boolean encrypt) { + public void decryptAuthenticationMechanism(String name, AuthenticationMechanism authenticationMechanism) { + if (authenticationMechanism != null) { + AuthenticationMechanismBuilder.addDefinedConfig(authenticationMechanism); + try { + decryptPasswordFields(authenticationMechanism); + } catch (Exception e) { + throw new CustomExceptionMessage( + Response.Status.BAD_REQUEST, String.format("Failed to encrypt user bot instance [%s]", name)); + } + } + } + + public void encryptIngestionPipeline(IngestionPipeline ingestionPipeline) { OpenMetadataConnection openMetadataConnection = - encryptOrDecryptOpenMetadataConnection(ingestionPipeline.getOpenMetadataServerConnection(), encrypt, true); + encryptOpenMetadataConnection(ingestionPipeline.getOpenMetadataServerConnection(), true); ingestionPipeline.setOpenMetadataServerConnection(null); // we don't store OM conn sensitive data IngestionPipelineBuilder.addDefinedConfig(ingestionPipeline); try { - encryptOrDecryptPasswordFields( - ingestionPipeline, buildSecretId(true, "pipeline", ingestionPipeline.getName()), encrypt, true); + encryptPasswordFields(ingestionPipeline, buildSecretId(true, "pipeline", ingestionPipeline.getName()), true); } catch (Exception e) { throw new CustomExceptionMessage( Response.Status.BAD_REQUEST, @@ -101,15 +126,30 @@ public abstract class SecretsManager { ingestionPipeline.setOpenMetadataServerConnection(openMetadataConnection); } - public Workflow encryptOrDecryptWorkflow(Workflow workflow, boolean encrypt) { + public void decryptIngestionPipeline(IngestionPipeline ingestionPipeline) { OpenMetadataConnection openMetadataConnection = - encryptOrDecryptOpenMetadataConnection(workflow.getOpenMetadataServerConnection(), encrypt, true); + decryptOpenMetadataConnection(ingestionPipeline.getOpenMetadataServerConnection(), true); + ingestionPipeline.setOpenMetadataServerConnection(null); + // we don't store OM conn sensitive data + IngestionPipelineBuilder.addDefinedConfig(ingestionPipeline); + try { + decryptPasswordFields(ingestionPipeline); + } catch (Exception e) { + throw new CustomExceptionMessage( + Response.Status.BAD_REQUEST, + String.format("Failed to encrypt ingestion pipeline instance [%s]", ingestionPipeline.getName())); + } + ingestionPipeline.setOpenMetadataServerConnection(openMetadataConnection); + } + + public Workflow encryptWorkflow(Workflow workflow) { + OpenMetadataConnection openMetadataConnection = + encryptOpenMetadataConnection(workflow.getOpenMetadataServerConnection(), true); Workflow workflowConverted = (Workflow) ClassConverterFactory.getConverter(Workflow.class).convert(workflow); // we don't store OM conn sensitive data workflowConverted.setOpenMetadataServerConnection(null); try { - encryptOrDecryptPasswordFields( - workflowConverted, buildSecretId(true, "workflow", workflow.getName()), encrypt, true); + encryptPasswordFields(workflowConverted, buildSecretId(true, "workflow", workflow.getName()), true); } catch (Exception e) { throw new CustomExceptionMessage( Response.Status.BAD_REQUEST, String.format("Failed to encrypt workflow instance [%s]", workflow.getName())); @@ -118,15 +158,30 @@ public abstract class SecretsManager { return workflowConverted; } - public OpenMetadataConnection encryptOrDecryptOpenMetadataConnection( - OpenMetadataConnection openMetadataConnection, boolean encrypt, boolean store) { + public Workflow decryptWorkflow(Workflow workflow) { + OpenMetadataConnection openMetadataConnection = + decryptOpenMetadataConnection(workflow.getOpenMetadataServerConnection(), true); + Workflow workflowConverted = (Workflow) ClassConverterFactory.getConverter(Workflow.class).convert(workflow); + // we don't store OM conn sensitive data + workflowConverted.setOpenMetadataServerConnection(null); + try { + decryptPasswordFields(workflowConverted); + } catch (Exception e) { + throw new CustomExceptionMessage( + Response.Status.BAD_REQUEST, String.format("Failed to encrypt workflow instance [%s]", workflow.getName())); + } + workflowConverted.setOpenMetadataServerConnection(openMetadataConnection); + return workflowConverted; + } + + public OpenMetadataConnection encryptOpenMetadataConnection( + OpenMetadataConnection openMetadataConnection, boolean store) { if (openMetadataConnection != null) { OpenMetadataConnection openMetadataConnectionConverted = (OpenMetadataConnection) ClassConverterFactory.getConverter(OpenMetadataConnection.class).convert(openMetadataConnection); try { - encryptOrDecryptPasswordFields( - openMetadataConnectionConverted, buildSecretId(true, "serverconnection"), encrypt, store); + encryptPasswordFields(openMetadataConnectionConverted, buildSecretId(true, "serverconnection"), store); } catch (Exception e) { throw new CustomExceptionMessage( Response.Status.BAD_REQUEST, "Failed to encrypt OpenMetadataConnection instance."); @@ -136,16 +191,24 @@ public abstract class SecretsManager { return null; } - private Object encryptOrDecryptPasswordFields(Object targetObject, String name, boolean encrypt, boolean store) { - if (encrypt) { - encryptPasswordFields(targetObject, name, store); - } else { - decryptPasswordFields(targetObject); + public OpenMetadataConnection decryptOpenMetadataConnection( + OpenMetadataConnection openMetadataConnection, boolean store) { + if (openMetadataConnection != null) { + OpenMetadataConnection openMetadataConnectionConverted = + (OpenMetadataConnection) + ClassConverterFactory.getConverter(OpenMetadataConnection.class).convert(openMetadataConnection); + try { + decryptPasswordFields(openMetadataConnectionConverted); + } catch (Exception e) { + throw new CustomExceptionMessage( + Response.Status.BAD_REQUEST, "Failed to encrypt OpenMetadataConnection instance."); + } + return openMetadataConnectionConverted; } - return targetObject; + return null; } - private void encryptPasswordFields(Object toEncryptObject, String secretId, boolean store) { + private Object encryptPasswordFields(Object toEncryptObject, String secretId, boolean store) { if (!DO_NOT_ENCRYPT_CLASSES.contains(toEncryptObject.getClass())) { // for each get method Arrays.stream(toEncryptObject.getClass().getMethods()) @@ -174,9 +237,10 @@ public abstract class SecretsManager { } }); } + return toEncryptObject; } - private void decryptPasswordFields(Object toDecryptObject) { + private Object decryptPasswordFields(Object toDecryptObject) { // for each get method Arrays.stream(toDecryptObject.getClass().getMethods()) .filter(ReflectionUtil::isGetMethodOfObject) @@ -198,6 +262,7 @@ public abstract class SecretsManager { toDecryptObject, Fernet.isTokenized(fieldValue) ? fernet.decrypt(fieldValue) : fieldValue, toSet); } }); + return toDecryptObject; } protected abstract String storeValue(String fieldName, String value, String secretId, boolean store); diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/secrets/SecretsManagerUpdateService.java b/openmetadata-service/src/main/java/org/openmetadata/service/secrets/SecretsManagerUpdateService.java index ba20298d002..d3f7cdb27c2 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/secrets/SecretsManagerUpdateService.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/secrets/SecretsManagerUpdateService.java @@ -117,21 +117,16 @@ public class SecretsManagerUpdateService { service .getConnection() .setConfig( - oldSecretManager.encryptOrDecryptServiceConnectionConfig( - service.getConnection().getConfig(), - service.getServiceType().value(), - service.getName(), - repository.getServiceType(), - false)); + oldSecretManager.decryptServiceConnectionConfig( + service.getConnection().getConfig(), service.getServiceType().value(), repository.getServiceType())); service .getConnection() .setConfig( - secretManager.encryptOrDecryptServiceConnectionConfig( + secretManager.encryptServiceConnectionConfig( service.getConnection().getConfig(), service.getServiceType().value(), service.getName(), - repository.getServiceType(), - true)); + repository.getServiceType())); repository.dao.update(service); } catch (IOException e) { throw new SecretsManagerUpdateException(e.getMessage(), e.getCause()); @@ -218,9 +213,8 @@ public class SecretsManagerUpdateService { private void updateBotUser(User botUser) { try { User user = userRepository.dao.findEntityById(botUser.getId()); - oldSecretManager.encryptOrDecryptAuthenticationMechanism( - botUser.getName(), user.getAuthenticationMechanism(), false); - secretManager.encryptOrDecryptAuthenticationMechanism(botUser.getName(), user.getAuthenticationMechanism(), true); + oldSecretManager.decryptAuthenticationMechanism(botUser.getName(), user.getAuthenticationMechanism()); + secretManager.encryptAuthenticationMechanism(botUser.getName(), user.getAuthenticationMechanism()); userRepository.dao.update(user); } catch (IOException e) { throw new SecretsManagerUpdateException(e.getMessage(), e.getCause()); @@ -261,8 +255,8 @@ public class SecretsManagerUpdateService { try { IngestionPipeline ingestion = ingestionPipelineRepository.dao.findEntityById(ingestionPipeline.getId()); // we have to decrypt using the old secrets manager and encrypt again with the new one - oldSecretManager.encryptOrDecryptIngestionPipeline(ingestionPipeline, false); - secretManager.encryptOrDecryptIngestionPipeline(ingestionPipeline, true); + oldSecretManager.decryptIngestionPipeline(ingestionPipeline); + secretManager.encryptIngestionPipeline(ingestionPipeline); ingestionPipelineRepository.dao.update(ingestion); } catch (IOException e) { throw new SecretsManagerUpdateException(e.getMessage(), e.getCause()); @@ -273,8 +267,8 @@ public class SecretsManagerUpdateService { try { Workflow workflowObject = workflowRepository.dao.findEntityById(workflow.getId()); // we have to decrypt using the old secrets manager and encrypt again with the new one - workflowObject = oldSecretManager.encryptOrDecryptWorkflow(workflowObject, false); - workflowObject = secretManager.encryptOrDecryptWorkflow(workflowObject, true); + workflowObject = oldSecretManager.decryptWorkflow(workflowObject); + workflowObject = secretManager.encryptWorkflow(workflowObject); ingestionPipelineRepository.dao.update(workflowObject); } catch (IOException e) { throw new SecretsManagerUpdateException(e.getMessage(), e.getCause()); diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java index f7637c1af4f..987a92f14e4 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/EntityResourceTest.java @@ -403,10 +403,6 @@ public abstract class EntityResourceTest authHeaders) { assertEquals(createRequest.getName(), service.getName()); - boolean maskPasswords = true; - if (INGESTION_BOT_AUTH_HEADERS.equals(authHeaders)) { - maskPasswords = false; - } + boolean maskPasswords = !INGESTION_BOT_AUTH_HEADERS.equals(authHeaders); validateDatabaseConnection( createRequest.getConnection(), service.getConnection(), service.getServiceType(), maskPasswords); } diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/services/MetadataServiceResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/services/MetadataServiceResourceTest.java index 2213a58bf54..e41223dfc8c 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/services/MetadataServiceResourceTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/services/MetadataServiceResourceTest.java @@ -126,7 +126,7 @@ public class MetadataServiceResourceTest extends EntityResourceTest mysqlConnection = Map.of("password", password); + + // Ensure encrypted service connection config encrypts the password + MysqlConnection actualConnection = + (MysqlConnection) + secretsManager.encryptServiceConnectionConfig(mysqlConnection, Mysql.value(), "test", ServiceType.DATABASE); + assertNotEquals(password, actualConnection.getPassword()); + + // Decrypt the encrypted password and validate + actualConnection = + (MysqlConnection) + secretsManager.decryptServiceConnectionConfig(mysqlConnection, Mysql.value(), ServiceType.DATABASE); + assertEquals(password, actualConnection.getPassword()); + assertEquals(expectedConnection, actualConnection); } @Test - void testEncryptDatabaseServiceConnectionConfig() { - testEncryptDecryptServiceConnection(ENCRYPT); + void testEncryptDecryptSSSOConfig() { + String privateKey = "secret:/openmetadata/bot/bot/config/authconfig/privatekey"; + OktaSSOClientConfig config = new OktaSSOClientConfig().withPrivateKey(privateKey); + AuthenticationMechanism expectedAuthMechanism = + new AuthenticationMechanism() + .withAuthType(AuthenticationMechanism.AuthType.SSO) + .withConfig( + new SSOAuthMechanism().withAuthConfig(config).withSsoServiceType(SSOAuthMechanism.SsoServiceType.OKTA)); + + AuthenticationMechanism actualAuthMechanism = + JsonUtils.convertValue(expectedAuthMechanism, AuthenticationMechanism.class); + + // Encrypt private key and ensure it is indeed encrypted + secretsManager.encryptAuthenticationMechanism("bot", actualAuthMechanism); + assertNotEquals(privateKey, getPrivateKey(actualAuthMechanism)); + System.out.println("XXX privateKey encrypted is " + getPrivateKey(actualAuthMechanism)); + + // Decrypt private key and ensure it is decrypted + secretsManager.decryptAuthenticationMechanism("bot", actualAuthMechanism); + System.out.println("XXX privateKey decrypted is " + getPrivateKey(actualAuthMechanism)); + assertEquals(privateKey, getPrivateKey(actualAuthMechanism)); } @Test - void testDecryptSSOConfig() { - testEncryptDecryptSSOConfig(DECRYPT); + void testEncryptDecryptIngestionPipelineDBTConfig() { + String secretKey = + "secret:/openmetadata/pipeline/my-pipeline/sourceconfig/config/dbtconfigsource" + + "/dbtsecurityconfig/awssecretaccesskey"; + AWSCredentials credentials = new AWSCredentials().withAwsSecretAccessKey(secretKey).withAwsRegion("eu-west-1"); + DbtS3Config config = new DbtS3Config().withDbtSecurityConfig(credentials); + DbtPipeline dbtPipeline = new DbtPipeline().withDbtConfigSource(config); + SourceConfig sourceConfig = new SourceConfig().withConfig(dbtPipeline); + IngestionPipeline expectedIngestionPipeline = + new IngestionPipeline() + .withName("my-pipeline") + .withPipelineType(PipelineType.DBT) + .withService(new DatabaseService().getEntityReference().withType(Entity.DATABASE_SERVICE)) + .withSourceConfig(sourceConfig); + + IngestionPipeline actualIngestionPipeline = + JsonUtils.convertValue(expectedIngestionPipeline, IngestionPipeline.class); + + // Encrypt the pipeline and make sure it is secret key encrypted + secretsManager.encryptIngestionPipeline(actualIngestionPipeline); + System.out.println("XXX encrypted aws secret access key is " + getAwsSecretAccessKey(actualIngestionPipeline)); + assertNotEquals(secretKey, getAwsSecretAccessKey(actualIngestionPipeline)); + + // Decrypt the pipeline and make sure the secret key is decrypted + secretsManager.decryptIngestionPipeline(actualIngestionPipeline); + System.out.println("XXX decrypted aws secret access key is " + getAwsSecretAccessKey(actualIngestionPipeline)); + assertEquals(secretKey, getAwsSecretAccessKey(actualIngestionPipeline)); + assertEquals(expectedIngestionPipeline, actualIngestionPipeline); } @Test - void testEncryptSSSOConfig() { - testEncryptDecryptSSOConfig(ENCRYPT); - } + void testEncryptDecryptWorkflow() { + String password = "secret:/openmetadata/workflow/my-workflow/request/connection/config/password"; + String secretKey = "secret:/openmetadata/serverconnection/securityconfig/secretkey"; + OpenMetadataConnection connection = + new OpenMetadataConnection().withSecurityConfig(new GoogleSSOClientConfig().withSecretKey(secretKey)); + DatabaseConnection dbConnection = new DatabaseConnection().withConfig(new MysqlConnection().withPassword(password)); + TestServiceConnectionRequest testRequest = + new TestServiceConnectionRequest() + .withConnection(dbConnection) + .withServiceType(ServiceType.DATABASE) + .withConnectionType("Mysql"); + Workflow expectedWorkflow = + new Workflow().withName("my-workflow").withOpenMetadataServerConnection(connection).withRequest(testRequest); + Workflow actualWorkflow = JsonUtils.convertValue(expectedWorkflow, Workflow.class); - @Test - void testDecryptIngestionPipelineDBTConfig() { - testEncryptDecryptDBTConfig(DECRYPT); - } + // Encrypt the workflow and ensure password and secrete key are encrypted + actualWorkflow = secretsManager.encryptWorkflow(actualWorkflow); + assertNotEquals(password, getPassword(actualWorkflow)); + assertNotEquals(secretKey, getSecretKey(actualWorkflow)); - @Test - void testEncryptIngestionPipelineDBTConfig() { - testEncryptDecryptDBTConfig(ENCRYPT); - } - - @Test - void testDecryptWorkflow() { - testEncryptWorkflowObject(DECRYPT); - } - - @Test - void testEncryptWorkflow() { - testEncryptWorkflowObject(ENCRYPT); + // Decrypt the workflow and ensure password and secrete key are decrypted + actualWorkflow = secretsManager.decryptWorkflow(actualWorkflow); + assertEquals(password, getPassword(actualWorkflow)); + assertEquals(secretKey, getSecretKey(actualWorkflow)); + assertEquals(expectedWorkflow, actualWorkflow); } @Test void testExceptionConnection() { - CreateDatabaseService.DatabaseServiceType databaseServiceType = CreateDatabaseService.DatabaseServiceType.Mysql; - String connectionName = "test"; Map mysqlConnection = Map.of("password", "openmetadata-test", "username1", "openmetadata-test"); - InvalidServiceConnectionException thrown = Assertions.assertThrows( InvalidServiceConnectionException.class, () -> - secretsManager.encryptOrDecryptServiceConnectionConfig( - mysqlConnection, databaseServiceType.value(), connectionName, ServiceType.DATABASE, true)); + secretsManager.encryptServiceConnectionConfig( + mysqlConnection, Mysql.value(), "test", ServiceType.DATABASE)); Assertions.assertEquals( "Failed to encrypt 'Mysql' connection stored in DB due to an unrecognized field: 'username1'", @@ -126,9 +183,7 @@ public abstract class ExternalSecretsManagerTest { thrown = Assertions.assertThrows( InvalidServiceConnectionException.class, - () -> - secretsManager.encryptOrDecryptServiceConnectionConfig( - mysqlConnection, databaseServiceType.value(), connectionName, ServiceType.DATABASE, false)); + () -> secretsManager.decryptServiceConnectionConfig(mysqlConnection, Mysql.value(), ServiceType.DATABASE)); Assertions.assertEquals( "Failed to decrypt 'Mysql' connection stored in DB due to an unrecognized field: 'username1'", @@ -142,131 +197,25 @@ public abstract class ExternalSecretsManagerTest { abstract void setUpSpecific(SecretsManagerConfiguration config); - void testEncryptDecryptServiceConnection(boolean decrypt) { - MysqlConnection expectedMysqlConnection = new MysqlConnection(); - expectedMysqlConnection.setPassword("openmetadata-test"); - CreateDatabaseService.DatabaseServiceType databaseServiceType = CreateDatabaseService.DatabaseServiceType.Mysql; - String connectionName = "test"; - - Map mysqlConnection = Map.of("password", "openmetadata-test"); - - MysqlConnection actualMysqlConnection = - (MysqlConnection) - secretsManager.encryptOrDecryptServiceConnectionConfig( - mysqlConnection, databaseServiceType.value(), connectionName, ServiceType.DATABASE, decrypt); - - if (decrypt) { - expectedMysqlConnection.setPassword("secret:/openmetadata/database/test/password"); - actualMysqlConnection.setPassword(Fernet.getInstance().decrypt(actualMysqlConnection.getPassword())); - } - - assertEquals(expectedMysqlConnection, actualMysqlConnection); - } - - void testEncryptDecryptSSOConfig(boolean decrypt) { - OktaSSOClientConfig config = new OktaSSOClientConfig(); - config.setPrivateKey(decrypt ? "secret:/openmetadata/bot/bot/config/authconfig/privatekey" : "this-is-a-test"); - AuthenticationMechanism expectedAuthenticationMechanism = - new AuthenticationMechanism() - .withAuthType(AuthenticationMechanism.AuthType.SSO) - .withConfig( - new SSOAuthMechanism().withAuthConfig(config).withSsoServiceType(SSOAuthMechanism.SsoServiceType.OKTA)); - - AuthenticationMechanism actualAuthenticationMechanism = - JsonUtils.convertValue(expectedAuthenticationMechanism, AuthenticationMechanism.class); - - secretsManager.encryptOrDecryptAuthenticationMechanism("bot", actualAuthenticationMechanism, decrypt); - - if (decrypt) { - String privateKey = - ((OktaSSOClientConfig) ((SSOAuthMechanism) actualAuthenticationMechanism.getConfig()).getAuthConfig()) - .getPrivateKey(); - ((OktaSSOClientConfig) ((SSOAuthMechanism) actualAuthenticationMechanism.getConfig()).getAuthConfig()) - .setPrivateKey(Fernet.getInstance().decrypt(privateKey)); - } - - assertEquals(expectedAuthenticationMechanism, actualAuthenticationMechanism); - } - - void testEncryptDecryptDBTConfig(boolean decrypt) { - IngestionPipeline expectedIngestionPipeline = - new IngestionPipeline() - .withName("my-pipeline") - .withPipelineType(PipelineType.DBT) - .withService(new DatabaseService().getEntityReference().withType(Entity.DATABASE_SERVICE)) - .withSourceConfig( - new SourceConfig() - .withConfig( - new DbtPipeline() - .withDbtConfigSource( - new DbtS3Config() - .withDbtSecurityConfig( - new AWSCredentials() - .withAwsSecretAccessKey("secret-password") - .withAwsRegion("eu-west-1"))))); - - IngestionPipeline actualIngestionPipeline = - JsonUtils.convertValue(expectedIngestionPipeline, IngestionPipeline.class); - - secretsManager.encryptOrDecryptIngestionPipeline(actualIngestionPipeline, decrypt); - - if (decrypt) { - DbtPipeline expectedDbtPipeline = ((DbtPipeline) expectedIngestionPipeline.getSourceConfig().getConfig()); - DbtPipeline actualDbtPipeline = ((DbtPipeline) actualIngestionPipeline.getSourceConfig().getConfig()); - ((DbtS3Config) expectedDbtPipeline.getDbtConfigSource()) - .getDbtSecurityConfig() - .setAwsSecretAccessKey( - "secret:/openmetadata/pipeline/my-pipeline/sourceconfig/config/dbtconfigsource/dbtsecurityconfig/awssecretaccesskey"); - ((DbtS3Config) actualDbtPipeline.getDbtConfigSource()) - .getDbtSecurityConfig() - .setAwsSecretAccessKey( - Fernet.getInstance() - .decrypt( - ((DbtS3Config) actualDbtPipeline.getDbtConfigSource()) - .getDbtSecurityConfig() - .getAwsSecretAccessKey())); - } - - assertEquals(expectedIngestionPipeline, actualIngestionPipeline); - } - - void testEncryptWorkflowObject(boolean encrypt) { - Workflow expectedWorkflow = - new Workflow() - .withName("my-workflow") - .withOpenMetadataServerConnection( - new OpenMetadataConnection() - .withSecurityConfig(new GoogleSSOClientConfig().withSecretKey("google-secret"))) - .withRequest( - new TestServiceConnectionRequest() - .withConnection( - new DatabaseConnection().withConfig(new MysqlConnection().withPassword("openmetadata-test"))) - .withServiceType(ServiceType.DATABASE) - .withConnectionType("Mysql")); - - Workflow workflow = JsonUtils.convertValue(expectedWorkflow, Workflow.class); - - Workflow actualWorkflow = secretsManager.encryptOrDecryptWorkflow(workflow, encrypt); - - if (encrypt) { - ((MysqlConnection) - ((DatabaseConnection) ((TestServiceConnectionRequest) expectedWorkflow.getRequest()).getConnection()) - .getConfig()) - .setPassword("secret:/openmetadata/workflow/my-workflow/request/connection/config/password"); - MysqlConnection mysqlConnection = - (MysqlConnection) - ((DatabaseConnection) ((TestServiceConnectionRequest) actualWorkflow.getRequest()).getConnection()) - .getConfig(); - mysqlConnection.setPassword(Fernet.getInstance().decrypt(mysqlConnection.getPassword())); - ((GoogleSSOClientConfig) (expectedWorkflow.getOpenMetadataServerConnection()).getSecurityConfig()) - .setSecretKey("secret:/openmetadata/serverconnection/securityconfig/secretkey"); - GoogleSSOClientConfig googleSSOClientConfig = - ((GoogleSSOClientConfig) (actualWorkflow.getOpenMetadataServerConnection()).getSecurityConfig()); - googleSSOClientConfig.setSecretKey(Fernet.getInstance().decrypt(googleSSOClientConfig.getSecretKey())); - } - - assertEquals(expectedWorkflow, actualWorkflow); - } - protected abstract SecretsManagerProvider expectedSecretManagerProvider(); + + private String getPrivateKey(AuthenticationMechanism authMechanism) { + return ((OktaSSOClientConfig) ((SSOAuthMechanism) authMechanism.getConfig()).getAuthConfig()).getPrivateKey(); + } + + private String getAwsSecretAccessKey(IngestionPipeline ingestionPipeline) { + DbtPipeline expectedDbtPipeline = ((DbtPipeline) ingestionPipeline.getSourceConfig().getConfig()); + return ((DbtS3Config) expectedDbtPipeline.getDbtConfigSource()).getDbtSecurityConfig().getAwsSecretAccessKey(); + } + + private String getPassword(Workflow workflow) { + return ((MysqlConnection) + ((DatabaseConnection) ((TestServiceConnectionRequest) workflow.getRequest()).getConnection()).getConfig()) + .getPassword(); + } + + private String getSecretKey(Workflow expectedWorkflow) { + return ((GoogleSSOClientConfig) (expectedWorkflow.getOpenMetadataServerConnection()).getSecurityConfig()) + .getSecretKey(); + } } diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/secrets/NoopSecretsManagerTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/secrets/NoopSecretsManagerTest.java index 96e5251d341..17592b25def 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/secrets/NoopSecretsManagerTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/secrets/NoopSecretsManagerTest.java @@ -16,6 +16,9 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.lenient; +import static org.openmetadata.schema.api.services.CreateDatabaseService.DatabaseServiceType.Mysql; +import static org.openmetadata.schema.api.services.CreateMlModelService.MlModelServiceType.Sklearn; +import static org.openmetadata.schema.entity.services.ServiceType.ML_MODEL; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -23,8 +26,6 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; -import org.openmetadata.schema.api.services.CreateDatabaseService; -import org.openmetadata.schema.api.services.CreateMlModelService; import org.openmetadata.schema.entity.services.ServiceType; import org.openmetadata.schema.security.secrets.SecretsManagerProvider; import org.openmetadata.schema.services.connections.database.MysqlConnection; @@ -34,8 +35,6 @@ import org.openmetadata.service.fernet.Fernet; @ExtendWith(MockitoExtension.class) public class NoopSecretsManagerTest { - private static final boolean ENCRYPT = true; - private static final boolean DECRYPT = false; private static final String ENCRYPTED_VALUE = "fernet:abcdef"; private static final String DECRYPTED_VALUE = "123456"; private static NoopSecretsManager secretsManager; @@ -58,22 +57,26 @@ public class NoopSecretsManagerTest { @Test void testEncryptDatabaseServiceConnectionConfig() { - testEncryptDecryptServiceConnection(ENCRYPT); + testEncryptServiceConnection(); } @Test void testDecryptDatabaseServiceConnectionConfig() { - testEncryptDecryptServiceConnection(DECRYPT); + testDecryptServiceConnection(); } @Test void testEncryptServiceConnectionWithoutPassword() { - testEncryptDecryptServiceConnectionWithoutPassword(ENCRYPT); + SklearnConnection connection = new SklearnConnection(); + Object actualConfig = secretsManager.encryptServiceConnectionConfig(connection, Sklearn.value(), "test", ML_MODEL); + assertNotSame(connection, actualConfig); } @Test - void testEncryptDecryptServiceConnectionWithoutPassword() { - testEncryptDecryptServiceConnectionWithoutPassword(DECRYPT); + void testDecryptServiceConnectionWithoutPassword() { + SklearnConnection connection = new SklearnConnection(); + Object actualConfig = secretsManager.decryptServiceConnectionConfig(connection, Sklearn.value(), ML_MODEL); + assertNotSame(connection, actualConfig); } @Test @@ -81,29 +84,19 @@ public class NoopSecretsManagerTest { assertEquals(SecretsManagerProvider.NOOP, secretsManager.getSecretsManagerProvider()); } - private void testEncryptDecryptServiceConnectionWithoutPassword(boolean decrypt) { - SklearnConnection sklearnConnection = new SklearnConnection(); - CreateMlModelService.MlModelServiceType databaseServiceType = CreateMlModelService.MlModelServiceType.Sklearn; - String connectionName = "test"; - + private void testEncryptServiceConnection() { + MysqlConnection connection = new MysqlConnection().withPassword(ENCRYPTED_VALUE); Object actualConfig = - secretsManager.encryptOrDecryptServiceConnectionConfig( - sklearnConnection, databaseServiceType.value(), connectionName, ServiceType.ML_MODEL, decrypt); - - assertNotSame(sklearnConnection, actualConfig); + secretsManager.encryptServiceConnectionConfig(connection, Mysql.value(), "test", ServiceType.DATABASE); + assertEquals(ENCRYPTED_VALUE, ((MysqlConnection) actualConfig).getPassword()); + assertNotSame(connection, actualConfig); } - private void testEncryptDecryptServiceConnection(boolean encrypt) { - MysqlConnection mysqlConnection = new MysqlConnection(); - mysqlConnection.setPassword(encrypt ? ENCRYPTED_VALUE : DECRYPTED_VALUE); - CreateDatabaseService.DatabaseServiceType databaseServiceType = CreateDatabaseService.DatabaseServiceType.Mysql; - String connectionName = "test"; - + private void testDecryptServiceConnection() { + MysqlConnection mysqlConnection = new MysqlConnection().withPassword(DECRYPTED_VALUE); Object actualConfig = - secretsManager.encryptOrDecryptServiceConnectionConfig( - mysqlConnection, databaseServiceType.value(), connectionName, ServiceType.DATABASE, encrypt); - - assertEquals(encrypt ? ENCRYPTED_VALUE : DECRYPTED_VALUE, ((MysqlConnection) actualConfig).getPassword()); + secretsManager.decryptServiceConnectionConfig(mysqlConnection, Mysql.value(), ServiceType.DATABASE); + assertEquals(DECRYPTED_VALUE, ((MysqlConnection) actualConfig).getPassword()); assertNotSame(mysqlConnection, actualConfig); } }