Skip to content

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

Open
marcelo-maciel wants to merge 26 commits into
fullstackhero:mainfrom
marcelo-maciel:fix/identity-origin-multifront
Open

marcelo-maciel wants to merge 26 commits into
fullstackhero:mainfrom
marcelo-maciel:fix/identity-origin-multifront

Conversation

@marcelo-maciel

@marcelo-maciel marcelo-maciel commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Reopened from #1323. That PR was closed automatically on 2026-09-14, when the head fork
was deleted. It reopened at 124f182e, with the earlier review history left on #1323; the
head is now 024cb07c. What landed since: 0be4ede0 wires FrontendOptions into the
shipped docker and Terraform deploys (the Codex P1 below), and e36d4a00, 7231beec,
95b12f63, 7bd6059f, 362e500a and 024cb07c answer review. Two more are repo-wide
breakages unrelated to this change, carried only so CI can reach this code: 78bc5803
bumps Testcontainers to 4.14.0 and SourceLink past their advisories, and 84aae7a0 pulls
MinIO from quay.io, on a pinned tag, after it left Docker Hub. Both also stand alone as
#1388, since main is broken on them too. The Testcontainers bump is the same fix as
#1369, on purpose, so the two do not conflict.


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 startup diagnostics: a Warning when AllowedOrigins is empty, an Error when DefaultOrigin is unset.
  • modified src/BuildingBlocks/Web/Web.csprojInternalsVisibleTo("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, but the two shipped deployment paths populate them from the SPA URLs they already know: deploy/docker/docker-compose.yml from FSH_ADMIN_URL / FSH_DASHBOARD_URL, and the AWS Terraform stack from the resolved admin_url / dashboard_url. api_extra_cors_origins is deliberately not folded in (app_stack/main.tf:344-348): that variable grants permission to call the API, which is not the same as permission to appear inside an e-mail link. An earlier version of this paragraph claimed the opposite; the code was always right and the sentence was wrong. The API domain is deliberately not carried over from the CORS list: allow-listing the API origin is what puts the link back on the API.

An existing deployment keeps booting after the upgrade, but DefaultOrigin is required in practice. 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 and logs a startup Error naming the setting, the config file, and the four flows that will answer 500 until it is set (confirm-email, resend-confirmation, forgot-password, reset-password). ResolveDefault() has no fallback tier at all: it returns FrontendOptions:DefaultOrigin or throws.

An earlier revision of this PR walked a chain — OriginOptions:OriginUrl, then the current request's host. Both tiers are gone, and review is what removed them:

  • The request host is attacker-controlled. Host is whatever the caller puts in the header, so a password-reset e-mail built from it delivers a working token to a domain the attacker chose. A credential-bearing link must never be derived from request input.
  • The API's own origin stopped being serviceable the moment this PR pointed these links at the SPA pages (/confirm-email, /reset-password). The API answers those paths with 404, so the tier reliably produces a dead link rather than a degraded one.

Failing loudly, with the operator told at startup and the exception naming the setting to configure, is the only remaining outcome that is neither unsafe nor broken. Forged-origin rejection is untouched: a present Origin that misses a configured allow-list is still a 400.

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 and is not consulted when building links at all.

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. The resolver logs rejections 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. That is only the resolver's own line, and review was right to call the claim out: the 400 it throws unwinds into GlobalExceptionHandler, which logs every handled exception at Error with a stack trace, so a forged-origin bot still produces one Error line per request. That handler is pre-existing and shared by every endpoint in the app, so re-levelling it by status code is not this PR's call to make — flagged rather than changed.

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: DefaultOrigin unset (including the empty string appsettings.Production.json ships) throws rather than deriving an origin from anywhere else, with or without an HTTP request in scope; the thrown message names the setting and does not leak the attacker-supplied host (asserted); and a forged header is still 400.
  • ForgotPasswordCommandHandlerTests updated to the resolver.
  • IntegrationForgotPassword_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, rewritten in fullstackhero/docs@049ecf0b to the fail-loud behaviour (Identity module page, CORS and headers page, production checklist, changelog). Site build green.

Review follow-up: the shipped Production config was not empty

An independent review found the premise under the paragraph above to be false, and it was.

appsettings.Production.json shipped "AllowedOrigins": [] on the assumption that an empty array
clears the base file. It does not. A JSON array is flattened into indexed keys, an empty one writes
no indices at all, and the binder concatenates what the earlier provider left. A production
deployment therefore started with http://localhost:5173 and :5174 in the allow-list — so the
"empty list falls through to DefaultOrigin" branch was never reached, a real SPA origin got a 400,
and the startup warning about an empty list never fired because the list was not empty.

Fixed by moving the dev origins into appsettings.Development.json, which Production never loads.
ShippedConfigurationTests now loads the shipped files the way the host does and asserts what each
environment actually gets; putting one origin back in appsettings.json turns it red.

CorsOptions:AllowedOrigins had the identical defect and is fixed in the same commit. That half is
pre-existing — main ships the same shape — but leaving it would mean production still trusted
localhost for CORS while this PR claimed it no longer did for e-mail links.


Infra carve-outs, corrected after review. Two things in the out-of-topic hunks were wrong and
are fixed on the branch:

  • The MinIO carve-out only moved minio/minio to quay.io. minio/mc is gone from Docker Hub too
    (hub.docker.com/v2/repositories/minio/mc/ answers 404) and it is what minio-init runs, so both
    dotnet run --project src/Host/FSH.Starter.AppHost and docker compose up died on the pull and the
    fsh bucket was never created. Now pinned to the same quay tag #1388 uses.
  • The SSH.NET pin is gone: it pinned nothing. Its own comment claimed bumping Testcontainers
    does not help, but 4.14.0 — which this branch also carries — declares SSH.NET >= 2026.0.0.
    Measured rather than argued: with the pin removed, dotnet restore src/FSH.Starter.slnx --force
    reports zero NU1902/NU1903 and exits 0. (The MessagePack pin next to it stays; removing that one
    does bring its advisory straight back.)

With both applied, deploy/docker/docker-compose.yml and src/Directory.Packages.props are now
genuinely byte-identical to #1388 (git diff --exit-code, checked today), which the earlier claim
was not.


Second review round. DefaultOrigin was only checked for emptiness, so
app.example.com (no scheme, the usual .env slip) bound cleanly, drew no startup
diagnostic, and made every e-mail link a relative URL no mail client turns into a link. It
is validated as an absolute URI now, and the resolver drops an unusable value rather than
handing it back, so the run time agrees with what startup logs: an unusable value is an unset
value, failing the same way. A base path survives validation (https://example.com/app must
not collapse to its authority, or /reset-password 404s) and both branches are covered by
tests that go red when the guard is reverted.

The rejection log wrote the caller-controlled Origin header verbatim; it is stripped of line
breaks and truncated to 200 characters now, the treatment the global exception handler already
gives the request path. And three comments still described the fallback chain this PR removes,
one of them inside src/BuildingBlocks, where the next maintainer would have read it as "safe
degradation exists" and put the tier back. They match the code now.

…inks

Password-reset and e-mail-confirmation links were built from a single
configured OriginUrl (which pointed at the API and was empty in
Production, throwing "Origin URL is not configured") or from the raw
request host (the API), so neither could target the correct SPA when
more than one front-end is served (admin :5173, dashboard :5174).

Introduce IOriginResolver:
- FrontendOrigin(): takes the request Origin header and validates it
  against CorsOptions.AllowedOrigins, so the reset/confirmation link
  lands on the SPA the request came from. The allow-list check is the
  security boundary: a forged Origin on the anonymous forgot-password
  flow can never be injected into an e-mail. Throws when no allow-listed
  origin is present.
- ApiOrigin(): configured origin, else request host (unchanged
  behaviour) for API-served assets (avatars) and RequestContextService.

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

- forgot-password, register, self-register and resend-confirmation now
  resolve the front-end origin via the resolver.
- avatar URL building and RequestContextService delegate to ApiOrigin().
- appsettings: add the dev SPA origins to CorsOptions.AllowedOrigins.
  Production deployments must list their SPA URLs there.
- tests: OriginResolverTests (allow-list, case/slash/port, forged origin,
  missing header), updated ForgotPassword handler + RequestContext tests,
  and the integration harness now sends an Origin header like a browser.
…-end

Drives the failure path through the real HTTP pipeline: a forgot-password
request carrying an Origin header outside CorsOptions.AllowedOrigins is
rejected (500) instead of returning the uniform OK, proving a spoofed
origin can never be turned into a reset link.
Adds EmailLinkOriginTests: drives forgot-password and register through
the real pipeline and inspects the captured MailRequest body, asserting
the reset link points at the SPA origin from the request's Origin header
(:5174 vs :5173, proving per-front resolution) and that the confirmation
link targets the SPA /confirm-email page rather than the API route.

Adds the two dev SPA origins to the integration harness allow-list so
per-front resolution can be exercised.

Not yet executed locally: Windows Smart App Control blocks the freshly
rebuilt unsigned test DLLs (0x800711C7); runs in CI (Linux).
…meout

The register flow also emits a welcome e-mail (via the UserRegistered
integration event), so matching only by recipient grabbed the wrong
message. Match the confirmation e-mail by its subject, and likewise the
reset e-mail, and include the captured messages in the timeout error to
diagnose misses.
The integration harness does not execute enqueued Hangfire mail jobs
(mail-asserting tests such as TenantExpiryScanJobTests invoke the job
synchronously), so the confirmation/reset e-mails never reach the
capturing mail service and EmailLinkOriginTests could not observe them.

The link content is already covered where it is built: UserPasswordServiceTests
asserts the reset link (origin + tenant + encoding) by capturing the enqueued
MailRequest, OriginResolverTests covers origin resolution, and an integration
test asserts a forged Origin is rejected. Reverts the harness allow-list
entries that only that test needed.
The explanatory comment above the confirm-email URI build read like
commented-out code to SonarAnalyzer (S125) because of its parentheses
and trailing semicolon, failing the -warnaserror backend build. Reword
it as plain prose; behaviour is unchanged.
Address review on fullstackhero#1323. Replace the CorsOptions-coupled, throw-on-miss
OriginResolver with a framework-level front-end origin resolver, so any module
that builds user-facing links (Identity today; Notifications/Billing/Tickets
next) resolves them the same way.

- New FSH.Framework.Web.Frontend: FrontendOptions (AllowedOrigins + DefaultOrigin)
  + IFrontendOriginResolver/FrontendOriginResolver. Validated at startup
  (ValidateOnStart) so a deployment missing both fails loud on boot instead of
  500-ing on the first password-reset — resolves the silent CorsOptions.AllowAll
  and empty-Production-list traps.
- ResolveForCurrentRequest() (self-service: forgot-password, self-register):
  validates the Origin header against the allow-list, returns the canonical
  entry (not the client's casing), falls back to DefaultOrigin when no header is
  present (curl / Scalar / mobile / server-to-server), and throws a 400-mapped
  CustomException on a present-but-forged origin (was InvalidOperationException
  -> 500). Matching is component-wise via Uri (port exact).
- ResolveDefault() (operator-driven: register, resend-confirmation): targets the
  recipient's app via DefaultOrigin instead of the operator's Origin, so a
  tenant user provisioned from the admin app no longer gets a link into :5173.
  Also serves background jobs that have no HttpContext.
- Dedup: ApiOrigin() folded into IRequestContext.Origin (its existing contract);
  RequestContextService owns the config-first/request-host logic and
  UserProfileService reads IRequestContextService.Origin for avatar URLs.
- appsettings: FrontendOptions (dev 5173/5174 + default 5174; Production empty =
  deploy requirement). Rebased onto main (fullstackhero#1324 CORS allow-list).
…boot message)

- Log rejected origins at Debug, not Warning: the auth endpoints are
  anonymous, so bot/forged traffic would flood the aggregator; a genuine
  deployer misconfig still surfaces as a 400 to the affected SPA's users.
- Document that FrontendOptions:DefaultOrigin is a single global (not
  per-tenant/custom-domain aware) so operator-driven links land on one SPA.
- Make the FrontendOptions startup-validation message first-run actionable,
  matching the JwtOptions "set it before starting the host" precedent.
The boot validation accepted AllowedOrigins-only (DefaultOrigin empty), yet
operator-driven register/resend, every non-browser caller (no Origin header)
and background jobs resolve through DefaultOrigin. Such a host booted clean
then 500'd on the first admin register or non-browser request - the same
surprise-runtime-break the fail-loud validation was meant to prevent.

Require DefaultOrigin unconditionally; AllowedOrigins stays additive (widening
which request origins may be echoed into self-service links). Same-origin /
reverse-proxy topologies still work with DefaultOrigin alone. Fold the
redundant second AddHttpContextAccessor() call into the platform's existing one.
DefaultOrigin was validated with ValidateOnStart, so an existing deployment
that upgraded without configuring it stopped booting — a setting it may never
exercise took the whole host down, and the operator's first signal was a
container that would not come up.

Fail loud at first use of the feature, not at process start:

- drop the startup validation; the host boots with DefaultOrigin unset
- ResolveDefault falls back to the API's own origin (OriginOptions:OriginUrl)
  so links land somewhere serviceable instead of going dark
- UseHeroPlatform logs one startup Warning naming the setting, the file and
  what degrades without it

The fallback is deliberately the configured API origin and never the current
request's host: ResolveDefault exists because the caller is not the recipient,
so an operator-driven confirmation link must not point at the admin app.
Forged-origin rejection is unchanged — a present-but-unlisted Origin is still
a 400, never swapped for the fallback.
…at all

appsettings.Production.json ships OriginOptions:OriginUrl empty as well, so a
deployment that upgraded without touching either setting still had no origin to
build a link from and 500'd on the first operator-driven register/resend - the
exact failure the boot-safety fallback was meant to remove.

ResolveDefault now walks DefaultOrigin, then the configured API origin, then the
current request's host, and only throws when there is no request either (a
background job). The request host is the API's own, never the caller's Origin
header, so an operator-driven link still cannot point at the admin SPA.
…figured

appsettings.Production.json ships FrontendOptions:AllowedOrigins empty, and
browsers attach an Origin header to the forgot-password and self-register POSTs
even same-origin. Matching a present header against an empty list returned no
canonical entry, so every legitimate password reset and self-registration came
back 400 on the shipped Production config - and on any single-SPA or
reverse-proxy deployment.

With no allow-list there is nothing to validate against, so the header is
discarded and the link resolves through the server-side default. The client's
value is never echoed, so a forged origin against a configured list is still
rejected with 400.

The startup Warning now reports an empty AllowedOrigins independently of a
missing DefaultOrigin: a deployment can configure one and not the other, and
setting only the default silently sends every user to the same front-end.

Also matches origins through IdnHost, so a list entry written in Unicode
matches the punycode form browsers actually send instead of failing closed, and
pins the handler contract on CustomException rather than the arbitrary
exception type the old test stubbed.
…ning

Unparseable entries are dropped when the resolver normalizes the list, so a
list of nothing but typos matched the empty-list fallback at runtime while the
warning, reading the raw config array, saw a configured list and stayed quiet.
The operator got neither their allow-list nor a diagnostic.

The warning now counts the normalized list, and reports separately when only
some entries were dropped - those origins are rejected with 400 rather than
silently ignored.
Scalar.AspNetCore 2.14.14 ships no default proxy URL (the option exists but
binds null, and no proxy host is baked into the assembly), so the try-it panel
fetches straight from the browser and sends the API's own origin. Listing it
alongside curl and server-to-server callers was wrong: those genuinely send no
Origin and fall back to the default, while Scalar hits the allow-list branch
and needs the API origin listed to exercise forgot-password or self-register.
The rule file agents read before touching CORS, headers or rate limiting had no
entry for FrontendOptions, so the next person to add an e-mail link had nothing
telling them which resolver method matches which recipient - a choice where both
options compile and both return a plausible origin.
The index line is how an agent decides whether to open security.md at all.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 124f182e8a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Host/FSH.Starter.Api/appsettings.Production.json

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d0ed861dcb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread deploy/terraform/apps/starter/app_stack/main.tf Outdated
@chatgpt-codex-connector

Copy link
Copy Markdown

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

1 similar comment
@chatgpt-codex-connector

Copy link
Copy Markdown

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

@marcelo-maciel
marcelo-maciel force-pushed the fix/identity-origin-multifront branch from d974f26 to c9a7f0f Compare September 14, 2026 16:29
@chatgpt-codex-connector

Copy link
Copy Markdown

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

Both shipped deployment paths left `FrontendOptions` empty, so the resolver fell
through to the API origin and every password-reset / e-mail-confirmation link
pointed at `https://api.../reset-password` and `https://api.../confirm-email` --
SPA routes that do not exist on the API. Each path already knows the SPA URLs, so
the fix is to pass them through:

- `docker-compose.yml` -- `FrontendOptions__AllowedOrigins__0/1` from the existing
  `FSH_ADMIN_URL` / `FSH_DASHBOARD_URL`, with the dashboard as `DefaultOrigin` so
  an operator-driven register / resend lands on the tenant app, not on admin.
- Terraform `app_stack` -- a `frontend_environment_variables` map mirroring the
  CORS one, built from the resolved `admin_url` / `dashboard_url` plus
  `api_extra_cors_origins` (extra SPA origins the deployer already trusts, which
  would otherwise start getting a 400 on forgot-password once the list is
  non-empty). The API domain is deliberately *not* carried over from the CORS
  list: allow-listing it reintroduces the same wrong-destination link.
  `DefaultOrigin` is the dashboard, falling back to admin, and stays empty when
  the stack hosts neither -- the pre-existing `OriginOptions__OriginUrl` behaviour.

The Docker README gains the link-building meaning of those two `.env` URLs and a
troubleshooting row for a link that lands on the API.

Verified: `docker compose config` renders the three new keys; `terraform fmt
-check -recursive` and `terraform validate` pass; the `DefaultOrigin` expression
checked in `terraform console` for all three branches (dashboard, admin-only,
neither).
…advisories

`dotnet restore` fails for the whole solution under `TreatWarningsAsErrors`, on
`main` and on every open PR alike. Advisory-database drift, not a regression from
any change: a commit green on 2026-08-10 is red today with no edits.

- `Testcontainers.PostgreSql` / `.Redis` / `.Minio` 4.11.0 -> 4.14.0 (NU1903,
  GHSA-q939-rpr3-3284). 4.11.0 depends on `SSH.NET` 2025.1.0; 4.14.0 already
  depends on the patched 2026.0.0, so the advisory clears with no transitive pin
  to remember to remove later. Same fix as fullstackhero#1369, so the two do not conflict.
- `Microsoft.SourceLink.GitHub` 8.0.0 -> 10.0.401 (NU1902,
  GHSA-23fw-v26w-5fgq). 8.0.0 drags in `Microsoft.Build.Tasks.Git` 8.0.0 and the
  8.x line has no patched release, so a transitive pin cannot fix it; the package
  itself has to move. 10.0.401 depends on `Microsoft.Build.Tasks.Git` 10.0.401,
  past the patched 10.0.303. Build-time only (`PrivateAssets="all"`), referenced
  only where `IsPackable == true`, which is the CLI alone - and `src/Tools/**` is
  excluded from the template, so the scaffold never sees it.

Verified: `dotnet restore src/FSH.Starter.slnx` exits 0 with no NU19xx, and
`dotnet build src/FSH.Starter.slnx -c Release -warnaserror` reports 0 warnings
and 0 errors.
MinIO withdrew `minio/minio` from Docker Hub. Docker Hub's API now answers
`object not found` for the repository, and a pull fails with:

    pull access denied for minio/minio, repository does not exist or may
    require 'docker login'

That takes down every Testcontainers-backed integration test (the harness boots
a MinIO container per fixture, so all 724 tests in `Integration.Tests` fail at
container start), the Aspire AppHost, and the Docker Compose deployment. The
image is still published at `quay.io/minio/minio`:

- `Integration.Tests` and `Integration.Middleware.Tests` harnesses
- `AppHost.cs`, via Aspire's `WithImageRegistry` / `WithImageTag`
- `deploy/docker/docker-compose.yml` and the image table in its README

The tag is pinned to `RELEASE.2025-09-07T16-13-09Z` rather than `:latest`. quay
has not moved `:latest` since 2025-09-07, so the two resolve to the same digest
today; pinning only removes the surprise of a silent move later, and keeps the
test harness off a floating tag. Whether to track a newer release, or a different
S3-compatible image, is a separate call.

While in the README's image table: `postgres` and `redis` rows had drifted from
what compose actually ships (`postgres:18-alpine`, `valkey/valkey:9.1.0-alpine`).

Verified: `docker pull minio/minio:latest` fails with the error above;
`docker pull quay.io/minio/minio:RELEASE.2025-09-07T16-13-09Z` succeeds
(`sha256:14cea493d9a34af32f524e538b8346cf79f3321eff8e708c1e2960462bd8936e`, the
same digest `:latest` resolves to). `dotnet test Integration.Tests -c Release`
passes against the pinned image, and the Aspire manifest renders the container
as `quay.io/minio/minio:RELEASE.2025-09-07T16-13-09Z`.
@marcelo-maciel
marcelo-maciel force-pushed the fix/identity-origin-multifront branch from c9a7f0f to 84aae7a Compare September 14, 2026 17:46
@chatgpt-codex-connector

Copy link
Copy Markdown

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

…list

api_extra_cors_origins grants an origin permission to CALL the API, which is
what its description promises. Copying it into FrontendOptions:AllowedOrigins
also let those origins receive a password-reset or e-mail-confirmation URL with
the token in it, turning a CORS grant into a credential-link grant.

The two lists stay separate: CORS still includes the extra origins, e-mail links
only the SPAs this stack hosts, which arrive via admin_url/dashboard_url.

An origin listed only for CORS now gets a 400 from the anonymous self-service
endpoints, which is the intended fail-closed behaviour.
@chatgpt-codex-connector

Copy link
Copy Markdown

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

ResolveDefault() fell through DefaultOrigin -> OriginOptions:OriginUrl ->
the request's own Host header. Both fallbacks are now gone.

The Host tier is the security half. appsettings.Production.json ships
FrontendOptions:AllowedOrigins [], DefaultOrigin "", OriginUrl "" and
AllowedHosts "*", so on the shipped production config a forgot-password
POST with a forged Host header mails the reset token to the attacker's
domain. Before this resolver existed the same path threw, so this was a
regression introduced by the fallback, not a pre-existing hole.

The OriginUrl tier is the correctness half, and it is why the second
fallback goes too. These links address SPA routes (/confirm-email,
/reset-password); the API serves confirm-email under
api/v{version}/identity, so a link built on the API's own origin is a
404. "Degrade to the API origin" stopped being serviceable the moment
the paths changed.

What is left is DefaultOrigin or a 500 naming the setting, and the
startup log for a missing DefaultOrigin moves from Warning to Error to
match: the consequence is no longer degradation. Both shipped deploy
paths (docker compose, terraform) already set it; the gap is a bare
appsettings.Production.json. Upgrade note for same-origin reverse-proxy
deployments that set only OriginUrl: set FrontendOptions:DefaultOrigin
to the same value.

The eight tests that pinned the removed tiers are inverted, not deleted;
the request-host one now asserts the throw and that the attacker's host
never reaches the message. Framework.Tests 152/152, Identity.Tests
312/312, solution builds clean under TreatWarningsAsErrors.
@chatgpt-codex-connector

Copy link
Copy Markdown

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

@marcelo-maciel

marcelo-maciel commented Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

Follow-ups this PR deliberately does not carry, so they are not lost:

No automated coverage for the generated link. I tried to add an integration test asserting the confirm-email URL in the outgoing message and could not make it observe anything. The Hangfire job reports succeeded=1 failed=0 while the mailbox captured through _factory.Services stays empty: the job resolves a different IMailService instance than the factory hands back. That disconnect is pre-existing and is why commit 4cafc42 dropped the equivalent test. Until the harness exposes the same mail sink the job uses, no test in Integration.Tests can assert an e-mail body, so the behaviour here is covered only at the resolver level (FrontendOriginResolverTests).

Docs and changelog (AGENTS.md rule 10): done. Correcting an earlier version of this comment: docs#232 already carried the docs and changelog for this PR, so nothing was missing, it was stale. It described the fallback chain this revision removes. Rewritten in fullstackhero/docs@049ecf0b across the Identity module page, the CORS and headers page, the production checklist and the changelog entry: FrontendOptions:DefaultOrigin is documented as required in practice, with the reason both fallback tiers were dropped. Site build green.

Scope note. The change removes two fallback tiers, not one. Dropping request.Host was the security fix; dropping the OriginOptions:OriginUrl tier came with it because this PR's own route change points these links at the SPA (/confirm-email, /reset-password), which the API origin answers with 404. A fallback that reliably produces a broken link is worse than failing loudly at startup, which is what the new LogError does.

marcelo-maciel added a commit to marcelo-maciel/docs that referenced this pull request Sep 17, 2026
Follows the revision of fullstackhero/dotnet-starter-kit#1377 that removed both
fallback tiers from ResolveDefault(). The pages still described the old chain:
DefaultOrigin, then OriginOptions:OriginUrl, then the request host, with a startup
Warning.

Neither tier survived review. The request host is whatever the caller puts in the
Host header, so a password-reset link derived from it delivers a working token to a
domain the attacker picked. The API's own origin returns 404 for the SPA pages these
links now target, so it produced a dead link rather than a degraded one.

DefaultOrigin is now required in practice: unset, the host boots, logs a startup
Error, and confirm-email, resend-confirmation, forgot-password and reset-password
answer 500.
`appsettings.Production.json` shipped `"AllowedOrigins": []` for both CorsOptions
and FrontendOptions, on the assumption that an empty array clears the base file.
It does not: a JSON array is flattened to indexed keys, an empty one writes no
indices at all, and the binder concatenates whatever the earlier provider left.
A production deployment therefore trusted `http://localhost:5173` and `:5174` —
as a CORS origin, and as an origin that may appear inside a password-reset link.

The dev origins move to `appsettings.Development.json`, which Production never
loads, so the empty arrays in the Production file are now true.

`ShippedConfigurationTests` loads the shipped files the way the host does and
asserts what each environment actually gets. Verified by mutation: putting one
origin back in `appsettings.json` turns it red.

The CorsOptions half of this is pre-existing (`main` has the same shape) and is
fixed here because it is the same defect in the same file; without it the fix
would read as "localhost is untrusted now", which would only be half true.
@chatgpt-codex-connector

Copy link
Copy Markdown

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

The MinIO carve-out this branch carries only moved `minio/minio`. `minio/mc` is
gone from Docker Hub as well (`hub.docker.com/v2/repositories/minio/mc/` answers
404), and it is what `minio-init` runs: without it `dotnet run --project
src/Host/FSH.Starter.AppHost` and `docker compose up` both die on the image pull,
and the `fsh` bucket is never created, so the first upload fails with
NoSuchBucket.

Same pinned tag as fullstackhero#1388, which owns the fix, so the copy stays byte-identical
to it and can be dropped once that lands.
@chatgpt-codex-connector

Copy link
Copy Markdown

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

…comments

`DefaultOrigin` was only checked for emptiness while `AllowedOrigins` went
through `Uri.TryCreate`. A value like `app.example.com` — no scheme, the usual
`.env` slip — bound cleanly, produced no startup diagnostic, and turned every
e-mail link into a relative URL no mail client makes clickable. It is now
required to parse as an absolute URI, and failing that is the same Error as
being unset.

Three comments still described the fallback chain this PR removed:

- `Web/Extensions.cs` said the resolver falls back to the API's own origin and
  logs a Warning. There is no fallback (it throws) and the log is an Error. That
  one sits in protected code, where the next maintainer would have read it as
  "safe degradation exists" and re-introduced the tier.
- `app_stack/main.tf` said an empty list leaves the API resolving links from
  `OriginOptions__OriginUrl`. That tier is gone; those flows answer 500.
- The rejection log wrote the caller-controlled `Origin` header verbatim. It is
  truncated and stripped of line breaks now, the same treatment the global
  exception handler gives the request path.
@chatgpt-codex-connector

Copy link
Copy Markdown

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

… at run time

The startup check added in the previous commit calls a `DefaultOrigin` that is
not an absolute URL "the same failure class as an unset value", but only the log
line agreed: the resolver still handed the raw string back, so `app.example.com`
produced a relative URL in every e-mail, which no mail client makes clickable,
and nothing on the request path reported a problem.

It now goes through the same normalization the allow-list gets: a value that
does not parse as an absolute URI is dropped, and `ResolveDefault()` fails the
way it does when nothing is configured. A base path is preserved (validating must
not collapse `https://example.com/app` to its authority, or `/reset-password`
404s), and both branches are covered. Reverting the guard turns the first red.

The startup message said "is not set" for a value that is set but unusable; it
says "is not set to an absolute URL" now.
@chatgpt-codex-connector

Copy link
Copy Markdown

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant