Commit 35d928c4 authored by Roger Meier's avatar Roger Meier

refactor: apply "require 2FA" to all subgroup and ancestor group members, when changing

parent ff22dfbf
...@@ -423,7 +423,7 @@ class Group < Namespace ...@@ -423,7 +423,7 @@ class Group < Namespace
def update_two_factor_requirement def update_two_factor_requirement
return unless saved_change_to_require_two_factor_authentication? || saved_change_to_two_factor_grace_period? return unless saved_change_to_require_two_factor_authentication? || saved_change_to_two_factor_grace_period?
User.from_union([users_with_descendants, project_users_with_descendants]).find_each(&:update_two_factor_requirement) direct_and_indirect_members.find_each(&:update_two_factor_requirement)
end end
def path_changed_hook def path_changed_hook
......
...@@ -728,8 +728,7 @@ class User < ApplicationRecord ...@@ -728,8 +728,7 @@ class User < ApplicationRecord
end end
def expanded_groups_requiring_two_factor_authentication def expanded_groups_requiring_two_factor_authentication
Group.from_union([all_expanded_groups.where(require_two_factor_authentication: true), all_expanded_groups.where(require_two_factor_authentication: true)
authorized_groups.where(require_two_factor_authentication: true)])
end end
# rubocop: disable CodeReuse/ServiceClass # rubocop: disable CodeReuse/ServiceClass
......
--- title: Apply the group setting "require 2FA" across all subgroup and ancestor group members as well when changing the group setting
title: Apply the group setting "require 2FA" to be inherited across all subgroups
and descendant projects of the group where it was set. (This works only for postgresql
databases, not for mysql/mariadb)
merge_request: 24965 merge_request: 24965
author: rroger author: rroger
type: changed type: changed
...@@ -39,8 +39,26 @@ If you want to enforce 2FA only for certain groups, you can: ...@@ -39,8 +39,26 @@ If you want to enforce 2FA only for certain groups, you can:
To change this setting, you need to be administrator or owner of the group. 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 > [From](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/24965) GitLab 12.0, 2FA settings for a group are also applied to subgroups.
groups) the shortest grace period will be used.
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.
The following are important notes about 2FA:
- Projects belonging to a 2FA-enabled group that
[is shared](../user/project/members/share_project_with_groups.md)
with a 2FA-disabled group will *not* require members of the 2FA-disabled group to use
2FA for the project. For example, if project *P* belongs to 2FA-enabled group *A* and
is shared with 2FA-disabled group *B*, members of group *B* can access project *P*
without 2FA. To ensure this scenario doesn't occur,
[prevent sharing of projects](../user/group/index.md#share-with-group-lock)
for the 2FA-enabled group.
- If you add additional members to a project within a group or subgroup that has
2FA enabled, 2FA is **not** required for those individually added members.
- If there are multiple 2FA requirements (for example, group + all users, or multiple
groups) the shortest grace period will be used.
## Disabling 2FA for everyone ## Disabling 2FA for everyone
......
...@@ -603,73 +603,96 @@ describe Group do ...@@ -603,73 +603,96 @@ describe Group do
describe '#update_two_factor_requirement' do describe '#update_two_factor_requirement' do
let(:user) { create(:user) } let(:user) { create(:user) }
before do context 'group membership' do
group.add_user(user, GroupMember::OWNER) before do
end 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) it 'is called when require_two_factor_authentication is changed' do
end expect_any_instance_of(User).to receive(:update_two_factor_requirement)
it 'is called when two_factor_grace_period is changed' do group.update!(require_two_factor_authentication: true)
expect_any_instance_of(User).to receive(:update_two_factor_requirement) end
group.update!(two_factor_grace_period: 23) it 'is called when two_factor_grace_period is changed' do
end expect_any_instance_of(User).to receive(:update_two_factor_requirement)
it 'is not called when other attributes are changed' do group.update!(two_factor_grace_period: 23)
expect_any_instance_of(User).not_to receive(:update_two_factor_requirement) end
group.update!(description: 'foobar') it 'is not called when other attributes are changed' do
end expect_any_instance_of(User).not_to receive(:update_two_factor_requirement)
def expects_other_user_to_require_two_factors(expected_calls_mysql_db = 1) group.update!(description: 'foobar')
calls = 0
allow_any_instance_of(User).to receive(:update_two_factor_requirement) do
calls += 1
end end
group.update!(require_two_factor_authentication: true, two_factor_grace_period: 23) 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)
if Group.supports_nested_objects?
expect(calls).to eq 2 expect(calls).to eq 2
else
expect(calls).to eq expected_calls_mysql_db
end end
end end
it 'calls #update_two_factor_requirement on each group member' do context 'sub groups and projects', :nested_groups do
other_user = create(:user) it 'enables two_factor_requirement for group member' do
group.add_user(other_user, GroupMember::OWNER) group.add_user(user, GroupMember::OWNER)
expects_other_user_to_require_two_factors(2) group.update!(require_two_factor_authentication: true)
end
it 'calls #update_two_factor_requirement on each subgroup member' do expect(user.reload.require_two_factor_authentication_from_group).to be_truthy
subgroup = create(:group, :nested, parent: group) end
subgroup_user = create(:user)
subgroup.add_user(subgroup_user, GroupMember::OWNER)
expects_other_user_to_require_two_factors context 'expanded group members', :nested_groups do
end let(:indirect_user) { create(:user) }
it 'calls #update_two_factor_requirement on each child project member' do it 'enables two_factor_requirement for subgroup member' do
project = create(:project, group: group) subgroup = create(:group, :nested, parent: group)
project_user = create(:user) subgroup.add_user(indirect_user, GroupMember::OWNER)
project.add_developer(project_user)
expects_other_user_to_require_two_factors(2) group.update!(require_two_factor_authentication: true)
end
expect(indirect_user.reload.require_two_factor_authentication_from_group).to be_truthy
end
it 'enables two_factor_requirement for ancestor group member' do
ancestor_group = create(:group)
ancestor_group.add_user(indirect_user, GroupMember::OWNER)
group.update!(parent: ancestor_group)
group.update!(require_two_factor_authentication: true)
expect(indirect_user.reload.require_two_factor_authentication_from_group).to be_truthy
end
end
it 'calls #update_two_factor_requirement on each subgroups child project member' do context 'project members' do
subgroup = create(:group, :nested, parent: group) it 'does not enable two_factor_requirement for child project member' do
project = create(:project, group: subgroup) project = create(:project, group: group)
project_user = create(:user) project.add_maintainer(user)
project.add_developer(project_user)
expects_other_user_to_require_two_factors group.update!(require_two_factor_authentication: true)
expect(user.reload.require_two_factor_authentication_from_group).to be_falsey
end
it 'does not enable two_factor_requirement for subgroup child project member', :nested_groups do
subgroup = create(:group, :nested, parent: group)
project = create(:project, group: subgroup)
project.add_maintainer(user)
group.update!(require_two_factor_authentication: true)
expect(user.reload.require_two_factor_authentication_from_group).to be_falsey
end
end
end end
end end
......
...@@ -2655,9 +2655,9 @@ describe User do ...@@ -2655,9 +2655,9 @@ describe User do
end end
end end
context 'with 2FA requirement on nested parent group', :nested_groups do context 'with 2FA requirement from expanded groups', :nested_groups do
let!(:group1) { create :group, require_two_factor_authentication: true } let!(:group1) { create :group, require_two_factor_authentication: true }
let!(:group1a) { create :group, require_two_factor_authentication: false, parent: group1 } let!(:group1a) { create :group, parent: group1 }
before do before do
group1a.add_user(user, GroupMember::OWNER) group1a.add_user(user, GroupMember::OWNER)
...@@ -2685,6 +2685,27 @@ describe User do ...@@ -2685,6 +2685,27 @@ describe User do
end end
end end
context "with 2FA requirement from shared project's group" do
let!(:group1) { create :group, require_two_factor_authentication: true }
let!(:group2) { create :group }
let(:shared_project) { create(:project, namespace: group1) }
before do
shared_project.project_group_links.create!(
group: group2,
group_access: ProjectGroupLink.default_access
)
group2.add_user(user, GroupMember::OWNER)
end
it 'does not require 2FA' do
user.update_two_factor_requirement
expect(user.require_two_factor_authentication_from_group).to be false
end
end
context 'without 2FA requirement on groups' do context 'without 2FA requirement on groups' do
let(:group) { create :group } let(:group) { create :group }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment