Fix voting security: block voting before game starts and after game ends

copi.owasp.org/lib/copi/cornucopia/game.ex

@@ -62,4 +62,8 @@ defmodule Copi.Cornucopia.Game do
 
     Enum.count(players_still_to_play) > 0
   end
+
+  def game_active?(game) do
+    game.started_at != nil and game.finished_at == nil
+  end
 end

copi.owasp.org/lib/copi_web/live/player_live/show.ex

@@ -96,59 +96,80 @@ defmodule CopiWeb.PlayerLive.Show do
     game = socket.assigns.game
     player = socket.assigns.player
 
-    # Check if player already voted
-    if Copi.Cornucopia.Game.has_continue_vote?(game, player) do
-      # Remove their vote
-      continue_vote = Enum.find(game.continue_votes, fn vote -> vote.player_id == player.id end)
-      if continue_vote do
-        Copi.Repo.delete!(continue_vote)
-      end
+    # Validate game lifecycle - continue voting only allowed during active games
+    unless Copi.Cornucopia.Game.game_active?(game) do
+      Logger.warning("Continue vote attempt on inactive game: player_id: #{player.id}, game_id: #{game.id}, started_at: #{game.started_at}, finished_at: #{game.finished_at}")
+      {:noreply, socket}
     else
-      # Add their vote
-      Logger.debug("Adding continue vote for player_id: #{player.id}, game_id: #{game.id}")
-      Copi.Repo.insert(%Copi.Cornucopia.ContinueVote{player_id: player.id, game_id: game.id})
-    end
-
-    {:ok, updated_game} = Game.find(game.id)
-
-    CopiWeb.Endpoint.broadcast(topic(updated_game.id), "game:updated", updated_game)
+      # Check if player already voted
+      if Copi.Cornucopia.Game.has_continue_vote?(game, player) do
+        # Remove their vote
+        continue_vote = Enum.find(game.continue_votes, fn vote -> vote.player_id == player.id end)
+        if continue_vote do
+          Copi.Repo.delete!(continue_vote)
+        end
+      else
+        # Add their vote
+        case Copi.Repo.insert(%Copi.Cornucopia.ContinueVote{player_id: player.id, game_id: game.id}) do
+          {:ok, _vote} ->
+            Logger.debug("Continue vote added successfully for player_id: #{player.id}, game_id: #{game.id}")
+          {:error, changeset} ->
+            Logger.warning("Continue voting failed for player_id: #{player.id}, game_id: #{game.id}, errors: #{inspect(changeset.errors)}")
+        end
+      end
 
-    {:noreply, assign(socket, :game, updated_game)}
+      # Reload fresh game record after continue vote mutations
+      case Game.find(game.id) do
+        {:ok, updated_game} ->
+          CopiWeb.Endpoint.broadcast(topic(updated_game.id), "game:updated", updated_game)
+          {:noreply, assign(socket, :game, updated_game)}
+        {:error, reason} ->
+          Logger.warning("Failed to reload game after continue vote: game_id: #{game.id}, reason: #{inspect(reason)}")
+          {:noreply, socket}
+      end
+    end
   end
 
   @impl true
   def handle_event("toggle_vote", %{"dealt_card_id" => dealt_card_id}, socket) do
     game = socket.assigns.game
     player = socket.assigns.player
 
-    {:ok, dealt_card} = DealtCard.find(dealt_card_id)
-
-    game_card_ids = game.players
-      |> Enum.flat_map(fn p -> p.dealt_cards end)
-      |> Enum.map(fn dc -> dc.id end)
-
-    if dealt_card.id in game_card_ids do
-      vote = get_vote(dealt_card, player)
-
-      if vote do
-        Logger.debug("Player has voted: player_id: #{player.id}, dealt_card_id: #{dealt_card_id}, game_id: #{game.id}")
-        Copi.Repo.delete!(vote)
-      else
-        Logger.debug("Player has not voted: player_id: #{player.id}, dealt_card_id: #{dealt_card_id}, game_id: #{game.id}")
-        case Copi.Repo.insert(%Copi.Cornucopia.Vote{dealt_card_id: String.to_integer(dealt_card_id), player_id: player.id}) do
-          {:ok, _vote} ->
-            Logger.debug("Vote added successfully for player_id: #{player.id}, dealt_card_id: #{dealt_card_id}, game_id: #{game.id}")
-          {:error, changeset} ->
-            Logger.warning("Voting failed for player_id: #{player.id}, dealt_card_id: #{dealt_card_id}, game_id: #{game.id}, errors: #{inspect(changeset.errors)}")
-        end
-      end
-
-      {:ok, updated_game} = Game.find(game.id)
-      CopiWeb.Endpoint.broadcast(topic(updated_game.id), "game:updated", updated_game)
-      {:noreply, assign(socket, :game, updated_game)}
+    # Validate game lifecycle - voting only allowed during active games
+    unless Copi.Cornucopia.Game.game_active?(game) do
+      Logger.warning("Voting attempt on inactive game: player_id: #{player.id}, game_id: #{game.id}, started_at: #{game.started_at}, finished_at: #{game.finished_at}")
+      {:noreply, socket}
     else
