From c3e35917fbfbe64a068e387207aa510d76852e61 Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Fri, 17 Jul 2015 15:26:09 +0200 Subject: Validate format of project_url and token for GitLab CI service. --- app/models/project_services/gitlab_ci_service.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/app/models/project_services/gitlab_ci_service.rb b/app/models/project_services/gitlab_ci_service.rb index c284e19fe50..5aaa4e85cbc 100644 --- a/app/models/project_services/gitlab_ci_service.rb +++ b/app/models/project_services/gitlab_ci_service.rb @@ -22,8 +22,12 @@ class GitlabCiService < CiService API_PREFIX = "api/v1" prop_accessor :project_url, :token - validates :project_url, presence: true, if: :activated? - validates :token, presence: true, if: :activated? + validates :project_url, + presence: true, + format: { with: /\A#{URI.regexp(%w(http https))}\z/, message: "should be a valid url" }, if: :activated? + validates :token, + presence: true, + format: { with: /\A([A-Za-z0-9]+)\z/ }, if: :activated? after_save :compose_service_hook, if: :activated? -- cgit v1.2.1 From 302c52551a0155f3955144690f1b67ac4ef98282 Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Fri, 17 Jul 2015 15:58:26 +0200 Subject: Fix failing specs after adding format verification for token and project url in gitlab ci service. --- spec/requests/api/projects_spec.rb | 2 +- spec/requests/api/services_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 4178bb2e836..5bd8206b890 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -89,7 +89,7 @@ describe API::API, api: true do it 'returns projects in the correct order when ci_enabled_first parameter is passed' do [project, project2, project3].each{ |project| project.build_missing_services } - project2.gitlab_ci_service.update(active: true, token: "token", project_url: "url") + project2.gitlab_ci_service.update(active: true, token: "token", project_url: "http://ci.example.com/projects/1") get api('/projects', user), { ci_enabled_first: 'true' } expect(response.status).to eq(200) expect(json_response).to be_an Array diff --git a/spec/requests/api/services_spec.rb b/spec/requests/api/services_spec.rb index 51c543578df..cedb5f15ead 100644 --- a/spec/requests/api/services_spec.rb +++ b/spec/requests/api/services_spec.rb @@ -7,7 +7,7 @@ describe API::API, api: true do describe "POST /projects/:id/services/gitlab-ci" do it "should update gitlab-ci settings" do - put api("/projects/#{project.id}/services/gitlab-ci", user), token: 'secret-token', project_url: "http://ci.example.com/projects/1" + put api("/projects/#{project.id}/services/gitlab-ci", user), token: 'secrettoken', project_url: "http://ci.example.com/projects/1" expect(response.status).to eq(200) end -- cgit v1.2.1 From c7daa5f17fdb45cb7acc058a3a418be19c42712a Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Fri, 17 Jul 2015 17:03:15 +0200 Subject: Add specs for gitlab ci service validity. --- .../project_services/gitlab_ci_service_spec.rb | 27 ++++++++++++++++++++++ spec/requests/api/services_spec.rb | 12 ++++++++++ 2 files changed, 39 insertions(+) diff --git a/spec/models/project_services/gitlab_ci_service_spec.rb b/spec/models/project_services/gitlab_ci_service_spec.rb index fedc37c9b94..4f8a1986995 100644 --- a/spec/models/project_services/gitlab_ci_service_spec.rb +++ b/spec/models/project_services/gitlab_ci_service_spec.rb @@ -26,6 +26,33 @@ describe GitlabCiService do it { is_expected.to have_one(:service_hook) } end + describe 'validations' do + context 'active' do + before { allow(subject).to receive(:activated?).and_return(true) } + + it { is_expected.to validate_presence_of(:token) } + it { is_expected.to validate_presence_of(:project_url) } + it { is_expected.to allow_value('ewf9843kdnfdfs89234n').for(:token) } + it { is_expected.to allow_value('http://ci.example.com/project/1').for(:project_url) } + it { is_expected.not_to allow_value('token with spaces').for(:token) } + it { is_expected.not_to allow_value('token/with%spaces').for(:token) } + it { is_expected.not_to allow_value('this is not url').for(:project_url) } + it { is_expected.not_to allow_value('http//noturl').for(:project_url) } + it { is_expected.not_to allow_value('ftp://ci.example.com/projects/3').for(:project_url) } + end + + context 'inactive' do + before { allow(subject).to receive(:activated?).and_return(false) } + + it { is_expected.not_to validate_presence_of(:token) } + it { is_expected.not_to validate_presence_of(:project_url) } + it { is_expected.to allow_value('ewf9843kdnfdfs89234n').for(:token) } + it { is_expected.to allow_value('http://ci.example.com/project/1').for(:project_url) } + it { is_expected.to allow_value('token with spaces').for(:token) } + it { is_expected.to allow_value('ftp://ci.example.com/projects/3').for(:project_url) } + end + end + describe 'commits methods' do before do @service = GitlabCiService.new diff --git a/spec/requests/api/services_spec.rb b/spec/requests/api/services_spec.rb index cedb5f15ead..6d29a28580a 100644 --- a/spec/requests/api/services_spec.rb +++ b/spec/requests/api/services_spec.rb @@ -17,6 +17,18 @@ describe API::API, api: true do expect(response.status).to eq(400) end + + it "should return if the format of token is invalid" do + put api("/projects/#{project.id}/services/gitlab-ci", user), token: 'token-with dashes and spaces%', project_url: "http://ci.example.com/projects/1", active: true + + expect(response.status).to eq(404) + end + + it "should return if the format of token is invalid" do + put api("/projects/#{project.id}/services/gitlab-ci", user), token: 'token-with dashes and spaces%', project_url: "ftp://ci.example/projects/1", active: true + + expect(response.status).to eq(404) + end end describe "DELETE /projects/:id/services/gitlab-ci" do -- cgit v1.2.1 From 77f325a49fd955f73197a6270c82d28053e2c19e Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Fri, 17 Jul 2015 17:17:33 +0200 Subject: Do not disappoint rubocop. --- spec/models/project_services/gitlab_ci_service_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/project_services/gitlab_ci_service_spec.rb b/spec/models/project_services/gitlab_ci_service_spec.rb index 4f8a1986995..a14384c87b4 100644 --- a/spec/models/project_services/gitlab_ci_service_spec.rb +++ b/spec/models/project_services/gitlab_ci_service_spec.rb @@ -26,7 +26,7 @@ describe GitlabCiService do it { is_expected.to have_one(:service_hook) } end - describe 'validations' do + describe 'validations' do context 'active' do before { allow(subject).to receive(:activated?).and_return(true) } -- cgit v1.2.1