Merge branch 'revert-6df61942' into 'master'

Revert "Merge branch '18000-remember-me-for-oauth-login' into 'master'"

See merge request !12660

app/assets/javascripts/dispatcher.js

@@ -56,7 +56,6 @@ import GfmAutoComplete from './gfm_auto_complete';
 import ShortcutsBlob from './shortcuts_blob';
 import initSettingsPanels from './settings_panels';
 import initExperimentalFlags from './experimental_flags';
-import OAuthRememberMe from './oauth_remember_me';
 
 (function() {
   var Dispatcher;
@@ -128,7 +127,6 @@ import OAuthRememberMe from './oauth_remember_me';
         case 'sessions:new':
           new UsernameValidator();
           new ActiveTabMemoizer();
-          new OAuthRememberMe({ container: $(".omniauth-container") }).bindEvents();
           break;
         case 'projects:boards:show':
         case 'projects:boards:index':

app/assets/javascripts/oauth_remember_me.js

@@ -1,32 +0,0 @@
-/**
- * OAuth-based login buttons have a separate "remember me" checkbox.
- *
- * Toggling this checkbox adds/removes a `remember_me` parameter to the
- * login buttons' href, which is passed on to the omniauth callback.
- **/
-
-export default class OAuthRememberMe {
-  constructor(opts = {}) {
-    this.container = opts.container || '';
-    this.loginLinkSelector = '.oauth-login';
-  }
-
-  bindEvents() {
-    $('#remember_me', this.container).on('click', this.toggleRememberMe);
-  }
-
-  // eslint-disable-next-line class-methods-use-this
-  toggleRememberMe(event) {
-    const rememberMe = $(event.target).is(':checked');
-
-    $('.oauth-login', this.container).each((i, element) => {
-      const href = $(element).attr('href');
-
-      if (rememberMe) {
-        $(element).attr('href', `${href}?remember_me=1`);
-      } else {
-        $(element).attr('href', href.replace('?remember_me=1', ''));
-      }
-    });
-  }
-}

app/controllers/omniauth_callbacks_controller.rb

@@ -1,6 +1,5 @@
 class OmniauthCallbacksController < Devise::OmniauthCallbacksController
   include AuthenticatesWithTwoFactor
-  include Devise::Controllers::Rememberable
 
   protect_from_forgery except: [:kerberos, :saml, :cas3]
 
@@ -116,10 +115,8 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
     if @user.persisted? && @user.valid?
       log_audit_event(@user, with: oauth['provider'])
       if @user.two_factor_enabled?
-        params[:remember_me] = '1' if remember_me?
         prompt_for_two_factor(@user)
       else
-        remember_me(@user) if remember_me?
         sign_in_and_redirect(@user)
       end
     else
@@ -150,9 +147,4 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
     AuditEventService.new(user, user, options)
       .for_authentication.security_event
   end
-
-  def remember_me?
-    request_params = request.env['omniauth.params']
-    (request_params['remember_me'] == '1') if request_params.present?
-  end
 end

app/views/devise/shared/_omniauth_box.html.haml

@@ -6,7 +6,4 @@
     - providers.each do |provider|
       %span.light
         - has_icon = provider_has_icon?(provider)
-        = 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}"
+        = link_to provider_image_tag(provider), omniauth_authorize_path(:user, provider), method: :post, class: (has_icon ? 'oauth-image-link' : 'btn')
-    %fieldset
-      = check_box_tag :remember_me
-      = label_tag :remember_me, 'Remember Me'

changelogs/unreleased/18000-remember-me-for-oauth-login.yml

@@ -1,4 +0,0 @@
----
-title: Honor the "Remember me" parameter for OAuth-based login
-merge_request: 11963
-author:

config/gitlab.yml.example

@@ -619,53 +619,6 @@ test:
       title: "JIRA"
       url: https://sample_company.atlassian.net
       project_key: PROJECT
-
-  omniauth:
-    enabled: true
-    allow_single_sign_on: true
-    external_providers: []
-
-    providers:
-      - { name: 'cas3',
-          label: 'cas3',
-          args: { url: 'https://sso.example.com',
-                  disable_ssl_verification: false,
-                  login_url: '/cas/login',
-                  service_validate_url: '/cas/p3/serviceValidate',
-                  logout_url: '/cas/logout'} }
-      - { name: 'authentiq',
-          app_id: 'YOUR_CLIENT_ID',
-          app_secret: 'YOUR_CLIENT_SECRET',
-          args: { scope: 'aq:name email~rs address aq:push' } }
-      - { name: 'github',
-          app_id: 'YOUR_APP_ID',
-          app_secret: 'YOUR_APP_SECRET',
-          url: "https://github.com/",
-          verify_ssl: false,
-          args: { scope: 'user:email' } }
-      - { name: 'bitbucket',
-          app_id: 'YOUR_APP_ID',
-          app_secret: 'YOUR_APP_SECRET' }
-      - { name: 'gitlab',
-          app_id: 'YOUR_APP_ID',
-          app_secret: 'YOUR_APP_SECRET',
-          args: { scope: 'api' } }
-      - { name: 'google_oauth2',
-          app_id: 'YOUR_APP_ID',
-          app_secret: 'YOUR_APP_SECRET',
-          args: { access_type: 'offline', approval_prompt: '' } }
-      - { name: 'facebook',
-          app_id: 'YOUR_APP_ID',
-          app_secret: 'YOUR_APP_SECRET' }
-      - { name: 'twitter',
-          app_id: 'YOUR_APP_ID',
-          app_secret: 'YOUR_APP_SECRET' }
-      - { name: 'auth0',
-          args: {
-            client_id: 'YOUR_AUTH0_CLIENT_ID',
-            client_secret: 'YOUR_AUTH0_CLIENT_SECRET',
-            namespace: 'YOUR_AUTH0_DOMAIN' } }
-
   ldap:
     enabled: false
     servers:

lib/tasks/gitlab/info.rake

@@ -42,7 +42,8 @@ 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.map { |provider| provider['name'] }
+      omniauth_providers = Gitlab.config.omniauth.providers
+      omniauth_providers.map! { |provider| provider['name'] }
 
       puts ""
       puts "GitLab information".color(:yellow)

spec/features/oauth_login_spec.rb

@@ -1,112 +0,0 @@
-require 'spec_helper'
-
-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"))
-    Rails.application.env_config['devise.mapping'] = Devise.mappings[:user]
-    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
-        it 'logs the user in' 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')
-
-          expect(current_path).to eq root_path
-        end
-      end
-
-      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)
-          login_via(provider.to_s, user, 'my-uid')
-
-          enter_code(user.current_otp)
-          expect(current_path).to eq root_path
-        end
-      end
-
-      context 'when "remember me" is checked' 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)
-
-            clear_browser_session
-
-            visit(root_path)
-            expect(current_path).to eq root_path
-          end
-        end
-
-        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)
-
-            clear_browser_session
-
-            visit(root_path)
-            expect(current_path).to eq root_path
-          end
-        end
-      end
-
-      context 'when "remember me" is not checked' 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)
-
-            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 '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)
-
-            clear_browser_session
-
-            visit(root_path)
-            expect(current_path).to eq new_user_session_path
-          end
-        end
-      end
-    end
-  end
-end

