Merge branch 'security-11-7-22076-sanitize-url-in-names' into 'security-11-7'

[11.7] Sanitize user full name to clean up any URL to prevent mail clients from auto-linking URLs

See merge request gitlab/gitlabhq!2828

(cherry picked from commit a38c1f3567a2c89eeb82dc79ca9f0bf620acbb5a)

1c1b45da Add `sanitize_name` helper to sanitize URLs in user full name
aa974e9a Use `sanitize_name` to sanitize URL in user full name
0a09919e Add changelog entry

2 files changed, 19 insertions, 3 deletions

diff --git a/spec/helpers/emails_helper_spec.rb b/spec/helpers/emails_helper_spec.rb
index 3820cf5cb9d..23d7e41803e 100644
--- a/spec/helpers/emails_helper_spec.rb
+++ b/spec/helpers/emails_helper_spec.rb
@@ -1,6 +1,20 @@
 require 'spec_helper'
 
 describe EmailsHelper do
+  describe 'sanitize_name' do
+    context 'when name contains a valid URL string' do
+      it 'returns name with `.` replaced with `_` to prevent mail clients from auto-linking URLs' do
+        expect(sanitize_name('https://about.gitlab.com')).to eq('https://about_gitlab_com')
+        expect(sanitize_name('www.gitlab.com')).to eq('www_gitlab_com')
+        expect(sanitize_name('//about.gitlab.com/handbook/security/#best-practices')).to eq('//about_gitlab_com/handbook/security/#best-practices')
+      end
+
+      it 'returns name as it is when it does not contain a URL' do
+        expect(sanitize_name('Foo Bar')).to eq('Foo Bar')
+      end
+    end
+  end
+
   describe 'password_reset_token_valid_time' do
     def validate_time_string(time_limit, expected_string)
       Devise.reset_password_within = time_limit
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index f2d99872401..ec3972ac8db 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -9,8 +9,10 @@ describe Notify do
 
   include_context 'gitlab email notification'
 
+  let(:current_user_sanitized) { 'www_example_com' }
+
   set(:user) { create(:user) }
-  set(:current_user) { create(:user, email: "current@email.com") }
+  set(:current_user) { create(:user, email: "current@email.com", name: 'www.example.com') }
   set(:assignee) { create(:user, email: 'assignee@example.com', name: 'John Doe') }
 
   set(:merge_request) do
@@ -182,7 +184,7 @@ describe Notify do
           aggregate_failures do
             is_expected.to have_referable_subject(issue, reply: true)
             is_expected.to have_body_text(status)
-            is_expected.to have_body_text(current_user.name)
+            is_expected.to have_body_text(current_user_sanitized)
             is_expected.to have_body_text(project_issue_path project, issue)
           end
         end
@@ -361,7 +363,7 @@ describe Notify do
           aggregate_failures do
             is_expected.to have_referable_subject(merge_request, reply: true)
             is_expected.to have_body_text(status)
-            is_expected.to have_body_text(current_user.name)
+            is_expected.to have_body_text(current_user_sanitized)
             is_expected.to have_body_text(project_merge_request_path(project, merge_request))
           end
         end