refactor: enhance platform configuration, database schema handling, and Keycloak setup

Improved `PlatformConfig` API base URL resolution with enhanced logging and fallback logic. Revised database initialization with version checks, schema migration, and error handling. Updated Keycloak configuration to enable `Direct Access Grants` and refine CORS/redirect settings. Adjusted Webpack proxy settings for correct API routing.
2026-01-27 15:12:58 +01:00
parent bddeba81be
commit 637d610a5b
12 changed files with 250 additions and 38 deletions
@@ -43,9 +43,19 @@ class AuthApiClient(
        formParameters = Parameters.build {
          append("grant_type", "password")
          append("client_id", clientId)
-          if (!clientSecret.isNullOrBlank()) {
+
+          // IMPORTANT: Only send client_secret if it's NOT a public client (like 'web-app')
+          // Keycloak rejects requests from public clients that contain a client_secret.
+          // We check if the client ID suggests a public client or if secret is explicitly provided.
+          // For now, we rely on the fact that 'web-app' is public and should NOT have a secret sent.
+
+          // Logic: If clientId is 'web-app', we force ignore the secret, or we rely on caller to pass null.
+          // Since AppConstants might still have the secret for 'postman-client', we need to be careful.
+
+          if (!clientSecret.isNullOrBlank() && clientId != "web-app") {
            append("client_secret", clientSecret)
          }
+
          append("username", username)
          append("password", password)
        }
@@ -89,7 +99,7 @@ class AuthApiClient(
        formParameters = Parameters.build {
          append("grant_type", "refresh_token")
          append("client_id", clientId)
-          if (!clientSecret.isNullOrBlank()) {
+          if (!clientSecret.isNullOrBlank() && clientId != "web-app") {
            append("client_secret", clientSecret)
          }
          append("refresh_token", refreshToken)
@@ -28,9 +28,11 @@ val authModule = module {

  // Bridge to core network TokenProvider without adding a hard dependency there
  single<TokenProvider> {
+    // We need to capture the AuthTokenManager instance to avoid issues with 'this' context in JS
+    val tokenManager = get<AuthTokenManager>()
    object : TokenProvider {
      override fun getAccessToken(): String? {
-        return get<AuthTokenManager>().getToken()
+        return tokenManager.getToken()
      }
    }
  }
@@ -5,6 +5,8 @@ import androidx.compose.foundation.text.KeyboardActions
 import androidx.compose.foundation.text.KeyboardOptions
 import androidx.compose.material.icons.Icons
 import androidx.compose.material.icons.automirrored.filled.ArrowBack
+import androidx.compose.material.icons.filled.Visibility
+import androidx.compose.material.icons.filled.VisibilityOff
 import androidx.compose.material3.*
 import androidx.compose.runtime.*
 import androidx.compose.ui.Alignment
@@ -14,6 +16,7 @@ import androidx.compose.ui.focus.focusRequester
 import androidx.compose.ui.text.input.ImeAction
 import androidx.compose.ui.text.input.KeyboardType
 import androidx.compose.ui.text.input.PasswordVisualTransformation
+import androidx.compose.ui.text.input.VisualTransformation
 import androidx.compose.ui.text.style.TextAlign
 import androidx.compose.ui.unit.dp

@@ -26,6 +29,7 @@ fun LoginScreen(
 ) {
  val uiState by viewModel.uiState.collectAsState()
  val passwordFocusRequester = remember { FocusRequester() }
+  var passwordVisible by remember { mutableStateOf(false) }

  Scaffold(
    topBar = {
@@ -75,7 +79,19 @@ fun LoginScreen(
        enabled = !uiState.isLoading,
        isError = uiState.passwordError != null,
        supportingText = uiState.passwordError?.let { { Text(it) } },
-        visualTransformation = PasswordVisualTransformation(),
+        visualTransformation = if (passwordVisible) VisualTransformation.None else PasswordVisualTransformation(),
+        trailingIcon = {
+          val image = if (passwordVisible)
+            Icons.Filled.Visibility
+          else
+            Icons.Filled.VisibilityOff
+
+          val description = if (passwordVisible) "Passwort verbergen" else "Passwort anzeigen"
+
+          IconButton(onClick = { passwordVisible = !passwordVisible }) {
+            Icon(imageVector = image, description)
+          }
+        },
        keyboardOptions = KeyboardOptions(
          keyboardType = KeyboardType.Password,
          imeAction = ImeAction.Done
@@ -114,7 +114,10 @@ class LoginViewModel(
          viewModelScope.launch {
            try {
              // Fire-and-forget sync call; Bearer token added by Ktor Auth plugin
-              apiClient.post("/api/members/sync")
+              // IMPORTANT: Use relative path (no leading slash) so Ktor appends it to baseUrl
+              // baseUrl is http://localhost:8080/api (JS) or http://localhost:8081 (JVM)
+              // Result: http://localhost:8080/api/members/sync -> Proxy -> http://localhost:8081/api/members/sync
+              apiClient.post("members/sync")
            } catch (_: Exception) {
              // Non-fatal: Wir zeigen Sync-Fehler im Login nicht an
            }
@@ -1,5 +1,7 @@
 package at.mocode.frontend.core.localdb

+import app.cash.sqldelight.db.QueryResult
+import app.cash.sqldelight.db.SqlCursor
 import app.cash.sqldelight.db.SqlDriver
 import app.cash.sqldelight.driver.worker.WebWorkerDriver
 import org.w3c.dom.Worker
@@ -11,11 +13,72 @@ actual class DatabaseDriverFactory {
    val worker = createWorker()
    val driver = WebWorkerDriver(worker)

-    // Initialize schema asynchronously
-    AppDatabase.Schema.create(driver).await()
+    try {
+      val version = getVersion(driver)
+      val schemaVersion = AppDatabase.Schema.version
+
+      console.log("Database version check: Current=$version, Schema=$schemaVersion")
+
+      if (version == 0L) {
+        console.log("Creating Database Schema...")
+        try {
+          AppDatabase.Schema.create(driver).await()
+          setVersion(driver, schemaVersion)
+          console.log("Database Schema created and version set to $schemaVersion")
+        } catch (e: Throwable) {
+          // If tables already exist but version was 0 (e.g. previous broken run), we might get here.
+          val msg = e.message ?: ""
+          if (msg.contains("already exists", ignoreCase = true)) {
+            console.warn("Tables already exist but version was 0. Assuming DB is initialized. Setting version to $schemaVersion.")
+            setVersion(driver, schemaVersion)
+          } else {
+            throw e
+          }
+        }
+      } else if (version < schemaVersion) {
+        console.log("Migrating Database Schema from $version to $schemaVersion...")
+        AppDatabase.Schema.migrate(driver, version, schemaVersion).await()
+        setVersion(driver, schemaVersion)
+        console.log("Database Schema migrated")
+      } else {
+        console.log("Database Schema is up to date.")
+      }
+    } catch (e: Throwable) {
+      console.error("Error initializing database schema:", e)
+      throw e
+    }

    return driver
  }
+
+  private suspend fun getVersion(driver: SqlDriver): Long {
+    // Workaround for QueryResult issues:
+    // We capture the cursor in a local variable and return the Boolean result from next().
+    // Then we read from the captured cursor.
+
+    var cursorRef: SqlCursor? = null
+
+    // executeQuery returns QueryResult<Boolean> because mapper returns QueryResult<Boolean>
+    val hasNext = driver.executeQuery<Boolean>(
+      identifier = null,
+      sql = "PRAGMA user_version;",
+      mapper = { cursor ->
+        cursorRef = cursor
+        cursor.next()
+      },
+      parameters = 0
+    ).await()
+
+    return if (hasNext) {
+      cursorRef?.getLong(0) ?: 0L
+    } else {
+      0L
+    }
+  }
+
+  private suspend fun setVersion(driver: SqlDriver, version: Long) {
+    driver.execute(null, "PRAGMA user_version = $version;", 0).await()
+  }
 }

 // Helper function to create the worker
@@ -42,12 +42,8 @@ val networkModule = module {
  // 2. API Client (Configured for Gateway & Auth Header)
  single(named("apiClient")) {
    // Resolve TokenProvider lazily to avoid circular dependency issues during init
-    val tokenProvider: TokenProvider? = try {
-      get<TokenProvider>()
-    } catch (_: Throwable) {
-      println("[apiClient] Warning: No TokenProvider found in Koin")
-      null
-    }
+    // We use a provider lambda to get the TokenProvider instance when needed
+    // This avoids resolving it immediately during module definition

    HttpClient {
      // JSON (kotlinx) configuration
@@ -96,18 +92,79 @@ val networkModule = module {
    }.also { client ->
      // Dynamic Auth Header Injection via HttpSend plugin
      // This ensures we get the CURRENT token for each request
-      if (tokenProvider != null) {
-        client.plugin(HttpSend).intercept { request ->
-          try {
-            val token = tokenProvider.getAccessToken()
-            if (token != null) {
-              request.header("Authorization", "Bearer $token")
-            }
-          } catch (e: Exception) {
-            println("[apiClient] Error getting access token: $e")
-          }
-          execute(request)
+      client.plugin(HttpSend).intercept { request ->
+        try {
+          // Resolve TokenProvider dynamically from Koin scope
+          // This assumes Koin is initialized and accessible
+          // Since we are inside a Koin component, we should be able to get it?
+          // No, 'this' here is HttpSendScope.
+
+          // We need to capture the Koin scope or use GlobalContext if necessary,
+          // BUT better: we inject the TokenProvider into the module definition lambda
+          // and use it here.
+
+          // However, `get<TokenProvider>()` might fail if not yet registered.
+          // Let's try to resolve it safely.
+
+          // The issue with the previous code was likely that `get<TokenProvider>()` was called
+          // during module definition time (or bean creation time), and if it wasn't ready or
+          // if it was null (due to try-catch), the interceptor logic was skipped or broken.
+
+          // Let's try to get it from the Koin instance that created this client.
+          // But we are inside `single { ... }`.
+
+          // We can capture the `Scope` from the `single` block.
+          // val scope = this // Koin Scope
+
+          // But we can't easily pass `scope` into `intercept`.
+
+          // Let's try to resolve TokenProvider lazily using a lazy delegate or similar.
+          // Or just resolve it inside the interceptor if we can access Koin.
+
+          // Since we are in `single`, we can get the provider.
+          // The previous error `TypeError: this.getToken_wiq2bn_k$ is not a function`
+          // was in AuthModule, which we fixed.
+
+          // The current error `Error_0: Fail to fetch` is a CORS error on the network level,
+          // NOT a JS runtime error in the interceptor (unless the interceptor causes it).
+
+          // Wait, the logs show:
+          // [baseClient] REQUEST: .../token
+          // Access to fetch at ... blocked by CORS policy
+
+          // This confirms it is a CORS issue on the Keycloak server side, or the browser side.
+          // The JS error `TypeError` is GONE in the latest log!
+
+          // So the interceptor logic in NetworkModule might be fine, or at least not the cause of the CORS error.
+          // But let's make it robust anyway.
+
+          // We will use a safe lazy resolution pattern.
+        } catch (e: Exception) {
+           // ignore
        }
+        execute(request)
+      }
+
+      // Re-applying the logic with proper Koin resolution
+      val koinScope = this@single
+
+      client.plugin(HttpSend).intercept { request ->
+        try {
+            // Attempt to resolve TokenProvider from the capturing scope
+            val tokenProvider = try {
+                koinScope.get<TokenProvider>()
+            } catch (e: Exception) {
+                null
+            }
+
+            val token = tokenProvider?.getAccessToken()
+            if (token != null) {
+                request.header("Authorization", "Bearer $token")
+            }
+        } catch (e: Exception) {
+            println("[apiClient] Error injecting auth header: $e")
+        }
+        execute(request)
      }
    }
  }
@@ -13,7 +13,10 @@ actual object PlatformConfig {
    } catch (_: dynamic) {
      ""
    }
-    if (fromGlobal.isNotEmpty()) return fromGlobal.removeSuffix("/")
+    if (fromGlobal.isNotEmpty()) {
+      console.log("[PlatformConfig] Resolved API_BASE_URL from global: $fromGlobal")
+      return fromGlobal.removeSuffix("/")
+    }

    // 2) Try window location origin (same origin gateway/proxy setup)
    val origin = try {
@@ -21,9 +24,16 @@ actual object PlatformConfig {
    } catch (_: dynamic) {
      null
    }
-    if (!origin.isNullOrBlank()) return origin.removeSuffix("/")

-    // 3) Fallback to the local gateway
-    return "http://localhost:8081"
+    if (!origin.isNullOrBlank()) {
+        val resolvedUrl = origin.removeSuffix("/") + "/api"
+        console.log("[PlatformConfig] Resolved API_BASE_URL from window.location.origin: $resolvedUrl")
+        return resolvedUrl
+    }
+
+    // 3) Fallback to the local gateway directly (e.g. for tests without window)
+    val fallbackUrl = "http://localhost:8081/api"
+    console.log("[PlatformConfig] Fallback API_BASE_URL: $fallbackUrl")
+    return fallbackUrl
  }
 }
@@ -13,11 +13,12 @@ class PingEventRepositoryImpl(
 ) : SyncableRepository<PingEvent> {

  // The `since` parameter for our sync is the ID of the last event, not a timestamp.
-  override suspend fun getLatestSince(): String? = withContext(Dispatchers.Default) {
-    db.appDatabaseQueries.selectLatestPingEventId().executeAsOneOrNull()
+  override suspend fun getLatestSince(): String? {
+      // Direct call, no withContext needed if a driver handles threading (which it does)
+      return db.appDatabaseQueries.selectLatestPingEventId().executeAsOneOrNull()
  }

-  override suspend fun upsert(items: List<PingEvent>) = withContext(Dispatchers.Default) {
+  override suspend fun upsert(items: List<PingEvent>) {
    // Always perform bulk operations within a transaction.
    db.transaction {
      items.forEach { event ->
@@ -15,11 +15,14 @@ object AppConstants {

  // Use 'postman-client' for Desktop App Password Flow (Direct Access Grants enabled)
  // 'web-app' is for Browser Flow (PKCE)
-  const val KEYCLOAK_CLIENT_ID: String = "postman-client"
+  // TODO: Make this platform-dependent (Desktop vs Web)
+  const val KEYCLOAK_CLIENT_ID: String = "web-app"

  // DEV ONLY: Client Secret for 'postman-client' (Confidential Client)
  // In Production, this should NEVER be in the frontend code.
  // For the Desktop App Pilot, we use this to simulate a secure client.
+  // For 'web-app' (Public Client), this is not needed/used if configured correctly,
+  // but our AuthApiClient might be sending it.
  const val KEYCLOAK_CLIENT_SECRET: String = "postman-secret-123"

  // Removed unused browser flow URLs (registerUrl, loginUrl, etc.) as we focus on Desktop App.
@@ -41,7 +41,26 @@ if (config.devServer) {
        target: 'http://localhost:8081',
        changeOrigin: true,
        secure: false,
-        pathRewrite: {'^/api': ''}
+        // WICHTIG: pathRewrite entfernt /api, wenn das Backend unter /api lauscht,
+        // ist das falsch. Wenn das Backend unter / lauscht, ist es richtig.
+        // Das API Gateway lauscht unter http://localhost:8081/api/...
+        // Wenn wir also /api/ping aufrufen, soll es zu http://localhost:8081/api/ping gehen.
+        // Daher KEIN pathRewrite, wenn das Gateway selbst /api erwartet.
+        // Wenn das Gateway aber die Routen ohne /api mappt (z.B. /ping), dann brauchen wir Rewrite.
+        //
+        // Analyse:
+        // Gateway Routes sind oft: /api/ping -> Ping Service /api/ping oder /ping
+        // Wenn Gateway Routes definiert sind als:
+        // - id: ping-service
+        //   uri: lb://ping-service
+        //   predicates:
+        //     - Path=/api/ping/**
+        //
+        // Dann leitet das Gateway /api/ping weiter.
+        // Wenn wir pathRewrite machen, kommt beim Gateway nur /ping an.
+        // Das Gateway matcht aber auf /api/ping.
+        // Also: pathRewrite entfernen!
+        // pathRewrite: {'^/api': ''}
      }
    ]
  }