refactor: update default value $hashCost to 12

[datamweb]

Description
PHP is increasing the default bcrypt cost to either 11 or 12 to keep up with increases in computing, so we should do the same within Shield. The current default of 10 was set in PHP 11 years ago, which is no longer a suitable default.

12 appears to be the sweet spot between performance and security, as confirmed by a member of the Hashcat team. Symfony uses a cost of 13, however that may be too high for some servers. Laravel uses a cost of 12.

Due to the way hashing works, there are no BC issues - older passwords with lower rounds will still be handled properly, and code that automatically rehashes passwords will upgrade them over time. The RFC contains hash calculation timings if you'd like more information on the impacts.

Now, due to the increase in hardware speed, using 12 is more suitable for the Shield.

More info see.

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[kenjis]

Explain what you have changed, and why.

Please write it. We know what you did from git commits, but it is difficult to know why.

[datamweb]

Update PR Description.

[datamweb]

The execution time of the tests is very high, it seems that the main part of this issue is related to the public int $hashCost = 12;.

@lonnieezell in #PR 284 changed as follows.

-public int $hashCost = 12;
+public int $hashCost = ENVIRONMENT === 'testing' ? 4 : 12;

Interesting result on my PC. You will definitely get different results according to different systems.

Before:

Time: 01:26.330, Memory: 62.00 MB

Nexus\PHPUnit\Extension\Tachycardia identified these 30 slow tests:
+------------------------------------------------------------------------------------------+---------------+-------------+
| Test Case                                                                                | Time Consumed | Time Limit  |
+------------------------------------------------------------------------------------------+---------------+-------------+
| Tests\\Controllers\\RegisterTest::testRegisteredAndSessionExpiredAndLogin                | 00:00:01.73   | 00:00:00.50 |
| Tests\\Controllers\\RegisterTest::testRegisterActionSuccessWithNoEmailLogin              | 00:00:01.52   | 00:00:00.50 |
| Tests\\Controllers\\RegisterTest::testRegisteredButNotActivatedAndLogin                  | 00:00:01.50   | 00:00:00.50 |
| Tests\\Controllers\\RegisterTest::testRegisteredButNotActivatedAndRegisterAgain          | 00:00:01.49   | 00:00:00.50 |
| Tests\\Controllers\\RegisterTest::testRegisterRedirectsToActionIfDefined                 | 00:00:01.49   | 00:00:00.50 |
| Tests\\Controllers\\RegisterTest::testRegisterActionSuccess                              | 00:00:01.49   | 00:00:00.50 |
| Tests\\Unit\\PasswordsTest::testHash                                                     | 00:00:01.13   | 00:00:00.50 |
| Tests\\Commands\\SetupTest::testRunEmailConfigIsFine                                     | 00:00:01.13   | 00:00:00.50 |
| Tests\\Controllers\\RegisterTest::testRegisterActionWithBadEmailValue                    | 00:00:01.06   | 00:00:00.50 |
| Tests\\Unit\\DictionaryValidatorTest::testCheckTrueOnNotFound                            | 00:00:00.92   | 00:00:00.50 |
| Tests\\Authentication\\ForcePasswordResetTest::testForceGlobalPasswordReset              | 00:00:00.78   | 00:00:00.50 |
| Tests\\Unit\\UserTest::testUpdatePassword                                                | 00:00:00.76   | 00:00:00.50 |
| Tests\\Commands\\UserTest::testPassword                                                  | 00:00:00.72   | 00:00:00.50 |
| Tests\\Unit\\UserIdentityModelTest::testForceMultiplePasswordReset                       | 00:00:00.72   | 00:00:00.50 |
| Tests\\Commands\\UserTest::testList                                                      | 00:00:00.71   | 00:00:00.50 |
| Tests\\Commands\\UserTest::testPasswordWithoutOptionsAndSpecifyEmail                     | 00:00:00.71   | 00:00:00.50 |
| Tests\\Commands\\UserTest::testListByEmail                                               | 00:00:00.69   | 00:00:00.50 |
| Tests\\Commands\\UserTest::testCreate                                                    | 00:00:00.69   | 00:00:00.50 |
| Tests\\Commands\\HmacTest::testDecrypt                                                   | 00:00:00.59   | 00:00:00.50 |
| Tests\\Commands\\UserTest::testRemovegroupCancel                                         | 00:00:00.57   | 00:00:00.50 |
| Tests\\Controllers\\LoginTest::testAfterLoggedInNotDisplayLoginPage                      | 00:00:00.56   | 00:00:00.50 |
| Tests\\Controllers\\MagicLinkTest::testAfterLoggedInNotAllowDisplayMagicLink             | 00:00:00.56   | 00:00:00.50 |
| Tests\\Controllers\\LoginTest::testLoginActionEmailSuccess                               | 00:00:00.55   | 00:00:00.50 |
| Tests\\Controllers\\LoginTest::testLoginActionUsernameSuccess                            | 00:00:00.55   | 00:00:00.50 |
| Tests\\Controllers\\LoginTest::testLoginRedirectsToActionIfDefined                       | 00:00:00.55   | 00:00:00.50 |
| Tests\\Commands\\UserTest::testDeactivate                                                | 00:00:00.55   | 00:00:00.50 |
| Tests\\Authentication\\Authenticators\\SessionAuthenticatorTest::testAttemptUsernameOnly | 00:00:00.54   | 00:00:00.50 |
| Tests\\Authentication\\Authenticators\\SessionAuthenticatorTest::testAttemptCustomField  | 00:00:00.54   | 00:00:00.50 |
| Tests\\Authentication\\Authenticators\\SessionAuthenticatorTest::testAttemptSuccess      | 00:00:00.53   | 00:00:00.50 |
| Tests\\Unit\\UserModelTest::testSaveUpdateUserObjectWithoutUserDataToUpdate              | 00:00:00.53   | 00:00:00.50 |
+------------------------------------------------------------------------------------------+---------------+-------------+
...and 20 more tests hidden from view.


