From cf82dd5d78c64010df7cd4a8bafacd0de0fa05cc Mon Sep 17 00:00:00 2001 From: James Maes Date: Fri, 6 Nov 2026 10:40:01 -0600 Subject: [PATCH] fix(auth): run provider logout before host session end; show OAuth user name Company sign-out on the MC9400 (production APK, real Authentik) never opened the browser: QViewModel.performLogout ran the host beforeLogout hook (scanner device-session end, which runs the core logout server-side) before the provider logout request, so POST /qqq/v1/logout found no session and returned no nativeLogoutData. Company sign-out now requests the provider material first, then ends the host session (401 tolerated), then clears local state. Ordinary sign-out keeps the host-hook-first order. Browser login now adopts the server-reported values.user.name so Account no longer shows a blank current user. Adds QViewModelLogoutTest (ordering, failing host end, ordinary order, user name adoption, ManageSessionResponse.userName parsing) and documents the finding in AUTHENTICATION.md and SESSION-STATE.md. Co-Authored-By: Claude Fable 5.1 --- docs/AUTHENTICATION.md | 2 + docs/SESSION-STATE.md | 1 + .../authentication/ManageSessionResponse.kt | 5 + qqq/android/mobileapp/build.gradle.kts | 3 + .../android/mobileapp/viewmodel/QViewModel.kt | 34 ++- .../viewmodel/QViewModelLogoutTest.kt | 224 ++++++++++++++++++ 6 files changed, 260 insertions(+), 9 deletions(-) create mode 100644 qqq/android/mobileapp/src/test/java/com/kingsrook/qqq/frontend/android/mobileapp/viewmodel/QViewModelLogoutTest.kt diff --git a/docs/AUTHENTICATION.md b/docs/AUTHENTICATION.md index 7e81baa..460d581 100644 --- a/docs/AUTHENTICATION.md +++ b/docs/AUTHENTICATION.md @@ -67,6 +67,8 @@ Ordinary **Sign out** continues to end only the QQQ session. The scanner also ex Register the installed package's exact callback separately as a provider **post-logout** redirect, even when it matches the existing login callback. The client sends `{"nativeLogout":{"postLogoutRedirectUri":""}}` to the existing `POST /qqq/v1/logout`. Its optional `nativeLogoutData` contains `issuer`, `endSessionEndpoint`, `postLogoutRedirectUri` and sensitive `idTokenHint`. Old sessions or unsupported servers may legitimately omit this data. The ordinary logout request remains unchanged. +**Ordering (fix prepared 2026-11-05, not released):** for company sign-out the client sends the provider logout request *before* running the host's `beforeLogout` hook, then clears local state, then opens the browser. The scanner host's hook ends the device session (`POST /qbit/mobile-scanners/sessions/current/end`), which on the server runs the core logout and removes the QQQ session; running it first left no session for `POST /qqq/v1/logout` to describe, so `nativeLogoutData` was never returned and the browser never opened (observed on an MC9400 against a real Authentik fixture). The device-session end may now answer 401 because the login already ended; the client tolerates that and still clears local authentication. Ordinary sign-out is unchanged: host hook first, then the core logout. + The client clears QQQ session/private process state before opening AppAuth's end-session browser. It independently fetches discovery from the original login binding and checks the issuer, HTTPS end-session endpoint (without user information, query or fragment), and exact callback. A distinct in-memory, single-use state attempt binds completion to server, provider and endpoint. Login cannot start during company logout; cancellation, wrong state, reset or a late callback cannot restore a QQQ session. Browser cancellation or unavailable data explicitly reports incomplete company sign-out. Returning from the provider is protocol completion, not an independent claim about every upstream SSO session. The hint is deliberately held in transient ordinary classes with no generated value `toString`, never application preferences, ViewModel state, diagnostics, process values or application saved state. Parsing failures and HTTP diagnostics do not expose response bodies. AppAuth0.11.1 necessarily serializes its end-session request into Android Intents/saved Activity state, and the browser receives the hint in a URL query. This is not a zero-persistence guarantee. Do not enable URI/body logging, browser traces or proxy access logs containing query strings. AppAuth's warning on a state mismatch can contain state values; do not treat SDK debug output as a safe diagnostics feed. Native synthetic tests include a hint logcat/preferences canary; real-provider and hardware qualification are separate gates. diff --git a/docs/SESSION-STATE.md b/docs/SESSION-STATE.md index c1eaf76..650c221 100644 --- a/docs/SESSION-STATE.md +++ b/docs/SESSION-STATE.md @@ -10,6 +10,7 @@ As of **2026-11-05**. Scope: scanner host and maintained local Echo workflow. [T - **Network:** the local QQQ server stays on a PC, reached over trusted HTTPS temporarily through a user-approved private Tailscale network. This is not a customer production deployment; customer instructions require a customer HTTPS URL with a trusted certificate. TLS is not weakened; no hostnames, IPs, ports, device/vault IDs, private paths or passwords are published. - **Server/Authentik:** real shared Authentik server discovery verified (UI API 1.10, strict nonce, scanner and printing channels/extensions/callbacks). Production-APK native login/logout/account switching is **open**, awaiting the MC wireless-debug endpoint; no native-Authentik PASS is claimed. Print nonce fix PR 11 (merged `abc3d0d`, 532 hosted tests passed) postdates published print 1.1.1; the next installer remains with the active printing owner. - **Photo policy:** live. A signed MC9400 gallery synthetic PNG became a Saved photo, `AVAILABLE`, with exact parent/type/size/hash; the previous three photos were released normally. The app has no admission policy; the server receipt controls the message. The root is running the updated disposable TC22 E2E; no full new-preview or native PASS is claimed until it completes. +- **Company sign-out bug (2026-11-05):** on the physical MC9400 with the production APK against the real Authentik fixture, **Sign out of company** never opened the browser and reported "Company sign-out did not complete". Cause: `QViewModel.performLogout` ran the host `beforeLogout` hook (scanner device-session end, which runs the core logout server-side) before the provider logout request, so `nativeLogoutData` was never returned. A fix reordering provider logout before the host hook, plus adoption of the server-reported user name after browser login (Account showed a blank "Current User"), is prepared on branch `fix/company-signout-before-session-end` with unit tests; it is **not released** and no new APK is built. See [AUTHENTICATION](AUTHENTICATION.md#optional-company-sign-out-implementation-awaiting-qualification). - **Open:** public compatible Core/Collab/document/files/scripting/bridges, final public cohort gate, GS1 physical print/rescan, full release-asset publication/distribution. Company profiles/wallpaper are tracked separately; signing CI is optional and not blocking a direct APK release. Release authorizations already exist. Statements below were written before these facts and are historical where they conflict (for example "debug update", MC9400 trigger check "outstanding", signed-build HTTPS qualification "open"). diff --git a/qqq/android/core/src/main/java/com/kingsrook/qqq/frontend/android/core/model/metadata/authentication/ManageSessionResponse.kt b/qqq/android/core/src/main/java/com/kingsrook/qqq/frontend/android/core/model/metadata/authentication/ManageSessionResponse.kt index d0d3397..e97b812 100644 --- a/qqq/android/core/src/main/java/com/kingsrook/qqq/frontend/android/core/model/metadata/authentication/ManageSessionResponse.kt +++ b/qqq/android/core/src/main/java/com/kingsrook/qqq/frontend/android/core/model/metadata/authentication/ManageSessionResponse.kt @@ -31,4 +31,9 @@ data class ManageSessionResponse( val uuid: String? = null, val values: MapOfStringAny = emptyMap(), ) { + /******************************************************************************* + ** Display name from the optional values.user object; absent, non-object or blank names yield null. + *******************************************************************************/ + val userName: String? + get() = ((values["user"] as? Map<*, *>)?.get("name") as? String)?.takeIf { it.isNotBlank() } } diff --git a/qqq/android/mobileapp/build.gradle.kts b/qqq/android/mobileapp/build.gradle.kts index d64a319..2e1548e 100644 --- a/qqq/android/mobileapp/build.gradle.kts +++ b/qqq/android/mobileapp/build.gradle.kts @@ -72,6 +72,9 @@ android { kotlinOptions { jvmTarget = "1.8" } + testOptions { + unitTests.isReturnDefaultValues = true + } } dependencies { diff --git a/qqq/android/mobileapp/src/main/java/com/kingsrook/qqq/frontend/android/mobileapp/viewmodel/QViewModel.kt b/qqq/android/mobileapp/src/main/java/com/kingsrook/qqq/frontend/android/mobileapp/viewmodel/QViewModel.kt index 7f8ebc9..291b1c7 100644 --- a/qqq/android/mobileapp/src/main/java/com/kingsrook/qqq/frontend/android/mobileapp/viewmodel/QViewModel.kt +++ b/qqq/android/mobileapp/src/main/java/com/kingsrook/qqq/frontend/android/mobileapp/viewmodel/QViewModel.kt @@ -582,8 +582,23 @@ open class QViewModel( *******************************************************************************/ fun logOut(): Job = performLogout() + /******************************************************************************* + ** Host session resources end in a bounded, best-effort step; core logout and local clearing never depend on it. + *******************************************************************************/ + private suspend fun endHostSession() { + try { + kotlinx.coroutines.withTimeoutOrNull(3000) { beforeLogout() } + } catch (exception: kotlinx.coroutines.CancellationException) { + throw exception + } catch (exception: Exception) { /* Core logout and local clearing still run. */ + } + } + /******************************************************************************* ** Optional provider material is captured only by the initiating coroutine, never by ViewModel state. + ** Ordinary logout ends host resources first. Provider logout runs first instead: it both returns the + ** transient hint and ends the QQQ session, and a host session end (such as the scanner device-session end, + ** which itself runs the core logout server-side) would otherwise remove that session before the hint is read. *******************************************************************************/ private fun performLogout(providerLogout: (suspend () -> Unit)? = null): Job { logoutJob?.takeIf { @@ -606,16 +621,15 @@ open class QViewModel( var serverConfirmed = false try { settingsMutex.withLock { settingsRepository.setSessionUUID(""); settingsRepository.setSessionUserFullName("") } + if (providerLogout == null) endHostSession() try { - kotlinx.coroutines.withTimeoutOrNull(3000) { beforeLogout() } - } catch (exception: kotlinx.coroutines.CancellationException) { - throw exception - } catch (exception: Exception) { /* Core logout and local clearing still run. */ + serverConfirmed = kotlinx.coroutines.withTimeoutOrNull(3000) { + if (providerLogout == null) qqqRepository.logout() else providerLogout() + true + } ?: false + } finally { + if (providerLogout != null) endHostSession() } - serverConfirmed = kotlinx.coroutines.withTimeoutOrNull(3000) { - if (providerLogout == null) qqqRepository.logout() else providerLogout() - true - } ?: false } catch (exception: kotlinx.coroutines.CancellationException) { throw exception } catch (exception: Exception) { /* Local sign-out still completes when the network is unavailable. */ @@ -672,7 +686,8 @@ open class QViewModel( binding.login.server == environment.baseUrl && isLogoutRequested /******************************************************************************* - ** Local invalidation finishes before the browser sees the optional transient hint. + ** Local invalidation finishes before the browser sees the optional transient hint: the provider logout + ** request runs inside performLogout, whose completion clears local state before this returns. *******************************************************************************/ suspend fun logOutWithProvider(generation: Long, binding: OAuth2LogoutBinding): NativeLogoutData? { if (!isCurrentCompanyLogout(generation, binding) || companyLogoutPhase != OAuth2LogoutPhase.PREPARING) return null @@ -846,6 +861,7 @@ open class QViewModel( if (!isCurrentOAuth2Login(generation, binding)) return@launch sessionUUID = requireNotNull(newSession) qqqRepository.setSessionUUID(sessionUUID) + response.userName?.let { doSetSessionUserFullName(it) } manageSessionLoadState = LoadState.Success(response) logInSuccessful() adopted = true diff --git a/qqq/android/mobileapp/src/test/java/com/kingsrook/qqq/frontend/android/mobileapp/viewmodel/QViewModelLogoutTest.kt b/qqq/android/mobileapp/src/test/java/com/kingsrook/qqq/frontend/android/mobileapp/viewmodel/QViewModelLogoutTest.kt new file mode 100644 index 0000000..f942eda --- /dev/null +++ b/qqq/android/mobileapp/src/test/java/com/kingsrook/qqq/frontend/android/mobileapp/viewmodel/QViewModelLogoutTest.kt @@ -0,0 +1,224 @@ +/* Copyright (c) 2026 QRunIO. SPDX-License-Identifier: Apache-2.0 */ +package com.kingsrook.qqq.frontend.android.mobileapp.viewmodel + +import com.kingsrook.qqq.frontend.android.core.controllers.QQQRepository +import com.kingsrook.qqq.frontend.android.core.model.Environment +import com.kingsrook.qqq.frontend.android.core.model.metadata.QInstance +import com.kingsrook.qqq.frontend.android.core.model.metadata.QProcessMetaData +import com.kingsrook.qqq.frontend.android.core.model.metadata.authentication.BaseAuthenticationMetaData +import com.kingsrook.qqq.frontend.android.core.model.metadata.authentication.ManageSessionRequest +import com.kingsrook.qqq.frontend.android.core.model.metadata.authentication.ManageSessionResponse +import com.kingsrook.qqq.frontend.android.core.model.metadata.authentication.NativeLogoutData +import com.kingsrook.qqq.frontend.android.core.model.metadata.authentication.OAuth2AuthenticationMetaData +import com.kingsrook.qqq.frontend.android.core.model.processes.ProcessStepResult +import com.kingsrook.qqq.frontend.android.mobileapp.container.QAppContainer +import com.kingsrook.qqq.frontend.android.mobileapp.data.MockPreferencesDataStore +import com.kingsrook.qqq.frontend.android.mobileapp.ui.authentication.OAuth2LoginBinding +import com.kingsrook.qqq.frontend.android.mobileapp.ui.authentication.OAuth2LogoutBinding +import com.kingsrook.qqq.frontend.android.mobileapp.ui.authentication.OAuth2LogoutPhase +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.StandardTestDispatcher +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain +import kotlinx.serialization.json.Json +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +private const val SERVER = "https://example.invalid" +private const val ISSUER = "https://idp.example.invalid" +private const val REDIRECT = "io.qrun.test.oauth://callback/oauth2redirect" +private const val STATE = "state-0123456789abcdef" +private const val NONCE = "nonce-0123456789abcdef" +private const val VERIFIER = "verifier-0123456789abcdefghijklmnopqrstuvwxyz0123456789" + +/******************************************************************************* + ** Ordering of host session end, provider logout and local clearing; identity adoption after browser login. + *******************************************************************************/ +@OptIn(ExperimentalCoroutinesApi::class) +class QViewModelLogoutTest { + /******************************************************************************* + ** (a) Provider logout must read the hint before the host ends its device session (which ends the QQQ session). + *******************************************************************************/ + @Test + fun companyLogoutRequestsProviderMaterialBeforeHostSessionEnd() = runTest() + { + Dispatchers.setMain(StandardTestDispatcher(testScheduler)) + try { + val repository = LogoutRepository() + val model = signedInModel(repository) + model.beforeLogout = { repository.calls += "beforeLogout" } + val material = companyLogout(model) + assertNotNull(material) + assertEquals(listOf("logoutWithProvider", "beforeLogout"), repository.calls) + assertEquals("Signed out.", model.logoutMessage) + assertFalse(model.isAuthenticated) + assertEquals("", model.sessionUUID) + assertEquals("", model.sessionUserFullName) + } finally { + Dispatchers.resetMain() + } + } + + /******************************************************************************* + ** (b) A failing host session end after the provider logout neither loses the hint nor leaves local state. + *******************************************************************************/ + @Test + fun failingHostSessionEndAfterProviderLogoutKeepsMaterialAndClearsLocally() = runTest() + { + Dispatchers.setMain(StandardTestDispatcher(testScheduler)) + try { + val repository = LogoutRepository() + val model = signedInModel(repository) + model.beforeLogout = { repository.calls += "beforeLogout"; throw IllegalStateException("401 Unauthorized") } + val material = companyLogout(model) + assertNotNull(material) + assertEquals(listOf("logoutWithProvider", "beforeLogout"), repository.calls) + assertEquals("Signed out.", model.logoutMessage) + assertFalse(model.isAuthenticated) + assertFalse(model.logoutInProgress) + assertEquals("", model.sessionUUID) + assertEquals("", repository.activeSession) + } finally { + Dispatchers.resetMain() + } + } + + /******************************************************************************* + ** (c) Ordinary sign-out keeps ending host resources before the core logout. + *******************************************************************************/ + @Test + fun ordinaryLogoutEndsHostSessionBeforeCoreLogout() = runTest() + { + Dispatchers.setMain(StandardTestDispatcher(testScheduler)) + try { + val repository = LogoutRepository() + val model = signedInModel(repository) + model.beforeLogout = { repository.calls += "beforeLogout" } + model.requestLogout() + model.logOut().join() + assertEquals(listOf("beforeLogout", "logout"), repository.calls) + assertEquals("Signed out.", model.logoutMessage) + assertFalse(model.isAuthenticated) + assertEquals("", model.sessionUUID) + } finally { + Dispatchers.resetMain() + } + } + + /******************************************************************************* + ** Browser login adopts the server-reported display name, as password login already does. + *******************************************************************************/ + @Test + fun browserLoginAdoptsServerReportedUserName() = runTest() + { + Dispatchers.setMain(StandardTestDispatcher(testScheduler)) + try { + val repository = LogoutRepository() + val model = signedInModel(repository) + assertTrue(model.isAuthenticated) + assertEquals("scanner-b", model.sessionUserFullName) + } finally { + Dispatchers.resetMain() + } + } + + /******************************************************************************* + ** The user object is optional and unknown keys stay tolerated; blank or non-object users yield no name. + *******************************************************************************/ + @Test + fun manageSessionResponseExposesOptionalUserName() { + val json = Json { ignoreUnknownKeys = true } + val full = json.decodeFromString( + """{"uuid":"s","values":{"user":{"name":"scanner-b","email":"scanner-b@example.invalid","extra":1},"other":true},"unknown":"x"}""" + ) + assertEquals("scanner-b", full.userName) + assertNull(json.decodeFromString("""{"uuid":"s"}""").userName) + assertNull(json.decodeFromString("""{"uuid":"s","values":{"user":"scanner-b"}}""").userName) + assertNull(json.decodeFromString("""{"uuid":"s","values":{"user":{"name":" "}}}""").userName) + } + + /******************************************************************************* + ** Complete a native browser login against the fake repository so sign-out tests start authenticated. + *******************************************************************************/ + private fun TestScope.signedInModel(repository: LogoutRepository): QViewModel { + QAppContainer.availableEnvironments = listOf(Environment("Test", SERVER)) + val model = QViewModel(repository, MockPreferencesDataStore()) + advanceUntilIdle() + val generation = requireNotNull(model.prepareOAuth2Login(loginBinding)) + assertTrue(model.beginOAuth2Browser(generation, loginBinding, STATE, NONCE, VERIFIER)) + model.completeOAuth2Login(loginBinding, STATE, STATE, NONCE, "client", REDIRECT, "login", "code") + advanceUntilIdle() + assertTrue(model.isAuthenticated) + assertEquals("session-1", model.sessionUUID) + repository.calls.clear() + return model + } + + /******************************************************************************* + ** Run the company sign-out preparation exactly as QOAuth2Logout does before opening the browser. + *******************************************************************************/ + private suspend fun companyLogout(model: QViewModel): NativeLogoutData? { + val binding = OAuth2LogoutBinding(loginBinding, REDIRECT) + model.requestCompanyLogout(binding) + assertEquals(OAuth2LogoutPhase.PREPARING, model.companyLogoutPhase) + return model.logOutWithProvider(model.companyLogoutGeneration, binding) + } + + private val loginBinding = OAuth2LoginBinding( + SERVER, "oauth", ISSUER, "$ISSUER/.well-known/openid-configuration", "client", "openid", REDIRECT, "login", "REQUIRED" + ) + + /******************************************************************************* + ** Records the order of logout-related calls; any call after the session ended returns no provider material. + *******************************************************************************/ + private class LogoutRepository : QQQRepository { + val calls = mutableListOf() + var activeSession = "" + + override suspend fun getAuthenticationMetaData(): BaseAuthenticationMetaData = OAuth2AuthenticationMetaData( + "oauth", values = OAuth2AuthenticationMetaData.Values( + clientId = "client", scopes = "openid", + nativeSessionLogin = OAuth2AuthenticationMetaData.NativeSessionLogin( + OAuth2AuthenticationMetaData.SESSION_FLOW, "S256", ISSUER, "$ISSUER/.well-known/openid-configuration", listOf(REDIRECT), "login", "REQUIRED" + ), + nativeSessionLogout = OAuth2AuthenticationMetaData.NativeSessionLogout(OAuth2AuthenticationMetaData.LOGOUT_FLOW, listOf(REDIRECT)) + ) + ) + + override suspend fun exchangeNativeSession(body: ManageSessionRequest): ManageSessionResponse = + ManageSessionResponse("session-1", mapOf("user" to mapOf("name" to "scanner-b", "email" to "scanner-b@example.invalid"))) + + override suspend fun logout() { + calls += "logout" + activeSession = "" + } + + override suspend fun logoutWithProvider(postLogoutRedirectUri: String): NativeLogoutData? { + calls += "logoutWithProvider" + val material = if (activeSession.isBlank()) null else NativeLogoutData(ISSUER, "$ISSUER/end-session", postLogoutRedirectUri, "hint") + activeSession = "" + return material + } + + override fun setSessionUUID(sessionUUID: String) { + activeSession = sessionUUID + } + + override fun setEnvironment(environment: Environment) {} + override suspend fun manageSession(body: ManageSessionRequest): ManageSessionResponse = error("Not used") + override suspend fun getMetaData(frontendName: String, frontendVersion: String, applicationName: String, applicationVersion: String): QInstance = QInstance() + override suspend fun getProcessMetaData(processName: String): QProcessMetaData = error("Not used") + override suspend fun processInit(processName: String, values: Map, recordsParam: String?, recordIds: String?, filterJSON: String?, stepTimeoutMillis: Int?): ProcessStepResult = error("Not used") + override suspend fun processStep(processName: String, processUUID: String, stepName: String, values: Map, stepTimeoutMillis: Int?): ProcessStepResult = error("Not used") + override suspend fun processJobStatus(processName: String, processUUID: String, jobUUID: String): ProcessStepResult = error("Not used") + override fun getURI(path: String): String = error("Not used") + } +} -- 2.54.0