Closed Bug 309681 Opened 19 years ago Closed 19 years ago

Prevent users from adding another user who shouldn't have access to a bug as assignee or CC member

Categories

(Bugzilla :: Creating/Changing Bugs, enhancement, P2)

2.21
enhancement

Tracking

()

RESOLVED FIXED
Bugzilla 2.22

People

(Reporter: batosti, Assigned: gabriel.sales)

References

Details

(Keywords: selenium)

Attachments

(3 files, 3 obsolete files)

User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.10) Gecko/20050725 Firefox/1.0.6 (Ubuntu package 1.0.6) Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.10) Gecko/20050725 Firefox/1.0.6 (Ubuntu package 1.0.6) an user can add another user can't have access to bug as assignee or in CC list. perhaps this user will be able to see the bug after this. Reproducible: Always
Attached patch batosti_v1 (deleted) — Splinter Review
Attachment #197104 - Flags: review?(bugreport)
I'm marking this as an enhancment since it really adds a level of isolation of users optionally.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P2
Target Milestone: --- → Bugzilla 2.22
Version: unspecified → 2.21
You know, I think it's generally silly to do this -- you can just remove the "CC List" checkbox from the bug, and then only the people who are actually in the group can see the bug. Then, even if people are on the CC List, they won't get any emails, because BugMail will check and see that they don't actually have permissions. I recommend WONTFIX or INVALID or WORKSFORME, depending on how you look at it. :-)
Severity: normal → enhancement
That doesn't do it. For compartmentalized installations, it is important that CClist and assignee still work, but users need to be protected from making really big goofs. Naturally, this type of operation is controlled by a param.
Summary: an user can add another user can't have access to bug as assignee or in CC list → Prevent users from adding another user who shouldn't have access to a bug as assignee or CC member
Comment on attachment 197104 [details] [diff] [review] batosti_v1 >Index: process_bug.cgi >+sub CanAddUserToBug { >+ my ($prod_id, $id, $uid) = (@_); >+ my $user = new Bugzilla::User($uid); >+ unless ($user->can_edit_product_id($prod_id)) { >+ ThrowUserError("invalid_user_group", { 'user' => >+ $cgi->param('assigned_to'), bug_id => $id }); >+ } >+} I don't think this routine should be in process_bug.cgi, but rather in Bug.pm. I think post_bug.cgi would need this routine too?
I agree with Joel. I have an IT company with many customers. They report their issues using Bugzilla and I'm concerned that Acme and Zoom can't send messages to each other through this interface. I'm also concerned that I am not going to accidentally be able to add CC's from Zoom when a bug belongs to Acme. To allow for the best of both situations, when a user's visibility expands beyond the limitations of a particular group, I think it's appropriate to have two CC boxes instead of one. The first box would list associated CC's (meaning those associated with the product), and the second box would list all CC's not associated with the product. This is especially true for installations that have drop-down lists enabled. I also feel the display of the second box should be a param.
From my standpoint, we want to prevent the addition of any user who would violate a CANEDIT restriction on a bug. It is less a matter of keeping the user from seing the bug as it is to keep the others who can see the bug from seeing the user. Imagine if Oracle's interest in Bugzilla were a highly sensitive bit if commercial information and someone accidentally copied an Oracle person on a Bugzilla bug. Suddenly, it would be obvious to a whole bunch of people what Oracle was and was not interested in. In some enviroments (clealy including Kevin's and mine - and I suspec a number of others), that is a very key issue.
Comment on attachment 197104 [details] [diff] [review] batosti_v1 Cleanups suggested regarding usage of CanAddUserToBug() are valid suggestions. Really, the key ussue is ... does this user have any business being on any bug in this product? I think the test is can_edit_product where the product may be the product of a particular bug or of a bug about to be posted. We should also apply the same restriction to requests. As a distinct bug, we should not let a product change carry inappropriate users into a new product.
Attachment #197104 - Flags: review?(bugreport) → review-
If you really have different populations of users and products, why not having separate installations of Bugzilla?
(In reply to comment #9) > If you really have different populations of users and products, why not having > separate installations of Bugzilla? They all get to see some bugs in common. Also, the internal team has to deal with all of them.
Assignee: create-and-change → gabriel
One additional benefit of this is an implicit "documentation" of groups. Looking at the list of users in Bugzilla for a given defect, I can easily bring these users into a single room and get things rolling. I'd much appreciate finding this feature in a future release of Bugzilla.
Attached patch v2-move can_add_user_to_bug to Bug.pm (obsolete) (deleted) — Splinter Review
can_add_user_to_bug moved to Bug.pm
Attachment #199091 - Flags: review?(bugreport)
Comment on attachment 199091 [details] [diff] [review] v2-move can_add_user_to_bug to Bug.pm Close... a few items... 1) defparms says the user must be able to see the bug. Actually, the user must be able to edit bugs in the product. 2) This should also cover qa contact if that is in use. 3) When the assignee is rejected, the error message format is munged up. That may only happen if realname is not set. I suggest oyu just consistently use the login_name for these messages.
Attachment #199091 - Flags: review?(bugreport) → review-
Attached patch v3- qa-contact added (obsolete) (deleted) — Splinter Review
Qa-contact added and paramaters description updated.
Attachment #199091 - Attachment is obsolete: true
Attachment #199180 - Flags: review?(bugreport)
Attachment #199180 - Flags: review?(bugreport) → review+
Flags: approval?
This got bitrotted by bug 46296. defparams.pl no longer exists.
Flags: approval?
Attached patch v3-fix (obsolete) (deleted) — Splinter Review
Unbitrotted version after bug 46296
Attachment #199180 - Attachment is obsolete: true
Attachment #199293 - Flags: review?(bugreport)
Attachment #199293 - Flags: review?(bugreport) → review+
Flags: approval?
Flags: approval?
Change can_edit_product_id() to can_edit_product()
Attachment #199293 - Attachment is obsolete: true
Attachment #199308 - Flags: review?(bugreport)
Comment on attachment 199308 [details] [diff] [review] fix change can_edit_product_id to can_edit_product When strictisolation is off, rather than permitting the CC entries, it just silently ignores them.
Attachment #199308 - Flags: review?(bugreport) → review-
Comment on attachment 199308 [details] [diff] [review] fix change can_edit_product_id to can_edit_product Actually, there is another problem with this... If I try to add 4 users to the CClist and 3 of them are OK and one is not, I will get an error message, but the rest of the CC changes will have gone through, but not the rest of the bug changes altogether. The check for inappropriate CC members needs to happen at the validation stage not at the actual insertion stage so that, if we are going to throw an error, we don't let ANY of the changes happen.
Attached patch v4-fix previous mistakes (deleted) — Splinter Review
cc add is treat separate.
Attachment #199830 - Flags: review?(bugreport)
Comment on attachment 199830 [details] [diff] [review] v4-fix previous mistakes r=joel Still need to do a similar protection for post_bug... but, that is another bug. (needs to be done by 2.22)
Attachment #199830 - Flags: review?(bugreport) → review+
Flags: approval?
Flags: approval? → approval+
Checking in attachment.cgi; /cvsroot/mozilla/webtools/bugzilla/attachment.cgi,v <-- attachment.cgi new revision: 1.97; previous revision: 1.96 done Checking in globals.pl; /cvsroot/mozilla/webtools/bugzilla/globals.pl,v <-- globals.pl new revision: 1.343; previous revision: 1.342 done Checking in process_bug.cgi; /cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v <-- process_bug.cgi new revision: 1.288; previous revision: 1.287 done Checking in Bugzilla/Bug.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v <-- Bug.pm new revision: 1.98; previous revision: 1.97 done Checking in Bugzilla/User.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/User.pm,v <-- User.pm new revision: 1.89; previous revision: 1.88 done Checking in Bugzilla/Config/GroupSecurity.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Config/GroupSecurity.pm,v <-- Grou pSecurity.pm new revision: 1.2; previous revision: 1.1 done Checking in template/en/default/admin/params/groupsecurity.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/params/groupsecurit y.html.tmpl,v <-- groupsecurity.html.tmpl new revision: 1.2; previous revision: 1.1 done Checking in template/en/default/global/user-error.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/user-error.html.tm pl,v <-- user-error.html.tmpl new revision: 1.133; previous revision: 1.132 done
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Blocks: 312773
Blocks: 315332
*** Bug 328360 has been marked as a duplicate of this bug. ***
Flags: testcase?
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/qa/4.2/ added t/test_strict_isolation.t Committed revision 206. Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/qa/4.0/ added t/test_strict_isolation.t Committed revision 195.
Flags: testcase? → testcase+
(In reply to Frédéric Buclin from comment #24) > Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/qa/4.2/ > added t/test_strict_isolation.t > Committed revision 206. > > Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/qa/4.0/ > added t/test_strict_isolation.t > Committed revision 195. Wow, this is awesome!! Thank you so much, LpSolit!! This is a huge deal. :-)
Keywords: selenium
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: