makiolaj/agendula
1
0
Fork 0
Code Issues Pull Requests Actions Releases 8 Activity

sync: the login flow is written down before the browser gets it

Browse Source 
Flow's own doc says to persist it before launching the browser, because
the flow outlives our process — and nothing did. The browser is a
separate task, so dying while the user approves is ordinary rather than
exotic, and it stranded a one-shot app password that nothing could then
collect or revoke: the poll token was the only way back to it and it
lived in a ViewModel field.

The token goes to the sync-state store, not the Keystore: it authorises
one poll of one flow the user is in the middle of approving, and it is
worthless past the twenty-minute window. It is cleared the moment the
flow stops mattering — spent, expired, cancelled, started over.

A genuine app open reclaims what a dead process left: poll once, and if
the user did approve, hand the password straight back. Revoked rather
than used, because the address they typed, the collections they ticked
and the account name went with the process — what is left is a live
credential in their device list under the same name as every other
attempt, which is exactly the one they cannot tell apart and so dare not
prune. Once per process, so a rotation cannot consume the one-shot 200 a
live wizard is waiting for.

The window after approval, where the password itself is only in memory,
stays open — that needs the wizard's own state to survive, which is the
same work as the wizard restructure.
This commit is contained in:
makiolaj
2026-09-09 11:39:21 +02:00
parent 1119507585
commit ead84c3195
5 changed files with 259 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/src/main/java/de/jeanlucmakiola/agendula/MainActivity.kt
+6
View File
@@ -21,6 +21,7 @@ import de.jeanlucmakiola.agendula.data.prefs.ThemeMode
import de.jeanlucmakiola.agendula.ui.RootScreen
import de.jeanlucmakiola.agendula.ui.crash.CrashReportActivity
import de.jeanlucmakiola.agendula.data.sync.AccountRepository
import de.jeanlucmakiola.agendula.data.sync.PendingLoginFlowStore
import de.jeanlucmakiola.agendula.data.sync.SyncTrigger
import de.jeanlucmakiola.agendula.ui.settings.SettingsViewModel
import de.jeanlucmakiola.agendula.ui.theme.AgendulaTheme
@@ -44,6 +45,8 @@ class MainActivity : ComponentActivity() {
@Inject lateinit var syncTrigger: SyncTrigger
@Inject lateinit var pendingLoginFlows: PendingLoginFlowStore
// A captured crash report awaiting the user's decision, surfaced as a dialog
// over the app on the next launch (the single-crash path). A startup
// crash-loop is handled out of band, before setContent — see below.
@@ -80,6 +83,9 @@ class MainActivity : ComponentActivity() {
runCatching {
accounts.rescheduleAll()
accounts.syncable().forEach { syncTrigger.enqueue(it.displayName) }
// A login flow the previous process died in the middle of.
// Its password, if the user approved, exists nowhere else.
pendingLoginFlows.reclaim()
}
}
}
app/src/main/java/de/jeanlucmakiola/agendula/data/di/DataModule.kt
+6
View File
@@ -15,6 +15,8 @@ import dagger.hilt.components.SingletonComponent
import de.jeanlucmakiola.agendula.data.sync.AccountCreator
import de.jeanlucmakiola.agendula.data.sync.AccountRepository
import de.jeanlucmakiola.agendula.data.sync.CalDavGateway
import de.jeanlucmakiola.agendula.data.sync.LoginFlowRecord
import de.jeanlucmakiola.agendula.data.sync.PendingLoginFlowStore
import de.jeanlucmakiola.agendula.data.sync.OkHttpCalDavGateway
import de.jeanlucmakiola.agendula.data.tasks.AndroidProviderEnvironment
import de.jeanlucmakiola.agendula.data.tasks.AndroidTasksDataSource
@@ -78,6 +80,10 @@ abstract class DataBindModule {
@Binds
@Singleton
abstract fun bindAccountCreator(impl: AccountRepository): AccountCreator
@Binds
@Singleton
abstract fun bindLoginFlowRecord(impl: PendingLoginFlowStore): LoginFlowRecord
}
@Module
app/src/main/java/de/jeanlucmakiola/agendula/data/sync/PendingLoginFlowStore.kt
+144
View File
@@ -0,0 +1,144 @@
package de.jeanlucmakiola.agendula.data.sync
import androidx.datastore.core.DataStore
import androidx.datastore.preferences.core.Preferences
import androidx.datastore.preferences.core.edit
import androidx.datastore.preferences.core.longPreferencesKey
import androidx.datastore.preferences.core.stringPreferencesKey
import de.jeanlucmakiola.agendula.data.di.SyncStateDataStore
import de.jeanlucmakiola.caldav.NextcloudLoginFlow
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
import okhttp3.HttpUrl.Companion.toHttpUrlOrNull
import javax.inject.Inject
import javax.inject.Singleton
/**
* Where a started login flow is written down, so it can outlive this process.
*
* A seam for the same reason [AccountCreator] and [CalDavGateway] are: the flow
* that decides *when* a one-shot password stops being ours has to be testable
* without a DataStore.
*/
interface LoginFlowRecord {
/** Called **before** the browser is handed the URL. */
suspend fun remember(flow: NextcloudLoginFlow.Flow)
/** The flow is over, however it ended. */
suspend fun forget()
}
/**
* The Nextcloud login flow that is currently out at a browser.
*
* ⚠️ [NextcloudLoginFlow.Flow]'s own doc says to persist it **before** launching
* the browser, because the flow outlives our process — and it did not. The
* browser is a separate task, so process death while the user is approving is
* ordinary rather than exotic, and it stranded a one-shot app password that
* nothing could then collect *or* revoke: the flow's poll token was the only way
* back to it, and it lived in a ViewModel field.
*
* The token is not a credential. It authorises exactly one poll of one flow the
* user is in the middle of approving, and it is useless past the twenty-minute
* window — so it belongs in the sync-state store rather than the Keystore.
*
* ⚠️ What this does **not** cover is the window *after* approval, where the
* password itself lives only in memory. That needs the wizard's own state to
* survive, which is a different piece of work.
*/
@Singleton
class PendingLoginFlowStore @Inject constructor(
@SyncStateDataStore private val dataStore: DataStore<Preferences>,
private val gateway: CalDavGateway,
) : LoginFlowRecord {
private val lock = Mutex()
private var reclaimed = false
/** Records [flow] so a process that dies mid-approval can still finish with it. */
override suspend fun remember(flow: NextcloudLoginFlow.Flow) {
dataStore.edit { prefs ->
prefs[LOGIN_URL] = flow.loginUrl.toString()
prefs[POLL_ENDPOINT] = flow.pollEndpoint.toString()
prefs[POLL_TOKEN] = flow.pollToken
prefs[DEADLINE] = flow.deadlineEpochSeconds
}
}
/** The flow is finished, one way or another. */
override suspend fun forget() {
dataStore.edit { prefs ->
prefs.remove(LOGIN_URL)
prefs.remove(POLL_ENDPOINT)
prefs.remove(POLL_TOKEN)
prefs.remove(DEADLINE)
}
}
/**
* Collects and hands back a password nobody is left to own.
*
* Revoked rather than used: the address the user typed, the collections they
* ticked and the account name are all gone with the process, so there is
* nothing to finish. What is left is a live app password in the user's
* device list, under the same name as every other attempt — which is exactly
* what they cannot tell apart, and so dare not prune.
*
* ⚠️ **Once per process.** This activity is recreated on every rotation,
* theme switch and locale change, and a second run against a flow the *live*
* wizard is still polling would consume its one-shot 200 and revoke the
* password it was about to be handed. A flow remembered after this has run
* belongs to a wizard that is alive to finish it.
*/
suspend fun reclaim() {
lock.withLock {
if (reclaimed) return
reclaimed = true
}
val flow = pending() ?: return
when (val result = gateway.pollLoginFlow(flow)) {
is NextcloudLoginFlow.PollResult.Approved -> {
// Cleared first: a revocation that fails must not leave a token
// that would be polled again, and the 200 is already spent.
forget()
gateway.revokeIssuedAppPassword(
CalDavGateway.Credentials(
username = result.credentials.loginName,
password = result.credentials.appPassword,
origin = result.credentials.server,
),
)
}
is NextcloudLoginFlow.PollResult.Expired -> forget()
// Still inside the window, or the server had a moment. Either way the
// token is still worth something; the deadline retires it.
NextcloudLoginFlow.PollResult.Pending,
is NextcloudLoginFlow.PollResult.Failed,
-> Unit
}
}
private suspend fun pending(): NextcloudLoginFlow.Flow? {
val prefs = dataStore.data.first()
val endpoint = prefs[POLL_ENDPOINT]?.toHttpUrlOrNull() ?: return null
val token = prefs[POLL_TOKEN] ?: return null
val deadline = prefs[DEADLINE] ?: return null
return NextcloudLoginFlow.Flow(
loginUrl = prefs[LOGIN_URL]?.toHttpUrlOrNull() ?: endpoint,
pollEndpoint = endpoint,
pollToken = token,
deadlineEpochSeconds = deadline,
)
}
private companion object {
val LOGIN_URL = stringPreferencesKey("login_flow_url")
val POLL_ENDPOINT = stringPreferencesKey("login_flow_poll_endpoint")
val POLL_TOKEN = stringPreferencesKey("login_flow_poll_token")
val DEADLINE = longPreferencesKey("login_flow_deadline")
}
}
app/src/main/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountViewModel.kt
+20 -1
View File
@@ -6,6 +6,7 @@ import dagger.hilt.android.lifecycle.HiltViewModel
import de.jeanlucmakiola.agendula.data.sync.AccountCreator
import de.jeanlucmakiola.agendula.data.sync.AccountRepository
import de.jeanlucmakiola.agendula.data.sync.CalDavGateway
import de.jeanlucmakiola.agendula.data.sync.LoginFlowRecord
import de.jeanlucmakiola.caldav.CalDavDiscovery
import de.jeanlucmakiola.caldav.NextcloudLoginFlow
import de.jeanlucmakiola.caldav.ServerQuirk
@@ -91,6 +92,7 @@ data class AddAccountUiState(
class AddAccountViewModel @Inject constructor(
private val repository: AccountCreator,
private val gateway: CalDavGateway,
private val pendingFlow: LoginFlowRecord,
// ⚠️ Not viewModelScope. Handing the password back has to survive the
// ViewModel that minted it, and androidx closes viewModelScope *before*
// onCleared runs, so a launch there never executes its body.
@@ -263,6 +265,11 @@ class AddAccountViewModel @Inject constructor(
return
}
// ⚠️ Written down **before** the browser is offered the URL, which is
// what the flow's own doc asks for: the browser is a separate task, so
// dying while the user approves is ordinary, and the poll token is the
// only way back to the password the server is about to mint.
pendingFlow.remember(flow)
_state.update {
it.copy(
step = AddAccountStep.WaitingForBrowser(hostMismatch = flow.hostMismatch),
@@ -290,6 +297,10 @@ class AddAccountViewModel @Inject constructor(
// credential with nothing left holding it. Recording it is
// what makes handing it back possible.
withContext(NonCancellable) {
// Spent: the server deleted the row before answering,
// so a persisted token is now a thing that would be
// polled again for nothing.
pendingFlow.forget()
username = result.credentials.loginName
appPassword = result.credentials.appPassword
serverRoot = result.credentials.server
@@ -384,6 +395,7 @@ class AddAccountViewModel @Inject constructor(
// The one exit that never reaches onStartOver: if approval already
// landed, that password is about to be replaced by a typed one.
discardMintedPassword()
appScope.launch { runCatching { pendingFlow.forget() } }
pollJob?.cancel()
_state.update {
it.copy(step = AddAccountStep.EnterCredentials(error = null), openInBrowser = null)
@@ -485,6 +497,7 @@ class AddAccountViewModel @Inject constructor(
*/
fun onStartOver() {
discardMintedPassword()
appScope.launch { runCatching { pendingFlow.forget() } }
pollJob?.cancel()
pollJob = null
found = null
@@ -569,7 +582,13 @@ class AddAccountViewModel @Inject constructor(
it.copy(step = AddAccountStep.EnterServer(input = typedInput, error = cause))
}
private fun browserFailed(reason: AddAccountMessage) = _state.update {
private fun browserFailed(reason: AddAccountMessage) {
// Whatever ended it, nothing is coming back through this token.
appScope.launch { runCatching { pendingFlow.forget() } }
failedInBrowser(reason)
}
private fun failedInBrowser(reason: AddAccountMessage) = _state.update {
// Keeps whatever the step was carrying. The host-mismatch note is often
// the *explanation* for the failure, so dropping it removes the warning
// exactly when it becomes worth reading.
app/src/test/java/de/jeanlucmakiola/agendula/ui/accounts/AddAccountViewModelTest.kt
+83 -1
View File
@@ -4,6 +4,7 @@ import com.google.common.truth.Truth.assertThat
import de.jeanlucmakiola.agendula.data.sync.AccountCreator
import de.jeanlucmakiola.agendula.data.sync.AccountRepository
import de.jeanlucmakiola.agendula.data.sync.CalDavGateway
import de.jeanlucmakiola.agendula.data.sync.LoginFlowRecord
import de.jeanlucmakiola.caldav.CalDavDiscovery
import de.jeanlucmakiola.caldav.NextcloudLoginFlow
import de.jeanlucmakiola.caldav.TaskCollection
@@ -37,6 +38,7 @@ class AddAccountViewModelTest {
private val dispatcher = StandardTestDispatcher()
private val gateway = FakeGateway()
private val creator = FakeCreator()
private val record = FakeLoginFlowRecord()
@BeforeEach fun setUp() = Dispatchers.setMain(dispatcher)
@@ -50,7 +52,7 @@ class AddAccountViewModelTest {
*/
private val appScope = CoroutineScope(dispatcher)
private fun viewModel() = AddAccountViewModel(creator, gateway, appScope)
private fun viewModel() = AddAccountViewModel(creator, gateway, record, appScope)
@Nested
inner class TheAddressStep {
@@ -259,6 +261,70 @@ class AddAccountViewModelTest {
}
}
@Nested
inner class SurvivingTheProcess {
@Test
fun `the flow is written down before the browser is offered the URL`() =
runTest(dispatcher) {
gateway.discoveryOutcomes += CalDavDiscovery.Outcome.NeedsAuthentication(
listOf("cloud.example.com"),
)
gateway.loginFlow = flow()
val vm = viewModel()
vm.onServerInputChanged("https://cloud.example.com/")
vm.onServerSubmitted()
// Not advanceUntilIdle: that would run the whole twenty-minute
// poll loop out to its expiry, which forgets the flow again.
runCurrent()
// ⚠️ The browser is a separate task, so dying while the user
// approves is ordinary — and the poll token is the only way back
// to the password the server is about to mint.
assertThat(record.remembered).isEqualTo(gateway.loginFlow)
assertThat(vm.state.value.openInBrowser).isNotNull()
}
@Test
fun `a spent flow is not left behind to be polled again`() = runTest(dispatcher) {
gateway.discoveryOutcomes += CalDavDiscovery.Outcome.NeedsAuthentication(emptyList())
gateway.loginFlow = flow()
gateway.pollResults += NextcloudLoginFlow.PollResult.Approved(
NextcloudLoginFlow.Credentials(
server = "https://cloud.example.com/".toHttpUrl(),
loginName = "me",
appPassword = "app-pw",
),
)
gateway.discoveryOutcomes += found(collection("Tasks"))
val vm = viewModel()
vm.onServerInputChanged("https://cloud.example.com/")
vm.onServerSubmitted()
advanceUntilIdle()
// The server deleted the flow row before answering, so the token now
// buys nothing and a reclaim would poll it for no reason.
assertThat(record.remembered).isNull()
assertThat(vm.state.value.step).isInstanceOf(AddAccountStep.ChooseLists::class.java)
}
@Test
fun `a flow that ends badly is forgotten too`() = runTest(dispatcher) {
gateway.discoveryOutcomes += CalDavDiscovery.Outcome.NeedsAuthentication(emptyList())
gateway.loginFlow = flow()
gateway.pollResults += NextcloudLoginFlow.PollResult.Expired("window closed")
val vm = viewModel()
vm.onServerInputChanged("https://cloud.example.com/")
vm.onServerSubmitted()
advanceUntilIdle()
assertThat(record.remembered).isNull()
}
}
@Nested
inner class AbandoningTheBrowserFlow {
@@ -697,6 +763,22 @@ class AddAccountViewModelTest {
}
}
/** Remembers what the flow told it, in the order it was told. */
private class FakeLoginFlowRecord : LoginFlowRecord {
val log = mutableListOf<String>()
var remembered: NextcloudLoginFlow.Flow? = null
override suspend fun remember(flow: NextcloudLoginFlow.Flow) {
remembered = flow
log += "remember"
}
override suspend fun forget() {
remembered = null
log += "forget"
}
}
private class FakeCreator : AccountCreator {
val created = mutableListOf<String>()
var thrown: Throwable? = null