Merge branch 'feature/enforce-2fa-per-group' into 'master'

Support 2FA requirement per-group

See merge request !8763

6 files changed, 85 insertions, 33 deletions

diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb
index cea3d088e94..f28bbdeff5a 100644
--- a/app/controllers/admin/groups_controller.rb
+++ b/app/controllers/admin/groups_controller.rb
@@ -72,7 +72,9 @@ class Admin::GroupsController < Admin::ApplicationController
       :name,
       :path,
       :request_access_enabled,
-      :visibility_level
+      :visibility_level,
+      :require_two_factor_authentication,
+      :two_factor_grace_period
     ]
   end
 end
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 6a6e335d314..e77094fe2a8 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -8,12 +8,12 @@ class ApplicationController < ActionController::Base
   include PageLayoutHelper
   include SentryHelper
   include WorkhorseHelper
+  include EnforcesTwoFactorAuthentication
 
   before_action :authenticate_user_from_private_token!
   before_action :authenticate_user!
   before_action :validate_user_service_ticket!
   before_action :check_password_expiration
-  before_action :check_2fa_requirement
   before_action :ldap_security_check
   before_action :sentry_context
   before_action :default_headers
@@ -151,12 +151,6 @@ class ApplicationController < ActionController::Base
     end
   end
 
-  def check_2fa_requirement
-    if two_factor_authentication_required? && current_user && !current_user.two_factor_enabled? && !skip_two_factor?
-      redirect_to profile_two_factor_auth_path
-    end
-  end
-
   def ldap_security_check
     if current_user && current_user.requires_ldap_check?
       return unless current_user.try_obtain_ldap_lease
@@ -265,23 +259,6 @@ class ApplicationController < ActionController::Base
     current_application_settings.import_sources.include?('gitlab_project')
   end
 
-  def two_factor_authentication_required?
-    current_application_settings.require_two_factor_authentication
-  end
-
-  def two_factor_grace_period
-    current_application_settings.two_factor_grace_period
-  end
-
-  def two_factor_grace_period_expired?
-    date = current_user.otp_grace_period_started_at
-    date && (date + two_factor_grace_period.hours) < Time.current
-  end
-
-  def skip_two_factor?
-    session[:skip_tfa] && session[:skip_tfa] > Time.current
-  end
-
   # U2F (universal 2nd factor) devices need a unique identifier for the application
   # to perform authentication.
   # https://developers.yubico.com/U2F/App_ID.html
diff --git a/app/controllers/concerns/enforces_two_factor_authentication.rb b/app/controllers/concerns/enforces_two_factor_authentication.rb
new file mode 100644
index 00000000000..688e8bd4a37
--- /dev/null
+++ b/app/controllers/concerns/enforces_two_factor_authentication.rb
@@ -0,0 +1,58 @@
+# == EnforcesTwoFactorAuthentication
+#
+# Controller concern to enforce two-factor authentication requirements
+#
+# Upon inclusion, adds `check_two_factor_requirement` as a before_action,
+# and makes `two_factor_grace_period_expired?` and `two_factor_skippable?`
+# available as view helpers.
+module EnforcesTwoFactorAuthentication
+  extend ActiveSupport::Concern
+
+  included do
+    before_action :check_two_factor_requirement
+    helper_method :two_factor_grace_period_expired?, :two_factor_skippable?
+  end
+
+  def check_two_factor_requirement
+    if two_factor_authentication_required? && current_user && !current_user.two_factor_enabled? && !skip_two_factor?
+      redirect_to profile_two_factor_auth_path
+    end
+  end
+
+  def two_factor_authentication_required?
+    current_application_settings.require_two_factor_authentication? ||
+      current_user.try(:require_two_factor_authentication_from_group?)
+  end
+
+  def two_factor_authentication_reason(global: -> {}, group: -> {})
+    if two_factor_authentication_required?
+      if current_application_settings.require_two_factor_authentication?
+        global.call
+      else
+        groups = current_user.expanded_groups_requiring_two_factor_authentication.reorder(name: :asc)
+        group.call(groups)
+      end
+    end
+  end
+
+  def two_factor_grace_period
+    periods = [current_application_settings.two_factor_grace_period]
+    periods << current_user.two_factor_grace_period if current_user.try(:require_two_factor_authentication_from_group?)
+    periods.min
+  end
+
+  def two_factor_grace_period_expired?
+    date = current_user.otp_grace_period_started_at
+    date && (date + two_factor_grace_period.hours) < Time.current
+  end
+
+  def two_factor_skippable?
+    two_factor_authentication_required? &&
+      !current_user.two_factor_enabled? &&
+      !two_factor_grace_period_expired?
+  end
+
+  def skip_two_factor?
+    session[:skip_two_factor] && session[:skip_two_factor] > Time.current
+  end
+end
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 78c9f1f7004..593001e6396 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -151,7 +151,9 @@ class GroupsController < Groups::ApplicationController
       :visibility_level,
       :parent_id,
       :create_chat_team,
