Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds sandboxed in-browser previews for three transactional auth emails, extracts email rendering into pure builders, implements client-branded template fetching/rendering with cache and safety checks, refactors the sender to use builders/branded templates, updates demo client metadata/template, and adds unit/integration tests and changesets. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant AuthService
participant DemoClient
participant TemplateCache
Browser->>AuthService: GET /preview/emails/returning-user?client_id=<meta_url>&otp=123456
AuthService->>AuthService: check AUTH_PREVIEW_ROUTES === '1'
AuthService->>AuthService: parse query (otp, client_id, app, verify_url)
AuthService->>TemplateCache: lookup cached client metadata/template
alt cached
TemplateCache-->>AuthService: return cached template/subject
else not cached
AuthService->>DemoClient: fetch client metadata (meta_url)
DemoClient-->>AuthService: metadata (includes email_template_uri, subject template)
AuthService->>TemplateCache: fetchTemplate(email_template_uri) via safe fetch
TemplateCache-->>AuthService: fetched template HTML or null
end
AuthService->>AuthService: renderTemplate / renderSubjectTemplate with vars (code, app_name, logo_uri, is_new_user, email)
AuthService-->>Browser: 200 text/html (preview page with escaped headers + iframe srcdoc containing rendered HTML)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🚅 Deployed to the ePDS-pr-93 environment in ePDS
|
Coverage Report for CI Build 24688371860Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage increased (+1.8%) to 38.629%Details
Uncovered Changes
Coverage Regressions103 previously-covered lines in 1 file lost coverage.
Coverage Stats
💛 - Coveralls |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/auth-service/src/routes/preview-emails.ts`:
- Around line 16-22: The preview handlers in preview-emails.ts advertise
supporting ?client_id but never read it, so branded templates (clients with
email_template_uri) aren't used; update the preview endpoints to reuse the same
branded-template rendering path used by sendOtpCode (the custom-template branch)
— e.g., lookup the client by client_id, check trusted-client gating and
email_template_uri, and call the branded/template renderer instead of the
default builders; alternatively remove the ?client_id mention from the JSDoc and
release note if you prefer not to implement branded previews. Ensure you update
the functions referenced in preview-emails.ts to pass through the same
client/template selection logic used by sendOtpCode and related email builder
functions so previews match real-sender output.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3b82ac00-8231-4c3b-a2f9-a2d2e1b72021
📒 Files selected for processing (7)
.changeset/in-browser-email-previews.mdpackages/auth-service/src/__tests__/email-templates.test.tspackages/auth-service/src/email/sender.tspackages/auth-service/src/email/templates.tspackages/auth-service/src/index.tspackages/auth-service/src/routes/preview-emails.tspackages/shared/src/preview-ui.ts
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
aspiers
force-pushed
the
feat/email-preview-endpoint
branch
from
bf30b56 to
1fed4ea
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/auth-service/src/email/client-template.ts (1)
127-136: Optional: makerenderSubjectTemplateresilient to placeholders appearing inside substituted values.
replaceAllapplied iteratively means if one variable's value contains another variable's{{placeholder}}, it gets substituted in a later iteration. For current callers the inputs are controlled (client metadata + OTP), so this isn't exploitable, but a single-pass regex replace would make the function robust against future callers.Defer if you'd rather keep it as-is.
♻️ Single-pass variant
export function renderSubjectTemplate( template: string, vars: Record<string, string>, ): string { - let subject = template - for (const [key, value] of Object.entries(vars)) { - subject = subject.replaceAll(`{{${key}}}`, value) - } - return subject + return template.replace(/\{\{(\w+)\}\}/g, (match, key: string) => + Object.prototype.hasOwnProperty.call(vars, key) ? vars[key] : match, + ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/auth-service/src/email/client-template.ts` around lines 127 - 136, renderSubjectTemplate currently iterates replaceAll over vars which allows a substituted value containing another placeholder to be reprocessed; change it to a single-pass replacement by building a single regex that matches all {{key}} placeholders and use a single replacement callback that looks up the key in vars (falling back to original text or empty) so replacements happen atomically. Update the implementation inside renderSubjectTemplate to compute the combined pattern from Object.keys(vars) and call string.replace with a replacer that returns vars[key], preventing substituted values from being rescanned.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/auth-service/src/__tests__/preview-emails.test.ts`:
- Around line 54-67: Save the original process.env.AUTH_PREVIEW_ROUTES before
mutating it (e.g., const _origAuthPreview_ROUTES =
process.env.AUTH_PREVIEW_ROUTES at module scope), then restore it in the
afterAll teardown used with server.close by setting
process.env.AUTH_PREVIEW_ROUTES back to the saved value (or deleting it if
undefined); keep existing calls to clearClientMetadataCache() and
_clearTemplateCacheForTest() unchanged and perform the restore alongside
server.close callback so the environment is not leaked to other tests.
In `@packages/auth-service/src/email/client-template.ts`:
- Around line 193-203: The subject branch is inconsistent: templated subjects
receive the raw code while fallbacks use formatOtpPlain(code), causing different
OTP display; update the renderSubjectTemplate call so templates get a formatted
OTP too (e.g. pass both code and code_formatted or replace code with
formatOtpPlain(code)), by changing the metadata.email_subject_template branch
where renderSubjectTemplate is called (keep renderSubjectTemplate,
metadata.email_subject_template, formatOtpPlain and code identifiers) —
preferred: pass both keys (code: raw code, code_formatted: formatOtpPlain(code))
so templates can opt in, and update the demo template to use {{code_formatted}}.
---
Nitpick comments:
In `@packages/auth-service/src/email/client-template.ts`:
- Around line 127-136: renderSubjectTemplate currently iterates replaceAll over
vars which allows a substituted value containing another placeholder to be
reprocessed; change it to a single-pass replacement by building a single regex
that matches all {{key}} placeholders and use a single replacement callback that
looks up the key in vars (falling back to original text or empty) so
replacements happen atomically. Update the implementation inside
renderSubjectTemplate to compute the combined pattern from Object.keys(vars) and
call string.replace with a replacer that returns vars[key], preventing
substituted values from being rescanned.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c34dd85e-9b50-49a4-83f6-0c655c432e2b
📒 Files selected for processing (15)
.changeset/demo-client-email-branding.md.changeset/in-browser-email-previews.mde2e/step-definitions/email.steps.tsfeatures/email-delivery.featurefeatures/passwordless-authentication.featurepackages/auth-service/src/__tests__/email-templates.test.tspackages/auth-service/src/__tests__/preview-emails.test.tspackages/auth-service/src/email/client-template.tspackages/auth-service/src/email/sender.tspackages/auth-service/src/email/templates.tspackages/auth-service/src/index.tspackages/auth-service/src/routes/preview-emails.tspackages/demo/src/app/client-metadata.json/route.tspackages/demo/src/app/email-template.html/route.tspackages/shared/src/preview-ui.ts
💤 Files with no reviewable changes (1)
- e2e/step-definitions/email.steps.ts
✅ Files skipped from review due to trivial changes (4)
- features/email-delivery.feature
- packages/shared/src/preview-ui.ts
- .changeset/in-browser-email-previews.md
- packages/auth-service/src/tests/email-templates.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/auth-service/src/routes/preview-emails.ts
- packages/auth-service/src/email/templates.ts
- packages/auth-service/src/email/sender.ts
Add /preview/emails/{new-user,returning-user,recovery} to the
auth-service. Each route renders the exact HTML the real sender
would deliver, inside a sandboxed iframe so the email's CSS can't
bleed into or read from the outer preview page.
Extract the three email builders (welcome code, sign-in code,
backup-email verification) into a pure templates.ts so the
preview routes and EmailSender share one source of truth — what
renders in the browser is bit-for-bit what production would put
in the envelope.
Gated by the existing AUTH_PREVIEW_ROUTES=1 flag; no new env vars.
Linked from the /preview index via three new AUTH_PREVIEW_ROUTES
entries.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Coveralls flagged the in-browser email previews commit for a -0.3% coverage delta: preview-emails.ts was 0% covered because the existing auth-service tests unit-test pure functions only. Bring up a real express app with the router mounted on an ephemeral port and hit it with fetch, covering: - the AUTH_PREVIEW_ROUTES=1 gate (404 when unset); - each of the three email routes rendering an iframe with srcdoc and cache-control: no-store; - the from/to/subject headers in the preview shell; - the otp / app / verify_url / to query-param overrides. Raises preview-emails.ts coverage to 100% / 85.71% / 100% / 100%. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Pulls `fetchTemplate`, `renderTemplate`, `renderSubjectTemplate`,
the template cache, and the branded-mail assembly out of
`sender.ts` into a new `email/client-template.ts`, behind a single
`buildClientBrandedEmail({ clientId, code, isNewUser, toEmail,
fallbackAppName, fallbackFromName, trustedClients })` entry point.
- Trust-list gating lives on `buildClientBrandedEmail` now, so any
caller must pass `trustedClients` explicitly — there is no code
path that renders a branded email without going through the
gate. `sender.ts` and the `/preview/emails/*` routes both go
through this function.
- `sender.ts` re-exports `_seedTemplateCacheForTest` /
`_clearTemplateCacheForTest` from `client-template.js` for the
existing test, but new code should import from the new module.
- `/preview/emails/new-user` and `/preview/emails/returning-user`
now accept `?client_id=<URL>`. When the client is on the
trusted-clients list and advertises an `email_template_uri`, the
preview renders the branded template end-to-end — so what you
see in the browser is bit-for-bit what the real sender will
emit.
Arrow-shorthand lint fixes in preview-emails.test.ts are
unrelated drive-by corrections (the rule was already triggering
on rebase).
…ient
- `/email-template.html` route on the demo: minimal Mustache-
style HTML email using the active `EPDS_CLIENT_THEME` palette
(bg / surface / primary / text / border). Inline styles only,
since several popular clients strip or mangle `<style>`.
- `/client-metadata.json` now advertises `email_template_uri` and
`email_subject_template` (`{{code}} — your {{app_name}} code`).
- Integration tests for the `/preview/emails/*?client_id=<URL>`
branded path: trusted client renders branded HTML + subject,
{{#is_new_user}} conditionals honoured on the new-user route,
untrusted client gets default PDS template (metadata is seeded
but never surfaces), trusted client with no template URI falls
back cleanly.
Changeset added.
Demo client now advertises email_subject_template, so OTP subjects are
shaped "{{code}} — your {{app_name}} code" for both welcome and sign-in
emails. The welcome vs sign-in distinction is still asserted by preview
integration tests and template unit tests.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- preview pdsIdentity now uses config.email.fromName (mirrors
better-auth.ts which derives pdsName from SMTP_FROM_NAME).
Keeps preview subject/footer bit-for-bit with production.
- backup-email template wrapped in <!DOCTYPE html>/<html>/<body>
so the preview iframe renders standards-mode like the other two
builders. Added a DOCTYPE assertion to the builder test.
- AUTH_PREVIEW_ROUTES env var snapshot/restore in preview-emails
tests so flipping/deleting it cannot leak into sibling workers.
- buildClientBrandedEmail exposes both {{code}} (raw) and
{{code_formatted}} (grouped) to subject templates so clients can
opt into the PDS-style readable grouping without us forcing it.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
aspiers
force-pushed
the
feat/email-preview-endpoint
branch
from
9e4f532 to
de01a60
Compare
🦋 Changeset detectedLatest commit: d02ff54 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/auth-service/src/email/client-template.ts (1)
127-136:renderSubjectTemplatesubstitutes values raw — consider stripping CRLF defensively.
app_namehere ismetadata.client_namewhich, while sourced from trusted-client metadata, is still fetched over the network. A CR/LF in that value would be injected verbatim into theSubject:header. Nodemailer generally sanitizes headers, but a tinyvalue.replace(/[\r\n]+/g, ' ')inside the substitution loop would make this robust independent of the transport. Low priority given the trusted-clients gate, but cheap insurance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/auth-service/src/email/client-template.ts` around lines 127 - 136, renderSubjectTemplate currently inserts vars raw into the Subject which can allow embedded CR/LF from remote metadata (e.g., metadata.client_name) to affect headers; inside the substitution loop in renderSubjectTemplate, sanitize each replacement by stripping CR and LF (e.g., replace any [\r\n]+ with a single space) before calling subject.replaceAll so the substituted values cannot inject newlines into the Subject header.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/auth-service/src/email/client-template.ts`:
- Line 210: The plain-text body in client-template.ts is currently a single-line
string (text = `Your code for ${appName} is: ${code}...`) which drops the PDS
footer and differs from the default builders; update the plain-text assembly
used by the branded path to mirror buildSignInCodeEmail / buildWelcomeCodeEmail
by creating a multi-line text alternative that includes the expiry sentence, the
"If you didn't request this..." ignore-this-email copy, and the signature footer
formatted as "-- \n${pdsName} (${pdsDomain})", interpolating appName and code;
ensure this plain-text is produced alongside the remote HTML (not derived from
it) so branded templates control only HTML while text remains PDS-controlled.
---
Nitpick comments:
In `@packages/auth-service/src/email/client-template.ts`:
- Around line 127-136: renderSubjectTemplate currently inserts vars raw into the
Subject which can allow embedded CR/LF from remote metadata (e.g.,
metadata.client_name) to affect headers; inside the substitution loop in
renderSubjectTemplate, sanitize each replacement by stripping CR and LF (e.g.,
replace any [\r\n]+ with a single space) before calling subject.replaceAll so
the substituted values cannot inject newlines into the Subject header.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d29d57c3-13ea-4fd3-b938-75375cda2437
📒 Files selected for processing (15)
.changeset/demo-client-email-branding.md.changeset/in-browser-email-previews.mde2e/step-definitions/email.steps.tsfeatures/email-delivery.featurefeatures/passwordless-authentication.featurepackages/auth-service/src/__tests__/email-templates.test.tspackages/auth-service/src/__tests__/preview-emails.test.tspackages/auth-service/src/email/client-template.tspackages/auth-service/src/email/sender.tspackages/auth-service/src/email/templates.tspackages/auth-service/src/index.tspackages/auth-service/src/routes/preview-emails.tspackages/demo/src/app/client-metadata.json/route.tspackages/demo/src/app/email-template.html/route.tspackages/shared/src/preview-ui.ts
💤 Files with no reviewable changes (1)
- e2e/step-definitions/email.steps.ts
✅ Files skipped from review due to trivial changes (3)
- packages/shared/src/preview-ui.ts
- .changeset/in-browser-email-previews.md
- packages/auth-service/src/email/sender.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/auth-service/src/index.ts
- features/email-delivery.feature
- packages/auth-service/src/tests/email-templates.test.ts
The branded path only owns HTML. Plain-text alternative now mirrors
the default builders: branches on isNewUser, includes expiry and
ignore-this-email copy, and ends with the `-- \n${pdsName} (${pdsDomain})`
footer. Thread pdsName/pdsDomain through buildClientBrandedEmail from
both call sites (sender.sendOtpCode, preview-emails.brandedOrFallback).
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/auth-service/src/email/client-template.ts`:
- Around line 52-55: The logger currently includes the raw template URL (uri)
which may contain secrets; update the warn calls in client-template.ts (the
logger.warn invocation near the Email template domain not in allowlist message
and the similar block at lines 73-90) to stop logging the full `uri` value and
instead log only non-sensitive parsed components (e.g., `hostname` and
`protocol`) or a redacted indicator; locate the calls by the message text or the
`logger.warn` usage in this module and replace the structured payload so it
omits `uri` and only emits safe fields like `hostname`/`protocol` (or a boolean
`redacted: true`) to avoid leaking signed URLs or credentials.
- Around line 197-249: The subject and fromName returned by
buildClientBrandedEmail are derived from client metadata
(metadata.email_subject_template via renderSubjectTemplate and
metadata.client_name) and must be normalized to prevent SMTP header injection;
strip CR and LF (e.g., remove '\r' and '\n') and trim the resulting subject and
fromName after rendering (including the fallback subject branches that use
formatOtpPlain) before returning them so only header-safe values propagate to
sender.sendMail.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3f1dbd13-0cdd-47fa-a4b3-5562bc44199a
📒 Files selected for processing (3)
packages/auth-service/src/email/client-template.tspackages/auth-service/src/email/sender.tspackages/auth-service/src/routes/preview-emails.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/auth-service/src/email/sender.ts
Two hardenings in client-template.ts: - fetchTemplate now logs only parsed hostname/protocol (via a new templateLogContext helper), not the full `uri`. Template URIs can be signed URLs or carry credentials in the query string, so the full URI is sensitive and must stay out of application logs. - buildClientBrandedEmail now runs `subject` and `fromName` through a sanitizeHeaderValue helper that strips CR/LF. Even trusted clients' remotely-fetched metadata could be compromised, and a newline in a header value would let an attacker inject additional SMTP headers (Bcc, Reply-To, …) after it. Added a header-injection regression test seeding metadata with CR/LF in client_name and email_subject_template, and asserting the outgoing Subject / From display-name contain no CR/LF.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/auth-service/src/email/client-template.ts (1)
129-143:$-substitution hazard in boolean block replacement.
html.replace(showRegex, value ? '$1' : '')uses a string replacement, so if any trusted client's branded template happens to include$&,$',$`, or$<n>sequences inside a{{#is_new_user}}…{{/is_new_user}}block,String.prototype.replacewill interpret them and the rendered HTML will be wrong (e.g.$&expands to the whole match, re-duplicating the conditional). Prefer a function replacement so the captured group is emitted verbatim.♻️ Proposed fix
- html = html.replace(showRegex, value ? '$1' : '') - html = html.replace(hideRegex, value ? '' : '$1') + html = html.replace(showRegex, (_m, inner: string) => (value ? inner : '')) + html = html.replace(hideRegex, (_m, inner: string) => (value ? '' : inner))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/auth-service/src/email/client-template.ts` around lines 129 - 143, The boolean-block replacements in the loop use string replacements that allow $-expansion (html.replace(showRegex, value ? '$1' : '') and hideRegex similarly); change both replacements to use a function callback so the captured group is returned verbatim (use the replace callback parameters to access the first capture group and return it when value dictates, otherwise return an empty string), updating the replacements for showRegex and hideRegex inside the Object.entries(vars) loop that references html, showRegex, and hideRegex.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/auth-service/src/email/client-template.ts`:
- Around line 129-143: The boolean-block replacements in the loop use string
replacements that allow $-expansion (html.replace(showRegex, value ? '$1' : '')
and hideRegex similarly); change both replacements to use a function callback so
the captured group is returned verbatim (use the replace callback parameters to
access the first capture group and return it when value dictates, otherwise
return an empty string), updating the replacements for showRegex and hideRegex
inside the Object.entries(vars) loop that references html, showRegex, and
hideRegex.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9dce0473-49c0-4a28-a2ca-861903f0d26c
📒 Files selected for processing (2)
packages/auth-service/src/__tests__/email-template.test.tspackages/auth-service/src/email/client-template.ts
On pull_request, `actions/checkout`'s default is the GitHub-synthesized merge commit (refs/pull/N/merge = base + head). Railway, however, deploys the PR branch head — Railway has no native way to deploy merge commits (https://docs.railway.com/deployments/github-autodeploys). The mismatch means when main advances between rebases, E2E test code from the merge commit can assert against UI/behaviour that exists in main but not yet in the PR branch's Railway deployment. Symptom: tests newly added on main fail on unrelated PRs until the PR is rebased (e.g. the "Current Handle:" row introduced by #99 broke #93's CI). Pin checkout to `github.event.pull_request.head.sha` so test code and deployed runtime share one SHA. Empty on push / workflow_dispatch falls back to actions/checkout's default (GITHUB_SHA for the triggering event), verified against the v6.0.2 source. Trade-off: PR CI no longer catches main-incompatibilities until the branch is rebased, but rebase was the mitigation anyway — this just makes the contract explicit instead of surfacing as confusing unrelated failures.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/preview/emails/{new-user,returning-user,recovery}to the auth service — rendering the exact HTML the real sender would deliver, inside a sandboxed iframe so email CSS can't bleed into the outer preview pagetemplates.tsso the preview routes andEmailSendershare one source of truthsender.tsintoemail/client-template.ts#buildClientBrandedEmail, so the preview routes render bit-for-bit what production sends. Preview's?client_id=<URL>now honours the real trusted-clients gate, falling back to the default template if the client is untrusted, has noemail_template_uri, or the fetch fails.email_template_uri(pointing at/email-template.htmlon its own origin) andemail_subject_template, so operators running the demo as a trusted client see a coherent login + email experience out of the box. The template is inline-styled HTML (no<style>block) that follows the demo'sEPDS_CLIENT_THEMEpalette.AUTH_PREVIEW_ROUTES=1flag; no new env vars on auth-service; linked from the/previewindex via three newAUTH_PREVIEW_ROUTESentriesRelationship to #95
#95 (merged) closed the bug that untrusted clients'
email_template_uriwas being honoured. This PR is rebased on top of #95; the newbuildClientBrandedEmailhelper takes the sametrustedClientslist and enforces the gate in both production and preview paths through one code path.Test plan
pnpm format:checkcleanpnpm lintcleanpnpm typecheckcleanpnpm test— 457/457 tests pass, including 5 builder tests inemail-templates.test.tsand 4 new integration tests inpreview-emails.test.tscovering the?client_idbranded path (trusted returning-user, trusted new-user with{{#is_new_user}}conditional, untrusted fallback, trusted-without-template fallback)preview-emails.ts100% stmts,client-template.ts63.88% stmts,sender.tsup to 86.66% after refactorAUTH_PREVIEW_ROUTES=1set, hit/preview/emails/new-user,/preview/emails/returning-user,/preview/emails/recoveryin a browser and visually check the rendered iframe + headersAUTH_PREVIEW_ROUTESunset, confirm all three routes return 404 like the rest of/preview/*?otp=<code>,?app=<name>,?verify_url=<url>overrides render as expectedPDS_OAUTH_TRUSTED_CLIENTS, hit/preview/emails/returning-user?client_id=<demo-base-url>/client-metadata.jsonand confirm the branded template renders with demo theme coloursEPDS_CLIENT_THEME(e.g.ocean) and confirm the branded email preview picks up the new palette🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests