Merge branch...

Merge branch '32961-hosted-website-on-gitlab-pages-asking-for-authorization-via-oauth-login' into 'master'

Resolve "Hosted website on Gitlab Pages asking for authorization via OAuth login."

Closes #32961

See merge request gitlab-org/gitlab!18386

db/post_migrate/20191017045817_schedule_fix_gitlab_com_pages_access_level.rb

+# frozen_string_literal: true
+
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+# TODO: remove this migration after execution on gitlab.com https://gitlab.com/gitlab-org/gitlab/issues/34018
+class ScheduleFixGitlabComPagesAccessLevel < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  MIGRATION = 'FixGitlabComPagesAccessLevel'
+  BATCH_SIZE = 20_000
+  BATCH_TIME = 2.minutes
+
+  # Project
+  class Project < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'projects'
+    self.inheritance_column = :_type_disabled
+  end
+
+  disable_ddl_transaction!
+
+  def up
+    return unless ::Gitlab.com?
+
+    queue_background_migration_jobs_by_range_at_intervals(
+      Project,
+      MIGRATION,
+      BATCH_TIME,
+      batch_size: BATCH_SIZE)
+  end
+
+  def down
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_10_16_220135) do
+ActiveRecord::Schema.define(version: 2019_10_17_045817) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"

lib/gitlab/background_migration/fix_gitlab_com_pages_access_level.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    # Fixes https://gitlab.com/gitlab-org/gitlab/issues/32961
+    class FixGitlabComPagesAccessLevel
+      # 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 < ActiveRecord::Base
+        self.table_name = 'namespaces'
+        self.inheritance_column = :_type_disabled
+
+        include Routable
+
+        belongs_to :parent, class_name: "Namespace"
+      end
+
+      # ProjectPagesMetadatum
+      class ProjectPagesMetadatum < ActiveRecord::Base
+        self.primary_key = :project_id
+
+        belongs_to :project, inverse_of: :pages_metadatum
+
+        scope :deployed, -> { where(deployed: true) }
+      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
+
+        has_one :project_feature, inverse_of: :project
+
+        has_one :pages_metadatum, class_name: 'ProjectPagesMetadatum', inverse_of: :project
+
+        scope :with_pages_deployed, -> do
+          joins(:pages_metadatum).merge(ProjectPagesMetadatum.deployed)
+        end
+
+        PRIVATE = 0
+        INTERNAL = 10
+        PUBLIC = 20
+
+        delegate :public_pages?, to: :project_feature
+
+        def public_pages_path
+          File.join(pages_path, 'public')
+        end
+
+        def pages_path
+          File.join(Settings.pages.path, build_full_path)
+        end
+
+        def public?
+          visibility_level == PUBLIC
+        end
+      end
+
+      # ProjectFeature
+      class ProjectFeature < ActiveRecord::Base
+        self.table_name = 'project_features'
+
+        belongs_to :project
+
+        DISABLED = 0
+        PRIVATE  = 10
+        ENABLED  = 20
+        PUBLIC   = 30
+      end
+
+      def perform(start_id, stop_id)
+        logger = Gitlab::BackgroundMigration::Logger.build
+
+        Project.where(id: start_id..stop_id).with_pages_deployed.includes(:project_feature).find_each do |project|
+          config_path = File.join(project.pages_path, 'config.json')
+          ac_is_enabled_in_config = JSON.parse(File.read(config_path))["access_control"]
+
+          next if ac_is_enabled_in_config # we already made site private and don't want to surprise the user
+
+          next if project.project_feature.pages_access_level == ProjectFeature::DISABLED
+
+          new_access_level = project.public? ? ProjectFeature::ENABLED : ProjectFeature::PUBLIC
+
+          next if project.project_feature.pages_access_level == new_access_level
+
+          logger.info(
+            message: "Changing pages access control level",
+            project_id: project.id,
+            access_level_before: project.project_feature.pages_access_level,
+            access_level_after: new_access_level
+          )
+
+          project.project_feature.update_column(:pages_access_level, new_access_level)
+        rescue => e
+          Gitlab::Sentry.track_exception(e, extra: { project_id: project.id })
+        end
+      end
+    end
+  end
+end

spec/migrations/schedule_fix_gitlab_com_pages_access_level_spec.rb

+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20191017045817_schedule_fix_gitlab_com_pages_access_level.rb')
+
+describe ScheduleFixGitlabComPagesAccessLevel, :migration, :sidekiq, schema: 2019_10_16_072826 do
+  using RSpec::Parameterized::TableSyntax
+
+  let(:migration_name) { 'FixGitlabComPagesAccessLevel' }
+
+  ProjectClass = ::Gitlab::BackgroundMigration::FixGitlabComPagesAccessLevel::Project
+  FeatureClass = ::Gitlab::BackgroundMigration::FixGitlabComPagesAccessLevel::ProjectFeature
+
+  let(:namespaces_table) { table(:namespaces) }
+  let(:projects_table) { table(:projects) }
+  let(:features_table) { table(:project_features) }
+  let(:pages_metadata_table) { table(:project_pages_metadata) }
+
+  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
+
+  before do
+    allow(::Gitlab).to receive(:com?).and_return true
+  end
+
+  describe 'scheduling migration' do
+    let!(:first_project) { create_project(ProjectClass::PRIVATE, FeatureClass::PRIVATE, false, false, 'first' ) }
+    let!(:last_project) { create_project(ProjectClass::PRIVATE, FeatureClass::PRIVATE, false, false, 'second' ) }
+
+    subject do
+      Sidekiq::Testing.fake! do
+        migrate!
+      end
+    end
+
+    it 'schedules background migrations' do
+      Timecop.freeze do
+        subject
+
+        expect(migration_name).to be_scheduled_delayed_migration(2.minutes, first_project.id, last_project.id)
+        expect(BackgroundMigrationWorker.jobs.size).to eq(1)
+      end
+    end
+
+    context 'not on gitlab.com' do
+      before do
+        allow(::Gitlab).to receive(:com?).and_return false
+      end
+
+      it 'does not schedule background migrations' do
+        Timecop.freeze do
+          subject
+
+          expect(BackgroundMigrationWorker.jobs.size).to eq(0)
+        end
+      end
+    end
+  end
+
+  where(:visibility_level, :pages_access_level,
+        :pages_deployed, :ac_is_enabled_in_config,
+        :result_pages_access_level) do
+    # Does not change anything if pages are not deployed
+    ProjectClass::PRIVATE  | FeatureClass::DISABLED | false | false | FeatureClass::DISABLED
+    ProjectClass::PRIVATE  | FeatureClass::PRIVATE  | false | false | FeatureClass::PRIVATE
+    ProjectClass::PRIVATE  | FeatureClass::ENABLED  | false | false | FeatureClass::ENABLED
+    ProjectClass::PRIVATE  | FeatureClass::PUBLIC   | false | false | FeatureClass::PUBLIC
+    ProjectClass::INTERNAL | FeatureClass::DISABLED | false | false | FeatureClass::DISABLED
+    ProjectClass::INTERNAL | FeatureClass::PRIVATE  | false | false | FeatureClass::PRIVATE
+    ProjectClass::INTERNAL | FeatureClass::ENABLED  | false | false | FeatureClass::ENABLED
+    ProjectClass::INTERNAL | FeatureClass::PUBLIC   | false | false | FeatureClass::PUBLIC
+    ProjectClass::PUBLIC   | FeatureClass::DISABLED | false | false | FeatureClass::DISABLED
+    ProjectClass::PUBLIC   | FeatureClass::PRIVATE  | false | false | FeatureClass::PRIVATE
+    ProjectClass::PUBLIC   | FeatureClass::ENABLED  | false | false | FeatureClass::ENABLED
+    ProjectClass::PUBLIC   | FeatureClass::PUBLIC   | false | false | FeatureClass::PUBLIC
+
+    # Does not change anything if pages are already private in config.json
+    # many of these cases are invalid and will not occur in production
+    ProjectClass::PRIVATE  | FeatureClass::DISABLED | true | true | FeatureClass::DISABLED
+    ProjectClass::PRIVATE  | FeatureClass::PRIVATE  | true | true | FeatureClass::PRIVATE
+    ProjectClass::PRIVATE  | FeatureClass::ENABLED  | true | true | FeatureClass::ENABLED
+    ProjectClass::PRIVATE  | FeatureClass::PUBLIC   | true | true | FeatureClass::PUBLIC
+    ProjectClass::INTERNAL | FeatureClass::DISABLED | true | true | FeatureClass::DISABLED
+    ProjectClass::INTERNAL | FeatureClass::PRIVATE  | true | true | FeatureClass::PRIVATE
+    ProjectClass::INTERNAL | FeatureClass::ENABLED  | true | true | FeatureClass::ENABLED
+    ProjectClass::INTERNAL | FeatureClass::PUBLIC   | true | true | FeatureClass::PUBLIC
+    ProjectClass::PUBLIC   | FeatureClass::DISABLED | true | true | FeatureClass::DISABLED
+    ProjectClass::PUBLIC   | FeatureClass::PRIVATE  | true | true | FeatureClass::PRIVATE
+    ProjectClass::PUBLIC   | FeatureClass::ENABLED  | true | true | FeatureClass::ENABLED
+    ProjectClass::PUBLIC   | FeatureClass::PUBLIC   | true | true | FeatureClass::PUBLIC
+
+    # when pages are deployed and ac is disabled in config
+    ProjectClass::PRIVATE  | FeatureClass::DISABLED | true | false | FeatureClass::DISABLED
+    ProjectClass::PRIVATE  | FeatureClass::PRIVATE  | true | false | FeatureClass::PUBLIC   # need to update
+    ProjectClass::PRIVATE  | FeatureClass::ENABLED  | true | false | FeatureClass::PUBLIC   # invalid state, need to update
+    ProjectClass::PRIVATE  | FeatureClass::PUBLIC   | true | false | FeatureClass::PUBLIC
+    ProjectClass::INTERNAL | FeatureClass::DISABLED | true | false | FeatureClass::DISABLED
+    ProjectClass::INTERNAL | FeatureClass::PRIVATE  | true | false | FeatureClass::PUBLIC   # need to update
+    ProjectClass::INTERNAL | FeatureClass::ENABLED  | true | false | FeatureClass::PUBLIC   # invalid state, need to update
+    ProjectClass::INTERNAL | FeatureClass::PUBLIC   | true | false | FeatureClass::PUBLIC
+    ProjectClass::PUBLIC   | FeatureClass::DISABLED | true | false | FeatureClass::DISABLED
+    ProjectClass::PUBLIC   | FeatureClass::PRIVATE  | true | false | FeatureClass::ENABLED  # need to update
+    ProjectClass::PUBLIC   | FeatureClass::ENABLED  | true | false | FeatureClass::ENABLED
+    ProjectClass::PUBLIC   | FeatureClass::PUBLIC   | true | false | FeatureClass::ENABLED  # invalid state, need to update
+  end
+
+  with_them do
+    it 'fixes settings' do
+      perform_enqueued_jobs do
+        project = create_project(visibility_level, pages_access_level, pages_deployed, ac_is_enabled_in_config)
+
+        expect(features_table.find_by(project_id: project.id).pages_access_level).to eq(pages_access_level)
+
+        migrate!
+
+        expect(features_table.find_by(project_id: project.id).pages_access_level).to eq(result_pages_access_level)
+      end
+    end
+  end
+
+  def create_project(visibility_level, pages_access_level, pages_deployed, ac_is_enabled_in_config, path = 'project')
+    project = projects_table.create!(path: path, visibility_level: visibility_level,
+                                     namespace_id: subgroup.id)
+
+    pages_metadata_table.create!(project_id: project.id, deployed: pages_deployed)
+
+    if pages_deployed
+      FileUtils.mkdir_p(ProjectClass.find(project.id).public_pages_path)
+
+      # write config.json
+      allow(project).to receive(:public_pages?).and_return(!ac_is_enabled_in_config)
+      allow(project).to receive(:pages_domains).and_return([])
+      allow(project).to receive(:project_id).and_return(project.id)
+      allow(project).to receive(:pages_path).and_return(ProjectClass.find(project.id).pages_path)
+      Projects::UpdatePagesConfigurationService.new(project).execute
+    end
+
+    project.update!(visibility_level: visibility_level)
+    features_table.create!(project_id: project.id, pages_access_level: pages_access_level)
+
+    project
+  end
+end