Support 2FA requirement per-group

20 files changed, 433 insertions, 21 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 b197fd2157e..28c4380ca84 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -267,11 +267,19 @@ class ApplicationController < ActionController::Base
   end
 
   def two_factor_authentication_required?
-    current_application_settings.require_two_factor_authentication
+    current_application_settings.require_two_factor_authentication ||
+      current_user.try(:require_two_factor_authentication)
   end
 
   def two_factor_grace_period
-    current_application_settings.two_factor_grace_period
+    if current_user.try(:require_two_factor_authentication)
+      [
+        current_application_settings.two_factor_grace_period,
+        current_user.two_factor_grace_period
+      ].min
+    else
+      current_application_settings.two_factor_grace_period
+    end
   end
 
   def two_factor_grace_period_expired?
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 05f9ee1ee90..5f90df579a8 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -150,7 +150,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/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..564e99df77b 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -963,6 +963,15 @@ class User < ActiveRecord::Base
     super
   end
 
+  def update_two_factor_requirement
+    periods = groups.where(require_two_factor_authentication: true).pluck(:two_factor_grace_period)
+
+    self.require_two_factor_authentication = periods.any?
+    self.two_factor_grace_period = periods.min || User.column_defaults['two_factor_grace_period']
+
+    save
+  end
+
   private
 
   def ci_projects_union
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..bef1b2062c8
--- /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, :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)
+    remove_column(:users, :two_factor_grace_period)
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index ccf18d07179..2d2507828d4 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -692,6 +692,8 @@ ActiveRecord::Schema.define(version: 20170402231018) 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: 20170402231018) 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|
@@ -1246,6 +1249,8 @@ ActiveRecord::Schema.define(version: 20170402231018) do
     t.boolean "authorized_projects_populated"
     t.boolean "ghost"
     t.boolean "notified_of_own_activity"
+    t.boolean "require_two_factor_authentication", 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..64cfb87da5d 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_2fa_requirement' do
+      subject { controller.send :check_2fa_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 = 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: 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_tfa: 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/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..b2f686a1819 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1521,4 +1521,46 @@ 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).to be true
+      end
+
+      it 'uses the shortest grace period' do
+        expect(user.two_factor_grace_period).to be 23
+      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).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