98818 - Should not require password to change Real Name

Status: RESOLVED WONTFIX

(Bugzilla :: User Accounts, defect, P1)

Description

[Christopher Aillon (sabbatical, not receiving bugmail)]

Recently, you have been required to enter your password to change your real name
in your user account.

This did not occur in the past, and I don't know exactly when it occured:
sometime between 2.12 and 2.14 though.

It does not make sense to ask for a password here but not for more critical
tasks such as tweakparams or editgroups.  Changing a realname leaves no damage
in the database. If someone changes it, you can just change it back. 
Tweakparams and editgroups can leave damage though.  I'm filing a separate bug
about a param to require users to enter a password before tweaking params or
editing groups.

Comment 1

[Christopher Aillon (sabbatical, not receiving bugmail)]

Filed bug 98819 for the global param for requiring passwords on editing global
aspects of bugzilla.

Also setting regression keyword.

Comment 2

[Gervase Markham [:gerv]]

justdave said:

<justdave> re: password to change names, that may be caused by the patch to
prevent it from making you authenticate twice when you change your password.
<justdave> when you change you password, it makes you enter the old password and
your new password twice on the form...
<justdave> after doing that, if you have cookies off, it makes you authenticate
again.
<justdave> the catch is, you have to authenticate with your OLD password, not
the new one
<justdave> once you do that, it resets your password, and then you have to
authenticate AGAIN because your cookie is no longer valid because the password
doesn't match
<justdave> that was solved by renaming the old password field to
Bugzilla_password and embedding your login name as Bugzilla_login in a hidden
element on the page.
<justdave> this then makes the cgi think you just logged in when you submit the
form and it doesn't make you authenticate again.
<justdave> unfortunately, since the name change is on the same form, if all you
want to do is change your name, if you leave the old password blank, it thinks
you just tried to log in without a password
<justdave> and it'll tell you you had an invalid login, and press back to try again.
<justdave> I think the solution here will be to put the password change on a
pane all by itself
<justdave> move the name change to a different panel.
<justdave> (probably the same one that lets you change your email address, which
we're adding in the near future)
<justdave> the same trick could be used on the params and groups pages to force
you to log in...
<justdave> put a hidden Bugzilla_login in the form, and make a password field
with Bugzilla_password
<justdave> just answer no to all the optional stuff
<justdave> I'm about to hit the sack here... and I don't have an easy way to
copy the above from XWindows to Aqua to get it to my browser.
<justdave> would someone mind pasting the above into the bug caillon just posted?
<justdave> (about the password to change your name)

Comment 3

[Christopher Aillon (sabbatical, not receiving bugmail)]

Just for reference, this was caused by the fix to bug 45918, checked in on Jun
19 15:02.  Changing the version to 2.13 to reflect when this problem first occured.

Comment 4

[Matthew Tuck [:CodeMachine]]

Can't it just interpret a blank password as no change?

Comment 5

[Dave Miller [:justdave]]

No, because it's not trying to change the password at that point, it's trying to
log you in.  We'd have to change confirm_login() to accept a blank password as
valid, and that's not a good idea. :)

Comment 6

[Jacob Steenhagen]

Actually, we should be able to fix this by calling quietly_check_login() at the
top of the confirm_login() sub, rather than in the middle of it.  Then, if we
have a valid login already, we'll just return true.  If not, we'll start running
through the part where it checks if the bugzilla_login/password fields are present.

Comment 7

[Dave Miller [:justdave]]

Hmmm....  is there any reason that confirm_login doesn't call
quietly_check_login first before looking for form values?  It calls
quietly_check_login to get the cookie if form fields aren't present.  Maybe we
should just fix confirm_login.  Although I suppose this would screw up people
who have more than one account and use bookmarks to switch between them without
logging off first...

Comment 8

[Christopher Aillon (sabbatical, not receiving bugmail)]

> Although I suppose this would screw up people who 
> have more than one account and use bookmarks to 
> switch between them without logging off first...

How many people actually do this?  I have two accounts but I use different
profiles to go between them.  Yes, at the moment it is a pain to switch between
them.  But have a look at bug 97622....

Comment 9

[Matthew Tuck [:CodeMachine]]

I think separation makes sense in any case.  We're going to want to add email
address to real name.

Comment 10

[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 11

[Christopher Aillon (sabbatical, not receiving bugmail)]

Regression.  Dave OK'd moving this back to 2.16.

The solution will be to move the Real Name stuff to wherever the "Change Email
Account" stuff is (bug 23067).  Adding dependency and re-assigning to dave since
he is working on 23067 - both of these most likely should get done at the same time.

Comment 12

[Jaime Rodriguez, Jr.]

Adding nsbeta1 keyword to all regressions so they *get some love* and attention.

Comment 13

[Dave Miller [:justdave]]

Bugzilla bugs are by definition not nsbeta1 :-)

Comment 14

[Jaime Rodriguez, Jr.]

doh! i may have been over zealous in my desire to capture all regressions. :-)

