Spelunk sign-in: refuse a token that is not an app session's, follow no redirects, and say every way the browser fails to come back

A token Spelunk's token endpoint returns must begin with spelunkapp_, the prefix Spelunk reads a bearer
token as an app session's by; anything else is refused as an answer this app does not understand. Neither
the sign-in's client nor the tunnel's follows redirects, so the code, its verifier and the token go to the
address signed in to and nowhere else, and signing out sends the token only to an address that still
normalises to itself, within ten seconds of its own.

Opening the browser and waiting for it now always end in a SpelunkSignInException rather than whatever
the platform threw: a browser that will not open, a phone's browser closed without an answer (the
OidcException AndroidRedirectCallback.Abandon raises, which escaped the Security page and the editor
before), another sign-in taking over the phone's redirect, and the ten-minute timeout are each said. A
refused code says it in Spelunk's own words, and a rate limit says to wait.

Tests: a refused code's detail, a non-app token, a browser that will not open, an abandoned phone sign-in,
the timeout, the user cancelling, a phone redirect sent byte for byte both times, the real loopback
listener on 127.0.0.1 taking one answer and closing, and a sign-out that will not send the token to an
address it could not have come from.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

src/DodoSSH.Client.Session/SpelunkAccounts.cs

@@ -77,6 +77,19 @@ public sealed class SpelunkAccounts : ISpelunkAccounts
     /// <summary>The longest token kept. Far beyond Spelunk's; a bound on what goes into a header and the cache.</summary>
     private const int MaxTokenLength = 512;
 
+    /// <summary>What every token Spelunk issues a signed-in app begins with, and how Spelunk tells it from an API key.</summary>
+    /// <remarks>
+    /// Checked rather than assumed: a token without it is one Spelunk would read as something else, so an answer
+    /// carrying one is not Spelunk's app sign-in, whatever else it looks like.
+    /// </remarks>
+    internal const string TokenPrefix = "spelunkapp_";
+
+    /// <summary>The longest error body read, so a misbehaving proxy cannot hand this a gigabyte.</summary>
+    private const int MaxProblemBytes = 16 * 1024;
+
+    /// <summary>How long signing out waits for Spelunk before forgetting the sign-in here regardless.</summary>
+    private static readonly TimeSpan SignOutTimeout = TimeSpan.FromSeconds(10);
+
     /// <summary>The page the loopback listener shows once Spelunk has sent the browser back.</summary>
     private const string CompletionHtml =
         """
@@ -86,8 +99,12 @@ public sealed class SpelunkAccounts : ISpelunkAccounts
         </body></html>
         """;
 
+    /// <remarks>
+    /// Redirects are not followed. The code, its verifier and the token go to the address the user signed in
+    /// to and nowhere else, and a Spelunk that answered with a redirect would be asking for them to be sent on.
+    /// </remarks>
     private static readonly HttpClient SharedHttp = new(
-        new SocketsHttpHandler { PooledConnectionLifetime = TimeSpan.FromMinutes(5) })
+        new SocketsHttpHandler { PooledConnectionLifetime = TimeSpan.FromMinutes(5), AllowAutoRedirect = false })
     {
         Timeout = TimeSpan.FromSeconds(30),
     };
@@ -146,35 +163,17 @@ public sealed class SpelunkAccounts : ISpelunkAccounts
         var pkce = PkcePair.Create();
         var state = Base64Url.EncodeToString(RandomNumberGenerator.GetBytes(32));
 
+        // Disposed on the way out whatever happens, so the loopback port is closed once it has had its one
+        // answer - or none.
         using var callback = callbackFactory("/callback");
+
+        // Read once and sent twice, byte for byte: Spelunk compares the token request's against the authorize one.
         var redirectUri = callback.RedirectUri.ToString();
 
-        var query = string.Join('&', new Dictionary<string, string>(StringComparer.Ordinal)
-        {
-            ["client_id"] = ClientId,
-            ["redirect_uri"] = redirectUri,
-            ["response_type"] = "code",
-            ["code_challenge"] = pkce.CodeChallenge,
-            ["code_challenge_method"] = PkcePair.Method,
-            ["state"] = state,
-            ["device_name"] = deviceName,
-        }.Select(pair => $"{pair.Key}={Uri.EscapeDataString(pair.Value)}"));
-
-        await browser.OpenAsync(new Uri($"{normalized}/api/app-sign-in/authorize?{query}", UriKind.Absolute), cancellationToken)
+        await OpenBrowserAsync(AuthorizeUrl(normalized, redirectUri, pkce.CodeChallenge, state), cancellationToken)
             .ConfigureAwait(false);
 
