From 814d853a1af2a06bc19ecf60d78ef8fd99b3f682 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 1 Mar 2016 13:59:19 +0100 Subject: [PATCH 1/4] Fix deprecated CI build status badge permissions --- app/controllers/ci/projects_controller.rb | 3 ++ .../ci/projects_controller_spec.rb | 53 +++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 spec/controllers/ci/projects_controller_spec.rb diff --git a/app/controllers/ci/projects_controller.rb b/app/controllers/ci/projects_controller.rb index d1824b481d..471cebc82f 100644 --- a/app/controllers/ci/projects_controller.rb +++ b/app/controllers/ci/projects_controller.rb @@ -3,6 +3,7 @@ module Ci before_action :project before_action :authorize_read_project!, except: [:badge] before_action :no_cache, only: [:badge] + skip_before_action :authenticate_user!, only: [:badge] protect_from_forgery def show @@ -18,6 +19,8 @@ module Ci # def badge return render_404 unless @project + authenticate_user! unless @project.public? + image = Ci::ImageForBuildService.new.execute(@project, params) send_file image.path, filename: image.name, disposition: 'inline', type:"image/svg+xml" end diff --git a/spec/controllers/ci/projects_controller_spec.rb b/spec/controllers/ci/projects_controller_spec.rb new file mode 100644 index 0000000000..e048c5a51e --- /dev/null +++ b/spec/controllers/ci/projects_controller_spec.rb @@ -0,0 +1,53 @@ +require 'spec_helper' + +describe Ci::ProjectsController do + let(:visibility) { :public } + let!(:project) { create(:project, visibility, ci_id: 1) } + let(:ci_id) { project.ci_id } + + ## + # Specs for *deprecated* CI badge + # + describe '#badge' do + context 'user not signed in' + before { get(:badge, id: ci_id) } + + context 'project has no ci_id reference' do + let(:ci_id) { 123 } + + it 'returns 404' do + expect(response.status).to eq 404 + end + end + + context 'project is public' do + let(:visibility) { :public } + + it 'is available without authentication' do + expect(response.status).to eq 200 + end + end + + context 'project is private' do + let(:visibility) { :private } + + it 'requires authentication' do + expect(response.status).to eq 302 + end + end + + context 'user signed in' do + let(:user) { create(:user) } + before { sign_in(user) } + before { get(:badge, id: ci_id) } + + context 'private is internal' do + let(:visibility) { :internal } + + it 'shows badge to signed in user' do + expect(response.status).to eq 200 + end + end + end + end +end From 45f318bf45af4569354389f12bf9e44979f6744e Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 1 Mar 2016 14:01:32 +0100 Subject: [PATCH 2/4] Add Changelog entry for CI badge fix --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index d3e28dcfc7..c30a7c2ae3 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -9,6 +9,7 @@ v 8.6.0 (unreleased) v 8.5.2 - Fix sidebar overlapping content when screen width was below 1200px + - Fix permissions for deprecated CI build status badge - Fix error 500 when commenting on a commit v 8.5.1 From 6be22dbbe3b1f7bc8c396a98a83f5c5b51a4bbca Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 1 Mar 2016 14:42:52 +0100 Subject: [PATCH 3/4] Fix specs for deprecated CI build status badge --- spec/controllers/ci/projects_controller_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/controllers/ci/projects_controller_spec.rb b/spec/controllers/ci/projects_controller_spec.rb index e048c5a51e..569ed7c0f6 100644 --- a/spec/controllers/ci/projects_controller_spec.rb +++ b/spec/controllers/ci/projects_controller_spec.rb @@ -9,7 +9,7 @@ describe Ci::ProjectsController do # Specs for *deprecated* CI badge # describe '#badge' do - context 'user not signed in' + context 'user not signed in' do before { get(:badge, id: ci_id) } context 'project has no ci_id reference' do @@ -35,6 +35,7 @@ describe Ci::ProjectsController do expect(response.status).to eq 302 end end + end context 'user signed in' do let(:user) { create(:user) } From 8b02d962abd47e9e9c3bbd51bdd285bbb476b8d1 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 1 Mar 2016 20:32:30 +0100 Subject: [PATCH 4/4] Do not require authentication for CI status badge This changes only deprecated CI badge that we keep for backwards compatibility. See !3030#note_4041498. --- app/controllers/ci/projects_controller.rb | 1 - .../ci/projects_controller_spec.rb | 23 +++++++++---------- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/app/controllers/ci/projects_controller.rb b/app/controllers/ci/projects_controller.rb index 471cebc82f..081e01a75e 100644 --- a/app/controllers/ci/projects_controller.rb +++ b/app/controllers/ci/projects_controller.rb @@ -19,7 +19,6 @@ module Ci # def badge return render_404 unless @project - authenticate_user! unless @project.public? image = Ci::ImageForBuildService.new.execute(@project, params) send_file image.path, filename: image.name, disposition: 'inline', type:"image/svg+xml" diff --git a/spec/controllers/ci/projects_controller_spec.rb b/spec/controllers/ci/projects_controller_spec.rb index 569ed7c0f6..db0748f323 100644 --- a/spec/controllers/ci/projects_controller_spec.rb +++ b/spec/controllers/ci/projects_controller_spec.rb @@ -9,6 +9,14 @@ describe Ci::ProjectsController do # Specs for *deprecated* CI badge # describe '#badge' do + shared_examples 'badge provider' do + it 'shows badge' do + expect(response.status).to eq 200 + expect(response.headers) + .to include('Content-Type' => 'image/svg+xml') + end + end + context 'user not signed in' do before { get(:badge, id: ci_id) } @@ -22,18 +30,12 @@ describe Ci::ProjectsController do context 'project is public' do let(:visibility) { :public } - - it 'is available without authentication' do - expect(response.status).to eq 200 - end + it_behaves_like 'badge provider' end context 'project is private' do let(:visibility) { :private } - - it 'requires authentication' do - expect(response.status).to eq 302 - end + it_behaves_like 'badge provider' end end @@ -44,10 +46,7 @@ describe Ci::ProjectsController do context 'private is internal' do let(:visibility) { :internal } - - it 'shows badge to signed in user' do - expect(response.status).to eq 200 - end + it_behaves_like 'badge provider' end end end