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

Support 2FA requirement per-group

See merge request !8763

27 files changed, 829 insertions, 92 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,
diff --git a/app/helpers/auth_helper.rb b/app/helpers/auth_helper.rb
index 101fe579da2..9c71d6c7f4c 100644
--- a/app/helpers/auth_helper.rb
+++ b/app/helpers/auth_helper.rb
@@ -64,18 +64,6 @@ module AuthHelper
     current_user.identities.exists?(provider: provider.to_s)
   end
 
-  def two_factor_skippable?
-    current_application_settings.require_two_factor_authentication &&
-      !current_user.two_factor_enabled? &&
-      current_application_settings.two_factor_grace_period &&
-      !two_factor_grace_period_expired?
-  end
-
-  def two_factor_grace_period_expired?
-    current_user.otp_grace_period_started_at &&
-      (current_user.otp_grace_period_started_at + current_application_settings.two_factor_grace_period.hours) < Time.current
-  end
-
   def unlink_allowed?(provider)
     %w(saml cas3).exclude?(provider.to_s)
   end
diff --git a/app/models/concerns/routable.rb b/app/models/concerns/routable.rb
index 529fb5ce988..aca99feee53 100644
--- a/app/models/concerns/routable.rb
+++ b/app/models/concerns/routable.rb
@@ -83,6 +83,74 @@ module Routable
                AND members.source_type = r2.source_type").
         where('members.user_id = ?', user_id)
     end
+
+    # Builds a relation to find multiple objects that are nested under user
+    # membership. Includes the parent, as opposed to `#member_descendants`
+    # which only includes the descendants.
+    #
+    # Usage:
+    #
+    #     Klass.member_self_and_descendants(1)
+    #
+    # Returns an ActiveRecord::Relation.
+    def member_self_and_descendants(user_id)
+      joins(:route).
+        joins("INNER JOIN routes r2 ON routes.path LIKE CONCAT(r2.path, '/%')
+               OR routes.path = r2.path
+               INNER JOIN members ON members.source_id = r2.source_id
+               AND members.source_type = r2.source_type").
+        where('members.user_id = ?', user_id)
+    end
+
+    # Returns all objects in a hierarchy, where any node in the hierarchy is
+    # under the user membership.
+    #
+    # Usage:
+    #
+    #     Klass.member_hierarchy(1)
+    #
+    # Examples:
+    #
+    #     Given the following group tree...
+    #
+    #            _______group_1_______
+    #           |                     |
+    #           |                     |
+    #     nested_group_1        nested_group_2
+    #           |                     |
+    #           |                     |
+    #     nested_group_1_1      nested_group_2_1
+    #
+    #
+    #     ... the following results are returned:
+    #
+    #     * the user is a member of group 1
+    #       => 'group_1',
+    #          'nested_group_1', nested_group_1_1',
+    #          'nested_group_2', 'nested_group_2_1'
+    #
+    #     * the user is a member of nested_group_2
+    #       => 'group1',
+    #          'nested_group_2', 'nested_group_2_1'
+    #
+    #     * the user is a member of nested_group_2_1
+    #       => 'group1',
+    #          'nested_group_2', 'nested_group_2_1'
+    #
+    # Returns an ActiveRecord::Relation.
+    def member_hierarchy(user_id)
+      paths = member_self_and_descendants(user_id).pluck('routes.path')
+
+      return none if paths.empty?
+
+      wheres = paths.map do |path|
+        "#{connection.quote(path)} = routes.path
+         OR
+         #{connection.quote(path)} LIKE CONCAT(routes.path, '/%')"
+      end
+
+      joins(:route).where(wheres.join(' OR '))
+    end
   end
 
   def full_name
diff --git a/app/models/group.rb b/app/models/group.rb
index 60274386103..106084175ff 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -27,11 +27,14 @@ class Group < Namespace
 
   validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
 
+  validates :two_factor_grace_period, presence: true, numericality: { greater_than_or_equal_to: 0 }
+
   mount_uploader :avatar, AvatarUploader
   has_many :uploads, as: :model, dependent: :destroy
 
   after_create :post_create_hook
   after_destroy :post_destroy_hook
+  after_save :update_two_factor_requirement
 
   class << self
     # Searches for groups matching the given query.
@@ -223,4 +226,12 @@ class Group < Namespace
       type: public? ? 'O' : 'I' # Open vs Invite-only
     }
   end
+
+  protected
+
+  def update_two_factor_requirement
+    return unless require_two_factor_authentication_changed? || two_factor_grace_period_changed?
+
+    users.find_each(&:update_two_factor_requirement)
+  end
 end
diff --git a/app/models/members/group_member.rb b/app/models/members/group_member.rb
index 446f9f8f8a7..483425cd30f 100644
--- a/app/models/members/group_member.rb
+++ b/app/models/members/group_member.rb
@@ -3,11 +3,16 @@ class GroupMember < Member
 
   belongs_to :group, foreign_key: 'source_id'
 
+  delegate :update_two_factor_requirement, to: :user
+
   # Make sure group member points only to group as it source
   default_value_for :source_type, SOURCE_TYPE
   validates :source_type, format: { with: /\ANamespace\z/ }
   default_scope { where(source_type: SOURCE_TYPE) }
 
+  after_create :update_two_factor_requirement, unless: :invite?
+  after_destroy :update_two_factor_requirement, unless: :invite?
+
   def self.access_level_roles
     Gitlab::Access.options_with_owner
   end
diff --git a/app/models/user.rb b/app/models/user.rb
index 95a766f2ede..c358b1b8d2b 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -484,6 +484,14 @@ class User < ActiveRecord::Base
     Group.member_descendants(id)
   end
 
+  def all_expanded_groups
+    Group.member_hierarchy(id)
+  end
+
+  def expanded_groups_requiring_two_factor_authentication
+    all_expanded_groups.where(require_two_factor_authentication: true)
+  end
+
   def nested_groups_projects
     Project.joins(:namespace).where('namespaces.parent_id IS NOT NULL').
       member_descendants(id)
@@ -955,6 +963,15 @@ class User < ActiveRecord::Base
     self.admin = (new_level == 'admin')
   end
 
+  def update_two_factor_requirement
+    periods = expanded_groups_requiring_two_factor_authentication.pluck(:two_factor_grace_period)
+
+    self.require_two_factor_authentication_from_group = periods.any?
+    self.two_factor_grace_period = periods.min || User.column_defaults['two_factor_grace_period']
+
+    save
+  end
+
   protected
 
   # override, from Devise::Validatable
diff --git a/app/views/admin/groups/_form.html.haml b/app/views/admin/groups/_form.html.haml
index 589f4557b52..d9f05003904 100644
--- a/app/views/admin/groups/_form.html.haml
+++ b/app/views/admin/groups/_form.html.haml
@@ -13,7 +13,7 @@
     .col-sm-offset-2.col-sm-10
       = render 'shared/allow_request_access', form: f
 
-  = render 'groups/group_lfs_settings', f: f
+  = render 'groups/group_admin_settings', f: f
 
   - if @group.new_record?
     .form-group
diff --git a/app/views/groups/_group_admin_settings.html.haml b/app/views/groups/_group_admin_settings.html.haml
new file mode 100644
index 00000000000..2ace1e2dd1e
--- /dev/null
+++ b/app/views/groups/_group_admin_settings.html.haml
@@ -0,0 +1,28 @@
+- if current_user.admin?
+  .form-group
+    = f.label :lfs_enabled, 'Large File Storage', class: 'control-label'
+    .col-sm-10
+      .checkbox
+        = f.label :lfs_enabled do
+          = f.check_box :lfs_enabled, checked: @group.lfs_enabled?
+          %strong
+            Allow projects within this group to use Git LFS
+            = link_to icon('question-circle'), help_page_path('workflow/lfs/manage_large_binaries_with_git_lfs')
+          %br/
+          %span.descr This setting can be overridden in each project.
+
+- if can? current_user, :admin_group, @group
+  .form-group
+    = f.label :require_two_factor_authentication, 'Two-factor authentication', class: 'control-label col-sm-2'
+    .col-sm-10
+      .checkbox
+        = f.label :require_two_factor_authentication do
+          = f.check_box :require_two_factor_authentication
+          %strong
+            Require all users in this group to setup Two-factor authentication
+            = link_to icon('question-circle'), help_page_path('security/two_factor_authentication', anchor: 'enforcing-2fa-for-all-users-in-a-group')
+  .form-group
+    .col-sm-offset-2.col-sm-10
+      .checkbox
+        = f.text_field :two_factor_grace_period, class: 'form-control'
+        .help-block Amount of time (in hours) that users are allowed to skip forced configuration of two-factor authentication
diff --git a/app/views/groups/_group_lfs_settings.html.haml b/app/views/groups/_group_lfs_settings.html.haml
deleted file mode 100644
index 3c622ca5c3c..00000000000
--- a/app/views/groups/_group_lfs_settings.html.haml
+++ /dev/null
@@ -1,11 +0,0 @@
-- if current_user.admin?
-  .form-group
-    .col-sm-offset-2.col-sm-10
-      .checkbox
-        = f.label :lfs_enabled do
-          = f.check_box :lfs_enabled, checked: @group.lfs_enabled?
-          %strong
-            Allow projects within this group to use Git LFS
-            = link_to icon('question-circle'), help_page_path('workflow/lfs/manage_large_binaries_with_git_lfs')
-          %br/
-          %span.descr This setting can be overridden in each project.
diff --git a/app/views/groups/edit.html.haml b/app/views/groups/edit.html.haml
index 80a77dab97f..00ff40224ba 100644
--- a/app/views/groups/edit.html.haml
+++ b/app/views/groups/edit.html.haml
@@ -27,7 +27,7 @@
         .col-sm-offset-2.col-sm-10
           = render 'shared/allow_request_access', form: f
 
