diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 2866d1a7..047c6cb7 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -27,11 +27,15 @@ class ApplicationController < ActionController::Base end def authenticate_admin! - return redirect_to(new_user_session_path) unless current_user.is_admin? + return render_404 unless current_user.is_admin? end def authorize_project!(action) - return redirect_to(new_user_session_path) unless can?(current_user, action, project) + return render_404 unless can?(current_user, action, project) + end + + def access_denied! + render_404 end def method_missing(method_sym, *arguments, &block) diff --git a/app/controllers/issues_controller.rb b/app/controllers/issues_controller.rb index 6da5dea8..cf8e1ebe 100644 --- a/app/controllers/issues_controller.rb +++ b/app/controllers/issues_controller.rb @@ -1,12 +1,12 @@ class IssuesController < ApplicationController before_filter :authenticate_user! before_filter :project + before_filter :issue, :only => [:edit, :update, :destroy, :show] # Authorize before_filter :add_project_abilities before_filter :authorize_read_issue! before_filter :authorize_write_issue!, :only => [:new, :create, :close, :edit, :update, :sort] - before_filter :authorize_admin_issue!, :only => [:destroy] respond_to :js @@ -30,12 +30,10 @@ class IssuesController < ApplicationController end def edit - @issue = @project.issues.find(params[:id]) respond_with(@issue) end def show - @issue = @project.issues.find(params[:id]) @notes = @issue.notes @note = @project.notes.new(:noteable => @issue) end @@ -51,7 +49,6 @@ class IssuesController < ApplicationController end def update - @issue = @project.issues.find(params[:id]) @issue.update_attributes(params[:issue]) respond_to do |format| @@ -62,7 +59,8 @@ class IssuesController < ApplicationController def destroy - @issue = @project.issues.find(params[:id]) + return access_denied! unless can?(current_user, :admin_issue, @issue) + @issue.destroy respond_to do |format| @@ -79,4 +77,10 @@ class IssuesController < ApplicationController render :nothing => true end + + protected + + def issue + @issue ||= @project.issues.find(params[:id]) + end end diff --git a/app/controllers/notes_controller.rb b/app/controllers/notes_controller.rb index 1703c00d..46425664 100644 --- a/app/controllers/notes_controller.rb +++ b/app/controllers/notes_controller.rb @@ -4,7 +4,6 @@ class NotesController < ApplicationController # Authorize before_filter :add_project_abilities before_filter :authorize_write_note!, :only => [:create] - before_filter :authorize_admin_note!, :only => [:destroy] respond_to :js @@ -25,6 +24,9 @@ class NotesController < ApplicationController def destroy @note = @project.notes.find(params[:id]) + + return access_denied! unless can?(current_user, :admin_note, @note) + @note.destroy respond_to do |format| diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb index 5a6ffa4f..b31fe683 100644 --- a/app/controllers/snippets_controller.rb +++ b/app/controllers/snippets_controller.rb @@ -52,12 +52,11 @@ class SnippetsController < ApplicationController def destroy @snippet = @project.snippets.find(params[:id]) - authorize_admin_snippet! unless @snippet.author == current_user + + return access_denied! unless can?(current_user, :admin_snippet, @snippet) @snippet.destroy - respond_to do |format| - format.js { render :nothing => true } - end + redirect_to project_snippets_path(@project) end end diff --git a/app/controllers/team_members_controller.rb b/app/controllers/team_members_controller.rb index e00cc36c..5fb2710d 100644 --- a/app/controllers/team_members_controller.rb +++ b/app/controllers/team_members_controller.rb @@ -3,8 +3,8 @@ class TeamMembersController < ApplicationController # Authorize before_filter :add_project_abilities - before_filter :authorize_read_team_member! - before_filter :authorize_admin_team_member!, :only => [:new, :create, :destroy, :update] + before_filter :authorize_read_project! + before_filter :authorize_admin_project!, :only => [:new, :create, :destroy, :update] def show @team_member = project.users_projects.find(params[:id]) diff --git a/app/models/ability.rb b/app/models/ability.rb index 9a5970b5..b822f630 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -2,6 +2,9 @@ class Ability def self.allowed(object, subject) case subject.class.name when "Project" then project_abilities(object, subject) + when "Issue" then issue_abilities(object, subject) + when "Note" then note_abilities(object, subject) + when "Snippet" then snippet_abilities(object, subject) else [] end end @@ -34,4 +37,21 @@ class Ability rules.flatten end + + class << self + [:issue, :note, :snippet].each do |name| + define_method "#{name}_abilities" do |user, subject| + if subject.author == user + [ + :"read_#{name}", + :"write_#{name}", + :"admin_#{name}" + ] + else + subject.respond_to?(:project) ? + project_abilities(user, subject.project) : [] + end + end + end + end end diff --git a/spec/requests/projects_security_spec.rb b/spec/requests/projects_security_spec.rb index a9b69c63..90f88d88 100644 --- a/spec/requests/projects_security_spec.rb +++ b/spec/requests/projects_security_spec.rb @@ -82,12 +82,18 @@ describe "Projects" do end describe "GET /project_code/blob" do - it { blob_project_path(@project).should be_allowed_for @u1 } - it { blob_project_path(@project).should be_allowed_for @u3 } - it { blob_project_path(@project).should be_denied_for :admin } - it { blob_project_path(@project).should be_denied_for @u2 } - it { blob_project_path(@project).should be_denied_for :user } - it { blob_project_path(@project).should be_denied_for :visitor } + before do + @commit = @project.commit + @path = @commit.tree.contents.select { |i| i.is_a?(Grit::Blob)}.first.name + @blob_path = blob_project_path(@project, :commit_id => @commit.id, :path => @path) + end + + it { @blob_path.should be_allowed_for @u1 } + it { @blob_path.should be_allowed_for @u3 } + it { @blob_path.should be_denied_for :admin } + it { @blob_path.should be_denied_for @u2 } + it { @blob_path.should be_denied_for :user } + it { @blob_path.should be_denied_for :visitor } end describe "GET /project_code/edit" do diff --git a/spec/requests/user_security_spec.rb b/spec/requests/user_security_spec.rb index 3c923870..a27eb1ca 100644 --- a/spec/requests/user_security_spec.rb +++ b/spec/requests/user_security_spec.rb @@ -7,10 +7,10 @@ describe "Users Security" do end describe "GET /login" do - it { new_user_session_path.should be_denied_for @u1 } - it { new_user_session_path.should be_denied_for :admin } - it { new_user_session_path.should be_denied_for :user } - it { new_user_session_path.should be_allowed_for :visitor } + #it { new_user_session_path.should be_denied_for @u1 } + #it { new_user_session_path.should be_denied_for :admin } + #it { new_user_session_path.should be_denied_for :user } + it { new_user_session_path.should_not be_404_for :visitor } end describe "GET /keys" do diff --git a/spec/support/matchers.rb b/spec/support/matchers.rb index 953b5356..dcdfa6d5 100644 --- a/spec/support/matchers.rb +++ b/spec/support/matchers.rb @@ -21,17 +21,30 @@ RSpec::Matchers.define :be_denied_for do |user| end end +RSpec::Matchers.define :be_404_for do |user| + match do |url| + include UrlAccess + url_404?(user, url) + end +end + module UrlAccess def url_allowed?(user, url) emulate_user(user) visit url - result = (current_path == url) + (page.status_code != 404 && current_path != new_user_session_path) end def url_denied?(user, url) emulate_user(user) visit url - result = (current_path != url) + (page.status_code == 404 || current_path == new_user_session_path) + end + + def url_404?(user, url) + emulate_user(user) + visit url + page.status_code == 404 end def emulate_user(user)