This instance will be upgraded to Heptapod 0.26.0rc1 on 2021-10-26 at 14:30 UTC+2 (a few minutes of down time)

Commit fd05e266 authored by Ahmad Sherif's avatar Ahmad Sherif
Browse files

Precalculate user's authorized projects in database

Closes #23150
parent b52bf9d5a156
module SelectForProjectAuthorization
extend ActiveSupport::Concern
module ClassMethods
def select_for_project_authorization
select("members.user_id, projects.id AS project_id, members.access_level")
end
end
end
......@@ -5,6 +5,7 @@ class Group < Namespace
include Gitlab::VisibilityLevel
include AccessRequestable
include Referable
include SelectForProjectAuthorization
has_many :group_members, -> { where(requested_at: nil) }, dependent: :destroy, as: :source
alias_method :members, :group_members
......@@ -61,6 +62,14 @@ def reference_pattern
def visible_to_user(user)
where(id: user.authorized_groups.select(:id).reorder(nil))
end
def select_for_project_authorization
if current_scope.joins_values.include?(:shared_projects)
select("members.user_id, projects.id AS project_id, project_group_links.group_access")
else
super
end
end
end
def to_reference(_from_project = nil)
......@@ -176,4 +185,8 @@ def post_destroy_hook
def system_hook_service
SystemHooksService.new
end
def refresh_members_authorized_projects
UserProjectAccessChangedService.new(users.pluck(:id)).execute
end
end
......@@ -113,6 +113,8 @@ def add_user(source, user, access_level, current_user: nil, expires_at: nil)
member.save
end
UserProjectAccessChangedService.new(user.id).execute if user.is_a?(User)
member
end
......@@ -239,6 +241,7 @@ def send_request
end
def post_create_hook
UserProjectAccessChangedService.new(user.id).execute
system_hook_service.execute_hooks_for(self, :create)
end
......@@ -247,9 +250,19 @@ def post_update_hook
end
def post_destroy_hook
refresh_member_authorized_projects
system_hook_service.execute_hooks_for(self, :destroy)
end
def refresh_member_authorized_projects
# If user/source is being destroyed, project access are gonna be destroyed eventually
# because of DB foreign keys, so we shouldn't bother with refreshing after each
# member is destroyed through association
return if destroyed_by_association.present?
UserProjectAccessChangedService.new(user_id).execute
end
def after_accept_invite
post_create_hook
end
......
......@@ -13,6 +13,7 @@ class Project < ActiveRecord::Base
include CaseSensitivity
include TokenAuthenticatable
include ProjectFeaturesCompatibility
include SelectForProjectAuthorization
extend Gitlab::ConfigHelper
......@@ -1289,16 +1290,10 @@ def secret_variables
# Checks if `user` is authorized for this project, with at least the
# `min_access_level` (if given).
#
# If you change the logic of this method, please also update `User#authorized_projects`
def authorized_for_user?(user, min_access_level = nil)
return false unless user
return true if personal? && namespace_id == user.namespace_id
authorized_for_user_by_group?(user, min_access_level) ||
authorized_for_user_by_members?(user, min_access_level) ||
authorized_for_user_by_shared_projects?(user, min_access_level)
user.authorized_project?(self, min_access_level)
end
def append_or_update_attribute(name, value)
......@@ -1358,30 +1353,6 @@ def default_branch_protected?
current_application_settings.default_branch_protection == Gitlab::Access::PROTECTION_DEV_CAN_MERGE
end
def authorized_for_user_by_group?(user, min_access_level)
member = user.group_members.find_by(source_id: group)
member && (!min_access_level || member.access_level >= min_access_level)
end
def authorized_for_user_by_members?(user, min_access_level)
member = members.find_by(user_id: user)
member && (!min_access_level || member.access_level >= min_access_level)
end
def authorized_for_user_by_shared_projects?(user, min_access_level)
shared_projects = user.group_members.joins(group: :shared_projects).
where(project_group_links: { project_id: self })
if min_access_level
members_scope = { access_level: Gitlab::Access.values.select { |access| access >= min_access_level } }
shared_projects = shared_projects.where(members: members_scope)
end
shared_projects.any?
end
# Similar to the normal callbacks that hook into the life cycle of an
# Active Record object, you can also define callbacks that get triggered
# when you add an object to an association collection. If any of these
......
class ProjectAuthorization < ActiveRecord::Base
belongs_to :user
belongs_to :project
validates :project, presence: true
validates :access_level, inclusion: { in: Gitlab::Access.all_values }, presence: true
validates :user, uniqueness: { scope: [:project, :access_level] }, presence: true
end
......@@ -16,6 +16,9 @@ class ProjectGroupLink < ActiveRecord::Base
validates :group_access, inclusion: { in: Gitlab::Access.values }, presence: true
validate :different_group
after_create :refresh_group_members_authorized_projects
after_destroy :refresh_group_members_authorized_projects
def self.access_options
Gitlab::Access.options
end
......@@ -35,4 +38,8 @@ def different_group
errors.add(:base, "Project cannot be shared with the project it is in.")
end
end
def refresh_group_members_authorized_projects
group.refresh_members_authorized_projects
end
end
......@@ -72,6 +72,8 @@ class User < ActiveRecord::Base
has_many :created_projects, foreign_key: :creator_id, class_name: 'Project'
has_many :users_star_projects, dependent: :destroy
has_many :starred_projects, through: :users_star_projects, source: :project
has_many :project_authorizations, dependent: :destroy
has_many :authorized_projects, through: :project_authorizations, source: :project
has_many :snippets, dependent: :destroy, foreign_key: :author_id
has_many :issues, dependent: :destroy, foreign_key: :author_id
......@@ -438,11 +440,44 @@ def authorized_groups
Group.where("namespaces.id IN (#{union.to_sql})")
end
# Returns projects user is authorized to access.
#
# If you change the logic of this method, please also update `Project#authorized_for_user`
def refresh_authorized_projects
loop do
begin
Gitlab::Database.serialized_transaction do
project_authorizations.delete_all
# project_authorizations_union can return multiple records for the same project/user with
# different access_level so we take row with the maximum access_level
project_authorizations.connection.execute <<-SQL
INSERT INTO project_authorizations (user_id, project_id, access_level)
SELECT user_id, project_id, MAX(access_level) AS access_level
FROM (#{project_authorizations_union.to_sql}) sub
GROUP BY user_id, project_id
SQL
update_column(:authorized_projects_populated, true) unless authorized_projects_populated
end
break
# In the event of a concurrent modification Rails raises StatementInvalid.
# In this case we want to keep retrying until the transaction succeeds
rescue ActiveRecord::StatementInvalid
end
end
end
def authorized_projects(min_access_level = nil)
Project.where("projects.id IN (#{projects_union(min_access_level).to_sql})")
refresh_authorized_projects unless authorized_projects_populated
# We're overriding an association, so explicitly call super with no arguments or it would be passed as `force_reload` to the association
projects = super()
projects = projects.where('project_authorizations.access_level >= ?', min_access_level) if min_access_level
projects
end
def authorized_project?(project, min_access_level = nil)
authorized_projects(min_access_level).exists?({ id: project.id })
end
# Returns the projects this user has reporter (or greater) access to, limited
......@@ -456,8 +491,9 @@ def projects_with_reporter_access_limited_to(projects)
end
def viewable_starred_projects
starred_projects.where("projects.visibility_level IN (?) OR projects.id IN (#{projects_union.to_sql})",
[Project::PUBLIC, Project::INTERNAL])
starred_projects.where("projects.visibility_level IN (?) OR projects.id IN (?)",
[Project::PUBLIC, Project::INTERNAL],
authorized_projects.select(:project_id))
end
def owned_projects
......@@ -887,16 +923,14 @@ def increment_failed_attempts!
private
def projects_union(min_access_level = nil)
relations = [personal_projects.select(:id),
groups_projects.select(:id),
projects.select(:id),
groups.joins(:shared_projects).select(:project_id)]
if min_access_level
scope = { access_level: Gitlab::Access.all_values.select { |access| access >= min_access_level } }
relations = [relations.shift] + relations.map { |relation| relation.where(members: scope) }
end
# Returns a union query of projects that the user is authorized to access
def project_authorizations_union
relations = [
personal_projects.select("#{id} AS user_id, projects.id AS project_id, #{Gitlab::Access::OWNER} AS access_level"),
groups_projects.select_for_project_authorization,
projects.select_for_project_authorization,
groups.joins(:shared_projects).select_for_project_authorization
]
Gitlab::SQL::Union.new(relations)
end
......
......@@ -106,6 +106,8 @@ def after_create_actions
unless @project.group || @project.gitlab_project_import?
@project.team << [current_user, :master, current_user]
end
@project.group.refresh_members_authorized_projects if @project.group
end
def skip_wiki?
......
class UserProjectAccessChangedService
def initialize(user_ids)
@user_ids = Array.wrap(user_ids)
end
def execute
AuthorizedProjectsWorker.bulk_perform_async(@user_ids.map { |id| [id] })
end
end
class AuthorizedProjectsWorker
include Sidekiq::Worker
include DedicatedSidekiqQueue
def self.bulk_perform_async(args_list)
Sidekiq::Client.push_bulk('class' => self, 'args' => args_list)
end
def perform(user_id)
user = User.find_by(id: user_id)
return unless user
user.refresh_authorized_projects
end
end
---
title: Precalculate user's authorized projects in database
merge_request: 6839
author:
......@@ -35,6 +35,7 @@
- [clear_database_cache, 1]
- [delete_user, 1]
- [delete_merged_branches, 1]
- [authorized_projects, 1]
- [expire_build_instance_artifacts, 1]
- [group_destroy, 1]
- [irker, 1]
......
require 'sidekiq/testing'
require './db/fixtures/support/serialized_transaction'
Sidekiq::Testing.inline! do
Gitlab::Seeder.quiet do
......
require 'sidekiq/testing'
require './spec/support/test_env'
require './db/fixtures/support/serialized_transaction'
class Gitlab::Seeder::CycleAnalytics
def initialize(project, perf: false)
......
require 'gitlab/database'
module Gitlab
module Database
def self.serialized_transaction
connection.transaction { yield }
end
end
end
class CreateProjectAuthorizations < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def change
create_table :project_authorizations do |t|
t.references :user, foreign_key: { on_delete: :cascade }
t.references :project, foreign_key: { on_delete: :cascade }
t.integer :access_level
t.index [:user_id, :project_id, :access_level], unique: true, name: 'index_project_authorizations_on_user_id_project_id_access_level'
end
end
end
class AddAuthorizedProjectsPopulatedToUsers < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def change
add_column :users, :authorized_projects_populated, :boolean
end
end
......@@ -844,6 +844,14 @@
add_index "personal_access_tokens", ["token"], name: "index_personal_access_tokens_on_token", unique: true, using: :btree
add_index "personal_access_tokens", ["user_id"], name: "index_personal_access_tokens_on_user_id", using: :btree
create_table "project_authorizations", force: :cascade do |t|
t.integer "user_id"
t.integer "project_id"
t.integer "access_level"
end
add_index "project_authorizations", ["user_id", "project_id", "access_level"], name: "index_project_authorizations_on_user_id_project_id_access_level", unique: true, using: :btree
create_table "project_features", force: :cascade do |t|
t.integer "project_id"
t.integer "merge_requests_access_level"
......@@ -1187,6 +1195,7 @@
t.boolean "external", default: false
t.string "organization"
t.string "incoming_email_token"
t.boolean "authorized_projects_populated"
end
add_index "users", ["admin"], name: "index_users_on_admin", using: :btree
......@@ -1248,6 +1257,8 @@
add_foreign_key "merge_requests_closing_issues", "issues", on_delete: :cascade
add_foreign_key "merge_requests_closing_issues", "merge_requests", on_delete: :cascade
add_foreign_key "personal_access_tokens", "users"
add_foreign_key "project_authorizations", "projects", on_delete: :cascade
add_foreign_key "project_authorizations", "users", on_delete: :cascade
add_foreign_key "protected_branch_merge_access_levels", "protected_branches"
add_foreign_key "protected_branch_push_access_levels", "protected_branches"
add_foreign_key "trending_projects", "projects", on_delete: :cascade
......
......@@ -35,6 +35,13 @@ def self.nulls_last_order(field, direction = 'ASC')
order
end
def self.serialized_transaction
opts = {}
opts[:isolation] = :serializable unless Rails.env.test? && connection.transaction_open?
connection.transaction(opts) { yield }
end
def self.random
Gitlab::Database.postgresql? ? "RANDOM()" : "RAND()"
end
......
......@@ -38,7 +38,10 @@
end
describe 'without group member current_user' do
before { shared_project_2.team << [current_user, Gitlab::Access::MASTER] }
before do
shared_project_2.team << [current_user, Gitlab::Access::MASTER]
current_user.reload
end
context "only shared" do
context "without external user" do
......
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