-        using var timeout = new CancellationTokenSource(BrowserTimeout, clock);
-        using var linked = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, timeout.Token);
-
-        CallbackResult answer;
-        try
-        {
-            answer = await callback.WaitForCallbackAsync(CompletionHtml, linked.Token).ConfigureAwait(false);
-        }
-        catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested)
-        {
-            throw new SpelunkSignInException("Spelunk did not send the browser back in time. Try signing in again.");
-        }
+        var answer = await WaitForAnswerAsync(callback, cancellationToken).ConfigureAwait(false);
 
         // Checked before anything else is read: an answer to some other sign-in is no answer to this one.
         if (!string.Equals(answer.State, state, StringComparison.Ordinal))
@@ -186,7 +185,7 @@ public sealed class SpelunkAccounts : ISpelunkAccounts
         {
             throw new SpelunkSignInException(string.Equals(error, "access_denied", StringComparison.Ordinal)
                 ? "Signing in to Spelunk was cancelled in the browser."
-                : $"Spelunk refused the sign-in ({answer.ErrorDescription ?? error}).");
+                : $"Spelunk refused the sign-in ({SpelunkTunnelRoute.Readable(answer.ErrorDescription) ?? SpelunkTunnelRoute.Readable(error)}).");
         }
 
         if (answer.Code is not { Length: > 0 } code)
@@ -197,6 +196,70 @@ public sealed class SpelunkAccounts : ISpelunkAccounts
         return await RedeemAsync(normalized, code, pkce.CodeVerifier, redirectUri, cancellationToken).ConfigureAwait(false);
     }
 
+    /// <summary>Spelunk's authorize address for this sign-in: OAuth's own parameter names, and this device's.</summary>
+    private Uri AuthorizeUrl(string apiBase, string redirectUri, string codeChallenge, string state)
+    {
+        var query = string.Join('&', new Dictionary<string, string>(StringComparer.Ordinal)
+        {
+            ["client_id"] = ClientId,
+            ["redirect_uri"] = redirectUri,
+            ["response_type"] = "code",
+            ["code_challenge"] = codeChallenge,
+            ["code_challenge_method"] = PkcePair.Method,
+            ["state"] = state,
+            ["device_name"] = deviceName,
+        }.Select(pair => $"{pair.Key}={Uri.EscapeDataString(pair.Value)}"));
+
+        return new Uri($"{apiBase}/api/app-sign-in/authorize?{query}", UriKind.Absolute);
+    }
+
+    private async Task OpenBrowserAsync(Uri authorize, CancellationToken cancellationToken)
+    {
+        try
+        {
+            await browser.OpenAsync(authorize, cancellationToken).ConfigureAwait(false);
+        }
+        catch (Exception exception) when (exception is not (OperationCanceledException or OutOfMemoryException))
+        {
+            throw new SpelunkSignInException($"The browser could not be opened to sign in to Spelunk: {exception.Message}", exception);
+        }
+    }
+
+    /// <summary>Waits for the browser to come back, and turns every way it does not into a sentence.</summary>
+    /// <exception cref="OperationCanceledException">The user abandoned the sign-in.</exception>
+    private async Task<CallbackResult> WaitForAnswerAsync(IAuthorizationCallback callback, CancellationToken cancellationToken)
+    {
+        using var timeout = new CancellationTokenSource(BrowserTimeout, clock);
+        using var linked = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, timeout.Token);
+
+        try
+        {
+            return await callback.WaitForCallbackAsync(CompletionHtml, linked.Token).ConfigureAwait(false);
+        }
+        catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
+        {
+            throw;
+        }
+        catch (OperationCanceledException) when (timeout.IsCancellationRequested)
+        {
+            throw new SpelunkSignInException("Spelunk did not send the browser back in time. Try signing in again.");
+        }
+        catch (OperationCanceledException exception)
+        {
+            // Neither the user nor the clock: a phone's redirect is handed to one sign-in at a time, and another
+            // one - to DodoSSH, or to Spelunk again - took it over.
+            throw new SpelunkSignInException("Another sign-in started before this one finished. Try signing in again.", exception);
+        }
+        catch (Exception exception) when (exception is not OutOfMemoryException)
+        {
+            // A phone's browser closing without an answer arrives as an OidcException with the reason in it; a
+            // desktop's listener failing to read the browser's request arrives as whatever the socket threw.
+            throw new SpelunkSignInException(
+                exception is OidcException ? exception.Message : $"The browser's answer from Spelunk could not be read: {exception.Message}",
+                exception);
+        }
+    }
+
     private async Task<SpelunkSignInRecord> RedeemAsync(
         string apiBase, string code, string verifier, string redirectUri, CancellationToken cancellationToken)
     {
@@ -228,49 +291,108 @@ public sealed class SpelunkAccounts : ISpelunkAccounts
         {
             if (!response.IsSuccessStatusCode)
             {
-                throw new SpelunkSignInException(response.StatusCode == HttpStatusCode.BadRequest
-                    ? "Spelunk did not accept the sign-in. It may have taken too long; try again."
-                    : string.Create(CultureInfo.InvariantCulture, $"Spelunk would not finish the sign-in (HTTP {(int)response.StatusCode})."));
+                throw new SpelunkSignInException(await DescribeRefusalAsync(response, cancellationToken).ConfigureAwait(false));
             }
 
-            SpelunkTokenDocument? token;
-            try
-            {
-                token = await response.Content
-                    .ReadFromJsonAsync(SpelunkAccountsJsonContext.Default.SpelunkTokenDocument, cancellationToken)
-                    .ConfigureAwait(false);
-            }
-            catch (JsonException exception)
-            {
-                throw new SpelunkSignInException(Unintelligible, exception);
-            }
-
-            // The token goes into a header on every ticket request, so it has to be one.
-            if (token is not { Token: { Length: > 0 and <= MaxTokenLength } value, SessionId: { } sessionId, User: { Email: { } email } user }
-                || !value.All(character => char.IsAsciiLetterOrDigit(character) || character == '_'))
-            {
-                throw new SpelunkSignInException(Unintelligible);
-            }
-
-            return new SpelunkSignInRecord(apiBase, value, sessionId, email, user.DisplayName ?? email, clock.GetUtcNow());
+            return await ReadSignInAsync(apiBase, response, cancellationToken).ConfigureAwait(false);
         }
     }
 
