Merge branch 'master' into '41416-making-instance-wide-data-tools-more-accessible'

# Conflicts:
#   app/models/application_setting.rb
#   lib/api/settings.rb

2 files changed, 225 insertions, 36 deletions

diff --git a/spec/features/projects/wiki/user_views_wiki_page_spec.rb b/spec/features/projects/wiki/user_views_wiki_page_spec.rb
index 0ef7f35f64a..760324adacc 100644
--- a/spec/features/projects/wiki/user_views_wiki_page_spec.rb
+++ b/spec/features/projects/wiki/user_views_wiki_page_spec.rb
@@ -137,6 +137,26 @@ describe 'User views a wiki page' do
       end
     end
 
+    context 'when page has invalid content encoding' do
+      let(:content) { 'whatever'.force_encoding('ISO-8859-1') }
+
+      before do
+        allow(Gitlab::EncodingHelper).to receive(:encode!).and_return(content)
+
+        visit(project_wiki_path(project, wiki_page))
+      end
+
+      it 'does not show "Edit" button' do
+        expect(page).not_to have_selector('a.btn', text: 'Edit')
+      end
+
+      it 'shows error' do
+        page.within(:css, '.flash-notice') do
+          expect(page).to have_content('The content of this page is not encoded in UTF-8. Edits can only be made via the Git repository.')
+        end
+      end
+    end
+
     it 'opens a default wiki page', :js do
       visit(project_path(project))
 
diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb
index 21891b9ccda..44758f862a8 100644
--- a/spec/features/users/login_spec.rb
+++ b/spec/features/users/login_spec.rb
@@ -3,28 +3,40 @@ require 'spec_helper'
 describe 'Login' do
   include TermsHelper
 
-  it 'Successful user signin invalidates password reset token' do
-    user = create(:user)
+  before do
+    stub_authentication_activity_metrics(debug: true)
+  end
+
+  describe 'password reset token after successful sign in' do
+    it 'invalidates password reset token' do
+      expect(authentication_metrics)
+        .to increment(:user_authenticated_counter)
+
+      user = create(:user)
 
-    expect(user.reset_password_token).to be_nil
+      expect(user.reset_password_token).to be_nil
 
-    visit new_user_password_path
-    fill_in 'user_email', with: user.email
-    click_button 'Reset password'
+      visit new_user_password_path
+      fill_in 'user_email', with: user.email
+      click_button 'Reset password'
 
-    user.reload
-    expect(user.reset_password_token).not_to be_nil
+      user.reload
+      expect(user.reset_password_token).not_to be_nil
 
-    find('a[href="#login-pane"]').click
-    gitlab_sign_in(user)
-    expect(current_path).to eq root_path
+      find('a[href="#login-pane"]').click
+      gitlab_sign_in(user)
+      expect(current_path).to eq root_path
 
-    user.reload
-    expect(user.reset_password_token).to be_nil
+      user.reload
+      expect(user.reset_password_token).to be_nil
+    end
   end
 
   describe 'initial login after setup' do
     it 'allows the initial admin to create a password' do
+      expect(authentication_metrics)
+        .to increment(:user_authenticated_counter)
+
       # This behavior is dependent on there only being one user
       User.delete_all
 
@@ -56,6 +68,11 @@ describe 'Login' do
 
   describe 'with a blocked account' do
     it 'prevents the user from logging in' do
+      expect(authentication_metrics)
+        .to increment(:user_blocked_counter)
+        .and increment(:user_unauthenticated_counter)
+        .and increment(:user_session_destroyed_counter).twice
+
       user = create(:user, :blocked)
 
       gitlab_sign_in(user)
@@ -64,6 +81,11 @@ describe 'Login' do
     end
 
     it 'does not update Devise trackable attributes', :clean_gitlab_redis_shared_state do
+      expect(authentication_metrics)
+        .to increment(:user_blocked_counter)
+        .and increment(:user_unauthenticated_counter)
+        .and increment(:user_session_destroyed_counter).twice
+
       user = create(:user, :blocked)
 
       expect { gitlab_sign_in(user) }.not_to change { user.reload.sign_in_count }
@@ -72,13 +94,22 @@ describe 'Login' do
 
   describe 'with the ghost user' do
     it 'disallows login' do
+      expect(authentication_metrics)
+        .to increment(:user_unauthenticated_counter)
+        .and increment(:user_password_invalid_counter)
+
       gitlab_sign_in(User.ghost)
 
       expect(page).to have_content('Invalid Login or password.')
     end
 
     it 'does not update Devise trackable attributes', :clean_gitlab_redis_shared_state do
-      expect { gitlab_sign_in(User.ghost) }.not_to change { User.ghost.reload.sign_in_count }
+      expect(authentication_metrics)
+        .to increment(:user_unauthenticated_counter)
+        .and increment(:user_password_invalid_counter)
+
+      expect { gitlab_sign_in(User.ghost) }
+        .not_to change { User.ghost.reload.sign_in_count }
     end
   end
 
@@ -93,17 +124,30 @@ describe 'Login' do
 
       before do
         gitlab_sign_in(user, remember: true)
+
         expect(page).to have_content('Two-Factor Authentication')
       end
 
       it 'does not show a "You are already signed in." error message' do
+        expect(authentication_metrics)
+          .to increment(:user_authenticated_counter)
+          .and increment(:user_session_override_counter)
+          .and increment(:user_two_factor_authenticated_counter)
+
         enter_code(user.current_otp)
+
         expect(page).not_to have_content('You are already signed in.')
       end
 
       context 'using one-time code' do
         it 'allows login with valid code' do
+          expect(authentication_metrics)
+            .to increment(:user_authenticated_counter)
+            .and increment(:user_session_override_counter)
+            .and increment(:user_two_factor_authenticated_counter)
+
           enter_code(user.current_otp)
+
           expect(current_path).to eq root_path
         end
 
@@ -114,11 +158,20 @@ describe 'Login' do
         end
 
         it 'blocks login with invalid code' do
+          # TODO invalid 2FA code does not generate any events
+          # See gitlab-org/gitlab-ce#49785
+
           enter_code('foo')
+
           expect(page).to have_content('Invalid two-factor code')
         end
 
         it 'allows login with invalid code, then valid code' do
+          expect(authentication_metrics)
+            .to increment(:user_authenticated_counter)
+            .and increment(:user_session_override_counter)
+            .and increment(:user_two_factor_authenticated_counter)
+
           enter_code('foo')
           expect(page).to have_content('Invalid two-factor code')
 
@@ -139,16 +192,33 @@ describe 'Login' do
 
         context 'with valid code' do
           it 'allows login' do
+            expect(authentication_metrics)
+              .to increment(:user_authenticated_counter)
+              .and increment(:user_session_override_counter)
+              .and increment(:user_two_factor_authenticated_counter)
+
             enter_code(codes.sample)
+
             expect(current_path).to eq root_path
           end
 
           it 'invalidates the used code' do
+            expect(authentication_metrics)
+              .to increment(:user_authenticated_counter)
+              .and increment(:user_session_override_counter)
+              .and increment(:user_two_factor_authenticated_counter)
+
             expect { enter_code(codes.sample) }
               .to change { user.reload.otp_backup_codes.size }.by(-1)
           end
 
           it 'invalidates backup codes twice in a row' do
+            expect(authentication_metrics)
+              .to increment(:user_authenticated_counter).twice
+              .and increment(:user_session_override_counter).twice
+              .and increment(:user_two_factor_authenticated_counter).twice
+              .and increment(:user_session_destroyed_counter)
+
             random_code = codes.delete(codes.sample)
             expect { enter_code(random_code) }
               .to change { user.reload.otp_backup_codes.size }.by(-1)
@@ -163,6 +233,9 @@ describe 'Login' do
 
         context 'with invalid code' do
           it 'blocks login' do
+            # TODO, invalid two factor authentication does not increment
+            # metrics / counters, see gitlab-org/gitlab-ce#49785
+
             code = codes.sample
             expect(user.invalidate_otp_backup_code!(code)).to eq true
 
@@ -176,7 +249,7 @@ describe 'Login' do
       end
     end
 
-    context 'logging in via OAuth' do
+    context 'when logging in via OAuth' do
       let(:user) { create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: 'saml')}
       let(:mock_saml_response) do
         File.read('spec/fixtures/authentication/saml_response.xml')
@@ -185,49 +258,80 @@ describe 'Login' do
       before do
         stub_omniauth_saml_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'],
                                   providers: [mock_saml_config_with_upstream_two_factor_authn_contexts])
-        gitlab_sign_in_via('saml', user, 'my-uid', mock_saml_response)
       end
 
       context 'when authn_context is worth two factors' do
         let(:mock_saml_response) do
           File.read('spec/fixtures/authentication/saml_response.xml')
-              .gsub('urn:oasis:names:tc:SAML:2.0:ac:classes:Password', 'urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorOTPSMS')
+              .gsub('urn:oasis:names:tc:SAML:2.0:ac:classes:Password',
+                    'urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorOTPSMS')
         end
 
         it 'signs user in without prompting for second factor' do
