From 160984baf1ee5aca582e48d0c6578ea96d60bf45 Mon Sep 17 00:00:00 2001 From: Sriharsha Chintalapani Date: Tue, 23 May 2023 23:54:14 -0700 Subject: [PATCH] Fix #11673: Service connection details will be viewable based on view permissions and by default masked for users and unmasked for bots (#11738) --- conf/openmetadata.yaml | 4 - .../service/OpenMetadataApplication.java | 2 +- .../OpenMetadataApplicationConfig.java | 4 - .../services/ServiceEntityResource.java | 35 +++++--- .../secrets/masker/EntityMaskerFactory.java | 6 +- .../secrets/masker/NoopEntityMasker.java | 68 ---------------- .../secrets/masker/PasswordEntityMasker.java | 2 +- .../DashboardServiceResourceTest.java | 45 +++-------- .../services/DatabaseServiceResourceTest.java | 80 ++++++++++++------- .../services/MetadataServiceResourceTest.java | 12 --- .../services/PipelineServiceResourceTest.java | 28 ++----- .../services/StorageServiceResourceTest.java | 1 - .../IngestionPipelineResourceTest.java | 18 +++-- .../masker/EntityMaskerFactoryTest.java | 8 +- .../secrets/masker/NoopEntityMaskerTest.java | 8 -- .../masker/PasswordEntityMaskerTest.java | 4 +- .../secrets/masker/TestEntityMasker.java | 36 ++++----- .../resources/openmetadata-secure-test.yaml | 4 +- 18 files changed, 129 insertions(+), 236 deletions(-) delete mode 100644 openmetadata-service/src/main/java/org/openmetadata/service/secrets/masker/NoopEntityMasker.java delete mode 100644 openmetadata-service/src/test/java/org/openmetadata/service/secrets/masker/NoopEntityMaskerTest.java diff --git a/conf/openmetadata.yaml b/conf/openmetadata.yaml index b79dc15a675..550681aedaa 100644 --- a/conf/openmetadata.yaml +++ b/conf/openmetadata.yaml @@ -278,10 +278,6 @@ secretsManagerConfiguration: accessKeyId: ${OM_SM_ACCESS_KEY_ID:-""} secretAccessKey: ${OM_SM_ACCESS_KEY:-""} -security: -# it will mask all the password fields in the responses sent from the API except for the bots - maskPasswordsAPI: ${MASK_PASSWORDS_API:-false} - health: delayedShutdownHandlerEnabled: true shutdownWaitPeriod: 1s diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/OpenMetadataApplication.java b/openmetadata-service/src/main/java/org/openmetadata/service/OpenMetadataApplication.java index 51f377681fb..3059d8ec1e9 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/OpenMetadataApplication.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/OpenMetadataApplication.java @@ -140,7 +140,7 @@ public class OpenMetadataApplication extends Application updateEntity(update, OK, ADMIN_AUTH_HEADERS), BAD_REQUEST, - "InvalidServiceConnectionException for service [Snowflake] due to [Failed to encrypt connection instance of Snowflake]"); + "InvalidServiceConnectionException for service [Snowflake] due to [Failed to unmask connection instance of Snowflake]."); } @Test @@ -280,24 +283,16 @@ public class DatabaseServiceResourceTest extends EntityResourceTest authHeaders) { assertEquals(createRequest.getName(), service.getName()); - validateDatabaseConnection(createRequest.getConnection(), service.getConnection(), service.getServiceType()); + boolean maskPasswords = true; + if (INGESTION_BOT_AUTH_HEADERS.equals(authHeaders)) { + maskPasswords = false; + } + validateDatabaseConnection( + createRequest.getConnection(), service.getConnection(), service.getServiceType(), maskPasswords); } @Override @@ -340,7 +335,8 @@ public class DatabaseServiceResourceTest extends EntityResourceTest authHeaders) { diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/services/PipelineServiceResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/services/PipelineServiceResourceTest.java index 274cab66132..1686428e6ea 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/services/PipelineServiceResourceTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/services/PipelineServiceResourceTest.java @@ -145,7 +145,7 @@ public class PipelineServiceResourceTest extends EntityResourceTest { - EntityMaskerFactory.createEntityMasker(CONFIG) + EntityMaskerFactory.createEntityMasker() .maskServiceConnectionConfig(mysqlConnectionObject, "Mysql", ServiceType.DATABASE); }); @@ -38,7 +38,7 @@ public class PasswordEntityMaskerTest extends TestEntityMasker { Assertions.assertThrows( EntityMaskException.class, () -> { - EntityMaskerFactory.createEntityMasker(CONFIG) + EntityMaskerFactory.createEntityMasker() .unmaskServiceConnectionConfig( mysqlConnectionObject, new MysqlConnection(), "Mysql", ServiceType.DATABASE); }); diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/secrets/masker/TestEntityMasker.java b/openmetadata-service/src/test/java/org/openmetadata/service/secrets/masker/TestEntityMasker.java index 9f50321ab20..3cf448a2937 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/secrets/masker/TestEntityMasker.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/secrets/masker/TestEntityMasker.java @@ -48,13 +48,13 @@ abstract class TestEntityMasker { AirflowConnection airflowConnection = new AirflowConnection().withConnection(buildMysqlConnection()); AirflowConnection masked = (AirflowConnection) - EntityMaskerFactory.createEntityMasker(CONFIG) + EntityMaskerFactory.createEntityMasker() .maskServiceConnectionConfig(airflowConnection, "Airflow", ServiceType.PIPELINE); assertNotNull(masked); assertEquals(((MysqlConnection) masked.getConnection()).getPassword(), getMaskedPassword()); AirflowConnection unmasked = (AirflowConnection) - EntityMaskerFactory.createEntityMasker(CONFIG) + EntityMaskerFactory.createEntityMasker() .unmaskServiceConnectionConfig(masked, airflowConnection, "Airflow", ServiceType.PIPELINE); assertEquals(((MysqlConnection) unmasked.getConnection()).getPassword(), PASSWORD); } @@ -64,13 +64,13 @@ abstract class TestEntityMasker { BigQueryConnection bigQueryConnection = new BigQueryConnection().withCredentials(buildGcsCredentials()); BigQueryConnection masked = (BigQueryConnection) - EntityMaskerFactory.createEntityMasker(CONFIG) + EntityMaskerFactory.createEntityMasker() .maskServiceConnectionConfig(bigQueryConnection, "BigQuery", ServiceType.DATABASE); assertNotNull(masked); assertEquals(getPrivateKeyFromGcsConfig(masked.getCredentials()), getMaskedPassword()); BigQueryConnection unmasked = (BigQueryConnection) - EntityMaskerFactory.createEntityMasker(CONFIG) + EntityMaskerFactory.createEntityMasker() .unmaskServiceConnectionConfig(masked, bigQueryConnection, "BigQuery", ServiceType.DATABASE); assertEquals(getPrivateKeyFromGcsConfig(unmasked.getCredentials()), PASSWORD); } @@ -80,14 +80,14 @@ abstract class TestEntityMasker { DatalakeConnection datalakeConnection = new DatalakeConnection().withConfigSource(buildGcsConfig()); DatalakeConnection masked = (DatalakeConnection) - EntityMaskerFactory.createEntityMasker(CONFIG) + EntityMaskerFactory.createEntityMasker() .maskServiceConnectionConfig(datalakeConnection, "Datalake", ServiceType.DATABASE); assertNotNull(masked); assertEquals( getPrivateKeyFromGcsConfig(((GCSConfig) masked.getConfigSource()).getSecurityConfig()), getMaskedPassword()); DatalakeConnection unmasked = (DatalakeConnection) - EntityMaskerFactory.createEntityMasker(CONFIG) + EntityMaskerFactory.createEntityMasker() .unmaskServiceConnectionConfig(masked, datalakeConnection, "Datalake", ServiceType.DATABASE); assertEquals(getPrivateKeyFromGcsConfig(((GCSConfig) unmasked.getConfigSource()).getSecurityConfig()), PASSWORD); } @@ -96,7 +96,7 @@ abstract class TestEntityMasker { void testDbtPipelineMasker() { IngestionPipeline dbtPipeline = buildIngestionPipeline(); IngestionPipeline originalDbtPipeline = buildIngestionPipeline(); - EntityMaskerFactory.createEntityMasker(CONFIG).maskIngestionPipeline(dbtPipeline); + EntityMaskerFactory.createEntityMasker().maskIngestionPipeline(dbtPipeline); assertNotNull(dbtPipeline); assertEquals( getPrivateKeyFromGcsConfig( @@ -106,7 +106,7 @@ abstract class TestEntityMasker { assertEquals( ((GoogleSSOClientConfig) dbtPipeline.getOpenMetadataServerConnection().getSecurityConfig()).getSecretKey(), getMaskedPassword()); - EntityMaskerFactory.createEntityMasker(CONFIG).unmaskIngestionPipeline(dbtPipeline, originalDbtPipeline); + EntityMaskerFactory.createEntityMasker().unmaskIngestionPipeline(dbtPipeline, originalDbtPipeline); assertEquals( getPrivateKeyFromGcsConfig( ((DbtGCSConfig) ((DbtPipeline) dbtPipeline.getSourceConfig().getConfig()).getDbtConfigSource()) @@ -123,13 +123,13 @@ abstract class TestEntityMasker { buildAuthenticationMechanism(AuthenticationMechanism.AuthType.SSO); AuthenticationMechanism originalSsoAuthenticationMechanism = buildAuthenticationMechanism(AuthenticationMechanism.AuthType.SSO); - EntityMaskerFactory.createEntityMasker(CONFIG).maskAuthenticationMechanism("test", authenticationMechanism); + EntityMaskerFactory.createEntityMasker().maskAuthenticationMechanism("test", authenticationMechanism); assertNotNull(authenticationMechanism.getConfig()); assertEquals( ((GoogleSSOClientConfig) ((SSOAuthMechanism) authenticationMechanism.getConfig()).getAuthConfig()) .getSecretKey(), getMaskedPassword()); - EntityMaskerFactory.createEntityMasker(CONFIG) + EntityMaskerFactory.createEntityMasker() .unmaskAuthenticationMechanism("test", authenticationMechanism, originalSsoAuthenticationMechanism); assertEquals( ((GoogleSSOClientConfig) ((SSOAuthMechanism) authenticationMechanism.getConfig()).getAuthConfig()) @@ -143,9 +143,9 @@ abstract class TestEntityMasker { buildAuthenticationMechanism(AuthenticationMechanism.AuthType.JWT); AuthenticationMechanism originalSsoAuthenticationMechanism = buildAuthenticationMechanism(AuthenticationMechanism.AuthType.JWT); - EntityMaskerFactory.createEntityMasker(CONFIG).maskAuthenticationMechanism("test", authenticationMechanism); + EntityMaskerFactory.createEntityMasker().maskAuthenticationMechanism("test", authenticationMechanism); assertTrue(authenticationMechanism.getConfig() instanceof JWTAuthMechanism); - EntityMaskerFactory.createEntityMasker(CONFIG) + EntityMaskerFactory.createEntityMasker() .unmaskAuthenticationMechanism("test", authenticationMechanism, originalSsoAuthenticationMechanism); assertTrue(authenticationMechanism.getConfig() instanceof JWTAuthMechanism); } @@ -155,13 +155,13 @@ abstract class TestEntityMasker { SupersetConnection supersetConnection = new SupersetConnection().withConnection(buildMysqlConnection()); SupersetConnection masked = (SupersetConnection) - EntityMaskerFactory.createEntityMasker(CONFIG) + EntityMaskerFactory.createEntityMasker() .maskServiceConnectionConfig(supersetConnection, "Superset", ServiceType.DASHBOARD); assertNotNull(masked); assertEquals(((MysqlConnection) masked.getConnection()).getPassword(), getMaskedPassword()); SupersetConnection unmasked = (SupersetConnection) - EntityMaskerFactory.createEntityMasker(CONFIG) + EntityMaskerFactory.createEntityMasker() .unmaskServiceConnectionConfig(masked, supersetConnection, "Superset", ServiceType.DASHBOARD); assertEquals(((MysqlConnection) unmasked.getConnection()).getPassword(), PASSWORD); } @@ -176,7 +176,7 @@ abstract class TestEntityMasker { .withServiceType(ServiceType.DATABASE) .withConnectionType("Mysql")) .withOpenMetadataServerConnection(buildOpenMetadataConnection()); - Workflow masked = EntityMaskerFactory.createEntityMasker(CONFIG).maskWorkflow(workflow); + Workflow masked = EntityMaskerFactory.createEntityMasker().maskWorkflow(workflow); assertNotNull(masked); assertEquals( ((MysqlConnection) @@ -186,7 +186,7 @@ abstract class TestEntityMasker { assertEquals( ((GoogleSSOClientConfig) masked.getOpenMetadataServerConnection().getSecurityConfig()).getSecretKey(), getMaskedPassword()); - Workflow unmasked = EntityMaskerFactory.createEntityMasker(CONFIG).unmaskWorkflow(masked, workflow); + Workflow unmasked = EntityMaskerFactory.createEntityMasker().unmaskWorkflow(masked, workflow); assertEquals( ((MysqlConnection) ((DatabaseConnection) ((TestServiceConnectionRequest) unmasked.getRequest()).getConnection()) @@ -203,13 +203,13 @@ abstract class TestEntityMasker { MysqlConnection mysqlConnection = buildMysqlConnection(); MysqlConnection masked = (MysqlConnection) - EntityMaskerFactory.createEntityMasker(CONFIG) + EntityMaskerFactory.createEntityMasker() .maskServiceConnectionConfig(mysqlConnection, "Mysql", ServiceType.DATABASE); assertNotNull(masked); assertEquals(masked.getPassword(), getMaskedPassword()); MysqlConnection unmasked = (MysqlConnection) - EntityMaskerFactory.createEntityMasker(CONFIG) + EntityMaskerFactory.createEntityMasker() .unmaskServiceConnectionConfig(masked, mysqlConnection, "Mysql", ServiceType.DATABASE); assertEquals(unmasked.getPassword(), PASSWORD); } diff --git a/openmetadata-service/src/test/resources/openmetadata-secure-test.yaml b/openmetadata-service/src/test/resources/openmetadata-secure-test.yaml index 2593098d441..09320ab119b 100644 --- a/openmetadata-service/src/test/resources/openmetadata-secure-test.yaml +++ b/openmetadata-service/src/test/resources/openmetadata-secure-test.yaml @@ -116,10 +116,8 @@ migrationConfiguration: # port: 0 secretsManagerConfiguration: - secretsManager: in-memory + secretsManager: noop -security: - maskPasswordsAPI: false health: delayedShutdownHandlerEnabled: true