97662 - Bugzilla allows bugs to be assigned to disabled users

Status: RESOLVED WONTFIX

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

Description

[seph]

From Bugzilla Helper:
User-Agent: Mozilla/4.75 [en] (X11; U; Linux 2.2.17 i686)
BuildID:    cvs20010709

deleting users is generally a bad idea, since old bug reports referance them, so
there's this handy disable user feature. Unfortunatly I can still assign bugs to
a disabled user, which is horrible.

Reproducible: Always
Steps to Reproduce:
1. create user
2. disable user
3. create new bug assigned to disabled user

Actual Results:  the bug was created assigned to the disabled user

Expected Results:  bugzilla shouldn't have allowed the bug to be assigned.

I'm using the debian package of a cvs checkout, so I might have the build ID
wrong. I don't think this is a bug specific to the debian package.

Comment 1

[Christopher Aillon (sabbatical, not receiving bugmail)]

This is a Bugzilla issue, not a browser one.

Comment 2

[seph]

the bugzilla helper is horrible and confusing and doesn't seem to have a
bugzilla entry. sorry for mis-catagorizing it.

Comment 3

[Dave Miller [:justdave]]

Confirming (only because we don't use UNCONFIRMED in the Bugzilla product)

Comment 4

[Gervase Markham [:gerv]]

Dave: one way to fix this would be to check for disabled-ness in DBname_to_id()
in globals.pl. We would then return a relevant error code and
DBNameToIdAndCheck() could print an error.

However, would this prevent people doing legal things with a disabled username?
For example, would it stop people making changes to bugs on which they were CCed
or QA contact until they removed them?

If we don't fix this globally, we need to come up with the list of things you
can't do for a disabled username, and check for all of them explicitly. :-(

Gerv

Comment 5

[Dave Miller [:justdave]]

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.

Comment 6

[Gervase Markham [:gerv]]

I guess the approach here is to get a User object to expose disabledtext, and
then check it in the cases you don't want disabled users used.

What are those cases? Any time that user is put on a bug (assignee, QA, CC)?

Gerv

Comment 7

[Gervase Markham [:gerv]]

Note: the User object will expose disabledtext when my email table changes get
checked in (bug 73665).

Gerv

Comment 8

[Brendan Leber]

(In reply to comment #6)

> What are those cases? Any time that user is put on a bug (assignee, QA, CC)?

My thought is exactly that.  Prevent a disabled user from becoming the 
assignee, QA contact or being added to the CC list.  Also, sanitycheck.cgi 
should warn when any open defect has a disabled user in any of those three 
fields.

Comment 9

[Dave Miller [:justdave]]

Pushing out bugs that aren't blockers.  If someone's working on one of these, we
can move it back.

Comment 10

[Dave Miller [:justdave]]

*** Bug 252128 has been marked as a duplicate of this bug. ***

Comment 11

[timeless]

note that [e-mail] is disabled. simply enforcing this will mess bmo up
pretty badly.

Comment 12

[Jouni Heikniemi]

*** Bug 148079 has been marked as a duplicate of this bug. ***

Comment 13

[Max Kanat-Alexander]

This bug has not been touched by its owner in over six months, even though it is
targeted to 2.20, for which the freeze is 10 days away. Unsetting the target
milestone, on the assumption that nobody is actually working on it or has any
plans to soon.

If you are the owner, and you plan to work on the bug, please give it a real
target milestone. If you are the owner, and you do *not* plan to work on it,
please reassign it to [e-mail] or a .bugs component owner. If you are
*anybody*, and you get this comment, and *you* plan to work on the bug, please
reassign it to yourself if you have the ability.

Comment 14

[Paul]

Attachment: patch to check the user is not disabled according to the parameter of the function match()

$exclude_disabled was not being respected on single user instances.

Comment 15

[timeless]

Comment on attachment 204296 [details] [diff] [review]
patch to check the user is not disabled according to the parameter of the function match()

this will break bmo. try again.

Comment 16

[Paul]

(In reply to comment #11)
> note that [e-mail] is disabled. simply enforcing this will mess bmo
> up pretty badly.
 

Because bmo is using a hack to get [e-mail] to work??   Why can't [e-mail] get his disabled text removed?

Comment 17

[Olav Vitters]

Implementing this will break bugzilla.gnome.org as well.

Comment 18

[Max Kanat-Alexander]

As stated in various comments, I think this is something we're not going to fix. "Disabled" means "can't log in." We should probably just make that clearer by making the checkbox say "disable login" or something like that.

Perhaps we could have another checkbox for "this user cannot be added to any new bugs." That does seem to start to get complex, though.