-      Logger.warning("Unauthorized vote attempt: player_id: #{player.id}, dealt_card_id: #{dealt_card_id}, game_id: #{game.id}")
-      {:noreply, socket |> put_flash(:error, "Invalid card selection")}
+      case DealtCard.find(dealt_card_id) do
+        {:ok, dealt_card} ->
+          # Validate that dealt card belongs to current game
+          unless dealt_card_belongs_to_game?(dealt_card, game) do
+            Logger.warning("Unauthorized voting attempt: player_id: #{player.id}, dealt_card_id: #{dealt_card_id}, game_id: #{game.id}")
+            {:noreply, socket}
+          else
+            vote = get_vote(dealt_card, player)
+
+            if vote do
+              Logger.debug("Player has voted: player_id: #{player.id}, dealt_card_id: #{dealt_card_id}, game_id: #{game.id}")
+              Copi.Repo.delete!(vote)
+            else
+              Logger.debug("Player has not voted: player_id: #{player.id}, dealt_card_id: #{dealt_card_id}, game_id: #{game.id}")
+              case Copi.Repo.insert(%Copi.Cornucopia.Vote{dealt_card_id: String.to_integer(dealt_card_id), player_id: player.id}) do
+                {:ok, _vote} ->
+                  Logger.debug("Vote added successfully for player_id: #{player.id}, dealt_card_id: #{dealt_card_id}, game_id: #{game.id}")
+                {:error, changeset} ->
+                  Logger.warning("Voting failed for player_id: #{player.id}, dealt_card_id: #{dealt_card_id}, game_id: #{game.id}, errors: #{inspect(changeset.errors)}")
+              end
+            end
+
+            {:ok, updated_game} = Game.find(game.id)
+            CopiWeb.Endpoint.broadcast(topic(updated_game.id), "game:updated", updated_game)
+            {:noreply, assign(socket, :game, updated_game)}
+          end
+        {:error, reason} ->
+          Logger.warning("Failed to find dealt card: dealt_card_id: #{dealt_card_id}, player_id: #{player.id}, game_id: #{game.id}, reason: #{inspect(reason)}")
+          {:noreply, socket}
+      end
     end
   end
 
@@ -198,16 +219,25 @@ defmodule CopiWeb.PlayerLive.Show do
     Enum.find(dealt_card.votes, fn vote -> vote.player_id == player.id end)
   end
 
+  defp dealt_card_belongs_to_game?(dealt_card, game) do
+    # Check if the dealt card's player belongs to the current game
+    player_ids = Enum.map(game.players, & &1.id)
+    dealt_card.player_id in player_ids
+  end
+
+  def topic(game_id) do
+    "game:#{game_id}"
+  end
+
   def display_game_session(edition) do
     case edition do
       "webapp" -> "Cornucopia Web Session:"
       "ecommerce" -> "Cornucopia Web Session:"
       "mobileapp" -> "Cornucopia Mobile Session:"
-      "masvs" -> "Cornucopia Mobile Session:"
-      "cumulus" -> "OWASP Cumulus Session:"
       "mlsec" -> "Elevation of MLSec Session:"
+      "cumulus" -> "OWASP Cumulus Session:"
+      "masvs" -> "Cornucopia Mobile Session:"
       _ -> "EoP Session:"
     end
   end
-
-end
+end

copi.owasp.org/test/copi_web/live/player_live/show_test.exs

@@ -296,6 +296,112 @@ defmodule CopiWeb.PlayerLive.ShowTest do
       {:ok, updated_dealt2} = Copi.Cornucopia.DealtCard.find(to_string(dealt.id))
       assert length(updated_dealt2.votes) == 0
     end
