Merge branch 'security-fixes'

app/controllers/application_controller.rb

@@ -24,6 +24,8 @@ class ApplicationController < ActionController::Base
   rescue_from ActiveRecord::RecordNotFound, :with => :respond_to_not_found
   rescue_from CanCan::AccessDenied,         :with => :respond_to_access_denied
 
+  include ERB::Util # to give us h and j methods
+
   # Common auto_complete handler for all core controllers.
   #----------------------------------------------------------------------------
   def auto_complete
@@ -40,7 +42,7 @@ def auto_complete
     respond_to do |format|
       format.any(:js, :html)   { render :partial => 'auto_complete' }
       format.json { render :json => @auto_complete.inject({}){|h,a|
-        h[a.id] = a.respond_to?(:full_name) ? a.full_name : a.name; h
+        h[a.id] = a.respond_to?(:full_name) ? j(a.full_name) : j(a.name); h
       }}
     end
   end
@@ -145,7 +147,7 @@ def redirect_back_or_default(default)
 
   #----------------------------------------------------------------------------
   def can_signup?
-    [ :allowed, :needs_approval ].include? Setting.user_signup
+    User.can_signup?
   end
 
   #----------------------------------------------------------------------------
@@ -199,10 +201,10 @@ def respond_to_not_found(*types)
     flash[:warning] = t(:msg_asset_not_available, asset)
 
     respond_to do |format|
-      format.html { redirect_to :action => :index }
+      format.html { redirect_to(redirection_url) }
       format.js   { render(:update) { |page| page.reload } }
-      format.json { render :text => flash[:warning], :status => :not_found }
-      format.xml  { render :text => flash[:warning], :status => :not_found }
+      format.json { render :text => flash[:warning],  :status => :not_found }
+      format.xml  { render :xml => [flash[:warning]], :status => :not_found }
     end
   end
 
@@ -213,32 +215,32 @@ def respond_to_related_not_found(related, *types)
 
     url = send("#{related.pluralize}_path")
     respond_to do |format|
-      format.html { redirect_to url }
-      format.js   { render(:update) { |page| page.redirect_to url } }
-      format.json { render :text => flash[:warning], :status => :not_found }
-      format.xml  { render :text => flash[:warning], :status => :not_found }
+      format.html { redirect_to(url) }
+      format.js   { render(:update) { |page| page.redirect_to(url) } }
+      format.json { render :text => flash[:warning],  :status => :not_found }
+      format.xml  { render :xml => [flash[:warning]], :status => :not_found }
     end
   end
 
   #----------------------------------------------------------------------------
   def respond_to_access_denied
-    if self.action_name == "show"
-      flash[:warning] = t(:msg_asset_not_authorized, asset)
-
-    else
-      flick = case self.action_name
-        when "destroy" then "delete"
-        when "promote" then "convert"
-        else self.action_name
-      end
-      flash[:warning] = t(:msg_cant_do, :action => flick, :asset => asset)
-    end
-
+    flash[:warning] = t(:msg_not_authorized, default: 'You are not authorized to take this action.')
     respond_to do |format|
-      format.html { redirect_to :action => :index }
+      format.html { redirect_to(redirection_url) }
       format.js   { render(:update) { |page| page.reload } }
-      format.json { render :text => flash[:warning], :status => :unauthorized }
-      format.xml  { render :text => flash[:warning], :status => :unauthorized }
+      format.json { render :text => flash[:warning],  :status => :unauthorized }
+      format.xml  { render :xml => [flash[:warning]], :status => :unauthorized }
     end
   end
+
+  #----------------------------------------------------------------------------
+  def redirection_url
+    # Try to redirect somewhere sensible. Note: not all controllers have an index action
+    url = if current_user.present?
+      (respond_to?(:index) and self.action_name != 'index') ? { action: 'index' } : root_url
+    else
+      login_url
+    end
+  end
+
 end

app/controllers/home_controller.rb

@@ -6,7 +6,6 @@
 class HomeController < ApplicationController
   before_filter :require_user, :except => [ :toggle, :timezone ]
   before_filter :set_current_tab, :only => :index
-  before_filter "hook(:home_before_filter, self, :amazing => true)"
 
   #----------------------------------------------------------------------------
   def index

app/controllers/passwords_controller.rb

@@ -44,8 +44,9 @@ def update
     end
   end
 
-  #----------------------------------------------------------------------------
   private
+
+  #----------------------------------------------------------------------------
   def load_user_using_perishable_token
     @user = User.find_using_perishable_token(params[:id])
     unless @user

app/controllers/tasks_controller.rb

@@ -11,7 +11,7 @@ class TasksController < ApplicationController
   # GET /tasks
   #----------------------------------------------------------------------------
   def index
