Add latest changes from gitlab-org/security/gitlab@13-0-stable-ee

10 files changed, 145 insertions, 17 deletions

diff --git a/spec/controllers/profiles/notifications_controller_spec.rb b/spec/controllers/profiles/notifications_controller_spec.rb
index 47d6f11fecf..343f29ef687 100644
--- a/spec/controllers/profiles/notifications_controller_spec.rb
+++ b/spec/controllers/profiles/notifications_controller_spec.rb
@@ -5,8 +5,8 @@ require 'spec_helper'
 describe Profiles::NotificationsController do
   let(:user) do
     create(:user) do |user|
-      user.emails.create(email: 'original@example.com')
-      user.emails.create(email: 'new@example.com')
+      user.emails.create(email: 'original@example.com', confirmed_at: Time.current)
+      user.emails.create(email: 'new@example.com', confirmed_at: Time.current)
       user.notification_email = 'original@example.com'
       user.save!
     end
diff --git a/spec/controllers/projects/deploy_keys_controller_spec.rb b/spec/controllers/projects/deploy_keys_controller_spec.rb
index 1b2b326b6e9..9d41e2f59cb 100644
--- a/spec/controllers/projects/deploy_keys_controller_spec.rb
+++ b/spec/controllers/projects/deploy_keys_controller_spec.rb
@@ -256,7 +256,7 @@ describe Projects::DeployKeysController do
     end
 
     def deploy_key_params(title, can_push)
-      deploy_keys_projects_attributes = { '0' => { id: deploy_keys_project, can_push: can_push } }
+      deploy_keys_projects_attributes = { '0' => { can_push: can_push } }
       { deploy_key: { title: title, deploy_keys_projects_attributes: deploy_keys_projects_attributes } }
     end
 
@@ -300,6 +300,42 @@ describe Projects::DeployKeysController do
           expect { subject }.to change { deploy_keys_project.reload.can_push }.from(false).to(true)
         end
       end
+
+      context 'when a different deploy key id param is injected' do
+        let(:extra_params) { deploy_key_params('updated title', '1') }
+        let(:hacked_params) do
+          extra_params.reverse_merge(id: other_deploy_key_id,
+                                     namespace_id: project.namespace,
+                                     project_id: project)
+        end
+
+        subject { put :update, params: hacked_params }
+
+        context 'and that deploy key id exists' do
+          let(:other_project) { create(:project) }
+          let(:other_deploy_key) do
+            key = create(:deploy_key)
+            project.deploy_keys << key
+            key
+          end
+
+          let(:other_deploy_key_id) { other_deploy_key.id }
+
+          it 'does not update the can_push attribute' do
+            expect { subject }.not_to change { deploy_key.deploy_keys_project_for(project).can_push }
+          end
+        end
+
+        context 'and that deploy key id does not exist' do
+          let(:other_deploy_key_id) { 9999 }
+
+          it 'returns 404' do
+            subject
+
+            expect(response).to have_gitlab_http_status(:not_found)
+          end
+        end
+      end
     end
 
     context 'with admin as project maintainer' do
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index a4e49f88115..d6e76258491 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -110,6 +110,11 @@ describe Group do
       let(:group_notification_email) { 'user+group@example.com' }
       let(:subgroup_notification_email) { 'user+subgroup@example.com' }
 
+      before do
+        create(:email, :confirmed, user: user, email: group_notification_email)
+        create(:email, :confirmed, user: user, email: subgroup_notification_email)
+      end
+
       subject { subgroup.notification_email_for(user) }
 
       context 'when both group notification emails are set' do
diff --git a/spec/models/notification_setting_spec.rb b/spec/models/notification_setting_spec.rb
index 9ab9ae494ec..67738eaec20 100644
--- a/spec/models/notification_setting_spec.rb
+++ b/spec/models/notification_setting_spec.rb
@@ -48,6 +48,33 @@ RSpec.describe NotificationSetting do
         expect(notification_setting.reopen_merge_request).to eq(false)
       end
     end
+
+    context 'notification_email' do
+      let_it_be(:user) { create(:user) }
+      subject { described_class.new(source_id: 1, source_type: 'Project', user_id: user.id) }
+
+      it 'allows to change email to verified one' do
+        email = create(:email, :confirmed, user: user)
+
+        subject.update(notification_email: email.email)
+
+        expect(subject).to be_valid
+      end
+
+      it 'does not allow to change email to not verified one' do
+        email = create(:email, user: user)
+
+        subject.update(notification_email: email.email)
+
+        expect(subject).to be_invalid
+      end
+
+      it 'allows to change email to empty one' do
+        subject.update(notification_email: '')
+
+        expect(subject).to be_valid
+      end
+    end
   end
 
   describe '#for_projects' do
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index abc52263298..60def077239 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -310,7 +310,7 @@ describe User do
     end
 
     it_behaves_like 'an object with RFC3696 compliant email-formated attributes', :public_email, :notification_email do
