Fix a bug with the User#abuse_report association.

Introduction
------------

1. The foreign key was not explicitly specified on the association.
2. The `AbuseReport` model contains two references to user - `reporter_id` and
   `user_id`
3. `user.abuse_report` is supposed to return the single abuse report where
   `user_id` refers to the given user.

Bug Description
---------------

1. `user.abuse_report` would return an abuse report where `reporter_id` referred
   to the current user, if such an abuse report was present.

2. This implies a slightly more serious bug as well:

   - Assume User A filed an abuse report against User B
   - We have an abuse report where `reporter_id` is User A and `user_id` is User B
   - If User A is updated (`user_a.block`, for example), the abuse report would
     also be updated, such that both `reporter_id` _and_ `user_id` point to User A.

Fix
---

Explicitly declare the foreign key `user_id` in the `has_one` declaration

2 files changed, 28 insertions, 2 deletions

diff --git a/app/models/user.rb b/app/models/user.rb
index 95a766f2ede..8c2f0011be8 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -89,7 +89,7 @@ class User < ActiveRecord::Base
   has_many :subscriptions,            dependent: :destroy
   has_many :recent_events, -> { order "id DESC" }, foreign_key: :author_id,   class_name: "Event"
   has_many :oauth_applications, class_name: 'Doorkeeper::Application', as: :owner, dependent: :destroy
-  has_one  :abuse_report,             dependent: :destroy
+  has_one  :abuse_report,             dependent: :destroy, foreign_key: :user_id
   has_many :spam_logs,                dependent: :destroy
   has_many :builds,                   dependent: :nullify, class_name: 'Ci::Build'
   has_many :pipelines,                dependent: :nullify, class_name: 'Ci::Pipeline'
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index a9e37be1157..6a787f6b0df 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -28,7 +28,6 @@ describe User, models: true do
     it { is_expected.to have_many(:merge_requests).dependent(:destroy) }
     it { is_expected.to have_many(:assigned_merge_requests).dependent(:nullify) }
     it { is_expected.to have_many(:identities).dependent(:destroy) }
-    it { is_expected.to have_one(:abuse_report) }
     it { is_expected.to have_many(:spam_logs).dependent(:destroy) }
     it { is_expected.to have_many(:todos).dependent(:destroy) }
     it { is_expected.to have_many(:award_emoji).dependent(:destroy) }
@@ -38,6 +37,33 @@ describe User, models: true do
     it { is_expected.to have_many(:chat_names).dependent(:destroy) }
     it { is_expected.to have_many(:uploads).dependent(:destroy) }
 
+    describe "#abuse_report" do
+      let(:current_user) { create(:user) }
+      let(:other_user) { create(:user) }
+
+      it { is_expected.to have_one(:abuse_report) }
+
+      it "refers to the abuse report whose user_id is the current user" do
+        abuse_report = create(:abuse_report, reporter: other_user, user: current_user)
+
+        expect(current_user.abuse_report).to eq(abuse_report)
+      end
+
+      it "does not refer to the abuse report whose reporter_id is the current user" do
+        create(:abuse_report, reporter: current_user, user: other_user)
+
+        expect(current_user.abuse_report).to be_nil
+      end
+
+      it "does not update the user_id of an abuse report when the user is updated" do
+        abuse_report = create(:abuse_report, reporter: current_user, user: other_user)
+
+        current_user.block
+
+        expect(abuse_report.reload.user).to eq(other_user)
+      end
+    end
+
     describe '#group_members' do
       it 'does not include group memberships for which user is a requester' do
         user = create(:user)