Abilities refactoring

7 files changed, 83 insertions, 8 deletions

diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index daaf8fa2f19..9bf22d8cddc 100644
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -6,8 +6,18 @@ class IssuesController < ApplicationController
 
   # Authorize
   before_filter :add_project_abilities
+
+  # Allow read any issue
   before_filter :authorize_read_issue!
-  before_filter :authorize_write_issue!, :only => [:new, :create, :close, :edit, :update, :sort]
+
+  # Allow write(create) issue
+  before_filter :authorize_write_issue!, :only => [:new, :create]
+
+  # Allow modify issue
+  before_filter :authorize_modify_issue!, :only => [:close, :edit, :update, :sort]
+
+  # Allow destroy issue
+  before_filter :authorize_admin_issue!, :only => [:destroy]
 
   respond_to :js, :html
 
@@ -115,4 +125,13 @@ class IssuesController < ApplicationController
   def issue
     @issue ||= @project.issues.find(params[:id])
   end
+
+  def authorize_modify_issue!
+    can?(current_user, :modify_issue, @issue) || 
+      @issue.assignee == current_user
+  end
+
+  def authorize_admin_issue!
+    can?(current_user, :admin_issue, @issue)
+  end
 end
diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb
index f01a8a1db0c..6a1404106ea 100644
--- a/app/controllers/merge_requests_controller.rb
+++ b/app/controllers/merge_requests_controller.rb
@@ -6,8 +6,18 @@ class MergeRequestsController < ApplicationController
 
   # Authorize
   before_filter :add_project_abilities
-  before_filter :authorize_read_project!
-  before_filter :authorize_write_project!, :only => [:new, :create, :edit, :update]
+
+  # Allow read any merge_request
+  before_filter :authorize_read_merge_request!
+
+  # Allow write(create) merge_request
+  before_filter :authorize_write_merge_request!, :only => [:new, :create]
+
+  # Allow modify merge_request
+  before_filter :authorize_modify_merge_request!, :only => [:close, :edit, :update, :sort]
+
+  # Allow destroy merge_request
+  before_filter :authorize_admin_merge_request!, :only => [:destroy]
 
   def index
     @merge_requests = @project.merge_requests
@@ -85,4 +95,13 @@ class MergeRequestsController < ApplicationController
   def merge_request
     @merge_request ||= @project.merge_requests.find(params[:id])
   end
+
+  def authorize_modify_merge_request!
+    can?(current_user, :modify_merge_request, @merge_request) || 
+      @merge_request.assignee == current_user
+  end
+
+  def authorize_admin_merge_request!
+    can?(current_user, :admin_merge_request, @merge_request)
+  end
 end
diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb
index 5bf300566ee..b8e04f1c1a8 100644
--- a/app/controllers/notes_controller.rb
+++ b/app/controllers/notes_controller.rb
@@ -3,6 +3,8 @@ class NotesController < ApplicationController
 
   # Authorize
   before_filter :add_project_abilities
+
+  before_filter :authorize_read_note!
   before_filter :authorize_write_note!, :only => [:create]
 
   respond_to :js
diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb
index 54ad6019f75..45b3f529c4c 100644
--- a/app/controllers/snippets_controller.rb
+++ b/app/controllers/snippets_controller.rb
@@ -5,8 +5,18 @@ class SnippetsController < ApplicationController
 
   # Authorize
   before_filter :add_project_abilities
+
+  # Allow read any snippet
   before_filter :authorize_read_snippet!
-  before_filter :authorize_write_snippet!, :only => [:new, :create, :close, :edit, :update, :sort]
+
+  # Allow write(create) snippet
+  before_filter :authorize_write_snippet!, :only => [:new, :create]
+
+  # Allow modify snippet
+  before_filter :authorize_modify_snippet!, :only => [:edit, :update]
+
+  # Allow destroy snippet
+  before_filter :authorize_admin_snippet!, :only => [:destroy]
 
   respond_to :html
 
@@ -60,4 +70,14 @@ class SnippetsController < ApplicationController
 
     redirect_to project_snippets_path(@project)
   end
+
+  protected
+
+  def authorize_modify_snippet!
+    can?(current_user, :modify_snippet, @snippet)
+  end
+
+  def authorize_admin_snippet!
+    can?(current_user, :admin_snippet, @snippet)
+  end
 end
diff --git a/app/controllers/team_members_controller.rb b/app/controllers/team_members_controller.rb
index b17c9a30d88..d9a7e29be04 100644
--- a/app/controllers/team_members_controller.rb
+++ b/app/controllers/team_members_controller.rb
@@ -5,7 +5,7 @@ class TeamMembersController < ApplicationController
   # Authorize
   before_filter :add_project_abilities
   before_filter :authorize_read_project!
-  before_filter :authorize_admin_project!, :only => [:new, :create, :destroy, :update]
+  before_filter :authorize_admin_project!, :except => [:show]
 
   def show
     @team_member = project.users_projects.find(params[:id])
diff --git a/app/models/ability.rb b/app/models/ability.rb
index c41704f9a11..a02f44a4cb8 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -19,7 +19,7 @@ class Ability
       :read_team_member,
       :read_merge_request,
       :read_note
-    ] if project.readers.include?(user)
+    ] if project.allow_read_for?(user)
 
     rules << [
       :write_project,
@@ -27,16 +27,18 @@ class Ability
       :write_snippet,
       :write_merge_request,
       :write_note
-    ] if project.writers.include?(user)
+    ] if project.allow_write_for?(user)
 
     rules << [
+      :modify_issue,
+      :modify_snippet,
       :admin_project,
       :admin_issue,
       :admin_snippet,
       :admin_team_member,
       :admin_merge_request,
       :admin_note
-    ] if project.admins.include?(user)
+    ] if project.allow_admin_for?(user)
 
     rules.flatten
   end
@@ -48,6 +50,7 @@ class Ability
           [
             :"read_#{name}",
             :"write_#{name}",
+            :"modify_#{name}",
             :"admin_#{name}"
           ]
         else
diff --git a/app/models/project.rb b/app/models/project.rb
index 98b482affca..56d55fa2bc8 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -161,6 +161,18 @@ class Project < ActiveRecord::Base
     @admins ||= users_projects.includes(:user).where(:project_access => PROJECT_RWA).map(&:user)
   end
 
+  def allow_read_for?(user)
+    !users_projects.where(:user_id => user.id, :project_access => [PROJECT_R, PROJECT_RW, PROJECT_RWA]).empty?
+  end
+
+  def allow_write_for?(user)
+    !users_projects.where(:user_id => user.id, :project_access => [PROJECT_RW, PROJECT_RWA]).empty?
+  end
+
+  def allow_admin_for?(user)
+    !users_projects.where(:user_id => user.id, :project_access => [PROJECT_RWA]).empty? || owner_id == user.id
+  end
+
   def root_ref 
     default_branch || "master"
   end