191192 - no-authority user cannot reassign an issue that he owns

Status: RESOLVED WONTFIX

(Bugzilla :: Administration, task)

Description

[Casey Daniell]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1)
Build Identifier: 

When a no-authority user attempts to reassign an issues that he owns bugzilla 
seems to want to change the status to NEW, but since the user doesn't have
the authority to confirm the issue, it won't let him reassign it.

Similarly, i've just noticed that if the user DOES have the authority to
edit/confirm issues, an unconfirmed issue's status is changed to NEW upon
reassigning it to someone else (even though everconfirmed is still 0).  I
believe that the status should remain UNCONFIRMED.


Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Dave Miller [:justdave]]

I'm pretty sure this is a dupe, but I don't remember where the original is, and
can't find it in my quick glance.  If nobody beats me to it I'll come back and
do a more thorough search later when I have time.  This may already be fixed,
I'm not sure.  I remember bbaetz and Gerv had a big argument over how to fix it. :-)

Comment 2

[Bradley Baetz (:bbaetz)]

I don't know about the first one being a dupe. IS this CVS bugzilla, or an older
rlease? If CVS, that could be an interaction with that otehr bug I can't recall
teh number of ATM.

The cancofirm longic is painful... :)

Comment 3

[Casey Daniell]

This is the 2.16 release of Bugzilla, downloaded off of the downloads area, 
this is not a CVS (nightly) build.  

Comment 4

[Bradley Baetz (:bbaetz)]

Hmm. Hang on a sec. Someone without canedit isn't meant to be able to reassign a
bug (unless they're the reporter/qa/assigneed). If they're not, then the error
message is correct.

Comment 5

[Casey Daniell]

To create this problem you need two users that i'll refer to as dummy and
power.  Assume dummy has no special authority and power has the permission bits
for confirming issues and editing issues set.

1.  set up (or find) users with permissions described as above
2.  log in as power
3.  enter an UNCONFIRMED issue, call it issue 10
4.  assign the issue to dummy (as if needing some question answered)
5.  log out
6.  log in as dummy
7.  add a comment to issue 10 (as if answering the question)
8.  try to reassign the issue back to power
9.  you should see the following error message:
       You tried to change the bug_status field from UNCONFIRMED to NEW, but 
       only the owner or submitter of the issue, or a sufficiently empowered 
       user, may change that field.

Does that help?

Comment 6

[Dave Miller [:justdave]]

So it sounds like process_bug is treating it as if the "And confirm the bug"
checkbox underneath the "reassign to" is checkmarked, even if it isn't...

Comment 7

[Casey Daniell]

Just a clairfication. In my comment 5, the dummy user becomes an assignee. 
Acording to Bradley in comment 4, this should allow the user to then reassign 
the defect/bug to someone else keeping the bug in the UNCONFIRMED state.

Comment 8

[Dave Miller [:justdave]]

a quick glance at the template tells me the "and confirm bug" checkbox is not
being shown to the user if they don't have canconfirm...

here's the relevant process_bug code from 2.16.2:

    /^reassign$/ && CheckonComment( "reassign" ) && do {
        if ($::FORM{'andconfirm'}) {
            DoConfirm();
        }
        ChangeStatus('NEW');

note that ChangeStatus is supposed to deal with setting it to UNCONFIRMED if the
bug hasn't been confirmed and you try to set it to new, like so:

        } elsif (IsOpenedState($str)) {
            $::query .= "bug_status = IF(everconfirmed = 1, '$str',
'$::unconfirmedstate')";

The only way I can figure out that this wouldn't work correctly is if the user's
browser was sending the "andconfirm" field with it blank instead of not sending
it, since we only check if it was sent at all and not the contents of it (the
HTML spec says it's not supposed to be sent at all if the box isn't checked).

Comment 9

[Mark Drew]

Dave, the problem appears to be with this line in sub ChangeStatus of
process_bug.cgi:

        $::FORM{'bug_status'} = $str; # Used later for call to
                                      # CheckCanChangeField to make sure this
                                      # is really kosher.

ChangeStatus is being called with $str = "NEW", so when CheckCanChangeField is
called later (as the comment mentions), I get the message:

   You tried to change the bug_status field from UNCONFIRMED to NEW, but only 
   the owner or submitter of the issue, or a sufficiently empowered user, may 
   change that field.

Which looks like it's coming from the CheckCanChangeField sub.

Comment 10

[Jacob Steenhagen]

Mark, where are you trying to do this?  I just tried it on the tip and it worked
as expected.

Comment 11

[Dave Miller [:justdave]]

Earlier comments indicate he's using 2.16.2

Comment 12

[Bradley Baetz (:bbaetz)]

The ChangeStatus("NEW") stuff is Fun, because there are some IF statemetns
futher on, which test everconfirmed. Its really a mess - I tried tracing through
it at one stage a while back, and I remmebr how convoluted it is.

(side note: I don't think that thats portable with postgres :)

I seem to have hosed my 2.16 install at some point; Ill try to test this later
this afternoon.

Comment 13

[Dave Miller [:justdave]]

I actually made that work on Sybase with a convoluted nested CASE WHEN statement....

            $::query .= "bug_status = " .
                        SQLIf("bug_status IN($open_state)",
                              SQLIf("everconfirmed = 1",
                                    SqlQuote($str),
                                    SqlQuote($::unconfirmedstate) ),
                              "bug_status");

SQLIf() converts to IF($a, $b, $c) in MySQL and to 
CASE WHEN $a THEN $b ELSE $c END in Sybase.  I think Postgres' syntax on CASE
WHEN is the same, actually...

Comment 14

[Bradley Baetz (:bbaetz)]

Yeah, postgres follows ansi. ISTR there being a shorter IF/THEN/ELSE thing
though, (similar to mysql), but I'm not sure.

Comment 15

[Bradley Baetz (:bbaetz)]

OK, I reproed this under 2.16.2.

What happens is that we always set teh bug_status to NEW in the perl, but then
in the SQL we use hte IF construct to work arround this. CheckCanChangeField
doesn't know, and so it errors out.

This affects any bug where everconfirmed has never been set.

This doesn't happen in CVS because we now allow the assignee to confirm the bug.

Since this is fixed in CVS, its a pain to backport, and I can repro this in 2.14
as well (I didn't try older versions), and noone else has reported this in the
past 18 months, I'm tempted to WONTFIX this.

Thoughts?

Comment 16

[Dave Miller [:justdave]]

It's not dataloss, and it's not a security hole, so it doesn't meet the
qualifications for a checkin to a stable branch, anyway.

If we know what patch fixed this, let's mark it a dupe of that bug, or resolve
it fixed and note that the patch on that bug fixed it.

Comment 17

[Bradley Baetz (:bbaetz)]

I'm not sure which bug fixed it. It wasn't actually fixed; it was just made
irrelevent.

We do checkin fixes for large breakages for the branch, but since it went so
long wthout being noticed.... WONTFIX