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

@@ -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'

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