Commit 4b4fdf58 authored by Stan Hu's avatar Stan Hu

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
parent 70cba8e9
......@@ -4,6 +4,7 @@ v 8.4.0 (unreleased)
- The default GitLab logo now acts as a loading indicator
- Accept 2xx status codes for successful Web hook triggers (Stan Hu)
- Fix missing date of month in network graph when commits span a month (Stan Hu)
- Fix Error 500 when visiting build page of project with nil runners_token (Stan Hu)
- Expire view caches when application settings change (e.g. Gravatar disabled) (Stan Hu)
- Don't notify users twice if they are both project watchers and subscribers (Stan Hu)
- Implement new UI for group page
......
......@@ -206,7 +206,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
......
......@@ -50,6 +50,7 @@ class Project < ActiveRecord::Base
include Sortable
include AfterCommitQueue
include CaseSensitivity
include TokenAuthenticatable
extend Gitlab::ConfigHelper
......@@ -193,10 +194,7 @@ 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
mount_uploader :avatar, AvatarUploader
......@@ -900,4 +898,8 @@ class Project < ActiveRecord::Base
return true unless forked?
Gitlab::VisibilityLevel.allowed_fork_levels(forked_from_project.visibility_level).include?(level.to_i)
end
def runners_token
ensure_runners_token!
end
end
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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment