Merge branch 'if-215385-project_auth_refresh_for_project_create' into 'master'

Specialized worker for updating project authorizations on project create

See merge request gitlab-org/gitlab!30870

app/models/group.rb

@@ -335,6 +335,11 @@ class Group < Namespace
       .where(source_id: source_ids)
   end
 
+  def members_from_self_and_ancestors_with_effective_access_level
+    members_with_parents.select([:user_id, 'MAX(access_level) AS access_level'])
+                        .group(:user_id)
+  end
+
   def members_with_descendants
     GroupMember
       .active_without_invites_and_requests

app/models/member.rb

 # frozen_string_literal: true
 
 class Member < ApplicationRecord
+  include EachBatch
   include AfterCommitQueue
   include Sortable
   include Importable

app/services/authorized_project_update/project_create_service.rb

+# frozen_string_literal: true
+
+module AuthorizedProjectUpdate
+  class ProjectCreateService < BaseService
+    BATCH_SIZE = 1000
+
+    def initialize(project)
+      @project = project
+    end
+
+    def execute
+      group = project.group
+
+      unless group
+        return ServiceResponse.error(message: 'Project does not have a group')
+      end
+
+      group.members_from_self_and_ancestors_with_effective_access_level
+           .each_batch(of: BATCH_SIZE, column: :user_id) do |members|
+        attributes = members.map do |member|
+          { user_id: member.user_id, project_id: project.id, access_level: member.access_level }
+        end
+
+        ProjectAuthorization.insert_all(attributes)
+      end
+
+      ServiceResponse.success
+    end
+
+    private
+
+    attr_reader :project
+  end
+end

app/workers/all_queues.yml

@@ -3,6 +3,13 @@
 #
 # Do not edit it manually!
 ---
+- :name: authorized_project_update:authorized_project_update_project_create
+  :feature_category: :authentication_and_authorization
+  :has_external_dependencies: 
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: true
 - :name: auto_devops:auto_devops_disable
   :feature_category: :auto_devops
   :has_external_dependencies: 

app/workers/authorized_project_update/project_create_worker.rb

+# frozen_string_literal: true
+
+module AuthorizedProjectUpdate
+  class ProjectCreateWorker
+    include ApplicationWorker
+
+    feature_category :authentication_and_authorization
+    urgency :low
+    queue_namespace :authorized_project_update
+
+    idempotent!
+
+    def perform(project_id)
+      project = Project.find(project_id)
+
+      AuthorizedProjectUpdate::ProjectCreateService.new(project).execute
+    end
+  end
+end

config/sidekiq_queues.yml

@@ -32,6 +32,8 @@
   - 1
 - - authorized_keys
   - 2
+- - authorized_project_update
+  - 1
 - - authorized_project_update_user_refresh_with_low_urgency
   - 1
 - - authorized_projects

spec/models/group_spec.rb

@@ -659,6 +659,42 @@ describe Group do
     end
   end
 
+  describe '#members_from_self_and_ancestors_with_effective_access_level' do
+    let!(:group_parent) { create(:group, :private) }
+    let!(:group) { create(:group, :private, parent: group_parent) }
+    let!(:group_child) { create(:group, :private, parent: group) }
+
+    let!(:user) { create(:user) }
+
+    let(:parent_group_access_level) { Gitlab::Access::REPORTER }
+    let(:group_access_level) { Gitlab::Access::DEVELOPER }
+    let(:child_group_access_level) { Gitlab::Access::MAINTAINER }
+
+    before do
+      create(:group_member, user: user, group: group_parent, access_level: parent_group_access_level)
+      create(:group_member, user: user, group: group, access_level: group_access_level)
+      create(:group_member, user: user, group: group_child, access_level: child_group_access_level)
+    end
+
+    it 'returns effective access level for user' do
+      expect(group_parent.members_from_self_and_ancestors_with_effective_access_level.as_json).to(
+        contain_exactly(
+          hash_including('user_id' => user.id, 'access_level' => parent_group_access_level)
+        )
+      )
+      expect(group.members_from_self_and_ancestors_with_effective_access_level.as_json).to(
+        contain_exactly(
+          hash_including('user_id' => user.id, 'access_level' => group_access_level)
+        )
+      )
+      expect(group_child.members_from_self_and_ancestors_with_effective_access_level.as_json).to(
+        contain_exactly(
+          hash_including('user_id' => user.id, 'access_level' => child_group_access_level)
+        )
+      )
+    end
+  end
+
   describe '#direct_and_indirect_members' do
     let!(:group) { create(:group, :nested) }
     let!(:sub_group) { create(:group, parent: group) }

spec/services/authorized_project_update/project_create_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe AuthorizedProjectUpdate::ProjectCreateService do
+  let_it_be(:group_parent) { create(:group, :private) }
+  let_it_be(:group) { create(:group, :private, parent: group_parent) }
+  let_it_be(:group_child) { create(:group, :private, parent: group) }
+
+  let_it_be(:group_project) { create(:project, group: group) }
+
+  let_it_be(:parent_group_user) { create(:user) }
+  let_it_be(:group_user) { create(:user) }
+  let_it_be(:child_group_user) { create(:user) }
+
+  let(:access_level) { Gitlab::Access::MAINTAINER }
+
+  subject(:service) { described_class.new(group_project) }
+
+  describe '#perform' do
+    context 'direct group members' do
+      before do
+        create(:group_member, access_level: access_level, group: group, user: group_user)
+        ProjectAuthorization.delete_all
+      end
+
+      it 'creates project authorization' do
+        expect { service.execute }.to(
+          change { ProjectAuthorization.count }.from(0).to(1))
+
+        project_authorization = ProjectAuthorization.where(
+          project_id: group_project.id,
+          user_id: group_user.id,
+          access_level: access_level)
+
+        expect(project_authorization).to exist
+      end
+    end
+
+    context 'inherited group members' do
+      before do
+        create(:group_member, access_level: access_level, group: group_parent, user: parent_group_user)
+        ProjectAuthorization.delete_all
+      end
+
+      it 'creates project authorization' do
+        expect { service.execute }.to(
+          change { ProjectAuthorization.count }.from(0).to(1))
+
+        project_authorization = ProjectAuthorization.where(
+          project_id: group_project.id,
+          user_id: parent_group_user.id,
+          access_level: access_level)
+        expect(project_authorization).to exist
+      end
+    end
+
+    context 'membership overrides' do
+      before do
+        create(:group_member, access_level: Gitlab::Access::REPORTER, group: group_parent, user: group_user)
+        create(:group_member, access_level: Gitlab::Access::DEVELOPER, group: group, user: group_user)
+        ProjectAuthorization.delete_all
+      end
+
+      it 'creates project authorization' do
+        expect { service.execute }.to(
+          change { ProjectAuthorization.count }.from(0).to(1))
+
+        project_authorization = ProjectAuthorization.where(
+          project_id: group_project.id,
+          user_id: group_user.id,
+          access_level: Gitlab::Access::DEVELOPER)
+        expect(project_authorization).to exist
+      end
+    end
+
+    context 'no group member' do
+      it 'does not create project authorization' do
+        expect { service.execute }.not_to(
+          change { ProjectAuthorization.count }.from(0))
+      end
+    end
+
+    context 'unapproved access requests' do
+      before do
+        create(:group_member, :guest, :access_request, user: group_user, group: group)
+      end
+
+      it 'does not create project authorization' do
+        expect { service.execute }.not_to(
+          change { ProjectAuthorization.count }.from(0))
+      end
+    end
+
+    context 'project has more user than BATCH_SIZE' do
+      let(:batch_size) { 2 }
+      let(:users) { create_list(:user, batch_size + 1 ) }
+
+      before do
+        stub_const("#{described_class.name}::BATCH_SIZE", batch_size)
+
+        users.each do |user|
+          create(:group_member, access_level: access_level, group: group_parent, user: user)
+        end
+
+        ProjectAuthorization.delete_all
+      end
+
+      it 'bulk creates project authorizations in batches' do
+        users.each_slice(batch_size) do |batch|
+          attributes = batch.map do |user|
+            { user_id: user.id, project_id: group_project.id, access_level: access_level }
+          end
+
+          expect(ProjectAuthorization).to(
+            receive(:insert_all).with(array_including(attributes)).and_call_original)
+        end
+
+        expect { service.execute }.to(
+          change { ProjectAuthorization.count }.from(0).to(batch_size + 1))
+      end
+    end
+
+    context 'ignores existing project authorizations' do
+      before do
+        # ProjectAuthorizations is also created because of an after_commit
+        # callback on Member model
+        create(:group_member, access_level: access_level, group: group, user: group_user)
+      end
+
+      it 'does not create project authorization' do
+        project_authorization = ProjectAuthorization.where(
+          project_id: group_project.id,
+          user_id: group_user.id,
+          access_level: access_level)
+
+        expect { service.execute }.not_to(
+          change { project_authorization.reload.exists? }.from(true))
+      end
+    end
+  end
+end

spec/workers/authorized_project_update/project_create_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe AuthorizedProjectUpdate::ProjectCreateWorker do
+  let_it_be(:group) { create(:group, :private) }
+  let_it_be(:group_project) { create(:project, group: group) }
+  let_it_be(:group_user) { create(:user) }
+
+  let(:access_level) { Gitlab::Access::MAINTAINER }
+
+  subject(:worker) { described_class.new }
+
+  it 'calls AuthorizedProjectUpdate::ProjectCreateService' do
+    expect_next_instance_of(AuthorizedProjectUpdate::ProjectCreateService) do |service|
+      expect(service).to(receive(:execute))
+    end
+
+    worker.perform(group_project.id)
+  end
+
+  it 'returns ServiceResponse.success' do
+    result = worker.perform(group_project.id)
+
+    expect(result.success?).to be_truthy
+  end
+
+  context 'idempotence' do
+    before do
+      create(:group_member, access_level: Gitlab::Access::MAINTAINER, group: group, user: group_user)
+      ProjectAuthorization.delete_all
+    end
+
+    include_examples 'an idempotent worker' do
+      let(:job_args) { group_project.id }
+
+      it 'creates project authorization' do
+        subject
+
+        project_authorization = ProjectAuthorization.where(
+          project_id: group_project.id,
+          user_id: group_user.id,
+          access_level: access_level)
+
+        expect(project_authorization).to exist
+        expect(ProjectAuthorization.count).to eq(1)
+      end
+    end
+  end
+end