Merge branch 'ce-to-ee-2018-07-19' into 'master'

CE upstream - 2018-07-19 12:22 UTC

See merge request gitlab-org/gitlab-ee!6592

app/models/user.rb

@@ -14,7 +14,6 @@ class User < ActiveRecord::Base
   include IgnorableColumn
   include FeatureGate
   include CreatedAtFilterable
-  include IgnorableColumn
   include BulkMemberAccessLoad
   include BlocksJsonSerialization
   include WithUploads

app/views/profiles/two_factor_auths/show.html.haml

@@ -13,10 +13,16 @@
       - if current_user.two_factor_otp_enabled?
         %p
           You've already enabled two-factor authentication using mobile authenticator applications. In order to register a different device, you must first disable two-factor authentication.
+        %p
+          If you lose your recovery codes you can generate new ones, invalidating all previous codes.
+        %div
           = link_to 'Disable two-factor authentication', profile_two_factor_auth_path,
                 method: :delete,
                 data: { confirm: "Are you sure? This will invalidate your registered applications and U2F devices." },
-                class: 'btn btn-danger'
+                class: 'btn btn-danger append-right-10'
+          = form_tag codes_profile_two_factor_auth_path, {style: 'display: inline-block', method: :post} do |f|
+            = submit_tag 'Regenerate recovery codes', class: 'btn'
+
       - else
         %p
           Download the Google Authenticator application from App Store or Google Play Store and scan this code.

app/workers/emails_on_push_worker.rb

@@ -51,7 +51,7 @@ class EmailsOnPushWorker
       end
     end
 
