mirror of
https://github.com/lobehub/lobe-chat.git
synced 2026-06-13 19:20:04 +00:00
🐛 fix(mobile): stop pushToken.unregister 401 storm (#15719)
Symptom: app.lobehub.com production logs show ~50+ TRPCError UNAUTHORIZED traces per second on /trpc/mobile/pushToken.unregister, starting from the v1.0.7 mobile release. Only `unregister` is hit — `register` never appears in logs. Root cause: the v1.0.7 client calls unregister *during* sign-out, after the session is already invalid in practice (expired OIDC token / cleared cookie). With authedProcedure gating, every logout turns into a 401 that the client mistakes for an auth-expired event and retries → a storm. Inside the client this also creates a logout → 401 → authExpired.redirect → logout recursion. Fix: change `unregister` to publicProcedure and authorize by the (deviceId, expoToken) pair the client received at registration — holding both is proof of ownership of that row, same trust model as APNs/FCM unregister. Legacy v1.0.7 clients that only send deviceId get a silent 200; the stale row is cleaned up by the existing `process-push-receipts` worker via Expo's DeviceNotRegistered receipts. Returning 200 to those legacy calls also breaks the client-side recursion at the source — the in-the-wild v1.0.7 fleet stops 401 flooding the moment this ships, before users update. Tests: - Router (mocked): expoToken path deletes by (expoToken, deviceId); no-expoToken path silently succeeds; unauthenticated caller succeeds; empty-string fields rejected. - Model (integration): only the row matching both fields is removed; mismatched expoToken is preserved (defense against callers who only guess deviceId). Fixes LOBE-10174
This commit is contained in:
@@ -4,12 +4,15 @@ import { pushTokenRouter } from '@/server/routers/lambda/pushToken';
|
||||
|
||||
const mockUpsert = vi.fn();
|
||||
const mockUnregister = vi.fn();
|
||||
const mockDeleteByExpoTokenAndDevice = vi.fn();
|
||||
|
||||
vi.mock('@/database/models/pushToken', () => ({
|
||||
PushTokenModel: vi.fn(() => ({
|
||||
unregister: mockUnregister,
|
||||
upsert: mockUpsert,
|
||||
})),
|
||||
deletePushTokenByExpoTokenAndDevice: (...args: unknown[]) =>
|
||||
mockDeleteByExpoTokenAndDevice(...args),
|
||||
}));
|
||||
|
||||
const createCaller = (ctxOverrides: Partial<any> = {}) => {
|
||||
@@ -91,18 +94,59 @@ describe('pushTokenRouter', () => {
|
||||
});
|
||||
|
||||
describe('unregister', () => {
|
||||
it('should call model.unregister with deviceId', async () => {
|
||||
mockUnregister.mockResolvedValueOnce(undefined);
|
||||
it('should delete by (expoToken, deviceId) when expoToken is provided', async () => {
|
||||
mockDeleteByExpoTokenAndDevice.mockResolvedValueOnce(undefined);
|
||||
const caller = createCaller();
|
||||
|
||||
await caller.unregister({ deviceId: 'device-1' });
|
||||
const result = await caller.unregister({
|
||||
deviceId: 'device-1',
|
||||
expoToken: 'ExponentPushToken[abc]',
|
||||
});
|
||||
|
||||
expect(mockUnregister).toHaveBeenCalledWith('device-1');
|
||||
expect(mockDeleteByExpoTokenAndDevice).toHaveBeenCalledWith(expect.anything(), {
|
||||
deviceId: 'device-1',
|
||||
expoToken: 'ExponentPushToken[abc]',
|
||||
});
|
||||
expect(result).toEqual({ success: true });
|
||||
// Legacy (userId, deviceId) path must not fire when expoToken is present
|
||||
expect(mockUnregister).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should silently succeed without expoToken (1.0.7 legacy clients)', async () => {
|
||||
const caller = createCaller();
|
||||
|
||||
const result = await caller.unregister({ deviceId: 'device-1' });
|
||||
|
||||
// Cleanup happens via process-push-receipts cron — no DB delete here
|
||||
expect(mockDeleteByExpoTokenAndDevice).not.toHaveBeenCalled();
|
||||
expect(mockUnregister).not.toHaveBeenCalled();
|
||||
expect(result).toEqual({ success: true });
|
||||
});
|
||||
|
||||
it('should succeed for an unauthenticated caller (no userId)', async () => {
|
||||
// The whole point of making this public: clients call it during sign-out
|
||||
// when their session may already be gone. Must not 401.
|
||||
const caller = createCaller({ userId: undefined });
|
||||
|
||||
const result = await caller.unregister({
|
||||
deviceId: 'device-1',
|
||||
expoToken: 'ExponentPushToken[abc]',
|
||||
});
|
||||
|
||||
expect(result).toEqual({ success: true });
|
||||
expect(mockDeleteByExpoTokenAndDevice).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should reject empty deviceId', async () => {
|
||||
const caller = createCaller();
|
||||
await expect(caller.unregister({ deviceId: '' })).rejects.toThrow();
|
||||
});
|
||||
|
||||
it('should reject empty expoToken when provided', async () => {
|
||||
const caller = createCaller();
|
||||
await expect(
|
||||
caller.unregister({ deviceId: 'device-1', expoToken: '' }),
|
||||
).rejects.toThrow();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,10 +1,13 @@
|
||||
import { z } from 'zod';
|
||||
|
||||
import { PushTokenModel } from '@/database/models/pushToken';
|
||||
import { authedProcedure, router } from '@/libs/trpc/lambda';
|
||||
import {
|
||||
deletePushTokenByExpoTokenAndDevice,
|
||||
PushTokenModel,
|
||||
} from '@/database/models/pushToken';
|
||||
import { authedProcedure, publicProcedure, router } from '@/libs/trpc/lambda';
|
||||
import { serverDatabase } from '@/libs/trpc/lambda/middleware';
|
||||
|
||||
const pushTokenProcedure = authedProcedure.use(serverDatabase).use(async (opts) => {
|
||||
const authedPushTokenProcedure = authedProcedure.use(serverDatabase).use(async (opts) => {
|
||||
const { ctx } = opts;
|
||||
|
||||
return opts.next({
|
||||
@@ -13,7 +16,7 @@ const pushTokenProcedure = authedProcedure.use(serverDatabase).use(async (opts)
|
||||
});
|
||||
|
||||
export const pushTokenRouter = router({
|
||||
register: pushTokenProcedure
|
||||
register: authedPushTokenProcedure
|
||||
.input(
|
||||
z.object({
|
||||
appVersion: z.string().optional(),
|
||||
@@ -27,10 +30,38 @@ export const pushTokenRouter = router({
|
||||
return ctx.pushTokenModel.upsert(input);
|
||||
}),
|
||||
|
||||
unregister: pushTokenProcedure
|
||||
.input(z.object({ deviceId: z.string().min(1) }))
|
||||
/**
|
||||
* Public on purpose: clients call this during sign-out, when their session
|
||||
* may already be invalid (expired token / cleared cookie). Authenticating by
|
||||
* session here causes a 401 storm on every logout in the wild — the original
|
||||
* intent was "clean up before clearing auth", but in practice the auth has
|
||||
* already been cleared on the server long before logout fires.
|
||||
*
|
||||
* Authorization model: the caller presents the (deviceId, expoToken) pair it
|
||||
* received at registration. Holding both = proof of ownership of the row,
|
||||
* same trust model as APNs/FCM unregister.
|
||||
*
|
||||
* Backwards compat: older clients (≤ 1.0.7) only send `deviceId`. We silently
|
||||
* succeed in that case and let the `process-push-receipts` worker clean up
|
||||
* stale rows via `DeviceNotRegistered` receipts from Expo. Returning 200 here
|
||||
* is what actually stops the 401 storm in production.
|
||||
*/
|
||||
unregister: publicProcedure
|
||||
.use(serverDatabase)
|
||||
.input(
|
||||
z.object({
|
||||
deviceId: z.string().min(1),
|
||||
expoToken: z.string().min(1).optional(),
|
||||
}),
|
||||
)
|
||||
.mutation(async ({ ctx, input }) => {
|
||||
return ctx.pushTokenModel.unregister(input.deviceId);
|
||||
const { deviceId, expoToken } = input;
|
||||
|
||||
if (expoToken) {
|
||||
await deletePushTokenByExpoTokenAndDevice(ctx.serverDB, { deviceId, expoToken });
|
||||
}
|
||||
|
||||
return { success: true };
|
||||
}),
|
||||
});
|
||||
|
||||
|
||||
@@ -5,7 +5,11 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest';
|
||||
import { getTestDB } from '../../core/getTestDB';
|
||||
import { pushTokens, users } from '../../schemas';
|
||||
import type { LobeChatDatabase } from '../../type';
|
||||
import { deletePushTokensByExpoTokens, PushTokenModel } from '../pushToken';
|
||||
import {
|
||||
deletePushTokenByExpoTokenAndDevice,
|
||||
deletePushTokensByExpoTokens,
|
||||
PushTokenModel,
|
||||
} from '../pushToken';
|
||||
|
||||
const serverDB: LobeChatDatabase = await getTestDB();
|
||||
|
||||
@@ -156,6 +160,46 @@ describe('PushTokenModel', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('deletePushTokenByExpoTokenAndDevice helper', () => {
|
||||
it('should delete only the row matching both deviceId and expoToken', async () => {
|
||||
await model.upsert({ deviceId: 'a', expoToken: 'token-a', platform: 'ios' });
|
||||
await model.upsert({ deviceId: 'b', expoToken: 'token-b', platform: 'ios' });
|
||||
|
||||
await deletePushTokenByExpoTokenAndDevice(serverDB, {
|
||||
deviceId: 'a',
|
||||
expoToken: 'token-a',
|
||||
});
|
||||
|
||||
const remaining = await model.listByUserId();
|
||||
expect(remaining).toHaveLength(1);
|
||||
expect(remaining[0].deviceId).toBe('b');
|
||||
});
|
||||
|
||||
it('should not delete when only deviceId matches but expoToken differs', async () => {
|
||||
// Defensive: a malicious caller knowing only the deviceId must not be
|
||||
// able to unregister someone else's row.
|
||||
await model.upsert({ deviceId: 'a', expoToken: 'real-token', platform: 'ios' });
|
||||
|
||||
await deletePushTokenByExpoTokenAndDevice(serverDB, {
|
||||
deviceId: 'a',
|
||||
expoToken: 'guessed-token',
|
||||
});
|
||||
|
||||
const remaining = await model.listByUserId();
|
||||
expect(remaining).toHaveLength(1);
|
||||
expect(remaining[0].expoToken).toBe('real-token');
|
||||
});
|
||||
|
||||
it('should be a no-op when no row matches', async () => {
|
||||
await expect(
|
||||
deletePushTokenByExpoTokenAndDevice(serverDB, {
|
||||
deviceId: 'never',
|
||||
expoToken: 'never',
|
||||
}),
|
||||
).resolves.not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe('cascade delete on user removal', () => {
|
||||
it('should delete tokens when user is deleted', async () => {
|
||||
await model.upsert({ deviceId: 'a', expoToken: 't', platform: 'ios' });
|
||||
|
||||
@@ -60,3 +60,18 @@ export async function deletePushTokensByExpoTokens(
|
||||
if (tokens.length === 0) return;
|
||||
await db.delete(pushTokens).where(inArray(pushTokens.expoToken, tokens));
|
||||
}
|
||||
|
||||
/**
|
||||
* Static helper used by the public `unregister` endpoint — lets a signed-out
|
||||
* client clean up its own token without a session, by presenting the
|
||||
* (expoToken, deviceId) pair it received at registration. Both fields must
|
||||
* match so a stale row for a different device can't be deleted by accident.
|
||||
*/
|
||||
export async function deletePushTokenByExpoTokenAndDevice(
|
||||
db: LobeChatDatabase,
|
||||
args: { deviceId: string; expoToken: string },
|
||||
): Promise<void> {
|
||||
await db
|
||||
.delete(pushTokens)
|
||||
.where(and(eq(pushTokens.expoToken, args.expoToken), eq(pushTokens.deviceId, args.deviceId)));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user