Commit 9c5ccc6f authored by Tetiana Chupryna's avatar Tetiana Chupryna

Add permissions for Security bot

- Add reporter rights for projects where auto-fix enabled
- Add additional right to create vuln feedback
parent 9097f561
......@@ -722,6 +722,7 @@ class User < ApplicationRecord
u.name = 'GitLab Security Bot'
u.website_url = Gitlab::Routing.url_helpers.help_page_url('user/application_security/security_bot/index.md')
u.avatar = bot_avatar(image: 'security-bot.png')
u.confirmed_at = Time.zone.now
end
end
......
......@@ -25,6 +25,10 @@ class BasePolicy < DeclarativePolicy::Base
with_options scope: :user, score: 0
condition(:support_bot) { @user&.support_bot? }
desc "User is security bot"
with_options scope: :user, score: 0
condition(:security_bot) { @user&.security_bot? }
desc "User email is unconfirmed or user account is locked"
with_options scope: :user, score: 0
condition(:inactive) { @user&.confirmation_required_on_sign_in? || @user&.access_locked? }
......
......@@ -49,6 +49,10 @@ module PolicyActor
false
end
def security_bot?
false
end
def deactivated?
false
end
......
......@@ -48,7 +48,7 @@ class GlobalPolicy < BasePolicy
prevent :use_slash_commands
end
rule { blocked | (internal & ~migration_bot) }.policy do
rule { blocked | (internal & ~migration_bot & ~security_bot) }.policy do
prevent :access_git
end
......
......@@ -75,3 +75,5 @@ class PipelineSerializer < BaseSerializer
]
end
end
PipelineSerializer.prepend_if_ee('EE::PipelineSerializer')
......@@ -31,6 +31,8 @@ module EE
after_update :remove_mirror_repository_reference,
if: ->(project) { project.mirror? && project.import_url_updated? }
after_create :create_security_setting, unless: :security_setting
belongs_to :mirror_user, class_name: 'User'
belongs_to :deleting_user, foreign_key: 'marked_for_deletion_by_user_id', class_name: 'User'
......
......@@ -163,6 +163,7 @@ class License < ApplicationRecord
status_page
subepics
threat_monitoring
vulnerability_auto_fix
quality_management
]
EEU_FEATURES.freeze
......
......@@ -3,11 +3,23 @@
class ProjectSecuritySetting < ApplicationRecord
self.primary_key = :project_id
# Note: Even if we store settings for all types of security scanning
# Currently, Auto-fix feature is available only for container_scanning and
# dependency_scanning features.
AVAILABLE_AUTO_FIX_TYPES = [:dependency_scanning, :container_scanning].freeze
belongs_to :project, inverse_of: :security_setting
def self.safe_find_or_create_for(project)
project.security_setting || project.create_security_setting
rescue ActiveRecord::RecordNotUnique
retry
def auto_fix_enabled?
return false if Feature.disabled?(:security_auto_fix, project)
return false unless project.feature_available?(:vulnerability_auto_fix)
auto_fix_enabled_types.any?
end
def auto_fix_enabled_types
AVAILABLE_AUTO_FIX_TYPES.filter_map do |type|
type if public_send("auto_fix_#{type}") # rubocop:disable GitlabSecurity/PublicSend
end
end
end
......@@ -6,6 +6,9 @@ module EE
extend ::Gitlab::Utils::Override
prepended do
with_scope :subject
condition(:auto_fix_enabled) { @subject.security_setting&.auto_fix_enabled? }
with_scope :subject
condition(:repository_mirrors_enabled) { @subject.feature_available?(:repository_mirrors) }
......@@ -226,6 +229,12 @@ module EE
enable :admin_vulnerability_external_issue_link
end
rule { security_bot & auto_fix_enabled }.policy do
enable :push_code
enable :create_merge_request_from
enable :create_vulnerability_feedback
end
rule { issues_disabled & merge_requests_disabled }.policy do
prevent(*create_read_update_admin_destroy(:iteration))
end
......@@ -375,6 +384,7 @@ module EE
def lookup_access_level!
return ::Gitlab::Access::NO_ACCESS if needs_new_sso_session?
return ::Gitlab::Access::NO_ACCESS if visual_review_bot?
return ::Gitlab::Access::REPORTER if security_bot? && auto_fix_enabled?
super
end
......
......@@ -7,6 +7,7 @@ module Vulnerabilities
condition(:issue) { @subject.for_issue? }
condition(:merge_request) { @subject.for_merge_request? }
condition(:dismissal) { @subject.for_dismissal? }
condition(:auto_fix_enabled) { @subject.project.security_setting&.auto_fix_enabled? }
rule { issue & ~can?(:create_issue) }.prevent :create_vulnerability_feedback
......@@ -14,6 +15,14 @@ module Vulnerabilities
merge_request & ~can?(:create_merge_request_in)
end.prevent :create_vulnerability_feedback
# Prevent Security bot from creating vulnerability feedback
# if auto-fix feature is disabled
rule do
merge_request &
security_bot &
~auto_fix_enabled
end.prevent :create_vulnerability_feedback
rule { ~dismissal }.prevent :destroy_vulnerability_feedback, :update_vulnerability_feedback
end
end
......@@ -82,8 +82,8 @@ module Projects
def autofix_enabled
{
dependency_scanning: project_settings.auto_fix_dependency_scanning,
container_scanning: project_settings.auto_fix_container_scanning
dependency_scanning: project_settings&.auto_fix_dependency_scanning,
container_scanning: project_settings&.auto_fix_container_scanning
}
end
......@@ -177,7 +177,7 @@ module Projects
end
def project_settings
ProjectSecuritySetting.safe_find_or_create_for(project)
project.security_setting
end
def configuration_path(type)
......
# frozen_string_literal: true
module EE
module PipelineSerializer
extend ActiveSupport::Concern
private
def preloaded_relations
relations = super
project_relation = relations.detect { |item| item.is_a?(Hash) }
project_relation[:project].push(:security_setting)
relations
end
end
end
......@@ -15,17 +15,13 @@ module Security
def execute(enabled:)
return unless valid?
project_settings.update(toggle_params(enabled))
project&.security_setting&.update(toggle_params(enabled))
end
private
attr_reader :enabled, :feature, :project
def project_settings
@project_settings ||= ProjectSecuritySetting.safe_find_or_create_for(project)
end
def toggle_params(enabled)
if feature == 'all'
{
......
......@@ -107,7 +107,6 @@ RSpec.describe Projects::Security::ConfigurationController do
context 'with sufficient permissions' do
let(:user) { maintainer }
let(:setting) { project.security_setting }
it 'shows auto fix disable for dependency scanning for json format' do
get :show, params: { namespace_id: project.namespace, project_id: project, format: :json }
......@@ -121,7 +120,7 @@ RSpec.describe Projects::Security::ConfigurationController do
it 'processes request and updates setting' do
expect(response).to have_gitlab_http_status(:ok)
expect(setting.auto_fix_dependency_scanning).to be_falsey
expect(project.security_setting.reload.auto_fix_dependency_scanning).to be_falsey
expect(response[:dependency_scanning]).to be_falsey
end
end
......@@ -130,6 +129,8 @@ RSpec.describe Projects::Security::ConfigurationController do
let(:feature) { '' }
it 'processes request and updates setting' do
setting = project.reload.security_setting
expect(response).to have_gitlab_http_status(:ok)
expect(setting.auto_fix_dependency_scanning).to be_falsey
expect(setting.auto_fix_dast).to be_falsey
......@@ -139,11 +140,10 @@ RSpec.describe Projects::Security::ConfigurationController do
context 'without processable feature' do
let(:feature) { :dep_scan }
let(:setting) { project.create_security_setting }
it 'does not pass validation' do
expect(response).to have_gitlab_http_status(:unprocessable_entity)
expect(setting.auto_fix_dependency_scanning).to be_truthy
expect(project.security_setting.auto_fix_dependency_scanning).to be_truthy
end
end
end
......
......@@ -7,5 +7,12 @@ FactoryBot.define do
auto_fix_dast { true }
auto_fix_dependency_scanning { true }
auto_fix_sast { true }
trait :disabled_auto_fix do
auto_fix_container_scanning { false }
auto_fix_dast { false }
auto_fix_dependency_scanning { false }
auto_fix_sast { false }
end
end
end
......@@ -102,12 +102,10 @@ RSpec.describe Gitlab::ImportExport::Project::TreeRestorer do
before do
setup_import_export_config('complex', 'ee')
restored_project_json
end
it 'creates security setting' do
project = Project.find_by_path('project')
expect { restored_project_json }.to change { ProjectSecuritySetting.count }.from(0).to(1)
expect(project.security_setting.auto_fix_dependency_scanning).to be_falsey
expect(project.security_setting.auto_fix_container_scanning).to be_truthy
end
......
......@@ -15,7 +15,6 @@ RSpec.describe Gitlab::ImportExport::Project::TreeSaver do
let_it_be(:export_path) { "#{Dir.tmpdir}/project_tree_saver_spec_ee" }
let_it_be(:security_setting) { create(:project_security_setting, project: project, auto_fix_dast: false) }
let_it_be(:push_rule) { create(:push_rule, project: project, max_file_size: 10) }
after :all do
......@@ -75,7 +74,6 @@ RSpec.describe Gitlab::ImportExport::Project::TreeSaver do
end
it 'has security settings' do
expect(security_json['auto_fix_dast']).to be_falsey
expect(security_json['auto_fix_dependency_scanning']).to be_truthy
end
end
......
......@@ -4,32 +4,79 @@ require 'spec_helper'
RSpec.describe ProjectSecuritySetting do
describe 'associations' do
subject { create(:project_security_setting) }
subject { create(:project).security_setting }
it { is_expected.to belong_to(:project) }
end
describe '.safe_find_or_create_for' do
subject { described_class.safe_find_or_create_for(project) }
describe '#auto_fix_enabled?' do
subject { setting.auto_fix_enabled? }
let_it_be(:project) { create :project }
let_it_be(:setting) { build(:project_security_setting) }
context 'without existing setting' do
it 'creates a new entry' do
expect { subject }.to change { ProjectSecuritySetting.count }.by(1)
expect(subject).to be_a_kind_of(ProjectSecuritySetting)
context 'when licensed feature is enabled' do
before do
stub_licensed_features(vulnerability_auto_fix: true)
end
context 'when auto fix is enabled for available feature' do
before do
setting.auto_fix_container_scanning = false
setting.auto_fix_dependency_scanning = true
end
it 'marks auto_fix as enabled' do
is_expected.to be_truthy
end
end
context 'when a auto_fix setting is disabled for available features' do
before do
setting.auto_fix_container_scanning = false
setting.auto_fix_dependency_scanning = false
setting.auto_fix_sast = false
end
it 'marks auto_fix as disabled' do
is_expected.to be_falsey
end
end
context 'when feature is disabled' do
before do
stub_feature_flags(security_auto_fix: false)
end
it 'marks auto_fix as disabled' do
is_expected.to be_falsey
end
end
end
context 'with existing setting' do
context 'when license feature is disabled' do
before do
project.create_security_setting
stub_licensed_features(vulnerability_auto_fix: false)
end
it 'reuses existing entry' do
expect { subject }.not_to change { ProjectSecuritySetting.count }
expect(subject).to be_a_kind_of(ProjectSecuritySetting)
it 'marks auto_fix as disabled' do
is_expected.to be_falsey
end
end
end
describe '#auto_fix_enabled_types' do
subject { setting.auto_fix_enabled_types }
let_it_be(:setting) { build(:project_security_setting) }
before do
setting.auto_fix_container_scanning = false
setting.auto_fix_dependency_scanning = true
setting.auto_fix_sast = true
end
it 'return status only for available types' do
is_expected.to eq([:dependency_scanning])
end
end
end
......@@ -527,6 +527,69 @@ RSpec.describe ProjectPolicy do
end
end
describe 'permissions for security bot' do
let_it_be(:current_user) { create(:user, :security_bot) }
let(:project) { private_project }
let(:permissions) do
%i(
reporter_access
push_code
create_merge_request_from
create_merge_request_in
create_vulnerability_feedback
read_project
)
end
context 'when auto_fix feature is enabled' do
context 'when licensed feature is enabled' do
before do
stub_licensed_features(vulnerability_auto_fix: true)
end
it { is_expected.to be_allowed(*permissions) }
context 'when feature flag is disabled' do
before do
stub_feature_flags(security_auto_fix: false)
end
it { is_expected.to be_disallowed(*permissions) }
end
end
context 'when licensed feature is disabled' do
before do
stub_licensed_features(vulnerability_auto_fix: false)
end
it { is_expected.to be_disallowed(*permissions) }
end
end
context 'when auto_fix feature is disabled' do
before do
stub_licensed_features(vulnerability_auto_fix: true)
project.security_setting.update!(auto_fix_dependency_scanning: false, auto_fix_container_scanning: false)
end
it { is_expected.to be_disallowed(*permissions) }
end
context 'when project does not have a security_setting' do
before do
stub_licensed_features(vulnerability_auto_fix: true)
project.security_setting.delete
project.reload
end
it do
is_expected.to be_disallowed(*permissions)
end
end
end
describe 'read_threat_monitoring' do
context 'when threat monitoring feature is available' do
before do
......
......@@ -71,6 +71,27 @@ RSpec.describe Vulnerabilities::FeedbackPolicy do
is_expected.to be_disallowed(:create_vulnerability_feedback)
end
end
context 'with security bot' do
let(:current_user) { create(:user, :security_bot) }
before do
stub_licensed_features(vulnerability_auto_fix: true)
end
context 'when auto-fix is enabled' do
it { is_expected.to be_allowed(:create_vulnerability_feedback) }
end
context 'when auto-fix is disabled' do
before do
project.security_setting.update!(auto_fix_dependency_scanning: false,
auto_fix_container_scanning: false)
end
it { is_expected.to be_disallowed(:create_vulnerability_feedback) }
end
end
end
end
......
......@@ -4,7 +4,7 @@ require 'spec_helper'
RSpec.describe Security::Configuration::SaveAutoFixService do
describe '#execute' do
let_it_be(:project) { create(:project) }
let_it_be_with_reload(:project) { create(:project) }
subject(:service) { described_class.new(project, feature) }
......@@ -32,10 +32,6 @@ RSpec.describe Security::Configuration::SaveAutoFixService do
context 'with not supported scanner type' do
let(:feature) { :dep_scan }
before do
project.create_security_setting
end
it 'does not change setting' do
expect(project.security_setting.auto_fix_dependency_scanning).to be_truthy
end
......
......@@ -47,6 +47,10 @@ FactoryBot.define do
user_type { :migration_bot }
end
trait :security_bot do
user_type { :security_bot }
end
trait :external do
external { true }
end
......
......@@ -7,6 +7,8 @@ RSpec.describe GlobalPolicy do
let_it_be(:project_bot) { create(:user, :project_bot) }
let_it_be(:migration_bot) { create(:user, :migration_bot) }
let_it_be(:security_bot) { create(:user, :security_bot) }
let(:current_user) { create(:user) }
let(:user) { create(:user) }
......@@ -223,6 +225,12 @@ RSpec.describe GlobalPolicy do
it { is_expected.not_to be_allowed(:access_api) }
end
context 'security bot' do
let(:current_user) { security_bot }
it { is_expected.not_to be_allowed(:access_api) }
end
context 'user blocked pending approval' do
before do
current_user.block_pending_approval
......@@ -353,6 +361,12 @@ RSpec.describe GlobalPolicy do
it { is_expected.to be_allowed(:access_git) }
end
context 'security bot' do
let(:current_user) { security_bot }
it { is_expected.to be_allowed(:access_git) }
end
describe 'deactivated user' do
before do
current_user.deactivate
......@@ -513,6 +527,12 @@ RSpec.describe GlobalPolicy do
it { is_expected.not_to be_allowed(:log_in) }
end
context 'security bot' do
let(:current_user) { security_bot }
it { is_expected.not_to be_allowed(:log_in) }
end
context 'user blocked pending approval' do
before do
current_user.block_pending_approval
......
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