-      = render 'group_lfs_settings', f: f
+      = render 'group_admin_settings', f: f
 
       .form-group
         %hr
diff --git a/changelogs/unreleased/feature-enforce-2fa-per-group.yml b/changelogs/unreleased/feature-enforce-2fa-per-group.yml
new file mode 100644
index 00000000000..6dd99e4245f
--- /dev/null
+++ b/changelogs/unreleased/feature-enforce-2fa-per-group.yml
@@ -0,0 +1,4 @@
+---
+title: Support 2FA requirement per-group
+merge_request: 8763
+author: Markus Koller
diff --git a/db/migrate/20170124193147_add_two_factor_columns_to_namespaces.rb b/db/migrate/20170124193147_add_two_factor_columns_to_namespaces.rb
new file mode 100644
index 00000000000..ca4429c676c
--- /dev/null
+++ b/db/migrate/20170124193147_add_two_factor_columns_to_namespaces.rb
@@ -0,0 +1,21 @@
+class AddTwoFactorColumnsToNamespaces < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_column_with_default(:namespaces, :require_two_factor_authentication, :boolean, default: false)
+    add_column_with_default(:namespaces, :two_factor_grace_period, :integer, default: 48)
+
+    add_concurrent_index(:namespaces, :require_two_factor_authentication)
+  end
+
+  def down
+    remove_column(:namespaces, :require_two_factor_authentication)
+    remove_column(:namespaces, :two_factor_grace_period)
+
+    remove_index(:namespaces, :require_two_factor_authentication) if index_exists?(:namespaces, :require_two_factor_authentication)
+  end
+end
diff --git a/db/migrate/20170124193205_add_two_factor_columns_to_users.rb b/db/migrate/20170124193205_add_two_factor_columns_to_users.rb
new file mode 100644
index 00000000000..1d1021fcbb3
--- /dev/null
+++ b/db/migrate/20170124193205_add_two_factor_columns_to_users.rb
@@ -0,0 +1,17 @@
+class AddTwoFactorColumnsToUsers < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_column_with_default(:users, :require_two_factor_authentication_from_group, :boolean, default: false)
+    add_column_with_default(:users, :two_factor_grace_period, :integer, default: 48)
+  end
+
+  def down
+    remove_column(:users, :require_two_factor_authentication_from_group)
+    remove_column(:users, :two_factor_grace_period)
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 582f68cbee7..5ea307911f5 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -692,6 +692,8 @@ ActiveRecord::Schema.define(version: 20170405080720) do
     t.text "description_html"
     t.boolean "lfs_enabled"
     t.integer "parent_id"
+    t.boolean "require_two_factor_authentication", default: false, null: false
+    t.integer "two_factor_grace_period", default: 48, null: false
   end
 
   add_index "namespaces", ["created_at"], name: "index_namespaces_on_created_at", using: :btree
@@ -702,6 +704,7 @@ ActiveRecord::Schema.define(version: 20170405080720) do
   add_index "namespaces", ["parent_id", "id"], name: "index_namespaces_on_parent_id_and_id", unique: true, using: :btree
   add_index "namespaces", ["path"], name: "index_namespaces_on_path", using: :btree
   add_index "namespaces", ["path"], name: "index_namespaces_on_path_trigram", using: :gin, opclasses: {"path"=>"gin_trgm_ops"}
+  add_index "namespaces", ["require_two_factor_authentication"], name: "index_namespaces_on_require_two_factor_authentication", using: :btree
   add_index "namespaces", ["type"], name: "index_namespaces_on_type", using: :btree
 
   create_table "notes", force: :cascade do |t|
@@ -1247,6 +1250,8 @@ ActiveRecord::Schema.define(version: 20170405080720) do
     t.boolean "authorized_projects_populated"
     t.boolean "ghost"
     t.boolean "notified_of_own_activity"
+    t.boolean "require_two_factor_authentication_from_group", default: false, null: false
+    t.integer "two_factor_grace_period", default: 48, null: false
   end
 
   add_index "users", ["admin"], name: "index_users_on_admin", using: :btree