spec/javascripts/fixtures/oauth_remember_me.html.haml

@@ -1,5 +0,0 @@
-#oauth-container
-  %input#remember_me{ type: "checkbox" }
-
-  %a.oauth-login.twitter{ href: "http://example.com/" }
-  %a.oauth-login.github{ href: "http://example.com/" }

spec/javascripts/oauth_remember_me_spec.js

@@ -1,26 +0,0 @@
-import OAuthRememberMe from '~/oauth_remember_me';
-
-describe('OAuthRememberMe', () => {
-  preloadFixtures('static/oauth_remember_me.html.raw');
-
-  beforeEach(() => {
-    loadFixtures('static/oauth_remember_me.html.raw');
-
-    new OAuthRememberMe({ container: $('#oauth-container') }).bindEvents();
-  });
-
-  it('adds the "remember_me" query parameter to all OAuth login buttons', () => {
-    $('#oauth-container #remember_me').click();
-
-    expect($('#oauth-container .oauth-login.twitter').attr('href')).toBe('http://example.com/?remember_me=1');
-    expect($('#oauth-container .oauth-login.github').attr('href')).toBe('http://example.com/?remember_me=1');
-  });
-
-  it('removes the "remember_me" query parameter from all OAuth login buttons', () => {
-    $('#oauth-container #remember_me').click();
-    $('#oauth-container #remember_me').click();
-
-    expect($('#oauth-container .oauth-login.twitter').attr('href')).toBe('http://example.com/');
-    expect($('#oauth-container .oauth-login.github').attr('href')).toBe('http://example.com/');
-  });
-});

spec/support/capybara_helpers.rb

@@ -35,11 +35,6 @@ module CapybaraHelpers
     visit 'about:blank'
     visit url
   end
-
-  # Simulate a browser restart by clearing the session cookie.
-  def clear_browser_session
-    page.driver.remove_cookie('_gitlab_session')
-  end
 end
 
 RSpec.configure do |config|

spec/support/login_helpers.rb

@@ -62,16 +62,6 @@ module LoginHelpers
     Thread.current[:current_user] = user
   end
 
-  def login_via(provider, user, uid, remember_me: false)
-    mock_auth_hash(provider, uid, user.email)
-    visit new_user_session_path
-    expect(page).to have_content('Sign in with')
-
-    check 'Remember Me' if remember_me
-
-    click_link "oauth-login-#{provider}"
-  end
-
   def mock_auth_hash(provider, uid, email)
     # The mock_auth configuration allows you to set per-provider (or default)
     # authentication hashes to return during integration testing.
@@ -118,7 +108,6 @@ module LoginHelpers
     end
     allow(Gitlab::OAuth::Provider).to receive_messages(providers: [:saml], config_for: mock_saml_config)
     stub_omniauth_setting(messages)
-    allow_any_instance_of(Object).to receive(:user_saml_omniauth_authorize_path).and_return('/users/auth/saml')
+    expect_any_instance_of(Object).to receive(:omniauth_authorize_path).with(:user, "saml").and_return('/users/auth/saml')
-    allow_any_instance_of(Object).to receive(:omniauth_authorize_path).with(:user, "saml").and_return('/users/auth/saml')
   end
 end