[#12267] - Joomla 4.0: Removing deprecated code from JUser* classes
- Fixed in Code Base
- 7 Oct 2016
- Medium
- Build: 4.0-dev
- # 12267
- Diff
- Hackwar:j4userhelper
- Success continuous-integration/drone the build was successful Details
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
User tests: Successful: Unsuccessful:
This PR removes deprecated code from the JUser* classes. This also removes the SHA256 password encryption!
This PR has 3 debateable changes:
- The SHA256 password encryption gets removed. That encryption was introduced in Joomla 3.2.0 and deprecated in 3.2.1. This means that passwords encrypted under 3.2.0 and whose users have since NOT logged in again, will not be verifyable in Joomla 4.0 and onwards. Considering that 3.2.0 was released in 11/2013 (3 years ago) and 3.2.1 was released in 12/2013 and fixed the issue then, I would argue that users with those old hashing can be omitted. They still can reactivate their accounts by requesting to reset their password. To make this clear: This will only affect users that have created their account in 3.2.0 and have NOT logged in since then or where the site was not updated to a later Joomla version. Considering that the login was broken for those people anyway, I can't see a site that stuck to that version. If a user had an outdated password hash, it automatically gets updated upon next login.
- The method JUserHelper::_bin is not deprecated, but I'm removing it anyway. The method is set to private and is only used in the getCryptedPassword method, which gets removed with this PR. No other code can user this method, so this should be save, even though we didn't deprecate it.
- The method JUserHelper::_toAPRMD5() is in a similar situation. Again only used in JUserHelper::getCryptedPassword() and set to protected, so it is not really a part of the public API. However, by being protected and not private, a class extending JUserHelper could theoretically use it.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Labels |
Added:
?
?
|
||
| Category | ⇒ | SQL Administration Components Libraries Unit Tests |
OK I'm willing to hear sides here BUT I'm unconvinced with the sha stuff. We need to do one of two things
- Keep the SHA verification only (obviously we never need to generate these PW's)
- Automatically go through the database and check for users with sha passwords and enable the password reset required flag.
I know it sucks like hell. But we can't leave the users on upgrade in a state of limbo...
Since I don't know how to properly update the composer stuff to remove the class from @ircmaxell, I'm leaving that to someone else.
Leave it for now. Until we start getting other dependencies bumped up (i.e. the Registry package to Framework v2 branch) the polyfill is still going to be there (the Registry v1 package requires Symfony's PHP 5.5 polyfill which requires the password_compat package).
@wilsonge 3.2.0 was available for one month and it was unusable for several reasons. Among other things, it couldn't verify the passwords that it generated itself. The number of users that were affected was small. We are practically at the point where the only accounts still having this password are those, where a normal visitor registered on the site and has since NEVER come back. 3.2.0 didn't only break the password system and the authentication system, it broke several other things, too. I'm close to betting money that there is no website out there still running 3.2.0, because it was so broken. I would strongly suggest to simply drop SHA256 and be done with it.
I understand. But iirc it was largely bcrypt passwords that had partial backfills in some old linux versions that were broken though. Not the sha/md5 or the 'correctly implemented' bcrypt support.
I don't particularly mind dropping sha support. But if we do we need to force all users who have sha passwords into a password reset (some sort of function in the script.php file). It shouldn't be upto site administrators to have to manually review the database on their own...
Would the SQL query be enough?
Looks ok to me. Needs a quick test tho to be sure
Test ok
- Keep the SHA verification only (obviously we never need to generate these PW's)
- Automatically go through the database and check for users with sha passwords and enable the password reset required flag.
I know it sucks like hell. But we can't leave the users on upgrade in a state of limbo...
I know that we have the .sql file with all database changes but some document with developers anotations could be very helpful for future upgrade extensions.
We need the update query also for postgres
FWIW I discussed with Hannes last night and this simplistic approach doesn't work as password reset requires validating passwords. The only option is on upgrade to do email reset of passwords :( There is no other way that would work other than allowing SHA's forever
So, with just 5 commits,
| Status | Pending | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-10-07 00:42:58 |
| Closed_By | ⇒ | wilsonge |
| Labels |
Added:
?
|
||
I'm happy now. Well not happy because I think we still need to form a plan for removing SHA support. But all the other changes are good.
I've also removed JCrypt::hasStrongPasswordSupport(), since our minimum requirement is PHP 5.5.0 and that has this feature built in. Since I don't know how to properly update the composer stuff to remove the class from @ircmaxell, I'm leaving that to someone else.