Commit 9367d4ef authored by Stan Hu's avatar Stan Hu

Revert "Merge branch '215669-security-bot-permissions' into 'master'"

This reverts merge request !48676
parent eb411321
...@@ -727,7 +727,6 @@ class User < ApplicationRecord ...@@ -727,7 +727,6 @@ class User < ApplicationRecord
u.name = 'GitLab Security Bot' u.name = 'GitLab Security Bot'
u.website_url = Gitlab::Routing.url_helpers.help_page_url('user/application_security/security_bot/index.md') 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.avatar = bot_avatar(image: 'security-bot.png')
u.confirmed_at = Time.zone.now
end end
end end
......
...@@ -25,10 +25,6 @@ class BasePolicy < DeclarativePolicy::Base ...@@ -25,10 +25,6 @@ class BasePolicy < DeclarativePolicy::Base
with_options scope: :user, score: 0 with_options scope: :user, score: 0
condition(:support_bot) { @user&.support_bot? } 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" desc "User email is unconfirmed or user account is locked"
with_options scope: :user, score: 0 with_options scope: :user, score: 0
condition(:inactive) { @user&.confirmation_required_on_sign_in? || @user&.access_locked? } condition(:inactive) { @user&.confirmation_required_on_sign_in? || @user&.access_locked? }
......
...@@ -48,7 +48,7 @@ class GlobalPolicy < BasePolicy ...@@ -48,7 +48,7 @@ class GlobalPolicy < BasePolicy
prevent :use_slash_commands prevent :use_slash_commands
end end
rule { blocked | (internal & ~migration_bot & ~security_bot) }.policy do rule { blocked | (internal & ~migration_bot) }.policy do
prevent :access_git prevent :access_git
end end
......
...@@ -75,5 +75,3 @@ class PipelineSerializer < BaseSerializer ...@@ -75,5 +75,3 @@ class PipelineSerializer < BaseSerializer
] ]
end end
end end
PipelineSerializer.prepend_if_ee('EE::PipelineSerializer')
...@@ -3,11 +3,6 @@ ...@@ -3,11 +3,6 @@
class ProjectSecuritySetting < ApplicationRecord class ProjectSecuritySetting < ApplicationRecord
self.primary_key = :project_id 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 belongs_to :project, inverse_of: :security_setting
def self.safe_find_or_create_for(project) def self.safe_find_or_create_for(project)
...@@ -15,14 +10,4 @@ class ProjectSecuritySetting < ApplicationRecord ...@@ -15,14 +10,4 @@ class ProjectSecuritySetting < ApplicationRecord
rescue ActiveRecord::RecordNotUnique rescue ActiveRecord::RecordNotUnique
retry retry
end end
def auto_fix_enabled?
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 end
...@@ -6,9 +6,6 @@ module EE ...@@ -6,9 +6,6 @@ module EE
extend ::Gitlab::Utils::Override extend ::Gitlab::Utils::Override
prepended do prepended do
with_scope :subject
condition(:auto_fix_enabled) { @subject.security_setting&.auto_fix_enabled? }
with_scope :subject with_scope :subject
condition(:repository_mirrors_enabled) { @subject.feature_available?(:repository_mirrors) } condition(:repository_mirrors_enabled) { @subject.feature_available?(:repository_mirrors) }
...@@ -228,12 +225,6 @@ module EE ...@@ -228,12 +225,6 @@ module EE
enable :admin_vulnerability_issue_link enable :admin_vulnerability_issue_link
end 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 rule { issues_disabled & merge_requests_disabled }.policy do
prevent(*create_read_update_admin_destroy(:iteration)) prevent(*create_read_update_admin_destroy(:iteration))
end end
...@@ -382,7 +373,6 @@ module EE ...@@ -382,7 +373,6 @@ module EE
def lookup_access_level! def lookup_access_level!
return ::Gitlab::Access::NO_ACCESS if needs_new_sso_session? return ::Gitlab::Access::NO_ACCESS if needs_new_sso_session?
return ::Gitlab::Access::NO_ACCESS if visual_review_bot? return ::Gitlab::Access::NO_ACCESS if visual_review_bot?
return ::Gitlab::Access::REPORTER if security_bot? && auto_fix_enabled?
super super
end end
......
...@@ -7,7 +7,6 @@ module Vulnerabilities ...@@ -7,7 +7,6 @@ module Vulnerabilities
condition(:issue) { @subject.for_issue? } condition(:issue) { @subject.for_issue? }
condition(:merge_request) { @subject.for_merge_request? } condition(:merge_request) { @subject.for_merge_request? }
condition(:dismissal) { @subject.for_dismissal? } 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 rule { issue & ~can?(:create_issue) }.prevent :create_vulnerability_feedback
...@@ -15,14 +14,6 @@ module Vulnerabilities ...@@ -15,14 +14,6 @@ module Vulnerabilities
merge_request & ~can?(:create_merge_request_in) merge_request & ~can?(:create_merge_request_in)
end.prevent :create_vulnerability_feedback 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 rule { ~dismissal }.prevent :destroy_vulnerability_feedback, :update_vulnerability_feedback
end end
end end
# 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
...@@ -32,49 +32,4 @@ RSpec.describe ProjectSecuritySetting do ...@@ -32,49 +32,4 @@ RSpec.describe ProjectSecuritySetting do
end end
end end
end end
describe '#auto_fix_enabled?' do
subject { setting.auto_fix_enabled? }
let_it_be(:setting) { build(:project_security_setting) }
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
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 end
...@@ -527,34 +527,6 @@ RSpec.describe ProjectPolicy do ...@@ -527,34 +527,6 @@ RSpec.describe ProjectPolicy do
end end
end end
describe 'permissions for security bot' do
let_it_be(:current_user) { create(:user, :security_bot) }
let(:project) { private_project }
context 'when auto_fix feature is enabled' do
before do
build(:project_security_setting, project: project)
end
it { is_expected.to be_allowed(:reporter_access) }
it { is_expected.to be_allowed(:push_code) }
it { is_expected.to be_allowed(:create_merge_request_from) }
it { is_expected.to be_allowed(:create_vulnerability_feedback) }
it { is_expected.to be_allowed(:create_merge_request_in) }
it { is_expected.to be_allowed(:read_project) }
end
context 'when auto_fix feature is disabled' do
it { is_expected.to be_disallowed(:reporter_access) }
it { is_expected.to be_disallowed(:push_code) }
it { is_expected.to be_disallowed(:create_merge_request_from) }
it { is_expected.to be_disallowed(:create_vulnerability_feedback) }
it { is_expected.to be_disallowed(:create_merge_request_in) }
it { is_expected.to be_disallowed(:read_project) }
end
end
describe 'read_threat_monitoring' do describe 'read_threat_monitoring' do
context 'when threat monitoring feature is available' do context 'when threat monitoring feature is available' do
before do before do
......
...@@ -71,20 +71,6 @@ RSpec.describe Vulnerabilities::FeedbackPolicy do ...@@ -71,20 +71,6 @@ RSpec.describe Vulnerabilities::FeedbackPolicy do
is_expected.to be_disallowed(:create_vulnerability_feedback) is_expected.to be_disallowed(:create_vulnerability_feedback)
end end
end end
context 'with security bot' do
let(:current_user) { create(:user, :security_bot) }
context 'when auto-fix is enabled' do
let!(:setting) { build(:project_security_setting, project: project) }
it { is_expected.to be_allowed(:create_vulnerability_feedback) }
end
context 'when auto-fix is disabled' do
it { is_expected.to be_disallowed(:create_vulnerability_feedback) }
end
end
end end
end end
......
...@@ -47,10 +47,6 @@ FactoryBot.define do ...@@ -47,10 +47,6 @@ FactoryBot.define do
user_type { :migration_bot } user_type { :migration_bot }
end end
trait :security_bot do
user_type { :security_bot }
end
trait :external do trait :external do
external { true } external { true }
end end
......
...@@ -7,8 +7,6 @@ RSpec.describe GlobalPolicy do ...@@ -7,8 +7,6 @@ RSpec.describe GlobalPolicy do
let_it_be(:project_bot) { create(:user, :project_bot) } let_it_be(:project_bot) { create(:user, :project_bot) }
let_it_be(:migration_bot) { create(:user, :migration_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(:current_user) { create(:user) }
let(:user) { create(:user) } let(:user) { create(:user) }
...@@ -225,12 +223,6 @@ RSpec.describe GlobalPolicy do ...@@ -225,12 +223,6 @@ RSpec.describe GlobalPolicy do
it { is_expected.not_to be_allowed(:access_api) } it { is_expected.not_to be_allowed(:access_api) }
end 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 context 'user blocked pending approval' do
before do before do
current_user.block_pending_approval current_user.block_pending_approval
...@@ -361,12 +353,6 @@ RSpec.describe GlobalPolicy do ...@@ -361,12 +353,6 @@ RSpec.describe GlobalPolicy do
it { is_expected.to be_allowed(:access_git) } it { is_expected.to be_allowed(:access_git) }
end end
context 'security bot' do
let(:current_user) { security_bot }
it { is_expected.to be_allowed(:access_git) }
end
describe 'deactivated user' do describe 'deactivated user' do
before do before do
current_user.deactivate current_user.deactivate
...@@ -527,12 +513,6 @@ RSpec.describe GlobalPolicy do ...@@ -527,12 +513,6 @@ RSpec.describe GlobalPolicy do
it { is_expected.not_to be_allowed(:log_in) } it { is_expected.not_to be_allowed(:log_in) }
end 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 context 'user blocked pending approval' do
before do before do
current_user.block_pending_approval 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