337026 - Users with no privs cannot edit bugs anymore (if being logged out first)

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[Frédéric Buclin]

A powerless user (= without editbugs privs) who has no role related to some given bug and who is logged out cannot edit the bug anymore (after entering his username and password), because show_bug.cgi displays the resolution field by default and CheckCanChangeField() no longer ignores the resolution since bug 92515 is fixed. 

The problem is that process_bug.cgi passes *all* fields to CheckCanChangeField() for validation, even those which will be ignored later (such as the resolution when the user doesn't try to change it). We should let ChangeResolution() do this check itself.

Comment 1

[Frédéric Buclin]

Attachment: patch, v1

Comment 2

[Dave Miller [:justdave]]

Comment on attachment 221229 [details] [diff] [review]
patch, v1

as I mentioned earlier on IRC I think it'd be good to get it to be able to figure out what you were changing from and to that you weren't allowed to during a change multiple, and the "user has privs if they get here" comment isn't accurate.

The first can be dealt with later, this fixes the regression at least.  You should fix the comment on checkin though.

Comment 3

[Dave Miller [:justdave]]

Oh, something else I just thought of....

We need to be careful that we're not throwing errors in the middle of a change-multiple.  If anything in a change-multiple is going to cause an error we need to throw the error ahead of time before we change anything in the list.  I'm not quite clear from the flow here if that's the case here or not, but it looks like we're still safe from my guess.  Someone who's looked at the code more than me recently should double-check though.

Comment 4

[Frédéric Buclin]

Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.317; previous revision: 1.316
done

Comment 5

[Marc Schumann [:Wurblzap]]

Does this take product-specific editbugs privs by group access controls into account?

Comment 6

[Frédéric Buclin]

(In reply to comment #5)
> Does this take product-specific editbugs privs by group access controls into
> account?
> 

Yup, the first thing process_bug.cgi does is to call Bugzilla::Bug::ValidateBugID(), which makes sure you can access the bug. And later in the code, it makes sure you can edit bugs in the given product (the CANEDIT bit you can find when viewing product-group settings). CheckCanChangeField() doesn't care about group access controls. Its job is to make sure you have enough privs to edit some given fields the bug (i.e. either you have editbugs privs or you are the reporter/assignee/QA contact).