+    /// <summary>Why the token endpoint said no, in Spelunk's own words where it gave some.</summary>
+    private static async Task<string> DescribeRefusalAsync(HttpResponseMessage response, CancellationToken cancellationToken)
+    {
+        var detail = await ReadProblemDetailAsync(response, cancellationToken).ConfigureAwait(false);
+
+        return response.StatusCode switch
+        {
+            // Spelunk says which: a code that was wrong, used or expired, or sessions ended meanwhile.
+            HttpStatusCode.BadRequest => "Spelunk did not accept the sign-in"
+                + (detail is null ? ". It may have taken too long; try again." : $": {detail}"),
+
+            HttpStatusCode.TooManyRequests =>
+                "Spelunk is refusing sign-ins from this address for a moment. Wait a minute, then try again.",
+
+            _ => string.Create(CultureInfo.InvariantCulture, $"Spelunk would not finish the sign-in (HTTP {(int)response.StatusCode})")
+                + (detail is null ? "." : $": {detail}"),
+        };
+    }
+
+    /// <summary>Reads the token endpoint's answer, and refuses one that is not an app sign-in's.</summary>
+    private async Task<SpelunkSignInRecord> ReadSignInAsync(string apiBase, HttpResponseMessage response, CancellationToken cancellationToken)
+    {
+        SpelunkTokenDocument? token;
+        try
+        {
+            token = await response.Content
+                .ReadFromJsonAsync(SpelunkAccountsJsonContext.Default.SpelunkTokenDocument, cancellationToken)
+                .ConfigureAwait(false);
+        }
+        catch (JsonException exception)
+        {
+            throw new SpelunkSignInException(Unintelligible, exception);
+        }
+
+        // The token goes into a header on every ticket request, so it has to be one - and one Spelunk reads as
+        // an app's.
+        if (token is not { Token: { Length: > 0 and <= MaxTokenLength } value, SessionId: { } sessionId, User: { Email: { } email } user }
+            || !value.StartsWith(TokenPrefix, StringComparison.Ordinal)
+            || value.Length == TokenPrefix.Length
+            || !value.All(character => char.IsAsciiLetterOrDigit(character) || character == '_'))
+        {
+            throw new SpelunkSignInException(Unintelligible);
+        }
+
+        return new SpelunkSignInRecord(apiBase, value, sessionId, email, user.DisplayName ?? email, clock.GetUtcNow());
+    }
+
     /// <inheritdoc />
     public async Task SignOutAsync(SpelunkSignInRecord signIn, CancellationToken cancellationToken)
     {
         ArgumentNullException.ThrowIfNull(signIn);
 
+        // Only to an address a sign-in could have come from: the cache is this device's own, but a record that
+        // no longer normalises to itself is one to forget rather than to send a token on behalf of.
+        if (!SpelunkRoute.TryNormalizeApiBase(signIn.ApiBase, out var apiBase, out _)
+            || !string.Equals(apiBase, signIn.ApiBase, StringComparison.Ordinal))
+        {
+            return;
+        }
+
+        // Bounded on its own: signing out of DodoSSH waits for this, and a Spelunk that does not answer must not
+        // hold that up for the transport's whole timeout.
+        using var timeout = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
+        timeout.CancelAfter(SignOutTimeout);
+
         try
         {
-            using var request = new HttpRequestMessage(HttpMethod.Delete, new Uri($"{signIn.ApiBase}/api/app-sign-in/session", UriKind.Absolute));
+            using var request = new HttpRequestMessage(HttpMethod.Delete, new Uri($"{apiBase}/api/app-sign-in/session", UriKind.Absolute));
             request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", signIn.Token);
-            using var response = await http.SendAsync(request, cancellationToken).ConfigureAwait(false);
+            using var response = await http.SendAsync(request, timeout.Token).ConfigureAwait(false);
         }
-        catch (Exception exception) when (exception is HttpRequestException or TaskCanceledException or InvalidOperationException)
+        catch (Exception exception) when (exception is HttpRequestException or OperationCanceledException or InvalidOperationException or FormatException)
         {
-            // Unreachable, or already signed out: either way this device forgets it, which is what signing out
-            // here means. Spelunk's list under Your account still offers to end it there.
+            // Unreachable, already signed out, or slow: either way this device forgets it, which is what signing
+            // out here means. Spelunk's list under Your account still offers to end it there.
+        }
+    }
+
+    /// <summary>Spelunk's own words about a refusal, cut to one line, or null when it gave none.</summary>
+    private static async Task<string?> ReadProblemDetailAsync(HttpResponseMessage response, CancellationToken cancellationToken)
+    {
+        try
+        {
+            var body = await response.Content.ReadAsByteArrayAsync(cancellationToken).ConfigureAwait(false);
+            if (body.Length is 0 or > MaxProblemBytes)
+            {
+                return null;
+            }
+
+            var problem = JsonSerializer.Deserialize(body, SpelunkAccountsJsonContext.Default.SpelunkSignInProblemDocument);
+            return SpelunkTunnelRoute.Readable(problem?.Detail);
+        }
+        catch (Exception exception) when (exception is JsonException or HttpRequestException)
+        {
+            return null;
         }
     }
 
