Merge branch 'security-10-4-todo-api-reveals-sensitive-information' into 'security-10-4'

Restrict Todo API mark_as_done endpoint to the user's todos only

5 files changed, 19 insertions, 2 deletions

diff --git a/changelogs/unreleased/security-10-4-todo-api-reveals-sensitive-information.yml b/changelogs/unreleased/security-10-4-todo-api-reveals-sensitive-information.yml
new file mode 100644
index 00000000000..329825d1e73
--- /dev/null
+++ b/changelogs/unreleased/security-10-4-todo-api-reveals-sensitive-information.yml
@@ -0,0 +1,5 @@
+---
+title: Restrict Todo API mark_as_done endpoint to the user's todos only
+merge_request:
+author:
+type: security
diff --git a/lib/api/todos.rb b/lib/api/todos.rb
index ffccfebe752..c6dbcf84e3a 100644
--- a/lib/api/todos.rb
+++ b/lib/api/todos.rb
@@ -60,7 +60,7 @@ module API
       end
       post ':id/mark_as_done' do
         TodoService.new.mark_todos_as_done_by_ids(params[:id], current_user)
-        todo = Todo.find(params[:id])
+        todo = current_user.todos.find(params[:id])
 
         present todo, with: Entities::Todo, current_user: current_user
       end
diff --git a/lib/api/v3/todos.rb b/lib/api/v3/todos.rb
index 2f2cf259987..3e2c61f6dbd 100644
--- a/lib/api/v3/todos.rb
+++ b/lib/api/v3/todos.rb
@@ -12,7 +12,7 @@ module API
         end
         delete ':id' do
           TodoService.new.mark_todos_as_done_by_ids(params[:id], current_user)
-          todo = Todo.find(params[:id])
+          todo = current_user.todos.find(params[:id])
 
           present todo, with: ::API::Entities::Todo, current_user: current_user
         end
diff --git a/spec/requests/api/todos_spec.rb b/spec/requests/api/todos_spec.rb
index fb3a33cadff..2ee8d150dc8 100644
--- a/spec/requests/api/todos_spec.rb
+++ b/spec/requests/api/todos_spec.rb
@@ -129,6 +129,12 @@ describe API::Todos do
 
         post api("/todos/#{pending_1.id}/mark_as_done", john_doe)
       end
+
+      it 'returns 404 if the todo does not belong to the current user' do
+        post api("/todos/#{pending_1.id}/mark_as_done", author_1)
+
+        expect(response.status).to eq(404)
+      end
     end
   end
 
diff --git a/spec/requests/api/v3/todos_spec.rb b/spec/requests/api/v3/todos_spec.rb
index 53fd962272a..ea648e3917f 100644
--- a/spec/requests/api/v3/todos_spec.rb
+++ b/spec/requests/api/v3/todos_spec.rb
@@ -38,6 +38,12 @@ describe API::V3::Todos do
 
         delete v3_api("/todos/#{pending_1.id}", john_doe)
       end
+
+      it 'returns 404 if the todo does not belong to the current user' do
+        delete v3_api("/todos/#{pending_1.id}", author_1)
+
+        expect(response.status).to eq(404)
+      end
     end
   end