fix(identity): resolve front-end origin per-request for auth e-mail links

[marcelo-maciel]

Problem

The kit ships two front-ends (admin on :5173, dashboard on :5174), but the back-end had no way to build a user-facing link that targets the front-end a request actually came from:

• forgot-password built the reset link from a single configured OriginOptions.OriginUrl. In appsettings.json that value is the API URL (https://localhost:7030), and in appsettings.Production.json it is empty, so the handler threw "Origin URL is not configured.".
• register / self-register / resend-confirmation built the confirmation link from the raw request host, i.e. the API, and pointed it at the API route api/v1/identity/confirm-email (which returns JSON) rather than a front-end page.
• The HTTP Origin header was never consulted, so with more than one SPA there was no way to send the link to the correct one.

This is the structural follow-up to #1302, which fixed only the reset-link string format (trailing slash, tenant param, URL-encoding).

Solution

A framework-level IFrontendOriginResolver with two notions of origin, matched to who receives the link:

• ResolveForCurrentRequest() (self-service: forgot-password, self-register) reads the request Origin header, validates it against an allow-list, and returns the canonical configured entry (never the client's raw casing). A present-but-unlisted origin is a forged or misconfigured client, so it throws a 400-mapped exception. When the request carries no Origin header (non-browser callers: curl, the Scalar try-it UI, mobile, server-to-server), it falls back to a configured default rather than failing an otherwise valid flow.
• ResolveDefault() (operator-driven: an admin registering or re-inviting a tenant user, whose confirmation link must land on the tenant's app rather than the operator's; and background jobs with no HTTP request) returns the configured default front-end origin.

Matching is component-wise Uri comparison (scheme + host + port, port exact), normalized once at startup, so an entry like :443 or an IDN form does not silently fail a raw string compare.

The confirmation e-mail now points at the SPA /confirm-email page (which already exists in both clients/admin and clients/dashboard and calls the API) instead of the API route directly.

Changes to src/BuildingBlocks (Golden Rule #4, requesting sign-off)

The first revision of this PR kept the resolver inside the Identity module and coupled it to CorsOptions. Per your review (coupling the e-mail-link trust list to the CORS list breaks same-origin / reverse-proxy topologies), the resolver is now framework-level so any module that sends user-facing links (Identity today, Notifications / Billing / Tickets tomorrow) resolves the origin the same way. That places it in protected code, and the PR description must say so plainly:

• new src/BuildingBlocks/Web/Frontend/ — IFrontendOriginResolver, FrontendOriginResolver (internal), FrontendOptions.
• modified src/BuildingBlocks/Web/Extensions.cs — binds FrontendOptions, registers the resolver and IHttpContextAccessor, and logs the one startup Warning when DefaultOrigin is unset.
• modified src/BuildingBlocks/Web/Web.csproj — InternalsVisibleTo("Framework.Tests") so the internal resolver is unit-testable.

Flagging explicitly for approval under Golden Rule #4; the earlier "no changes to BuildingBlocks" line was wrong and is corrected here.

Config and upgrade note (Golden Rule #10)

A dedicated FrontendOptions, deliberately separate from CorsOptions:

• FrontendOptions:AllowedOrigins — SPA origins trusted to appear in e-mail links.
• FrontendOptions:DefaultOrigin — fallback SPA for non-browser and operator-driven flows.

appsettings.json lists the dev SPA origins (http://localhost:5173, http://localhost:5174) plus a DefaultOrigin, so a local run and the Aspire stack work unchanged. appsettings.Production.json ships both empty.

An existing deployment keeps booting after the upgrade. There is no ValidateOnStart on these settings: loud at first use of the feature is right, loud at process start for a feature the deployment may never exercise is not. With DefaultOrigin unset the host starts, logs a single startup Warning naming the setting, the config file and what degrades, and ResolveDefault() walks a fallback chain:

1. FrontendOptions:DefaultOrigin
2. OriginOptions:OriginUrl (the API's own configured public base)
3. the current request's host — because appsettings.Production.json ships OriginUrl empty too, so a deployment that touched neither setting must still produce a link
4. otherwise throw — a background job has no request to derive a host from, and there is genuinely nothing to build a link out of

Tiers 2–3 put the link on the API rather than the SPA: serviceable, and the same place register / self-register / resend derived their links from before this PR. Nothing goes dark, and the operator is told.

Tier 3 is the API's own request host, never the caller's Origin header — that distinction is the whole reason ResolveDefault() exists apart from ResolveForCurrentRequest(), so an operator-driven confirmation link still cannot point back at the admin SPA the request came from. Forged-origin rejection is untouched: a present Origin that misses a configured allow-list is still a 400, never swapped for a fallback.

AllowedOrigins is purely additive: it only widens which request origins may be echoed into self-service links. An empty list means there is nothing to validate against, so the header is discarded and the link uses DefaultOrigin — browsers attach Origin to these POSTs even same-origin, so matching an empty list would 400 every legitimate reset on the shipped Production config and on any single-SPA or reverse-proxy topology. The client's value is never echoed either way, so this is not a relaxation: a forged origin against a configured list is still a 400. The startup Warning names the empty list separately from a missing DefaultOrigin, since a deployment can get one right and the other wrong. OriginOptions:OriginUrl keeps its meaning as the API public base (avatars / IRequestContext.Origin); it is no longer overloaded as the reset-link base, but it is now also the fallback for link building.

Known limitation: DefaultOrigin is a single global, not per-tenant / custom-domain aware, so operator-driven register / resend point every tenant's link at that one SPA. That fits the kit's single-dashboard model; a per-tenant-custom-domain deployment would resolve the recipient tenant's own origin instead. Documented on the option.

Security

The allow-list check is the security boundary: because forgot-password is anonymous, a forged Origin header must never be turned into a link inside an e-mail. The resolver validates against FrontendOptions:AllowedOrigins independently of CorsOptions.AllowAll, returns only the canonical listed entry, and rejects anything else with a 400. Rejections log at Debug (anonymous endpoints, so bot traffic would flood the aggregator at Warning); a genuine deployer misconfig still surfaces as a 400 to the affected SPA's own users.

Tests

• FrontendOriginResolverTests (Framework.Tests) — allow-listed origin returns the canonical entry; trailing-slash / case match; differing port does not match; forged origin throws 400; missing header falls back to DefaultOrigin. Boot-safety tiers: DefaultOrigin unset (and the empty string appsettings.Production.json ships) falls back to the API origin; a configured DefaultOrigin wins over it; a non-absolute OriginUrl (also shipped as "") is skipped in favour of the request host; nothing configured and no request throws; and a forged header is still 400 even when a fallback is available.
• ForgotPasswordCommandHandlerTests updated to the resolver.
• Integration — ForgotPassword_Should_Reject_When_OriginNotAllowed drives a forged Origin end-to-end (rejected, no reset link); the harness sends an Origin header like a browser.

Docs

Docs + changelog land in the separate fullstackhero/docs site: docs#232.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[iammukeshm]

Thanks for this — the problem statement is exactly right (the kit had no way to target the correct SPA per request, and forgot-password was broken out of the box), the writeup is excellent, and the test coverage is thorough. The direction (validate the Origin header against an allow-list before it can end up in an e-mail) is the right security posture. That said, a deep review surfaced several issues that make this unsafe to merge as-is. Requesting changes.

Blocking

1. FrontendOrigin() ignores CorsOptions.AllowAll — the default config bricks all four auth flows (OriginResolver.cs:20)

CorsOptions defaults to AllowAll = true with AllowedOrigins = [], and the options validation in AddHeroCors only requires AllowedOrigins when AllowAll is false — so an AllowAll deployment (including appsettings.Development.json as shipped, and any deployment that omits the section) boots cleanly, passes CORS for every origin, and then 500s on every register / self-register / forgot-password / resend-confirmation call because the resolver consults only the empty static list. The unit test FrontendOrigin_Should_Throw_When_AllowListEmpty_EvenWithHeader codifies the inconsistency rather than resolving it. Coupling the e-mail-link trust list to the CORS list also breaks the same-origin/reverse-proxy topology (SPA + API on one domain: no CORS entries needed, but the browser still sends Origin on the POST and it's never in the list). I think the deeper fix is a dedicated FrontendOptions:AllowedOrigins (or similar) with its own startup validation, rather than overloading CORS policy — that also documents the list's second duty instead of leaving it implicit.

2. Every non-browser client is hard-broken, with no fallback (OriginResolver.cs:29, all four endpoints)

register/self-register/resend previously derived the link base from the request host; forgot-password used the configured OriginUrl. Now all four hard-require an allow-listed Origin header. That breaks: the Scalar try-it UI (same-origin fetch sends Origin: https://localhost:7030, the API's own origin, never in the list), curl/Postman, mobile apps, and server-to-server provisioning. A documented fallback (e.g. a configured default frontend origin used when no header is present) preserves those callers while keeping the forged-header rejection for mismatches.

3. Silent breaking change for existing production deployments (appsettings.Production.json)

Production ships AllowedOrigins: [] and this PR doesn't touch it. A deployment that satisfied the old contract by configuring OriginOptions:OriginUrl upgrades and finds forgot-password/register dead with 500s, with the previously-required setting now silently ignored for these flows. There's no startup validation or migration note. At minimum this needs the docs/changelog treatment (see below) plus ideally a startup check.

4. Client-caused rejection surfaces as HTTP 500 (OriginResolver.cs:30)

A missing/forged Origin is a request condition, not a server fault, but InvalidOperationException falls into GlobalExceptionHandler's 500 bucket — and ForgotPassword_Should_Reject_When_OriginNotAllowed pins InternalServerError as the contract. Per .agents/rules/api-conventions.md this should be a framework exception type (e.g. CustomException(msg, null, HttpStatusCode.BadRequest)) so callers get a 4xx ProblemDetails and error-rate alerting doesn't page on bot traffic to anonymous endpoints.

5. The link targets the caller's SPA, not the recipient's (RegisterUserEndpoint.cs:22, ResendConfirmationEmailEndpoint.cs:36)

/register and resend-confirmation are invoked from the admin app, so a tenant dashboard user provisioned (or re-sent) by an operator gets a confirmation link into the admin SPA at :5173 — confirmation succeeds, then "continue to sign in" lands them on the operator login where their credentials don't work; if the admin app is network-restricted, the link is dead entirely. The old API-route link was ugly (raw JSON) but client-agnostic. The origin needs to be derived from the target user's app, not the request's Origin — which probably reinforces the case for explicit frontend config over header sniffing for these two operator-driven flows.

Non-blocking

• Return the matched allow-list entry rather than the raw header (OriginResolver.cs:22): today attacker/client casing (HTTP://LOCALHOST:5173) is embedded verbatim into e-mails (the FrontendOrigin_Should_MatchIgnoringCase test asserts this). Returning the canonical entry costs nothing and removes client influence over the emailed URL.
• Matcher footguns (OriginResolver.cs:42): exact string compare means an entry like https://app.example.com:443 or an IDN form never matches what browsers actually send — a fail-closed config trap with no startup signal. Consider component-wise Uri comparison or normalizing the list once at startup.
• ApiOrigin() duplicates the existing IRequestContext.Origin contract; consider keeping that logic in one place so X-Forwarded-* handling etc. only ever needs one change.
• AGENTS.md golden rule 10: this changes config semantics and e-mail-link behavior, so the docs-repo update + changelog entry need to land with the change, not in a follow-up.
• Heads-up: #1324 (merged) replaced the stale :4200/:7140 entries in the same AllowedOrigins block, so this branch needs a rebase — which also resolves the leftover stale entries this PR kept (with the header-validation role, dead entries in that list are no longer harmless cruft).

Happy to re-review quickly — the core of this (allow-list-validated per-request origin + SPA confirm page) is something the kit genuinely needs.

[marcelo-maciel]

Thanks for the deep review — this reshaped the change for the better. Reworked around a dedicated, framework-level resolver; all five blocking points plus the non-blocking ones are addressed.

New shape: FSH.Framework.Web.Frontend — FrontendOptions (AllowedOrigins + DefaultOrigin) and IFrontendOriginResolver/FrontendOriginResolver. It lives in BuildingBlocks (alongside CorsOptions/OriginOptions) rather than Identity because building user-facing e-mail links is cross-cutting — Notifications, Billing, Tickets and the localized e-mail-template work will all need the same resolution.

Blocking

1. CorsOptions coupling / AllowAll trap — gone. A dedicated FrontendOptions:AllowedOrigins carries the link-trust duty explicitly, decoupled from CORS. Validated with .ValidateOnStart(): a deployment with neither AllowedOrigins nor DefaultOrigin now fails loud on boot instead of passing CORS and 500-ing on the first request. Same-origin/reverse-proxy topologies work via DefaultOrigin with an empty allow-list.
2. Non-browser clients hard-broken — ResolveForCurrentRequest() falls back to DefaultOrigin when the request carries no Origin header (curl, Scalar try-it, mobile, server-to-server), while still rejecting a present-but-forged origin.
3. Silent prod breaking change — now a loud startup failure (ValidateOnStart) plus the docs/changelog treatment; appsettings.Production.json ships FrontendOptions empty as an explicit deploy requirement.
4. Client-caused rejection was 500 — a forged/missing-from-list origin now throws CustomException(..., HttpStatusCode.BadRequest), so callers get a 4xx ProblemDetails and alerting doesn't page on bot traffic. The integration test now pins BadRequest.
5. Link targeted the caller's SPA — /register and resend-confirmation now call ResolveDefault() (the recipient's app via DefaultOrigin), not the operator's Origin. A tenant user provisioned from the admin app gets a link into the tenant dashboard. Self-service flows (forgot-password, self-register) still use the request origin.

Non-blocking

• Returns the canonical allow-list entry, never the client's raw casing (test: uppercased header resolves to the configured entry).
• Matcher now uses component-wise Uri.Compare(SchemeAndServer) (port exact, case-insensitive scheme/host), normalized once at construction — no :443/IDN footgun.
• ApiOrigin() deduped: folded into the existing IRequestContext.Origin contract. RequestContextService owns the config-first/request-host logic; UserProfileService reads IRequestContextService.Origin for avatar URLs. The separate ApiOrigin() method is gone.
• Rebased onto main (picks up chore(host): point default CORS allow-list at the React client origins #1324's allow-list; dropped the stale :4200/:7140 entries).

Docs-repo companion (#232) will be updated to the FrontendOptions model before merge.

Verified locally: full solution build 0 warnings / 0 errors (-warnaserror); Framework.Tests 121/121 (incl. new FrontendOriginResolverTests), Identity.Tests 312/312; origin-flow integration tests 15/15 against real Postgres (forgot / self-register / register / confirm-email happy paths + the forged-origin → 400 adversarial case).

[iammukeshm]

The per-request origin mechanism is now sound — allow-list validated, canonical entry returned, forged origin → 400, non-browser requests fall back to DefaultOrigin, and the match is exact (scheme + host + port), which also neutralizes the Uri.TryCreate leading-slash file:// footgun. Good. The blockers are release-safety and process, not the core design:

🔴 Blocker — ships AllowedOrigins: [] + DefaultOrigin: "" with ValidateOnStart(), so the API fails to boot in Production until configured. Fail-loud beats the old 500, but for existing deployers this is a silent breaking upgrade — the app just won't start. Please ship a safe default (or a clear, documented first-run config step) and a migration note so upgraders aren't stranded.

🔴 Blocker — the PR description says "Identity module, no changes to BuildingBlocks," but this adds src/BuildingBlocks/Web/Frontend/* and touches Web/Extensions.cs + Web.csproj. That's protected code (Golden Rule #4) and needs explicit sign-off — please correct the description so reviewers aren't approving on a false premise.

📝 Docs/changelog (Golden Rule #10) — this is a config-breaking, user-facing change; it needs a changelog entry + docs update traveling with it, not deferred to a follow-up.

⚠️ MEDIUM — DefaultOrigin is a single global; operator-driven register/resend will point every tenant's confirmation link at the one default SPA, so it isn't per-tenant/custom-domain aware. Acceptable for the kit's model, but worth documenting explicitly.

nit — the forgot-password path warns per rejected request with the attacker-controlled {Origin}; template-safe, but it'll log-flood under bot traffic on an anonymous endpoint.

[marcelo-maciel]

Thanks for the round-two review. All points addressed in 22eeaca4 (code) and fullstackhero/docs#232 87f76a3 (docs). Rundown:

🔴 Boot fails until configured (AllowedOrigins: [] + DefaultOrigin: "" + ValidateOnStart). Kept fail-loud, because there is no safe default value to ship (we cannot guess a deployer's SPA origin), and this is the same contract the kit already enforces for JwtOptions:SigningKey: also empty in appsettings.Production.json, also ValidateOnStart, so an unconfigured Production host already refuses to boot today. So this is a documented first-run config step rather than a new class of breakage. To make that explicit I:

• Sharpened the validation message to be first-run actionable ("set FrontendOptions:AllowedOrigins and/or DefaultOrigin before starting the host"), matching the JWT message style.
• Added a migration note to the PR description and to the docs (production checklist + CORS page): upgraders must populate FrontendOptions before the host starts.

If you would rather ship a permissive default instead of fail-loud, say the word and I will flip it, but the JWT precedent made fail-loud feel like the consistent call.

🔴 "No changes to BuildingBlocks" was wrong. Corrected the PR description: it now has a dedicated "Changes to src/BuildingBlocks" section listing the new Web/Frontend/* (the framework-level IFrontendOriginResolver + FrontendOptions), the Web/Extensions.cs registration, and the Web.csproj InternalsVisibleTo("Framework.Tests"), with the rationale (the resolver moved to framework level precisely because of your round-one point about not coupling e-mail-link trust to the CORS list, so any module that sends links resolves origins the same way). Requesting your Golden-Rule-#4 sign-off on that surface.

📝 Docs + changelog (Golden Rule #10). They travel with the change in fullstackhero/docs#232: changelog entry, the Identity module callout, a new "Front-end origin for auth e-mail links" section on the CORS page, and the production checklist item.

⚠️ MEDIUM: DefaultOrigin is a single global. Documented explicitly, both on the option's XML doc and in the CORS docs page: operator-driven register/resend point every tenant's link at that one SPA, which fits the kit's single-dashboard model; a per-tenant-custom-domain deployment would resolve the recipient tenant's own origin instead.

nit: rejection log flood. Dropped to Debug (behind an IsEnabled guard): these endpoints are anonymous, so bot/forged traffic would flood the aggregator at Warning, while a genuine deployer misconfig still surfaces as a 400 to that SPA's own users.

CI is green across the board (unit, integration, coverage gate, CodeQL, both scaffolds). Ready for another look whenever you have a moment.

[marcelo-maciel]

Two follow-ups since the round-two review, both green on Backend + Frontend CI now.

1. Self-caught gap — FrontendOptions:DefaultOrigin is now required at startup (9022b8f0).
The previous boot validation accepted AllowedOrigins-only (empty DefaultOrigin), but operator-driven register/resend, every non-browser caller (no Origin header) and background jobs all resolve through DefaultOrigin. So a host could pass ValidateOnStart and then 500 on the first admin-register or non-browser request — exactly the surprise-runtime-break the fail-loud validation was meant to prevent. DefaultOrigin is now required unconditionally; AllowedOrigins stays additive (it only widens which request origins may be echoed into self-service links). Same-origin / reverse-proxy topologies still work with DefaultOrigin alone.

2. Unblocked the currently-red main CI (516c4d08, NU1903).
System.Security.Cryptography.Xml 10.0.8 floats in transitively and trips fresh HIGH-severity advisories (GHSA-23rf-6693-g89p and four siblings) under NuGetAudit + TreatWarningsAsErrors, failing restore on every test project — this breaks main today, not just this PR. Pinned to the patched 10.0.10 in Directory.Packages.props, mirroring your Microsoft.OpenApi pin in #1319 and the MessagePack pin in #1299. Flagging the extra Directory.Packages.props change explicitly so it isn't silent scope.

The round-two blockers are all in: fail-loud boot, corrected BuildingBlocks disclosure, docs/changelog in #232, single-global DefaultOrigin documented, and Debug-level logging for rejected origins. Ready for re-review whenever you have a moment.

[marcelo-maciel]

@iammukeshm gentle nudge — I can't hit the "re-request review" button from a fork, so a comment is the only way to clear the stale Changes requested state.

All three blockers from the round-two review are closed on the branch, and the PR description was rewritten accordingly:

• boot safety — DefaultOrigin is now required unconditionally at ValidateOnStart (9022b8f0); AllowedOrigins is purely additive. The fail-loud contract plus a migration note is documented in the description, mirroring the existing JwtOptions:SigningKey behaviour.
• BuildingBlocks disclosure — the description now has a dedicated Golden Rule Serilog Integration #4 section naming the three touched files and explicitly retracting the earlier "no changes to BuildingBlocks" line.
• docs + changelog — docs(identity): per-request front-end origin for auth e-mail links docs#232 (changelog entry, identity module, cors-and-headers, production-checklist).

Also unblocked the NU1903 restore failure by cherry-picking the System.Security.Cryptography.Xml 10.0.10 pin (516c4d08) — that advisory currently fails restore repo-wide on main, not just here.

CI is fully green: Backend CI, Frontend CI, Unit, Integration, Coverage Gate, DbMigrator Smoke and both Scaffold jobs all pass. Ready for another look whenever you have a slot.

[iammukeshm]

The resolver itself is now genuinely good, and I want to be clear that the design argument has been settled in your favour. Splitting FrontendOptions away from CorsOptions was the right call, the two-method split (ResolveForCurrentRequest for self-service vs ResolveDefault for operator-driven) correctly models who receives the link rather than who sent the request, returning the canonical configured entry removes client influence over the emailed URL, and component-wise Uri comparison kills the string-compare footgun. My round-2 nits on log level and documenting the single-global limitation are both addressed.

Three things still stand between this and merge. Only the first is a judgement call.

🔴 Blocker — Production still ships DefaultOrigin: "" under ValidateOnStart()

This is my previous blocker #1, unchanged: appsettings.Production.json carries "DefaultOrigin": "", and validation rejects null-or-whitespace on start. Every existing production deployment stops booting on upgrade.

I've read the counter-argument in the description and I don't accept it. JwtOptions:SigningKey is not the precedent it looks like: it was always required, so nobody has ever had a running deployment without it — the fail-loud only ever fires on a first run that was never going to work. This setting is new. It converts deployments that are working right now into deployments that will not start, and the operator's first signal is a container that won't come up.

"Fail loud beats a 500" is the right principle applied at the wrong boundary. Loud at first use of the feature is correct; loud at process start, for a feature the deployment may not even use is not. A deployment that never calls forgot-password or self-registration is taken down by a setting it has no need for.

Resolve it one of these two ways — your choice, both are fine by me:

1. Preferred — when DefaultOrigin is unset, fall back to the API's own origin (OriginOptions:OriginUrl, or the request host) and log a single startup Warning naming the setting and what degrades without it. Links land somewhere serviceable, the operator is told, and nothing goes dark. This restores parity with the pre-PR behaviour for register/self-register/resend, which derived from the request host.
2. Acceptable — keep ValidateOnStart, but the validation message must name the setting, the file, and an example value, and the changelog must lead with a "required config, will not start without it" upgrade note. Fail-loud is defensible if and only if the operator is handed the fix in the failure itself.

What isn't acceptable is shipping it as-is, where an upgrader gets a startup exception for a setting they've never heard of.

🔴 Blocker — merge conflict

mergeable: CONFLICTING. Needs a rebase onto main before I can do anything with it.

🔴 Blocker — drop the System.Security.Cryptography.Xml pin

Your reasoning for including it was correct at the time, but it's now stale: that pin landed on main independently and currently sits at 10.0.10 with a superset of the advisories you cite (five, not four). Re-adding it produces a duplicate PackageVersion item. Drop the Directory.Packages.props change entirely during the rebase — the rest of the PR doesn't depend on it, and #1351 is no longer blocked on this branch either.

Thanks for flagging the scope explicitly rather than slipping it in; that's exactly the right instinct, and it's only being removed because the problem it solved is gone.

Confirmed resolved from earlier rounds

• Description now states the BuildingBlocks changes plainly and retracts the earlier "no changes to BuildingBlocks" line. Appreciated — that's the kind of correction that keeps reviews honest.
• Forged origin → 400 rather than 500, per api-conventions.md.
• Non-browser callers fall back to the default instead of hard-failing.
• Rejection logs at Debug, so anonymous-endpoint bot traffic won't flood the aggregator.
• Single-global limitation documented on the option itself, which is where someone hitting it will actually look.

BuildingBlocks sign-off (Golden Rule #4)

Granted for Web/Frontend/*, the Web/Extensions.cs wiring and the InternalsVisibleTo. Framework-level is the right home now that the resolver is meant to serve Notifications, Billing and Tickets as well as Identity.

Docs (Golden Rule #10)

This changes config semantics and email-link behaviour, so the docs-repo update and changelog entry need to land alongside it — and per the blocker above, the changelog is now carrying the upgrade note, so it's load-bearing rather than ceremonial.

Re-ping me once the boot behaviour is settled and it's rebased; the rest of this is ready.

[marcelo-maciel]

All three blockers are closed. Taking your option 1 — you're right that the boundary was wrong, and I'm not re-arguing the JWT precedent.

Boot behaviour — fallback, not fail-loud (67e026f0, 7e0198c5)

ValidateOnStart is gone. An upgrader who never sets FrontendOptions keeps booting; UseHeroPlatform logs one startup Warning naming the setting, the config file and what degrades, and ResolveDefault() walks a chain:

1. FrontendOptions:DefaultOrigin
2. OriginOptions:OriginUrl
3. the current request's host
4. otherwise throw

Tier 3 exists because appsettings.Production.json ships OriginUrl empty too — I had the chain stop at tier 2 first, which would have left the exact deployment your blocker is about (touched nothing, upgraded) 500-ing on the first admin register instead of failing loud at boot. That is the pre-PR failure mode wearing a different hat, so the request host is in. It's also the parity you named: register / self-register / resend derived their links from the request host before this PR.

Tier 3 is the API's own request host, never the caller's Origin header. That distinction is the reason ResolveDefault() exists apart from ResolveForCurrentRequest(), so your round-1 blocker #5 stays closed: an operator-driven confirmation link cannot point back at the admin SPA the request came from. Forged-origin rejection is untouched — a present-but-unlisted Origin is still a 400, never quietly swapped for a fallback.

The warning fires from UseHeroPlatform, after AddHeroLogging(), so it lands in the configured Serilog sink rather than a bootstrap logger. One line at boot, not per request: the resolver is scoped, so logging there would either flood the aggregator or stay silent on a host that never sends a link.

New unit coverage for every tier: DefaultOrigin unset (and the empty string production ships) falls back to the API origin; a configured DefaultOrigin wins over it; a non-absolute OriginUrl ("" binds relative — AbsoluteUri would throw) is skipped in favour of the request host; nothing configured and no request (a background job) throws; and a forged header is still 400 even when a fallback is available.

Merge conflict

Rebased onto main. mergeable: MERGEABLE.

System.Security.Cryptography.Xml pin

Dropped — the commit is gone from the branch, and git diff main...HEAD -- src/Directory.Packages.props is empty. Confirmed your pin on main is at 10.0.10 with the five advisories.

Docs (#232)

Rebased and rewritten to match, since the upgrade note changed shape: it's no longer "required config, will not start without it" but "configure it or your links point at the API." Changelog entry, identity, cors-and-headers and production-checklist all carry the fallback chain and the startup warning.

PR description

Updated: the fallback chain replaces the fail-loud section, and the Directory.Packages.props scope paragraph is gone.

CI is green on 7e0198c5 — Backend CI, Frontend CI, Unit, Integration, Coverage Gate, CodeQL, DbMigrator Smoke and both scaffolds.

[marcelo-maciel]

Follow-up to my last comment — a fresh adversarial pass over the branch found a defect of exactly the class you've been catching, so I'd rather flag it myself than have you find it. Fixed in a48e9574.

The shipped Production config 400'd every password reset

appsettings.Production.json ships FrontendOptions:AllowedOrigins: []. Browsers attach an Origin header to the forgot-password and self-register POSTs — even same-origin — so ResolveForCurrentRequest() took the allow-list branch, matched a present header against an empty list, found nothing, and threw the 400. Every legitimate reset and self-registration, on the config the kit ships.

It also made the claim in my own description false: I wrote "AllowedOrigins is purely additive … a same-origin / reverse-proxy topology works with DefaultOrigin alone." It didn't — that topology sends an Origin too.

Fix: with an empty allow-list there is nothing to validate against, so the header is discarded and the link resolves through the server-side default. Not a relaxation — the client's value is never echoed on that path, and a forged origin against a configured list is still a 400. Same shape as your blocker #6: the fallback wasn't safe for the deployment it was meant to protect.

The startup warning missed it too. WarnOnMissingFrontendOrigin returned early as soon as DefaultOrigin was set and never looked at AllowedOrigins, so a deployer who did exactly what the warning asked still had both anonymous flows dead with no diagnostic. The empty list is now reported independently, naming what degrades: every self-service link points at DefaultOrigin, which is wrong the moment there is more than one front-end.

Two smaller things in the same commit

• IDN matching was fail-closed. Uri.Compare(…SchemeAndServer…) compares Host, not IdnHost, so a list entry written in Unicode (https://bücher.example) never matched the punycode form the browser sends (https://xn--bcher-kva.example) — a deployer listing an IDN origin would 400 their own users, which is the matcher footgun you flagged in round 1 wearing a different hat. Matching is now explicit on scheme + IdnHost + Port; :443 and the bare host still compare equal.
• A test codified an accident. Handle_Should_Propagate_When_OriginResolverThrows stubbed InvalidOperationException while the resolver actually throws CustomException, so a handler that caught CustomException specifically — swallowing a rejected origin into a sent e-mail — would still have passed. It now stubs the real type and asserts the 400.

New unit coverage: empty allow-list with a header present resolves to the default; a header carrying userinfo (http://evil.com@localhost:5173) returns the configured entry, not anything the client sent; a Unicode list entry matches a punycode header; :443 matches the bare host.

Known gap, stated rather than papered over

Nothing pins that register / resend-confirmation call ResolveDefault() rather than ResolveForCurrentRequest() — the regression test for your round-1 blocker #5. I tried an integration test asserting the emitted link, but the confirmation mail goes out through the Hangfire email queue and the harness doesn't drain it deterministically, so the test was flaky-by-construction and I dropped it rather than ship a test that passes for the wrong reason. Happy to add it if you'd like the harness to gain a synchronous job runner, but that's a change to shared test infrastructure and felt out of scope for this PR.

Full suite green locally (Framework 154/154, Identity 312/312, Architecture 55/55, Integration 747 passed / 1 skipped against real Postgres) and CI is green on a48e9574. Docs #232 updated in the same flow.

[marcelo-maciel]

One more from the same pass, this time on the warning I'd just added — 93745107.

The empty-allow-list warning read FrontendOptions:AllowedOrigins.Length off the raw config array, but the resolver matches against the normalized list, and normalization silently drops any entry that isn't an absolute URL. So a list of nothing but typos ("https;//app.example.com") looked configured to the warning and empty to the resolver: every self-service link quietly used DefaultOrigin, and the diagnostic I'd just added stayed silent on exactly the case it existed for.

The warning now counts the normalized list, and reports separately when only some entries were dropped — those origins get a 400 rather than being silently ignored, which is a different problem deserving a different line. Two tests pin it: an all-unparseable list falls back to the default, and a partly-malformed list stays live (an unlisted origin is still a 400, so a typo can't widen the list into the fallback).

Also tightened one line in the description that hadn't caught up: forged-origin rejection is "a present Origin that misses a configured allow-list", not "a present-but-unlisted Origin" — with an empty list there is no list to be unlisted from.

CI green on 93745107; Framework.Tests now 156/156.

[marcelo-maciel]

A third adversarial pass came back essentially clean — the Identity endpoints, the IRequestContext.Origin dedup, the confirm-email round trip against both SPAs, and the docs all traced sound. Two things worth reporting anyway.

Your round-1 Scalar observation was right and my prose had quietly contradicted it (dc767b52)

You wrote that the Scalar try-it UI sends Origin: https://localhost:7030 — the API's own origin, never in the list. Somewhere across the rewrites I started listing Scalar alongside curl and server-to-server as callers that send no Origin and therefore fall back to DefaultOrigin. That reading made the no-header fallback look like it covered your blocker #2 in full, which it doesn't.

Checked it rather than guessing: Scalar.AspNetCore 2.14.14 exposes ProxyUrl but binds it null by default, and no proxy host is baked into the assembly (grep over lib/net10.0/Scalar.AspNetCore.dll finds only the proxyUrl property name). UseHeroOpenApi doesn't set one either. So the try-it panel fetches straight from the browser and does send an Origin.

Corrected in the resolver comment, the FrontendOptions XML doc and two docs pages: curl / mobile / server-to-server fall back; Scalar hits the allow-list branch and needs the API's own origin listed to exercise forgot-password or self-register. I did not add that origin to the dev AllowedOrigins — embedding the API's own origin in a list whose job is to name front-ends felt like the wrong default to ship, but say the word and I'll add it so the docs UI works out of the box.

The per-endpoint regression test: attempted again, and it poisons the harness

I said last time the confirmation-mail assertion wasn't deterministic because of the Hangfire email queue. The suggestion came back to sidestep the mail entirely and just assert which resolver method each endpoint calls, with a substituted IFrontendOriginResolver. That's the right idea and it does work for two endpoints — but it needs WithWebHostBuilder, and disposing the derived factory tears down the shared Hangfire.InMemory dispatcher:

BackgroundJobClientException: Background job creation failed.
--> ObjectDisposedException: Cannot access a disposed object.
Object name: 'Hangfire.InMemory.State.Dispatcher`1[System.UInt64]'.

Every subsequent register in the collection then 500s. Two of the four tests I wrote passed and the other two failed because the first two had already run — a test that breaks its neighbours is worse than the gap it closes, so I removed it rather than ship it.

So the gap stands as stated: nothing pins that register / resend-confirmation call ResolveDefault() while forgot-password / self-register call ResolveForCurrentRequest(). Both routes to closing it run into the same shared-state harness limitation. If you'd like it covered, the enabling change is a per-test host that doesn't share Hangfire storage (or a synchronous job runner in the harness) — happy to do it as its own PR, but it's test infrastructure the whole suite sits on and I'd rather not fold it into this one.

Everything else that pass looked at came back clean: no partial state on a resolver 400 (it runs before mediator.Send and before any user creation), the idempotency filter propagates the CustomException untouched, avatar-URL behaviour is unchanged by the Origin dedup, and both clients/admin and clients/dashboard read the exact userId / code / tenant and token / email / tenant keys the backend emits.

CI green on dc767b52. Local: Framework 156/156, Integration 747 passed / 1 skipped.

[marcelo-maciel]

@iammukeshm this is ready for another look whenever you have a slot — I can't use the re-request-review button from a fork, so a comment is the only way to clear the stale Changes requested.

All three round-3 blockers are closed: boot behaviour took your preferred option 1 (fallback + one startup Warning, no ValidateOnStart), the branch is rebased (MERGEABLE), and the System.Security.Cryptography.Xml pin is gone. Three further defects I found on my own passes since then are fixed and written up in the comments above.

One addition since the last comment: .agents/rules/security.md now documents the resolver (124f182e). That rule file is what an agent reads before touching CORS, headers or rate limiting, and it had no entry for FrontendOptions — so the next person adding an e-mail link had nothing telling them which of the two resolver methods matches which recipient. Picking the wrong one compiles and returns a plausible origin, which is exactly how your round-1 blocker #5 happened in the first place.

CI green on 124f182e; docs #232 is rebased, CLEAN and matches the code as it now stands.