-    @view = params[:view] || "pending"
+    @view = view
     @tasks = Task.find_all_grouped(current_user, @view)
 
     respond_with @tasks do |format|
@@ -25,14 +25,13 @@ def index
   #----------------------------------------------------------------------------
   def show
     @task = Task.tracked_by(current_user).find(params[:id])
-
     respond_with(@task)
   end
 
   # GET /tasks/new
   #----------------------------------------------------------------------------
   def new
-    @view = params[:view] || "pending"
+    @view = view
     @task = Task.new
     @bucket = Setting.unroll(:task_bucket)[1..-1] << [ t(:due_specific_date, :default => 'On Specific Date...'), :specific_time ]
     @category = Setting.unroll(:task_category)
@@ -52,7 +51,7 @@ def new
   # GET /tasks/1/edit                                                      AJAX
   #----------------------------------------------------------------------------
   def edit
-    @view = params[:view] || "pending"
+    @view = view
     @task = Task.tracked_by(current_user).find(params[:id])
     @bucket = Setting.unroll(:task_bucket)[1..-1] << [ t(:due_specific_date, :default => 'On Specific Date...'), :specific_time ]
     @category = Setting.unroll(:task_category)
@@ -68,7 +67,7 @@ def edit
   # POST /tasks
   #----------------------------------------------------------------------------
   def create
-    @view = params[:view] || "pending"
+    @view = view
     @task = Task.new(params[:task]) # NOTE: we don't display validation messages for tasks.
 
     respond_with(@task) do |format|
@@ -81,7 +80,7 @@ def create
   # PUT /tasks/1
   #----------------------------------------------------------------------------
   def update
-    @view = params[:view] || "pending"
+    @view = view
     @task = Task.tracked_by(current_user).find(params[:id])
     @task_before_update = @task.dup
 
@@ -107,7 +106,7 @@ def update
   # DELETE /tasks/1
   #----------------------------------------------------------------------------
   def destroy
-    @view = params[:view] || "pending"
+    @view = view
     @task = Task.tracked_by(current_user).find(params[:id])
     @task.destroy
 
@@ -142,7 +141,7 @@ def complete
   # Ajax request to filter out a list of tasks.                            AJAX
   #----------------------------------------------------------------------------
   def filter
-    @view = params[:view] || "pending"
+    @view = view
 
     update_session do |filters|
       if params[:checked].true?
@@ -167,8 +166,7 @@ def update_session
   # Collect data necessary to render filters sidebar.
   #----------------------------------------------------------------------------
   def update_sidebar
-    @view = params[:view]
-    @view = "pending" unless %w(pending assigned completed).include?(@view)
+    @view = view
     @task_total = Task.totals(current_user, @view)
 
     # Update filters session if we added, deleted, or completed a task.
@@ -189,4 +187,13 @@ def update_sidebar
       session[name] = filters unless filters.blank?
     end
   end
+
+  # Ensure view is allowed
+  #----------------------------------------------------------------------------
+  def view
+    view = params[:view]
+    views = Task::ALLOWED_VIEWS
+    views.include?(view) ? view : views.first
+  end
+
 end

app/controllers/users_controller.rb

@@ -5,44 +5,30 @@
 #------------------------------------------------------------------------------
 class UsersController < ApplicationController
 
-  before_filter :require_no_user, :only => [ :new, :create ]
-  before_filter :require_user, :only => [ :show, :redraw ]
   before_filter :set_current_tab, :only => [ :show, :opportunities_overview ] # Don't hightlight any tabs.
-  before_filter :require_and_assign_user, :except => [ :new, :create, :show, :avatar, :upload_avatar ]
-  before_filter :assign_given_or_current_user, :only => [ :show, :avatar, :upload_avatar, :edit, :update ]
 
-  load_resource
+  check_authorization
+  load_and_authorize_resource # handles all security
 
   respond_to :html, :only => [ :show, :new ]
 
   # GET /users/1
-  # GET /users/1.json
-  # GET /users/1.xml                                                       HTML
+  # GET /users/1.js
   #----------------------------------------------------------------------------
   def show
+    @user = current_user if params[:id].nil?
     respond_with(@user)
   end
 
   # GET /users/new
-  # GET /users/new.json
-  # GET /users/new.xml                                                     HTML
+  # GET /users/new.js
   #----------------------------------------------------------------------------
   def new
-    if can_signup?
-      respond_with(@user)
-    else
-      redirect_to login_path
-    end
-  end
-
-  # GET /users/1/edit                                                      AJAX
-  #----------------------------------------------------------------------------
-  def edit
     respond_with(@user)
   end
 
   # POST /users
-  # POST /users.xml                                                        HTML
+  # POST /users.js
   #----------------------------------------------------------------------------
   def create
     if @user.save
