Commit 6df61942 authored by Sean McGivern's avatar Sean McGivern

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

Honor the "Remember me" parameter for OAuth-based login

Closes #18000

See merge request !11963
parents 3f0e37f6 2ae8158c
...@@ -56,6 +56,7 @@ import GfmAutoComplete from './gfm_auto_complete'; ...@@ -56,6 +56,7 @@ 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';
(function() { (function() {
var Dispatcher; var Dispatcher;
...@@ -127,6 +128,7 @@ import initExperimentalFlags from './experimental_flags'; ...@@ -127,6 +128,7 @@ import initExperimentalFlags from './experimental_flags';
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]
...@@ -115,8 +116,10 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController ...@@ -115,8 +116,10 @@ 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
...@@ -147,4 +150,9 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController ...@@ -147,4 +150,9 @@ 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,4 +6,7 @@ ...@@ -6,4 +6,7 @@
- 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: (has_icon ? 'oauth-image-link' : 'btn') = 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'
---
title: Honor the "Remember me" parameter for OAuth-based login
merge_request: 11963
author:
...@@ -619,6 +619,53 @@ test: ...@@ -619,6 +619,53 @@ 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:
......
...@@ -42,8 +42,7 @@ namespace :gitlab do ...@@ -42,8 +42,7 @@ namespace :gitlab do
http_clone_url = project.http_url_to_repo http_clone_url = project.http_url_to_repo
ssh_clone_url = project.ssh_url_to_repo ssh_clone_url = project.ssh_url_to_repo
omniauth_providers = Gitlab.config.omniauth.providers omniauth_providers = Gitlab.config.omniauth.providers.map { |provider| provider['name'] }
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,6 +35,11 @@ module CapybaraHelpers ...@@ -35,6 +35,11 @@ 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,6 +62,16 @@ module LoginHelpers ...@@ -62,6 +62,16 @@ 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.
...@@ -108,6 +118,7 @@ module LoginHelpers ...@@ -108,6 +118,7 @@ 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)
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(: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')
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