fix(webpush): improve iOS push subscription endpoint cleanup (#2140)

This commit is contained in:
0xsysr3ll
2025-12-31 13:44:45 +01:00
committed by GitHub
parent 57d583e1bd
commit d0c9afc16e
7 changed files with 222 additions and 42 deletions

View File

@@ -1,8 +1,15 @@
import { DbAwareColumn } from '@server/utils/DbColumnHelper'; import { DbAwareColumn } from '@server/utils/DbColumnHelper';
import { Column, Entity, ManyToOne, PrimaryGeneratedColumn } from 'typeorm'; import {
Column,
Entity,
ManyToOne,
PrimaryGeneratedColumn,
Unique,
} from 'typeorm';
import { User } from './User'; import { User } from './User';
@Entity() @Entity()
@Unique(['endpoint', 'user'])
export class UserPushSubscription { export class UserPushSubscription {
@PrimaryGeneratedColumn() @PrimaryGeneratedColumn()
public id: number; public id: number;

View File

@@ -24,6 +24,15 @@ interface PushNotificationPayload {
isAdmin?: boolean; isAdmin?: boolean;
} }
interface WebPushError extends Error {
statusCode?: number;
status?: number;
body?: string | unknown;
response?: {
body?: string | unknown;
};
}
class WebPushAgent class WebPushAgent
extends BaseAgent<NotificationAgentConfig> extends BaseAgent<NotificationAgentConfig>
implements NotificationAgent implements NotificationAgent
@@ -188,19 +197,30 @@ class WebPushAgent
notificationPayload notificationPayload
); );
} catch (e) { } catch (e) {
const webPushError = e as WebPushError;
const statusCode = webPushError.statusCode || webPushError.status;
const errorMessage = webPushError.message || String(e);
// RFC 8030: 410/404 are permanent failures, others are transient
const isPermanentFailure = statusCode === 410 || statusCode === 404;
logger.error( logger.error(
'Error sending web push notification; removing subscription', isPermanentFailure
? 'Error sending web push notification; removing invalid subscription'
: 'Error sending web push notification (transient error, keeping subscription)',
{ {
label: 'Notifications', label: 'Notifications',
recipient: pushSub.user.displayName, recipient: pushSub.user.displayName,
type: Notification[type], type: Notification[type],
subject: payload.subject, subject: payload.subject,
errorMessage: e.message, errorMessage,
statusCode: statusCode || 'unknown',
} }
); );
// Failed to send notification so we need to remove the subscription if (isPermanentFailure) {
userPushSubRepository.remove(pushSub); await userPushSubRepository.remove(pushSub);
}
} }
}; };

View File

@@ -0,0 +1,19 @@
import type { MigrationInterface, QueryRunner } from 'typeorm';
export class AddUniqueConstraintToPushSubscription1765233385034
implements MigrationInterface
{
name = 'AddUniqueConstraintToPushSubscription1765233385034';
public async up(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(
`ALTER TABLE "user_push_subscription" ADD CONSTRAINT "UQ_6427d07d9a171a3a1ab87480005" UNIQUE ("endpoint", "userId")`
);
}
public async down(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(
`ALTER TABLE "user_push_subscription" DROP CONSTRAINT "UQ_6427d07d9a171a3a1ab87480005"`
);
}
}

View File

@@ -0,0 +1,17 @@
import type { MigrationInterface, QueryRunner } from 'typeorm';
export class AddUniqueConstraintToPushSubscription1765233385034
implements MigrationInterface
{
name = 'AddUniqueConstraintToPushSubscription1765233385034';
public async up(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(
`CREATE UNIQUE INDEX "UQ_6427d07d9a171a3a1ab87480005" ON "user_push_subscription" ("endpoint", "userId")`
);
}
public async down(queryRunner: QueryRunner): Promise<void> {
await queryRunner.query(`DROP INDEX "UQ_6427d07d9a171a3a1ab87480005"`);
}
}

View File

