From 1db8bed872851a48e89ddc23e89aed3eadc87322 Mon Sep 17 00:00:00 2001 From: Jaap-Jan de Wit | DodoTech Date: Sat, 1 Aug 2026 21:10:37 +0200 Subject: [PATCH] Let a cancelled sign-in end the sign-in rather than the timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backing out of the login page on Android left the shell showing "Opening your browser to sign in…" with the button disabled for five minutes. Nothing was wrong except that nobody told it: the redirect callback only ever completed when an intent arrived, so a user who pressed back was waiting on OidcClient's browser timeout to expire before the flow failed and the button came back. There is no cancel event to subscribe to on this platform. Pressing back, dismissing the browser and closing a provider's error page are indistinguishable from here — the browser goes away and this application is foreground again with nothing delivered — so being resumed while a sign-in is still waiting is the signal, and the only one there is. The launcher records that a browser took the intent, OnResume fails the wait, and the guard means the resumes that have nothing to do with signing in (a launch, recents, the keystore's fingerprint prompt) go through untouched. An exception rather than a cancellation, because OidcClient reads a cancelled wait as its own timeout expiring and would report five minutes passing to somebody who waited two seconds. It cannot steal a successful sign-in either: Android delivers the redirect to OnNewIntent before resuming the activity, so the completion is already settled and the attempt does nothing. The enrollment key-binding trip through the browser is covered by the same change, since it waits on the same callback. The OnNewIntent remark had been sitting above OnResume, describing a method two below it. Moved back, since the new remark wanted the space and the old one was wrong where it was. Verified by building the head in Debug and Release. The behaviour itself is unverified for the reason docs/android-port.md gives about this whole head: nothing has been run on a device. Co-Authored-By: Claude Opus 5 (1M context) --- src/DodoSSH.Client.Android/MainActivity.cs | 20 +++++-- .../Platform/AndroidAuthorization.cs | 59 ++++++++++++++++++- 2 files changed, 71 insertions(+), 8 deletions(-) diff --git a/src/DodoSSH.Client.Android/MainActivity.cs b/src/DodoSSH.Client.Android/MainActivity.cs index 87e9f0d..a61b308 100644 --- a/src/DodoSSH.Client.Android/MainActivity.cs +++ b/src/DodoSSH.Client.Android/MainActivity.cs @@ -58,18 +58,19 @@ public sealed class MainActivity : AvaloniaMainActivity { /// /// - /// OnNewIntent rather than OnCreate, and that is what SingleTask above buys: the - /// activity is already running with a sign-in waiting inside it, so the redirect has to be delivered - /// into that instance. Any other launch mode would start a second copy of the activity — and with it a - /// second Avalonia application over a live one — leaving the original waiting for a response that had - /// already been consumed. + /// Resuming is also how a cancelled sign-in is noticed. There is no event for a user pressing back out + /// of the login page — the browser simply goes away and this application is foreground again with + /// nothing delivered — so being resumed while a sign-in is still waiting for its redirect is the + /// signal, and the only one there is. See , which is a + /// no-op unless a browser was opened and has not answered. /// - /// protected override void OnResume() { base.OnResume(); PhoneEnvironment.CurrentActivity = this; + + AndroidRedirectCallback.Abandon(); } /// @@ -88,6 +89,13 @@ public sealed class MainActivity : AvaloniaMainActivity } /// + /// + /// OnNewIntent rather than OnCreate, and that is what SingleTask above buys: the + /// activity is already running with a sign-in waiting inside it, so the redirect has to be delivered + /// into that instance. Any other launch mode would start a second copy of the activity — and with it a + /// second Avalonia application over a live one — leaving the original waiting for a response that had + /// already been consumed. + /// protected override void OnNewIntent(Intent? intent) { base.OnNewIntent(intent); diff --git a/src/DodoSSH.Client.Android/Platform/AndroidAuthorization.cs b/src/DodoSSH.Client.Android/Platform/AndroidAuthorization.cs index 378363f..48a7c9a 100644 --- a/src/DodoSSH.Client.Android/Platform/AndroidAuthorization.cs +++ b/src/DodoSSH.Client.Android/Platform/AndroidAuthorization.cs @@ -42,6 +42,10 @@ internal sealed class AndroidBrowserLauncher : IBrowserLauncher PhoneEnvironment.Require().StartActivity(intent); + // Only now, and only if the browser actually took the intent: this is what lets a return to this + // application with no response be read as a cancellation. See AndroidRedirectCallback.Abandon. + AndroidRedirectCallback.NotifyBrowserOpened(); + return Task.CompletedTask; } } @@ -81,6 +85,9 @@ internal sealed class AndroidRedirectCallback : IAuthorizationCallback private static AndroidRedirectCallback? waiting; + /// Whether a browser was opened for a sign-in that has not answered yet. + private static volatile bool browserOpen; + /// /// The redirect path, from OidcClientOptions. Carried through so the desktop's configured value and /// this one cannot silently differ. @@ -98,6 +105,48 @@ internal sealed class AndroidRedirectCallback : IAuthorizationCallback /// public Uri RedirectUri { get; } + /// Records that the authorization page has been handed to the browser. + /// + /// One half of the cancellation story; is the other. Called by + /// rather than set when the wait starts, so that only a return + /// from a browser this application actually opened counts as an answer that never came. + /// + internal static void NotifyBrowserOpened() => browserOpen = true; + + /// Fails the waiting sign-in when the user comes back without a response. + /// + /// + /// Called from MainActivity.OnResume, because this platform has no cancel event to subscribe + /// to. Pressing back out of the login page, dismissing the browser, or closing a provider error page + /// all look identical from here: this application is foreground again and no intent ever arrived. + /// Left alone the flow sits on the five-minute browser timeout — that long with a disabled + /// Sign in button under "Opening your browser to sign in…", which reads as a hung + /// application rather than as the cancellation it is. + /// + /// + /// It cannot steal a successful sign-in. Android delivers the redirect to OnNewIntent before + /// resuming the activity, so by the time this runs the completion is already settled and the attempt + /// below does nothing. + /// + /// + internal static void Abandon() + { + // Guarded rather than unconditional, because most resumes have nothing to do with signing in: + // launching the application, coming back from recents, returning from the keystore's fingerprint + // prompt. Only a resume that follows a browser this class opened is an answer that never came. + if (!browserOpen) + { + return; + } + + browserOpen = false; + + // An exception rather than a cancellation, because OidcClient reads a cancelled wait as its own + // timeout expiring and would report five minutes passing. This says what actually happened. + Volatile.Read(ref waiting)?.completion.TrySetException( + new OidcException("Sign-in was cancelled: the browser closed without signing you in.")); + } + /// Hands a received redirect to whichever sign-in is waiting for it. /// /// Called from MainActivity.OnNewIntent. Returns quietly when nothing is waiting: a redirect @@ -106,6 +155,8 @@ internal sealed class AndroidRedirectCallback : IAuthorizationCallback /// internal static void Complete(Uri redirect) { + browserOpen = false; + var target = Volatile.Read(ref waiting); if (target is null) @@ -142,8 +193,12 @@ internal sealed class AndroidRedirectCallback : IAuthorizationCallback public void Dispose() { // Only if it is still this one. A later sign-in has already replaced it, and clearing the slot - // then would silently break the attempt that is actually in flight. - Interlocked.CompareExchange(ref waiting, null, this); + // then would silently break the attempt that is actually in flight — including its claim on the + // browser, which is why the flag is cleared under the same condition and not beside it. + if (ReferenceEquals(Interlocked.CompareExchange(ref waiting, null, this), this)) + { + browserOpen = false; + } completion.TrySetCanceled(); }