| @@ -9,18 +9,16 @@ | ||
| 9 | 9 | |
| 10 | 10 | class TwoFaHandler |
| 11 | 11 | { |
| 12 | 12 | /** |
| 13 | - * How long an issued 2FA code stays valid, in seconds. Kept as a single | |
| 14 | - * constant so the "valid_till" value, the email copy, and the expiry | |
| 15 | - * check in verify2FaEmailCode() can never drift apart again. | |
| 13 | + * Shared by valid_till, the email copy and the expiry check in verify2FaEmailCode(). | |
| 16 | 14 | */ |
| 17 | 15 | const CODE_TTL_SECONDS = 10 * MINUTE_IN_SECONDS; |
| 18 | 16 | |
| 19 | 17 | public function maybe2FaRedirect($user = null) |
| 20 | 18 | { |
| 21 | - // IP bucket is a generous volumetric backstop (shared office/NAT IPs can have | |
| 22 | - // many unrelated accounts logging in); the account bucket is the primary throttle. | |
| 19 | + // The account bucket is the main throttle. The IP bucket is looser because | |
| 20 | + // offices and NATs share IPs. | |
| 23 | 21 | $ipKey = 'fs_2fa_send_ip_' . wp_hash(Helper::getIp()); |
| 24 | 22 | $accountKey = 'fs_2fa_send_act_' . wp_hash($user->ID); |
| 25 | 23 | |
| 26 | 24 | $ipExceeded = Helper::hitRateLimit($ipKey, 20); |
| @@ -34,10 +32,9 @@ | ||
| 34 | 32 | |
| 35 | 33 | $return = $this->sendAndGet2FaConfirmFormUrl($user, 'both'); |
| 36 | 34 | |
| 37 | 35 | if (!$return) { |
| 38 | - // Fail closed: if the OTP couldn't be issued/persisted, do not let the | |
| 39 | - // caller fall through to a normal (non-2FA) login with the already-verified password. | |
| 36 | + // Fail closed so the caller can't fall through to a password-only login. | |
| 40 | 37 | wp_send_json([ |
| 41 | 38 | 'message' => __('Unable to send verification code. Please try again later.', 'fluent-support') |
| 42 | 39 | ], 500); |
| 43 | 40 | } |
| @@ -66,9 +63,9 @@ | ||
| 66 | 63 | $data = array( |
| 67 | 64 | 'login_hash' => $hash, |
| 68 | 65 | 'user_id' => $user->ID, |
| 69 | 66 | 'status' => 'issued', |
| 70 | - 'ip_address' => isset($_SERVER['HTTP_USER_AGENT']) ? sanitize_text_field(wp_unslash($_SERVER['HTTP_USER_AGENT'])) : '', | |
| 67 | + 'ip_address' => Helper::getIp(), | |
| 71 | 68 | 'use_type' => 'email_2_fa', |
| 72 | 69 | 'user_email' => $user->user_email, |
| 73 | 70 | 'two_fa_code_hash' => wp_hash_password($twoFaCode), |
| 74 | 71 | 'valid_till' => gmdate('Y-m-d H:i:s', current_time('timestamp') + self::CODE_TTL_SECONDS), |
| @@ -76,10 +73,9 @@ | ||
| 76 | 73 | 'updated_at' => current_time('mysql'), |
| 77 | 74 | 'used_count' => 0 |
| 78 | 75 | ); |
| 79 | 76 | |
| 80 | - // Only one outstanding 2FA challenge per account: invalidate any previous | |
| 81 | - // issued codes before minting a new one, instead of letting them pile up. | |
| 77 | + // One outstanding challenge per account. | |
| 82 | 78 | Meta::where('object_type', 'fs_2fa') |
| 83 | 79 | ->where('object_id', $user->ID) |
| 84 | 80 | ->delete(); |
| 85 | 81 | |
| @@ -125,9 +121,9 @@ | ||
| 125 | 121 | 'message' => __('Too many verification attempts. Please try again after 15 minutes.', 'fluent-support') |
| 126 | 122 | ], 429); |
| 127 | 123 | } |
| 128 | 124 | |
| 129 | - $logHashMeta = Meta::where('key', $hash)->first(); | |
| 125 | + $logHashMeta = Meta::where('object_type', 'fs_2fa')->where('key', $hash)->first(); | |
| 130 | 126 | |
| 131 | 127 | if (!$logHashMeta) { |
| 132 | 128 | wp_send_json([ |
| 133 | 129 | 'message' => __('Your provided code or url is not valid', 'fluent-support') |
| @@ -148,13 +144,10 @@ | ||
| 148 | 144 | 'message' => __('Too many verification attempts. Please try again after 15 minutes.', 'fluent-support') |
| 149 | 145 | ], 429); |
| 150 | 146 | } |
| 151 | 147 | |
| 152 | - // created_at is a current_time('mysql') string (gmdate() on a current_time('timestamp') | |
| 153 | - // basis, always UTC-equivalent since WP resets the runtime timezone to UTC on every | |
| 154 | - // request), so it must be parsed back as UTC here - otherwise strtotime() would silently | |
| 155 | - // reinterpret it under whatever timezone a plugin/theme may have switched to via | |
| 156 | - // date_default_timezone_set() without restoring it, causing false expiry (or non-expiry) | |
| 148 | + // created_at and current_time('timestamp') share WP's site-offset clock. Parse it as | |
| 149 | + // UTC so a plugin changing PHP's default timezone can't shift the expiry. | |
| 157 | 150 | $createdAt = $logHash['created_at'] ?? ''; |
| 158 | 151 | if (($createdAt && strtotime($createdAt . ' UTC') < current_time('timestamp') - self::CODE_TTL_SECONDS) || ($logHash['used_count'] ?? 0) > 5 || ($logHash['status'] ?? '') != 'issued') { |
| 159 | 152 | wp_send_json([ |
| 160 | 153 | 'message' => __('Sorry, your login code has been expired. Please try to login again', 'fluent-support') |
| @@ -164,11 +157,10 @@ | ||
| 164 | 157 | if (!wp_check_password($code, $logHash['two_fa_code_hash'])) { |
| 165 | 158 | |
| 166 | 159 | $logHash['used_count'] += 1; |
| 167 | 160 | |
| 168 | - // Atomic conditional update: only applies if the row hasn't changed since we read it, | |
| 169 | - // preventing concurrent requests from racing past the attempt limit. | |
| 170 | - Meta::where('key', $hash)->where('value', $logHashMeta->value)->update([ | |
| 161 | + // Compare-and-set on the old value so concurrent guesses can't race past the attempt limit. | |
| 162 | + Meta::where('object_type', 'fs_2fa')->where('key', $hash)->where('value', $logHashMeta->value)->update([ | |
| 171 | 163 | 'value' => maybe_serialize($logHash) |
| 172 | 164 | ]); |
| 173 | 165 | |
| 174 | 166 | wp_send_json([ |
| @@ -174,12 +166,11 @@ | ||
| 174 | 166 | wp_send_json([ |
| 175 | 167 | 'message' => __('Invalid verification code', 'fluent-support') |
| 176 | 168 | ], 423); |
| 177 | 169 | } |
| 178 | - // Consume the code atomically before logging in: only one concurrent request | |
| 179 | - // can win this update, so only one can ever log in with this code. | |
| 170 | + // Compare-and-set consume, so only one concurrent request can log in with this code. | |
| 180 | 171 | $logHash['status'] = 'used'; |
| 181 | - $consumed = Meta::where('key', $hash)->where('value', $logHashMeta->value)->update([ | |
| 172 | + $consumed = Meta::where('object_type', 'fs_2fa')->where('key', $hash)->where('value', $logHashMeta->value)->update([ | |
| 182 | 173 | 'value' => maybe_serialize($logHash) |
| 183 | 174 | ]); |
| 184 | 175 | |
| 185 | 176 | if (!$consumed) { |
| @@ -187,11 +178,14 @@ | ||
| 187 | 178 | 'message' => __('Your provided code or url is not valid', 'fluent-support') |
| 188 | 179 | ], 423); |
| 189 | 180 | } |
| 190 | 181 | |
| 191 | - $user = get_user_by('email', $logHash['user_email']); | |
| 182 | + // Log in the account the code was issued to, and only while it still owns the | |
| 183 | + // email the code went to. An email lookup alone would follow the address to | |
| 184 | + // whichever account holds it now. | |
| 185 | + $user = get_user_by('ID', (int) ($logHash['user_id'] ?? 0)); | |
| 192 | 186 | |
| 193 | - if (!$user) { | |
| 187 | + if (!$user || strcasecmp((string) $user->user_email, (string) ($logHash['user_email'] ?? '')) !== 0) { | |
| 194 | 188 | wp_send_json([ |
| 195 | 189 | 'message' => __('Your provided code or url is not valid', 'fluent-support') |
| 196 | 190 | ], 423); |
| 197 | 191 | } |
