Commit 0c2aa86f authored by Douwe Maan's avatar Douwe Maan

Merge branch 'revert-eb2681a6' into 'master'

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

See merge request !2345
parents 784d472e 55e6292f
...@@ -59,7 +59,6 @@ import GfmAutoComplete from './gfm_auto_complete'; ...@@ -59,7 +59,6 @@ import GfmAutoComplete from './gfm_auto_complete';
import ShortcutsBlob from './shortcuts_blob'; import ShortcutsBlob from './shortcuts_blob';
import initSettingsPanels from './settings_panels'; import initSettingsPanels from './settings_panels';
import initExperimentalFlags from './experimental_flags'; import initExperimentalFlags from './experimental_flags';
import OAuthRememberMe from './oauth_remember_me';
// EE-only // EE-only
import ApproversSelect from './approvers_select'; import ApproversSelect from './approvers_select';
...@@ -135,7 +134,6 @@ import AuditLogs from './audit_logs'; ...@@ -135,7 +134,6 @@ import AuditLogs from './audit_logs';
case 'sessions:new': case 'sessions:new':
new UsernameValidator(); new UsernameValidator();
new ActiveTabMemoizer(); new ActiveTabMemoizer();
new OAuthRememberMe({ container: $(".omniauth-container") }).bindEvents();
break; break;
case 'projects:boards:show': case 'projects:boards:show':
case 'projects:boards:index': case 'projects:boards:index':
......
/**
* 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', ''));
}
});
}
}
class OmniauthCallbacksController < Devise::OmniauthCallbacksController class OmniauthCallbacksController < Devise::OmniauthCallbacksController
include AuthenticatesWithTwoFactor include AuthenticatesWithTwoFactor
include Devise::Controllers::Rememberable
protect_from_forgery except: [:kerberos, :saml, :cas3] protect_from_forgery except: [:kerberos, :saml, :cas3]
...@@ -129,10 +128,8 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController ...@@ -129,10 +128,8 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
if @user.persisted? && @user.valid? if @user.persisted? && @user.valid?
log_audit_event(@user, with: oauth['provider']) log_audit_event(@user, with: oauth['provider'])
if @user.two_factor_enabled? if @user.two_factor_enabled?
params[:remember_me] = '1' if remember_me?
prompt_for_two_factor(@user) prompt_for_two_factor(@user)
else else
remember_me(@user) if remember_me?
sign_in_and_redirect(@user) sign_in_and_redirect(@user)
end end
else else
...@@ -163,9 +160,4 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController ...@@ -163,9 +160,4 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController
AuditEventService.new(user, user, options) AuditEventService.new(user, user, options)
.for_authentication.security_event .for_authentication.security_event
end end
def remember_me?
request_params = request.env['omniauth.params']
(request_params['remember_me'] == '1') if request_params.present?
end
end end
...@@ -6,7 +6,4 @@ ...@@ -6,7 +6,4 @@
- providers.each do |provider| - providers.each do |provider|
%span.light %span.light
- has_icon = provider_has_icon?(provider) - 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'
---
title: Honor the "Remember me" parameter for OAuth-based login
merge_request: 11963
author:
...@@ -716,53 +716,6 @@ test: ...@@ -716,53 +716,6 @@ test:
title: "JIRA" title: "JIRA"
url: https://sample_company.atlassian.net url: https://sample_company.atlassian.net
project_key: PROJECT 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: ldap:
enabled: false enabled: false
servers: servers:
......
...@@ -53,7 +53,8 @@ namespace :gitlab do ...@@ -53,7 +53,8 @@ namespace :gitlab do
'Undefined'.color(:red) 'Undefined'.color(:red)
end end
omniauth_providers = Gitlab.config.omniauth.providers.map { |provider| provider['name'] } omniauth_providers = Gitlab.config.omniauth.providers
omniauth_providers.map! { |provider| provider['name'] }
puts "" puts ""
puts "GitLab information".color(:yellow) puts "GitLab information".color(:yellow)
......
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
#oauth-container
%input#remember_me{ type: "checkbox" }
%a.oauth-login.twitter{ href: "http://example.com/" }
%a.oauth-login.github{ href: "http://example.com/" }
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/');
});
});
...@@ -35,11 +35,6 @@ module CapybaraHelpers ...@@ -35,11 +35,6 @@ module CapybaraHelpers
visit 'about:blank' visit 'about:blank'
visit url visit url
end end
# Simulate a browser restart by clearing the session cookie.
def clear_browser_session
page.driver.remove_cookie('_gitlab_session')
end
end end
RSpec.configure do |config| RSpec.configure do |config|
......
...@@ -62,16 +62,6 @@ module LoginHelpers ...@@ -62,16 +62,6 @@ module LoginHelpers
Thread.current[:current_user] = user Thread.current[:current_user] = user
end 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) def mock_auth_hash(provider, uid, email)
# The mock_auth configuration allows you to set per-provider (or default) # The mock_auth configuration allows you to set per-provider (or default)
# authentication hashes to return during integration testing. # authentication hashes to return during integration testing.
...@@ -118,7 +108,6 @@ module LoginHelpers ...@@ -118,7 +108,6 @@ module LoginHelpers
end end
allow(Gitlab::OAuth::Provider).to receive_messages(providers: [:saml], config_for: mock_saml_config) allow(Gitlab::OAuth::Provider).to receive_messages(providers: [:saml], config_for: mock_saml_config)
stub_omniauth_setting(messages) 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
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment