Add latest changes from gitlab-org/security/gitlab@14-4-stable-ee

8 files changed, 40 insertions, 13 deletions

diff --git a/app/models/namespace.rb b/app/models/namespace.rb
index e6406293c66..07f9bb99952 100644
--- a/app/models/namespace.rb
+++ b/app/models/namespace.rb
@@ -34,6 +34,8 @@ class Namespace < ApplicationRecord
   SHARED_RUNNERS_SETTINGS = [SR_DISABLED_AND_UNOVERRIDABLE, SR_DISABLED_WITH_OVERRIDE, SR_ENABLED].freeze
   URL_MAX_LENGTH = 255
 
+  PATH_TRAILING_VIOLATIONS = %w[.git .atom .].freeze
+
   cache_markdown_field :description, pipeline: :description
 
   has_many :projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
@@ -200,9 +202,14 @@ class Namespace < ApplicationRecord
       # Remove everything that's not in the list of allowed characters.
       path.gsub!(/[^a-zA-Z0-9_\-\.]/,    "")
       # Remove trailing violations ('.atom', '.git', or '.')
-      path.gsub!(/(\.atom|\.git|\.)*\z/, "")
+      loop do
+        orig = path
+        PATH_TRAILING_VIOLATIONS.each { |ext| path = path.chomp(ext) }
+        break if orig == path
+      end
+
       # Remove leading violations ('-')
-      path.gsub!(/\A\-+/,                "")
+      path.gsub!(/\A\-+/, "")
 
       # Users with the great usernames of "." or ".." would end up with a blank username.
       # Work around that by setting their username to "blank", followed by a counter.
diff --git a/app/models/user.rb b/app/models/user.rb
index 25a2588a6a7..0e19e6e4a79 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -1434,7 +1434,7 @@ class User < ApplicationRecord
       name: name,
       username: username,
       avatar_url: avatar_url(only_path: false),
-      email: email
+      email: public_email.presence || _('[REDACTED]')
     }
   end
 
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 7d758ceca88..bdb3c99969b 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -39711,6 +39711,9 @@ msgstr ""
 msgid "[No reason]"
 msgstr ""
 
+msgid "[REDACTED]"
+msgstr ""
+
 msgid "[Redacted]"
 msgstr ""
 
diff --git a/spec/factories/users.rb b/spec/factories/users.rb
index 325f62f6028..8aa9654956e 100644
--- a/spec/factories/users.rb
+++ b/spec/factories/users.rb
@@ -15,6 +15,10 @@ FactoryBot.define do
       admin { true }
     end
 
+    trait :public_email do
+      public_email { email }
+    end
+
     trait :blocked do
       after(:build) { |user, _| user.block! }
     end
diff --git a/spec/lib/gitlab/data_builder/build_spec.rb b/spec/lib/gitlab/data_builder/build_spec.rb
index 325fdb90929..9cee0802e87 100644
--- a/spec/lib/gitlab/data_builder/build_spec.rb
+++ b/spec/lib/gitlab/data_builder/build_spec.rb
@@ -5,7 +5,7 @@ require 'spec_helper'
 RSpec.describe Gitlab::DataBuilder::Build do
   let!(:tag_names) { %w(tag-1 tag-2) }
   let(:runner) { create(:ci_runner, :instance, tag_list: tag_names.map { |n| ActsAsTaggableOn::Tag.create!(name: n)}) }
-  let(:user) { create(:user) }
+  let(:user) { create(:user, :public_email) }
   let(:build) { create(:ci_build, :running, runner: runner, user: user) }
 
   describe '.build' do
diff --git a/spec/lib/gitlab/data_builder/pipeline_spec.rb b/spec/lib/gitlab/data_builder/pipeline_spec.rb
index 0e574c7aa84..8b57da8e60b 100644
--- a/spec/lib/gitlab/data_builder/pipeline_spec.rb
+++ b/spec/lib/gitlab/data_builder/pipeline_spec.rb
@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::DataBuilder::Pipeline do
-  let_it_be(:user) { create(:user) }
+  let_it_be(:user) { create(:user, :public_email) }
   let_it_be(:project) { create(:project, :repository) }
 
   let_it_be_with_reload(:pipeline) do
@@ -46,7 +46,7 @@ RSpec.describe Gitlab::DataBuilder::Pipeline do
         name: user.name,
         username: user.username,
         avatar_url: user.avatar_url(only_path: false),
-        email: user.email
+        email: user.public_email
         })
     end
 
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 98b55ccb76b..5f3aad0ab24 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -7,7 +7,7 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
   include StubRequests
   include Ci::SourcePipelineHelpers
 
-  let_it_be(:user) { create(:user) }
+  let_it_be(:user) { create(:user, :public_email) }
   let_it_be(:namespace) { create_default(:namespace).freeze }
   let_it_be(:project) { create_default(:project, :repository).freeze }
 
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index db805a804c8..21c5aea514a 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -5679,16 +5679,29 @@ RSpec.describe User do
   end
 
   describe '#hook_attrs' do
-    it 'includes id, name, username, avatar_url, and email' do
-      user = create(:user)
-      user_attributes = {
+    let(:user) { create(:user) }
+    let(:user_attributes) do
+      {
         id: user.id,
         name: user.name,
         username: user.username,
-        avatar_url: user.avatar_url(only_path: false),
-        email: user.email
+        avatar_url: user.avatar_url(only_path: false)
       }
-      expect(user.hook_attrs).to eq(user_attributes)
+    end
+
+    context 'with a public email' do
+      it 'includes id, name, username, avatar_url, and email' do
+        user.public_email = "hello@hello.com"
+        user_attributes[:email] = user.public_email
+        expect(user.hook_attrs).to eq(user_attributes)
+      end
+    end
+
+    context 'without a public email' do
+      it "does not include email if user's email is private" do
+        user_attributes[:email] = "[REDACTED]"
+        expect(user.hook_attrs).to eq(user_attributes)
+      end
     end
   end