I noticed the other day that reports.cgi doesn't check for the usebuggroups parameter, and thus doesn't restrict access to reports for products by bug group if it's on. I consider this a bug, and so I've corrected it on my local installation. If people don't have permission to access the bugs of a product, they shouldn't be able to get reports on that product either, right?
I'll take this. Since you said you've corrected it on your site, I assume you can post a patch?
Assignee: tara → dave
Created attachment 11692 [details] [diff] [review] Patch to make reports.cgi respect usebuggroups parameter
Question.... since you're the one that invented the usebuggroups and usebuggroupsentry params, what specifically is each supposed to restrict? when I added a similar patch to this to query.cgi to keep it out of the product selection box on the query screen, I used the 'usebuggroupsentry' param. Is that what I should have used? Judging by this patch, I should have used the 'usebuggroups' one... Also, I don't like the confirm_login() in there. Now anyone that doesn't have an account can't get reports, not even for publicly visible products. (I have some of each on my system). Better to do the quietly_check_login() to see if they are logged in or not, and just pretend they don't have access if they're not logged in. Anyone who wants to run a report on a restricted product will know they need to log in first to see it.
1) I hadn't looked at your patch to query.cgi, so I didn't realize that you were doing it that way. (Can you let me know the bug that that's attached to? I'd like to look at it, and probably add it to my installation...) The intended use was for "usebuggroups" to be used for querying and viewing bugs, and "usebuggroupsentry" to be for entering new bugs. The reasoning was that you might want to have people testing something who can enter new bugs, but you wouldn't want them to be querying other bugs. If you turned on "usebuggroups" but left "usebuggroupsentry" off, then they could enter bugs against a product without having permissions for that product. (It occurs to me now that this would also prevent them from returning to the bug that they themselves had entered, and that might be a bad thing. Maybe I should create a new bug for that and look into it...) So in the case of query.cgi, and in the reports.cgi patch I checked in, it would seem to me that "usebuggroups" would be the parameter you'd want, not "usebuggroupsentry". 2) You're right... That thought hadn't occured to me, but using quietly_check_login() would be just fine. However, in looking at that a bit, I realized that my patch is flawed; I forgot to deal with the "-All-" case, so if a user doesn't have permissions to any groups, they can still see full stats by selecting the "-All-" option. I'll fix this case, and switch the login mechanism as well, and I'll post a new patch when that's done.
The query.cgi patch is in bug 45586. If you're updating from CVS, it's already been checked in, so just do a 'cvs update' and you'll have it. I don't see any product-specific information in any of the current reports, so I don't think "-All-" will be much of a concern. It gives you an accurate picture of the workload a given engineer has, without having to know exactly what he's working on. If the same engineer is responsible for "A", which is public, and "B", which is private, and he has 2 open bugs in "A", and 50 in "B", and the user runs a report on -All- that is restricted to what he can see, it looks like that engineer only has 2 bugs. Now this user will try to claim the engineer is being lazy because he won't look at his 2 bugs.... :-) See bug 39816 for a proposed solution to your problem with someone not being able to see bugs they entered if they didn't have access to that product.
Status: NEW → ASSIGNED
Yeah, you've got a point. And since they won't be able to actually view the bugs if they click on them, all they see are bug IDs, so that's not really a problem. So the only change needed to the patch is to change the login mechanism called. Should I put a new patch up, or can whoever checks it in change the patch themselves before doing it?
That's straight-forward enough, I think I can fix it when I check it in.
Fixed and checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
In search of accurate queries.... (sorry for the spam)
Target Milestone: --- → Bugzilla 2.12
Moving closed bugs to Bugzilla product
Component: Bugzilla → Bugzilla-General
Product: Webtools → Bugzilla
Version: other → unspecified
You need to log in before you can comment on or make changes to this bug.