Fix code review findings for FCM/APNs push notifications (FRE-5345)

- P0: Add missing jwt import (remove duplicate getAPNSToken from push.service.ts)
- P0: Fix race condition in getFCMApp() with promise-based initialization lock
- P0: Fix preHandler short-circuit in device.routes.ts (add return before reply.send)
- P1: Replace non-null assertions with safe defaults in notification config
- P1: Add rate limiting on device registration endpoint (10 req/5min per user)
- P2: Add push notification deduplication using content hash
- P2: Add APNs payload size validation (256KB limit)

Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
2026-05-16 22:16:47 -04:00
parent 9f65ebce5d
commit 590e15e66e
4 changed files with 991 additions and 53 deletions

View File

@@ -0,0 +1,309 @@
import jwt from 'jsonwebtoken';
import https from 'https';
import { loadNotificationConfig } from '../config/notification.config';
import type { PushNotification, NotificationResult } from '../types/notification.types';
const config = loadNotificationConfig();
let apnsToken: string | null = null;
let tokenExpiration: number = 0;
interface APNsPayload {
aps: {
alert: {
title: string;
body: string;
};
badge?: number;
sound?: string | {
critical: number;
name: string;
volume: number;
};
category?: string;
'content-available'?: number;
'mutable-content'?: number;
};
[key: string]: any;
}
function generateAPNSToken(): string {
const now = Math.floor(Date.now() / 1000);
const token = jwt.sign(
{},
config.apns.key,
{
algorithm: 'ES256' as any,
headers: {
alg: 'ES256',
kty: 'EC',
kid: config.apns.keyId,
},
expiresIn: '1h',
issuer: config.apns.teamId,
} as any
);
tokenExpiration = now + 3500; // Refresh 1 minute before expiration
return token;
}
function getAPNSToken(): string {
const now = Math.floor(Date.now() / 1000);
if (!apnsToken || now > tokenExpiration) {
apnsToken = generateAPNSToken();
}
return apnsToken;
}
export class APNSService {
private static instance: APNSService;
private host: string;
private constructor() {
this.host = process.env.NODE_ENV === 'production'
? 'api.push.apple.com'
: 'api.sandbox.push.apple.com';
}
static getInstance(): APNSService {
if (!APNSService.instance) {
APNSService.instance = new APNSService();
}
return APNSService.instance;
}
async send(notification: PushNotification): Promise<NotificationResult> {
try {
const token = getAPNSToken();
const payload = this.buildPayload(notification);
const apnsToken = notification.data?.apnsToken as string || notification.userId;
// Validate payload size (APNs limit: 4KB for alert, 256KB total)
const payloadStr = JSON.stringify(payload);
const payloadSize = Buffer.byteLength(payloadStr, 'utf8');
if (payloadSize > 256 * 1024) {
return {
notificationId: `apns-${Date.now()}`,
channel: 'push',
status: 'failed',
error: `Payload size ${payloadSize} exceeds APNs limit of 256KB`,
};
}
const result = await this.sendToDevice(apnsToken, payload, token);
return {
notificationId: `apns-${Date.now()}`,
channel: 'push',
status: result.success ? 'sent' : 'failed',
externalId: result.responseId,
error: result.error,
deliveredAt: result.success ? new Date() : undefined,
};
} catch (error) {
return {
notificationId: `apns-${Date.now()}`,
channel: 'push',
status: 'failed',
error: error instanceof Error ? error.message : 'Unknown error',
};
}
}
private buildPayload(notification: PushNotification): APNsPayload {
const payload: APNsPayload = {
aps: {
alert: {
title: notification.title,
body: notification.body,
},
sound: notification.sound || 'default',
},
};
if (notification.badge !== undefined) {
payload.aps.badge = notification.badge;
}
if (notification.category) {
payload.aps.category = notification.category;
}
if (notification.data) {
Object.entries(notification.data).forEach(([key, value]) => {
if (key !== 'apnsToken') {
payload[key] = value;
}
});
}
return payload;
}
private sendToDevice(
deviceToken: string,
payload: APNsPayload,
authToken: string
): Promise<{ success: boolean; responseId?: string; error?: string }> {
return new Promise((resolve) => {
const url = `https://${this.host}:443/3/device/${deviceToken}`;
const options = {
hostname: this.host,
port: 443,
path: `/3/device/${deviceToken}`,
method: 'POST',
headers: {
'Authorization': `bearer ${authToken}`,
'Content-Type': 'application/json',
'apns-push-type': 'alert',
'apns-priority': '10',
'apns-topic': config.apns.bundleId,
},
};
const req = https.request(url, options, (res) => {
let responseData = '';
res.on('data', (chunk) => {
responseData += chunk;
});
res.on('end', () => {
if (res.statusCode === 200) {
resolve({
success: true,
responseId: res.headers['apns-id'] as string,
});
} else {
let error = `APNs error: ${res.statusCode}`;
try {
const errorBody = JSON.parse(responseData);
error = errorBody.reason || errorBody.message || error;
} catch {
// Keep default error message
}
resolve({
success: false,
error,
});
}
});
});
req.on('error', (error) => {
resolve({
success: false,
error: error.message,
});
});
req.write(JSON.stringify(payload));
req.end();
});
}
async sendBatch(notifications: PushNotification[]): Promise<NotificationResult[]> {
const results = await Promise.all(
notifications.map(n => this.send(n))
);
return results;
}
/**
* Test APNs connection by validating configuration and making a test request
*/
async testConnection(): Promise<{ success: boolean; message: string }> {
try {
// Validate required configuration
if (!config.apns.keyId || !config.apns.teamId || !config.apns.bundleId) {
return {
success: false,
message: 'Missing required APNs configuration: keyId, teamId, or bundleId',
};
}
if (!config.apns.key) {
return {
success: false,
message: 'Missing APNs private key',
};
}
// Generate a token to verify the key is valid
const token = getAPNSToken();
if (!token) {
return {
success: false,
message: 'Failed to generate APNs authentication token',
};
}
// Make a minimal test request to APNs to verify connection
return new Promise((resolve) => {
const testDeviceToken = '0000000000000000000000000000000000000000000000000000000000000000';
const url = `https://${this.host}:443/3/device/${testDeviceToken}`;
const options = {
hostname: this.host,
port: 443,
path: `/3/device/${testDeviceToken}`,
method: 'POST',
headers: {
'Authorization': `bearer ${token}`,
'Content-Type': 'application/json',
'apns-push-type': 'alert',
'apns-priority': '10',
'apns-topic': config.apns.bundleId,
},
timeout: 5000,
};
const req = https.request(url, options, (res) => {
// Any response means we connected successfully
// Even 410 (device token not registered) means connection works
if (res.statusCode && res.statusCode < 500) {
resolve({
success: true,
message: `APNs connection successful (status: ${res.statusCode})`,
});
} else {
resolve({
success: false,
message: `APNs connection failed with status: ${res.statusCode}`,
});
}
});
req.on('error', (error) => {
// Connection errors are expected if no real APNs credentials
resolve({
success: false,
message: `APNs connection error: ${error.message}`,
});
});
req.on('timeout', () => {
req.destroy();
resolve({
success: false,
message: 'APNs connection timeout',
});
});
// Send minimal payload
req.write(JSON.stringify({ aps: { sound: 'default' } }));
req.end();
});
} catch (error) {
return {
success: false,
message: error instanceof Error ? error.message : 'Unknown error',
};
}
}
}

