diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-02-28 23:03:42 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-02-28 23:03:42 +0300 |
commit | 557577fba32186d09a11983618aabb5295b287e8 (patch) | |
tree | 9c35b08802932a7359f2d16e1efcf650f0b94731 | |
parent | e316c4740c1b604de112bbad52c2531d2261a8f8 (diff) |
Add latest changes from gitlab-org/security/gitlab@12-8-stable-ee
7 files changed, 93 insertions, 3 deletions
diff --git a/app/models/members/group_member.rb b/app/models/members/group_member.rb index bdff9e28df1..bc3be67bd32 100644 --- a/app/models/members/group_member.rb +++ b/app/models/members/group_member.rb @@ -66,6 +66,7 @@ class GroupMember < Member def after_accept_invite notification_service.accept_group_invite(self) + update_two_factor_requirement super end diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index 09a84950755..629c1cbdc5c 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -3,12 +3,24 @@ module Auth class ContainerRegistryAuthenticationService < BaseService AUDIENCE = 'container_registry' + REGISTRY_LOGIN_ABILITIES = [ + :read_container_image, + :create_container_image, + :destroy_container_image, + :update_container_image, + :admin_container_image, + :build_read_container_image, + :build_create_container_image, + :build_destroy_container_image + ].freeze def execute(authentication_abilities:) @authentication_abilities = authentication_abilities return error('UNAVAILABLE', status: 404, message: 'registry not enabled') unless registry.enabled + return error('DENIED', status: 403, message: 'access forbidden') unless has_registry_ability? + unless scopes.any? || current_user || project return error('DENIED', status: 403, message: 'access forbidden') end @@ -197,5 +209,11 @@ module Auth def has_authentication_ability?(capability) @authentication_abilities.to_a.include?(capability) end + + def has_registry_ability? + @authentication_abilities.any? do |ability| + REGISTRY_LOGIN_ABILITIES.include?(ability) + end + end end end diff --git a/changelogs/unreleased/enfoce-group-member-2fa.yml b/changelogs/unreleased/enfoce-group-member-2fa.yml new file mode 100644 index 00000000000..1e10f678eda --- /dev/null +++ b/changelogs/unreleased/enfoce-group-member-2fa.yml @@ -0,0 +1,5 @@ +--- +title: Update user 2fa when accepting a group invite +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-deploy-token-registry-access.yml b/changelogs/unreleased/security-deploy-token-registry-access.yml new file mode 100644 index 00000000000..3b7a0553b2e --- /dev/null +++ b/changelogs/unreleased/security-deploy-token-registry-access.yml @@ -0,0 +1,6 @@ +--- +title: Update container registry authentication to account for login request when + checking permissions +merge_request: +author: +type: security diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb index ad7dfac87af..9b5cce1aebf 100644 --- a/spec/models/members/group_member_spec.rb +++ b/spec/models/members/group_member_spec.rb @@ -65,10 +65,10 @@ describe GroupMember do 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 + user = build :user + group_member = build :group_member, user: user + expect(user).to receive(:update_two_factor_requirement) group_member.save @@ -79,6 +79,21 @@ describe GroupMember do end end + describe '#after_accept_invite' do + it 'calls #update_two_factor_requirement' do + email = 'foo@email.com' + user = build(:user, email: email) + group = create(:group, require_two_factor_authentication: true) + group_member = create(:group_member, group: group, invite_token: '1234', invite_email: email) + + expect(user).to receive(:require_two_factor_authentication_from_group).and_call_original + + group_member.accept_invite!(user) + + expect(user.require_two_factor_authentication_from_group).to be_truthy + end + end + context 'access levels' do context 'with parent group' do it_behaves_like 'inherited access level as a member of entity' do diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb index 5003dfcc951..84f4a7a4e7a 100644 --- a/spec/services/auth/container_registry_authentication_service_spec.rb +++ b/spec/services/auth/container_registry_authentication_service_spec.rb @@ -769,6 +769,15 @@ describe Auth::ContainerRegistryAuthenticationService do context 'when deploy token has read_registry as a scope' do let(:current_user) { create(:deploy_token, projects: [project]) } + shared_examples 'able to login' do + context 'registry provides read_container_image authentication_abilities' do + let(:current_params) { {} } + let(:authentication_abilities) { [:read_container_image] } + + it_behaves_like 'an authenticated' + end + end + context 'for public project' do let(:project) { create(:project, :public) } @@ -783,6 +792,8 @@ describe Auth::ContainerRegistryAuthenticationService do it_behaves_like 'an inaccessible' end + + it_behaves_like 'able to login' end context 'for internal project' do @@ -799,6 +810,8 @@ describe Auth::ContainerRegistryAuthenticationService do it_behaves_like 'an inaccessible' end + + it_behaves_like 'able to login' end context 'for private project' do @@ -815,18 +828,38 @@ describe Auth::ContainerRegistryAuthenticationService do it_behaves_like 'an inaccessible' end + + it_behaves_like 'able to login' end end context 'when deploy token does not have read_registry scope' do let(:current_user) { create(:deploy_token, projects: [project], read_registry: false) } + shared_examples 'unable to login' do + context 'registry provides no container authentication_abilities' do + let(:current_params) { {} } + let(:authentication_abilities) { [] } + + it_behaves_like 'a forbidden' + end + + context 'registry provides inapplicable container authentication_abilities' do + let(:current_params) { {} } + let(:authentication_abilities) { [:download_code] } + + it_behaves_like 'a forbidden' + end + end + context 'for public project' do let(:project) { create(:project, :public) } context 'when pulling' do it_behaves_like 'a pullable' end + + it_behaves_like 'unable to login' end context 'for internal project' do @@ -835,6 +868,8 @@ describe Auth::ContainerRegistryAuthenticationService do context 'when pulling' do it_behaves_like 'an inaccessible' end + + it_behaves_like 'unable to login' end context 'for private project' do @@ -843,6 +878,15 @@ describe Auth::ContainerRegistryAuthenticationService do context 'when pulling' do it_behaves_like 'an inaccessible' end + + context 'when logging in' do + let(:current_params) { {} } + let(:authentication_abilities) { [] } + + it_behaves_like 'a forbidden' + end + + it_behaves_like 'unable to login' end end diff --git a/spec/support/shared_contexts/policies/group_policy_shared_context.rb b/spec/support/shared_contexts/policies/group_policy_shared_context.rb index 63ebbcb93f9..3a306f80b3c 100644 --- a/spec/support/shared_contexts/policies/group_policy_shared_context.rb +++ b/spec/support/shared_contexts/policies/group_policy_shared_context.rb @@ -7,6 +7,7 @@ RSpec.shared_context 'GroupPolicy context' do let_it_be(:maintainer) { create(:user) } let_it_be(:owner) { create(:user) } let_it_be(:admin) { create(:admin) } + let_it_be(:non_group_member) { create(:user) } let_it_be(:group, refind: true) { create(:group, :private, :owner_subgroup_creation_only) } let(:guest_permissions) do |