Skip to content

Commit f229b71

Browse files
HugoGresseclaude
andauthored
Identify OpenPlanner in speaker self-edit emails (#259)
* Identify OpenPlanner in speaker self-edit emails Each event organiser configures their own MAIL_FROM domain, so the From header alone is not enough for a speaker to recognise that the magic-link email or the approval/rejection notification comes from OpenPlanner. Speakers receiving the link from an unfamiliar address are likely to flag it as phishing. - requestEditLinkPOST.ts renderEmail (FR + EN) now appends a short footer: "This email was sent by OpenPlanner on behalf of '<event name>'. For questions, contact contact@email.openplanner.fr." - renderPendingEditDecisionEmail.ts (approve + reject) gains the same footer via a shared buildFooter helper so all three speaker-facing emails carry the OpenPlanner attribution + a real reply-to. - New unit tests on renderApprovedEmail / renderRejectedEmail lock in the footer presence, the contact address, and the existing field/reviewer-note rendering. 222/222 backend tests pass. The footer text is identical to the request-link mail so any address filter / signature detection the speaker has set up keeps working. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Address Copilot review on PR 259: shared footer + real Reply-To header Two themes from the review, both fixed: 1) "Reply-to" comment was inaccurate — only the body carried the contact address, no SMTP header was set. sendEmail now accepts an optional `replyTo` on EmailMessage and forwards it to nodemailer so a real Reply-To header is emitted. The audit row in `mail/` also persists the value (or `null`) so ops can see what was advertised. All three speaker-facing call sites (request-link, approve, reject) pass `replyTo: OPENPLANNER_CONTACT_EMAIL`, meaning a speaker who hits Reply lands in the OpenPlanner contact inbox even when MAIL_FROM is a per-event no-reply alias. 2) `OPENPLANNER_CONTACT` and `buildFooter` were duplicated between requestEditLinkPOST.ts and renderPendingEditDecisionEmail.ts — exactly the drift risk the review flagged. Introduce a single `functions/src/api/other/speakerEmailFooter.ts` module that exports `OPENPLANNER_CONTACT_EMAIL` + `buildSpeakerEmailFooter(eventName)`. The FR variant of the magic-link mail keeps its own localised wording but pulls the contact constant from the shared module so the address only lives in one place. The English footer + the approve/reject notifications consume `buildSpeakerEmailFooter` directly. New tests: - speakerEmailFooter.test.ts pins the contact constant and the footer shape (leading `--\n`, OpenPlanner attribution, event name + contact interpolation). - sendEmail.test.ts gains two cases for the replyTo path: header is forwarded to nodemailer + persisted on the audit row when present; undefined / null respectively when caller omits it. 226/226 backend tests pass; tsc clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 1c53f8b commit f229b71

9 files changed

Lines changed: 176 additions & 7 deletions
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import { describe, expect, test } from 'vitest'
2+
import { renderApprovedEmail, renderRejectedEmail } from './renderPendingEditDecisionEmail'
3+
4+
const eventName = 'My Event'
5+
6+
describe('renderApprovedEmail', () => {
7+
test('lists the applied fields and identifies OpenPlanner as the sender', () => {
8+
const { subject, text } = renderApprovedEmail('Jane', eventName, {
9+
name: 'Jane Doe',
10+
jobTitle: 'Lead Engineer',
11+
})
12+
expect(subject).toMatch(/approved/i)
13+
expect(text).toMatch(/Jane/)
14+
expect(text).toMatch(/Lead Engineer/)
15+
// Footer must always carry the OpenPlanner attribution + reply-to
16+
// contact so the speaker knows who is writing regardless of the
17+
// From header.
18+
expect(text).toContain('OpenPlanner')
19+
expect(text).toContain('contact@email.openplanner.fr')
20+
expect(text).toContain(eventName)
21+
})
22+
})
23+
24+
describe('renderRejectedEmail', () => {
25+
test('mentions the rejection note and identifies OpenPlanner as the sender', () => {
26+
const { subject, text } = renderRejectedEmail('Jane', eventName, { bio: 'lorem' }, 'Bio too long')
27+
expect(subject).toMatch(/not applied|rejected/i)
28+
expect(text).toMatch(/Bio too long/)
29+
expect(text).toContain('OpenPlanner')
30+
expect(text).toContain('contact@email.openplanner.fr')
31+
})
32+
33+
test('omits the reviewer-note section when none provided', () => {
34+
const { text } = renderRejectedEmail('Jane', eventName, { bio: 'lorem' })
35+
expect(text).not.toMatch(/Reviewer note/i)
36+
expect(text).toContain('contact@email.openplanner.fr')
37+
})
38+
})