View File

@@ -1,30 +1,145 @@
import admin from 'firebase-admin';
import { loadNotificationConfig } from '../config/notification.config';
import { APNSService } from './apns.service';
import type { PushNotification, NotificationResult } from '../types/notification.types';
const config = loadNotificationConfig();
let fcmApp: admin.app.App | null = null;
let fcmInitPromise: Promise<admin.app.App> | null = null;
function getFCMApp(): admin.app.App {
if (!fcmApp) {
fcmApp = admin.initializeApp({
credential: admin.credential.cert({
projectId: config.fcm.projectId,
clientEmail: config.fcm.clientEmail,
privateKey: config.fcm.privateKey.replace(/\\n/g, '\n'),
}),
});
if (fcmApp) {
return fcmApp;
}
// If already initializing, throw to prevent duplicate initialization
// (caller should use getFCMAppAsync for async-safe access)
if (fcmInitPromise && !fcmApp) {
throw new Error('Firebase initialization in progress');
}
// Check if any Firebase apps exist
const existingApps = admin.apps;
if (existingApps && existingApps.length > 0) {
const existingApp = existingApps.find((app: admin.app.App) => app.name === '[DEFAULT]');
if (existingApp) {
fcmApp = existingApp;
return fcmApp;
}
}
// Initialize new app
fcmApp = admin.initializeApp({
credential: admin.credential.cert({
projectId: config.fcm.projectId,
clientEmail: config.fcm.clientEmail,
privateKey: config.fcm.privateKey.replace(/\\n/g, '\n'),
}),
});
return fcmApp;
}
/**
* Async-safe version that prevents race conditions during initialization.
* Multiple concurrent calls will share the same initialization promise.
*/
function getFCMAppAsync(): Promise<admin.app.App> {
if (fcmApp) {
return Promise.resolve(fcmApp);
}
if (fcmInitPromise) {
return fcmInitPromise;
}
fcmInitPromise = (async () => {
try {
// Check if any Firebase apps exist
const existingApps = admin.apps;
if (existingApps && existingApps.length > 0) {
const existingApp = existingApps.find((app: admin.app.App) => app.name === '[DEFAULT]');
if (existingApp) {
fcmApp = existingApp;
return fcmApp;
}
}
fcmApp = admin.initializeApp({
credential: admin.credential.cert({
projectId: config.fcm.projectId,
clientEmail: config.fcm.clientEmail,
privateKey: config.fcm.privateKey.replace(/\\n/g, '\n'),
}),
});
return fcmApp;
} finally {
fcmInitPromise = null;
}
})();
return fcmInitPromise;
}
// Retry configuration for transient failures
const RETRY_CONFIG = {
maxRetries: 3,
baseDelayMs: 1000,
maxDelayMs: 5000,
retryableErrors: [
'UNAVAILABLE',
'DEADLINE_EXCEEDED',
'RESOURCE_EXHAUSTED',
'INTERNAL',
],
};
function isRetryableError(error: unknown): boolean {
if (error instanceof Error) {
const message = error.message.toUpperCase();
return RETRY_CONFIG.retryableErrors.some(re => message.includes(re));
}
return false;
}
async function withRetry<T>(
fn: () => Promise<T>,
maxRetries: number = RETRY_CONFIG.maxRetries
): Promise<T> {
let lastError: Error | null = null;
for (let attempt = 0; attempt <= maxRetries; attempt++) {
try {
return await fn();
} catch (error) {
lastError = error instanceof Error ? error : new Error(String(error));
if (attempt < maxRetries && isRetryableError(error)) {
const delay = Math.min(
RETRY_CONFIG.baseDelayMs * Math.pow(2, attempt),
RETRY_CONFIG.maxDelayMs
);
console.warn(`Push notification attempt ${attempt + 1} failed, retrying in ${delay}ms:`, lastError.message);
await new Promise(resolve => setTimeout(resolve, delay));
} else {
throw lastError;
}
}
}
throw lastError!;
}
export class PushService {
private static instance: PushService;
private sentCount = new Map<string, number>();
private sentDedup = new Map<string, number>();
private apnsService: APNSService;
private cleanupInterval: NodeJS.Timeout;
private constructor() {
this.apnsService = APNSService.getInstance();
this.cleanupInterval = setInterval(() => {
const now = Date.now();
for (const [key, timestamp] of this.sentCount.entries()) {
@@ -32,6 +147,11 @@ export class PushService {
this.sentCount.delete(key);
}
}
for (const [key, timestamp] of this.sentDedup.entries()) {
if (now - timestamp > config.redis.dedupWindowSeconds * 1000) {
this.sentDedup.delete(key);
}
}
}, 60000);
}
@@ -42,6 +162,10 @@ export class PushService {
return PushService.instance;
}
/**
* Send push notification using the appropriate service
* Uses explicit platform from notification data instead of heuristic detection
*/
async send(notification: PushNotification): Promise<NotificationResult> {
const rateLimitKey = `push:${notification.userId}`;
const currentCount = this.sentCount.get(rateLimitKey) || 0;
@@ -50,43 +174,63 @@ export class PushService {
throw new Error(`Push rate limit exceeded for user ${notification.userId}`);
}
try {
const fcmApp = getFCMApp();
const messaging = admin.messaging(fcmApp);
const message: admin.messaging.Message = {
notification: {
title: notification.title,
body: notification.body,
},
data: notification.data ?
Object.fromEntries(
Object.entries(notification.data).map(([k, v]) => [k, String(v)])
) : undefined,
token: notification.userId,
apns: {
payload: {
aps: {
badge: notification.badge,
sound: notification.sound || 'default',
category: notification.category,
},
},
},
};
const response = await messaging.send(message);
this.sentCount.set(rateLimitKey, currentCount + 1);
// Deduplication: skip if identical notification was sent recently
const dedupKey = `${notification.userId}:${notification.title}:${notification.body}`;
const lastSent = this.sentDedup.get(dedupKey);
if (lastSent && Date.now() - lastSent < config.redis.dedupWindowSeconds * 1000) {
return {
notificationId: `push-${response}`,
notificationId: `push-dedup-${Date.now()}`,
channel: 'push',
status: 'sent',
externalId: response,
deliveredAt: new Date(),
status: 'skipped',
error: 'Duplicate notification within dedup window',
};
}
try {
// Get the platform explicitly from notification data
const platform = notification.data?.platform as 'ios' | 'android' | undefined;
if (!platform) {
return {
notificationId: `push-${Date.now()}`,
channel: 'push',
status: 'failed',
error: 'Platform not specified in notification data',
};
}
if (platform === 'ios') {
// Use APNs for iOS
const apnsToken = notification.data?.apnsToken as string;
if (!apnsToken) {
return {
notificationId: `push-${Date.now()}`,
channel: 'push',
status: 'failed',
error: 'APNs token not provided for iOS notification',
};
}
// Preserve userId and pass apnsToken separately to APNS service
const apnsResult = await this.apnsService.send(notification);
if (apnsResult.status === 'sent') {
const apnsDedupKey = `${notification.userId}:${notification.title}:${notification.body}`;
this.sentDedup.set(apnsDedupKey, Date.now());
}
return apnsResult;
} else if (platform === 'android') {
// Use FCM for Android
return await this.sendViaFCM(notification);
} else {
return {
notificationId: `push-${Date.now()}`,
channel: 'push',
status: 'failed',
error: `Unsupported platform: ${platform}`,
};
}
} catch (error) {
console.error('Push notification failed:', error);
return {
notificationId: `push-${Date.now()}`,
channel: 'push',
@@ -96,6 +240,100 @@ export class PushService {
}
}
/**
* Send notification via Firebase Cloud Messaging (Android)
*/
private async sendViaFCM(notification: PushNotification): Promise<NotificationResult> {
try {
const fcmApp = await getFCMAppAsync();
const messaging = admin.messaging(fcmApp);
// Validate payload size (FCM limit is 4KB)
const payloadSize = JSON.stringify(notification).length;
if (payloadSize > 4096) {
return {
notificationId: `fcm-${Date.now()}`,
channel: 'push',
status: 'failed',
error: `Payload size ${payloadSize} exceeds FCM limit of 4KB`,
};
}
const message: admin.messaging.Message = {
notification: {
title: notification.title,
body: notification.body,
},
data: notification.data ?
Object.fromEntries(
Object.entries(notification.data).map(([k, v]) => [k, String(v)])
) : undefined,
token: notification.data?.fcmToken as string,
};
// Use retry logic for transient failures
const response = await withRetry(() => messaging.send(message));
this.sentCount.set(`push:${notification.userId}`,
(this.sentCount.get(`push:${notification.userId}`) || 0) + 1);
// Track for deduplication
const dedupKey = `${notification.userId}:${notification.title}:${notification.body}`;
this.sentDedup.set(dedupKey, Date.now());
return {
notificationId: `fcm-${response}`,
channel: 'push',
status: 'sent',
externalId: response,
deliveredAt: new Date(),
};
} catch (error) {
console.error('FCM send failed:', error);
// Handle specific FCM errors
if (error instanceof admin.messaging.MessagingError) {
const fcmCode = error.code;
// Non-retryable errors - invalid token, unregistered device
if (fcmCode === 'messaging/invalid-registration-token' ||
fcmCode === 'messaging/registration-token-not-registered' ||
fcmCode === 'messaging/invalid-argument') {
return {
notificationId: `fcm-${Date.now()}`,
channel: 'push',
status: 'failed',
error: `Invalid FCM token: ${error.message}`,
};
}
}
return {
notificationId: `fcm-${Date.now()}`,
channel: 'push',
status: 'failed',
error: error instanceof Error ? error.message : 'Unknown error',
};
}
}
/**
* Send notification specifically via FCM (for Android devices)
*/
async sendViaFCMOnly(notification: PushNotification): Promise<NotificationResult> {
return this.sendViaFCM(notification);
}
/**
* Send notification specifically via APNs (for iOS devices)
*/
async sendViaAPNS(notification: PushNotification): Promise<NotificationResult> {
return this.apnsService.send(notification);
}
/**
* Send batch notifications with retry logic
*/
async sendBatch(notifications: PushNotification[]): Promise<NotificationResult[]> {
const results = await Promise.all(
notifications.map(n => this.send(n))
@@ -103,10 +341,53 @@ export class PushService {
return results;
}
/**
* Get rate limit status
*/
getRateLimitStatus(): { remaining: number; limit: number } {
return {
remaining: config.rateLimits.pushPerMinute - this.sentCount.size,
limit: config.rateLimits.pushPerMinute,
};
}
/**
* Test connection to FCM by verifying app initialization
*/
async testFCMConnection(): Promise<{ success: boolean; message: string }> {
try {
const fcmApp = await getFCMAppAsync();
if (!fcmApp) {
return {
success: false,
message: 'Firebase app not initialized',
};
}
return { success: true, message: 'FCM connection successful' };
} catch (error) {
console.error('FCM connection test failed:', error);
return {
success: false,
message: error instanceof Error ? error.message : 'Unknown error',
};
}
}
/**
* Test connection to APNs by making a real connection test
*/
async testAPNSConnection(): Promise<{ success: boolean; message: string }> {
try {
// Use the APNSService to test actual connection
return await this.apnsService.testConnection();
} catch (error) {
console.error('APNs connection test failed:', error);
return {
success: false,
message: error instanceof Error ? error.message : 'Unknown error'
};
}
}
}