fix: correct NanoId validation and better reassignment

jkrumm · Jan 19, 2024 · 8b5fa44 · 8b5fa44
1 parent f33d7ad
commit 8b5fa44
Show file tree

Hide file tree

Showing 11 changed files with 91 additions and 34 deletions.
diff --git a/src/components/room/room-wrapper.tsx b/src/components/room/room-wrapper.tsx
@@ -13,6 +13,7 @@ import { AblyProvider } from 'ably/react';
 import { useLogger } from 'next-axiom';
 
 import { api } from 'fpp/utils/api';
+import { validateNanoId } from 'fpp/utils/validate-nano-id.util';
 
 import { useLocalstorageStore } from 'fpp/store/local-storage.store';
 import { useRoomStateStore } from 'fpp/store/room-state.store';
@@ -35,15 +36,15 @@ const RoomWrapper = () => {
   );
 
   const setUserIdRoomState = useRoomStateStore((state) => state.setUserId);
-  if (userId) {
-    setUserIdRoomState(userId);
+  if (validateNanoId(userId)) {
+    setUserIdRoomState(userId!);
   }
 
   let ablyClient;
-  if (userId) {
+  if (validateNanoId(userId)) {
     ablyClient = new Ably.Realtime.Promise({
       authUrl: `${env.NEXT_PUBLIC_API_ROOT}api/ably-token`,
-      clientId: userId,
+      clientId: userId!,
     });
   }
 

diff --git a/src/hooks/use-tracking.hook.ts b/src/hooks/use-tracking.hook.ts
@@ -7,6 +7,8 @@ import { type Logger } from 'next-axiom';
 
 import { logEndpoint } from 'fpp/constants/logging.constant';
 
+import { validateNanoId } from 'fpp/utils/validate-nano-id.util';
+
 import { useLocalstorageStore } from 'fpp/store/local-storage.store';
 
 import { type RouteType } from 'fpp/server/db/schema';
@@ -59,7 +61,7 @@ export const sendTrackPageView = ({
     });
     const url = `${env.NEXT_PUBLIC_API_ROOT}api/track-page-view`;
 
-    if (navigator.sendBeacon && userId) {
+    if (navigator.sendBeacon && userId && validateNanoId(userId)) {
       navigator.sendBeacon(url, body);
       logger.debug(logEndpoint.TRACK_PAGE_VIEW, {
         withBeacon: true,

diff --git a/src/pages/api/ably-token.ts b/src/pages/api/ably-token.ts
@@ -11,6 +11,7 @@ import {
 import { logEndpoint } from 'fpp/constants/logging.constant';
 
 import { withLogger } from 'fpp/utils/api-logger.util';
+import { validateNanoId } from 'fpp/utils/validate-nano-id.util';
 
 export const runtime = 'edge';
 export const dynamic = 'force-dynamic'; // no caching
@@ -25,8 +26,8 @@ const AblyToken = withLogger(async (request: AxiomRequest) => {
 
   const clientId = req.nextUrl.searchParams.get('clientId');
 
-  if (!clientId || !/^[A-Za-z0-9_~]{21}$/.test(clientId)) {
-    throw new BadRequestError('clientId invalid');
+  if (!validateNanoId(clientId)) {
+    throw new BadRequestError('userId invalid');
   }
 
   try {

diff --git a/src/pages/api/track-event.ts b/src/pages/api/track-event.ts
@@ -11,6 +11,7 @@ import { logEndpoint } from 'fpp/constants/logging.constant';
 import { withLogger } from 'fpp/utils/api-logger.util';
 import { findUserById } from 'fpp/utils/db-api.util';
 import { decodeBlob } from 'fpp/utils/decode.util';
+import { validateNanoId } from 'fpp/utils/validate-nano-id.util';
 
 import db from 'fpp/server/db/db';
 import { EventType, events } from 'fpp/server/db/schema';
@@ -53,8 +54,8 @@ const validateInput = ({
   userId: string;
   event: keyof typeof EventType;
 }): void => {
-  if (!userId || userId.length !== 21) {
-    throw new BadRequestError('invalid visitorId');
+  if (!validateNanoId(userId)) {
+    throw new BadRequestError('invalid userId');
   }
 
   if (EventType[event] === undefined) {

diff --git a/src/pages/api/track-page-view.ts b/src/pages/api/track-page-view.ts
@@ -13,6 +13,7 @@ import { logEndpoint } from 'fpp/constants/logging.constant';
 
 import { withLogger } from 'fpp/utils/api-logger.util';
 import { decodeBlob } from 'fpp/utils/decode.util';
+import { validateNanoId } from 'fpp/utils/validate-nano-id.util';
 
 import db from 'fpp/server/db/db';
 import { RouteType, pageViews, users } from 'fpp/server/db/schema';
@@ -43,7 +44,7 @@ const TrackPageView = withLogger(async (req: AxiomRequest) => {
     throw new BadRequestError('invalid route');
   }
 
-  userId = !userId || userId.length !== 21 ? nanoid() : userId;
+  userId = (!validateNanoId(userId) ? nanoid() : userId)!;
 
   const userExists = !!(
     await db.select().from(users).where(eq(users.id, userId))

diff --git a/src/server/api/routers/room.router.ts b/src/server/api/routers/room.router.ts
@@ -6,6 +6,7 @@ import { z } from 'zod';
 
 import { isValidMediumint } from 'fpp/utils/number.utils';
 import { generateRoomNumber } from 'fpp/utils/room-number.util';
+import { validateNanoId } from 'fpp/utils/validate-nano-id.util';
 
 import { createTRPCRouter, publicProcedure } from 'fpp/server/api/trpc';
 import {
@@ -45,7 +46,7 @@ export const roomRouter = createTRPCRouter({
     .input(
       z.object({
         queryRoom: z.string().max(15).min(2).toLowerCase().trim(),
-        userId: z.string().length(21).nullable(),
+        userId: z.string().nullable(),
         roomEvent: z.enum([
           RoomEvent.ENTERED_ROOM_DIRECTLY,
           RoomEvent.ENTERED_RECENT_ROOM,
@@ -57,7 +58,7 @@ export const roomRouter = createTRPCRouter({
       async ({ ctx: { req, db }, input: { queryRoom, userId, roomEvent } }) => {
         let room: IRoom | undefined;
 
-        if (!userId) {
+        if (!validateNanoId(userId)) {
           userId = nanoid();
           const userPayload = getVisitorPayload(req as AxiomRequest);
           await db.insert(users).values({
@@ -85,11 +86,11 @@ export const roomRouter = createTRPCRouter({
               ? EventType.ENTERED_EXISTING_ROOM
               : roomEvent;
           await db.insert(events).values({
-            userId,
+            userId: userId!,
             event,
           });
           return {
-            userId,
+            userId: userId!,
             roomId: room.id,
             roomNumber: room.number,
             roomName: room.name,
@@ -148,12 +149,12 @@ export const roomRouter = createTRPCRouter({
             ? EventType.ENTERED_NEW_ROOM
             : roomEvent;
         await db.insert(events).values({
-          userId,
+          userId: userId!,
           event,
         });
 
         return {
-          userId,
+          userId: userId!,
           roomId: room!.id,
           roomNumber: room!.number,
           roomName: room!.name,

diff --git a/src/server/room-state/room-state.router.ts b/src/server/room-state/room-state.router.ts
@@ -2,6 +2,8 @@ import { z } from 'zod';
 
 import { fibonacciSequence } from 'fpp/constants/fibonacci.constant';
 
+import { validateNanoId } from 'fpp/utils/validate-nano-id.util';
+
 import { createTRPCRouter, publicProcedure } from 'fpp/server/api/trpc';
 import { EventType, type ICreateEvent, events } from 'fpp/server/db/schema';
 
@@ -19,9 +21,9 @@ export const roomStateRouter = createTRPCRouter({
     .input(
       z.object({
         roomId: z.number(),
-        userId: z
-          .string()
-          .regex(/^[A-Za-z0-9_~]{21}$/, 'userId regex mismatch'),
+        userId: z.string().refine((userId) => {
+          return validateNanoId(userId);
+        }, 'not a valid nanoId'),
         username: z
           .string()
           .min(3)
@@ -60,7 +62,14 @@ export const roomStateRouter = createTRPCRouter({
       return await getRoomStateOrFail(roomId);
     }),
   heartbeat: publicProcedure
-    .input(z.object({ roomId: z.number(), userId: z.string().length(21) }))
+    .input(
+      z.object({
+        roomId: z.number(),
+        userId: z.string().refine((userId) => {
+          return validateNanoId(userId);
+        }, 'not a valid nanoId'),
+      }),
+    )
     .mutation(async ({ ctx: { db }, input: { roomId, userId } }) => {
       const roomState = await getRoomStateOrNull(roomId);
 
@@ -98,7 +107,14 @@ export const roomStateRouter = createTRPCRouter({
       });
     }),
   flip: publicProcedure
-    .input(z.object({ roomId: z.number(), userId: z.string().length(21) }))
+    .input(
+      z.object({
+        roomId: z.number(),
+        userId: z.string().refine((userId) => {
+          return validateNanoId(userId);
+        }, 'not a valid nanoId'),
+      }),
+    )
     .mutation(async ({ ctx: { db }, input: { roomId, userId } }) => {
       const roomState = await getRoomStateOrFail(roomId);
 
@@ -115,7 +131,9 @@ export const roomStateRouter = createTRPCRouter({
     .input(
       z.object({
         roomId: z.number(),
-        userId: z.string().length(21),
+        userId: z.string().refine((userId) => {
+          return validateNanoId(userId);
+        }, 'not a valid nanoId'),
         estimation: z
           .number()
           .refine((estimation) => {
@@ -144,7 +162,9 @@ export const roomStateRouter = createTRPCRouter({
     .input(
       z.object({
         roomId: z.number(),
-        userId: z.string().length(21),
+        userId: z.string().refine((userId) => {
+          return validateNanoId(userId);
+        }, 'not a valid nanoId'),
         isSpectator: z.boolean(),
       }),
     )
@@ -166,7 +186,9 @@ export const roomStateRouter = createTRPCRouter({
     .input(
       z.object({
         roomId: z.number(),
-        userId: z.string().length(21),
+        userId: z.string().refine((userId) => {
+          return validateNanoId(userId);
+        }, 'not a valid nanoId'),
         isAutoFlip: z.boolean(),
       }),
     )
@@ -185,7 +207,14 @@ export const roomStateRouter = createTRPCRouter({
       },
     ),
   reset: publicProcedure
-    .input(z.object({ roomId: z.number(), userId: z.string().length(21) }))
+    .input(
+      z.object({
+        roomId: z.number(),
+        userId: z.string().refine((userId) => {
+          return validateNanoId(userId);
+        }, 'not a valid nanoId'),
+      }),
+    )
     .mutation(async ({ ctx: { db }, input: { roomId, userId } }) => {
       const roomState = await getRoomStateOrFail(roomId);
 
@@ -199,7 +228,14 @@ export const roomStateRouter = createTRPCRouter({
       });
     }),
   leave: publicProcedure
-    .input(z.object({ roomId: z.number(), userId: z.string().length(21) }))
+    .input(
+      z.object({
+        roomId: z.number(),
+        userId: z.string().refine((userId) => {
+          return validateNanoId(userId);
+        }, 'not a valid nanoId'),
+      }),
+    )
     .mutation(async ({ ctx: { db }, input: { roomId, userId } }) => {
       const roomState = await getRoomStateOrFail(roomId);
 

diff --git a/src/store/local-storage.store.ts b/src/store/local-storage.store.ts
@@ -1,6 +1,8 @@
 import * as Sentry from '@sentry/nextjs';
 import { create } from 'zustand';
 
+import { validateNanoId } from 'fpp/utils/validate-nano-id.util';
+
 import { RoomEvent } from 'fpp/server/db/schema';
 
 function saveToLocalstorage(key: string, value: string) {
@@ -58,9 +60,9 @@ export const useLocalstorageStore = create<LocalstorageStore>((set, get) => ({
   roomEvent: RoomEvent.ENTERED_ROOM_DIRECTLY,
   userId: (() => {
     const userId = getFromLocalstorage('userId');
-    // TODO: remove this after a while (2023-12-08)
-    if (userId?.length !== 21 && typeof window !== 'undefined') {
-      localStorage.removeItem('vote');
+    if (!validateNanoId(userId) && typeof window !== 'undefined') {
+      localStorage.removeItem('userId');
+      set({ userId: null });
       return null;
     }
     if (userId !== null) {
@@ -144,7 +146,7 @@ export const useLocalstorageStore = create<LocalstorageStore>((set, get) => ({
     set({ roomEvent });
   },
   setUserId: (userId: string) => {
-    if (get().userId === userId) {
+    if (get().userId === userId || !validateNanoId(userId)) {
       return;
     }
     Sentry.setUser({ id: userId });

diff --git a/src/store/room-state.store.ts b/src/store/room-state.store.ts
@@ -1,5 +1,7 @@
 import { create } from 'zustand';
 
+import { validateNanoId } from 'fpp/utils/validate-nano-id.util';
+
 import {
   type RoomStateClient,
   type User,
@@ -28,7 +30,7 @@ type RoomStateStore = {
 export const useRoomStateStore = create<RoomStateStore>((set, get) => ({
   // User
   userId: null,
-  setUserId: (userId) => set({ userId }),
+  setUserId: (userId) => validateNanoId(userId) && set({ userId }),
   estimation: null,
   isSpectator: false,
   // Game State

diff --git a/src/utils/db-api.util.ts b/src/utils/db-api.util.ts
@@ -3,16 +3,18 @@ import { type MySqlTable } from 'drizzle-orm/mysql-core/table';
 
 import { BadRequestError, NotFoundError } from 'fpp/constants/error.constant';
 
+import { validateNanoId } from 'fpp/utils/validate-nano-id.util';
+
 import db from 'fpp/server/db/db';
 import { type IUser, users } from 'fpp/server/db/schema';
 
 export async function findUserById(userId: string | null): Promise<IUser> {
-  if (!userId || userId.length !== 21) {
-    throw new BadRequestError('invalid visitorId');
+  if (!validateNanoId(userId)) {
+    throw new BadRequestError('invalid userId');
   }
 
   const user: IUser | null =
-    (await db.select().from(users).where(eq(users.id, userId)))[0] ?? null;
+    (await db.select().from(users).where(eq(users.id, userId!)))[0] ?? null;
 
   if (!user) {
     throw new NotFoundError('visitor not found');

diff --git a/src/utils/validate-nano-id.util.ts b/src/utils/validate-nano-id.util.ts
@@ -0,0 +1,8 @@
+export function validateNanoId(nanoId: string | null): boolean {
+  if (nanoId === null) return false;
+  const allowedChars =
+    '0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz_-';
+  return (
+    nanoId.length === 21 && [...nanoId].every((x) => allowedChars.includes(x))
+  );
+}