Abilities extended. Resources security improved

16 files changed, 51 insertions, 52 deletions

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 5a5b4aeb8d4..ee2240b2f0f 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -48,6 +48,10 @@ class ApplicationController < ActionController::Base
     return render_404 unless can?(current_user, action, project)
   end
 
+  def authorize_code_access!
+    return render_404 unless can?(current_user, :download_code, project)
+  end
+
   def access_denied!
     render_404
   end
diff --git a/app/controllers/commits_controller.rb b/app/controllers/commits_controller.rb
index c7fcae3b79a..0b976fa8a2c 100644
--- a/app/controllers/commits_controller.rb
+++ b/app/controllers/commits_controller.rb
@@ -7,6 +7,7 @@ class CommitsController < ApplicationController
   # Authorize
   before_filter :add_project_abilities
   before_filter :authorize_read_project!
+  before_filter :authorize_code_access!
   before_filter :require_non_empty_project
   before_filter :load_refs, :only => :index # load @branch, @tag & @ref
   before_filter :render_full_content
diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb
index ed1a5864f23..36c9c8f6c51 100644
--- a/app/controllers/issues_controller.rb
+++ b/app/controllers/issues_controller.rb
@@ -126,12 +126,11 @@ class IssuesController < ApplicationController
   end
 
   def authorize_modify_issue!
-    can?(current_user, :modify_issue, @issue) || 
-      @issue.assignee == current_user
+    return render_404 unless can?(current_user, :modify_issue, @issue)
   end
 
   def authorize_admin_issue!
-    can?(current_user, :admin_issue, @issue)
+    return render_404 unless can?(current_user, :admin_issue, @issue)
   end
 
   def module_enabled
diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb
index 02c8246e37e..fa2e73291e0 100644
--- a/app/controllers/merge_requests_controller.rb
+++ b/app/controllers/merge_requests_controller.rb
@@ -112,12 +112,11 @@ class MergeRequestsController < ApplicationController
   end
 
   def authorize_modify_merge_request!
-    can?(current_user, :modify_merge_request, @merge_request) || 
-      @merge_request.assignee == current_user
+    return render_404 unless can?(current_user, :modify_merge_request, @merge_request)
   end
 
   def authorize_admin_merge_request!
-    can?(current_user, :admin_merge_request, @merge_request)
+    return render_404 unless can?(current_user, :admin_merge_request, @merge_request)
   end
 
   def module_enabled
diff --git a/app/controllers/refs_controller.rb b/app/controllers/refs_controller.rb
index 16cde44fd89..b8ab1bce219 100644
--- a/app/controllers/refs_controller.rb
+++ b/app/controllers/refs_controller.rb
@@ -4,6 +4,7 @@ class RefsController < ApplicationController
   # Authorize
   before_filter :add_project_abilities
   before_filter :authorize_read_project!
+  before_filter :authorize_code_access!
   before_filter :require_non_empty_project
 
   before_filter :ref
diff --git a/app/controllers/repositories_controller.rb b/app/controllers/repositories_controller.rb
index 7bdcdf885d9..036eb3713bf 100644
--- a/app/controllers/repositories_controller.rb
+++ b/app/controllers/repositories_controller.rb
@@ -4,6 +4,7 @@ class RepositoriesController < ApplicationController
   # Authorize
   before_filter :add_project_abilities
   before_filter :authorize_read_project!
+  before_filter :authorize_code_access!
   before_filter :require_non_empty_project
   before_filter :render_full_content
 
diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb
index efab4c4467d..ec1adeaa66f 100644
--- a/app/controllers/snippets_controller.rb
+++ b/app/controllers/snippets_controller.rb
@@ -1,6 +1,7 @@
 class SnippetsController < ApplicationController
   before_filter :authenticate_user!
   before_filter :project
+  before_filter :snippet, :only => [:show, :edit, :destroy, :update]
   layout "project"
 
   # Authorize
