Commit da903949 authored by Avielle Wolfe's avatar Avielle Wolfe Committed by Grzegorz Bizon

Fix empty security dashboard for public projects

Project security dashboards were displaying no vulnerabilities for
public projects, even when the latest pipeline reported
vulnerabilities.

The issue was our Namespace#store_security_reports_available? method,
which had no knowledge of the project or whether it was public, and
only checked for availability based on the instance's license.

I've added Project#can_store_security_reports?, which calls into
Namespace to check for license availability and _also_ checks
whether the project is public. Now public projects can get the
security scanning they've been wanting!

https://gitlab.com/gitlab-org/gitlab/issues/13422
parent 42e9daf2
---
title: Fix empty security dashboard for public projects
merge_request: 17915
author:
type: fixed
...@@ -166,8 +166,6 @@ module EE ...@@ -166,8 +166,6 @@ module EE
default_value_for :packages_enabled, true default_value_for :packages_enabled, true
delegate :store_security_reports_available?, to: :namespace
accepts_nested_attributes_for :tracing_setting, update_only: true, allow_destroy: true accepts_nested_attributes_for :tracing_setting, update_only: true, allow_destroy: true
accepts_nested_attributes_for :alerting_setting, update_only: true accepts_nested_attributes_for :alerting_setting, update_only: true
accepts_nested_attributes_for :incident_management_setting, update_only: true accepts_nested_attributes_for :incident_management_setting, update_only: true
...@@ -186,6 +184,10 @@ module EE ...@@ -186,6 +184,10 @@ module EE
end end
end end
def can_store_security_reports?
namespace.store_security_reports_available? || public?
end
def tracing_external_url def tracing_external_url
self.tracing_setting.try(:external_url) self.tracing_setting.try(:external_url)
end end
......
...@@ -8,7 +8,7 @@ class StoreSecurityReportsWorker ...@@ -8,7 +8,7 @@ class StoreSecurityReportsWorker
def perform(pipeline_id) def perform(pipeline_id)
Ci::Pipeline.find(pipeline_id).try do |pipeline| Ci::Pipeline.find(pipeline_id).try do |pipeline|
break unless pipeline.project.store_security_reports_available? break unless pipeline.project.can_store_security_reports?
::Security::StoreReportsService.new(pipeline).execute ::Security::StoreReportsService.new(pipeline).execute
end end
......
...@@ -333,6 +333,35 @@ describe Project do ...@@ -333,6 +333,35 @@ describe Project do
end end
end end
describe '#can_store_security_reports?' do
context 'when the feature is enabled for the namespace' do
it 'returns true' do
stub_licensed_features(sast: true)
project = create(:project, :private)
expect(project.can_store_security_reports?).to be_truthy
end
end
context 'when the project is public' do
it 'returns true' do
stub_licensed_features(sast: false)
project = create(:project, :public)
expect(project.can_store_security_reports?).to be_truthy
end
end
context 'when the feature is disabled for the namespace and the project is not public' do
it 'returns false' do
stub_licensed_features(sast: false)
project = create(:project, :private)
expect(project.can_store_security_reports?).to be_falsy
end
end
end
describe '#deployment_variables' do describe '#deployment_variables' do
context 'when project has a deployment platforms' do context 'when project has a deployment platforms' do
context 'when multiple clusters (EEP) is enabled' do context 'when multiple clusters (EEP) is enabled' do
...@@ -1661,18 +1690,6 @@ describe Project do ...@@ -1661,18 +1690,6 @@ describe Project do
end end
end end
describe '#store_security_reports_available?' do
let(:project) { create(:project) }
subject { project.store_security_reports_available? }
it 'delegates to namespace' do
expect(project.namespace).to receive(:store_security_reports_available?).once.and_call_original
subject
end
end
describe '#has_pool_repository?' do describe '#has_pool_repository?' do
it 'returns false when there is no pool repository' do it 'returns false when there is no pool repository' do
project = create(:project) project = create(:project)
......
...@@ -5,7 +5,7 @@ require 'spec_helper' ...@@ -5,7 +5,7 @@ require 'spec_helper'
describe StoreSecurityReportsWorker do describe StoreSecurityReportsWorker do
describe '#perform' do describe '#perform' do
let(:group) { create(:group) } let(:group) { create(:group) }
let(:project) { create(:project, :public, namespace: group) } let(:project) { create(:project, namespace: group) }
let(:pipeline) { create(:ci_pipeline, ref: 'master', project: project) } let(:pipeline) { create(:ci_pipeline, ref: 'master', project: project) }
before do before do
......
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