Commit 40d99004 authored by Yorick Peterse's avatar Yorick Peterse

Merge branch 'security-import-project-visibility' into 'master'

[master] Fix Imported Project Retains Prior Visibility Setting

See merge request gitlab/gitlabhq!2734
parents 4d2a666c 19f9666a
---
title: Restrict project import visibility based on its group
merge_request:
author:
type: security
# frozen_string_literal: true
class UpdateProjectImportVisibilityLevel < ActiveRecord::Migration[5.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
BATCH_SIZE = 100
PRIVATE = 0
INTERNAL = 10
disable_ddl_transaction!
class Namespace < ActiveRecord::Base
self.table_name = 'namespaces'
end
class Project < ActiveRecord::Base
include EachBatch
belongs_to :namespace
IMPORT_TYPE = 'gitlab_project'
scope :with_group_visibility, ->(visibility) do
joins(:namespace)
.where(namespaces: { type: 'Group', visibility_level: visibility })
.where(import_type: IMPORT_TYPE)
.where('projects.visibility_level > namespaces.visibility_level')
end
self.table_name = 'projects'
end
def up
# Update project's visibility to be the same as the group
# if it is more restrictive than `PUBLIC`.
update_projects_visibility(PRIVATE)
update_projects_visibility(INTERNAL)
end
def down
# no-op: unrecoverable data migration
end
private
def update_projects_visibility(visibility)
say_with_time("Updating project visibility to #{visibility} on #{Project::IMPORT_TYPE} imports.") do
Project.with_group_visibility(visibility).select(:id).each_batch(of: BATCH_SIZE) do |batch, _index|
batch_sql = Gitlab::Database.mysql? ? batch.pluck(:id).join(', ') : batch.select(:id).to_sql
say("Updating #{batch.size} items.", true)
execute("UPDATE projects SET visibility_level = '#{visibility}' WHERE id IN (#{batch_sql})")
end
end
end
end
...@@ -107,7 +107,7 @@ module Gitlab ...@@ -107,7 +107,7 @@ module Gitlab
def project_params def project_params
@project_params ||= begin @project_params ||= begin
attrs = json_params.merge(override_params) attrs = json_params.merge(override_params).merge(visibility_level)
# Cleaning all imported and overridden params # Cleaning all imported and overridden params
Gitlab::ImportExport::AttributeCleaner.clean(relation_hash: attrs, Gitlab::ImportExport::AttributeCleaner.clean(relation_hash: attrs,
...@@ -127,6 +127,13 @@ module Gitlab ...@@ -127,6 +127,13 @@ module Gitlab
end end
end end
def visibility_level
level = override_params['visibility_level'] || json_params['visibility_level'] || @project.visibility_level
level = @project.group.visibility_level if @project.group && level > @project.group.visibility_level
{ 'visibility_level' => level }
end
# Given a relation hash containing one or more models and its relationships, # Given a relation hash containing one or more models and its relationships,
# loops through each model and each object from a model type and # loops through each model and each object from a model type and
# and assigns its correspondent attributes hash from +tree_hash+ # and assigns its correspondent attributes hash from +tree_hash+
......
...@@ -273,6 +273,11 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do ...@@ -273,6 +273,11 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
it 'has group milestone' do it 'has group milestone' do
expect(project.group.milestones.size).to eq(results.fetch(:milestones, 0)) expect(project.group.milestones.size).to eq(results.fetch(:milestones, 0))
end end
it 'has the correct visibility level' do
# INTERNAL in the `project.json`, group's is PRIVATE
expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE)
end
end end
context 'Light JSON' do context 'Light JSON' do
...@@ -347,7 +352,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do ...@@ -347,7 +352,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
:issues_disabled, :issues_disabled,
name: 'project', name: 'project',
path: 'project', path: 'project',
group: create(:group)) group: create(:group, visibility_level: Gitlab::VisibilityLevel::PRIVATE))
end end
before do before do
...@@ -434,4 +439,58 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do ...@@ -434,4 +439,58 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
end end
end end
end end
describe '#restored_project' do
let(:project) { create(:project) }
let(:shared) { project.import_export_shared }
let(:tree_hash) { { 'visibility_level' => visibility } }
let(:restorer) { described_class.new(user: nil, shared: shared, project: project) }
before do
restorer.instance_variable_set(:@tree_hash, tree_hash)
end
context 'no group visibility' do
let(:visibility) { Gitlab::VisibilityLevel::PRIVATE }
it 'uses the project visibility' do
expect(restorer.restored_project.visibility_level).to eq(visibility)
end
end
context 'with group visibility' do
before do
group = create(:group, visibility_level: group_visibility)
project.update(group: group)
end
context 'private group visibility' do
let(:group_visibility) { Gitlab::VisibilityLevel::PRIVATE }
let(:visibility) { Gitlab::VisibilityLevel::PUBLIC }
it 'uses the group visibility' do
expect(restorer.restored_project.visibility_level).to eq(group_visibility)
end
end
context 'public group visibility' do
let(:group_visibility) { Gitlab::VisibilityLevel::PUBLIC }
let(:visibility) { Gitlab::VisibilityLevel::PRIVATE }
it 'uses the project visibility' do
expect(restorer.restored_project.visibility_level).to eq(visibility)
end
end
context 'internal group visibility' do
let(:group_visibility) { Gitlab::VisibilityLevel::INTERNAL }
let(:visibility) { Gitlab::VisibilityLevel::PUBLIC }
it 'uses the group visibility' do
expect(restorer.restored_project.visibility_level).to eq(group_visibility)
end
end
end
end
end end
# frozen_string_literal: true
require 'spec_helper'
require Rails.root.join('db', 'post_migrate', '20181219130552_update_project_import_visibility_level.rb')
describe UpdateProjectImportVisibilityLevel, :migration do
let(:namespaces) { table(:namespaces) }
let(:projects) { table(:projects) }
let(:project) { projects.find_by_name(name) }
before do
stub_const("#{described_class}::BATCH_SIZE", 1)
end
context 'private visibility level' do
let(:name) { 'private-public' }
it 'updates the project visibility' do
create_namespace(name, Gitlab::VisibilityLevel::PRIVATE)
create_project(name, Gitlab::VisibilityLevel::PUBLIC)
expect { migrate! }.to change { project.reload.visibility_level }.to(Gitlab::VisibilityLevel::PRIVATE)
end
end
context 'internal visibility level' do
let(:name) { 'internal-public' }
it 'updates the project visibility' do
create_namespace(name, Gitlab::VisibilityLevel::INTERNAL)
create_project(name, Gitlab::VisibilityLevel::PUBLIC)
expect { migrate! }.to change { project.reload.visibility_level }.to(Gitlab::VisibilityLevel::INTERNAL)
end
end
context 'public visibility level' do
let(:name) { 'public-public' }
it 'does not update the project visibility' do
create_namespace(name, Gitlab::VisibilityLevel::PUBLIC)
create_project(name, Gitlab::VisibilityLevel::PUBLIC)
expect { migrate! }.not_to change { project.reload.visibility_level }
end
end
context 'private project visibility level' do
let(:name) { 'public-private' }
it 'does not update the project visibility' do
create_namespace(name, Gitlab::VisibilityLevel::PUBLIC)
create_project(name, Gitlab::VisibilityLevel::PRIVATE)
expect { migrate! }.not_to change { project.reload.visibility_level }
end
end
context 'no namespace' do
let(:name) { 'no-namespace' }
it 'does not update the project visibility' do
create_namespace(name, Gitlab::VisibilityLevel::PRIVATE, type: nil)
create_project(name, Gitlab::VisibilityLevel::PUBLIC)
expect { migrate! }.not_to change { project.reload.visibility_level }
end
end
def create_namespace(name, visibility, options = {})
namespaces.create({
name: name,
path: name,
type: 'Group',
visibility_level: visibility
}.merge(options))
end
def create_project(name, visibility)
projects.create!(namespace_id: namespaces.find_by_name(name).id,
name: name,
path: name,
import_type: 'gitlab_project',
visibility_level: visibility)
end
end
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