| @@ -7,9 +7,9 @@ | ||
| 7 | 7 | * which is how the trusted-device check kept identifying a browser by its |
| 8 | 8 | * User-Agent in two places at once (S1 of the 28 Aug 2026 audit). |
| 9 | 9 | * |
| 10 | 10 | * The using class must provide $this->database (Vigilante_Database), |
| 11 | - * $this->options (the two_factor settings array) and log_event(). | |
| 11 | + * $this->policy() (the two_factor settings array) and log_event(). | |
| 12 | 12 | * |
| 13 | 13 | * @package Vigilante |
| 14 | 14 | * @since 2.11.0 |
| 15 | 15 | */ |
| @@ -24,8 +24,87 @@ | ||
| 24 | 24 | */ |
| 25 | 25 | trait Vigilante_Two_Factor_Session { |
| 26 | 26 | |
| 27 | 27 | /** |
| 28 | + * The second factor mechanics in force, resolved once per request | |
| 29 | + * | |
| 30 | + * The method, the expiry and the grace period come from the main site on a | |
| 31 | + * network, so they are the same wherever the login arrives. Read here and not | |
| 32 | + * in the constructor because the constructors run on init on EVERY request of | |
| 33 | + * every site: reading the main site's settings there meant a switch_to_blog() | |
| 34 | + * and the whole autoloaded option set of the main site on every front page | |
| 35 | + * view of every subsite, measured at 318 rows and 89 KB by the third cross | |
| 36 | + * review of 2.11.10. Nothing outside a login needs this value. | |
| 37 | + * | |
| 38 | + * Whether an account NEEDS a second factor, and which class asks for it, are | |
| 39 | + * separate questions with their own answers: two_factor_required_for() and | |
| 40 | + * two_factor_handler_for(). | |
| 41 | + * | |
| 42 | + * @since 2.11.10 | |
| 43 | + * | |
| 44 | + * @return array | |
| 45 | + */ | |
| 46 | + protected function policy() { | |
| 47 | + if ( null === $this->options ) { | |
| 48 | + $this->options = Vigilante_Settings::two_factor_policy(); | |
| 49 | + } | |
| 50 | + | |
| 51 | + return $this->options; | |
| 52 | + } | |
| 53 | + | |
| 54 | + /** | |
| 55 | + * Whether this class is the one that must ask this account for its factor | |
| 56 | + * | |
| 57 | + * @since 2.11.10 | |
| 58 | + * @since 2.11.11 The enrolment can be passed in by a caller that has just read it. | |
| 59 | + * | |
| 60 | + * @param WP_User $user User being authenticated. | |
| 61 | + * @param string $method Method this class implements, 'email' or 'totp'. | |
| 62 | + * @param bool|null $enrolled Whether the account has a TOTP enrolment, when the | |
| 63 | + * caller already read its row. On a network that read | |
| 64 | + * can search every site the account belongs to, and | |
| 65 | + * the dashboard hooks run on every screen. | |
| 66 | + * @return bool | |
| 67 | + */ | |
| 68 | + protected function handles_second_factor( $user, $method, $enrolled = null ) { | |
| 69 | + if ( null === $enrolled ) { | |
| 70 | + $enrolled = $this->database && method_exists( $this->database, 'has_totp_enrolment' ) | |
| 71 | + ? $this->database->has_totp_enrolment( $user->ID ) | |
| 72 | + : false; | |
| 73 | + } | |
| 74 | + | |
| 75 | + return ( $method === Vigilante_Settings::two_factor_handler_for( $user, $enrolled ) ); | |
| 76 | + } | |
| 77 | + | |
| 78 | + /** | |
| 79 | + * Whether the verification pending in this request belongs to this class | |
| 80 | + * | |
| 81 | + * Both second factor classes hang off login_form_vigilante_2fa and login_form | |
| 82 | + * since 2.11.10, so without this the two of them printed a form on the same | |
| 83 | + * page and both tried to verify the same code. Measured as "the second factor | |
| 84 | + * is asked for twice" by the release matrix. The same election as the | |
| 85 | + * authenticate filter, so a given pending session is handled start to finish | |
| 86 | + * by one class. | |
| 87 | + * | |
| 88 | + * @since 2.11.10 | |
| 89 | + * | |
| 90 | + * @param string $method Method this class implements, 'email' or 'totp'. | |
| 91 | + * @return bool True also when there is nothing pending, so each class goes on | |
| 92 | + * applying its own rules. | |
| 93 | + */ | |
| 94 | + protected function pending_belongs_to( $method ) { | |
| 95 | + $user_id = $this->get_pending_user_id(); | |
| 96 | + | |
| 97 | + if ( ! $user_id ) { | |
| 98 | + return true; | |
| 99 | + } | |
| 100 | + | |
| 101 | + $user = get_userdata( $user_id ); | |
| 102 | + | |
| 103 | + return $user ? $this->handles_second_factor( $user, $method ) : true; | |
| 104 | + } | |
| 105 | + | |
| 106 | + /** | |
| 28 | 107 | * User ID authenticated through an application password in this request, or 0. |
| 29 | 108 | * |
| 30 | 109 | * Set by the core action application_password_did_authenticate, which only |
| 31 | 110 | * fires when the credentials were an application password. That is a second |
| @@ -67,11 +146,76 @@ | ||
| 67 | 146 | * Called from the module's init_hooks(). |
| 68 | 147 | */ |
| 69 | 148 | protected function init_session_hooks() { |
| 70 | 149 | add_action( 'application_password_did_authenticate', array( $this, 'remember_app_password_user' ) ); |
| 150 | + | |
| 151 | + // Why the verification session ended, explained on the login screen it | |
| 152 | + // sends the visitor back to. Both classes use the trait, so both | |
| 153 | + // register this; the notice itself prints once (see the method). | |
| 154 | + add_filter( 'login_message', array( $this, 'show_2fa_session_notice' ) ); | |
| 71 | 155 | } |
| 72 | 156 | |
| 73 | 157 | /** |
| 158 | + * Explain on the login screen why a verification session ended | |
| 159 | + * | |
| 160 | + * Until 2.11.12 running out of verification attempts cleared the pending | |
| 161 | + * session and redirected to wp-login.php with no message at all: the visitor | |
| 162 | + * was back at the password form with no idea why, typed the password again, | |
| 163 | + * and that correct password was counted as one more failed login. | |
| 164 | + * | |
| 165 | + * The query argument only picks one of the literal strings below. It decides | |
| 166 | + * nothing and it is not trusted for anything (rule 21): anyone can add it to | |
| 167 | + * a URL, and all it can produce is one of these notices on a login screen. | |
| 168 | + * | |
| 169 | + * @since 2.11.12 | |
| 170 | + * | |
| 171 | + * @param string $message Login screen message so far. | |
| 172 | + * @return string | |
| 173 | + */ | |
| 174 | + public function show_2fa_session_notice( $message ) { | |
| 175 | + // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Read-only display of a static notice on the login screen; nothing is decided or written. | |
| 176 | + $notice = isset( $_GET['vigilante_2fa_notice'] ) ? sanitize_key( wp_unslash( $_GET['vigilante_2fa_notice'] ) ) : ''; | |
| 177 | + | |
| 178 | + if ( '' === $notice ) { | |
| 179 | + return $message; | |
| 180 | + } | |
| 181 | + | |
| 182 | + // Both two-factor classes use this trait and both register the filter, | |
| 183 | + // so without this the notice would print twice on a site that has them | |
| 184 | + // both loaded. A trait gives each using class its own statics, hence the | |
| 185 | + // prefixed global rather than a static property. | |
| 186 | + if ( ! empty( $GLOBALS['vigilante_2fa_notice_printed'] ) ) { | |
| 187 | + return $message; | |
| 188 | + } | |
| 189 | + | |
| 190 | + $texts = array( | |
| 191 | + 'attempts' => __( 'Too many incorrect verification codes. The verification session was closed for security. Log in again to start a new one.', 'vigilante' ), | |
| 192 | + 'expired' => __( 'The verification session expired. Log in again to start a new one.', 'vigilante' ), | |
| 193 | + ); | |
| 194 | + | |
| 195 | + if ( ! isset( $texts[ $notice ] ) ) { | |
| 196 | + return $message; | |
| 197 | + } | |
| 198 | + | |
| 199 | + $GLOBALS['vigilante_2fa_notice_printed'] = true; | |
| 200 | + | |
| 201 | + return $message . '<div id="login_error" class="notice notice-error"><p>' . esc_html( $texts[ $notice ] ) . '</p></div>'; | |
| 202 | + } | |
| 203 | + | |
| 204 | + /** | |
| 205 | + * Send the visitor back to the login screen with an explanation | |
| 206 | + * | |
| 207 | + * @since 2.11.12 | |
| 208 | + * | |
| 209 | + * @param string $notice One of the keys of show_2fa_session_notice(). | |
| 210 | + * @return void | |
| 211 | + */ | |
| 212 | + private function redirect_to_login_with_notice( $notice ) { | |
| 213 | + wp_safe_redirect( add_query_arg( 'vigilante_2fa_notice', rawurlencode( $notice ), wp_login_url() ) ); | |
| 214 | + exit; | |
| 215 | + } | |
| 216 | + | |
| 217 | + /** | |
| 74 | 218 | * Remember which user authenticated with an application password. |
| 75 | 219 | * |
| 76 | 220 | * @param WP_User $user Authenticated user. |
| 77 | 221 | */ |
| @@ -118,8 +262,12 @@ | ||
| 118 | 262 | * |
| 119 | 263 | * @return WP_Error |
| 120 | 264 | */ |
| 121 | 265 | private function api_requires_2fa_error() { |
| 266 | + // A controlled rejection, not a wrong password: the credentials were | |
| 267 | + // right and the account simply needs its second factor. The error code | |
| 268 | + // is what keeps it out of the brute force count; see | |
| 269 | + // Vigilante_Login_Security::CONTROLLED_REJECTIONS. | |
| 122 | 270 | return new WP_Error( |
| 123 | 271 | 'vigilante_2fa_required', |
| 124 | 272 | __( 'This account requires two-factor authentication. Log in from a browser, or use an application password for API access.', 'vigilante' ) |
| 125 | 273 | ); |
| @@ -152,14 +300,26 @@ | ||
| 152 | 300 | // The attempt counter survives a fresh password login within the hour, |
| 153 | 301 | // so re-authenticating does not reset it (S2). |
| 154 | 302 | $attempts = ( is_array( $data ) && isset( $data['attempts'] ) ) ? absint( $data['attempts'] ) : 0; |
| 155 | 303 | |
| 304 | + // Where the login was headed. The verification form is a second request | |
| 305 | + // with its own POST, so redirect_to has to travel in the session or it | |
| 306 | + // is lost and every login lands on the dashboard (2.11.12). Kept from a | |
| 307 | + // previous pending session when this request does not carry one. | |
| 308 | + // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Not a decision: stored as-is and validated against the site by wp_validate_redirect() before use, in pending_login_redirect(). | |
| 309 | + $redirect_to = isset( $_REQUEST['redirect_to'] ) ? esc_url_raw( wp_unslash( $_REQUEST['redirect_to'] ) ) : ''; | |
| 310 | + | |
| 311 | + if ( '' === $redirect_to && is_array( $data ) && ! empty( $data['redirect_to'] ) ) { | |
| 312 | + $redirect_to = (string) $data['redirect_to']; | |
| 313 | + } | |
| 314 | + | |
| 156 | 315 | set_transient( |
| 157 | 316 | 'vigilante_2fa_pending_' . $token, |
| 158 | 317 | array( |
| 159 | - 'user_id' => $user_id, | |
| 160 | - 'created_at' => time(), | |
| 161 | - 'attempts' => $attempts, | |
| 318 | + 'user_id' => $user_id, | |
| 319 | + 'created_at' => time(), | |
| 320 | + 'attempts' => $attempts, | |
| 321 | + 'redirect_to' => $redirect_to, | |
| 162 | 322 | ), |
| 163 | 323 | HOUR_IN_SECONDS |
| 164 | 324 | ); |
| 165 | 325 | |
| @@ -252,8 +412,28 @@ | ||
| 252 | 412 | return $session ? $session['user_id'] : false; |
| 253 | 413 | } |
| 254 | 414 | |
| 255 | 415 | /** |
| 416 | + * Where to send the visitor once the second factor is verified | |
| 417 | + * | |
| 418 | + * @since 2.11.12 | |
| 419 | + * | |
| 420 | + * @return string URL on this site. | |
| 421 | + */ | |
| 422 | + private function pending_login_redirect() { | |
| 423 | + $session = $this->get_pending_session(); | |
| 424 | + $stored = ( is_array( $session ) && ! empty( $session['redirect_to'] ) ) ? (string) $session['redirect_to'] : ''; | |
| 425 | + | |
| 426 | + if ( '' === $stored ) { | |
| 427 | + return admin_url(); | |
| 428 | + } | |
| 429 | + | |
| 430 | + // Same gate core uses: anything off this site falls back to the | |
| 431 | + // dashboard, so a stored value cannot send anyone off-site. | |
| 432 | + return wp_validate_redirect( $stored, admin_url() ); | |
| 433 | + } | |
| 434 | + | |
| 435 | + /** | |
| 256 | 436 | * Failed attempts recorded on the pending session presented by this request. |
| 257 | 437 | * |
| 258 | 438 | * @return int |
| 259 | 439 | */ |
| @@ -349,9 +529,9 @@ | ||
| 349 | 529 | * @param int $user_id User ID. |
| 350 | 530 | * @return bool |
| 351 | 531 | */ |
| 352 | 532 | private function is_device_trusted( $user_id ) { |
| 353 | - if ( empty( $this->options['allow_remember_device'] ) ) { | |
| 533 | + if ( empty( $this->policy()['allow_remember_device'] ) ) { | |
| 354 | 534 | return false; |
| 355 | 535 | } |
| 356 | 536 | |
| 357 | 537 | $token = $this->present_device_token(); |
| @@ -372,9 +552,9 @@ | ||
| 372 | 552 | * @param int $user_id User ID. |
| 373 | 553 | * @return bool True if a device row was written. |
| 374 | 554 | */ |
| 375 | 555 | private function trust_device( $user_id ) { |
| 376 | - if ( empty( $this->options['allow_remember_device'] ) ) { | |
| 556 | + if ( empty( $this->policy()['allow_remember_device'] ) ) { | |
| 377 | 557 | return false; |
| 378 | 558 | } |
| 379 | 559 | |
| 380 | 560 | try { |
| @@ -383,9 +563,9 @@ | ||
| 383 | 563 | return false; |
| 384 | 564 | } |
| 385 | 565 | |
| 386 | 566 | $user_agent = isset( $_SERVER['HTTP_USER_AGENT'] ) ? sanitize_text_field( wp_unslash( $_SERVER['HTTP_USER_AGENT'] ) ) : ''; |
| 387 | - $remember_days = absint( $this->options['remember_device_days'] ?? 30 ); | |
| 567 | + $remember_days = absint( $this->policy()['remember_device_days'] ?? 30 ); | |
| 388 | 568 | |
| 389 | 569 | if ( $remember_days < 1 ) { |
| 390 | 570 | $remember_days = 30; |
| 391 | 571 | } |
| @@ -443,15 +623,43 @@ | ||
| 443 | 623 | if ( headers_sent() ) { |
| 444 | 624 | return; |
| 445 | 625 | } |
| 446 | 626 | |
| 627 | + /* | |
| 628 | + * On a network these secrets used to travel to every site, while the rows | |
| 629 | + * that validate them carry the blog prefix and belong to one: a device | |
| 630 | + * trusted on one site handed its 64 hex secret to every other site, | |
| 631 | + * where a site administrator, or anything running there, could read it | |
| 632 | + * from the request and replay it. | |
| 633 | + * | |
| 634 | + * Both halves of the scope have to move, and the first attempt only moved | |
| 635 | + * one. An empty domain says "this host only", which isolates the sites of | |
| 636 | + * a network by subdomains; but in a network by subdirectories every site | |
| 637 | + * shares the host and the core leaves COOKIE_DOMAIN empty anyway | |
| 638 | + * (wp-includes/ms-default-constants.php sets it only for subdomain | |
| 639 | + * installs), so that change alone did nothing there. What separates those | |
| 640 | + * sites is the path. Found by the cross review of 2.11.10. | |
| 641 | + * | |
| 642 | + * So on a network the cookie is scoped to this site's own host and path, | |
| 643 | + * which is exactly the reach of the table that validates it. On a single | |
| 644 | + * site both come out as the core's own values and nothing changes. | |
| 645 | + */ | |
| 646 | + $domain = COOKIE_DOMAIN; | |
| 647 | + $path = COOKIEPATH; | |
| 648 | + | |
| 649 | + if ( is_multisite() ) { | |
| 650 | + $domain = ''; | |
| 651 | + $site_path = wp_parse_url( home_url( '/' ), PHP_URL_PATH ); | |
| 652 | + $path = ( is_string( $site_path ) && '' !== $site_path ) ? $site_path : '/'; | |
| 653 | + } | |
| 654 | + | |
| 447 | 655 | setcookie( |
| 448 | 656 | $name, |
| 449 | 657 | $value, |
| 450 | 658 | array( |
| 451 | 659 | 'expires' => $expires, |
| 452 | - 'path' => COOKIEPATH, | |
| 453 | - 'domain' => COOKIE_DOMAIN, | |
| 660 | + 'path' => $path, | |
| 661 | + 'domain' => $domain, | |
| 454 | 662 | 'secure' => is_ssl(), |
| 455 | 663 | 'httponly' => true, |
| 456 | 664 | 'samesite' => $samesite, |
| 457 | 665 | ) |