postit: replace loopback HTTP listener with custom URI scheme
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).
This commit is contained in:
parent
8a9851575a
commit
16508e9fb2
7 changed files with 282 additions and 187 deletions
|
|
@ -15,24 +15,24 @@ namespace PostIt.Tests;
|
|||
/// </summary>
|
||||
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;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,92 +0,0 @@
|
|||
using System.Net;
|
||||
using System.Net.Sockets;
|
||||
using PostIt.Services;
|
||||
|
||||
namespace PostIt.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// 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."
|
||||
/// </summary>
|
||||
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;
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue