Repository navigation
Conversation
…lures Tapping "Log in" (or "Reauthenticate") could appear to do nothing: -Every IpnViewModel.login() call runs editPrefs() + start(). In the Go backend, start() calls resetControlClientLocked(), which cancels the in-flight register request and clears the cached auth URL. Repeated taps (or a tap racing another start) therefore keep cancelling the request that would produce the BrowseToURL, which is the "TryLogin: register request: context canceled" seen in the logs. -Failures in the login chain were only logged; there was no feedback and no timeout when no auth URL arrived. -Notifier.browseToURL is a StateFlow that was only cleared after a successful login, so the backend re-sending the same URL (which StartLoginInteractive does for a still-valid URL) was conflated away and no browser opened. -The browseToURL collector ran regardless of lifecycle, so a URL that arrived while the app was in the background tried to start the browser from the background, which Android silently blocks. This change: -Tracks the login phase across view models. A plain interactive login requested while one is in progress no longer calls editPrefs()/start(): it is ignored while start() is in flight, and afterwards only calls startLoginInteractive() to re-request the URL. -Arms a watchdog: if neither an auth URL nor a completed login is seen within 30s, the login is reset and the user is told. Failures of the interactive login chain are shown as a toast. -Collects browseToURL only while MainActivity is STARTED and consumes the URL after launching the browser, so the same URL can be delivered again and is not reopened on activity restart. A counter in Notifier lets the watchdog see repeated URLs. -Shows a message when no browser can open the login URL. Updates tailscale/tailscale#18304 Signed-off-by: Denis Ulmer <22558132+denisulmer@users.noreply.github.com>
The app dropped all health warnings unless the IPN state was Running, so login failures were invisible exactly when they matter: the backend reports the last login error (a register request canceled by a racing start, a refused control connection, ...) via the login-state warnable regardless of IPN state, and the app logged "Ignoring and dropping all health messages in state NeedsLogin" while the login silently died (tailscale/tailscale#18304). Surface login-relevant warnings (login-state, network-status) in the pre-login states (NeedsLogin, NeedsMachineAuth, Starting), with the same debounce as while Running; other warnables remain filtered as pre-login noise. Move the warnable codes the app uses to constants in Health, and expose the login-state error arg so the login UX can show the backend's own diagnosis. Updates tailscale/tailscale#18304 Signed-off-by: James Tucker <james@tailscale.com>
…state changes Replace the ad-hoc per-view login bookkeeping with LoginFlow, an app-wide state machine (IDLE/STARTING/AWAITING_URL/AUTHENTICATING/ STALLED) that all view models share, driven by the IPN bus rather than by the login chain's completion callbacks: - A plain interactive login requested while one is in flight no longer restarts the login chain (each start() tears down the control client and cancels the in-flight register request, which is the tailscale/tailscale#18304 race); it only re-requests the auth URL via startLoginInteractive(). - A watchdog observes BrowseToURL (counted, so a repeated URL is delivered), LoginFinished (counted: a reauthentication while Running does not change the IPN state), and the health subsystem's login-state error, which fails a dead login fast (after a short confirmation window; a transient self-induced cancel is followed by progress and must not fail the login). - No progress at all for 30s is an offer, not a deadline: the machine transitions to STALLED, stays armed, and keeps watching, so a login on a severely degraded network still completes on its own; a retry while stalled re-requests the auth URL rather than restarting the chain, and only after a 10-minute wedge guard does the machine reset silently. - The UI communicates progress: ConnectView disables the button while the login is in flight (but not while stalled), shows what the login is waiting for, and surfaces the backend's own login error from the health subsystem. Debounce the other major state changes the same way: logout is CAS-guarded (with a 30s safety release), and the VPN toggle stays disabled until the IPN state settles out of Starting/Stopping (or 30s), so rapid toggles cannot cancel each other's backend work. Updates tailscale/tailscale#18304 Signed-off-by: James Tucker <james@tailscale.com>
…e-login health Two emulator-backed integration tests, using the repo's testcontrol + DERP harness and the debug-only integration login receiver: - TestAndroidInteractiveLogin reproduces tailscale/tailscale#18304 from the app's real UI: seed an auth-key login against testcontrol, then spam the user switcher's "Reauthenticate" row three times (the exact workaround users found in the issue thread). On unfixed builds it observes the race (each tap runs the whole editPrefs+start chain, tearing down the control client; 'context canceled' login errors) and fails; on fixed builds it requires exactly one login chain run, all login state machine phase transitions, and a completed end-to-end login (auth URL delivered, authenticated via testcontrol. CompleteAuth, node re-registered). - TestAndroidPreLoginHealthSurfacing drives a login against an unreachable control URL and requires the backend's login-state health warning to surface pre-login (it is dropped on unfixed builds). Note: the control client multiplexes register requests over its ts2021 noise connection, so auth URLs are observed from the app's logcat ("control: AuthURL is ...") rather than via an HTTP wrapper, and auth completion goes through testcontrol.CompleteAuth. Updates tailscale/tailscale#18304 Signed-off-by: James Tucker <james@tailscale.com>
raggi
force-pushed
the
raggi/android-start
branch
from
October 13, 2026 19:43
802db22 to
99a4ddf
Compare
Collaborator
|
Android and Apple both have a lot of bespoke login orchestration code because clients need to coordinate pref changes, backend startup or reset, and the authentication flow. What do you think of exposing a higher level |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This stacks atop #895 to integrate more deeply with the existing health infrastructure and state machines.
It's still a little sloppy and I think we may want to consider pushing more of this behavior down into OSS with a better formalized interface.