Fix P1/P2 security findings from adversarial review

assets/js/api/client.ts

@@ -3,7 +3,7 @@ import type { User, RecentGame } from '@/types/domain'
 const BASE = '/api'
 
 function authHeader(): Record<string, string> {
-  const token = localStorage.getItem('auth_token')
+  const token = sessionStorage.getItem('auth_token')
   return token ? { Authorization: `Bearer ${token}` } : {}
 }
 

assets/js/pages/Auth.vue

@@ -48,7 +48,7 @@ onMounted(async () => {
     window.history.replaceState({}, '', window.location.pathname)
     try {
       auth.token = token
-      localStorage.setItem('auth_token', token)
+      sessionStorage.setItem('auth_token', token)
       const { user: u } = await api.user.me()
       auth.login(u, token)
       router.replace((route.query.redirect as string) ?? '/')
@@ -103,7 +103,7 @@ async function verifyOtp() {
     const { token, user, needs_name } = await api.auth.verifyOtp(phone.value, otpCode.value)
     if (needs_name) {
       auth.token = token
-      localStorage.setItem('auth_token', token)
+      sessionStorage.setItem('auth_token', token)
       auth.login(user, token)
       phoneStep.value = 'name'
     } else {

assets/js/router.ts

@@ -42,4 +42,24 @@ router.beforeEach(async (to) => {
   if (to.meta.requiresAuth && !auth.isAuthenticated) {
     return { path: '/auth', query: { redirect: to.fullPath } }
   }
+
+  // Enforce host-only routes — redirect non-hosts to the player view
+  if (to.meta.requiresHost && auth.user) {
+    const code = to.params.code as string
+    if (code) {
+      try {
+        const res = await fetch(`/api/games/${code}`, {
+          headers: { Authorization: `Bearer ${auth.token}` },
+        })
+        if (res.ok) {
+          const { game } = await res.json()
+          if (game.host_id !== auth.user.id) {
+            return { path: `/game/${code}` }
+          }
+        }
+      } catch {
+        // If we can't verify, let the page handle it
+      }
+    }
+  }
 })

assets/js/stores/auth.ts

@@ -5,20 +5,20 @@ import { api } from '@/api/client'
 
 export const useAuthStore = defineStore('auth', () => {
   const user = ref<User | null>(null)
-  const token = ref<string | null>(localStorage.getItem('auth_token'))
+  const token = ref<string | null>(sessionStorage.getItem('auth_token'))
 
   const isAuthenticated = computed(() => user.value !== null && token.value !== null)
 
   function login(u: User, t: string) {
     user.value = u
     token.value = t
-    localStorage.setItem('auth_token', t)
+    sessionStorage.setItem('auth_token', t)
   }
 
   function logout() {
     user.value = null
     token.value = null
-    localStorage.removeItem('auth_token')
+    sessionStorage.removeItem('auth_token')
   }
 
   async function loadUser() {

assets/js/stores/game.ts

@@ -100,7 +100,14 @@ export const useGameStore = defineStore('game', () => {
       picks: [...board.value.picks, event.number],
       count: event.count,
     }
-    nextPickAt.value = event.next_pick_at
+    // Compensate for clock skew between server and client
+    if (event.server_now) {
+      const serverNow = new Date(event.server_now).getTime()
+      const offset = Date.now() - serverNow
+      nextPickAt.value = new Date(new Date(event.next_pick_at).getTime() + offset).toISOString()
+    } else {
+      nextPickAt.value = event.next_pick_at
+    }
 
     // Auto-strike if number is on any of my tickets and not yet struck and auto-strike is enabled
     const onMyTicket = myTickets.value.some(t => t.numbers.includes(event.number))

lib/mocha/auth/auth.ex

@@ -74,32 +74,30 @@ defmodule Mocha.Auth do
   ## Magic link tokens
 
   def build_magic_link_token(email) when is_binary(email) do
-    {token, token_record} =
-      case get_user_by_email(email) do
-        %User{} = user ->
-          {t, rec} = UserToken.build_magic_link_token(email)
-          {t, %{rec | user_id: user.id}}
-
-        nil ->
-          {:ok, user} = register(%{email: email, name: email_to_name(email)})
-          {t, rec} = UserToken.build_magic_link_token(email)
-          {t, %{rec | user_id: user.id}}
-      end
+    case get_user_by_email(email) do
+      %User{} = user ->
+        {t, rec} = UserToken.build_magic_link_token(email)
+        token_record = %{rec | user_id: user.id}
+        Repo.insert!(token_record)
+        {:ok, t, token_record}
 
-    Repo.insert!(token_record)
-    {token, token_record}
+      nil ->
+        # Don't auto-register unknown emails — prevents account creation spam
+        {:error, :user_not_found}
+    end
   end
 
   def verify_magic_link(token) when is_binary(token) do
     case UserToken.verify_token_query(token, "magic_link") do
       {:ok, query} ->
         case Repo.one(query) do
           {user, token_record} ->
-            token_record
-            |> Ecto.Changeset.change(used_at: DateTime.truncate(DateTime.utc_now(), :second))
-            |> Repo.update!()
+            # Atomic delete prevents replay — concurrent requests can't both succeed
+            {deleted, _} = Repo.delete_all(
+              from(t in UserToken, where: t.id == ^token_record.id and is_nil(t.used_at))
+            )
 
-            {:ok, user}
+            if deleted == 1, do: {:ok, user}, else: :error
 
           nil ->
             :error
@@ -143,6 +141,21 @@ defmodule Mocha.Auth do
 
   ## Token management
 
+  def delete_api_token(raw_token) when is_binary(raw_token) do
+    case UserToken.verify_token_query(raw_token, "api") do
+      {:ok, query} ->
+        case Repo.one(query) do
+          {_user, token_record} -> Repo.delete(token_record)
+          nil -> :ok
+        end
+
+      :error ->
+        :ok
+    end
+
+    :ok
+  end
+
   def revoke_all_tokens(%User{} = user) do
     Repo.delete_all(from t in UserToken, where: t.user_id == ^user.id)
     :ok
@@ -155,16 +168,6 @@ defmodule Mocha.Auth do
     |> Map.new()
   end
 
-  ## Helpers
-
-  defp email_to_name(email) do
-    email
-    |> String.split("@")
-    |> List.first()
-    |> String.replace(~r/[._]/, " ")
-    |> String.capitalize()
-  end
-
   ## Phone OTP
 
   @otp_expiry_seconds 600

lib/mocha/game/board.ex

@@ -36,7 +36,7 @@ defmodule Mocha.Game.Board do
   @doc "Restores a board from a snapshot map."
   def from_snapshot(%{"picks" => picks, "count" => count}) do
     picked_set = MapSet.new(picks)
-    remaining = Enum.reject(1..90, &MapSet.member?(picked_set, &1)) |> Enum.shuffle()
+    remaining = Enum.reject(1..90, &MapSet.member?(picked_set, &1)) |> Enum.sort()
     %__MODULE__{bag: remaining, picks: picks, count: count}
   end
 end

lib/mocha/game/code.ex

@@ -1,7 +1,7 @@
 defmodule Mocha.Game.Code do
   @moduledoc """
-  Generates human-friendly room codes in WORD-NN format.
-  Word list: ~2000 common English words. Code space: ~200,000.
+  Generates human-friendly room codes in WORD-NNN format.
+  Word list: ~250 words × 1000 numbers = ~250,000 codes.
   """
 
   @words ~w(
@@ -55,8 +55,8 @@ defmodule Mocha.Game.Code do
 
   defp generate_with_retries(excluded, retries) do
     word = Enum.random(@words)
-    number = :rand.uniform(100) - 1
-    code = "#{word}-#{String.pad_leading(Integer.to_string(number), 2, "0")}"
+    number = :rand.uniform(1000) - 1
+    code = "#{word}-#{String.pad_leading(Integer.to_string(number), 3, "0")}"
 
     if MapSet.member?(excluded, code) do
       generate_with_retries(excluded, retries - 1)

lib/mocha/game/game.ex

@@ -13,7 +13,24 @@ defmodule Mocha.Game do
     join_secret: nil
   }
 
+  @max_active_games_per_user 5
+
   def create_game(host_id, attrs) do
+    # Limit active games per host using Registry metadata
+    active_host_games =
+      Registry.select(Mocha.Game.Registry, [{{:_, :_, :"$1"}, [], [:"$1"]}])
+      |> Enum.count(fn meta ->
+        meta.host_id == host_id and meta.status in [:lobby, :running, :paused]
+      end)
+
+    if active_host_games >= @max_active_games_per_user do
+      {:error, :too_many_games}
+    else
+      do_create_game(host_id, attrs)
+    end
+  end
+
+  defp do_create_game(host_id, attrs) do
     settings = Map.merge(@default_settings, Map.get(attrs, :settings, %{}))
     settings = validate_settings(settings)
 
@@ -22,28 +39,37 @@ defmodule Mocha.Game do
       |> MapSet.new()
 
     code = Code.generate(existing_codes)
+    name = non_empty(attrs[:name]) || non_empty(attrs["name"]) || "Untitled Game"
+
+    # Use transaction to ensure DB record and GenServer are created atomically
+    result =
+      Repo.transaction(fn ->
+        case %Record{}
+             |> Record.changeset(%{code: code, name: name, host_id: host_id, settings: settings})
+             |> Repo.insert() do
+          {:ok, record} ->
+            case DynamicSupervisor.start_child(Mocha.Game.DynSup, {
+                   Server,
+                   %{
+                     code: code,
+                     name: record.name,
+                     host_id: host_id,
+                     settings: settings,
+                     game_record_id: record.id
+                   }
+                 }) do
+              {:ok, _pid} -> code
+              {:error, reason} -> Repo.rollback(reason)
+            end
+
+          {:error, changeset} ->
+            Repo.rollback(changeset)
+        end
+      end)
 
-    with {:ok, record} <-
-           %Record{}
-           |> Record.changeset(%{
-             code: code,
-             name: non_empty(attrs[:name]) || non_empty(attrs["name"]) || "Untitled Game",
-             host_id: host_id,
-             settings: settings
-           })
-           |> Repo.insert(),
-         {:ok, _pid} <-
-           DynamicSupervisor.start_child(Mocha.Game.DynSup, {
-             Server,
-             %{
-               code: code,
-               name: record.name,
-               host_id: host_id,
-               settings: settings,
-               game_record_id: record.id
-             }
-           }) do
-      {:ok, code}
+    case result do
+      {:ok, code} -> {:ok, code}
+      {:error, _} -> {:error, :create_failed}
     end
   end
 
@@ -123,27 +149,20 @@ defmodule Mocha.Game do
   end
 
   def list_public_games do
-    Registry.select(Mocha.Game.Registry, [{{:"$1", :"$2", :_}, [], [{{:"$1", :"$2"}}]}])
-    |> Enum.map(fn {_code, pid} ->
-      try do
-        state = Server.get_state(pid)
-        vis = Map.get(state.settings, :visibility) || Map.get(state.settings, "visibility", "public")
-        if vis == "public" and state.status in [:lobby, :running] do
-          %{
-            code: state.code,
-            name: state.name,
-            status: state.status,
-            host_id: state.host_id,
-            players_count: MapSet.size(state.players)
-          }
-        else
-          nil
-        end
-      catch
-        :exit, _ -> nil
-      end
+    # Read from Registry metadata — no GenServer calls needed
+    Registry.select(Mocha.Game.Registry, [{{:"$1", :_, :"$3"}, [], [{{:"$1", :"$3"}}]}])
+    |> Enum.filter(fn {_code, meta} ->
+      meta.visibility == "public" and meta.status in [:lobby, :running]
+    end)
+    |> Enum.map(fn {code, meta} ->
+      %{
+        code: code,
+        name: meta.name,
+        status: meta.status,
+        host_id: meta.host_id,
+        players_count: meta.player_count
+      }
     end)
-    |> Enum.reject(&is_nil/1)
   end
 
   def clone_game(old_code, host_id) do

lib/mocha/game/monitor.ex

@@ -45,23 +45,19 @@ defmodule Mocha.Game.Monitor do
     now = System.monotonic_time(:millisecond)
 
     Enum.each(games, fn {code, pid, meta} ->
-      try do
-        state = GenServer.call(pid, :state, 5_000)
-
-        cond do
-          state.status == :lobby and stale?(meta, now, @lobby_timeout) ->
-            Logger.info("Reaping stale lobby game: #{code}")
-            DynamicSupervisor.terminate_child(Mocha.Game.DynSup, pid)
-
-          state.status == :finished and stale?(meta, now, @finished_cooldown) ->
-            Logger.info("Reaping finished game: #{code}")
-            DynamicSupervisor.terminate_child(Mocha.Game.DynSup, pid)
-
-          true ->
-            :ok
-        end
-      catch
-        :exit, _ -> :ok
+      status = Map.get(meta, :status, :lobby)
+
+      cond do
+        status == :lobby and stale?(meta, now, @lobby_timeout) ->
+          Logger.info("Reaping stale lobby game: #{code}")
+          DynamicSupervisor.terminate_child(Mocha.Game.DynSup, pid)
+
+        status == :finished and stale?(meta, now, @finished_cooldown) ->
+          Logger.info("Reaping finished game: #{code}")
+          DynamicSupervisor.terminate_child(Mocha.Game.DynSup, pid)
+
+        true ->
+          :ok
       end
     end)
   end