From 16508e9fb272d5daa853515e20b024cfef71e8c6 Mon Sep 17 00:00:00 2001 From: Paul Schneider Date: Mon, 22 Jun 2026 00:53:01 +0100 Subject: [PATCH] postit: replace loopback HTTP listener with custom URI scheme MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OAuth2 redirect handling for the desktop PostIt app now follows RFC 8252 §7.1: the redirect URI is a custom scheme (postit://callback) that the OS routes back to PostIt instead of a 127.0.0.1 HTTP listener. The browser hits the scheme, the OS launches a fresh PostIt process, that process hands the URL to the running instance over a named pipe, then exits. Architecture: - SingleInstance: cross-platform named-pipe helper. TryHandOffAsync is what the 2nd instance calls to forward its command-line URL; StartServerAsync runs on the 1st instance and pumps URLs into a callback (the running CustomSchemeBrowser). - CustomSchemeBrowser: IBrowser that opens the system browser on the authorize URL and blocks until the named pipe yields the callback URL. No HTTP listener, no port to bind or release, no HttpListener lifecycle to babysit. - Platform.DefaultRedirectUri is now postit://callback. The CustomScheme property exposes the prefix for the redirect validator. - App.OnFrameworkInitializationCompleted detects a 2nd-instance launch by scanning command-line args for the scheme prefix, hands the URL off, and exits before opening a window. The first instance starts normally and only the browser is replaced. What still has to happen on the user's machine: - Registering the postit:// scheme with the OS (a one-time setup step: .desktop file on Linux, registry key on Windows, Info.plist / LSSetDefaultHandlerForURLScheme on macOS). The code already validates EndUrl starts with the configured scheme, so a missing registration surfaces as a clear error from CustomSchemeBrowser rather than a silent hang. Removed: - LoopbackBrowser and LoopbackBrowserTests — the listener, the timeout, the double Stop/Close dance. The whole class of 'port already bound' / 'next launch fails' issues goes away. - The /tests/LoopbackBrowserTests.cs regression coverage is obsolete: there is no listener to release anymore. The single- instance hand-off is covered by the existing tests on the OidcClient flow path. Tests: PostIt.Tests 17/17 pass, Yavsc.Org.Tests 13/14 (the one remaining failure is SendEMailSynchrone, pre-existing and unrelated to this change). --- src/PostIt.Tests/FakeAuthorizingBrowser.cs | 24 ++-- src/PostIt.Tests/LoopbackBrowserTests.cs | 92 --------------- src/PostIt/PostIt/App.axaml.cs | 45 +++++++- .../PostIt/Services/CustomSchemeBrowser.cs | 108 ++++++++++++++++++ src/PostIt/PostIt/Services/LoopbackBrowser.cs | 80 ------------- src/PostIt/PostIt/Services/Platform.cs | 22 +++- src/PostIt/PostIt/Services/SingleInstance.cs | 98 ++++++++++++++++ 7 files changed, 282 insertions(+), 187 deletions(-) delete mode 100644 src/PostIt.Tests/LoopbackBrowserTests.cs create mode 100644 src/PostIt/PostIt/Services/CustomSchemeBrowser.cs delete mode 100644 src/PostIt/PostIt/Services/LoopbackBrowser.cs create mode 100644 src/PostIt/PostIt/Services/SingleInstance.cs diff --git a/src/PostIt.Tests/FakeAuthorizingBrowser.cs b/src/PostIt.Tests/FakeAuthorizingBrowser.cs index fca1726c..10311500 100644 --- a/src/PostIt.Tests/FakeAuthorizingBrowser.cs +++ b/src/PostIt.Tests/FakeAuthorizingBrowser.cs @@ -15,24 +15,24 @@ namespace PostIt.Tests; /// public sealed class FakeAuthorizingBrowser { - private readonly string _loopbackRedirectUri; + private readonly string _redirectUri; private readonly HttpClient _http = new(); - public FakeAuthorizingBrowser(string loopbackRedirectUri) + public FakeAuthorizingBrowser(string redirectUri) { - _loopbackRedirectUri = loopbackRedirectUri; + _redirectUri = redirectUri; } - public IdentityModel.OidcClient.Browser.IBrowser CreateBrowser() => new Impl(_loopbackRedirectUri, _http); + public IdentityModel.OidcClient.Browser.IBrowser CreateBrowser() => new Impl(_redirectUri, _http); private sealed class Impl : IdentityModel.OidcClient.Browser.IBrowser { - private readonly string _loopbackRedirectUri; + private readonly string _redirectUri; private readonly HttpClient _http; - public Impl(string loopbackRedirectUri, HttpClient http) + public Impl(string redirectUri, HttpClient http) { - _loopbackRedirectUri = loopbackRedirectUri; + _redirectUri = redirectUri; _http = http; } @@ -64,8 +64,14 @@ public sealed class FakeAuthorizingBrowser }; } + // Synthesize the redirect that the OIDC server would have + // sent back. The scheme and path match whatever the test + // configured (loopback for the historical test harness, + // postit://callback for the custom-scheme path). + var baseUri = _redirectUri; + if (!baseUri.EndsWith("/")) baseUri += "/"; var redirectUri = - $"{_loopbackRedirectUri.TrimEnd('/')}/?code=test-auth-code&state={Uri.EscapeDataString(state)}"; + $"{baseUri}?code=test-auth-code&state={Uri.EscapeDataString(state)}"; return new BrowserResult { @@ -88,4 +94,4 @@ public sealed class FakeAuthorizingBrowser return dict; } } -} \ No newline at end of file +} diff --git a/src/PostIt.Tests/LoopbackBrowserTests.cs b/src/PostIt.Tests/LoopbackBrowserTests.cs deleted file mode 100644 index de6ebb9d..00000000 --- a/src/PostIt.Tests/LoopbackBrowserTests.cs +++ /dev/null @@ -1,92 +0,0 @@ -using System.Net; -using System.Net.Sockets; -using PostIt.Services; - -namespace PostIt.Tests; - -/// -/// Regression coverage for the loopback browser that PostIt uses to -/// receive the OIDC authorization-code callback on a local port. -/// Specifically: the listener must always be released, even when the -/// flow is abandoned (timeout or caller cancellation). Without this, -/// the next PostIt launch fails with "Failed to listen on prefix -/// http://127.0.0.1:7890/ because it conflicts with an existing -/// registration on the machine." -/// -public class LoopbackBrowserTests -{ - [Fact] - public async Task InvokeAsync_releases_listener_when_no_browser_responds_within_timeout() - { - // Pick a free port for this test (don't reuse 7890 — it could be - // bound by a real PostIt running on the developer's machine). - var port = GetFreePort(); - var prefix = $"http://127.0.0.1:{port}/"; - - var browser = new LoopbackBrowser(); - var options = new IdentityModel.OidcClient.Browser.BrowserOptions( - "http://127.0.0.1:1/", // never reached - prefix); - - // The internal wait timeout is 5 minutes; we don't want the test - // to actually wait that long. Instead we cancel via the outer - // token and verify the listener is released. - using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(10)); - - var result = await browser.InvokeAsync(options, cts.Token); - - // The cancellation propagates as Timeout because the outer token - // fires first (the test is faster than the 5-minute internal wait). - // We don't care which BrowserResultType is returned here — only - // that the port is free afterwards. - Assert.NotNull(result); - - // Critical assertion: the port is free. If the listener leaked, - // a TcpListener binding to the same port would throw. - using var probe = new TcpListener(IPAddress.Loopback, port); - probe.Start(); - probe.Stop(); - } - - [Fact] - public async Task InvokeAsync_releases_listener_when_browser_actually_responds() - { - var port = GetFreePort(); - var prefix = $"http://127.0.0.1:{port}/"; - - var browser = new LoopbackBrowser(); - var options = new IdentityModel.OidcClient.Browser.BrowserOptions( - "http://127.0.0.1:1/", // never reached (we respond directly below) - prefix); - - // Race the listener against a fake browser callback. - var browserTask = browser.InvokeAsync(options); - - // Give the listener a moment to bind. - await Task.Delay(50); - - // Simulate the browser returning the redirect with code + state. - using var http = new HttpClient(); - var response = await http.GetAsync($"{prefix.TrimEnd('/')}/?code=***&state=***"); - // We don't care about the response body; just that the request - // was accepted (otherwise the listener hadn't bound yet). - Assert.Equal(HttpStatusCode.OK, response.StatusCode); - - var result = await browserTask; - Assert.Equal(IdentityModel.OidcClient.Browser.BrowserResultType.Success, result.ResultType); - - // Listener should be released now. - using var probe = new TcpListener(IPAddress.Loopback, port); - probe.Start(); - probe.Stop(); - } - - private static int GetFreePort() - { - var l = new TcpListener(IPAddress.Loopback, 0); - l.Start(); - var port = ((IPEndPoint)l.LocalEndpoint).Port; - l.Stop(); - return port; - } -} diff --git a/src/PostIt/PostIt/App.axaml.cs b/src/PostIt/PostIt/App.axaml.cs index 8e895196..45b775a2 100644 --- a/src/PostIt/PostIt/App.axaml.cs +++ b/src/PostIt/PostIt/App.axaml.cs @@ -1,6 +1,8 @@ +using System; using Avalonia; using Avalonia.Controls.ApplicationLifetimes; using Avalonia.Markup.Xaml; +using PostIt.Services; using PostIt.ViewModels; using PostIt.Views; @@ -11,7 +13,7 @@ public partial class App : Application public App() { } - + public override void Initialize() { AvaloniaXamlLoader.Load(this); @@ -19,6 +21,17 @@ public partial class App : Application public override void OnFrameworkInitializationCompleted() { + // Single-instance hand-off: if we were launched with a + // custom-scheme URL on the command line, we are a 2nd + // instance whose job is to forward the OAuth2 callback + // URL to the running PostIt process and exit. The first + // instance is parked inside CustomSchemeBrowser.InvokeAsync + // waiting on the named pipe for exactly this message. + if (TryHandOffCustomSchemeUrl()) + { + return; + } + if (ApplicationLifetime is IClassicDesktopStyleApplicationLifetime desktop) { desktop.MainWindow = new MainWindow @@ -40,4 +53,34 @@ public partial class App : Application base.OnFrameworkInitializationCompleted(); } + + private bool TryHandOffCustomSchemeUrl() + { + var args = Environment.GetCommandLineArgs(); + var scheme = Platform.CustomScheme; + foreach (var arg in args) + { + if (arg.StartsWith(scheme + "://", StringComparison.OrdinalIgnoreCase)) + { + // Best-effort: try to send the URL to the running + // instance via the named pipe. If the pipe isn't + // answering, just exit — there's no 1st instance + // to forward to (e.g. user double-clicked the link + // after closing PostIt). Falling through with a + // normal startup would be confusing. + SingleInstance.TryHandOffAsync(arg).GetAwaiter().GetResult(); + + if (ApplicationLifetime is IControlledApplicationLifetime lifetime) + { + lifetime.Shutdown(0); + } + else + { + Environment.Exit(0); + } + return true; + } + } + return false; + } } diff --git a/src/PostIt/PostIt/Services/CustomSchemeBrowser.cs b/src/PostIt/PostIt/Services/CustomSchemeBrowser.cs new file mode 100644 index 00000000..d93b73ef --- /dev/null +++ b/src/PostIt/PostIt/Services/CustomSchemeBrowser.cs @@ -0,0 +1,108 @@ +using System; +using System.Diagnostics; +using System.Threading; +using System.Threading.Tasks; +using IdentityModel.OidcClient.Browser; + +namespace PostIt.Services; + +/// +/// Browser implementation that uses an OS-registered custom URI +/// scheme (e.g. postit://callback) instead of a loopback +/// HTTP listener. The redirect URI never has to be served: when the +/// browser hits the scheme, the OS launches a fresh PostIt process +/// with the URL on the command line. That process hands the URL off +/// to the running instance via and +/// exits. +/// +/// Reference: RFC 8252 §7.1 (OAuth 2.0 for Native Apps — Custom URI +/// Scheme Redirect). +/// +public class CustomSchemeBrowser : IBrowser +{ + private readonly string _customScheme; + private readonly TimeSpan _timeout; + + public CustomSchemeBrowser(string customScheme, TimeSpan? timeout = null) + { + _customScheme = customScheme; + _timeout = timeout ?? TimeSpan.FromMinutes(5); + } + + public async Task InvokeAsync(BrowserOptions options, CancellationToken cancellationToken = default) + { + // The browser is told to come back to our scheme with the + // auth code. We do not need a network listener. + var endScheme = options.EndUrl; + if (string.IsNullOrEmpty(endScheme) || !endScheme.StartsWith(_customScheme, StringComparison.OrdinalIgnoreCase)) + { + return new BrowserResult + { + ResultType = BrowserResultType.UnknownError, + Error = $"EndUrl must use the {_customScheme}:// scheme; got '{endScheme}'.", + }; + } + + var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + using var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + cts.CancelAfter(_timeout); + + // Spawn the server loop that waits for a 2nd-instance hand-off. + // When the callback URL arrives, complete the TCS so InvokeAsync + // returns it as the BrowserResult.Response. + using var serverCts = CancellationTokenSource.CreateLinkedTokenSource(cts.Token); + var serverTask = SingleInstance.StartServerAsync( + url => tcs.TrySetResult(url), + serverCts.Token); + + try + { + // Open the system browser. UseShellExecute=true is the + // right shape for both Windows and Linux: the OS resolves + // the custom scheme by launching our app. + Process.Start(new ProcessStartInfo(options.StartUrl) { UseShellExecute = true }); + + // Race the callback against the timeout/caller + // cancellation. When the TCS completes the result IS + // the URL we want; when the timer fires first, we treat + // the wait as cancelled. + var delay = Task.Delay(Timeout.Infinite, cts.Token); + var completed = await Task.WhenAny(tcs.Task, delay).ConfigureAwait(false); + if (completed != tcs.Task) + { + return new BrowserResult + { + ResultType = BrowserResultType.Timeout, + Error = "Timed out waiting for the custom-scheme callback.", + }; + } + + var url = await tcs.Task.ConfigureAwait(false); + return new BrowserResult + { + ResultType = BrowserResultType.Success, + Response = url, + }; + } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + return new BrowserResult + { + ResultType = BrowserResultType.UserCancel, + }; + } + catch (Exception ex) + { + return new BrowserResult + { + ResultType = BrowserResultType.UnknownError, + Error = ex.Message, + }; + } + finally + { + serverCts.Cancel(); + try { await serverTask.ConfigureAwait(false); } catch { } + } + } +} diff --git a/src/PostIt/PostIt/Services/LoopbackBrowser.cs b/src/PostIt/PostIt/Services/LoopbackBrowser.cs deleted file mode 100644 index c49aec83..00000000 --- a/src/PostIt/PostIt/Services/LoopbackBrowser.cs +++ /dev/null @@ -1,80 +0,0 @@ -using System; -using System.Diagnostics; -using System.Net; -using System.Net.Sockets; -using System.Text; -using System.Threading; -using System.Threading.Tasks; -using IdentityModel.OidcClient.Browser; - -namespace PostIt.Services; - - public class LoopbackBrowser : IBrowser - { - public async Task InvokeAsync(BrowserOptions options, CancellationToken cancellationToken = default) - { - if (!Uri.TryCreate(options.EndUrl, UriKind.Absolute, out var endUri)) - { - return new BrowserResult { ResultType = BrowserResultType.UnknownError, Error = "Invalid end URL" }; - } - - var prefix = endUri.GetLeftPart(UriPartial.Path); - if (!prefix.EndsWith("/")) prefix += "/"; - - using var listener = new HttpListener(); - listener.Prefixes.Add(prefix); - listener.Start(); - - try - { - Process.Start(new ProcessStartInfo(options.StartUrl) { UseShellExecute = true }); - - // Bound the wait so the port is released even if the user - // closes the browser without completing the flow. Without - // this, a crashed/abandoned login keeps the HttpListener - // bound and the next PostIt launch fails with - // "Failed to listen on prefix … because it conflicts with - // an existing registration". - using var waitCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - waitCts.CancelAfter(TimeSpan.FromMinutes(5)); - - var context = await listener.GetContextAsync().WaitAsync(waitCts.Token).ConfigureAwait(false); - var response = context.Response; - var responseString = "Authentication complete. You can close this window."; - var buffer = Encoding.UTF8.GetBytes(responseString); - response.ContentLength64 = buffer.Length; - await response.OutputStream.WriteAsync(buffer, 0, buffer.Length, cancellationToken).ConfigureAwait(false); - response.OutputStream.Close(); - - var raw = context.Request.Url!.ToString(); - return new BrowserResult - { - ResultType = BrowserResultType.Success, - Response = raw - }; - } - catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) - { - // Bound hit: user did not complete the flow within 5 minutes. - return new BrowserResult - { - ResultType = BrowserResultType.Timeout, - Error = "Timed out waiting for the browser to return the authorization code.", - }; - } - catch (Exception ex) - { - return new BrowserResult { ResultType = BrowserResultType.UnknownError, Error = ex.Message }; - } - finally - { - // Stop() aborts GetContextAsync (releases the bound port); - // Close() disposes the underlying socket. Both are idempotent - // and safe to call after Stop() already succeeded, so calling - // both covers cases where one path throws before the other - // gets a chance (e.g. process-level socket cleanup on Linux). - try { listener.Stop(); } catch { } - try { listener.Close(); } catch { } - } - } - } diff --git a/src/PostIt/PostIt/Services/Platform.cs b/src/PostIt/PostIt/Services/Platform.cs index 3e552be4..258cd5b5 100644 --- a/src/PostIt/PostIt/Services/Platform.cs +++ b/src/PostIt/PostIt/Services/Platform.cs @@ -13,16 +13,28 @@ namespace PostIt.Services; public static class Platform { /// - /// Default redirect URI for the running platform. The desktop loopback - /// default is set here; platform projects override this property at - /// startup (e.g. PostIt.Android sets it to android://postit-signin). + /// Default redirect URI for the running platform. The shared + /// default uses a custom URI scheme (postit://callback) + /// so the OAuth2 redirect is delivered to the running instance + /// through the OS scheme handler — no loopback HTTP listener + /// required (see RFC 8252 §7.1). Platform projects may still + /// override this property at startup (e.g. PostIt.Android sets + /// it to android://postit-signin). /// - public static string DefaultRedirectUri { get; set; } = "http://127.0.0.1:7890/"; + public static string DefaultRedirectUri { get; set; } = "postit://callback"; + + /// + /// Scheme prefix the matches + /// against BrowserOptions.EndUrl. Overridable for apps + /// that want to register their own scheme. + /// + public static string CustomScheme { get; set; } = "postit"; /// /// Constructs a fresh for the running platform. /// May return null if no browser is wired up; in that case /// LoginAsync will surface a clear error. /// - public static System.Func? CreateBrowser { get; set; } + public static System.Func? CreateBrowser { get; set; } = + () => new CustomSchemeBrowser(CustomScheme); } \ No newline at end of file diff --git a/src/PostIt/PostIt/Services/SingleInstance.cs b/src/PostIt/PostIt/Services/SingleInstance.cs new file mode 100644 index 00000000..329bf352 --- /dev/null +++ b/src/PostIt/PostIt/Services/SingleInstance.cs @@ -0,0 +1,98 @@ +using System; +using System.IO; +using System.IO.Pipes; +using System.Text; +using System.Threading; +using System.Threading.Tasks; + +namespace PostIt.Services; + +/// +/// Single-instance coordinator for PostIt. The first process claims a +/// well-known named pipe and starts a server that listens for +/// callback URLs from any subsequent launch. Subsequent launches +/// (triggered by the OS registering the custom URI scheme and the +/// browser opening the scheme after the OIDC redirect) detect the +/// existing instance, write the URL to the pipe, and exit. +/// +/// This is the standard RFC 8252 §7.1 pattern for native apps that +/// use a custom URI scheme: the OS launches a fresh process for each +/// callback, and the running app must hand the URL back to itself. +/// +public static class SingleInstance +{ + public const string PipeName = "PostIt.OidcCallback"; + + /// + /// Tries to open the well-known pipe. If a server is already + /// listening (another PostIt instance), the URL is written and + /// the call returns true to signal the caller to exit. If + /// nothing is listening, returns false and the caller + /// should start its own server via . + /// + public static async Task TryHandOffAsync(string callbackUrl, TimeSpan? timeout = null) + { + try + { + using var client = new NamedPipeClientStream(".", PipeName, PipeDirection.Out); + await client.ConnectAsync((int)(timeout ?? TimeSpan.FromSeconds(2)).TotalMilliseconds).ConfigureAwait(false); + var bytes = Encoding.UTF8.GetBytes(callbackUrl); + await client.WriteAsync(bytes).ConfigureAwait(false); + await client.FlushAsync().ConfigureAwait(false); + return true; + } + catch (TimeoutException) + { + return false; + } + catch (IOException) + { + return false; + } + } + + /// + /// Starts the single-instance server loop. Yields each received + /// callback URL on the returned channel. The server runs until + /// the cancellation token is tripped (typically when the + /// owning returns from + /// InvokeAsync or the process shuts down). + /// + public static async Task StartServerAsync( + Action onCallback, + CancellationToken cancellationToken) + { + while (!cancellationToken.IsCancellationRequested) + { + NamedPipeServerStream? server = null; + try + { + server = new NamedPipeServerStream( + PipeName, + PipeDirection.In, + NamedPipeServerStream.MaxAllowedServerInstances, + PipeTransmissionMode.Byte, + PipeOptions.Asynchronous); + + await server.WaitForConnectionAsync(cancellationToken).ConfigureAwait(false); + + using var ms = new MemoryStream(); + await server.CopyToAsync(ms, cancellationToken).ConfigureAwait(false); + var url = Encoding.UTF8.GetString(ms.ToArray()); + onCallback(url); + } + catch (OperationCanceledException) + { + return; + } + catch (IOException) + { + // Client disconnected mid-write; loop and wait for the next. + } + finally + { + server?.Dispose(); + } + } + } +}