Open
Bug 90619
Opened 23 years ago
Updated 8 years ago
Restriction of user rights
Categories
(Bugzilla :: Creating/Changing Bugs, defect, P3)
Tracking
()
NEW
People
(Reporter: afrey, Unassigned)
References
(Depends on 2 open bugs, Blocks 3 open bugs)
Details
(Whiteboard: [permissions:edit])
Attachments
(1 file, 4 obsolete files)
(deleted),
patch
|
LpSolit
:
review-
|
Details | Diff | Splinter Review |
Is there any way to restrict the access rights of a user, so that he can only
submit bugs and edit the comments field?
Comment 1•23 years ago
|
||
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
Comment 2•23 years ago
|
||
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
Updated•23 years ago
|
Priority: -- → P3
Comment 3•23 years ago
|
||
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?
Comment 4•23 years ago
|
||
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)
Comment 5•23 years ago
|
||
I've tried this, but it doesn't work - for some reason the reporter still can
resolve and close the bug...
Comment 6•23 years ago
|
||
-> 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
Comment 7•23 years ago
|
||
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
Updated•21 years ago
|
OS: Linux → All
Hardware: PC → All
Comment 10•21 years ago
|
||
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
Comment 11•20 years ago
|
||
(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.
Comment 12•20 years ago
|
||
(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!
Comment 13•20 years ago
|
||
(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?
Comment 14•20 years ago
|
||
(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-
Comment 15•20 years ago
|
||
(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.
Comment 16•20 years ago
|
||
(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?
Comment 17•20 years ago
|
||
Following justdave's suggestion, I put all patches from my comment 16 in a
single text file.
Updated•20 years ago
|
Attachment #158052 -
Attachment is patch: true
Attachment #158052 -
Flags: review?
Comment 18•20 years ago
|
||
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!
Updated•20 years ago
|
Attachment #158052 -
Attachment is obsolete: true
Comment 19•20 years ago
|
||
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 20•20 years ago
|
||
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?
Comment 21•20 years ago
|
||
(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 22•20 years ago
|
||
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-
Comment 23•20 years ago
|
||
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
Comment 24•20 years ago
|
||
Per justdave's comments: use the new param name "allowreportertoresolve" and
give a better param description.
Attachment #158090 -
Attachment is obsolete: true
Comment 25•20 years ago
|
||
(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.
Updated•20 years ago
|
Attachment #158679 -
Flags: review?(justdave)
Comment 26•20 years ago
|
||
> 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 27•20 years ago
|
||
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)
Comment 28•20 years ago
|
||
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?
Updated•20 years ago
|
Attachment #158679 -
Attachment is obsolete: true
Comment 29•20 years ago
|
||
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 30•20 years ago
|
||
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)
Comment 31•20 years ago
|
||
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
Comment 32•20 years ago
|
||
I think we should prevent that as well:
https://bugzilla.mozilla.org/show_activity.cgi?id=274514
Updated•20 years ago
|
Target Milestone: Bugzilla 2.20 → Bugzilla 2.22
Comment 33•19 years ago
|
||
I won't have time to play with it before the 2.22 freeze
Target Milestone: Bugzilla 2.22 → ---
Comment 34•19 years ago
|
||
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
Comment 35•19 years ago
|
||
*** Bug 318658 has been marked as a duplicate of this bug. ***
Comment 36•18 years ago
|
||
*** Bug 361514 has been marked as a duplicate of this bug. ***
Comment 37•18 years ago
|
||
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
Updated•18 years ago
|
Depends on: bz-field-security
Comment 38•17 years ago
|
||
Assignee: create-and-change → timeless
Status: NEW → ASSIGNED
Attachment #275403 -
Flags: review?(LpSolit)
Comment 39•17 years ago
|
||
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-
Updated•15 years ago
|
Assignee: nobody → create-and-change
Comment 41•11 years ago
|
||
What is the conclusion? I am using v4.4 , Is there any way to prevent from users the ability to edit parameters in bugs?
Comment 42•11 years ago
|
||
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
No longer blocks: 81457
You need to log in
before you can comment on or make changes to this bug.
Description
•