Add latest changes from gitlab-org/gitlab@master

8 files changed, 46 insertions, 274 deletions

diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb
index 9700852e567..bc72d28ae77 100644
--- a/spec/models/event_spec.rb
+++ b/spec/models/event_spec.rb
@@ -431,8 +431,6 @@ RSpec.describe Event do
         include_examples 'visibility examples' do
           let(:visibility) { visible_to_none_except(:member) }
         end
-
-        include_examples 'visible to author', true
       end
 
       context 'private project' do
diff --git a/spec/models/integrations/assembla_spec.rb b/spec/models/integrations/assembla_spec.rb
index 960dfea3dc4..e9f4274952d 100644
--- a/spec/models/integrations/assembla_spec.rb
+++ b/spec/models/integrations/assembla_spec.rb
@@ -5,6 +5,10 @@ require 'spec_helper'
 RSpec.describe Integrations::Assembla do
   include StubRequests
 
+  it_behaves_like Integrations::ResetSecretFields do
+    let(:integration) { described_class.new }
+  end
+
   describe "Execute" do
     let(:user)    { create(:user) }
     let(:project) { create(:project, :repository) }
diff --git a/spec/models/integrations/bamboo_spec.rb b/spec/models/integrations/bamboo_spec.rb
index e92226d109f..ac8ea52dd3e 100644
--- a/spec/models/integrations/bamboo_spec.rb
+++ b/spec/models/integrations/bamboo_spec.rb
@@ -23,6 +23,8 @@ RSpec.describe Integrations::Bamboo, :use_clean_rails_memory_store_caching do
     )
   end
 
+  it_behaves_like Integrations::ResetSecretFields
+
   include_context Integrations::EnableSslVerification
 
   describe 'Validations' do
@@ -77,48 +79,6 @@ RSpec.describe Integrations::Bamboo, :use_clean_rails_memory_store_caching do
     end
   end
 
-  describe 'Callbacks' do
-    describe 'before_validation :reset_password' do
-      context 'when a password was previously set' do
-        it 'resets password if url changed' do
-          integration.bamboo_url = 'http://gitlab1.com'
-
-          expect(integration).not_to be_valid
-          expect(integration.password).to be_nil
-        end
-
-        it 'does not reset password if username changed' do
-          integration.username = 'some_name'
-
-          expect(integration).to be_valid
-          expect(integration.password).to eq('password')
-        end
-
-        it "does not reset password if new url is set together with password, even if it's the same password" do
-          integration.bamboo_url = 'http://gitlab_edited.com'
-          integration.password = 'password'
-
-          expect(integration).to be_valid
-          expect(integration.password).to eq('password')
-          expect(integration.bamboo_url).to eq('http://gitlab_edited.com')
-        end
-      end
-
-      it 'saves password if new url is set together with password when no password was previously set' do
-        integration.password = nil
-
-        integration.bamboo_url = 'http://gitlab_edited.com'
-        integration.password = 'password'
-        integration.save!
-
-        expect(integration.reload).to have_attributes(
-          bamboo_url: 'http://gitlab_edited.com',
-          password: 'password'
-        )
-      end
-    end
-  end
-
   describe '#execute' do
     it 'runs update and build action' do
       stub_update_and_build_request
diff --git a/spec/models/integrations/buildkite_spec.rb b/spec/models/integrations/buildkite_spec.rb
index ef686c0ae3c..5535a13db7f 100644
--- a/spec/models/integrations/buildkite_spec.rb
+++ b/spec/models/integrations/buildkite_spec.rb
@@ -18,6 +18,8 @@ RSpec.describe Integrations::Buildkite, :use_clean_rails_memory_store_caching do
     )
   end
 
+  it_behaves_like Integrations::ResetSecretFields
+
   it_behaves_like Integrations::HasWebHook do
     let(:hook_url) { 'https://webhook.buildkite.com/deliver/{webhook_token}' }
   end
diff --git a/spec/models/integrations/jenkins_spec.rb b/spec/models/integrations/jenkins_spec.rb
index 929473b0f02..4e787f958af 100644
--- a/spec/models/integrations/jenkins_spec.rb
+++ b/spec/models/integrations/jenkins_spec.rb
@@ -23,6 +23,10 @@ RSpec.describe Integrations::Jenkins do
     }
   end
 
+  it_behaves_like Integrations::ResetSecretFields do
+    let(:integration) { jenkins_integration }
+  end
+
   include_context Integrations::EnableSslVerification do
     let(:integration) { jenkins_integration }
   end
@@ -38,7 +42,7 @@ RSpec.describe Integrations::Jenkins do
     expect(jenkins_integration.tag_push_events).to eq(false)
   end
 