Time: 01:26.330, Memory: 62.00 MB

OK (504 tests, 1086 assertions)

After:

Time: 00:44.995, Memory: 60.00 MB

Nexus\PHPUnit\Extension\Tachycardia identified these 10 slow tests:
+---------------------------------------------------------------------------------+---------------+-------------+
| Test Case                                                                       | Time Consumed | Time Limit  |
+---------------------------------------------------------------------------------+---------------+-------------+
| Tests\\Controllers\\RegisterTest::testRegisteredButNotActivatedAndLogin         | 00:00:01.01   | 00:00:00.50 |
| Tests\\Controllers\\RegisterTest::testRegisteredAndSessionExpiredAndLogin       | 00:00:01.00   | 00:00:00.50 |
| Tests\\Controllers\\RegisterTest::testRegisteredButNotActivatedAndRegisterAgain | 00:00:00.98   | 00:00:00.50 |
| Tests\\Controllers\\RegisterTest::testRegisterRedirectsToActionIfDefined        | 00:00:00.98   | 00:00:00.50 |
| Tests\\Controllers\\RegisterTest::testRegisterActionSuccess                     | 00:00:00.98   | 00:00:00.50 |
| Tests\\Controllers\\RegisterTest::testRegisterActionSuccessWithNoEmailLogin     | 00:00:00.97   | 00:00:00.50 |
| Tests\\Controllers\\RegisterTest::testRegisterActionWithBadEmailValue           | 00:00:00.96   | 00:00:00.50 |
| Tests\\Unit\\PasswordsTest::testHash                                            | 00:00:00.87   | 00:00:00.50 |
| Tests\\Unit\\DictionaryValidatorTest::testCheckTrueOnNotFound                   | 00:00:00.87   | 00:00:00.50 |
| Tests\\Unit\\UserIdentityModelTest::testForceMultiplePasswordReset              | 00:00:00.63   | 00:00:00.50 |
+---------------------------------------------------------------------------------+---------------+-------------+


Time: 00:44.995, Memory: 60.00 MB

I'd love to see this change applied to the Shield, but I'm concerned that setting a value of public int $hashCost = ENVIRONMENT === 'testing' ? 4 : 12; will seem a bit complicated for users.

@kenjis What do you think?

[lonnieezell]

We could probably do this internally so the user doesn't have to think about it. I did it that way as a test in the forums because they were running soooooo slowly and there wasn't any real need to.

It might be possible to find another way where we can do away with hash costs altogether in testing, but I'm afraid that might be challenging and (maybe?) limit some other's testing abilities. Unsure exactly how that would work at the moment.

[datamweb]

It might be possible to find another way where we can do away with hash costs altogether in testing, but I'm afraid that might be challenging and (maybe?)

How about the change phpunit.xml.dist?

<env name="auth.hashCost" value="4"/>

[lonnieezell]

doh! That's perfect for Shield. That wouldn't take for user's apps, but we could add a note to the testing section of the docs about that change for their apps.

[kenjis]

Nexus\PHPUnit\Extension\Tachycardia identified these 30 slow tests:
...

Time: 00:53.530, Memory: 50.00 MB

OK (504 tests, 1086 assertions)

Added <env name="auth.hashCost" value="4"/>:

Nexus\PHPUnit\Extension\Tachycardia identified these 10 slow tests:
...

Time: 00:22.088, Memory: 50.00 MB

OK (504 tests, 1086 assertions)

[datamweb]

All thanks, I sent #1041.