-      :chat_team_name
+      :chat_team_name,
+      :require_two_factor_authentication,
+      :two_factor_grace_period
     ]
   end
 
diff --git a/app/controllers/profiles/two_factor_auths_controller.rb b/app/controllers/profiles/two_factor_auths_controller.rb
index 26e7e93533e..d3fa81cd623 100644
--- a/app/controllers/profiles/two_factor_auths_controller.rb
+++ b/app/controllers/profiles/two_factor_auths_controller.rb
@@ -1,5 +1,5 @@
 class Profiles::TwoFactorAuthsController < Profiles::ApplicationController
-  skip_before_action :check_2fa_requirement
+  skip_before_action :check_two_factor_requirement
 
   def show
     unless current_user.otp_secret
@@ -13,11 +13,24 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController
     current_user.save! if current_user.changed?
 
     if two_factor_authentication_required? && !current_user.two_factor_enabled?
-      if two_factor_grace_period_expired?
-        flash.now[:alert] = 'You must enable Two-Factor Authentication for your account.'
-      else
+      two_factor_authentication_reason(
+        global: lambda do
+          flash.now[:alert] =
+            'The global settings require you to enable Two-Factor Authentication for your account.'
+        end,
+        group: lambda do |groups|
+          group_links = groups.map { |group| view_context.link_to group.full_name, group_path(group) }.to_sentence
+
+          flash.now[:alert] = %{
+            The group settings for #{group_links} require you to enable
+            Two-Factor Authentication for your account.
+          }.html_safe
+        end
+      )
+
+      unless two_factor_grace_period_expired?
         grace_period_deadline = current_user.otp_grace_period_started_at + two_factor_grace_period.hours
-        flash.now[:alert] = "You must enable Two-Factor Authentication for your account before #{l(grace_period_deadline)}."
+        flash.now[:alert] << " You need to do this before #{l(grace_period_deadline)}."
       end
     end
 
@@ -71,7 +84,7 @@ class Profiles::TwoFactorAuthsController < Profiles::ApplicationController
     if two_factor_grace_period_expired?
       redirect_to new_profile_two_factor_auth_path, alert: 'Cannot skip two factor authentication setup'
     else
-      session[:skip_tfa] = current_user.otp_grace_period_started_at + two_factor_grace_period.hours
+      session[:skip_two_factor] = current_user.otp_grace_period_started_at + two_factor_grace_period.hours
       redirect_to root_path
     end
   end
diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb
index d8561871098..d3091a4f8e9 100644
--- a/app/controllers/sessions_controller.rb
+++ b/app/controllers/sessions_controller.rb
@@ -3,7 +3,7 @@ class SessionsController < Devise::SessionsController
   include Devise::Controllers::Rememberable
   include Recaptcha::ClientHelper
 
-  skip_before_action :check_2fa_requirement, only: [:destroy]
+  skip_before_action :check_two_factor_requirement, only: [:destroy]
 
   prepend_before_action :check_initial_setup, only: [:new]
   prepend_before_action :authenticate_with_two_factor,