Closed
Bug 39816
Opened 25 years ago
Closed 23 years ago
Anyone in CC/reporter/QA Contact fields should see restricted bugs
Categories
(Bugzilla :: Bugzilla-General, defect, P3)
Bugzilla
Bugzilla-General
Tracking
()
RESOLVED
FIXED
Bugzilla 2.14
People
(Reporter: justdave, Assigned: myk)
References
(Blocks 1 open bug, )
Details
(Whiteboard: security code)
Attachments
(8 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
Anyone listed in the CC, reporter, QA Contact, or owner fields on a bug should be
able to see that bug, regardless of the group permissions set on that bug. If
you really don't want that person to see that bug, you should take their name off
it, also (since they're going to get emails on it anyway).
This would also have the added bonus of being able to set up a group for "private
bugs" that only the reporter, QA, and the owner could see, by making sure no one
was a member of that "private bugs" group.
Main use of this would be for customer support tickets where the customer could
still be able to view their own bug, while keeping the general public out of it,
and not requiring a group to be created for each customer.
Comment 1•24 years ago
|
||
Sounds like a good idea. As for how to do this, I think we could probably
manage it pretty easily by modifying the initial select statement in
bug_form.pl. The current version has:
select
...
from bugs left join votes using(bug_id)
where bugs.bug_id = $id
and bugs.groupset & $::usergroupset = bugs.groupset
group by bugs.bug_id
We could update it to read:
select
...
from bugs left join votes using(bug_id)
where bugs.bug_id = $id
and ((bugs.groupset & $::usergroupset = bugs.groupset) or
(something to check the reporter, assigned_to, qa_contact, and cc fields))
group by bugs.bug_id
I'd do it myself, but my SQL knowledge is slightly lacking for the task, because
cc is in a separate table, and I'm not certain enough of my knowledge of mySQL
to do joins and all that.
It looks like this would be sufficient to allow the user to see bugs that he was
involved in, even if not in the group for it, right? Anyone with a slightly
better working knowledge of SQL want to step up and give this a whirl?
Reporter | ||
Comment 2•24 years ago
|
||
This works:
select
......
from bugs
left join votes on bugs.bug_id = votes.bug_id
left join cc on bugs.bug_id = cc.bug_id
where bugs.bug_id = 131where bugs.bug_id = $id
and ((bugs.groupset & $::usergroupset = bugs.groupset) or
(reporter = $::userid) or (assigned_to = $::userid) or
(qa_contact = $::userid) or (cc.who = $::userid))
group by bugs.bug_id
had to change the using(bug_id) to a specific match since there were now more
than 2 bug_id columns and it confused it.
My initial testing here presents another problem though...
The groupset selectors where you can set what groups it has access to... if the
user doesn't have access to a group, they don't get a selector to move the bug in
and out of that group. If the selector isn't there, it's not counted when it
adds up the groupset when you hit Commit. So if someone in one of those other
fields looks at a hidden bug and then changes it, poof, it's no longer hidden.
The solution for that part is to put the groupset selectors in the form anyway,
but make them hidden if the user doesn't have access, instead of not having them
there at all.
Reporter | ||
Comment 3•24 years ago
|
||
Can't forget about buglist.cgi either... they'd have to be able to see their
bugs there, too.
Chris, I'm dragging you into this because we'll probably have a patch available
for this within a few days, and this one is far-reaching enough it'll need some
play on landfill before we check it in.
Reporter | ||
Comment 4•24 years ago
|
||
And we also can't forget processmail. Currently, processmail restricts mailings
to those who have access to the bug, and ignores CCs if the CC'd people don't
have access to it.
Comment 6•24 years ago
|
||
Dave, Chris and co: Implementing this RFE would be very useful for security
bugs, where the strong suggestion in n.p.m.security (where Frank Hecker has just
kicked off that debate again) is that they should be restricted to a certain
group _plus_the_reporter_.
Is there any chance of munging the thoughts in the comments in this bug into a
patch?
Gerv
Reporter | ||
Updated•24 years ago
|
Assignee: tara → dave
Reporter | ||
Comment 7•24 years ago
|
||
I already have it partially implemented on my system as of about a month or two
ago, but it didn't get uploaded because I didn't finish it. (i.e. you can view a
bug that you're the reporter of (show_bug), but you can't find it in a query
(bug_list) and it won't send you mail about it (processmail)).
Since I already have part of it done, and had actually forgotten about it (thanks
for reminding me) I'll go ahead and grab this bug report and see if I can finish
it up.
Comment 8•24 years ago
|
||
IMO, this is a bug. SEVERITY -> normal. There were several NS-"confidental" bug
on which I was first cced, but then got neither notifications nor could access
it (but I got no notification about being removed from cc).
Severity: enhancement → normal
Comment 9•24 years ago
|
||
But there's security issues involved in this...
First, you might not always want the reporter to see the bug. Sure it sucks,
but if you've got all kinds of internal code in the bug report, the reporter
can't see it...
And what happens when a bug is made confidential, but all non-internal people
aren't removed? I can see that happening quite a lot.
Comment 10•24 years ago
|
||
> Sure it sucks,
> but if you've got all kinds of internal code in the bug report, the reporter
> can't see it...
(Note: That might be true for closed source project, but it isn't for
bugzilla.mozilla.org. There shouldn't be proprietary code posted to Mozilla
resources. Which opens the question why *any* non-security bug can be
confidental on bugzilla.mozilla.org.)
> And what happens when a bug is made confidential, but all non-internal people
> aren't removed? I can see that happening quite a lot.
Yes, but they should still have access. Otherwise remove them. I should be
possible to allow selected "outsiders" to access the bug.
Comment 11•24 years ago
|
||
"(Note: That might be true for closed source project, but it isn't for
bugzilla.mozilla.org. There shouldn't be proprietary code posted to Mozilla
resources. Which opens the question why *any* non-security bug can be
confidental on bugzilla.mozilla.org.)"
Yes, this is something that has been discussed many times. But we're not
getting into that argument again in this bug, because that's not what this bug
is for.
"Yes, but they should still have access. Otherwise remove them. "
But people make mistakes. And it's all tiresome to manually remove 30 non-
internal people from a CC list. And how do you remove the reporter?
Comment 12•24 years ago
|
||
OK, but how do you allow selected people access? I had cases where Netscape
wanted to do changes to my code for N6 only, or checkin the changes to
mozilla.org (e.g. a new pref), but the intention was N6 (-> different default
prefs). In one case I even revied the changes for a bug I could not access.
That's not right.
Comment 13•24 years ago
|
||
OK, here's my thoughts on how this should work... good, bad or indifferent.
* Completely remove the groupset from the WHERE part of SQL Queries (But leave
it in the SELECT part [esp. when a large number of bugs is being returned a
la buglist.cgi]).
* Create a new sub in globals.pl call "UserCanSeeBug" (See Below).
* Change the security dropdown to a series of checkboxes (See Below).
* Add a new option in editcomponents to set the initial security setting
(New bugs are secure)
The UserCanSeeBug subroutine could probably then be used to kill some of the
bugs attached to bug 66091.
--- Below ---
sub UserCanSeeBug {
my ($bugid, $grpset) = (@_);
if (defined $grpset && $grpset == 0) {
return 1;
}
# A bunch of code to get information about bug number $bugid and the current
# user. After that, there should be some code to check permissions. If at
# any point it's determined that the user can see the bug, simply
# "return 1;" (Basically, assume the user can't see the bug, unless
# determined otherwise).
# All options are exausted...
return 0;
}
--- Below #2 ---
+----------------------------------------------------------------------------+
| Bug Security: |
| If none of the checkboxes below are checked, the bug can be seen by all |
| [ ] Users in Group "Security" |
| [ ] Users in Group "Software Engineers" |
| ( ) And (*) Or |
+----------------------------------------------------------------------------+
| Allow these additional people to see the bug: |
| [X] Reporter [ ] QA Contact [ ] CC List |
+----------------------------------------------------------------------------+
Comment 14•24 years ago
|
||
Jake put it the way I would like to see it.
Reporter | ||
Comment 15•24 years ago
|
||
I like the way Jake described too. That'll require a bit of reworking of the way
groups are handled though.
Updated•24 years ago
|
Whiteboard: security
Updated•24 years ago
|
Whiteboard: security → 2.14, security
Reporter | ||
Comment 16•24 years ago
|
||
moving to real milestones...
Whiteboard: 2.14, security → security
Target Milestone: --- → Bugzilla 2.14
Comment 17•23 years ago
|
||
*** Bug 39067 has been marked as a duplicate of this bug. ***
Comment 18•23 years ago
|
||
When this feature is added, we need to make sure that someone watching a user
CCed on a restricted bug doesn't get mail from the restricted bug unless the
watcher is also a part of the group.
Reporter | ||
Comment 19•23 years ago
|
||
no point in doing this twice. Let's get the group management stuff fixed first.
Adding dependency on the group table redo bug.
Depends on: 68022
Comment 20•23 years ago
|
||
First, some comments about Jake's proposal:
I like the use of check boxes for groups instead of drop down lists. That's
what Red Hat does and I have modified my local installation of bugzilla to do
the same. I also like the addition of the and/or construct. I believe this is
easier for the user to understand. Currently, if you check more than one group,
only users in all groups can see the bug.
Now, a couple of questions about Jake's proposal:
1. Is it optional? I don't want to have to click a bunch of boxes to get a
bunch of people to see a bug that currently see the bug by default now. I don't
even want to see these controls on the interface. I would want to keep the
group UI though.
2. If you don't select CC, does that mean that non of the CC list sees it or
only that CCers can only see it if they belong to the proper group?
Comment 21•23 years ago
|
||
> I also like the addition of the and/or construct.
So did I :) My day job is as a Network Administrator so to me assigning group
permissions is normally an "or" statement (If you are in group CCN or SNR then
you can see the Security directory). But realizing that the current bugzilla is
based on an AND, it wouldn't be very nice of us to suddenly change it to OR.
> 1. Is it optional?
Well, if you aren't in any bug groups, it won't show up (like the present
drop-downs). Anything more than that is really up to Dave (as the owner of this
bug).
> 2. If you don't select CC ...
Then the CC list doesn't get added to the list of ppl that can view the bug.
The section labeled "Allow these addition people" would not effect anyone who
otherwise would have permission (is in all the correct groups).
Also, most of what I proposed for UserCanSeeBug() has been implimented in
ValidateBugID() [Validate also makes sure that bug ID's are numeric but it
doesn't have the ability to optionally accept $groupset which would be a
requirement to use it from buglist (it would also have to be able to cache some
information about the logged in user and accept information about the bugs
reporter, CC list, etc. in order to be used for this bug, otherwise querying
becomes a much more expensive operation)].
Assignee | ||
Updated•23 years ago
|
Whiteboard: security → security code
Assignee | ||
Comment 22•23 years ago
|
||
This bug was previously waiting for groups to be redone in bug 68022, which has
since been rescheduled to 2.16. Should this bug also be rescheduled?
Reporter | ||
Comment 23•23 years ago
|
||
maybe. It can be done with the current group code, we just didn't want to have
to do it over again with the new group code being so close. Now that it's not so
close anymore... I dunno. I suppose this one is mine, though, since I sort of
had it working on InTrec...
Assignee | ||
Comment 24•23 years ago
|
||
Dave, are you working on this?
Assignee | ||
Comment 25•23 years ago
|
||
Assignee | ||
Comment 26•23 years ago
|
||
This attachment does the second part of what Jake wanted (the first part having
already been done in some other bug whose number I don't remember now). It
works except for the strange habit of detecting a collision with every change
that has previously occurred. I will look into it.
Assignee | ||
Comment 27•23 years ago
|
||
Assignee | ||
Comment 28•23 years ago
|
||
Assignee | ||
Comment 29•23 years ago
|
||
The second/third versions of the patch adds the assignee to the list of roles
who can be granted access to an otherwise inaccessible bug, and it fixes the
problem with mid-air collisions (Bugzilla apparently doesn't like it when code
calls &MoreSQLData and then doesn't call &FetchSQLData). Version three contains
very minor adjustments in wording (Cc -> CC for consistency and role -> user for
clarity).
Patch ready for review.
Assignee | ||
Comment 30•23 years ago
|
||
adding the URL to the test installation on landfill where you can see the patch
in action:
http://landfill.tequilarista.org/bz66235/
Reporter | ||
Comment 31•23 years ago
|
||
I like it. But I think we might need to massage the wording a little... it's
obvious to me, but probably not blatently obvious to someone looking at it for
the first time...
Comment 32•23 years ago
|
||
I think the proper way to fix that problem of calling MoreSQLData() but not
FetchSQLData() causing problems on the next query would be to add |undef
@::fetchahead;| to the SendSQLData() routine.
Also, as for the wording, I think saying (possibly in smaller type underneath
the "But always..." text) "These aditional options will not be effective unless
at least on of the group checkboxes is selected" (or something similar).
See also my comments in bug 66235.
Assignee | ||
Comment 33•23 years ago
|
||
Assignee | ||
Comment 34•23 years ago
|
||
Assignee | ||
Comment 35•23 years ago
|
||
Assignee | ||
Comment 36•23 years ago
|
||
Reporter | ||
Comment 37•23 years ago
|
||
Bug 95024 has been filed to allow the Reporter/QA/Assignee/CClist to be able to
see these bugs in their buglist also (this patch doesn't allow for that)
r= justdave
checked in.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 38•23 years ago
|
||
test
Reporter | ||
Comment 39•23 years ago
|
||
Moving to Bugzilla product
Component: Bugzilla → Bugzilla-General
Product: Webtools → Bugzilla
Version: other → unspecified
Comment 40•23 years ago
|
||
We also may want to only display the QA Contact checkbox (in bug_form.pl) if the
Param('useqacontact') is set.
Comment 41•23 years ago
|
||
Updated•12 years ago
|
QA Contact: matty_is_a_geek → default-qa
You need to log in
before you can comment on or make changes to this bug.
Description
•