From abd24d1c145fce609f9caed27c39ba758571ebd9 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 11 May 2018 14:55:53 +0000 Subject: Merge branch 'bvl-terms-redirect-loop' into 'master' Enforce terms acceptance before configuring 2FA Closes #46256 See merge request gitlab-org/gitlab-ce!18896 --- app/controllers/users/terms_controller.rb | 4 ++ spec/features/users/login_spec.rb | 102 ++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+) diff --git a/app/controllers/users/terms_controller.rb b/app/controllers/users/terms_controller.rb index 95c5c3432d5..ab685b9106e 100644 --- a/app/controllers/users/terms_controller.rb +++ b/app/controllers/users/terms_controller.rb @@ -3,6 +3,10 @@ module Users include InternalRedirect skip_before_action :enforce_terms! + skip_before_action :check_password_expiration + skip_before_action :check_two_factor_requirement + skip_before_action :require_email + before_action :terms layout 'terms' diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb index 94a2b289e64..6f968a2c590 100644 --- a/spec/features/users/login_spec.rb +++ b/spec/features/users/login_spec.rb @@ -437,5 +437,107 @@ feature 'Login' do expect(current_path).to eq(root_path) end + + context 'when 2FA is required for the user' do + before do + group = create(:group, require_two_factor_authentication: true) + group.add_developer(user) + end + + context 'when the user did not enable 2FA' do + it 'asks to set 2FA before asking to accept the terms' do + visit new_user_session_path + + fill_in 'user_login', with: user.email + fill_in 'user_password', with: '12345678' + + click_button 'Sign in' + + expect_to_be_on_terms_page + click_button 'Accept terms' + + expect(current_path).to eq(profile_two_factor_auth_path) + + fill_in 'pin_code', with: user.reload.current_otp + + click_button 'Register with two-factor app' + click_link 'Proceed' + + expect(current_path).to eq(profile_account_path) + end + end + + context 'when the user already enabled 2FA' do + before do + user.update!(otp_required_for_login: true, + otp_secret: User.generate_otp_secret(32)) + end + + it 'asks the user to accept the terms' do + visit new_user_session_path + + fill_in 'user_login', with: user.email + fill_in 'user_password', with: '12345678' + click_button 'Sign in' + + fill_in 'user_otp_attempt', with: user.reload.current_otp + click_button 'Verify code' + + expect_to_be_on_terms_page + click_button 'Accept terms' + + expect(current_path).to eq(root_path) + end + end + end + + context 'when the users password is expired' do + before do + user.update!(password_expires_at: Time.parse('2018-05-08 11:29:46 UTC')) + end + + it 'asks the user to accept the terms before setting a new password' do + visit new_user_session_path + + fill_in 'user_login', with: user.email + fill_in 'user_password', with: '12345678' + click_button 'Sign in' + + expect_to_be_on_terms_page + click_button 'Accept terms' + + expect(current_path).to eq(new_profile_password_path) + + fill_in 'user_current_password', with: '12345678' + fill_in 'user_password', with: 'new password' + fill_in 'user_password_confirmation', with: 'new password' + click_button 'Set new password' + + expect(page).to have_content('Password successfully changed') + end + end + + context 'when the user does not have an email configured' do + let(:user) { create(:omniauth_user, extern_uid: 'my-uid', provider: 'saml', email: 'temp-email-for-oauth-user@gitlab.localhost') } + + before do + stub_omniauth_saml_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], providers: [mock_saml_config]) + end + + it 'asks the user to accept the terms before setting an email' do + gitlab_sign_in_via('saml', user, 'my-uid') + + expect_to_be_on_terms_page + click_button 'Accept terms' + + expect(current_path).to eq(profile_path) + + fill_in 'Email', with: 'hello@world.com' + + click_button 'Update profile settings' + + expect(page).to have_content('Profile was successfully updated') + end + end end end -- cgit v1.2.3