Commit fb7b8265 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 47608924
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.8.0 (unreleased) v 8.6.8
- Fix a window.opener bug that could lead to XSS and open redirects
v 8.7.0 (unreleased) - Prevent XSS via Git branch and tag names
- Gitlab::GitAccess and Gitlab::GitAccessWiki are now instrumented - Prevent XSS via custom issue tracker URL
- Fix vulnerability that made it possible to gain access to private labels and milestones - Fix vulnerability that leaks private labels and milestones
- The number of InfluxDB points stored per UDP packet can now be configured - Prevent XSS with in label dropdown
- Fix error when cross-project label reference used with non-existent project - Prevent privilege escalation via "impersonate" feature
- Transactions for /internal/allowed now have an "action" tag set - Prevent information disclosure via milestone API
- Method instrumentation now uses Module#prepend instead of aliasing methods - Prevent information disclosure via snippet API
- Repository.clean_old_archives is now instrumented - Prevent information disclosure via new merge request page
- Add support for environment variables on a job level in CI configuration file
- SQL query counts are now tracked per transaction
- The Projects::HousekeepingService class has extra instrumentation
- All service classes (those residing in app/services) are now instrumented
- Developers can now add custom tags to transactions
- Loading of an issue's referenced merge requests and related branches is now done asynchronously
- Enable gzip for assets, makes the page size significantly smaller. !3544 / !3632 (Connor Shea)
- Add support to cherry-pick any commit into any branch in the web interface (Minqi Pan)
- Project switcher uses new dropdown styling
- Load award emoji images separately unless opening the full picker. Saves several hundred KBs of data for most pages. (Connor Shea)
- Do not include award_emojis in issue and merge_request comment_count !3610 (Lucas Charles)
- Restrict user profiles when public visibility level is restricted.
- Add ability set due date to issues, sort and filter issues by due date (Mehmet Beydogan)
- All images in discussions and wikis now link to their source files !3464 (Connor Shea).
- Return status code 303 after a branch DELETE operation to avoid project deletion (Stan Hu)
- Add setting for customizing the list of trusted proxies !3524
- Allow projects to be transfered to a lower visibility level group
- Fix `signed_in_ip` being set to 127.0.0.1 when using a reverse proxy !3524
- Improved Markdown rendering performance !3389
- Make shared runners text in box configurable
- Don't attempt to look up an avatar in repo if repo directory does not exist (Stan Hu)
- API: Ability to subscribe and unsubscribe from issues and merge requests (Robert Schilling)
- Expose project badges in project settings
- Make /profile/keys/new redirect to /profile/keys for back-compat. !3717
- Preserve time notes/comments have been updated at when moving issue
- Make HTTP(s) label consistent on clone bar (Stan Hu)
- Add support for `after_script`, requires Runner 1.2 (Kamil Trzciński)
- Expose label description in API (Mariusz Jachimowicz)
- API: Ability to update a group (Robert Schilling)
- API: Ability to move issues (Robert Schilling)
- Fix Error 500 after renaming a project path (Stan Hu)
- Fix a bug whith trailing slash in teamcity_url (Charles May)
- Allow back dating on issues when created or updated through the API
- Allow back dating on issue notes when created through the API
- Propose license template when creating a new LICENSE file
- API: Expose /licenses and /licenses/:key
- Fix avatar stretching by providing a cropping feature
- API: Expose `subscribed` for issues and merge requests (Robert Schilling)
- Allow SAML to handle external users based on user's information !3530
- Allow Omniauth providers to be marked as `external` !3657
- Add endpoints to archive or unarchive a project !3372
- Fix a bug whith trailing slash in bamboo_url
- Add links to CI setup documentation from project settings and builds pages
- Display project members page to all members
- Handle nil descriptions in Slack issue messages (Stan Hu)
- Add automated repository integrity checks (OFF by default)
- API: Expose open_issues_count, closed_issues_count, open_merge_requests_count for labels (Robert Schilling)
- API: Ability to star and unstar a project (Robert Schilling)
- Add default scope to projects to exclude projects pending deletion
- Allow to close merge requests which source projects(forks) are deleted.
- Ensure empty recipients are rejected in BuildsEmailService
- Use rugged to change HEAD in Project#change_head (P.S.V.R)
- API: Ability to filter milestones by state `active` and `closed` (Robert Schilling)
- API: Fix milestone filtering by `iid` (Robert Schilling)
- Make before_script and after_script overridable on per-job (Kamil Trzciński)
- API: Delete notes of issues, snippets, and merge requests (Robert Schilling)
- Implement 'Groups View' as an option for dashboard preferences !3379 (Elias W.)
- Better errors handling when creating milestones inside groups
- Fix high CPU usage when PostReceive receives refs/merge-requests/<id>
- Hide `Create a group` help block when creating a new project in a group
- Implement 'TODOs View' as an option for dashboard preferences !3379 (Elias W.)
- Allow issues and merge requests to be assigned to the author !2765
- Make Ci::Commit to group only similar builds and make it stateful (ref, tag)
- Gracefully handle notes on deleted commits in merge requests (Stan Hu)
- Decouple membership and notifications
- Fix creation of merge requests for orphaned branches (Stan Hu)
- API: Ability to retrieve a single tag (Robert Schilling)
- While signing up, don't persist the user password across form redisplays
- Fall back to `In-Reply-To` and `References` headers when sub-addressing is not available (David Padilla)
- Remove "Congratulations!" tweet button on newly-created project. (Connor Shea)
- Fix admin/projects when using visibility levels on search (PotHix)
- Build status notifications
- API: Expose user location (Robert Schilling)
- API: Do not leak group existence via return code (Robert Schilling)
- ClosingIssueExtractor regex now also works with colons. e.g. "Fixes: #1234" !3591
- Update number of Todos in the sidebar when it's marked as "Done". !3600
- Sanitize branch names created for confidential issues
- API: Expose 'updated_at' for issue, snippet, and merge request notes (Robert Schilling)
- API: User can leave a project through the API when not master or owner. !3613
- Fix repository cache invalidation issue when project is recreated with an empty repo (Stan Hu)
- Fix: Allow empty recipients list for builds emails service when pushed is added (Frank Groeneveld)
- Improved markdown forms
- Diff design updates (colors, button styles, etc)
- Copying and pasting a diff no longer pastes the line numbers or +/-
- Add null check to formData when updating profile content to fix Firefox bug
- Disable spellcheck and autocorrect for username field in admin page
- Delete tags using Rugged for performance reasons (Robert Schilling)
- Add Slack notifications when Wiki is edited (Sebastian Klier)
- Diffs load at the correct point when linking from from number
- Selected diff rows highlight
- Fix emoji categories in the emoji picker
- API: Properly display annotated tags for GET /projects/:id/repository/tags (Robert Schilling)
- Add encrypted credentials for imported projects and migrate old ones
- Properly format all merge request references with ! rather than # !3740 (Ben Bodenmiller)
- Author and participants are displayed first on users autocompletion
- Show number sign on external issue reference text (Florent Baldino)
- Updated print style for issues
- Use GitHub Issue/PR number as iid to keep references
- Import GitHub labels
- Import GitHub milestones
- Fix emoji catgories in the emoji picker
- Execute system web hooks on push to the project
- Allow enable/disable push events for system hooks
- Fix GitHub project's link in the import page when provider has a custom URL
- Add RAW build trace output and button on build page
- Add incremental build trace update into CI API
v 8.6.7 v 8.6.7
- 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?
url =
if options[:only_path] if options[:only_path]
project.issues_tracker.project_path project.issues_tracker.project_path
else else
project.issues_tracker.project_url project.issues_tracker.project_url
end 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?
url =
if options[:only_path] if options[:only_path]
project.issues_tracker.new_issue_path project.issues_tracker.new_issue_path
else else
project.issues_tracker.new_issue_url project.issues_tracker.new_issue_url
end 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?
url =
if options[:only_path] if options[:only_path]
project.issues_tracker.issue_path(issue_iid) project.issues_tracker.issue_path(issue_iid)
else else
project.issues_tracker.issue_url(issue_iid) project.issues_tracker.issue_url(issue_iid)
end 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?
......
...@@ -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
bamboo_service.active = false
context "when a password was previously set" do expect(bamboo_service).not_to validate_presence_of(:bamboo_url)
before do
@bamboo_service = BambooService.create(
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 describe '#build_key' do
@bamboo_service.username = "some_name" it 'does not validate the presence of build_key if service is not active' do
@bamboo_service.save bamboo_service = service
expect(@bamboo_service.password).to eq("password") bamboo_service.active = false
expect(bamboo_service).not_to validate_presence_of(:build_key)
end
it 'validates the presence of build_key if service is active' do
bamboo_service = service
bamboo_service.active = true
expect(bamboo_service).to validate_presence_of(:build_key)
end
end
describe '#username' do
it 'does not validate the presence of username if service is not active' do
bamboo_service = service
bamboo_service.active = false
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
describe '#password' do
it 'does not validate the presence of password if service is not active' do
bamboo_service = service
bamboo_service.active = false
expect(bamboo_service).not_to validate_presence_of(:password)
end
it 'does not validate the presence of password if username is nil' do
bamboo_service = service
bamboo_service.active = true
bamboo_service.username = nil
expect(bamboo_service).not_to validate_presence_of(:password)
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 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 reset password if new url is set together with password, even if it's the same password" do
@bamboo_service.bamboo_url = 'http://gitlab_edited.com' bamboo_service = service
@bamboo_service.password = 'password'
@bamboo_service.save bamboo_service.bamboo_url = 'http://gitlab_edited.com'
expect(@bamboo_service.password).to eq("password") bamboo_service.password = 'password'
expect(@bamboo_service.bamboo_url).to eq("http://gitlab_edited.com") bamboo_service.save
expect(bamboo_service.password).to eq('password')
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 it 'saves password if new url is set together with password when no password was previously set' do
@bamboo_service.bamboo_url = 'http://gitlab1.com' bamboo_service = service
@bamboo_service.bamboo_url = 'http://gitlab1.com' bamboo_service.password = nil
@bamboo_service.save
expect(@bamboo_service.password).to be_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
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 end
context "when no password was previously set" do it 'sets commit status to "pending" when response has no results' do
before do stub_request(body: %Q({"results":{"results":{"size":"0"}}}))
@bamboo_service = BambooService.create(
project: create(:project), 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: { properties: {
bamboo_url: 'http://gitlab.com', bamboo_url: bamboo_url,
username: 'mic' username: 'mic',
password: 'password',
build_key: 'foo'
} }
) )
end end
it "saves password if new url is set together with password" do def stub_request(status: 200, body: nil, build_state: 'success')
@bamboo_service.bamboo_url = 'http://gitlab_edited.com' bamboo_full_url = 'http://mic:password@gitlab.com/rest/api/latest/result?label=123&os_authType=basic'
@bamboo_service.password = 'password' body ||= %Q({"results":{"results":{"result":{"buildState":"#{build_state}"}}}})
@bamboo_service.save
expect(@bamboo_service.password).to eq("password")
expect(@bamboo_service.bamboo_url).to eq("http://gitlab_edited.com")
end
end WebMock.stub_request(:get, bamboo_full_url).to_return(
status: status,
headers: { 'Content-Type' => 'application/json' },
body: body
)
end 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
it { is_expected.to validate_presence_of(:external_wiki_url) }
it_behaves_like 'issue tracker service URL attribute', :external_wiki_url
end end
it { should validate_presence_of :external_wiki_url } context 'when service is inactive' do
before { subject.active = false }
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
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(:priority) }
end end
it { is_expected.to validate_presence_of :api_key } context 'when service is inactive' do
it { is_expected.to validate_presence_of :user_key } before { subject.active = false }
it { is_expected.to validate_presence_of :priority }
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
it { is_expected.to validate_presence_of(:webhook) }
it_behaves_like 'issue tracker service URL attribute', :webhook
end end
it { is_expected.to validate_presence_of :webhook } 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
teamcity_service.active = false
context "when a password was previously set" do expect(teamcity_service).not_to validate_presence_of(:teamcity_url)
before do end
@teamcity_service = TeamcityService.create(
project: create(:project), it 'validates the presence of teamcity_url if service is active' do
properties: { teamcity_service = service
teamcity_url: 'http://gitlab.com', teamcity_service.active = true
username: 'mic',
password: "password" 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 end
it "reset password if url changed" do describe '#username' do
@teamcity_service.teamcity_url = 'http://gitlab1.com' it 'does not validate the presence of username if service is not active' do
@teamcity_service.save teamcity_service = service
expect(@teamcity_service.password).to be_nil teamcity_service.active = false
expect(teamcity_service).not_to validate_presence_of(:username)
end end
it "does not reset password if username changed" do it 'does not validate the presence of username if username is nil' do
@teamcity_service.username = "some_name" teamcity_service = service
@teamcity_service.save teamcity_service.active = true
expect(@teamcity_service.password).to eq("password") teamcity_service.password = nil
expect(teamcity_service).not_to validate_presence_of(:username)
end
it 'validates the presence of username if service is active and username is present' do
teamcity_service = service
teamcity_service.active = true
teamcity_service.password = 'secret'
expect(teamcity_service).to validate_presence_of(:username)
end
end
describe '#password' do
it 'does not validate the presence of password if service is not active' do
teamcity_service = service
teamcity_service.active = false
expect(teamcity_service).not_to validate_presence_of(:password)
end
it 'does not validate the presence of password if username is nil' do
teamcity_service = service
teamcity_service.active = true
teamcity_service.username = nil
expect(teamcity_service).not_to validate_presence_of(:password)
end
it 'validates the presence of password if service is active and username is present' do
teamcity_service = service
teamcity_service.active = true
teamcity_service.username = 'john'
expect(teamcity_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
teamcity_service = service
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 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 reset password if new url is set together with password, even if it's the same password" do
@teamcity_service.teamcity_url = 'http://gitlab_edited.com' teamcity_service = service
@teamcity_service.password = 'password'
@teamcity_service.save teamcity_service.teamcity_url = 'http://gitlab_edited.com'
expect(@teamcity_service.password).to eq("password") teamcity_service.password = 'password'
expect(@teamcity_service.teamcity_url).to eq("http://gitlab_edited.com") teamcity_service.save
expect(teamcity_service.password).to eq('password')
expect(teamcity_service.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_service = service
teamcity_service.password = nil
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
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 end
it "should reset password if url changed, even if setter called multiple times" do it 'sets commit status to "failed" when build status contains FAILURE' do
@teamcity_service.teamcity_url = 'http://gitlab1.com' stub_request(build_status: 'NO FAILURE!')
@teamcity_service.teamcity_url = 'http://gitlab1.com'
@teamcity_service.save expect(service.commit_status('123', 'unused')).to eq('failed')
expect(@teamcity_service.password).to be_nil
end 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 end
context "when no password was previously set" do it 'sets commit status to :error when build status is unknown' do
before do stub_request(build_status: 'FOO BAR!')
@teamcity_service = TeamcityService.create(
project: create(:project), 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: { properties: {
teamcity_url: 'http://gitlab.com', teamcity_url: teamcity_url,
username: 'mic' username: 'mic',
password: 'password',
build_type: 'foo'
} }
) )
end end
it "saves password if new url is set together with password" do def stub_request(status: 200, body: nil, build_status: 'success')
@teamcity_service.teamcity_url = 'http://gitlab_edited.com' teamcity_full_url = 'http://mic:password@gitlab.com/httpAuth/app/rest/builds/branch:unspecified:any,number:123'
@teamcity_service.password = 'password' body ||= %Q({"build":{"status":"#{build_status}","id":"666"}})
@teamcity_service.save
expect(@teamcity_service.password).to eq("password") WebMock.stub_request(:get, teamcity_full_url).to_return(
expect(@teamcity_service.teamcity_url).to eq("http://gitlab_edited.com") status: status,
end headers: { 'Content-Type' => 'application/json' },
end body: body
)
end 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