Public Access
Run M1's end-to-end slice, and fix the two bugs it found
The whole vertical slice now runs against a real Keycloak, a real API, a real PostgreSQL and a real sshd: sign in through the browser flow, enroll with the identity-provider key binding, unlock, create a host, sync it, read it back on a second machine, unlock again with no network, accept an unseen host key, and open an interactive shell. Opt-in, because it needs the development stack; skipped with a message naming the commands. It found two bugs on its first run, and both are the same class: two sides of a stub agreeing with each other about something the specification never said. **The API never applied DodoSshJsonContext to its HTTP JSON options.** Minimal APIs therefore used the framework's web defaults, which write an enum as a number. Every request DTO carrying one failed to bind against a client writing the specified string form — which is the entire sync surface, unreachable from the real client, with a 400 naming only the parameter. The documented guarantee that request bodies reject unmapped members was likewise not in effect anywhere. Nothing caught it because the API tests posted with PostAsJsonAsync's defaults, so they and the server had independently settled on integers. Those tests now serialise through the contract, which is the deeper fix: removing the new configuration fails 13 of them. Copying settings into options a host owns is itself the hazard the context warns about, so ApplyTo lives beside the settings it mirrors and ApplyToTests pins the transformation, including that inserting the resolver leaves the caller's own in place. **The realm registered a loopback redirect URI Keycloak rejects.** `http://127.0.0.1:*/callback` looks more explicit than the RFC 8252 form and is broken: Keycloak's wildcards are trailing-only, so the `*` parses as a literal port and every authorization request came back "Invalid parameter: redirect_uri". Providers ignore the port for loopback hosts, which is the whole mechanism, so the correct registration is `http://127.0.0.1/callback` — path pinned, port free. The value the server advertises through the discovery document said the same wrong thing and now says the right one. Two smaller things, both documented in docs/platform-flags.md: - --import-realm skips a realm that already exists, so editing the realm file and restarting Keycloak changes nothing and serves stale configuration. The container has to be recreated. The compose comment claimed the opposite. - Keycloak marks its session cookies Secure even over plain HTTP, because SameSite=None requires it. A spec-conformant client drops them and the login POST answers 400 with no message; browsers complete the flow only because they exempt loopback. Harmless for the product, fatal for automation, so ScriptedBrowser carries the cookies by hand and says why. Also: the server enforces a 64 MiB floor on the passphrase KDF, so this suite cannot use the 8 MiB profile the other client suites take for speed. Those only get away with it because their in-memory servers have no policy — worth knowing rather than rediscovering. 638 tests. The solution-wide run stays green with the stack down: exit code 8 means "no tests ran", which the platform reports as failure, so the opt-in project ignores exactly that code.
This commit is contained in:
@@ -11,6 +11,7 @@ var builder = WebApplication.CreateBuilder(args);
|
||||
builder.Configuration.AddKeyPerFile("/run/secrets", optional: true);
|
||||
builder.Configuration.AddEnvironmentVariables(prefix: "DODOSSH_");
|
||||
|
||||
builder.Services.AddDodoJson();
|
||||
builder.Services.AddDodoOptions();
|
||||
builder.Services.AddDodoPersistence(builder.Configuration);
|
||||
builder.Services.AddDodoAuthentication();
|
||||
|
||||
@@ -34,7 +34,15 @@ public sealed class OidcOptions
|
||||
/// an ephemeral port with the system browser, never a custom scheme and never an embedded
|
||||
/// browser, so the user can see the real address bar.
|
||||
/// </summary>
|
||||
public string LoopbackRedirectPattern { get; set; } = "http://127.0.0.1:*/callback";
|
||||
/// <remarks>
|
||||
/// No wildcard in the port. RFC 8252 requires a native client to use an ephemeral loopback port, and
|
||||
/// providers implement that by ignoring the port when the host is a loopback literal — Keycloak
|
||||
/// included. Writing <c>http://127.0.0.1:*/callback</c> looks more explicit and is worse: Keycloak
|
||||
/// parses the <c>*</c> as a literal port and rejects every real redirect with "Invalid parameter:
|
||||
/// redirect_uri". Pinning the path is the part that matters, since it stops another local process
|
||||
/// having a code delivered somewhere else.
|
||||
/// </remarks>
|
||||
public string LoopbackRedirectPattern { get; set; } = "http://127.0.0.1/callback";
|
||||
|
||||
/// <summary>Whether HTTPS metadata is required. Only ever false for local development.</summary>
|
||||
public bool RequireHttpsMetadata { get; set; } = true;
|
||||
|
||||
@@ -0,0 +1,28 @@
|
||||
using DodoSSH.Contracts;
|
||||
|
||||
namespace DodoSSH.Api.Setup;
|
||||
|
||||
/// <summary>Brings the host's JSON handling into line with the shared contract.</summary>
|
||||
internal static class Json
|
||||
{
|
||||
/// <summary>
|
||||
/// Applies <see cref="DodoSshJsonContext"/>'s settings to the minimal-API serialiser.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// Not optional, and not a performance tweak. Without it the framework's web defaults apply: camelCase
|
||||
/// property names — which happen to match — but enums as <em>numbers</em>, and no rejection of unmapped
|
||||
/// members. Every request DTO carrying an enum then fails to bind against a client that writes the
|
||||
/// specified string form, which is the entire sync surface. The failure is a 400 naming only the
|
||||
/// parameter, and it is invisible to any test that posts with its own default options rather than the
|
||||
/// contract's.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// Found by running the real client against the real server for the first time. Every test until then
|
||||
/// serialised its requests with <c>PostAsJsonAsync</c>'s defaults, so both sides agreed on integers and
|
||||
/// nothing disagreed with anything.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
internal static IServiceCollection AddDodoJson(this IServiceCollection services) =>
|
||||
services.ConfigureHttpJsonOptions(options => DodoSshJsonContext.ApplyTo(options.SerializerOptions));
|
||||
}
|
||||
@@ -13,7 +13,7 @@
|
||||
"Oidc": {
|
||||
"Audience": "dodossh-api",
|
||||
"ClientId": "dodossh-desktop",
|
||||
"LoopbackRedirectPattern": "http://127.0.0.1:*/callback",
|
||||
"LoopbackRedirectPattern": "http://127.0.0.1/callback",
|
||||
"RequireHttpsMetadata": true,
|
||||
"AllowEmailLinking": false
|
||||
},
|
||||
|
||||
@@ -72,6 +72,55 @@ public sealed partial class DodoSshJsonContext : JsonSerializerContext
|
||||
/// </remarks>
|
||||
public static JsonSerializerOptions StrictRequestOptions => LazyStrictRequestOptions.Value;
|
||||
|
||||
/// <summary>
|
||||
/// Applies these settings to options this process does not own.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// <para>
|
||||
/// ASP.NET Core exposes its JSON options as a get-only <see cref="JsonSerializerOptions"/> already
|
||||
/// constructed from <see cref="JsonSerializerDefaults.Web"/>, so a host cannot simply hand it
|
||||
/// <see cref="StrictRequestOptions"/> — it has to be mutated in place. That copying lives here, next
|
||||
/// to the settings it has to mirror, so adding a setting above is one edit rather than two in different
|
||||
/// projects with nothing connecting them.
|
||||
/// </para>
|
||||
/// <para>
|
||||
/// <b>Why this matters more than it looks.</b> Without it the framework defaults apply, and those
|
||||
/// serialise an enum as a number. Every request DTO carrying one — which is the whole sync surface —
|
||||
/// then fails to bind against a client that writes the specified string form, with a 400 that names
|
||||
/// only the parameter. Nothing catches it if the tests post with their own default options, because
|
||||
/// both sides then agree on a form the specification never described.
|
||||
/// </para>
|
||||
/// </remarks>
|
||||
/// <param name="target">Options to bring into line. Must not already be read-only.</param>
|
||||
public static void ApplyTo(JsonSerializerOptions target)
|
||||
{
|
||||
ArgumentNullException.ThrowIfNull(target);
|
||||
|
||||
var source = StrictRequestOptions;
|
||||
|
||||
target.PropertyNamingPolicy = source.PropertyNamingPolicy;
|
||||
target.PropertyNameCaseInsensitive = source.PropertyNameCaseInsensitive;
|
||||
target.NumberHandling = source.NumberHandling;
|
||||
target.DefaultIgnoreCondition = source.DefaultIgnoreCondition;
|
||||
target.UnmappedMemberHandling = source.UnmappedMemberHandling;
|
||||
target.DictionaryKeyPolicy = source.DictionaryKeyPolicy;
|
||||
|
||||
foreach (var converter in source.Converters)
|
||||
{
|
||||
target.Converters.Add(converter);
|
||||
}
|
||||
|
||||
// Inserted at the front rather than assigned, so contract types use the source-generated metadata
|
||||
// while any resolver the caller already installed keeps handling everything else — ProblemDetails
|
||||
// among them, which a host must still be able to write.
|
||||
//
|
||||
// Options carrying no resolver at all end up with a chain containing only this context, and
|
||||
// anything outside the contract then throws NotSupportedException. That is deliberate: adding a
|
||||
// reflection fallback here would quietly cost trimmability, which is half the reason this context
|
||||
// is source-generated. ASP.NET Core's own JSON options already have one.
|
||||
target.TypeInfoResolverChain.Insert(0, Default);
|
||||
}
|
||||
|
||||
/// <remarks>
|
||||
/// Lazy, not a static initialiser. The generated <c>Default</c> property is a static of this
|
||||
/// same class, so reading it from this type's initialiser is a cycle: the accessor runs
|
||||
|
||||
@@ -523,6 +523,7 @@ static DodoSSH.Contracts.DirectoryEntry.operator !=(DodoSSH.Contracts.DirectoryE
|
||||
static DodoSSH.Contracts.DirectoryEntry.operator ==(DodoSSH.Contracts.DirectoryEntry? left, DodoSSH.Contracts.DirectoryEntry? right) -> bool
|
||||
static DodoSSH.Contracts.DodoSshConfiguration.operator !=(DodoSSH.Contracts.DodoSshConfiguration? left, DodoSSH.Contracts.DodoSshConfiguration? right) -> bool
|
||||
static DodoSSH.Contracts.DodoSshConfiguration.operator ==(DodoSSH.Contracts.DodoSshConfiguration? left, DodoSSH.Contracts.DodoSshConfiguration? right) -> bool
|
||||
static DodoSSH.Contracts.DodoSshJsonContext.ApplyTo(System.Text.Json.JsonSerializerOptions! target) -> void
|
||||
static DodoSSH.Contracts.DodoSshJsonContext.ResponseOptions.get -> System.Text.Json.JsonSerializerOptions!
|
||||
static DodoSSH.Contracts.DodoSshJsonContext.StrictRequestOptions.get -> System.Text.Json.JsonSerializerOptions!
|
||||
static DodoSSH.Contracts.EncryptedPayload.operator !=(DodoSSH.Contracts.EncryptedPayload? left, DodoSSH.Contracts.EncryptedPayload? right) -> bool
|
||||
|
||||
Reference in New Issue
Block a user