users_controller requires security on its actions. Use CanCan 'load_a…

…nd_authorize_resource' to control this.

Conflicts:
	spec/controllers/users_controller_spec.rb

app/controllers/application_controller.rb

@@ -145,7 +145,7 @@ def redirect_back_or_default(default)
 
   #----------------------------------------------------------------------------
   def can_signup?
-    [ :allowed, :needs_approval ].include? Setting.user_signup
+    User.can_signup?
   end
 
   #----------------------------------------------------------------------------
@@ -199,10 +199,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 +213,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/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/users/ability.rb

@@ -9,11 +9,22 @@ class Ability
   include CanCan::Ability
 
   def initialize(user)
+
+    # handle signup
+    can(:create, User) if User.can_signup?
+
     if user.present?
       entities = [Account, Campaign, Contact, Lead, Opportunity]
 
-      can :create, :all
-      can :read, [User] # for search autocomplete
+      # User
+      can :manage, User, id: user.id # can do any action on themselves
+
+      # Tasks
+      can :create, Task
+      can :manage, Task, user: user.id
+      can :manage, Task, assigned_to: user.id
+
+      # Entities
       can :manage, entities, :access => 'Public'
       can :manage, entities + [Task], :user_id => user.id
       can :manage, entities + [Task], :assigned_to => user.id

app/models/users/user.rb

@@ -187,6 +187,10 @@ def current_ability
       Ability.new(User.current_user)
     end
 
+    def can_signup?
+      [ :allowed, :needs_approval ].include? Setting.user_signup
+    end
+
   end
 
   ActiveSupport.run_load_hooks(:fat_free_crm_user, self)

config/locales/en-US_fat_free_crm.yml

@@ -189,7 +189,7 @@ en-US:
   msg_account_not_approved: Your account has not been approved yet.
   msg_asset_deleted: "%{value} has been deleted."
   msg_asset_not_available: This %{value} is no longer available.
-  msg_asset_not_authorized: You are not authorized to view this %{value}.
+  msg_not_authorized: You are not authorized to take this action.
   msg_assets_not_available: The %{value} are not available.
   msg_asset_rejected: "%{value} has been rejected."
   msg_bad_image_file: "^Could't upload or resize the image file you specified."

spec/controllers/admin/users_controller_spec.rb

@@ -54,10 +54,8 @@
   #----------------------------------------------------------------------------
   describe "GET new" do
     it "assigns a new user as @user and renders [new] template" do
-      @user = User.new
-
       xhr :get, :new
-      assigns[:user].attributes.should == @user.attributes
+      expect(assigns[:user]).to be_new_record
       response.should render_template("admin/users/new")
     end
   end