Commit 3b72afe0 authored by Robert Speicher's avatar Robert Speicher Committed by Rémy Coutable

Merge branch '15437-fix-xss-in-issue-tracker-service' into 'master'

Prevent XSS via custom issue tracker URL

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/15437

See merge request !1955
Signed-off-by: default avatarRémy Coutable <remy@rymai.me>
parent d9114c15
Please view this file on the master branch, on stable branches it's out of date. Please view this file on the master branch, on stable branches it's out of date.
v 8.5.12
- Fix a window.opener bug that could lead to XSS and open redirects
- Prevent XSS via Git branch and tag names
- Prevent XSS via custom issue tracker URL
- Fix vulnerability that leaks private labels and milestones
- Prevent XSS with in label dropdown
- Prevent privilege escalation via "impersonate" feature
- Prevent information disclosure via milestone API
- Prevent information disclosure via snippet API
- Prevent information disclosure via new merge request page
v 8.5.11 v 8.5.11
- Fix persistent XSS vulnerability in `commit_person_link` helper - Fix persistent XSS vulnerability in `commit_person_link` helper
......
...@@ -16,31 +16,49 @@ module IssuesHelper ...@@ -16,31 +16,49 @@ module IssuesHelper
def url_for_project_issues(project = @project, options = {}) def url_for_project_issues(project = @project, options = {})
return '' if project.nil? return '' if project.nil?
if options[:only_path] url =
project.issues_tracker.project_path if options[:only_path]
else project.issues_tracker.project_path
project.issues_tracker.project_url else
end project.issues_tracker.project_url
end
# Ensure we return a valid URL to prevent possible XSS.
URI.parse(url).to_s
rescue URI::InvalidURIError
''
end end
def url_for_new_issue(project = @project, options = {}) def url_for_new_issue(project = @project, options = {})
return '' if project.nil? return '' if project.nil?
if options[:only_path] url =
project.issues_tracker.new_issue_path if options[:only_path]
else project.issues_tracker.new_issue_path
project.issues_tracker.new_issue_url else
end project.issues_tracker.new_issue_url
end
# Ensure we return a valid URL to prevent possible XSS.
URI.parse(url).to_s
rescue URI::InvalidURIError
''
end end
def url_for_issue(issue_iid, project = @project, options = {}) def url_for_issue(issue_iid, project = @project, options = {})
return '' if project.nil? return '' if project.nil?
if options[:only_path] url =
project.issues_tracker.issue_path(issue_iid) if options[:only_path]
else project.issues_tracker.issue_path(issue_iid)
project.issues_tracker.issue_url(issue_iid) else
end project.issues_tracker.issue_url(issue_iid)
end
# Ensure we return a valid URL to prevent possible XSS.
URI.parse(url).to_s
rescue URI::InvalidURIError
''
end end
def bulk_update_milestone_options def bulk_update_milestone_options
......
...@@ -26,7 +26,7 @@ class BuildkiteService < CiService ...@@ -26,7 +26,7 @@ class BuildkiteService < CiService
prop_accessor :project_url, :token, :enable_ssl_verification prop_accessor :project_url, :token, :enable_ssl_verification
validates :project_url, presence: true, if: :activated? validates :project_url, presence: true, url: true, if: :activated?
validates :token, presence: true, if: :activated? validates :token, presence: true, if: :activated?
after_save :compose_service_hook, if: :activated? after_save :compose_service_hook, if: :activated?
...@@ -91,7 +91,7 @@ class BuildkiteService < CiService ...@@ -91,7 +91,7 @@ class BuildkiteService < CiService
{ type: 'text', { type: 'text',
name: 'project_url', name: 'project_url',
placeholder: "#{ENDPOINT}/example/project" }, placeholder: "#{ENDPOINT}/example/project" },
{ type: 'checkbox', { type: 'checkbox',
name: 'enable_ssl_verification', name: 'enable_ssl_verification',
title: "Enable SSL verification" } title: "Enable SSL verification" }
......
...@@ -21,7 +21,7 @@ ...@@ -21,7 +21,7 @@
class IssueTrackerService < Service class IssueTrackerService < Service
validates :project_url, :issues_url, :new_issue_url, presence: true, if: :activated? validates :project_url, :issues_url, :new_issue_url, presence: true, url: true, if: :activated?
default_value_for :category, 'issue_tracker' default_value_for :category, 'issue_tracker'
......
...@@ -28,6 +28,8 @@ class JiraService < IssueTrackerService ...@@ -28,6 +28,8 @@ class JiraService < IssueTrackerService
prop_accessor :username, :password, :api_url, :jira_issue_transition_id, prop_accessor :username, :password, :api_url, :jira_issue_transition_id,
:title, :description, :project_url, :issues_url, :new_issue_url :title, :description, :project_url, :issues_url, :new_issue_url
validates :api_url, presence: true, url: true, if: :activated?
before_validation :set_api_url, :set_jira_issue_transition_id before_validation :set_api_url, :set_jira_issue_transition_id
before_update :reset_password before_update :reset_password
......
...@@ -22,7 +22,7 @@ ...@@ -22,7 +22,7 @@
class SlackService < Service class SlackService < Service
prop_accessor :webhook, :username, :channel prop_accessor :webhook, :username, :channel
boolean_accessor :notify_only_broken_builds boolean_accessor :notify_only_broken_builds
validates :webhook, presence: true, if: :activated? validates :webhook, presence: true, url: true, if: :activated?
def initialize_properties def initialize_properties
if properties.nil? if properties.nil?
......
...@@ -30,6 +30,18 @@ describe IssuesHelper do ...@@ -30,6 +30,18 @@ describe IssuesHelper do
expect(url_for_project_issues).to eq "" expect(url_for_project_issues).to eq ""
end end
it 'returns an empty string if project_url is invalid' do
expect(project).to receive_message_chain('issues_tracker.project_url') { 'javascript:alert("foo");' }
expect(url_for_project_issues(project)).to eq ''
end
it 'returns an empty string if project_path is invalid' do
expect(project).to receive_message_chain('issues_tracker.project_path') { 'javascript:alert("foo");' }
expect(url_for_project_issues(project, only_path: true)).to eq ''
end
describe "when external tracker was enabled and then config removed" do describe "when external tracker was enabled and then config removed" do
before do before do
@project = ext_project @project = ext_project
...@@ -68,6 +80,18 @@ describe IssuesHelper do ...@@ -68,6 +80,18 @@ describe IssuesHelper do
expect(url_for_issue(issue.iid)).to eq "" expect(url_for_issue(issue.iid)).to eq ""
end end
it 'returns an empty string if issue_url is invalid' do
expect(project).to receive_message_chain('issues_tracker.issue_url') { 'javascript:alert("foo");' }
expect(url_for_issue(issue.iid, project)).to eq ''
end
it 'returns an empty string if issue_path is invalid' do
expect(project).to receive_message_chain('issues_tracker.issue_path') { 'javascript:alert("foo");' }
expect(url_for_issue(issue.iid, project, only_path: true)).to eq ''
end
describe "when external tracker was enabled and then config removed" do describe "when external tracker was enabled and then config removed" do
before do before do
@project = ext_project @project = ext_project
...@@ -105,6 +129,18 @@ describe IssuesHelper do ...@@ -105,6 +129,18 @@ describe IssuesHelper do
expect(url_for_new_issue).to eq "" expect(url_for_new_issue).to eq ""
end end
it 'returns an empty string if issue_url is invalid' do
expect(project).to receive_message_chain('issues_tracker.new_issue_url') { 'javascript:alert("foo");' }
expect(url_for_new_issue(project)).to eq ''
end
it 'returns an empty string if issue_path is invalid' do
expect(project).to receive_message_chain('issues_tracker.new_issue_path') { 'javascript:alert("foo");' }
expect(url_for_new_issue(project, only_path: true)).to eq ''
end
describe "when external tracker was enabled and then config removed" do describe "when external tracker was enabled and then config removed" do
before do before do
@project = ext_project @project = ext_project
......
...@@ -21,74 +21,226 @@ ...@@ -21,74 +21,226 @@
require 'spec_helper' require 'spec_helper'
describe BambooService, models: true do describe BambooService, models: true do
describe "Associations" do describe 'Associations' do
it { is_expected.to belong_to :project } it { is_expected.to belong_to :project }
it { is_expected.to have_one :service_hook } it { is_expected.to have_one :service_hook }
end end
describe "Execute" do describe 'Validations' do
let(:user) { create(:user) } describe '#bamboo_url' do
let(:project) { create(:project) } it 'does not validate the presence of bamboo_url if service is not active' do
bamboo_service = service
context "when a password was previously set" do bamboo_service.active = false
before do
@bamboo_service = BambooService.create( expect(bamboo_service).not_to validate_presence_of(:bamboo_url)
project: create(:project),
properties: {
bamboo_url: 'http://gitlab.com',
username: 'mic',
password: "password"
}
)
end end
it "reset password if url changed" do it 'validates the presence of bamboo_url if service is active' do
@bamboo_service.bamboo_url = 'http://gitlab1.com' bamboo_service = service
@bamboo_service.save bamboo_service.active = true
expect(@bamboo_service.password).to be_nil
expect(bamboo_service).to validate_presence_of(:bamboo_url)
end end
end
it "does not reset password if username changed" do
@bamboo_service.username = "some_name" describe '#build_key' do
@bamboo_service.save it 'does not validate the presence of build_key if service is not active' do
expect(@bamboo_service.password).to eq("password") bamboo_service = service
bamboo_service.active = false
expect(bamboo_service).not_to validate_presence_of(:build_key)
end end
it "does not reset password if new url is set together with password, even if it's the same password" do it 'validates the presence of build_key if service is active' do
@bamboo_service.bamboo_url = 'http://gitlab_edited.com' bamboo_service = service
@bamboo_service.password = 'password' bamboo_service.active = true
@bamboo_service.save
expect(@bamboo_service.password).to eq("password") expect(bamboo_service).to validate_presence_of(:build_key)
expect(@bamboo_service.bamboo_url).to eq("http://gitlab_edited.com")
end end
end
it "should reset password if url changed, even if setter called multiple times" do describe '#username' do
@bamboo_service.bamboo_url = 'http://gitlab1.com' it 'does not validate the presence of username if service is not active' do
@bamboo_service.bamboo_url = 'http://gitlab1.com' bamboo_service = service
@bamboo_service.save bamboo_service.active = false
expect(@bamboo_service.password).to be_nil
expect(bamboo_service).not_to validate_presence_of(:username)
end
it 'does not validate the presence of username if username is nil' do
bamboo_service = service
bamboo_service.active = true
bamboo_service.password = nil
expect(bamboo_service).not_to validate_presence_of(:username)
end
it 'validates the presence of username if service is active and username is present' do
bamboo_service = service
bamboo_service.active = true
bamboo_service.password = 'secret'
expect(bamboo_service).to validate_presence_of(:username)
end end
end end
context "when no password was previously set" do describe '#password' do
before do it 'does not validate the presence of password if service is not active' do
@bamboo_service = BambooService.create( bamboo_service = service
project: create(:project), bamboo_service.active = false
properties: {
bamboo_url: 'http://gitlab.com', expect(bamboo_service).not_to validate_presence_of(:password)
username: 'mic'
}
)
end end
it "saves password if new url is set together with password" do it 'does not validate the presence of password if username is nil' do
@bamboo_service.bamboo_url = 'http://gitlab_edited.com' bamboo_service = service
@bamboo_service.password = 'password' bamboo_service.active = true
@bamboo_service.save bamboo_service.username = nil
expect(@bamboo_service.password).to eq("password")
expect(@bamboo_service.bamboo_url).to eq("http://gitlab_edited.com") expect(bamboo_service).not_to validate_presence_of(:password)
end end
it 'validates the presence of password if service is active and username is present' do
bamboo_service = service
bamboo_service.active = true
bamboo_service.username = 'john'
expect(bamboo_service).to validate_presence_of(:password)
end
end
end
describe 'Callbacks' do
describe 'before_update :reset_password' do
context 'when a password was previously set' do
it 'resets password if url changed' do
bamboo_service = service
bamboo_service.bamboo_url = 'http://gitlab1.com'
bamboo_service.save
expect(bamboo_service.password).to be_nil
end
it 'does not reset password if username changed' do
bamboo_service = service
bamboo_service.username = 'some_name'
bamboo_service.save
expect(bamboo_service.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
bamboo_service = service
bamboo_service.bamboo_url = 'http://gitlab_edited.com'
bamboo_service.password = 'password'
bamboo_service.save
expect(bamboo_service.password).to eq('password')
expect(bamboo_service.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
bamboo_service = service
bamboo_service.password = nil
bamboo_service.bamboo_url = 'http://gitlab_edited.com'
bamboo_service.password = 'password'
bamboo_service.save
expect(bamboo_service.password).to eq('password')
expect(bamboo_service.bamboo_url).to eq('http://gitlab_edited.com')
end
end end
end end
describe '#build_page' do
it 'returns a specific URL when status is 500' do
stub_request(status: 500)
expect(service.build_page('123', 'unused')).to eq('http://gitlab.com/browse/foo')
end
it 'returns a specific URL when response has no results' do
stub_request(body: %Q({"results":{"results":{"size":"0"}}}))
expect(service.build_page('123', 'unused')).to eq('http://gitlab.com/browse/foo')
end
it 'returns a build URL when bamboo_url has no trailing slash' do
stub_request(body: %Q({"results":{"results":{"result":{"planResultKey":{"key":"42"}}}}}))
expect(service(bamboo_url: 'http://gitlab.com').build_page('123', 'unused')).to eq('http://gitlab.com/browse/42')
end
end
describe '#commit_status' do
it 'sets commit status to :error when status is 500' do
stub_request(status: 500)
expect(service.commit_status('123', 'unused')).to eq(:error)
end
it 'sets commit status to "pending" when status is 404' do
stub_request(status: 404)
expect(service.commit_status('123', 'unused')).to eq('pending')
end
it 'sets commit status to "pending" when response has no results' do
stub_request(body: %Q({"results":{"results":{"size":"0"}}}))
expect(service.commit_status('123', 'unused')).to eq('pending')
end
it 'sets commit status to "success" when build state contains Success' do
stub_request(build_state: 'YAY Success!')
expect(service.commit_status('123', 'unused')).to eq('success')
end
it 'sets commit status to "failed" when build state contains Failed' do
stub_request(build_state: 'NO Failed!')
expect(service.commit_status('123', 'unused')).to eq('failed')
end
it 'sets commit status to "pending" when build state contains Pending' do
stub_request(build_state: 'NO Pending!')
expect(service.commit_status('123', 'unused')).to eq('pending')
end
it 'sets commit status to :error when build state is unknown' do
stub_request(build_state: 'FOO BAR!')
expect(service.commit_status('123', 'unused')).to eq(:error)
end
end
def service(bamboo_url: 'http://gitlab.com')
described_class.create(
project: build_stubbed(:empty_project),
properties: {
bamboo_url: bamboo_url,
username: 'mic',
password: 'password',
build_key: 'foo'
}
)
end
def stub_request(status: 200, body: nil, build_state: 'success')
bamboo_full_url = 'http://mic:password@gitlab.com/rest/api/latest/result?label=123&os_authType=basic'
body ||= %Q({"results":{"results":{"result":{"buildState":"#{build_state}"}}}})
WebMock.stub_request(:get, bamboo_full_url).to_return(
status: status,
headers: { 'Content-Type' => 'application/json' },
body: body
)
end
end end
...@@ -26,6 +26,23 @@ describe BuildkiteService, models: true do ...@@ -26,6 +26,23 @@ describe BuildkiteService, models: true do
it { is_expected.to have_one :service_hook } it { is_expected.to have_one :service_hook }
end end
describe 'Validations' do
context 'when service is active' do
before { subject.active = true }
it { is_expected.to validate_presence_of(:project_url) }
it { is_expected.to validate_presence_of(:token) }
it_behaves_like 'issue tracker service URL attribute', :project_url
end
context 'when service is inactive' do
before { subject.active = false }
it { is_expected.not_to validate_presence_of(:project_url) }
it { is_expected.not_to validate_presence_of(:token) }
end
end
describe 'commits methods' do describe 'commits methods' do
before do before do
@project = Project.new @project = Project.new
......
...@@ -3,21 +3,62 @@ require 'spec_helper' ...@@ -3,21 +3,62 @@ require 'spec_helper'
describe BuildsEmailService do describe BuildsEmailService do
let(:build) { create(:ci_build) } let(:build) { create(:ci_build) }
let(:data) { Gitlab::BuildDataBuilder.build(build) } let(:data) { Gitlab::BuildDataBuilder.build(build) }
let(:service) { BuildsEmailService.new } let!(:project) { create(:project, :public, ci_id: 1) }
let(:service) { described_class.new(project: project, active: true) }
describe :execute do describe '#execute' do
it "sends email" do it 'sends email' do
service.recipients = 'test@gitlab.com' service.recipients = 'test@gitlab.com'
data[:build_status] = 'failed' data[:build_status] = 'failed'
expect(BuildEmailWorker).to receive(:perform_async) expect(BuildEmailWorker).to receive(:perform_async)
service.execute(data) service.execute(data)
end end
it "does not sends email with failed build and allowed_failure on" do it 'does not send email with succeeded build and notify_only_broken_builds on' do
expect(service).to receive(:notify_only_broken_builds).and_return(true)
data[:build_status] = 'success'
expect(BuildEmailWorker).not_to receive(:perform_async)
service.execute(data)
end
it 'does not send email with failed build and build_allow_failure is true' do
data[:build_status] = 'failed' data[:build_status] = 'failed'
data[:build_allow_failure] = true data[:build_allow_failure] = true
expect(BuildEmailWorker).not_to receive(:perform_async) expect(BuildEmailWorker).not_to receive(:perform_async)
service.execute(data) service.execute(data)
end end
it 'does not send email with unknown build status' do
data[:build_status] = 'foo'
expect(BuildEmailWorker).not_to receive(:perform_async)
service.execute(data)
end
end
describe 'validations' do
context 'when pusher is not added' do
before { service.add_pusher = false }
it 'does not allow empty recipient input' do
service.recipients = ''
expect(service.valid?).to be false
end
it 'does allow non-empty recipient input' do
service.recipients = 'test@example.com'
expect(service.valid?).to be true
end
end
context 'when pusher is added' do
before { service.add_pusher = true }
it 'does allow non-empty recipient input' do
service.recipients = 'test@example.com'
expect(service.valid?).to be true
end
end
end end
end end
# == Schema Information
#
# Table name: services
#
# id :integer not null, primary key
# type :string(255)
# title :string(255)
# project_id :integer
# created_at :datetime
# updated_at :datetime
# active :boolean default(FALSE), not null
# properties :text
# template :boolean default(FALSE)
# push_events :boolean default(TRUE)
# issues_events :boolean default(TRUE)
# merge_requests_events :boolean default(TRUE)
# tag_push_events :boolean default(TRUE)
# note_events :boolean default(TRUE), not null
#
require 'spec_helper'
describe CampfireService, models: true do
describe 'Associations' do
it { is_expected.to belong_to :project }
it { is_expected.to have_one :service_hook }
end
describe 'Validations' do
context 'when service is active' do
before { subject.active = true }
it { is_expected.to validate_presence_of(:token) }
end
context 'when service is inactive' do
before { subject.active = false }
it { is_expected.not_to validate_presence_of(:token) }
end
end
end
# == Schema Information
#
# Table name: services
#
# id :integer not null, primary key
# type :string(255)
# title :string(255)
# project_id :integer
# created_at :datetime
# updated_at :datetime
# active :boolean default(FALSE), not null
# properties :text
# template :boolean default(FALSE)
# push_events :boolean default(TRUE)
# issues_events :boolean default(TRUE)
# merge_requests_events :boolean default(TRUE)
# tag_push_events :boolean default(TRUE)
# note_events :boolean default(TRUE), not null
#
require 'spec_helper'
describe CustomIssueTrackerService, models: true do
describe 'Associations' do
it { is_expected.to belong_to :project }
it { is_expected.to have_one :service_hook }
end
describe 'Validations' do
context 'when service is active' do
before { subject.active = true }
it { is_expected.to validate_presence_of(:project_url) }
it { is_expected.to validate_presence_of(:issues_url) }
it { is_expected.to validate_presence_of(:new_issue_url) }
it_behaves_like 'issue tracker service URL attribute', :project_url
it_behaves_like 'issue tracker service URL attribute', :issues_url
it_behaves_like 'issue tracker service URL attribute', :new_issue_url
end
context 'when service is inactive' do
before { subject.active = false }
it { is_expected.not_to validate_presence_of(:project_url) }
it { is_expected.not_to validate_presence_of(:issues_url) }
it { is_expected.not_to validate_presence_of(:new_issue_url) }
end
end
end
...@@ -28,25 +28,18 @@ describe DroneCiService, models: true do ...@@ -28,25 +28,18 @@ describe DroneCiService, models: true do
describe 'validations' do describe 'validations' do
context 'active' do context 'active' do
before { allow(subject).to receive(:activated?).and_return(true) } before { subject.active = true }
it { is_expected.to validate_presence_of(:token) } it { is_expected.to validate_presence_of(:token) }
it { is_expected.to validate_presence_of(:drone_url) } it { is_expected.to validate_presence_of(:drone_url) }
it { is_expected.to allow_value('ewf9843kdnfdfs89234n').for(:token) } it_behaves_like 'issue tracker service URL attribute', :drone_url
it { is_expected.to allow_value('http://ci.example.com').for(:drone_url) }
it { is_expected.not_to allow_value('this is not url').for(:drone_url) }
it { is_expected.not_to allow_value('http//noturl').for(:drone_url) }
it { is_expected.not_to allow_value('ftp://ci.example.com').for(:drone_url) }
end end
context 'inactive' do context 'inactive' do
before { allow(subject).to receive(:activated?).and_return(false) } before { subject.active = false }
it { is_expected.not_to validate_presence_of(:token) } it { is_expected.not_to validate_presence_of(:token) }
it { is_expected.not_to validate_presence_of(:drone_url) } it { is_expected.not_to validate_presence_of(:drone_url) }
it { is_expected.to allow_value('ewf9843kdnfdfs89234n').for(:token) }
it { is_expected.to allow_value('http://drone.example.com').for(:drone_url) }
it { is_expected.to allow_value('ftp://drone.example.com').for(:drone_url) }
end end
end end
......
require 'spec_helper'
describe EmailsOnPushService do
describe 'Validations' do
context 'when service is active' do
before { subject.active = true }
it { is_expected.to validate_presence_of(:recipients) }
end
context 'when service is inactive' do
before { subject.active = false }
it { is_expected.not_to validate_presence_of(:recipients) }
end
end
end
...@@ -28,13 +28,18 @@ describe ExternalWikiService, models: true do ...@@ -28,13 +28,18 @@ describe ExternalWikiService, models: true do
it { should have_one :service_hook } it { should have_one :service_hook }
end end
describe "Validations" do describe 'Validations' do
context "active" do context 'when service is active' do
before do before { subject.active = true }
subject.active = true
end it { is_expected.to validate_presence_of(:external_wiki_url) }
it_behaves_like 'issue tracker service URL attribute', :external_wiki_url
end
context 'when service is inactive' do
before { subject.active = false }
it { should validate_presence_of :external_wiki_url } it { is_expected.not_to validate_presence_of(:external_wiki_url) }
end end
end end
......
...@@ -26,6 +26,20 @@ describe FlowdockService, models: true do ...@@ -26,6 +26,20 @@ describe FlowdockService, models: true do
it { is_expected.to have_one :service_hook } it { is_expected.to have_one :service_hook }
end end
describe 'Validations' do
context 'when service is active' do
before { subject.active = true }
it { is_expected.to validate_presence_of(:token) }
end
context 'when service is inactive' do
before { subject.active = false }
it { is_expected.not_to validate_presence_of(:token) }
end
end
describe "Execute" do describe "Execute" do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:project) { create(:project) } let(:project) { create(:project) }
......
...@@ -26,6 +26,22 @@ describe GemnasiumService, models: true do ...@@ -26,6 +26,22 @@ describe GemnasiumService, models: true do
it { is_expected.to have_one :service_hook } it { is_expected.to have_one :service_hook }
end end
describe 'Validations' do
context 'when service is active' do
before { subject.active = true }
it { is_expected.to validate_presence_of(:token) }
it { is_expected.to validate_presence_of(:api_key) }
end
context 'when service is inactive' do
before { subject.active = false }
it { is_expected.not_to validate_presence_of(:token) }
it { is_expected.not_to validate_presence_of(:api_key) }
end
end
describe "Execute" do describe "Execute" do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:project) { create(:project) } let(:project) { create(:project) }
......
...@@ -26,6 +26,20 @@ describe GitlabIssueTrackerService, models: true do ...@@ -26,6 +26,20 @@ describe GitlabIssueTrackerService, models: true do
it { is_expected.to have_one :service_hook } it { is_expected.to have_one :service_hook }
end end
describe 'Validations' do
context 'when service is active' do
subject { described_class.new(project: create(:project), active: true) }
it { is_expected.to validate_presence_of(:issues_url) }
it_behaves_like 'issue tracker service URL attribute', :issues_url
end
context 'when service is inactive' do
subject { described_class.new(project: create(:project), active: false) }
it { is_expected.not_to validate_presence_of(:issues_url) }
end
end
describe 'project and issue urls' do describe 'project and issue urls' do
let(:project) { create(:project) } let(:project) { create(:project) }
......
...@@ -26,6 +26,20 @@ describe HipchatService, models: true do ...@@ -26,6 +26,20 @@ describe HipchatService, models: true do
it { is_expected.to have_one :service_hook } it { is_expected.to have_one :service_hook }
end end
describe 'Validations' do
context 'when service is active' do
before { subject.active = true }
it { is_expected.to validate_presence_of(:token) }
end
context 'when service is inactive' do
before { subject.active = false }
it { is_expected.not_to validate_presence_of(:token) }
end
end
describe "Execute" do describe "Execute" do
let(:hipchat) { HipchatService.new } let(:hipchat) { HipchatService.new }
let(:user) { create(:user, username: 'username') } let(:user) { create(:user, username: 'username') }
......
...@@ -29,14 +29,16 @@ describe IrkerService, models: true do ...@@ -29,14 +29,16 @@ describe IrkerService, models: true do
end end
describe 'Validations' do describe 'Validations' do
before do context 'when service is active' do
subject.active = true before { subject.active = true }
subject.properties['recipients'] = _recipients
it { is_expected.to validate_presence_of(:recipients) }
end end
context 'active' do context 'when service is inactive' do
let(:_recipients) { nil } before { subject.active = false }
it { should validate_presence_of :recipients }
it { is_expected.not_to validate_presence_of(:recipients) }
end end
end end
......
...@@ -26,6 +26,30 @@ describe JiraService, models: true do ...@@ -26,6 +26,30 @@ describe JiraService, models: true do
it { is_expected.to have_one :service_hook } it { is_expected.to have_one :service_hook }
end end
describe 'Validations' do
context 'when service is active' do
before { subject.active = true }
it { is_expected.to validate_presence_of(:api_url) }
it { is_expected.to validate_presence_of(:project_url) }
it { is_expected.to validate_presence_of(:issues_url) }
it { is_expected.to validate_presence_of(:new_issue_url) }
it_behaves_like 'issue tracker service URL attribute', :api_url
it_behaves_like 'issue tracker service URL attribute', :project_url
it_behaves_like 'issue tracker service URL attribute', :issues_url
it_behaves_like 'issue tracker service URL attribute', :new_issue_url
end
context 'when service is inactive' do
before { subject.active = false }
it { is_expected.not_to validate_presence_of(:api_url) }
it { is_expected.not_to validate_presence_of(:project_url) }
it { is_expected.not_to validate_presence_of(:issues_url) }
it { is_expected.not_to validate_presence_of(:new_issue_url) }
end
end
describe "Execute" do describe "Execute" do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:project) { create(:project) } let(:project) { create(:project) }
...@@ -72,7 +96,7 @@ describe JiraService, models: true do ...@@ -72,7 +96,7 @@ describe JiraService, models: true do
context "when a password was previously set" do context "when a password was previously set" do
before do before do
@jira_service = JiraService.create( @jira_service = JiraService.create!(
project: create(:project), project: create(:project),
properties: { properties: {
api_url: 'http://jira.example.com/rest/api/2', api_url: 'http://jira.example.com/rest/api/2',
......
# == Schema Information
#
# Table name: services
#
# id :integer not null, primary key
# type :string(255)
# title :string(255)
# project_id :integer
# created_at :datetime
# updated_at :datetime
# active :boolean default(FALSE), not null
# properties :text
# template :boolean default(FALSE)
# push_events :boolean default(TRUE)
# issues_events :boolean default(TRUE)
# merge_requests_events :boolean default(TRUE)
# tag_push_events :boolean default(TRUE)
# note_events :boolean default(TRUE), not null
#
require 'spec_helper'
describe PivotaltrackerService, models: true do
describe 'Associations' do
it { is_expected.to belong_to :project }
it { is_expected.to have_one :service_hook }
end
describe 'Validations' do
context 'when service is active' do
before { subject.active = true }
it { is_expected.to validate_presence_of(:token) }
end
context 'when service is inactive' do
before { subject.active = false }
it { is_expected.not_to validate_presence_of(:token) }
end
end
end
...@@ -27,14 +27,20 @@ describe PushoverService, models: true do ...@@ -27,14 +27,20 @@ describe PushoverService, models: true do
end end
describe 'Validations' do describe 'Validations' do
context 'active' do context 'when service is active' do
before do before { subject.active = true }
subject.active = true
end
it { is_expected.to validate_presence_of :api_key } it { is_expected.to validate_presence_of(:api_key) }
it { is_expected.to validate_presence_of :user_key } it { is_expected.to validate_presence_of(:user_key) }
it { is_expected.to validate_presence_of :priority } it { is_expected.to validate_presence_of(:priority) }
end
context 'when service is inactive' do
before { subject.active = false }
it { is_expected.not_to validate_presence_of(:api_key) }
it { is_expected.not_to validate_presence_of(:user_key) }
it { is_expected.not_to validate_presence_of(:priority) }
end end
end end
......
# == Schema Information
#
# Table name: services
#
# id :integer not null, primary key
# type :string(255)
# title :string(255)
# project_id :integer
# created_at :datetime
# updated_at :datetime
# active :boolean default(FALSE), not null
# properties :text
# template :boolean default(FALSE)
# push_events :boolean default(TRUE)
# issues_events :boolean default(TRUE)
# merge_requests_events :boolean default(TRUE)
# tag_push_events :boolean default(TRUE)
# note_events :boolean default(TRUE), not null
#
require 'spec_helper'
describe RedmineService, models: true do
describe 'Associations' do
it { is_expected.to belong_to :project }
it { is_expected.to have_one :service_hook }
end
describe 'Validations' do
context 'when service is active' do
before { subject.active = true }
it { is_expected.to validate_presence_of(:project_url) }
it { is_expected.to validate_presence_of(:issues_url) }
it { is_expected.to validate_presence_of(:new_issue_url) }
it_behaves_like 'issue tracker service URL attribute', :project_url
it_behaves_like 'issue tracker service URL attribute', :issues_url
it_behaves_like 'issue tracker service URL attribute', :new_issue_url
end
context 'when service is inactive' do
before { subject.active = false }
it { is_expected.not_to validate_presence_of(:project_url) }
it { is_expected.not_to validate_presence_of(:issues_url) }
it { is_expected.not_to validate_presence_of(:new_issue_url) }
end
end
end
...@@ -26,13 +26,18 @@ describe SlackService, models: true do ...@@ -26,13 +26,18 @@ describe SlackService, models: true do
it { is_expected.to have_one :service_hook } it { is_expected.to have_one :service_hook }
end end
describe "Validations" do describe 'Validations' do
context "active" do context 'when service is active' do
before do before { subject.active = true }
subject.active = true
end
it { is_expected.to validate_presence_of :webhook } it { is_expected.to validate_presence_of(:webhook) }
it_behaves_like 'issue tracker service URL attribute', :webhook
end
context 'when service is inactive' do
before { subject.active = false }
it { is_expected.not_to validate_presence_of(:webhook) }
end end
end end
......
...@@ -21,73 +21,214 @@ ...@@ -21,73 +21,214 @@
require 'spec_helper' require 'spec_helper'
describe TeamcityService, models: true do describe TeamcityService, models: true do
describe "Associations" do describe 'Associations' do
it { is_expected.to belong_to :project } it { is_expected.to belong_to :project }
it { is_expected.to have_one :service_hook } it { is_expected.to have_one :service_hook }
end end
describe "Execute" do describe 'Validations' do
let(:user) { create(:user) } describe '#teamcity_url' do
let(:project) { create(:project) } it 'does not validate the presence of teamcity_url if service is not active' do
teamcity_service = service
context "when a password was previously set" do teamcity_service.active = false
before do
@teamcity_service = TeamcityService.create( expect(teamcity_service).not_to validate_presence_of(:teamcity_url)
project: create(:project), end
properties: {
teamcity_url: 'http://gitlab.com', it 'validates the presence of teamcity_url if service is active' do
username: 'mic', teamcity_service = service
password: "password" teamcity_service.active = true
}
) expect(teamcity_service).to validate_presence_of(:teamcity_url)
end
end
describe '#build_type' do
it 'does not validate the presence of build_type if service is not active' do
teamcity_service = service
teamcity_service.active = false
expect(teamcity_service).not_to validate_presence_of(:build_type)
end
it 'validates the presence of build_type if service is active' do
teamcity_service = service
teamcity_service.active = true
expect(teamcity_service).to validate_presence_of(:build_type)
end
end
describe '#username' do
it 'does not validate the presence of username if service is not active' do
teamcity_service = service
teamcity_service.active = false
expect(teamcity_service).not_to validate_presence_of(:username)
end
it 'does not validate the presence of username if username is nil' do
teamcity_service = service
teamcity_service.active = true
teamcity_service.password = nil
expect(teamcity_service).not_to validate_presence_of(:username)
end end
it "reset password if url changed" do it 'validates the presence of username if service is active and username is present' do
@teamcity_service.teamcity_url = 'http://gitlab1.com' teamcity_service = service
@teamcity_service.save teamcity_service.active = true
expect(@teamcity_service.password).to be_nil teamcity_service.password = 'secret'
expect(teamcity_service).to validate_presence_of(:username)
end end
end
it "does not reset password if username changed" do
@teamcity_service.username = "some_name" describe '#password' do
@teamcity_service.save it 'does not validate the presence of password if service is not active' do
expect(@teamcity_service.password).to eq("password") teamcity_service = service
teamcity_service.active = false
expect(teamcity_service).not_to validate_presence_of(:password)
end end
it "does not reset password if new url is set together with password, even if it's the same password" do it 'does not validate the presence of password if username is nil' do
@teamcity_service.teamcity_url = 'http://gitlab_edited.com' teamcity_service = service
@teamcity_service.password = 'password' teamcity_service.active = true
@teamcity_service.save teamcity_service.username = nil
expect(@teamcity_service.password).to eq("password")
expect(@teamcity_service.teamcity_url).to eq("http://gitlab_edited.com") expect(teamcity_service).not_to validate_presence_of(:password)
end end
it "should reset password if url changed, even if setter called multiple times" do it 'validates the presence of password if service is active and username is present' do
@teamcity_service.teamcity_url = 'http://gitlab1.com' teamcity_service = service
@teamcity_service.teamcity_url = 'http://gitlab1.com' teamcity_service.active = true
@teamcity_service.save teamcity_service.username = 'john'
expect(@teamcity_service.password).to be_nil
expect(teamcity_service).to validate_presence_of(:password)
end end
end end
end
context "when no password was previously set" do
before do describe 'Callbacks' do
@teamcity_service = TeamcityService.create( describe 'before_update :reset_password' do
project: create(:project), context 'when a password was previously set' do
properties: { it 'resets password if url changed' do
teamcity_url: 'http://gitlab.com', teamcity_service = service
username: 'mic'
} teamcity_service.teamcity_url = 'http://gitlab1.com'
) teamcity_service.save
expect(teamcity_service.password).to be_nil
end
it 'does not reset password if username changed' do
teamcity_service = service
teamcity_service.username = 'some_name'
teamcity_service.save
expect(teamcity_service.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_service = service
teamcity_service.teamcity_url = 'http://gitlab_edited.com'
teamcity_service.password = 'password'
teamcity_service.save
expect(teamcity_service.password).to eq('password')
expect(teamcity_service.teamcity_url).to eq('http://gitlab_edited.com')
end
end end
it "saves password if new url is set together with password" do it 'saves password if new url is set together with password when no password was previously set' do
@teamcity_service.teamcity_url = 'http://gitlab_edited.com' teamcity_service = service
@teamcity_service.password = 'password' teamcity_service.password = nil
@teamcity_service.save
expect(@teamcity_service.password).to eq("password") teamcity_service.teamcity_url = 'http://gitlab_edited.com'
expect(@teamcity_service.teamcity_url).to eq("http://gitlab_edited.com") teamcity_service.password = 'password'
teamcity_service.save
expect(teamcity_service.password).to eq('password')
expect(teamcity_service.teamcity_url).to eq('http://gitlab_edited.com')
end end
end end
end end
describe '#build_page' do
it 'returns a specific URL when status is 500' do
stub_request(status: 500)
expect(service.build_page('123', 'unused')).to eq('http://gitlab.com/viewLog.html?buildTypeId=foo')
end
it 'returns a build URL when teamcity_url has no trailing slash' do
stub_request(body: %Q({"build":{"id":"666"}}))
expect(service(teamcity_url: 'http://gitlab.com').build_page('123', 'unused')).to eq('http://gitlab.com/viewLog.html?buildId=666&buildTypeId=foo')
end
end
describe '#commit_status' do
it 'sets commit status to :error when status is 500' do
stub_request(status: 500)
expect(service.commit_status('123', 'unused')).to eq(:error)
end
it 'sets commit status to "pending" when status is 404' do
stub_request(status: 404)
expect(service.commit_status('123', 'unused')).to eq('pending')
end
it 'sets commit status to "success" when build status contains SUCCESS' do
stub_request(build_status: 'YAY SUCCESS!')
expect(service.commit_status('123', 'unused')).to eq('success')
end
it 'sets commit status to "failed" when build status contains FAILURE' do
stub_request(build_status: 'NO FAILURE!')
expect(service.commit_status('123', 'unused')).to eq('failed')
end
it 'sets commit status to "pending" when build status contains Pending' do
stub_request(build_status: 'NO Pending!')
expect(service.commit_status('123', 'unused')).to eq('pending')
end
it 'sets commit status to :error when build status is unknown' do
stub_request(build_status: 'FOO BAR!')
expect(service.commit_status('123', 'unused')).to eq(:error)
end
end
def service(teamcity_url: 'http://gitlab.com')
described_class.create(
project: build_stubbed(:empty_project),
properties: {
teamcity_url: teamcity_url,
username: 'mic',
password: 'password',
build_type: 'foo'
}
)
end
def stub_request(status: 200, body: nil, build_status: 'success')
teamcity_full_url = 'http://mic:password@gitlab.com/httpAuth/app/rest/builds/branch:unspecified:any,number:123'
body ||= %Q({"build":{"status":"#{build_status}","id":"666"}})
WebMock.stub_request(:get, teamcity_full_url).to_return(
status: status,
headers: { 'Content-Type' => 'application/json' },
body: body
)
end
end end
RSpec.shared_examples 'issue tracker service URL attribute' do |url_attr|
it { is_expected.to allow_value('https://example.com').for(url_attr) }
it { is_expected.not_to allow_value('example.com').for(url_attr) }
it { is_expected.not_to allow_value('ftp://example.com').for(url_attr) }
it { is_expected.not_to allow_value('herp-and-derp').for(url_attr) }
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