Merge branch 'check-for-present-runner-token' into 'master'

Fix Error 500 when visiting build page of project with nil runners_token

Properly ensure that the token exists and add defensively check for a
non-nil value.

Closes #4294

See merge request !2294

4 files changed, 31 insertions, 5 deletions

diff --git a/CHANGELOG b/CHANGELOG
index cac843cc807..075d598081d 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -9,6 +9,7 @@ v 8.3.3 (unreleased)
   - Better support for referencing and closing issues in Asana service (Mike Wyatt)
   - Enable "Add key" button when user fills in a proper key (Stan Hu)
   - Fix error in processing reply-by-email messages (Jason Lee)
+  - Fix Error 500 when visiting build page of project with nil runners_token (Stan Hu)
 
 v 8.3.2
   - Disable --follow in `git log` to avoid loading duplicate commit data in infinite scroll (Stan Hu)
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index e9c8d0ea4e7..0cf04cc3a94 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -196,7 +196,7 @@ module Ci
 
     def trace
       trace = raw_trace
-      if project && trace.present?
+      if project && trace.present? && project.runners_token.present?
         trace.gsub(project.runners_token, 'xxxxxx')
       else
         trace
diff --git a/app/models/project.rb b/app/models/project.rb
index a8f69fd1eec..a660a5116b2 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -43,6 +43,7 @@ class Project < ActiveRecord::Base
   include Sortable
   include AfterCommitQueue
   include CaseSensitivity
+  include TokenAuthenticatable
 
   extend Gitlab::ConfigHelper
 
@@ -169,10 +170,8 @@ class Project < ActiveRecord::Base
     if: ->(project) { project.avatar.present? && project.avatar_changed? }
   validates :avatar, file_size: { maximum: 200.kilobytes.to_i }
 
-  before_validation :set_runners_token_token
-  def set_runners_token_token
-    self.runners_token = SecureRandom.hex(15) if self.runners_token.blank?
-  end
+  add_authentication_token_field :runners_token
+  before_save :ensure_runners_token
 
   mount_uploader :avatar, AvatarUploader
 
@@ -864,4 +863,8 @@ class Project < ActiveRecord::Base
   def open_issues_count
     issues.opened.count
   end
+
+  def runners_token
+    ensure_runners_token!
+  end
 end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
new file mode 100644
index 00000000000..36d10636ae9
--- /dev/null
+++ b/spec/models/ci/build_spec.rb
@@ -0,0 +1,22 @@
+require 'spec_helper'
+
+describe Ci::Build, models: true do
+  let(:build) { create(:ci_build) }
+  let(:test_trace) { 'This is a test' }
+
+  describe '#trace' do
+    it 'obfuscates project runners token' do
+      allow(build).to receive(:raw_trace).and_return("Test: #{build.project.runners_token}")
+
+      expect(build.trace).to eq("Test: xxxxxx")
+    end
+
+    it 'empty project runners token' do
+      allow(build).to receive(:raw_trace).and_return(test_trace)
+      # runners_token can't normally be set to nil
+      allow(build.project).to receive(:runners_token).and_return(nil)
+
+      expect(build.trace).to eq(test_trace)
+    end
+  end
+end