Merge branch 'dblessing_known_sign_in_global_setting' into 'master'

Add global setting to disable/enable unknown sign in email

See merge request gitlab-org/gitlab!34562

app/controllers/concerns/known_sign_in.rb

@@ -10,7 +10,7 @@ module KnownSignIn
   private
 
   def verify_known_sign_in
-    return unless current_user
+    return unless Gitlab::CurrentSettings.notify_on_unknown_sign_in? && current_user
 
     notify_user unless known_device? || known_remote_ip?
 

app/helpers/application_settings_helper.rb

@@ -244,6 +244,7 @@ module ApplicationSettingsHelper
       :metrics_method_call_threshold,
       :minimum_password_length,
       :mirror_available,
+      :notify_on_unknown_sign_in,
       :pages_domain_verification_enabled,
       :password_authentication_enabled_for_web,
       :password_authentication_enabled_for_git,

app/models/application_setting_implementation.rb

@@ -88,6 +88,7 @@ module ApplicationSettingImplementation
         max_attachment_size: Settings.gitlab['max_attachment_size'],
         max_import_size: 50,
         mirror_available: true,
+        notify_on_unknown_sign_in: true,
         outbound_local_requests_whitelist: [],
         password_authentication_enabled_for_git: true,
         password_authentication_enabled_for_web: Settings.gitlab['signin_enabled'],

app/views/admin/application_settings/_signin.html.haml

@@ -32,6 +32,15 @@
         = f.check_box :require_two_factor_authentication, class: 'form-check-input'
         = f.label :require_two_factor_authentication, class: 'form-check-label' do
           Require all users to set up Two-factor authentication
+    .form-group
+      = f.label :unknown_sign_in, _('Email notification for unknown sign-ins'), class: 'label-bold'
+      .form-check
+        = f.check_box :notify_on_unknown_sign_in, class: 'form-check-input'
+        = f.label :notify_on_unknown_sign_in, class: 'form-check-label' do
+          = _('Notify users by email when sign-in location is not recognized')
+          = link_to icon('question-circle'),
+          'https://docs.gitlab.com/ee/user/profile/unknown_sign_in_notification.html',
+          target: '_blank'
     .form-group
       = f.label :two_factor_authentication, 'Two-factor grace period (hours)', class: 'label-bold'
       = f.number_field :two_factor_grace_period, min: 0, class: 'form-control', placeholder: '0'

changelogs/unreleased/dblessing_known_sign_in_global_setting.yml

+---
+title: Add global setting to disable/enable email notification on unknown sign-ins
+merge_request: 34562
+author:
+type: added

db/migrate/20200615193524_add_verify_known_sign_in_to_application_settings.rb

+# frozen_string_literal: true
+
+class AddVerifyKnownSignInToApplicationSettings < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def change
+    add_column :application_settings, :notify_on_unknown_sign_in, :boolean, default: true, null: false
+  end
+end

db/structure.sql

