Skip to content

Commit

Permalink
Do not validate params in TriggerController
Browse files Browse the repository at this point in the history
This was removed in the PR #12353 and it was wrong. This controller
receives webhooks from SCMs, so skipping this validation is needed.

Specs are also covering what wasn't covered in #12353.
  • Loading branch information
Dany Marcoux committed Apr 4, 2022
1 parent 6e8d508 commit 5506587
Show file tree
Hide file tree
Showing 5 changed files with 273 additions and 3 deletions.
3 changes: 3 additions & 0 deletions src/api/app/controllers/trigger_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ class TriggerController < ApplicationController
skip_before_action :extract_user
# Authentication happens with tokens, so no login is required
skip_before_action :require_login
# SCMs like GitLab/GitHub send data as parameters which are not strings (e.g.: GitHub - PR number is a integer, GitLab - project is a hash)
# Other SCMs might also do this, so we're not validating parameters.
skip_before_action :validate_params
after_action :verify_authorized

before_action :validate_gitlab_event, if: :gitlab_webhook?
Expand Down
3 changes: 0 additions & 3 deletions src/api/app/controllers/trigger_workflow_controller.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
class TriggerWorkflowController < TriggerController
# We don't need to validate that the body of the request is XML. We receive JSON
skip_before_action :validate_xml_request, :set_project_name, :set_package_name, :set_project, :set_package, :set_object_to_authorize, :set_multibuild_flavor
# GitLab/Github send data as parameters which are not strings
# e.g. integer PR number (GitHub) and project hash (GitLab)
skip_before_action :validate_params

before_action :set_scm_event
before_action :abort_trigger_if_ignored_pull_request_action
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 13 additions & 0 deletions src/api/spec/controllers/trigger_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -184,5 +184,18 @@

it_behaves_like 'it verifies the signature'
end

context 'when some parameters are not strings' do
before do
request.headers['ACCEPT'] = '*/*'
request.headers['CONTENT_TYPE'] = 'application/json'
request.headers['HTTP_X_OBS_SIGNATURE'] = signature
post :create, body: { a_hash: { integer1: 123 }, integer2: 456 }.to_json
end

it 'still processes the request without validating parameters' do
expect(response).to have_http_status(:success)
end
end
end
end
28 changes: 28 additions & 0 deletions src/api/spec/controllers/trigger_workflow_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -260,5 +260,33 @@
it { expect(WorkflowRun.last.status).to eq('success') }
it { expect(response.body).to include('Ok') }
end

context 'the SCM is unsupported' do
let(:token_extractor_instance) { instance_double(::TriggerControllerService::TokenExtractor) }
let(:token) { build_stubbed(:workflow_token, user: build_stubbed(:confirmed_user, :in_beta)) }
let(:scm_payload) do
{ super: 'duper' }
end

before do
allow(token).to receive(:call).and_return([])

allow(::TriggerControllerService::TokenExtractor).to receive(:new).and_return(token_extractor_instance)
allow(token_extractor_instance).to receive(:call).and_return(token)

request.headers['ACCEPT'] = '*/*'
request.headers['CONTENT_TYPE'] = 'application/json'

post :create, body: scm_payload.to_json
end

it { expect(response).to have_http_status(:bad_request) }
it { expect(response.content_type).to eq('application/xml; charset=utf-8') }

it 'returns an error message in the response body' do
expect(response.body).to include('Only GitHub and GitLab are supported. ' \
'Could not find the required HTTP request headers X-GitHub-Event or X-Gitlab-Event')
end
end
end
end

0 comments on commit 5506587

Please sign in to comment.