diff --git a/doc/security/img/two_factor_authentication_group_settings.png b/doc/security/img/two_factor_authentication_group_settings.png
new file mode 100644
index 00000000000..a1b3c58bfdc
--- /dev/null
+++ b/doc/security/img/two_factor_authentication_group_settings.png
diff --git a/doc/security/two_factor_authentication.md b/doc/security/two_factor_authentication.md
index c8499380c18..f02f7b807cf 100644
--- a/doc/security/two_factor_authentication.md
+++ b/doc/security/two_factor_authentication.md
@@ -8,7 +8,7 @@ their phone.
 You can read more about it here:
 [Two-factor Authentication (2FA)](../profile/two_factor_authentication.md)
 
-## Enabling 2FA
+## Enforcing 2FA for all users
 
 Users on GitLab, can enable it without any admin's intervention. If you want to
 enforce everyone to setup 2FA, you can choose from two different ways:
@@ -28,6 +28,21 @@ period to `0`.
 
 ---
 
+## Enforcing 2FA for all users in a group
+
+If you want to enforce 2FA only for certain groups, you can enable it in the
+group settings and specify a grace period as above. To change this setting you
+need to be administrator or owner of the group.
+
+If there are multiple 2FA requirements (i.e. group + all users, or multiple
+groups) the shortest grace period will be used.
+
+---
+
+![Two factor authentication group settings](img/two_factor_authentication_group_settings.png)
+
+---
+
 ## Disabling 2FA for everyone
 
 There may be some special situations where you want to disable 2FA for everyone
diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb
index 81cbccd5436..760f33b09c1 100644
--- a/spec/controllers/application_controller_spec.rb
+++ b/spec/controllers/application_controller_spec.rb
@@ -100,8 +100,6 @@ describe ApplicationController do
   end
 
   describe '#route_not_found' do
-    let(:controller) { ApplicationController.new }
-
     it 'renders 404 if authenticated' do
       allow(controller).to receive(:current_user).and_return(user)
       expect(controller).to receive(:not_found)
@@ -115,4 +113,203 @@ describe ApplicationController do
       controller.send(:route_not_found)
     end
   end
+
+  context 'two-factor authentication' do
+    let(:controller) { ApplicationController.new }
+
+    describe '#check_two_factor_requirement' do
+      subject { controller.send :check_two_factor_requirement }
+
+      it 'does not redirect if 2FA is not required' do
+        allow(controller).to receive(:two_factor_authentication_required?).and_return(false)
+        expect(controller).not_to receive(:redirect_to)
+
+        subject
+      end
+
+      it 'does not redirect if user is not logged in' do
+        allow(controller).to receive(:two_factor_authentication_required?).and_return(true)
+        allow(controller).to receive(:current_user).and_return(nil)
+        expect(controller).not_to receive(:redirect_to)
+
+        subject
+      end
+
+      it 'does not redirect if user has 2FA enabled' do
+        allow(controller).to receive(:two_factor_authentication_required?).and_return(true)
+        allow(controller).to receive(:current_user).twice.and_return(user)
+        allow(user).to receive(:two_factor_enabled?).and_return(true)
+        expect(controller).not_to receive(:redirect_to)
+
+        subject
+      end
+
+      it 'does not redirect if 2FA setup can be skipped' do
+        allow(controller).to receive(:two_factor_authentication_required?).and_return(true)
+        allow(controller).to receive(:current_user).twice.and_return(user)
+        allow(user).to receive(:two_factor_enabled?).and_return(false)
+        allow(controller).to receive(:skip_two_factor?).and_return(true)
+        expect(controller).not_to receive(:redirect_to)
+
+        subject
+      end
+
+      it 'redirects to 2FA setup otherwise' do
+        allow(controller).to receive(:two_factor_authentication_required?).and_return(true)
+        allow(controller).to receive(:current_user).twice.and_return(user)
+        allow(user).to receive(:two_factor_enabled?).and_return(false)
+        allow(controller).to receive(:skip_two_factor?).and_return(false)
+        allow(controller).to receive(:profile_two_factor_auth_path)
+        expect(controller).to receive(:redirect_to)
+
+        subject
+      end
+    end
+
+    describe '#two_factor_authentication_required?' do
+      subject { controller.send :two_factor_authentication_required? }
+
+      it 'returns false if no 2FA requirement is present' do
+        allow(controller).to receive(:current_user).and_return(nil)
+
+        expect(subject).to be_falsey
+      end
+
+      it 'returns true if a 2FA requirement is set in the application settings' do
+        stub_application_setting require_two_factor_authentication: true
+        allow(controller).to receive(:current_user).and_return(nil)
+
+        expect(subject).to be_truthy
+      end
+
+      it 'returns true if a 2FA requirement is set on the user' do
+        user.require_two_factor_authentication_from_group = true
+        allow(controller).to receive(:current_user).and_return(user)
+
+        expect(subject).to be_truthy
+      end
+    end
+
+    describe '#two_factor_grace_period' do
+      subject { controller.send :two_factor_grace_period }
+
+      it 'returns the grace period from the application settings' do
+        stub_application_setting two_factor_grace_period: 23
+        allow(controller).to receive(:current_user).and_return(nil)
+
+        expect(subject).to eq 23
+      end
+
+      context 'with a 2FA requirement set on the user' do
+        let(:user) { create :user, require_two_factor_authentication_from_group: true, two_factor_grace_period: 23 }
+
+        it 'returns the user grace period if lower than the application grace period' do
+          stub_application_setting two_factor_grace_period: 24
+          allow(controller).to receive(:current_user).and_return(user)
+
+          expect(subject).to eq 23
+        end
+
+        it 'returns the application grace period if lower than the user grace period' do
+          stub_application_setting two_factor_grace_period: 22
+          allow(controller).to receive(:current_user).and_return(user)
+
+          expect(subject).to eq 22
+        end
+      end
+    end
+
+    describe '#two_factor_grace_period_expired?' do
+      subject { controller.send :two_factor_grace_period_expired? }
+
+      before do
+        allow(controller).to receive(:current_user).and_return(user)
+      end
+
+      it 'returns false if the user has not started their grace period yet' do
+        expect(subject).to be_falsey
+      end
+
+      context 'with grace period started' do
+        let(:user) { create :user, otp_grace_period_started_at: 2.hours.ago }
+
+        it 'returns true if the grace period has expired' do
+          allow(controller).to receive(:two_factor_grace_period).and_return(1)
+
+          expect(subject).to be_truthy
+        end
+
+        it 'returns false if the grace period is still active' do
+          allow(controller).to receive(:two_factor_grace_period).and_return(3)
+
+          expect(subject).to be_falsey
+        end
+      end
+    end
+
+    describe '#two_factor_skippable' do
+      subject { controller.send :two_factor_skippable? }
+
+      before do
+        allow(controller).to receive(:current_user).and_return(user)
+      end
+
+      it 'returns false if 2FA is not required' do
+        allow(controller).to receive(:two_factor_authentication_required?).and_return(false)
+
+        expect(subject).to be_falsey
+      end
+
+      it 'returns false if the user has already enabled 2FA' do
+        allow(controller).to receive(:two_factor_authentication_required?).and_return(true)
+        allow(user).to receive(:two_factor_enabled?).and_return(true)
+
+        expect(subject).to be_falsey
+      end
+
+      it 'returns false if the 2FA grace period has expired' do
+        allow(controller).to receive(:two_factor_authentication_required?).and_return(true)
+        allow(user).to receive(:two_factor_enabled?).and_return(false)
+        allow(controller).to receive(:two_factor_grace_period_expired?).and_return(true)
+
+        expect(subject).to be_falsey
+      end
+
+      it 'returns true otherwise' do
+        allow(controller).to receive(:two_factor_authentication_required?).and_return(true)
+        allow(user).to receive(:two_factor_enabled?).and_return(false)
+        allow(controller).to receive(:two_factor_grace_period_expired?).and_return(false)
+
+        expect(subject).to be_truthy
+      end
+    end
+
+    describe '#skip_two_factor?' do
+      subject { controller.send :skip_two_factor? }
+
+      it 'returns false if 2FA setup was not skipped' do
+        allow(controller).to receive(:session).and_return({})
+
+        expect(subject).to be_falsey
+      end
+
+      context 'with 2FA setup skipped' do
+        before do
+          allow(controller).to receive(:session).and_return({ skip_two_factor: 2.hours.from_now })
+        end
+
+        it 'returns false if the grace period has expired' do
+          Timecop.freeze(3.hours.from_now) do
+            expect(subject).to be_falsey
+          end
+        end
+
+        it 'returns true if the grace period is still active' do
+          Timecop.freeze(1.hour.from_now) do
+            expect(subject).to be_truthy
+          end
+        end
+      end
+    end
+  end
 end
diff --git a/spec/features/login_spec.rb b/spec/features/login_spec.rb
index f32d1f78b40..11d417c253d 100644
--- a/spec/features/login_spec.rb
+++ b/spec/features/login_spec.rb
@@ -199,52 +199,125 @@ feature 'Login', feature: true do
 
   describe 'with required two-factor authentication enabled' do
     let(:user) { create(:user) }
-    before(:each) { stub_application_setting(require_two_factor_authentication: true) }
+    #  TODO: otp_grace_period_started_at
 
-    context 'with grace period defined' do
-      before(:each) do
-        stub_application_setting(two_factor_grace_period: 48)
-        login_with(user)
-      end
+    context 'global setting' do
+      before(:each) { stub_application_setting(require_two_factor_authentication: true) }
 
