Commit 6796dcf2 authored by Vladimir Shushlin's avatar Vladimir Shushlin Committed by Nick Thomas

Fix wrong pages access level default

- Set access level in before_validation hook
- Add post migration for updating existing project_features
parent 9c3dfd20
...@@ -86,6 +86,8 @@ class ProjectFeature < ApplicationRecord ...@@ -86,6 +86,8 @@ class ProjectFeature < ApplicationRecord
default_value_for :wiki_access_level, value: ENABLED, allows_nil: false default_value_for :wiki_access_level, value: ENABLED, allows_nil: false
default_value_for :repository_access_level, value: ENABLED, allows_nil: false default_value_for :repository_access_level, value: ENABLED, allows_nil: false
default_value_for(:pages_access_level, allows_nil: false) { |feature| feature.project&.public? ? ENABLED : PRIVATE }
def feature_available?(feature, user) def feature_available?(feature, user)
# This feature might not be behind a feature flag at all, so default to true # This feature might not be behind a feature flag at all, so default to true
return false unless ::Feature.enabled?(feature, user, default_enabled: true) return false unless ::Feature.enabled?(feature, user, default_enabled: true)
......
# frozen_string_literal: true
class FixWrongPagesAccessLevel < ActiveRecord::Migration[5.1]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
MIGRATION = 'FixPagesAccessLevel'
BATCH_SIZE = 20_000
BATCH_TIME = 2.minutes
disable_ddl_transaction!
class ProjectFeature < ActiveRecord::Base
include ::EachBatch
self.table_name = 'project_features'
self.inheritance_column = :_type_disabled
end
def up
queue_background_migration_jobs_by_range_at_intervals(
ProjectFeature,
MIGRATION,
BATCH_TIME,
batch_size: BATCH_SIZE)
end
end
# frozen_string_literal: true
class DropProjectFeaturesPagesAccessLevelDefault < ActiveRecord::Migration[5.1]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
ENABLED_VALUE = 20
def change
change_column_default :project_features, :pages_access_level, from: ENABLED_VALUE, to: nil
end
end
...@@ -10,7 +10,7 @@ ...@@ -10,7 +10,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 2019_07_03_130053) do ActiveRecord::Schema.define(version: 2019_07_15_114644) do
# These are extensions that must be enabled in order to support this database # These are extensions that must be enabled in order to support this database
enable_extension "pg_trgm" enable_extension "pg_trgm"
...@@ -2507,7 +2507,7 @@ ActiveRecord::Schema.define(version: 2019_07_03_130053) do ...@@ -2507,7 +2507,7 @@ ActiveRecord::Schema.define(version: 2019_07_03_130053) do
t.datetime "created_at" t.datetime "created_at"
t.datetime "updated_at" t.datetime "updated_at"
t.integer "repository_access_level", default: 20, null: false t.integer "repository_access_level", default: 20, null: false
t.integer "pages_access_level", default: 20, null: false t.integer "pages_access_level", null: false
t.index ["project_id"], name: "index_project_features_on_project_id", unique: true, using: :btree t.index ["project_id"], name: "index_project_features_on_project_id", unique: true, using: :btree
end end
......
# frozen_string_literal: true
module Gitlab
module BackgroundMigration
# corrects stored pages access level on db depending on project visibility
class FixPagesAccessLevel
# Copy routable here to avoid relying on application logic
module Routable
def build_full_path
if parent && path
parent.build_full_path + '/' + path
else
path
end
end
end
# Namespace
class Namespace < ApplicationRecord
self.table_name = 'namespaces'
self.inheritance_column = :_type_disabled
include Routable
belongs_to :parent, class_name: "Namespace"
end
# Project
class Project < ActiveRecord::Base
self.table_name = 'projects'
self.inheritance_column = :_type_disabled
include Routable
belongs_to :namespace
alias_method :parent, :namespace
alias_attribute :parent_id, :namespace_id
PRIVATE = 0
INTERNAL = 10
PUBLIC = 20
def pages_deployed?
Dir.exist?(public_pages_path)
end
def public_pages_path
File.join(pages_path, 'public')
end
def pages_path
# TODO: when we migrate Pages to work with new storage types, change here to use disk_path
File.join(Settings.pages.path, build_full_path)
end
end
# ProjectFeature
class ProjectFeature < ActiveRecord::Base
include ::EachBatch
self.table_name = 'project_features'
belongs_to :project
PRIVATE = 10
ENABLED = 20
PUBLIC = 30
end
def perform(start_id, stop_id)
fix_public_access_level(start_id, stop_id)
make_internal_projects_public(start_id, stop_id)
fix_private_access_level(start_id, stop_id)
end
private
def access_control_is_enabled
@access_control_is_enabled = Gitlab.config.pages.access_control
end
# Public projects are allowed to have only enabled pages_access_level
# which is equivalent to public
def fix_public_access_level(start_id, stop_id)
project_features(start_id, stop_id, ProjectFeature::PUBLIC, Project::PUBLIC).each_batch do |features|
features.update_all(pages_access_level: ProjectFeature::ENABLED)
end
end
# If access control is disabled and project has pages deployed
# project will become unavailable when access control will become enabled
# we make these projects public to avoid negative surprise to user
def make_internal_projects_public(start_id, stop_id)
return if access_control_is_enabled
project_features(start_id, stop_id, ProjectFeature::ENABLED, Project::INTERNAL).find_each do |project_feature|
next unless project_feature.project.pages_deployed?
project_feature.update(pages_access_level: ProjectFeature::PUBLIC)
end
end
# Private projects are not allowed to have enabled access level, only `private` and `public`
# If access control is enabled, these projects currently behave as if the have `private` pages_access_level
# if access control is disabled, these projects currently behave as if the have `public` pages_access_level
# so we preserve this behaviour for projects with pages already deployed
# for project without pages we always set `private` access_level
def fix_private_access_level(start_id, stop_id)
project_features(start_id, stop_id, ProjectFeature::ENABLED, Project::PRIVATE).find_each do |project_feature|
if access_control_is_enabled
project_feature.update!(pages_access_level: ProjectFeature::PRIVATE)
else
fixed_access_level = project_feature.project.pages_deployed? ? ProjectFeature::PUBLIC : ProjectFeature::PRIVATE
project_feature.update!(pages_access_level: fixed_access_level)
end
end
end
def project_features(start_id, stop_id, pages_access_level, project_visibility_level)
ProjectFeature.where(id: start_id..stop_id).joins(:project)
.where(pages_access_level: pages_access_level)
.where(projects: { visibility_level: project_visibility_level })
end
end
end
end
require 'spec_helper'
require Rails.root.join('db', 'post_migrate', '20190703185326_fix_wrong_pages_access_level.rb')
describe FixWrongPagesAccessLevel, :migration, :sidekiq, schema: 20190628185004 do
using RSpec::Parameterized::TableSyntax
let(:migration_class) { described_class::MIGRATION }
let(:migration_name) { migration_class.to_s.demodulize }
project_class = ::Gitlab::BackgroundMigration::FixPagesAccessLevel::Project
feature_class = ::Gitlab::BackgroundMigration::FixPagesAccessLevel::ProjectFeature
let(:namespaces_table) { table(:namespaces) }
let(:projects_table) { table(:projects) }
let(:features_table) { table(:project_features) }
let(:subgroup) do
root_group = namespaces_table.create(path: "group", name: "group")
namespaces_table.create!(path: "subgroup", name: "group", parent_id: root_group.id)
end
def create_project_feature(path, project_visibility, pages_access_level)
project = projects_table.create!(path: path, visibility_level: project_visibility,
namespace_id: subgroup.id)
features_table.create!(project_id: project.id, pages_access_level: pages_access_level)
end
it 'correctly schedules background migrations' do
Sidekiq::Testing.fake! do
Timecop.freeze do
first_id = create_project_feature("project1", project_class::PRIVATE, feature_class::PRIVATE).id
last_id = create_project_feature("project2", project_class::PRIVATE, feature_class::PUBLIC).id
migrate!
expect(migration_name).to be_scheduled_delayed_migration(2.minutes, first_id, last_id)
expect(BackgroundMigrationWorker.jobs.size).to eq(1)
end
end
end
def expect_migration
expect do
perform_enqueued_jobs do
migrate!
end
end
end
where(:project_visibility, :pages_access_level, :access_control_is_enabled,
:pages_deployed, :resulting_pages_access_level) do
# update settings for public projects regardless of access_control being enabled
project_class::PUBLIC | feature_class::PUBLIC | true | true | feature_class::ENABLED
project_class::PUBLIC | feature_class::PUBLIC | false | true | feature_class::ENABLED
# don't update public level for private and internal projects
project_class::PRIVATE | feature_class::PUBLIC | true | true | feature_class::PUBLIC
project_class::INTERNAL | feature_class::PUBLIC | true | true | feature_class::PUBLIC
# if access control is disabled but pages are deployed we make them public
project_class::INTERNAL | feature_class::ENABLED | false | true | feature_class::PUBLIC
# don't change anything if one of the conditions is not satisfied
project_class::INTERNAL | feature_class::ENABLED | true | true | feature_class::ENABLED
project_class::INTERNAL | feature_class::ENABLED | true | false | feature_class::ENABLED
# private projects
# if access control is enabled update pages_access_level to private regardless of deployment
project_class::PRIVATE | feature_class::ENABLED | true | true | feature_class::PRIVATE
project_class::PRIVATE | feature_class::ENABLED | true | false | feature_class::PRIVATE
# if access control is disabled and pages are deployed update pages_access_level to public
project_class::PRIVATE | feature_class::ENABLED | false | true | feature_class::PUBLIC
# if access control is disabled but pages aren't deployed update pages_access_level to private
project_class::PRIVATE | feature_class::ENABLED | false | false | feature_class::PRIVATE
end
with_them do
let!(:project_feature) do
create_project_feature("projectpath", project_visibility, pages_access_level)
end
before do
tested_path = File.join(Settings.pages.path, "group/subgroup/projectpath", "public")
allow(Dir).to receive(:exist?).with(tested_path).and_return(pages_deployed)
stub_pages_setting(access_control: access_control_is_enabled)
end
it "sets proper pages_access_level" do
expect(project_feature.reload.pages_access_level).to eq(pages_access_level)
perform_enqueued_jobs do
migrate!
end
expect(project_feature.reload.pages_access_level).to eq(resulting_pages_access_level)
end
end
end
...@@ -150,4 +150,32 @@ describe ProjectFeature do ...@@ -150,4 +150,32 @@ describe ProjectFeature do
end end
end end
end end
describe 'default pages access level' do
subject { project.project_feature.pages_access_level }
before do
# project factory overrides all values in project_feature after creation
project.project_feature.destroy!
project.build_project_feature.save!
end
context 'when new project is private' do
let(:project) { create(:project, :private) }
it { is_expected.to eq(ProjectFeature::PRIVATE) }
end
context 'when new project is internal' do
let(:project) { create(:project, :internal) }
it { is_expected.to eq(ProjectFeature::PRIVATE) }
end
context 'when new project is public' do
let(:project) { create(:project, :public) }
it { is_expected.to eq(ProjectFeature::ENABLED) }
end
end
end end
...@@ -65,6 +65,10 @@ module StubConfiguration ...@@ -65,6 +65,10 @@ module StubConfiguration
allow(Gitlab.config.artifacts).to receive_messages(to_settings(messages)) allow(Gitlab.config.artifacts).to receive_messages(to_settings(messages))
end end
def stub_pages_setting(messages)
allow(Gitlab.config.pages).to receive_messages(to_settings(messages))
end
def stub_storage_settings(messages) def stub_storage_settings(messages)
messages.deep_stringify_keys! messages.deep_stringify_keys!
......
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