diff --git a/.gitignore b/.gitignore index da34d1d0262b..3cfd1a88cc38 100644 --- a/.gitignore +++ b/.gitignore @@ -197,5 +197,5 @@ ingestion/.claude/agents # Connector audit working files — per-session, never committed .claude/audit-results/ .claude/connector-audit.json - +.claude/scheduled_tasks.lock test-results/ diff --git a/conf/openmetadata.yaml b/conf/openmetadata.yaml index 926e6cba01d3..670d97271b48 100644 --- a/conf/openmetadata.yaml +++ b/conf/openmetadata.yaml @@ -419,14 +419,14 @@ authenticationConfiguration: spPrivateKey: ${SAML_SP_PRIVATE_KEY:-""} callback: ${SAML_SP_CALLBACK:-"http://localhost:8585/saml/callback"} security: - strictMode: ${SAML_STRICT_MODE:-false} + strictMode: ${SAML_STRICT_MODE:-true} validateXml: ${SAML_VALIDATE_XML:-false} tokenValidity: ${SAML_SP_TOKEN_VALIDITY:-"3600"} sendEncryptedNameId: ${SAML_SEND_ENCRYPTED_NAME_ID:-false} sendSignedAuthRequest: ${SAML_SEND_SIGNED_AUTH_REQUEST:-false} signSpMetadata: ${SAML_SIGNED_SP_METADATA:-false} - wantMessagesSigned: ${SAML_WANT_MESSAGE_SIGNED:-false} - wantAssertionsSigned: ${SAML_WANT_ASSERTION_SIGNED:-false} + wantMessagesSigned: ${SAML_WANT_MESSAGE_SIGNED:-true} + wantAssertionsSigned: ${SAML_WANT_ASSERTION_SIGNED:-true} wantAssertionEncrypted: ${SAML_WANT_ASSERTION_ENCRYPTED:-false} keyStoreFilePath: ${SAML_KEYSTORE_FILE_PATH:-""} keyStoreAlias: ${SAML_KEYSTORE_ALIAS:-""} diff --git a/docker/development/distributed-test/local/server1.yaml b/docker/development/distributed-test/local/server1.yaml index d2bd886791a2..cfc90ab706d1 100644 --- a/docker/development/distributed-test/local/server1.yaml +++ b/docker/development/distributed-test/local/server1.yaml @@ -303,14 +303,14 @@ authenticationConfiguration: spPrivateKey: ${SAML_SP_PRIVATE_KEY:-""} callback: ${SAML_SP_CALLBACK:-"http://localhost:8585/saml/callback"} security: - strictMode: ${SAML_STRICT_MODE:-false} + strictMode: ${SAML_STRICT_MODE:-true} validateXml: ${SAML_VALIDATE_XML:-false} tokenValidity: ${SAML_SP_TOKEN_VALIDITY:-"3600"} sendEncryptedNameId: ${SAML_SEND_ENCRYPTED_NAME_ID:-false} sendSignedAuthRequest: ${SAML_SEND_SIGNED_AUTH_REQUEST:-false} signSpMetadata: ${SAML_SIGNED_SP_METADATA:-false} - wantMessagesSigned: ${SAML_WANT_MESSAGE_SIGNED:-false} - wantAssertionsSigned: ${SAML_WANT_ASSERTION_SIGNED:-false} + wantMessagesSigned: ${SAML_WANT_MESSAGE_SIGNED:-true} + wantAssertionsSigned: ${SAML_WANT_ASSERTION_SIGNED:-true} wantAssertionEncrypted: ${SAML_WANT_ASSERTION_ENCRYPTED:-false} keyStoreFilePath: ${SAML_KEYSTORE_FILE_PATH:-""} keyStoreAlias: ${SAML_KEYSTORE_ALIAS:-""} diff --git a/docker/development/distributed-test/local/server2.yaml b/docker/development/distributed-test/local/server2.yaml index ef1f1e00010c..fc36ee3a2707 100644 --- a/docker/development/distributed-test/local/server2.yaml +++ b/docker/development/distributed-test/local/server2.yaml @@ -303,14 +303,14 @@ authenticationConfiguration: spPrivateKey: ${SAML_SP_PRIVATE_KEY:-""} callback: ${SAML_SP_CALLBACK:-"http://localhost:8585/saml/callback"} security: - strictMode: ${SAML_STRICT_MODE:-false} + strictMode: ${SAML_STRICT_MODE:-true} validateXml: ${SAML_VALIDATE_XML:-false} tokenValidity: ${SAML_SP_TOKEN_VALIDITY:-"3600"} sendEncryptedNameId: ${SAML_SEND_ENCRYPTED_NAME_ID:-false} sendSignedAuthRequest: ${SAML_SEND_SIGNED_AUTH_REQUEST:-false} signSpMetadata: ${SAML_SIGNED_SP_METADATA:-false} - wantMessagesSigned: ${SAML_WANT_MESSAGE_SIGNED:-false} - wantAssertionsSigned: ${SAML_WANT_ASSERTION_SIGNED:-false} + wantMessagesSigned: ${SAML_WANT_MESSAGE_SIGNED:-true} + wantAssertionsSigned: ${SAML_WANT_ASSERTION_SIGNED:-true} wantAssertionEncrypted: ${SAML_WANT_ASSERTION_ENCRYPTED:-false} keyStoreFilePath: ${SAML_KEYSTORE_FILE_PATH:-""} keyStoreAlias: ${SAML_KEYSTORE_ALIAS:-""} diff --git a/docker/development/distributed-test/local/server3.yaml b/docker/development/distributed-test/local/server3.yaml index c80666349d65..20093387e6dd 100644 --- a/docker/development/distributed-test/local/server3.yaml +++ b/docker/development/distributed-test/local/server3.yaml @@ -304,14 +304,14 @@ authenticationConfiguration: spPrivateKey: ${SAML_SP_PRIVATE_KEY:-""} callback: ${SAML_SP_CALLBACK:-"http://localhost:8585/saml/callback"} security: - strictMode: ${SAML_STRICT_MODE:-false} + strictMode: ${SAML_STRICT_MODE:-true} validateXml: ${SAML_VALIDATE_XML:-false} tokenValidity: ${SAML_SP_TOKEN_VALIDITY:-"3600"} sendEncryptedNameId: ${SAML_SEND_ENCRYPTED_NAME_ID:-false} sendSignedAuthRequest: ${SAML_SEND_SIGNED_AUTH_REQUEST:-false} signSpMetadata: ${SAML_SIGNED_SP_METADATA:-false} - wantMessagesSigned: ${SAML_WANT_MESSAGE_SIGNED:-false} - wantAssertionsSigned: ${SAML_WANT_ASSERTION_SIGNED:-false} + wantMessagesSigned: ${SAML_WANT_MESSAGE_SIGNED:-true} + wantAssertionsSigned: ${SAML_WANT_ASSERTION_SIGNED:-true} wantAssertionEncrypted: ${SAML_WANT_ASSERTION_ENCRYPTED:-false} keyStoreFilePath: ${SAML_KEYSTORE_FILE_PATH:-""} keyStoreAlias: ${SAML_KEYSTORE_ALIAS:-""} diff --git a/ingestion/tests/integration/fivetran/__init__.py b/ingestion/tests/integration/fivetran/__init__.py new file mode 100644 index 000000000000..8b137891791f --- /dev/null +++ b/ingestion/tests/integration/fivetran/__init__.py @@ -0,0 +1 @@ + diff --git a/ingestion/tests/integration/fivetran/conftest.py b/ingestion/tests/integration/fivetran/conftest.py new file mode 100644 index 000000000000..9629d2d9cbb4 --- /dev/null +++ b/ingestion/tests/integration/fivetran/conftest.py @@ -0,0 +1,356 @@ +# Copyright 2025 Collate +# Licensed under the Collate Community License, Version 1.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# https://github.com/open-metadata/OpenMetadata/blob/main/ingestion/LICENSE +# 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. +""" +Fivetran integration test fixtures — mock HTTP server +""" +import json +import re +import threading +from http.server import BaseHTTPRequestHandler, HTTPServer +from urllib.parse import parse_qs, urlparse + +import pytest + +from metadata.generated.schema.entity.services.connections.pipeline.fivetranConnection import ( + FivetranConnection, +) +from metadata.ingestion.source.pipeline.fivetran.client import FivetranClient + +# --------------------------------------------------------------------------- +# Mock data: Scenario 1 — PostgreSQL RDS → Redshift (standard connector) +# --------------------------------------------------------------------------- +POSTGRES_CONNECTOR = { + "id": "conn_pg_rds", + "group_id": "group_postgres_rds", + "service": "postgres_rds", + "schema": "public", + "connected_by": "user@example.com", + "setup_tests": [], + "config": { + "host": "my-rds-instance.amazonaws.com", + "port": 5432, + "database": "source_db", + }, + "status": {"setup_state": "connected", "sync_state": "scheduled"}, +} + +POSTGRES_DESTINATION = { + "id": "group_postgres_rds", + "service": "redshift", + "config": { + "host": "redshift-cluster.amazonaws.com", + "port": 5439, + "database": "dest_db", + }, +} + +POSTGRES_SCHEMAS = { + "schemas": { + "public": { + "name_in_destination": "public", + "enabled": True, + "tables": { + "users": { + "name_in_destination": "users", + "enabled": True, + "columns": {}, + }, + "audit_log": { + "name_in_destination": "audit_log", + "enabled": False, + "columns": {}, + }, + }, + }, + "internal": { + "name_in_destination": "internal", + "enabled": False, + "tables": {}, + }, + } +} + +POSTGRES_COLUMNS_PUBLIC_USERS = { + "columns": { + "id": { + "name_in_destination": "user_id", + "enabled": True, + "type": "INTEGER", + }, + "email": { + "name_in_destination": "email_address", + "enabled": True, + "type": "VARCHAR", + }, + "internal_flag": { + "name_in_destination": "internal_flag", + "enabled": False, + "type": "BOOLEAN", + }, + } +} + +# --------------------------------------------------------------------------- +# Mock data: Scenario 2 — SQL Server HVA → Snowflake (HVR/HVA connector) +# --------------------------------------------------------------------------- +HVA_CONNECTOR = { + "id": "conn_hva_sqlserver", + "group_id": "group_hva_sqlserver", + "service": "sql_server_hva", + "schema": "dbo", + "connected_by": "admin@example.com", + "setup_tests": [], + "config": { + "host": "sqlserver.example.com", + "port": 1433, + "database": "erp_db", + }, + "status": {"setup_state": "connected", "sync_state": "scheduled"}, +} + +HVA_DESTINATION = { + "id": "group_hva_sqlserver", + "service": "snowflake", + "config": { + "host": "account.snowflakecomputing.com", + "database": "ANALYTICS_DB", + }, +} + +HVA_SCHEMAS = { + "schemas": { + "dbo": { + "name_in_destination": "DBO_DEST", + "enabled": True, + "tables": { + "orders": { + "name_in_destination": "ORDERS_DEST", + "enabled": True, + "columns": {}, + }, + "customers": { + "name_in_destination": "CUSTOMERS_DEST", + "enabled": True, + "columns": {}, + }, + }, + } + } +} + +HVA_COLUMNS_DBO_ORDERS = { + "columns": { + "order_id": { + "name_in_destination": "ORDER_ID", + "enabled": True, + "type": "INTEGER", + }, + "customer_id": { + "name_in_destination": "CUSTOMER_ID", + "enabled": True, + "type": "INTEGER", + }, + "order_date": { + "name_in_destination": "ORDER_DATE", + "enabled": True, + "type": "DATE", + }, + } +} + +HVA_COLUMNS_DBO_CUSTOMERS = { + "columns": { + "customer_id": { + "name_in_destination": "CUSTOMER_ID", + "enabled": True, + "type": "INTEGER", + }, + "name": { + "name_in_destination": "NAME", + "enabled": True, + "type": "VARCHAR", + }, + } +} + +# --------------------------------------------------------------------------- +# Groups & pagination support +# --------------------------------------------------------------------------- +GROUP_PAGE_1 = { + "id": "group_postgres_rds", + "name": "Postgres RDS Pipeline", +} + +GROUP_HVA = { + "id": "group_hva_sqlserver", + "name": "SQL Server HVA Pipeline", +} + +GROUP_PAGE_2 = { + "id": "group_snowflake", + "name": "Snowflake Analytics", +} + +ALL_GROUPS = [GROUP_PAGE_1, GROUP_HVA, GROUP_PAGE_2] + +CONNECTORS_BY_GROUP = { + "group_postgres_rds": [POSTGRES_CONNECTOR], + "group_hva_sqlserver": [HVA_CONNECTOR], +} + +CONNECTOR_DETAILS = { + "conn_pg_rds": POSTGRES_CONNECTOR, + "conn_hva_sqlserver": HVA_CONNECTOR, +} + +DESTINATION_DETAILS = { + "group_postgres_rds": POSTGRES_DESTINATION, + "group_hva_sqlserver": HVA_DESTINATION, +} + +SCHEMA_DETAILS = { + "conn_pg_rds": POSTGRES_SCHEMAS, + "conn_hva_sqlserver": HVA_SCHEMAS, +} + +COLUMN_DETAILS = { + ("conn_pg_rds", "public", "users"): POSTGRES_COLUMNS_PUBLIC_USERS, + ("conn_hva_sqlserver", "dbo", "orders"): HVA_COLUMNS_DBO_ORDERS, + ("conn_hva_sqlserver", "dbo", "customers"): HVA_COLUMNS_DBO_CUSTOMERS, +} + +# --------------------------------------------------------------------------- +# Route patterns (compiled once) +# --------------------------------------------------------------------------- +RE_GROUPS = re.compile(r"^/v1/groups$") +RE_GROUP_CONNECTORS = re.compile(r"^/v1/groups/(?P[^/]+)/connectors$") +RE_CONNECTOR = re.compile(r"^/v1/connectors/(?P[^/]+)$") +RE_DESTINATION = re.compile(r"^/v1/destinations/(?P[^/]+)$") +RE_CONNECTOR_SCHEMAS = re.compile(r"^/v1/connectors/(?P[^/]+)/schemas$") +RE_COLUMN_LINEAGE = re.compile( + r"^/v1/connectors/(?P[^/]+)" + r"/schemas/(?P[^/]+)" + r"/tables/(?P[^/]+)/columns$" +) + + +class FivetranMockHandler(BaseHTTPRequestHandler): + paginate_groups = False + + def do_GET(self): + parsed = urlparse(self.path) + path = parsed.path + params = parse_qs(parsed.query) + + if m := RE_GROUPS.match(path): + self._handle_groups(params) + elif m := RE_GROUP_CONNECTORS.match(path): + self._handle_group_connectors(m.group("group_id"), params) + elif m := RE_CONNECTOR.match(path): + self._handle_detail(CONNECTOR_DETAILS, m.group("connector_id")) + elif m := RE_DESTINATION.match(path): + self._handle_detail(DESTINATION_DETAILS, m.group("dest_id")) + elif m := RE_CONNECTOR_SCHEMAS.match(path): + self._handle_schemas(m.group("connector_id")) + elif m := RE_COLUMN_LINEAGE.match(path): + self._handle_columns( + m.group("connector_id"), m.group("schema"), m.group("table") + ) + else: + self._respond_json( + {"code": "NotFound", "message": f"Unknown resource: {path}"}, + status=404, + ) + + # -- paginated endpoints ------------------------------------------------ + + def _handle_groups(self, params): + cursor = params.get("cursor", [None])[0] + if self.__class__.paginate_groups: + if cursor is None: + self._respond_paginated([GROUP_PAGE_1, GROUP_HVA], next_cursor="page2") + else: + self._respond_paginated([GROUP_PAGE_2], next_cursor=None) + else: + self._respond_paginated(ALL_GROUPS, next_cursor=None) + + def _handle_group_connectors(self, group_id, params): + connectors = CONNECTORS_BY_GROUP.get(group_id, []) + self._respond_paginated(connectors, next_cursor=None) + + # -- detail endpoints --------------------------------------------------- + + def _handle_detail(self, registry, key): + data = registry.get(key) + if data is None: + self._respond_json( + {"code": "NotFound", "message": f"Not found: {key}"}, status=404 + ) + return + self._respond_json({"data": data}) + + def _handle_schemas(self, connector_id): + data = SCHEMA_DETAILS.get(connector_id) + if data is None: + self._respond_json( + {"code": "NotFound", "message": f"No schemas for {connector_id}"}, + status=404, + ) + return + self._respond_json({"data": data}) + + def _handle_columns(self, connector_id, schema, table): + key = (connector_id, schema, table) + data = COLUMN_DETAILS.get(key) + if data is None: + self._respond_json( + {"code": "NotFound", "message": f"No columns for {key}"}, status=404 + ) + return + self._respond_json({"data": data}) + + # -- response helpers --------------------------------------------------- + + def _respond_paginated(self, items, next_cursor=None): + payload = {"data": {"items": items, "next_cursor": next_cursor or ""}} + self._respond_json(payload) + + def _respond_json(self, data, status=200): + body = json.dumps(data).encode() + self.send_response(status) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def log_message(self, _format, *_args): + pass + + +@pytest.fixture(scope="module") +def fivetran_mock_server(): + server = HTTPServer(("127.0.0.1", 0), FivetranMockHandler) + port = server.server_address[1] + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + yield f"http://127.0.0.1:{port}" + server.shutdown() + + +@pytest.fixture(scope="module") +def fivetran_client(fivetran_mock_server): + connection = FivetranConnection( + apiKey="test_key", + apiSecret="test_secret", + hostPort=fivetran_mock_server, + limit=100, + ) + yield FivetranClient(connection) diff --git a/ingestion/tests/integration/fivetran/test_fivetran_client.py b/ingestion/tests/integration/fivetran/test_fivetran_client.py new file mode 100644 index 000000000000..266f0b8488ea --- /dev/null +++ b/ingestion/tests/integration/fivetran/test_fivetran_client.py @@ -0,0 +1,125 @@ +# Copyright 2025 Collate +# Licensed under the Collate Community License, Version 1.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# https://github.com/open-metadata/OpenMetadata/blob/main/ingestion/LICENSE +# 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. +""" +Fivetran integration tests using a mock HTTP server +""" +import pytest + +from metadata.generated.schema.entity.services.connections.pipeline.fivetranConnection import ( + FivetranConnection, +) +from metadata.ingestion.source.pipeline.fivetran.client import FivetranClient + +from .conftest import FivetranMockHandler + + +@pytest.mark.integration +class TestFivetranClient: + def test_list_groups(self, fivetran_client): + groups = fivetran_client.list_groups() + assert len(groups) == 3 + ids = {g["id"] for g in groups} + assert "group_postgres_rds" in ids + assert "group_hva_sqlserver" in ids + assert "group_snowflake" in ids + + def test_list_group_connectors(self, fivetran_client): + connectors = fivetran_client.list_group_connectors("group_postgres_rds") + assert len(connectors) == 1 + assert connectors[0]["service"] == "postgres_rds" + + def test_get_connector_details(self, fivetran_client): + details = fivetran_client.get_connector_details("conn_pg_rds") + assert details["service"] == "postgres_rds" + assert details["config"]["database"] == "source_db" + assert details["group_id"] == "group_postgres_rds" + + def test_get_destination_details(self, fivetran_client): + dest = fivetran_client.get_destination_details("group_postgres_rds") + assert dest["service"] == "redshift" + assert dest["config"]["database"] == "dest_db" + + def test_get_connector_schema_details(self, fivetran_client): + schemas = fivetran_client.get_connector_schema_details("conn_pg_rds") + assert "public" in schemas + assert "internal" in schemas + assert schemas["public"]["enabled"] is True + assert schemas["internal"]["enabled"] is False + + tables = schemas["public"]["tables"] + assert "users" in tables + assert "audit_log" in tables + assert tables["users"]["enabled"] is True + assert tables["audit_log"]["enabled"] is False + + def test_get_connector_column_lineage(self, fivetran_client): + columns = fivetran_client.get_connector_column_lineage( + "conn_pg_rds", "public", "users" + ) + assert "id" in columns + assert columns["id"]["name_in_destination"] == "user_id" + assert columns["id"]["enabled"] is True + + assert "email" in columns + assert columns["email"]["name_in_destination"] == "email_address" + assert columns["email"]["enabled"] is True + + assert "internal_flag" in columns + assert columns["internal_flag"]["enabled"] is False + + def test_pagination(self, fivetran_mock_server): + FivetranMockHandler.paginate_groups = True + try: + connection = FivetranConnection( + apiKey="test_key", + apiSecret="test_secret", + hostPort=fivetran_mock_server, + limit=1, + ) + client = FivetranClient(connection) + groups = client.list_groups() + assert len(groups) == 3 + ids = [g["id"] for g in groups] + assert ids[0] == "group_postgres_rds" + assert ids[1] == "group_hva_sqlserver" + assert ids[2] == "group_snowflake" + finally: + FivetranMockHandler.paginate_groups = False + + def test_hva_connector_listed(self, fivetran_client): + connectors = fivetran_client.list_group_connectors("group_hva_sqlserver") + assert len(connectors) == 1 + assert connectors[0]["id"] == "conn_hva_sqlserver" + assert connectors[0]["service"] == "sql_server_hva" + + def test_hva_schema_details(self, fivetran_client): + schemas = fivetran_client.get_connector_schema_details("conn_hva_sqlserver") + assert "dbo" in schemas + assert schemas["dbo"]["enabled"] is True + assert schemas["dbo"]["name_in_destination"] == "DBO_DEST" + + tables = schemas["dbo"]["tables"] + assert tables["orders"]["name_in_destination"] == "ORDERS_DEST" + assert tables["customers"]["name_in_destination"] == "CUSTOMERS_DEST" + + def test_hva_column_lineage(self, fivetran_client): + columns = fivetran_client.get_connector_column_lineage( + "conn_hva_sqlserver", "dbo", "orders" + ) + assert columns["order_id"]["name_in_destination"] == "ORDER_ID" + assert columns["customer_id"]["name_in_destination"] == "CUSTOMER_ID" + assert columns["order_date"]["name_in_destination"] == "ORDER_DATE" + assert all(columns[col]["enabled"] for col in columns) + + def test_hva_destination_snowflake(self, fivetran_client): + dest = fivetran_client.get_destination_details("group_hva_sqlserver") + assert dest["service"] == "snowflake" + assert dest["config"]["database"] == "ANALYTICS_DB" 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 index 8fad2fe71e88..f03e1fd79a10 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/security/AuthenticationCodeFlowHandler.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/security/AuthenticationCodeFlowHandler.java @@ -396,10 +396,10 @@ public void handleLogin(HttpServletRequest req, HttpServletResponse resp) { LOG.debug("Web login flow detected - using configured callback URL"); } - checkAndStoreRedirectUriInSession(session, finalRedirectUri); - session.setAttribute(SESSION_SSO_CALLBACK_URL, ssoCallbackUrl); + checkAndStoreRedirectUriInSession(session, finalRedirectUri, serverUrl, ssoCallbackUrl); + LOG.debug( "Auth Login - Session: {}, requestedRedirectUri: {}, ssoCallbackUrl: {}, finalRedirectUri: {}", session.getId(), @@ -454,12 +454,24 @@ public static HttpSession getHttpSession(HttpServletRequest request, boolean cre } public static void checkAndStoreRedirectUriInSession(HttpSession session, String redirectUri) { + checkAndStoreRedirectUriInSession(session, redirectUri, new String[0]); + } + + public static void checkAndStoreRedirectUriInSession( + HttpSession session, String redirectUri, String... trustedBaseUrls) { if (nullOrEmpty(redirectUri)) { throw new TechnicalException("Redirect URI is required"); } + if (trustedBaseUrls != null + && trustedBaseUrls.length > 0 + && !RedirectUriValidator.isSafe(redirectUri, trustedBaseUrls)) { + LOG.warn("[OIDC] Rejecting disallowed redirect URI on login request"); + throw new TechnicalException("Redirect URI is not in the allowlist"); + } + session.setAttribute(SESSION_REDIRECT_URI, redirectUri); - LOG.debug("Stored redirect URI in session {}: {}", session.getId(), redirectUri); + LOG.debug("Stored redirect URI in session {}", session.getId()); } // Callback diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/security/DefaultAuthorizer.java b/openmetadata-service/src/main/java/org/openmetadata/service/security/DefaultAuthorizer.java index a1e568b48ac8..838baf13454c 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/security/DefaultAuthorizer.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/security/DefaultAuthorizer.java @@ -139,6 +139,9 @@ public void authorizeRequests( @Override public void authorizeAdmin(SecurityContext securityContext) { SubjectContext subjectContext = getSubjectContext(securityContext); + if (subjectContext.impersonatedBy() != null) { + checkImpersonationAuthorization(subjectContext); + } if (subjectContext.isAdmin()) { return; } @@ -157,6 +160,9 @@ public void authorizeAdmin(String adminName) { @Override public void authorizeAdminOrBot(SecurityContext securityContext) { SubjectContext subjectContext = getSubjectContext(securityContext); + if (subjectContext.impersonatedBy() != null) { + checkImpersonationAuthorization(subjectContext); + } if (subjectContext.isAdmin() || subjectContext.isBot()) { return; } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/security/RedirectUriValidator.java b/openmetadata-service/src/main/java/org/openmetadata/service/security/RedirectUriValidator.java new file mode 100644 index 000000000000..017259b58dcd --- /dev/null +++ b/openmetadata-service/src/main/java/org/openmetadata/service/security/RedirectUriValidator.java @@ -0,0 +1,108 @@ +/* + * 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. + */ + +package org.openmetadata.service.security; + +import java.net.URI; +import java.net.URISyntaxException; +import java.util.Locale; +import java.util.Set; +import lombok.extern.slf4j.Slf4j; +import org.openmetadata.common.utils.CommonUtil; + +/** + * Validates post-authentication redirect URIs against an allowlist of trusted base URLs to + * prevent open-redirect attacks that could leak freshly minted JWT access tokens to + * attacker-controlled hosts. + */ +@Slf4j +public final class RedirectUriValidator { + private static final Set ALLOWED_SCHEMES = Set.of("http", "https"); + + private RedirectUriValidator() {} + + /** + * Returns true if {@code candidate} is safe to use as a post-login redirect target. + * + *

