Bugzilla should warn when a user is made component owner, but isn't in the group

NEW
Unassigned

Status

()

P3
minor
17 years ago
5 years ago

People

(Reporter: kevin.brannen, Unassigned)

Tracking

Details

Attachments

(1 attachment)

(Reporter)

Description

17 years ago
We've found that when you use groups, a person can be made a component who has
no permission to that group.  Or at least I think this is the root problem.  The
ramifications seem to be that when bugs are filed against that component, the
component owner is not sent email about it, nor does s/he have the ability to
assign the bug to the proper person, etc.  From the standpoint of most of BZ, I
think that is correct as they don't have visibility to the project, component,
and bug; however, there is an underlying error in how did the person get there?

Yes, there is user error here; but I think the software (editcomponents.cgi)
needs to prevent this from happening.  There needs to be a check that the "new"
component owner exists AND has visibility to the group of the component (if
groups are used).
see also bug 39816, which proposes to allow the reporter, QA, and assignee to see 
the bug even if they aren't in the group it's restricted to.  I'm not going to 
dupe this though, since the other will likely be implemented as a preference, and 
if it's turned off, the issue in this bug still needs to be dealt with.
Depends on: 39816
Priority: -- → P2
Target Milestone: --- → Bugzilla 2.16
OK, the way it ended up getting done in bug 39816 should adaquately deal with
this.  Please reopen if I'm wrong.
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
Target Milestone: Bugzilla 2.16 → Bugzilla 2.14
Moving to Bugzilla product
Component: Bugzilla → Bugzilla-General
Product: Webtools → Bugzilla
Version: Bugzilla 2.11 → unspecified
(Reporter)

Comment 4

17 years ago
I hate to do this, but I must reopen it.  I'm testing this on the tarball of
2.14.  How to test for this:

1. pick a person (e.g. Kevin) to test with, look up his permissions, find a
project (e.g. Sales) he is NOT a member of.
2. goto the components screen, pick Sales, edit components, pick any component
(e.g. Domestic Sales), put Kevin's id in the "initial owner" field, and click
UPDATE.

That works successfully, and I think it should not; life goes down hill from
there in other parts of BZ's software as errors happen (see first post).  I know
2.14 added the ability to have non-group people report/qa/etc., but I think the
component owner _needs_ to be part of the product group.  I'll work on a patch.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
-> Administration.
Status: REOPENED → NEW
Component: Bugzilla-General → Administration
OS: Linux → All
Hardware: PC → All
Target Milestone: Bugzilla 2.14 → Bugzilla 2.16
Version: unspecified → 2.15
OK, it should do a warning first that they're not in the group, and then add
them.

I'm wondering how this is gonna work if and when we have groups defined in terms
of other groups - maybe just refuse.
See bug 97469 as welll as bug 97471.
We are currently trying to wrap up Bugzilla 2.16.  We are now close enough to
release time that anything that wasn't already ranked at P1 isn't going to make
the cut.  Thus this is being retargetted at 2.18.  If you strongly disagree with
this retargetting, please comment, however, be aware that we only have about 2
weeks left to review and test anything at this point, and we intend to devote
this time to the remaining bugs that were designated as release blockers.
Target Milestone: Bugzilla 2.16 → Bugzilla 2.18
Is this fixed now that the assignee can always see the bug?
paritally, but not completely.  editcomponents.cgi should still warn if
buggroups are on and the person you're putting in isn't a member of that product
group.  Even though the assignee can still see the bugs, they won't be able to
file bugs on themselves, so it'd be nice to at least give a warning.

Comment 11

15 years ago
These unloved bugs have been sitting untouched since June 2002 or longer.  If
nobody does anything else to them, they certainly won't make 2.18
Retargetting to 2.20.  If you really plan to push them right now, you might pull
them back in.
Target Milestone: Bugzilla 2.18 → Bugzilla 2.20

Updated

14 years ago
Summary: person made component owner when not in group → Bugzilla should warn when a user is made component owner, but isn't in the group

Updated

14 years ago
Severity: normal → minor
Priority: P2 → P3
Target Milestone: Bugzilla 2.20 → Bugzilla 2.22
Assignee: justdave → nobody

Comment 12

13 years ago
(In reply to comment #10)
> paritally, but not completely.  editcomponents.cgi should still warn if
> buggroups are on and the person you're putting in isn't a member of that product
> group.  Even though the assignee can still see the bugs, they won't be able to
> file bugs on themselves, so it'd be nice to at least give a warning.

Display a warning or reject this default assignee (same goes for the default QA contact)? I would prefer the second solution and write something like:

 $user->in_group('foo') || ThrowUserError('user_not_in_group')
Assignee: nobody → administration
QA Contact: mattyt-bugzilla → default-qa
Target Milestone: Bugzilla 2.22 → ---

Comment 13

12 years ago
Created attachment 226705 [details] [diff] [review]
patch, v1

The list of reviewers says to ask GavinS for patches about editcomponents.cgi. ;)
Assignee: administration → LpSolit
Status: NEW → ASSIGNED
Attachment #226705 - Flags: review?(bugzilla)

Updated

12 years ago
Target Milestone: --- → Bugzilla 2.24

Updated

12 years ago
Attachment #226705 - Flags: review?(mkanat)

Updated

12 years ago
Attachment #226705 - Flags: review?(bugzilla) → review?(wicked+bz)

Comment 14

12 years ago
Comment on attachment 226705 [details] [diff] [review]
patch, v1

>Index: editcomponents.cgi

>@@ -155,12 +155,29 @@
>+    # Make sure the assignee and QA contact can edit bugs being
>+    # in the product this component belongs to.
>+    # If defined, we know they exist thanks to Bugzilla::User::match().
>+    my $default_assignee = Bugzilla::User->new_from_login($default_assignee_login);
>+    $default_assignee->can_edit_product($product->id)

  Why can_edit_product instead of can_see_product?

>+      || ThrowUserError('invalid_default_user_group',

  I don't think this should be an error (unless strict_isolation is on, but even then I don't really care). He'll still be able to see and modify bugs assigned to him, he just won't be able to file them.

  Also, we wanted to warn people about users who can't *see* the product. So there should be one warning for the user being unable to see, and another for the user being unable to edit.

  This might also require "make $vars->{'messages'} into an arrayref" which I think would be a good idea anyway.

  Also, all this code is in two places. Wouldn't it be better to make a check_user() subroutine?
Attachment #226705 - Flags: review?(mkanat) → review-

Updated

12 years ago
Attachment #226705 - Flags: review?(wicked+bz)

Updated

12 years ago
Target Milestone: Bugzilla 3.0 → Bugzilla 3.2

Updated

11 years ago
Assignee: LpSolit → administration
Status: ASSIGNED → NEW

Comment 15

9 years ago
Bugzilla 3.2 is restricted to security bugs only. Moreover, this bug is either assigned to nobody or got no traction for several months now. Rather than retargetting it at each new release, I'm clearing the target milestone and the bug will be retargetted to some sensible release when someone starts fixing this bug for real (Bugzilla 3.8 more likely).
Target Milestone: Bugzilla 3.2 → ---
You need to log in before you can comment on or make changes to this bug.