From 32a1644eee56345c64d609aa174b1eee3e9e8b14 Mon Sep 17 00:00:00 2001 From: Khushal Malhotra Date: Thu, 5 Mar 2026 20:00:09 +0530 Subject: [PATCH] Add comprehensive security test suite for IDOR vulnerability fix - Add security tests for PlayerLive.Show IDOR protection - Test valid access scenarios work correctly - Test invalid game_id attacks are blocked - Test non-existent resource handling - Test malformed parameter edge cases - Test security logging verification - Test regression scenarios for existing functionality Tests ensure the critical IDOR vulnerability fix works correctly without breaking existing features. --- .../copi_web/live/player_live/show_test.exs | 138 +++++++++++++++++- 1 file changed, 137 insertions(+), 1 deletion(-) diff --git a/copi.owasp.org/test/copi_web/live/player_live/show_test.exs b/copi.owasp.org/test/copi_web/live/player_live/show_test.exs index d41fec568..57a24f7a5 100644 --- a/copi.owasp.org/test/copi_web/live/player_live/show_test.exs +++ b/copi.owasp.org/test/copi_web/live/player_live/show_test.exs @@ -2,6 +2,7 @@ defmodule CopiWeb.PlayerLive.ShowTest do use CopiWeb.ConnCase, async: false import Phoenix.LiveViewTest + import ExUnit.CaptureLog alias Copi.Cornucopia @@ -10,7 +11,12 @@ defmodule CopiWeb.PlayerLive.ShowTest do defp create_player(_) do {:ok, game} = Cornucopia.create_game(@game_attrs) {:ok, player} = Cornucopia.create_player(%{name: "Player 1", game_id: game.id}) - %{player: player} + %{player: player, game: game} + end + + defp create_second_game(_) do + {:ok, second_game} = Cornucopia.create_game(%{name: "Second Game"}) + %{second_game: second_game} end describe "Show - additional coverage" do @@ -120,4 +126,134 @@ defmodule CopiWeb.PlayerLive.ShowTest do assert Show.display_game_session("eop") == "EoP Session:" end end + + describe "Security - IDOR Protection" do + setup [:create_player, :create_second_game] + + test "allows access with correct game_id and player_id", %{conn: conn, player: player, game: game} do + {:ok, _show_live, _html} = live(conn, "/games/#{game.id}/players/#{player.id}") + # Should successfully mount without redirect + end + + test "redirects when player accessed from wrong game context", %{conn: conn, player: player, second_game: second_game} do + log = capture_log(fn -> + assert {:error, {:redirect, %{to: "/error"}}} = + live(conn, "/games/#{second_game.id}/players/#{player.id}") + end) + + assert log =~ "Security: Player #{player.id} access attempted from wrong game context" + assert log =~ "URL game_id: #{second_game.id} vs actual game_id: #{player.game_id}" + end + + test "redirects when player_id does not exist", %{conn: conn, game: game} do + fake_player_id = "01KJQJAR0B57E96P7YJBDCEVYG" + + log = capture_log(fn -> + assert {:error, {:redirect, %{to: "/error"}}} = + live(conn, "/games/#{game.id}/players/#{fake_player_id}") + end) + + assert log =~ "Security: Player lookup failed for player_id: #{fake_player_id}" + end + + test "redirects when game_id does not exist but player exists", %{conn: conn, player: player} do + fake_game_id = "01KJQJAR0B57E96P7YJBDCEVYG" + + log = capture_log(fn -> + assert {:error, {:redirect, %{to: "/error"}}} = + live(conn, "/games/#{fake_game_id}/players/#{player.id}") + end) + + assert log =~ "Security: Player #{player.id} access attempted from wrong game context" + assert log =~ "URL game_id: #{fake_game_id} vs actual game_id: #{player.game_id}" + end + + test "redirects when both game_id and player_id do not exist", %{conn: conn} do + fake_game_id = "01KJQJAR0B57E96P7YJBDCEVYG" + fake_player_id = "01KJQJAR0B57E96P7YJBDCEVYF" + + log = capture_log(fn -> + assert {:error, {:redirect, %{to: "/error"}}} = + live(conn, "/games/#{fake_game_id}/players/#{fake_player_id}") + end) + + assert log =~ "Security: Player lookup failed for player_id: #{fake_player_id}" + end + + test "handles malformed game_id parameter", %{conn: conn, player: player} do + malformed_game_id = "invalid-game-id" + + log = capture_log(fn -> + assert {:error, {:redirect, %{to: "/error"}}} = + live(conn, "/games/#{malformed_game_id}/players/#{player.id}") + end) + + assert log =~ "Security: Player #{player.id} access attempted from wrong game context" + end + + test "handles malformed player_id parameter", %{conn: conn, game: game} do + malformed_player_id = "invalid-player-id" + + log = capture_log(fn -> + assert {:error, {:redirect, %{to: "/error"}}} = + live(conn, "/games/#{game.id}/players/#{malformed_player_id}") + end) + + assert log =~ "Security: Player lookup failed for player_id: #{malformed_player_id}" + end + + test "security logging includes client IP address", %{conn: conn, player: player, second_game: second_game} do + log = capture_log(fn -> + assert {:error, {:redirect, %{to: "/error"}}} = + live(conn, "/games/#{second_game.id}/players/#{player.id}") + end) + + assert log =~ "IP:" + end + end + + describe "Security - Regression Tests" do + setup [:create_player] + + test "existing functionality still works after security fix", %{conn: conn, player: player} do + game_id = player.game_id + {:ok, show_live, html} = live(conn, "/games/#{game_id}/players/#{player.id}") + + # Verify the LiveView mounts successfully + assert html =~ player.name + assert render(show_live) =~ player.name + end + + test "handle_info :proceed_to_next_round still works after security fix", %{conn: conn, player: player} do + game_id = player.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, show_live, _html} = live(conn, "/games/#{game_id}/players/#{player.id}") + + send(show_live.pid, :proceed_to_next_round) + :timer.sleep(100) + + {:ok, updated_game} = Cornucopia.Game.find(game_id) + assert updated_game.rounds_played == 1 + end + + test "handle_event functionality still works after security fix", %{conn: conn, player: player} do + game_id = player.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, show_live, _html} = live(conn, "/games/#{game_id}/players/#{player.id}") + + # Test that existing events still work + html = render_click(show_live, "toggle_continue_vote", %{}) + assert is_binary(html) + end + end end