fix: prevent voting outside active game lifecycle (#2568)

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

@@ -129,28 +129,33 @@ defmodule CopiWeb.PlayerLive.Show do
     game = socket.assigns.game
     player = socket.assigns.player
 
-    {:ok, dealt_card} = DealtCard.find(dealt_card_id)
+    if is_nil(game.started_at) or not is_nil(game.finished_at) do
+      Logger.warning("toggle_vote rejected: game not active. game_id: #{game.id}, player_id: #{player.id}")
+      {:noreply, socket}
+    else
+      {:ok, dealt_card} = DealtCard.find(dealt_card_id)
 
-    vote = get_vote(dealt_card, player)
+      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)}")
+      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
-    end
 
-    {:ok, updated_game} = Game.find(game.id)
+      {:ok, updated_game} = Game.find(game.id)
 
-    CopiWeb.Endpoint.broadcast(topic(updated_game.id), "game:updated", updated_game)
+      CopiWeb.Endpoint.broadcast(topic(updated_game.id), "game:updated", updated_game)
 
-    {:noreply, assign(socket, :game, updated_game)}
+      {:noreply, assign(socket, :game, updated_game)}
+    end
   end
 
   def topic(game_id) do

copi.owasp.org/test/copi/cornucopia_test.exs

@@ -255,4 +255,24 @@ defmodule Copi.CornucopiaTest do
       assert %Ecto.Changeset{} = Cornucopia.change_card(card)
     end
   end
+
+  describe "dealt_cards" do
+    alias Copi.Cornucopia.DealtCard
+
+    test "find/1 returns error when dealt_card not found" do
+      assert {:error, :not_found} = DealtCard.find(999_999_999)
+    end
+
+    test "changeset/2 returns a dealt_card changeset" do
+      assert %Ecto.Changeset{} = DealtCard.changeset(%DealtCard{}, %{})
+    end
+  end
+
+  describe "votes" do
+    alias Copi.Cornucopia.Vote
+
+    test "changeset/2 returns a vote changeset" do
+      assert %Ecto.Changeset{} = Vote.changeset(%Vote{}, %{})
+    end
+  end
 end

copi.owasp.org/test/copi_web/live/player_live_test.exs

@@ -153,6 +153,74 @@ defmodule CopiWeb.PlayerLiveTest do
       assert updated_game.rounds_played == 1
     end
 
+    test "toggle_vote removes existing vote when already voted", %{conn: conn, player: player} do
+      game_id = player.game_id
+      {:ok, other_player} = Cornucopia.create_player(%{name: "OtherToggle", game_id: game_id})
+
+      {:ok, game} = Cornucopia.Game.find(game_id)
+      Copi.Repo.update!(Ecto.Changeset.change(game, started_at: DateTime.truncate(DateTime.utc_now(), :second)))
+
+      {:ok, card} = Cornucopia.create_card(%{
+        category: "C", value: "V", description: "D", edition: "webapp",
+        version: "2.2", external_id: "EXT_TOG", language: "en",
+        misc: "misc", owasp_scp: [], owasp_devguide: [], owasp_asvs: [],
+        owasp_appsensor: [], capec: [], safecode: [], owasp_mastg: [], owasp_masvs: []
+      })
+      {:ok, dealt} = Copi.Repo.insert(%Copi.Cornucopia.DealtCard{player_id: other_player.id, card_id: card.id, played_in_round: 1})
+      Copi.Repo.insert!(%Copi.Cornucopia.Vote{dealt_card_id: dealt.id, player_id: player.id})
+
+      {:ok, show_live, _html} = live(conn, "/games/#{game_id}/players/#{player.id}")
+
+      # Toggling should remove the existing vote
+      show_live |> render_click("toggle_vote", %{"dealt_card_id" => "#{dealt.id}"})
+      refute Copi.Repo.get_by(Copi.Cornucopia.Vote, dealt_card_id: dealt.id, player_id: player.id)
+    end
+
+    test "rejects toggle_vote before game starts (started_at is nil)", %{conn: conn, player: player} do
+      game_id = player.game_id
+      {:ok, other_player} = Cornucopia.create_player(%{name: "Other", game_id: game_id})
+
+      {:ok, card} = Cornucopia.create_card(%{
+        category: "C", value: "V", description: "D", edition: "webapp",
+        version: "2.2", external_id: "EXT2", language: "en",
+        misc: "misc", owasp_scp: [], owasp_devguide: [], owasp_asvs: [],
+        owasp_appsensor: [], capec: [], safecode: [], owasp_mastg: [], owasp_masvs: []
+      })
+      {:ok, dealt} = Copi.Repo.insert(%Copi.Cornucopia.DealtCard{player_id: other_player.id, card_id: card.id, played_in_round: 1})
+
+      # Game has not started (started_at is nil)
+      {:ok, show_live, _html} = live(conn, "/games/#{game_id}/players/#{player.id}")
+
+      show_live |> render_click("toggle_vote", %{"dealt_card_id" => "#{dealt.id}"})
+
+      # Vote must NOT be created
+      refute Copi.Repo.get_by(Copi.Cornucopia.Vote, dealt_card_id: dealt.id, player_id: player.id)
+    end
+
+    test "rejects toggle_vote after game ends (finished_at is set)", %{conn: conn, player: player} do
+      game_id = player.game_id
+      {:ok, other_player} = Cornucopia.create_player(%{name: "Other2", game_id: game_id})
+
+      {:ok, game} = Cornucopia.Game.find(game_id)
+      now = DateTime.truncate(DateTime.utc_now(), :second)
+      Copi.Repo.update!(Ecto.Changeset.change(game, started_at: now, finished_at: now))
+
+      {:ok, card} = Cornucopia.create_card(%{
+        category: "C", value: "V", description: "D", edition: "webapp",
+        version: "2.2", external_id: "EXT3", language: "en",
+        misc: "misc", owasp_scp: [], owasp_devguide: [], owasp_asvs: [],
+        owasp_appsensor: [], capec: [], safecode: [], owasp_mastg: [], owasp_masvs: []
+      })
+      {:ok, dealt} = Copi.Repo.insert(%Copi.Cornucopia.DealtCard{player_id: other_player.id, card_id: card.id, played_in_round: 1})
+
+      {:ok, show_live, _html} = live(conn, "/games/#{game_id}/players/#{player.id}")
+
+      show_live |> render_click("toggle_vote", %{"dealt_card_id" => "#{dealt.id}"})
+
+      # Vote must NOT be created
+      refute Copi.Repo.get_by(Copi.Cornucopia.Vote, dealt_card_id: dealt.id, player_id: player.id)
+    end
+
     test "allows toggling continue vote off", %{conn: conn, player: player} do
       game_id = player.game_id
       {:ok, game} = Cornucopia.Game.find(game_id)