security: fix webhook signature fail-open (now throws 500 on missing key), fix overly restrictive couple update rules
This commit is contained in:
parent
50c44d2afd
commit
b8b2cc68c4
|
|
@ -125,16 +125,12 @@ service cloud.firestore {
|
||||||
// - user IDs are immutable (cannot change who is in the couple)
|
// - user IDs are immutable (cannot change who is in the couple)
|
||||||
// - invite code is immutable (cannot change the code)
|
// - invite code is immutable (cannot change the code)
|
||||||
// - createdAt is immutable (cannot change when the couple was formed)
|
// - createdAt is immutable (cannot change when the couple was formed)
|
||||||
// - currentQuestionId: either member can set (either can pick a question)
|
|
||||||
// - streakCount and lastStreakAt: server-only (via Cloud Functions or admin SDK)
|
// - streakCount and lastStreakAt: server-only (via Cloud Functions or admin SDK)
|
||||||
// - Any other fields: both members can update normally
|
// - All other fields: both members can update normally
|
||||||
allow update: if isCouplesMember(coupleId)
|
allow update: if isCouplesMember(coupleId)
|
||||||
// Check immutable fields haven't changed
|
// Check immutable fields haven't changed
|
||||||
&& isImmutable(['userIds', 'inviteCode', 'createdAt'])
|
&& isImmutable(['userIds', 'inviteCode', 'createdAt'])
|
||||||
// Allow currentQuestionId updates
|
// streakCount and lastStreakAt must not be modified by clients
|
||||||
&& request.resource.data.diff(resource.data).affectedKeys().hasOnly(['currentQuestionId'])
|
|
||||||
// No other fields should be changed
|
|
||||||
// Check that streakCount and lastStreakAt are not in the update
|
|
||||||
&& !request.resource.data.diff(resource.data).affectedKeys().hasAny(['streakCount', 'lastStreakAt']);
|
&& !request.resource.data.diff(resource.data).affectedKeys().hasAny(['streakCount', 'lastStreakAt']);
|
||||||
|
|
||||||
// Delete: server-only (admin SDK only)
|
// Delete: server-only (admin SDK only)
|
||||||
|
|
|
||||||
|
|
@ -9,15 +9,15 @@ const router = Router()
|
||||||
/**
|
/**
|
||||||
* Verifies RevenueCat webhook signature using Ed25519.
|
* Verifies RevenueCat webhook signature using Ed25519.
|
||||||
* RevenueCat signs the raw request body with their signing key.
|
* RevenueCat signs the raw request body with their signing key.
|
||||||
* If REVENUECAT_SIGNING_KEY is not set, logs warning but skips verification (dev mode).
|
* If REVENUECAT_SIGNING_KEY is not set, throws an error (fail-closed).
|
||||||
*/
|
*/
|
||||||
function verifyRevenueCatSignature(req: Request): boolean {
|
function verifyRevenueCatSignature(req: Request): boolean {
|
||||||
const signingKey = process.env.REVENUECAT_SIGNING_KEY
|
const signingKey = process.env.REVENUECAT_SIGNING_KEY
|
||||||
|
|
||||||
// If signing key is not configured, skip verification (development mode)
|
// If signing key is not configured, fail closed (not dev mode)
|
||||||
if (!signingKey || signingKey.trim() === '') {
|
if (!signingKey || signingKey.trim() === '') {
|
||||||
console.warn('[webhook] REVENUECAT_SIGNING_KEY not set — skipping signature verification')
|
console.error('[webhook] REVENUECAT_SIGNING_KEY not set — rejecting all requests (fail-closed)')
|
||||||
return true
|
throw new Error('REVENUECAT_SIGNING_KEY must be set in production')
|
||||||
}
|
}
|
||||||
|
|
||||||
const signatureHeader = req.headers['x-revenuecat-signature']
|
const signatureHeader = req.headers['x-revenuecat-signature']
|
||||||
|
|
@ -76,16 +76,16 @@ function verifyRevenueCatSecret(req: Request): boolean {
|
||||||
}
|
}
|
||||||
|
|
||||||
router.post('/revenuecat', async (req: Request, res: Response) => {
|
router.post('/revenuecat', async (req: Request, res: Response) => {
|
||||||
// Try signature verification first (modern)
|
// Try signature verification first (modern, prefered)
|
||||||
const signatureValid = verifyRevenueCatSignature(req)
|
try {
|
||||||
if (!signatureValid) {
|
if (!verifyRevenueCatSignature(req)) {
|
||||||
res.status(401).json({ error: 'unauthorized' })
|
res.status(401).json({ error: 'unauthorized' })
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
} catch (err: any) {
|
||||||
// Fallback to secret verification if no signature (legacy)
|
// If signature verification throws (e.g., missing key), reject with 500
|
||||||
if (!signatureValid && !verifyRevenueCatSecret(req)) {
|
console.error('[webhook] Signature verification error:', err)
|
||||||
res.status(401).json({ error: 'unauthorized' })
|
res.status(500).json({ error: 'signature_verification_failed', message: err.message })
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue