Remove UUID

docs/GUEST_AUTHENTICATION_FLOW.md

@@ -8,6 +8,6 @@ A hit to the `/verify` endpoint verifies if a user is *identified* or a *guest*
 
 An uncredentialed user can acquire guest credentials via the `/gauth` endpoint (*guest auth*). These credentials allow a user to create a *guest user* on the `/user` endpoint.
 
-A *guest user* does not have a password and cannot log in, but can comment according to `policy` settings, and can edit non-admin properties like `name` and `email`. *guest auth* credentials are expected to be stored locally in the visitor's browser and reused. The guest user's id is always a `uuid` and therefore an *identifed user* can never have a `uuid` as its id.
+A *guest user* does not have a password and cannot log in, but can comment according to `policy` settings, and can edit non-admin properties like `name` and `email`. *guest auth* credentials are expected to be stored locally in the visitor's browser and reused. The guest user's id is always of the form `guest-xxxxx-xxxxx` and therefore an *identifed user* cannot have this id format.
 
 If an *identified user* logs in or is created with *guest auth* credentials of a guest user, that guest user can be linked to the *identified* user.

package.json

@@ -4,8 +4,7 @@
     "@types/bcryptjs": "^2.4.2",
     "@types/jest": "^29.5.3",
     "@types/jsonwebtoken": "^8.5.0",
-    "@types/picomatch": "^2.3.0",
-    "@types/uuid": "^8.3.0"
+    "@types/picomatch": "^2.3.0"
   },
   "optionalDependencies": {
     "cypress": "^12.17.4",
@@ -83,7 +82,6 @@
     "ts-loader": "^9.3.0",
     "ts-node": "^10.1.0",
     "typescript": "^5.1.6",
-    "uuid": "8.3.2",
     "webpack": "^5.76.0",
     "webpack-bundle-analyzer": "^4.9.0",
     "webpack-cli": "^4.9.2",

src/frontend-utilities.ts

@@ -100,7 +100,7 @@ const transliterate = (char: string = ""): string =>
   ) ?? [char, undefined])[0]
 
 /** Convert a display name to a default user id */
-export const formatUserId = displayName => {
+export const formatUserId = (displayName: string) => {
   return displayName
     .trim()
     .toLowerCase()

src/lib/MongodbService.ts

@@ -25,21 +25,22 @@ import { MongoClient } from "mongodb"
 import { Service } from "./Service"
 import {
   adminOnlyModifiableUserProperties,
+  generateCommentId,
+  generateGuestId,
+  getAllowedOrigins,
+  isAllowedReferer,
   isComment,
   isDeleted,
-  isGuestId,
   isDeletedComment,
+  isEmail,
+  normalizeUrl,
   toAdminSafeUser,
   toPublicSafeUser,
   toSafeUser,
   toTopic,
   toUpdatedUser,
-  validateUser,
-  isAllowedReferer,
-  getAllowedOrigins,
-  isEmail,
-  normalizeUrl,
   validateGuestUser,
+  validateUser,
 } from "./backend-utilities"
 import policy from "../policy.json"
 import {
@@ -68,9 +69,9 @@ import {
   success204CommentUpdated,
   success204UserUpdated,
 } from "./messages"
-import { comparePassword, getAuthToken, hashPassword, uuidv4 } from "./crypt"
+import { comparePassword, getAuthToken, hashPassword } from "./crypt"
 import * as jwt from "jsonwebtoken"
-import { isValidResult } from "./shared-utilities"
+import { isGuestId, isValidResult } from "./shared-utilities"
 export class MongodbService extends Service {
   private isCrossSite = process.env.IS_CROSS_SITE === "true"
   private _client: MongoClient
@@ -554,7 +555,7 @@ export class MongodbService extends Service {
     }
 
     const adminSafeUser = toAdminSafeUser(authUser)
-    const id = uuidv4()
+    const id = generateCommentId(parentId)
     const insertComment: Comment = {
       id,
       text,
@@ -957,7 +958,7 @@ export class MongodbService extends Service {
         })
         return
       }
-      const guestUserId = uuidv4()
+      const guestUserId = generateGuestId()
       const gauthToken = getAuthToken(guestUserId)
       resolve({ ...success200OK, body: gauthToken })
     })

src/lib/backend-utilities.ts

@@ -1,5 +1,5 @@
 /** Server-side utilities.  Using any of these in the front end will lead to darkness and despair. */
-import { validate as isUuid } from "uuid"
+import crypto from "crypto"
 import * as jwt from "jsonwebtoken"
 import * as dotenv from "dotenv"
 import * as picomatch from "picomatch"
@@ -20,9 +20,9 @@ import type {
   Email,
   ValidationResult,
 } from "./simple-comment-types"
-import { uuidv4 } from "./crypt"
 import urlNormalizer from "normalize-url"
 import {
+  isGuestId,
   joinValidations,
   validateDisplayName,
   validateEmail,
@@ -39,13 +39,44 @@ const BASIC_SCHEME = "Basic"
 const isDefined = <T>(x: T | undefined | null): x is T =>
   x !== undefined && x !== null
 
+const dateToString = (d = new Date()) =>
+  (d.getFullYear() * 10000 + d.getMonth() * 100 + d.getDate()).toString(36)
+const timeToString = (d = new Date()) =>
+  (d.getHours() * 10000 + d.getMinutes() * 100 + d.getSeconds()).toString(36)
+const generateAlpha = (length = 12) =>
+  generateString("abcdefghijklmnopqrstuvwxyz", length)
+const generateNumeric = (length = 12) => generateString("0123456789", length)
+const generateAlphaNumeric = (length = 12): string =>
+  generateString("abcdefghijklmnopqrstuvwxyz0123456789", length)
+const generateString = (alpha: string, length = 12, id = ""): string =>
+  length <= 0
+    ? id
+    : generateString(
+        alpha,
+        length - 1,
+        id + alpha.charAt(crypto.randomInt(alpha.length))
+      )
+
 /** Creates an id specifically for a guest */
-export const createGuestId = () => uuidv4()
+export const generateGuestId = () =>
+  `guest-${generateAlpha(2)}${generateNumeric(3)}-${dateToString()}`
+
+/** Creates an id for a comment */
+export const generateCommentId = (parentId = "") => {
+  const cId = `${generateAlphaNumeric(3)}-${timeToString()}-${dateToString()}`
+  if (parentId === "") return cId
+  const appendIndex = parentId.lastIndexOf("_")
+  const pId = parentId.slice(appendIndex + 1)
+  if (pId === "") return cId
+
+  // if the commentId will be longer than 36 characters, truncate it
+  if (pId.length > 36 - cId.length - 1) {
+    const to36 = pId.slice(0, 36)
+    return `${to36.slice(0, -cId.length - 1)}_${cId}`
+  }
 
-/**
- * Returns true if userId is a guest id
- */
-export const isGuestId = (userId: UserId) => isUuid(userId)
+  return `${pId}_${cId}`
+}
 
 /**
  * These are user properties that are unsafe to return to admins

src/lib/crypt.ts

@@ -1,4 +1,3 @@
-import { v4, validate as isValidUuid } from "uuid"
 import { hash, compare } from "bcryptjs"
 import * as jwt from "jsonwebtoken"
 import * as dotenv from "dotenv"
@@ -33,7 +32,3 @@ export const getAuthToken = (
   user: string,
   exp: number = getExpirationTime(YEAR_SECONDS)
 ): string => jwt.sign({ user, exp }, process.env.JWT_SECRET)
-
-export const uuidv4 = () => v4()
-
-export const isUuid = isValidUuid

src/lib/policyEnforcement.ts

@@ -1,7 +1,7 @@
 import type { ActionType, Policy, UserId } from "./simple-comment-types"
 import { Action } from "./simple-comment-types"
 import policy from "../policy.json"
-import { isGuestId } from "../lib/backend-utilities"
+import { isGuestId } from "./shared-utilities"
 
 /** Return true iff action can be performed by user according to
  * policy, false otherwise

src/lib/shared-utilities.ts

@@ -1,5 +1,6 @@
 import type {
   InvalidResult,
+  UserId,
   ValidResult,
   ValidationResult,
 } from "./simple-comment-types"
@@ -89,3 +90,6 @@ export const validateDisplayName = (name: string): ValidationResult => {
     return { isValid: false, reason: "Display name is too long." }
   return { isValid: true }
 }
+
+export const isGuestId = (userId?: UserId) =>
+  userId && userId.match(/^guest-[a-z]{2}\d{3}-[a-z0-9]{5}$/) !== null

src/tests/backend/MongodbService.test.ts

@@ -33,7 +33,7 @@ import {
 import { getAuthToken, hashPassword } from "../../../src/lib/crypt"
 import policy from "../../policy.json"
 import {
-  createGuestId,
+  generateGuestId,
   isComment,
   isDeletedComment,
   isDiscussion,
@@ -247,14 +247,14 @@ describe("Full API service test", () => {
   })
 
   test("POST to /user with guest id and admin credentials should fail", async () => {
-    const guestUser = { ...testNewUser, id: createGuestId() }
+    const guestUser = { ...testNewUser, id: generateGuestId() }
     const authUser = getAuthUser(u => u.isAdmin!)
     expect.assertions(1)
     const e = await service.userPOST(guestUser, authUser.id)
     expect(e).toHaveProperty("statusCode", 403)
   })
   test("POST to /user with guest id and same credentials should succeed", () => {
-    const id = createGuestId()
+    const id = generateGuestId()
     const guestUser = {
       id,
       name: randomString(alphaUserInput),
@@ -341,7 +341,7 @@ describe("Full API service test", () => {
   test("POST to /user with a guestUserId as targetId should fail", async () => {
     const newUser = {
       ...mockUser(),
-      id: createGuestId(),
+      id: generateGuestId(),
       password: mockPassword(),
       email: mockEmail(),
       isAdmin: false,
@@ -567,7 +567,7 @@ describe("Full API service test", () => {
 
   test("Admins should be able to modify guest users", async () => {
     // PUT to /user/{userId} with userId as guestId should succeed
-    const id = createGuestId()
+    const id = generateGuestId()
     const guestUser = {
       id,
       name: randomString(alphaUserInput),
@@ -595,7 +595,7 @@ describe("Full API service test", () => {
 
   test("Guest users should never be made into admins", async () => {
     // PUT to /user/{userId} with guestUser  changing isAdmin should fail
-    const id = createGuestId()
+    const id = generateGuestId()
     const guestUser = {
       id,
       name: randomString(alphaUserInput),
@@ -737,7 +737,7 @@ describe("Full API service test", () => {
 
   // Comment Read
   test("GET comment to /comment/{commentId} where commentId does not exist should return 404", async () => {
-    const parentCommentId = createGuestId()
+    const parentCommentId = generateGuestId()
     const user = getAuthUser()
     expect.assertions(1)
     const e = await service.commentGET(parentCommentId, user.id)
@@ -817,7 +817,7 @@ describe("Full API service test", () => {
     expect(e).toBe(error403UserNotAuthorized)
   })
   test("PUT comment to /comment/{commentId} where Id does not exist should return 404", async () => {
-    const unknownComment = { text: randomString(), id: createGuestId() }
+    const unknownComment = { text: randomString(), id: generateGuestId() }
     const adminUserTest = authUserTest
     expect.assertions(1)
     const e = await service.commentPUT(
@@ -1035,7 +1035,7 @@ describe("Full API service test", () => {
       .catch(e => expect(e).toBe(error403UserNotAuthorized))
   })
   test("DELETE to /topic/{topicId} where Id does not exist should return 404", () => {
-    const deleteTopicId = createGuestId()
+    const deleteTopicId = generateGuestId()
     const adminUserTest = getAuthUser(u => u.isAdmin!)
     return service
       .topicDELETE(deleteTopicId, adminUserTest.id)

src/tests/backend/policyEnforcement.test.ts

@@ -1,3 +1,4 @@
+import { generateGuestId } from "../../lib/backend-utilities"
 import { isUserAllowedTo } from "../../lib/policyEnforcement"
 import type { Policy, User } from "../../lib/simple-comment-types"
 import { Action } from "../../lib/simple-comment-types"
@@ -16,7 +17,7 @@
 }
 
 const mockGuestUser: User = {
-  id: "6e3c9dd2-fb1f-4c10-8d19-c6bdc0156b07",
+  id: "guest-ih517-c1m4i",
   name: "",
   email: "",
 }

src/tests/backend/utilities.test.ts

@@ -1,13 +1,18 @@
 import {
   addHeaders,
+  generateGuestId,
   getAllowOriginHeaders,
+  generateCommentId,
   isAllowedReferer,
-  isGuestId,
   parseQuery,
 } from "../../../src/lib/backend-utilities"
-import { v4 as uuidv4 } from "uuid"
 import type { Email } from "../../../src/lib/simple-comment-types"
-import { validateEmail, validateUserId } from "../../lib/shared-utilities"
+import {
+  isGuestId,
+  validateEmail,
+  validateUserId,
+} from "../../lib/shared-utilities"
+import { mockUserId } from "../mockData"
 
 describe("test the `getAllowOriginHeaders` function", () => {
   it("should return {headers} if there is a header match", () => {
@@ -47,21 +52,52 @@ describe("test the `getAllowOriginHeaders` function", () => {
 })
 
 describe("Test guest id utility", () => {
-  test("isGuestId should fail anything other than a uuid", () => {
+  test("isGuestId should pass id from generateGuestId", () => {
     //TODO: make this more random
     expect(isGuestId("rendall")).toBe(false)
-    const guestId = uuidv4()
+    const guestId = generateGuestId()
     expect(isGuestId(guestId)).toBe(true)
   })
 })
+describe("generateCommentId", () => {
+  const commentPattern = "[a-z0-9]{3}-[a-z0-9]{4}-[a-z0-9]{5}"
+  test("generates a comment ID with a given parent ID", () => {
+    const commentId = generateCommentId("topic-1")
+    const expectedRegex = new RegExp(`^topic-1_${commentPattern}$`)
+    expect(commentId).toMatch(expectedRegex)
+  })
+
+  test("has parent id in string", () => {
+    const cId = "tuw-26xt-c1m4i"
+    const pId = "not_in-string"
+    const parentId = `${pId}_${cId}`
+    const commentId = generateCommentId(parentId)
+    const regex = new RegExp(`^${cId}_${commentPattern}`)
+    expect(commentId).toContain(cId)
+    expect(commentId).toMatch(regex)
+    expect(commentId).not.toContain(pId)
+  })
+
+  test("generates a comment ID without a parent ID", () => {
+    const commentId = generateCommentId()
+    const regex = new RegExp(`^${commentPattern}$`)
+    expect(commentId).toMatch(regex)
+  })
+
+  test("generates unique comment IDs", () => {
+    const commentId1 = generateCommentId("parent1")
+    const commentId2 = generateCommentId("parent2")
+    expect(commentId1).not.toBe(commentId2)
+  })
+})
 
 describe("Test validations", () => {
   test("good validateUserId", () => {
     expect(validateUserId("rendall-775-id")).toEqual({ isValid: true })
   })
 
-  test("uuidv4 in validateUserId", () => {
-    expect(validateUserId(uuidv4())).toEqual({ isValid: true })
+  test("guestId is validateUserId", () => {
+    expect(validateUserId(generateGuestId())).toEqual({ isValid: true })
   })
 
   test("incorrect characters in validateUserId", () => {
@@ -80,8 +116,8 @@ describe("Test validations", () => {
     })
   })
 
-  test("too many  characters in validateUserId", () => {
-    const tooMany = uuidv4() + "a"
+  test("too many characters in validateUserId", () => {
+    const tooMany = mockUserId(37)
     expect(tooMany.length).toBeGreaterThan(36)
     expect(validateUserId(tooMany)).toEqual({
       isValid: false,