Skip to content

Commit ea384b6

Browse files
claudeclaude
committed
fix(webapp): recover push subscriptions and clear them on sign-out
A device the server switched off stayed off for up to 30 days, and the 30-day refresh could destroy a working subscription. The client now saves the subscription at most once a day without unsubscribing, repairs a granted but missing subscription, and subscribes again on a VAPID key change. Sign-out unregisters the device first. The service worker handles pushsubscriptionchange, always shows a notification, titles every server type, and opens same-origin URLs only. Refs #417 Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_0127SE8ZfVh4rcGP7nTDaWNF
1 parent da0d832 commit ea384b6

4 files changed

Lines changed: 153 additions & 78 deletions

File tree

‎apps/webapp/public/service-worker.js‎

Lines changed: 65 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,18 @@ function buildNotificationTitle(type, senderName) {
9595
return `Message from ${name}`;
9696
case "thread_message":
9797
return `${name} replied in thread`;
98+
case "message":
99+
case "channel_event":
98100
case "channel_message":
99101
return `${name} sent a message`;
102+
case "invitation":
100103
case "invite":
101104
return `${name} invited you`;
105+
// These two can come with no sender, so the title leaves the name out.
106+
case "content_change":
107+
return "A document you follow changed";
108+
case "system_alert":
109+
return "Account notice";
102110
default:
103111
return "New notification";
104112
}
@@ -137,53 +145,74 @@ function buildNotificationBody(data) {
137145

138146
// Handle incoming push notifications
139147
self.addEventListener("push", (event) => {
140-
if (!event.data) {
141-
return;
142-
}
148+
// Chrome and Android can revoke the subscription when a push shows nothing,
149+
// so a bad or empty payload still shows this generic notification.
150+
let title = "New notification";
151+
let options = {
152+
icon: "/icons/android-chrome-192x192.png",
153+
badge: "/icons/favicon-32x32.png",
154+
data: { url: "/" },
155+
};
143156

144-
let data;
145157
try {
146-
data = event.data.json();
147-
} catch (e) {
148-
data = { type: "unknown", message_preview: event.data.text() };
158+
const data = event.data ? event.data.json() : null;
159+
if (data && typeof data === "object") {
160+
title = buildNotificationTitle(data.type, data.sender_name);
161+
const body = buildNotificationBody(data);
162+
163+
// Use sender avatar if available, otherwise fall back to app icon
164+
// The sender_avatar comes from the push payload (set in 19-push-notifications.sql)
165+
const notificationIcon = data.sender_avatar || "/icons/android-chrome-192x192.png";
166+
167+
// Use notification_id as tag so each notification is unique on iOS.
168+
// iOS Safari does NOT support `renotify` — using a generic tag like "mention"
169+
// would cause each new mention to silently replace the previous one.
170+
const tag = data.notification_id || `${data.type}-${Date.now()}`;
171+
172+
options = {
173+
body: body,
174+
icon: notificationIcon,
175+
badge: "/icons/favicon-32x32.png",
176+
tag: tag,
177+
renotify: true,
178+
requireInteraction: false,
179+
data: {
180+
url: data.action_url || "/",
181+
notification_id: data.notification_id,
182+
},
183+
// Include image preview if message has an attachment
184+
...(data.image_url && { image: data.image_url }),
185+
};
186+
}
187+
} catch (error) {
188+
console.warn("[SW Extension] Bad push payload, showing a generic notification", error);
149189
}
150190

151-
// Build title and body from raw data
152-
const title = buildNotificationTitle(data.type, data.sender_name);
153-
const body = buildNotificationBody(data);
154-
155-
// Use sender avatar if available, otherwise fall back to app icon
156-
// The sender_avatar comes from the push payload (set in 19-push-notifications.sql)
157-
const notificationIcon = data.sender_avatar || "/icons/android-chrome-192x192.png";
158-
159-
// Use notification_id as tag so each notification is unique on iOS.
160-
// iOS Safari does NOT support `renotify` — using a generic tag like "mention"
161-
// would cause each new mention to silently replace the previous one.
162-
const tag = data.notification_id || `${data.type}-${Date.now()}`;
163-
164-
const options = {
165-
body: body,
166-
icon: notificationIcon,
167-
badge: "/icons/favicon-32x32.png",
168-
tag: tag,
169-
renotify: true,
170-
requireInteraction: false,
171-
data: {
172-
url: data.action_url || "/",
173-
notification_id: data.notification_id,
174-
},
175-
// Include image preview if message has an attachment
176-
...(data.image_url && { image: data.image_url }),
177-
};
178-
179191
event.waitUntil(self.registration.showNotification(title, options));
180192
});
181193

194+
// The worker has no user session, so it cannot save the new endpoint.
195+
// The client saves it on its next signed-in load.
196+
self.addEventListener("pushsubscriptionchange", (event) => {
197+
const key = event.oldSubscription?.options?.applicationServerKey;
198+
if (!key) return;
199+
event.waitUntil(
200+
self.registration.pushManager
201+
.subscribe({ userVisibleOnly: true, applicationServerKey: key })
202+
.catch((error) => console.error("[SW Extension] Could not subscribe again", error))
203+
);
204+
});
205+
182206
// Handle notification click
183207
self.addEventListener("notificationclick", (event) => {
184208
event.notification.close();
185209

186-
const url = event.notification.data?.url || "/";
210+
// action_url comes from the payload, so only a same-origin path may open.
211+
let url = "/";
212+
try {
213+
const target = new URL(event.notification.data?.url || "/", self.location.origin);
214+
if (target.origin === self.location.origin) url = target.href;
215+
} catch {}
187216

188217
event.waitUntil(
189218
clients

‎apps/webapp/src/components/settings/hooks/useSignOut.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { signOut } from '@api'
22
import * as toast from '@components/toast'
3+
import { isPushSupported, unregisterPushSubscription } from '@utils/push-notifications'
34
import { supabaseClient } from '@utils/supabase'
45
import { useState } from 'react'
56

@@ -12,6 +13,9 @@ export const useSignOut = () => {
1213
const handleSignOut = async () => {
1314
setIsLoading(true)
1415
await flushPendingPreferenceWrites()
16+
// Before signOut, because the RPC needs a live session. It stops the next person here
17+
// getting this account's pushes.
18+
if (isPushSupported()) await unregisterPushSubscription()
1519
const { error } = await signOut()
1620
if (error) {
1721
// auth-js drops the local session even when the server call fails. Stay only if it did not,

‎apps/webapp/src/hooks/usePushNotifications.ts‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { useAuthStore } from '@stores'
12
import {
23
getPermissionStatus,
34
isPushSupported,
@@ -36,6 +37,7 @@ export function usePushNotifications(): UsePushNotificationsReturn {
3637
const [error, setError] = useState<string | null>(null)
3738
const [errorCode, setErrorCode] = useState<string | null>(null)
3839
const [isRecoverable, setIsRecoverable] = useState(false)
40+
const userId = useAuthStore((s) => s.profile?.id)
3941

4042
useEffect(() => {
4143
if (!isSupported) {
@@ -47,15 +49,9 @@ export function usePushNotifications(): UsePushNotificationsReturn {
4749

4850
const initSubscription = async () => {
4951
try {
50-
const subscribed = await checkSubscribed()
51-
setIsSubscribed(subscribed)
52-
53-
if (subscribed) {
54-
const refreshResult = await refreshSubscriptionIfNeeded()
55-
if (refreshResult === 'failed') {
56-
// Refresh failed, but don't show error to user - subscription might still work
57-
}
58-
}
52+
// Sync first so the toggle shows the repaired state. The RPCs need a session.
53+
if (userId) await refreshSubscriptionIfNeeded()
54+
setIsSubscribed(await checkSubscribed())
5955
} catch {
6056
// Ignore errors during init
6157
} finally {
@@ -65,7 +61,7 @@ export function usePushNotifications(): UsePushNotificationsReturn {
6561
}
6662

6763
initSubscription()
68-
}, [isSupported])
64+
}, [isSupported, userId])
6965

7066
// Fires when the user revokes permission in browser settings.
7167
useEffect(() => {

‎apps/webapp/src/utils/push-notifications.ts‎

Lines changed: 78 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ const VAPID_PUBLIC_KEY = process.env.NEXT_PUBLIC_VAPID_PUBLIC_KEY
66

77
const SUBSCRIPTION_TIMESTAMP_KEY = 'docsplus_push_subscription_timestamp'
88

9-
// Subscriptions can expire, so re-subscribe once one is older than 30 days.
10-
const SUBSCRIPTION_MAX_AGE_MS = 30 * 24 * 60 * 60 * 1000
9+
// Save again at most daily: the server may have switched this device off.
10+
const SUBSCRIPTION_MAX_AGE_MS = 24 * 60 * 60 * 1000
1111

1212
export type PushErrorCode =
1313
| 'NOT_SUPPORTED'
@@ -99,6 +99,16 @@ function urlBase64ToUint8Array(base64String: string): ArrayBuffer {
9999
return outputArray.buffer
100100
}
101101

102+
// A null key means the browser does not say; treat it as current so we never churn it.
103+
function hasCurrentKey(subscription: PushSubscription, key: string): boolean {
104+
const current = subscription.options.applicationServerKey
105+
if (!current) return true
106+
const a = new Uint8Array(current)
107+
const b = new Uint8Array(urlBase64ToUint8Array(key))
108+
if (a.length !== b.length) return false
109+
return a.every((byte, i) => byte === b[i])
110+
}
111+
102112
function arrayBufferToBase64(buffer: ArrayBuffer | null): string {
103113
if (!buffer) return ''
104114
const bytes = new Uint8Array(buffer)
@@ -231,10 +241,12 @@ export async function registerPushSubscription(): Promise<string | null> {
231241
for (let attempt = 1; attempt <= 3; attempt++) {
232242
try {
233243
const existing = await registration.pushManager.getSubscription()
234-
if (existing) {
244+
if (existing && hasCurrentKey(existing, VAPID_PUBLIC_KEY)) {
235245
subscription = existing
236246
break
237247
}
248+
// A subscription made with another VAPID key gets 403 on every send.
249+
await existing?.unsubscribe()
238250

239251
subscription = await registration.pushManager.subscribe({
240252
userVisibleOnly: true,
@@ -279,30 +291,32 @@ export async function registerPushSubscription(): Promise<string | null> {
279291
return getDeviceId()
280292
}
281293

294+
/**
295+
* The RPC goes first because sign-out calls this while the session is still valid.
296+
* getRegistration() resolves at once with no worker, where serviceWorker.ready would hang.
297+
* The stamp is cleared even on failure, so the load path never resubscribes an opt-out.
298+
*/
282299
export async function unregisterPushSubscription(): Promise<boolean> {
283300
try {
284-
const registration = await navigator.serviceWorker.ready
285-
const subscription = await registration.pushManager.getSubscription()
286-
287-
if (subscription) {
288-
await subscription.unsubscribe()
289-
}
290-
291301
const { data, error } = await supabaseClient.rpc('unregister_push_subscription', {
292302
p_device_id: getDeviceId()
293303
})
294304

305+
const registration = await navigator.serviceWorker.getRegistration()
306+
const subscription = await registration?.pushManager.getSubscription()
307+
await subscription?.unsubscribe()
308+
295309
if (error) {
296310
console.error('Failed to unregister push subscription:', error)
297311
return false
298312
}
299313

300-
clearSubscriptionTimestamp()
301-
302314
return data as boolean
303315
} catch (err) {
304316
console.error('Failed to unsubscribe from push:', err)
305317
return false
318+
} finally {
319+
clearSubscriptionTimestamp()
306320
}
307321
}
308322

@@ -318,18 +332,30 @@ export async function isSubscribed(): Promise<boolean> {
318332
}
319333
}
320334

321-
function shouldRefreshSubscription(): boolean {
322-
const timestamp = localStorage.getItem(SUBSCRIPTION_TIMESTAMP_KEY)
323-
if (!timestamp) return true
324-
return Date.now() - parseInt(timestamp, 10) > SUBSCRIPTION_MAX_AGE_MS
335+
// The stamp doubles as the opt-in record: present means this device registered and kept push on.
336+
function readSubscriptionTimestamp(): number | null {
337+
try {
338+
const timestamp = localStorage.getItem(SUBSCRIPTION_TIMESTAMP_KEY)
339+
return timestamp ? parseInt(timestamp, 10) : null
340+
} catch {
341+
return null
342+
}
325343
}
326344

327345
function markSubscriptionFresh(): void {
328-
localStorage.setItem(SUBSCRIPTION_TIMESTAMP_KEY, String(Date.now()))
346+
try {
347+
localStorage.setItem(SUBSCRIPTION_TIMESTAMP_KEY, String(Date.now()))
348+
} catch {
349+
// Storage blocked: the next load saves again, which is harmless.
350+
}
329351
}
330352

331353
function clearSubscriptionTimestamp(): void {
332-
localStorage.removeItem(SUBSCRIPTION_TIMESTAMP_KEY)
354+
try {
355+
localStorage.removeItem(SUBSCRIPTION_TIMESTAMP_KEY)
356+
} catch {
357+
// Storage blocked: nothing was stored to clear.
358+
}
333359
}
334360

335361
async function saveSubscriptionToDatabase(subscription: PushSubscription): Promise<void> {
@@ -353,28 +379,48 @@ async function saveSubscriptionToDatabase(subscription: PushSubscription): Promi
353379
markSubscriptionFresh()
354380
}
355381

356-
export async function refreshSubscriptionIfNeeded(): Promise<
357-
'fresh' | 'refreshed' | 'failed' | 'not_subscribed'
358-
> {
359-
if (!isPushSupported() || !VAPID_PUBLIC_KEY) return 'not_subscribed'
382+
type RefreshResult = 'fresh' | 'refreshed' | 'failed' | 'not_subscribed'
383+
384+
let refreshInFlight: Promise<RefreshResult> | null = null
385+
386+
/**
387+
* Load-path sync: re-saves a live subscription daily, restores a lost one, and replaces one
388+
* made with an old VAPID key. It never unsubscribes a working subscription or asks for
389+
* permission. Concurrent callers share one run, so two mounted hooks send one RPC.
390+
*/
391+
export function refreshSubscriptionIfNeeded(): Promise<RefreshResult> {
392+
refreshInFlight ??= syncSubscription().finally(() => {
393+
refreshInFlight = null
394+
})
395+
return refreshInFlight
396+
}
397+
398+
async function syncSubscription(): Promise<RefreshResult> {
399+
if (!isPushSupported() || !VAPID_PUBLIC_KEY || Notification.permission !== 'granted') {
400+
return 'not_subscribed'
401+
}
360402

361403
try {
362404
const registration = await Promise.race([
363405
navigator.serviceWorker.ready,
364406
new Promise<never>((_, reject) => setTimeout(() => reject(new Error('timeout')), 3000))
365407
])
366408

367-
const existingSubscription = await registration.pushManager.getSubscription()
368-
if (!existingSubscription) return 'not_subscribed'
369-
if (!shouldRefreshSubscription()) return 'fresh'
370-
371-
await existingSubscription.unsubscribe()
372-
const newSubscription = await registration.pushManager.subscribe({
373-
userVisibleOnly: true,
374-
applicationServerKey: urlBase64ToUint8Array(VAPID_PUBLIC_KEY)
375-
})
409+
const existing = await registration.pushManager.getSubscription()
410+
const savedAt = readSubscriptionTimestamp()
411+
412+
// No subscription and no stamp: never opted in here, or turned push off.
413+
if (!existing && savedAt === null) return 'not_subscribed'
414+
if (
415+
existing &&
416+
hasCurrentKey(existing, VAPID_PUBLIC_KEY) &&
417+
savedAt !== null &&
418+
Date.now() - savedAt < SUBSCRIPTION_MAX_AGE_MS
419+
) {
420+
return 'fresh'
421+
}
376422

377-
await saveSubscriptionToDatabase(newSubscription)
423+
await registerPushSubscription()
378424
return 'refreshed'
379425
} catch {
380426
return 'failed'

0 commit comments

Comments
 (0)