Re-activate tomstoned subscriptions when email is the same
This commit is contained in:
parent
3a7d0d110a
commit
fd815282c2
4 changed files with 341 additions and 0 deletions
|
|
@ -152,6 +152,25 @@ export const updateSubscription = instrument(
|
||||||
}
|
}
|
||||||
)
|
)
|
||||||
|
|
||||||
|
const getMostRecentTombstonedSubscription = async (pubkey: string) =>
|
||||||
|
parseSubscription(
|
||||||
|
await get(
|
||||||
|
`SELECT * FROM subscriptions
|
||||||
|
WHERE pubkey = ? AND unsubscribed_at IS NOT NULL
|
||||||
|
ORDER BY created_at DESC, id DESC LIMIT 1`,
|
||||||
|
[pubkey]
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
const reactivateSubscription = async (tombstoned: Subscription, frequency: string) =>
|
||||||
|
parseSubscription(
|
||||||
|
await get(
|
||||||
|
`UPDATE subscriptions SET key = ?, frequency = ?, unsubscribed_at = NULL
|
||||||
|
WHERE id = ? RETURNING *`,
|
||||||
|
[crypto.randomBytes(32).toString('hex'), frequency, tombstoned.id]
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
export const insertSubscription = instrument(
|
export const insertSubscription = instrument(
|
||||||
'database.insertSubscription',
|
'database.insertSubscription',
|
||||||
async (pubkey: string, email: string, frequency: string) => {
|
async (pubkey: string, email: string, frequency: string) => {
|
||||||
|
|
@ -161,6 +180,28 @@ export const insertSubscription = instrument(
|
||||||
return assertResult(await updateSubscription(existing, email, frequency))
|
return assertResult(await updateSubscription(existing, email, frequency))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// No active row. If this account previously subscribed to the same email
|
||||||
|
// and was unsubscribed, reactivate that row instead of inserting a fresh
|
||||||
|
// one. Otherwise every turn-on → turn-off → turn-on cycle would create a
|
||||||
|
// new row, abandoning the confirmed state (and the already-known key).
|
||||||
|
const tombstoned = await getMostRecentTombstonedSubscription(pubkey)
|
||||||
|
|
||||||
|
if (tombstoned?.email === email) {
|
||||||
|
try {
|
||||||
|
return assertResult(await reactivateSubscription(tombstoned, frequency))
|
||||||
|
} catch (err: any) {
|
||||||
|
if (err.message?.includes('UNIQUE constraint')) {
|
||||||
|
const concurrent = await getSubscriptionByPubkey(pubkey)
|
||||||
|
|
||||||
|
if (concurrent) {
|
||||||
|
return assertResult(await updateSubscription(concurrent, email, frequency))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
throw err
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
try {
|
try {
|
||||||
return assertResult(
|
return assertResult(
|
||||||
parseSubscription(
|
parseSubscription(
|
||||||
|
|
@ -276,6 +317,20 @@ export const getActiveSubscriptions = instrument('database.getActiveSubscription
|
||||||
return rows.map(parseSubscription) as Subscription[]
|
return rows.map(parseSubscription) as Subscription[]
|
||||||
})
|
})
|
||||||
|
|
||||||
|
// Every row ever created for a pubkey — both active and tombstoned. Exposed
|
||||||
|
// primarily for tests asserting re-subscribe never grows the table.
|
||||||
|
export const getAllSubscriptionsByPubkey = instrument(
|
||||||
|
'database.getAllSubscriptionsByPubkey',
|
||||||
|
async (pubkey: string) => {
|
||||||
|
const rows = await all<Row>(
|
||||||
|
`SELECT * FROM subscriptions WHERE pubkey = ? ORDER BY created_at`,
|
||||||
|
[pubkey]
|
||||||
|
)
|
||||||
|
|
||||||
|
return rows.map(parseSubscription) as Subscription[]
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
export const updateLastDigestAt = instrument(
|
export const updateLastDigestAt = instrument(
|
||||||
'database.updateLastDigestAt',
|
'database.updateLastDigestAt',
|
||||||
async (id: string, timestamp: number) => {
|
async (id: string, timestamp: number) => {
|
||||||
|
|
|
||||||
109
test/confirm-code-on-email-change.test.ts
Normal file
109
test/confirm-code-on-email-change.test.ts
Normal file
|
|
@ -0,0 +1,109 @@
|
||||||
|
import { describe, it, expect, beforeAll, afterAll, vi } from 'vitest'
|
||||||
|
import * as db from '../src/database.js'
|
||||||
|
import { registerSubscription } from '../src/actions.js'
|
||||||
|
import * as mailer from '../src/mailer.js'
|
||||||
|
|
||||||
|
// Regression guards for the row-reactivation fix (src/database.ts):
|
||||||
|
// * same-email re-subscribe → reactivates the SAME row and must NOT email a
|
||||||
|
// stale confirmation code to an unrelated address
|
||||||
|
// * new-email re-subscribe → MUST create a fresh row (fresh key) so the
|
||||||
|
// confirmation email carries a code bound to the new address
|
||||||
|
//
|
||||||
|
// We mock the mailer so these run hermetically (the unit env's SMTP is a dummy).
|
||||||
|
|
||||||
|
vi.mock('../src/mailer.js', async (importOriginal) => {
|
||||||
|
const actual: any = await importOriginal()
|
||||||
|
return { ...actual, sendConfirm: vi.fn(async () => {}) }
|
||||||
|
})
|
||||||
|
|
||||||
|
const pubkey = 'new-email-' + Date.now() + '-' + Math.random().toString(36).slice(2)
|
||||||
|
const oldEmail = `${pubkey}-old@example.com`
|
||||||
|
const newEmail = `${pubkey}-new@example.com`
|
||||||
|
let subscribedIds: string[] = []
|
||||||
|
|
||||||
|
const subscribeIdsFor = async () => {
|
||||||
|
const rows = await db.getAllSubscriptionsByPubkey(pubkey)
|
||||||
|
subscribedIds = rows.map((r: any) => r.id)
|
||||||
|
return rows
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('Re-subscribe with a NEW email dispatches the correct confirmation code', () => {
|
||||||
|
beforeAll(async () => {
|
||||||
|
await db.migrate()
|
||||||
|
vi.mocked(mailer.sendConfirm).mockClear()
|
||||||
|
})
|
||||||
|
|
||||||
|
afterAll(async () => {
|
||||||
|
const key = (await db.getAllSubscriptionsByPubkey(pubkey))[0]?.key
|
||||||
|
if (key) await db.unsubscribeSubscription(key)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('registers + confirms the original email', async () => {
|
||||||
|
const result = await registerSubscription({ pubkey, email: oldEmail, frequency: 'daily' })
|
||||||
|
expect(result.key).toBeTruthy()
|
||||||
|
await db.confirmSubscription(result.key)
|
||||||
|
expect(vi.mocked(mailer.sendConfirm)).toHaveBeenCalledTimes(1)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('unsubscribes (DELETE flow)', async () => {
|
||||||
|
const active = await db.getSubscriptionByPubkey(pubkey)
|
||||||
|
await db.unsubscribeSubscription(active!.key)
|
||||||
|
expect(await db.getSubscriptionByPubkey(pubkey)).toBeFalsy()
|
||||||
|
})
|
||||||
|
|
||||||
|
it('re-registering with the NEW email creates a fresh row, not a reactivation', async () => {
|
||||||
|
vi.mocked(mailer.sendConfirm).mockClear()
|
||||||
|
|
||||||
|
const result = await registerSubscription({ pubkey, email: newEmail, frequency: 'daily' })
|
||||||
|
|
||||||
|
// A NEW confirmation email was sent — to the NEW address, with the key
|
||||||
|
// returned to the caller (which the caller uses to confirm).
|
||||||
|
const sent = vi.mocked(mailer.sendConfirm).mock.calls[0][0]
|
||||||
|
expect(sent.email).toBe(newEmail)
|
||||||
|
expect(sent.key).toBe(result.key)
|
||||||
|
|
||||||
|
// And that key actually confirms the new-email row (not the old one).
|
||||||
|
const confirmed = await db.confirmSubscription(result.key)
|
||||||
|
expect(confirmed!.sub.email).toBe(newEmail)
|
||||||
|
expect(confirmed!.alreadyConfirmed).toBe(false)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('leaves the old row and new row as distinct rows', async () => {
|
||||||
|
const rows = await subscribeIdsFor()
|
||||||
|
expect(rows).toHaveLength(2)
|
||||||
|
expect(new Set(subscribedIds).size).toBe(2)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
describe('Reactivating the SAME email never emails an unrelated address', () => {
|
||||||
|
const k = 'same-email-' + Date.now() + '-' + Math.random().toString(36).slice(2)
|
||||||
|
const email = `${k}@example.com`
|
||||||
|
|
||||||
|
beforeAll(async () => {
|
||||||
|
vi.mocked(mailer.sendConfirm).mockClear()
|
||||||
|
})
|
||||||
|
|
||||||
|
afterAll(async () => {
|
||||||
|
const key = (await db.getAllSubscriptionsByPubkey(k))[0]?.key
|
||||||
|
if (key) await db.unsubscribeSubscription(key)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('register + confirm + unsubscribe, then re-register the same email', async () => {
|
||||||
|
const r1 = await registerSubscription({ pubkey: k, email, frequency: 'daily' })
|
||||||
|
await db.confirmSubscription(r1.key)
|
||||||
|
const row1 = (await db.getAllSubscriptionsByPubkey(k))[0]
|
||||||
|
await db.unsubscribeSubscription(r1.key)
|
||||||
|
|
||||||
|
vi.mocked(mailer.sendConfirm).mockClear()
|
||||||
|
const r2 = await registerSubscription({ pubkey: k, email, frequency: 'daily' })
|
||||||
|
|
||||||
|
// Same row, still confirmed, but a FRESH key is issued — no new
|
||||||
|
// confirmation email, and old tokens for this row are invalidated.
|
||||||
|
const rows = await db.getAllSubscriptionsByPubkey(k)
|
||||||
|
expect(rows).toHaveLength(1)
|
||||||
|
expect(r2.key).not.toBe(r1.key)
|
||||||
|
expect(rows[0].id).toBe(row1.id)
|
||||||
|
expect(rows[0].confirmed_at).toBeTruthy()
|
||||||
|
expect(vi.mocked(mailer.sendConfirm)).not.toHaveBeenCalled()
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
@ -266,6 +266,31 @@ echo "14. Delete without auth returns 401"
|
||||||
DEL_NO_AUTH=$(curl -s "$BASE_URL/subscription/$KEY" -X DELETE)
|
DEL_NO_AUTH=$(curl -s "$BASE_URL/subscription/$KEY" -X DELETE)
|
||||||
check_field "No-auth DELETE returns 401" "$DEL_NO_AUTH" "error" "NIP-98 authorization required"
|
check_field "No-auth DELETE returns 401" "$DEL_NO_AUTH" "error" "NIP-98 authorization required"
|
||||||
|
|
||||||
|
# Test 15: Re-subscribe after delete reactivates the same row (no duplicate)
|
||||||
|
# Regression: DELETE tombstones the row; re-subscribing with the SAME pubkey +
|
||||||
|
# email must reactivate it, not insert a second row (off/on cycle previously
|
||||||
|
# accumulated one row per cycle).
|
||||||
|
echo ""
|
||||||
|
echo "15. Re-subscribe after delete (de-dupe regression)"
|
||||||
|
OLD_ID=$(sqlite3 "$DB_PATH" "SELECT id FROM subscriptions WHERE pubkey='$CLIENT_PUBKEY' AND unsubscribed_at IS NOT NULL ORDER BY created_at DESC LIMIT 1;" 2>/dev/null)
|
||||||
|
AUTH_RE=$(nip98_auth "$BASE_URL/subscription/email" PUT '{"email":"test@example.com","frequency":"daily"}')
|
||||||
|
RE_REG=$(curl -s "$BASE_URL/subscription/email" -X PUT -H "Content-Type: application/json" \
|
||||||
|
-H "Authorization: $AUTH_RE" \
|
||||||
|
-d '{"email":"test@example.com","frequency":"daily"}')
|
||||||
|
NEW_ID=$(sqlite3 "$DB_PATH" "SELECT id FROM subscriptions WHERE pubkey='$CLIENT_PUBKEY' AND unsubscribed_at IS NULL LIMIT 1;" 2>/dev/null)
|
||||||
|
if [ -n "$OLD_ID" ] && [ "$NEW_ID" = "$OLD_ID" ]; then
|
||||||
|
pass "Re-subscribe reactivates the same row"
|
||||||
|
else
|
||||||
|
fail "Re-subscribe created a duplicate row (old=$OLD_ID, new=$NEW_ID)"
|
||||||
|
fi
|
||||||
|
|
||||||
|
TOTAL_ROWS=$(sqlite3 "$DB_PATH" "SELECT COUNT(*) FROM subscriptions WHERE pubkey='$CLIENT_PUBKEY';" 2>/dev/null)
|
||||||
|
if [ "$TOTAL_ROWS" = "1" ]; then
|
||||||
|
pass "Exactly one row for this pubkey"
|
||||||
|
else
|
||||||
|
fail "Expected 1 row for this pubkey, got $TOTAL_ROWS"
|
||||||
|
fi
|
||||||
|
|
||||||
# Summary
|
# Summary
|
||||||
echo ""
|
echo ""
|
||||||
echo "========================================="
|
echo "========================================="
|
||||||
|
|
|
||||||
152
test/resubscribe-reactivates.test.ts
Normal file
152
test/resubscribe-reactivates.test.ts
Normal file
|
|
@ -0,0 +1,152 @@
|
||||||
|
import { describe, it, expect, beforeAll } from 'vitest'
|
||||||
|
import * as db from '../src/database.js'
|
||||||
|
|
||||||
|
// Regression test for the "two rows created when turning email alerts back on" bug.
|
||||||
|
//
|
||||||
|
// Decoded production sequence (Sep 18 2026):
|
||||||
|
// 13:42:05 register → row 1 (unconfirmed), confirm email #1
|
||||||
|
// 13:44:35 DELETE → row 1 tombstoned (unsubscribed_at set)
|
||||||
|
// 13:44:36 register → OLD BUG: a brand-new row 2 was inserted, confirm email #2
|
||||||
|
// 14:02:32 confirm → row 2 finally confirmed
|
||||||
|
//
|
||||||
|
// Fix: when the same pubkey re-subscribes to the same email after being
|
||||||
|
// unsubscribed, reactivate the tombstoned row instead of inserting a new one,
|
||||||
|
// preserving the existing confirmed state and key.
|
||||||
|
|
||||||
|
const unique = (label: string) => `${label}-${Date.now()}-${Math.random().toString(36).slice(2)}`
|
||||||
|
|
||||||
|
describe('Re-subscribe after DELETE reactivates the same subscription (off/on cycle)', () => {
|
||||||
|
const pubkey = unique('resub')
|
||||||
|
const email = `${unique('resub')}@example.com`
|
||||||
|
|
||||||
|
beforeAll(async () => {
|
||||||
|
await db.migrate()
|
||||||
|
})
|
||||||
|
|
||||||
|
it('creates an unconfirmed subscription', async () => {
|
||||||
|
const sub = await db.insertSubscription(pubkey, email, 'daily')
|
||||||
|
expect(sub).toBeTruthy()
|
||||||
|
expect(sub.confirmed_at).toBeFalsy()
|
||||||
|
expect(sub.unsubscribed_at).toBeFalsy()
|
||||||
|
})
|
||||||
|
|
||||||
|
it('confirms it (user clicks the confirm link)', async () => {
|
||||||
|
const active = await db.getSubscriptionByPubkey(pubkey)
|
||||||
|
const result = await db.confirmSubscription(active!.key)
|
||||||
|
expect(result).toBeTruthy()
|
||||||
|
expect(result!.alreadyConfirmed).toBe(false)
|
||||||
|
expect(result!.sub.confirmed_at).toBeTruthy()
|
||||||
|
})
|
||||||
|
|
||||||
|
it('unsubscribes it (the flotilla DELETE path)', async () => {
|
||||||
|
const active = await db.getSubscriptionByPubkey(pubkey)
|
||||||
|
const gone = await db.unsubscribeSubscription(active!.key)
|
||||||
|
expect(gone).toBeTruthy()
|
||||||
|
expect(gone!.unsubscribed_at).toBeTruthy()
|
||||||
|
expect(await db.getSubscriptionByPubkey(pubkey)).toBeFalsy()
|
||||||
|
})
|
||||||
|
|
||||||
|
it('re-registers the SAME email — must reactivate row, NOT create a second row', async () => {
|
||||||
|
const resigned = await db.insertSubscription(pubkey, email, 'daily')
|
||||||
|
|
||||||
|
// Restores the very same row
|
||||||
|
const active = await db.getSubscriptionByPubkey(pubkey)
|
||||||
|
expect(active).toBeTruthy()
|
||||||
|
expect(active!.key).toBe(resigned.key)
|
||||||
|
expect(active!.unsubscribed_at).toBeFalsy()
|
||||||
|
|
||||||
|
// Confirmed state is preserved — no second confirmation email needed
|
||||||
|
expect(active!.confirmed_at).toBeTruthy()
|
||||||
|
|
||||||
|
// Exhaustive: there exists exactly one row total for this pubkey
|
||||||
|
const rows = await db.getAllSubscriptionsByPubkey(pubkey)
|
||||||
|
expect(rows).toHaveLength(1)
|
||||||
|
expect(rows[0].id).toBe(active!.id)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
describe('Re-subscribe after DELETE before ever confirming', () => {
|
||||||
|
const pubkey = unique('resub-unconfirmed')
|
||||||
|
const email = `${unique('resub-unconfirmed')}@example.com`
|
||||||
|
|
||||||
|
beforeAll(async () => {
|
||||||
|
await db.migrate()
|
||||||
|
})
|
||||||
|
|
||||||
|
it('registers then unsubscribes without confirming', async () => {
|
||||||
|
const sub = await db.insertSubscription(pubkey, email, 'daily')
|
||||||
|
await db.unsubscribeSubscription(sub.key)
|
||||||
|
expect(await db.getSubscriptionByPubkey(pubkey)).toBeFalsy()
|
||||||
|
})
|
||||||
|
|
||||||
|
it('re-registers the same email — reactivates same row, still unconfirmed', async () => {
|
||||||
|
const resigned = await db.insertSubscription(pubkey, email, 'daily')
|
||||||
|
const active = await db.getSubscriptionByPubkey(pubkey)
|
||||||
|
|
||||||
|
expect(active!.key).toBe(resigned.key)
|
||||||
|
expect(active!.id).toBe(resigned.id)
|
||||||
|
expect(active!.unsubscribed_at).toBeFalsy()
|
||||||
|
// Was never confirmed, and re-subscribing does not skip confirmation
|
||||||
|
expect(active!.confirmed_at).toBeFalsy()
|
||||||
|
|
||||||
|
const rows = await db.getAllSubscriptionsByPubkey(pubkey)
|
||||||
|
expect(rows).toHaveLength(1)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
describe('Re-subscribe with a DIFFERENT email after DELETE', () => {
|
||||||
|
const pubkey = unique('resub-2')
|
||||||
|
const email = `${unique('resub-2')}@example.com`
|
||||||
|
|
||||||
|
beforeAll(async () => {
|
||||||
|
await db.migrate()
|
||||||
|
})
|
||||||
|
|
||||||
|
it('registers, confirms, and unsubscribes', async () => {
|
||||||
|
const sub = await db.insertSubscription(pubkey, email, 'daily')
|
||||||
|
await db.confirmSubscription(sub.key)
|
||||||
|
await db.unsubscribeSubscription(sub.key)
|
||||||
|
expect(await db.getSubscriptionByPubkey(pubkey)).toBeFalsy()
|
||||||
|
})
|
||||||
|
|
||||||
|
it('a new email creates a new row, leaving the old one tombstoned', async () => {
|
||||||
|
const newEmail = `${unique('resub-2-new')}@example.com`
|
||||||
|
const resigned = await db.insertSubscription(pubkey, newEmail, 'daily')
|
||||||
|
|
||||||
|
expect(resigned.email).toBe(newEmail)
|
||||||
|
expect(resigned.confirmed_at).toBeFalsy() // new address must re-confirm
|
||||||
|
|
||||||
|
const rows = await db.getAllSubscriptionsByPubkey(pubkey)
|
||||||
|
expect(rows).toHaveLength(2)
|
||||||
|
expect(rows[0].email).toBe(email) // old, tombstoned
|
||||||
|
expect(rows[0].unsubscribed_at).toBeTruthy()
|
||||||
|
expect(rows[1].email).toBe(newEmail) // new, active
|
||||||
|
expect(rows[1].unsubscribed_at).toBeFalsy()
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
describe('Re-subscribe with a changed frequency reactivates and updates cadence', () => {
|
||||||
|
const pubkey = unique('resub-freq')
|
||||||
|
const email = `${unique('resub-freq')}@example.com`
|
||||||
|
|
||||||
|
beforeAll(async () => {
|
||||||
|
await db.migrate()
|
||||||
|
})
|
||||||
|
|
||||||
|
it('registers confirm-free, unsubscribes', async () => {
|
||||||
|
const sub = await db.insertSubscription(pubkey, email, 'daily')
|
||||||
|
await db.unsubscribeSubscription(sub.key)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('re-registers with weekly — reactivates the same row at the new cadence', async () => {
|
||||||
|
const resigned = await db.insertSubscription(pubkey, email, 'weekly')
|
||||||
|
const active = await db.getSubscriptionByPubkey(pubkey)
|
||||||
|
|
||||||
|
expect(active!.key).toBe(resigned.key)
|
||||||
|
expect(active!.frequency).toBe('weekly')
|
||||||
|
expect(active!.unsubscribed_at).toBeFalsy()
|
||||||
|
|
||||||
|
const rows = await db.getAllSubscriptionsByPubkey(pubkey)
|
||||||
|
expect(rows).toHaveLength(1)
|
||||||
|
})
|
||||||
|
})
|
||||||
Loading…
Reference in a new issue