Commit 00ffd379 authored by Vitali Tatarintev's avatar Vitali Tatarintev

Move User.alert_bot out of EE

User.alert_bot is one of the dependencies for
moving Alerting functionality to the core
parent d9613bc7
...@@ -59,6 +59,12 @@ class User < ApplicationRecord ...@@ -59,6 +59,12 @@ class User < ApplicationRecord
MINIMUM_INACTIVE_DAYS = 180 MINIMUM_INACTIVE_DAYS = 180
enum bot_type: {
support_bot: 1,
alert_bot: 2,
visual_review_bot: 3
}
# Override Devise::Models::Trackable#update_tracked_fields! # Override Devise::Models::Trackable#update_tracked_fields!
# to limit database writes to at most once every hour # to limit database writes to at most once every hour
# rubocop: disable CodeReuse/ServiceClass # rubocop: disable CodeReuse/ServiceClass
...@@ -322,6 +328,8 @@ class User < ApplicationRecord ...@@ -322,6 +328,8 @@ class User < ApplicationRecord
scope :with_emails, -> { preload(:emails) } scope :with_emails, -> { preload(:emails) }
scope :with_dashboard, -> (dashboard) { where(dashboard: dashboard) } scope :with_dashboard, -> (dashboard) { where(dashboard: dashboard) }
scope :with_public_profile, -> { where(private_profile: false) } scope :with_public_profile, -> { where(private_profile: false) }
scope :bots, -> { where.not(bot_type: nil) }
scope :humans, -> { where(bot_type: nil) }
scope :with_expiring_and_not_notified_personal_access_tokens, ->(at) do scope :with_expiring_and_not_notified_personal_access_tokens, ->(at) do
where('EXISTS (?)', where('EXISTS (?)',
...@@ -598,6 +606,33 @@ class User < ApplicationRecord ...@@ -598,6 +606,33 @@ class User < ApplicationRecord
end end
end end
def alert_bot
email_pattern = "alert%s@#{Settings.gitlab.host}"
unique_internal(where(bot_type: :alert_bot), 'alert-bot', email_pattern) do |u|
u.bio = 'The GitLab alert bot'
u.name = 'GitLab Alert Bot'
end
end
def support_bot
email_pattern = "support%s@#{Settings.gitlab.host}"
unique_internal(where(bot_type: :support_bot), 'support-bot', email_pattern) do |u|
u.bio = 'The GitLab support bot used for Service Desk'
u.name = 'GitLab Support Bot'
end
end
def visual_review_bot
email_pattern = "visual_review%s@#{Settings.gitlab.host}"
unique_internal(where(bot_type: :visual_review_bot), 'visual-review-bot', email_pattern) do |u|
u.bio = 'The Gitlab Visual Review feedback bot'
u.name = 'Gitlab Visual Review Bot'
end
end
# Return true if there is only single non-internal user in the deployment, # Return true if there is only single non-internal user in the deployment,
# ghost user is ignored. # ghost user is ignored.
def single_user? def single_user?
...@@ -613,16 +648,23 @@ class User < ApplicationRecord ...@@ -613,16 +648,23 @@ class User < ApplicationRecord
username username
end end
def bot?
return bot_type.present? if has_attribute?(:bot_type)
# Some older *migration* specs utilize this removed column
read_attribute(:support_bot)
end
def internal? def internal?
ghost? ghost? || bot?
end end
def self.internal def self.internal
where(ghost: true) where(ghost: true).or(bots)
end end
def self.non_internal def self.non_internal
without_ghosts without_ghosts.humans
end end
# #
......
...@@ -44,6 +44,15 @@ class BasePolicy < DeclarativePolicy::Base ...@@ -44,6 +44,15 @@ class BasePolicy < DeclarativePolicy::Base
::Gitlab::ExternalAuthorization.perform_check? ::Gitlab::ExternalAuthorization.perform_check?
end end
with_options scope: :user, score: 0
condition(:support_bot) { @user&.support_bot? }
with_options scope: :user, score: 0
condition(:alert_bot) { @user&.alert_bot? }
with_options scope: :user, score: 0
condition(:visual_review_bot) { @user&.visual_review_bot? }
rule { external_authorization_enabled & ~can?(:read_all_resources) }.policy do rule { external_authorization_enabled & ~can?(:read_all_resources) }.policy do
prevent :read_cross_project prevent :read_cross_project
end end
......
...@@ -33,6 +33,18 @@ module PolicyActor ...@@ -33,6 +33,18 @@ module PolicyActor
def can_create_group def can_create_group
false false
end end
def support_bot?
false
end
def alert_bot?
false
end
def visual_review_bot?
false
end
end end
PolicyActor.prepend_if_ee('EE::PolicyActor') PolicyActor.prepend_if_ee('EE::PolicyActor')
...@@ -515,6 +515,8 @@ class ProjectPolicy < BasePolicy ...@@ -515,6 +515,8 @@ class ProjectPolicy < BasePolicy
end end
def lookup_access_level! def lookup_access_level!
return ::Gitlab::Access::REPORTER if alert_bot?
# NOTE: max_member_access has its own cache # NOTE: max_member_access has its own cache
project.team.max_member_access(@user.id) project.team.max_member_access(@user.id)
end end
......
...@@ -70,9 +70,6 @@ module EE ...@@ -70,9 +70,6 @@ module EE
joins(:identities).where(identities: { provider: provider }) joins(:identities).where(identities: { provider: provider })
end end
scope :bots, -> { where.not(bot_type: nil) }
scope :humans, -> { where(bot_type: nil) }
scope :with_invalid_expires_at_tokens, ->(expiration_date) do scope :with_invalid_expires_at_tokens, ->(expiration_date) do
where(id: ::PersonalAccessToken.with_invalid_expires_at(expiration_date).select(:user_id)) where(id: ::PersonalAccessToken.with_invalid_expires_at(expiration_date).select(:user_id))
end end
...@@ -85,54 +82,11 @@ module EE ...@@ -85,54 +82,11 @@ module EE
# Note: When adding an option, it's value MUST equal to the last value + 1. # Note: When adding an option, it's value MUST equal to the last value + 1.
enum group_view: { details: 1, security_dashboard: 2 }, _prefix: true enum group_view: { details: 1, security_dashboard: 2 }, _prefix: true
scope :group_view_details, -> { where('group_view = ? OR group_view IS NULL', group_view[:details]) } scope :group_view_details, -> { where('group_view = ? OR group_view IS NULL', group_view[:details]) }
enum bot_type: {
support_bot: 1,
alert_bot: 2,
visual_review_bot: 3
}
end end
class_methods do class_methods do
extend ::Gitlab::Utils::Override extend ::Gitlab::Utils::Override
def support_bot
email_pattern = "support%s@#{Settings.gitlab.host}"
unique_internal(where(bot_type: :support_bot), 'support-bot', email_pattern) do |u|
u.bio = 'The GitLab support bot used for Service Desk'
u.name = 'GitLab Support Bot'
end
end
def alert_bot
email_pattern = "alert%s@#{Settings.gitlab.host}"
unique_internal(where(bot_type: :alert_bot), 'alert-bot', email_pattern) do |u|
u.bio = 'The GitLab alert bot'
u.name = 'GitLab Alert Bot'
end
end
def visual_review_bot
email_pattern = "visual_review%s@#{Settings.gitlab.host}"
unique_internal(where(bot_type: :visual_review_bot), 'visual-review-bot', email_pattern) do |u|
u.bio = 'The Gitlab Visual Review feedback bot'
u.name = 'Gitlab Visual Review Bot'
end
end
override :internal
def internal
super.or(bots)
end
override :non_internal
def non_internal
super.humans
end
def non_ldap def non_ldap
joins('LEFT JOIN identities ON identities.user_id = users.id') joins('LEFT JOIN identities ON identities.user_id = users.id')
.where('identities.provider IS NULL OR identities.provider NOT LIKE ?', 'ldap%') .where('identities.provider IS NULL OR identities.provider NOT LIKE ?', 'ldap%')
...@@ -334,18 +288,6 @@ module EE ...@@ -334,18 +288,6 @@ module EE
super super
end end
override :internal?
def internal?
super || bot?
end
def bot?
return bot_type.present? if has_attribute?(:bot_type)
# Some older *migration* specs utilize this removed column
read_attribute(:support_bot)
end
protected protected
override :password_required? override :password_required?
......
...@@ -8,15 +8,6 @@ module EE ...@@ -8,15 +8,6 @@ module EE
with_scope :user with_scope :user
condition(:auditor, score: 0) { @user&.auditor? } condition(:auditor, score: 0) { @user&.auditor? }
with_scope :user
condition(:support_bot, score: 0) { @user&.support_bot? }
with_scope :user
condition(:alert_bot, score: 0) { @user&.alert_bot? }
with_scope :user
condition(:visual_review_bot, score: 0) { @user&.visual_review_bot? }
with_scope :global with_scope :global
condition(:license_block) { License.block_changes? } condition(:license_block) { License.block_changes? }
......
...@@ -5,17 +5,5 @@ module EE ...@@ -5,17 +5,5 @@ module EE
def auditor? def auditor?
false false
end end
def support_bot?
false
end
def alert_bot?
false
end
def visual_review_bot?
false
end
end end
end end
...@@ -316,7 +316,6 @@ module EE ...@@ -316,7 +316,6 @@ module EE
override :lookup_access_level! override :lookup_access_level!
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::REPORTER if alert_bot?
return ::Gitlab::Access::REPORTER if support_bot? && service_desk_enabled? return ::Gitlab::Access::REPORTER if support_bot? && service_desk_enabled?
return ::Gitlab::Access::NO_ACCESS if visual_review_bot? return ::Gitlab::Access::NO_ACCESS if visual_review_bot?
......
...@@ -9,10 +9,6 @@ FactoryBot.modify do ...@@ -9,10 +9,6 @@ FactoryBot.modify do
trait :group_managed do trait :group_managed do
association :managing_group, factory: :group association :managing_group, factory: :group
end end
trait :bot do
bot_type { User.bot_types[:support_bot] }
end
end end
factory :omniauth_user do factory :omniauth_user do
......
...@@ -74,16 +74,6 @@ describe User do ...@@ -74,16 +74,6 @@ describe User do
end end
end end
describe 'bots & humans' do
it 'returns corresponding users' do
human = create(:user)
bot = create(:user, :bot)
expect(described_class.humans).to match_array([human])
expect(described_class.bots).to match_array([bot])
end
end
describe 'with_invalid_expires_at_tokens' do describe 'with_invalid_expires_at_tokens' do
it 'only includes users with invalid tokens' do it 'only includes users with invalid tokens' do
valid_pat = create(:personal_access_token, expires_at: 7.days.from_now) valid_pat = create(:personal_access_token, expires_at: 7.days.from_now)
...@@ -593,37 +583,6 @@ describe User do ...@@ -593,37 +583,6 @@ describe User do
end end
end end
describe 'internal methods' do
let!(:user) { create(:user) }
let!(:ghost) { described_class.ghost }
let!(:support_bot) { described_class.support_bot }
let!(:alert_bot) { described_class.alert_bot }
let!(:visual_review_bot) { described_class.visual_review_bot }
let!(:non_internal) { [user] }
let!(:internal) { [ghost, support_bot, alert_bot, visual_review_bot] }
it 'returns non internal users' do
expect(described_class.internal).to eq(internal)
expect(internal.all?(&:internal?)).to eq(true)
end
it 'returns internal users' do
expect(described_class.non_internal).to eq(non_internal)
expect(non_internal.all?(&:internal?)).to eq(false)
end
describe '#bot?' do
it 'marks bot users' do
expect(user.bot?).to eq(false)
expect(ghost.bot?).to eq(false)
expect(support_bot.bot?).to eq(true)
expect(alert_bot.bot?).to eq(true)
expect(visual_review_bot.bot?).to eq(true)
end
end
end
describe '#using_license_seat?' do describe '#using_license_seat?' do
let(:user) { create(:user) } let(:user) { create(:user) }
......
...@@ -1021,18 +1021,6 @@ describe ProjectPolicy do ...@@ -1021,18 +1021,6 @@ describe ProjectPolicy do
end end
end end
context 'alert bot' do
let(:current_user) { User.alert_bot }
it { is_expected.to be_allowed(:reporter_access) }
context 'within a private project' do
let(:project) { create(:project, :private) }
it { is_expected.to be_allowed(:admin_issue) }
end
end
context 'support bot' do context 'support bot' do
let(:current_user) { User.support_bot } let(:current_user) { User.support_bot }
......
...@@ -23,6 +23,10 @@ FactoryBot.define do ...@@ -23,6 +23,10 @@ FactoryBot.define do
after(:build) { |user, _| user.block! } after(:build) { |user, _| user.block! }
end end
trait :bot do
bot_type { User.bot_types[:support_bot] }
end
trait :external do trait :external do
external { true } external { true }
end end
......
...@@ -4126,4 +4126,45 @@ describe User, :do_not_mock_admin_mode do ...@@ -4126,4 +4126,45 @@ describe User, :do_not_mock_admin_mode do
end end
end end
end end
describe 'internal methods' do
let!(:user) { create(:user) }
let!(:ghost) { described_class.ghost }
let!(:support_bot) { described_class.support_bot }
let!(:alert_bot) { described_class.alert_bot }
let!(:visual_review_bot) { described_class.visual_review_bot }
let!(:non_internal) { [user] }
let!(:internal) { [ghost, support_bot, alert_bot, visual_review_bot] }
it 'returns non internal users' do
expect(described_class.internal).to eq(internal)
expect(internal.all?(&:internal?)).to eq(true)
end
it 'returns internal users' do
expect(described_class.non_internal).to eq(non_internal)
expect(non_internal.all?(&:internal?)).to eq(false)
end
describe '#bot?' do
it 'marks bot users' do
expect(user.bot?).to eq(false)
expect(ghost.bot?).to eq(false)
expect(support_bot.bot?).to eq(true)
expect(alert_bot.bot?).to eq(true)
expect(visual_review_bot.bot?).to eq(true)
end
end
end
describe 'bots & humans' do
it 'returns corresponding users' do
human = create(:user)
bot = create(:user, :bot)
expect(described_class.humans).to match_array([human])
expect(described_class.bots).to match_array([bot])
end
end
end end
...@@ -559,4 +559,18 @@ describe ProjectPolicy do ...@@ -559,4 +559,18 @@ describe ProjectPolicy do
end end
end end
end end
context 'alert bot' do
let(:current_user) { User.alert_bot }
subject { described_class.new(current_user, project) }
it { is_expected.to be_allowed(:reporter_access) }
context 'within a private project' do
let(:project) { create(:project, :private) }
it { is_expected.to be_allowed(:admin_issue) }
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