fix: newsletter double opt-in bug fixes

- Fix rate limiter: await formLimiter.check() (was missing await)
- Prevent duplicate confirmation emails: add context.skipNewsletterEmail flag
  - Service sets flag when creating/updating subscribers via API
  - Hook skips email sending when flag is present
  - Admin panel creations still trigger the hook
- Fix unsubscribe links: use subscriber ID instead of token for welcome/unsubscribe emails
  - Token is nullified after confirmation, making old links invalid
  - ID-based lookups always work

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Martin Porwoll 2025-12-10 20:17:28 +00:00
parent 79577626e2
commit 411f1a040e
3 changed files with 39 additions and 8 deletions

View file

@ -5,7 +5,7 @@ import { getPayload } from 'payload'
import config from '@payload-config' import config from '@payload-config'
import { createNewsletterService } from '@/lib/email/newsletter-service' import { createNewsletterService } from '@/lib/email/newsletter-service'
import { getTenantFromRequest } from '@/lib/email/tenant-email-service' import { getTenantFromRequest } from '@/lib/email/tenant-email-service'
import { rateLimiters } from '@/lib/security/rate-limiter' import { formLimiter } from '@/lib/security/rate-limiter'
/** /**
* POST /api/newsletter/subscribe * POST /api/newsletter/subscribe
@ -28,7 +28,7 @@ export async function POST(request: Request): Promise<Response> {
request.headers.get('x-real-ip') || request.headers.get('x-real-ip') ||
'unknown' 'unknown'
const rateLimitResult = rateLimiters.form.check(clientIp) const rateLimitResult = await formLimiter.check(clientIp)
if (!rateLimitResult.allowed) { if (!rateLimitResult.allowed) {
return NextResponse.json( return NextResponse.json(
{ {
@ -38,7 +38,7 @@ export async function POST(request: Request): Promise<Response> {
{ {
status: 429, status: 429,
headers: { headers: {
'Retry-After': String(Math.ceil(rateLimitResult.retryAfter / 1000)), 'Retry-After': String(rateLimitResult.retryAfter || 60),
}, },
}, },
) )

View file

@ -13,14 +13,25 @@ import {
* Hook: Sendet automatisch Double Opt-In E-Mail bei neuen Newsletter-Anmeldungen * Hook: Sendet automatisch Double Opt-In E-Mail bei neuen Newsletter-Anmeldungen
* *
* Wird nur bei neuen Subscribern mit Status "pending" ausgeführt. * Wird nur bei neuen Subscribern mit Status "pending" ausgeführt.
* Vermeidet doppelten Versand durch Prüfung ob E-Mail bereits über API gesendet wurde. * Vermeidet doppelten Versand durch Prüfung ob E-Mail bereits über API/Service gesendet wurde.
*
* Der Hook wird NICHT ausgeführt wenn:
* - Die Erstellung via overrideAccess erfolgt (Newsletter-Service nutzt dies)
* - Ein Kontext-Flag gesetzt ist
*/ */
export const sendNewsletterConfirmation: CollectionAfterChangeHook<NewsletterSubscriber> = async ({ export const sendNewsletterConfirmation: CollectionAfterChangeHook<NewsletterSubscriber> = async ({
doc, doc,
operation, operation,
req, req,
previousDoc, previousDoc,
context,
}) => { }) => {
// Skip wenn via Newsletter-Service erstellt (verwendet overrideAccess)
// Der Service sendet die E-Mail selbst
if (context?.skipNewsletterEmail) {
return doc
}
// Nur bei neuen Anmeldungen (create) oder wenn Status auf "pending" geändert wird // Nur bei neuen Anmeldungen (create) oder wenn Status auf "pending" geändert wird
const isNew = operation === 'create' const isNew = operation === 'create'
const isResubscribe = const isResubscribe =
@ -74,7 +85,8 @@ export const sendNewsletterConfirmation: CollectionAfterChangeHook<NewsletterSub
firstName: doc.firstName || undefined, firstName: doc.firstName || undefined,
email: doc.email, email: doc.email,
confirmationUrl: `${baseUrl}/api/newsletter/confirm?token=${doc.confirmationToken}`, confirmationUrl: `${baseUrl}/api/newsletter/confirm?token=${doc.confirmationToken}`,
unsubscribeUrl: `${baseUrl}/api/newsletter/unsubscribe?token=${doc.confirmationToken}`, // Für Confirmation-E-Mail Token verwenden (noch gültig), aber ID als Fallback
unsubscribeUrl: `${baseUrl}/api/newsletter/unsubscribe?token=${doc.confirmationToken || doc.id}`,
tenantName: tenant.name, tenantName: tenant.name,
tenantWebsite, tenantWebsite,
privacyPolicyUrl: tenantWebsite ? `${tenantWebsite}/datenschutz` : undefined, privacyPolicyUrl: tenantWebsite ? `${tenantWebsite}/datenschutz` : undefined,

View file

@ -99,6 +99,7 @@ export class NewsletterService {
subscribedAt: new Date().toISOString(), subscribedAt: new Date().toISOString(),
}, },
overrideAccess: true, overrideAccess: true,
context: { skipNewsletterEmail: true }, // Hook soll keine E-Mail senden
}) })
// Double Opt-In E-Mail erneut senden // Double Opt-In E-Mail erneut senden
@ -133,6 +134,7 @@ export class NewsletterService {
source: data.source || subscriber.source, source: data.source || subscriber.source,
}, },
overrideAccess: true, overrideAccess: true,
context: { skipNewsletterEmail: true }, // Hook soll keine E-Mail senden
}) })
await this.sendConfirmationEmail(tenantId, { await this.sendConfirmationEmail(tenantId, {
@ -164,6 +166,7 @@ export class NewsletterService {
tenant: tenantId, tenant: tenantId,
}, },
overrideAccess: true, overrideAccess: true,
context: { skipNewsletterEmail: true }, // Hook soll keine E-Mail senden, Service macht das
}) })
// Double Opt-In E-Mail senden // Double Opt-In E-Mail senden
@ -361,13 +364,16 @@ export class NewsletterService {
/** /**
* Willkommens-E-Mail nach erfolgreicher Bestätigung senden * Willkommens-E-Mail nach erfolgreicher Bestätigung senden
* Verwendet immer die Subscriber-ID für Unsubscribe-Links,
* da der Token nach Bestätigung gelöscht wird.
*/ */
private async sendWelcomeEmail( private async sendWelcomeEmail(
tenantId: number, tenantId: number,
subscriber: NewsletterSubscriber, subscriber: NewsletterSubscriber,
): Promise<void> { ): Promise<void> {
const tenant = await this.getTenant(tenantId) const tenant = await this.getTenant(tenantId)
const templateData = this.buildTemplateData(tenant, subscriber) // Immer ID für Unsubscribe verwenden, da Token nach Bestätigung null ist
const templateData = this.buildTemplateData(tenant, subscriber, { useIdForUnsubscribe: true })
await sendTenantEmail(this.payload, tenantId, { await sendTenantEmail(this.payload, tenantId, {
to: subscriber.email, to: subscriber.email,
@ -384,13 +390,15 @@ export class NewsletterService {
/** /**
* Abmelde-Bestätigung senden * Abmelde-Bestätigung senden
* Verwendet die Subscriber-ID für Links, da kein Token mehr benötigt wird.
*/ */
private async sendUnsubscribeEmail( private async sendUnsubscribeEmail(
tenantId: number, tenantId: number,
subscriber: NewsletterSubscriber, subscriber: NewsletterSubscriber,
): Promise<void> { ): Promise<void> {
const tenant = await this.getTenant(tenantId) const tenant = await this.getTenant(tenantId)
const templateData = this.buildTemplateData(tenant, subscriber) // ID für Links verwenden
const templateData = this.buildTemplateData(tenant, subscriber, { useIdForUnsubscribe: true })
await sendTenantEmail(this.payload, tenantId, { await sendTenantEmail(this.payload, tenantId, {
to: subscriber.email, to: subscriber.email,
@ -419,10 +427,15 @@ export class NewsletterService {
/** /**
* Template-Daten zusammenstellen * Template-Daten zusammenstellen
*
* Für Confirmation-E-Mails wird der Token verwendet.
* Für Willkommens- und andere E-Mails wird immer die ID verwendet,
* da der Token nach Bestätigung gelöscht wird.
*/ */
private buildTemplateData( private buildTemplateData(
tenant: Tenant, tenant: Tenant,
subscriber: NewsletterSubscriber, subscriber: NewsletterSubscriber,
options?: { useIdForUnsubscribe?: boolean },
): NewsletterTemplateData { ): NewsletterTemplateData {
// Tenant-Website URL ermitteln // Tenant-Website URL ermitteln
const tenantWebsite = tenant.domains?.[0]?.domain const tenantWebsite = tenant.domains?.[0]?.domain
@ -434,11 +447,17 @@ export class NewsletterService {
? `${tenantWebsite}/datenschutz` ? `${tenantWebsite}/datenschutz`
: undefined : undefined
// Für Unsubscribe immer ID verwenden wenn kein Token vorhanden
// oder wenn explizit angefordert (z.B. für Willkommens-E-Mail nach Bestätigung)
const unsubscribeToken = options?.useIdForUnsubscribe || !subscriber.confirmationToken
? String(subscriber.id)
: subscriber.confirmationToken
return { return {
firstName: subscriber.firstName || undefined, firstName: subscriber.firstName || undefined,
email: subscriber.email, email: subscriber.email,
confirmationUrl: `${this.baseUrl}/api/newsletter/confirm?token=${subscriber.confirmationToken}`, confirmationUrl: `${this.baseUrl}/api/newsletter/confirm?token=${subscriber.confirmationToken}`,
unsubscribeUrl: `${this.baseUrl}/api/newsletter/unsubscribe?token=${subscriber.confirmationToken || subscriber.id}`, unsubscribeUrl: `${this.baseUrl}/api/newsletter/unsubscribe?token=${unsubscribeToken}`,
tenantName: tenant.name, tenantName: tenant.name,
tenantWebsite, tenantWebsite,
privacyPolicyUrl, privacyPolicyUrl,