From b22498fc1aeafda1b9dfd85c99ecd91a96a2cb1b Mon Sep 17 00:00:00 2001 From: Jens Kraemer Date: Sat, 11 Jul 2026 18:46:38 +0800 Subject: [PATCH 2/2] Add context menu tests guarding against data leaks to unauthorized users The projects and users context menus require admin, and the time entries context menu requires the entries to be visible, but nothing tested what a non-admin / non-member actually receives. Add assertions that the forbidden / not-found response leaks neither the record's existence nor privileged state: * projects: private project identifier, archived state, and no existence oracle between an existing private id and a missing id * users: locked / registered account state, admin edit and delete links, and no existence oracle * time entries: activity names from an invisible project --- .../context_menus/projects_controller_test.rb | 40 ++++++++++++++++ .../time_entries_controller_test.rb | 18 ++++++++ .../context_menus/users_controller_test.rb | 46 +++++++++++++++++++ 3 files changed, 104 insertions(+) diff --git a/test/functional/context_menus/projects_controller_test.rb b/test/functional/context_menus/projects_controller_test.rb index 38d51a93e9..a66a6ebb7a 100644 --- a/test/functional/context_menus/projects_controller_test.rb +++ b/test/functional/context_menus/projects_controller_test.rb @@ -46,5 +46,45 @@ module ContextMenus assert_response :forbidden end + + def test_index_should_not_leak_identifier_to_non_admin + # Project 5 is private; user 4 is not a member, so Project.visible would + # exclude it. Without authorization the action would render and the + # project's identifier ("private-child") would surface in link hrefs. + private_project = Project.find(5) + assert_equal false, private_project.is_public + assert_not User.find(4).member_of?(private_project) + + @request.session[:user_id] = 4 + get(:index, :params => {:ids => [5]}) + + assert_response :forbidden + assert_not_includes @response.body, private_project.identifier + end + + def test_index_should_not_expose_existence_oracle + # Existing private id and missing id must both return 403 — same + # status, no enumeration oracle. + @request.session[:user_id] = 4 + + get(:index, :params => {:ids => [5]}) + assert_response :forbidden + + get(:index, :params => {:ids => [999_999]}) + assert_response :forbidden + end + + def test_index_should_not_leak_archived_state + # The response must not distinguish active ("Archive" button) from + # archived ("Unarchive" button) projects the user cannot see. + Project.find(5).archive + + @request.session[:user_id] = 4 + get(:index, :params => {:ids => [5]}) + + assert_response :forbidden + assert_select 'a.icon-unlock', :text => /Unarchive/i, :count => 0 + assert_select 'a.icon-lock', :text => /Archive/i, :count => 0 + end end end diff --git a/test/functional/context_menus/time_entries_controller_test.rb b/test/functional/context_menus/time_entries_controller_test.rb index 9df0e34d5f..cb70126874 100644 --- a/test/functional/context_menus/time_entries_controller_test.rb +++ b/test/functional/context_menus/time_entries_controller_test.rb @@ -116,5 +116,23 @@ module ContextMenus assert_select 'a.disabled', :text => 'Bulk edit' end + + def test_index_should_not_leak_activity_names_from_invisible_project + # time_entry 5 belongs to project 5 (private). User 4 is not a member; + # the menu must not render and no activity names from the private + # project may leak via the response body. + entry = TimeEntry.find(5) + assert_equal 5, entry.project_id + assert_not entry.visible?(User.find(4)) + + @request.session[:user_id] = 4 + get(:index, :params => {:ids => [5]}) + + assert_response :not_found + + Project.find(5).activities.each do |activity| + assert_not_includes @response.body, activity.name + end + end end end diff --git a/test/functional/context_menus/users_controller_test.rb b/test/functional/context_menus/users_controller_test.rb index 210b9a0cf4..7ec0e24e65 100644 --- a/test/functional/context_menus/users_controller_test.rb +++ b/test/functional/context_menus/users_controller_test.rb @@ -86,5 +86,51 @@ module ContextMenus get :index, :params => {:ids => [8]} assert_response :forbidden end + + def test_index_should_not_leak_lock_state_to_non_admin + # User 5 (Dave Lopper2) is locked in fixtures. The rendered menu must + # not show an "Unlock" link to non-admins, revealing lock status. + assert User.find(5).locked? + + @request.session[:user_id] = 4 + get :index, :params => {:ids => [5]} + + assert_response :forbidden + assert_select 'a.icon-unlock', :text => /Unlock/i, :count => 0 + assert_select 'a', :text => /Activate/i, :count => 0 + end + + def test_index_should_not_leak_registered_state + # Registered (not-yet-activated) users must not surface an "Activate" + # link, revealing their pending-activation state. + User.find(5).update_columns(:status => User::STATUS_REGISTERED) + + @request.session[:user_id] = 4 + get :index, :params => {:ids => [5]} + + assert_response :forbidden + assert_select 'a', :text => /Activate/i, :count => 0 + end + + def test_index_should_not_expose_existence_oracle + @request.session[:user_id] = 4 + + get :index, :params => {:ids => [8]} + assert_response :forbidden + + get :index, :params => {:ids => [999_999]} + assert_response :forbidden + end + + def test_index_should_not_render_admin_links_for_non_admin + # The menu must not expose /users/:id/edit and DELETE /users/:id for + # any caller, confirming the target id exists. + @request.session[:user_id] = 4 + get :index, :params => {:ids => [8]} + + assert_response :forbidden + assert_select 'a.icon-edit', :count => 0 + assert_select 'a.icon-del', :count => 0 + end end end -- 2.55.0