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