-  describe 'username validation' do
+  describe 'Validations' do
     let(:jenkins_integration) do
       described_class.create!(
         active: active,
@@ -57,28 +61,44 @@ RSpec.describe Integrations::Jenkins do
     context 'when the integration is active' do
       let(:active) { true }
 
-      context 'when password was not touched' do
-        before do
-          allow(subject).to receive(:password_touched?).and_return(false)
+      describe '#username' do
+        context 'when password was not touched' do
+          before do
+            allow(subject).to receive(:password_touched?).and_return(false)
+          end
+
+          it { is_expected.not_to validate_presence_of :username }
         end
 
-        it { is_expected.not_to validate_presence_of :username }
-      end
+        context 'when password was touched' do
+          before do
+            allow(subject).to receive(:password_touched?).and_return(true)
+          end
 
-      context 'when password was touched' do
-        before do
-          allow(subject).to receive(:password_touched?).and_return(true)
+          it { is_expected.to validate_presence_of :username }
         end
 
-        it { is_expected.to validate_presence_of :username }
+        context 'when password is blank' do
+          it 'does not validate the username' do
+            expect(subject).not_to validate_presence_of :username
+
+            subject.password = ''
+            subject.save!
+          end
+        end
       end
 
-      context 'when password is blank' do
-        it 'does not validate the username' do
-          expect(subject).not_to validate_presence_of :username
+      describe '#password' do
+        it 'does not validate the presence of password if username is nil' do
+          subject.username = nil
+
+          expect(subject).not_to validate_presence_of(:password)
+        end
+
+        it 'validates the presence of password if username is present' do
+          subject.username = 'john'
 
-          subject.password = ''
-          subject.save!
+          expect(subject).to validate_presence_of(:password)
         end
       end
     end
@@ -87,6 +107,7 @@ RSpec.describe Integrations::Jenkins do
       let(:active) { false }
 
       it { is_expected.not_to validate_presence_of :username }
+      it { is_expected.not_to validate_presence_of :password }
     end
   end
 
@@ -190,80 +211,4 @@ RSpec.describe Integrations::Jenkins do
       ).to have_been_made.once
     end
   end
-
-  describe 'Stored password invalidation' do
-    context 'when a password was previously set' do
-      let(:jenkins_integration) do
-        described_class.create!(
-          project: project,
-          properties: {
-            jenkins_url: 'http://jenkins.example.com/',
-            username: 'jenkins',
-            password: 'password'
-          }
-        )
-      end
-
-      it 'resets password if url changed' do
-        jenkins_integration.jenkins_url = 'http://jenkins-edited.example.com/'
-        jenkins_integration.valid?
-
-        expect(jenkins_integration.password).to be_nil
-      end
-
-      it 'resets password if username is blank' do
-        jenkins_integration.username = ''
-        jenkins_integration.valid?
-
-        expect(jenkins_integration.password).to be_nil
-      end
-
-      it 'does not reset password if username changed' do
-        jenkins_integration.username = 'some_name'
-        jenkins_integration.valid?
-
-        expect(jenkins_integration.password).to eq('password')
-      end
-
-      it 'does not reset password if new url is set together with password, even if it\'s the same password' do
-        jenkins_integration.jenkins_url = 'http://jenkins_edited.example.com/'
-        jenkins_integration.password = 'password'
-        jenkins_integration.valid?
-
-        expect(jenkins_integration.password).to eq('password')
-        expect(jenkins_integration.jenkins_url).to eq('http://jenkins_edited.example.com/')
-      end
-
-      it 'resets password if url changed, even if setter called multiple times' do
-        jenkins_integration.jenkins_url = 'http://jenkins1.example.com/'
-        jenkins_integration.jenkins_url = 'http://jenkins1.example.com/'
-        jenkins_integration.valid?
-
-        expect(jenkins_integration.password).to be_nil
-      end
-    end
-
-    context 'when no password was previously set' do
-      let(:jenkins_integration) do
-        described_class.create!(
-          project: project,
-          properties: {
-            jenkins_url: 'http://jenkins.example.com/',
-            username: 'jenkins'
-          }
-        )
-      end
-
-      it 'saves password if new url is set together with password' do
-        jenkins_integration.jenkins_url = 'http://jenkins_edited.example.com/'
-        jenkins_integration.password = 'password'
-        jenkins_integration.save!
-
-        expect(jenkins_integration.reload).to have_attributes(
-          jenkins_url: 'http://jenkins_edited.example.com/',
-          password: 'password'
-        )
-      end
-    end
-  end
 end
diff --git a/spec/models/integrations/teamcity_spec.rb b/spec/models/integrations/teamcity_spec.rb
index ae3e6658b3c..5160b410514 100644
--- a/spec/models/integrations/teamcity_spec.rb
+++ b/spec/models/integrations/teamcity_spec.rb
@@ -22,6 +22,8 @@ RSpec.describe Integrations::Teamcity, :use_clean_rails_memory_store_caching do
     )
   end
 
+  it_behaves_like Integrations::ResetSecretFields
+
   include_context Integrations::EnableSslVerification do
     describe '#enable_ssl_verification' do
       before do
@@ -120,50 +122,6 @@ RSpec.describe Integrations::Teamcity, :use_clean_rails_memory_store_caching do
     end
   end
 