-    recipients.split.each do |recipient|
+    valid_recipients(recipients).each do |recipient|
       begin
         send_email(
           recipient,
@@ -89,4 +89,10 @@ class EmailsOnPushWorker
     email.header[:skip_premailer] = true if skip_premailer
     email.deliver_now
   end
+
+  def valid_recipients(recipients)
+    recipients.split.select do |recipient|
+      recipient.include?('@')
+    end
+  end
 end

changelogs/unreleased/accept-rf3-2822-compliant-addresses.yml

+---
+title: Emails on push recipients now accepts formats like John Doe <johndoe@example.com>
+merge_request:
+author: George Thomas
+type: added

changelogs/unreleased/rails5-fix-revert-modal-spec.yml

+---
+title: Rails5 fix user sees revert modal spec
+merge_request: 20706
+author: Jasper Maes
+type: fixed

changelogs/unreleased/regen-2fa-codes.yml

+---
+title: Added button to regenerate 2FA codes
+merge_request:
+author: Luke Picciau
+type: added

changelogs/unreleased/sh-normalize-urls.yml

+---
+title: Escape username and password in UrlSanitizer#full_url
+merge_request: 20684
+author:
+type: fixed

doc/user/project/integrations/emails_on_push.md

@@ -6,7 +6,7 @@ every change that is pushed to your project.
 Navigate to the [Integrations page](project_services.md#accessing-the-project-services)
 and select the **Emails on push** service to configure it.
 
-In the _Recipients_ area, provide a list of emails separated by commas.
+In the _Recipients_ area, provide a list of emails separated by spaces or newlines.
 
 You can configure any of the following settings depending on your preference.
 

lib/gitlab/git_ref_validator.rb

@@ -7,11 +7,11 @@ module Gitlab
     #
     # Returns true for a valid reference name, false otherwise
     def validate(ref_name)
-      return false if ref_name.start_with?('refs/heads/')
-      return false if ref_name.start_with?('refs/remotes/')
+      not_allowed_prefixes = %w(refs/heads/ refs/remotes/ -)
+      return false if ref_name.start_with?(*not_allowed_prefixes)
+      return false if ref_name == 'HEAD'
 
-      Gitlab::Utils.system_silent(
-        %W(#{Gitlab.config.git.bin_path} check-ref-format --branch #{ref_name}))
+      Rugged::Reference.valid_name? "refs/heads/#{ref_name}"
     end
   end
 end

lib/gitlab/url_sanitizer.rb

@@ -71,12 +71,10 @@ module Gitlab
     def generate_full_url
       return @url unless valid_credentials?
 
-      @full_url = @url.dup
-
-      @full_url.password = credentials[:password] if credentials[:password].present?
-      @full_url.user = credentials[:user] if credentials[:user].present?
-
-      @full_url
+      @url.dup.tap do |generated|
+        generated.password = encode_percent(credentials[:password]) if credentials[:password].present?
+        generated.user = encode_percent(credentials[:user]) if credentials[:user].present?
+      end
     end
 
     def safe_url
@@ -89,5 +87,10 @@ module Gitlab
     def valid_credentials?
       credentials && credentials.is_a?(Hash) && credentials.any?
     end
+
+    def encode_percent(string)
+      # CGI.escape converts spaces to +, but this doesn't work for git clone
+      CGI.escape(string).gsub('+', '%20')
+    end
   end
 end

scripts/lint-rugged

@@ -17,7 +17,10 @@ ALLOWED = [
   'lib/tasks/gitlab/cleanup.rake',
 
   # The only place where Rugged code is still allowed in production
-  'lib/gitlab/git/'
+  'lib/gitlab/git/',
+
+  # Needed to avoid using the git binary to validate a branch name
+  'lib/gitlab/git_ref_validator.rb'
 ].freeze
 
 rugged_lines = IO.popen(%w[git grep -i -n rugged -- app config lib], &:read).lines

spec/features/user_sees_revert_modal_spec.rb

@@ -9,6 +9,9 @@ describe 'Merge request > User sees revert modal', :js do
     sign_in(user)
     visit(project_merge_request_path(project, merge_request))
     click_button('Merge')
+
+    wait_for_requests
+
     visit(merge_request_path(merge_request))
     click_link('Revert')
   end

spec/lib/gitlab/url_sanitizer_spec.rb

@@ -145,6 +145,10 @@ describe Gitlab::UrlSanitizer do
         'http://foo:@example.com'    | 'http://foo@example.com'
         'http://:bar@example.com'    | :same
         'http://foo:bar@example.com' | :same
+        'http://foo:g p@example.com' | 'http://foo:g%20p@example.com'
+        'http://foo:s/h@example.com' | 'http://foo:s%2Fh@example.com'
+        'http://t u:a#b@example.com' | 'http://t%20u:a%23b@example.com'
+        'http://t+u:a#b@example.com' | 'http://t%2Bu:a%23b@example.com'
       end
 
       with_them do
@@ -160,7 +164,7 @@ describe Gitlab::UrlSanitizer do
       url_sanitizer = described_class.new("https://foo:b?r@github.com/me/project.git")
 
       expect(url_sanitizer.sanitized_url).to eq("https://github.com/me/project.git")
-      expect(url_sanitizer.full_url).to eq("https://foo:b?r@github.com/me/project.git")
+      expect(url_sanitizer.full_url).to eq("https://foo:b%3Fr@github.com/me/project.git")
     end
   end
 end

spec/workers/emails_on_push_worker_spec.rb

@@ -100,10 +100,6 @@ describe EmailsOnPushWorker, :mailer do
     end
 
     context "when there are multiple recipients" do
-      let(:recipients) do
-        1.upto(5).map { |i| user.email.sub('@', "+#{i}@") }.join("\n")
-      end
-
       before do
         # This is a hack because we modify the mail object before sending, for efficency,
         # but the TestMailer adapter just appends the objects to an array. To clone a mail
@@ -114,16 +110,57 @@ describe EmailsOnPushWorker, :mailer do
         end
       end
 
-      it "sends the mail to each of the recipients" do
-        perform
-        expect(ActionMailer::Base.deliveries.count).to eq(5)
-        expect(ActionMailer::Base.deliveries.map(&:to).flatten).to contain_exactly(*recipients.split)
+      context "when the recipient addresses are a list of email addresses" do
+        let(:recipients) do
+          1.upto(5).map { |i| user.email.sub('@', "+#{i}@") }.join("\n")
+        end
+
+        it "sends the mail to each of the recipients" do
+          perform
+
+          expect(ActionMailer::Base.deliveries.count).to eq(5)
+          expect(email_recipients).to contain_exactly(*recipients.split)
+        end
+
+        it "only generates the mail once" do
+          expect(Notify).to receive(:repository_push_email).once.and_call_original
+          expect(Premailer::Rails::CustomizedPremailer).to receive(:new).once.and_call_original
+
+          perform
+        end
       end
 
-      it "only generates the mail once" do
-        expect(Notify).to receive(:repository_push_email).once.and_call_original
-        expect(Premailer::Rails::CustomizedPremailer).to receive(:new).once.and_call_original
-        perform
+      context "when the recipient addresses contains angle brackets and are separated by spaces" do
+        let(:recipients) { "John Doe <johndoe@example.com> Jane Doe <janedoe@example.com>" }
+
+        it "accepts emails separated by whitespace" do
+          perform
+
+          expect(ActionMailer::Base.deliveries.count).to eq(2)
+          expect(email_recipients).to contain_exactly("johndoe@example.com", "janedoe@example.com")
+        end
+      end
+
+      context "when the recipient addresses contain a mix of emails with and without angle brackets" do
+        let(:recipients) { "johndoe@example.com Jane Doe <janedoe@example.com>" }
+
+        it "accepts both kind of emails" do
+          perform
+
+          expect(ActionMailer::Base.deliveries.count).to eq(2)
+          expect(email_recipients).to contain_exactly("johndoe@example.com", "janedoe@example.com")
+        end
+      end
+
+      context "when the recipient addresses contains angle brackets and are separated by newlines" do
+        let(:recipients) { "John Doe <johndoe@example.com>\nJane Doe <janedoe@example.com>" }
+
+        it "accepts emails separated by newlines" do
+          perform
+
+          expect(ActionMailer::Base.deliveries.count).to eq(2)
+          expect(email_recipients).to contain_exactly("johndoe@example.com", "janedoe@example.com")
+        end
       end
     end
   end