Commit e70ee35d authored by GitLab Release Tools Bot's avatar GitLab Release Tools Bot
Browse files

Merge remote-tracking branch 'dev/12-6-stable' into 12-6-stable

parents 023ceeae 52c681e2
......@@ -2,6 +2,13 @@
documentation](doc/development/changelog.md) for instructions on adding your own
entry.
 
## 12.6.7
### Security (1 change)
- Fix ProjectAuthorization calculation for shared groups.
## 12.6.6
 
### Security (1 change)
......
......@@ -19,8 +19,10 @@ module Users
LEASE_TIMEOUT = 1.minute.to_i
 
# user - The User for which to refresh the authorized projects.
def initialize(user)
def initialize(user, incorrect_auth_found_callback: nil, missing_auth_found_callback: nil)
@user = user
@incorrect_auth_found_callback = incorrect_auth_found_callback
@missing_auth_found_callback = missing_auth_found_callback
 
# We need an up to date User object that has access to all relations that
# may have been created earlier. The only way to ensure this is to reload
......@@ -55,6 +57,10 @@ module Users
# rows not in the new list or with a different access level should be
# removed.
if !fresh[project_id] || fresh[project_id] != row.access_level
if incorrect_auth_found_callback
incorrect_auth_found_callback.call(project_id, row.access_level)
end
array << row.project_id
end
end
......@@ -63,6 +69,10 @@ module Users
# rows not in the old list or with a different access level should be
# added.
if !current[project_id] || current[project_id].access_level != level
if missing_auth_found_callback
missing_auth_found_callback.call(project_id, level)
end
array << [user.id, project_id, level]
end
end
......@@ -104,5 +114,9 @@ module Users
def fresh_authorizations
Gitlab::ProjectAuthorizations.new(user).calculate
end
private
attr_reader :incorrect_auth_found_callback, :missing_auth_found_callback
end
end
# frozen_string_literal: true
class ScheduleRecalculateProjectAuthorizations < ActiveRecord::Migration[5.1]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
MIGRATION = 'RecalculateProjectAuthorizations'
BATCH_SIZE = 2_500
DELAY_INTERVAL = 2.minutes.to_i
disable_ddl_transaction!
class Namespace < ActiveRecord::Base
include ::EachBatch
self.table_name = 'namespaces'
end
class ProjectAuthorization < ActiveRecord::Base
include ::EachBatch
self.table_name = 'project_authorizations'
end
def up
say "Scheduling #{MIGRATION} jobs"
max_group_id = Namespace.where(type: 'Group').maximum(:id)
project_authorizations = ProjectAuthorization.where('project_id <= ?', max_group_id)
.select(:user_id)
.distinct
project_authorizations.each_batch(of: BATCH_SIZE, column: :user_id) do |authorizations, index|
delay = index * DELAY_INTERVAL
user_ids = authorizations.map(&:user_id)
BackgroundMigrationWorker.perform_in(delay, MIGRATION, [user_ids])
end
end
def down
end
end
......@@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
 
ActiveRecord::Schema.define(version: 2019_12_16_183532) do
ActiveRecord::Schema.define(version: 2020_02_04_113223) do
 
# These are extensions that must be enabled in order to support this database
enable_extension "pg_trgm"
......
# frozen_string_literal: true
module Gitlab
module BackgroundMigration
# rubocop:disable Style/Documentation
class RecalculateProjectAuthorizations
def perform(user_ids)
user_ids.each do |user_id|
user = User.find_by(id: user_id)
next unless user
service = Users::RefreshAuthorizedProjectsService.new(
user,
incorrect_auth_found_callback:
->(project_id, access_level) do
logger.info(message: 'Removing ProjectAuthorizations',
user_id: user.id,
project_id: project_id,
access_level: access_level)
end,
missing_auth_found_callback:
->(project_id, access_level) do
logger.info(message: 'Creating ProjectAuthorizations',
user_id: user.id,
project_id: project_id,
access_level: access_level)
end
)
service.execute
end
end
private
def logger
@logger ||= Gitlab::BackgroundMigration::Logger.build
end
end
end
end
......@@ -68,12 +68,9 @@ module Gitlab
.select([namespaces[:id], members[:access_level]])
.except(:order)
 
if Feature.enabled?(:share_group_with_group)
# Namespaces shared with any of the group
cte << Group.select([namespaces[:id], 'group_group_links.group_access AS access_level'])
.joins(join_group_group_links)
.joins(join_members_on_group_group_links)
end
cte << Group.select([namespaces[:id], 'group_group_links.group_access AS access_level'])
.joins(join_group_group_links)
.joins(join_members_on_group_group_links)
 
# Sub groups of any groups the user is a member of.
cte << Group.select([
......@@ -114,6 +111,8 @@ module Gitlab
members = Member.arel_table
 
cond = group_group_links[:shared_with_group_id].eq(members[:source_id])
.and(members[:source_type].eq('Namespace'))
.and(members[:requested_at].eq(nil))
.and(members[:user_id].eq(user.id))
Arel::Nodes::InnerJoin.new(members, Arel::Nodes::On.new(cond))
end
......
# frozen_string_literal: true
FactoryBot.define do
factory :project_authorization do
user
project
access_level { Gitlab::Access::REPORTER }
end
end
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::BackgroundMigration::RecalculateProjectAuthorizations, :migration, schema: 20200204113223 do
let(:users_table) { table(:users) }
let(:namespaces_table) { table(:namespaces) }
let(:projects_table) { table(:projects) }
let(:project_authorizations_table) { table(:project_authorizations) }
let(:members_table) { table(:members) }
let(:group_group_links) { table(:group_group_links) }
let(:project_group_links) { table(:project_group_links) }
let(:user) { users_table.create!(id: 1, email: 'user@example.com', projects_limit: 10) }
let(:group) { namespaces_table.create!(type: 'Group', name: 'group', path: 'group') }
subject { described_class.new.perform([user.id]) }
context 'missing authorization' do
context 'personal project' do
before do
user_namespace = namespaces_table.create!(owner_id: user.id, name: 'User', path: 'user')
projects_table.create!(id: 1,
name: 'personal-project',
path: 'personal-project',
visibility_level: 0,
namespace_id: user_namespace.id)
end
it 'creates correct authorization' do
expect { subject }.to change { project_authorizations_table.count }.from(0).to(1)
expect(project_authorizations_table.all).to(
match_array([have_attributes(user_id: 1, project_id: 1, access_level: 40)]))
end
end
context 'group membership' do
before do
projects_table.create!(id: 1, name: 'group-project', path: 'group-project',
visibility_level: 0, namespace_id: group.id)
members_table.create!(user_id: user.id, source_id: group.id, source_type: 'Namespace',
type: 'GroupMember', access_level: 20, notification_level: 3)
end
it 'creates correct authorization' do
expect { subject }.to change { project_authorizations_table.count }.from(0).to(1)
expect(project_authorizations_table.all).to(
match_array([have_attributes(user_id: 1, project_id: 1, access_level: 20)]))
end
end
context 'inherited group membership' do
before do
sub_group = namespaces_table.create!(type: 'Group', name: 'subgroup',
path: 'subgroup', parent_id: group.id)
projects_table.create!(id: 1, name: 'group-project', path: 'group-project',
visibility_level: 0, namespace_id: sub_group.id)
members_table.create!(user_id: user.id, source_id: group.id, source_type: 'Namespace',
type: 'GroupMember', access_level: 20, notification_level: 3)
end
it 'creates correct authorization' do
expect { subject }.to change { project_authorizations_table.count }.from(0).to(1)
expect(project_authorizations_table.all).to(
match_array([have_attributes(user_id: 1, project_id: 1, access_level: 20)]))
end
end
context 'project membership' do
before do
project = projects_table.create!(id: 1, name: 'group-project', path: 'group-project',
visibility_level: 0, namespace_id: group.id)
members_table.create!(user_id: user.id, source_id: project.id, source_type: 'Project',
type: 'ProjectMember', access_level: 20, notification_level: 3)
end
it 'creates correct authorization' do
expect { subject }.to change { project_authorizations_table.count }.from(0).to(1)
expect(project_authorizations_table.all).to(
match_array([have_attributes(user_id: 1, project_id: 1, access_level: 20)]))
end
end
context 'shared group' do
before do
members_table.create!(user_id: user.id, source_id: group.id, source_type: 'Namespace',
type: 'GroupMember', access_level: 30, notification_level: 3)
shared_group = namespaces_table.create!(type: 'Group', name: 'shared group',
path: 'shared-group')
projects_table.create!(id: 1, name: 'project', path: 'project', visibility_level: 0,
namespace_id: shared_group.id)
group_group_links.create(shared_group_id: shared_group.id, shared_with_group_id: group.id,
group_access: 20)
end
it 'creates correct authorization' do
expect { subject }.to change { project_authorizations_table.count }.from(0).to(1)
expect(project_authorizations_table.all).to(
match_array([have_attributes(user_id: 1, project_id: 1, access_level: 20)]))
end
end
context 'shared project' do
before do
members_table.create!(user_id: user.id, source_id: group.id, source_type: 'Namespace',
type: 'GroupMember', access_level: 30, notification_level: 3)
another_group = namespaces_table.create!(type: 'Group', name: 'another group', path: 'another-group')
shared_project = projects_table.create!(id: 1, name: 'shared project', path: 'shared-project',
visibility_level: 0, namespace_id: another_group.id)
project_group_links.create(project_id: shared_project.id, group_id: group.id, group_access: 20)
end
it 'creates correct authorization' do
expect { subject }.to change { project_authorizations_table.count }.from(0).to(1)
expect(project_authorizations_table.all).to(
match_array([have_attributes(user_id: 1, project_id: 1, access_level: 20)]))
end
end
end
context 'unapproved access requests' do
context 'group membership' do
before do
projects_table.create!(id: 1, name: 'group-project', path: 'group-project',
visibility_level: 0, namespace_id: group.id)
members_table.create!(user_id: user.id, source_id: group.id, source_type: 'Namespace',
type: 'GroupMember', access_level: 20, requested_at: Time.now, notification_level: 3)
end
it 'does not create authorization' do
expect { subject }.not_to change { project_authorizations_table.count }.from(0)
end
end
context 'inherited group membership' do
before do
sub_group = namespaces_table.create!(type: 'Group', name: 'subgroup', path: 'subgroup',
parent_id: group.id)
projects_table.create!(id: 1, name: 'group-project', path: 'group-project',
visibility_level: 0, namespace_id: sub_group.id)
members_table.create!(user_id: user.id, source_id: group.id, source_type: 'Namespace',
type: 'GroupMember', access_level: 20, requested_at: Time.now, notification_level: 3)
end
it 'does not create authorization' do
expect { subject }.not_to change { project_authorizations_table.count }.from(0)
end
end
context 'project membership' do
before do
project = projects_table.create!(id: 1, name: 'group-project', path: 'group-project',
visibility_level: 0, namespace_id: group.id)
members_table.create!(user_id: user.id, source_id: project.id, source_type: 'Project',
type: 'ProjectMember', access_level: 20, requested_at: Time.now, notification_level: 3)
end
it 'does not create authorization' do
expect { subject }.not_to change { project_authorizations_table.count }.from(0)
end
end
context 'shared group' do
before do
members_table.create!(user_id: user.id, source_id: group.id, source_type: 'Namespace',
type: 'GroupMember', access_level: 30, requested_at: Time.now, notification_level: 3)
shared_group = namespaces_table.create!(type: 'Group', name: 'shared group',
path: 'shared-group')
projects_table.create!(id: 1, name: 'project', path: 'project', visibility_level: 0,
namespace_id: shared_group.id)
group_group_links.create(shared_group_id: shared_group.id, shared_with_group_id: group.id,
group_access: 20)
end
it 'does not create authorization' do
expect { subject }.not_to change { project_authorizations_table.count }.from(0)
end
end
context 'shared project' do
before do
members_table.create!(user_id: user.id, source_id: group.id, source_type: 'Namespace',
type: 'GroupMember', access_level: 30, requested_at: Time.now, notification_level: 3)
another_group = namespaces_table.create!(type: 'Group', name: 'another group', path: 'another-group')
shared_project = projects_table.create!(id: 1, name: 'shared project', path: 'shared-project',
visibility_level: 0, namespace_id: another_group.id)
project_group_links.create(project_id: shared_project.id, group_id: group.id, group_access: 20)
end
it 'does not create authorization' do
expect { subject }.not_to change { project_authorizations_table.count }.from(0)
end
end
end
context 'incorrect authorization' do
before do
project = projects_table.create!(id: 1, name: 'group-project', path: 'group-project',
visibility_level: 0, namespace_id: group.id)
members_table.create!(user_id: user.id, source_id: group.id, source_type: 'Namespace',
type: 'GroupMember', access_level: 30, notification_level: 3)
project_authorizations_table.create!(user_id: user.id, project_id: project.id,
access_level: 10)
end
it 'fixes authorization' do
expect { subject }.not_to change { project_authorizations_table.count }.from(1)
expect(project_authorizations_table.all).to(
match_array([have_attributes(user_id: 1, project_id: 1, access_level: 30)]))
end
end
context 'unwanted authorization' do
before do
project = projects_table.create!(name: 'group-project', path: 'group-project',
visibility_level: 0, namespace_id: group.id)
project_authorizations_table.create!(user_id: user.id, project_id: project.id,
access_level: 10)
end
it 'deletes authorization' do
expect { subject }.to change { project_authorizations_table.count }.from(1).to(0)
end
end
context 'deleted user' do
let(:nonexistent_user_id) { User.maximum(:id).to_i + 999 }
it 'does not fail' do
expect { described_class.new.perform([nonexistent_user_id]) }.not_to raise_error
end
end
end
......@@ -97,87 +97,68 @@ describe Gitlab::ProjectAuthorizations do
create(:group_group_link, shared_group: shared_group, shared_with_group: group)
end
 
context 'when feature flag share_group_with_group is enabled' do
before do
stub_feature_flags(share_group_with_group: true)
end
context 'group user' do
let(:user) { group_user }
context 'group user' do
let(:user) { group_user }
 
it 'creates proper authorizations' do
mapping = map_access_levels(authorizations)
it 'creates proper authorizations' do
mapping = map_access_levels(authorizations)
 
expect(mapping[project_parent.id]).to be_nil
expect(mapping[project.id]).to eq(Gitlab::Access::DEVELOPER)
expect(mapping[project_child.id]).to eq(Gitlab::Access::DEVELOPER)
end
expect(mapping[project_parent.id]).to be_nil
expect(mapping[project.id]).to eq(Gitlab::Access::DEVELOPER)
expect(mapping[project_child.id]).to eq(Gitlab::Access::DEVELOPER)
end
end
 
context 'parent group user' do
let(:user) { parent_group_user }
context 'parent group user' do
let(:user) { parent_group_user }
 
it 'creates proper authorizations' do
mapping = map_access_levels(authorizations)
it 'creates proper authorizations' do
mapping = map_access_levels(authorizations)
 
expect(mapping[project_parent.id]).to be_nil
expect(mapping[project.id]).to be_nil
expect(mapping[project_child.id]).to be_nil
end
expect(mapping[project_parent.id]).to be_nil
expect(mapping[project.id]).to be_nil
expect(mapping[project_child.id]).to be_nil
end
end
 
context 'child group user' do
let(:user) { child_group_user }
context 'child group user' do
let(:user) { child_group_user }
 
it 'creates proper authorizations' do
mapping = map_access_levels(authorizations)
it 'creates proper authorizations' do
mapping = map_access_levels(authorizations)
 
expect(mapping[project_parent.id]).to be_nil
expect(mapping[project.id]).to be_nil
expect(mapping[project_child.id]).to be_nil
end
expect(mapping[project_parent.id]).to be_nil
expect(mapping[project.id]).to be_nil
expect(mapping[project_child.id]).to be_nil
end
end
 
context 'when feature flag share_group_with_group is disabled' do
before do
stub_feature_flags(share_group_with_group: false)
end
context 'group user' do
let(:user) { group_user }
it 'creates proper authorizations' do
mapping = map_access_levels(authorizations)
context 'user without accepted access request' do
let!(:user) { create(:user) }
 
expect(mapping[project_parent.id]).to be_nil
expect(mapping[project.id]).to be_nil
expect(mapping[project_child.id]).to be_nil
end
end
it 'does not have access to group and its projects' do
create(:group_member, :developer, :access_request, user: user, group: group)
 
context 'parent group user' do
let(:user) { parent_group_user }
mapping = map_access_levels(authorizations)
 
it 'creates proper authorizations' do
mapping = map_access_levels(authorizations)
expect(mapping[project_parent.id]).to be_nil
expect(mapping[project.id]).to be_nil
expect(mapping[project_child.id]).to be_nil
end
expect(mapping[project_parent.id]).to be_nil
expect(mapping[project.id]).to be_nil
expect(mapping[project_child.id]).to be_nil
end
end
 
context 'child group user' do
let(:user) { child_group_user }
context 'unrelated project owner' do
let(:common_id) { [Project.maximum(:id).to_i, Namespace.maximum(:id).to_i].max + 999 }
let!(:group) { create(:group, id: common_id) }
let!(:unrelated_project) { create(:project, id: common_id) }
let(:user) { unrelated_project.owner }
 
it 'creates proper authorizations' do
mapping = map_access_levels(authorizations)
it 'does not have access to group and its projects' do
mapping = map_access_levels(authorizations)
 
expect(mapping[project_parent.id]).to be_nil
expect(mapping[project.id]).to be_nil
expect(mapping[project_child.id]).to be_nil
end
expect(mapping[project_parent.id]).to be_nil
expect(mapping[project.id]).to be_nil
expect(mapping[project_child.id]).to be_nil
end
end
end
......
# frozen_string_literal: true
require 'spec_helper'