diff --git a/src/dotnet/Modgud.Api.Tests/Authorization/CocoarPasskeyGrantFlowTests.OriginSuffix.cs b/src/dotnet/Modgud.Api.Tests/Authorization/CocoarPasskeyGrantFlowTests.OriginSuffix.cs new file mode 100644 index 00000000..804abdbc --- /dev/null +++ b/src/dotnet/Modgud.Api.Tests/Authorization/CocoarPasskeyGrantFlowTests.OriginSuffix.cs @@ -0,0 +1,66 @@ +using System.Text; +using System.Text.Json; +using Modgud.Api.Tests.Infrastructure; + +namespace Modgud.Api.Tests.Authorization; + +/// +/// Regression for the per-client RP-ID origin mismatch: when the per-client +/// WebAuthnRpId is a registrable SUFFIX of the app origin (RP-ID +/// a.localhost, page on sub.a.localhost), enrollment used to fail with +/// 400 "Passkey enrollment failed." because the accepted origin was derived as +/// https://{rpId}. The fix accepts the signed origin when it is the RP-ID host +/// or a subdomain of it. +/// +public partial class CocoarPasskeyGrantFlowTests +{ + [Fact] + public async Task NativeEnroll_SignedOriginIsSubdomainOfClientRpId_Succeeds() + { + await EnableNativeGrantsAsync(); + await SeedPasskeyClientAsync("app-a", rpId: "a.localhost"); + + // Bootstrap a login → an app-a access token (existing seeded credential). + using var bootstrap = new SoftwareWebAuthnAuthenticator(DefaultUser!.Id.ToByteArray()); + await SeedCredentialAsync(bootstrap.CredentialId, bootstrap.CosePublicKey(), bootstrap.UserHandle, rpId: "a.localhost"); + var (bootCeremony, bootChallenge, bootRpId) = await BeginAsync(clientId: "app-a"); + var bootAssertion = bootstrap.CreateAssertionJson(bootChallenge, bootRpId, $"https://{bootRpId}"); + var tokenResp = await PostPasskeyAsync("app-a", bootCeremony, bootAssertion); + var tokenBody = await tokenResp.Content.ReadAsStringAsync(TestContext.Current.CancellationToken); + Assert.True(tokenResp.IsSuccessStatusCode, $"bootstrap login failed ({(int)tokenResp.StatusCode}): {tokenBody}"); + var accessToken = JsonDocument.Parse(tokenBody).RootElement.GetProperty("access_token").GetString()!; + + var bearer = Factory.CreateClient(); + bearer.DefaultRequestHeaders.Authorization = new("Bearer", accessToken); + + // Enroll begin → options pinned to the client's RP-ID a.localhost. + var beginResp = await bearer.PostAsync("/connect/passkey/enroll/begin", content: null, TestContext.Current.CancellationToken); + var beginBody = await beginResp.Content.ReadAsStringAsync(TestContext.Current.CancellationToken); + Assert.True(beginResp.IsSuccessStatusCode, $"enroll/begin failed ({(int)beginResp.StatusCode}): {beginBody}"); + using var beginJson = JsonDocument.Parse(beginBody); + var enrollCeremonyId = beginJson.RootElement.GetProperty("ceremonyId").GetString()!; + var opts = beginJson.RootElement.GetProperty("options"); + var enrollChallenge = opts.GetProperty("challenge").GetString()!; + var enrollRpId = opts.GetProperty("rp").GetProperty("id").GetString()!; + Assert.Equal("a.localhost", enrollRpId); + + // The credential is created on a SUBDOMAIN of the RP-ID — the case that broke. + using var enrolling = new SoftwareWebAuthnAuthenticator(Encoding.UTF8.GetBytes(DefaultUser!.Id.ToString())); + var origin = $"https://sub.{enrollRpId}"; // https://sub.a.localhost — a registrable suffix of a.localhost + var attestation = enrolling.CreateAttestationJson(enrollChallenge, enrollRpId, origin); + var enrollReqBody = $"{{\"ceremonyId\":\"{enrollCeremonyId}\",\"attestation\":{attestation}}}"; + + var enrollResp = await bearer.PostAsync("/connect/passkey/enroll", + new StringContent(enrollReqBody, Encoding.UTF8, "application/json"), + TestContext.Current.CancellationToken); + + var enrollRespBody = await enrollResp.Content.ReadAsStringAsync(TestContext.Current.CancellationToken); + Assert.True(enrollResp.IsSuccessStatusCode, + $"enroll from subdomain origin failed ({(int)enrollResp.StatusCode}): {enrollRespBody}"); + + // Stored under the client's RP-ID (a.localhost), not the subdomain. + var stored = await LoadCredentialAsync(enrolling.CredentialId); + Assert.NotNull(stored); + Assert.Equal("a.localhost", stored!.RpId); + } +} diff --git a/src/dotnet/Modgud.Api/Features/Auth/NativePasskeyEnrollEndpoints.cs b/src/dotnet/Modgud.Api/Features/Auth/NativePasskeyEnrollEndpoints.cs index 40226141..8620b3eb 100644 --- a/src/dotnet/Modgud.Api/Features/Auth/NativePasskeyEnrollEndpoints.cs +++ b/src/dotnet/Modgud.Api/Features/Auth/NativePasskeyEnrollEndpoints.cs @@ -166,12 +166,23 @@ public static WebApplication MapNativePasskeyEnrollEndpoints(this WebApplication session.Delete(ceremony); await session.SaveChangesAsync(ct); + // Parse the attestation up-front so the origin the authenticator actually + // signed can scope the relying party (below) — a per-client RP ID that is + // a registrable suffix of the app origin (RP-ID amzettel.at for a page on + // app.amzettel.at) is then accepted instead of failing the origin check. + var attestation = JsonSerializer.Deserialize( + attEl.GetRawText(), new JsonSerializerOptions { PropertyNameCaseInsensitive = true }); + if (attestation is null) + return Results.BadRequest(new { Message = "Invalid attestation response." }); + IFido2 fido2; try { // Pinned RP ID from begin — never re-resolved (admin edits mid-ceremony - // cannot drift the attestation's RP ID). - fido2 = await fido2Factory.CreateAsync(ct, rpIdOverride: ceremony.RpId); + // cannot drift the attestation's RP ID). The accepted origin is the + // signed one, filtered to this RP-ID's own subdomains in BuildConfiguration. + fido2 = await fido2Factory.CreateAsync(ct, rpIdOverride: ceremony.RpId, + additionalOrigins: PresentedOrigins(attestation.Response?.ClientDataJson)); } catch (RelyingPartyUnavailableException ex) { @@ -188,11 +199,6 @@ public static WebApplication MapNativePasskeyEnrollEndpoints(this WebApplication return Results.BadRequest(new { Message = "Enrollment session expired. Please try again." }); } - var attestation = JsonSerializer.Deserialize( - attEl.GetRawText(), new JsonSerializerOptions { PropertyNameCaseInsensitive = true }); - if (attestation is null) - return Results.BadRequest(new { Message = "Invalid attestation response." }); - RegisteredPublicKeyCredential credential; try { @@ -210,10 +216,16 @@ public static WebApplication MapNativePasskeyEnrollEndpoints(this WebApplication }, }, ct); } - catch + catch (Exception ex) { // Fail closed on any attestation failure (bad/forged response, - // duplicate credential) — never a 500. + // duplicate credential, origin/RP-ID mismatch) — never a 500. Log the + // reason into the per-realm error feed so an origin mismatch is + // diagnosable instead of surfacing as a reasonless 400. + context.RequestServices.GetRequiredService() + .CreateLogger(LoggerCategory) + .LogWarning(ex, "Native passkey enrollment verification failed (UserId={UserId}, RpId={RpId}).", + user!.Id, ceremony.RpId); return Results.BadRequest(new { Message = "Passkey enrollment failed." }); } @@ -248,6 +260,12 @@ public static WebApplication MapNativePasskeyEnrollEndpoints(this WebApplication return application; } + /// The single signed origin (if any) read from the attestation's + /// clientDataJSON, as a one-element array for CreateAsync(additionalOrigins:). + /// null when absent/malformed — then only the RP-ID host is accepted. + private static string[]? PresentedOrigins(byte[]? clientDataJson) + => RealmFido2.TryGetClientDataOrigin(clientDataJson) is { } origin ? [origin] : null; + private static IResult RpUnavailable(HttpContext context, RelyingPartyUnavailableException ex) { context.RequestServices.GetRequiredService() diff --git a/src/dotnet/Modgud.Api/Features/Auth/OAuth/AuthorizationEndpoints.cs b/src/dotnet/Modgud.Api/Features/Auth/OAuth/AuthorizationEndpoints.cs index 816efadd..887af59e 100644 --- a/src/dotnet/Modgud.Api/Features/Auth/OAuth/AuthorizationEndpoints.cs +++ b/src/dotnet/Modgud.Api/Features/Auth/OAuth/AuthorizationEndpoints.cs @@ -1,4 +1,5 @@ using System.Security.Claims; +using System.Text.Json; using Modgud.Authentication.Applications; using Modgud.Authentication.Domain; using Modgud.Authentication.Identity; @@ -820,10 +821,24 @@ private static async Task ExchangeNativePasskeyAsync( var primaryDomain = await rpIdResolver.GetPrimaryDomainAsync(ct); var activeRpId = string.IsNullOrWhiteSpace(ceremony.RpId) ? primaryDomain : ceremony.RpId; + // Accept the origin the authenticator actually signed (scoped in + // BuildConfiguration to this RP-ID's own subdomains) so a per-client RP-ID + // that is a registrable suffix of the app origin still verifies. Malformed + // input yields no extra origin; the shared verifier then fails closed. + string[]? presentedOrigins = null; + try + { + var assertion = JsonSerializer.Deserialize( + assertionJson, new JsonSerializerOptions { PropertyNameCaseInsensitive = true }); + if (RealmFido2.TryGetClientDataOrigin(assertion?.Response?.ClientDataJson) is { } origin) + presentedOrigins = [origin]; + } + catch (JsonException) { /* leave null — verifier fails closed below */ } + IFido2 fido2; try { - fido2 = await fido2Factory.CreateAsync(ct, rpIdOverride: activeRpId); + fido2 = await fido2Factory.CreateAsync(ct, rpIdOverride: activeRpId, additionalOrigins: presentedOrigins); } catch (RelyingPartyUnavailableException) { diff --git a/src/dotnet/Modgud.Authentication/Identity/RealmFido2.cs b/src/dotnet/Modgud.Authentication/Identity/RealmFido2.cs index d7c88ca5..dfaf17c5 100644 --- a/src/dotnet/Modgud.Authentication/Identity/RealmFido2.cs +++ b/src/dotnet/Modgud.Authentication/Identity/RealmFido2.cs @@ -1,3 +1,4 @@ +using System.Text.Json; using Fido2NetLib; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; @@ -36,8 +37,23 @@ public static class RealmFido2 /// SPA dev-server origin (http://{PrimaryDomain}:4300) since that is /// where the WebAuthn ceremony runs in dev. localhost / /// *.localhost are browser secure-contexts, so dev passkeys work. + /// + /// — origins the authenticator + /// actually signed (read from the ceremony's clientDataJSON). Each is accepted + /// ONLY if it is the RP-ID host or a subdomain of it (see + /// ). This is what makes a per-client RP-ID that + /// is a registrable SUFFIX of the app origin work: RP-ID amzettel.at for a + /// page on app.amzettel.at is spec-valid (the browser only ever presents + /// an origin whose effective domain has the RP-ID as a suffix), but deriving the + /// accepted origin as https://{rpId} wrongly rejected it. The RP-ID hash + /// and signature checks remain the primary boundary; this only widens the origin + /// allow-list to the set the WebAuthn spec already scopes to this RP-ID. /// - public static Fido2Configuration BuildConfiguration(Realm realm, IWebHostEnvironment env, string? rpIdOverride = null) + public static Fido2Configuration BuildConfiguration( + Realm realm, + IWebHostEnvironment env, + string? rpIdOverride = null, + IEnumerable? additionalOrigins = null) { ArgumentNullException.ThrowIfNull(realm); ArgumentNullException.ThrowIfNull(env); @@ -74,6 +90,17 @@ public static Fido2Configuration BuildConfiguration(Realm realm, IWebHostEnviron origins.Add($"https://{host}"); } + // Widen the accepted origins to the actual signed origin(s), but only those + // genuinely under this RP-ID — never a foreign host. + if (additionalOrigins is not null) + { + foreach (var origin in additionalOrigins) + { + if (IsOriginUnderRpId(origin, host, env.IsDevelopment())) + origins.Add(origin); + } + } + return new Fido2Configuration { // RP ID — the effective domain a passkey is bound to. @@ -82,6 +109,44 @@ public static Fido2Configuration BuildConfiguration(Realm realm, IWebHostEnviron Origins = origins, }; } + + /// + /// True when is an absolute https URL (also + /// http when , i.e. dev) whose host equals + /// or is a subdomain of it — exactly the set of origins + /// WebAuthn already scopes to this RP-ID. The dotted-suffix test (host ends + /// with "." + rpId) deliberately rejects look-alikes like + /// amzettel.at.evil.com and evilamzettel.at. + /// + public static bool IsOriginUnderRpId(string? origin, string? rpId, bool allowInsecure) + { + if (string.IsNullOrWhiteSpace(origin) || string.IsNullOrWhiteSpace(rpId)) return false; + if (!Uri.TryCreate(origin, UriKind.Absolute, out var uri)) return false; + if (uri.Scheme != Uri.UriSchemeHttps && !(allowInsecure && uri.Scheme == Uri.UriSchemeHttp)) + return false; + return string.Equals(uri.Host, rpId, StringComparison.OrdinalIgnoreCase) + || uri.Host.EndsWith("." + rpId, StringComparison.OrdinalIgnoreCase); + } + + /// + /// Extracts the WebAuthn origin from a clientDataJSON byte payload (the + /// value Fido2NetLib already base64url-decoded onto the raw response). Returns + /// null on any malformed input — the caller then simply passes no extra + /// origin and the verify fails closed as before. + /// + public static string? TryGetClientDataOrigin(byte[]? clientDataJson) + { + if (clientDataJson is null || clientDataJson.Length == 0) return null; + try + { + using var doc = JsonDocument.Parse(clientDataJson); + return doc.RootElement.TryGetProperty("origin", out var o) ? o.GetString() : null; + } + catch (JsonException) + { + return null; + } + } } /// @@ -97,7 +162,10 @@ public sealed class RealmScopedFido2Factory( IWebHostEnvironment env, IMetadataService? metadataService = null) { - public async Task CreateAsync(CancellationToken ct = default, string? rpIdOverride = null) + public async Task CreateAsync( + CancellationToken ct = default, + string? rpIdOverride = null, + IEnumerable? additionalOrigins = null) { var http = httpContextAccessor.HttpContext ?? throw new InvalidOperationException( @@ -110,7 +178,9 @@ public async Task CreateAsync(CancellationToken ct = default, string? rp // ADR-0009 seam: rpIdOverride is null for every Phase-2 caller (RP-ID stays // realm.PrimaryDomain). Threaded through so the Phase-3 per-client RP-ID // only has to supply the value here — no new RP-ID code path. - var config = RealmFido2.BuildConfiguration(realm, env, rpIdOverride); + // additionalOrigins carries the actual signed origin at verify time so a + // per-client RP-ID that is a suffix of the app origin is accepted. + var config = RealmFido2.BuildConfiguration(realm, env, rpIdOverride, additionalOrigins); // metadataService is optional — the previous global setup used the // library's NullMetadataService (no attestation-metadata validation), // and passing null here gives the identical behaviour. diff --git a/src/dotnet/Modgud.Tests.Unit/Identity/RealmFido2Tests.cs b/src/dotnet/Modgud.Tests.Unit/Identity/RealmFido2Tests.cs new file mode 100644 index 00000000..0b3fdc3c --- /dev/null +++ b/src/dotnet/Modgud.Tests.Unit/Identity/RealmFido2Tests.cs @@ -0,0 +1,101 @@ +using System.Text; +using Microsoft.AspNetCore.Hosting; +using Microsoft.Extensions.FileProviders; +using Modgud.Authentication.Identity; +using Modgud.Domain.Realms; + +namespace Modgud.Tests.Unit.Identity; + +/// +/// Pins the per-client WebAuthn RP-ID / origin behaviour in . +/// The bug: a per-client RP-ID is meant to be a registrable SUFFIX of the app origin +/// (RP-ID amzettel.at for a page on app.amzettel.at), but the accepted +/// origin was derived as https://{rpId}, so a passkey enroll/login from the +/// real subdomain origin failed the FIDO2 origin allow-list. The fix accepts any +/// signed origin that is the RP-ID host or a subdomain of it — exactly the set +/// WebAuthn already scopes to that RP-ID — and nothing else. +/// +public class RealmFido2Tests +{ + // ── IsOriginUnderRpId — the security-critical suffix filter ────────────────── + + [Theory] + // Same host and genuine subdomains are accepted. + [InlineData("https://amzettel.at", "amzettel.at", false, true)] + [InlineData("https://app.amzettel.at", "amzettel.at", false, true)] + [InlineData("https://deep.app.amzettel.at", "amzettel.at", false, true)] + // Look-alikes and foreign hosts are rejected. + [InlineData("https://evil.at", "amzettel.at", false, false)] + [InlineData("https://amzettel.at.evil.com", "amzettel.at", false, false)] + [InlineData("https://evilamzettel.at", "amzettel.at", false, false)] + // Scheme: https only in prod; http only when insecure (dev) is allowed. + [InlineData("http://app.amzettel.at", "amzettel.at", false, false)] + [InlineData("http://app.amzettel.at", "amzettel.at", true, true)] + // Junk / empty / non-absolute is rejected. + [InlineData("app.amzettel.at", "amzettel.at", false, false)] + [InlineData("", "amzettel.at", false, false)] + [InlineData("https://app.amzettel.at", "", false, false)] + public void IsOriginUnderRpId_AcceptsOnlyRpIdHostOrSubdomain( + string origin, string rpId, bool allowInsecure, bool expected) + => Assert.Equal(expected, RealmFido2.IsOriginUnderRpId(origin, rpId, allowInsecure)); + + // ── TryGetClientDataOrigin ─────────────────────────────────────────────────── + + [Fact] + public void TryGetClientDataOrigin_ReadsOrigin() + { + var clientData = Encoding.UTF8.GetBytes( + "{\"type\":\"webauthn.create\",\"challenge\":\"abc\",\"origin\":\"https://app.amzettel.at\"}"); + Assert.Equal("https://app.amzettel.at", RealmFido2.TryGetClientDataOrigin(clientData)); + } + + [Fact] + public void TryGetClientDataOrigin_NullOrGarbage_ReturnsNull() + { + Assert.Null(RealmFido2.TryGetClientDataOrigin(null)); + Assert.Null(RealmFido2.TryGetClientDataOrigin([])); + Assert.Null(RealmFido2.TryGetClientDataOrigin(Encoding.UTF8.GetBytes("not json"))); + Assert.Null(RealmFido2.TryGetClientDataOrigin(Encoding.UTF8.GetBytes("{\"type\":\"webauthn.create\"}"))); + } + + // ── BuildConfiguration — the end of the wiring ─────────────────────────────── + + [Fact] + public void BuildConfiguration_PerClientRpId_AcceptsSignedSubdomainOrigin() + { + var realm = new Realm { Slug = "system", DisplayName = "Acme", PrimaryDomain = "auth.cocoar.dev" }; + + var config = RealmFido2.BuildConfiguration( + realm, ProdEnv, rpIdOverride: "amzettel.at", + additionalOrigins: ["https://app.amzettel.at"]); + + Assert.Equal("amzettel.at", config.ServerDomain); // RP-ID unchanged + Assert.Contains("https://amzettel.at", config.Origins); // the RP-ID host itself + Assert.Contains("https://app.amzettel.at", config.Origins); // the real signed origin + } + + [Fact] + public void BuildConfiguration_ForeignSignedOrigin_NotAccepted() + { + var realm = new Realm { Slug = "system", DisplayName = "Acme", PrimaryDomain = "auth.cocoar.dev" }; + + var config = RealmFido2.BuildConfiguration( + realm, ProdEnv, rpIdOverride: "amzettel.at", + additionalOrigins: ["https://evil.at"]); + + Assert.DoesNotContain("https://evil.at", config.Origins); + Assert.Contains("https://amzettel.at", config.Origins); + } + + private static readonly IWebHostEnvironment ProdEnv = new FakeWebHostEnvironment("Production"); + + private sealed class FakeWebHostEnvironment(string environmentName) : IWebHostEnvironment + { + public string EnvironmentName { get; set; } = environmentName; + public string ApplicationName { get; set; } = "Modgud.Tests.Unit"; + public string WebRootPath { get; set; } = ""; + public IFileProvider WebRootFileProvider { get; set; } = null!; + public string ContentRootPath { get; set; } = ""; + public IFileProvider ContentRootFileProvider { get; set; } = null!; + } +}