@@ -41,11 +42,9 @@ class SnippetsController < ApplicationController
   end
 
   def edit
-    @snippet = @project.snippets.find(params[:id])
   end
 
   def update
-    @snippet = @project.snippets.find(params[:id])
     @snippet.update_attributes(params[:snippet])
 
     if @snippet.valid?
@@ -56,15 +55,12 @@ class SnippetsController < ApplicationController
   end
 
   def show
-    @snippet = @project.snippets.find(params[:id])
     @notes = @snippet.notes
     @note = @project.notes.new(:noteable => @snippet)
     render_full_content
   end
 
   def destroy
-    @snippet = @project.snippets.find(params[:id])
-
     return access_denied! unless can?(current_user, :admin_snippet, @snippet)
 
     @snippet.destroy
@@ -73,12 +69,15 @@ class SnippetsController < ApplicationController
   end
 
   protected
+  def snippet
+    @snippet ||= @project.snippets.find(params[:id])
+  end
 
   def authorize_modify_snippet!
-    can?(current_user, :modify_snippet, @snippet)
+    return render_404 unless can?(current_user, :modify_snippet, @snippet)
   end
 
   def authorize_admin_snippet!
-    can?(current_user, :admin_snippet, @snippet)
+    return render_404 unless can?(current_user, :admin_snippet, @snippet)
   end
 end
diff --git a/app/controllers/wikis_controller.rb b/app/controllers/wikis_controller.rb
index 5e8365cffa5..9bcd20c3187 100644
--- a/app/controllers/wikis_controller.rb
+++ b/app/controllers/wikis_controller.rb
@@ -2,7 +2,7 @@ class WikisController < ApplicationController
   before_filter :project
   before_filter :add_project_abilities
   before_filter :authorize_read_wiki!
-  before_filter :authorize_write_wiki!, :except => [:show, :destroy]
+  before_filter :authorize_write_wiki!, :only => [:edit, :create, :history]
   before_filter :authorize_admin_wiki!, :only => :destroy
   layout "project"
   
@@ -12,6 +12,11 @@ class WikisController < ApplicationController
     else
       @wiki = @project.wikis.where(:slug => params[:id]).order("created_at").last
     end
+
+    unless @wiki
+      return render_404 unless can?(current_user, :write_wiki, @project)
+    end
+
     respond_to do |format|
       if @wiki
         format.html
@@ -51,18 +56,4 @@ class WikisController < ApplicationController
       format.html { redirect_to project_wiki_path(@project, :index), notice: "Page was successfully deleted" }
     end
   end
-
-  protected 
-
-  def authorize_read_wiki!
-    can?(current_user, :read_wiki, @project)
-  end
-
-  def authorize_write_wiki!
-    can?(current_user, :write_wiki, @project)
-  end
-
-  def authorize_admin_wiki!
-    can?(current_user, :admin_wiki, @project)
-  end
 end
diff --git a/app/models/ability.rb b/app/models/ability.rb
index c7fddec21f6..e97b662b8ce 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -5,7 +5,7 @@ class Ability
     when "Issue" then issue_abilities(object, subject)
     when "Note" then note_abilities(object, subject)
     when "Snippet" then snippet_abilities(object, subject)
-    when "Wiki" then wiki_abilities(object, subject)
+    when "MergeRequest" then merge_request_abilities(object, subject)
     else []
     end
   end
@@ -23,13 +23,13 @@ class Ability
       :read_note,
       :write_project,
       :write_issue,
-      :write_snippet,
-      :write_merge_request,
       :write_note
     ] if project.guest_access_for?(user)
 
     rules << [
       :download_code,
+      :write_merge_request,
+      :write_snippet
     ] if project.report_access_for?(user)
 
     rules << [
@@ -39,7 +39,7 @@ class Ability
     rules << [
       :modify_issue,
       :modify_snippet,
-      :modify_wiki,
+      :modify_merge_request,
       :admin_project,
       :admin_issue,
       :admin_snippet,
@@ -47,7 +47,7 @@ class Ability
       :admin_merge_request,
       :admin_note,
       :admin_wiki
-    ] if project.master_access_for?(user)
+    ] if project.master_access_for?(user) || project.owner == user
 
 
     rules.flatten