functions/src/api/other/renderPendingEditDecisionEmail.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { Speaker } from '../../types'
2+
import { buildSpeakerEmailFooter } from './speakerEmailFooter'
23

34
const FIELD_LABELS: Record<string, string> = {
45
name: 'Name',
@@ -55,7 +56,8 @@ export const renderApprovedEmail = (
5556
`Hello ${speakerName},\n\n` +
5657
`Your profile changes for "${eventName}" have been approved by an administrator and will be public soon.\n\n` +
5758
`Changes applied:\n${changes}\n\n` +
58-
`If you did not request these changes, please contact the event organisers.`,
59+
`If you did not request these changes, please contact the event organisers.\n\n` +
60+
buildSpeakerEmailFooter(eventName),
5961
}
6062
}
6163

@@ -73,6 +75,7 @@ export const renderRejectedEmail = (
7375
`Hello ${speakerName},\n\n` +
7476
`Your recent profile changes for "${eventName}" were not applied by the administrators.\n\n` +
7577
`Changes you proposed:\n${changes}${noteSection}\n\n` +
76-
`You can request a fresh edit link from the speaker self-edit page if you want to retry.`,
78+
`You can request a fresh edit link from the speaker self-edit page if you want to retry.\n\n` +
79+
buildSpeakerEmailFooter(eventName),
7780
}
7881
}

functions/src/api/other/sendEmail.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,40 @@ describe('sendEmail', () => {
132132
})
133133
})
134134

