Defect #15123

"Add watcher" leaks all active users

Added by Felix Schäfer almost 4 years ago. Updated almost 4 years ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:-% Done:

0%

Category:Security
Target version:-
Resolution:Duplicate Affected version:

Description

When adding watchers, all active users of the current installation are visible (on new issues from the get-go, on existing issues you might have to type a few characters to trigger the autocomplete).

All other places in Redmine exposing users go to great lengths to only show users that are "visible" to the current user. Attached is a patch that limits the proposed users in the watcher autocomplete to users that are members of projects visible to the current user.

(This patch was written on behalf of and contributed by Planio)

watchers_autocomplete_visible_only.patch Magnifier (2.13 KB) Felix Schäfer, 2013-10-14 10:07


Related issues

Related to Redmine - Defect #9500: Watchers list before and after creation issue New 2011-10-31
Related to Redmine - Feature #5159: Ability to add Non-Member watchers to the watch list Closed 2010-03-23
Duplicates Redmine - Feature #11724: Prevent users from seeing other users based on their proj... Closed
Duplicated by Redmine - Defect #15613: 'Add watchers' within the new issue reveals all the accounts Closed

History

#1 Updated by Toshi MARUYAMA almost 4 years ago

  • Related to Defect #9500: Watchers list before and after creation issue added

#2 Updated by Toshi MARUYAMA almost 4 years ago

  • Related to Feature #5159: Ability to add Non-Member watchers to the watch list added

#3 Updated by Toshi MARUYAMA almost 4 years ago

I think this is intended behavior of #5159.

#4 Updated by Mischa The Evil almost 4 years ago

This actually is a duplicate of #11724, where #9500 isn't actually tightly related. I'd suggest to keep this issue open because it contains a patch with tests.
Thanks for sharing this!

#5 Updated by Mischa The Evil almost 4 years ago

  • Related to Feature #11724: Prevent users from seeing other users based on their project membership added

#6 Updated by Felix Schäfer almost 4 years ago

I had thought about that, and also about private issues (I'm still not 100% sure how those work, but IIRC watchers can see private issues too?), but I still think the current solution can be improved. The user pages for example still go to great lengths to make sure you can only see the user pages of users that have some activity in a project you can see source:/branches/2.3-stable/app/controllers/users_controller.rb#L68, probably to not disclose too many users.

I'm not really in favor of adding even more permissions, but what about a second permission for adding watchers: Rename the current permission to "Add any user as watcher" and "Add users you can see as watcher" or something similar?

#7 Updated by Felix Schäfer almost 4 years ago

Mischa The Evil wrote:

I'd suggest to keep this issue open because it contains a patch

Yes.

with tests.

No.

This currently should rather be considered a working and solid proof of concept, I especially wanted some discussion as to wether this behavior is intended, if it could or should be improved upon or if the Redmine core is happy with the current state and doesn't want to change it.

#8 Updated by Mischa The Evil almost 4 years ago

Felix Schäfer wrote:

[...] and also about private issues (I'm still not 100% sure how those work, but IIRC watchers can see private issues too?) [...]

The watcher mechanism is not (and should not) being used for access control. It is used for notification purposes only. See #8488.
(please post to the forum with questions about the current implementation of private issues, I'd be happy to catch you up on the subject ;)

but I still think the current solution can be improved.

I totally agree.

The user pages for example still go to great lengths to make sure you can only see the user pages of users that have some activity in a project you can see source:/branches/2.3-stable/app/controllers/users_controller.rb#L68, probably to not disclose too many users.

Yes, indeed. And I think this good. See r2986 which introduced these checks for #3720 and #4129.

I'm not really in favor of adding even more permissions, but what about a second permission for adding watchers: Rename the current permission to "Add any user as watcher" and "Add users you can see as watcher" or something similar?

That would solve the issue as far as I can see. Considering the nature of the issue I tend to think that it could justify adding such permission.

[...] I especially wanted some discussion as to wether this behavior is intended, if it could or should be improved upon or if the Redmine core is happy with the current state and doesn't want to change it.

As Toshi stated in note-3 it indeed seems the intended behavior as per #5159.
I definitely think it would be good if this is going to be improved. #11724 was filed initially as a defect, which I think this behavior is in the light of r2986.

Mischa The Evil wrote:

with tests.

No.

Hmm, I think I was a bit distracted and made a Freudian typo... ;)

#9 Updated by Jean-Philippe Lang almost 4 years ago

  • Status changed from New to Closed
  • Resolution set to Duplicate

I'm closing it in favour of #11724. Please have a look at my note #11724-8.

#10 Updated by Toshi MARUYAMA almost 4 years ago

  • Related to deleted (Feature #11724: Prevent users from seeing other users based on their project membership)

#11 Updated by Toshi MARUYAMA almost 4 years ago

  • Duplicates Feature #11724: Prevent users from seeing other users based on their project membership added

#12 Updated by Toshi MARUYAMA almost 4 years ago

  • Duplicated by Defect #15613: 'Add watchers' within the new issue reveals all the accounts added

Also available in: Atom PDF