@@ -63,6 +63,12 @@ class Ability
             :"modify_#{name}",
             :"admin_#{name}"
           ]
+        elsif subject.respond_to?(:assignee) && subject.assignee == user
+          [
+            :"read_#{name}",
+            :"write_#{name}",
+            :"modify_#{name}",
+          ]
         else
           subject.respond_to?(:project) ?
             project_abilities(user, subject.project) : []
diff --git a/app/models/project.rb b/app/models/project.rb
index f5b9b54c0aa..b59dcd80c87 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -188,7 +188,7 @@ class Project < ActiveRecord::Base
              elsif access.include?(:write)
                { :project_access => UsersProject::DEVELOPER } 
              else
-               { :project_access => UsersProject::GUEST } 
+               { :project_access => UsersProject::REPORTER } 
              end
     opts = { :user => user }
     opts.merge!(access)
diff --git a/app/views/help/permissions.html.haml b/app/views/help/permissions.html.haml
index 5c7ba54ca4d..0e6e351b43e 100644
--- a/app/views/help/permissions.html.haml
+++ b/app/views/help/permissions.html.haml
@@ -4,15 +4,17 @@
 %h4 Guest
 %ul
   %li Create new issue
-  %li Create new merge request
+  %li Leave comments
   %li Write on project wall
 
 %h4 Reporter
 %ul
   %li Pull project code
+  %li Download project
   %li Create new issue
   %li Create new merge request
   %li Write on project wall
+  %li Create a code snippets
 
 
 %h4 Developer
@@ -25,6 +27,7 @@
   %li Create new issue
   %li Create new merge request
   %li Write on project wall
+  %li Write a wiki
 
 %h4 Master
 %ul
diff --git a/app/views/issues/_show.html.haml b/app/views/issues/_show.html.haml
index 3229aa5c91b..1d5cbd21d75 100644
--- a/app/views/issues/_show.html.haml
+++ b/app/views/issues/_show.html.haml
@@ -1,11 +1,10 @@
 %li.wll{ :id => dom_id(issue), :class => "issue #{issue.critical ? "critical" : ""}", :url => project_issue_path(issue.project, issue) }
   .right
-    - if can? current_user, :write_issue, issue
+    - if can? current_user, :modify_issue, issue
       - if issue.closed
         = link_to 'Reopen', project_issue_path(issue.project, issue, :issue => {:closed => false }, :status_only => true), :method => :put,  :class => "btn small", :remote => true
       - else
         = link_to 'Resolve', project_issue_path(issue.project, issue, :issue => {:closed => true }, :status_only => true), :method => :put, :class => "success btn small", :remote => true
-    - if can? current_user, :write_issue, issue
       = link_to 'Edit', edit_project_issue_path(issue.project, issue), :class => "btn small edit-issue-link", :remote => true
     -#- if can?(current_user, :admin_issue, @project) || issue.author == current_user
       = link_to 'Remove', [issue.project, issue], :confirm => 'Are you sure?', :method => :delete, :remote => true, :class => "danger btn small delete-issue", :id => "destroy_issue_#{issue.id}"
diff --git a/app/views/layouts/_project_menu.html.haml b/app/views/layouts/_project_menu.html.haml
index 465b550e434..1f1b2e601a2 100644
--- a/app/views/layouts/_project_menu.html.haml
+++ b/app/views/layouts/_project_menu.html.haml
@@ -4,8 +4,9 @@
     Project
 
   - if @project.repo_exists?
