Merge branch 'feature/require-2fa-for-all-entities-in-group' into 'master'
inherit require 2fa for all subgroups and projects See merge request gitlab-org/gitlab-ce!24965
This commit is contained in:
commit
6de8cb7e9c
5 changed files with 135 additions and 36 deletions
|
@ -423,7 +423,7 @@ class Group < Namespace
|
|||
def update_two_factor_requirement
|
||||
return unless saved_change_to_require_two_factor_authentication? || saved_change_to_two_factor_grace_period?
|
||||
|
||||
users.find_each(&:update_two_factor_requirement)
|
||||
members_with_descendants.find_each(&:update_two_factor_requirement)
|
||||
end
|
||||
|
||||
def path_changed_hook
|
||||
|
|
|
@ -0,0 +1,4 @@
|
|||
title: Apply the group setting "require 2FA" across all subgroup members as well when changing the group setting
|
||||
merge_request: 24965
|
||||
author: rroger
|
||||
type: changed
|
|
@ -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.
|
||||
|
||||
If there are multiple 2FA requirements (i.e. group + all users, or multiple
|
||||
groups) the shortest grace period will be used.
|
||||
> [From](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/24965) GitLab 12.0, 2FA settings for a group are also applied to subgroups.
|
||||
|
||||
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
|
||||
|
||||
|
|
|
@ -603,40 +603,96 @@ describe Group do
|
|||
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
|
||||
context 'group membership' do
|
||||
before do
|
||||
group.add_user(user, GroupMember::OWNER)
|
||||
end
|
||||
|
||||
group.update!(require_two_factor_authentication: true, two_factor_grace_period: 23)
|
||||
it 'is called when require_two_factor_authentication is changed' do
|
||||
expect_any_instance_of(User).to receive(:update_two_factor_requirement)
|
||||
|
||||
expect(calls).to eq 2
|
||||
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
|
||||
|
||||
context 'sub groups and projects', :nested_groups do
|
||||
it 'enables two_factor_requirement for group member' do
|
||||
group.add_user(user, GroupMember::OWNER)
|
||||
|
||||
group.update!(require_two_factor_authentication: true)
|
||||
|
||||
expect(user.reload.require_two_factor_authentication_from_group).to be_truthy
|
||||
end
|
||||
|
||||
context 'expanded group members', :nested_groups do
|
||||
let(:indirect_user) { create(:user) }
|
||||
|
||||
it 'enables two_factor_requirement for subgroup member' do
|
||||
subgroup = create(:group, :nested, parent: group)
|
||||
subgroup.add_user(indirect_user, GroupMember::OWNER)
|
||||
|
||||
group.update!(require_two_factor_authentication: true)
|
||||
|
||||
expect(indirect_user.reload.require_two_factor_authentication_from_group).to be_truthy
|
||||
end
|
||||
|
||||
it 'does not enable 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_falsey
|
||||
end
|
||||
end
|
||||
|
||||
context 'project members' do
|
||||
it 'does not enable two_factor_requirement for child project member' do
|
||||
project = create(:project, group: group)
|
||||
project.add_maintainer(user)
|
||||
|
||||
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
|
||||
|
||||
|
|
|
@ -2655,9 +2655,9 @@ describe User do
|
|||
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!(:group1a) { create :group, require_two_factor_authentication: false, parent: group1 }
|
||||
let!(:group1a) { create :group, parent: group1 }
|
||||
|
||||
before do
|
||||
group1a.add_user(user, GroupMember::OWNER)
|
||||
|
@ -2685,6 +2685,27 @@ describe User do
|
|||
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
|
||||
let(:group) { create :group }
|
||||
|
||||
|
|
Loading…
Reference in a new issue