-      context 'within the grace period' do
-        it 'redirects to two-factor configuration page' do
-          expect(current_path).to eq profile_two_factor_auth_path
-          expect(page).to have_content('You must enable Two-Factor Authentication for your account before')
+      context 'with grace period defined' do
+        before(:each) do
+          stub_application_setting(two_factor_grace_period: 48)
+          login_with(user)
         end
 
-        it 'allows skipping two-factor configuration', js: true do
-          expect(current_path).to eq profile_two_factor_auth_path
+        context 'within the grace period' do
+          it 'redirects to two-factor configuration page' do
+            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
 
-          click_link 'Configure it later'
-          expect(current_path).to eq root_path
+          it 'allows skipping two-factor configuration', js: true do
+            expect(current_path).to eq profile_two_factor_auth_path
+
+            click_link 'Configure it later'
+            expect(current_path).to eq root_path
+          end
         end
-      end
 
-      context 'after the grace period' do
-        let(:user) { create(:user, otp_grace_period_started_at: 9999.hours.ago) }
+        context 'after the grace period' do
+          let(:user) { create(:user, otp_grace_period_started_at: 9999.hours.ago) }
 
-        it 'redirects to two-factor configuration page' do
-          expect(current_path).to eq profile_two_factor_auth_path
-          expect(page).to have_content('You must enable Two-Factor Authentication for your account.')
+          it 'redirects to two-factor configuration page' do
+            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.'
+            )
+          end
+
+          it 'disallows skipping two-factor configuration', js: true do
+            expect(current_path).to eq profile_two_factor_auth_path
+            expect(page).not_to have_link('Configure it later')
+          end
+        end
+      end
+
+      context 'without grace period defined' do
+        before(:each) do
+          stub_application_setting(two_factor_grace_period: 0)
+          login_with(user)
         end
 
-        it 'disallows skipping two-factor configuration', js: true do
+        it 'redirects to two-factor configuration page' do
           expect(current_path).to eq profile_two_factor_auth_path
-          expect(page).not_to have_link('Configure it later')
+          expect(page).to have_content(
+            'The global settings require you to enable Two-Factor Authentication for your account.'
+          )
         end
       end
     end
 
-    context 'without grace period defined' do
-      before(:each) do
-        stub_application_setting(two_factor_grace_period: 0)
-        login_with(user)
+    context 'group setting' do
+      before do
+        group1 = create :group, name: 'Group 1', require_two_factor_authentication: true
+        group1.add_user(user, GroupMember::DEVELOPER)
+        group2 = create :group, name: 'Group 2', require_two_factor_authentication: true
+        group2.add_user(user, GroupMember::DEVELOPER)
       end
 
-      it 'redirects to two-factor configuration page' do
-        expect(current_path).to eq profile_two_factor_auth_path
-        expect(page).to have_content('You must enable Two-Factor Authentication for your account.')
+      context 'with grace period defined' do
+        before(:each) do
+          stub_application_setting(two_factor_grace_period: 48)
+          login_with(user)
+        end
+
+        context 'within the grace period' do
+          it 'redirects to two-factor configuration page' do
+            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 ' \
+              'Two-Factor Authentication for your account. You need to do this ' \
+              'before ')
+          end
+
+          it 'allows skipping two-factor configuration', js: true do
+            expect(current_path).to eq profile_two_factor_auth_path
+
+            click_link 'Configure it later'
+            expect(current_path).to eq root_path
+          end
+        end
+
+        context 'after the grace period' do
+          let(:user) { create(:user, otp_grace_period_started_at: 9999.hours.ago) }
+
+          it 'redirects to two-factor configuration page' do
+            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 ' \
+              'Two-Factor Authentication for your account.'
+            )
+          end
+
+          it 'disallows skipping two-factor configuration', js: true do
+            expect(current_path).to eq profile_two_factor_auth_path
+            expect(page).not_to have_link('Configure it later')
+          end
+        end
+      end
+
+      context 'without grace period defined' do
+        before(:each) do
+          stub_application_setting(two_factor_grace_period: 0)
+          login_with(user)
+        end
+
+        it 'redirects to two-factor configuration page' do
+          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 ' \
+            'Two-Factor Authentication for your account.'
+          )
+        end
       end
     end
   end
diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb
index 677e60e1282..f191605dbdb 100644
--- a/spec/models/concerns/routable_spec.rb
+++ b/spec/models/concerns/routable_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Group, 'Routable' do
-  let!(:group) { create(:group) }
+  let!(:group) { create(:group, name: 'foo') }
 
   describe 'Validations' do
     it { is_expected.to validate_presence_of(:route) }
@@ -81,6 +81,113 @@ describe Group, 'Routable' do
     it { is_expected.to eq([nested_group]) }
   end
 