@@ -479,6 +479,7 @@ CREATE TABLE public.application_settings (
     max_import_size integer DEFAULT 50 NOT NULL,
     enforce_pat_expiration boolean DEFAULT true NOT NULL,
     compliance_frameworks smallint[] DEFAULT '{}'::smallint[] NOT NULL,
+    notify_on_unknown_sign_in boolean DEFAULT true NOT NULL,
     CONSTRAINT check_d03919528d CHECK ((char_length(container_registry_vendor) <= 255)),
     CONSTRAINT check_d820146492 CHECK ((char_length(spam_check_endpoint_url) <= 255)),
     CONSTRAINT check_e5aba18f02 CHECK ((char_length(container_registry_version) <= 255))
@@ -14077,6 +14078,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200615083635
 20200615121217
 20200615123055
+20200615193524
 20200615232735
 20200617000757
 20200617001001

doc/user/admin_area/settings/sign_in_restrictions.md

@@ -4,9 +4,14 @@ type: reference
 
 # Sign-in restrictions **(CORE ONLY)**
 
-You can use sign-in restrictions to limit the authentication with password
-for web interface and Git over HTTP(S), two-factor authentication enforcing, as well as
-as configuring the home page URL and after sign-out path.
+You can use **Sign-in restrictions** to customize authentication restrictions for web interfaces as well as Git over HTTP(S).
+
+## Settings
+
+To access sign-in restriction settings:
+
+1. Navigate to the **Admin Area > Settings > General**.
+1. Expand the **Sign-in restrictions** section.
 
 ## Password authentication enabled
 
@@ -25,6 +30,13 @@ period in hours.
 
 ![Two-factor grace period](img/two_factor_grace_period.png)
 
+## Email notification for unknown sign-ins
+
+When enabled, GitLab notifies users of sign-ins from unknown IP addresses or devices. For more information,
+see [Email notification for unknown sign-ins](../../profile/unknown_sign_in_notification.md).
+
+![Email notification for unknown sign-ins](img/email_notification_for_unknown_sign_ins_v13_2.png)
+
 ## Sign-in information
 
 All users that are not logged-in will be redirected to the page represented by the configured
@@ -36,13 +48,6 @@ after sign out if value is not empty.
 If a "Sign in text" in Markdown format is provided, then every user will be presented with
 this message after logging-in.
 
-## Settings
-
-To access this feature:
-
-1. Navigate to the **Admin Area > Settings > General**.
-1. Expand the **Sign-in restrictions** section.
-
 <!-- ## Troubleshooting
 
 Include any troubleshooting steps that you can foresee. If you know beforehand what issues

doc/user/profile/unknown_sign_in_notification.md

@@ -9,6 +9,11 @@ info: To determine the technical writer assigned to the Stage/Group associated w
 
 > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/27211) in GitLab 13.0.
 
+NOTE: **Note:**
+This feature is enabled by default for self-managed instances. Administrators may disable this feature
+through the [Sign-in restrictions](../admin_area/settings/sign_in_restrictions.md#email-notification-for-unknown-sign-ins) section of the UI.
+The feature is always enabled on GitLab.com.
+
 When a user successfully signs in from a previously unknown IP address or device,
 GitLab notifies the user by email. In this way, GitLab proactively alerts users of potentially
 malicious or unauthorized sign-ins.

locale/gitlab.pot

@@ -8279,6 +8279,9 @@ msgstr ""
 msgid "Email not verified. Please verify your email in Salesforce."
 msgstr ""
 
+msgid "Email notification for unknown sign-ins"
+msgstr ""
+
 msgid "Email patch"
 msgstr ""
 
@@ -15431,6 +15434,9 @@ msgstr ""
 msgid "Notifications on"
 msgstr ""
 
+msgid "Notify users by email when sign-in location is not recognized"
+msgstr ""
+
 msgid "Nov"
 msgstr ""
 

spec/support/shared_examples/controllers/known_sign_in_shared_examples.rb

@@ -22,7 +22,7 @@ RSpec.shared_examples 'known sign in' do
     end
 
     it 'does not notify the user' do
-      expect_any_instance_of(NotificationService).not_to receive(:unknown_sign_in)
+      expect(NotificationService).not_to receive(:new)
 
       post_action
     end
@@ -68,6 +68,24 @@ RSpec.shared_examples 'known sign in' do
       end
     end
 
+    context 'when notify_on_unknown_sign_in global setting is false' do
+      before do
+        stub_application_setting(notify_on_unknown_sign_in: false)
+      end
+
+      it 'does not notify the user' do
+        expect(NotificationService).not_to receive(:new)
+
+        post_action
+      end
+
+      it 'does not set a cookie' do
+        post_action
+
+        expect(cookies.encrypted[KnownSignIn::KNOWN_SIGN_IN_COOKIE]).to be_nil
+      end
+    end
+
     it 'notifies the user when the cookie is for another user' do
       stub_cookie(create(:user).id)
 
@@ -81,7 +99,7 @@ RSpec.shared_examples 'known sign in' do
     it 'does not notify the user when remote IP matches an active session' do
       ActiveSession.set(user, request)
 
-      expect_any_instance_of(NotificationService).not_to receive(:unknown_sign_in)
+      expect(NotificationService).not_to receive(:new)
 
       post_action
     end
@@ -89,7 +107,7 @@ RSpec.shared_examples 'known sign in' do
     it 'does not notify the user when the cookie is present and not expired' do
       stub_cookie
 
-      expect_any_instance_of(NotificationService).not_to receive(:unknown_sign_in)
+      expect(NotificationService).not_to receive(:new)
 
       post_action
     end