Open Bug 90619 Opened 23 years ago Updated 8 years ago

Restriction of user rights

Categories

(Bugzilla :: Creating/Changing Bugs, defect, P3)

2.12
defect

Tracking

()

People

(Reporter: afrey, Unassigned)

References

(Depends on 2 open bugs, Blocks 3 open bugs)

Details

(Whiteboard: [permissions:edit])

Attachments

(1 file, 4 obsolete files)

Is there any way to restrict the access rights of a user, so that he can only submit bugs and edit the comments field?
Not currently, but it's definitely a good feature request. If you implement this please post a patch here. :-) Otherwise I'm guessing about a month or two before any of the usuals get to it. process_bug.cgi is the file you'd have to play with. There are some checks in there to see if the user has permission to change a bug (look for references to editbugs). I think it has overrides for reporter on it. It should be relatively easy to remove those overrides.
Depends on: 68022
Target Milestone: --- → Bugzilla 2.16
oops, what am I thinking... :-) This needs a param, not a group. Bug 68022 is for groups. :-) removing the dependency... What I was thinking is that the "right" way to fix this is to add a param about whether to restrict reporters or not, so the systems that like having the reporters be able to modify don't have to give it up. Fixing it for your own install should be a couple lines in the code though.
No longer depends on: 68022
Priority: -- → P3
You've said that "Fixing it for your own install should be a couple lines in the code though". Can you please attach an example?
well, ok, say for example you want to prevent the reporter from changing the status on a bug. That's checked at line 293 of process_bug.cgi in the current cvs code, with the following chunk of code: # Let reporter change bug status, even if they can't edit bugs. # If reporter can't re-open their bug they will just file a duplicate. # While we're at it, let them close their own bugs as well. if ( ($f eq "bug_status") && ($whoid eq $reporterid) ) { return 1; } If you just comment out the three lines that aren't commented yet there, it would disable the reporter from being able to change the status of a bug (unless he had some other reason to have that capability, say he owned the bug, or had 'editbugs' privs)
I've tried this, but it doesn't work - for some reason the reporter still can resolve and close the bug...
-> Bugzilla product, Changing Bugs component, reassigning. When the lines that have to be changed are identified, this may go to Administration to add a Param. :)
Assignee: tara → myk
Component: Bugzilla → Creating/Changing Bugs
Product: Webtools → Bugzilla
Whiteboard: [permissions:edit]
Version: Bugzilla 2.12 → 2.12
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
Similar to bug#95579
*** Bug 95579 has been marked as a duplicate of this bug. ***
OS: Linux → All
Hardware: PC → All
Unloved bugs targetted for 2.18 but untouched since 9-15-2003 are being retargeted to 2.20 If you plan to act on one immediately, go ahead and pull it back to 2.18.
Target Milestone: Bugzilla 2.18 → Bugzilla 2.20
(In reply to comment #4) > If you just comment out the three lines that aren't commented yet there, it > would disable the reporter from being able to change the status of a bug (unless > he had some other reason to have that capability, say he owned the bug, or had > 'editbugs' privs) I would say the reporter should only be able to mark bugs as RESOLVED INVALID and/or RESOLVED DUPLICATED as long as his bug is in the UNCONFIRMED state, e.g. if he realizes too late he did a 'mistake'. He should lose this feature as soon as the bug is marked NEW by a user with editbugs privs, because this means that this bug is valid and should be considered by competent developers.
(In reply to comment #11) I tried successfully to prevent the reporter to close an open bug by editing these two files: --- /root/tmp/bugzilla-2.18rc2/process_bug.cgi 2004-07-17 18:09:27.000000000 +0200 +++ process_bug.cgi 2004-08-13 13:12:43.389302048 +0200 @@ -457,6 +457,12 @@ return 0; } + # - close a confirmed (open) bug + if (($field eq "bug_status") && + IsOpenedState($oldvalue)) + { + return 0; + } # Allow reporter to change anything else. return 1; } And: --- /root/tmp/user-error.html.tmpl 2004-07-22 08:57:09.000000000 +0200 +++ template/en/default/global/user-error.html.tmpl 2004-08-13 12:42:57.751760216 +0200 @@ -261,8 +261,8 @@ <strong>[% field_descs.$field FILTER html %]</strong> field from <em>[% oldvalue FILTER html %]</em> to <em>[% newvalue FILTER html %]</em>, - but only the owner or submitter of the [% terms.bug %], or a - sufficiently empowered user, may change that field. + but only the owner of the [% terms.bug %] or a + sufficiently empowered user may change that field. [% ELSIF error == "illegal_changed_in_last_x_days" %] [% title = "Your Search Makes No Sense" %] Unfortunately, I cannot find where to prevent the display of the following when editing bugs: Resolve bug, changing resolution to Resolve bug, mark it as duplicate of bug # Reassign bug to Reassign bug to owner and QA contact of selected component Could someone tell me where to look for? Thanks!
(In reply to comment #12) > Unfortunately, I cannot find where to prevent the display of the following when > editing bugs: > > Resolve bug, changing resolution to > Resolve bug, mark it as duplicate of bug # > Reassign bug to > Reassign bug to owner and QA contact of selected component > > > Could someone tell me where to look for? Thanks! > OK, I found this solution, editing Bug.pm and knob.html.tmpl: --- /root/tmp/Bug.pm 2004-07-06 09:08:02.000000000 +0200 +++ Bug.pm 2004-09-06 03:52:55.923494184 +0200 @@ -402,7 +402,7 @@ # in the world; their permissions will get checked when they log in # and actually try to make the change. $self->{'user'}->{'canedit'} = $::userid == 0 - || $::userid == $self->{'reporter'}{'id'} + # || $::userid == $self->{'reporter'}{'id'} || (Param('useqacontact') && $self->{'qa_contact'} && $::userid == $self->{'qa_contact'}{'id'}) || $::userid == $self->{'assigned_to'}{'id'} || &::UserInGroup("editbugs"); And: --- /root/tmp/knob.html.tmpl 2004-07-10 16:51:25.000000000 +0200 +++ knob.html.tmpl 2004-09-06 05:22:52.347113096 +0200 @@ -44,7 +44,7 @@ [% knum = knum + 1 %] [% END %] - [% IF bug.user.canedit %] + [% IF bug.user.canedit || (bug.reporter && bug.bug_status == "UNCONFIRMED") %] [% IF bug.isopened %] [% IF bug.bug_status != "ASSIGNED" && bug.user.canconfirm %] <input type="radio" id="knob-accept" name="knob" value="accept"> @@ -153,6 +153,15 @@ [% knum = knum + 1 %] [% END %] [% END %] + [% ELSE %] + [% IF bug.reporter && !bug.isopened && bug.bug_status != "CLOSED" && bug.resolution != "MOVED" %] + <input type="radio" id="knob-reopen" name="knob" value="reopen"> + <label for="knob-reopen"> + Reopen [% terms.bug %] + </label> + <br> + [% knum = knum + 1 %] + [% END %] [% END %] <input type="submit" value="Commit"> Now, the reporter can only close his bugs as long as they are in the UNCONFIRMED state and reopen them as long as they are not marked CLOSED (ie RESOLVED or VERIFIED). Unnecessary entries "Resolve bug..." and "Reassign bug..." are not displayed anymore when the bug is opened. Not that users with canconfirm or canedit privs are not affected. Bugzilla 2.18 is still at a release candidate state. Is it too late for implementing this (if wanted)?
Flags: blocking2.18?
(In reply to comment #13) > Bugzilla 2.18 is still at a release candidate state. Is it too late for > implementing this (if wanted)? Yes, this counts as a feature change, which we can no longer do on the 2.18 branch, only bugfixes. 2.20 isn't very far off though. The snippets of patch posted here, however, just outright make this change, they don't provide an option. I know there are folks (bugzilla.mozilla.org for example) that won't want this. If it goes in, it needs to be controllable with a param, and/or with a role group that defaults to making everyone a member of it.
Flags: blocking2.18? → blocking2.18-
(In reply to comment #14) > example) that won't want this. If it goes in, it needs to be controllable with > a param, and/or with a role group that defaults to making everyone a member of it. I suggest a global option to enable/disable this feature (ie for all products). If this feature is enabled, we could imagine a second option to restrict/allow this feature on a per product basis in order to get a finer control on this restriction. I think bug 180879 should be marked as a blocker of this one as bug 180879 needs to be considered as well to get a global solution, see bug 180879 comment 1 and bug 180879 comment 5 for two points of view. From current bug's comment 0, it seems that the reporter also wants to prevent any unprivileged user from modifying flags.
(In reply to comment #15) > I suggest a global option to enable/disable this feature (ie for all products). OK! I have a full patch for this. Now forget my partial patches in comments 12 and 13 and consider the following ones: --- /root/tmp/defparams.pl 2004-07-06 03:12:29.000000000 +0200 +++ defparams.pl 2004-09-06 23:23:25.907213920 +0200 @@ -710,6 +710,13 @@ }, { + name => 'usereporterrestrictions', + desc => 'Do you wish to restrict reporter\'s rights? ', + type => 'b', + default => 0 + }, + + { name => 'webdotbase', desc => 'It is possible to show graphs of dependent bugs. You may set ' . 'this parameter to any of the following: Here, the new variable usereporterrestrictions is used to allow/restrict reporter's rights. By default, its value is 0 (no additional restriction). My previous patches are modified in order to take this new variable into account, as follow: --- /root/tmp/process_bug.cgi 2004-07-17 18:09:27.000000000 +0200 +++ process_bug.cgi 2004-09-06 23:53:07.043440056 +0200 @@ -457,6 +457,13 @@ return 0; } + # - close a confirmed (open) bug if usereporterrestrictions is on + if (($field eq "bug_status") && + IsOpenedState($oldvalue) && + Param('usereporterrestrictions')) + { + return 0; + } # Allow reporter to change anything else. return 1; } Here, we return 0 only if usereporterrestrictions==1. --- /root/tmp/user-error.html.tmpl 2004-07-22 08:57:09.000000000 +0200 +++ template/en/default/global/user-error.html.tmpl 2004-09-07 00:27:15.993952232 +0200 @@ -261,7 +261,8 @@ <strong>[% field_descs.$field FILTER html %]</strong> field from <em>[% oldvalue FILTER html %]</em> to <em>[% newvalue FILTER html %]</em>, - but only the owner or submitter of the [% terms.bug %], or a + but only the owner [% IF !Param('usereporterrestrictions') %] + or submitter [% END %] of the [% terms.bug %], or a sufficiently empowered user, may change that field. [% ELSIF error == "illegal_changed_in_last_x_days" %] Here, the error message show/hide the string "or submitter" depending on the value of usereporterrestrictions. --- /root/tmp/Bug.pm 2004-07-06 09:08:02.000000000 +0200 +++ Bugzilla/Bug.pm 2004-09-07 00:06:32.039062120 +0200 @@ -402,7 +402,7 @@ # in the world; their permissions will get checked when they log in # and actually try to make the change. $self->{'user'}->{'canedit'} = $::userid == 0 - || $::userid == $self->{'reporter'}{'id'} + || ($::userid == $self->{'reporter'}{'id'} && !Param('usereporterrestrictions')) || (Param('useqacontact') && $self->{'qa_contact'} && $::userid == $self->{'qa_contact'}{'id'}) || $::userid == $self->{'assigned_to'}{'id'} || &::UserInGroup("editbugs"); Here, I give $::userid == $self->{'reporter'}{'id'} back, but only as long as usereporterrestrictions==0. --- /root/tmp/knob.html.tmpl 2004-07-10 16:51:25.000000000 +0200 +++ knob.html.tmpl 2004-09-06 05:22:52.347113096 +0200 @@ -44,7 +44,7 @@ [% knum = knum + 1 %] [% END %] - [% IF bug.user.canedit %] + [% IF bug.user.canedit || (bug.reporter && bug.bug_status == "UNCONFIRMED") %] [% IF bug.isopened %] [% IF bug.bug_status != "ASSIGNED" && bug.user.canconfirm %] <input type="radio" id="knob-accept" name="knob" value="accept"> @@ -153,6 +153,15 @@ [% knum = knum + 1 %] [% END %] [% END %] + [% ELSE %] + [% IF bug.reporter && !bug.isopened && bug.bug_status != "CLOSED" && bug.resolution != "MOVED" %] + <input type="radio" id="knob-reopen" name="knob" value="reopen"> + <label for="knob-reopen"> + Reopen [% terms.bug %] + </label> + <br> + [% knum = knum + 1 %] + [% END %] [% END %] <input type="submit" value="Commit"> This part is identical to the one in comment 13. I use the fact that bug.user.canedit=1 if usereporterrestrictions==0 so that the reporter can close open bugs. justdave, what do you think about this solution?
Attached patch First attempt, see comment 16 (obsolete) (deleted) — Splinter Review
Following justdave's suggestion, I put all patches from my comment 16 in a single text file.
Attachment #158052 - Attachment is patch: true
Attachment #158052 - Flags: review?
Attached patch Full patch, see comment 16 (obsolete) (deleted) — Splinter Review
My previous attachment was only informative (not a patch), ie with my comments inside, and was wrongly displayed using the Diff button. This should now be corrected!
Attachment #158052 - Attachment is obsolete: true
Comment on attachment 158090 [details] [diff] [review] Full patch, see comment 16 I think justdave is the right person to ask for review. This patch is exactly the same as my previous attachment, but in a better format.
Attachment #158090 - Flags: review?(justdave)
Comment on attachment 158052 [details] [diff] [review] First attempt, see comment 16 Clearing review for this patch. Please consider attachment 158090 [details] [diff] [review] instead.
Attachment #158052 - Flags: review?
(In reply to comment #19) > (From update of attachment 158090 [details] [diff] [review]) > I think justdave is the right person to ask for review. This patch is exactly > the same as my previous attachment, but in a better format. > This patch is for the 2.18 branch. I can write the same one for the trunk, if wanted. But I first want a developer's opinion to know if it worth doing this or not. justdave, bugreport?
Comment on attachment 158090 [details] [diff] [review] Full patch, see comment 16 Mostly this looks pretty good. I think we need to come up with something a little more specifically descriptive for the param name and description though. Perhaps "allowreportertoresolve" and have it default to On (then flip the logic referring to it), since that's the only thing we're actually restricting. The param description should mention that it only affects people that don't have editbugs privs.
Attachment #158090 - Flags: review?(justdave) → review-
If allowreportertoresolve is all we are doing, what makes this so special that people who want it shouldn't just hack CheckCanChangeField() like everyone else? :-) Gerv
Per justdave's comments: use the new param name "allowreportertoresolve" and give a better param description.
Attachment #158090 - Attachment is obsolete: true
(In reply to comment #24) > Created an attachment (id=158679) gerv, it is not enough to hack CheckCanChangeField() in process_bug.cgi because of the UI which should hide unnecessary radio buttons and display a specific error message when the reporter tries to close an open bug and allowreportertoresolve==0. In the same way, we have a homogeneous solution which will be kept when upgrading.
Attachment #158679 - Flags: review?(justdave)
> gerv, it is not enough to hack CheckCanChangeField() in process_bug.cgi because > of the UI which should hide unnecessary radio buttons and display a specific > error message when the reporter tries to close an open bug and > allowreportertoresolve==0. But that doesn't answer my point. If anyone hacks CheckCanChangeField(), they will probably end up with UI being displayed to people who can't use it, and a generic error message if they try. Why is this particular configuration so important that it requires special UI support and a specific error message? If you like, you can try and argue that more people want to make this change than any other. If that was true, then I could see the possible use of this change. But if it's not, we then open ourselves up to anyone who wants another customisation of this type asking for a specific parameter for that too, and our UI templates descend into a mess of complex conditionals. Gerv
Comment on attachment 158679 [details] [diff] [review] Replace usereporterrestrictions by allowreportertoresolve removing my request for review since IsOpenedState($oldvalue)=1 when $oldvalue=$::unconfirmedstate which is unwanted. I want to allow the reporter to close his bug if it has the UNCONFIRMED status.
Attachment #158679 - Flags: review?(justdave)
Attached patch 2nd patch (obsolete) (deleted) — Splinter Review
Using usereporterrestrictions again, this patch prevents the reporter to modify fields and to close its bugs as soon as a bug is confirmed. The idea is that users with canedit privs do not want (I guess) that the reporter modify anything as soon as they have confirmed the bug and set flags correctly. In the same way, one certainly does not want the reporter to close its bug if they are actually working on it. I could resume this patch as: "You have submitted a valid bug. Thanks! Now, let us do the rest." gerv, I guess you still do not see the interest in such a patch? I think this could be worth considering in parallel with bug 180879 (about flags), using the same param (usereporterrestrictions could then become restrictuserrights). justdave, could you please review it?
Attachment #158679 - Attachment is obsolete: true
Comment on attachment 159623 [details] [diff] [review] 2nd patch justdave, I think this patch really does what is expected.
Attachment #159623 - Flags: review?(justdave)
Comment on attachment 159623 [details] [diff] [review] 2nd patch removing my request for review as the check-in of kiko's patch (bug 236678) on July 28 changed Bug.pm in a way that my patch does not apply anymore. Moreover, I got no feedback (especially from justdave) about this patch so that I don't really know what Bugzilla developers really want. :(
Attachment #159623 - Attachment is obsolete: true
Attachment #159623 - Flags: review?(justdave)
Blocks: 95923
Blocks: 95580
Blocks: 81457
Blocks: 268466
I'd be willing to review an updated patch, but I think: - The param should be "restrictreporteractions" or something like that - You wrap that long line in Bug.pm - It needs some language review (I can do that when it comes)
Assignee: myk → LpSolit
No longer blocks: 268466
Blocks: 231594
Blocks: 71520
I think we should prevent that as well: https://bugzilla.mozilla.org/show_activity.cgi?id=274514
Target Milestone: Bugzilla 2.20 → Bugzilla 2.22
Blocks: 295338
I won't have time to play with it before the 2.22 freeze
Target Milestone: Bugzilla 2.22 → ---
If someone wants to play with this bug, please take it. This bug is not part of the roadmap for 3.0 so it has a very low priority in my TODO list. But this doesn't mean we wouldn't take it for 3.0 if someone could attach a patch. ;)
Assignee: LpSolit → create-and-change
QA Contact: mattyt-bugzilla → default-qa
Blocks: 49808
*** Bug 318658 has been marked as a duplicate of this bug. ***
No longer blocks: 95923
*** Bug 361514 has been marked as a duplicate of this bug. ***
I don't want to allow the user to change the version of the already existeing bug into its older version. e.g. If bug xxxx is created with version 3.0 then I don't want to allow the user to update this bug with version 2.0 or 1.0 or anything which older than 3.0. Please let me know what need to be done and which file need to be update
Blocks: 364243
No longer blocks: 71520
Depends on: bz-field-security
Blocks: 387240
Attached patch merge (deleted) — Splinter Review
Assignee: create-and-change → timeless
Status: NEW → ASSIGNED
Attachment #275403 - Flags: review?(LpSolit)
Comment on attachment 275403 [details] [diff] [review] merge No, this is another hack we don't want. Any new hardcoded rule is the wrong approach. What we need is a way to add your own rules without altering the main code, e.g. using a code hook in Bug::check_can_change_field.
Attachment #275403 - Flags: review?(LpSolit) → review-
Assignee: timeless → nobody
Status: ASSIGNED → NEW
Depends on: 391059
Blocks: 368042
No longer blocks: 295338
Assignee: nobody → create-and-change
What is the conclusion? I am using v4.4 , Is there any way to prevent from users the ability to edit parameters in bugs?
You need to write code to hook into the hook in Bug::check_can_change_field which is there for this purpose. http://www.bugzilla.org/docs/4.4/en/html/api/Bugzilla/Hook.html#bug_check_can_change_field Gerv
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: