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(); }