fix(api): strip email PII from remaining email/marketing log sites (#243)

[thomasluizon]

Removes recipient email addresses and raw provider response bodies from the log sites that merged PR #325 deliberately left out of its scope, so no PII lands in prod logs or Sentry. Each site keeps its non-PII diagnostics (subject, HTTP status, a user/correlation id, error).

Log sites scrubbed

Site	Level	Before	After
VerifyCodeCommand.LogWelcomeEmailFailed	Warning	for user {Email}	for user {UserId}
ResendEmailService.LogEmailSent	Information	to {To} subject={Subject}	subject={Subject}
ResendEmailService.LogEmailFailed	Error	to {To} status={Status} body={Body}	subject={Subject} status={Status}
ResendEmailService.LogEmailSendException	Error	to {To}	(no PII)
ResendEmailService.LogSkippingTestEmail	Information	{To} subject={Subject}	subject={Subject}
ResendEmailService.LogMarketingRetry	Warning	to {To} ...	... (attempt/backoff only)
SendMarketingBroadcastCommand.LogPreviewSent	Information	to {TestEmail}	subject={Subject}
AuthController.LogVerificationCodeSent	Information	to {Email}. RequestId=	RequestId=
AuthController.LogUserLoggedInViaCode	Information	code {Email}. RequestId=	UserId={UserId} RequestId=

Cleanup pulled in (same-PR, maximum-implementation)

• The two now-orphaned response.Content.ReadAsStringAsync reads that only fed {Body} are removed.
• LogBroadcastQueued drops its redundant {QueuedAtUtc} param — the logging framework already timestamps every entry, and the DateTime.UtcNow tripped the csharp-tz rule.

Scope

Internal logging only — no endpoint, DTO, or contract change, so no packages/shared/consumer edits. Levels unchanged (task is a PII scrub, not a level change). GoogleAuth's welcome-email + login-via-Google logs were already PII-free from #325.

Tests

• New ResendEmailServicePiiScrubbingTests (capturing logger): success/failure/exception/marketing paths assert the recipient email and raw response body never reach the rendered log line, while status/subject survive.
• New AuthControllerTests capturing-logger cases: send-code and login-via-code logs omit the email; the login log carries the UserId.
• New SendMarketingBroadcastCommandHandlerTests case: the preview log omits the test email and carries the subject.
• Full Orbit.Infrastructure.Tests (1540) + Orbit.Application.Tests (2629) green locally.

Refs thomasluizon/orbit-ui-mobile#243

[claude]

PR Review — #331: fix(api): strip email PII from remaining email/marketing log sites

Recommendation: APPROVE

Summary

Narrow, logging-only PII scrub: removes recipient email addresses and raw HTTP response bodies from nine [LoggerMessage] sites across AuthController.cs, VerifyCodeCommand.cs, SendMarketingBroadcastCommand.cs, and ResendEmailService.cs, replacing them with non-PII diagnostics (UserId, subject, HTTP status). No endpoint, DTO, or contract surface is touched.

Severity	Count
Critical	0
High	0
Medium	0
Low / Info	1

Findings

Low/Info (non-blocking) — Three test files (AuthControllerTests.cs, SendMarketingBroadcastCommandHandlerTests.cs, ResendEmailServicePiiScrubbingTests.cs) each add their own private CollectingLogger<T> : ILogger<T> test double (~15 identical lines). This mirrors a pre-existing pattern already in the suite (AiRetryLoggingPolicyTests.cs has its own copy too), so it's not a regression — just a note that a shared test helper could be hoisted if a fourth copy appears.

Verified

• No stale callers of the old [LoggerMessage] signatures remain anywhere in src/ or tests/.
• The two now-orphaned response.Content.ReadAsStringAsync(...) reads were correctly deleted; using (response) disposal scopes are preserved.
• LoginResponse record shape, AuthController routes/request records, and SendMarketingBroadcastCommand's public shape are unchanged — no contract surface moved.
• Test-added constructor calls match current source signatures.
• Log levels are unchanged everywhere, matching the PR's stated scope.
• LogBroadcastQueued also dropped its DateTime.UtcNow param as a bonus csharp-tz-hook fix, called out in the PR body.

Independent security review (security-reviewer subagent)

Result: PASS, no issues introduced.

• Scrubbing is complete at every modified log site; only UserId (Guid, not PII), subject, status, attempt/backoff remain.
• [Authorize]/[AllowAnonymous]/[DistributedRateLimit] attributes on auth endpoints are untouched.
• Response-body disposal (using (response)) is unaffected by the body-read removal.
• Flagged one pre-existing, out-of-scope residual for a future PR: ResendEmailService.cs:43 builds a signInUrl embedding the raw verification code/email into a URL — not a log site, not a regression from this diff.

Deferred / not verifiable in CI

• Contract-aligner / orbit-ui-mobile backward-compat: sibling repo not checked out this run; structurally N/A here since no DTO/endpoint field changed.
• Phase 6 (dotnet build/test): skipped — Build/Unit Tests/SonarCloud run as separate required CI checks on this PR.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
93.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[claude]

Code Review: PR #331

Scope: fix(api): strip email PII from remaining email/marketing log sites (+ follow-up test commit)
Recommendation: APPROVE

Summary

This PR finishes the PII-scrub sweep left out of scope by #325: it removes recipient email addresses and raw provider response bodies from the remaining email/marketing log sites in AuthController, VerifyCodeCommand, SendMarketingBroadcastCommand, and ResendEmailService, replacing them with non-PII diagnostics (UserId, Subject, HttpStatusCode). A follow-up commit (418b7ba) adds unit-test coverage for the marketing success/retry/exception and test-account skip log paths — test-only, no production code changed. It's a pure internal-logging change with no endpoint/DTO/contract surface touched.

Findings

Critical

None.

High

None.

Medium

[MEDIUM] Duplicate CollectingLogger<T> capturing-logger test double defined identically in multiple files across the test suite
· location: tests/Orbit.Infrastructure.Tests/Controllers/AuthControllerTests.cs, tests/Orbit.Infrastructure.Tests/Services/ResendEmailServicePiiScrubbingTests.cs (plus a non-generic variant already in AiRetryLoggingPolicyTests.cs and another copy in Orbit.Application.Tests)
· issue: The same small ILogger<T>-capturing test double is redeclared in each file instead of living in one shared test-support location.
· risk: Low — nothing breaks today, but a future fix to the helper (e.g. scope capture, thread-safety) would need to be applied in multiple places.
· fix: Extract a single CollectingLogger<T> into a shared test helper in tests/Orbit.Infrastructure.Tests/ and reference it from both files. Not a blocker for this PR.

Low / Info

None surfaced (signal gate).

Validation

Check	Result
Build / Unit Tests / SonarCloud	Deferred to this PR's separate required CI checks

What's good

• The scrub is thorough: every log site the PR claims to fix has verified changes — to/Email params are gone from [LoggerMessage] signatures, replaced with UserId (auth) or Subject/Status (email), and orphaned response.Content.ReadAsStringAsync() reads were removed alongside the {Body} params that consumed them.
• New capturing-logger tests assert the negative (entry.Should().NotContain(email) / OnlyContain(!Contains(email))) rather than only the positive shape — the right test for a PII-scrub change.
• The follow-up commit closes the remaining coverage gaps (marketing retry/exception/skip paths, transactional skip path) without touching production code.

Recommendation

Safe to merge as-is. The Medium (test-helper duplication) is a nice-to-have cleanup, not a blocker.