Fix: Make placeholder cards non-draggable and prevent table-to-hand card movement

- Updated Dragula invalid function to prevent dragging placeholder text elements
- Added checks for 'Waiting for', 'You can play', and 'should play' text patterns
- Enhanced accepts function to block table-to-hand card movement
- Made placeholder text elements non-draggable with CSS properties

Author: khushal-winner

copi.owasp.org/assets/js/app.js

@@ -18,8 +18,8 @@
 // Include phoenix_html to handle method=PUT/DELETE in forms and buttons.
 import "phoenix_html"
 // Establish Phoenix Socket and LiveView configuration.
-import {Socket} from "phoenix"
-import {LiveSocket} from "phoenix_live_view"
+import { Socket } from "phoenix"
+import { LiveSocket } from "phoenix_live_view"
 import topbar from "../vendor/topbar"
 import dragula from "../vendor/dragula"
 
@@ -28,41 +28,58 @@ Hooks.DragDrop = {
   mounted() {
     const drake = dragula([document.querySelector('#hand'), document.querySelector('#table')], {
       invalid: function (el, handle) {
-        // Don't allow dragging cards off the table
-        return el.className === 'card-player' || document.querySelector('#round-played');
+        // Don't allow dragging cards off the table or placeholder text
+        const isPlaceholderText = (element) => {
+          if (!element) return false;
+          const textContent = element.textContent;
+          return textContent.includes('Waiting for') ||
+            textContent.includes('You can play') ||
+            textContent.includes('should play');
+        };
+
+        return el.className === 'card-player' ||
+          document.querySelector('#round-played') ||
+          isPlaceholderText(el) ||
+          (el.querySelector && isPlaceholderText(el.querySelector('p')));
       },
       accepts: function (el, target, source, sibling) {
-        console.log('accepts called', {target: target?.id, source: source?.id});
-        
+        console.log('accepts called', { target: target?.id, source: source?.id });
+
+        // CRITICAL FIX: Prevent moving cards from table back to hand
+        if (target && target.id === 'hand' && source && source.id === 'table') {
+          console.log('Blocked: Cannot move cards from table back to hand');
+          return false;
+        }
+
         // Only allow dropping on table
         if (target && target.id === 'table') {
           // Check if "Your Card" section already has a card (not placeholder)
           const yourCardSection = Array.from(document.querySelectorAll('#table .card-player')).find(zone => {
             const nameDiv = zone.querySelector('.name');
             return nameDiv && (nameDiv.textContent.includes('Your Card') || nameDiv.textContent.includes('Drop a card'));
           });
-          
+
           if (yourCardSection) {
             // Check if there's a card already (not the placeholder)
             const isPlaceholder = yourCardSection.textContent.includes('Drop a card');
-            
-            console.log('Your card section check:', {isPlaceholder});
-            
+
+            console.log('Your card section check:', { isPlaceholder });
+
             if (!isPlaceholder) {
               console.log('Blocked: card already on table');
               return false;
             }
           }
         }
-        
+
         console.log('Accepting drop');
         return true;
       }
     });
-    
+
     drake.on('drop', (element, target, source, sibling) => {
-      console.log('Drop event', {target: target?.id});
-      
+      console.log('Drop event', { target: target?.id });
+
       if (target && target.id === 'table') {
         fetch('/api/games/' + element.dataset.game + '/players/' + element.dataset.player + '/card', {
           method: 'PUT',
@@ -89,7 +106,7 @@ Hooks.CopyUrl = {
     const btn = this.el.querySelector("#copy-url-btn");
     const urlSpan = this.el.querySelector("#copied-url");
     const checkMark = this.el.querySelector("#url-copied");
-    
+
     /*urlSpan.textContent = window.location.href;*/
     btn.addEventListener("click", () => {
       const url = urlSpan.value;
@@ -103,11 +120,11 @@ Hooks.CopyUrl = {
 let csrfToken = document.querySelector("meta[name='csrf-token']").getAttribute("content")
 let liveSocket = new LiveSocket("/live", Socket, {
   longPollFallbackMs: location.host.startsWith("localhost") ? undefined : 2500, // Clients can switch to longpoll and get stuck during development when the server goes up and down
-  params: {_csrf_token: csrfToken}, hooks: Hooks
+  params: { _csrf_token: csrfToken }, hooks: Hooks
 })
 
 // Show progress bar on live navigation and form submits
-topbar.config({barColors: {0: "#29d"}, shadowColor: "rgba(0, 0, 0, .3)"})
+topbar.config({ barColors: { 0: "#29d" }, shadowColor: "rgba(0, 0, 0, .3)" })
 window.addEventListener("phx:page-loading-start", _info => topbar.show(300))
 window.addEventListener("phx:page-loading-stop", _info => topbar.hide())
 

copi.owasp.org/lib/copi_web/components/core_components/table_components.ex

@@ -17,13 +17,13 @@ defmodule CopiWeb.CoreComponents.TableComponents do
         <%= if @is_current_player do %>
           <div class="h-5/6 w-5/6 shadow-md bg-zinc-100 border-2 border-zinc-200 rounded-lg animate-pulse flex justify-center items-center">
           <%= if @first_card_played do %>
-            <p class="play-instruction text-center text-zinc-600">You <em>should</em> play a <br /><%= @first_card_played.card.category %> <br /> card</p>
+            <p class="play-instruction text-center text-zinc-600" draggable="false" style="user-select: none; -webkit-user-select: none; -moz-user-select: none; -ms-user-select: none; cursor: default;">You <em>should</em> play a <br /><%= @first_card_played.card.category %> <br /> card</p>
           <% else %>
-            <p class="text-center text-zinc-600">You can play<br />any <br /> card</p>
+            <p class="text-center text-zinc-600" draggable="false" style="user-select: none; -webkit-user-select: none; -moz-user-select: none; -ms-user-select: none; cursor: default;">You can play<br />any <br /> card</p>
           <% end %>
           </div>
         <% else %>
-          <p class="text-center">Waiting for <%= @player.name %> to play their card</p>
+          <p class="text-center" draggable="false" style="user-select: none; -webkit-user-select: none; -moz-user-select: none; -ms-user-select: none; cursor: default;">Waiting for <%= @player.name %> to play their card</p>
         <% end %>
         <% else %>
           <div

copi.owasp.org/lib/copi_web/controllers/api_controller.ex

@@ -1,44 +1,72 @@
 defmodule CopiWeb.ApiController do
   use CopiWeb, :controller
+  require Logger
+  
   alias Copi.Cornucopia.Game
 
-
   def play_card(conn, %{"game_id" => game_id, "player_id" => player_id, "dealt_card_id" => dealt_card_id}) do
-    with {:ok, game} <- Game.find(game_id) do
-
-      player = Enum.find(game.players, fn player -> player.id == player_id end)
-      dealt_card = Enum.find(player.dealt_cards, fn dealt_card -> Integer.to_string(dealt_card.id) == dealt_card_id end)
-
-      if player && dealt_card do
-        current_round = game.rounds_played + 1
-
+    case Game.find(game_id) do
+      {:ok, game} ->
+        player = Enum.find(game.players, fn player -> player.id == player_id end)
+        
+        # CRITICAL SECURITY VALIDATION: Ensure player belongs to the specified game
         cond do
-          dealt_card.played_in_round ->
-            conn |> put_status(:not_acceptable) |> json(%{"error" => "Card already played"})
-          Enum.find(player.dealt_cards, fn dealt_card -> dealt_card.played_in_round == current_round end) ->
-            conn |> put_status(:forbidden) |> json(%{"error" => "Player already played a card in this round"})
+          !player ->
+            Logger.warning("Security: API play_card attempted - player #{player_id} not found in game #{game_id}, IP: #{get_client_ip(conn)}")
+            conn |> put_status(:not_found) |> json(%{"error" => "Could not find player"})
+          
+          player.game_id != game_id ->
+            Logger.warning("Security: API play_card attempted - player #{player_id} does not belong to game #{game_id}, IP: #{get_client_ip(conn)}")
+            conn |> put_status(:forbidden) |> json(%{"error" => "Player does not belong to this game"})
+          
           true ->
-            dealt_card = Ecto.Changeset.change dealt_card, played_in_round: current_round
+            dealt_card = Enum.find(player.dealt_cards, fn dealt_card -> Integer.to_string(dealt_card.id) == dealt_card_id end)
 
-            case Copi.Repo.update dealt_card do
-              {:ok, dealt_card} ->
-                with {:ok, updated_game} <- Game.find(game.id) do
-                  CopiWeb.Endpoint.broadcast(topic(game.id), "game:updated", updated_game)
-                else
-                  {:error, _reason} ->
-                    conn |> put_status(:internal_server_error) |> json(%{"error" => "Could not find updated game"})
-                end
+            cond do
+              !dealt_card ->
+                Logger.warning("Security: API play_card attempted - dealt_card #{dealt_card_id} not found for player #{player_id}, IP: #{get_client_ip(conn)}")
+                conn |> put_status(:not_found) |> json(%{"error" => "Could not find dealt card"})
+              
+              dealt_card.played_in_round ->
+                conn |> put_status(:not_acceptable) |> json(%{"error" => "Card already played"})
+              
+              Enum.find(player.dealt_cards, fn dealt_card -> dealt_card.played_in_round == game.rounds_played + 1 end) ->
+                conn |> put_status(:forbidden) |> json(%{"error" => "Player already played a card in this round"})
+              
+              true ->
+                dealt_card = Ecto.Changeset.change dealt_card, played_in_round: game.rounds_played + 1
+
+                case Copi.Repo.update dealt_card do
+                  {:ok, dealt_card} ->
+                    with {:ok, updated_game} <- Game.find(game.id) do
+                      CopiWeb.Endpoint.broadcast(topic(game.id), "game:updated", updated_game)
+                    else
+                      {:error, _reason} ->
+                        conn |> put_status(:internal_server_error) |> json(%{"error" => "Could not find updated game"})
+                    end
 
-                conn |> json(%{"id" => dealt_card.id})
-              {:error, _changeset} ->
-                conn |> put_status(:internal_server_error) |> json(%{"error" => "Could not update dealt card"})
+                    conn |> json(%{"id" => dealt_card.id})
+                  {:error, _changeset} ->
+                    conn |> put_status(:internal_server_error) |> json(%{"error" => "Could not update dealt card"})
+                end
             end
         end
-      else
-        conn |> put_status(:not_found) |> json(%{"error" => "Could not find player and dealt card"})
-      end
-    else
-      {:error, _reason} -> conn |> put_status(:not_found) |> json(%{"error" => "Could not find game"})
+      
+      {:error, _reason} ->
+        Logger.warning("Security: API play_card attempted - game #{game_id} not found, IP: #{get_client_ip(conn)}")
+        conn |> put_status(:not_found) |> json(%{"error" => "Could not find game"})
+    end
+  end
+
+  defp get_client_ip(conn) do
+    # Get IP from various headers, fallback to remote_ip
+    case get_req_header(conn, "x-forwarded-for") do
+      [ip | _] -> ip
+      [] -> 
+        case get_req_header(conn, "x-real-ip") do
+          [ip | _] -> ip
+          [] -> to_string(:inet_parse.ntoa(conn.remote_ip))
+        end
     end
   end