Merge branch 'kc-escape-dashboard-path-annotations' into 'master'

Decode dashboard_path when creating annotations

See merge request gitlab-org/gitlab!31665

changelogs/unreleased/kc-escape-dashboard-path-annotations.yml

+---
+title: Decode dashboard_path when creating annotations
+merge_request: 31665
+author:
+type: fixed

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])
 

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&copy.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