From 2efb0b0e6017a87f3c547f6e7c67579930fda65b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jarka=20Ko=C5=A1anov=C3=A1?= Date: Thu, 11 May 2023 16:44:33 +0200 Subject: [PATCH] Put elevating_permissions behind a FF --- ee/app/models/members/member_role.rb | 1 + .../development/elevated_guests.yml | 8 ++++ ee/lib/api/member_roles.rb | 2 +- ee/spec/models/ee/user_spec.rb | 1 + ee/spec/models/members/member_role_spec.rb | 45 +++++++++++++------ ee/spec/requests/api/member_roles_spec.rb | 2 + 6 files changed, 45 insertions(+), 14 deletions(-) create mode 100644 ee/config/feature_flags/development/elevated_guests.yml diff --git a/ee/app/models/members/member_role.rb b/ee/app/models/members/member_role.rb index 98dbc2972e0e0a..83328538881143 100644 --- a/ee/app/models/members/member_role.rb +++ b/ee/app/models/members/member_role.rb @@ -24,6 +24,7 @@ class MemberRole < ApplicationRecord # rubocop:disable Gitlab/NamespacedClass validates_associated :members scope :elevating, -> do + return none unless Feature.enabled?(:elevated_guests) return none if elevating_permissions.empty? query = elevating_permissions.map { |permission| "#{permission} = true" } diff --git a/ee/config/feature_flags/development/elevated_guests.yml b/ee/config/feature_flags/development/elevated_guests.yml new file mode 100644 index 00000000000000..24bb599ac337b2 --- /dev/null +++ b/ee/config/feature_flags/development/elevated_guests.yml @@ -0,0 +1,8 @@ +--- +name: elevated_guests +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/120488 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/410805 +milestone: '16.0' +type: development +group: group::authentication and authorization +default_enabled: false diff --git a/ee/lib/api/member_roles.rb b/ee/lib/api/member_roles.rb index e0234ad599e60a..e5af5463ae0bf8 100644 --- a/ee/lib/api/member_roles.rb +++ b/ee/lib/api/member_roles.rb @@ -43,7 +43,7 @@ class MemberRoles < ::API::Base ) ::MemberRole::ALL_CUSTOMIZABLE_PERMISSIONS.each do |permission_name, permission_description| - optional permission_name.to_s, type: Boolean, desc: permission_description + optional permission_name.to_s, type: Boolean, desc: permission_description, default: false end end diff --git a/ee/spec/models/ee/user_spec.rb b/ee/spec/models/ee/user_spec.rb index df1c0f655ffc4e..5477538588d86a 100644 --- a/ee/spec/models/ee/user_spec.rb +++ b/ee/spec/models/ee/user_spec.rb @@ -745,6 +745,7 @@ before do license = double('License', exclude_guests_from_active_count?: true) allow(License).to receive(:current) { license } + stub_feature_flags(elevated_guests: false) end it 'validates the sql matches the specific index we have' do diff --git a/ee/spec/models/members/member_role_spec.rb b/ee/spec/models/members/member_role_spec.rb index 07149eac613092..86bc800ad8cc4a 100644 --- a/ee/spec/models/members/member_role_spec.rb +++ b/ee/spec/models/members/member_role_spec.rb @@ -133,25 +133,44 @@ describe 'scopes' do describe '.elevating' do - it 'creates proper query' do - stub_const("#{described_class.name}::ALL_CUSTOMIZABLE_PERMISSIONS", { read_code: 'Permission to read code', - see_code: 'Test permission' }) + context 'with elevated_guests FF enabled' do + before do + stub_feature_flags(elevated_guests: true) + end - expect(described_class.elevating.to_sql).to include('WHERE (see_code = true)') - end + it 'creates proper query' do + stub_const("#{described_class.name}::ALL_CUSTOMIZABLE_PERMISSIONS", { read_code: 'Permission to read code', + see_code: 'Test permission' }) - it 'creates proper query with multiple permissions' do - stub_const("#{described_class.name}::ALL_CUSTOMIZABLE_PERMISSIONS", { read_code: 'Permission to read code', - see_code: 'Test permission', - remove_code: 'Test second permission' }) + expect(described_class.elevating.to_sql).to include('WHERE (see_code = true)') + end - expect(described_class.elevating.to_sql).to include('WHERE (see_code = true OR remove_code = true)') + it 'creates proper query with multiple permissions' do + stub_const("#{described_class.name}::ALL_CUSTOMIZABLE_PERMISSIONS", { read_code: 'Permission to read code', + see_code: 'Test permission', + remove_code: 'Test second permission' }) + + expect(described_class.elevating.to_sql).to include('WHERE (see_code = true OR remove_code = true)') + end + + it 'returns nothing when there are no elevating permissions' do + create(:member_role) + + expect(described_class.elevating).to be_empty + end + end + end + + context 'with elevated_guests FF disabled' do + before do + stub_feature_flags(elevated_guests: false) end - it 'returns nothing when there are no elevating permissions' do - create(:member_role) + it 'returns nothing' do + stub_const("#{described_class.name}::ALL_CUSTOMIZABLE_PERMISSIONS", { read_code: 'Permission to read code', + see_code: 'Test permission' }) - expect(described_class.elevating).to be_empty + expect(described_class.elevating.to_sql).not_to include('WHERE (see_code = true)') end end end diff --git a/ee/spec/requests/api/member_roles_spec.rb b/ee/spec/requests/api/member_roles_spec.rb index 5cff4002c3b8b5..44c81a8e285cf8 100644 --- a/ee/spec/requests/api/member_roles_spec.rb +++ b/ee/spec/requests/api/member_roles_spec.rb @@ -93,12 +93,14 @@ "id" => member_role_1.id, "base_access_level" => ::Gitlab::Access::REPORTER, "read_code" => false, + "read_vulnerability" => false, "group_id" => group_id }, { "id" => member_role_2.id, "base_access_level" => ::Gitlab::Access::REPORTER, "read_code" => true, + "read_vulnerability" => false, "group_id" => group_id } ] -- GitLab