-    = link_to "Files", tree_project_ref_path(@project, @project.root_ref), :class => tree_tab_class
-    = link_to "Commits", project_commits_path(@project), :class => commit_tab_class
+    - if can? current_user, :download_code, @project
+      = link_to "Files", tree_project_ref_path(@project, @project.root_ref), :class => tree_tab_class
+      = link_to "Commits", project_commits_path(@project), :class => commit_tab_class
 
     = link_to "Network", graph_project_path(@project), :class => current_page?(:controller => "projects", :action => "graph", :id => @project) ? "current" : nil
     - if @project.issues_enabled
diff --git a/app/views/merge_requests/show.html.haml b/app/views/merge_requests/show.html.haml
index dd3fc7c608b..0a07f0bc6ae 100644
--- a/app/views/merge_requests/show.html.haml
+++ b/app/views/merge_requests/show.html.haml
@@ -10,12 +10,11 @@
     = @merge_request.created_at.stamp("Aug 21, 2011")
 
   %span.right
-    - if can?(current_user, :admin_project, @project) || @merge_request.author == current_user
+    - if can?(current_user, :modify_merge_request, @merge_request)
       - if @merge_request.closed
         = link_to 'Reopen', project_merge_request_path(@project, @merge_request, :merge_request => {:closed => false }, :status_only => true), :method => :put,  :class => "btn"
       - else
         = link_to 'Close', project_merge_request_path(@project, @merge_request, :merge_request => {:closed => true }, :status_only => true), :method => :put, :class => "btn", :title => "Close merge request"
-    - if can?(current_user, :admin_project, @project) || @merge_request.author == current_user
       = link_to edit_project_merge_request_path(@project, @merge_request), :class => "btn small" do 
         Edit
 
diff --git a/app/views/widgets/_project_member.html.haml b/app/views/widgets/_project_member.html.haml
index 5756ecc931d..131853fa5af 100644
--- a/app/views/widgets/_project_member.html.haml
+++ b/app/views/widgets/_project_member.html.haml
@@ -11,23 +11,19 @@
   %p 
     - if @project.issues_enabled
       %span
-        Assigned issues:
+        Assigned Issues:
         = current_user.assigned_issues.opened.count
       %br
     - if @project.merge_requests_enabled
       %span
-        Assigned merge request:
-        = current_user.assigned_merge_requests.opened.count
-      %br
-      %span
-        Your merge requests:
+        Assigned Requests:
         = current_user.assigned_merge_requests.opened.count
       %br
     %br
-    - if @project.merge_requests_enabled
+    - if @project.merge_requests_enabled && can?(current_user, :write_merge_request, @project)
       = link_to new_project_merge_request_path(@project), :title => "New Merge Request", :class => "btn small padded" do 
         Merge Request
-    - if @project.issues_enabled
+    - if @project.issues_enabled && can?(current_user, :write_issue, @project)
       = link_to new_project_issue_path(@project), :title => "New Issue", :class => "btn small" do 
         Issue
 
diff --git a/app/views/wikis/show.html.haml b/app/views/wikis/show.html.haml
index 1395a5905f0..696f6ec753c 100644
--- a/app/views/wikis/show.html.haml
+++ b/app/views/wikis/show.html.haml
@@ -4,13 +4,13 @@
     - if can? current_user, :write_wiki, @project
       = link_to history_project_wiki_path(@project, @wiki), :class => "btn small padded" do
         History
-    = link_to edit_project_wiki_path(@project, @wiki), :class => "btn small" do
-      Edit
+      = link_to edit_project_wiki_path(@project, @wiki), :class => "btn small" do
+        Edit
 %hr
 
 = markdown_to_html @wiki.content
 
 %p.time Last edited by #{@wiki.user.name}, in #{time_ago_in_words @wiki.created_at}
-- if can? current_user, :write_wiki, @project
+- if can? current_user, :admin_wiki, @project
   = link_to project_wiki_path(@project, @wiki), :confirm => "Are you sure you want to delete this page?", :method => :delete do
     Delete this page