+
+    test "toggle_vote should not work before game starts", %{conn: conn, player: player} do
+      game_id = player.game_id
+      {:ok, game} = Cornucopia.Game.find(game_id)
+
+      # Ensure game has NOT started (started_at is nil)
+      assert game.started_at == nil
+
+      {:ok, card} =
+        Cornucopia.create_card(%{
+          category: "C", value: "TV1", description: "D", edition: "webapp",
+          version: "2.2", external_id: "TV_CARD_PRE_GAME", language: "en", misc: "m",
+          owasp_scp: [], owasp_devguide: [], owasp_asvs: [], owasp_appsensor: [],
+          capec: [], safecode: [], owasp_mastg: [], owasp_masvs: []
+        })
+
+      dealt = Copi.Repo.insert!(%Copi.Cornucopia.DealtCard{
+        player_id: player.id, card_id: card.id, played_in_round: 1
+      })
+
+      {:ok, show_live, _html} = live(conn, "/games/#{game_id}/players/#{player.id}")
+
+      # Attempt to vote before game starts - should be ignored
+      render_click(show_live, "toggle_vote", %{"dealt_card_id" => to_string(dealt.id)})
+      :timer.sleep(100)
+
+      {:ok, updated_dealt} = Copi.Cornucopia.DealtCard.find(to_string(dealt.id))
+      assert length(updated_dealt.votes) == 0
+    end
+
+    test "toggle_vote should not work after game ends", %{conn: conn, player: player} do
+      game_id = player.game_id
+      {:ok, game} = Cornucopia.Game.find(game_id)
+
+      # Start the game
+      Copi.Repo.update!(
+        Ecto.Changeset.change(game, started_at: DateTime.truncate(DateTime.utc_now(), :second))
+      )
+
+      {:ok, card} =
+        Cornucopia.create_card(%{
+          category: "C", value: "TV1", description: "D", edition: "webapp",
+          version: "2.2", external_id: "TV_CARD_AFTER_END", language: "en", misc: "m",
+          owasp_scp: [], owasp_devguide: [], owasp_asvs: [], owasp_appsensor: [],
+          capec: [], safecode: [], owasp_mastg: [], owasp_masvs: []
+        })
+
+      dealt = Copi.Repo.insert!(%Copi.Cornucopia.DealtCard{
+        player_id: player.id, card_id: card.id, played_in_round: 1
+      })
+
+      # Now end the game
+      Copi.Repo.update!(
+        Ecto.Changeset.change(game, finished_at: DateTime.truncate(DateTime.utc_now(), :second))
+      )
+
+      {:ok, show_live, _html} = live(conn, "/games/#{game_id}/players/#{player.id}")
+
+      # Attempt to vote after game ends - should be ignored
+      render_click(show_live, "toggle_vote", %{"dealt_card_id" => to_string(dealt.id)})
+      :timer.sleep(100)
+
+      {:ok, updated_dealt} = Copi.Cornucopia.DealtCard.find(to_string(dealt.id))
+      assert length(updated_dealt.votes) == 0
+    end
+
+    test "toggle_continue_vote should not work before game starts", %{conn: conn, player: player} do
+      game_id = player.game_id
+      {:ok, game} = Cornucopia.Game.find(game_id)
+
+      # Ensure game has NOT started (started_at is nil)
+      assert game.started_at == nil
+
+      {:ok, show_live, _html} = live(conn, "/games/#{game_id}/players/#{player.id}")
+
+      # Attempt to continue vote before game starts - should be ignored
+      render_click(show_live, "toggle_continue_vote", %{})
+      :timer.sleep(100)
+
+      {:ok, updated_game} = Cornucopia.Game.find(game_id)
+      assert length(updated_game.continue_votes) == 0
+    end
+
+    test "toggle_continue_vote should not work after game ends", %{conn: conn, player: player} do
+      game_id = player.game_id
+      {:ok, game} = Cornucopia.Game.find(game_id)
+
+      # Start the game
+      Copi.Repo.update!(
+        Ecto.Changeset.change(game, started_at: DateTime.truncate(DateTime.utc_now(), :second))
+      )
+
+      # Now end the game
+      Copi.Repo.update!(
+        Ecto.Changeset.change(game, finished_at: DateTime.truncate(DateTime.utc_now(), :second))
+      )
+
+      {:ok, show_live, _html} = live(conn, "/games/#{game_id}/players/#{player.id}")
+
+      # Attempt to continue vote after game ends - should be ignored
+      render_click(show_live, "toggle_continue_vote", %{})
+      :timer.sleep(100)
+
+      {:ok, updated_game} = Cornucopia.Game.find(game_id)
+      assert length(updated_game.continue_votes) == 0
+    end
   end
 
   describe "toggle_vote authorization" do