Add latest changes from gitlab-org/security/gitlab@13-11-stable-ee

12 files changed, 346 insertions, 55 deletions

diff --git a/app/assets/javascripts/notebook/cells/markdown.vue b/app/assets/javascripts/notebook/cells/markdown.vue
index c09db6851e5..97b0557cbe2 100644
--- a/app/assets/javascripts/notebook/cells/markdown.vue
+++ b/app/assets/javascripts/notebook/cells/markdown.vue
@@ -160,6 +160,7 @@ export default {
           'var',
         ],
         ALLOWED_ATTR: ['class', 'style', 'href', 'src'],
+        ALLOW_DATA_ATTR: false,
       });
     },
   },
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/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 db463645810..e09851aa108 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -24170,6 +24170,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/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/lib/gitlab/x509/signature.rb b/lib/gitlab/x509/signature.rb
index edff1540cb3..72bbf3d6e8b 100644
--- a/lib/gitlab/x509/signature.rb
+++ b/lib/gitlab/x509/signature.rb
@@ -23,7 +23,7 @@ module Gitlab
       end
 
       def user
-        User.find_by_any_email(@email)
+        strong_memoize(:user) { User.find_by_any_email(@email) }
       end
 
       def verified_signature
@@ -31,9 +31,13 @@ module Gitlab
       end
 
       def verification_status
-        return :unverified if x509_certificate.nil? || x509_certificate.revoked?
+        return :unverified if
+          x509_certificate.nil? ||
+          x509_certificate.revoked? ||
+          !verified_signature ||
+          user.nil?
 
-        if verified_signature && certificate_email == @email
+        if user.verified_emails.include?(@email) && certificate_email == @email
           :verified
         else
           :unverified
diff --git a/spec/frontend/notebook/cells/markdown_spec.js b/spec/frontend/notebook/cells/markdown_spec.js
index 219d74595bd..5c6371530d0 100644
--- a/spec/frontend/notebook/cells/markdown_spec.js
+++ b/spec/frontend/notebook/cells/markdown_spec.js
@@ -39,7 +39,7 @@ describe('Markdown component', () => {
     expect(vm.$el.querySelector('.markdown h1')).not.toBeNull();
   });
 
-  it('sanitizes output', async () => {
+  it('sanitizes Markdown output', async () => {
     Object.assign(cell, {
       source: [
         '[XSS](data:text/html;base64,PHNjcmlwdD5hbGVydChkb2N1bWVudC5kb21haW4pPC9zY3JpcHQ+Cg==)\n',
@@ -50,6 +50,17 @@ describe('Markdown component', () => {
     expect(vm.$el.querySelector('a').getAttribute('href')).toBeNull();
   });
 
+  it('sanitizes HTML', async () => {
+    const findLink = () => vm.$el.querySelector('.xss-link');
+    Object.assign(cell, {
+      source: ['<a href="test.js" data-remote=true data-type="script" class="xss-link">XSS</a>\n'],
+    });
+
+    await vm.$nextTick();
+    expect(findLink().getAttribute('data-remote')).toBe(null);
+    expect(findLink().getAttribute('data-type')).toBe(null);
+  });
+
   describe('katex', () => {
     beforeEach(() => {
       json = getJSONFixture('blob/notebook/math.json');
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/lib/gitlab/x509/signature_spec.rb b/spec/lib/gitlab/x509/signature_spec.rb
index 2ac9c1f3a3b..7ba15faf910 100644
--- a/spec/lib/gitlab/x509/signature_spec.rb
+++ b/spec/lib/gitlab/x509/signature_spec.rb
@@ -12,20 +12,30 @@ RSpec.describe Gitlab::X509::Signature do
   end
 
   shared_examples "a verified signature" do
-    it 'returns a verified signature if email does match' do
-      signature = described_class.new(
+    let_it_be(:user) { create(:user, email: X509Helpers::User1.certificate_email) }
+
+    subject(:signature) do
+      described_class.new(
         X509Helpers::User1.signed_commit_signature,
         X509Helpers::User1.signed_commit_base_data,
         X509Helpers::User1.certificate_email,
         X509Helpers::User1.signed_commit_time
       )
+    end
 
+    it 'returns a verified signature if email does match' do
       expect(signature.x509_certificate).to have_attributes(certificate_attributes)
       expect(signature.x509_certificate.x509_issuer).to have_attributes(issuer_attributes)
       expect(signature.verified_signature).to be_truthy
       expect(signature.verification_status).to eq(:verified)
     end
 
+    it "returns an unverified signature if the email matches but isn't confirmed" do
+      user.update!(confirmed_at: nil)
+
+      expect(signature.verification_status).to eq(:unverified)
+    end
+
     it 'returns an unverified signature if email does not match' do
       signature = described_class.new(
         X509Helpers::User1.signed_commit_signature,
@@ -55,13 +65,6 @@ RSpec.describe Gitlab::X509::Signature do
     end
 
     it 'returns an unverified signature if certificate is revoked' do
-      signature = described_class.new(
-        X509Helpers::User1.signed_commit_signature,
-        X509Helpers::User1.signed_commit_base_data,
-        X509Helpers::User1.certificate_email,
-        X509Helpers::User1.signed_commit_time
-      )
-
       expect(signature.verification_status).to eq(:verified)
 
       signature.x509_certificate.revoked!
@@ -253,23 +256,25 @@ RSpec.describe Gitlab::X509::Signature do
   end
 
   describe '#user' do
-    signature = described_class.new(
-      X509Helpers::User1.signed_tag_signature,
-      X509Helpers::User1.signed_tag_base_data,
-      X509Helpers::User1.certificate_email,
-      X509Helpers::User1.signed_commit_time
-    )
+    subject do
+      described_class.new(
+        X509Helpers::User1.signed_tag_signature,
+        X509Helpers::User1.signed_tag_base_data,
+        X509Helpers::User1.certificate_email,
+        X509Helpers::User1.signed_commit_time
+      ).user
+    end
 
     context 'if email is assigned to a user' do
       let!(:user) { create(:user, email: X509Helpers::User1.certificate_email) }
 
       it 'returns user' do
-        expect(signature.user).to eq(user)
+        is_expected.to eq(user)
       end
     end
 
     it 'if email is not assigned to a user, return nil' do
-      expect(signature.user).to be_nil
+      is_expected.to be_nil
     end
   end
 
@@ -292,6 +297,17 @@ RSpec.describe Gitlab::X509::Signature do
     end
 
     context 'verified signature' do
+      let_it_be(:user) { create(:user, email: X509Helpers::User1.certificate_email) }
+
+      subject(:signature) do
+        described_class.new(
+          X509Helpers::User1.signed_tag_signature,
+          X509Helpers::User1.signed_tag_base_data,
+          X509Helpers::User1.certificate_email,
+          X509Helpers::User1.signed_commit_time
+        )
+      end
+
       context 'with trusted certificate store' do
         before do
           store = OpenSSL::X509::Store.new
@@ -301,19 +317,18 @@ RSpec.describe Gitlab::X509::Signature do
         end
 
         it 'returns a verified signature if email does match' do
-          signature = described_class.new(
-            X509Helpers::User1.signed_tag_signature,
-            X509Helpers::User1.signed_tag_base_data,
-            X509Helpers::User1.certificate_email,
-            X509Helpers::User1.signed_commit_time
-          )
-
           expect(signature.x509_certificate).to have_attributes(certificate_attributes)
           expect(signature.x509_certificate.x509_issuer).to have_attributes(issuer_attributes)
           expect(signature.verified_signature).to be_truthy
           expect(signature.verification_status).to eq(:verified)
         end
 
+        it "returns an unverified signature if the email matches but isn't confirmed" do
+          user.update!(confirmed_at: nil)
+
+          expect(signature.verification_status).to eq(:unverified)
+        end
+
         it 'returns an unverified signature if email does not match' do
           signature = described_class.new(
             X509Helpers::User1.signed_tag_signature,
@@ -343,13 +358,6 @@ RSpec.describe Gitlab::X509::Signature do
         end
 
         it 'returns an unverified signature if certificate is revoked' do
-          signature = described_class.new(
-            X509Helpers::User1.signed_tag_signature,
-            X509Helpers::User1.signed_tag_base_data,
-            X509Helpers::User1.certificate_email,
-            X509Helpers::User1.signed_commit_time
-          )
-
           expect(signature.verification_status).to eq(:verified)
 
           signature.x509_certificate.revoked!
@@ -368,13 +376,6 @@ RSpec.describe Gitlab::X509::Signature do
         end
 
         it 'returns an unverified signature' do
-          signature = described_class.new(
-            X509Helpers::User1.signed_tag_signature,
-            X509Helpers::User1.signed_tag_base_data,
-            X509Helpers::User1.certificate_email,
-            X509Helpers::User1.signed_commit_time
-          )
-
           expect(signature.x509_certificate).to have_attributes(certificate_attributes)
           expect(signature.x509_certificate.x509_issuer).to have_attributes(issuer_attributes)
           expect(signature.verified_signature).to be_falsey
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
diff --git a/spec/tasks/gitlab/x509/update_rake_spec.rb b/spec/tasks/gitlab/x509/update_rake_spec.rb
index 93e97ab38ad..b166e73935a 100644
--- a/spec/tasks/gitlab/x509/update_rake_spec.rb
+++ b/spec/tasks/gitlab/x509/update_rake_spec.rb
@@ -8,12 +8,13 @@ RSpec.describe 'gitlab:x509 namespace rake task' do
   end
 
   describe 'update_signatures' do
-    subject { run_rake_task('gitlab:x509:update_signatures') }
-
-    let(:project) { create :project, :repository, path: X509Helpers::User1.path }
+    let(:user) { create(:user, email: X509Helpers::User1.certificate_email) }
+    let(:project) { create(:project, :repository, path: X509Helpers::User1.path, creator: user) }
     let(:x509_signed_commit) { project.commit_by(oid: '189a6c924013fc3fe40d6f1ec1dc20214183bc97') }
     let(:x509_commit) { Gitlab::X509::Commit.new(x509_signed_commit).signature }
 
+    subject { run_rake_task('gitlab:x509:update_signatures') }
+
     it 'changes from unverified to verified if the certificate store contains the root certificate' do
       x509_commit
 
@@ -22,21 +23,14 @@ RSpec.describe 'gitlab:x509 namespace rake task' do
       store.add_cert(certificate)
       allow(OpenSSL::X509::Store).to receive(:new).and_return(store)
 
-      expect(x509_commit.verification_status).to eq('unverified')
       expect_any_instance_of(Gitlab::X509::Commit).to receive(:update_signature!).and_call_original
-
-      subject
-
-      x509_commit.reload
-      expect(x509_commit.verification_status).to eq('verified')
+      expect { subject }.to change { x509_commit.reload.verification_status }.from('unverified').to('verified')
     end
 
     it 'returns if no signature is available' do
-      expect_any_instance_of(Gitlab::X509::Commit) do |x509_commit|
-        expect(x509_commit).not_to receive(:update_signature!)
+      expect_any_instance_of(Gitlab::X509::Commit).not_to receive(:update_signature!)
 
-        subject
-      end
+      subject
     end
   end
 end