From 5e4f0f9fe8bd33a585be54030462abfa716eda72 Mon Sep 17 00:00:00 2001
From: bsag <bsag@a4c988fc-2ded-0310-b66e-134b36920a42>
Date: Mon, 10 Oct 2005 21:14:34 +0000
Subject: [PATCH] Improved the security of the new drag and drop features of
 the projects and contexts lists by checking that the logged in user owns
 those items.

Also changed references to Form.focus_first which were defined in the removed file prototype-ex.js to Form.focusFirstElement, which is defined in prototype.js. Fixes #133.



git-svn-id: http://www.rousette.org.uk/svn/tracks-repos/trunk@149 a4c988fc-2ded-0310-b66e-134b36920a42
---
 tracks/app/controllers/context_controller.rb   | 18 +++++++++++++++---
 tracks/app/controllers/project_controller.rb   | 17 +++++++++++++++--
 .../app/views/context/_context_listing.rhtml   |  4 +---
 tracks/app/views/context/list.rhtml            |  4 ++--
 .../app/views/project/_project_listing.rhtml   |  4 +---
 tracks/app/views/project/list.rhtml            |  4 ++--
 tracks/public/stylesheets/standard.css         | 11 ++---------
 7 files changed, 38 insertions(+), 24 deletions(-)

diff --git a/tracks/app/controllers/context_controller.rb b/tracks/app/controllers/context_controller.rb
index 8956919e..9d85ce69 100644
--- a/tracks/app/controllers/context_controller.rb
+++ b/tracks/app/controllers/context_controller.rb
@@ -76,9 +76,10 @@ class ContextController < ApplicationController
   #
   def order
     @params["list-contexts"].each_with_index do |id, position|
-      Context.update(id, :position => position + 1)
+      if check_user_matches_context_user(id)
+        Context.update(id, :position => position + 1)
+      end
     end
-    render_text ""
   end
   
   protected
@@ -101,7 +102,18 @@ class ContextController < ApplicationController
       end
     end
 
-
+    def check_user_matches_context_user(id)
+       @user = @session['user']
+       @context = Context.find_by_id_and_user_id(id, @user.id)
+       if @user == @context.user
+         return @context
+       else
+         @context = nil
+         flash["warning"] = "Project and session user mis-match: #{@context.user_id} and #{@session['user'].id}!"
+         render_text ""
+       end
+    end
+     
     def init
       @user = @session['user']
       @projects = @user.projects.collect { |x| x.done? ? nil:x }.compact
diff --git a/tracks/app/controllers/project_controller.rb b/tracks/app/controllers/project_controller.rb
index 8e5f9174..8294f193 100644
--- a/tracks/app/controllers/project_controller.rb
+++ b/tracks/app/controllers/project_controller.rb
@@ -73,9 +73,10 @@ class ProjectController < ApplicationController
   #
   def order
     @params["list-projects"].each_with_index do |id, position|
-      Project.update(id, :position => position + 1)
+      if check_user_matches_project_user(id)
+        Project.update(id, :position => position + 1)
+      end
     end
-    render_text ""
   end
   
   protected
@@ -98,6 +99,18 @@ class ProjectController < ApplicationController
       end
     end
     
+    def check_user_matches_project_user(id)
+      @user = @session['user']
+      @project = Project.find_by_id_and_user_id(id, @user.id)
+      if @user == @project.user
+        return @project
+      else
+        @project = nil
+        flash["warning"] = "Project and session user mis-match: #{@project.user_id} and #{@session['user'].id}!"
+        render_text ""
+      end
+    end
+    
     def init
       @user = @session['user']
       @projects = @user.projects
diff --git a/tracks/app/views/context/_context_listing.rhtml b/tracks/app/views/context/_context_listing.rhtml
index 2ddcbeab..af17241b 100644
--- a/tracks/app/views/context/_context_listing.rhtml
+++ b/tracks/app/views/context/_context_listing.rhtml
@@ -1,7 +1,6 @@
 <%  context = context_listing %>
-<div id="container_<%= context.id %>">
+<div id="container_<%= context.id %>" class="list">
 <!-- %= error_messages_for 'context' % -->
-<div id="context-<%= context.id %>-display" class="list">
 <div id="context-<%= context.id %>" class="even_row" style="display:'';">
 <div class="position">
   <span class="handle">DRAG</span>
@@ -27,7 +26,6 @@
 </div>
 </div><!-- [end:context-context.id] -->
 
-</div><!-- [end:context-context.id-display] -->
 <div id="context-<%= context.id %>-edit-form" class="edit-form" style="display:none;">
 <%= form_remote_tag  :url => { :controller => 'context', :action => 'update', :id => context.id },
                :html => { :id => "form-context-#{context.id}", :class => "inline-form" },
diff --git a/tracks/app/views/context/list.rhtml b/tracks/app/views/context/list.rhtml
index 8e23a13f..2e3798fb 100644
--- a/tracks/app/views/context/list.rhtml
+++ b/tracks/app/views/context/list.rhtml
@@ -13,14 +13,14 @@
                      :url => {:controller => 'context', :action => 'order'}
                      %> 
 <br />
-<a href="javascript:void(0)" onClick="Element.toggle('context_new'); Form.focus_first('context-form');" accesskey="n" title="Create a new context [Alt+n]">Create new context &#187;</a>
+<a href="javascript:void(0)" onClick="Element.toggle('context_new'); Form.focusFirstElement('context-form');" accesskey="n" title="Create a new context [Alt+n]">Create new context &#187;</a>
 <div id="context_new" class="context_new" style="display:none">
 <!--[form:context]-->
 <%= form_remote_tag :url => { :action => "new_context" },
   :update=> "list-contexts",
   :position=> "bottom",
   :loading => "context.reset()",
-  :complete => "Form.focus_first('context-form');",
+  :complete => "Form.focusFirstElement('context-form');",
   :html=> { :id=>'context-form', :name=>'context', :class => 'inline-form' } %>
 <%= hidden_field( "context", "id" ) %>
 <label for="context_name">Context name</label>                                  
diff --git a/tracks/app/views/project/_project_listing.rhtml b/tracks/app/views/project/_project_listing.rhtml
index 9b6a693e..6f97f7f7 100644
--- a/tracks/app/views/project/_project_listing.rhtml
+++ b/tracks/app/views/project/_project_listing.rhtml
@@ -1,7 +1,6 @@
 <%  project = project_listing %>
-<div id="container_<%= project.id %>">
+<div id="container_<%= project.id %>" class="list">
 <!-- %= error_messages_for 'project' % -->
-<div id="project-<%= project.id %>-display" class="list">
 <div id="project-<%= project.id %>" class="even_row" style="display:'';">
 <div class="position">
   <span class="handle">DRAG</span>
@@ -25,7 +24,6 @@
 </div>
 </div><!-- [end:project-project.id] -->
 
-</div><!-- [end:project-project.id-display] -->
 <div id="project-<%= project.id %>-edit-form" class="edit-form" style="display:none;">
 <%= form_remote_tag  :url => { :controller => 'project', :action => 'update', :id => project.id },
                :html => { :id => "form-project-#{project.id}", :class => "form" },
diff --git a/tracks/app/views/project/list.rhtml b/tracks/app/views/project/list.rhtml
index 5482f0e0..979c6fec 100644
--- a/tracks/app/views/project/list.rhtml
+++ b/tracks/app/views/project/list.rhtml
@@ -13,14 +13,14 @@
                      :url => {:controller => 'project', :action => 'order'}
                      %> 
 <br />
-<a href="javascript:void(0)" onClick="Element.toggle('project_new'); Form.focus_first('project-form');" accesskey="n" title="Create a new project [Alt+n]">Create new project &#187;</a>
+<a href="javascript:void(0)" onClick="Element.toggle('project_new'); Form.focusFirstElement('project-form');" accesskey="n" title="Create a new project [Alt+n]">Create new project &#187;</a>
 <div id="project_new" class="project_new" style="display:none">
 <!--[form:project]-->
 <%= form_remote_tag :url => { :action => "new_project" },
   :update=> "list-projects",
   :position=> "bottom",
   :loading => "project.reset()",
-  :complete => "Form.focus_first('project-form');",
+  :complete => "Form.focusFirstElement('project-form');",
   :html=> { :id=>'project-form', :name=>'project', :class => 'inline-form' } %>
   <label for="project_name">Name:</label><br />
   <%= text_field 'project', 'name'  %><br />
diff --git a/tracks/public/stylesheets/standard.css b/tracks/public/stylesheets/standard.css
index 918f6d7f..9519a5fc 100644
--- a/tracks/public/stylesheets/standard.css
+++ b/tracks/public/stylesheets/standard.css
@@ -392,14 +392,7 @@ input {
 }
 
 /* Positioning the 'cells' in the list */
-/* table.list {
-    margin-top: 0px;
-    border-top: 1px solid #ccc;
-    border-left: 1px solid #ccc;
-    border-right: 1px solid #ccc;
-    background: #fff;
-    } */
-    
+
 .position {
     float: left;
     }
@@ -412,7 +405,7 @@ img.position, a:hover img.position {
     
 .data {
     text-align: left;
-    margin-left: 40px;
+    margin-left: 20px;
     float: left;
     }