Allow only indexed columns in #order_and_sort. Remove present (Because unnecessary) from condition. Added spec just in case.

2 files changed, 11 insertions, 3 deletions

diff --git a/app/finders/pipelines_finder.rb b/app/finders/pipelines_finder.rb
index 5e50eb46c7e..6a92aedc873 100644
--- a/app/finders/pipelines_finder.rb
+++ b/app/finders/pipelines_finder.rb
@@ -108,12 +108,12 @@ class PipelinesFinder
   end
 
   def order_and_sort(items)
-    order_by = if params[:order_by].present? && items.column_names.include?(params[:order_by])
+    order_by = if %w[id status ref user_id].include?(params[:order_by]) # Allow only indexed columns
                  params[:order_by]
                else
                  :id
                end
-    sort = if params[:sort].present? && params[:sort] =~ /\A(ASC|DESC)\z/i
+    sort = if params[:sort] =~ /\A(ASC|DESC)\z/i
              params[:sort]
            else
              :desc
diff --git a/spec/finders/pipelines_finder_spec.rb b/spec/finders/pipelines_finder_spec.rb
index 02422bb6ebc..02c91cbf465 100644
--- a/spec/finders/pipelines_finder_spec.rb
+++ b/spec/finders/pipelines_finder_spec.rb
@@ -208,7 +208,7 @@ describe PipelinesFinder do
       context 'when order_by and sort are valid' do
         let(:params) { { order_by: 'created_at', sort: 'asc' } }
 
-        it 'sorts pipelines' do
+        it 'sorts pipelines by default' do
           expect(subject).to eq(Ci::Pipeline.order(created_at: :asc))
         end
       end
@@ -228,6 +228,14 @@ describe PipelinesFinder do
           expect(subject).to eq(Ci::Pipeline.order(created_at: :desc))
         end
       end
+
+      context 'when both are nil' do
+        let(:params) { { order_by: nil, sort: nil } }
+
+        it 'sorts pipelines by default' do
+          expect(subject).to eq(Ci::Pipeline.order(id: :desc))
+        end
+      end
     end
   end
 end