+          # TODO, OAuth authentication does not fire events,
+          # see gitlab-org/gitlab-ce#49786
+
+          expect(authentication_metrics)
+            .to increment(:user_authenticated_counter)
+            .and increment(:user_session_override_counter)
+
+          sign_in_using_saml!
+
           expect(page).not_to have_content('Two-Factor Authentication')
           expect(current_path).to eq root_path
         end
       end
 
-      context 'when authn_context is not worth two factors' do
+      context 'when two factor authentication is required' do
         it 'shows 2FA prompt after OAuth login' do
+          expect(authentication_metrics)
+            .to increment(:user_authenticated_counter)
+            .and increment(:user_session_override_counter)
+            .and increment(:user_two_factor_authenticated_counter)
+
+          sign_in_using_saml!
+
           expect(page).to have_content('Two-Factor Authentication')
+
           enter_code(user.current_otp)
+
           expect(current_path).to eq root_path
         end
       end
+
+      def sign_in_using_saml!
+        gitlab_sign_in_via('saml', user, 'my-uid', mock_saml_response)
+      end
     end
   end
 
   describe 'without two-factor authentication' do
-    let(:user) { create(:user) }
+    context 'with correct username and password' do
+      let(:user) { create(:user) }
 
-    it 'allows basic login' do
-      gitlab_sign_in(user)
-      expect(current_path).to eq root_path
-    end
+      it 'allows basic login' do
+        expect(authentication_metrics)
+          .to increment(:user_authenticated_counter)
 
-    it 'does not show a "You are already signed in." error message' do
-      gitlab_sign_in(user)
-      expect(page).not_to have_content('You are already signed in.')
+        gitlab_sign_in(user)
+
+        expect(current_path).to eq root_path
+        expect(page).not_to have_content('You are already signed in.')
+      end
     end
 
-    it 'blocks invalid login' do
-      user = create(:user, password: 'not-the-default')
+    context 'with invalid username and password' do
+      let(:user) { create(:user, password: 'not-the-default') }
 
-      gitlab_sign_in(user)
-      expect(page).to have_content('Invalid Login or password.')
+      it 'blocks invalid login' do
+        expect(authentication_metrics)
+          .to increment(:user_unauthenticated_counter)
+          .and increment(:user_password_invalid_counter)
+
+        gitlab_sign_in(user)
+
+        expect(page).to have_content('Invalid Login or password.')
+      end
     end
   end
 
@@ -243,18 +347,26 @@ describe 'Login' do
       context 'with grace period defined' do
         before do
           stub_application_setting(two_factor_grace_period: 48)
-          gitlab_sign_in(user)
         end
 
         context 'within the grace period' do
           it 'redirects to two-factor configuration page' do
+            expect(authentication_metrics)
+              .to increment(:user_authenticated_counter)
+
+            gitlab_sign_in(user)
+
             expect(current_path).to eq profile_two_factor_auth_path
             expect(page).to have_content('The global settings require you to enable Two-Factor Authentication for your account. You need to do this before ')
           end
 
           it 'allows skipping two-factor configuration', :js do
-            expect(current_path).to eq profile_two_factor_auth_path
+            expect(authentication_metrics)
+              .to increment(:user_authenticated_counter)
+
+            gitlab_sign_in(user)
 
+            expect(current_path).to eq profile_two_factor_auth_path
             click_link 'Configure it later'
             expect(current_path).to eq root_path
           end
@@ -264,6 +376,11 @@ describe 'Login' do
           let(:user) { create(:user, otp_grace_period_started_at: 9999.hours.ago) }
 
           it 'redirects to two-factor configuration page' do
+            expect(authentication_metrics)
+              .to increment(:user_authenticated_counter)
+
+            gitlab_sign_in(user)
+
             expect(current_path).to eq profile_two_factor_auth_path
             expect(page).to have_content(
               'The global settings require you to enable Two-Factor Authentication for your account.'
@@ -271,6 +388,11 @@ describe 'Login' do
           end
 
           it 'disallows skipping two-factor configuration', :js do
+            expect(authentication_metrics)
+              .to increment(:user_authenticated_counter)
+
+            gitlab_sign_in(user)
+
             expect(current_path).to eq profile_two_factor_auth_path
             expect(page).not_to have_link('Configure it later')
           end
@@ -280,10 +402,14 @@ describe 'Login' do
       context 'without grace period defined' do
         before do
           stub_application_setting(two_factor_grace_period: 0)
-          gitlab_sign_in(user)
         end
 
         it 'redirects to two-factor configuration page' do
+          expect(authentication_metrics)
+            .to increment(:user_authenticated_counter)
+
+          gitlab_sign_in(user)
+
           expect(current_path).to eq profile_two_factor_auth_path
           expect(page).to have_content(
             'The global settings require you to enable Two-Factor Authentication for your account.'
@@ -303,11 +429,15 @@ describe 'Login' do
       context 'with grace period defined' do
         before do
           stub_application_setting(two_factor_grace_period: 48)
-          gitlab_sign_in(user)
         end
 
         context 'within the grace period' do
           it 'redirects to two-factor configuration page' do
+            expect(authentication_metrics)
+              .to increment(:user_authenticated_counter)
+
+            gitlab_sign_in(user)
+
             expect(current_path).to eq profile_two_factor_auth_path
             expect(page).to have_content(
               'The group settings for Group 1 and Group 2 require you to enable ' \
@@ -316,8 +446,12 @@ describe 'Login' do
           end
 
           it 'allows skipping two-factor configuration', :js do
-            expect(current_path).to eq profile_two_factor_auth_path
+            expect(authentication_metrics)
+              .to increment(:user_authenticated_counter)
 
+            gitlab_sign_in(user)
+
+            expect(current_path).to eq profile_two_factor_auth_path
             click_link 'Configure it later'
             expect(current_path).to eq root_path
           end
@@ -327,6 +461,11 @@ describe 'Login' do
           let(:user) { create(:user, otp_grace_period_started_at: 9999.hours.ago) }
 
           it 'redirects to two-factor configuration page' do
+            expect(authentication_metrics)
+              .to increment(:user_authenticated_counter)
+
+            gitlab_sign_in(user)
+
             expect(current_path).to eq profile_two_factor_auth_path
             expect(page).to have_content(
               'The group settings for Group 1 and Group 2 require you to enable ' \
@@ -335,6 +474,11 @@ describe 'Login' do
           end
 
           it 'disallows skipping two-factor configuration', :js do
+            expect(authentication_metrics)
+              .to increment(:user_authenticated_counter)
+
+            gitlab_sign_in(user)
+
             expect(current_path).to eq profile_two_factor_auth_path
             expect(page).not_to have_link('Configure it later')
           end
@@ -344,10 +488,14 @@ describe 'Login' do
       context 'without grace period defined' do
         before do
           stub_application_setting(two_factor_grace_period: 0)
-          gitlab_sign_in(user)
         end
 
         it 'redirects to two-factor configuration page' do
+          expect(authentication_metrics)
+            .to increment(:user_authenticated_counter)
+
+          gitlab_sign_in(user)
+
           expect(current_path).to eq profile_two_factor_auth_path
           expect(page).to have_content(
             'The group settings for Group 1 and Group 2 require you to enable ' \
@@ -431,6 +579,9 @@ describe 'Login' do
     end
 
     it 'asks to accept the terms on first login' do
+      expect(authentication_metrics)
+        .to increment(:user_authenticated_counter)
+
       visit new_user_session_path
 
       fill_in 'user_login', with: user.email
@@ -447,6 +598,9 @@ describe 'Login' do
     end
 
     it 'does not ask for terms when the user already accepted them' do
+      expect(authentication_metrics)
+        .to increment(:user_authenticated_counter)
+
       accept_terms(user)
 
       visit new_user_session_path
@@ -467,6 +621,9 @@ describe 'Login' do
 
       context 'when the user did not enable 2FA' do
         it 'asks to set 2FA before asking to accept the terms' do
+          expect(authentication_metrics)
+            .to increment(:user_authenticated_counter)
+
           visit new_user_session_path
 
           fill_in 'user_login', with: user.email
@@ -495,6 +652,11 @@ describe 'Login' do
         end
 
         it 'asks the user to accept the terms' do
+          expect(authentication_metrics)
+            .to increment(:user_authenticated_counter)
+            .and increment(:user_session_override_counter)
+            .and increment(:user_two_factor_authenticated_counter)
+
           visit new_user_session_path
 
           fill_in 'user_login', with: user.email
@@ -518,6 +680,9 @@ describe 'Login' do
       end
 
       it 'asks the user to accept the terms before setting a new password' do
+        expect(authentication_metrics)
+          .to increment(:user_authenticated_counter)
+
         visit new_user_session_path
 
         fill_in 'user_login', with: user.email
@@ -546,6 +711,10 @@ describe 'Login' do
       end
 
       it 'asks the user to accept the terms before setting an email' do
+        expect(authentication_metrics)
+          .to increment(:user_authenticated_counter)
+          .and increment(:user_session_override_counter)
+
         gitlab_sign_in_via('saml', user, 'my-uid')
 
         expect_to_be_on_terms_page