@@ -58,31 +44,29 @@ def create
     end
   end
 
-  # PUT /users/1
-  # PUT /users/1.json
-  # PUT /users/1.xml                                                       AJAX
+  # GET /users/1/edit.js
   #----------------------------------------------------------------------------
-  def update
-    @user.update_attributes(params[:user])
+  def edit
     respond_with(@user)
   end
 
-  # DELETE /users/1
-  # DELETE /users/1.xml                HTML and AJAX (not directly exposed yet)
+  # PUT /users/1
+  # PUT /users/1.js
   #----------------------------------------------------------------------------
-  def destroy
-    # not exposed
+  def update
+    @user.update_attributes(params[:user])
+    respond_with(@user)
   end
 
   # GET /users/1/avatar
-  # GET /users/1/avatar.xml                                                AJAX
+  # GET /users/1/avatar.js
   #----------------------------------------------------------------------------
   def avatar
     respond_with(@user)
   end
 
   # PUT /users/1/upload_avatar
-  # PUT /users/1/upload_avatar.xml                                         AJAX
+  # PUT /users/1/upload_avatar.js
   #----------------------------------------------------------------------------
   def upload_avatar
     if params[:gravatar]
@@ -106,19 +90,21 @@ def upload_avatar
   end
 
   # GET /users/1/password
-  # GET /users/1/password.xml                                              AJAX
+  # GET /users/1/password.js
   #----------------------------------------------------------------------------
   def password
     respond_with(@user)
   end
 
   # PUT /users/1/change_password
-  # PUT /users/1/change_password.xml                                       AJAX
+  # PUT /users/1/change_password.js
   #----------------------------------------------------------------------------
   def change_password
     if @user.valid_password?(params[:current_password], true) || @user.password_hash.blank?
       unless params[:user][:password].blank?
-        @user.update_attributes(params[:user])
+        @user.password = params[:user][:password]
+        @user.password_confirmation = params[:user][:password_confirmation]
+        @user.save
         flash[:notice] = t(:msg_password_changed)
       else
         flash[:notice] = t(:msg_password_not_changed)
@@ -130,27 +116,18 @@ def change_password
     respond_with(@user)
   end
 
-  # POST /users/1/redraw                                                   AJAX
+  # POST /users/1/redraw
   #----------------------------------------------------------------------------
   def redraw
     current_user.preference[:locale] = params[:locale]
     render(:update) { |page| page.redirect_to user_path(current_user) }
   end
 
+  # GET /users/opportunities_overview
+  #----------------------------------------------------------------------------
   def opportunities_overview
     @users_with_opportunities = User.have_assigned_opportunities.order(:first_name)
     @unassigned_opportunities = Opportunity.unassigned.pipeline.order(:stage)
   end
 
-  private
-
-  #----------------------------------------------------------------------------
-  def require_and_assign_user
-    require_user
-    @user = current_user
-  end
-
-  def assign_given_or_current_user
-    @user = params[:id] ? User.find(params[:id]) : current_user
-  end
 end

app/models/polymorphic/task.rb

@@ -27,6 +27,7 @@
 
 class Task < ActiveRecord::Base
   attr_accessor :calendar
+  ALLOWED_VIEWS = %w(pending assigned completed)
 
   belongs_to :user
   belongs_to :assignee, :class_name => "User", :foreign_key => :assigned_to
@@ -171,6 +172,7 @@ def computed_bucket
   # Returns list of tasks grouping them by due date as required by tasks/index.
   #----------------------------------------------------------------------------
   def self.find_all_grouped(user, view)
+    return {} unless ALLOWED_VIEWS.include?(view)
     settings = (view == "completed" ? Setting.task_completed : Setting.task_bucket)
     Hash[
       settings.map do |key, value|
@@ -182,7 +184,7 @@ def self.find_all_grouped(user, view)
   # Returns bucket if it's empty (i.e. we have to hide it), nil otherwise.
   #----------------------------------------------------------------------------
   def self.bucket_empty?(bucket, user, view = "pending")
-    return false if bucket.blank?
+    return false if bucket.blank? or !ALLOWED_VIEWS.include?(view)
     if view == "assigned"
       assigned_by(user).send(bucket).pending.count
     else
@@ -193,6 +195,7 @@ def self.bucket_empty?(bucket, user, view = "pending")
   # Returns task totals for each of the views as needed by tasks sidebar.
   #----------------------------------------------------------------------------
   def self.totals(user, view = "pending")
+    return {} unless ALLOWED_VIEWS.include?(view)
     settings = (view == "completed" ? Setting.task_completed : Setting.task_bucket)
     settings.inject({ :all => 0 }) do |hash, key|
       hash[key] = (view == "assigned" ? assigned_by(user).send(key).pending.count : my(user).send(key).send(view).count)