+  describe '.member_self_and_descendants' do
+    let!(:user) { create(:user) }
+    let!(:nested_group) { create(:group, parent: group) }
+
+    before { group.add_owner(user) }
+    subject { described_class.member_self_and_descendants(user.id) }
+
+    it { is_expected.to match_array [group, nested_group] }
+  end
+
+  describe '.member_hierarchy' do
+    # foo/bar would also match foo/barbaz instead of just foo/bar and foo/bar/baz
+    let!(:user) { create(:user) }
+
+    #                group
+    #        _______ (foo) _______
+    #       |                     |
+    #       |                     |
+    # nested_group_1        nested_group_2
+    # (bar)                 (barbaz)
+    #       |                     |
+    #       |                     |
+    # nested_group_1_1      nested_group_2_1
+    # (baz)                 (baz)
+    #
+    let!(:nested_group_1) { create :group, parent: group, name: 'bar' }
+    let!(:nested_group_1_1) { create :group, parent: nested_group_1, name: 'baz' }
+    let!(:nested_group_2) { create :group, parent: group, name: 'barbaz' }
+    let!(:nested_group_2_1) { create :group, parent: nested_group_2, name: 'baz' }
+
+    context 'user is not a member of any group' do
+      subject { described_class.member_hierarchy(user.id) }
+
+      it 'returns an empty array' do
+        is_expected.to eq []
+      end
+    end
+
+    context 'user is member of all groups' do
+      before do
+        group.add_owner(user)
+        nested_group_1.add_owner(user)
+        nested_group_1_1.add_owner(user)
+        nested_group_2.add_owner(user)
+        nested_group_2_1.add_owner(user)
+      end
+      subject { described_class.member_hierarchy(user.id) }
+
+      it 'returns all groups' do
+        is_expected.to match_array [
+          group,
+          nested_group_1, nested_group_1_1,
+          nested_group_2, nested_group_2_1
+        ]
+      end
+    end
+
+    context 'user is member of the top group' do
+      before { group.add_owner(user) }
+      subject { described_class.member_hierarchy(user.id) }
+
+      it 'returns all groups' do
+        is_expected.to match_array [
+          group,
+          nested_group_1, nested_group_1_1,
+          nested_group_2, nested_group_2_1
+        ]
+      end
+    end
+
+    context 'user is member of the first child (internal node), branch 1' do
+      before { nested_group_1.add_owner(user) }
+      subject { described_class.member_hierarchy(user.id) }
+
+      it 'returns the groups in the hierarchy' do
+        is_expected.to match_array [
+          group,
+          nested_group_1, nested_group_1_1
+        ]
+      end
+    end
+
+    context 'user is member of the first child (internal node), branch 2' do
+      before { nested_group_2.add_owner(user) }
+      subject { described_class.member_hierarchy(user.id) }
+
+      it 'returns the groups in the hierarchy' do
+        is_expected.to match_array [
+          group,
+          nested_group_2, nested_group_2_1
+        ]
+      end
+    end
+
+    context 'user is member of the last child (leaf node)' do
+      before { nested_group_1_1.add_owner(user) }
+      subject { described_class.member_hierarchy(user.id) }
+
+      it 'returns the groups in the hierarchy' do
+        is_expected.to match_array [
+          group,
+          nested_group_1, nested_group_1_1
+        ]
+      end
+    end
+  end
+
   describe '#full_path' do
     let(:group) { create(:group) }
     let(:nested_group) { create(:group, parent: group) }
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 5d87938235a..8ffde6f7fbb 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -55,6 +55,8 @@ describe Group, models: true do
     it { is_expected.to validate_uniqueness_of(:name).scoped_to(:parent_id) }
     it { is_expected.to validate_presence_of :path }
     it { is_expected.not_to validate_presence_of :owner }
+    it { is_expected.to validate_presence_of :two_factor_grace_period }
+    it { is_expected.to validate_numericality_of(:two_factor_grace_period).is_greater_than_or_equal_to(0) }
   end
 
   describe '.visible_to_user' do
@@ -315,4 +317,44 @@ describe Group, models: true do
         to include(master.id, developer.id)
     end
   end
+
+  describe '#update_two_factor_requirement' do
+    let(:user) { create(:user) }
+
+    before do
+      group.add_user(user, GroupMember::OWNER)
+    end
+
+    it 'is called when require_two_factor_authentication is changed' do
+      expect_any_instance_of(User).to receive(:update_two_factor_requirement)
+
+      group.update!(require_two_factor_authentication: true)
+    end
+
+    it 'is called when two_factor_grace_period is changed' do
+      expect_any_instance_of(User).to receive(:update_two_factor_requirement)
+
+      group.update!(two_factor_grace_period: 23)
+    end
+
+    it 'is not called when other attributes are changed' do
+      expect_any_instance_of(User).not_to receive(:update_two_factor_requirement)
+
+      group.update!(description: 'foobar')
+    end
+
+    it 'calls #update_two_factor_requirement on each group member' do
+      other_user = create(:user)
+      group.add_user(other_user, GroupMember::OWNER)
+
+      calls = 0
+      allow_any_instance_of(User).to receive(:update_two_factor_requirement) do
+        calls += 1
+      end
+
+      group.update!(require_two_factor_authentication: true, two_factor_grace_period: 23)
+
+      expect(calls).to eq 2
+    end
+  end
 end
diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb
index 370aeb9e0a9..024380b7ebb 100644
--- a/spec/models/members/group_member_spec.rb
+++ b/spec/models/members/group_member_spec.rb
@@ -61,7 +61,7 @@ describe GroupMember, models: true do
 
     describe '#after_accept_request' do
       it 'calls NotificationService.accept_group_access_request' do
-        member = create(:group_member, user: build_stubbed(:user), requested_at: Time.now)
+        member = create(:group_member, user: build(:user), requested_at: Time.now)
 
         expect_any_instance_of(NotificationService).to receive(:new_group_member)
 
@@ -75,4 +75,19 @@ describe GroupMember, models: true do
       it { is_expected.to eq 'Group' }
     end
   end
+
+  describe '#update_two_factor_requirement' do
+    let(:user) { build :user }
+    let(:group_member) { build :group_member, user: user }
+
+    it 'is called after creation and deletion' do
+      expect(user).to receive(:update_two_factor_requirement)
+
+      group_member.save
+
+      expect(user).to receive(:update_two_factor_requirement)
+
+      group_member.destroy
+    end
+  end
 end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index a9e37be1157..3977af55176 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1407,6 +1407,17 @@ describe User, models: true do
     it { expect(user.nested_groups).to eq([nested_group]) }
   end
 
+  describe '#all_expanded_groups' do
+    let!(:user) { create(:user) }
+    let!(:group) { create(:group) }
+    let!(:nested_group_1) { create(:group, parent: group) }
+    let!(:nested_group_2) { create(:group, parent: group) }
+
+    before { nested_group_1.add_owner(user) }
+
+    it { expect(user.all_expanded_groups).to match_array [group, nested_group_1] }
+  end
+
   describe '#nested_groups_projects' do
     let!(:user) { create(:user) }
     let!(:group) { create(:group) }
@@ -1521,4 +1532,76 @@ describe User, models: true do
       end
     end
   end
+
+  describe '#update_two_factor_requirement' do
+    let(:user) { create :user }
+
+    context 'with 2FA requirement on groups' do
+      let(:group1) { create :group, require_two_factor_authentication: true, two_factor_grace_period: 23 }
+      let(:group2) { create :group, require_two_factor_authentication: true, two_factor_grace_period: 32 }
+
+      before do
+        group1.add_user(user, GroupMember::OWNER)
+        group2.add_user(user, GroupMember::OWNER)
+
+        user.update_two_factor_requirement
+      end
+
+      it 'requires 2FA' do
+        expect(user.require_two_factor_authentication_from_group).to be true
+      end
+
+      it 'uses the shortest grace period' do
+        expect(user.two_factor_grace_period).to be 23
+      end
+    end
+
+    context 'with 2FA requirement on nested parent group' do
+      let!(:group1) { create :group, require_two_factor_authentication: true }
+      let!(:group1a) { create :group, require_two_factor_authentication: false, parent: group1 }
+
+      before do
+        group1a.add_user(user, GroupMember::OWNER)
+
+        user.update_two_factor_requirement
+      end
+
+      it 'requires 2FA' do
+        expect(user.require_two_factor_authentication_from_group).to be true
+      end
+    end
+
+    context 'with 2FA requirement on nested child group' do
+      let!(:group1) { create :group, require_two_factor_authentication: false }
+      let!(:group1a) { create :group, require_two_factor_authentication: true, parent: group1 }
+
+      before do
+        group1.add_user(user, GroupMember::OWNER)
+
+        user.update_two_factor_requirement
+      end
+
+      it 'requires 2FA' do
+        expect(user.require_two_factor_authentication_from_group).to be true
+      end
+    end
+
+    context 'without 2FA requirement on groups' do
+      let(:group) { create :group }
+
+      before do
+        group.add_user(user, GroupMember::OWNER)
+
+        user.update_two_factor_requirement
+      end
+
+      it 'does not require 2FA' do
+        expect(user.require_two_factor_authentication_from_group).to be false
+      end
+
+      it 'falls back to the default grace period' do
+        expect(user.two_factor_grace_period).to be 48
+      end
+    end
+  end
 end