Always show LocationForSecureConnection when using HTTP while onboarding (#6223)

---------

Co-authored-by: Joris Pelgröm <jpelgrom@users.noreply.github.com>

Author: TimoPtr

app/src/main/kotlin/io/homeassistant/companion/android/onboarding/OnboardingNavigation.kt

@@ -1,11 +1,8 @@
 package io.homeassistant.companion.android.onboarding
 
 import android.app.Activity
-import android.content.pm.PackageManager
 import android.net.Uri
 import androidx.annotation.VisibleForTesting
-import androidx.compose.ui.util.fastAny
-import androidx.core.content.ContextCompat
 import androidx.navigation.NavController
 import androidx.navigation.NavGraphBuilder
 import androidx.navigation.NavOptions
@@ -42,7 +39,6 @@ import io.homeassistant.companion.android.onboarding.wearmtls.navigation.wearMTL
 import io.homeassistant.companion.android.onboarding.welcome.navigation.WelcomeRoute
 import io.homeassistant.companion.android.onboarding.welcome.navigation.welcomeScreen
 import io.homeassistant.companion.android.util.canGoBack
-import io.homeassistant.companion.android.util.compose.locationPermissions
 import io.homeassistant.companion.android.util.compose.navigateToUri
 import kotlinx.serialization.Serializable
 
@@ -84,23 +80,25 @@ data class WearOnboardingRoute(val wearName: String, val urlToOnboard: String? =
  * - Server accessibility (local vs public)
  * - App flavor (full with Google Play Services vs minimal FOSS)
  * - Connection security (HTTP vs HTTPS)
- * - Permission state (location access)
  *
  * ## Flow Overview
- * 1. Welcome screen (only shown if [skipWelcome] is false)
- * 2. Server discovery (only shown if [urlToOnboard] is empty)
- * 3. Connection
- * 3. Device naming and registration
- * 4. Location/security configuration (conditional)
- * 5. Home network configuration (if applicable)
+ * 1. Welcome screen (skipped if [skipWelcome] is true)
+ * 2. Server discovery (skipped if [urlToOnboard] is provided)
+ * 3. Connection and authentication
+ * 4. Device naming and registration
+ * 5. Post-registration flow based on server configuration:
+ *    - Local-only servers: Local first screen → (full flavor only: Location sharing →) Secure connection (HTTP) or done (HTTPS)
+ *    - Public servers with full flavor: Location sharing → Secure connection (HTTP) or done (HTTPS)
+ *    - Public servers with minimal flavor: Secure connection (HTTP) or done (HTTPS)
+ * 6. Home network configuration (if user opts for secure-only connection)
  *
  * @param navController Navigation controller for managing navigation actions
  * @param onShowSnackbar Callback to display snackbar messages to the user
  * @param onOnboardingDone Callback invoked when onboarding completes successfully
  * @param urlToOnboard Optional server URL to onboard directly, bypassing server discovery
  * @param hideExistingServers When true, hides already registered servers from discovery
  * @param skipWelcome When true, skips the welcome screen and goes directly to server discovery or connection
- * @param hasLocationTracking Whether location tracking is available (default to full flavor = true, minimal = false)
+ * @param hasLocationTracking Whether location tracking is available (full flavor = true, minimal = false)
  */
 internal fun NavGraphBuilder.onboarding(
     navController: NavController,
@@ -172,7 +170,7 @@ internal fun NavGraphBuilder.onboarding(
                         navOptions = navOptions,
                     )
                 } else {
-                    navController.navigateForMinimalFlavor(
+                    navController.navigateToLocationForSecureConnectionConditionally(
                         serverId = serverId,
                         hasPlainTextAccess = hasPlainTextAccess,
                         navOptions = navOptions,
@@ -187,12 +185,12 @@ internal fun NavGraphBuilder.onboarding(
                 navController.navigateToUri(URL_GETTING_STARTED_DOCUMENTATION)
             },
             onGotoNextScreen = { serverId, hasPlainTextAccess ->
-                navController.navigateForMinimalFlavor(
+                navController.navigateToLocationForSecureConnectionConditionally(
                     serverId = serverId,
-                    hasPlainTextAccess = hasPlainTextAccess,
                     navOptions = navOptions {
                         popUpTo<LocationSharingRoute> { inclusive = true }
                     },
+                    hasPlainTextAccess = hasPlainTextAccess,
                     onOnboardingDone = onOnboardingDone,
                 )
             },
@@ -297,26 +295,13 @@ private fun NavGraphBuilder.commonScreens(navController: NavController, wearName
     )
 }
 
-/**
- * Checks if location permissions need to be requested.
- *
- * @return `true` if any location permission is not granted, `false` if all are granted
- */
-private fun NavController.shouldRequestLocationPermissions(): Boolean {
-    return locationPermissions.fastAny {
-        ContextCompat.checkSelfPermission(context, it) == PackageManager.PERMISSION_DENIED
-    }
-}
-
 /**
  * Navigates to the appropriate next screen after device registration based on server configuration.
  *
- * This function encapsulates the complex decision tree for post-registration navigation,
- * considering:
- * - Whether the server is publicly accessible
- * - Whether location tracking is available (full vs minimal flavor)
- * - Whether the connection uses plain text HTTP
- * - Whether location permissions are already granted
+ * Navigation decision tree:
+ * - Local-only server (!isPubliclyAccessible) → Local first screen
+ * - Public server with location tracking (full flavor) → Location sharing screen
+ * - Public server without location tracking (minimal flavor) → Secure connection (HTTP) or done (HTTPS)
  *
  * @param serverId The ID of the registered server
  * @param hasPlainTextAccess Whether the server connection uses HTTP (insecure)
@@ -346,8 +331,7 @@ private fun NavController.navigateAfterDeviceRegistration(
             hasPlainTextAccess = hasPlainTextAccess,
             navOptions = navOptions,
         )
-        // Minimal flavor: handle location and security based on connection type
-        else -> navigateForMinimalFlavor(
+        else -> navigateToLocationForSecureConnectionConditionally(
             serverId = serverId,
             hasPlainTextAccess = hasPlainTextAccess,
             navOptions = navOptions,
@@ -357,14 +341,13 @@ private fun NavController.navigateAfterDeviceRegistration(
 }
 
 /**
- * Navigates to the appropriate screen for minimal flavor after location-related setup.
+ * Conditionally navigates to the secure connection screen based on connection security.
  *
- * For minimal flavor (without location tracking), the flow is:
- * - If HTTPS: onboarding is complete
- * - If HTTP and no location permission: ask for location to enable secure connection detection
- * - If HTTP and has location permission: configure home network
+ * Navigation decision:
+ * - HTTPS connection (!hasPlainTextAccess) → Onboarding complete
+ * - HTTP connection → Navigate to secure connection screen to configure home network detection
  */
-private fun NavController.navigateForMinimalFlavor(
+private fun NavController.navigateToLocationForSecureConnectionConditionally(
     serverId: Int,
     hasPlainTextAccess: Boolean,
     navOptions: NavOptions,
@@ -376,12 +359,7 @@ private fun NavController.navigateForMinimalFlavor(
         return
     }
 
-    // HTTP connection: need location for secure connection detection
-    if (shouldRequestLocationPermissions()) {
-        navigateToLocationForSecureConnection(serverId = serverId, navOptions = navOptions)
-    } else {
-        navigateToSetHomeNetworkRoute(serverId = serverId, navOptions = navOptions)
-    }
+    navigateToLocationForSecureConnection(serverId = serverId, navOptions = navOptions)
 }
 
 /**

app/src/test/kotlin/io/homeassistant/companion/android/onboarding/localfirst/navigation/LocalFirstNavigationTest.kt

@@ -10,7 +10,6 @@ import io.homeassistant.companion.android.common.R as commonR
 import io.homeassistant.companion.android.onboarding.BaseOnboardingNavigationTest
 import io.homeassistant.companion.android.onboarding.locationforsecureconnection.navigation.LocationForSecureConnectionRoute
 import io.homeassistant.companion.android.onboarding.locationsharing.navigation.LocationSharingRoute
-import io.homeassistant.companion.android.onboarding.sethomenetwork.navigation.SetHomeNetworkRoute
 import io.homeassistant.companion.android.onboarding.welcome.navigation.WelcomeRoute
 import io.homeassistant.companion.android.testing.unit.stringResource
 import junit.framework.TestCase.assertTrue
@@ -46,9 +45,8 @@ internal class LocalFirstNavigationTest : BaseOnboardingNavigationTest() {
     }
 
     @Test
-    fun `Given no location tracking from LocalFirst with HTTP and no permission when next clicked then show LocationForSecureConnection`() {
+    fun `Given no location tracking from LocalFirst with HTTP when next clicked then show LocationForSecureConnection`() {
         testNavigation(hasLocationTracking = false) {
-            mockCheckPermission(false)
             navController.navigateToLocalFirst(serverId = 42, hasPlainTextAccess = true)
             assertTrue(navController.currentBackStackEntry?.destination?.hasRoute<LocalFirstRoute>() == true)
 
@@ -62,21 +60,6 @@ internal class LocalFirstNavigationTest : BaseOnboardingNavigationTest() {
         }
     }
 
-    @Test
-    fun `Given no location tracking from LocalFirst with HTTP and has permission when next clicked then show SetHomeNetwork`() {
-        testNavigation(hasLocationTracking = false) {
-            mockCheckPermission(true)
-            navController.navigateToLocalFirst(serverId = 42, hasPlainTextAccess = true)
-            assertTrue(navController.currentBackStackEntry?.destination?.hasRoute<LocalFirstRoute>() == true)
-
-            onNodeWithText(stringResource(commonR.string.local_first_next))
-                .performScrollTo()
-                .performClick()
-
-            assertTrue(navController.currentBackStackEntry?.destination?.hasRoute<SetHomeNetworkRoute>() == true)
-        }
-    }
-
     @Test
     fun `Given no location tracking from LocalFirst with HTTPS when next clicked then onboarding completes`() {
         testNavigation(hasLocationTracking = false) {

app/src/test/kotlin/io/homeassistant/companion/android/onboarding/locationsharing/navigation/LocationSharingNavigationTest.kt

@@ -9,7 +9,6 @@ import dagger.hilt.android.testing.HiltTestApplication
 import io.homeassistant.companion.android.common.R as commonR
 import io.homeassistant.companion.android.onboarding.BaseOnboardingNavigationTest
 import io.homeassistant.companion.android.onboarding.locationforsecureconnection.navigation.LocationForSecureConnectionRoute
-import io.homeassistant.companion.android.onboarding.sethomenetwork.navigation.SetHomeNetworkRoute
 import io.homeassistant.companion.android.onboarding.welcome.navigation.WelcomeRoute
 import io.homeassistant.companion.android.testing.unit.stringResource
 import junit.framework.TestCase.assertTrue
@@ -28,7 +27,7 @@ import org.robolectric.annotation.Config
 internal class LocationSharingNavigationTest : BaseOnboardingNavigationTest() {
 
     @Test
-    fun `Given LocationSharing when agreeing with plain text access to share then show SetHomeNetwork`() {
+    fun `Given LocationSharing when agreeing with plain text access to share then show LocationForSecureConnection then goes back stop the app`() {
         testNavigation {
             navController.navigateToLocationSharing(42, true)
             assertTrue(navController.currentBackStackEntry?.destination?.hasRoute<LocationSharingRoute>() == true)
@@ -38,7 +37,12 @@ internal class LocationSharingNavigationTest : BaseOnboardingNavigationTest() {
                 .performScrollTo()
                 .performClick()
 
-            assertTrue(navController.currentBackStackEntry?.destination?.hasRoute<SetHomeNetworkRoute>() == true)
+            assertTrue(navController.currentBackStackEntry?.destination?.hasRoute<LocationForSecureConnectionRoute>() == true)
+
+            composeTestRule.activity.onBackPressedDispatcher.onBackPressed()
+
+            // In the test scenario since we never opened NameYourDevice the stack still contains Welcome
+            assertTrue(navController.currentBackStackEntry?.destination?.hasRoute<WelcomeRoute>() == true)
         }
     }
 

app/src/test/kotlin/io/homeassistant/companion/android/onboarding/nameyourdevice/navigation/NameYourDeviceNavigationTest.kt

@@ -16,9 +16,7 @@ import io.homeassistant.companion.android.onboarding.locationforsecureconnection
 import io.homeassistant.companion.android.onboarding.locationsharing.navigation.LocationSharingRoute
 import io.homeassistant.companion.android.onboarding.nameyourdevice.NameYourDeviceNavigationEvent
 import io.homeassistant.companion.android.onboarding.nameyourdevice.NameYourDeviceViewModel
-import io.homeassistant.companion.android.onboarding.sethomenetwork.navigation.SetHomeNetworkRoute
 import io.homeassistant.companion.android.testing.unit.stringResource
-import io.mockk.coEvery
 import io.mockk.every
 import io.mockk.mockk
 import junit.framework.TestCase.assertTrue
@@ -150,7 +148,7 @@ internal class NameYourDeviceNavigationTest : BaseOnboardingNavigationTest() {
     }
 
     @Test
-    fun `Given no location tracking with HTTP public server and no location permission when device named then show LocationForSecureConnection`() {
+    fun `Given no location tracking with HTTP public server when device named then show LocationForSecureConnection`() {
         every { nameYourDeviceViewModel.onSaveClick() } coAnswers {
             nameYourDeviceNavigationFlow.emit(
                 NameYourDeviceNavigationEvent.DeviceNameSaved(
@@ -161,7 +159,6 @@ internal class NameYourDeviceNavigationTest : BaseOnboardingNavigationTest() {
             )
         }
         testNavigation(hasLocationTracking = false) {
-            mockCheckPermission(false)
             navController.navigateToNameYourDevice("http://homeassistant.local", "code")
             assertTrue(navController.currentBackStackEntry?.destination?.hasRoute<NameYourDeviceRoute>() == true)
 
@@ -176,30 +173,4 @@ internal class NameYourDeviceNavigationTest : BaseOnboardingNavigationTest() {
             )
         }
     }
-
-    @Test
-    fun `Given no location tracking with HTTP public server and has location permission when device named then show SetHomeNetwork`() {
-        coEvery { nameYourDeviceViewModel.onSaveClick() } coAnswers {
-            nameYourDeviceNavigationFlow.emit(
-                NameYourDeviceNavigationEvent.DeviceNameSaved(
-                    serverId = 42,
-                    hasPlainTextAccess = true,
-                    isPubliclyAccessible = true,
-                ),
-            )
-        }
-        testNavigation(hasLocationTracking = false) {
-            mockCheckPermission(true)
-            navController.navigateToNameYourDevice("http://homeassistant.local", "code")
-            assertTrue(navController.currentBackStackEntry?.destination?.hasRoute<NameYourDeviceRoute>() == true)
-
-            onNodeWithText(stringResource(commonR.string.name_your_device_save))
-                .performScrollTo()
-                .assertIsDisplayed()
-                .assertIsEnabled()
-                .performClick()
-
-            assertTrue(navController.currentBackStackEntry?.destination?.hasRoute<SetHomeNetworkRoute>() == true)
-        }
-    }
 }