summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@gitlab.com>2017-07-05 16:06:31 +0000
committerDouwe Maan <douwe@gitlab.com>2017-07-05 16:06:31 +0000
commit4a67f4ee39ae3e994448d9a0935f0a30ad36706a (patch)
tree75f6b8d0a59615c38e855fff6092a0e69892db0f
parent960d9e3c0326587975efceacba3c37aa0388aebf (diff)
parent8a3022a69826272fb5eb111c016770965a5484b3 (diff)
downloadgitlab-ce-4a67f4ee39ae3e994448d9a0935f0a30ad36706a.tar.gz
Merge branch 'revert-6df61942' into 'master'
Revert "Merge branch '18000-remember-me-for-oauth-login' into 'master'" See merge request !12660
-rw-r--r--app/assets/javascripts/dispatcher.js2
-rw-r--r--app/assets/javascripts/oauth_remember_me.js32
-rw-r--r--app/controllers/omniauth_callbacks_controller.rb8
-rw-r--r--app/views/devise/shared/_omniauth_box.html.haml5
-rw-r--r--changelogs/unreleased/18000-remember-me-for-oauth-login.yml4
-rw-r--r--config/gitlab.yml.example47
-rw-r--r--lib/tasks/gitlab/info.rake3
-rw-r--r--spec/features/oauth_login_spec.rb112
-rw-r--r--spec/javascripts/fixtures/oauth_remember_me.html.haml5
-rw-r--r--spec/javascripts/oauth_remember_me_spec.js26
-rw-r--r--spec/support/capybara_helpers.rb5
-rw-r--r--spec/support/login_helpers.rb13
12 files changed, 4 insertions, 258 deletions
diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js
index e924fde60bf..4247540de22 100644
--- a/app/assets/javascripts/dispatcher.js
+++ b/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':
diff --git a/app/assets/javascripts/oauth_remember_me.js b/app/assets/javascripts/oauth_remember_me.js
deleted file mode 100644
index ffc2dd6bbca..00000000000
--- a/app/assets/javascripts/oauth_remember_me.js
+++ /dev/null
@@ -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', ''));
- }
- });
- }
-}
diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb
index 323d5d26eb6..b82681b197e 100644
--- a/app/controllers/omniauth_callbacks_controller.rb
+++ b/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
diff --git a/app/views/devise/shared/_omniauth_box.html.haml b/app/views/devise/shared/_omniauth_box.html.haml
index e80d10dc8f1..f92f89e73ff 100644
--- a/app/views/devise/shared/_omniauth_box.html.haml
+++ b/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}"
- %fieldset
- = check_box_tag :remember_me
- = label_tag :remember_me, 'Remember Me'
+ = link_to provider_image_tag(provider), omniauth_authorize_path(:user, provider), method: :post, class: (has_icon ? 'oauth-image-link' : 'btn')
diff --git a/changelogs/unreleased/18000-remember-me-for-oauth-login.yml b/changelogs/unreleased/18000-remember-me-for-oauth-login.yml
deleted file mode 100644
index 1ef92756a76..00000000000
--- a/changelogs/unreleased/18000-remember-me-for-oauth-login.yml
+++ /dev/null
@@ -1,4 +0,0 @@
----
-title: Honor the "Remember me" parameter for OAuth-based login
-merge_request: 11963
-author:
diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example
index fdc2b24e110..4b81fd90f59 100644
--- a/config/gitlab.yml.example
+++ b/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:
diff --git a/lib/tasks/gitlab/info.rake b/lib/tasks/gitlab/info.rake
index e9fb6a008b0..e3883278886 100644
--- a/lib/tasks/gitlab/info.rake
+++ b/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)
diff --git a/spec/features/oauth_login_spec.rb b/spec/features/oauth_login_spec.rb
deleted file mode 100644
index 452b920307c..00000000000
--- a/spec/features/oauth_login_spec.rb
+++ /dev/null
@@ -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
diff --git a/spec/javascripts/fixtures/oauth_remember_me.html.haml b/spec/javascripts/fixtures/oauth_remember_me.html.haml
deleted file mode 100644
index 7886e995e57..00000000000
--- a/spec/javascripts/fixtures/oauth_remember_me.html.haml
+++ /dev/null
@@ -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/" }
diff --git a/spec/javascripts/oauth_remember_me_spec.js b/spec/javascripts/oauth_remember_me_spec.js
deleted file mode 100644
index f90e0093d25..00000000000
--- a/spec/javascripts/oauth_remember_me_spec.js
+++ /dev/null
@@ -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/');
- });
-});
diff --git a/spec/support/capybara_helpers.rb b/spec/support/capybara_helpers.rb
index 3eb7bea3227..b57a3493aff 100644
--- a/spec/support/capybara_helpers.rb
+++ b/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|
diff --git a/spec/support/login_helpers.rb b/spec/support/login_helpers.rb
index 99e7806353d..4c88958264b 100644
--- a/spec/support/login_helpers.rb
+++ b/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')
- allow_any_instance_of(Object).to receive(:omniauth_authorize_path).with(:user, "saml").and_return('/users/auth/saml')
+ expect_any_instance_of(Object).to receive(:omniauth_authorize_path).with(:user, "saml").and_return('/users/auth/saml')
end
end