summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2021-05-31 11:41:25 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2021-05-31 11:41:25 +0000
commit6e4e4023b46c786a99e1cfe8832fa5eff2728e0d (patch)
tree9ba106ae15fffc24fc4b5ad71c4d4d70ad1e5c0c
parent63a19a71aedcafe0148912c536a36768ed126533 (diff)
downloadgitlab-ce-6e4e4023b46c786a99e1cfe8832fa5eff2728e0d.tar.gz
Add latest changes from gitlab-org/security/gitlab@13-12-stable-ee
-rw-r--r--app/controllers/jira_connect/app_descriptor_controller.rb1
-rw-r--r--app/helpers/markup_helper.rb1
-rw-r--r--db/migrate/20210519154058_schedule_update_users_where_two_factor_auth_required_from_group.rb34
-rw-r--r--db/schema_migrations/202105191540581
-rw-r--r--db/structure.sql2
-rw-r--r--lib/banzai/filter/truncate_source_filter.rb21
-rw-r--r--lib/gitlab/background_migration/update_users_where_two_factor_auth_required_from_group.rb129
-rw-r--r--spec/helpers/markup_helper_spec.rb7
-rw-r--r--spec/lib/banzai/filter/truncate_source_filter_spec.rb70
-rw-r--r--spec/lib/gitlab/background_migration/update_users_where_two_factor_auth_required_from_group_spec.rb84
-rw-r--r--spec/migrations/schedule_update_users_where_two_factor_auth_required_from_group_spec.rb29
11 files changed, 364 insertions, 15 deletions
diff --git a/app/controllers/jira_connect/app_descriptor_controller.rb b/app/controllers/jira_connect/app_descriptor_controller.rb
index 137f830e40b..fee8b43aa6b 100644
--- a/app/controllers/jira_connect/app_descriptor_controller.rb
+++ b/app/controllers/jira_connect/app_descriptor_controller.rb
@@ -31,6 +31,7 @@ class JiraConnect::AppDescriptorController < JiraConnect::ApplicationController
scopes: %w(READ WRITE DELETE),
apiVersion: 1,
apiMigrations: {
+ 'context-qsh': true,
gdpr: true
}
}
diff --git a/app/helpers/markup_helper.rb b/app/helpers/markup_helper.rb
index 05a55a09271..7525481047e 100644
--- a/app/helpers/markup_helper.rb
+++ b/app/helpers/markup_helper.rb
@@ -118,6 +118,7 @@ module MarkupHelper
def markup(file_name, text, context = {})
context[:project] ||= @project
+ context[:text_source] ||= :blob
html = context.delete(:rendered) || markup_unsafe(file_name, text, context)
prepare_for_rendering(html, context)
end
diff --git a/db/migrate/20210519154058_schedule_update_users_where_two_factor_auth_required_from_group.rb b/db/migrate/20210519154058_schedule_update_users_where_two_factor_auth_required_from_group.rb
new file mode 100644
index 00000000000..2da84301a72
--- /dev/null
+++ b/db/migrate/20210519154058_schedule_update_users_where_two_factor_auth_required_from_group.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+class ScheduleUpdateUsersWhereTwoFactorAuthRequiredFromGroup < ActiveRecord::Migration[6.0]
+ include Gitlab::Database::MigrationHelpers
+
+ MIGRATION = 'UpdateUsersWhereTwoFactorAuthRequiredFromGroup'
+ DELAY_INTERVAL = 2.minutes
+ BATCH_SIZE = 10_000
+ INDEX_NAME = 'index_users_require_two_factor_authentication_from_group_false'
+
+ disable_ddl_transaction!
+
+ class User < ActiveRecord::Base
+ include EachBatch
+
+ self.table_name = 'users'
+ end
+
+ def up
+ add_concurrent_index :users,
+ :require_two_factor_authentication_from_group,
+ where: 'require_two_factor_authentication_from_group = FALSE',
+ name: INDEX_NAME
+
+ relation = User.where(require_two_factor_authentication_from_group: false)
+
+ queue_background_migration_jobs_by_range_at_intervals(
+ relation, MIGRATION, DELAY_INTERVAL, batch_size: BATCH_SIZE)
+ end
+
+ def down
+ remove_concurrent_index_by_name :users, INDEX_NAME
+ end
+end
diff --git a/db/schema_migrations/20210519154058 b/db/schema_migrations/20210519154058
new file mode 100644
index 00000000000..9bd277e92db
--- /dev/null
+++ b/db/schema_migrations/20210519154058
@@ -0,0 +1 @@
+bdd82fc5cb2bbb322125c153c741002725853e23cd0ae0edbfd80563a4a87f2f \ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index 5d33a61e942..77b7557b14f 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -24646,6 +24646,8 @@ CREATE INDEX index_users_ops_dashboard_projects_on_project_id ON users_ops_dashb
CREATE UNIQUE INDEX index_users_ops_dashboard_projects_on_user_id_and_project_id ON users_ops_dashboard_projects USING btree (user_id, project_id);
+CREATE INDEX index_users_require_two_factor_authentication_from_group_false ON users USING btree (require_two_factor_authentication_from_group) WHERE (require_two_factor_authentication_from_group = false);
+
CREATE INDEX index_users_security_dashboard_projects_on_user_id ON users_security_dashboard_projects USING btree (user_id);
CREATE INDEX index_users_star_projects_on_project_id ON users_star_projects USING btree (project_id);
diff --git a/lib/banzai/filter/truncate_source_filter.rb b/lib/banzai/filter/truncate_source_filter.rb
index 44f88b253d9..a21d4a44295 100644
--- a/lib/banzai/filter/truncate_source_filter.rb
+++ b/lib/banzai/filter/truncate_source_filter.rb
@@ -3,12 +3,29 @@
module Banzai
module Filter
class TruncateSourceFilter < HTML::Pipeline::TextFilter
+ CHARACTER_COUNT_LIMIT = 1.megabyte
+ USER_MSG_LIMIT = 10_000
+
def call
- return text unless context.key?(:limit)
+ # don't truncate if it's a :blob and no limit is set
+ return text if context[:text_source] == :blob && !context.key?(:limit)
+
+ limit = context[:limit] || CHARACTER_COUNT_LIMIT
+
+ # no sense in allowing `truncate_bytes` to duplicate a large
+ # string unless it's too big
+ return text if text.bytesize <= limit
# Use three dots instead of the ellipsis Unicode character because
# some clients show the raw Unicode value in the merge commit.
- text.truncate_bytes(context[:limit], omission: '...')
+ trunc = text.truncate_bytes(limit, omission: '...')
+
+ # allows us to indicate to the user that what they see is a truncated copy
+ if limit > USER_MSG_LIMIT
+ trunc.prepend("_The text is longer than #{limit} characters and has been visually truncated._\n\n")
+ end
+
+ trunc
end
end
end
diff --git a/lib/gitlab/background_migration/update_users_where_two_factor_auth_required_from_group.rb b/lib/gitlab/background_migration/update_users_where_two_factor_auth_required_from_group.rb
new file mode 100644
index 00000000000..f5ba9e63333
--- /dev/null
+++ b/lib/gitlab/background_migration/update_users_where_two_factor_auth_required_from_group.rb
@@ -0,0 +1,129 @@
+# frozen_string_literal: true
+# rubocop:disable Style/Documentation
+
+module Gitlab
+ module BackgroundMigration
+ class UpdateUsersWhereTwoFactorAuthRequiredFromGroup # rubocop:disable Metrics/ClassLength
+ def perform(start_id, stop_id)
+ ActiveRecord::Base.connection.execute <<~SQL
+ UPDATE
+ users
+ SET
+ require_two_factor_authentication_from_group = TRUE
+ WHERE
+ users.id BETWEEN #{start_id}
+ AND #{stop_id}
+ AND users.require_two_factor_authentication_from_group = FALSE
+ AND users.id IN (
+ SELECT
+ DISTINCT users_groups_query.user_id
+ FROM
+ (
+ SELECT
+ users.id AS user_id,
+ members.source_id AS group_ids
+ FROM
+ users
+ LEFT JOIN members ON members.source_type = 'Namespace'
+ AND members.requested_at IS NULL
+ AND members.user_id = users.id
+ AND members.type = 'GroupMember'
+ WHERE
+ users.require_two_factor_authentication_from_group = FALSE
+ AND users.id BETWEEN #{start_id}
+ AND #{stop_id}) AS users_groups_query
+ INNER JOIN LATERAL (
+ WITH RECURSIVE "base_and_ancestors" AS (
+ (
+ SELECT
+ "namespaces"."type",
+ "namespaces"."id",
+ "namespaces"."parent_id",
+ "namespaces"."require_two_factor_authentication"
+ FROM
+ "namespaces"
+ WHERE
+ "namespaces"."type" = 'Group'
+ AND "namespaces"."id" = users_groups_query.group_ids
+ )
+ UNION
+ (
+ SELECT
+ "namespaces"."type",
+ "namespaces"."id",
+ "namespaces"."parent_id",
+ "namespaces"."require_two_factor_authentication"
+ FROM
+ "namespaces",
+ "base_and_ancestors"
+ WHERE
+ "namespaces"."type" = 'Group'
+ AND "namespaces"."id" = "base_and_ancestors"."parent_id"
+ )
+ ),
+ "base_and_descendants" AS (
+ (
+ SELECT
+ "namespaces"."type",
+ "namespaces"."id",
+ "namespaces"."parent_id",
+ "namespaces"."require_two_factor_authentication"
+ FROM
+ "namespaces"
+ WHERE
+ "namespaces"."type" = 'Group'
+ AND "namespaces"."id" = users_groups_query.group_ids
+ )
+ UNION
+ (
+ SELECT
+ "namespaces"."type",
+ "namespaces"."id",
+ "namespaces"."parent_id",
+ "namespaces"."require_two_factor_authentication"
+ FROM
+ "namespaces",
+ "base_and_descendants"
+ WHERE
+ "namespaces"."type" = 'Group'
+ AND "namespaces"."parent_id" = "base_and_descendants"."id"
+ )
+ )
+ SELECT
+ "namespaces".*
+ FROM
+ (
+ (
+ SELECT
+ "namespaces"."type",
+ "namespaces"."id",
+ "namespaces"."parent_id",
+ "namespaces"."require_two_factor_authentication"
+ FROM
+ "base_and_ancestors" AS "namespaces"
+ WHERE
+ "namespaces"."type" = 'Group'
+ )
+ UNION
+ (
+ SELECT
+ "namespaces"."type",
+ "namespaces"."id",
+ "namespaces"."parent_id",
+ "namespaces"."require_two_factor_authentication"
+ FROM
+ "base_and_descendants" AS "namespaces"
+ WHERE
+ "namespaces"."type" = 'Group'
+ )
+ ) namespaces
+ WHERE
+ "namespaces"."type" = 'Group'
+ AND "namespaces"."require_two_factor_authentication" = TRUE
+ ) AS hierarchy_tree ON TRUE
+ );
+ SQL
+ end
+ end
+ end
+end
diff --git a/spec/helpers/markup_helper_spec.rb b/spec/helpers/markup_helper_spec.rb
index 00a59f037e0..e946857ac77 100644
--- a/spec/helpers/markup_helper_spec.rb
+++ b/spec/helpers/markup_helper_spec.rb
@@ -418,6 +418,13 @@ FooBar
describe '#markup' do
let(:content) { 'Noël' }
+ it 'sets the :text_source to :blob in the context' do
+ context = {}
+ helper.markup('foo.md', content, context)
+
+ expect(context).to include(text_source: :blob)
+ end
+
it 'preserves encoding' do
expect(content.encoding.name).to eq('UTF-8')
expect(helper.markup('foo.rst', content).encoding.name).to eq('UTF-8')
diff --git a/spec/lib/banzai/filter/truncate_source_filter_spec.rb b/spec/lib/banzai/filter/truncate_source_filter_spec.rb
index d5eb8b738b1..8970aa1d382 100644
--- a/spec/lib/banzai/filter/truncate_source_filter_spec.rb
+++ b/spec/lib/banzai/filter/truncate_source_filter_spec.rb
@@ -8,24 +8,68 @@ RSpec.describe Banzai::Filter::TruncateSourceFilter do
let(:short_text) { 'foo' * 10 }
let(:long_text) { ([short_text] * 10).join(' ') }
- it 'does nothing when limit is unspecified' do
- output = filter(long_text)
-
- expect(output).to eq(long_text)
+ before do
+ stub_const("#{described_class}::CHARACTER_COUNT_LIMIT", 50)
+ stub_const("#{described_class}::USER_MSG_LIMIT", 20)
end
- it 'does nothing to a short-enough text' do
- output = filter(short_text, limit: short_text.bytesize)
+ context 'when markdown belongs to a blob' do
+ it 'does nothing when limit is unspecified' do
+ output = filter(long_text, text_source: :blob)
+
+ expect(output).to eq(long_text)
+ end
+
+ it 'truncates normally when limit specified' do
+ truncated = 'foofoof...'
+
+ output = filter(long_text, text_source: :blob, limit: 10)
- expect(output).to eq(short_text)
+ expect(output).to eq(truncated)
+ end
end
- it 'truncates UTF-8 text by bytes, on a character boundary' do
- utf8_text = '日本語の文字が大きい'
- truncated = '日...'
+ context 'when markdown belongs to a field (non-blob)' do
+ it 'does nothing when limit is greater' do
+ output = filter(long_text, limit: 1.megabyte)
+
+ expect(output).to eq(long_text)
+ end
+
+ it 'truncates to the default when limit is unspecified' do
+ stub_const("#{described_class}::USER_MSG_LIMIT", 200)
+ truncated = 'foofoofoofoofoofoofoofoofoofoo foofoofoofoofoof...'
+
+ output = filter(long_text)
+
+ expect(output).to eq(truncated)
+ end
+
+ it 'prepends the user message' do
+ truncated = <<~TEXT
+ _The text is longer than 50 characters and has been visually truncated._
+
+ foofoofoofoofoofoofoofoofoofoo foofoofoofoofoof...
+ TEXT
+
+ output = filter(long_text)
+
+ expect(output).to eq(truncated.strip)
+ end
+
+ it 'does nothing to a short-enough text' do
+ output = filter(short_text, limit: short_text.bytesize)
+
+ expect(output).to eq(short_text)
+ end
+
+ it 'truncates UTF-8 text by bytes, on a character boundary' do
+ utf8_text = '日本語の文字が大きい'
+ truncated = '日...'
- expect(filter(utf8_text, limit: truncated.bytesize)).to eq(truncated)
- expect(filter(utf8_text, limit: utf8_text.bytesize)).to eq(utf8_text)
- expect(filter(utf8_text, limit: utf8_text.mb_chars.size)).not_to eq(utf8_text)
+ expect(filter(utf8_text, limit: truncated.bytesize)).to eq(truncated)
+ expect(filter(utf8_text, limit: utf8_text.bytesize)).to eq(utf8_text)
+ expect(filter(utf8_text, limit: utf8_text.mb_chars.size)).not_to eq(utf8_text)
+ end
end
end
diff --git a/spec/lib/gitlab/background_migration/update_users_where_two_factor_auth_required_from_group_spec.rb b/spec/lib/gitlab/background_migration/update_users_where_two_factor_auth_required_from_group_spec.rb
new file mode 100644
index 00000000000..e14328b6150
--- /dev/null
+++ b/spec/lib/gitlab/background_migration/update_users_where_two_factor_auth_required_from_group_spec.rb
@@ -0,0 +1,84 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::BackgroundMigration::UpdateUsersWhereTwoFactorAuthRequiredFromGroup, :migration, schema: 20210519154058 do
+ include MigrationHelpers::NamespacesHelpers
+
+ let(:group_with_2fa_parent) { create_namespace('parent', Gitlab::VisibilityLevel::PRIVATE, require_two_factor_authentication: true) }
+ let(:group_with_2fa_child) { create_namespace('child', Gitlab::VisibilityLevel::PRIVATE, parent_id: group_with_2fa_parent.id) }
+ let(:members_table) { table(:members) }
+ let(:users_table) { table(:users) }
+
+ subject { described_class.new }
+
+ describe '#perform' do
+ context 'with group members' do
+ let(:user_1) { create_user('user@example.com') }
+ let!(:member) { create_group_member(user_1, group_with_2fa_parent) }
+ let!(:user_without_group) { create_user('user_without@example.com') }
+ let(:user_other) { create_user('user_other@example.com') }
+ let!(:member_other) { create_group_member(user_other, group_with_2fa_parent) }
+
+ it 'updates user when user should be required to establish two factor authentication' do
+ subject.perform(user_1.id, user_without_group.id)
+
+ expect(user_1.reload.require_two_factor_authentication_from_group).to eq(true)
+ end
+
+ it 'does not update user who is not in current batch' do
+ subject.perform(user_1.id, user_without_group.id)
+
+ expect(user_other.reload.require_two_factor_authentication_from_group).to eq(false)
+ end
+
+ it 'updates all users in current batch' do
+ subject.perform(user_1.id, user_other.id)
+
+ expect(user_other.reload.require_two_factor_authentication_from_group).to eq(true)
+ end
+
+ it 'updates user when user is member of group in which parent group requires two factor authentication' do
+ member.destroy!
+
+ subgroup = create_namespace('subgroup', Gitlab::VisibilityLevel::PRIVATE, require_two_factor_authentication: false, parent_id: group_with_2fa_child.id)
+ create_group_member(user_1, subgroup)
+
+ subject.perform(user_1.id, user_other.id)
+
+ expect(user_1.reload.require_two_factor_authentication_from_group).to eq(true)
+ end
+
+ it 'updates user when user is member of a group and the subgroup requires two factor authentication' do
+ member.destroy!
+
+ parent = create_namespace('other_parent', Gitlab::VisibilityLevel::PRIVATE, require_two_factor_authentication: false)
+ create_namespace('other_subgroup', Gitlab::VisibilityLevel::PRIVATE, require_two_factor_authentication: true, parent_id: parent.id)
+ create_group_member(user_1, parent)
+
+ subject.perform(user_1.id, user_other.id)
+
+ expect(user_1.reload.require_two_factor_authentication_from_group).to eq(true)
+ end
+
+ it 'does not update user when not a member of a group that requires two factor authentication' do
+ member_other.destroy!
+
+ other_group = create_namespace('other_group', Gitlab::VisibilityLevel::PRIVATE, require_two_factor_authentication: false)
+ create_group_member(user_other, other_group)
+
+ subject.perform(user_1.id, user_other.id)
+
+ expect(user_other.reload.require_two_factor_authentication_from_group).to eq(false)
+ end
+ end
+ end
+
+ def create_user(email, require_2fa: false)
+ users_table.create!(email: email, projects_limit: 10, require_two_factor_authentication_from_group: require_2fa)
+ end
+
+ def create_group_member(user, group)
+ members_table.create!(user_id: user.id, source_id: group.id, access_level: GroupMember::MAINTAINER, source_type: "Namespace", type: "GroupMember", notification_level: 3)
+ end
+end
diff --git a/spec/migrations/schedule_update_users_where_two_factor_auth_required_from_group_spec.rb b/spec/migrations/schedule_update_users_where_two_factor_auth_required_from_group_spec.rb
new file mode 100644
index 00000000000..cec141cacc9
--- /dev/null
+++ b/spec/migrations/schedule_update_users_where_two_factor_auth_required_from_group_spec.rb
@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'migrate', '20210519154058_schedule_update_users_where_two_factor_auth_required_from_group.rb')
+
+RSpec.describe ScheduleUpdateUsersWhereTwoFactorAuthRequiredFromGroup do
+ let(:users) { table(:users) }
+ let!(:user_1) { users.create!(require_two_factor_authentication_from_group: false, name: "user1", email: "user1@example.com", projects_limit: 1) }
+ let!(:user_2) { users.create!(require_two_factor_authentication_from_group: true, name: "user2", email: "user2@example.com", projects_limit: 1) }
+ let!(:user_3) { users.create!(require_two_factor_authentication_from_group: false, name: "user3", email: "user3@example.com", projects_limit: 1) }
+
+ before do
+ stub_const("#{described_class.name}::BATCH_SIZE", 1)
+ end
+
+ it 'schedules jobs for users that do not require two factor authentication' do
+ Sidekiq::Testing.fake! do
+ freeze_time do
+ migrate!
+
+ expect(described_class::MIGRATION).to be_scheduled_delayed_migration(
+ 2.minutes, user_1.id, user_1.id)
+ expect(described_class::MIGRATION).to be_scheduled_delayed_migration(
+ 4.minutes, user_3.id, user_3.id)
+ expect(BackgroundMigrationWorker.jobs.size).to eq(2)
+ end
+ end
+ end
+end