summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2021-02-10 23:12:47 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2021-02-10 23:12:47 +0000
commit29339ca07c9e7c3a5eb5f4219c4c3863c8d7fbdd (patch)
tree17a2438c033361190b6fcdc72f954b19044cec6a
parent3b1cfe1993499d9bd68d5ecc29d8b49af3b05592 (diff)
downloadgitlab-ce-29339ca07c9e7c3a5eb5f4219c4c3863c8d7fbdd.tar.gz
Add latest changes from gitlab-org/security/gitlab@13-6-stable-ee
-rw-r--r--app/models/member.rb19
-rw-r--r--app/services/members/create_service.rb13
-rw-r--r--changelogs/unreleased/security-limit-invitations.yml5
-rw-r--r--db/migrate/20201007033527_add_daily_invites_to_plan_limits.rb9
-rw-r--r--db/migrate/20201007033723_insert_daily_invites_plan_limits.rb25
-rw-r--r--db/schema_migrations/202010070335271
-rw-r--r--db/schema_migrations/202010070337231
-rw-r--r--db/structure.sql3
-rw-r--r--doc/administration/instance_limits.md7
-rw-r--r--locale/gitlab.pot3
-rw-r--r--spec/factories/project_members.rb2
-rw-r--r--spec/migrations/insert_daily_invites_plan_limits_spec.rb55
-rw-r--r--spec/models/member_spec.rb52
-rw-r--r--spec/models/plan_limits_spec.rb1
-rw-r--r--[-rwxr-xr-x]vendor/gitignore/C++.gitignore0
-rw-r--r--[-rwxr-xr-x]vendor/gitignore/Java.gitignore0
16 files changed, 191 insertions, 5 deletions
diff --git a/app/models/member.rb b/app/models/member.rb
index 687830f5267..3000c13f4b1 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -45,6 +45,19 @@ class Member < ApplicationRecord
},
if: :project_bot?
+ scope :in_hierarchy, ->(source) do
+ groups = source.root_ancestor.self_and_descendants
+ group_members = Member.default_scoped.where(source: groups)
+
+ projects = source.root_ancestor.all_projects
+ project_members = Member.default_scoped.where(source: projects)
+
+ Member.default_scoped.from_union([
+ group_members,
+ project_members
+ ]).merge(self)
+ end
+
# This scope encapsulates (most of) the conditions a row in the member table
# must satisfy if it is a valid permission. Of particular note:
#
@@ -77,12 +90,18 @@ class Member < ApplicationRecord
scope :invite, -> { where.not(invite_token: nil) }
scope :non_invite, -> { where(invite_token: nil) }
+
scope :request, -> { where.not(requested_at: nil) }
scope :non_request, -> { where(requested_at: nil) }
scope :not_accepted_invitations, -> { invite.where(invite_accepted_at: nil) }
scope :not_accepted_invitations_by_user, -> (user) { not_accepted_invitations.where(created_by: user) }
scope :not_expired, -> (today = Date.current) { where(arel_table[:expires_at].gt(today).or(arel_table[:expires_at].eq(nil))) }
+
+ scope :created_today, -> do
+ now = Date.current
+ where(created_at: now.beginning_of_day..now.end_of_day)
+ end
scope :last_ten_days_excluding_today, -> (today = Date.current) { where(created_at: (today - 10).beginning_of_day..(today - 1).end_of_day) }
scope :has_access, -> { active.where('access_level > 0') }
diff --git a/app/services/members/create_service.rb b/app/services/members/create_service.rb
index 088e6f031c8..8dbf1ab1c9c 100644
--- a/app/services/members/create_service.rb
+++ b/app/services/members/create_service.rb
@@ -2,12 +2,12 @@
module Members
class CreateService < Members::BaseService
+ include Gitlab::Utils::StrongMemoize
+
DEFAULT_LIMIT = 100
def execute(source)
- return error(s_('AddMember|No users specified.')) if params[:user_ids].blank?
-
- user_ids = params[:user_ids].split(',').uniq.flatten
+ return error(s_('AddMember|No users specified.')) if user_ids.blank?
return error(s_("AddMember|Too many users specified (limit is %{user_limit})") % { user_limit: user_limit }) if
user_limit && user_ids.size > user_limit
@@ -45,6 +45,13 @@ module Members
private
+ def user_ids
+ strong_memoize(:user_ids) do
+ ids = params[:user_ids] || ''
+ ids.split(',').uniq.flatten
+ end
+ end
+
def user_limit
limit = params.fetch(:limit, DEFAULT_LIMIT)
diff --git a/changelogs/unreleased/security-limit-invitations.yml b/changelogs/unreleased/security-limit-invitations.yml
new file mode 100644
index 00000000000..353d1cec727
--- /dev/null
+++ b/changelogs/unreleased/security-limit-invitations.yml
@@ -0,0 +1,5 @@
+---
+title: Limit daily invitations to groups and projects
+merge_request:
+author:
+type: security
diff --git a/db/migrate/20201007033527_add_daily_invites_to_plan_limits.rb b/db/migrate/20201007033527_add_daily_invites_to_plan_limits.rb
new file mode 100644
index 00000000000..8f0079cd639
--- /dev/null
+++ b/db/migrate/20201007033527_add_daily_invites_to_plan_limits.rb
@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class AddDailyInvitesToPlanLimits < ActiveRecord::Migration[6.0]
+ DOWNTIME = false
+
+ def change
+ add_column(:plan_limits, :daily_invites, :integer, default: 0, null: false)
+ end
+end
diff --git a/db/migrate/20201007033723_insert_daily_invites_plan_limits.rb b/db/migrate/20201007033723_insert_daily_invites_plan_limits.rb
new file mode 100644
index 00000000000..dcdcbbb0964
--- /dev/null
+++ b/db/migrate/20201007033723_insert_daily_invites_plan_limits.rb
@@ -0,0 +1,25 @@
+# frozen_string_literal: true
+
+class InsertDailyInvitesPlanLimits < ActiveRecord::Migration[6.0]
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ def up
+ return unless Gitlab.com?
+
+ create_or_update_plan_limit('daily_invites', 'free', 20)
+ create_or_update_plan_limit('daily_invites', 'bronze', 0)
+ create_or_update_plan_limit('daily_invites', 'silver', 0)
+ create_or_update_plan_limit('daily_invites', 'gold', 0)
+ end
+
+ def down
+ return unless Gitlab.com?
+
+ create_or_update_plan_limit('daily_invites', 'free', 0)
+ create_or_update_plan_limit('daily_invites', 'bronze', 0)
+ create_or_update_plan_limit('daily_invites', 'silver', 0)
+ create_or_update_plan_limit('daily_invites', 'gold', 0)
+ end
+end
diff --git a/db/schema_migrations/20201007033527 b/db/schema_migrations/20201007033527
new file mode 100644
index 00000000000..b2cedd57973
--- /dev/null
+++ b/db/schema_migrations/20201007033527
@@ -0,0 +1 @@
+1200747265d5095a86250020786d6f1e9e50bc75328a71de497046807afa89d7 \ No newline at end of file
diff --git a/db/schema_migrations/20201007033723 b/db/schema_migrations/20201007033723
new file mode 100644
index 00000000000..c874ae0475b
--- /dev/null
+++ b/db/schema_migrations/20201007033723
@@ -0,0 +1 @@
+febefead6f966960f6493d29add5f35fc4a1080b5118c5526502fa5fe1d29023 \ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index 1ed6ea64eca..ae84f8aa79b 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -14732,7 +14732,8 @@ CREATE TABLE plan_limits (
golang_max_file_size bigint DEFAULT 104857600 NOT NULL,
debian_max_file_size bigint DEFAULT '3221225472'::bigint NOT NULL,
project_feature_flags integer DEFAULT 200 NOT NULL,
- ci_max_artifact_size_api_fuzzing integer DEFAULT 0 NOT NULL
+ ci_max_artifact_size_api_fuzzing integer DEFAULT 0 NOT NULL,
+ daily_invites integer DEFAULT 0 NOT NULL
);
CREATE SEQUENCE plan_limits_id_seq
diff --git a/doc/administration/instance_limits.md b/doc/administration/instance_limits.md
index ec56a59fdc2..cfd9bccb24d 100644
--- a/doc/administration/instance_limits.md
+++ b/doc/administration/instance_limits.md
@@ -96,6 +96,13 @@ Read more on the [Rack Attack initializer](../security/rack_attack.md) method of
- **Default rate limit** - Disabled
+### Member Invitations
+
+Limit the maximum daily member invitations allowed per group hierarchy.
+
+- GitLab.com: Free members may invite 20 members per day.
+- Self-managed: Invites are not limited.
+
## Gitaly concurrency limit
Clone traffic can put a large strain on your Gitaly service. To prevent such workloads from overwhelming your Gitaly server, you can set concurrency limits in Gitaly’s configuration file.
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 68643cb4a62..c7ed47a279c 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -1762,6 +1762,9 @@ msgstr ""
msgid "AddContextCommits|Add/remove"
msgstr ""
+msgid "AddMember|Invite limit of %{daily_invites} per day exceeded"
+msgstr ""
+
msgid "AddMember|No users specified."
msgstr ""
diff --git a/spec/factories/project_members.rb b/spec/factories/project_members.rb
index 0c2ffac4112..1ce07f36021 100644
--- a/spec/factories/project_members.rb
+++ b/spec/factories/project_members.rb
@@ -15,7 +15,7 @@ FactoryBot.define do
trait(:invited) do
user_id { nil }
invite_token { 'xxx' }
- invite_email { 'email@email.com' }
+ sequence(:invite_email) { |n| "email#{n}@email.com" }
end
trait :blocked do
diff --git a/spec/migrations/insert_daily_invites_plan_limits_spec.rb b/spec/migrations/insert_daily_invites_plan_limits_spec.rb
new file mode 100644
index 00000000000..3265efcb0ce
--- /dev/null
+++ b/spec/migrations/insert_daily_invites_plan_limits_spec.rb
@@ -0,0 +1,55 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'migrate', '20201007033723_insert_daily_invites_plan_limits.rb')
+
+RSpec.describe InsertDailyInvitesPlanLimits do
+ let(:plans) { table(:plans) }
+ let(:plan_limits) { table(:plan_limits) }
+ let!(:free_plan) { plans.create!(name: 'free') }
+ let!(:bronze_plan) { plans.create!(name: 'bronze') }
+ let!(:silver_plan) { plans.create!(name: 'silver') }
+ let!(:gold_plan) { plans.create!(name: 'gold') }
+
+ context 'when on Gitlab.com' do
+ before do
+ expect(Gitlab).to receive(:com?).at_most(:twice).and_return(true)
+ end
+
+ it 'correctly migrates up and down' do
+ reversible_migration do |migration|
+ migration.before -> {
+ expect(plan_limits.where.not(daily_invites: 0)).to be_empty
+ }
+
+ # Expectations will run after the up migration.
+ migration.after -> {
+ expect(plan_limits.pluck(:plan_id, :daily_invites)).to contain_exactly(
+ [free_plan.id, 20],
+ [bronze_plan.id, 0],
+ [silver_plan.id, 0],
+ [gold_plan.id, 0]
+ )
+ }
+ end
+ end
+ end
+
+ context 'when on self hosted' do
+ before do
+ expect(Gitlab).to receive(:com?).at_most(:twice).and_return(false)
+ end
+
+ it 'correctly migrates up and down' do
+ reversible_migration do |migration|
+ migration.before -> {
+ expect(plan_limits.pluck(:daily_invites)).to eq []
+ }
+
+ migration.after -> {
+ expect(plan_limits.pluck(:daily_invites)).to eq []
+ }
+ end
+ end
+ end
+end
diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb
index 1a791820f1b..b60af7abade 100644
--- a/spec/models/member_spec.rb
+++ b/spec/models/member_spec.rb
@@ -171,6 +171,43 @@ RSpec.describe Member do
end
end
+ describe '.in_hierarchy' do
+ let(:root_ancestor) { create(:group) }
+ let(:project) { create(:project, group: root_ancestor) }
+ let(:subgroup) { create(:group, parent: root_ancestor) }
+ let(:subgroup_project) { create(:project, group: subgroup) }
+
+ let!(:root_ancestor_member) { create(:group_member, group: root_ancestor) }
+ let!(:project_member) { create(:project_member, project: project) }
+ let!(:subgroup_member) { create(:group_member, group: subgroup) }
+ let!(:subgroup_project_member) { create(:project_member, project: subgroup_project) }
+
+ let(:hierarchy_members) do
+ [
+ root_ancestor_member,
+ project_member,
+ subgroup_member,
+ subgroup_project_member
+ ]
+ end
+
+ subject { Member.in_hierarchy(project) }
+
+ it { is_expected.to contain_exactly(*hierarchy_members) }
+
+ context 'with scope prefix' do
+ subject { Member.where.not(source: project).in_hierarchy(subgroup) }
+
+ it { is_expected.to contain_exactly(root_ancestor_member, subgroup_member, subgroup_project_member) }
+ end
+
+ context 'with scope suffix' do
+ subject { Member.in_hierarchy(project).where.not(source: project) }
+
+ it { is_expected.to contain_exactly(root_ancestor_member, subgroup_member, subgroup_project_member) }
+ end
+ end
+
describe '.invite' do
it { expect(described_class.invite).not_to include @maintainer }
it { expect(described_class.invite).to include @invited_member }
@@ -251,6 +288,21 @@ RSpec.describe Member do
it { is_expected.to include(expiring_tomorrow, not_expiring) }
end
+ describe '.created_today' do
+ let_it_be(:now) { Time.current }
+ let_it_be(:created_today) { create(:group_member, created_at: now.beginning_of_day) }
+ let_it_be(:created_yesterday) { create(:group_member, created_at: now - 1.day) }
+
+ before do
+ travel_to now
+ end
+
+ subject { described_class.created_today }
+
+ it { is_expected.not_to include(created_yesterday) }
+ it { is_expected.to include(created_today) }
+ end
+
describe '.last_ten_days_excluding_today' do
let_it_be(:now) { Time.current }
let_it_be(:created_today) { create(:group_member, created_at: now.beginning_of_day) }
diff --git a/spec/models/plan_limits_spec.rb b/spec/models/plan_limits_spec.rb
index 67fb11f34e0..4259c8b708b 100644
--- a/spec/models/plan_limits_spec.rb
+++ b/spec/models/plan_limits_spec.rb
@@ -209,6 +209,7 @@ RSpec.describe PlanLimits do
ci_pipeline_size
ci_active_jobs
storage_size_limit
+ daily_invites
] + disabled_max_artifact_size_columns
end
diff --git a/vendor/gitignore/C++.gitignore b/vendor/gitignore/C++.gitignore
index 259148fa18f..259148fa18f 100755..100644
--- a/vendor/gitignore/C++.gitignore
+++ b/vendor/gitignore/C++.gitignore
diff --git a/vendor/gitignore/Java.gitignore b/vendor/gitignore/Java.gitignore
index a1c2a238a96..a1c2a238a96 100755..100644
--- a/vendor/gitignore/Java.gitignore
+++ b/vendor/gitignore/Java.gitignore