-      subject { build(:user).tap { |user| user.emails << build(:email, email: email_value) } }
+      subject { create(:user).tap { |user| user.emails << build(:email, email: email_value, confirmed_at: Time.current) } }
     end
 
     describe '#commit_email' do
@@ -567,6 +567,32 @@ describe User do
           user = build(:user, email: "temp-email-for-oauth@example.com")
           expect(user).to be_valid
         end
+
+        it 'does not accept not verified emails' do
+          email = create(:email)
+          user = email.user
+          user.update(notification_email: email.email)
+
+          expect(user).to be_invalid
+        end
+      end
+
+      context 'owns_public_email' do
+        it 'accepts verified emails' do
+          email = create(:email, :confirmed, email: 'test@test.com')
+          user = email.user
+          user.update(public_email: email.email)
+
+          expect(user).to be_valid
+        end
+
+        it 'does not accept not verified emails' do
+          email = create(:email)
+          user = email.user
+          user.update(public_email: email.email)
+
+          expect(user).to be_invalid
+        end
       end
 
       context 'set_commit_email' do
@@ -2171,6 +2197,31 @@ describe User do
     end
   end
 
+  describe '#public_verified_emails' do
+    let(:user) { create(:user) }
+
+    it 'returns only confirmed public emails' do
+      email_confirmed = create :email, user: user, confirmed_at: Time.current
+      create :email, user: user
+
+      expect(user.public_verified_emails).to contain_exactly(
+        user.email,
+        email_confirmed.email
+      )
+    end
+
+    it 'returns confirmed public emails plus main user email when user is not confirmed' do
+      user = create(:user, confirmed_at: nil)
+      email_confirmed = create :email, user: user, confirmed_at: Time.current
+      create :email, user: user
+
+      expect(user.public_verified_emails).to contain_exactly(
+        user.email,
+        email_confirmed.email
+      )
+    end
+  end
+
   describe '#verified_email?' do
     let(:user) { create(:user) }
 
@@ -4333,9 +4384,10 @@ describe User do
           context 'when an ancestor has a level other than Global' do
             let(:ancestor) { create(:group) }
             let(:group) { create(:group, parent: ancestor) }
+            let(:email) { create(:email, :confirmed, email: 'ancestor@example.com', user: user) }
 
             before do
-              create(:notification_setting, user: user, source: ancestor, level: 'participating', notification_email: 'ancestor@example.com')
+              create(:notification_setting, user: user, source: ancestor, level: 'participating', notification_email: email.email)
             end
 
             it 'has the same level set' do
@@ -4360,10 +4412,12 @@ describe User do
             let(:grand_ancestor) { create(:group) }
             let(:ancestor) { create(:group, parent: grand_ancestor) }
             let(:group) { create(:group, parent: ancestor) }
+            let(:ancestor_email) { create(:email, :confirmed, email: 'ancestor@example.com', user: user) }
+            let(:grand_email) { create(:email, :confirmed, email: 'grand@example.com', user: user) }
 
             before do
-              create(:notification_setting, user: user, source: grand_ancestor, level: 'participating', notification_email: 'grand@example.com')
-              create(:notification_setting, user: user, source: ancestor, level: 'global', notification_email: 'ancestor@example.com')
+              create(:notification_setting, user: user, source: grand_ancestor, level: 'participating', notification_email: grand_email.email)
+              create(:notification_setting, user: user, source: ancestor, level: 'global', notification_email: ancestor_email.email)
             end
 
             it 'has the same email set' do
@@ -4401,7 +4455,7 @@ describe User do
     context 'when group has notification email set' do
       it 'returns group notification email' do
         group_notification_email = 'user+group@example.com'
-
+        create(:email, :confirmed, user: user, email: group_notification_email)
         create(:notification_setting, user: user, source: group, notification_email: group_notification_email)
 
         is_expected.to eq(group_notification_email)
diff --git a/spec/requests/api/notification_settings_spec.rb b/spec/requests/api/notification_settings_spec.rb
index cbdab2f53a6..2dfde4c8ec9 100644
--- a/spec/requests/api/notification_settings_spec.rb
+++ b/spec/requests/api/notification_settings_spec.rb
@@ -19,7 +19,7 @@ describe API::NotificationSettings do
   end
 
   describe "PUT /notification_settings" do
