From b17b925b3b0bdc0791e9160c0531de3e0706ee62 Mon Sep 17 00:00:00 2001
From: Scott Anderson <scottanderson@users.noreply.github.com>
Date: Mon, 16 Jun 2025 21:45:30 -0400
Subject: [PATCH] Better handling of bad tokens (#1180)

## Description:

- Better handling of bad tokens
- Send a code and reason when closing the websocket more consistently.

## Please complete the following:

- [x] I have added screenshots for all UI updates
- [x] I process any text displayed to the user through translateText()
and I've added it to the en.json file
- [x] I have added relevant tests to the test directory
- [x] I confirm I have thoroughly tested these changes and take full
responsibility for any bugs introduced
- [x] I understand that submitting code with bugs that could have been
caught through manual testing blocks releases and new features for all
contributors

Co-authored-by: Scott Anderson <662325+scottanderson@users.noreply.github.com>
---
 src/core/Schemas.ts      |  2 +-
 src/server/GameServer.ts |  4 ++--
 src/server/Worker.ts     | 22 +++++++++++++--------
 src/server/jwt.ts        | 41 +++++++++++++++++++++++-----------------
 4 files changed, 41 insertions(+), 28 deletions(-)

diff --git a/src/core/Schemas.ts b/src/core/Schemas.ts
index 285c704a7..bc6903bf5 100644
--- a/src/core/Schemas.ts
+++ b/src/core/Schemas.ts
@@ -144,7 +144,7 @@ const SafeString = z
   )
   .max(1000);
 
-const PersistentIdSchema = z.string().uuid();
+export const PersistentIdSchema = z.string().uuid();
 const JwtTokenSchema = z.string().jwt();
 const TokenSchema = z
   .string()
diff --git a/src/server/GameServer.ts b/src/server/GameServer.ts
index 57fced752..f08dcc5e8 100644
--- a/src/server/GameServer.ts
+++ b/src/server/GameServer.ts
@@ -193,7 +193,7 @@ export class GameServer {
             this.log.error("Failed to parse client message", error, {
               clientID: client.clientID,
             });
-            client.ws.close();
+            client.ws.close(1002, "ClientMessageSchema");
             return;
           }
           const clientMsg = parsed.data;
@@ -253,7 +253,7 @@ export class GameServer {
     client.ws.removeAllListeners("error");
     client.ws.on("error", (error: Error) => {
       if ((error as any).code === "WS_ERR_UNEXPECTED_RSV_1") {
-        client.ws.close(1002);
+        client.ws.close(1002, "WS_ERR_UNEXPECTED_RSV_1");
       }
     });
 
diff --git a/src/server/Worker.ts b/src/server/Worker.ts
index 5bee6b603..2dd8f1490 100644
--- a/src/server/Worker.ts
+++ b/src/server/Worker.ts
@@ -300,7 +300,7 @@ export function startWorker() {
           if (!parsed.success) {
             const error = z.prettifyError(parsed.error);
             log.warn("Error parsing join message client", error);
-            ws.close();
+            ws.close(1002, "ClientJoinMessageSchema");
             return;
           }
           const clientMsg = parsed.data;
@@ -315,18 +315,24 @@ export function startWorker() {
               return;
             }
 
-            const { persistentId, claims } = await verifyClientToken(
-              clientMsg.token,
-              config,
-            );
+            const result = await verifyClientToken(clientMsg.token, config);
+            if (result === false) {
+              log.warn("Failed to verify token");
+              ws.close(1002, "Failed to verify token");
+              return;
+            }
+            const { persistentId, claims } = result;
 
             let roles: string[] | undefined;
 
-            // Check user roles
-            if (claims !== null) {
+            if (claims === null) {
+              // TODO: Verify that the persistendId is is not a registered player
+            } else {
+              // Verify token and get player permissions
               const result = await getUserMe(clientMsg.token, config);
               if (result === false) {
                 log.warn("Token is not valid", claims);
+                ws.close(1002, "Token is not valid");
                 return;
               }
               roles = result.player.roles;
@@ -374,7 +380,7 @@ export function startWorker() {
 
     ws.on("error", (error: Error) => {
       if ((error as any).code === "WS_ERR_UNEXPECTED_RSV_1") {
-        ws.close(1002);
+        ws.close(1002, "WS_ERR_UNEXPECTED_RSV_1");
       }
     });
   });
diff --git a/src/server/jwt.ts b/src/server/jwt.ts
index c7896bd91..d056b8445 100644
--- a/src/server/jwt.ts
+++ b/src/server/jwt.ts
@@ -6,31 +6,38 @@ import {
   UserMeResponseSchema,
 } from "../core/ApiSchemas";
 import { ServerConfig } from "../core/configuration/Config";
+import { PersistentIdSchema } from "../core/Schemas";
 
-type TokenVerificationResult = {
-  persistentId: string;
-  claims: TokenPayload | null;
-};
+type TokenVerificationResult =
+  | {
+      persistentId: string;
+      claims: TokenPayload | null;
+    }
+  | false;
 
 export async function verifyClientToken(
   token: string,
   config: ServerConfig,
 ): Promise<TokenVerificationResult> {
-  if (token.length === 36) {
+  if (PersistentIdSchema.safeParse(token).success) {
     return { persistentId: token, claims: null };
   }
-  const issuer = config.jwtIssuer();
-  const audience = config.jwtAudience();
-  const key = await config.jwkPublicKey();
-  const { payload, protectedHeader } = await jwtVerify(token, key, {
-    algorithms: ["EdDSA"],
-    issuer,
-    audience,
-    maxTokenAge: "6 days",
-  });
-  const claims = TokenPayloadSchema.parse(payload);
-  const persistentId = claims.sub;
-  return { persistentId, claims };
+  try {
+    const issuer = config.jwtIssuer();
+    const audience = config.jwtAudience();
+    const key = await config.jwkPublicKey();
+    const { payload, protectedHeader } = await jwtVerify(token, key, {
+      algorithms: ["EdDSA"],
+      issuer,
+      audience,
+      maxTokenAge: "6 days",
+    });
+    const claims = TokenPayloadSchema.parse(payload);
+    const persistentId = claims.sub;
+    return { persistentId, claims };
+  } catch (e) {
+    return false;
+  }
 }
 
 export async function getUserMe(