From 07efb17e10fe26a01b60d8441868f9fbda0768f2 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Wed, 17 Jun 2015 07:33:51 -0700 Subject: [PATCH] Fix 403 Access Denied error messages when accessing Labels section in a project that has MRs disabled but issues enabled Closes #1813 --- app/controllers/application_controller.rb | 7 +--- app/controllers/projects/labels_controller.rb | 2 +- app/models/ability.rb | 1 + .../application_controller_spec.rb | 40 +++++++++++++++++++ 4 files changed, 43 insertions(+), 7 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 62d46a5482e..a657d3c54ee 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -89,7 +89,7 @@ class ApplicationController < ActionController::Base end def after_sign_out_path_for(resource) - current_application_settings.after_sign_out_path || new_user_session_path + current_application_settings.after_sign_out_path || new_user_session_path end def abilities @@ -140,11 +140,6 @@ class ApplicationController < ActionController::Base return access_denied! unless can?(current_user, action, project) end - def authorize_labels! - # Labels should be accessible for issues and/or merge requests - authorize_read_issue! || authorize_read_merge_request! - end - def access_denied! render "errors/access_denied", layout: "errors", status: 404 end diff --git a/app/controllers/projects/labels_controller.rb b/app/controllers/projects/labels_controller.rb index 2f8cb203cf9..86d6e3e0f6b 100644 --- a/app/controllers/projects/labels_controller.rb +++ b/app/controllers/projects/labels_controller.rb @@ -1,7 +1,7 @@ class Projects::LabelsController < Projects::ApplicationController before_action :module_enabled before_action :label, only: [:edit, :update, :destroy] - before_action :authorize_labels! + before_action :authorize_read_label! before_action :authorize_admin_labels!, except: [:index] respond_to :js, :html diff --git a/app/models/ability.rb b/app/models/ability.rb index 4e6c60dc8ca..bcd2adee00b 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -138,6 +138,7 @@ class Ability :read_project, :read_wiki, :read_issue, + :read_label, :read_milestone, :read_project_snippet, :read_project_member, diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 186239d3096..55851befc8c 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -30,4 +30,44 @@ describe ApplicationController do controller.send(:check_password_expiration) end end + + describe 'check labels authorization' do + let(:project) { create(:project) } + let(:user) { create(:user) } + let(:controller) { ApplicationController.new } + + before do + project.team << [user, :guest] + allow(controller).to receive(:current_user).and_return(user) + allow(controller).to receive(:project).and_return(project) + end + + it 'should succeed if issues and MRs are enabled' do + project.issues_enabled = true + project.merge_requests_enabled = true + controller.send(:authorize_read_label!) + expect(response.status).to eq(200) + end + + it 'should succeed if issues are enabled, MRs are disabled' do + project.issues_enabled = true + project.merge_requests_enabled = false + controller.send(:authorize_read_label!) + expect(response.status).to eq(200) + end + + it 'should succeed if issues are disabled, MRs are enabled' do + project.issues_enabled = false + project.merge_requests_enabled = true + controller.send(:authorize_read_label!) + expect(response.status).to eq(200) + end + + it 'should fail if issues and MRs are disabled' do + project.issues_enabled = false + project.merge_requests_enabled = false + expect(controller).to receive(:access_denied!) + controller.send(:authorize_read_label!) + end + end end