Changeset View
Changeset View
Standalone View
Standalone View
src/applications/auth/factor/PhabricatorTOTPAuthFactor.php
| Show First 20 Lines • Show All 179 Lines • ▼ Show 20 Lines | if (self::verifyTOTPCode($viewer, $key, $code)) { | ||||
| 'value' => $code, | 'value' => $code, | ||||
| 'valid' => false, | 'valid' => false, | ||||
| ); | ); | ||||
| } | } | ||||
| } | } | ||||
| public static function generateNewTOTPKey() { | public static function generateNewTOTPKey() { | ||||
| return strtoupper(Filesystem::readRandomCharacters(16)); | return strtoupper(Filesystem::readRandomCharacters(32)); | ||||
amckinley: Just for giggles, I checked that `auth_factorconfig.factorSecret` was compatible with this… | |||||
| } | } | ||||
| public static function verifyTOTPCode( | public static function verifyTOTPCode( | ||||
| PhabricatorUser $user, | PhabricatorUser $user, | ||||
| PhutilOpaqueEnvelope $key, | PhutilOpaqueEnvelope $key, | ||||
| $code) { | $code) { | ||||
| $now = (int)(time() / 30); | $now = (int)(time() / 30); | ||||
| ▲ Show 20 Lines • Show All 110 Lines • Show Last 20 Lines | |||||
Just for giggles, I checked that auth_factorconfig.factorSecret was compatible with this change, and yeah, it's a LONGTEXT. I also don't think it's worth migrating existing secrets or throwing a setup warning for installs with the old "less secure" TOTP secrets, but maybe we should call out this change in particular in the changelog?