Commit d2a4ccdc authored by Jose Vargas's avatar Jose Vargas

Add permissions check to pipelines#show action

This check renders a 404 in case the user trying
to access the pipeline details page doesn't have enough
permissions

Changelog: security
parent 4ad862fa
...@@ -8,7 +8,7 @@ class Projects::PipelinesController < Projects::ApplicationController ...@@ -8,7 +8,7 @@ class Projects::PipelinesController < Projects::ApplicationController
before_action :pipeline, except: [:index, :new, :create, :charts, :config_variables] before_action :pipeline, except: [:index, :new, :create, :charts, :config_variables]
before_action :set_pipeline_path, only: [:show] before_action :set_pipeline_path, only: [:show]
before_action :authorize_read_pipeline! before_action :authorize_read_pipeline!
before_action :authorize_read_build!, only: [:index] before_action :authorize_read_build!, only: [:index, :show]
before_action :authorize_read_analytics!, only: [:charts] before_action :authorize_read_analytics!, only: [:charts]
before_action :authorize_create_pipeline!, only: [:new, :create, :config_variables] before_action :authorize_create_pipeline!, only: [:new, :create, :config_variables]
before_action :authorize_update_pipeline!, only: [:retry, :cancel] before_action :authorize_update_pipeline!, only: [:retry, :cancel]
......
...@@ -302,35 +302,46 @@ RSpec.describe Projects::PipelinesController do ...@@ -302,35 +302,46 @@ RSpec.describe Projects::PipelinesController do
end end
describe 'GET #show' do describe 'GET #show' do
render_views
let_it_be(:pipeline) { create(:ci_pipeline, project: project) }
subject { get_pipeline_html }
def get_pipeline_html def get_pipeline_html
get :show, params: { namespace_id: project.namespace, project_id: project, id: pipeline }, format: :html get :show, params: { namespace_id: project.namespace, project_id: project, id: pipeline }, format: :html
end end
def create_build_with_artifacts(stage, stage_idx, name) context 'when the project is public' do
create(:ci_build, :artifacts, :tags, pipeline: pipeline, stage: stage, stage_idx: stage_idx, name: name) render_views
end
before do let_it_be(:pipeline) { create(:ci_pipeline, project: project) }
create_build_with_artifacts('build', 0, 'job1')
create_build_with_artifacts('build', 0, 'job2') def create_build_with_artifacts(stage, stage_idx, name)
create(:ci_build, :artifacts, :tags, pipeline: pipeline, stage: stage, stage_idx: stage_idx, name: name)
end
before do
create_build_with_artifacts('build', 0, 'job1')
create_build_with_artifacts('build', 0, 'job2')
end
it 'avoids N+1 database queries', :request_store do
control_count = ActiveRecord::QueryRecorder.new { get_pipeline_html }.count
expect(response).to have_gitlab_http_status(:ok)
create_build_with_artifacts('build', 0, 'job3')
expect { get_pipeline_html }.not_to exceed_query_limit(control_count)
expect(response).to have_gitlab_http_status(:ok)
end
end end
it 'avoids N+1 database queries', :request_store do context 'when the project is private' do
get_pipeline_html let(:project) { create(:project, :private, :repository) }
let(:pipeline) { create(:ci_pipeline, project: project) }
control_count = ActiveRecord::QueryRecorder.new { get_pipeline_html }.count it 'returns `not_found` when the user does not have access' do
expect(response).to have_gitlab_http_status(:ok) sign_in(create(:user))
create_build_with_artifacts('build', 0, 'job3') get_pipeline_html
expect { get_pipeline_html }.not_to exceed_query_limit(control_count) expect(response).to have_gitlab_http_status(:not_found)
expect(response).to have_gitlab_http_status(:ok) end
end end
end end
......
...@@ -365,9 +365,8 @@ RSpec.describe 'Pipeline', :js do ...@@ -365,9 +365,8 @@ RSpec.describe 'Pipeline', :js do
let(:project) { create(:project, :public, :repository, public_builds: false) } let(:project) { create(:project, :public, :repository, public_builds: false) }
let(:role) { :guest } let(:role) { :guest }
it 'does not show failed jobs tab pane' do it 'does not show the pipeline details page' do
expect(page).to have_link('Pipeline') expect(page).to have_content('Not Found')
expect(page).not_to have_content('Failed Jobs')
end end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment