Terrence Daniels

PublishAsync ran its publish step through _pipeline.Execute(async () => {...}) — the synchronous Execute<TResult> overload with TResult inferred as Task, confirmed by reflecting the real Polly.Core 8.6.6 assembly rather than assumed. Any exception thrown after the lambda's first await — exactly where BrokerUnreachableException would occur — happened after Execute had already returned, so the retry pipeline never observed the failures it was configured to catch. Fixed by moving to the real ExecuteAsync overload as part of a full Decorator split.

TokenValidationParameters.ValidateAudience = false meant a token's aud claim was never actually checked, so a token issued for one downstream API could be replayed against any other API using this same authentication code. Removed the override — ValidateAudience defaults to true, and JwtBearerOptions.Audience already auto-populates the expected value. Flagged for re-verification against real issued tokens once Identity.API lands.

(await _rabbitMQConnection?.CreateChannelAsync()) ?? throw new InvalidOperationException(...) looks like it handles a closed connection gracefully. It doesn't — the ?. short-circuits the whole parenthesized expression to a null Task, and awaiting a null task throws NullReferenceException immediately, so the ?? throw branch was dead code. A caller during startup got an opaque NRE instead of the intended message. Fixed with an explicit null check before the await.

ExtractTraceContextFromBasicProperties did value as byte[] then passed the result straight to Encoding.UTF8.GetString unchecked — a trace header present under the expected key but not actually a byte[] would throw ArgumentNullException outside OnMessageReceived's own try/catch, crashing the entire receive path over one malformed header. Rewritten to pattern-match (value is byte[] bytes) and fall through to the existing empty-result path instead.

DeserializeMessage's as IntegrationEvent cast can genuinely return null for a malformed message body, but its return type and ProcessEvent's use of the result didn't account for that. Return type changed to IntegrationEvent?; ProcessEvent now logs and returns early on null, matching the existing pattern for an unresolvable event type. The exact same cast-can-return-null shape recurred later in IntegrationEventLogEntry.cs — caught the second time because the first fix was still fresh.

ClaimsPrincipalExtensions.GetUserId's "sub" claim lookup only worked because a different file, AuthenticationExtensions, happens to remove "sub" from the framework's default claim-type map elsewhere — nothing enforced that ordering or documented the dependency. Now falls back to ClaimTypes.NameIdentifier regardless of whether that removal ran, so it's correct on its own.

UseDefaultOpenApi picked the default API doc via descriptions[^1] — "take the last one" — assuming IApiVersionDescriptionProvider.ApiVersionDescriptions returns versions in ascending order, which is undocumented anywhere. Switched to descriptions.MaxBy(d => d.ApiVersion), correct regardless of provider ordering.

RetrieveEventLogsPendingToPublishAsync returned Task<IEnumerable<...>>, but the real implementation's .OrderBy().Select(e => e.DeserializeJsonContent(...)) chain is lazy, and DeserializeJsonContent mutates state as a side effect — returning it as IEnumerable risked that mutation re-running, or running late, on every enumeration. Changed to Task<IReadOnlyList<...>>, forcing materialization when the implementation lands.

BuildDescription's deprecation-notice branch checked for a trailing period before appending its message; the sunset-date branch immediately after it didn't do the same check — a real logic inconsistency, not a style nit. Extracted a shared AppendSentenceSeparator helper both branches now call.

The #if DEBUG Android-emulator issuer hardcoded https://10.0.2.2:5243 — 10.0.2.2 is a legitimate fixed emulator host alias, but :5243 was only correct if eShop.AppHost (not built yet) happens to pin Identity.API to that exact port. Now derived from identityUrl's real Uri.Port instead of a hardcoded guess.

ExternalController read AuthenticationProperties.Items["scheme"]/["returnUrl"] through the plain IDictionary indexer, which throws KeyNotFoundException on an absent key instead of returning null — so the ?? throw new Exception(...) and ?? "~/" fallbacks the code was written to lean on never actually ran for a missing key, only for one present but explicitly null. A callback missing either value crashed with an opaque exception instead of the intended message or default. Fixed with TryGetValue at both call sites.

DeviceController.Callback's own doc comment promised the redisplay-with-error outcome would show "the rebuilt form with the validation error" — but the code only ever returned the bare view-model, never actually including the error in the response. A user who left every scope unchecked saw the form redisplayed with no explanation of what went wrong. Fixed with a new DeviceCallbackResult wrapper carrying the view-model and the validation error together.

