feat(cors): enhance CORS configuration and logging

pflooky · pflooky · commit fbafc29e2aa9 · 2025-12-08T10:59:12.000+11:00
- Introduced configurable CORS settings via environment variables, allowing for stricter security controls.
- Updated PlanRoutes to log CORS configuration at startup for better visibility.
- Refactored connection testing service to utilize a shared SparkSession, improving performance and resource management.
- Fixed foreign key property name handling to align with backend expectations, ensuring proper loading of foreign key links in the UI.
diff --git a/app/src/main/resources/ui/connection/connection.js b/app/src/main/resources/ui/connection/connection.js
@@ -34,28 +34,24 @@ async function testConnection(connection, button) {
         });
         
         const result = await response.json();
-        // Limit to 100 characters, show ellipsis if truncated
-        const resultDetails = result.details ? ` - ${truncateString(result.details, 100)}` : "";
         
         if (result.success) {
             createToast(
                 `Test: ${connection.name}`,
-                `${result.message}${resultDetails}`,
+                `${result.message} ${result.details}`,
                 "success"
             );
         } else {
             createToast(
                 `Test: ${connection.name}`,
-                `${result.message}${resultDetails}`,
+                `${result.message} ${result.details}`,
                 "fail"
             );
         }
     } catch (err) {
-        // Limit to 100 characters, show ellipsis if truncated
-        const resultDetails = err.message ? ` - ${truncateString(err.message, 100)}` : "";
         createToast(
             `Test: ${connection.name}`,
-            `Connection test failed: ${resultDetails}`,
+            `Connection test failed: ${err.message}`,
             "fail"
         );
     } finally {
diff --git a/app/src/main/resources/ui/helper-foreign-keys.js b/app/src/main/resources/ui/helper-foreign-keys.js
@@ -49,7 +49,9 @@ async function createForeignKeyLinksFromPlan(newForeignKey, foreignKey, linkType
         foreignKeyLinkSources.removeChild(foreignKeyLinkSources.querySelectorAll(`.foreign-key-${linkType}-link-source`)[0]);
     }
     // Support both backend format (generate/delete) and legacy format (generationLinks/deleteLinks)
-    let links = foreignKey[linkType] || foreignKey[`${linkType}Links`] || [];
+    // Map linkType to the backend property name: "generation" -> "generate", "delete" -> "delete"
+    let backendPropertyName = linkType === "generation" ? "generate" : linkType;
+    let links = foreignKey[backendPropertyName] || foreignKey[linkType] || foreignKey[`${linkType}Links`] || [];
     for (const fkLink of Array.from(links)) {
         let newForeignKeyLink = await createForeignKeyInput(numForeignKeysLinks, `foreign-key-${linkType}-link`);
         foreignKeyLinkSources.insertBefore(newForeignKeyLink, foreignKeyLinkSources.lastChild);
diff --git a/app/src/main/scala/io/github/datacatering/datacaterer/core/ui/config/UiConfiguration.scala b/app/src/main/scala/io/github/datacatering/datacaterer/core/ui/config/UiConfiguration.scala
@@ -8,6 +8,44 @@ object UiConfiguration {
 
   val INSTALL_DIRECTORY: String = getInstallDirectory
 
+  /**
+   * CORS Configuration
+   * 
+   * Environment variables:
+   * - DATA_CATERER_CORS_ALLOWED_ORIGINS: Comma-separated list of allowed origins (default: "*" for all)
+   *   Examples: "*", "http://localhost:3000", "http://localhost:3000,https://example.com"
+   * - DATA_CATERER_CORS_ALLOWED_METHODS: Comma-separated list of allowed HTTP methods (default: "GET,POST,PUT,DELETE,OPTIONS")
+   * - DATA_CATERER_CORS_ALLOWED_HEADERS: Comma-separated list of allowed headers (default: "Content-Type,Authorization,X-Requested-With")
+   * - DATA_CATERER_CORS_MAX_AGE: Max age for preflight cache in seconds (default: "86400")
+   */
+  object Cors {
+    val allowedOrigins: String = Option(System.getenv("DATA_CATERER_CORS_ALLOWED_ORIGINS"))
+      .filter(_.nonEmpty)
+      .getOrElse("*")
+    
+    val allowedMethods: Seq[String] = Option(System.getenv("DATA_CATERER_CORS_ALLOWED_METHODS"))
+      .filter(_.nonEmpty)
+      .map(_.split(",").map(_.trim).toSeq)
+      .getOrElse(Seq("GET", "POST", "PUT", "DELETE", "OPTIONS"))
+    
+    val allowedHeaders: Seq[String] = Option(System.getenv("DATA_CATERER_CORS_ALLOWED_HEADERS"))
+      .filter(_.nonEmpty)
+      .map(_.split(",").map(_.trim).toSeq)
+      .getOrElse(Seq("Content-Type", "Authorization", "X-Requested-With"))
+    
+    val maxAge: Long = Option(System.getenv("DATA_CATERER_CORS_MAX_AGE"))
+      .filter(_.nonEmpty)
+      .flatMap(s => scala.util.Try(s.toLong).toOption)
+      .getOrElse(86400L) // 24 hours
+    
+    def logConfiguration(): Unit = {
+      LOGGER.info(s"CORS configuration: allowed-origins=$allowedOrigins, " +
+        s"allowed-methods=${allowedMethods.mkString(",")}, " +
+        s"allowed-headers=${allowedHeaders.mkString(",")}, " +
+        s"max-age=$maxAge")
+    }
+  }
+
   def getInstallDirectory: String = {
     val osName = System.getProperty("os.name").toLowerCase
     val overrideDirectory = System.getProperty("data-caterer-install-dir")
diff --git a/app/src/main/scala/io/github/datacatering/datacaterer/core/ui/plan/PlanRoutes.scala b/app/src/main/scala/io/github/datacatering/datacaterer/core/ui/plan/PlanRoutes.scala
@@ -3,6 +3,7 @@ package io.github.datacatering.datacaterer.core.ui.plan
 import com.fasterxml.jackson.databind.ObjectMapper
 import com.github.pjfanning.pekkohttpjackson.JacksonSupport
 import io.github.datacatering.datacaterer.api.model.Step
+import io.github.datacatering.datacaterer.core.ui.config.UiConfiguration
 import io.github.datacatering.datacaterer.core.ui.model.{Connection, ConnectionTestResult, EnhancedPlanRunRequest, MultiSchemaSampleResponse, PlanRunRequest, SampleError, SampleMetadata, SaveConnectionsRequest, SchemaSampleRequest, TestConnectionRequest}
 import io.github.datacatering.datacaterer.core.ui.service.ConnectionTestService
 import io.github.datacatering.datacaterer.core.ui.resource.SparkSessionManager
@@ -13,7 +14,7 @@ import org.apache.pekko.actor.typed.scaladsl.AskPattern.{Askable, schedulerFromA
 import org.apache.pekko.actor.typed.{ActorRef, ActorSystem}
 import org.apache.pekko.http.scaladsl.model.HttpMethods._
 import org.apache.pekko.http.scaladsl.model.headers._
-import org.apache.pekko.http.scaladsl.model.{ContentType, HttpEntity, HttpResponse, MediaTypes, StatusCodes}
+import org.apache.pekko.http.scaladsl.model.{ContentType, HttpEntity, HttpResponse, MediaTypes, StatusCodes, HttpHeader}
 import org.apache.pekko.http.scaladsl.server.{Directives, ExceptionHandler, Route}
 import org.apache.pekko.util.Timeout
 
@@ -39,17 +40,46 @@ class PlanRoutes(
   implicit val ec: ExecutionContextExecutor = ExecutionContext.global
   implicit val objectMapper: ObjectMapper = ObjectMapperUtil.jsonObjectMapper
 
+  // Log CORS configuration at startup
+  UiConfiguration.Cors.logConfiguration()
+  
   /**
-   * CORS configuration to allow cross-origin requests from any origin.
-   * This is necessary for the UI to communicate with the API when running
-   * in different network contexts (Docker, remote access, etc.)
+   * CORS configuration to allow cross-origin requests.
+   * Configurable via environment variables:
+   * - DATA_CATERER_CORS_ALLOWED_ORIGINS: Allowed origins (default: "*")
+   * - DATA_CATERER_CORS_ALLOWED_METHODS: Allowed methods (default: "GET,POST,PUT,DELETE,OPTIONS")
+   * - DATA_CATERER_CORS_ALLOWED_HEADERS: Allowed headers (default: "Content-Type,Authorization,X-Requested-With")
+   * - DATA_CATERER_CORS_MAX_AGE: Preflight cache duration in seconds (default: 86400)
    */
-  private val corsHeaders = List(
-    `Access-Control-Allow-Origin`.*,
-    `Access-Control-Allow-Methods`(GET, POST, PUT, DELETE, OPTIONS),
-    `Access-Control-Allow-Headers`("Content-Type", "Authorization", "X-Requested-With"),
-    `Access-Control-Max-Age`(86400) // 24 hours
-  )
+  private val corsHeaders: List[HttpHeader] = {
+    val corsConfig = UiConfiguration.Cors
+    
+    val originHeader = if (corsConfig.allowedOrigins == "*") {
+      `Access-Control-Allow-Origin`.*
+    } else {
+      // For specific origins, we'll use the first one as default
+      // Note: For multiple origins, you'd need to check the request origin header
+      `Access-Control-Allow-Origin`(HttpOrigin(corsConfig.allowedOrigins.split(",").head.trim))
+    }
+    
+    val methodsHeader = `Access-Control-Allow-Methods`(
+      corsConfig.allowedMethods.flatMap {
+        case "GET" => Some(GET)
+        case "POST" => Some(POST)
+        case "PUT" => Some(PUT)
+        case "DELETE" => Some(DELETE)
+        case "OPTIONS" => Some(OPTIONS)
+        case "HEAD" => Some(HEAD)
+        case "PATCH" => Some(PATCH)
+        case _ => None
+      }: _*
+    )
+    
+    val headersHeader = `Access-Control-Allow-Headers`(corsConfig.allowedHeaders: _*)
+    val maxAgeHeader = `Access-Control-Max-Age`(corsConfig.maxAge)
+    
+    List(originHeader, methodsHeader, headersHeader, maxAgeHeader)
+  }
 
   /**
    * Directive to add CORS headers to all responses
diff --git a/app/src/main/scala/io/github/datacatering/datacaterer/core/ui/service/ConnectionTestService.scala b/app/src/main/scala/io/github/datacatering/datacaterer/core/ui/service/ConnectionTestService.scala
@@ -1,9 +1,8 @@
 package io.github.datacatering.datacaterer.core.ui.service
 
 import io.github.datacatering.datacaterer.api.model.Constants._
-import io.github.datacatering.datacaterer.core.config.ConfigParser
 import io.github.datacatering.datacaterer.core.ui.model.{Connection, ConnectionTestResult}
-import io.github.datacatering.datacaterer.core.util.SparkProvider
+import io.github.datacatering.datacaterer.core.ui.resource.SparkSessionManager
 import org.apache.log4j.Logger
 import org.apache.spark.sql.SparkSession
 
@@ -339,17 +338,11 @@ object ConnectionTestService {
   }
 
   /**
-   * Helper to create a temporary SparkSession for connection testing
+   * Helper to use the shared SparkSession for connection testing
    */
   private def withSparkSession[T](f: SparkSession => T): T = {
-    val config = ConfigParser.toDataCatererConfiguration
-    val sparkProvider = new SparkProvider(config.master, config.runtimeConfig)
-    val spark = sparkProvider.getSparkSession
-    try {
-      f(spark)
-    } finally {
-      // Don't stop the session as it might be shared
-    }
+    val spark = SparkSessionManager.getOrCreate()
+    f(spark)
   }
 
   private def getStackTraceAsString(ex: Throwable): String = {
diff --git a/docs/use-case/changelog/0.17.3.md b/docs/use-case/changelog/0.17.3.md
@@ -15,6 +15,7 @@ Latest features and fixes for Data Caterer include fixing the connection details
 - Added connection testing functionality in the UI, allowing users to test connections before saving.
 - Connections now display their source (file or config) to indicate whether they can be deleted.
 - Added test buttons for existing saved connections in the UI.
+- CORS is now configurable via environment variables for stricter security settings.
 
 ## Bugfix
 
@@ -28,3 +29,5 @@ Latest features and fixes for Data Caterer include fixing the connection details
 - Modified build.yml to set version dynamically based on the branch, supporting feature and bugfix branches.
 - Fixed JSON serialization issue with Connection model that caused POST /connection endpoint to fail.
 - Fixed route matching for POST /connection endpoint to properly handle path conflicts.
+- Fixed foreign key property name mismatch between backend (`generate`/`delete`) and UI (`generation`/`delete`) causing foreign key links to not load correctly when editing saved plans.
+- Connection test service now uses the shared SparkSession instead of creating a new one, improving performance and resource usage.