135+
test('forwards replyTo to nodemailer and persists it on the audit row', async () => {
136+
const setSpy = vi.fn(() => Promise.resolve())
137+
const addSpy = vi.fn()
138+
nodemailerMocks.sendMail.mockResolvedValueOnce({ messageId: 'mid-rt', response: '250 OK' })
139+
140+
await sendEmail(makeFirebaseApp(setSpy, addSpy), {
141+
to: 'jane@example.com',
142+
subject: 'Hello',
143+
text: 'body',
144+
replyTo: 'contact@email.openplanner.fr',
145+
})
146+
const sendArg = nodemailerMocks.sendMail.mock.calls[0][0]
147+
expect(sendArg.replyTo).toBe('contact@email.openplanner.fr')
148+
149+
const pendingDoc = addSpy.mock.calls[0][0]
150+
expect(pendingDoc.replyTo).toBe('contact@email.openplanner.fr')
151+
})
152+
153+
test('omits replyTo header when caller did not pass one and stamps null on the audit row', async () => {
154+
const setSpy = vi.fn(() => Promise.resolve())
155+
const addSpy = vi.fn()
156+
nodemailerMocks.sendMail.mockResolvedValueOnce({ messageId: 'mid-no-rt', response: '250 OK' })
157+
158+
await sendEmail(makeFirebaseApp(setSpy, addSpy), {
159+
to: 'jane@example.com',
160+
subject: 'Hello',
161+
text: 'body',
162+
})
163+
const sendArg = nodemailerMocks.sendMail.mock.calls[0][0]
164+
expect(sendArg.replyTo).toBeUndefined()
165+
const pendingDoc = addSpy.mock.calls[0][0]
166+
expect(pendingDoc.replyTo).toBeNull()
167+
})
168+
135169
test('uses STARTTLS (secure=false) with requireTLS on port 587', async () => {
136170
__resetEmailTransporterForTests()
137171
process.env.MAILGUN_SMTP_PORT = '587'

functions/src/api/other/sendEmail.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,13 @@ export interface EmailMessage {
99
subject: string
1010
text: string
1111
html?: string
12+
/**
13+
* Optional Reply-To header. Callers that want replies routed to a
14+
* different mailbox than MAIL_FROM (e.g. the OpenPlanner contact inbox
15+
* for speaker-facing mail while keeping a per-event MAIL_FROM) should
16+
* pass the address here. Persisted on the audit row.
17+
*/
18+
replyTo?: string
1219
}
1320

1421
// Convert a plain-text email body into safe HTML. Escapes every character
@@ -122,6 +129,7 @@ export const sendEmail = async (
122129
const docRef = await db.collection(collection).add({
123130
to: message.to,
124131
from,
132+
replyTo: message.replyTo ?? null,
125133
message: { subject, text: message.text, html },
126134
createdAt: FieldValue.serverTimestamp(),
127135
delivery: { state: 'PENDING' },
@@ -148,6 +156,12 @@ export const sendEmail = async (
148156
const info = await transporter.sendMail({
149157
from,
150158
to: message.to,
159+
// nodemailer accepts `replyTo` and emits a real Reply-To
160+
// header so a recipient hitting Reply lands in the inbox we
161+
// specify instead of MAIL_FROM. Falls back to undefined when
162+
// the caller did not set one, matching nodemailer's
163+
// "no header" default.
164+
replyTo: message.replyTo,
151165
subject,
152166
text: message.text,
153167
html,
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import { describe, expect, test } from 'vitest'
2+
import { OPENPLANNER_CONTACT_EMAIL, buildSpeakerEmailFooter } from './speakerEmailFooter'
3+
4+
describe('speakerEmailFooter', () => {
5+
test('OPENPLANNER_CONTACT_EMAIL is the canonical address', () => {
6+
expect(OPENPLANNER_CONTACT_EMAIL).toBe('contact@email.openplanner.fr')
7+
})
8+
9+
test('buildSpeakerEmailFooter interpolates the event name and embeds the contact', () => {
10+
const out = buildSpeakerEmailFooter('My Conf')
11+
expect(out).toContain('OpenPlanner')
12+
expect(out).toContain('My Conf')
13+
expect(out).toContain(OPENPLANNER_CONTACT_EMAIL)
14+
// Leading separator line — kept for visual distinction in plain
15+
// text clients.
16+
expect(out.startsWith('--\n')).toBe(true)
17+
})
18+
})
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// Single source of truth for the OpenPlanner attribution shown to
2+
// speakers in every self-edit email (magic-link request, approval
3+
// notification, rejection notification). Centralising the constant +
4+
// the footer builder here prevents the wording from drifting across
5+
// templates the next time the contact address or the copy changes.
6+
7+
export const OPENPLANNER_CONTACT_EMAIL = 'contact@email.openplanner.fr'
8+
9+
/**
10+
* Plain-text attribution footer appended to every speaker-facing email
11+
* the OpenPlanner backend sends. Per-event MAIL_FROM domains mean the
12+
* From header alone is not a stable identifier for the speaker — this
13+
* line tells them which platform is writing AND gives them a real
14+
* inbox they can reply to (the SMTP Reply-To header is also set
15+
* server-side; see sendEmail).
16+
*/
17+
export const buildSpeakerEmailFooter = (eventName: string): string =>
18+
`--\nThis email was sent by OpenPlanner on behalf of "${eventName}". ` +
19+
`For questions, contact ${OPENPLANNER_CONTACT_EMAIL}.`

functions/src/api/routes/speakers/approvePendingEditPOST.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { SpeakerDao } from '../../dao/speakerDao'
55
import { EventDao } from '../../dao/eventDao'
66
import { Speaker } from '../../../types'
77
import { sendEmail } from '../../other/sendEmail'
8+
import { OPENPLANNER_CONTACT_EMAIL } from '../../other/speakerEmailFooter'
89
import { renderApprovedEmail } from '../../other/renderPendingEditDecisionEmail'
910

1011
const TypeBoxApproveBody = Type.Object(
@@ -100,7 +101,12 @@ export const approvePendingEditRouteHandler = (fastify: FastifyInstance) => {
100101
)
101102
await sendEmail(
102103
fastify.firebase,
103-
{ to: speakerBefore.email, subject: email.subject, text: email.text },
104+
{
105+
to: speakerBefore.email,
106+
subject: email.subject,
107+
text: email.text,
108+
replyTo: OPENPLANNER_CONTACT_EMAIL,
109+
},
104110
{ eventId, speakerId: pending.speakerId, type: 'speaker-edit-approved', requestId }
105111
)
106112
} catch (err) {

functions/src/api/routes/speakers/rejectPendingEditPOST.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { SpeakerDao } from '../../dao/speakerDao'
55
import { EventDao } from '../../dao/eventDao'
66
import { deletePendingPhotoFromUrl } from '../../other/deletePendingPhoto'
77
import { sendEmail } from '../../other/sendEmail'
8+
import { OPENPLANNER_CONTACT_EMAIL } from '../../other/speakerEmailFooter'
89
import { renderRejectedEmail } from '../../other/renderPendingEditDecisionEmail'
910
import { Speaker } from '../../../types'
1011

@@ -102,7 +103,12 @@ export const rejectPendingEditRouteHandler = (fastify: FastifyInstance) => {
102103
)
103104
await sendEmail(
104105
fastify.firebase,
105-
{ to: speakerBefore.email, subject: email.subject, text: email.text },
106+
{
107+
to: speakerBefore.email,
108+
subject: email.subject,
109+
text: email.text,
110+
replyTo: OPENPLANNER_CONTACT_EMAIL,
111+
},
106112
{ eventId, speakerId: pending.speakerId, type: 'speaker-edit-rejected', requestId }
107113
)
108114
}

functions/src/api/routes/speakers/requestEditLinkPOST.ts

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { SpeakerEditTokenDao } from '../../dao/speakerEditTokenDao'
66
import { SpeakerEditRateLimitDao } from '../../dao/speakerEditRateLimitDao'
77
import { verifyCaptchaToken } from '../../other/captchaVerify'
88
import { sendEmail } from '../../other/sendEmail'
9+
import { OPENPLANNER_CONTACT_EMAIL, buildSpeakerEmailFooter } from '../../other/speakerEmailFooter'
910

1011
const TypeBoxRequestEditLink = Type.Object(
1112
{
@@ -41,16 +42,37 @@ export const requestEditLinkPOSTSchema = {
4142
},
4243
}
4344

45+
// Localised footer for the FR magic-link mail. Mirrors the EN footer
46+
// from speakerEmailFooter.ts in shape, but keeping the wording on the
47+
// route file lets us drop the per-language branch entirely if we ever
48+
// merge templates. The shared OPENPLANNER_CONTACT_EMAIL constant is
49+
// imported so the address only lives in one place.
50+
const buildFrenchFooter = (eventName: string): string =>
51+
`--\nCet email est envoyé par OpenPlanner pour le compte de "${eventName}". ` +
52+
`Pour toute question, contactez ${OPENPLANNER_CONTACT_EMAIL}.`
53+
4454
const renderEmail = (speakerName: string, eventName: string, link: string, lang: 'fr' | 'en') => {
4555
if (lang === 'fr') {
4656
return {
4757
subject: `Modifier votre profil — ${eventName}`,
48-
text: `Bonjour ${speakerName},\n\nVous avez demandé un lien pour modifier votre profil public pour l'événement "${eventName}".\n\nCliquez ici (valable 7 jours) :\n${link}\n\nVos modifications seront vérifiées par un administrateur avant d'être publiées.\n\nSi vous n'êtes pas à l'origine de cette demande, ignorez cet email.`,
58+
text:
59+
`Bonjour ${speakerName},\n\n` +
60+
`Vous avez demandé un lien pour modifier votre profil public pour l'événement "${eventName}".\n\n` +
61+
`Cliquez ici (valable 7 jours) :\n${link}\n\n` +
62+
`Vos modifications seront vérifiées par un administrateur avant d'être publiées.\n\n` +
63+
`Si vous n'êtes pas à l'origine de cette demande, ignorez cet email.\n\n` +
64+
buildFrenchFooter(eventName),
4965
}
5066
}
5167
return {
5268
subject: `Edit your profile — ${eventName}`,
53-
text: `Hello ${speakerName},\n\nYou requested a link to edit your public profile for "${eventName}".\n\nClick here (valid 7 days):\n${link}\n\nYour changes will be reviewed by an administrator before going live.\n\nIf you did not request this, ignore this email.`,
69+
text:
70+
`Hello ${speakerName},\n\n` +
71+
`You requested a link to edit your public profile for "${eventName}".\n\n` +
72+
`Click here (valid 7 days):\n${link}\n\n` +
73+
`Your changes will be reviewed by an administrator before going live.\n\n` +
74+
`If you did not request this, ignore this email.\n\n` +
75+
buildSpeakerEmailFooter(eventName),
5476
}
5577
}
5678

@@ -130,7 +152,16 @@ export const requestEditLinkRouteHandler = (fastify: FastifyInstance) => {
130152
try {
131153
await sendEmail(
132154
fastify.firebase,
133-
{ to: matching.email as string, subject: email_.subject, text: email_.text },
155+
{
156+
to: matching.email as string,
157+
subject: email_.subject,
158+
text: email_.text,
159+
// Set the SMTP Reply-To header so a speaker hitting
160+
// Reply lands in the OpenPlanner contact inbox rather
161+
// than the per-event MAIL_FROM (which is often a
162+
// no-reply alias).
163+
replyTo: OPENPLANNER_CONTACT_EMAIL,
164+
},
134165
{ eventId, speakerId: matching.id, type: 'speaker-edit-link' }
135166
)
136167
} catch (err) {

0 commit comments

Comments
 (0)