Implement review comments for !11963 from @adamniedzielski.

- Change double quotes to single quotes.
- Why is `OmniAuth.config.full_host` being reassigned in the integration test?
- Use `map` over `map!` to avoid `dup` in the `gitlab:info` rake task
- Other minor changes

5 files changed, 26 insertions, 19 deletions

diff --git a/app/views/devise/shared/_omniauth_box.html.haml b/app/views/devise/shared/_omniauth_box.html.haml
index 493e18565c0..e80d10dc8f1 100644
--- a/app/views/devise/shared/_omniauth_box.html.haml
+++ b/app/views/devise/shared/_omniauth_box.html.haml
@@ -9,4 +9,4 @@
         = link_to provider_image_tag(provider), omniauth_authorize_path(:user, provider), method: :post, class: 'oauth-login' + (has_icon ? ' oauth-image-link' : ' btn'), id: "oauth-login-#{provider}"
     %fieldset
       = check_box_tag :remember_me
-      = label_tag :remember_me, "Remember Me"
+      = label_tag :remember_me, 'Remember Me'
diff --git a/lib/tasks/gitlab/info.rake b/lib/tasks/gitlab/info.rake
index 2245dfb7828..e9fb6a008b0 100644
--- a/lib/tasks/gitlab/info.rake
+++ b/lib/tasks/gitlab/info.rake
@@ -42,8 +42,7 @@ namespace :gitlab do
       http_clone_url = project.http_url_to_repo
       ssh_clone_url  = project.ssh_url_to_repo
 
-      omniauth_providers = Gitlab.config.omniauth.providers.dup
-      omniauth_providers.map! { |provider| provider['name'] }
+      omniauth_providers = Gitlab.config.omniauth.providers.map { |provider| provider['name'] }
 
       puts ""
       puts "GitLab information".color(:yellow)
diff --git a/spec/features/oauth_login_spec.rb b/spec/features/oauth_login_spec.rb
index 8e02bc88fad..452b920307c 100644
--- a/spec/features/oauth_login_spec.rb
+++ b/spec/features/oauth_login_spec.rb
@@ -1,27 +1,35 @@
 require 'spec_helper'
 
-feature 'OAuth Login', feature: true, js: true do
+feature 'OAuth Login', js: true do
   def enter_code(code)
     fill_in 'user_otp_attempt', with: code
     click_button 'Verify code'
   end
 
   def stub_omniauth_config(provider)
-    OmniAuth.config.add_mock(provider, OmniAuth::AuthHash.new({ provider: provider.to_s, uid: "12345" }))
+    OmniAuth.config.add_mock(provider, OmniAuth::AuthHash.new(provider: provider.to_s, uid: "12345"))
     Rails.application.env_config['devise.mapping'] = Devise.mappings[:user]
-    Rails.application.env_config["omniauth.auth"] = OmniAuth.config.mock_auth[provider]
+    Rails.application.env_config['omniauth.auth'] = OmniAuth.config.mock_auth[provider]
   end
 
   providers = [:github, :twitter, :bitbucket, :gitlab, :google_oauth2,
                :facebook, :authentiq, :cas3, :auth0]
 
   before(:all) do
+    # The OmniAuth `full_host` parameter doesn't get set correctly (it gets set to something like `http://localhost`
+    # here), and causes integration tests to fail with 404s. We set the `full_host` by removing the request path (and
+    # anything after it) from the request URI.
+    @omniauth_config_full_host = OmniAuth.config.full_host
     OmniAuth.config.full_host = ->(request) { request['REQUEST_URI'].sub(/#{request['REQUEST_PATH']}.*/, '') }
   end
 
+  after(:all) do
+    OmniAuth.config.full_host = @omniauth_config_full_host
+  end
+
   providers.each do |provider|
     context "when the user logs in using the #{provider} provider" do
-      context "when two-factor authentication is disabled" do
+      context 'when two-factor authentication is disabled' do
         it 'logs the user in' do
           stub_omniauth_config(provider)
           user = create(:omniauth_user, extern_uid: 'my-uid', provider: provider.to_s)
@@ -31,7 +39,7 @@ feature 'OAuth Login', feature: true, js: true do
         end
       end
 
-      context "when two-factor authentication is enabled" do
+      context 'when two-factor authentication is enabled' do
         it 'logs the user in' do
           stub_omniauth_config(provider)
           user = create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: provider.to_s)
@@ -43,27 +51,27 @@ feature 'OAuth Login', feature: true, js: true do
       end
 
       context 'when "remember me" is checked' do
-        context "when two-factor authentication is disabled" do
+        context 'when two-factor authentication is disabled' do
           it 'remembers the user after a browser restart' do
             stub_omniauth_config(provider)
             user = create(:omniauth_user, extern_uid: 'my-uid', provider: provider.to_s)
             login_via(provider.to_s, user, 'my-uid', remember_me: true)
 
-            restart_browser
+            clear_browser_session
 
             visit(root_path)
             expect(current_path).to eq root_path
           end
         end
 
-        context "when two-factor authentication is enabled" do
+        context 'when two-factor authentication is enabled' do
           it 'remembers the user after a browser restart' do
             stub_omniauth_config(provider)
             user = create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: provider.to_s)
             login_via(provider.to_s, user, 'my-uid', remember_me: true)
             enter_code(user.current_otp)
 
-            restart_browser
+            clear_browser_session
 
             visit(root_path)
             expect(current_path).to eq root_path
@@ -72,27 +80,27 @@ feature 'OAuth Login', feature: true, js: true do
       end
 
       context 'when "remember me" is not checked' do
-        context "when two-factor authentication is disabled" do
+        context 'when two-factor authentication is disabled' do
           it 'does not remember the user after a browser restart' do
             stub_omniauth_config(provider)
             user = create(:omniauth_user, extern_uid: 'my-uid', provider: provider.to_s)
             login_via(provider.to_s, user, 'my-uid', remember_me: false)
 
-            restart_browser
+            clear_browser_session
 
             visit(root_path)
             expect(current_path).to eq new_user_session_path
           end
         end
 
-        context "when two-factor authentication is enabled" do
-          it 'remembers the user after a browser restart' do
+        context 'when two-factor authentication is enabled' do
+          it 'does not remember the user after a browser restart' do
             stub_omniauth_config(provider)
             user = create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: provider.to_s)
             login_via(provider.to_s, user, 'my-uid', remember_me: false)
             enter_code(user.current_otp)
 
-            restart_browser
+            clear_browser_session
 
             visit(root_path)
             expect(current_path).to eq new_user_session_path
diff --git a/spec/support/capybara_helpers.rb b/spec/support/capybara_helpers.rb
index 1037e9def8c..3eb7bea3227 100644
--- a/spec/support/capybara_helpers.rb
+++ b/spec/support/capybara_helpers.rb
@@ -37,7 +37,7 @@ module CapybaraHelpers
   end
 
   # Simulate a browser restart by clearing the session cookie.
-  def restart_browser
+  def clear_browser_session
     page.driver.remove_cookie('_gitlab_session')
   end
 end
diff --git a/spec/support/login_helpers.rb b/spec/support/login_helpers.rb
index 789cf9baae2..184c7b5125a 100644
--- a/spec/support/login_helpers.rb
+++ b/spec/support/login_helpers.rb
@@ -67,7 +67,7 @@ module LoginHelpers
     visit new_user_session_path
     expect(page).to have_content('Sign in with')
 
-    check "Remember Me" if remember_me
+    check 'Remember Me' if remember_me
 
     click_link "oauth-login-#{provider}"
   end