Project

General

Profile

Feature #44261

Updated by Holger Just about 3 hours ago

Currently, the mention feature ( introduced in #13919 ) allows mentioning users 
 who are not members of the project: 

 # 1. In the auto-completion (@WatchersController#users_for_mention@), (WatchersController#users_for_mention), when the 
    user types @@@ "@" followed by some characters, the candidate list falls back 
    to @Principal.assignable_watchers.limit(10)@, `Principal.assignable_watchers.limit(10)`, which searches across ALL 
    active users regardless of project membership. Only the initial @@@ "@" 
    (empty query) case is scoped to @@project.principals@. 
 # `@project.principals`. 

 2. More importantly, the mention parser itself (@Redmine::Acts::Mentionable#get_mentioned_users@) 
    (Redmine::Acts::Mentionable#get_mentioned_users) resolves @@login@ "@login" to @User.visible.active@ 
    `User.visible.active` without any project membership check. So if a user 
    types a login manually (without using auto-completion), a user who is not 
    a member of the project can be mentioned and will receive a notification 
    email containing the issue/journal/wiki content, as long as the object is 
    visible to them (e.g. public projects, or roles with @users_visibility users_visibility = "all"@). 
    "all"). 

 This is an information-leak risk: a typo or a wrong pick from the 
 auto-completion list can send issue contents to someone outside the project 
 team. Restricting the users_visibility setting on roles (as often suggested 
 as a workaround) only narrows the candidate list for non-admin users; it 
 does not prevent case 2 above. 

 The attached patch changes both places so that only members of the object's 
 project can be mentioned: 

 * @WatchersController#users_for_mention@: WatchersController#users_for_mention: always scope the candidates to @@project.principals@ 
   @project.principals when a project is present, with or without a query 
   string. 
 * @Redmine::Acts::Mentionable#get_mentioned_users@: Redmine::Acts::Mentionable#get_mentioned_users: when the mentionable 
   object has a project, filter the mentioned users to the project's 
   principals. Administrators are exempt from this check, consistent with the 
   existing behavior/tests (admins can view everything anyway, so there is no 
   leak concern, and an existing test expects an admin who is not a member to 
   be notified for private notes). 

 The existing visibility check in notified_mentions (mentioned user must be 
 able to view the object) is kept as-is; the membership check is applied in 
 addition to it. 

 The patch includes regression tests for both changes: 

 
 * @Redmine::Acts::MentionableTest@: Redmine::Acts::MentionableTest: a user who does not belong to any project 
   is not included in mentioned_users. 
 * @WatchersControllerTest@: @autocomplete_for_mention@ WatchersControllerTest: autocomplete_for_mention with a query does not 
   return users outside the project. 

 All related tests pass (mentionable_test, watchers_controller_test, 
 issue_test, journal_test, wiki_content_test, issues_controller_test: 
 883 runs, 0 failures) and RuboCop reports no offenses on the changed files. 

 The patch is against current trunk (c8c8b82e).

Back