@@ -4,7 +4,7 @@ import TautulliAPI from '@server/api/tautulli';
import { MediaType } from '@server/constants/media'; import { MediaType } from '@server/constants/media';
import { MediaServerType } from '@server/constants/server'; import { MediaServerType } from '@server/constants/server';
import { UserType } from '@server/constants/user'; import { UserType } from '@server/constants/user';
import { getRepository } from '@server/datasource'; import dataSource, { getRepository } from '@server/datasource';
import Media from '@server/entity/Media'; import Media from '@server/entity/Media';
import { MediaRequest } from '@server/entity/MediaRequest'; import { MediaRequest } from '@server/entity/MediaRequest';
import { User } from '@server/entity/User'; import { User } from '@server/entity/User';
@@ -25,7 +25,8 @@ import { getHostname } from '@server/utils/getHostname';
import { Router } from 'express'; import { Router } from 'express';
import gravatarUrl from 'gravatar-url'; import gravatarUrl from 'gravatar-url';
import { findIndex, sortBy } from 'lodash'; import { findIndex, sortBy } from 'lodash';
import { In } from 'typeorm'; import type { EntityManager } from 'typeorm';
import { In, Not } from 'typeorm';
import userSettingsRoutes from './usersettings'; import userSettingsRoutes from './usersettings';
const router = Router(); const router = Router();
@@ -188,30 +189,82 @@ router.post<
} }
>('/registerPushSubscription', async (req, res, next) => { >('/registerPushSubscription', async (req, res, next) => {
try { try {
const userPushSubRepository = getRepository(UserPushSubscription); // This prevents race conditions where two requests both pass the checks
await dataSource.transaction(
async (transactionalEntityManager: EntityManager) => {
const transactionalRepo =
transactionalEntityManager.getRepository(UserPushSubscription);
const existingSubs = await userPushSubRepository.find({ // Check for existing subscription by auth or endpoint within transaction
relations: { user: true }, const existingSubscription = await transactionalRepo.findOne({
where: { auth: req.body.auth, user: { id: req.user?.id } }, relations: { user: true },
}); where: [
{ auth: req.body.auth, user: { id: req.user?.id } },
{ endpoint: req.body.endpoint, user: { id: req.user?.id } },
],
});
if (existingSubs.length > 0) { if (existingSubscription) {
logger.debug( // If endpoint matches but auth is different, update with new keys (iOS refresh case)
'User push subscription already exists. Skipping registration.', if (
{ label: 'API' } existingSubscription.endpoint === req.body.endpoint &&
); existingSubscription.auth !== req.body.auth
return res.status(204).send(); ) {
} existingSubscription.auth = req.body.auth;
existingSubscription.p256dh = req.body.p256dh;
existingSubscription.userAgent = req.body.userAgent;
const userPushSubscription = new UserPushSubscription({ await transactionalRepo.save(existingSubscription);
auth: req.body.auth,
endpoint: req.body.endpoint,
p256dh: req.body.p256dh,
userAgent: req.body.userAgent,
user: req.user,
});
userPushSubRepository.save(userPushSubscription); logger.debug(
'Updated existing push subscription with new keys for same endpoint.',
{ label: 'API' }
);
return;
}
logger.debug(
'Duplicate subscription detected. Skipping registration.',
{ label: 'API' }
);
return;
}
// Clean up old subscriptions from the same device (userAgent) for this user
// iOS can silently refresh endpoints, leaving stale subscriptions in the database
// Only clean up if we're creating a new subscription (not updating an existing one)
if (req.body.userAgent) {
const staleSubscriptions = await transactionalRepo.find({
relations: { user: true },
where: {
userAgent: req.body.userAgent,
user: { id: req.user?.id },
// Only remove subscriptions with different endpoints (stale ones)
// Keep subscriptions that might be from different browsers/tabs
endpoint: Not(req.body.endpoint),
},
});
if (staleSubscriptions.length > 0) {
await transactionalRepo.remove(staleSubscriptions);
logger.debug(
`Removed ${staleSubscriptions.length} stale push subscription(s) from same device.`,
{ label: 'API' }
);
}
}
const userPushSubscription = new UserPushSubscription({
auth: req.body.auth,
endpoint: req.body.endpoint,
p256dh: req.body.p256dh,
userAgent: req.body.userAgent,
user: req.user,
});
await transactionalRepo.save(userPushSubscription);
}
);
return res.status(204).send(); return res.status(204).send();
} catch (e) { } catch (e) {

View File

@@ -109,15 +109,28 @@ const UserWebPushSettings = () => {
// Deletes/disables corresponding push subscription from database // Deletes/disables corresponding push subscription from database
const disablePushNotifications = async (endpoint?: string) => { const disablePushNotifications = async (endpoint?: string) => {
try { try {
await unsubscribeToPushNotifications(user?.id, endpoint); const unsubscribedEndpoint = await unsubscribeToPushNotifications(
user?.id,
// Delete from backend if endpoint is available endpoint
if (subEndpoint) { );
await deletePushSubscriptionFromBackend(subEndpoint);
}
localStorage.setItem('pushNotificationsEnabled', 'false'); localStorage.setItem('pushNotificationsEnabled', 'false');
setWebPushEnabled(false); setWebPushEnabled(false);
// Only delete the current browser's subscription, not all devices
const endpointToDelete = unsubscribedEndpoint || subEndpoint || endpoint;
if (endpointToDelete) {
try {
await axios.delete(
`/api/v1/user/${user?.id}/pushSubscription/${encodeURIComponent(
endpointToDelete
)}`
);
} catch {
// Ignore deletion failures - backend cleanup is best effort
}
}
addToast(intl.formatMessage(messages.webpushhasbeendisabled), { addToast(intl.formatMessage(messages.webpushhasbeendisabled), {
autoDismiss: true, autoDismiss: true,
appearance: 'success', appearance: 'success',
@@ -157,7 +170,33 @@ const UserWebPushSettings = () => {
useEffect(() => { useEffect(() => {
const verifyWebPush = async () => { const verifyWebPush = async () => {
const enabled = await verifyPushSubscription(user?.id, currentSettings); const enabled = await verifyPushSubscription(user?.id, currentSettings);
setWebPushEnabled(enabled); let isEnabled = enabled;
if (!enabled && 'serviceWorker' in navigator) {
const { subscription } = await getPushSubscription();
if (subscription) {
isEnabled = true;
}
}
if (!isEnabled && dataDevices && dataDevices.length > 0) {
const currentUserAgent = navigator.userAgent;
const hasMatchingDevice = dataDevices.some(
(device) => device.userAgent === currentUserAgent
);
if (hasMatchingDevice) {
isEnabled = true;
}
}
setWebPushEnabled(isEnabled);
if (localStorage.getItem('pushNotificationsEnabled') === null) {
localStorage.setItem(
'pushNotificationsEnabled',
isEnabled ? 'true' : 'false'
);
}
}; };
if (user?.id) { if (user?.id) {

View File

@@ -49,13 +49,17 @@ export const verifyPushSubscription = async (
currentSettings.vapidPublic currentSettings.vapidPublic
).toString(); ).toString();
if (currentServerKey !== expectedServerKey) {
return false;
}
const endpoint = subscription.endpoint; const endpoint = subscription.endpoint;
const { data } = await axios.get<UserPushSubscription>( const { data } = await axios.get<UserPushSubscription>(
`/api/v1/user/${userId}/pushSubscription/${encodeURIComponent(endpoint)}` `/api/v1/user/${userId}/pushSubscription/${encodeURIComponent(endpoint)}`
); );
return expectedServerKey === currentServerKey && data.endpoint === endpoint; return data.endpoint === endpoint;
} catch { } catch {
return false; return false;
} }
@@ -65,20 +69,39 @@ export const verifyAndResubscribePushSubscription = async (
userId: number | undefined, userId: number | undefined,
currentSettings: PublicSettingsResponse currentSettings: PublicSettingsResponse
): Promise<boolean> => { ): Promise<boolean> => {
if (!userId) {
return false;
}
const { subscription } = await getPushSubscription();
const isValid = await verifyPushSubscription(userId, currentSettings); const isValid = await verifyPushSubscription(userId, currentSettings);
if (isValid) { if (isValid) {
return true; return true;
} }
if (subscription) {
return false;
}
if (currentSettings.enablePushRegistration) { if (currentSettings.enablePushRegistration) {
try { try {
// Unsubscribe from the backend to clear the existing push subscription (keys and endpoint) const oldEndpoint = await unsubscribeToPushNotifications(userId);
await unsubscribeToPushNotifications(userId);
// Subscribe again to generate a fresh push subscription with updated keys and endpoint
await subscribeToPushNotifications(userId, currentSettings); await subscribeToPushNotifications(userId, currentSettings);
if (oldEndpoint) {
try {
await axios.delete(
`/api/v1/user/${userId}/pushSubscription/${encodeURIComponent(
oldEndpoint
)}`
);
} catch (error) {
// Ignore errors when deleting old endpoint (it might not exist)
}
}
return true; return true;
} catch (error) { } catch (error) {
throw new Error(`[SW] Resubscribe failed: ${error.message}`); throw new Error(`[SW] Resubscribe failed: ${error.message}`);
@@ -136,24 +159,26 @@ export const subscribeToPushNotifications = async (
export const unsubscribeToPushNotifications = async ( export const unsubscribeToPushNotifications = async (
userId: number | undefined, userId: number | undefined,
endpoint?: string endpoint?: string
) => { ): Promise<string | null> => {
if (!('serviceWorker' in navigator) || !userId) { if (!('serviceWorker' in navigator) || !userId) {
return; return null;
} }
try { try {
const { subscription } = await getPushSubscription(); const { subscription } = await getPushSubscription();
if (!subscription) { if (!subscription) {
return false; return null;
} }
const { endpoint: currentEndpoint } = subscription.toJSON(); const { endpoint: currentEndpoint } = subscription.toJSON();
if (!endpoint || endpoint === currentEndpoint) { if (!endpoint || endpoint === currentEndpoint) {
await subscription.unsubscribe(); await subscription.unsubscribe();
return true; return currentEndpoint ?? null;
} }
return null;
} catch (error) { } catch (error) {
throw new Error( throw new Error(
`Issue unsubscribing to push notifications: ${error.message}` `Issue unsubscribing to push notifications: ${error.message}`