227686 - Move password change code into Bugzilla::Auth (part 2)

Status: NEW

(Bugzilla :: User Accounts, defect)

Description

[Christian Reis]

Second part of cleanup started in bug 226892. 

Password changing is spread out in three different files:

- editusers.cgi
- token.cgi
- userprefs.cgi

This bug is moving this to Bugzilla::Auth->change_password(), a virtual
method implemented in Bugzilla::Auth::DB and ::LDAP (and potentially others in
the future). I've refactored the DB->authenticate() code into separate functions
so we can use them -- we need to add the virtual method(s) now and fix the
callsites to use it/them.

Comment 1

[Christian Reis]

I could try fitting this in 2.18 if bug 226754 is fixed soon.. it's a nice cleanup.

Comment 2

[Christian Reis]

Want to do this as soon as Erik's authentication apocalypse has landed.

Comment 3

[Christian Reis]

And should remove the horrid has_db hack he's propagated.

Comment 4

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

[Justin B]

Attachment: Proposed Patch

Dylan - Per our chat yesterday, I took a look at this and think I have a possible solution. Not sure if this the best way, but my thought process was:

1. Retain the existing "set_password" mutator in User.pm, and have it call...
2. A new "change_password" method in Bugzilla::Auth, which in turn calls...
3. A new "change_password" method in Bugzilla::Auth::Verify::Stack which (if I understand correctly) will go down the list of authentication options until it finds a change password method.

I haven't tested it with my customization yet, just wanted to check in and see if I'm on the right track here...

Comment 6

[Dylan Hardison [:dylan] (he/him)]

Comment on attachment 8831488 [details] [diff] [review]
Proposed Patch

based on second chat, the committer is going to reverse this so that change_password in the verify/db layer calls set password

Comment 7

[Justin B]

Attachment: Proposed Patch

Dylan - Changes made per our conversation. Please advise

Comment 8

[Justin B]

Dylan - Please advise

Comment 9

[Dylan Hardison [:dylan] (he/him)]

I'll get to this today, thanks for the ping.

Comment 10

[Dylan Hardison [:dylan] (he/him)]

I made a lot of progress fixing nits that prevented bugzilla from running, and I've prepared to review this -- so it should be very soon.