diff --git a/changelogs/unreleased/kc-escape-dashboard-path-annotations.yml b/changelogs/unreleased/kc-escape-dashboard-path-annotations.yml new file mode 100644 index 0000000000000000000000000000000000000000..9b21cb7de03f49a39bc0aea5e21d5c1d110f1804 --- /dev/null +++ b/changelogs/unreleased/kc-escape-dashboard-path-annotations.yml @@ -0,0 +1,5 @@ +--- +title: Decode dashboard_path when creating annotations +merge_request: 31665 +author: +type: fixed diff --git a/lib/api/metrics/dashboard/annotations.rb b/lib/api/metrics/dashboard/annotations.rb index 81fee166175b3d88da21fe93d451e8905b2778da..c8ec4d296570ba17db89417833b86ac30ee502ec 100644 --- a/lib/api/metrics/dashboard/annotations.rb +++ b/lib/api/metrics/dashboard/annotations.rb @@ -8,16 +8,6 @@ class Annotations < Grape::API success Entities::Metrics::Dashboard::Annotation end - params do - requires :starting_at, type: DateTime, - desc: 'Date time indicating starting moment to which the annotation relates.' - optional :ending_at, type: DateTime, - desc: 'Date time indicating ending moment to which the annotation relates.' - requires :dashboard_path, type: String, - desc: 'The path to a file defining the dashboard on which the annotation should be added' - requires :description, type: String, desc: 'The description of the annotation' - end - ANNOTATIONS_SOURCES = [ { class: ::Environment, resource: :environments, create_service_param_key: :environment }, { class: Clusters::Cluster, resource: :clusters, create_service_param_key: :cluster } @@ -25,6 +15,16 @@ class Annotations < Grape::API ANNOTATIONS_SOURCES.each do |annotations_source| resource annotations_source[:resource] do + params do + requires :starting_at, type: DateTime, + desc: 'Date time indicating starting moment to which the annotation relates.' + optional :ending_at, type: DateTime, + desc: 'Date time indicating ending moment to which the annotation relates.' + requires :dashboard_path, type: String, coerce_with: -> (val) { CGI.unescape(val) }, + desc: 'The path to a file defining the dashboard on which the annotation should be added' + requires :description, type: String, desc: 'The description of the annotation' + end + post ':id/metrics_dashboard/annotations' do annotations_source_object = annotations_source[:class].find(params[:id]) diff --git a/spec/requests/api/metrics/dashboard/annotations_spec.rb b/spec/requests/api/metrics/dashboard/annotations_spec.rb index ec88b4db25656baa5472b005f6ec4e4e9c91e7f6..6377ef2435a0403a7e4883727c866fcc7beab827 100644 --- a/spec/requests/api/metrics/dashboard/annotations_spec.rb +++ b/spec/requests/api/metrics/dashboard/annotations_spec.rb @@ -35,7 +35,7 @@ context 'with invalid parameters' do it 'returns error messsage' do - post api(url, user), params: { dashboard_path: nil, starting_at: nil, description: nil } + post api(url, user), params: { dashboard_path: '', starting_at: nil, description: nil } expect(response).to have_gitlab_http_status(:bad_request) expect(json_response['message']).to include({ "starting_at" => ["can't be blank"], "description" => ["can't be blank"], "dashboard_path" => ["can't be blank"] }) @@ -53,6 +53,41 @@ post api(url, user), params: params end end + + context 'with special characers in dashboard_path in request body' do + let(:dashboard_escaped) { 'config/prometheus/common_metrics%26copy.yml' } + let(:dashboard_unescaped) { 'config/prometheus/common_metrics©.yml' } + + shared_examples 'special characters unescaped' do + let(:expected_params) do + { + 'starting_at' => starting_at.to_time, + 'ending_at' => ending_at.to_time, + "#{source_type}" => source, + 'dashboard_path' => dashboard_unescaped, + 'description' => params[:description] + } + end + + it 'unescapes the dashboard_path', :aggregate_failures do + expect(::Metrics::Dashboard::Annotations::CreateService).to receive(:new).with(user, expected_params) + + post api(url, user), params: params + end + end + + context 'with escaped characters' do + it_behaves_like 'special characters unescaped' do + let(:dashboard) { dashboard_escaped } + end + end + + context 'with unescaped characers' do + it_behaves_like 'special characters unescaped' do + let(:dashboard) { dashboard_unescaped } + end + end + end end context 'without correct permissions' do