-    let(:email) { create(:email, user: user) }
+    let(:email) { create(:email, :confirmed, user: user) }
 
     it "updates global notification settings for the current user" do
       put api("/notification_settings", user), params: { level: 'watch', notification_email: email.email }
diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb
index bd270679acd..785ab98a3d0 100644
--- a/spec/requests/openid_connect_spec.rb
+++ b/spec/requests/openid_connect_spec.rb
@@ -9,15 +9,11 @@ describe 'OpenID Connect requests' do
       name: 'Alice',
       username: 'alice',
       email: 'private@example.com',
-      emails: [public_email],
-      public_email: public_email.email,
       website_url: 'https://example.com',
       avatar: fixture_file_upload('spec/fixtures/dk.png')
     )
   end
 
-  let(:public_email) { build :email, email: 'public@example.com' }
-
   let(:access_grant) { create :oauth_access_grant, application: application, resource_owner_id: user.id }
   let(:access_token) { create :oauth_access_token, application: application, resource_owner_id: user.id }
 
@@ -37,7 +33,7 @@ describe 'OpenID Connect requests' do
       'name'           => 'Alice',
       'nickname'       => 'alice',
       'email'          => 'public@example.com',
-      'email_verified' => false,
+      'email_verified' => true,
       'website'        => 'https://example.com',
       'profile'        => 'http://localhost/alice',
       'picture'        => "http://localhost/uploads/-/system/user/avatar/#{user.id}/dk.png",
@@ -62,6 +58,11 @@ describe 'OpenID Connect requests' do
     get '/oauth/userinfo', params: {}, headers: { 'Authorization' => "Bearer #{access_token.token}" }
   end
 
+  before do
+    email = create(:email, :confirmed, email: 'public@example.com', user: user)
+    user.update!(public_email: email.email)
+  end
+
   context 'Application without OpenID scope' do
     let(:application) { create :oauth_application, scopes: 'api' }
 
@@ -123,7 +124,7 @@ describe 'OpenID Connect requests' do
       end
 
       it 'has false in email_verified claim' do
-        expect(json_response['email_verified']).to eq(false)
+        expect(json_response['email_verified']).to eq(true)
       end
     end
 
diff --git a/spec/requests/profiles/notifications_controller_spec.rb b/spec/requests/profiles/notifications_controller_spec.rb
index 41349d6c12d..0b2741677ab 100644
--- a/spec/requests/profiles/notifications_controller_spec.rb
+++ b/spec/requests/profiles/notifications_controller_spec.rb
@@ -5,8 +5,8 @@ require 'spec_helper'
 describe 'view user notifications' do
   let(:user) do
     create(:user) do |user|
-      user.emails.create(email: 'original@example.com')
-      user.emails.create(email: 'new@example.com')
+      user.emails.create(email: 'original@example.com', confirmed_at: Time.current)
+      user.emails.create(email: 'new@example.com', confirmed_at: Time.current)
       user.notification_email = 'original@example.com'
       user.save!
     end
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 2a7166e3895..46c80a86639 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -2457,6 +2457,8 @@ describe NotificationService, :mailer do
               group = create(:group)
 
               project.update(group: group)
+
+              create(:email, :confirmed, user: u_custom_notification_enabled, email: group_notification_email)
               create(:notification_setting, user: u_custom_notification_enabled, source: group, notification_email: group_notification_email)
             end
 
@@ -2491,6 +2493,7 @@ describe NotificationService, :mailer do
               group = create(:group)
 
               project.update(group: group)
+              create(:email, :confirmed, user: u_member, email: group_notification_email)
               create(:notification_setting, user: u_member, source: group, notification_email: group_notification_email)
             end
 
@@ -2584,6 +2587,7 @@ describe NotificationService, :mailer do
               group = create(:group)
 
               project.update(group: group)
+              create(:email, :confirmed, user: u_member, email: group_notification_email)
               create(:notification_setting, user: u_member, source: group, notification_email: group_notification_email)
             end
 
diff --git a/spec/support/shared_examples/mailers/notify_shared_examples.rb b/spec/support/shared_examples/mailers/notify_shared_examples.rb
index 45987059123..1f5803b90a0 100644
--- a/spec/support/shared_examples/mailers/notify_shared_examples.rb
+++ b/spec/support/shared_examples/mailers/notify_shared_examples.rb
@@ -28,6 +28,7 @@ RSpec.shared_examples 'an email sent to a user' do
     it 'is sent to user\'s group notification email' do
       group_notification_email = 'user+group@example.com'
 
+      create(:email, :confirmed, user: recipient, email: group_notification_email)
       create(:notification_setting, user: recipient, source: group, notification_email: group_notification_email)
 
       expect(subject).to deliver_to(group_notification_email)