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)
Tracking
()
RESOLVED
FIXED
Bugzilla 2.22
People
(Reporter: batosti, Assigned: gabriel.sales)
References
Details
(Keywords: selenium)
Attachments
(3 files, 3 obsolete files)
(deleted),
patch
|
bugreport
:
review-
|
Details | Diff | Splinter Review |
(deleted),
patch
|
bugreport
:
review-
|
Details | Diff | Splinter Review |
(deleted),
patch
|
bugreport
:
review+
|
Details | Diff | Splinter Review |
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
Reporter | ||
Comment 1•19 years ago
|
||
Attachment #197104 -
Flags: review?(bugreport)
Comment 2•19 years ago
|
||
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
Comment 3•19 years ago
|
||
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
Comment 4•19 years ago
|
||
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 5•19 years ago
|
||
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?
Comment 6•19 years ago
|
||
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.
Comment 7•19 years ago
|
||
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 8•19 years ago
|
||
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-
Comment 9•19 years ago
|
||
If you really have different populations of users and products, why not having
separate installations of Bugzilla?
Comment 10•19 years ago
|
||
(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.
Updated•19 years ago
|
Assignee: create-and-change → gabriel
Comment 11•19 years ago
|
||
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.
Assignee | ||
Comment 12•19 years ago
|
||
can_add_user_to_bug moved to Bug.pm
Attachment #199091 -
Flags: review?(bugreport)
Comment 13•19 years ago
|
||
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-
Assignee | ||
Comment 14•19 years ago
|
||
Qa-contact added and paramaters description updated.
Attachment #199091 -
Attachment is obsolete: true
Attachment #199180 -
Flags: review?(bugreport)
Updated•19 years ago
|
Attachment #199180 -
Flags: review?(bugreport) → review+
Updated•19 years ago
|
Flags: approval?
Comment 15•19 years ago
|
||
This got bitrotted by bug 46296. defparams.pl no longer exists.
Flags: approval?
Assignee | ||
Comment 16•19 years ago
|
||
Unbitrotted version after bug 46296
Attachment #199180 -
Attachment is obsolete: true
Attachment #199293 -
Flags: review?(bugreport)
Updated•19 years ago
|
Attachment #199293 -
Flags: review?(bugreport) → review+
Updated•19 years ago
|
Flags: approval?
Updated•19 years ago
|
Flags: approval?
Assignee | ||
Comment 17•19 years ago
|
||
Change can_edit_product_id() to can_edit_product()
Attachment #199293 -
Attachment is obsolete: true
Updated•19 years ago
|
Attachment #199308 -
Flags: review?(bugreport)
Comment 18•19 years ago
|
||
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 19•19 years ago
|
||
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.
Assignee | ||
Comment 20•19 years ago
|
||
cc add is treat separate.
Attachment #199830 -
Flags: review?(bugreport)
Comment 21•19 years ago
|
||
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+
Updated•19 years ago
|
Flags: approval?
Updated•19 years ago
|
Flags: approval? → approval+
Comment 22•19 years ago
|
||
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
Comment 23•19 years ago
|
||
*** Bug 328360 has been marked as a duplicate of this bug. ***
Updated•18 years ago
|
Flags: testcase?
Comment 24•13 years ago
|
||
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+
Comment 25•13 years ago
|
||
(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. :-)
You need to log in
before you can comment on or make changes to this bug.
Description
•