From 82228a10981d71a024276ae41b6196b8e2a6a419 Mon Sep 17 00:00:00 2001 From: Pere Miquel Brull Date: Thu, 18 Jul 2024 12:03:22 +0200 Subject: [PATCH 01/33] MINOR - Clean automations_workflow (#17005) --- .../sql/migrations/native/1.5.0/mysql/schemaChanges.sql | 7 +++++-- .../sql/migrations/native/1.5.0/postgres/schemaChanges.sql | 6 +++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/bootstrap/sql/migrations/native/1.5.0/mysql/schemaChanges.sql b/bootstrap/sql/migrations/native/1.5.0/mysql/schemaChanges.sql index d66c8e816131..36f3cf6daede 100644 --- a/bootstrap/sql/migrations/native/1.5.0/mysql/schemaChanges.sql +++ b/bootstrap/sql/migrations/native/1.5.0/mysql/schemaChanges.sql @@ -76,11 +76,14 @@ CREATE TABLE IF NOT EXISTS api_endpoint_entity ( INDEX (name) ); + +-- Clean dangling workflows not removed after test connection +truncate automations_workflow; + -- Remove date, dateTime, time from type_entity, as they are no more om-field-types, instead we have date-cp, time-cp, dateTime-cp as om-field-types DELETE FROM type_entity WHERE name IN ('date', 'dateTime', 'time'); - -- Update BigQuery,Bigtable & Datalake model for gcpCredentials to move `gcpConfig` value to `gcpConfig.path` UPDATE dbservice_entity SET json = JSON_INSERT( @@ -144,4 +147,4 @@ SET json = JSON_INSERT( JSON_EXTRACT(json, '$.sourceConfig.config.dbtConfigSource.dbtSecurityConfig.gcpConfig.type') OR JSON_EXTRACT(json, '$.sourceConfig.config.dbtConfigSource.dbtSecurityConfig.gcpConfig.externalType') OR JSON_EXTRACT(json, '$.sourceConfig.config.dbtConfigSource.dbtSecurityConfig.gcpConfig.path') -) is NULL AND JSON_EXTRACT(json, '$.sourceConfig.config.dbtConfigSource.dbtSecurityConfig.gcpConfig') is not null; \ No newline at end of file +) is NULL AND JSON_EXTRACT(json, '$.sourceConfig.config.dbtConfigSource.dbtSecurityConfig.gcpConfig') is not null; diff --git a/bootstrap/sql/migrations/native/1.5.0/postgres/schemaChanges.sql b/bootstrap/sql/migrations/native/1.5.0/postgres/schemaChanges.sql index b1ddaa2969e8..ee74264e75ea 100644 --- a/bootstrap/sql/migrations/native/1.5.0/postgres/schemaChanges.sql +++ b/bootstrap/sql/migrations/native/1.5.0/postgres/schemaChanges.sql @@ -70,6 +70,10 @@ CREATE TABLE IF NOT EXISTS api_endpoint_entity ( UNIQUE (fqnHash) ); + +-- Clean dangling workflows not removed after test connection +truncate automations_workflow; + -- Remove date, dateTime, time from type_entity, as they are no more om-field-types, instead we have date-cp, time-cp, dateTime-cp as om-field-types DELETE FROM type_entity WHERE name IN ('date', 'dateTime', 'time'); @@ -133,4 +137,4 @@ SET json = jsonb_set( AND json#>>'{sourceConfig,config,dbtConfigSource,dbtSecurityConfig,gcpConfig}' IS NOT NULL AND json#>>'{sourceConfig,config,dbtConfigSource,dbtSecurityConfig,gcpConfig,type}' IS NULL AND json#>>'{sourceConfig,config,dbtConfigSource,dbtSecurityConfig,gcpConfig,externalType}' IS NULL - AND json#>>'{sourceConfig,config,dbtConfigSource,dbtSecurityConfig,gcpConfig,path}' IS NULL; \ No newline at end of file + AND json#>>'{sourceConfig,config,dbtConfigSource,dbtSecurityConfig,gcpConfig,path}' IS NULL; From 43b4e392589f59a9def6ee67d00d27997b89ddbd Mon Sep 17 00:00:00 2001 From: IceS2 Date: Thu, 18 Jul 2024 12:20:55 +0200 Subject: [PATCH 02/33] Update Snowflake docs to better inform the needed permissions (#17070) --- .../connectors/database/snowflake/index.md | 20 +++++++++++++------ .../connectors/database/snowflake/index.md | 20 +++++++++++++------ 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/openmetadata-docs/content/v1.4.x/connectors/database/snowflake/index.md b/openmetadata-docs/content/v1.4.x/connectors/database/snowflake/index.md index 13ab3cbb3be3..92f5e3369403 100644 --- a/openmetadata-docs/content/v1.4.x/connectors/database/snowflake/index.md +++ b/openmetadata-docs/content/v1.4.x/connectors/database/snowflake/index.md @@ -61,14 +61,22 @@ GRANT SELECT ON ALL VIEWS IN SCHEMA TEST_SCHEMA TO ROLE NEW_ROLE; GRANT SELECT ON ALL DYNAMIC TABLES IN SCHEMA TEST_SCHEMA TO ROLE NEW_ROLE; ``` -While running the usage workflow, Openmetadata fetches the query logs by querying `snowflake.account_usage.query_history` table. For this the snowflake user should be granted the `ACCOUNTADMIN` role or a role granted IMPORTED PRIVILEGES on the database `SNOWFLAKE`. +{% note %} +If running any of: + - Incremental Extraction + - Ingesting Tags + - Usage Workflow -```sql --- Grant IMPORTED PRIVILEGES on all Schemas of SNOWFLAKE DB to New Role -GRANT IMPORTED PRIVILEGES ON ALL SCHEMAS IN DATABASE SNOWFLAKE TO ROLE NEW_ROLE; -``` +The following Grant is needed +{% /note %} + +- **Incremental Extraction**: Openmetadata fetches the information by querying `snowflake.account_usage.tables`. + +- **Ingesting Tags**: Openmetadata fetches the information by querying `snowflake.account_usage.tag_references`. + +- **Usage Workflow**: Openmetadata fetches the query logs by querying `snowflake.account_usage.query_history` table. For this the snowflake user should be granted the `ACCOUNTADMIN` role or a role granted IMPORTED PRIVILEGES on the database `SNOWFLAKE`. -If ingesting tags, the user should also have permissions to query `snowflake.account_usage.tag_references`.For this the snowflake user should be granted the `ACCOUNTADMIN` role or a role granted IMPORTED PRIVILEGES on the database +In order to be able to query those tables, the user should be either granted the `ACCOUNTADMIN` role or a role with the `IMPORTED PRIVILEGES` grant on the `SNOWFLAKE` database: ```sql -- Grant IMPORTED PRIVILEGES on all Schemas of SNOWFLAKE DB to New Role diff --git a/openmetadata-docs/content/v1.5.x-SNAPSHOT/connectors/database/snowflake/index.md b/openmetadata-docs/content/v1.5.x-SNAPSHOT/connectors/database/snowflake/index.md index 68b37c3a3893..a586ea97d0de 100644 --- a/openmetadata-docs/content/v1.5.x-SNAPSHOT/connectors/database/snowflake/index.md +++ b/openmetadata-docs/content/v1.5.x-SNAPSHOT/connectors/database/snowflake/index.md @@ -61,14 +61,22 @@ GRANT SELECT ON ALL VIEWS IN SCHEMA TEST_SCHEMA TO ROLE NEW_ROLE; GRANT SELECT ON ALL DYNAMIC TABLES IN SCHEMA TEST_SCHEMA TO ROLE NEW_ROLE; ``` -While running the usage workflow, Openmetadata fetches the query logs by querying `snowflake.account_usage.query_history` table. For this the snowflake user should be granted the `ACCOUNTADMIN` role or a role granted IMPORTED PRIVILEGES on the database `SNOWFLAKE`. +{% note %} +If running any of: + - Incremental Extraction + - Ingesting Tags + - Usage Workflow -```sql --- Grant IMPORTED PRIVILEGES on all Schemas of SNOWFLAKE DB to New Role -GRANT IMPORTED PRIVILEGES ON ALL SCHEMAS IN DATABASE SNOWFLAKE TO ROLE NEW_ROLE; -``` +The following Grant is needed +{% /note %} + +- **Incremental Extraction**: Openmetadata fetches the information by querying `snowflake.account_usage.tables`. + +- **Ingesting Tags**: Openmetadata fetches the information by querying `snowflake.account_usage.tag_references`. + +- **Usage Workflow**: Openmetadata fetches the query logs by querying `snowflake.account_usage.query_history` table. For this the snowflake user should be granted the `ACCOUNTADMIN` role or a role granted IMPORTED PRIVILEGES on the database `SNOWFLAKE`. -If ingesting tags, the user should also have permissions to query `snowflake.account_usage.tag_references`.For this the snowflake user should be granted the `ACCOUNTADMIN` role or a role granted IMPORTED PRIVILEGES on the database +In order to be able to query those tables, the user should be either granted the `ACCOUNTADMIN` role or a role with the `IMPORTED PRIVILEGES` grant on the `SNOWFLAKE` database: ```sql -- Grant IMPORTED PRIVILEGES on all Schemas of SNOWFLAKE DB to New Role From 552983446afe3186245b8c3608f5dde0cf4417a5 Mon Sep 17 00:00:00 2001 From: Sachin Chaurasiya Date: Thu, 18 Jul 2024 17:20:48 +0530 Subject: [PATCH 03/33] minor: add apiService, apiEndpoint and apiCollection entity SearchIndexingApplication-collate.json (#17067) --- .../resources/json/data/app/SearchIndexingApplication.json | 5 ++++- .../appMarketPlaceDefinition/SearchIndexingApplication.json | 5 ++++- .../utils/ApplicationSchemas/SearchIndexingApplication.json | 5 ++++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/openmetadata-service/src/main/resources/json/data/app/SearchIndexingApplication.json b/openmetadata-service/src/main/resources/json/data/app/SearchIndexingApplication.json index a9939936e444..189a3751009e 100644 --- a/openmetadata-service/src/main/resources/json/data/app/SearchIndexingApplication.json +++ b/openmetadata-service/src/main/resources/json/data/app/SearchIndexingApplication.json @@ -38,7 +38,10 @@ "domain", "storedProcedure", "dataProduct", - "testCaseResolutionStatus" + "testCaseResolutionStatus", + "apiService", + "apiEndpoint", + "apiCollection" ], "recreateIndex": true, "batchSize": "100", diff --git a/openmetadata-service/src/main/resources/json/data/appMarketPlaceDefinition/SearchIndexingApplication.json b/openmetadata-service/src/main/resources/json/data/appMarketPlaceDefinition/SearchIndexingApplication.json index 90d6dbfc4c14..2dd3c5eec87b 100644 --- a/openmetadata-service/src/main/resources/json/data/appMarketPlaceDefinition/SearchIndexingApplication.json +++ b/openmetadata-service/src/main/resources/json/data/appMarketPlaceDefinition/SearchIndexingApplication.json @@ -52,7 +52,10 @@ "domain", "storedProcedure", "dataProduct", - "testCaseResolutionStatus" + "testCaseResolutionStatus", + "apiService", + "apiEndpoint", + "apiCollection" ], "recreateIndex": true, "batchSize": "100", diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/ApplicationSchemas/SearchIndexingApplication.json b/openmetadata-ui/src/main/resources/ui/src/utils/ApplicationSchemas/SearchIndexingApplication.json index 93e58de68afd..eb98bd39bff1 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/ApplicationSchemas/SearchIndexingApplication.json +++ b/openmetadata-ui/src/main/resources/ui/src/utils/ApplicationSchemas/SearchIndexingApplication.json @@ -52,7 +52,10 @@ "domain", "storedProcedure", "dataProduct", - "testCaseResolutionStatus" + "testCaseResolutionStatus", + "apiService", + "apiEndpoint", + "apiCollection" ] }, "default": ["all"], From 25a0a2ccf4402300e78fdf1332efa213b1c82ddd Mon Sep 17 00:00:00 2001 From: Ashish Gupta Date: Thu, 18 Jul 2024 17:53:37 +0530 Subject: [PATCH 04/33] fix entity icon overlaping on text (#17072) --- .../Entity/EntityHeaderTitle/EntityHeaderTitle.component.tsx | 2 +- .../Entity/EntityHeaderTitle/entity-header-title.less | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Entity/EntityHeaderTitle/EntityHeaderTitle.component.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Entity/EntityHeaderTitle/EntityHeaderTitle.component.tsx index d1044d8de227..1e3ca56c9c88 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Entity/EntityHeaderTitle/EntityHeaderTitle.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Entity/EntityHeaderTitle/EntityHeaderTitle.component.tsx @@ -52,7 +52,7 @@ const EntityHeaderTitle = ({ data-testid={`${serviceName}-${name}`} gutter={12} wrap={false}> - {icon} + {icon} Date: Thu, 18 Jul 2024 14:54:15 +0200 Subject: [PATCH 05/33] Fix Vertica E2E Counts (#17071) --- ingestion/tests/cli_e2e/test_cli_vertica.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ingestion/tests/cli_e2e/test_cli_vertica.py b/ingestion/tests/cli_e2e/test_cli_vertica.py index 5b73a332e29d..6abe0a1d2e97 100644 --- a/ingestion/tests/cli_e2e/test_cli_vertica.py +++ b/ingestion/tests/cli_e2e/test_cli_vertica.py @@ -24,9 +24,9 @@ class VerticaCliTest(CliCommonDB.TestSuite, SQACommonMethods): create_table_query: str = """ - CREATE TABLE vendor_dimension_new AS - SELECT * - FROM vendor_dimension + CREATE TABLE vendor_dimension_new AS + SELECT * + FROM vendor_dimension WHERE 1=0; """ @@ -99,7 +99,7 @@ def expected_filtered_schema_excludes() -> int: @staticmethod def expected_filtered_table_includes() -> int: - return 8 + return 5 @staticmethod def expected_filtered_table_excludes() -> int: @@ -107,4 +107,4 @@ def expected_filtered_table_excludes() -> int: @staticmethod def expected_filtered_mix() -> int: - return 7 + return 4 From e4b31f9d6580ed1db46f37baf13b94c5aae5ec4b Mon Sep 17 00:00:00 2001 From: Teddy Date: Thu, 18 Jul 2024 16:47:05 +0200 Subject: [PATCH 06/33] fix: supportedDataType PSQL query (#17069) --- .../openmetadata/service/jdbi3/CollectionDAO.java | 15 +++++++++------ .../dqtests/TestDefinitionResourceTest.java | 11 +++++++++++ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java index 0a424b153945..d12d43203c59 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/CollectionDAO.java @@ -3688,8 +3688,9 @@ default List listBefore(ListFilter filter, int limit, String before) { if (supportedDataType != null) { filter.queryParams.put("supportedDataTypeLike", String.format("%%%s%%", supportedDataType)); - mysqlCondition.append("AND supported_data_types LIKE :supportedDataTypeLike"); - psqlCondition.append("AND supported_data_types @> :supportedDataTypeLike "); + mysqlCondition.append( + "AND json_extract(json, '$.supportedDataTypes') LIKE :supportedDataTypeLike "); + psqlCondition.append("AND json->>'supportedDataTypes' LIKE :supportedDataTypeLike "); } return listBefore( @@ -3731,8 +3732,9 @@ default List listAfter(ListFilter filter, int limit, String after) { if (supportedDataType != null) { filter.queryParams.put("supportedDataTypeLike", String.format("%%%s%%", supportedDataType)); - mysqlCondition.append("AND supported_data_types LIKE :supportedDataTypeLike"); - psqlCondition.append("AND supported_data_types @> :supportedDataTypeLike "); + mysqlCondition.append( + "AND json_extract(json, '$.supportedDataTypes') LIKE :supportedDataTypeLike "); + psqlCondition.append("AND json->>'supportedDataTypes' LIKE :supportedDataTypeLike "); } return listAfter( @@ -3774,8 +3776,9 @@ default int listCount(ListFilter filter) { if (supportedDataType != null) { filter.queryParams.put("supportedDataTypeLike", String.format("%%%s%%", supportedDataType)); - mysqlCondition.append("AND supported_data_types LIKE :supportedDataTypeLike"); - psqlCondition.append("AND supported_data_types @> :supportedDataTypeLike "); + mysqlCondition.append( + "AND json_extract(json, '$.supportedDataTypes') LIKE :supportedDataTypeLike "); + psqlCondition.append("AND json->>'supportedDataTypes' LIKE :supportedDataTypeLike "); } return listCount( getTableName(), diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestDefinitionResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestDefinitionResourceTest.java index 11e85e7aaa18..4e62a052d85b 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestDefinitionResourceTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestDefinitionResourceTest.java @@ -21,6 +21,7 @@ import org.openmetadata.schema.tests.TestCaseParameter; import org.openmetadata.schema.tests.TestDefinition; import org.openmetadata.schema.tests.TestPlatform; +import org.openmetadata.schema.type.ColumnDataType; import org.openmetadata.schema.type.TestDefinitionEntityType; import org.openmetadata.service.Entity; import org.openmetadata.service.resources.EntityResourceTest; @@ -51,6 +52,16 @@ public void setupTestDefinitions() throws IOException { "columnValuesMissingCount", "owner", ADMIN_AUTH_HEADERS); } + @Test + void list_testDefinitionsForBoolType(TestInfo test) throws HttpResponseException { + Map params = Map.of("supportedDataType", "BOOLEAN"); + ResultList testDefinitions = listEntities(params, ADMIN_AUTH_HEADERS); + boolean b = + testDefinitions.getData().stream() + .allMatch(t -> t.getSupportedDataTypes().contains(ColumnDataType.BOOLEAN)); + Assertions.assertTrue(b); + } + @Test void post_testDefinitionWithoutRequiredFields_4xx(TestInfo test) { // Test Platform is required field From c340fe94f30eeaa05f3db08f930476b7ab277731 Mon Sep 17 00:00:00 2001 From: Karan Hotchandani <33024356+karanh37@users.noreply.github.com> Date: Thu, 18 Jul 2024 23:23:50 +0530 Subject: [PATCH 07/33] saml reject token renewal (#17079) --- .../SamlAuthenticator.test.tsx | 63 +++++++++++++++++++ .../AppAuthenticators/SamlAuthenticator.tsx | 4 +- 2 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 openmetadata-ui/src/main/resources/ui/src/components/Auth/AppAuthenticators/SamlAuthenticator.test.tsx diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Auth/AppAuthenticators/SamlAuthenticator.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Auth/AppAuthenticators/SamlAuthenticator.test.tsx new file mode 100644 index 000000000000..b30074e79e81 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/src/components/Auth/AppAuthenticators/SamlAuthenticator.test.tsx @@ -0,0 +1,63 @@ +/* + * Copyright 2024 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { render, screen } from '@testing-library/react'; +import React from 'react'; +import { MemoryRouter } from 'react-router-dom'; +import { AuthenticatorRef } from '../AuthProviders/AuthProvider.interface'; +import SamlAuthenticator from './SamlAuthenticator'; + +jest.mock('../../../hooks/useApplicationStore', () => { + return { + useApplicationStore: jest.fn(() => ({ + authConfig: {}, + getOidcToken: jest.fn().mockReturnValue({ isExpired: false }), + setOidcToken: jest.fn(), + })), + }; +}); + +describe('Test SamlAuthenticator component', () => { + it('Checks if the SamlAuthenticator renders', async () => { + const onLogoutMock = jest.fn(); + const authenticatorRef = null; + render( + +

+ , + { + wrapper: MemoryRouter, + } + ); + const children = screen.getByTestId('children'); + + expect(children).toBeInTheDocument(); + }); + + it('Rejects promise when renew id token is called', async () => { + const onLogoutMock = jest.fn(); + const authenticatorRef = React.createRef(); + + render( + +

+ , + { + wrapper: MemoryRouter, + } + ); + + await expect(authenticatorRef.current?.renewIdToken()).rejects.toEqual( + 'SAML authenticator does not support token renewal' + ); + }); +}); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Auth/AppAuthenticators/SamlAuthenticator.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Auth/AppAuthenticators/SamlAuthenticator.tsx index c53e87358066..5dc694168148 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Auth/AppAuthenticators/SamlAuthenticator.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Auth/AppAuthenticators/SamlAuthenticator.tsx @@ -84,7 +84,9 @@ const SamlAuthenticator = forwardRef( logout(); }, async renewIdToken() { - return Promise.resolve(''); + return Promise.reject( + 'SAML authenticator does not support token renewal' + ); }, })); From ebdd7f7fd9c11f1ba41e1cc4f95e222dfbab8d45 Mon Sep 17 00:00:00 2001 From: Mohit Yadav <105265192+mohityadav766@users.noreply.github.com> Date: Thu, 18 Jul 2024 23:53:50 +0530 Subject: [PATCH 08/33] Centralize OIDC Flow to handler, and refresh logic (#17082) * Centralize OIDC Flow to handler, and refresh logic * Remove forced condition * Return on success --- conf/openmetadata.yaml | 1 + .../service/OpenMetadataApplication.java | 43 +- .../service/security/AuthCallbackServlet.java | 267 +---- .../service/security/AuthLoginServlet.java | 152 +-- .../service/security/AuthLogoutServlet.java | 21 +- .../service/security/AuthRefreshServlet.java | 44 +- .../AuthenticationCodeFlowHandler.java | 912 ++++++++++++++++++ .../service/security/SecurityUtil.java | 343 +------ .../saml/SamlAssertionConsumerServlet.java | 2 +- .../security/client/oidcClientConfig.json | 5 + 10 files changed, 945 insertions(+), 845 deletions(-) create mode 100644 openmetadata-service/src/main/java/org/openmetadata/service/security/AuthenticationCodeFlowHandler.java diff --git a/conf/openmetadata.yaml b/conf/openmetadata.yaml index 527840ac282c..bb4a99248e97 100644 --- a/conf/openmetadata.yaml +++ b/conf/openmetadata.yaml @@ -192,6 +192,7 @@ authenticationConfiguration: clientAuthenticationMethod: ${OIDC_CLIENT_AUTH_METHOD:-"client_secret_post"} tenant: ${OIDC_TENANT:-""} maxClockSkew: ${OIDC_MAX_CLOCK_SKEW:-""} + tokenValidity: ${OIDC_OM_REFRESH_TOKEN_VALIDITY:-"3600"} # in seconds customParams: ${OIDC_CUSTOM_PARAMS:-} samlConfiguration: debugMode: ${SAML_DEBUG_MODE:-false} 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 ed02a510efcc..c89e2d0078cb 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/OpenMetadataApplication.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/OpenMetadataApplication.java @@ -13,8 +13,6 @@ package org.openmetadata.service; -import static org.openmetadata.service.security.SecurityUtil.tryCreateOidcClient; - import io.dropwizard.Application; import io.dropwizard.configuration.EnvironmentVariableSubstitutor; import io.dropwizard.configuration.SubstitutingSourceProvider; @@ -105,6 +103,7 @@ import org.openmetadata.service.security.AuthLoginServlet; import org.openmetadata.service.security.AuthLogoutServlet; import org.openmetadata.service.security.AuthRefreshServlet; +import org.openmetadata.service.security.AuthenticationCodeFlowHandler; import org.openmetadata.service.security.Authorizer; import org.openmetadata.service.security.NoopAuthorizer; import org.openmetadata.service.security.NoopFilter; @@ -127,7 +126,6 @@ import org.openmetadata.service.util.jdbi.DatabaseAuthenticationProviderFactory; import org.openmetadata.service.util.jdbi.OMSqlLogger; import org.pac4j.core.util.CommonHelper; -import org.pac4j.oidc.client.OidcClient; import org.quartz.SchedulerException; /** Main catalog application */ @@ -273,55 +271,32 @@ private void registerAuthServlets(OpenMetadataApplicationConfig config, Environm contextHandler.setSessionHandler(new SessionHandler()); } + AuthenticationCodeFlowHandler authenticationCodeFlowHandler = + new AuthenticationCodeFlowHandler( + config.getAuthenticationConfiguration(), config.getAuthorizerConfiguration()); + // Register Servlets - OidcClient oidcClient = - tryCreateOidcClient(config.getAuthenticationConfiguration().getOidcConfiguration()); - oidcClient.setCallbackUrl( - config.getAuthenticationConfiguration().getOidcConfiguration().getCallbackUrl()); ServletRegistration.Dynamic authLogin = environment .servlets() - .addServlet( - "oauth_login", - new AuthLoginServlet( - oidcClient, - config.getAuthenticationConfiguration(), - config.getAuthorizerConfiguration())); + .addServlet("oauth_login", new AuthLoginServlet(authenticationCodeFlowHandler)); authLogin.addMapping("/api/v1/auth/login"); ServletRegistration.Dynamic authCallback = environment .servlets() - .addServlet( - "auth_callback", - new AuthCallbackServlet( - oidcClient, - config.getAuthenticationConfiguration(), - config.getAuthorizerConfiguration())); + .addServlet("auth_callback", new AuthCallbackServlet(authenticationCodeFlowHandler)); authCallback.addMapping("/callback"); ServletRegistration.Dynamic authLogout = environment .servlets() - .addServlet( - "auth_logout", - new AuthLogoutServlet( - config - .getAuthenticationConfiguration() - .getOidcConfiguration() - .getServerUrl())); + .addServlet("auth_logout", new AuthLogoutServlet(authenticationCodeFlowHandler)); authLogout.addMapping("/api/v1/auth/logout"); ServletRegistration.Dynamic refreshServlet = environment .servlets() - .addServlet( - "auth_refresh", - new AuthRefreshServlet( - oidcClient, - config - .getAuthenticationConfiguration() - .getOidcConfiguration() - .getServerUrl())); + .addServlet("auth_refresh", new AuthRefreshServlet(authenticationCodeFlowHandler)); refreshServlet.addMapping("/api/v1/auth/refresh"); } } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/security/AuthCallbackServlet.java b/openmetadata-service/src/main/java/org/openmetadata/service/security/AuthCallbackServlet.java index 0419c35c7b41..fc44e2059291 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/security/AuthCallbackServlet.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/security/AuthCallbackServlet.java @@ -1,281 +1,22 @@ package org.openmetadata.service.security; -import static org.openmetadata.common.utils.CommonUtil.listOrEmpty; -import static org.openmetadata.service.security.AuthLoginServlet.OIDC_CREDENTIAL_PROFILE; -import static org.openmetadata.service.security.SecurityUtil.getClientAuthentication; -import static org.openmetadata.service.security.SecurityUtil.getErrorMessage; -import static org.openmetadata.service.security.SecurityUtil.sendRedirectWithToken; -import static org.openmetadata.service.security.SecurityUtil.validatePrincipalClaimsMapping; - -import com.nimbusds.jose.proc.BadJOSEException; -import com.nimbusds.jwt.JWT; -import com.nimbusds.jwt.JWTClaimsSet; -import com.nimbusds.jwt.proc.BadJWTException; -import com.nimbusds.oauth2.sdk.AuthorizationCode; -import com.nimbusds.oauth2.sdk.AuthorizationCodeGrant; -import com.nimbusds.oauth2.sdk.AuthorizationGrant; -import com.nimbusds.oauth2.sdk.ErrorObject; -import com.nimbusds.oauth2.sdk.ParseException; -import com.nimbusds.oauth2.sdk.TokenErrorResponse; -import com.nimbusds.oauth2.sdk.TokenRequest; -import com.nimbusds.oauth2.sdk.TokenResponse; -import com.nimbusds.oauth2.sdk.auth.ClientAuthentication; -import com.nimbusds.oauth2.sdk.http.HTTPRequest; -import com.nimbusds.oauth2.sdk.http.HTTPResponse; -import com.nimbusds.oauth2.sdk.id.ClientID; -import com.nimbusds.oauth2.sdk.id.State; -import com.nimbusds.oauth2.sdk.pkce.CodeVerifier; -import com.nimbusds.oauth2.sdk.token.AccessToken; -import com.nimbusds.openid.connect.sdk.AuthenticationErrorResponse; -import com.nimbusds.openid.connect.sdk.AuthenticationResponse; -import com.nimbusds.openid.connect.sdk.AuthenticationResponseParser; -import com.nimbusds.openid.connect.sdk.AuthenticationSuccessResponse; -import com.nimbusds.openid.connect.sdk.OIDCTokenResponse; -import com.nimbusds.openid.connect.sdk.OIDCTokenResponseParser; -import com.nimbusds.openid.connect.sdk.op.OIDCProviderMetadata; -import com.nimbusds.openid.connect.sdk.token.OIDCTokens; -import com.nimbusds.openid.connect.sdk.validators.BadJWTExceptions; -import java.io.IOException; -import java.net.URI; -import java.net.URISyntaxException; -import java.util.Arrays; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.stream.Collectors; import javax.servlet.annotation.WebServlet; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import lombok.extern.slf4j.Slf4j; -import org.openmetadata.schema.api.security.AuthenticationConfiguration; -import org.openmetadata.schema.api.security.AuthorizerConfiguration; -import org.pac4j.core.exception.TechnicalException; -import org.pac4j.core.util.CommonHelper; -import org.pac4j.oidc.client.OidcClient; -import org.pac4j.oidc.credentials.OidcCredentials; @WebServlet("/callback") @Slf4j public class AuthCallbackServlet extends HttpServlet { - private final OidcClient client; - private final ClientAuthentication clientAuthentication; - private final List claimsOrder; - private final Map claimsMapping; - private final String serverUrl; - private final String principalDomain; + private final AuthenticationCodeFlowHandler authenticationCodeFlowHandler; - public AuthCallbackServlet( - OidcClient oidcClient, - AuthenticationConfiguration authenticationConfiguration, - AuthorizerConfiguration authorizerConfiguration) { - CommonHelper.assertNotBlank( - "ServerUrl", authenticationConfiguration.getOidcConfiguration().getServerUrl()); - this.client = oidcClient; - this.claimsOrder = authenticationConfiguration.getJwtPrincipalClaims(); - this.claimsMapping = - listOrEmpty(authenticationConfiguration.getJwtPrincipalClaimsMapping()).stream() - .map(s -> s.split(":")) - .collect(Collectors.toMap(s -> s[0], s -> s[1])); - validatePrincipalClaimsMapping(claimsMapping); - this.serverUrl = authenticationConfiguration.getOidcConfiguration().getServerUrl(); - this.clientAuthentication = getClientAuthentication(client.getConfiguration()); - this.principalDomain = authorizerConfiguration.getPrincipalDomain(); + public AuthCallbackServlet(AuthenticationCodeFlowHandler authenticationCodeFlowHandler) { + this.authenticationCodeFlowHandler = authenticationCodeFlowHandler; } @Override protected void doGet(HttpServletRequest req, HttpServletResponse resp) { - try { - LOG.debug("Performing Auth Callback For User Session: {} ", req.getSession().getId()); - String computedCallbackUrl = client.getCallbackUrl(); - Map> parameters = retrieveParameters(req); - AuthenticationResponse response = - AuthenticationResponseParser.parse(new URI(computedCallbackUrl), parameters); - - if (response instanceof AuthenticationErrorResponse authenticationErrorResponse) { - LOG.error( - "Bad authentication response, error={}", authenticationErrorResponse.getErrorObject()); - throw new TechnicalException("Bad authentication response"); - } - - LOG.debug("Authentication response successful"); - AuthenticationSuccessResponse successResponse = (AuthenticationSuccessResponse) response; - - OIDCProviderMetadata metadata = client.getConfiguration().getProviderMetadata(); - if (metadata.supportsAuthorizationResponseIssuerParam() - && !metadata.getIssuer().equals(successResponse.getIssuer())) { - throw new TechnicalException("Issuer mismatch, possible mix-up attack."); - } - - // Optional state validation - validateStateIfRequired(req, resp, successResponse); - - // Build Credentials - OidcCredentials credentials = buildCredentials(successResponse); - - // Validations - validateAndSendTokenRequest(req, credentials, computedCallbackUrl); - - // Log Error if the Refresh Token is null - if (credentials.getRefreshToken() == null) { - LOG.error("Refresh token is null for user session: {}", req.getSession().getId()); - } - - validateNonceIfRequired(req, credentials.getIdToken().getJWTClaimsSet()); - - // Put Credentials in Session - req.getSession().setAttribute(OIDC_CREDENTIAL_PROFILE, credentials); - - // Redirect - sendRedirectWithToken( - resp, credentials, serverUrl, claimsMapping, claimsOrder, principalDomain); - } catch (Exception e) { - getErrorMessage(resp, e); - } - } - - private OidcCredentials buildCredentials(AuthenticationSuccessResponse successResponse) { - OidcCredentials credentials = new OidcCredentials(); - // get authorization code - AuthorizationCode code = successResponse.getAuthorizationCode(); - if (code != null) { - credentials.setCode(code); - } - // get ID token - JWT idToken = successResponse.getIDToken(); - if (idToken != null) { - credentials.setIdToken(idToken); - } - // get access token - AccessToken accessToken = successResponse.getAccessToken(); - if (accessToken != null) { - credentials.setAccessToken(accessToken); - } - - return credentials; - } - - private void validateNonceIfRequired(HttpServletRequest req, JWTClaimsSet claimsSet) - throws BadJOSEException { - if (client.getConfiguration().isUseNonce()) { - String expectedNonce = - (String) req.getSession().getAttribute(client.getNonceSessionAttributeName()); - if (CommonHelper.isNotBlank(expectedNonce)) { - String tokenNonce; - try { - tokenNonce = claimsSet.getStringClaim("nonce"); - } catch (java.text.ParseException var10) { - throw new BadJWTException("Invalid JWT nonce (nonce) claim: " + var10.getMessage()); - } - - if (tokenNonce == null) { - throw BadJWTExceptions.MISSING_NONCE_CLAIM_EXCEPTION; - } - - if (!expectedNonce.equals(tokenNonce)) { - throw new BadJWTException("Unexpected JWT nonce (nonce) claim: " + tokenNonce); - } - } else { - throw new TechnicalException("Missing nonce parameter from Session."); - } - } - } - - private void validateStateIfRequired( - HttpServletRequest req, - HttpServletResponse resp, - AuthenticationSuccessResponse successResponse) { - if (client.getConfiguration().isWithState()) { - // Validate state for CSRF mitigation - State requestState = - (State) req.getSession().getAttribute(client.getStateSessionAttributeName()); - if (requestState == null || CommonHelper.isBlank(requestState.getValue())) { - getErrorMessage(resp, new TechnicalException("Missing state parameter")); - return; - } - - State responseState = successResponse.getState(); - if (responseState == null) { - throw new TechnicalException("Missing state parameter"); - } - - LOG.debug("Request state: {}/response state: {}", requestState, responseState); - if (!requestState.equals(responseState)) { - throw new TechnicalException( - "State parameter is different from the one sent in authentication request."); - } - } - } - - private void validateAndSendTokenRequest( - HttpServletRequest req, OidcCredentials oidcCredentials, String computedCallbackUrl) - throws IOException, ParseException, URISyntaxException { - if (oidcCredentials.getCode() != null) { - LOG.debug("Initiating Token Request for User Session: {} ", req.getSession().getId()); - CodeVerifier verifier = - (CodeVerifier) - req.getSession().getAttribute(client.getCodeVerifierSessionAttributeName()); - // Token request - TokenRequest request = - createTokenRequest( - new AuthorizationCodeGrant( - oidcCredentials.getCode(), new URI(computedCallbackUrl), verifier)); - executeTokenRequest(request, oidcCredentials); - } - } - - protected Map> retrieveParameters(HttpServletRequest request) { - Map requestParameters = request.getParameterMap(); - Map> map = new HashMap<>(); - for (var entry : requestParameters.entrySet()) { - map.put(entry.getKey(), Arrays.asList(entry.getValue())); - } - return map; - } - - protected TokenRequest createTokenRequest(final AuthorizationGrant grant) { - if (client.getConfiguration().getClientAuthenticationMethod() != null) { - return new TokenRequest( - client.getConfiguration().findProviderMetadata().getTokenEndpointURI(), - this.clientAuthentication, - grant); - } else { - return new TokenRequest( - client.getConfiguration().findProviderMetadata().getTokenEndpointURI(), - new ClientID(client.getConfiguration().getClientId()), - grant); - } - } - - private void executeTokenRequest(TokenRequest request, OidcCredentials credentials) - throws IOException, ParseException { - HTTPRequest tokenHttpRequest = request.toHTTPRequest(); - client.getConfiguration().configureHttpRequest(tokenHttpRequest); - - HTTPResponse httpResponse = tokenHttpRequest.send(); - LOG.debug( - "Token response: status={}, content={}", - httpResponse.getStatusCode(), - httpResponse.getContent()); - - TokenResponse response = OIDCTokenResponseParser.parse(httpResponse); - if (response instanceof TokenErrorResponse tokenErrorResponse) { - ErrorObject errorObject = tokenErrorResponse.getErrorObject(); - throw new TechnicalException( - "Bad token response, error=" - + errorObject.getCode() - + "," - + " description=" - + errorObject.getDescription()); - } - LOG.debug("Token response successful"); - OIDCTokenResponse tokenSuccessResponse = (OIDCTokenResponse) response; - - OIDCTokens oidcTokens = tokenSuccessResponse.getOIDCTokens(); - credentials.setAccessToken(oidcTokens.getAccessToken()); - credentials.setRefreshToken(oidcTokens.getRefreshToken()); - if (oidcTokens.getIDToken() != null) { - credentials.setIdToken(oidcTokens.getIDToken()); - } + authenticationCodeFlowHandler.handleCallback(req, resp); } } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/security/AuthLoginServlet.java b/openmetadata-service/src/main/java/org/openmetadata/service/security/AuthLoginServlet.java index b3e4ff54c13e..16e2f8bad7c0 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/security/AuthLoginServlet.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/security/AuthLoginServlet.java @@ -1,166 +1,22 @@ package org.openmetadata.service.security; -import static org.openmetadata.common.utils.CommonUtil.listOrEmpty; -import static org.openmetadata.service.security.SecurityUtil.getErrorMessage; -import static org.openmetadata.service.security.SecurityUtil.getUserCredentialsFromSession; -import static org.openmetadata.service.security.SecurityUtil.sendRedirectWithToken; -import static org.openmetadata.service.security.SecurityUtil.validatePrincipalClaimsMapping; - -import com.nimbusds.oauth2.sdk.id.State; -import com.nimbusds.oauth2.sdk.pkce.CodeChallenge; -import com.nimbusds.oauth2.sdk.pkce.CodeChallengeMethod; -import com.nimbusds.oauth2.sdk.pkce.CodeVerifier; -import com.nimbusds.openid.connect.sdk.AuthenticationRequest; -import com.nimbusds.openid.connect.sdk.Nonce; -import java.io.IOException; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.stream.Collectors; import javax.servlet.annotation.WebServlet; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import lombok.extern.slf4j.Slf4j; -import org.openmetadata.schema.api.security.AuthenticationConfiguration; -import org.openmetadata.schema.api.security.AuthorizerConfiguration; -import org.pac4j.core.exception.TechnicalException; -import org.pac4j.core.util.CommonHelper; -import org.pac4j.oidc.client.GoogleOidcClient; -import org.pac4j.oidc.client.OidcClient; -import org.pac4j.oidc.config.OidcConfiguration; -import org.pac4j.oidc.credentials.OidcCredentials; @WebServlet("/api/v1/auth/login") @Slf4j public class AuthLoginServlet extends HttpServlet { - public static final String OIDC_CREDENTIAL_PROFILE = "oidcCredentialProfile"; - private final OidcClient client; - private final List claimsOrder; - private final Map claimsMapping; - private final String serverUrl; - private final String principalDomain; + private final AuthenticationCodeFlowHandler authenticationCodeFlowHandler; - public AuthLoginServlet( - OidcClient oidcClient, - AuthenticationConfiguration authenticationConfiguration, - AuthorizerConfiguration authorizerConfiguration) { - this.client = oidcClient; - this.serverUrl = authenticationConfiguration.getOidcConfiguration().getServerUrl(); - this.claimsOrder = authenticationConfiguration.getJwtPrincipalClaims(); - this.claimsMapping = - listOrEmpty(authenticationConfiguration.getJwtPrincipalClaimsMapping()).stream() - .map(s -> s.split(":")) - .collect(Collectors.toMap(s -> s[0], s -> s[1])); - validatePrincipalClaimsMapping(claimsMapping); - this.principalDomain = authorizerConfiguration.getPrincipalDomain(); + public AuthLoginServlet(AuthenticationCodeFlowHandler authenticationCodeFlowHandler) { + this.authenticationCodeFlowHandler = authenticationCodeFlowHandler; } @Override protected void doGet(HttpServletRequest req, HttpServletResponse resp) { - try { - LOG.debug("Performing Auth Login For User Session: {} ", req.getSession().getId()); - Optional credentials = getUserCredentialsFromSession(req, client); - if (credentials.isPresent()) { - LOG.debug("Auth Tokens Located from Session: {} ", req.getSession().getId()); - sendRedirectWithToken( - resp, credentials.get(), serverUrl, claimsMapping, claimsOrder, principalDomain); - } else { - LOG.debug("Performing Auth Code Flow to Idp: {} ", req.getSession().getId()); - Map params = buildParams(); - - params.put(OidcConfiguration.REDIRECT_URI, client.getCallbackUrl()); - - addStateAndNonceParameters(req, params); - - // This is always used to prompt the user to login - if (client instanceof GoogleOidcClient) { - params.put(OidcConfiguration.PROMPT, "consent"); - } else { - params.put(OidcConfiguration.PROMPT, "login"); - } - params.put(OidcConfiguration.MAX_AGE, "0"); - - String location = buildAuthenticationRequestUrl(params); - LOG.debug("Authentication request url: {}", location); - - resp.sendRedirect(location); - } - } catch (Exception e) { - getErrorMessage(resp, new TechnicalException(e)); - } - } - - protected Map buildParams() { - Map authParams = new HashMap<>(); - authParams.put(OidcConfiguration.SCOPE, client.getConfiguration().getScope()); - authParams.put(OidcConfiguration.RESPONSE_TYPE, client.getConfiguration().getResponseType()); - authParams.put(OidcConfiguration.RESPONSE_MODE, "query"); - authParams.putAll(client.getConfiguration().getCustomParams()); - authParams.put(OidcConfiguration.CLIENT_ID, client.getConfiguration().getClientId()); - - return new HashMap<>(authParams); - } - - protected void addStateAndNonceParameters( - final HttpServletRequest request, final Map params) { - // Init state for CSRF mitigation - if (client.getConfiguration().isWithState()) { - State state = new State(CommonHelper.randomString(10)); - params.put(OidcConfiguration.STATE, state.getValue()); - request.getSession().setAttribute(client.getStateSessionAttributeName(), state); - } - - // Init nonce for replay attack mitigation - if (client.getConfiguration().isUseNonce()) { - Nonce nonce = new Nonce(); - params.put(OidcConfiguration.NONCE, nonce.getValue()); - request.getSession().setAttribute(client.getNonceSessionAttributeName(), nonce.getValue()); - } - - CodeChallengeMethod pkceMethod = client.getConfiguration().findPkceMethod(); - - // Use Default PKCE method if not disabled - if (pkceMethod == null && !client.getConfiguration().isDisablePkce()) { - pkceMethod = CodeChallengeMethod.S256; - } - if (pkceMethod != null) { - CodeVerifier verfifier = new CodeVerifier(CommonHelper.randomString(43)); - request.getSession().setAttribute(client.getCodeVerifierSessionAttributeName(), verfifier); - params.put( - OidcConfiguration.CODE_CHALLENGE, - CodeChallenge.compute(pkceMethod, verfifier).getValue()); - params.put(OidcConfiguration.CODE_CHALLENGE_METHOD, pkceMethod.getValue()); - } - } - - protected String buildAuthenticationRequestUrl(final Map params) { - // Build authentication request query string - String queryString; - try { - queryString = - AuthenticationRequest.parse( - params.entrySet().stream() - .collect( - Collectors.toMap( - Map.Entry::getKey, e -> Collections.singletonList(e.getValue())))) - .toQueryString(); - } catch (Exception e) { - throw new TechnicalException(e); - } - return client.getConfiguration().getProviderMetadata().getAuthorizationEndpointURI().toString() - + '?' - + queryString; - } - - public static void writeJsonResponse(HttpServletResponse response, String message) - throws IOException { - response.setContentType("application/json"); - response.setCharacterEncoding("UTF-8"); - response.getOutputStream().print(message); - response.getOutputStream().flush(); - response.setStatus(HttpServletResponse.SC_OK); + authenticationCodeFlowHandler.handleLogin(req, resp); } } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/security/AuthLogoutServlet.java b/openmetadata-service/src/main/java/org/openmetadata/service/security/AuthLogoutServlet.java index 4703a7500334..a0f79c16c1f9 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/security/AuthLogoutServlet.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/security/AuthLogoutServlet.java @@ -4,33 +4,20 @@ import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import javax.servlet.http.HttpSession; import lombok.extern.slf4j.Slf4j; @WebServlet("/api/v1/auth/logout") @Slf4j public class AuthLogoutServlet extends HttpServlet { - private final String url; + private final AuthenticationCodeFlowHandler authenticationCodeFlowHandler; - public AuthLogoutServlet(String url) { - this.url = url; + public AuthLogoutServlet(AuthenticationCodeFlowHandler authenticationCodeFlowHandler) { + this.authenticationCodeFlowHandler = authenticationCodeFlowHandler; } @Override protected void doGet( final HttpServletRequest httpServletRequest, final HttpServletResponse httpServletResponse) { - try { - LOG.debug("Performing application logout"); - HttpSession session = httpServletRequest.getSession(false); - if (session != null) { - LOG.debug("Invalidating the session for logout"); - session.invalidate(); - httpServletResponse.sendRedirect(url); - } else { - LOG.error("No session store available for this web context"); - } - } catch (Exception ex) { - LOG.error("[Auth Logout] Error while performing logout", ex); - } + authenticationCodeFlowHandler.handleLogout(httpServletRequest, httpServletResponse); } } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/security/AuthRefreshServlet.java b/openmetadata-service/src/main/java/org/openmetadata/service/security/AuthRefreshServlet.java index a40a7614b7ec..e1a669bc8954 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/security/AuthRefreshServlet.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/security/AuthRefreshServlet.java @@ -1,58 +1,22 @@ package org.openmetadata.service.security; -import static org.openmetadata.service.security.AuthLoginServlet.writeJsonResponse; -import static org.openmetadata.service.security.SecurityUtil.getErrorMessage; -import static org.openmetadata.service.security.SecurityUtil.getUserCredentialsFromSession; - -import java.util.Optional; import javax.servlet.annotation.WebServlet; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import lombok.extern.slf4j.Slf4j; -import org.openmetadata.service.auth.JwtResponse; -import org.openmetadata.service.util.JsonUtils; -import org.pac4j.core.exception.TechnicalException; -import org.pac4j.oidc.client.OidcClient; -import org.pac4j.oidc.credentials.OidcCredentials; @WebServlet("/api/v1/auth/refresh") @Slf4j public class AuthRefreshServlet extends HttpServlet { - private final OidcClient client; - private final String baseUrl; + private final AuthenticationCodeFlowHandler authenticationCodeFlowHandler; - public AuthRefreshServlet(OidcClient oidcClient, String url) { - this.client = oidcClient; - this.baseUrl = url; + public AuthRefreshServlet(AuthenticationCodeFlowHandler authenticationCodeFlowHandler) { + this.authenticationCodeFlowHandler = authenticationCodeFlowHandler; } @Override protected void doGet(HttpServletRequest req, HttpServletResponse resp) { - try { - LOG.debug("Performing Auth Refresh For User Session: {} ", req.getSession().getId()); - Optional credentials = getUserCredentialsFromSession(req, client); - if (credentials.isPresent()) { - LOG.debug("Credentials Found For User Session: {} ", req.getSession().getId()); - JwtResponse jwtResponse = new JwtResponse(); - jwtResponse.setAccessToken(credentials.get().getIdToken().getParsedString()); - jwtResponse.setExpiryDuration( - credentials - .get() - .getIdToken() - .getJWTClaimsSet() - .getExpirationTime() - .toInstant() - .getEpochSecond()); - writeJsonResponse(resp, JsonUtils.pojoToJson(jwtResponse)); - } else { - LOG.debug( - "Credentials Not Found For User Session: {}, Redirect to Logout ", - req.getSession().getId()); - resp.sendRedirect(String.format("%s/logout", baseUrl)); - } - } catch (Exception e) { - getErrorMessage(resp, new TechnicalException(e)); - } + authenticationCodeFlowHandler.handleRefresh(req, resp); } } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/security/AuthenticationCodeFlowHandler.java b/openmetadata-service/src/main/java/org/openmetadata/service/security/AuthenticationCodeFlowHandler.java new file mode 100644 index 000000000000..1f7511e074ee --- /dev/null +++ b/openmetadata-service/src/main/java/org/openmetadata/service/security/AuthenticationCodeFlowHandler.java @@ -0,0 +1,912 @@ +package org.openmetadata.service.security; + +import static org.openmetadata.common.utils.CommonUtil.listOrEmpty; +import static org.openmetadata.common.utils.CommonUtil.nullOrEmpty; +import static org.openmetadata.service.security.JwtFilter.EMAIL_CLAIM_KEY; +import static org.openmetadata.service.security.JwtFilter.USERNAME_CLAIM_KEY; +import static org.openmetadata.service.security.SecurityUtil.findEmailFromClaims; +import static org.openmetadata.service.security.SecurityUtil.getClaimOrObject; +import static org.openmetadata.service.security.SecurityUtil.getFirstMatchJwtClaim; +import static org.openmetadata.service.util.UserUtil.getRoleListFromUser; +import static org.pac4j.core.util.CommonHelper.assertNotNull; +import static org.pac4j.core.util.CommonHelper.isNotEmpty; + +import com.fasterxml.jackson.core.type.TypeReference; +import com.nimbusds.jose.JOSEException; +import com.nimbusds.jose.JWSAlgorithm; +import com.nimbusds.jose.proc.BadJOSEException; +import com.nimbusds.jwt.JWT; +import com.nimbusds.jwt.JWTClaimsSet; +import com.nimbusds.jwt.SignedJWT; +import com.nimbusds.jwt.proc.BadJWTException; +import com.nimbusds.oauth2.sdk.AuthorizationCode; +import com.nimbusds.oauth2.sdk.AuthorizationCodeGrant; +import com.nimbusds.oauth2.sdk.AuthorizationGrant; +import com.nimbusds.oauth2.sdk.ErrorObject; +import com.nimbusds.oauth2.sdk.RefreshTokenGrant; +import com.nimbusds.oauth2.sdk.TokenErrorResponse; +import com.nimbusds.oauth2.sdk.TokenRequest; +import com.nimbusds.oauth2.sdk.TokenResponse; +import com.nimbusds.oauth2.sdk.auth.ClientAuthentication; +import com.nimbusds.oauth2.sdk.auth.ClientAuthenticationMethod; +import com.nimbusds.oauth2.sdk.auth.ClientSecretBasic; +import com.nimbusds.oauth2.sdk.auth.ClientSecretPost; +import com.nimbusds.oauth2.sdk.auth.PrivateKeyJWT; +import com.nimbusds.oauth2.sdk.auth.Secret; +import com.nimbusds.oauth2.sdk.http.HTTPRequest; +import com.nimbusds.oauth2.sdk.http.HTTPResponse; +import com.nimbusds.oauth2.sdk.id.ClientID; +import com.nimbusds.oauth2.sdk.id.State; +import com.nimbusds.oauth2.sdk.pkce.CodeChallenge; +import com.nimbusds.oauth2.sdk.pkce.CodeChallengeMethod; +import com.nimbusds.oauth2.sdk.pkce.CodeVerifier; +import com.nimbusds.oauth2.sdk.token.AccessToken; +import com.nimbusds.oauth2.sdk.token.BearerAccessToken; +import com.nimbusds.oauth2.sdk.util.JSONObjectUtils; +import com.nimbusds.openid.connect.sdk.AuthenticationErrorResponse; +import com.nimbusds.openid.connect.sdk.AuthenticationRequest; +import com.nimbusds.openid.connect.sdk.AuthenticationResponse; +import com.nimbusds.openid.connect.sdk.AuthenticationResponseParser; +import com.nimbusds.openid.connect.sdk.AuthenticationSuccessResponse; +import com.nimbusds.openid.connect.sdk.Nonce; +import com.nimbusds.openid.connect.sdk.OIDCTokenResponse; +import com.nimbusds.openid.connect.sdk.OIDCTokenResponseParser; +import com.nimbusds.openid.connect.sdk.op.OIDCProviderMetadata; +import com.nimbusds.openid.connect.sdk.token.OIDCTokens; +import com.nimbusds.openid.connect.sdk.validators.BadJWTExceptions; +import java.io.BufferedWriter; +import java.io.IOException; +import java.io.OutputStreamWriter; +import java.net.HttpURLConnection; +import java.net.URI; +import java.net.URISyntaxException; +import java.net.URL; +import java.nio.charset.StandardCharsets; +import java.security.PrivateKey; +import java.text.ParseException; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.TreeMap; +import java.util.stream.Collectors; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; +import javax.ws.rs.BadRequestException; +import lombok.SneakyThrows; +import lombok.extern.slf4j.Slf4j; +import net.minidev.json.JSONObject; +import org.openmetadata.schema.api.security.AuthenticationConfiguration; +import org.openmetadata.schema.api.security.AuthorizerConfiguration; +import org.openmetadata.schema.auth.JWTAuthMechanism; +import org.openmetadata.schema.auth.ServiceTokenType; +import org.openmetadata.schema.entity.teams.User; +import org.openmetadata.schema.security.client.OidcClientConfig; +import org.openmetadata.schema.type.Include; +import org.openmetadata.service.Entity; +import org.openmetadata.service.auth.JwtResponse; +import org.openmetadata.service.security.jwt.JWTTokenGenerator; +import org.openmetadata.service.util.JsonUtils; +import org.pac4j.core.context.HttpConstants; +import org.pac4j.core.exception.TechnicalException; +import org.pac4j.core.util.CommonHelper; +import org.pac4j.core.util.HttpUtils; +import org.pac4j.oidc.client.AzureAd2Client; +import org.pac4j.oidc.client.GoogleOidcClient; +import org.pac4j.oidc.client.OidcClient; +import org.pac4j.oidc.config.AzureAd2OidcConfiguration; +import org.pac4j.oidc.config.OidcConfiguration; +import org.pac4j.oidc.config.PrivateKeyJWTClientAuthnMethodConfig; +import org.pac4j.oidc.credentials.OidcCredentials; + +@Slf4j +public class AuthenticationCodeFlowHandler { + private static final Collection SUPPORTED_METHODS = + Arrays.asList( + ClientAuthenticationMethod.CLIENT_SECRET_POST, + ClientAuthenticationMethod.CLIENT_SECRET_BASIC, + ClientAuthenticationMethod.PRIVATE_KEY_JWT, + ClientAuthenticationMethod.NONE); + + public static final String DEFAULT_PRINCIPAL_DOMAIN = "openmetadata.org"; + public static final String OIDC_CREDENTIAL_PROFILE = "oidcCredentialProfile"; + private final OidcClient client; + private final List claimsOrder; + private final Map claimsMapping; + private final String serverUrl; + private final ClientAuthentication clientAuthentication; + private final String principalDomain; + private final int tokenValidity; + + public AuthenticationCodeFlowHandler( + AuthenticationConfiguration authenticationConfiguration, + AuthorizerConfiguration authorizerConfiguration) { + // Assert oidcConfig and Callback Url + CommonHelper.assertNotNull( + "OidcConfiguration", authenticationConfiguration.getOidcConfiguration()); + CommonHelper.assertNotBlank( + "CallbackUrl", authenticationConfiguration.getOidcConfiguration().getCallbackUrl()); + CommonHelper.assertNotBlank( + "ServerUrl", authenticationConfiguration.getOidcConfiguration().getServerUrl()); + + // Build Required Params + this.client = buildOidcClient(authenticationConfiguration.getOidcConfiguration()); + client.setCallbackUrl(authenticationConfiguration.getOidcConfiguration().getCallbackUrl()); + this.clientAuthentication = getClientAuthentication(client.getConfiguration()); + this.serverUrl = authenticationConfiguration.getOidcConfiguration().getServerUrl(); + this.claimsOrder = authenticationConfiguration.getJwtPrincipalClaims(); + this.claimsMapping = + listOrEmpty(authenticationConfiguration.getJwtPrincipalClaimsMapping()).stream() + .map(s -> s.split(":")) + .collect(Collectors.toMap(s -> s[0], s -> s[1])); + validatePrincipalClaimsMapping(claimsMapping); + this.principalDomain = authorizerConfiguration.getPrincipalDomain(); + this.tokenValidity = authenticationConfiguration.getOidcConfiguration().getTokenValidity(); + } + + private OidcClient buildOidcClient(OidcClientConfig clientConfig) { + String id = clientConfig.getId(); + String secret = clientConfig.getSecret(); + if (CommonHelper.isNotBlank(id) && CommonHelper.isNotBlank(secret)) { + OidcConfiguration configuration = new OidcConfiguration(); + configuration.setClientId(id); + + configuration.setResponseMode("query"); + + // Add Secret + if (CommonHelper.isNotBlank(secret)) { + configuration.setSecret(secret); + } + + // Response Type + String responseType = clientConfig.getResponseType(); + if (CommonHelper.isNotBlank(responseType)) { + configuration.setResponseType(responseType); + } + + String scope = clientConfig.getScope(); + if (CommonHelper.isNotBlank(scope)) { + configuration.setScope(scope); + } + + String discoveryUri = clientConfig.getDiscoveryUri(); + if (CommonHelper.isNotBlank(discoveryUri)) { + configuration.setDiscoveryURI(discoveryUri); + } + + String useNonce = clientConfig.getUseNonce(); + if (CommonHelper.isNotBlank(useNonce)) { + configuration.setUseNonce(Boolean.parseBoolean(useNonce)); + } + + String jwsAlgo = clientConfig.getPreferredJwsAlgorithm(); + if (CommonHelper.isNotBlank(jwsAlgo)) { + configuration.setPreferredJwsAlgorithm(JWSAlgorithm.parse(jwsAlgo)); + } + + String maxClockSkew = clientConfig.getMaxClockSkew(); + if (CommonHelper.isNotBlank(maxClockSkew)) { + configuration.setMaxClockSkew(Integer.parseInt(maxClockSkew)); + } + + String clientAuthenticationMethod = clientConfig.getClientAuthenticationMethod().value(); + if (CommonHelper.isNotBlank(clientAuthenticationMethod)) { + configuration.setClientAuthenticationMethod( + ClientAuthenticationMethod.parse(clientAuthenticationMethod)); + } + + // Disable PKCE + configuration.setDisablePkce(clientConfig.getDisablePkce()); + + // Add Custom Params + if (clientConfig.getCustomParams() != null) { + for (int j = 1; j <= 5; ++j) { + if (clientConfig.getCustomParams().containsKey(String.format("customParamKey%d", j))) { + configuration.addCustomParam( + clientConfig.getCustomParams().get(String.format("customParamKey%d", j)), + clientConfig.getCustomParams().get(String.format("customParamValue%d", j))); + } + } + } + + String type = clientConfig.getType(); + OidcClient oidcClient; + if ("azure".equalsIgnoreCase(type)) { + AzureAd2OidcConfiguration azureAdConfiguration = + new AzureAd2OidcConfiguration(configuration); + String tenant = clientConfig.getTenant(); + if (CommonHelper.isNotBlank(tenant)) { + azureAdConfiguration.setTenant(tenant); + } + + oidcClient = new AzureAd2Client(azureAdConfiguration); + } else if ("google".equalsIgnoreCase(type)) { + oidcClient = new GoogleOidcClient(configuration); + // Google needs it as param + oidcClient.getConfiguration().getCustomParams().put("access_type", "offline"); + } else { + oidcClient = new OidcClient(configuration); + } + + oidcClient.setName(String.format("OMOidcClient%s", oidcClient.getName())); + return oidcClient; + } + throw new IllegalArgumentException( + "Client ID and Client Secret is required to create OidcClient"); + } + + // Login + public void handleLogin(HttpServletRequest req, HttpServletResponse resp) { + try { + LOG.debug("Performing Auth Login For User Session: {} ", req.getSession().getId()); + Optional credentials = getUserCredentialsFromSession(req); + if (credentials.isPresent()) { + LOG.debug("Auth Tokens Located from Session: {} ", req.getSession().getId()); + sendRedirectWithToken(resp, credentials.get()); + } else { + LOG.debug("Performing Auth Code Flow to Idp: {} ", req.getSession().getId()); + Map params = buildLoginParams(); + + params.put(OidcConfiguration.REDIRECT_URI, client.getCallbackUrl()); + + addStateAndNonceParameters(client, req, params); + + // This is always used to prompt the user to login + if (client instanceof GoogleOidcClient) { + params.put(OidcConfiguration.PROMPT, "consent"); + } else { + params.put(OidcConfiguration.PROMPT, "login"); + } + params.put(OidcConfiguration.MAX_AGE, "0"); + + String location = buildLoginAuthenticationRequestUrl(params); + LOG.debug("Authentication request url: {}", location); + + resp.sendRedirect(location); + } + } catch (Exception e) { + getErrorMessage(resp, new TechnicalException(e)); + } + } + + // Callback + public void handleCallback(HttpServletRequest req, HttpServletResponse resp) { + try { + LOG.debug("Performing Auth Callback For User Session: {} ", req.getSession().getId()); + String computedCallbackUrl = client.getCallbackUrl(); + Map> parameters = retrieveCallbackParameters(req); + AuthenticationResponse response = + AuthenticationResponseParser.parse(new URI(computedCallbackUrl), parameters); + + if (response instanceof AuthenticationErrorResponse authenticationErrorResponse) { + LOG.error( + "Bad authentication response, error={}", authenticationErrorResponse.getErrorObject()); + throw new TechnicalException("Bad authentication response"); + } + + LOG.debug("Authentication response successful"); + AuthenticationSuccessResponse successResponse = (AuthenticationSuccessResponse) response; + + OIDCProviderMetadata metadata = client.getConfiguration().getProviderMetadata(); + if (metadata.supportsAuthorizationResponseIssuerParam() + && !metadata.getIssuer().equals(successResponse.getIssuer())) { + throw new TechnicalException("Issuer mismatch, possible mix-up attack."); + } + + // Optional state validation + validateStateIfRequired(req, resp, successResponse); + + // Build Credentials + OidcCredentials credentials = buildCredentials(successResponse); + + // Validations + validateAndSendTokenRequest(req, credentials, computedCallbackUrl); + + // Log Error if the Refresh Token is null + if (credentials.getRefreshToken() == null) { + LOG.error("Refresh token is null for user session: {}", req.getSession().getId()); + } + + validateNonceIfRequired(req, credentials.getIdToken().getJWTClaimsSet()); + + // Put Credentials in Session + req.getSession().setAttribute(OIDC_CREDENTIAL_PROFILE, credentials); + + // Redirect + sendRedirectWithToken(resp, credentials); + } catch (Exception e) { + getErrorMessage(resp, e); + } + } + + // Logout + public void handleLogout( + HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse) { + try { + LOG.debug("Performing application logout"); + HttpSession session = httpServletRequest.getSession(false); + if (session != null) { + LOG.debug("Invalidating the session for logout"); + session.invalidate(); + httpServletResponse.sendRedirect(serverUrl); + } else { + LOG.error("No session store available for this web context"); + } + } catch (Exception ex) { + LOG.error("[Auth Logout] Error while performing logout", ex); + } + } + + // Refresh + public void handleRefresh( + HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse) { + try { + LOG.debug( + "Performing Auth Refresh For User Session: {} ", httpServletRequest.getSession().getId()); + Optional credentials = getUserCredentialsFromSession(httpServletRequest); + if (credentials.isPresent()) { + LOG.debug( + "Credentials Found For User Session: {} ", httpServletRequest.getSession().getId()); + JwtResponse jwtResponse = new JwtResponse(); + jwtResponse.setAccessToken(credentials.get().getIdToken().getParsedString()); + jwtResponse.setExpiryDuration( + credentials + .get() + .getIdToken() + .getJWTClaimsSet() + .getExpirationTime() + .toInstant() + .getEpochSecond()); + writeJsonResponse(httpServletResponse, JsonUtils.pojoToJson(jwtResponse)); + } else { + LOG.debug( + "Credentials Not Found For User Session: {}, Redirect to Logout ", + httpServletRequest.getSession().getId()); + httpServletResponse.sendRedirect(String.format("%s/logout", serverUrl)); + } + } catch (Exception e) { + getErrorMessage(httpServletResponse, new TechnicalException(e)); + } + } + + private String buildLoginAuthenticationRequestUrl(final Map params) { + // Build authentication request query string + String queryString; + try { + queryString = + AuthenticationRequest.parse( + params.entrySet().stream() + .collect( + Collectors.toMap( + Map.Entry::getKey, e -> Collections.singletonList(e.getValue())))) + .toQueryString(); + } catch (Exception e) { + throw new TechnicalException(e); + } + return client.getConfiguration().getProviderMetadata().getAuthorizationEndpointURI().toString() + + '?' + + queryString; + } + + private Map buildLoginParams() { + Map authParams = new HashMap<>(); + authParams.put(OidcConfiguration.SCOPE, client.getConfiguration().getScope()); + authParams.put(OidcConfiguration.RESPONSE_TYPE, client.getConfiguration().getResponseType()); + authParams.put(OidcConfiguration.RESPONSE_MODE, "query"); + authParams.putAll(client.getConfiguration().getCustomParams()); + authParams.put(OidcConfiguration.CLIENT_ID, client.getConfiguration().getClientId()); + + return new HashMap<>(authParams); + } + + private Optional getUserCredentialsFromSession(HttpServletRequest request) + throws URISyntaxException { + OidcCredentials credentials = + (OidcCredentials) request.getSession().getAttribute(OIDC_CREDENTIAL_PROFILE); + + if (credentials != null && credentials.getRefreshToken() != null) { + LOG.trace("Credentials found in session: {}", credentials); + renewOidcCredentials(request, credentials); + return Optional.of(credentials); + } else { + if (credentials == null) { + LOG.error("No credentials found against session. ID: {}", request.getSession().getId()); + } else { + LOG.error("No refresh token found against session. ID: {}", request.getSession().getId()); + } + } + return Optional.empty(); + } + + private void validateAndSendTokenRequest( + HttpServletRequest req, OidcCredentials oidcCredentials, String computedCallbackUrl) + throws IOException, com.nimbusds.oauth2.sdk.ParseException, URISyntaxException { + if (oidcCredentials.getCode() != null) { + LOG.debug("Initiating Token Request for User Session: {} ", req.getSession().getId()); + CodeVerifier verifier = + (CodeVerifier) + req.getSession().getAttribute(client.getCodeVerifierSessionAttributeName()); + // Token request + TokenRequest request = + createTokenRequest( + new AuthorizationCodeGrant( + oidcCredentials.getCode(), new URI(computedCallbackUrl), verifier)); + executeAuthorizationCodeTokenRequest(request, oidcCredentials); + } + } + + private void validateStateIfRequired( + HttpServletRequest req, + HttpServletResponse resp, + AuthenticationSuccessResponse successResponse) { + if (client.getConfiguration().isWithState()) { + // Validate state for CSRF mitigation + State requestState = + (State) req.getSession().getAttribute(client.getStateSessionAttributeName()); + if (requestState == null || CommonHelper.isBlank(requestState.getValue())) { + getErrorMessage(resp, new TechnicalException("Missing state parameter")); + return; + } + + State responseState = successResponse.getState(); + if (responseState == null) { + throw new TechnicalException("Missing state parameter"); + } + + LOG.debug("Request state: {}/response state: {}", requestState, responseState); + if (!requestState.equals(responseState)) { + throw new TechnicalException( + "State parameter is different from the one sent in authentication request."); + } + } + } + + private OidcCredentials buildCredentials(AuthenticationSuccessResponse successResponse) { + OidcCredentials credentials = new OidcCredentials(); + // get authorization code + AuthorizationCode code = successResponse.getAuthorizationCode(); + if (code != null) { + credentials.setCode(code); + } + // get ID token + JWT idToken = successResponse.getIDToken(); + if (idToken != null) { + credentials.setIdToken(idToken); + } + // get access token + AccessToken accessToken = successResponse.getAccessToken(); + if (accessToken != null) { + credentials.setAccessToken(accessToken); + } + + return credentials; + } + + private void validateNonceIfRequired(HttpServletRequest req, JWTClaimsSet claimsSet) + throws BadJOSEException { + if (client.getConfiguration().isUseNonce()) { + String expectedNonce = + (String) req.getSession().getAttribute(client.getNonceSessionAttributeName()); + if (CommonHelper.isNotBlank(expectedNonce)) { + String tokenNonce; + try { + tokenNonce = claimsSet.getStringClaim("nonce"); + } catch (java.text.ParseException var10) { + throw new BadJWTException("Invalid JWT nonce (nonce) claim: " + var10.getMessage()); + } + + if (tokenNonce == null) { + throw BadJWTExceptions.MISSING_NONCE_CLAIM_EXCEPTION; + } + + if (!expectedNonce.equals(tokenNonce)) { + throw new BadJWTException("Unexpected JWT nonce (nonce) claim: " + tokenNonce); + } + } else { + throw new TechnicalException("Missing nonce parameter from Session."); + } + } + } + + protected Map> retrieveCallbackParameters(HttpServletRequest request) { + Map requestParameters = request.getParameterMap(); + Map> map = new HashMap<>(); + for (var entry : requestParameters.entrySet()) { + map.put(entry.getKey(), Arrays.asList(entry.getValue())); + } + return map; + } + + private void writeJsonResponse(HttpServletResponse response, String message) throws IOException { + response.setContentType("application/json"); + response.setCharacterEncoding("UTF-8"); + response.getOutputStream().print(message); + response.getOutputStream().flush(); + response.setStatus(HttpServletResponse.SC_OK); + } + + private ClientAuthentication getClientAuthentication(OidcConfiguration configuration) { + ClientID clientID = new ClientID(configuration.getClientId()); + ClientAuthentication clientAuthenticationMechanism = null; + if (configuration.getSecret() != null) { + // check authentication methods + List metadataMethods = + configuration.findProviderMetadata().getTokenEndpointAuthMethods(); + + ClientAuthenticationMethod preferredMethod = getPreferredAuthenticationMethod(configuration); + + final ClientAuthenticationMethod chosenMethod; + if (isNotEmpty(metadataMethods)) { + if (preferredMethod != null) { + if (metadataMethods.contains(preferredMethod)) { + chosenMethod = preferredMethod; + } else { + throw new TechnicalException( + "Preferred authentication method (" + + preferredMethod + + ") not supported " + + "by provider according to provider metadata (" + + metadataMethods + + ")."); + } + } else { + chosenMethod = firstSupportedMethod(metadataMethods); + } + } else { + chosenMethod = + preferredMethod != null ? preferredMethod : ClientAuthenticationMethod.getDefault(); + LOG.info( + "Provider metadata does not provide Token endpoint authentication methods. Using: {}", + chosenMethod); + } + + if (ClientAuthenticationMethod.CLIENT_SECRET_POST.equals(chosenMethod)) { + Secret clientSecret = new Secret(configuration.getSecret()); + clientAuthenticationMechanism = new ClientSecretPost(clientID, clientSecret); + } else if (ClientAuthenticationMethod.CLIENT_SECRET_BASIC.equals(chosenMethod)) { + Secret clientSecret = new Secret(configuration.getSecret()); + clientAuthenticationMechanism = new ClientSecretBasic(clientID, clientSecret); + } else if (ClientAuthenticationMethod.PRIVATE_KEY_JWT.equals(chosenMethod)) { + PrivateKeyJWTClientAuthnMethodConfig privateKetJwtConfig = + configuration.getPrivateKeyJWTClientAuthnMethodConfig(); + assertNotNull("privateKetJwtConfig", privateKetJwtConfig); + JWSAlgorithm jwsAlgo = privateKetJwtConfig.getJwsAlgorithm(); + assertNotNull("privateKetJwtConfig.getJwsAlgorithm()", jwsAlgo); + PrivateKey privateKey = privateKetJwtConfig.getPrivateKey(); + assertNotNull("privateKetJwtConfig.getPrivateKey()", privateKey); + String keyID = privateKetJwtConfig.getKeyID(); + try { + clientAuthenticationMechanism = + new PrivateKeyJWT( + clientID, + configuration.findProviderMetadata().getTokenEndpointURI(), + jwsAlgo, + privateKey, + keyID, + null); + } catch (final JOSEException e) { + throw new TechnicalException( + "Cannot instantiate private key JWT client authentication method", e); + } + } + } + + return clientAuthenticationMechanism; + } + + private static ClientAuthenticationMethod getPreferredAuthenticationMethod( + OidcConfiguration config) { + ClientAuthenticationMethod configurationMethod = config.getClientAuthenticationMethod(); + if (configurationMethod == null) { + return null; + } + + if (!SUPPORTED_METHODS.contains(configurationMethod)) { + throw new TechnicalException( + "Configured authentication method (" + configurationMethod + ") is not supported."); + } + + return configurationMethod; + } + + private ClientAuthenticationMethod firstSupportedMethod( + final List metadataMethods) { + Optional firstSupported = + metadataMethods.stream().filter(SUPPORTED_METHODS::contains).findFirst(); + if (firstSupported.isPresent()) { + return firstSupported.get(); + } else { + throw new TechnicalException( + "None of the Token endpoint provider metadata authentication methods are supported: " + + metadataMethods); + } + } + + @SneakyThrows + public static void getErrorMessage(HttpServletResponse resp, Exception e) { + resp.setContentType("text/html; charset=UTF-8"); + LOG.error("[Auth Callback Servlet] Failed in Auth Login : {}", e.getMessage()); + resp.getOutputStream() + .println( + String.format( + "

[Auth Callback Servlet] Failed in Auth Login : %s

", e.getMessage())); + } + + private void sendRedirectWithToken(HttpServletResponse response, OidcCredentials credentials) + throws ParseException, IOException { + JWT jwt = credentials.getIdToken(); + Map claims = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); + claims.putAll(jwt.getJWTClaimsSet().getClaims()); + + String userName = findUserNameFromClaims(claimsMapping, claimsOrder, claims); + String email = findEmailFromClaims(claimsMapping, claimsOrder, claims, principalDomain); + + String url = + String.format( + "%s/auth/callback?id_token=%s&email=%s&name=%s", + serverUrl, credentials.getIdToken().getParsedString(), email, userName); + response.sendRedirect(url); + } + + private void renewOidcCredentials(HttpServletRequest request, OidcCredentials credentials) { + LOG.debug("Renewing Credentials for User Session {}", request.getSession().getId()); + if (client.getConfiguration() instanceof AzureAd2OidcConfiguration azureAd2OidcConfiguration) { + refreshAccessTokenAzureAd2Token(azureAd2OidcConfiguration, credentials); + } else { + refreshTokenRequest(request, credentials); + } + request.getSession().setAttribute(OIDC_CREDENTIAL_PROFILE, credentials); + } + + public void refreshTokenRequest( + final HttpServletRequest httpServletRequest, final OidcCredentials credentials) { + final var refreshToken = credentials.getRefreshToken(); + if (refreshToken != null) { + try { + final var request = createTokenRequest(new RefreshTokenGrant(refreshToken)); + HTTPResponse httpResponse = executeTokenHttpRequest(request); + if (httpResponse.getStatusCode() == 200) { + JSONObject jsonObjectResponse = httpResponse.getContentAsJSONObject(); + String idTokenKey = "id_token"; + if (jsonObjectResponse.containsKey(idTokenKey)) { + Object value = jsonObjectResponse.get(idTokenKey); + if (value == null) { + throw new com.nimbusds.oauth2.sdk.ParseException( + "JSON object member with key " + idTokenKey + " has null value"); + } else { + LOG.info("Found a JWT token in the response, trying to parse it"); + OIDCTokenResponse tokenSuccessResponse = + parseTokenResponseFromHttpResponse(httpResponse); + // Populate credentials + populateCredentialsFromTokenResponse(tokenSuccessResponse, credentials); + } + } else { + // Note: since the id_token is not present, we must receive accessToken + // We can do better and get userInfo from + // client.getConfiguration().findProviderMetadata().getUserInfoEndpointURI() + // but currently we are just return the OM created token in the response + String accessToken = JSONObjectUtils.getString(jsonObjectResponse, "access_token"); + LOG.info( + "Found an access token in the response, trying to parse it, Value : {}", + accessToken); + OIDCTokenResponse tokenSuccessResponse = + parseTokenResponseFromHttpResponse(httpResponse); + // Populate credentials + populateCredentialsFromTokenResponse(tokenSuccessResponse, credentials); + + OidcCredentials storedCredentials = + (OidcCredentials) + httpServletRequest.getSession().getAttribute(OIDC_CREDENTIAL_PROFILE); + + // Get the claims from the stored credentials + Map claims = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); + claims.putAll(storedCredentials.getIdToken().getJWTClaimsSet().getClaims()); + + String username = + SecurityUtil.findUserNameFromClaims(claimsMapping, claimsOrder, claims); + User user = Entity.getEntityByName(Entity.USER, username, "id", Include.NON_DELETED); + + // Create a JWT here + JWTAuthMechanism jwtAuthMechanism = + JWTTokenGenerator.getInstance() + .generateJWTToken( + username, + getRoleListFromUser(user), + !nullOrEmpty(user.getIsAdmin()) && user.getIsAdmin(), + user.getEmail(), + tokenValidity, + false, + ServiceTokenType.OM_USER); + // Set the access token to the new JWT token + credentials.setIdToken(SignedJWT.parse(jwtAuthMechanism.getJWTToken())); + } + return; + } else { + throw new TechnicalException( + String.format( + "Failed to refresh id_token, response code:%s , Error : %s", + httpResponse.getStatusCode(), httpResponse.getContent())); + } + + } catch (final IOException | com.nimbusds.oauth2.sdk.ParseException e) { + throw new TechnicalException(e); + } catch (ParseException e) { + throw new RuntimeException(e); + } + } + throw new BadRequestException("No refresh token available"); + } + + public static boolean isJWT(String token) { + return token.split("\\.").length == 3; + } + + private void refreshAccessTokenAzureAd2Token( + AzureAd2OidcConfiguration azureConfig, OidcCredentials azureAdProfile) { + HttpURLConnection connection = null; + try { + Map headers = new HashMap<>(); + headers.put( + HttpConstants.CONTENT_TYPE_HEADER, HttpConstants.APPLICATION_FORM_ENCODED_HEADER_VALUE); + headers.put(HttpConstants.ACCEPT_HEADER, HttpConstants.APPLICATION_JSON); + // get the token endpoint from discovery URI + URL tokenEndpointURL = azureConfig.findProviderMetadata().getTokenEndpointURI().toURL(); + connection = HttpUtils.openPostConnection(tokenEndpointURL, headers); + + BufferedWriter out = + new BufferedWriter( + new OutputStreamWriter(connection.getOutputStream(), StandardCharsets.UTF_8)); + out.write(azureConfig.makeOauth2TokenRequest(azureAdProfile.getRefreshToken().getValue())); + out.close(); + + int responseCode = connection.getResponseCode(); + if (responseCode != 200) { + throw new TechnicalException( + "request for access token failed: " + HttpUtils.buildHttpErrorMessage(connection)); + } + var body = HttpUtils.readBody(connection); + Map res = JsonUtils.readValue(body, new TypeReference<>() {}); + azureAdProfile.setAccessToken(new BearerAccessToken((String) res.get("access_token"))); + } catch (final IOException e) { + throw new TechnicalException(e); + } finally { + HttpUtils.closeConnection(connection); + } + } + + public static String findUserNameFromClaims( + Map jwtPrincipalClaimsMapping, + List jwtPrincipalClaimsOrder, + Map claims) { + if (!nullOrEmpty(jwtPrincipalClaimsMapping)) { + // We have a mapping available so we will use that + String usernameClaim = jwtPrincipalClaimsMapping.get(USERNAME_CLAIM_KEY); + String userNameClaimValue = getClaimOrObject(claims.get(usernameClaim)); + if (!nullOrEmpty(userNameClaimValue)) { + return userNameClaimValue; + } else { + throw new AuthenticationException("Invalid JWT token, 'username' claim is not present"); + } + } else { + String jwtClaim = getFirstMatchJwtClaim(jwtPrincipalClaimsOrder, claims); + String userName; + if (jwtClaim.contains("@")) { + userName = jwtClaim.split("@")[0]; + } else { + userName = jwtClaim; + } + return userName; + } + } + + public static void validatePrincipalClaimsMapping(Map mapping) { + if (!nullOrEmpty(mapping)) { + String username = mapping.get(USERNAME_CLAIM_KEY); + String email = mapping.get(EMAIL_CLAIM_KEY); + if (nullOrEmpty(username) || nullOrEmpty(email)) { + throw new IllegalArgumentException( + "Invalid JWT Principal Claims Mapping. Both username and email should be present"); + } + } + // If emtpy, jwtPrincipalClaims will be used so no need to validate + } + + private HTTPResponse executeTokenHttpRequest(TokenRequest request) throws IOException { + HTTPRequest tokenHttpRequest = request.toHTTPRequest(); + client.getConfiguration().configureHttpRequest(tokenHttpRequest); + + HTTPResponse httpResponse = tokenHttpRequest.send(); + LOG.debug( + "Token response: status={}, content={}", + httpResponse.getStatusCode(), + httpResponse.getContent()); + + return httpResponse; + } + + private TokenRequest createTokenRequest(final AuthorizationGrant grant) { + if (clientAuthentication != null) { + return new TokenRequest( + client.getConfiguration().findProviderMetadata().getTokenEndpointURI(), + this.clientAuthentication, + grant); + } else { + return new TokenRequest( + client.getConfiguration().findProviderMetadata().getTokenEndpointURI(), + new ClientID(client.getConfiguration().getClientId()), + grant); + } + } + + private void addStateAndNonceParameters( + final OidcClient client, final HttpServletRequest request, final Map params) { + // Init state for CSRF mitigation + if (client.getConfiguration().isWithState()) { + State state = new State(CommonHelper.randomString(10)); + params.put(OidcConfiguration.STATE, state.getValue()); + request.getSession().setAttribute(client.getStateSessionAttributeName(), state); + } + + // Init nonce for replay attack mitigation + if (client.getConfiguration().isUseNonce()) { + Nonce nonce = new Nonce(); + params.put(OidcConfiguration.NONCE, nonce.getValue()); + request.getSession().setAttribute(client.getNonceSessionAttributeName(), nonce.getValue()); + } + + CodeChallengeMethod pkceMethod = client.getConfiguration().findPkceMethod(); + + // Use Default PKCE method if not disabled + if (pkceMethod == null && !client.getConfiguration().isDisablePkce()) { + pkceMethod = CodeChallengeMethod.S256; + } + if (pkceMethod != null) { + CodeVerifier verfifier = new CodeVerifier(CommonHelper.randomString(43)); + request.getSession().setAttribute(client.getCodeVerifierSessionAttributeName(), verfifier); + params.put( + OidcConfiguration.CODE_CHALLENGE, + CodeChallenge.compute(pkceMethod, verfifier).getValue()); + params.put(OidcConfiguration.CODE_CHALLENGE_METHOD, pkceMethod.getValue()); + } + } + + private void executeAuthorizationCodeTokenRequest( + TokenRequest request, OidcCredentials credentials) + throws IOException, com.nimbusds.oauth2.sdk.ParseException { + HTTPResponse httpResponse = executeTokenHttpRequest(request); + OIDCTokenResponse tokenSuccessResponse = parseTokenResponseFromHttpResponse(httpResponse); + + // Populate credentials + populateCredentialsFromTokenResponse(tokenSuccessResponse, credentials); + } + + private void populateCredentialsFromTokenResponse( + OIDCTokenResponse tokenSuccessResponse, OidcCredentials credentials) { + OIDCTokens oidcTokens = tokenSuccessResponse.getOIDCTokens(); + credentials.setAccessToken(oidcTokens.getAccessToken()); + credentials.setRefreshToken(oidcTokens.getRefreshToken()); + if (oidcTokens.getIDToken() != null) { + credentials.setIdToken(oidcTokens.getIDToken()); + } + } + + private OIDCTokenResponse parseTokenResponseFromHttpResponse(HTTPResponse httpResponse) + throws com.nimbusds.oauth2.sdk.ParseException { + TokenResponse response = OIDCTokenResponseParser.parse(httpResponse); + if (response instanceof TokenErrorResponse tokenErrorResponse) { + ErrorObject errorObject = tokenErrorResponse.getErrorObject(); + throw new TechnicalException( + "Bad token response, error=" + + errorObject.getCode() + + "," + + " description=" + + errorObject.getDescription()); + } + LOG.debug("Token response successful"); + return (OIDCTokenResponse) response; + } +} diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/security/SecurityUtil.java b/openmetadata-service/src/main/java/org/openmetadata/service/security/SecurityUtil.java index f8055248484d..c5ad60f725ce 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/security/SecurityUtil.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/security/SecurityUtil.java @@ -14,82 +14,28 @@ package org.openmetadata.service.security; import static org.openmetadata.common.utils.CommonUtil.nullOrEmpty; -import static org.openmetadata.service.security.AuthLoginServlet.OIDC_CREDENTIAL_PROFILE; import static org.openmetadata.service.security.JwtFilter.BOT_CLAIM; import static org.openmetadata.service.security.JwtFilter.EMAIL_CLAIM_KEY; import static org.openmetadata.service.security.JwtFilter.USERNAME_CLAIM_KEY; -import static org.pac4j.core.util.CommonHelper.assertNotNull; -import static org.pac4j.core.util.CommonHelper.isNotEmpty; import com.auth0.jwt.interfaces.Claim; -import com.fasterxml.jackson.core.type.TypeReference; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap.Builder; -import com.nimbusds.jose.JOSEException; -import com.nimbusds.jose.JWSAlgorithm; -import com.nimbusds.jwt.JWT; -import com.nimbusds.oauth2.sdk.auth.ClientAuthentication; -import com.nimbusds.oauth2.sdk.auth.ClientAuthenticationMethod; -import com.nimbusds.oauth2.sdk.auth.ClientSecretBasic; -import com.nimbusds.oauth2.sdk.auth.ClientSecretPost; -import com.nimbusds.oauth2.sdk.auth.PrivateKeyJWT; -import com.nimbusds.oauth2.sdk.auth.Secret; -import com.nimbusds.oauth2.sdk.id.ClientID; -import com.nimbusds.oauth2.sdk.token.BearerAccessToken; -import java.io.BufferedWriter; -import java.io.IOException; -import java.io.OutputStreamWriter; -import java.net.HttpURLConnection; -import java.net.URL; -import java.nio.charset.StandardCharsets; import java.security.Principal; -import java.security.PrivateKey; -import java.text.ParseException; -import java.time.Instant; -import java.util.Arrays; -import java.util.Collection; -import java.util.Date; -import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Optional; -import java.util.TreeMap; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; import javax.ws.rs.client.Invocation; import javax.ws.rs.client.WebTarget; import javax.ws.rs.core.SecurityContext; -import lombok.SneakyThrows; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang.StringUtils; import org.openmetadata.common.utils.CommonUtil; -import org.openmetadata.schema.security.client.OidcClientConfig; import org.openmetadata.service.OpenMetadataApplicationConfig; -import org.openmetadata.service.util.JsonUtils; -import org.pac4j.core.context.HttpConstants; -import org.pac4j.core.exception.TechnicalException; -import org.pac4j.core.util.CommonHelper; -import org.pac4j.core.util.HttpUtils; -import org.pac4j.oidc.client.AzureAd2Client; -import org.pac4j.oidc.client.GoogleOidcClient; -import org.pac4j.oidc.client.OidcClient; -import org.pac4j.oidc.config.AzureAd2OidcConfiguration; -import org.pac4j.oidc.config.OidcConfiguration; -import org.pac4j.oidc.config.PrivateKeyJWTClientAuthnMethodConfig; -import org.pac4j.oidc.credentials.OidcCredentials; -import org.pac4j.oidc.credentials.authenticator.OidcAuthenticator; @Slf4j public final class SecurityUtil { public static final String DEFAULT_PRINCIPAL_DOMAIN = "openmetadata.org"; - private static final Collection SUPPORTED_METHODS = - Arrays.asList( - ClientAuthenticationMethod.CLIENT_SECRET_POST, - ClientAuthenticationMethod.CLIENT_SECRET_BASIC, - ClientAuthenticationMethod.PRIVATE_KEY_JWT, - ClientAuthenticationMethod.NONE); - private SecurityUtil() {} public static String getUserName(SecurityContext securityContext) { @@ -131,293 +77,6 @@ public static Invocation.Builder addHeaders(WebTarget target, Map metadataMethods = - configuration.findProviderMetadata().getTokenEndpointAuthMethods(); - - ClientAuthenticationMethod preferredMethod = getPreferredAuthenticationMethod(configuration); - - final ClientAuthenticationMethod chosenMethod; - if (isNotEmpty(metadataMethods)) { - if (preferredMethod != null) { - if (metadataMethods.contains(preferredMethod)) { - chosenMethod = preferredMethod; - } else { - throw new TechnicalException( - "Preferred authentication method (" - + preferredMethod - + ") not supported " - + "by provider according to provider metadata (" - + metadataMethods - + ")."); - } - } else { - chosenMethod = firstSupportedMethod(metadataMethods); - } - } else { - chosenMethod = - preferredMethod != null ? preferredMethod : ClientAuthenticationMethod.getDefault(); - LOG.info( - "Provider metadata does not provide Token endpoint authentication methods. Using: {}", - chosenMethod); - } - - if (ClientAuthenticationMethod.CLIENT_SECRET_POST.equals(chosenMethod)) { - Secret clientSecret = new Secret(configuration.getSecret()); - clientAuthenticationMechanism = new ClientSecretPost(clientID, clientSecret); - } else if (ClientAuthenticationMethod.CLIENT_SECRET_BASIC.equals(chosenMethod)) { - Secret clientSecret = new Secret(configuration.getSecret()); - clientAuthenticationMechanism = new ClientSecretBasic(clientID, clientSecret); - } else if (ClientAuthenticationMethod.PRIVATE_KEY_JWT.equals(chosenMethod)) { - PrivateKeyJWTClientAuthnMethodConfig privateKetJwtConfig = - configuration.getPrivateKeyJWTClientAuthnMethodConfig(); - assertNotNull("privateKetJwtConfig", privateKetJwtConfig); - JWSAlgorithm jwsAlgo = privateKetJwtConfig.getJwsAlgorithm(); - assertNotNull("privateKetJwtConfig.getJwsAlgorithm()", jwsAlgo); - PrivateKey privateKey = privateKetJwtConfig.getPrivateKey(); - assertNotNull("privateKetJwtConfig.getPrivateKey()", privateKey); - String keyID = privateKetJwtConfig.getKeyID(); - try { - clientAuthenticationMechanism = - new PrivateKeyJWT( - clientID, - configuration.findProviderMetadata().getTokenEndpointURI(), - jwsAlgo, - privateKey, - keyID, - null); - } catch (final JOSEException e) { - throw new TechnicalException( - "Cannot instantiate private key JWT client authentication method", e); - } - } - } - - return clientAuthenticationMechanism; - } - - private static ClientAuthenticationMethod getPreferredAuthenticationMethod( - OidcConfiguration config) { - ClientAuthenticationMethod configurationMethod = config.getClientAuthenticationMethod(); - if (configurationMethod == null) { - return null; - } - - if (!SUPPORTED_METHODS.contains(configurationMethod)) { - throw new TechnicalException( - "Configured authentication method (" + configurationMethod + ") is not supported."); - } - - return configurationMethod; - } - - private static ClientAuthenticationMethod firstSupportedMethod( - final List metadataMethods) { - Optional firstSupported = - metadataMethods.stream().filter(SUPPORTED_METHODS::contains).findFirst(); - if (firstSupported.isPresent()) { - return firstSupported.get(); - } else { - throw new TechnicalException( - "None of the Token endpoint provider metadata authentication methods are supported: " - + metadataMethods); - } - } - - @SneakyThrows - public static void getErrorMessage(HttpServletResponse resp, Exception e) { - resp.setContentType("text/html; charset=UTF-8"); - LOG.error("[Auth Callback Servlet] Failed in Auth Login : {}", e.getMessage()); - resp.getOutputStream() - .println( - String.format( - "

[Auth Callback Servlet] Failed in Auth Login : %s

", e.getMessage())); - } - - public static void sendRedirectWithToken( - HttpServletResponse response, - OidcCredentials credentials, - String serverUrl, - Map claimsMapping, - List claimsOrder, - String defaultDomain) - throws ParseException, IOException { - JWT jwt = credentials.getIdToken(); - Map claims = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); - claims.putAll(jwt.getJWTClaimsSet().getClaims()); - - String userName = findUserNameFromClaims(claimsMapping, claimsOrder, claims); - String email = findEmailFromClaims(claimsMapping, claimsOrder, claims, defaultDomain); - - String url = - String.format( - "%s/auth/callback?id_token=%s&email=%s&name=%s", - serverUrl, credentials.getIdToken().getParsedString(), email, userName); - response.sendRedirect(url); - } - - public static boolean isCredentialsExpired(OidcCredentials credentials) throws ParseException { - Date expiration = credentials.getIdToken().getJWTClaimsSet().getExpirationTime(); - return expiration != null && expiration.toInstant().isBefore(Instant.now().plusSeconds(30)); - } - - public static Optional getUserCredentialsFromSession( - HttpServletRequest request, OidcClient client) throws ParseException { - OidcCredentials credentials = - (OidcCredentials) request.getSession().getAttribute(OIDC_CREDENTIAL_PROFILE); - if (credentials != null && credentials.getRefreshToken() != null) { - removeOrRenewOidcCredentials(request, client, credentials); - return Optional.of(credentials); - } else { - if (credentials == null) { - LOG.error("No credentials found against session. ID: {}", request.getSession().getId()); - } else { - LOG.error("No refresh token found against session. ID: {}", request.getSession().getId()); - } - } - return Optional.empty(); - } - - private static void removeOrRenewOidcCredentials( - HttpServletRequest request, OidcClient client, OidcCredentials credentials) { - LOG.debug("Expired credentials found, trying to renew."); - if (client.getConfiguration() instanceof AzureAd2OidcConfiguration azureAd2OidcConfiguration) { - refreshAccessTokenAzureAd2Token(azureAd2OidcConfiguration, credentials); - } else { - OidcAuthenticator authenticator = new OidcAuthenticator(client.getConfiguration(), client); - authenticator.refresh(credentials); - } - request.getSession().setAttribute(OIDC_CREDENTIAL_PROFILE, credentials); - } - - private static void refreshAccessTokenAzureAd2Token( - AzureAd2OidcConfiguration azureConfig, OidcCredentials azureAdProfile) { - HttpURLConnection connection = null; - try { - Map headers = new HashMap<>(); - headers.put( - HttpConstants.CONTENT_TYPE_HEADER, HttpConstants.APPLICATION_FORM_ENCODED_HEADER_VALUE); - headers.put(HttpConstants.ACCEPT_HEADER, HttpConstants.APPLICATION_JSON); - // get the token endpoint from discovery URI - URL tokenEndpointURL = azureConfig.findProviderMetadata().getTokenEndpointURI().toURL(); - connection = HttpUtils.openPostConnection(tokenEndpointURL, headers); - - BufferedWriter out = - new BufferedWriter( - new OutputStreamWriter(connection.getOutputStream(), StandardCharsets.UTF_8)); - out.write(azureConfig.makeOauth2TokenRequest(azureAdProfile.getRefreshToken().getValue())); - out.close(); - - int responseCode = connection.getResponseCode(); - if (responseCode != 200) { - throw new TechnicalException( - "request for access token failed: " + HttpUtils.buildHttpErrorMessage(connection)); - } - var body = HttpUtils.readBody(connection); - Map res = JsonUtils.readValue(body, new TypeReference<>() {}); - azureAdProfile.setAccessToken(new BearerAccessToken((String) res.get("access_token"))); - } catch (final IOException e) { - throw new TechnicalException(e); - } finally { - HttpUtils.closeConnection(connection); - } - } - public static String findUserNameFromClaims( Map jwtPrincipalClaimsMapping, List jwtPrincipalClaimsOrder, @@ -470,7 +129,7 @@ public static String findEmailFromClaims( } } - private static String getClaimOrObject(Object obj) { + public static String getClaimOrObject(Object obj) { if (obj == null) { return ""; } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/security/saml/SamlAssertionConsumerServlet.java b/openmetadata-service/src/main/java/org/openmetadata/service/security/saml/SamlAssertionConsumerServlet.java index a17842e129f7..0a0b7c6ce3c3 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/security/saml/SamlAssertionConsumerServlet.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/security/saml/SamlAssertionConsumerServlet.java @@ -92,7 +92,7 @@ private void handleResponse(HttpServletRequest req, HttpServletResponse resp) th username, getRoleListFromUser(user), !nullOrEmpty(user.getIsAdmin()) && user.getIsAdmin(), - email, + user.getEmail(), SamlSettingsHolder.getInstance().getTokenValidity(), false, ServiceTokenType.OM_USER); diff --git a/openmetadata-spec/src/main/resources/json/schema/security/client/oidcClientConfig.json b/openmetadata-spec/src/main/resources/json/schema/security/client/oidcClientConfig.json index 2e45e5ea3a09..528da87ac366 100644 --- a/openmetadata-spec/src/main/resources/json/schema/security/client/oidcClientConfig.json +++ b/openmetadata-spec/src/main/resources/json/schema/security/client/oidcClientConfig.json @@ -56,6 +56,11 @@ "type": "string", "enum": ["client_secret_basic", "client_secret_post", "client_secret_jwt", "private_key_jwt"] }, + "tokenValidity": { + "description": "Validity for the JWT Token created from SAML Response", + "type": "integer", + "default": "3600" + }, "customParams": { "description": "Custom Params.", "existingJavaType" : "java.util.Map", From 28218ec61273ead3fc79f451a3e69bdde9c3eb33 Mon Sep 17 00:00:00 2001 From: Mohit Yadav <105265192+mohityadav766@users.noreply.github.com> Date: Fri, 19 Jul 2024 01:16:50 +0530 Subject: [PATCH 09/33] Ignore Entity No Change Events (#17087) --- .../openmetadata/service/events/ChangeEventHandler.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/events/ChangeEventHandler.java b/openmetadata-service/src/main/java/org/openmetadata/service/events/ChangeEventHandler.java index 287edcbe47d6..82d59d4aa826 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/events/ChangeEventHandler.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/events/ChangeEventHandler.java @@ -22,6 +22,7 @@ import lombok.SneakyThrows; import lombok.extern.slf4j.Slf4j; import org.openmetadata.schema.type.ChangeEvent; +import org.openmetadata.schema.type.EventType; import org.openmetadata.service.Entity; import org.openmetadata.service.OpenMetadataApplicationConfig; import org.openmetadata.service.util.JsonUtils; @@ -71,8 +72,10 @@ public Void process( changeEvent.setEntity(JsonUtils.pojoToMaskedJson(entity)); } - // Thread are created in FeedRepository Directly - Entity.getCollectionDAO().changeEventDAO().insert(JsonUtils.pojoToJson(changeEvent)); + // Insert ChangeEvents if ENTITY Changed + if (!changeEvent.getEventType().equals(EventType.ENTITY_NO_CHANGE)) { + Entity.getCollectionDAO().changeEventDAO().insert(JsonUtils.pojoToJson(changeEvent)); + } } } catch (Exception e) { LOG.error( From c7710481b55f29e75039bdf791879a25aeccdd9a Mon Sep 17 00:00:00 2001 From: Mohit Yadav <105265192+mohityadav766@users.noreply.github.com> Date: Fri, 19 Jul 2024 01:17:03 +0530 Subject: [PATCH 10/33] Fix App Issue (#17088) --- .../apps/AbstractNativeApplication.java | 5 +++-- .../service/resources/apps/AppResource.java | 19 ++++--------------- 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/apps/AbstractNativeApplication.java b/openmetadata-service/src/main/java/org/openmetadata/service/apps/AbstractNativeApplication.java index f92d546dbde2..378333cdbc72 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/apps/AbstractNativeApplication.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/apps/AbstractNativeApplication.java @@ -63,8 +63,9 @@ public void init(App app) { @Override public void install() { // If the app does not have any Schedule Return without scheduling - if (app.getAppSchedule() != null - && app.getAppSchedule().getScheduleTimeline().equals(ScheduleTimeline.NONE)) { + if (Boolean.FALSE.equals(app.getDeleted()) + || (app.getAppSchedule() != null + && app.getAppSchedule().getScheduleTimeline().equals(ScheduleTimeline.NONE))) { return; } if (app.getAppType() == AppType.Internal diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/resources/apps/AppResource.java b/openmetadata-service/src/main/java/org/openmetadata/service/resources/apps/AppResource.java index e30a9b218bb6..7cf4705987ec 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/resources/apps/AppResource.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/resources/apps/AppResource.java @@ -715,7 +715,8 @@ public Response delete( @Parameter(description = "Name of the App", schema = @Schema(type = "string")) @PathParam("name") String name) { - App app = repository.getByName(null, name, repository.getFields("bot,pipelines")); + App app = + repository.getByName(uriInfo, name, repository.getFields("bot,pipelines"), ALL, false); if (app.getSystem()) { throw new IllegalArgumentException( CatalogExceptionMessage.systemEntityDeleteNotAllowed(app.getName(), "SystemApp")); @@ -748,7 +749,7 @@ public Response delete( boolean hardDelete, @Parameter(description = "Id of the App", schema = @Schema(type = "UUID")) @PathParam("id") UUID id) { - App app = repository.get(null, id, repository.getFields("bot,pipelines")); + App app = repository.get(uriInfo, id, repository.getFields("bot,pipelines"), ALL, false); if (app.getSystem()) { throw new IllegalArgumentException( CatalogExceptionMessage.systemEntityDeleteNotAllowed(app.getName(), "SystemApp")); @@ -1054,19 +1055,7 @@ private void deleteApp(SecurityContext securityContext, App installedApp, boolea ingestionPipelineRepository.get( null, pipelineRef.getId(), ingestionPipelineRepository.getFields(FIELD_OWNER)); try { - if (hardDelete) { - // Remove the Pipeline in case of Delete - if (!nullOrEmpty(installedApp.getPipelines())) { - pipelineServiceClient.deletePipeline(ingestionPipeline); - } - } else { - // Just Kill Running ingestion - if (Boolean.TRUE.equals(ingestionPipeline.getDeployed())) { - decryptOrNullify( - securityContext, ingestionPipeline, installedApp.getBot().getName(), true); - pipelineServiceClient.killIngestion(ingestionPipeline); - } - } + pipelineServiceClient.deletePipeline(ingestionPipeline); } catch (Exception ex) { LOG.error("Failed in Pipeline Service Client : ", ex); } From 8561022c70d61d90124f442634b90ff07a064de7 Mon Sep 17 00:00:00 2001 From: Mohit Yadav <105265192+mohityadav766@users.noreply.github.com> Date: Fri, 19 Jul 2024 01:17:12 +0530 Subject: [PATCH 11/33] Add Sync Option in OpenmetadataOperations (#17089) --- .../service/util/OpenMetadataOperations.java | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/util/OpenMetadataOperations.java b/openmetadata-service/src/main/java/org/openmetadata/service/util/OpenMetadataOperations.java index 80b2f813c429..526ccdb5161e 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/util/OpenMetadataOperations.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/util/OpenMetadataOperations.java @@ -44,6 +44,8 @@ import org.openmetadata.schema.entity.app.App; import org.openmetadata.schema.entity.app.AppRunRecord; import org.openmetadata.schema.entity.services.ingestionPipelines.IngestionPipeline; +import org.openmetadata.schema.settings.Settings; +import org.openmetadata.schema.settings.SettingsType; import org.openmetadata.schema.system.EventPublisherJob; import org.openmetadata.schema.type.Include; import org.openmetadata.sdk.PipelineServiceClientInterface; @@ -60,6 +62,7 @@ import org.openmetadata.service.jdbi3.IngestionPipelineRepository; import org.openmetadata.service.jdbi3.ListFilter; import org.openmetadata.service.jdbi3.MigrationDAO; +import org.openmetadata.service.jdbi3.SystemRepository; import org.openmetadata.service.jdbi3.locator.ConnectionAwareAnnotationSqlLocator; import org.openmetadata.service.jdbi3.locator.ConnectionType; import org.openmetadata.service.migration.api.MigrationWorkflow; @@ -160,6 +163,26 @@ public Integer repair() { } } + @Command( + name = "syncEmailFromEnv", + description = "Sync the email configuration from environment variables") + public Integer syncEmailFromEnv() { + try { + parseConfig(); + Entity.setCollectionDAO(jdbi.onDemand(CollectionDAO.class)); + SystemRepository systemRepository = new SystemRepository(); + Settings updatedSettings = + new Settings() + .withConfigType(SettingsType.EMAIL_CONFIGURATION) + .withConfigValue(config.getSmtpSettings()); + systemRepository.createOrUpdate(updatedSettings); + return 0; + } catch (Exception e) { + LOG.error("Email Sync failed due to ", e); + return 1; + } + } + @Command( name = "check-connection", description = From b1620e682a8e048ca1e4a5425c2305250b319c94 Mon Sep 17 00:00:00 2001 From: Mohit Yadav <105265192+mohityadav766@users.noreply.github.com> Date: Fri, 19 Jul 2024 11:39:09 +0530 Subject: [PATCH 12/33] Fix Users Issue due to capital names (#16828) * Fix Users Issue due to capital names * Fixed Tests * No lowercasing name * Revert * toLowerCase * fix email casing from UI side on basic auth * compare lowercase email only from the loggedInUser * address comments * Updated LowerCase UserName on updates * revert ui changes * Migrate Name and Email * cypress failure * fix glossary test * fix activity feed test * Compare by Lowercasing Email and username --------- Co-authored-by: Chirag Madlani <12962843+chirag-madlani@users.noreply.github.com> Co-authored-by: Chira Madlani Co-authored-by: karanh37 --- .../mysql/postDataMigrationSQLScript.sql | 44 +++++++++++++++++++ .../postgres/postDataMigrationSQLScript.sql | 44 +++++++++++++++++++ .../java/org/openmetadata/csv/EntityCsv.java | 2 +- .../service/jdbi3/AppRepository.java | 6 +-- .../service/jdbi3/UserRepository.java | 28 ++++++++---- .../service/resources/teams/UserResource.java | 21 +-------- .../service/security/CatalogPrincipal.java | 14 +++--- .../service/security/NoopAuthorizer.java | 11 ++--- .../service/security/SecurityUtil.java | 25 +++++------ .../security/auth/BasicAuthenticator.java | 20 ++++----- .../security/auth/LdapAuthenticator.java | 22 +++------- .../openmetadata/service/util/UserUtil.java | 31 +++++++++---- .../service/resources/EntityResourceTest.java | 2 +- .../resources/teams/UserResourceTest.java | 44 ++++++++++++------- .../ui/cypress/e2e/Pages/Bots.spec.ts | 2 +- .../ui/cypress/e2e/Pages/Users.spec.ts | 2 +- .../e2e/Features/ActivityFeed.spec.ts | 2 +- .../ui/src/components/AppRouter/AppRouter.tsx | 5 +-- .../Auth/AuthProviders/BasicAuthProvider.tsx | 2 +- .../pages/SignUp/BasicSignup.component.tsx | 1 - 20 files changed, 206 insertions(+), 122 deletions(-) diff --git a/bootstrap/sql/migrations/native/1.5.0/mysql/postDataMigrationSQLScript.sql b/bootstrap/sql/migrations/native/1.5.0/mysql/postDataMigrationSQLScript.sql index 306fdd27d6d5..740cbab52e06 100644 --- a/bootstrap/sql/migrations/native/1.5.0/mysql/postDataMigrationSQLScript.sql +++ b/bootstrap/sql/migrations/native/1.5.0/mysql/postDataMigrationSQLScript.sql @@ -23,3 +23,47 @@ WHERE name IN ( 'columnValuesToBeBetween', 'tableRowCountToBeBetween' ); + +-- Remove Duplicate Usernames and Lowercase Them +WITH cte AS ( + SELECT + id, + ROW_NUMBER() OVER (PARTITION BY LOWER(JSON_UNQUOTE(JSON_EXTRACT(json, '$.name'))) ORDER BY id) as rn + FROM + user_entity +) +DELETE FROM user_entity +WHERE id IN ( + SELECT id + FROM cte + WHERE rn > 1 +); + +UPDATE user_entity +SET json = JSON_SET( + json, + '$.name', + LOWER(JSON_UNQUOTE(JSON_EXTRACT(json, '$.name'))) +); + +-- Remove Duplicate Emails and Lowercase Them +WITH cte AS ( + SELECT + id, + ROW_NUMBER() OVER (PARTITION BY LOWER(JSON_UNQUOTE(JSON_EXTRACT(json, '$.email'))) ORDER BY id) as rn + FROM + user_entity +) +DELETE FROM user_entity +WHERE id IN ( + SELECT id + FROM cte + WHERE rn > 1 +); + +UPDATE user_entity +SET json = JSON_SET( + json, + '$.email', + LOWER(JSON_UNQUOTE(JSON_EXTRACT(json, '$.email'))) +); diff --git a/bootstrap/sql/migrations/native/1.5.0/postgres/postDataMigrationSQLScript.sql b/bootstrap/sql/migrations/native/1.5.0/postgres/postDataMigrationSQLScript.sql index 2c578286e40c..3ac2106b72a4 100644 --- a/bootstrap/sql/migrations/native/1.5.0/postgres/postDataMigrationSQLScript.sql +++ b/bootstrap/sql/migrations/native/1.5.0/postgres/postDataMigrationSQLScript.sql @@ -22,3 +22,47 @@ WHERE name IN ( 'columnValuesToBeBetween', 'tableRowCountToBeBetween' ); + +-- Remove Duplicate UserNames and lowercase them +WITH cte AS ( + SELECT + id, + ROW_NUMBER() OVER (PARTITION BY to_jsonb(LOWER(json->>'name')) ORDER BY id) as rn + FROM + user_entity +) +DELETE from user_entity +WHERE id IN ( + SELECT id + FROM cte + WHERE rn > 1 +); + +UPDATE user_entity +SET json = jsonb_set( + json, + '{name}', + to_jsonb(LOWER(json->>'name')) +); + +-- Remove Duplicate Emails and lowercase them +WITH cte AS ( + SELECT + id, + ROW_NUMBER() OVER (PARTITION BY to_jsonb(LOWER(json->>'email')) ORDER BY id) as rn + FROM + user_entity +) +DELETE from user_entity +WHERE id IN ( + SELECT id + FROM cte + WHERE rn > 1 +); + +UPDATE user_entity +SET json = jsonb_set( + json, + '{email}', + to_jsonb(LOWER(json->>'email')) +); diff --git a/openmetadata-service/src/main/java/org/openmetadata/csv/EntityCsv.java b/openmetadata-service/src/main/java/org/openmetadata/csv/EntityCsv.java index d954688c7c2d..26a9d1be056c 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/csv/EntityCsv.java +++ b/openmetadata-service/src/main/java/org/openmetadata/csv/EntityCsv.java @@ -77,7 +77,7 @@ public abstract class EntityCsv { protected final CsvImportResult importResult = new CsvImportResult(); protected boolean processRecord; // When set to false record processing is discontinued protected final Map dryRunCreatedEntities = new HashMap<>(); - private final String importedBy; + protected final String importedBy; protected int recordIndex = 0; protected EntityCsv(String entityType, List csvHeaders, String importedBy) { diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/AppRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/AppRepository.java index a80fd38680cb..90869fa5c555 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/AppRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/AppRepository.java @@ -1,7 +1,7 @@ package org.openmetadata.service.jdbi3; import static org.openmetadata.service.exception.AppException.APP_RUN_RECORD_NOT_FOUND; -import static org.openmetadata.service.resources.teams.UserResource.getUser; +import static org.openmetadata.service.util.UserUtil.getUser; import java.util.ArrayList; import java.util.List; @@ -118,12 +118,12 @@ public EntityReference createNewAppBot(App application) { Bot appBot = new Bot() .withId(UUID.randomUUID()) - .withName(botUser.getName()) + .withName(botName) .withUpdatedBy("admin") .withUpdatedAt(System.currentTimeMillis()) .withBotUser(botUser.getEntityReference()) .withProvider(ProviderType.USER) - .withFullyQualifiedName(botUser.getName()); + .withFullyQualifiedName(botName); // Create Bot with above user bot = botRepository.createInternal(appBot); diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/UserRepository.java b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/UserRepository.java index efcbf82b20a2..0288722922b5 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/UserRepository.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/UserRepository.java @@ -42,6 +42,7 @@ import org.jdbi.v3.sqlobject.transaction.Transaction; import org.openmetadata.csv.EntityCsv; import org.openmetadata.schema.api.teams.CreateTeam.TeamType; +import org.openmetadata.schema.api.teams.CreateUser; import org.openmetadata.schema.entity.teams.AuthenticationMechanism; import org.openmetadata.schema.entity.teams.Team; import org.openmetadata.schema.entity.teams.User; @@ -338,7 +339,10 @@ public List getGroupTeams( public void validateLoggedInUserNameAndEmailMatches( String username, String email, User storedUser) { - if (!(username.equals(storedUser.getName()) && email.equals(storedUser.getEmail()))) { + String lowerCasedName = username.toLowerCase(); + String lowerCasedEmail = email.toLowerCase(); + if (!(lowerCasedName.equals(storedUser.getName().toLowerCase()) + && lowerCasedEmail.equals(storedUser.getEmail().toLowerCase()))) { throw EntityNotFoundException.byMessage(CatalogExceptionMessage.entityNotFound(USER, email)); } } @@ -436,13 +440,15 @@ protected void createEntity(CSVPrinter printer, List csvRecords) thro CSVRecord csvRecord = getNextRecord(printer, csvRecords); // Field 1, 2, 3, 4, 5, 6 - name, displayName, description, email, timezone, isAdmin User user = - new User() - .withName(csvRecord.get(0)) - .withDisplayName(csvRecord.get(1)) - .withDescription(csvRecord.get(2)) - .withEmail(csvRecord.get(3)) - .withTimezone(csvRecord.get(4)) - .withIsAdmin(getBoolean(printer, csvRecord, 5)) + UserUtil.getUser( + importedBy, + new CreateUser() + .withName(csvRecord.get(0)) + .withDisplayName(csvRecord.get(1)) + .withDescription(csvRecord.get(2)) + .withEmail(csvRecord.get(3)) + .withTimezone(csvRecord.get(4)) + .withIsAdmin(getBoolean(printer, csvRecord, 5))) .withTeams(getTeams(printer, csvRecord, csvRecord.get(0))) .withRoles(getEntityReferences(printer, csvRecord, 7, ROLE)); if (processRecord) { @@ -542,6 +548,10 @@ public UserUpdater(User original, User updated, Operation operation) { @Transaction @Override public void entitySpecificUpdate() { + // LowerCase Email + updated.setEmail(updated.getEmail().toLowerCase()); + + // Updates updateRoles(original, updated); updateTeams(original, updated); updatePersonas(original, updated); @@ -551,7 +561,7 @@ public void entitySpecificUpdate() { recordChange("timezone", original.getTimezone(), updated.getTimezone()); recordChange("isBot", original.getIsBot(), updated.getIsBot()); recordChange("isAdmin", original.getIsAdmin(), updated.getIsAdmin()); - recordChange("email", original.getEmail(), updated.getEmail()); + recordChange("email", original.getEmail(), updated.getEmail().toLowerCase()); recordChange("isEmailVerified", original.getIsEmailVerified(), updated.getIsEmailVerified()); updateAuthenticationMechanism(original, updated); } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/resources/teams/UserResource.java b/openmetadata-service/src/main/java/org/openmetadata/service/resources/teams/UserResource.java index dbfd14bd6ac9..f7e5a0425037 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/resources/teams/UserResource.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/resources/teams/UserResource.java @@ -29,6 +29,7 @@ import static org.openmetadata.service.security.jwt.JWTTokenGenerator.getExpiryDate; import static org.openmetadata.service.util.UserUtil.getRoleListFromUser; import static org.openmetadata.service.util.UserUtil.getRolesFromAuthorizationToken; +import static org.openmetadata.service.util.UserUtil.getUser; import static org.openmetadata.service.util.UserUtil.reSyncUserRolesFromToken; import static org.openmetadata.service.util.UserUtil.validateAndGetRolesRef; @@ -1450,26 +1451,6 @@ public CsvImportResult importCsv( return importCsvInternal(securityContext, team, csv, dryRun); } - public static User getUser(String updatedBy, CreateUser create) { - return new User() - .withId(UUID.randomUUID()) - .withName(create.getName()) - .withFullyQualifiedName(create.getName()) - .withEmail(create.getEmail()) - .withDescription(create.getDescription()) - .withDisplayName(create.getDisplayName()) - .withIsBot(create.getIsBot()) - .withIsAdmin(create.getIsAdmin()) - .withProfile(create.getProfile()) - .withPersonas(create.getPersonas()) - .withDefaultPersona(create.getDefaultPersona()) - .withTimezone(create.getTimezone()) - .withUpdatedBy(updatedBy) - .withUpdatedAt(System.currentTimeMillis()) - .withTeams(EntityUtil.toEntityReferences(create.getTeams(), Entity.TEAM)) - .withRoles(EntityUtil.toEntityReferences(create.getRoles(), Entity.ROLE)); - } - public void validateEmailAlreadyExists(String email) { if (repository.checkEmailAlreadyExists(email)) { throw new CustomExceptionMessage( diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/security/CatalogPrincipal.java b/openmetadata-service/src/main/java/org/openmetadata/service/security/CatalogPrincipal.java index 416df9e2cc8e..39c72f15664a 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/security/CatalogPrincipal.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/security/CatalogPrincipal.java @@ -16,14 +16,14 @@ import java.security.Principal; import lombok.Getter; -public record CatalogPrincipal(@Getter String name, @Getter String email) implements Principal { - @Override - public String getName() { - return name; - } +@Getter +public class CatalogPrincipal implements Principal { + private final String name; + private final String email; - public String getEmail() { - return email; + public CatalogPrincipal(String name, String email) { + this.name = name.toLowerCase(); + this.email = email.toLowerCase(); } @Override diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/security/NoopAuthorizer.java b/openmetadata-service/src/main/java/org/openmetadata/service/security/NoopAuthorizer.java index 5942557bf8a1..22a8bcf3ccd9 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/security/NoopAuthorizer.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/security/NoopAuthorizer.java @@ -14,9 +14,9 @@ package org.openmetadata.service.security; import java.util.List; -import java.util.UUID; import javax.ws.rs.core.SecurityContext; import lombok.extern.slf4j.Slf4j; +import org.openmetadata.schema.api.teams.CreateUser; import org.openmetadata.schema.entity.teams.User; import org.openmetadata.schema.type.EntityReference; import org.openmetadata.schema.type.Include; @@ -30,6 +30,7 @@ import org.openmetadata.service.security.policyevaluator.PolicyEvaluator; import org.openmetadata.service.security.policyevaluator.ResourceContextInterface; import org.openmetadata.service.util.RestUtil.PutResponse; +import org.openmetadata.service.util.UserUtil; @Slf4j public class NoopAuthorizer implements Authorizer { @@ -70,12 +71,8 @@ private void addAnonymousUser() { Entity.getEntityByName(Entity.USER, username, "", Include.NON_DELETED); } catch (EntityNotFoundException ex) { User user = - new User() - .withId(UUID.randomUUID()) - .withName(username) - .withEmail(username + "@domain.com") - .withUpdatedBy(username) - .withUpdatedAt(System.currentTimeMillis()); + UserUtil.getUser( + username, new CreateUser().withName(username).withEmail(username + "@domain.com")); addOrUpdateUser(user); } catch (Exception e) { LOG.error("Failed to create anonymous user {}", username, e); diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/security/SecurityUtil.java b/openmetadata-service/src/main/java/org/openmetadata/service/security/SecurityUtil.java index c5ad60f725ce..cef0c608dee3 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/security/SecurityUtil.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/security/SecurityUtil.java @@ -81,25 +81,21 @@ public static String findUserNameFromClaims( Map jwtPrincipalClaimsMapping, List jwtPrincipalClaimsOrder, Map claims) { + String userName; if (!nullOrEmpty(jwtPrincipalClaimsMapping)) { // We have a mapping available so we will use that String usernameClaim = jwtPrincipalClaimsMapping.get(USERNAME_CLAIM_KEY); String userNameClaimValue = getClaimOrObject(claims.get(usernameClaim)); if (!nullOrEmpty(userNameClaimValue)) { - return userNameClaimValue; + userName = userNameClaimValue; } else { throw new AuthenticationException("Invalid JWT token, 'username' claim is not present"); } } else { String jwtClaim = getFirstMatchJwtClaim(jwtPrincipalClaimsOrder, claims); - String userName; - if (jwtClaim.contains("@")) { - userName = jwtClaim.split("@")[0]; - } else { - userName = jwtClaim; - } - return userName; + userName = jwtClaim.contains("@") ? jwtClaim.split("@")[0] : jwtClaim; } + return userName.toLowerCase(); } public static String findEmailFromClaims( @@ -107,12 +103,13 @@ public static String findEmailFromClaims( List jwtPrincipalClaimsOrder, Map claims, String defaulPrincipalClaim) { + String email; if (!nullOrEmpty(jwtPrincipalClaimsMapping)) { // We have a mapping available so we will use that String emailClaim = jwtPrincipalClaimsMapping.get(EMAIL_CLAIM_KEY); String emailClaimValue = getClaimOrObject(claims.get(emailClaim)); if (!nullOrEmpty(emailClaimValue) && emailClaimValue.contains("@")) { - return emailClaimValue; + email = emailClaimValue; } else { throw new AuthenticationException( String.format( @@ -121,12 +118,12 @@ public static String findEmailFromClaims( } } else { String jwtClaim = getFirstMatchJwtClaim(jwtPrincipalClaimsOrder, claims); - if (jwtClaim.contains("@")) { - return jwtClaim; - } else { - return String.format("%s@%s", jwtClaim, defaulPrincipalClaim); - } + email = + jwtClaim.contains("@") + ? jwtClaim + : String.format("%s@%s", jwtClaim, defaulPrincipalClaim); } + return email.toLowerCase(); } public static String getClaimOrObject(Object obj) { diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/security/auth/BasicAuthenticator.java b/openmetadata-service/src/main/java/org/openmetadata/service/security/auth/BasicAuthenticator.java index 441c88603e09..bc5e433aca7a 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/security/auth/BasicAuthenticator.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/security/auth/BasicAuthenticator.java @@ -39,6 +39,7 @@ import static org.openmetadata.service.resources.teams.UserResource.USER_PROTECTED_FIELDS; import static org.openmetadata.service.util.EmailUtil.getSmtpSettings; import static org.openmetadata.service.util.UserUtil.getRoleListFromUser; +import static org.openmetadata.service.util.UserUtil.getUser; import at.favre.lib.crypto.bcrypt.BCrypt; import freemarker.template.TemplateException; @@ -449,17 +450,14 @@ private User getUserFromRegistrationRequest(RegistrationRequest create) { BCrypt.withDefaults().hashToString(HASHING_COST, create.getPassword().toCharArray()); BasicAuthMechanism newAuthMechanism = new BasicAuthMechanism().withPassword(hashedPwd); - return new User() - .withId(UUID.randomUUID()) - .withName(username) - .withFullyQualifiedName(username) - .withEmail(create.getEmail()) - .withDisplayName(create.getFirstName() + create.getLastName()) - .withIsBot(false) - .withIsAdmin(false) - .withUpdatedBy(username) - .withUpdatedAt(System.currentTimeMillis()) - .withIsEmailVerified(false) + return getUser( + username, + new CreateUser() + .withName(username) + .withEmail(create.getEmail()) + .withDisplayName(String.format("%s%s", create.getFirstName(), create.getLastName())) + .withIsBot(false) + .withIsAdmin(false)) .withAuthenticationMechanism( new AuthenticationMechanism() .withAuthType(AuthenticationMechanism.AuthType.BASIC) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/security/auth/LdapAuthenticator.java b/openmetadata-service/src/main/java/org/openmetadata/service/security/auth/LdapAuthenticator.java index 4480b4f86920..81565d4db106 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/security/auth/LdapAuthenticator.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/security/auth/LdapAuthenticator.java @@ -34,6 +34,7 @@ import lombok.extern.slf4j.Slf4j; import org.openmetadata.common.utils.CommonUtil; import org.openmetadata.schema.api.configuration.LoginConfiguration; +import org.openmetadata.schema.api.teams.CreateUser; import org.openmetadata.schema.auth.LdapConfiguration; import org.openmetadata.schema.auth.LoginRequest; import org.openmetadata.schema.auth.RefreshToken; @@ -254,31 +255,18 @@ public User lookUserInProvider(String email) { private User getUserForLdap(String email) { String userName = email.split("@")[0]; - return new User() - .withId(UUID.randomUUID()) - .withName(userName) - .withFullyQualifiedName(userName) - .withEmail(email) - .withIsBot(false) - .withUpdatedBy(userName) - .withUpdatedAt(System.currentTimeMillis()) + return UserUtil.getUser( + userName, new CreateUser().withName(userName).withEmail(email).withIsBot(false)) .withIsEmailVerified(false) .withAuthenticationMechanism(null); } private User getUserForLdap(String email, String userName, String userDn) { User user = - new User() - .withId(UUID.randomUUID()) - .withName(userName) - .withFullyQualifiedName(userName) - .withEmail(email) - .withIsBot(false) - .withUpdatedBy(userName) - .withUpdatedAt(System.currentTimeMillis()) + UserUtil.getUser( + userName, new CreateUser().withName(userName).withEmail(email).withIsBot(false)) .withIsEmailVerified(false) .withAuthenticationMechanism(null); - try { getRoleForLdap(user, userDn, false); } catch (LDAPException | JsonProcessingException e) { diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/util/UserUtil.java b/openmetadata-service/src/main/java/org/openmetadata/service/util/UserUtil.java index 69f884a1d801..cdbe9a61287a 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/util/UserUtil.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/util/UserUtil.java @@ -32,6 +32,7 @@ import javax.json.JsonPatch; import javax.ws.rs.core.UriInfo; import lombok.extern.slf4j.Slf4j; +import org.openmetadata.schema.api.teams.CreateUser; import org.openmetadata.schema.auth.BasicAuthMechanism; import org.openmetadata.schema.auth.JWTAuthMechanism; import org.openmetadata.schema.auth.JWTTokenExpiry; @@ -161,14 +162,8 @@ public static User addOrUpdateUser(User user) { } public static User user(String name, String domain, String updatedBy) { - return new User() - .withId(UUID.randomUUID()) - .withName(name) - .withFullyQualifiedName(EntityInterfaceUtil.quoteName(name)) - .withEmail(name + "@" + domain) - .withUpdatedBy(updatedBy) - .withUpdatedAt(System.currentTimeMillis()) - .withIsBot(false); + return getUser( + updatedBy, new CreateUser().withName(name).withEmail(name + "@" + domain).withIsBot(false)); } /** @@ -330,4 +325,24 @@ public static boolean reSyncUserRolesFromToken( return syncUser; } + + public static User getUser(String updatedBy, CreateUser create) { + return new User() + .withId(UUID.randomUUID()) + .withName(create.getName().toLowerCase()) + .withFullyQualifiedName(EntityInterfaceUtil.quoteName(create.getName().toLowerCase())) + .withEmail(create.getEmail().toLowerCase()) + .withDescription(create.getDescription()) + .withDisplayName(create.getDisplayName()) + .withIsBot(create.getIsBot()) + .withIsAdmin(create.getIsAdmin()) + .withProfile(create.getProfile()) + .withPersonas(create.getPersonas()) + .withDefaultPersona(create.getDefaultPersona()) + .withTimezone(create.getTimezone()) + .withUpdatedBy(updatedBy.toLowerCase()) + .withUpdatedAt(System.currentTimeMillis()) + .withTeams(EntityUtil.toEntityReferences(create.getTeams(), Entity.TEAM)) + .withRoles(EntityUtil.toEntityReferences(create.getRoles(), Entity.ROLE)); + } } 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 043f705dcd67..623023fe9d56 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 @@ -2925,7 +2925,7 @@ protected T patchEntityAndCheckAuthorization( return patchEntityAndCheck(entity, originalJson, authHeaders, MINOR_UPDATE, change); } - protected final void validateCommonEntityFields(T entity, CreateEntity create, String updatedBy) { + protected void validateCommonEntityFields(T entity, CreateEntity create, String updatedBy) { assertListNotNull(entity.getId(), entity.getHref(), entity.getFullyQualifiedName()); assertEquals(create.getName(), entity.getName()); assertEquals(create.getDisplayName(), entity.getDisplayName()); diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/teams/UserResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/teams/UserResourceTest.java index d30d76a21be5..a30e54bebc61 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/teams/UserResourceTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/teams/UserResourceTest.java @@ -95,6 +95,7 @@ import org.junit.jupiter.api.TestMethodOrder; import org.openmetadata.csv.EntityCsv; import org.openmetadata.csv.EntityCsvTest; +import org.openmetadata.schema.CreateEntity; import org.openmetadata.schema.api.CreateBot; import org.openmetadata.schema.api.teams.CreateTeam; import org.openmetadata.schema.api.teams.CreateUser; @@ -869,6 +870,17 @@ void delete_validUser_as_admin_200(TestInfo test) throws IOException { entityNotFound("user", user.getId())); } + protected void validateCommonEntityFields(User entity, CreateEntity create, String updatedBy) { + assertListNotNull(entity.getId(), entity.getHref(), entity.getFullyQualifiedName()); + assertEquals(create.getName().toLowerCase(), entity.getName()); + assertEquals(create.getDisplayName(), entity.getDisplayName()); + assertEquals(create.getDescription(), entity.getDescription()); + assertEquals( + JsonUtils.valueToTree(create.getExtension()), JsonUtils.valueToTree(entity.getExtension())); + assertReference(create.getOwner(), entity.getOwner()); + assertEquals(updatedBy, entity.getUpdatedBy()); + } + @Test void put_generateToken_bot_user_200_ok() throws HttpResponseException { AuthenticationMechanism authMechanism = @@ -940,7 +952,7 @@ void post_createUser_BasicAuth_AdminCreate_login_200_ok(TestInfo test) // jwtAuth Response should be null always user = getEntity(user.getId(), ADMIN_AUTH_HEADERS); assertNull(user.getAuthenticationMechanism()); - assertEquals(name, user.getName()); + assertEquals(name.toLowerCase(), user.getName()); assertEquals(name.toLowerCase(), user.getFullyQualifiedName()); // Login With Correct Password @@ -1006,9 +1018,9 @@ void post_createUser_BasicAuth_SignUp_200_ok() throws HttpResponseException { getResource("users/signup"), newRegistrationRequest, String.class, ADMIN_AUTH_HEADERS); // jwtAuth Response should be null always - User user = getEntityByName("testBasicAuth123", null, ADMIN_AUTH_HEADERS); + User user = getEntityByName(name, null, ADMIN_AUTH_HEADERS); assertNull(user.getAuthenticationMechanism()); - assertEquals(name, user.getName()); + assertEquals(name.toLowerCase(), user.getName()); assertEquals(name.toLowerCase(), user.getFullyQualifiedName()); // Login With Correct Password @@ -1024,7 +1036,7 @@ void post_createUser_BasicAuth_SignUp_200_ok() throws HttpResponseException { OK.getStatusCode(), ADMIN_AUTH_HEADERS); - validateJwtBasicAuth(jwtResponse, "testBasicAuth123"); + validateJwtBasicAuth(jwtResponse, name); // Login With Wrong email LoginRequest failedLoginWithWrongEmail = @@ -1157,36 +1169,36 @@ void testUserImportExport() throws IOException { // Create users in the team hierarchy // Headers - name,displayName,description,email,timezone,isAdmin,teams,roles String user = - "userImportExport,d,s,userImportExport@domain.com,America/Los_Angeles,true,teamImportExport,"; + "userimportexport,d,s,userimportexport@domain.com,America/Los_Angeles,true,teamImportExport,"; String user1 = - "userImportExport1,,,userImportExport1@domain.com,,false,teamImportExport1,DataConsumer"; - String user11 = "userImportExport11,,,userImportExport11@domain.com,,false,teamImportExport11,"; + "userimportexport1,,,userimportexport1@domain.com,,false,teamImportExport1,DataConsumer"; + String user11 = "userimportexport11,,,userimportexport11@domain.com,,false,teamImportExport11,"; List createRecords = listOf(user, user1, user11); // Update user descriptions - user = "userImportExport,displayName,,userImportExport@domain.com,,false,teamImportExport,"; + user = "userimportexport,displayName,,userimportexport@domain.com,,false,teamImportExport,"; user1 = - "userImportExport1,displayName1,,userImportExport1@domain.com,,false,teamImportExport1,"; + "userimportexport1,displayName1,,userimportexport1@domain.com,,false,teamImportExport1,"; user11 = - "userImportExport11,displayName11,,userImportExport11@domain.com,,false,teamImportExport11,"; + "userimportexport11,displayName11,,userimportexport11@domain.com,,false,teamImportExport11,"; List updateRecords = listOf(user, user1, user11); // Add new users String user2 = - "userImportExport2,displayName2,,userImportExport2@domain.com,,false,teamImportExport1,"; + "userimportexport2,displayName2,,userimportexport2@domain.com,,false,teamImportExport1,"; String user21 = - "userImportExport21,displayName21,,userImportExport21@domain.com,,false,teamImportExport11,"; + "userimportexport21,displayName21,,userimportexport21@domain.com,,false,teamImportExport11,"; List newRecords = listOf(user2, user21); testImportExport("teamImportExport", UserCsv.HEADERS, createRecords, updateRecords, newRecords); // Import to team11 a user in team1 - since team1 is not under team11 hierarchy, import should // fail String user3 = - "userImportExport3,displayName3,,userImportExport3@domain.com,,false,teamImportExport1,"; + "userimportexport3,displayName3,,userimportexport3@domain.com,,false,teamImportExport1,"; csv = EntityCsvTest.createCsv(UserCsv.HEADERS, listOf(user3), null); result = importCsv("teamImportExport11", csv, false); String error = - UserCsv.invalidTeam(6, "teamImportExport11", "userImportExport3", "teamImportExport1"); + UserCsv.invalidTeam(6, "teamImportExport11", "userimportexport3", "teamImportExport1"); assertTrue(result.getImportResultsCsv().contains(error)); } @@ -1200,7 +1212,7 @@ private void validateJwtBasicAuth(JwtResponse jwtResponse, String username) { Date date = jwt.getExpiresAt(); long hours = ((date.getTime() - jwt.getIssuedAt().getTime()) / (1000 * 60 * 60)); assertEquals(1, hours); - assertEquals(username, jwt.getClaims().get("sub").asString()); + assertEquals(username.toLowerCase(), jwt.getClaims().get("sub").asString().toLowerCase()); assertEquals(false, jwt.getClaims().get("isBot").asBoolean()); } @@ -1404,7 +1416,7 @@ protected void validateDeletedEntity( @Override public void validateCreatedEntity( User user, CreateUser createRequest, Map authHeaders) { - assertEquals(createRequest.getName(), user.getName()); + assertEquals(createRequest.getName().toLowerCase(), user.getName()); assertEquals(createRequest.getDisplayName(), user.getDisplayName()); assertEquals(createRequest.getTimezone(), user.getTimezone()); assertEquals(createRequest.getIsBot(), user.getIsBot()); diff --git a/openmetadata-ui/src/main/resources/ui/cypress/e2e/Pages/Bots.spec.ts b/openmetadata-ui/src/main/resources/ui/cypress/e2e/Pages/Bots.spec.ts index e0ac49276a8b..e456dce3b1f7 100644 --- a/openmetadata-ui/src/main/resources/ui/cypress/e2e/Pages/Bots.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/cypress/e2e/Pages/Bots.spec.ts @@ -23,7 +23,7 @@ import { import { DELETE_TERM } from '../../constants/constants'; import { GlobalSettingOptions } from '../../constants/settings.constant'; -const botName = `Bot-ct-test-${uuid()}`; +const botName = `bot-ct-test-${uuid()}`; const botEmail = `${botName}@mail.com`; const description = 'This is bot description'; const updatedDescription = 'This is updated bot description'; diff --git a/openmetadata-ui/src/main/resources/ui/cypress/e2e/Pages/Users.spec.ts b/openmetadata-ui/src/main/resources/ui/cypress/e2e/Pages/Users.spec.ts index 80a4e391979c..f798e8d1d522 100644 --- a/openmetadata-ui/src/main/resources/ui/cypress/e2e/Pages/Users.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/cypress/e2e/Pages/Users.spec.ts @@ -57,7 +57,7 @@ const expirationTime = { twomonths: '60', threemonths: '90', }; -const name = `Usercttest${uuid()}`; +const name = `usercttest${uuid()}`; const owner = generateRandomUser(); let userId = ''; const ownerName = `${owner.firstName}${owner.lastName}`; diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ActivityFeed.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ActivityFeed.spec.ts index 44f4805a3ced..4b1524de1fd6 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ActivityFeed.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ActivityFeed.spec.ts @@ -57,7 +57,7 @@ test.describe('Activity feed', () => { test('Assigned task should appear to task tab', async ({ page }) => { const value: TaskDetails = { term: entity.entity.name, - assignee: `${user.data.firstName}.${user.data.lastName}`, + assignee: user.responseData.name, }; await entity.visitEntityPage(page); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/AppRouter/AppRouter.tsx b/openmetadata-ui/src/main/resources/ui/src/components/AppRouter/AppRouter.tsx index 8a50e332213d..64d7c43f2dd8 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/AppRouter/AppRouter.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/AppRouter/AppRouter.tsx @@ -29,9 +29,8 @@ const AppRouter = () => { // web analytics instance const analytics = useAnalytics(); - const { currentUser } = useApplicationStore(); - - const { isAuthenticated, isApplicationLoading } = useApplicationStore(); + const { currentUser, isAuthenticated, isApplicationLoading } = + useApplicationStore(); useEffect(() => { const { pathname } = location; diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/BasicAuthProvider.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/BasicAuthProvider.tsx index fda2d9d93355..2f3dd982ad81 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/BasicAuthProvider.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/BasicAuthProvider.tsx @@ -115,7 +115,7 @@ const BasicAuthProvider = ({ onLoginSuccess({ id_token: response.accessToken, profile: { - email, + email: response.email, name: '', picture: '', sub: '', diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/SignUp/BasicSignup.component.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/SignUp/BasicSignup.component.tsx index 84bbef7f4792..78b452497408 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/SignUp/BasicSignup.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/SignUp/BasicSignup.component.tsx @@ -17,7 +17,6 @@ import React, { useMemo } from 'react'; import { useTranslation } from 'react-i18next'; import { useHistory } from 'react-router-dom'; import loginBG from '../../assets/img/login-bg.png'; - import { useBasicAuth } from '../../components/Auth/AuthProviders/BasicAuthProvider'; import BrandImage from '../../components/common/BrandImage/BrandImage'; import { ROUTES, VALIDATION_MESSAGES } from '../../constants/constants'; From bc49f9e8437de5a337c97499ed4ebbdeb6cea37d Mon Sep 17 00:00:00 2001 From: Aniket Katkar Date: Fri, 19 Jul 2024 11:46:17 +0530 Subject: [PATCH 13/33] Resolve the ws package version conflict due to conflicting dependencies (#16961) --- openmetadata-ui/src/main/resources/ui/package.json | 3 ++- openmetadata-ui/src/main/resources/ui/yarn.lock | 12 +----------- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/package.json b/openmetadata-ui/src/main/resources/ui/package.json index 7866e84fc1c5..42c90cbb59dc 100644 --- a/openmetadata-ui/src/main/resources/ui/package.json +++ b/openmetadata-ui/src/main/resources/ui/package.json @@ -248,6 +248,7 @@ "prosemirror-state": "1.4.1", "prosemirror-transform": "1.7.0", "prosemirror-view": "1.28.2", - "axios": "1.6.4" + "axios": "1.6.4", + "ws": "8.17.1" } } diff --git a/openmetadata-ui/src/main/resources/ui/yarn.lock b/openmetadata-ui/src/main/resources/ui/yarn.lock index 1fc5ce6f736d..a41845b07e28 100644 --- a/openmetadata-ui/src/main/resources/ui/yarn.lock +++ b/openmetadata-ui/src/main/resources/ui/yarn.lock @@ -15577,21 +15577,11 @@ write-file-atomic@^3.0.0: signal-exit "^3.0.2" typedarray-to-buffer "^3.1.5" -ws@^7.3.1, ws@^7.4.6: - version "7.5.10" - resolved "https://registry.yarnpkg.com/ws/-/ws-7.5.10.tgz#58b5c20dc281633f6c19113f39b349bd8bd558d9" - integrity sha512-+dbF1tHwZpXcbOJdVOkzLDxZP1ailvSxM6ZweXTegylPny803bFhA+vqBYw4s31NSAk4S2Qz+AKXK9a4wkdjcQ== - -ws@^8.4.2: +ws@8.17.1, ws@^7.3.1, ws@^7.4.6, ws@^8.4.2, ws@~8.2.3: version "8.17.1" resolved "https://registry.yarnpkg.com/ws/-/ws-8.17.1.tgz#9293da530bb548febc95371d90f9c878727d919b" integrity sha512-6XQFvXTkbfUOZOKKILFG1PDK2NDQs4azKQl26T0YS5CxqWLgXajbPZ+h4gZekJyRqFU8pvnbAbbs/3TgRPy+GQ== -ws@~8.2.3: - version "8.2.3" - resolved "https://registry.yarnpkg.com/ws/-/ws-8.2.3.tgz#63a56456db1b04367d0b721a0b80cae6d8becbba" - integrity sha512-wBuoj1BDpC6ZQ1B7DWQBYVLphPWkm8i9Y0/3YdHjHKHiohOJ1ws+3OccDWtH+PoC9DZD5WOTrJvNbWvjS6JWaA== - xhr2@0.1.3: version "0.1.3" resolved "https://registry.yarnpkg.com/xhr2/-/xhr2-0.1.3.tgz#cbfc4759a69b4a888e78cf4f20b051038757bd11" From 13daa3668a8fcaf30218b7416a4d73e53931a1c0 Mon Sep 17 00:00:00 2001 From: Teddy Date: Fri, 19 Jul 2024 08:19:45 +0200 Subject: [PATCH 14/33] Fix: test case indexing -- remove changeDescription from testSuites list (#17083) * fix: test case indexing -- remove changeDescription from testSuites list * fix: change type to interface * style: ran java linting --- .../service/search/indexes/TestCaseIndex.java | 10 +++++--- .../dqtests/TestCaseResourceTest.java | 25 +++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TestCaseIndex.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TestCaseIndex.java index 29a16d2f599c..d37149a872ef 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TestCaseIndex.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TestCaseIndex.java @@ -15,7 +15,7 @@ import org.openmetadata.service.search.models.SearchSuggest; public record TestCaseIndex(TestCase testCase) implements SearchIndex { - private static final Set excludeFields = Set.of("testSuites.changeDescription"); + private static final Set excludeFields = Set.of("changeDescription"); @Override public Object getEntity() { @@ -23,8 +23,12 @@ public Object getEntity() { } @Override - public Set getExcludedFields() { - return excludeFields; + public void removeNonIndexableFields(Map esDoc) { + SearchIndex.super.removeNonIndexableFields(esDoc); + List> testSuites = (List>) esDoc.get("testSuites"); + for (Map testSuite : testSuites) { + SearchIndexUtils.removeNonIndexableFields(testSuite, excludeFields); + } } @SneakyThrows diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestCaseResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestCaseResourceTest.java index a74422d86048..af6e6da9ed3b 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestCaseResourceTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestCaseResourceTest.java @@ -96,6 +96,7 @@ import org.openmetadata.service.resources.databases.TableResourceTest; import org.openmetadata.service.resources.feeds.FeedResourceTest; import org.openmetadata.service.resources.feeds.MessageParser; +import org.openmetadata.service.search.indexes.TestCaseIndex; import org.openmetadata.service.util.JsonUtils; import org.openmetadata.service.util.ResultList; import org.openmetadata.service.util.TestUtils; @@ -2026,6 +2027,30 @@ void wrongMinMaxTestParameter(TestInfo test) throws HttpResponseException { () -> createEntity(invalidTestCaseMixedTypes, ADMIN_AUTH_HEADERS), BAD_REQUEST, "Value"); } + @Test + void test_testCaseEsDocCleanUp(TestInfo testInfo) { + TestCase testCase = + new TestCase() + .withId(UUID.randomUUID()) + .withChangeDescription(new ChangeDescription()) + .withTestSuites( + List.of( + new TestSuite() + .withId(UUID.randomUUID()) + .withChangeDescription(new ChangeDescription()), + new TestSuite() + .withId(UUID.randomUUID()) + .withChangeDescription(new ChangeDescription()))); + + Map doc = JsonUtils.convertValue(testCase, Map.class); + + TestCaseIndex testCaseIndex = new TestCaseIndex(testCase); + testCaseIndex.removeNonIndexableFields(doc); + assertNull(doc.get("changeDescription")); + List> testSuites = (List>) doc.get("testSuites"); + assertNull(testSuites.get(0).get("changeDescription")); + } + public void deleteTestCaseResult(String fqn, Long timestamp, Map authHeaders) throws HttpResponseException { WebTarget target = getCollection().path("/" + fqn + "/testCaseResult/" + timestamp); From 5c05738e02a1f69b1dfffe7205acd09afd94778b Mon Sep 17 00:00:00 2001 From: Ashish Gupta Date: Fri, 19 Jul 2024 12:04:56 +0530 Subject: [PATCH 15/33] #17043: fix the same suggestion content being displayed on task (#17077) * fix the same suggestion content being displayed on task * added playwright test --- .../e2e/Features/ActivityFeed.spec.ts | 65 +++++++++++++++++++ .../ui/playwright/utils/activityFeed.ts | 48 ++++++++++++++ .../resources/ui/playwright/utils/task.ts | 13 +++- .../Entity/Task/TaskTab/TaskTab.component.tsx | 2 + 4 files changed, 126 insertions(+), 2 deletions(-) create mode 100644 openmetadata-ui/src/main/resources/ui/playwright/utils/activityFeed.ts diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ActivityFeed.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ActivityFeed.spec.ts index 4b1524de1fd6..21fcd6282fb7 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ActivityFeed.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ActivityFeed.spec.ts @@ -13,6 +13,7 @@ import test, { expect } from '@playwright/test'; import { TableClass } from '../../support/entity/TableClass'; import { UserClass } from '../../support/user/UserClass'; +import { checkDescriptionInEditModal } from '../../utils/activityFeed'; import { createNewPage, redirectToHomePage, @@ -181,4 +182,68 @@ test.describe('Activity feed', () => { ).toContainText(`Reply message ${i}`); } }); + + test('Update Description Task on Columns', async ({ page }) => { + const firstTaskValue: TaskDetails = { + term: entity.entity.name, + assignee: `${user.data.firstName}.${user.data.lastName}`, + description: 'Column Description 1', + columnName: entity.entity.columns[0].name, + oldDescription: entity.entity.columns[0].description, + }; + const secondTaskValue: TaskDetails = { + ...firstTaskValue, + description: 'Column Description 2', + columnName: entity.entity.columns[1].name, + oldDescription: entity.entity.columns[1].description, + }; + await entity.visitEntityPage(page); + + await page + .getByRole('cell', { name: 'The ID of the store. This' }) + .getByTestId('task-element') + .click(); + + // create description task + await createDescriptionTask(page, secondTaskValue); + + await page.getByTestId('schema').click(); + + // create 2nd task for column description + await page + .getByRole('cell', { name: 'Unique identifier for the' }) + .getByTestId('task-element') + .click(); + + await createDescriptionTask(page, firstTaskValue); + + // Task 1 - check the description in edit and accept suggestion + await checkDescriptionInEditModal(page, firstTaskValue); + + await page.getByText('Cancel').click(); + + await page.waitForSelector('[role="dialog"].ant-modal', { + state: 'detached', + }); + + // Task 2 - check the description in edit and accept suggestion + + await page.getByTestId('message-container').last().click(); + + await checkDescriptionInEditModal(page, secondTaskValue); + + await page.getByText('OK').click(); + + await toastNotification(page, /Task resolved successfully/); + + // Task 1 - Resolved the task + + await page.getByText('Accept Suggestion').click(); + + await toastNotification(page, /Task resolved successfully/); + + const closedTask = await page.getByTestId('closed-task').textContent(); + + expect(closedTask).toContain('2 Closed'); + }); }); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/utils/activityFeed.ts b/openmetadata-ui/src/main/resources/ui/playwright/utils/activityFeed.ts new file mode 100644 index 000000000000..e3fe92688247 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/playwright/utils/activityFeed.ts @@ -0,0 +1,48 @@ +/* + * Copyright 2024 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { expect, Page } from '@playwright/test'; +import { descriptionBox } from './common'; +import { TaskDetails } from './task'; + +export const checkDescriptionInEditModal = async ( + page: Page, + taskValue: TaskDetails +) => { + const taskContent = await page.getByTestId('task-title').innerText(); + + expect(taskContent).toContain(`Request to update description for`); + + await page.getByRole('button', { name: 'down' }).click(); + await page.waitForSelector('.ant-dropdown', { + state: 'visible', + }); + + await page.getByRole('menuitem', { name: 'edit' }).click(); + + await expect(page.locator('[role="dialog"].ant-modal')).toBeVisible(); + + await expect(page.locator('.ant-modal-title')).toContainText( + `Update description for table ${taskValue.term} columns/${taskValue.columnName}` + ); + + await expect(page.locator(descriptionBox)).toContainText( + taskValue.description ?? '' + ); + + // click on the Current tab + await page.getByRole('tab', { name: 'current' }).click(); + + await expect(page.getByTestId('markdown-parser')).toContainText( + taskValue.oldDescription ?? '' + ); +}; diff --git a/openmetadata-ui/src/main/resources/ui/playwright/utils/task.ts b/openmetadata-ui/src/main/resources/ui/playwright/utils/task.ts index ae4aa7f8e79d..6ab7b6895cfb 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/utils/task.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/utils/task.ts @@ -18,6 +18,9 @@ export type TaskDetails = { term: string; assignee?: string; tag?: string; + description?: string; + oldDescription?: string; + columnName?: string; }; const tag = 'PII.None'; @@ -28,7 +31,11 @@ export const createDescriptionTask = async ( assigneeDisabled?: boolean ) => { expect(await page.locator('#title').inputValue()).toBe( - `Update description for table ${value.term}` + `Update description for table ${ + value.columnName + ? `${value.term} columns/${value.columnName}` + : value.term + }` ); if (isUndefined(value.assignee) || assigneeDisabled) { @@ -62,7 +69,9 @@ export const createDescriptionTask = async ( await page.click('body'); } - await page.locator(descriptionBox).fill('Updated description'); + await page + .locator(descriptionBox) + .fill(value.description ?? 'Updated description'); await page.click('button[type="submit"]'); await toastNotification(page, /Task created successfully./); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Entity/Task/TaskTab/TaskTab.component.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Entity/Task/TaskTab/TaskTab.component.tsx index 8de07790fbd8..f126cfa0b07a 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Entity/Task/TaskTab/TaskTab.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Entity/Task/TaskTab/TaskTab.component.tsx @@ -813,6 +813,7 @@ export const TaskTab = ({ {isTaskTestCaseResult ? ( ) : ( Date: Fri, 19 Jul 2024 12:16:53 +0530 Subject: [PATCH 16/33] Move Migration to 1.4.6 (#17095) --- .../mysql/postDataMigrationSQLScript.sql | 43 +++++++++++++++++ .../native/1.4.6/mysql/schemaChanges.sql | 0 .../postgres/postDataMigrationSQLScript.sql | 43 +++++++++++++++++ .../native/1.4.6/postgres/schemaChanges.sql | 0 .../mysql/postDataMigrationSQLScript.sql | 46 +------------------ .../postgres/postDataMigrationSQLScript.sql | 43 ----------------- 6 files changed, 87 insertions(+), 88 deletions(-) create mode 100644 bootstrap/sql/migrations/native/1.4.6/mysql/postDataMigrationSQLScript.sql create mode 100644 bootstrap/sql/migrations/native/1.4.6/mysql/schemaChanges.sql create mode 100644 bootstrap/sql/migrations/native/1.4.6/postgres/postDataMigrationSQLScript.sql create mode 100644 bootstrap/sql/migrations/native/1.4.6/postgres/schemaChanges.sql diff --git a/bootstrap/sql/migrations/native/1.4.6/mysql/postDataMigrationSQLScript.sql b/bootstrap/sql/migrations/native/1.4.6/mysql/postDataMigrationSQLScript.sql new file mode 100644 index 000000000000..475f86a1ad7f --- /dev/null +++ b/bootstrap/sql/migrations/native/1.4.6/mysql/postDataMigrationSQLScript.sql @@ -0,0 +1,43 @@ +-- Remove Duplicate Usernames and Lowercase Them +WITH cte AS ( + SELECT + id, + ROW_NUMBER() OVER (PARTITION BY LOWER(JSON_UNQUOTE(JSON_EXTRACT(json, '$.name'))) ORDER BY id) as rn + FROM + user_entity +) +DELETE FROM user_entity +WHERE id IN ( + SELECT id + FROM cte + WHERE rn > 1 +); + +UPDATE user_entity +SET json = JSON_SET( + json, + '$.name', + LOWER(JSON_UNQUOTE(JSON_EXTRACT(json, '$.name'))) +); + +-- Remove Duplicate Emails and Lowercase Them +WITH cte AS ( + SELECT + id, + ROW_NUMBER() OVER (PARTITION BY LOWER(JSON_UNQUOTE(JSON_EXTRACT(json, '$.email'))) ORDER BY id) as rn + FROM + user_entity +) +DELETE FROM user_entity +WHERE id IN ( + SELECT id + FROM cte + WHERE rn > 1 +); + +UPDATE user_entity +SET json = JSON_SET( + json, + '$.email', + LOWER(JSON_UNQUOTE(JSON_EXTRACT(json, '$.email'))) +); diff --git a/bootstrap/sql/migrations/native/1.4.6/mysql/schemaChanges.sql b/bootstrap/sql/migrations/native/1.4.6/mysql/schemaChanges.sql new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/bootstrap/sql/migrations/native/1.4.6/postgres/postDataMigrationSQLScript.sql b/bootstrap/sql/migrations/native/1.4.6/postgres/postDataMigrationSQLScript.sql new file mode 100644 index 000000000000..adb0d2cd7ca7 --- /dev/null +++ b/bootstrap/sql/migrations/native/1.4.6/postgres/postDataMigrationSQLScript.sql @@ -0,0 +1,43 @@ +-- Remove Duplicate UserNames and lowercase them +WITH cte AS ( + SELECT + id, + ROW_NUMBER() OVER (PARTITION BY to_jsonb(LOWER(json->>'name')) ORDER BY id) as rn + FROM + user_entity +) +DELETE from user_entity +WHERE id IN ( + SELECT id + FROM cte + WHERE rn > 1 +); + +UPDATE user_entity +SET json = jsonb_set( + json, + '{name}', + to_jsonb(LOWER(json->>'name')) +); + +-- Remove Duplicate Emails and lowercase them +WITH cte AS ( + SELECT + id, + ROW_NUMBER() OVER (PARTITION BY to_jsonb(LOWER(json->>'email')) ORDER BY id) as rn + FROM + user_entity +) +DELETE from user_entity +WHERE id IN ( + SELECT id + FROM cte + WHERE rn > 1 +); + +UPDATE user_entity +SET json = jsonb_set( + json, + '{email}', + to_jsonb(LOWER(json->>'email')) +); diff --git a/bootstrap/sql/migrations/native/1.4.6/postgres/schemaChanges.sql b/bootstrap/sql/migrations/native/1.4.6/postgres/schemaChanges.sql new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/bootstrap/sql/migrations/native/1.5.0/mysql/postDataMigrationSQLScript.sql b/bootstrap/sql/migrations/native/1.5.0/mysql/postDataMigrationSQLScript.sql index 740cbab52e06..7b1d6e095d48 100644 --- a/bootstrap/sql/migrations/native/1.5.0/mysql/postDataMigrationSQLScript.sql +++ b/bootstrap/sql/migrations/native/1.5.0/mysql/postDataMigrationSQLScript.sql @@ -22,48 +22,4 @@ WHERE name IN ( 'columnValuesSumToBeBetween', 'columnValuesToBeBetween', 'tableRowCountToBeBetween' -); - --- Remove Duplicate Usernames and Lowercase Them -WITH cte AS ( - SELECT - id, - ROW_NUMBER() OVER (PARTITION BY LOWER(JSON_UNQUOTE(JSON_EXTRACT(json, '$.name'))) ORDER BY id) as rn - FROM - user_entity -) -DELETE FROM user_entity -WHERE id IN ( - SELECT id - FROM cte - WHERE rn > 1 -); - -UPDATE user_entity -SET json = JSON_SET( - json, - '$.name', - LOWER(JSON_UNQUOTE(JSON_EXTRACT(json, '$.name'))) -); - --- Remove Duplicate Emails and Lowercase Them -WITH cte AS ( - SELECT - id, - ROW_NUMBER() OVER (PARTITION BY LOWER(JSON_UNQUOTE(JSON_EXTRACT(json, '$.email'))) ORDER BY id) as rn - FROM - user_entity -) -DELETE FROM user_entity -WHERE id IN ( - SELECT id - FROM cte - WHERE rn > 1 -); - -UPDATE user_entity -SET json = JSON_SET( - json, - '$.email', - LOWER(JSON_UNQUOTE(JSON_EXTRACT(json, '$.email'))) -); +); \ No newline at end of file diff --git a/bootstrap/sql/migrations/native/1.5.0/postgres/postDataMigrationSQLScript.sql b/bootstrap/sql/migrations/native/1.5.0/postgres/postDataMigrationSQLScript.sql index 3ac2106b72a4..85af0338b845 100644 --- a/bootstrap/sql/migrations/native/1.5.0/postgres/postDataMigrationSQLScript.sql +++ b/bootstrap/sql/migrations/native/1.5.0/postgres/postDataMigrationSQLScript.sql @@ -23,46 +23,3 @@ WHERE name IN ( 'tableRowCountToBeBetween' ); --- Remove Duplicate UserNames and lowercase them -WITH cte AS ( - SELECT - id, - ROW_NUMBER() OVER (PARTITION BY to_jsonb(LOWER(json->>'name')) ORDER BY id) as rn - FROM - user_entity -) -DELETE from user_entity -WHERE id IN ( - SELECT id - FROM cte - WHERE rn > 1 -); - -UPDATE user_entity -SET json = jsonb_set( - json, - '{name}', - to_jsonb(LOWER(json->>'name')) -); - --- Remove Duplicate Emails and lowercase them -WITH cte AS ( - SELECT - id, - ROW_NUMBER() OVER (PARTITION BY to_jsonb(LOWER(json->>'email')) ORDER BY id) as rn - FROM - user_entity -) -DELETE from user_entity -WHERE id IN ( - SELECT id - FROM cte - WHERE rn > 1 -); - -UPDATE user_entity -SET json = jsonb_set( - json, - '{email}', - to_jsonb(LOWER(json->>'email')) -); From 75fcb030414b8d30353d926e486cef1932db4c51 Mon Sep 17 00:00:00 2001 From: Mohit Yadav <105265192+mohityadav766@users.noreply.github.com> Date: Fri, 19 Jul 2024 12:38:54 +0530 Subject: [PATCH 17/33] Fix Condition for deleted App (#17096) --- .../openmetadata/service/apps/AbstractNativeApplication.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/apps/AbstractNativeApplication.java b/openmetadata-service/src/main/java/org/openmetadata/service/apps/AbstractNativeApplication.java index 378333cdbc72..130abb1db109 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/apps/AbstractNativeApplication.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/apps/AbstractNativeApplication.java @@ -63,7 +63,7 @@ public void init(App app) { @Override public void install() { // If the app does not have any Schedule Return without scheduling - if (Boolean.FALSE.equals(app.getDeleted()) + if (Boolean.TRUE.equals(app.getDeleted()) || (app.getAppSchedule() != null && app.getAppSchedule().getScheduleTimeline().equals(ScheduleTimeline.NONE))) { return; From d145341026e3b74da9c18d319b578faaaf12ec2a Mon Sep 17 00:00:00 2001 From: IceS2 Date: Fri, 19 Jul 2024 10:32:22 +0200 Subject: [PATCH 18/33] MINOR: Fix Oracle E2E Tests (#17084) * Fix Oracle E2E Tests * Fix Checkstyle * Add link to issue in the reason --- ingestion/src/metadata/cmd.py | 4 ++-- ingestion/tests/cli_e2e/test_cli_oracle.py | 13 +++++++++---- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/ingestion/src/metadata/cmd.py b/ingestion/src/metadata/cmd.py index 19a9c45709b4..809b5064eca6 100644 --- a/ingestion/src/metadata/cmd.py +++ b/ingestion/src/metadata/cmd.py @@ -18,7 +18,7 @@ from pathlib import Path # pyright: reportUnusedCallResult=false -from typing import List, Optional +from typing import List, Optional, Union from metadata.__version__ import get_metadata_version from metadata.cli.app import run_app @@ -157,7 +157,7 @@ def metadata(args: Optional[List[str]] = None): if contains_args.get("debug"): set_loggers_level(logging.DEBUG) else: - log_level: str = contains_args.get("log_level", logging.INFO) + log_level: Union[str, int] = contains_args.get("log_level") or logging.INFO set_loggers_level(log_level) if path and metadata_workflow and metadata_workflow in RUN_PATH_METHODS: diff --git a/ingestion/tests/cli_e2e/test_cli_oracle.py b/ingestion/tests/cli_e2e/test_cli_oracle.py index b89cb599c2f6..f870b17da432 100644 --- a/ingestion/tests/cli_e2e/test_cli_oracle.py +++ b/ingestion/tests/cli_e2e/test_cli_oracle.py @@ -15,6 +15,8 @@ from typing import List +import pytest + from metadata.ingestion.api.status import Status from .base.e2e_types import E2EType @@ -96,11 +98,11 @@ def view_column_lineage_count(self) -> int: @staticmethod def fqn_created_table() -> str: - return "e2e_oracle.default.admin.ADMIN_EMP" + return "e2e_oracle.default.admin.admin_emp" @staticmethod def _fqn_deleted_table() -> str: - return "e2e_oracle.default.admin.ADMIN_EMP" + return "e2e_oracle.default.admin.admin_emp" @staticmethod def get_includes_schemas() -> List[str]: @@ -128,12 +130,15 @@ def expected_filtered_table_includes() -> int: @staticmethod def expected_filtered_table_excludes() -> int: - return 29 + return 30 @staticmethod def expected_filtered_mix() -> int: return 43 + @pytest.mark.xfail( + reason="Issue Raised: https://github.com/open-metadata/OpenMetadata/issues/17085" + ) def test_create_table_with_profiler(self) -> None: # delete table in case it exists self.delete_table_and_view() @@ -240,7 +245,7 @@ def test_table_filter_excludes(self) -> None: E2EType.INGEST_DB_FILTER_MIX, { "schema": {"includes": self.get_includes_schemas()}, - "table": {"excludes": self.get_includes_tables()}, + "table": {"excludes": self.get_excludes_tables()}, }, ) From 4c657de5d887b05043a3987a51049518991561c2 Mon Sep 17 00:00:00 2001 From: Ayush Shah Date: Fri, 19 Jul 2024 15:38:39 +0530 Subject: [PATCH 19/33] Fixes #16501: Optimize Rows Sample Type Snowflake (#17055) --- .../processor/sampler/sampler_factory.py | 33 ++++++++-- .../processor/sampler/sqlalchemy/sampler.py | 16 ++--- .../sampler/sqlalchemy/snowflake/sampler.py | 60 +++++++++++++++++++ 3 files changed, 91 insertions(+), 18 deletions(-) create mode 100644 ingestion/src/metadata/profiler/processor/sampler/sqlalchemy/snowflake/sampler.py diff --git a/ingestion/src/metadata/profiler/processor/sampler/sampler_factory.py b/ingestion/src/metadata/profiler/processor/sampler/sampler_factory.py index c21e4247f424..c2457f66944a 100644 --- a/ingestion/src/metadata/profiler/processor/sampler/sampler_factory.py +++ b/ingestion/src/metadata/profiler/processor/sampler/sampler_factory.py @@ -27,6 +27,9 @@ from metadata.generated.schema.entity.services.connections.database.mongoDBConnection import ( MongoDBConnection, ) +from metadata.generated.schema.entity.services.connections.database.snowflakeConnection import ( + SnowflakeConnection, +) from metadata.generated.schema.entity.services.connections.database.trinoConnection import ( TrinoConnection, ) @@ -37,6 +40,9 @@ BigQuerySampler, ) from metadata.profiler.processor.sampler.sqlalchemy.sampler import SQASampler +from metadata.profiler.processor.sampler.sqlalchemy.snowflake.sampler import ( + SnowflakeSampler, +) from metadata.profiler.processor.sampler.sqlalchemy.trino.sampler import TrinoSampler @@ -62,9 +68,24 @@ def create( sampler_factory_ = SamplerFactory() -sampler_factory_.register(DatabaseConnection.__name__, SQASampler) -sampler_factory_.register(BigQueryConnection.__name__, BigQuerySampler) -sampler_factory_.register(DatalakeConnection.__name__, DatalakeSampler) -sampler_factory_.register(TrinoConnection.__name__, TrinoSampler) -sampler_factory_.register(MongoDBConnection.__name__, NoSQLSampler) -sampler_factory_.register(DynamoDBConnection.__name__, NoSQLSampler) +sampler_factory_.register( + source_type=DatabaseConnection.__name__, sampler_class=SQASampler +) +sampler_factory_.register( + source_type=BigQueryConnection.__name__, sampler_class=BigQuerySampler +) +sampler_factory_.register( + source_type=DatalakeConnection.__name__, sampler_class=DatalakeSampler +) +sampler_factory_.register( + source_type=TrinoConnection.__name__, sampler_class=TrinoSampler +) +sampler_factory_.register( + source_type=MongoDBConnection.__name__, sampler_class=NoSQLSampler +) +sampler_factory_.register( + source_type=SnowflakeConnection.__name__, sampler_class=SnowflakeSampler +) +sampler_factory_.register( + source_type=DynamoDBConnection.__name__, sampler_class=NoSQLSampler +) diff --git a/ingestion/src/metadata/profiler/processor/sampler/sqlalchemy/sampler.py b/ingestion/src/metadata/profiler/processor/sampler/sqlalchemy/sampler.py index d72b7cb45ff1..3398d90e4b66 100644 --- a/ingestion/src/metadata/profiler/processor/sampler/sqlalchemy/sampler.py +++ b/ingestion/src/metadata/profiler/processor/sampler/sqlalchemy/sampler.py @@ -28,7 +28,6 @@ ) from metadata.profiler.orm.functions.modulo import ModuloFn from metadata.profiler.orm.functions.random_num import RandomNumFn -from metadata.profiler.orm.registry import Dialects from metadata.profiler.processor.handle_partition import partition_filter_handler from metadata.profiler.processor.sampler.sampler_interface import SamplerInterface from metadata.utils.helpers import is_safe_sql_query @@ -87,17 +86,10 @@ def _base_sample_query(self, column: Optional[Column], label=None): def get_sample_query(self, *, column=None) -> Query: """get query for sample data""" if self.profile_sample_type == ProfileSampleType.PERCENTAGE: - rnd = ( - self._base_sample_query( - column, - (ModuloFn(RandomNumFn(), 100)).label(RANDOM_LABEL), - ) - .suffix_with( - f"SAMPLE BERNOULLI ({self.profile_sample or 100})", - dialect=Dialects.Snowflake, - ) - .cte(f"{self.table.__tablename__}_rnd") - ) + rnd = self._base_sample_query( + column, + (ModuloFn(RandomNumFn(), 100)).label(RANDOM_LABEL), + ).cte(f"{self.table.__tablename__}_rnd") session_query = self.client.query(rnd) return session_query.where(rnd.c.random <= self.profile_sample).cte( f"{self.table.__tablename__}_sample" diff --git a/ingestion/src/metadata/profiler/processor/sampler/sqlalchemy/snowflake/sampler.py b/ingestion/src/metadata/profiler/processor/sampler/sqlalchemy/snowflake/sampler.py new file mode 100644 index 000000000000..57e72cc4ce43 --- /dev/null +++ b/ingestion/src/metadata/profiler/processor/sampler/sqlalchemy/snowflake/sampler.py @@ -0,0 +1,60 @@ +# Copyright 2021 Collate +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# http://www.apache.org/licenses/LICENSE-2.0 +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +""" +Helper module to handle data sampling +for the profiler +""" + +from typing import cast + +from sqlalchemy import Table +from sqlalchemy.sql.selectable import CTE + +from metadata.generated.schema.entity.data.table import ProfileSampleType +from metadata.profiler.processor.handle_partition import partition_filter_handler +from metadata.profiler.processor.sampler.sqlalchemy.sampler import SQASampler + + +class SnowflakeSampler(SQASampler): + """ + Generates a sample of the data to not + run the query in the whole table. + """ + + def __init__(self, *args, **kwargs) -> None: + super().__init__(*args, **kwargs) + + @partition_filter_handler(build_sample=True) + def get_sample_query(self, *, column=None) -> CTE: + """get query for sample data""" + # TABLESAMPLE SYSTEM is not supported for views + self.table = cast(Table, self.table) + + if self.profile_sample_type == ProfileSampleType.PERCENTAGE: + rnd = ( + self._base_sample_query( + column, + ) + .suffix_with( + f"SAMPLE BERNOULLI ({self.profile_sample or 100})", + ) + .cte(f"{self.table.__tablename__}_rnd") + ) + session_query = self.client.query(rnd) + return session_query.cte(f"{self.table.__tablename__}_sample") + + return ( + self._base_sample_query(column) + .suffix_with( + f"TABLESAMPLE ({self.profile_sample or 100} ROWS)", + ) + .cte(f"{self.table.__tablename__}_sample") + ) From d59b83f9d1597d3e387d957031d553f9d9930fca Mon Sep 17 00:00:00 2001 From: Imri Paran Date: Fri, 19 Jul 2024 12:12:34 +0200 Subject: [PATCH 20/33] MINOR[GEN-978]: Fix empty test suites (#16975) * tests: refactor refactor tests and consolidate common functionality in integrations.conftest this enables writing tests more concisely. demonstrated with postgres and mssql. will migrate more * format * removed helpers * changed scope of fictures * changed scope of fixtures * added profiler test for mssql * fixed import in data_quality test * json safe serialization * format * set MARS_Connection * fix(data-quality): empty test suite do not raise for empty test suite * format * dont need to check length in _get_test_cases_from_test_suite * fix * added warning if no test cases are found --- .../src/metadata/data_quality/api/models.py | 2 +- .../processor/test_case_runner.py | 32 +-- .../data_quality/source/test_suite.py | 5 +- ingestion/tests/integration/conftest.py | 16 ++ .../integration/data_quality/conftest.py | 66 ++++- .../data_quality/test_data_diff.py | 238 ++++++------------ .../data_quality/test_data_quality.py | 52 ++++ .../data_quality/source/test_test_suite.py | 8 + 8 files changed, 226 insertions(+), 193 deletions(-) create mode 100644 ingestion/tests/integration/data_quality/test_data_quality.py diff --git a/ingestion/src/metadata/data_quality/api/models.py b/ingestion/src/metadata/data_quality/api/models.py index eae7544f8c1c..e70def6e9edb 100644 --- a/ingestion/src/metadata/data_quality/api/models.py +++ b/ingestion/src/metadata/data_quality/api/models.py @@ -56,7 +56,7 @@ class TableAndTests(BaseModel): table: Table = Field(None, description="Table being processed by the DQ workflow") service_type: str = Field(..., description="Service type the table belongs to") - test_cases: Optional[List[TestCase]] = Field( + test_cases: List[TestCase] = Field( None, description="Test Cases already existing in the Test Suite, if any" ) executable_test_suite: Optional[CreateTestSuiteRequest] = Field( diff --git a/ingestion/src/metadata/data_quality/processor/test_case_runner.py b/ingestion/src/metadata/data_quality/processor/test_case_runner.py index e87b74cca970..73a145d415d2 100644 --- a/ingestion/src/metadata/data_quality/processor/test_case_runner.py +++ b/ingestion/src/metadata/data_quality/processor/test_case_runner.py @@ -14,7 +14,7 @@ """ import traceback from copy import deepcopy -from typing import List, Optional, cast +from typing import List, Optional from metadata.data_quality.api.models import ( TableAndTests, @@ -90,15 +90,6 @@ def _run(self, record: TableAndTests) -> Either: ), table_fqn=record.table.fullyQualifiedName.root, ) - - if not test_cases: - return Either( - left=StackTraceError( - name="No test Cases", - error=f"No tests cases found for table {record.table.fullyQualifiedName.root}", - ) - ) - openmetadata_test_cases = self.filter_for_om_test_cases(test_cases) openmetadata_test_cases = self.filter_incompatible_test_cases( record.table, openmetadata_test_cases @@ -111,6 +102,12 @@ def _run(self, record: TableAndTests) -> Either: record.table, ).get_data_quality_runner() + logger.debug( + f"Found {len(openmetadata_test_cases)} test cases for table {record.table.fullyQualifiedName.root}" + ) + if len(openmetadata_test_cases) == 0: + logger.warning("No test cases found for the table") + test_results = [ test_case_result for test_case in openmetadata_test_cases @@ -120,17 +117,14 @@ def _run(self, record: TableAndTests) -> Either: return Either(right=TestCaseResults(test_results=test_results)) def get_test_cases( - self, test_cases: Optional[List[TestCase]], test_suite_fqn: str, table_fqn: str + self, test_cases: List[TestCase], test_suite_fqn: str, table_fqn: str ) -> List[TestCase]: """ Based on the test suite test cases that we already know, pick up the rest from the YAML config, compare and create the new ones """ if self.processor_config.testCases is not None: - cli_test_cases = self.get_test_case_from_cli_config() # type: ignore - cli_test_cases = cast( - List[TestCaseDefinition], cli_test_cases - ) # satisfy type checker + cli_test_cases = self.get_test_case_from_cli_config() return self.compare_and_create_test_cases( cli_test_cases_definitions=cli_test_cases, test_cases=test_cases, @@ -142,15 +136,13 @@ def get_test_cases( def get_test_case_from_cli_config( self, - ) -> Optional[List[TestCaseDefinition]]: + ) -> List[TestCaseDefinition]: """Get all the test cases names defined in the CLI config file""" - if self.processor_config.testCases is not None: - return list(self.processor_config.testCases) - return None + return list(self.processor_config.testCases or []) def compare_and_create_test_cases( self, - cli_test_cases_definitions: Optional[List[TestCaseDefinition]], + cli_test_cases_definitions: List[TestCaseDefinition], test_cases: List[TestCase], table_fqn: str, test_suite_fqn: str, diff --git a/ingestion/src/metadata/data_quality/source/test_suite.py b/ingestion/src/metadata/data_quality/source/test_suite.py index 2790d83fb488..272a592eb157 100644 --- a/ingestion/src/metadata/data_quality/source/test_suite.py +++ b/ingestion/src/metadata/data_quality/source/test_suite.py @@ -80,7 +80,7 @@ def _get_table_entity(self) -> Optional[Table]: def _get_test_cases_from_test_suite( self, test_suite: Optional[TestSuite] - ) -> Optional[List[TestCase]]: + ) -> List[TestCase]: """Return test cases if the test suite exists and has them""" if test_suite: test_cases = self.metadata.list_all_entities( @@ -94,8 +94,7 @@ def _get_test_cases_from_test_suite( t for t in test_cases if t.name in self.source_config.testCases ] return test_cases - - return None + return [] def prepare(self): """Nothing to prepare""" diff --git a/ingestion/tests/integration/conftest.py b/ingestion/tests/integration/conftest.py index 308989383a20..3407ae89a3a8 100644 --- a/ingestion/tests/integration/conftest.py +++ b/ingestion/tests/integration/conftest.py @@ -1,9 +1,11 @@ import logging import sys +from typing import List, Tuple, Type import pytest from _openmetadata_testutils.ometa import int_admin_ometa +from ingestion.src.metadata.ingestion.api.common import Entity from metadata.generated.schema.entity.services.databaseService import DatabaseService from metadata.generated.schema.metadataIngestion.workflow import LogLevels from metadata.ingestion.ometa.ometa_api import OpenMetadata @@ -171,3 +173,17 @@ def inner(*args, **kwargs): "metadata.ingestion.ometa.ometa_api.OpenMetadata.get_by_id", override_password(OpenMetadata.get_by_id), ) + + +@pytest.fixture +def cleanup_fqns(metadata): + fqns: List[Tuple[Type[Entity], str]] = [] + + def inner(entity_type: Type[Entity], fqn: str): + fqns.append((entity_type, fqn)) + + yield inner + for etype, fqn in fqns: + entity = metadata.get_by_name(etype, fqn, fields=["*"]) + if entity: + metadata.delete(etype, entity.id, recursive=True, hard_delete=True) diff --git a/ingestion/tests/integration/data_quality/conftest.py b/ingestion/tests/integration/data_quality/conftest.py index 7f04d86289e0..726488fdb369 100644 --- a/ingestion/tests/integration/data_quality/conftest.py +++ b/ingestion/tests/integration/data_quality/conftest.py @@ -2,7 +2,20 @@ from testcontainers.mysql import MySqlContainer from _openmetadata_testutils.postgres.conftest import postgres_container, try_bind -from metadata.generated.schema.entity.services.databaseService import DatabaseService +from metadata.generated.schema.api.services.createDatabaseService import ( + CreateDatabaseServiceRequest, +) +from metadata.generated.schema.entity.services.connections.database.common.basicAuth import ( + BasicAuth, +) +from metadata.generated.schema.entity.services.connections.database.postgresConnection import ( + PostgresConnection, +) +from metadata.generated.schema.entity.services.databaseService import ( + DatabaseConnection, + DatabaseService, + DatabaseServiceType, +) from metadata.generated.schema.metadataIngestion.workflow import LogLevels from metadata.ingestion.models.custom_pydantic import CustomSecretStr from metadata.ingestion.ometa.ometa_api import OpenMetadata @@ -63,3 +76,54 @@ def ingest_mysql_service( ) yield db_service metadata.delete(DatabaseService, db_service.id, recursive=True, hard_delete=True) + + +@pytest.fixture(scope="module") +def create_service_request(tmp_path_factory, postgres_container): + return CreateDatabaseServiceRequest( + name="docker_test_" + tmp_path_factory.mktemp("postgres").name, + serviceType=DatabaseServiceType.Postgres, + connection=DatabaseConnection( + config=PostgresConnection( + username=postgres_container.username, + authType=BasicAuth(password=postgres_container.password), + hostPort="localhost:" + + postgres_container.get_exposed_port(postgres_container.port), + database="dvdrental", + ) + ), + ) + + +@pytest.fixture(scope="module") +def postgres_service(db_service): + return db_service + + +@pytest.fixture() +def ingest_postgres_metadata( + postgres_service, metadata: OpenMetadata, sink_config, workflow_config, run_workflow +): + workflow_config = { + "source": { + "type": postgres_service.connection.config.type.value.lower(), + "serviceName": postgres_service.fullyQualifiedName.root, + "serviceConnection": postgres_service.connection, + "sourceConfig": {"config": {}}, + }, + "sink": sink_config, + "workflowConfig": workflow_config, + } + run_workflow(MetadataWorkflow, workflow_config) + + +@pytest.fixture(scope="module") +def patch_password(postgres_container): + def inner(service: DatabaseService): + service.connection.config = cast(PostgresConnection, service.connection.config) + service.connection.config.authType.password = type( + service.connection.config.authType.password + )(postgres_container.password) + return service + + return inner diff --git a/ingestion/tests/integration/data_quality/test_data_diff.py b/ingestion/tests/integration/data_quality/test_data_diff.py index 1e2a5f5a8acf..0b29a8054b1c 100644 --- a/ingestion/tests/integration/data_quality/test_data_diff.py +++ b/ingestion/tests/integration/data_quality/test_data_diff.py @@ -14,33 +14,10 @@ from _openmetadata_testutils.postgres.conftest import postgres_container from _openmetadata_testutils.pydantic.test_utils import assert_equal_pydantic_objects from metadata.data_quality.api.models import TestCaseDefinition -from metadata.generated.schema.api.services.createDatabaseService import ( - CreateDatabaseServiceRequest, -) from metadata.generated.schema.entity.data.table import Table -from metadata.generated.schema.entity.services.connections.database.common.basicAuth import ( - BasicAuth, -) -from metadata.generated.schema.entity.services.connections.database.postgresConnection import ( - PostgresConnection, -) -from metadata.generated.schema.entity.services.databaseService import ( - DatabaseConnection, - DatabaseService, - DatabaseServiceType, -) +from metadata.generated.schema.entity.services.databaseService import DatabaseService from metadata.generated.schema.metadataIngestion.testSuitePipeline import ( TestSuiteConfigType, - TestSuitePipeline, -) -from metadata.generated.schema.metadataIngestion.workflow import ( - LogLevels, - OpenMetadataWorkflowConfig, - Processor, - Sink, - Source, - SourceConfig, - WorkflowConfig, ) from metadata.generated.schema.tests.basic import ( TestCaseResult, @@ -48,10 +25,8 @@ TestResultValue, ) from metadata.generated.schema.tests.testCase import TestCase, TestCaseParameterValue -from metadata.ingestion.models.custom_pydantic import CustomSecretStr from metadata.ingestion.ometa.ometa_api import OpenMetadata from metadata.workflow.data_quality import TestSuiteWorkflow -from metadata.workflow.metadata import MetadataWorkflow if not sys.version_info >= (3, 9): pytest.skip( @@ -258,6 +233,11 @@ def test_happy_paths( ingest_mysql_service, patched_metadata, parameters: TestParameters, + sink_config, + profiler_config, + run_workflow, + workflow_config, + cleanup_fqns, ): metadata = patched_metadata table1 = metadata.get_by_name( @@ -265,6 +245,10 @@ def test_happy_paths( f"{postgres_service.fullyQualifiedName.root}.dvdrental.public.customer", nullable=False, ) + cleanup_fqns( + TestCase, + f"{table1.fullyQualifiedName.root}.{parameters.test_case_defintion.name}", + ) table2_service = { "POSTGRES_SERVICE": postgres_service, "MYSQL_SERVICE": ingest_mysql_service, @@ -281,41 +265,30 @@ def test_happy_paths( ), ] ) - - workflow_config = OpenMetadataWorkflowConfig( - source=Source( - type=TestSuiteConfigType.TestSuite.value, - serviceName="MyTestSuite", - sourceConfig=SourceConfig( - config=TestSuitePipeline( - type=TestSuiteConfigType.TestSuite, - entityFullyQualifiedName=f"{table1.fullyQualifiedName.root}", - ) - ), - ), - processor=Processor( - type="orm-test-runner", - config={"testCases": [parameters.test_case_defintion]}, - ), - sink=Sink( - type="metadata-rest", - config={}, - ), - workflowConfig=WorkflowConfig( - loggerLevel=LogLevels.DEBUG, openMetadataServerConfig=metadata.config - ), - ) - - test_suite_procesor = TestSuiteWorkflow.create(workflow_config) - test_suite_procesor.execute() - test_suite_procesor.stop() - test_case_entity: TestCase = metadata.get_or_create_test_case( - f"{table1.fullyQualifiedName.root}.{parameters.test_case_defintion.name}" + workflow_config = { + "source": { + "type": TestSuiteConfigType.TestSuite.value, + "serviceName": "MyTestSuite", + "sourceConfig": { + "config": { + "type": TestSuiteConfigType.TestSuite.value, + "entityFullyQualifiedName": table1.fullyQualifiedName.root, + } + }, + }, + "processor": { + "type": "orm-test-runner", + "config": {"testCases": [parameters.test_case_defintion.dict()]}, + }, + "sink": sink_config, + "workflowConfig": workflow_config, + } + run_workflow(TestSuiteWorkflow, workflow_config) + test_case_entity = metadata.get_by_name( + TestCase, + f"{table1.fullyQualifiedName.root}.{parameters.test_case_defintion.name}", + fields=["*"], ) - try: - test_suite_procesor.raise_from_status() - finally: - metadata.delete(TestCase, test_case_entity.id, recursive=True, hard_delete=True) assert "ERROR: Unexpected error" not in test_case_entity.testCaseResult.result assert_equal_pydantic_objects(parameters.expected, test_case_entity.testCaseResult) @@ -393,6 +366,10 @@ def test_error_paths( ingest_mysql_service: DatabaseService, postgres_service: DatabaseService, patched_metadata: OpenMetadata, + sink_config, + workflow_config, + run_workflow, + cleanup_fqns, ): metadata = patched_metadata table1 = metadata.get_by_name( @@ -400,94 +377,37 @@ def test_error_paths( f"{postgres_service.fullyQualifiedName.root}.dvdrental.public.customer", nullable=False, ) + cleanup_fqns(TestCase, f"{table1.fullyQualifiedName.root}.{parameters.name}") for parameter in parameters.parameterValues: if parameter.name == "table2": parameter.value = parameter.value.replace( "POSTGRES_SERVICE", postgres_service.fullyQualifiedName.root ) - workflow_config = OpenMetadataWorkflowConfig( - source=Source( - type=TestSuiteConfigType.TestSuite.value, - serviceName="MyTestSuite", - sourceConfig=SourceConfig( - config=TestSuitePipeline( - type=TestSuiteConfigType.TestSuite, - entityFullyQualifiedName=f"{table1.fullyQualifiedName.root}", - ) - ), - serviceConnection=postgres_service.connection, - ), - processor=Processor( - type="orm-test-runner", - config={"testCases": [parameters]}, - ), - sink=Sink( - type="metadata-rest", - config={}, - ), - workflowConfig=WorkflowConfig( - loggerLevel=LogLevels.DEBUG, openMetadataServerConfig=metadata.config - ), - ) - test_suite_procesor = TestSuiteWorkflow.create(workflow_config) - test_suite_procesor.execute() - test_suite_procesor.stop() + workflow_config = { + "source": { + "type": TestSuiteConfigType.TestSuite.value, + "serviceName": "MyTestSuite", + "sourceConfig": { + "config": { + "type": TestSuiteConfigType.TestSuite.value, + "entityFullyQualifiedName": table1.fullyQualifiedName.root, + } + }, + }, + "processor": { + "type": "orm-test-runner", + "config": {"testCases": [parameters.dict()]}, + }, + "sink": sink_config, + "workflowConfig": workflow_config, + } + run_workflow(TestSuiteWorkflow, workflow_config) test_case_entity: TestCase = metadata.get_or_create_test_case( f"{table1.fullyQualifiedName.root}.{parameters.name}" ) - try: - test_suite_procesor.raise_from_status() - finally: - metadata.delete(TestCase, test_case_entity.id, recursive=True, hard_delete=True) assert_equal_pydantic_objects(expected, test_case_entity.testCaseResult) -@pytest.fixture(scope="module") -def postgres_service(metadata, postgres_container): - service = CreateDatabaseServiceRequest( - name="docker_test_postgres_db", - serviceType=DatabaseServiceType.Postgres, - connection=DatabaseConnection( - config=PostgresConnection( - username=postgres_container.username, - authType=BasicAuth(password=postgres_container.password), - hostPort="localhost:" - + postgres_container.get_exposed_port(postgres_container.port), - database="dvdrental", - ) - ), - ) - service_entity = metadata.create_or_update(data=service) - service_entity.connection.config.authType.password = CustomSecretStr( - postgres_container.password - ) - yield service_entity - metadata.delete( - DatabaseService, service_entity.id, recursive=True, hard_delete=True - ) - - -@pytest.fixture(scope="module") -def ingest_postgres_metadata(postgres_service, metadata: OpenMetadata): - workflow_config = OpenMetadataWorkflowConfig( - source=Source( - type=postgres_service.connection.config.type.value.lower(), - serviceName=postgres_service.fullyQualifiedName.root, - serviceConnection=postgres_service.connection, - sourceConfig=SourceConfig(config={}), - ), - sink=Sink( - type="metadata-rest", - config={}, - ), - workflowConfig=WorkflowConfig(openMetadataServerConfig=metadata.config), - ) - metadata_ingestion = MetadataWorkflow.create(workflow_config) - metadata_ingestion.execute() - metadata_ingestion.raise_from_status() - return - - def add_changed_tables(connection: Connection): connection.execute("CREATE TABLE customer_200 AS SELECT * FROM customer LIMIT 200;") connection.execute("CREATE TABLE changed_customer AS SELECT * FROM customer;") @@ -573,46 +493,28 @@ def copy_table(source_engine, destination_engine, table_name): @pytest.fixture def patched_metadata(metadata, postgres_service, ingest_mysql_service, monkeypatch): - openmetadata_get_by_name = OpenMetadata.get_by_name + dbs_by_name = { + service.fullyQualifiedName.root: service + for service in [postgres_service, ingest_mysql_service] + } - def get_by_name_override_service_password(self, entity, fqn, *args, **kwargs): - result = openmetadata_get_by_name(self, entity, fqn, *args, **kwargs) - if entity == DatabaseService: - return next( - ( - service - for service in [postgres_service, ingest_mysql_service] - if service.fullyQualifiedName.root == fqn - ), - result, - ) + def override_result_by_fqn(func): + def inner(*args, **kwargs): + result = func(*args, **kwargs) + if result.fullyQualifiedName.root in dbs_by_name: + return dbs_by_name[result.fullyQualifiedName.root] + return result - return result + return inner monkeypatch.setattr( "metadata.ingestion.ometa.ometa_api.OpenMetadata.get_by_name", - get_by_name_override_service_password, + override_result_by_fqn(OpenMetadata.get_by_name), ) - openmetadata_get_by_id = OpenMetadata.get_by_id - - def get_by_id_override_service_password(self, entity, entity_id, *args, **kwargs): - result = openmetadata_get_by_id(self, entity, entity_id, *args, **kwargs) - if entity == DatabaseService: - return next( - ( - service - for service in [postgres_service, ingest_mysql_service] - if service.id == entity_id - ), - result, - ) - - return result - monkeypatch.setattr( "metadata.ingestion.ometa.ometa_api.OpenMetadata.get_by_id", - get_by_id_override_service_password, + override_result_by_fqn(OpenMetadata.get_by_id), ) return metadata diff --git a/ingestion/tests/integration/data_quality/test_data_quality.py b/ingestion/tests/integration/data_quality/test_data_quality.py new file mode 100644 index 000000000000..d6cb28b8f88b --- /dev/null +++ b/ingestion/tests/integration/data_quality/test_data_quality.py @@ -0,0 +1,52 @@ +import sys + +import pytest + +from metadata.generated.schema.entity.data.table import Table +from metadata.generated.schema.entity.services.databaseService import DatabaseService +from metadata.generated.schema.metadataIngestion.testSuitePipeline import ( + TestSuiteConfigType, +) +from metadata.workflow.data_quality import TestSuiteWorkflow + +if not sys.version_info >= (3, 9): + pytest.skip( + "requires python 3.9+ due to incompatibility with testcontainers", + allow_module_level=True, + ) + + +def test_empty_test_suite( + postgres_service: DatabaseService, + run_workflow, + ingest_postgres_metadata, + patch_passwords_for_db_services, + metadata, + sink_config, + workflow_config, + cleanup_fqns, +): + table = metadata.get_by_name( + Table, + f"{postgres_service.fullyQualifiedName.root}.dvdrental.public.customer", + nullable=False, + ) + workflow_config = { + "source": { + "type": TestSuiteConfigType.TestSuite.value, + "serviceName": "MyTestSuite", + "sourceConfig": { + "config": { + "type": TestSuiteConfigType.TestSuite.value, + "entityFullyQualifiedName": table.fullyQualifiedName.root, + } + }, + }, + "processor": { + "type": "orm-test-runner", + "config": {"testCases": []}, + }, + "sink": sink_config, + "workflowConfig": workflow_config, + } + run_workflow(TestSuiteWorkflow, workflow_config) diff --git a/ingestion/tests/unit/data_quality/source/test_test_suite.py b/ingestion/tests/unit/data_quality/source/test_test_suite.py index e02e59813212..d909026a4f3e 100644 --- a/ingestion/tests/unit/data_quality/source/test_test_suite.py +++ b/ingestion/tests/unit/data_quality/source/test_test_suite.py @@ -38,6 +38,14 @@ }, ["test_case1"], ), + ( + { + "type": "TestSuite", + "entityFullyQualifiedName": "MyTestSuite", + "testCases": [], + }, + [], + ), ], ) def test_source_config(parameters, expected, monkeypatch): From e58a788f471c914c206124fdc50ca5e66cb8c917 Mon Sep 17 00:00:00 2001 From: Prajwal214 <167504578+Prajwal214@users.noreply.github.com> Date: Fri, 19 Jul 2024 16:10:43 +0530 Subject: [PATCH 21/33] Doc: Updating Athena Service Connection Docs (#17090) Co-authored-by: Prajwal Pandit --- .../ui/public/locales/en-US/Database/Athena.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/openmetadata-ui/src/main/resources/ui/public/locales/en-US/Database/Athena.md b/openmetadata-ui/src/main/resources/ui/public/locales/en-US/Database/Athena.md index 5eff48696488..d66261d7210f 100644 --- a/openmetadata-ui/src/main/resources/ui/public/locales/en-US/Database/Athena.md +++ b/openmetadata-ui/src/main/resources/ui/public/locales/en-US/Database/Athena.md @@ -184,3 +184,19 @@ Database Service > Database > Schema > Table In the case of Athena, we won't have a Database as such. If you'd like to see your data in a database named something other than `default`, you can specify the name in this field. $$ + +$$section +### Bucket Name $(id="bucketName") + +A bucket name in Data Lake is a unique identifier used to organize and store data objects. + +It's similar to a folder name, but it's used for object storage rather than file storage. +$$ + +$$section +### Prefix $(id="prefix") + +The prefix of a data source refers to the first part of the data path that identifies the source or origin of the data. + +It's used to organize and categorize data within the container, and can help users easily locate and access the data they need. +$$ \ No newline at end of file From b1abb7cced75fb1b49f8733dbdf5458396c93250 Mon Sep 17 00:00:00 2001 From: Prajwal214 <167504578+Prajwal214@users.noreply.github.com> Date: Fri, 19 Jul 2024 16:11:32 +0530 Subject: [PATCH 22/33] Doc: Updating BigQ Table Connection Docs (#17076) Co-authored-by: Prajwal Pandit --- .../public/locales/en-US/Database/BigQuery.md | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/openmetadata-ui/src/main/resources/ui/public/locales/en-US/Database/BigQuery.md b/openmetadata-ui/src/main/resources/ui/public/locales/en-US/Database/BigQuery.md index 79e6ccca11a9..0dcc060daea6 100644 --- a/openmetadata-ui/src/main/resources/ui/public/locales/en-US/Database/BigQuery.md +++ b/openmetadata-ui/src/main/resources/ui/public/locales/en-US/Database/BigQuery.md @@ -186,3 +186,39 @@ $$section Additional connection arguments such as security or protocol configs that can be sent to service during connection. $$ + +$$section +### Target Service Account Email $(id="impersonateServiceAccount") + +The impersonated service account email. +$$ + +$$section +### Lifetime $(id="lifetime") + +Number of seconds the delegated credential should be valid. +$$ + +$$section +### Audience $(id="audience") + +Google Security Token Service audience which contains the resource name for the workload identity pool and the provider identifier in that pool. +$$ + +$$section +### Subject Token Type $(id="subjectTokenType") + +Google Security Token Service subject token type based on the OAuth 2.0 token exchange spec. +$$ + +$$section +### Token URL $(id="tokenURL") + +Google Security Token Service token exchange endpoint. +$$ + +$$section +### Credential Source $(id="credentialSource") + +This object defines the mechanism used to retrieve the external credential from the local environment so that it can be exchanged for a GCP access token via the STS endpoint. +$$ \ No newline at end of file From b568444d5ef7f87307d2997ba7c2025bee8cf194 Mon Sep 17 00:00:00 2001 From: Teddy Date: Fri, 19 Jul 2024 13:07:48 +0200 Subject: [PATCH 23/33] fix: handle potential null pointer in test case reindex (#17100) --- .../openmetadata/service/search/indexes/TestCaseIndex.java | 6 ++++-- .../service/resources/dqtests/TestCaseResourceTest.java | 6 ++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TestCaseIndex.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TestCaseIndex.java index d37149a872ef..73caff3955dd 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TestCaseIndex.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/indexes/TestCaseIndex.java @@ -26,8 +26,10 @@ public Object getEntity() { public void removeNonIndexableFields(Map esDoc) { SearchIndex.super.removeNonIndexableFields(esDoc); List> testSuites = (List>) esDoc.get("testSuites"); - for (Map testSuite : testSuites) { - SearchIndexUtils.removeNonIndexableFields(testSuite, excludeFields); + if (testSuites != null) { + for (Map testSuite : testSuites) { + SearchIndexUtils.removeNonIndexableFields(testSuite, excludeFields); + } } } diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestCaseResourceTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestCaseResourceTest.java index af6e6da9ed3b..803727ec113e 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestCaseResourceTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/resources/dqtests/TestCaseResourceTest.java @@ -2049,6 +2049,12 @@ void test_testCaseEsDocCleanUp(TestInfo testInfo) { assertNull(doc.get("changeDescription")); List> testSuites = (List>) doc.get("testSuites"); assertNull(testSuites.get(0).get("changeDescription")); + + // Remove changeDescription logic handles null testSuites + testCase.setTestSuites(null); + doc = JsonUtils.convertValue(testCase, Map.class); + testCaseIndex = new TestCaseIndex(testCase); + testCaseIndex.removeNonIndexableFields(doc); } public void deleteTestCaseResult(String fqn, Long timestamp, Map authHeaders) From 9011127b1737467a123bd276e92722d611d234bf Mon Sep 17 00:00:00 2001 From: Prajwal214 <167504578+Prajwal214@users.noreply.github.com> Date: Fri, 19 Jul 2024 17:32:12 +0530 Subject: [PATCH 24/33] Doc: Updated Athena Docs (#17103) Co-authored-by: Prajwal Pandit --- .../content/v1.4.x/connectors/database/athena/index.md | 4 ++-- .../v1.5.x-SNAPSHOT/connectors/database/athena/index.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/openmetadata-docs/content/v1.4.x/connectors/database/athena/index.md b/openmetadata-docs/content/v1.4.x/connectors/database/athena/index.md index 1611b30804fd..52b56b9024c8 100644 --- a/openmetadata-docs/content/v1.4.x/connectors/database/athena/index.md +++ b/openmetadata-docs/content/v1.4.x/connectors/database/athena/index.md @@ -196,8 +196,8 @@ Find more information about the [Role Session Name](https://docs.aws.amazon.com/ Find more information about [Source Identity](https://docs.aws.amazon.com/STS/latest/APIReference/API_AssumeRole.html#:~:text=Required%3A%20No-,SourceIdentity,-The%20source%20identity). -- **S3 Staging Directory (optional)**: The S3 staging directory is an optional parameter. Enter a staging directory to override the default staging directory for AWS Athena. -- **Athena Workgroup (optional)**: The Athena workgroup is an optional parameter. If you wish to have your Athena connection related to an existing AWS workgroup add your workgroup name here. +- **S3 Staging Directory**: The S3 staging directory is an optional parameter. Enter a staging directory to override the default staging directory for AWS Athena. +- **Athena Workgroup**: The Athena workgroup is an optional parameter. If you wish to have your Athena connection related to an existing AWS workgroup add your workgroup name here. {% partial file="/v1.4/connectors/database/advanced-configuration.md" /%} diff --git a/openmetadata-docs/content/v1.5.x-SNAPSHOT/connectors/database/athena/index.md b/openmetadata-docs/content/v1.5.x-SNAPSHOT/connectors/database/athena/index.md index b139c451dcc0..a02b6b899598 100644 --- a/openmetadata-docs/content/v1.5.x-SNAPSHOT/connectors/database/athena/index.md +++ b/openmetadata-docs/content/v1.5.x-SNAPSHOT/connectors/database/athena/index.md @@ -196,8 +196,8 @@ Find more information about the [Role Session Name](https://docs.aws.amazon.com/ Find more information about [Source Identity](https://docs.aws.amazon.com/STS/latest/APIReference/API_AssumeRole.html#:~:text=Required%3A%20No-,SourceIdentity,-The%20source%20identity). -- **S3 Staging Directory (optional)**: The S3 staging directory is an optional parameter. Enter a staging directory to override the default staging directory for AWS Athena. -- **Athena Workgroup (optional)**: The Athena workgroup is an optional parameter. If you wish to have your Athena connection related to an existing AWS workgroup add your workgroup name here. +- **S3 Staging Directory**: The S3 staging directory is an optional parameter. Enter a staging directory to override the default staging directory for AWS Athena. +- **Athena Workgroup**: The Athena workgroup is an optional parameter. If you wish to have your Athena connection related to an existing AWS workgroup add your workgroup name here. {% partial file="/v1.5/connectors/database/advanced-configuration.md" /%} From 5be5a05390ca77404a326bfa67e2b5e5cfd5e9c4 Mon Sep 17 00:00:00 2001 From: Ayush Shah Date: Fri, 19 Jul 2024 17:33:19 +0530 Subject: [PATCH 25/33] Fixes #17051: Dynamic import for Profiler Interface (#17073) --- .../interface/profiler_interface_factory.py | 97 +++++++++---------- .../test_profiler_interface_factory.py | 43 ++++++++ 2 files changed, 89 insertions(+), 51 deletions(-) create mode 100644 ingestion/tests/unit/profiler/test_profiler_interface_factory.py diff --git a/ingestion/src/metadata/profiler/interface/profiler_interface_factory.py b/ingestion/src/metadata/profiler/interface/profiler_interface_factory.py index db8166c6344b..dc443e047161 100644 --- a/ingestion/src/metadata/profiler/interface/profiler_interface_factory.py +++ b/ingestion/src/metadata/profiler/interface/profiler_interface_factory.py @@ -13,7 +13,8 @@ Factory class for creating profiler interface objects """ -from typing import cast +import importlib +from typing import Dict, cast from metadata.generated.schema.entity.services.connections.database.bigQueryConnection import ( BigQueryConnection, @@ -50,63 +51,57 @@ ) from metadata.generated.schema.entity.services.databaseService import DatabaseConnection from metadata.profiler.factory import Factory -from metadata.profiler.interface.nosql.profiler_interface import NoSQLProfilerInterface -from metadata.profiler.interface.pandas.profiler_interface import ( - PandasProfilerInterface, -) from metadata.profiler.interface.profiler_interface import ProfilerInterface -from metadata.profiler.interface.sqlalchemy.bigquery.profiler_interface import ( - BigQueryProfilerInterface, -) -from metadata.profiler.interface.sqlalchemy.databricks.profiler_interface import ( - DatabricksProfilerInterface, -) -from metadata.profiler.interface.sqlalchemy.db2.profiler_interface import ( - DB2ProfilerInterface, -) -from metadata.profiler.interface.sqlalchemy.mariadb.profiler_interface import ( - MariaDBProfilerInterface, -) -from metadata.profiler.interface.sqlalchemy.profiler_interface import ( - SQAProfilerInterface, -) -from metadata.profiler.interface.sqlalchemy.single_store.profiler_interface import ( - SingleStoreProfilerInterface, -) -from metadata.profiler.interface.sqlalchemy.snowflake.profiler_interface import ( - SnowflakeProfilerInterface, -) -from metadata.profiler.interface.sqlalchemy.trino.profiler_interface import ( - TrinoProfilerInterface, -) -from metadata.profiler.interface.sqlalchemy.unity_catalog.profiler_interface import ( - UnityCatalogProfilerInterface, -) class ProfilerInterfaceFactory(Factory): def create(self, interface_type: str, *args, **kwargs): """Create interface object based on interface type""" - interface_class = self._interface_type.get(interface_type) - if not interface_class: - interface_class = self._interface_type.get(DatabaseConnection.__name__) - interface_class = cast(ProfilerInterface, interface_class) - return interface_class.create(*args, **kwargs) + interface_class_path = profiler_class_mapping.get( + interface_type, profiler_class_mapping[DatabaseConnection.__name__] + ) + try: + module_path, class_name = interface_class_path.rsplit(".", 1) + module = importlib.import_module(module_path) + profiler_class = getattr(module, class_name) + except (ImportError, AttributeError) as e: + raise ImportError(f"Error importing {class_name} from {module_path}: {e}") + profiler_class = cast(ProfilerInterface, profiler_class) + return profiler_class.create(*args, **kwargs) profiler_interface_factory = ProfilerInterfaceFactory() -profilers = { - DatabaseConnection.__name__: SQAProfilerInterface, - BigQueryConnection.__name__: BigQueryProfilerInterface, - SingleStoreConnection.__name__: SingleStoreProfilerInterface, - DatalakeConnection.__name__: PandasProfilerInterface, - MariaDBConnection.__name__: MariaDBProfilerInterface, - SnowflakeConnection.__name__: SnowflakeProfilerInterface, - TrinoConnection.__name__: TrinoProfilerInterface, - UnityCatalogConnection.__name__: UnityCatalogProfilerInterface, - DatabricksConnection.__name__: DatabricksProfilerInterface, - Db2Connection.__name__: DB2ProfilerInterface, - MongoDBConnection.__name__: NoSQLProfilerInterface, - DynamoDBConnection.__name__: NoSQLProfilerInterface, + +BASE_PROFILER_PATH = "metadata.profiler.interface" +SQLALCHEMY_PROFILER_PATH = f"{BASE_PROFILER_PATH}.sqlalchemy" +NOSQL_PROFILER_PATH = ( + f"{BASE_PROFILER_PATH}.nosql.profiler_interface.NoSQLProfilerInterface" +) +PANDAS_PROFILER_PATH = ( + f"{BASE_PROFILER_PATH}.pandas.profiler_interface.PandasProfilerInterface" +) + +# Configuration for dynamic imports +profiler_class_mapping: Dict[str, str] = { + DatabaseConnection.__name__: SQLALCHEMY_PROFILER_PATH + + ".profiler_interface.SQAProfilerInterface", + BigQueryConnection.__name__: SQLALCHEMY_PROFILER_PATH + + ".bigquery.profiler_interface.BigQueryProfilerInterface", + SingleStoreConnection.__name__: SQLALCHEMY_PROFILER_PATH + + ".single_store.profiler_interface.SingleStoreProfilerInterface", + DatalakeConnection.__name__: PANDAS_PROFILER_PATH, + MariaDBConnection.__name__: SQLALCHEMY_PROFILER_PATH + + ".mariadb.profiler_interface.MariaDBProfilerInterface", + SnowflakeConnection.__name__: SQLALCHEMY_PROFILER_PATH + + ".snowflake.profiler_interface.SnowflakeProfilerInterface", + TrinoConnection.__name__: SQLALCHEMY_PROFILER_PATH + + ".trino.profiler_interface.TrinoProfilerInterface", + UnityCatalogConnection.__name__: SQLALCHEMY_PROFILER_PATH + + ".unity_catalog.profiler_interface.UnityCatalogProfilerInterface", + DatabricksConnection.__name__: SQLALCHEMY_PROFILER_PATH + + ".databricks.profiler_interface.DatabricksProfilerInterface", + Db2Connection.__name__: SQLALCHEMY_PROFILER_PATH + + ".db2.profiler_interface.DB2ProfilerInterface", + MongoDBConnection.__name__: NOSQL_PROFILER_PATH, + DynamoDBConnection.__name__: NOSQL_PROFILER_PATH, } -profiler_interface_factory.register_many(profilers) diff --git a/ingestion/tests/unit/profiler/test_profiler_interface_factory.py b/ingestion/tests/unit/profiler/test_profiler_interface_factory.py new file mode 100644 index 000000000000..38a6263b0984 --- /dev/null +++ b/ingestion/tests/unit/profiler/test_profiler_interface_factory.py @@ -0,0 +1,43 @@ +# Copyright 2021 Collate +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# http://www.apache.org/licenses/LICENSE-2.0 +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +Factory class for creating profiler interface objects +""" + +from typing import Dict +from unittest import TestCase + +from metadata.profiler.interface.profiler_interface_factory import ( + profiler_class_mapping, +) + + +class TestProfilerClassMapping(TestCase): + def setUp(self): + self.expected_mapping: Dict[str, str] = { + "DatabaseConnection": "metadata.profiler.interface.sqlalchemy.profiler_interface.SQAProfilerInterface", + "BigQueryConnection": "metadata.profiler.interface.sqlalchemy.bigquery.profiler_interface.BigQueryProfilerInterface", + "SingleStoreConnection": "metadata.profiler.interface.sqlalchemy.single_store.profiler_interface.SingleStoreProfilerInterface", + "DatalakeConnection": "metadata.profiler.interface.pandas.profiler_interface.PandasProfilerInterface", + "MariaDBConnection": "metadata.profiler.interface.sqlalchemy.mariadb.profiler_interface.MariaDBProfilerInterface", + "SnowflakeConnection": "metadata.profiler.interface.sqlalchemy.snowflake.profiler_interface.SnowflakeProfilerInterface", + "TrinoConnection": "metadata.profiler.interface.sqlalchemy.trino.profiler_interface.TrinoProfilerInterface", + "UnityCatalogConnection": "metadata.profiler.interface.sqlalchemy.unity_catalog.profiler_interface.UnityCatalogProfilerInterface", + "DatabricksConnection": "metadata.profiler.interface.sqlalchemy.databricks.profiler_interface.DatabricksProfilerInterface", + "Db2Connection": "metadata.profiler.interface.sqlalchemy.db2.profiler_interface.DB2ProfilerInterface", + "MongoDBConnection": "metadata.profiler.interface.nosql.profiler_interface.NoSQLProfilerInterface", + "DynamoDBConnection": "metadata.profiler.interface.nosql.profiler_interface.NoSQLProfilerInterface", + } + + def test_profiler_class_mapping(self): + self.assertEqual(len(profiler_class_mapping), len(self.expected_mapping)) + self.assertEqual(profiler_class_mapping, self.expected_mapping) From 7ef90c3628cabcbcec03a8c6b21ad6ca985676e6 Mon Sep 17 00:00:00 2001 From: Pere Miquel Brull Date: Fri, 19 Jul 2024 15:18:41 +0200 Subject: [PATCH 26/33] MINOR - Update MetaPilot config (#17101) --- .../configuration/external/metaPilotAppConfig.json | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/openmetadata-spec/src/main/resources/json/schema/entity/applications/configuration/external/metaPilotAppConfig.json b/openmetadata-spec/src/main/resources/json/schema/entity/applications/configuration/external/metaPilotAppConfig.json index 84c4f6cac4f8..e073fdbbda39 100644 --- a/openmetadata-spec/src/main/resources/json/schema/entity/applications/configuration/external/metaPilotAppConfig.json +++ b/openmetadata-spec/src/main/resources/json/schema/entity/applications/configuration/external/metaPilotAppConfig.json @@ -31,6 +31,17 @@ "autoCompleteType": "database_search_index" } }, + "copilotDatabases": { + "title": "Databases for SQL Copilot", + "description": "Services and Databases configured to get enable the SQL Copilot.", + "type": "array", + "items": { + "placeholder": "Search Databases", + "type": "string", + "format": "autoComplete", + "autoCompleteType": "database_search_index" + } + }, "defaultScope": { "title": "Default Chatbot Database Scope", "description": "Default database scope for the chatbot.", From 8434fe2c50989bb06d3a3e73bee38105094bcf7c Mon Sep 17 00:00:00 2001 From: Sachin Chaurasiya Date: Fri, 19 Jul 2024 18:52:01 +0530 Subject: [PATCH 27/33] fix(#17016): apply Read-Only Mode to All Custom and Built-In Nodes in Block Editor (#17080) * fix(#17016): apply Read-Only Mode to All Custom and Built-In Nodes in Block Editor * test: add unit test * fix playwright test --------- Co-authored-by: Ashish Gupta --- .../e2e/Features/ActivityFeed.spec.ts | 2 +- .../BlockEditor/BlockMenu/BlockMenu.tsx | 9 ++++-- .../BlockEditor/BubbleMenu/BubbleMenu.tsx | 4 ++- .../components/BlockEditor/EditorSlots.tsx | 5 +++ .../Callout/CalloutComponent.test.tsx | 28 ++++++++++++++++ .../Extensions/Callout/CalloutComponent.tsx | 8 ++++- .../MathEquation/MathEquationComponent.tsx | 4 ++- .../Extensions/image/ImageComponent.test.tsx | 32 +++++++++++++++++++ .../Extensions/image/ImageComponent.tsx | 8 ++++- .../BlockEditor/TableMenu/TableMenu.tsx | 6 ++-- 10 files changed, 95 insertions(+), 11 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ActivityFeed.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ActivityFeed.spec.ts index 21fcd6282fb7..ecbc948cf4a0 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ActivityFeed.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ActivityFeed.spec.ts @@ -186,7 +186,7 @@ test.describe('Activity feed', () => { test('Update Description Task on Columns', async ({ page }) => { const firstTaskValue: TaskDetails = { term: entity.entity.name, - assignee: `${user.data.firstName}.${user.data.lastName}`, + assignee: user.responseData.name, description: 'Column Description 1', columnName: entity.entity.columns[0].name, oldDescription: entity.entity.columns[0].description, diff --git a/openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/BlockMenu/BlockMenu.tsx b/openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/BlockMenu/BlockMenu.tsx index 1e47efcd91ba..9f595fa62dc6 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/BlockMenu/BlockMenu.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/BlockMenu/BlockMenu.tsx @@ -33,7 +33,7 @@ interface BlockMenuProps { export const BlockMenu = (props: BlockMenuProps) => { const { t } = useTranslation(); const { editor } = props; - const { view } = editor; + const { view, isEditable } = editor; const menuRef = useRef(null); const popup = useRef(null); @@ -127,7 +127,10 @@ export const BlockMenu = (props: BlockMenuProps) => { }, [editor]); useEffect(() => { - if (menuRef.current) { + /** + * Create a new tippy instance for the block menu if the editor is editable + */ + if (menuRef.current && isEditable) { menuRef.current.remove(); menuRef.current.style.visibility = 'visible'; @@ -150,7 +153,7 @@ export const BlockMenu = (props: BlockMenuProps) => { popup.current?.destroy(); popup.current = null; }; - }, []); + }, [isEditable]); useEffect(() => { document.addEventListener('click', handleClickDragHandle); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/BubbleMenu/BubbleMenu.tsx b/openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/BubbleMenu/BubbleMenu.tsx index aab0809f3a6d..c728ddb7ecd2 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/BubbleMenu/BubbleMenu.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/BubbleMenu/BubbleMenu.tsx @@ -111,12 +111,14 @@ const BubbleMenu: FC = ({ editor, toggleLink }) => { // - the selection is empty // - the selection is a node selection (for drag handles) // - link is active + // - editor is not editable if ( editor.isActive('image') || empty || isNodeSelection(selection) || editor.isActive('link') || - editor.isActive('table') + editor.isActive('table') || + !editor.isEditable ) { return false; } diff --git a/openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/EditorSlots.tsx b/openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/EditorSlots.tsx index 7ecc0f78c1de..e3f56d35d120 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/EditorSlots.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/EditorSlots.tsx @@ -78,6 +78,11 @@ const EditorSlots = forwardRef( const handleLinkPopup = ( e: React.MouseEvent ) => { + // if editor is not editable, do not show the link popup + if (!editor?.isEditable) { + return; + } + let popup: Instance[] = []; let component: ReactRenderer; const target = e.target as HTMLElement; diff --git a/openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/Extensions/Callout/CalloutComponent.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/Extensions/Callout/CalloutComponent.test.tsx index a5a63bc77dd4..06bb3f8b2fde 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/Extensions/Callout/CalloutComponent.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/Extensions/Callout/CalloutComponent.test.tsx @@ -32,6 +32,9 @@ const mockNodeViewProps = { node: mockNode, extension: mockExtension, updateAttributes: mockUpdateAttributes, + editor: { + isEditable: true, + }, } as unknown as NodeViewProps; describe('CalloutComponent', () => { @@ -70,4 +73,29 @@ describe('CalloutComponent', () => { expect(screen.getByTestId('callout-note')).toBeInTheDocument(); expect(screen.getByTestId('callout-danger')).toBeInTheDocument(); }); + + it('should not render the popover when callout button is clicked and editor is not editable', async () => { + const nodeViewProps = { + node: mockNode, + extension: mockExtension, + updateAttributes: mockUpdateAttributes, + editor: { + isEditable: false, + }, + } as unknown as NodeViewProps; + + await act(async () => { + render(); + }); + + const calloutButton = screen.getByTestId('callout-info-btn'); + + await act(async () => { + userEvent.click(calloutButton); + }); + + const popover = screen.queryByRole('tooltip'); + + expect(popover).not.toBeInTheDocument(); + }); }); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/Extensions/Callout/CalloutComponent.tsx b/openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/Extensions/Callout/CalloutComponent.tsx index 71a463a46e4e..4f20840c4b77 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/Extensions/Callout/CalloutComponent.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/BlockEditor/Extensions/Callout/CalloutComponent.tsx @@ -45,12 +45,18 @@ const CalloutComponent: FC = ({ node, extension, updateAttributes, + editor, }) => { const { calloutType } = node.attrs; const [isPopupVisible, setIsPopupVisible] = useState(false); const CallOutIcon = CALLOUT_CONTENT[calloutType as keyof typeof CALLOUT_CONTENT]; + const handlePopoverVisibleChange = (visible: boolean) => { + // Only show the popover when the editor is in editable mode + setIsPopupVisible(visible && editor.isEditable); + }; + return (
= ({ placement="bottomRight" showArrow={false} trigger="click" - onOpenChange={setIsPopupVisible}> + onOpenChange={handlePopoverVisibleChange}> + + + + + ); +}; + +export default AddTestSuitePipeline; diff --git a/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddDataQualityTest/components/add-test-suite-pipeline.style.less b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddDataQualityTest/components/add-test-suite-pipeline.style.less new file mode 100644 index 000000000000..0bc5ce68c436 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddDataQualityTest/components/add-test-suite-pipeline.style.less @@ -0,0 +1,23 @@ +/* + * Copyright 2024 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +@import (reference) url('../../../../styles/variables.less'); + +.add-test-case-container { + padding: 16px; + border-radius: @border-radius-base; + background-color: @grey-1; + + .form-item-horizontal { + margin-bottom: 0px; + } +} diff --git a/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddDataQualityTest/rightPanelData.ts b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddDataQualityTest/rightPanelData.ts index 31c0155b3d92..c6ee9c55de42 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddDataQualityTest/rightPanelData.ts +++ b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddDataQualityTest/rightPanelData.ts @@ -52,15 +52,14 @@ export const INGESTION_DATA = { body: i18n.t('message.test-case-schedule-description'), }; -export const TEST_SUITE_INGESTION_PAGE_DATA = [ - { - title: i18n.t('label.select-field', { - field: i18n.t('label.test-case-plural'), - }), - body: i18n.t('message.select-test-case'), - }, - INGESTION_DATA, -]; +export const TEST_SUITE_INGESTION_PAGE_DATA = { + title: i18n.t('label.schedule-for-entity', { + entity: i18n.t('label.test-case-plural'), + }), + body: `${i18n.t('message.test-case-schedule-description')} & ${i18n.t( + 'message.select-test-case' + )}`, +}; export const addTestSuiteRightPanel = ( step: number, diff --git a/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddTestCaseList/AddTestCaseList.component.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddTestCaseList/AddTestCaseList.component.test.tsx new file mode 100644 index 000000000000..0b7b53d728b2 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddTestCaseList/AddTestCaseList.component.test.tsx @@ -0,0 +1,122 @@ +/* + * Copyright 2024 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { act, fireEvent, render, screen } from '@testing-library/react'; +import React from 'react'; +import { EntityReference } from '../../../generated/tests/testCase'; +import { AddTestCaseList } from './AddTestCaseList.component'; +import { AddTestCaseModalProps } from './AddTestCaseList.interface'; + +jest.mock('../../common/ErrorWithPlaceholder/ErrorPlaceHolder', () => { + return jest.fn().mockImplementation(() =>
Error Placeholder Mock
); +}); + +jest.mock('../../common/Loader/Loader', () => { + return jest.fn().mockImplementation(() =>
Loader Mock
); +}); + +jest.mock('../../common/SearchBarComponent/SearchBar.component', () => { + return jest.fn().mockImplementation(() =>
Search Bar Mock
); +}); +jest.mock('../../../utils/StringsUtils', () => { + return { + replacePlus: jest.fn().mockImplementation((fqn) => fqn), + }; +}); +jest.mock('../../../utils/FeedUtils', () => { + return { + getEntityFQN: jest.fn().mockImplementation((fqn) => fqn), + }; +}); +jest.mock('../../../utils/EntityUtils', () => { + return { + getEntityName: jest + .fn() + .mockImplementation( + (entity: EntityReference) => entity?.displayName ?? entity?.name + ), + getColumnNameFromEntityLink: jest.fn().mockImplementation((fqn) => fqn), + }; +}); +jest.mock('../../../utils/CommonUtils', () => { + return { + getNameFromFQN: jest.fn().mockImplementation((fqn) => fqn), + }; +}); +jest.mock('../../../rest/searchAPI', () => { + return { + searchQuery: jest.fn().mockResolvedValue({ + hits: { + hits: [], + total: { + value: 0, + }, + }, + }), + }; +}); + +jest.mock('../../../constants/constants', () => ({ + getEntityDetailsPath: jest.fn(), + PAGE_SIZE_MEDIUM: 25, +})); + +const mockProps: AddTestCaseModalProps = { + onCancel: jest.fn(), + onSubmit: jest.fn(), + cancelText: 'Cancel', + submitText: 'Submit', + selectedTest: [], + onChange: jest.fn(), + showButton: true, +}; + +describe('AddTestCaseList', () => { + it('renders the component', async () => { + await act(async () => { + render(); + }); + + expect(screen.getByText('Search Bar Mock')).toBeInTheDocument(); + expect(screen.getByTestId('cancel')).toBeInTheDocument(); + expect(screen.getByTestId('submit')).toBeInTheDocument(); + }); + + it('calls onCancel when cancel button is clicked', async () => { + await act(async () => { + render(); + }); + fireEvent.click(screen.getByTestId('cancel')); + + expect(mockProps.onCancel).toHaveBeenCalled(); + }); + + it('calls onSubmit when submit button is clicked', async () => { + await act(async () => { + render(); + }); + await act(async () => { + await fireEvent.click(screen.getByTestId('submit')); + }); + + expect(mockProps.onSubmit).toHaveBeenCalledWith([]); + }); + + it('does not render submit and cancel buttons when showButton is false', async () => { + await act(async () => { + render(); + }); + + expect(screen.queryByTestId('cancel')).toBeNull(); + expect(screen.queryByTestId('submit')).toBeNull(); + }); +}); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddTestCaseList/AddTestCaseList.component.tsx b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddTestCaseList/AddTestCaseList.component.tsx index 1db8330ca6e7..e811e57204c6 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddTestCaseList/AddTestCaseList.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddTestCaseList/AddTestCaseList.component.tsx @@ -51,6 +51,8 @@ export const AddTestCaseList = ({ submitText, filters, selectedTest, + onChange, + showButton = true, }: AddTestCaseModalProps) => { const { t } = useTranslation(); const [searchTerm, setSearchTerm] = useState(); @@ -104,7 +106,7 @@ export const AddTestCaseList = ({ const handleSubmit = async () => { setIsLoading(true); const testCaseIds = [...(selectedItems?.values() ?? [])]; - onSubmit(testCaseIds); + onSubmit?.(testCaseIds); setIsLoading(false); }; @@ -137,6 +139,9 @@ export const AddTestCaseList = ({ (item) => item.id !== id && selectedItemMap.set(item.id, item) ); + const testCases = [...(selectedItemMap?.values() ?? [])]; + onChange?.(testCases); + return selectedItemMap; }); } else { @@ -149,6 +154,8 @@ export const AddTestCaseList = ({ id, items.find((test) => test.id === id) ); + const testCases = [...(selectedItemMap?.values() ?? [])]; + onChange?.(testCases); return selectedItemMap; }); @@ -190,7 +197,7 @@ export const AddTestCaseList = ({ return ( handleCardClick(test)}> @@ -260,18 +267,22 @@ export const AddTestCaseList = ({ /> {renderList} - - - - + {showButton && ( + + + + + )} ); }; diff --git a/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddTestCaseList/AddTestCaseList.interface.ts b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddTestCaseList/AddTestCaseList.interface.ts index 3aa373f18253..421b5054d108 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddTestCaseList/AddTestCaseList.interface.ts +++ b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddTestCaseList/AddTestCaseList.interface.ts @@ -14,10 +14,12 @@ import { EntityReference, TestCase } from '../../../generated/tests/testCase'; export interface AddTestCaseModalProps { onCancel?: () => void; - onSubmit: (testCases: TestCase[]) => void; + onSubmit?: (testCases: TestCase[]) => void; + onChange?: (testCases: TestCase[]) => void; existingTest?: EntityReference[]; cancelText?: string; submitText?: string; filters?: string; selectedTest?: string[]; + showButton?: boolean; } diff --git a/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/TestSuite/TestSuitePipelineTab/TestSuitePipelineTab.component.tsx b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/TestSuite/TestSuitePipelineTab/TestSuitePipelineTab.component.tsx index c32be5d2ee01..90ec7b8365a8 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/TestSuite/TestSuitePipelineTab/TestSuitePipelineTab.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/TestSuite/TestSuitePipelineTab/TestSuitePipelineTab.component.tsx @@ -16,6 +16,7 @@ import { Button, Col, Divider, Row, Space, Tooltip } from 'antd'; import { ColumnsType } from 'antd/lib/table'; import { AxiosError } from 'axios'; import cronstrue from 'cronstrue'; +import { sortBy } from 'lodash'; import React, { Fragment, useEffect, useMemo, useState } from 'react'; import { useTranslation } from 'react-i18next'; import { Link, useHistory } from 'react-router-dom'; @@ -576,7 +577,16 @@ const TestSuitePipelineTab = ({ testSuite }: Props) => { ); const dataSource = useMemo(() => { - return testSuitePipelines.map((test) => ({ + const sortedByTestCaseLength = sortBy(testSuitePipelines, (pipeline) => { + const length = pipeline?.sourceConfig?.config?.testCases?.length; + if (!length) { + return -Infinity; // Use -Infinity to ensure these come first + } + + return length; + }); + + return sortedByTestCaseLength.map((test) => ({ ...test, key: test.name, })); diff --git a/openmetadata-ui/src/main/resources/ui/src/interface/FormUtils.interface.ts b/openmetadata-ui/src/main/resources/ui/src/interface/FormUtils.interface.ts index 855b30a066a2..8b0258c4f005 100644 --- a/openmetadata-ui/src/main/resources/ui/src/interface/FormUtils.interface.ts +++ b/openmetadata-ui/src/main/resources/ui/src/interface/FormUtils.interface.ts @@ -39,6 +39,7 @@ export enum FieldTypes { USER_TEAM_SELECT = 'user_team_select', USER_MULTI_SELECT = 'user_multi_select', COLOR_PICKER = 'color_picker', + CRON_EDITOR = 'cron_editor', } export enum HelperTextType { diff --git a/openmetadata-ui/src/main/resources/ui/src/locale/languages/de-de.json b/openmetadata-ui/src/main/resources/ui/src/locale/languages/de-de.json index 3030f94c6664..c1013f493b0d 100644 --- a/openmetadata-ui/src/main/resources/ui/src/locale/languages/de-de.json +++ b/openmetadata-ui/src/main/resources/ui/src/locale/languages/de-de.json @@ -1011,6 +1011,7 @@ "select-add-test-suite": "Auswahl/Hinzufügen einer Test-Suite", "select-column-plural-to-exclude": "Spalten zum Ausschließen auswählen", "select-column-plural-to-include": "Spalten zum Einschließen auswählen", + "select-entity": "Select {{entity}}", "select-field": "{{field}}-Feld auswählen", "select-to-search": "Zum Suchen auswählen", "select-type": "Ausgewählter Typ", diff --git a/openmetadata-ui/src/main/resources/ui/src/locale/languages/en-us.json b/openmetadata-ui/src/main/resources/ui/src/locale/languages/en-us.json index a62c865ee325..75431b7a8fd7 100644 --- a/openmetadata-ui/src/main/resources/ui/src/locale/languages/en-us.json +++ b/openmetadata-ui/src/main/resources/ui/src/locale/languages/en-us.json @@ -1011,6 +1011,7 @@ "select-add-test-suite": "Select/Add Test Suite", "select-column-plural-to-exclude": "Select Columns to Exclude", "select-column-plural-to-include": "Select Columns to Include", + "select-entity": "Select {{entity}}", "select-field": "Select {{field}}", "select-to-search": "Search to Select", "select-type": "Select type", diff --git a/openmetadata-ui/src/main/resources/ui/src/locale/languages/es-es.json b/openmetadata-ui/src/main/resources/ui/src/locale/languages/es-es.json index fdbb0af042b7..4757e7d1d7b0 100644 --- a/openmetadata-ui/src/main/resources/ui/src/locale/languages/es-es.json +++ b/openmetadata-ui/src/main/resources/ui/src/locale/languages/es-es.json @@ -1011,6 +1011,7 @@ "select-add-test-suite": "Seleccionar/Agregar Suite de Pruebas", "select-column-plural-to-exclude": "Seleccionar Columnas para Excluir", "select-column-plural-to-include": "Seleccionar Columnas para Incluir", + "select-entity": "Select {{entity}}", "select-field": "Seleccionar {{field}}", "select-to-search": "Buscar para Seleccionar", "select-type": "Seleccionar tipo", diff --git a/openmetadata-ui/src/main/resources/ui/src/locale/languages/fr-fr.json b/openmetadata-ui/src/main/resources/ui/src/locale/languages/fr-fr.json index ee50b2019145..8818ce7115f0 100644 --- a/openmetadata-ui/src/main/resources/ui/src/locale/languages/fr-fr.json +++ b/openmetadata-ui/src/main/resources/ui/src/locale/languages/fr-fr.json @@ -1011,6 +1011,7 @@ "select-add-test-suite": "Sélectionner/Ajouter un Ensemble de Tests", "select-column-plural-to-exclude": "Sélectionner les Colonnes à Exclure", "select-column-plural-to-include": "Sélectionner les Colonnes à Inclure", + "select-entity": "Select {{entity}}", "select-field": "Sélectionner le Champ {{field}}", "select-to-search": "Sélectionner pour Rechercher", "select-type": "Type Sélectionné", diff --git a/openmetadata-ui/src/main/resources/ui/src/locale/languages/he-he.json b/openmetadata-ui/src/main/resources/ui/src/locale/languages/he-he.json index 6d71c28b2d2c..fc112f56850f 100644 --- a/openmetadata-ui/src/main/resources/ui/src/locale/languages/he-he.json +++ b/openmetadata-ui/src/main/resources/ui/src/locale/languages/he-he.json @@ -1011,6 +1011,7 @@ "select-add-test-suite": "בחר/הוסף יחידת בדיקה", "select-column-plural-to-exclude": "בחר עמודות לא לכלול", "select-column-plural-to-include": "בחר עמודות לכלול", + "select-entity": "Select {{entity}}", "select-field": "בחר {{field}}", "select-to-search": "חפש כדי לבחור", "select-type": "בחר סוג", diff --git a/openmetadata-ui/src/main/resources/ui/src/locale/languages/ja-jp.json b/openmetadata-ui/src/main/resources/ui/src/locale/languages/ja-jp.json index 6186e6f60592..a1f4b099c8c1 100644 --- a/openmetadata-ui/src/main/resources/ui/src/locale/languages/ja-jp.json +++ b/openmetadata-ui/src/main/resources/ui/src/locale/languages/ja-jp.json @@ -1011,6 +1011,7 @@ "select-add-test-suite": "Select/Add Test Suite", "select-column-plural-to-exclude": "除外するカラムを選択", "select-column-plural-to-include": "含めるカラムを選択", + "select-entity": "Select {{entity}}", "select-field": "{{field}}を選択", "select-to-search": "Search to Select", "select-type": "Select type", diff --git a/openmetadata-ui/src/main/resources/ui/src/locale/languages/nl-nl.json b/openmetadata-ui/src/main/resources/ui/src/locale/languages/nl-nl.json index 575808d8e51b..d3074701f3f9 100644 --- a/openmetadata-ui/src/main/resources/ui/src/locale/languages/nl-nl.json +++ b/openmetadata-ui/src/main/resources/ui/src/locale/languages/nl-nl.json @@ -1011,6 +1011,7 @@ "select-add-test-suite": "Selecteer/voeg testsuite toe", "select-column-plural-to-exclude": "Selecteer kolommen om uit te sluiten", "select-column-plural-to-include": "Selecteer kolommen om op te nemen", + "select-entity": "Select {{entity}}", "select-field": "Selecteer {{field}}", "select-to-search": "Zoeken om te selecteren", "select-type": "Selecteer type", diff --git a/openmetadata-ui/src/main/resources/ui/src/locale/languages/pt-br.json b/openmetadata-ui/src/main/resources/ui/src/locale/languages/pt-br.json index eedf4a73fd8b..9f7e09a13b35 100644 --- a/openmetadata-ui/src/main/resources/ui/src/locale/languages/pt-br.json +++ b/openmetadata-ui/src/main/resources/ui/src/locale/languages/pt-br.json @@ -1011,6 +1011,7 @@ "select-add-test-suite": "Selecionar/Adicionar Conjunto de Testes", "select-column-plural-to-exclude": "Selecionar Colunas para Excluir", "select-column-plural-to-include": "Selecionar Colunas para Incluir", + "select-entity": "Select {{entity}}", "select-field": "Selecionar {{field}}", "select-to-search": "Pesquisar para Selecionar", "select-type": "Selecionar tipo", diff --git a/openmetadata-ui/src/main/resources/ui/src/locale/languages/ru-ru.json b/openmetadata-ui/src/main/resources/ui/src/locale/languages/ru-ru.json index e9ed6cd8cc7f..8d9e7a32da1f 100644 --- a/openmetadata-ui/src/main/resources/ui/src/locale/languages/ru-ru.json +++ b/openmetadata-ui/src/main/resources/ui/src/locale/languages/ru-ru.json @@ -1011,6 +1011,7 @@ "select-add-test-suite": "Выбрать/добавить набор тестов", "select-column-plural-to-exclude": "Выберите столбцы для исключения", "select-column-plural-to-include": "Выберите столбцы для исключения", + "select-entity": "Select {{entity}}", "select-field": "Выбрать {{field}}", "select-to-search": "Поиск для выбора", "select-type": "Выбрать тип", diff --git a/openmetadata-ui/src/main/resources/ui/src/locale/languages/zh-cn.json b/openmetadata-ui/src/main/resources/ui/src/locale/languages/zh-cn.json index 191ca747e24e..46870241d3fc 100644 --- a/openmetadata-ui/src/main/resources/ui/src/locale/languages/zh-cn.json +++ b/openmetadata-ui/src/main/resources/ui/src/locale/languages/zh-cn.json @@ -1011,6 +1011,7 @@ "select-add-test-suite": "选择/添加质控测试", "select-column-plural-to-exclude": "选择要排除的列", "select-column-plural-to-include": "选择要包含的列", + "select-entity": "Select {{entity}}", "select-field": "选择{{field}}", "select-to-search": "搜索以选择", "select-type": "选择类型", diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/TestSuiteIngestionPage/TestSuiteIngestionPage.test.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/TestSuiteIngestionPage/TestSuiteIngestionPage.test.tsx index 52cc9c297812..638b72acec00 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/TestSuiteIngestionPage/TestSuiteIngestionPage.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/TestSuiteIngestionPage/TestSuiteIngestionPage.test.tsx @@ -74,6 +74,9 @@ jest.mock('../../components/common/ResizablePanels/ResizablePanels', () => jest.mock('../../components/common/ErrorWithPlaceholder/ErrorPlaceHolder', () => jest.fn().mockReturnValue(
ErrorPlaceHolder.component
) ); +jest.mock('../../components/common/Loader/Loader', () => + jest.fn().mockReturnValue(
Loader.component
) +); jest.mock( '../../components/common/TitleBreadcrumb/TitleBreadcrumb.component', () => jest.fn().mockReturnValue(
TitleBreadcrumb.component
) @@ -90,22 +93,6 @@ jest.mock(
)) ); -jest.mock( - '../../components/DataQuality/AddTestCaseList/AddTestCaseList.component', - () => ({ - AddTestCaseList: jest.fn().mockImplementation(({ onSubmit, onCancel }) => ( -
-

AddTestCaseList.component

- - -
- )), - }) -); jest.mock( '../../components/DataQuality/AddDataQualityTest/components/RightPanel', () => jest.fn().mockReturnValue(
RightPanel.component
) @@ -120,13 +107,29 @@ describe('TestSuiteIngestionPage', () => { expect( await screen.findByText('TitleBreadcrumb.component') ).toBeInTheDocument(); - expect(await screen.findByTestId('pipeline-name')).toBeInTheDocument(); - expect( - await screen.findByText('AddTestCaseList.component') - ).toBeInTheDocument(); expect(await screen.findByText('RightPanel.component')).toBeInTheDocument(); }); + it('should render loading state', async () => { + jest.useFakeTimers(); + (getTestSuiteByName as jest.Mock).mockImplementationOnce( + () => + new Promise((resolve) => setTimeout(() => resolve(mockTestSuite), 1000)) + ); + + await act(async () => { + render(); + }); + + expect(screen.getByText('Loader.component')).toBeInTheDocument(); + + await act(async () => { + await jest.runAllTimers(); + }); + + expect(screen.queryByText('Loader.component')).not.toBeInTheDocument(); + }); + it('should render error placeholder', async () => { (getTestSuiteByName as jest.Mock).mockImplementationOnce(() => Promise.reject(new Error('Error')) @@ -154,15 +157,4 @@ describe('TestSuiteIngestionPage', () => { expect(getIngestionPipelineByFqn).toHaveBeenCalledWith('ingestionFQN'); }); - - it('should go back on back button click', async () => { - await act(async () => { - render(); - }); - - const backButton = screen.getByTestId('back-btn'); - backButton.click(); - - expect(mockUseHistory.goBack).toHaveBeenCalled(); - }); }); diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/TestSuiteIngestionPage/TestSuiteIngestionPage.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/TestSuiteIngestionPage/TestSuiteIngestionPage.tsx index bc0557f4f560..9fcc0fa4dfd1 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/TestSuiteIngestionPage/TestSuiteIngestionPage.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/TestSuiteIngestionPage/TestSuiteIngestionPage.tsx @@ -10,12 +10,10 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { Form, Input } from 'antd'; import { AxiosError } from 'axios'; -import { isUndefined, uniq } from 'lodash'; +import { isUndefined } from 'lodash'; import React, { useEffect, useState } from 'react'; import { useTranslation } from 'react-i18next'; -import { useHistory } from 'react-router-dom'; import ErrorPlaceHolder from '../../components/common/ErrorWithPlaceholder/ErrorPlaceHolder'; import Loader from '../../components/common/Loader/Loader'; import ResizablePanels from '../../components/common/ResizablePanels/ResizablePanels'; @@ -24,13 +22,9 @@ import { TitleBreadcrumbProps } from '../../components/common/TitleBreadcrumb/Ti import RightPanel from '../../components/DataQuality/AddDataQualityTest/components/RightPanel'; import { TEST_SUITE_INGESTION_PAGE_DATA } from '../../components/DataQuality/AddDataQualityTest/rightPanelData'; import TestSuiteIngestion from '../../components/DataQuality/AddDataQualityTest/TestSuiteIngestion'; -import { AddTestCaseList } from '../../components/DataQuality/AddTestCaseList/AddTestCaseList.component'; -import IngestionStepper from '../../components/Settings/Services/Ingestion/IngestionStepper/IngestionStepper.component'; import { getEntityDetailsPath } from '../../constants/constants'; -import { STEPS_FOR_ADD_TEST_SUITE_PIPELINE } from '../../constants/TestSuite.constant'; import { EntityTabs, EntityType } from '../../enums/entity.enum'; import { IngestionPipeline } from '../../generated/entity/services/ingestionPipelines/ingestionPipeline'; -import { TestCase } from '../../generated/tests/testCase'; import { TestSuite } from '../../generated/tests/testSuite'; import { useFqn } from '../../hooks/useFqn'; import { getIngestionPipelineByFqn } from '../../rest/ingestionPipelineAPI'; @@ -43,7 +37,6 @@ const TestSuiteIngestionPage = () => { const { fqn: testSuiteFQN, ingestionFQN } = useFqn(); const { t } = useTranslation(); - const history = useHistory(); const [isLoading, setIsLoading] = useState(true); const [testSuite, setTestSuite] = useState(); const [ingestionPipeline, setIngestionPipeline] = @@ -51,16 +44,11 @@ const TestSuiteIngestionPage = () => { const [slashedBreadCrumb, setSlashedBreadCrumb] = useState< TitleBreadcrumbProps['titleLinks'] >([]); - const [activeServiceStep, setActiveServiceStep] = useState(1); - const [testCases, setTestCases] = useState([]); - const [pipelineName, setPipelineName] = useState(); const fetchIngestionByName = async () => { setIsLoading(true); try { const response = await getIngestionPipelineByFqn(ingestionFQN); - setTestCases(response.sourceConfig.config?.testCases ?? []); - setPipelineName(response.displayName); setIngestionPipeline(response); } catch (error) { showErrorToast( @@ -118,20 +106,6 @@ const TestSuiteIngestionPage = () => { } }; - const handleAddTestSubmit = (testCases: TestCase[]) => { - const testCaseNames = testCases.map((testCase) => testCase.name); - setTestCases(uniq(testCaseNames)); - setActiveServiceStep(2); - }; - - const onNameChange = (e: React.ChangeEvent) => { - setPipelineName(() => e.target.value); - }; - - const handleCancelBtn = () => { - history.goBack(); - }; - useEffect(() => { fetchTestSuiteByName(); }, []); @@ -153,46 +127,11 @@ const TestSuiteIngestionPage = () => {
- -
- {activeServiceStep === 1 && ( -
- - - - - - -
- )} - {activeServiceStep === 2 && ( - setActiveServiceStep(1)} - /> - )} -
), @@ -203,11 +142,7 @@ const TestSuiteIngestionPage = () => { entity: t('label.test-suite'), })} secondPanel={{ - children: ( - - ), + children: , className: 'p-md p-t-xl content-resizable-panel-container', minWidth: 400, flex: 0.3, diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/formUtils.tsx b/openmetadata-ui/src/main/resources/ui/src/utils/formUtils.tsx index 2f5b091f2bfd..9d6cbf141a00 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/formUtils.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/utils/formUtils.tsx @@ -32,6 +32,8 @@ import React, { Fragment, ReactNode } from 'react'; import AsyncSelectList from '../components/common/AsyncSelectList/AsyncSelectList'; import { AsyncSelectListProps } from '../components/common/AsyncSelectList/AsyncSelectList.interface'; import ColorPicker from '../components/common/ColorPicker/ColorPicker.component'; +import CronEditor from '../components/common/CronEditor/CronEditor'; +import { CronEditorProp } from '../components/common/CronEditor/CronEditor.interface'; import FilterPattern from '../components/common/FilterPattern/FilterPattern'; import { FilterPatternProps } from '../components/common/FilterPattern/filterPattern.interface'; import FormItemLabel from '../components/common/Form/FormItemLabel'; @@ -199,6 +201,10 @@ export const getField = (field: FieldProp) => { case FieldTypes.COLOR_PICKER: fieldElement = ; + break; + case FieldTypes.CRON_EDITOR: + fieldElement = ; + break; default: From eafa6b8772103a5bfa49e76cdcb55e6d69a36733 Mon Sep 17 00:00:00 2001 From: Onkar Ravgan Date: Mon, 22 Jul 2024 11:17:28 +0530 Subject: [PATCH 33/33] Added domo federated dataset support (#17061) --- .../source/database/domodatabase/metadata.py | 67 ++++++++++++++----- .../source/database/domodatabase/models.py | 1 + 2 files changed, 52 insertions(+), 16 deletions(-) diff --git a/ingestion/src/metadata/ingestion/source/database/domodatabase/metadata.py b/ingestion/src/metadata/ingestion/source/database/domodatabase/metadata.py index 4cb00ebf1582..164305e5f1c6 100644 --- a/ingestion/src/metadata/ingestion/source/database/domodatabase/metadata.py +++ b/ingestion/src/metadata/ingestion/source/database/domodatabase/metadata.py @@ -14,7 +14,7 @@ """ import traceback -from typing import Any, Iterable, List, Optional, Tuple, Union +from typing import Any, Iterable, Optional, Tuple, Union from metadata.generated.schema.api.data.createDatabase import CreateDatabaseRequest from metadata.generated.schema.api.data.createDatabaseSchema import ( @@ -57,6 +57,7 @@ from metadata.ingestion.source.database.domodatabase.models import ( OutputDataset, Owner, + Schema, SchemaColumn, User, ) @@ -191,11 +192,7 @@ def yield_table( try: table_constraints = None table_object = OutputDataset(**self.domo_client.datasets.get(table_id)) - columns = ( - self.get_columns(table_object.schemas.columns) - if table_object.columns - else [] - ) + columns = self.get_columns(table_object) table_request = CreateTableRequest( name=EntityName(table_object.name), displayName=table_object.name, @@ -228,19 +225,57 @@ def yield_table( ) ) - def get_columns(self, table_object: List[SchemaColumn]): + def get_columns_from_federated_dataset(self, table_name: str, dataset_id: str): + """ + Method to retrieve the column metadata from federated datasets + """ + try: + # SQL query to get all columns without fetching any rows + sql_query = f'SELECT * FROM "{table_name}" LIMIT 1' + schema_columns = [] + response = self.domo_client.datasets.query(dataset_id, sql_query) + if response: + for i, column_name in enumerate(response["columns"] or []): + schema_column = SchemaColumn( + name=column_name, type=response["metadata"][i]["type"] + ) + schema_columns.append(schema_column) + if schema_columns: + return Schema(columns=schema_columns) + except Exception as exc: + logger.debug(traceback.format_exc()) + logger.warning( + f"Error while fetching columns from federated dataset {table_name} - {exc}" + ) + return None + + def get_columns(self, table_object: OutputDataset): + """ + Method to get domo table columns + """ row_order = 1 columns = [] - for column in table_object: - columns.append( - Column( - name=ColumnName(column.name), - description=column.description, - dataType=column.type, - ordinalPosition=row_order, - ) + if not table_object.schemas or not table_object.schemas.columns: + table_object.schemas = self.get_columns_from_federated_dataset( + table_name=table_object.name, dataset_id=table_object.id ) - row_order += 1 + + for column in table_object.schemas.columns or []: + try: + columns.append( + Column( + name=ColumnName(column.name), + description=column.description, + dataType=column.type, + ordinalPosition=row_order, + ) + ) + row_order += 1 + except Exception as exc: + logger.debug(traceback.format_exc()) + logger.warning( + f"Error while fetching details of column {column} - {exc}" + ) return columns def yield_tag( diff --git a/ingestion/src/metadata/ingestion/source/database/domodatabase/models.py b/ingestion/src/metadata/ingestion/source/database/domodatabase/models.py index ef4ce4e968df..f350cceeb01c 100644 --- a/ingestion/src/metadata/ingestion/source/database/domodatabase/models.py +++ b/ingestion/src/metadata/ingestion/source/database/domodatabase/models.py @@ -26,6 +26,7 @@ class DomoDatabaseBaseModel(BaseModel): class User(DomoDatabaseBaseModel): + id: int email: str role: str