Accepted forms: + * + *

    + *
  • Null / empty — caller decides, returns false here so callers fall back to defaults. + *
  • Site-relative path starting with {@code /} (but not {@code //}). + *
  • Absolute http/https URI whose scheme + host + port match one of {@code allowedBaseUrls}. + *
+ */ + public static boolean isSafe(String candidate, String... allowedBaseUrls) { + if (CommonUtil.nullOrEmpty(candidate)) { + return false; + } + if (candidate.startsWith("/") && !candidate.startsWith("//")) { + return true; + } + URI uri; + try { + uri = new URI(candidate); + } catch (URISyntaxException e) { + LOG.warn("Rejecting malformed redirect URI"); + return false; + } + String scheme = uri.getScheme(); + String host = uri.getHost(); + if (scheme == null + || host == null + || !ALLOWED_SCHEMES.contains(scheme.toLowerCase(Locale.ROOT))) { + LOG.warn("Rejecting redirect URI with unsupported scheme or missing host"); + return false; + } + int candidatePort = effectivePort(scheme, uri.getPort()); + if (allowedBaseUrls == null) { + LOG.warn("Rejecting redirect URI (no allowlist configured)"); + return false; + } + for (String base : allowedBaseUrls) { + if (CommonUtil.nullOrEmpty(base)) { + continue; + } + try { + URI baseUri = new URI(base); + if (baseUri.getHost() == null) { + continue; + } + int basePort = effectivePort(baseUri.getScheme(), baseUri.getPort()); + if (scheme.equalsIgnoreCase(baseUri.getScheme()) + && host.equalsIgnoreCase(baseUri.getHost()) + && candidatePort == basePort) { + return true; + } + } catch (URISyntaxException ignored) { + // Try next base URL. + } + } + LOG.warn("Rejecting redirect URI (host not in allowlist)"); + return false; + } + + private static int effectivePort(String scheme, int port) { + if (port != -1) { + return port; + } + if (scheme == null) { + return -1; + } + return switch (scheme.toLowerCase(Locale.ROOT)) { + case "https" -> 443; + case "http" -> 80; + default -> -1; + }; + } +} diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/security/auth/SamlAuthServletHandler.java b/openmetadata-service/src/main/java/org/openmetadata/service/security/auth/SamlAuthServletHandler.java index ca68404be35a..5d20bb54ad81 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/security/auth/SamlAuthServletHandler.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/security/auth/SamlAuthServletHandler.java @@ -39,6 +39,7 @@ import org.openmetadata.service.auth.JwtResponse; import org.openmetadata.service.exception.AuthenticationException; import org.openmetadata.service.security.AuthServeletHandler; +import org.openmetadata.service.security.RedirectUriValidator; import org.openmetadata.service.security.jwt.JWTTokenGenerator; import org.openmetadata.service.security.saml.SamlSettingsHolder; import org.openmetadata.service.util.TokenUtil; @@ -50,6 +51,7 @@ public class SamlAuthServletHandler implements AuthServeletHandler { private static final String SESSION_USER_ID = "userId"; private static final String SESSION_USERNAME = "username"; private static final String SESSION_REDIRECT_URI = "redirectUri"; + private static final String SESSION_SAML_REQUEST_ID = "samlAuthnRequestId"; final AuthenticationConfiguration authConfig; final AuthorizerConfiguration authorizerConfig; @@ -118,8 +120,14 @@ public void handleLogin(HttpServletRequest req, HttpServletResponse resp) { if (callbackUrl == null) { callbackUrl = req.getParameter("redirectUri"); } + HttpSession session = req.getSession(true); if (callbackUrl != null) { - req.getSession(true).setAttribute(SESSION_REDIRECT_URI, callbackUrl); + String trustedCallback = SamlSettingsHolder.getConfiguredRelayState(); + if (RedirectUriValidator.isSafe(callbackUrl, trustedCallback)) { + session.setAttribute(SESSION_REDIRECT_URI, callbackUrl); + } else { + LOG.warn("[SAML] Ignoring disallowed callback URL from login request"); + } } javax.servlet.http.HttpServletRequest wrappedRequest = new HttpServletRequestWrapper(req); @@ -127,6 +135,10 @@ public void handleLogin(HttpServletRequest req, HttpServletResponse resp) { Auth auth = new Auth(SamlSettingsHolder.getSaml2Settings(), wrappedRequest, wrappedResponse); auth.login(); + String authnRequestId = auth.getLastRequestId(); + if (authnRequestId != null) { + session.setAttribute(SESSION_SAML_REQUEST_ID, authnRequestId); + } } catch (SAMLException e) { LOG.error("Error initiating SAML login", e); @@ -145,7 +157,15 @@ public void handleCallback(HttpServletRequest req, HttpServletResponse resp) { javax.servlet.http.HttpServletResponse wrappedResponse = new HttpServletResponseWrapper(resp); Auth auth = new Auth(SamlSettingsHolder.getSaml2Settings(), wrappedRequest, wrappedResponse); - auth.processResponse(); + HttpSession preAuthSession = req.getSession(false); + String expectedRequestId = + preAuthSession == null + ? null + : (String) preAuthSession.getAttribute(SESSION_SAML_REQUEST_ID); + if (preAuthSession != null) { + preAuthSession.removeAttribute(SESSION_SAML_REQUEST_ID); + } + auth.processResponse(expectedRequestId); if (!auth.isAuthenticated()) { throw new AuthenticationException("SAML authentication failed"); @@ -227,15 +247,22 @@ public void handleCallback(HttpServletRequest req, HttpServletResponse resp) { // Get stored redirect URI from session String redirectUri = (String) req.getSession().getAttribute(SESSION_REDIRECT_URI); - LOG.debug("SAML Callback - redirectUri from session: {}", redirectUri); + LOG.debug("SAML Callback - redirectUri from session"); + String trustedCallback = SamlSettingsHolder.getConfiguredRelayState(); + boolean trusted = + redirectUri != null && RedirectUriValidator.isSafe(redirectUri, trustedCallback); String callbackUrl; - if (redirectUri != null) { + if (trusted) { callbackUrl = redirectUri + "?id_token=" + URLEncoder.encode(jwtAuthMechanism.getJWTToken(), StandardCharsets.UTF_8); } else { + if (redirectUri != null) { + LOG.warn("[SAML] Ignoring disallowed redirect URI on callback"); + req.getSession().removeAttribute(SESSION_REDIRECT_URI); + } callbackUrl = "/auth/callback?id_token=" + URLEncoder.encode(jwtAuthMechanism.getJWTToken(), StandardCharsets.UTF_8); diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/security/jwt/JWTTokenGenerator.java b/openmetadata-service/src/main/java/org/openmetadata/service/security/jwt/JWTTokenGenerator.java index 10e622b8d6df..67080904408d 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/security/jwt/JWTTokenGenerator.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/security/jwt/JWTTokenGenerator.java @@ -102,13 +102,15 @@ public void init( } public JWTAuthMechanism generateJWTToken(User user, JWTTokenExpiry expiry) { + boolean isBot = Boolean.TRUE.equals(user.getIsBot()); + ServiceTokenType tokenType = isBot ? ServiceTokenType.BOT : ServiceTokenType.OM_USER; return getJwtAuthMechanism( user.getName(), getRoleListFromUser(user), user.getIsAdmin(), user.getEmail(), - true, - ServiceTokenType.BOT, + isBot, + tokenType, getExpiryDate(expiry), expiry); } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/security/saml/SamlLoginServlet.java b/openmetadata-service/src/main/java/org/openmetadata/service/security/saml/SamlLoginServlet.java index 3f9b11044b1d..517af88ab3d8 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/security/saml/SamlLoginServlet.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/security/saml/SamlLoginServlet.java @@ -22,10 +22,12 @@ import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; +import jakarta.servlet.http.HttpSession; import java.io.IOException; import lombok.extern.slf4j.Slf4j; import org.apache.felix.http.javaxwrappers.HttpServletRequestWrapper; import org.apache.felix.http.javaxwrappers.HttpServletResponseWrapper; +import org.openmetadata.service.security.RedirectUriValidator; /** * This Servlet initiates a login and sends a login request to the IDP. After a successful processing it redirects user @@ -34,26 +36,39 @@ @Slf4j @WebServlet("/api/v1/saml/login") public class SamlLoginServlet extends HttpServlet { + public static final String SESSION_SAML_REQUEST_ID = "samlAuthnRequestId"; + @Override protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { try { - checkAndStoreRedirectUriInSession(request); + HttpSession session = request.getSession(true); + checkAndStoreRedirectUriInSession(request, session); javax.servlet.http.HttpServletRequest wrappedRequest = new HttpServletRequestWrapper(request); javax.servlet.http.HttpServletResponse wrappedResponse = new HttpServletResponseWrapper(response); Auth auth = new Auth(SamlSettingsHolder.getSaml2Settings(), wrappedRequest, wrappedResponse); auth.login(); + String authnRequestId = auth.getLastRequestId(); + if (authnRequestId != null) { + session.setAttribute(SESSION_SAML_REQUEST_ID, authnRequestId); + } } catch (SAMLException ex) { LOG.error("Error initiating SAML login", ex); throw new ServletException("Error initiating SAML login", ex); } } - private void checkAndStoreRedirectUriInSession(HttpServletRequest request) { + private void checkAndStoreRedirectUriInSession(HttpServletRequest request, HttpSession session) { String redirectUri = request.getParameter("callback"); - if (redirectUri != null) { - request.getSession().setAttribute(SESSION_REDIRECT_URI, redirectUri); + if (redirectUri == null) { + return; + } + String trustedCallback = SamlSettingsHolder.getConfiguredRelayState(); + if (RedirectUriValidator.isSafe(redirectUri, trustedCallback)) { + session.setAttribute(SESSION_REDIRECT_URI, redirectUri); + } else { + LOG.warn("[SAML] Ignoring disallowed callback URL from login request"); } } } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/security/saml/SamlSettingsHolder.java b/openmetadata-service/src/main/java/org/openmetadata/service/security/saml/SamlSettingsHolder.java index b5bd10b4b705..ea8e466bb428 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/security/saml/SamlSettingsHolder.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/security/saml/SamlSettingsHolder.java @@ -23,7 +23,6 @@ import java.security.cert.CertificateException; import java.util.HashMap; import java.util.Map; -import lombok.Getter; import lombok.extern.slf4j.Slf4j; import org.openmetadata.catalog.security.client.SamlSSOClientConfig; import org.openmetadata.catalog.type.SamlSecurityConfig; @@ -37,9 +36,9 @@ public class SamlSettingsHolder { private static volatile Saml2Settings saml2Settings; private static final Object lock = new Object(); + private static volatile String relayState; private Map samlData; private SettingsBuilder builder; - @Getter private String relayState; private SamlSettingsHolder() { samlData = new HashMap<>(); @@ -50,6 +49,14 @@ public static SamlSettingsHolder getInstance() { return new SamlSettingsHolder(); } + public String getRelayState() { + return relayState; + } + + public static String getConfiguredRelayState() { + return relayState; + } + public void initDefaultSettings(OpenMetadataApplicationConfig catalogApplicationConfig) throws IOException, KeyStoreException, CertificateException, NoSuchAlgorithmException { SamlSSOClientConfig samlConfig = diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/security/AuthenticationCodeFlowHandlerTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/security/AuthenticationCodeFlowHandlerTest.java index c6c810db9034..cd402f2cf8cf 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/security/AuthenticationCodeFlowHandlerTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/security/AuthenticationCodeFlowHandlerTest.java @@ -573,7 +573,7 @@ void handleLoginRedirectsToProviderWithSecurityParameters() throws Exception { when(request.getSession(false)).thenReturn(null); when(request.getSession(true)).thenReturn(session); when(request.getParameter(AuthenticationCodeFlowHandler.REDIRECT_URI_KEY)) - .thenReturn("https://app.example.com/post-login"); + .thenReturn("https://openmetadata.example.com/post-login"); when(session.getAttribute(AuthenticationCodeFlowHandler.OIDC_CREDENTIAL_PROFILE)) .thenReturn(null); when(session.getId()).thenReturn("session-id"); @@ -583,7 +583,7 @@ void handleLoginRedirectsToProviderWithSecurityParameters() throws Exception { verify(session) .setAttribute( AuthenticationCodeFlowHandler.SESSION_REDIRECT_URI, - "https://app.example.com/post-login"); + "https://openmetadata.example.com/post-login"); ArgumentCaptor redirectCaptor = ArgumentCaptor.forClass(String.class); verify(response).sendRedirect(redirectCaptor.capture()); String location = redirectCaptor.getValue(); diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/security/RedirectUriValidatorTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/security/RedirectUriValidatorTest.java new file mode 100644 index 000000000000..dbf71d3133e2 --- /dev/null +++ b/openmetadata-service/src/test/java/org/openmetadata/service/security/RedirectUriValidatorTest.java @@ -0,0 +1,128 @@ +/* + * 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. + */ + +package org.openmetadata.service.security; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; + +class RedirectUriValidatorTest { + + private static final String TRUSTED = "https://openmetadata.example.com"; + private static final String TRUSTED_WITH_PORT = "https://openmetadata.example.com:8585"; + + @Test + void nullAndEmptyAreRejected() { + assertFalse(RedirectUriValidator.isSafe(null, TRUSTED)); + assertFalse(RedirectUriValidator.isSafe("", TRUSTED)); + } + + @Test + void siteRelativePathsAreAccepted() { + assertTrue(RedirectUriValidator.isSafe("/auth/callback", TRUSTED)); + assertTrue(RedirectUriValidator.isSafe("/", TRUSTED)); + } + + @Test + void protocolRelativeUriIsRejected() { + assertFalse(RedirectUriValidator.isSafe("//evil.example.com/callback", TRUSTED)); + } + + @Test + void absoluteSameOriginIsAccepted() { + assertTrue(RedirectUriValidator.isSafe(TRUSTED + "/auth/callback", TRUSTED)); + } + + @Test + void absoluteDifferentHostIsRejected() { + assertFalse(RedirectUriValidator.isSafe("https://evil.example.com/cb", TRUSTED)); + } + + @Test + void schemeMismatchIsRejected() { + assertFalse(RedirectUriValidator.isSafe("http://openmetadata.example.com/cb", TRUSTED)); + } + + @Test + void defaultPortMatchesOmittedPort() { + // https default 443 equals the omitted-port form. + assertTrue(RedirectUriValidator.isSafe("https://openmetadata.example.com:443/cb", TRUSTED)); + assertTrue( + RedirectUriValidator.isSafe( + "https://openmetadata.example.com/cb", "https://openmetadata.example.com:443")); + } + + @Test + void nonDefaultPortMismatchIsRejected() { + assertFalse(RedirectUriValidator.isSafe("https://openmetadata.example.com:9999/cb", TRUSTED)); + assertFalse( + RedirectUriValidator.isSafe("https://openmetadata.example.com/cb", TRUSTED_WITH_PORT)); + } + + @Test + void javaScriptSchemeIsRejected() { + assertFalse(RedirectUriValidator.isSafe("javascript:alert(1)", TRUSTED)); + } + + @Test + void dataSchemeIsRejected() { + assertFalse(RedirectUriValidator.isSafe("data:text/html,", TRUSTED)); + } + + @Test + void malformedUriIsRejected() { + assertFalse(RedirectUriValidator.isSafe("https://:::evil", TRUSTED)); + } + + @Test + void userInfoSpoofingIsRejected() { + // `user@evil.com` in URI — the host is evil.com, not trusted host. + assertFalse( + RedirectUriValidator.isSafe("https://openmetadata.example.com@evil.com/cb", TRUSTED)); + } + + @Test + void nullAllowlistRejectsAbsoluteUri() { + assertFalse(RedirectUriValidator.isSafe("https://anything.example.com/cb", (String[]) null)); + } + + @Test + void nullAllowlistStillAcceptsRelativePaths() { + assertTrue(RedirectUriValidator.isSafe("/cb", (String[]) null)); + } + + @Test + void emptyAllowlistRejectsAbsoluteUri() { + assertFalse(RedirectUriValidator.isSafe("https://anything.example.com/cb", new String[0])); + } + + @Test + void nullEntriesInAllowlistAreIgnored() { + assertTrue(RedirectUriValidator.isSafe(TRUSTED + "/cb", null, TRUSTED, null)); + } + + @Test + void caseInsensitiveHostMatch() { + assertTrue(RedirectUriValidator.isSafe("https://OpenMetadata.Example.COM/cb", TRUSTED)); + } + + @Test + void multipleAllowedBaseUrls() { + String frontend = "https://app.example.com"; + assertTrue(RedirectUriValidator.isSafe(frontend + "/cb", TRUSTED, frontend)); + assertTrue(RedirectUriValidator.isSafe(TRUSTED + "/cb", TRUSTED, frontend)); + assertFalse(RedirectUriValidator.isSafe("https://evil.example.com/cb", TRUSTED, frontend)); + } +} diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/security/auth/SamlAuthServletHandlerTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/security/auth/SamlAuthServletHandlerTest.java index 8280315f5c5e..99979c4f2f72 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/security/auth/SamlAuthServletHandlerTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/security/auth/SamlAuthServletHandlerTest.java @@ -124,13 +124,16 @@ void setUp() throws Exception { @Test void testHandleLogin_WithCallbackParameter() { - // Test parameter precedence: 'callback' parameter is used - String callbackUrl = "https://example.com/callback"; + // Relative paths are always trusted by the allowlist validator. + String callbackUrl = "/saml/callback"; when(request.getParameter("callback")).thenReturn(callbackUrl); - when(request.getParameter("redirectUri")).thenReturn("https://example.com/other"); + when(request.getParameter("redirectUri")).thenReturn("/saml/other"); try (MockedStatic samlMock = mockStatic(SamlSettingsHolder.class)) { samlMock.when(SamlSettingsHolder::getSaml2Settings).thenReturn(null); + samlMock + .when(SamlSettingsHolder::getInstance) + .thenReturn(Mockito.mock(SamlSettingsHolder.class)); try { handler.handleLogin(request, response); @@ -144,13 +147,16 @@ void testHandleLogin_WithCallbackParameter() { @Test void testHandleLogin_WithRedirectUriParameter() { - // Test fallback: when 'callback' is null, 'redirectUri' is used - String redirectUri = "https://example.com/redirect"; + // Relative paths are always trusted by the allowlist validator. + String redirectUri = "/saml/redirect"; when(request.getParameter("callback")).thenReturn(null); when(request.getParameter("redirectUri")).thenReturn(redirectUri); try (MockedStatic samlMock = mockStatic(SamlSettingsHolder.class)) { samlMock.when(SamlSettingsHolder::getSaml2Settings).thenReturn(null); + samlMock + .when(SamlSettingsHolder::getInstance) + .thenReturn(Mockito.mock(SamlSettingsHolder.class)); try { handler.handleLogin(request, response); @@ -184,9 +190,12 @@ void testHandleLogin_WithNoParameters() { @Test void testHandleLogin_SAMLException() { - when(request.getParameter("callback")).thenReturn("https://example.com/callback"); + when(request.getParameter("callback")).thenReturn("/saml/callback"); try (MockedStatic samlMock = mockStatic(SamlSettingsHolder.class)) { + samlMock + .when(SamlSettingsHolder::getInstance) + .thenReturn(Mockito.mock(SamlSettingsHolder.class)); // Simulate SAML error by returning null which causes NPE in Auth constructor samlMock.when(SamlSettingsHolder::getSaml2Settings).thenReturn(null); diff --git a/openmetadata-spec/src/main/resources/json/schema/security/client/samlSSOClientConfig.json b/openmetadata-spec/src/main/resources/json/schema/security/client/samlSSOClientConfig.json index 70185a3ffc56..ae513c03cd06 100644 --- a/openmetadata-spec/src/main/resources/json/schema/security/client/samlSSOClientConfig.json +++ b/openmetadata-spec/src/main/resources/json/schema/security/client/samlSSOClientConfig.json @@ -77,7 +77,7 @@ "strictMode": { "description": "Only accept valid signed and encrypted assertions if the relevant flags are set", "type": "boolean", - "default": false + "default": true }, "validateXml": { "description": "In case of strict mode whether to validate XML format.", @@ -107,12 +107,12 @@ "wantMessagesSigned": { "description": "SP requires the messages received to be signed.", "type": "boolean", - "default": false + "default": true }, "wantAssertionsSigned": { "description": "SP requires the assertions received to be signed.", "type": "boolean", - "default": false + "default": true }, "wantAssertionEncrypted": { "description": "SP requires the assertion received to be encrypted.", diff --git a/openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/FeedEditor/FeedEditor.tsx b/openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/FeedEditor/FeedEditor.tsx index 8bebde552234..6d29724859eb 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/FeedEditor/FeedEditor.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/ActivityFeed/FeedEditor/FeedEditor.tsx @@ -168,33 +168,47 @@ export const FeedEditor = forwardRef( ? item.breadcrumbs.map((obj: { name: string }) => obj.name).join('/') : ''; - const breadcrumbEle = breadcrumbsData - ? `
- ${breadcrumbsData} -
` - : ''; - const icon = searchClassBase.getEntityIcon(item.type ?? ''); - const iconString = ReactDOMServer.renderToString(icon ?? <>); - const typeSpan = !breadcrumbEle - ? `${item.type}` - : ''; + const wrapper = document.createElement('div'); + wrapper.className = 'd-flex items-center gap-2'; + + const iconContainer = document.createElement('div'); + iconContainer.className = 'flex-center mention-icon-image'; + // Safe: iconString comes from ReactDOMServer rendering a static component, + // never from user input. + iconContainer.innerHTML = iconString; + wrapper.appendChild(iconContainer); + + const details = document.createElement('div'); + wrapper.appendChild(details); + + if (breadcrumbsData) { + const crumbRow = document.createElement('div'); + crumbRow.className = 'd-flex flex-wrap'; + const crumbSpan = document.createElement('span'); + crumbSpan.className = 'text-grey-muted truncate w-max-200 text-xss'; + crumbSpan.textContent = breadcrumbsData; + crumbRow.appendChild(crumbSpan); + details.appendChild(crumbRow); + } - const result = `
-
${iconString}
-
- ${breadcrumbEle} -
- ${typeSpan} - ${item.name} -
-
-
`; + const infoCol = document.createElement('div'); + infoCol.className = 'd-flex flex-col'; + details.appendChild(infoCol); - const wrapper = document.createElement('div'); - wrapper.innerHTML = result; + if (!breadcrumbsData) { + const typeSpan = document.createElement('span'); + typeSpan.className = 'text-grey-muted text-xs'; + typeSpan.textContent = item.type ?? ''; + infoCol.appendChild(typeSpan); + } + + const nameSpan = document.createElement('span'); + nameSpan.className = 'font-medium truncate w-56'; + nameSpan.textContent = item.name; + infoCol.appendChild(nameSpan); return wrapper; }, diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/AdvancedSearchUtils.test.tsx b/openmetadata-ui/src/main/resources/ui/src/utils/AdvancedSearchUtils.test.tsx index 82ff981b7094..a96b49b0b9a7 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/AdvancedSearchUtils.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/utils/AdvancedSearchUtils.test.tsx @@ -12,6 +12,8 @@ */ import { FieldOrGroup } from '@react-awesome-query-builder/antd'; +import { render } from '@testing-library/react'; +import React from 'react'; import { SearchOutputType } from '../components/Explore/AdvanceSearchProvider/AdvanceSearchProvider.interface'; import { AssetsOfEntity } from '../components/Glossary/GlossaryTerms/tabs/AssetsTabs.interface'; import { SearchDropdownOption } from '../components/SearchDropdown/SearchDropdown.interface'; @@ -141,10 +143,16 @@ describe('AdvancedSearchUtils tests', () => { expect(resultOptionsString).toBe(''); }); - it('Function getSearchLabel should return string with highlighted substring for matched searchKey', () => { - const resultSearchLabel = getSearchLabel(mockItemLabel, 'wa'); + it('Function getSearchLabel should return React nodes with highlighted segments for matched searchKey', () => { + const { container } = render(<>{getSearchLabel(mockItemLabel, 'wa')}); - expect(resultSearchLabel).toBe(highlightedItemLabel); + expect(container.textContent).toBe(mockItemLabel); + expect(container.querySelectorAll('mark')).toHaveLength(2); + expect( + Array.from(container.querySelectorAll('mark')).map((m) => m.textContent) + ).toEqual(['Wa', 'Wa']); + // Confirms the original string representation is still available for reference. + expect(highlightedItemLabel).toContain('Wa'); }); it('Function getSearchLabel should return original string if searchKey is not matched', () => { @@ -159,6 +167,14 @@ describe('AdvancedSearchUtils tests', () => { expect(resultSearchLabel).toBe(mockItemLabel); }); + it('Function getSearchLabel should HTML-escape user-controlled label content (no XSS via displayName)', () => { + const xssLabel = 'Warren'; + const { container } = render(<>{getSearchLabel(xssLabel, 'wa')}); + + expect(container.querySelectorAll('img')).toHaveLength(0); + expect(container.textContent).toBe(xssLabel); + }); + it('Function getServiceOptions should return displayName of the service', () => { const resultGetServiceOptions = getServiceOptions(mockGetServiceOptionData); diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/AdvancedSearchUtils.tsx b/openmetadata-ui/src/main/resources/ui/src/utils/AdvancedSearchUtils.tsx index e37b8a3c36c3..ac5c658096db 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/AdvancedSearchUtils.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/utils/AdvancedSearchUtils.tsx @@ -148,15 +148,28 @@ export const renderAdvanceSearchButtons: RenderSettings['renderButton'] = ( return <>; }; -export const getSearchLabel = (itemLabel: string, searchKey: string) => { - const regex = new RegExp(searchKey, 'gi'); - if (searchKey) { - const result = itemLabel.replace(regex, (match) => `${match}`); - - return result; - } else { +const escapeRegExp = (value: string) => + value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + +export const getSearchLabel = ( + itemLabel: string, + searchKey: string +): React.ReactNode => { + if (!searchKey) { + return itemLabel; + } + const segments = itemLabel.split(new RegExp(`(${escapeRegExp(searchKey)})`, 'gi')); + if (segments.length <= 1) { return itemLabel; } + + return segments.map((segment, index) => + index % 2 === 1 ? ( + {segment} + ) : ( + {segment} + ) + ); }; export const generateSearchDropdownLabel = ( @@ -189,11 +202,7 @@ export const generateSearchDropdownLabel = ( ellipsis className="dropdown-option-label" title={option.label}> - + {getSearchLabel(option.label, searchKey)} {option.description && (