ConsentController.BuildViewModelAsync interpolated a client-supplied returnUrl directly into _logger.LogError, unsanitized — CWE-117 log injection, caught by CodeQL rather than by manual review. A crafted return URL could inject fake lines or newlines into this service's own logs. Fixed with returnUrl?.ReplaceLineEndings("_") and a named log placeholder.

ProfileService.GetClaimsFromUser constructed Claims straight from user.UserName/user.Email with no null guard, but IdentityUser's own base properties are genuinely nullable (string?, reflection-confirmed against the real Microsoft.Extensions.Identity.Stores assembly) — and Claim's constructor doesn't accept a null value. A user record with a null email would have thrown at token-issuance time. Fixed with an ?? user.Id fallback and an added guard.

Its own doc comment already promised "the redisplayed form with a validation error," but the type never actually included one. AccountController.Login's invalid-credentials path called ModelState.AddModelError(...), which has zero effect on a JSON response — [ApiController]'s automatic 400 only fires from pre-action binding validation, never a manual call inside the action. The client got 200 OK with a redisplayed form and no indication why. Fixed by adding a real ValidationError field and setting it directly.

LogoutInputModel's JSON-body binding rejected application/x-www-form-urlencoded outright — a real HTML form can only send that or multipart, never JSON, so the external-IdP sign-out fallback (a genuine full-page POST, since fetch() can't follow a cross-origin redirect) would have failed every time. Fixed by switching logoutId to a plain query parameter, which then created a genuine C# overload conflict between the GET and POST Logout actions, resolved with [ActionName("Logout")].

BuildLogoutViewModelAsync's own comment says it "prevents attacks where the user is automatically signed out by another malicious web page" — but ShowLogoutPrompt was only ever explicitly forced false, never set true anywhere in the method, in this fork and in Microsoft's own real shipped Quickstart source. Fixed with an explicit override so Duende's own context.ShowSignoutPrompt == true security signal can't be silently demoted by the site-wide convenience toggle.

AccountController.Login/LoginCancel returned RedirectUrl: "~/" for the no-return-URL case — ASP.NET's server-side tilde-root syntax, meaningless to a JSON client doing window.location.href = redirectUrl. Confirmed live with a real browser: it navigated to a literal, nonexistent /Account/~/ path instead of the site root. Fixed with a plain "/" literal, after Url.Content("~/") itself proved fragile (a real NullReferenceException in one test context, silently empty in another) for a sub-path-hosting scenario this app has no evidence of needing.

Nothing in the SPA's router was ever mapped to bare "/" — so the fix above (a genuine, correct redirect target) still hit React Router's own 404 the moment a real browser reached it. Confirmed live via Playwright. Fixed with a route redirecting "/" to /Home/Index, this SPA's one real landing page.

IdentityServerOptions.UserInteraction.ConsentUrl's compiled-in default is /consent (reflection-confirmed), but ConsentController's real route is /Consent/Index — a missing segment, not a case difference. Confirmed live and load-bearing: with a real client's RequireConsent temporarily set to true, an authenticated /connect/authorize request genuinely redirected to /consent. Currently latent (no registered client requires consent today), but would have silently broken the instant one did. Fixed with an explicit ConsentUrl override.

Vite's dev-server proxy defaults (changeOrigin: false) are documented to preserve the browser's original Host header, but that didn't hold up empirically: a temporary diagnostic middleware confirmed Identity.API genuinely received its own address instead of the SPA's real origin. Every real, top-level browser navigation Duende itself triggers — Login's cookie-auth challenge, Consent's own redirect, Logout's /connect/endsession — sent the browser off the SPA entirely, onto Identity.API's raw port. Fixed with a proxyReq hook forcing the outgoing Host header back to the original, verified live against all three redirect chains plus a full device-flow round trip ending in a real token issued to the device.

ConsentController and DeviceController each had their own near-verbatim private CreateScopeViewModel helper, and the copies had drifted: Consent's appended a scope's parsed parameter (e.g. resource1.scope1:42) to the display name, Device's silently didn't. Confirmed this exact inconsistency exists in real upstream Duende source too — this fork had faithfully reproduced an upstream bug, not introduced a new one. Fixed by extracting one shared ScopeViewModelFactory both controllers call, with the correct behavior, plus a direct regression test — the total lack of any prior test on this logic is exactly why the drift went unnoticed.

Device's local Step union's confirm variant had no field to carry a validation error at all, so when DeviceCallbackResult.validationError came back on a failed submission, the form redisplayed with zero indication of what went wrong — unlike ConsentPage's equivalent redisplay path, which already showed the message correctly. Surfaced while extracting the shared ScopeConsentForm component both pages' confirm screens needed; fixed by adding error: string | null to the confirm step and threading the real value through instead of discarding it.