@@ -308,6 +430,12 @@ internal sealed class SpelunkTokenUserDocument
     public string? DisplayName { get; set; }
 }
 
+/// <summary>The part of an RFC 7807 refusal from Spelunk's token endpoint that is shown.</summary>
+internal sealed class SpelunkSignInProblemDocument
+{
+    public string? Detail { get; set; }
+}
+
 [JsonSourceGenerationOptions(
     PropertyNamingPolicy = JsonKnownNamingPolicy.CamelCase,
     PropertyNameCaseInsensitive = true,
@@ -315,4 +443,5 @@ internal sealed class SpelunkTokenUserDocument
     UnmappedMemberHandling = JsonUnmappedMemberHandling.Skip)]
 [JsonSerializable(typeof(SpelunkTokenRequestDocument))]
 [JsonSerializable(typeof(SpelunkTokenDocument))]
+[JsonSerializable(typeof(SpelunkSignInProblemDocument))]
 internal sealed partial class SpelunkAccountsJsonContext : JsonSerializerContext;

src/DodoSSH.Client.Session/SpelunkTunnel.cs

@@ -273,10 +273,12 @@ public sealed class SpelunkTunnel : ISpelunkTunnel
     /// </summary>
     /// <remarks>
     /// Bounded well short of the default hundred seconds: a ticket is one small request, and somebody
-    /// watching "Connecting…" should hear that Spelunk is not answering while they still care.
+    /// watching "Connecting…" should hear that Spelunk is not answering while they still care. Redirects are
+    /// not followed: the sign-in's token goes to the Spelunk it came from, and a redirect is a request to send
+    /// the ticket request somewhere else.
     /// </remarks>
     private static readonly HttpClient SharedHttp = new(
-        new SocketsHttpHandler { PooledConnectionLifetime = TimeSpan.FromMinutes(5) })
+        new SocketsHttpHandler { PooledConnectionLifetime = TimeSpan.FromMinutes(5), AllowAutoRedirect = false })
     {
         Timeout = TimeSpan.FromSeconds(30),
     };