Comment 15

[Gervase Markham [:gerv]]

This is fixed in the userprefs.cgi templatisation patch over in bug 117060.

Gerv

Comment 16

[Dave Miller [:justdave]]

Bug 117060 is in.  Was decided not to make this fix part of that bug.

After re-reviewing the circumstances, I think the best option is probably to
move the name (and probably email) to its own tab.  That would probably satisfy
bug 128158 as well.

Comment 17

[Bradley Baetz (:bbaetz)]

If we're not going to fix this for 2.16 (which seems likely, given comments),
can we at least change it so that you don't need to change your pw to change
your real name?

ie change:

    if ($::FORM{'Bugzilla_password'} ne "" || 
        $::FORM{'new_password1'} ne "" || 
        $::FORM{'new_password2'} ne "") 
to:

    if ($::FORM{'Bugzilla_password'} ne "" && 
        ($::FORM{'new_password1'} ne "" || 
         $::FORM{'new_password2'} ne "")) 

This will then silently fail if someone wants to change their password to the
empty string, though.

Comment 18

[Dave Miller [:justdave]]

If Bugzilla_password exists, confirm_login() will cut you off before you ever
make it to that code.

Comment 19

[Bradley Baetz (:bbaetz)]

Oops - forgot that.

No patch, -> 2.18

Comment 20

[Christopher Aillon (sabbatical, not receiving bugmail)]

Bah, if it needs a patch to be considered, then since nobody is stepping up, I
will.  This regression sucks.  There's no way we should ship without it IMO.

->Back to 2.16

Comment 21

[Dave Miller [:justdave]]

Attachment: screenshot from userprefs.cgi with the patch from bug 23067 applied

Please wait until after bug 23067 is checked in before you touch this. 
Personally I like the way it looks with the patch on bug 23067 applied.  I'm
tempted to WONTFIX this with that in mind (but if people really care that much
maybe we can do something about it anyway).  Check out the screenshot I just
attached.

Comment 22

[Bradley Baetz (:bbaetz)]

I'm happy to wontfix this - the new interface is clear, really.

If we want to discuss whether changing the real name should require the passwd
at all, then I don't think that that is a 2.16 bug, unless someone has a patch.

Comment 23

[Gervase Markham [:gerv]]

3's a quorum.

Gerv

Comment 24

[Dave Miller [:justdave]]

removing target on DUPL/WONTFIX (so they'll show up as untriaged if reopened)

Comment 25

[Peter Hozak]

_reopen suggestion_

I am not able to change my Real Name since I created this account via Persona and I don't have any bugzilla password.

Comment 26

[Peter Hozak]

(In reply to Peter Hozak from comment #25)
> _reopen suggestion_
> 
> I am not able to change my Real Name since I created this account via
> Persona and I don't have any bugzilla password.

huh.. looks like it did change my Real Name, even if it complained that old password does not match.. nevermind