-  describe 'Callbacks' do
-    let(:teamcity_integration) { integration }
-
-    describe 'before_validation :reset_password' do
-      context 'when a password was previously set' do
-        it 'resets password if url changed' do
-          teamcity_integration.teamcity_url = 'http://gitlab1.com'
-          teamcity_integration.valid?
-
-          expect(teamcity_integration.password).to be_nil
-        end
-
-        it 'does not reset password if username changed' do
-          teamcity_integration.username = 'some_name'
-          teamcity_integration.valid?
-
-          expect(teamcity_integration.password).to eq('password')
-        end
-
-        it "does not reset password if new url is set together with password, even if it's the same password" do
-          teamcity_integration.teamcity_url = 'http://gitlab_edited.com'
-          teamcity_integration.password = 'password'
-          teamcity_integration.valid?
-
-          expect(teamcity_integration.password).to eq('password')
-          expect(teamcity_integration.teamcity_url).to eq('http://gitlab_edited.com')
-        end
-      end
-
-      it 'saves password if new url is set together with password when no password was previously set' do
-        teamcity_integration.password = nil
-
-        teamcity_integration.teamcity_url = 'http://gitlab_edited.com'
-        teamcity_integration.password = 'password'
-        teamcity_integration.save!
-
-        expect(teamcity_integration.reload).to have_attributes(
-          teamcity_url: 'http://gitlab_edited.com',
-          password: 'password'
-        )
-      end
-    end
-  end
-
   describe '#build_page' do
     it 'returns the contents of the reactive cache' do
       stub_reactive_cache(integration, { build_page: 'foo' }, 'sha', 'ref')
diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index 670a6237788..100a604fdce 100644
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -1619,70 +1619,6 @@ RSpec.describe Note do
     end
   end
 
-  describe '#noteable_assignee_or_author?' do
-    let(:user) { create(:user) }
-    let(:noteable) { create(:issue) }
-    let(:note) { create(:note, project: noteable.project, noteable: noteable) }
-
-    subject { note.noteable_assignee_or_author?(user) }
-
-    shared_examples 'assignee check' do
-      context 'when the provided user is one of the assignees' do
-        before do
-          note.noteable.update!(assignees: [user, create(:user)])
-        end
-
-        it 'returns true' do
-          expect(subject).to be_truthy
-        end
-      end
-    end
-
-    shared_examples 'author check' do
-      context 'when the provided user is the author' do
-        before do
-          note.noteable.update!(author: user)
-        end
-
-        it 'returns true' do
-          expect(subject).to be_truthy
-        end
-      end
-
-      context 'when the provided user is neither author nor assignee' do
-        it 'returns true' do
-          expect(subject).to be_falsey
-        end
-      end
-    end
-
-    context 'when user is nil' do
-      let(:user) { nil }
-
-      it 'returns false' do
-        expect(subject).to be_falsey
-      end
-    end
-
-    context 'when noteable is an issue' do
-      it_behaves_like 'author check'
-      it_behaves_like 'assignee check'
-    end
-
-    context 'when noteable is a merge request' do
-      let(:noteable) { create(:merge_request) }
-
-      it_behaves_like 'author check'
-      it_behaves_like 'assignee check'
-    end
-
-    context 'when noteable is a snippet' do
-      let(:noteable) { create(:personal_snippet) }
-
-      it_behaves_like 'author check'
-    end
-  end
-
   describe 'banzai_render_context' do
     let(:project) { build(:project_empty_repo) }
 
diff --git a/spec/models/oauth_access_token_spec.rb b/spec/models/oauth_access_token_spec.rb
index a4540ac95bc..92e1ae8ac60 100644
--- a/spec/models/oauth_access_token_spec.rb
+++ b/spec/models/oauth_access_token_spec.rb
@@ -46,42 +46,11 @@ RSpec.describe OauthAccessToken do
         expect(described_class.by_token(plaintext_token)).to be_a(OauthAccessToken)
       end
     end
-
-    context 'when hash_oauth_secrets is disabled' do
-      let(:hashed_token) { create(:oauth_access_token, application_id: app_one.id) }
-
-      before do
-        hashed_token
-        stub_feature_flags(hash_oauth_tokens: false)
-      end
-
-      it 'stores the token in plaintext' do
-        expect(token.token).to eq(token.plaintext_token)
-      end
-
-      it 'finds a token by plaintext token' do
-        expect(described_class.by_token(token.plaintext_token)).to be_a(OauthAccessToken)
-      end
-
-      it 'does not find a token that was previously stored as hashed' do
-        expect(described_class.by_token(hashed_token.plaintext_token)).to be_nil
-      end
-    end
   end
 
   describe '.matching_token_for' do
     it 'does not find existing tokens' do
       expect(described_class.matching_token_for(app_one, token.resource_owner, token.scopes)).to be_nil
     end
-
-    context 'when hash oauth tokens is disabled' do
-      before do
-        stub_feature_flags(hash_oauth_tokens: false)
-      end
-
-      it 'finds an existing token' do
-        expect(described_class.matching_token_for(app_one, token.resource_owner, token.scopes)).to be_present
-      end
-    end
   end
 end