| @@ -42,20 +42,57 @@ | ||
| 42 | 42 | |
| 43 | 43 | /** |
| 44 | 44 | * Constructor |
| 45 | 45 | * |
| 46 | - * @param Vigilante_Settings $settings Settings instance. | |
| 47 | - * @param Vigilante_Activity_Log $activity_log Activity log instance. | |
| 46 | + * @param Vigilante_Settings $settings Settings instance. | |
| 47 | + * @param Vigilante_Activity_Log $activity_log Activity log instance. | |
| 48 | + * @param bool $enforcement_only Register only what enforces | |
| 49 | + * state already written to an | |
| 50 | + * account. See | |
| 51 | + * init_enforcement_hooks(). | |
| 48 | 52 | */ |
| 49 | - public function __construct( $settings, $activity_log ) { | |
| 53 | + public function __construct( $settings, $activity_log, $enforcement_only = false ) { | |
| 50 | 54 | $this->settings = $settings; |
| 51 | 55 | $this->activity_log = $activity_log; |
| 52 | 56 | $this->options = $settings->get_section( 'user_security' ); |
| 53 | 57 | |
| 58 | + if ( $enforcement_only ) { | |
| 59 | + $this->init_enforcement_hooks(); | |
| 60 | + return; | |
| 61 | + } | |
| 62 | + | |
| 54 | 63 | $this->init_hooks(); |
| 55 | 64 | } |
| 56 | 65 | |
| 57 | 66 | /** |
| 67 | + * The hooks that enforce state already written to an account | |
| 68 | + * | |
| 69 | + * A forced password reset and a registration waiting for approval are not | |
| 70 | + * settings, they are marks on somebody's account, and the action that wrote | |
| 71 | + * them already happened: sessions destroyed, emails sent, the activity log | |
| 72 | + * saying those accounts cannot get in until they reset or are approved. | |
| 73 | + * | |
| 74 | + * Until 2.11.10 both were registered inside the module gate, so turning User | |
| 75 | + * Security off let every one of those accounts back in with their old | |
| 76 | + * password, silently and with the flags still in place saying the opposite. | |
| 77 | + * The forced reset is deliberately not destructive on the password (see | |
| 78 | + * force_password_reset(), which avoids wp_set_password() so the reset link | |
| 79 | + * keeps working), so this filter was the only thing holding the door. | |
| 80 | + * Found by the file-by-file review of 2.11.10. | |
| 81 | + * | |
| 82 | + * These two are therefore registered whether the module is on or off. Both | |
| 83 | + * return immediately when the account carries no mark, so the cost on a site | |
| 84 | + * that never used either feature is one meta read at login. | |
| 85 | + * | |
| 86 | + * @since 2.11.10 | |
| 87 | + */ | |
| 88 | + private function init_enforcement_hooks() { | |
| 89 | + add_filter( 'authenticate', array( $this, 'check_force_reset_on_login' ), 30, 3 ); | |
| 90 | + add_action( 'after_password_reset', array( $this, 'clear_force_reset_meta' ), 10, 1 ); | |
| 91 | + add_filter( 'wp_authenticate_user', array( $this, 'block_pending_user_login' ), 15, 2 ); | |
| 92 | + } | |
| 93 | + | |
| 94 | + /** | |
| 58 | 95 | * Initialize hooks |
| 59 | 96 | */ |
| 60 | 97 | private function init_hooks() { |
| 61 | 98 | // Block insecure usernames |
| @@ -104,9 +141,10 @@ | ||
| 104 | 141 | // Registration approval |
| 105 | 142 | $registration_approval = $this->options['registration_approval'] ?? array(); |
| 106 | 143 | if ( ! empty( $registration_approval['enabled'] ) ) { |
| 107 | 144 | add_action( 'user_register', array( $this, 'set_user_pending_approval' ), 5 ); |
| 108 | - add_filter( 'wp_authenticate_user', array( $this, 'block_pending_user_login' ), 15, 2 ); | |
| 145 | + // The blocking half is registered by init_enforcement_hooks(), so an | |
| 146 | + // account already waiting keeps waiting if the feature is turned off. | |
| 109 | 147 | add_action( 'admin_notices', array( $this, 'show_pending_users_notice' ) ); |
| 110 | 148 | } |
| 111 | 149 | |
| 112 | 150 | // Session limits |
| @@ -165,11 +203,11 @@ | ||
| 165 | 203 | add_filter( 'registration_redirect', array( $this, 'custom_registration_redirect' ) ); |
| 166 | 204 | add_action( 'login_message', array( $this, 'show_registration_pending_message' ) ); |
| 167 | 205 | } |
| 168 | 206 | |
| 169 | - // Force password reset login message (always active, independent of settings) | |
| 170 | - add_filter( 'authenticate', array( $this, 'check_force_reset_on_login' ), 30, 3 ); | |
| 171 | - add_action( 'after_password_reset', array( $this, 'clear_force_reset_meta' ), 10, 1 ); | |
| 207 | + // What enforces marks already written to an account, which stays | |
| 208 | + // registered even with the module off. See init_enforcement_hooks(). | |
| 209 | + $this->init_enforcement_hooks(); | |
| 172 | 210 | } |
| 173 | 211 | |
| 174 | 212 | /** |
| 175 | 213 | * Validate username on profile update |
| @@ -1211,22 +1249,27 @@ | ||
| 1211 | 1249 | * @param string $password Password. |
| 1212 | 1250 | * @return WP_User|WP_Error|null |
| 1213 | 1251 | */ |
| 1214 | 1252 | public function check_force_reset_on_login( $user, $username, $password ) { |
| 1215 | - // Resolve the target user. The flag must be evaluated whether the | |
| 1216 | - // credentials matched (WP_User) or not (WP_Error). | |
| 1217 | - if ( $user instanceof WP_User ) { | |
| 1218 | - $login_user = $user; | |
| 1219 | - } else { | |
| 1220 | - $login_user = get_user_by( 'login', $username ); | |
| 1221 | - if ( ! $login_user ) { | |
| 1222 | - $login_user = get_user_by( 'email', $username ); | |
| 1223 | - } | |
| 1253 | + /* | |
| 1254 | + * Only a login that would otherwise have succeeded is turned into this | |
| 1255 | + * rejection. Wrong credentials are left exactly as WordPress reported | |
| 1256 | + * them, and they are counted like any other failed login. | |
| 1257 | + * | |
| 1258 | + * Until 2.11.12 this ran for a WP_Error too, resolving the account from | |
| 1259 | + * the username, and replaced an incorrect_password with the rejection | |
| 1260 | + * below. A rejection of Vigilant's own is not counted towards the brute | |
| 1261 | + * force lockout, so any account with a pending forced reset could be | |
| 1262 | + * guessed at without limit: measured on 17 Sep 2026 against 2.11.11 and | |
| 1263 | + * against the first build of 2.11.12, six wrong passwords in a row, none | |
| 1264 | + * of them counted and no lockout at the end. The message this function | |
| 1265 | + * exists to show belongs to whoever typed the right password. | |
| 1266 | + */ | |
| 1267 | + if ( ! ( $user instanceof WP_User ) ) { | |
| 1268 | + return $user; | |
| 1224 | 1269 | } |
| 1225 | 1270 | |
| 1226 | - if ( ! $login_user ) { | |
| 1227 | - return $user; | |
| 1228 | - } | |
| 1271 | + $login_user = $user; | |
| 1229 | 1272 | |
| 1230 | 1273 | // Check if this user has a pending forced reset. |
| 1231 | 1274 | $force_reset = get_user_meta( $login_user->ID, 'vigilante_force_reset_pending', true ); |
| 1232 | 1275 | if ( ! $force_reset ) { |
| @@ -1232,17 +1275,11 @@ | ||
| 1232 | 1275 | if ( ! $force_reset ) { |
| 1233 | 1276 | return $user; |
| 1234 | 1277 | } |
| 1235 | 1278 | |
| 1236 | - // If credentials were wrong with an error other than incorrect_password | |
| 1237 | - // (e.g. a Vigilant lockout, pending approval), don't shadow it. | |
| 1238 | - if ( is_wp_error( $user ) && ! in_array( 'incorrect_password', $user->get_error_codes(), true ) ) { | |
| 1239 | - return $user; | |
| 1240 | - } | |
| 1279 | + // Not counted towards the brute force lockout: the rejection is | |
| 1280 | + // recognised by its error code (Vigilante_Login_Security::CONTROLLED_REJECTIONS). | |
| 1241 | 1281 | |
| 1242 | - // Skip brute force counter for this controlled rejection. | |
| 1243 | - add_filter( 'vigilante_skip_failed_login_count', '__return_true' ); | |
| 1244 | - | |
| 1245 | 1282 | // Surface the controlled rejection in the activity log so the admin |
| 1246 | 1283 | // can tell apart "user fails login because they typed wrong password" |
| 1247 | 1284 | // from "user fails login because we are forcing a reset". |
| 1248 | 1285 | if ( $this->activity_log ) { |
| @@ -1314,11 +1351,11 @@ | ||
| 1314 | 1351 | if ( empty( $needs_approval ) ) { |
| 1315 | 1352 | return; |
| 1316 | 1353 | } |
| 1317 | 1354 | |
| 1318 | - // Set pending status | |
| 1319 | - update_user_meta( $user_id, 'vigilante_pending_approval', true ); | |
| 1320 | - update_user_meta( $user_id, 'vigilante_pending_since', time() ); | |
| 1355 | + // Set pending status, on this site only (see site_user_meta_key()). | |
| 1356 | + update_user_meta( $user_id, self::site_user_meta_key( 'vigilante_pending_approval' ), true ); | |
| 1357 | + update_user_meta( $user_id, self::site_user_meta_key( 'vigilante_pending_since' ), time() ); | |
| 1321 | 1358 | |
| 1322 | 1359 | // Log |
| 1323 | 1360 | if ( $this->activity_log ) { |
| 1324 | 1361 | $this->activity_log->log( |
| @@ -1351,14 +1388,9 @@ | ||
| 1351 | 1388 | if ( is_wp_error( $user ) ) { |
| 1352 | 1389 | return $user; |
| 1353 | 1390 | } |
| 1354 | 1391 | |
| 1355 | - $is_pending = get_user_meta( $user->ID, 'vigilante_pending_approval', true ); | |
| 1356 | - | |
| 1357 | - if ( $is_pending ) { | |
| 1358 | - // Mark this as a controlled rejection (not a brute force attempt) | |
| 1359 | - add_filter( 'vigilante_skip_failed_login_count', '__return_true' ); | |
| 1360 | - | |
| 1392 | + if ( self::is_pending_anywhere( $user->ID ) ) { | |
| 1361 | 1393 | return new WP_Error( |
| 1362 | 1394 | 'pending_approval', |
| 1363 | 1395 | __( '<strong>Account pending:</strong> Your account is awaiting administrator approval. You will receive an email once approved.', 'vigilante' ) |
| 1364 | 1396 | ); |
| @@ -1407,16 +1439,131 @@ | ||
| 1407 | 1439 | <?php |
| 1408 | 1440 | } |
| 1409 | 1441 | |
| 1410 | 1442 | /** |
| 1443 | + * A user meta key that belongs to one site, even on a network | |
| 1444 | + * | |
| 1445 | + * Registration approval is a per-site setting, but user meta is network | |
| 1446 | + * wide, so a single global key made the pending queue shared: an | |
| 1447 | + * administrator of one site saw, approved and rejected accounts waiting on | |
| 1448 | + * another, and clearing the flag cleared it for the whole network. Reported | |
| 1449 | + * by the wp.org automated review of 2.11.9. | |
| 1450 | + * | |
| 1451 | + * On a network the key carries the blog prefix, the way core does with | |
| 1452 | + * capabilities (wp_2_capabilities), so each site keeps its own queue. On a | |
| 1453 | + * single site the key is returned unchanged, so nothing has to be migrated | |
| 1454 | + * there and the stored data of every existing install keeps working. | |
| 1455 | + * | |
| 1456 | + * @since 2.11.10 | |
| 1457 | + * | |
| 1458 | + * Note the default is null and not 0: wpdb::get_blog_prefix() reads null as | |
| 1459 | + * "the current blog", and 0 as the main site, so passing 0 here gave every | |
| 1460 | + * subsite the key of the main site and kept the queue shared. Caught by | |
| 1461 | + * matriz-red-repaso-21110.sh before this shipped. | |
| 1462 | + * | |
| 1463 | + * @param string $key Base meta key. | |
| 1464 | + * @param int|null $blog_id Blog to build it for. Current blog when null. | |
| 1465 | + * @return string | |
| 1466 | + */ | |
| 1467 | + public static function site_user_meta_key( $key, $blog_id = null ) { | |
| 1468 | + global $wpdb; | |
| 1469 | + | |
| 1470 | + if ( ! is_multisite() ) { | |
| 1471 | + return $key; | |
| 1472 | + } | |
| 1473 | + | |
| 1474 | + return $wpdb->get_blog_prefix( $blog_id ) . $key; | |
| 1475 | + } | |
| 1476 | + | |
| 1477 | + /** | |
| 1478 | + * Whether this account is waiting for approval on ANY site of the network | |
| 1479 | + * | |
| 1480 | + * The queue is per site and stays per site, because approving somebody is a | |
| 1481 | + * decision of the site they signed up to. Blocking them is a different | |
| 1482 | + * question with a different answer, and giving it the same one was a hole: | |
| 1483 | + * the session cookie WordPress issues is valid on every host of the network | |
| 1484 | + * (COOKIE_DOMAIN and COOKIEPATH, wp-includes/ms-default-constants.php:58-59 | |
| 1485 | + * and :84-88), so an account held back on demo1 logged in through the main | |
| 1486 | + * site, where it carried no flag, and walked straight back into demo1 with | |
| 1487 | + * that cookie. Reproduced over HTTP by the second cross review of 2.11.10. | |
| 1488 | + * It is the same reasoning that two_factor_required_for() already applies: | |
| 1489 | + * network-wide cookie, network-wide enforcement. | |
| 1490 | + * | |
| 1491 | + * Read from the account's own meta in one pass rather than by asking site by | |
| 1492 | + * site, so the cost does not grow with the network. The legacy key with no | |
| 1493 | + * prefix is included because the migration that moves it runs on the first | |
| 1494 | + * admin page load and until then a waiting account has to keep being | |
| 1495 | + * blocked; reading both fails closed. | |
| 1496 | + * | |
| 1497 | + * @since 2.11.10 | |
| 1498 | + * | |
| 1499 | + * @param int $user_id User ID. | |
| 1500 | + * @return bool | |
| 1501 | + */ | |
| 1502 | + public static function is_pending_anywhere( $user_id ) { | |
| 1503 | + global $wpdb; | |
| 1504 | + | |
| 1505 | + if ( get_user_meta( $user_id, 'vigilante_pending_approval', true ) ) { | |
| 1506 | + return true; | |
| 1507 | + } | |
| 1508 | + | |
| 1509 | + if ( ! is_multisite() ) { | |
| 1510 | + return false; | |
| 1511 | + } | |
| 1512 | + | |
| 1513 | + $all = get_user_meta( $user_id ); | |
| 1514 | + | |
| 1515 | + if ( ! is_array( $all ) ) { | |
| 1516 | + return false; | |
| 1517 | + } | |
| 1518 | + | |
| 1519 | + $pattern = '/^' . preg_quote( $wpdb->base_prefix, '/' ) . '(\d+_)?vigilante_pending_approval$/'; | |
| 1520 | + | |
| 1521 | + foreach ( $all as $key => $values ) { | |
| 1522 | + if ( ! preg_match( $pattern, $key, $m ) ) { | |
| 1523 | + continue; | |
| 1524 | + } | |
| 1525 | + | |
| 1526 | + /* | |
| 1527 | + * A mark left behind by a site that no longer exists asks nobody for | |
| 1528 | + * anything: deleting a subsite does not touch this plugin's user meta, | |
| 1529 | + * so the account stayed blocked on the whole network with no queue | |
| 1530 | + * anywhere to clear it from, in a plugin whose users have no WP-CLI. | |
| 1531 | + * Found by the third cross review of 2.11.10. get_site() is cached, so | |
| 1532 | + * this costs nothing in the usual case of no leftovers. | |
| 1533 | + */ | |
| 1534 | + if ( ! empty( $m[1] ) && ! get_site( (int) rtrim( $m[1], '_' ) ) ) { | |
| 1535 | + continue; | |
| 1536 | + } | |
| 1537 | + | |
| 1538 | + foreach ( (array) $values as $value ) { | |
| 1539 | + if ( ! empty( $value ) ) { | |
| 1540 | + return true; | |
| 1541 | + } | |
| 1542 | + } | |
| 1543 | + } | |
| 1544 | + | |
| 1545 | + return false; | |
| 1546 | + } | |
| 1547 | + | |
| 1548 | + /** | |
| 1411 | 1549 | * Get pending users |
| 1412 | 1550 | * |
| 1551 | + * The meta key is what scopes this list to the current site, so on a network | |
| 1552 | + * the query deliberately does not add the site's own membership filter on | |
| 1553 | + * top. Core's WP_User_Query turns the default into "{$prefix}capabilities | |
| 1554 | + * EXISTS" (wp-includes/class-wp-user-query.php:598-604), and an account that | |
| 1555 | + * is waiting for approval can perfectly well have no role yet: it then held | |
| 1556 | + * this site's flag, was blocked from logging in, and appeared in no queue at | |
| 1557 | + * all, so nobody could ever approve or reject it. Found by the second cross | |
| 1558 | + * review of 2.11.10. | |
| 1559 | + * | |
| 1413 | 1560 | * @return array Array of pending user objects. |
| 1414 | 1561 | */ |
| 1415 | 1562 | public function get_pending_users() { |
| 1416 | 1563 | // phpcs:disable WordPress.DB.SlowDBQuery.slow_db_query_meta_key, WordPress.DB.SlowDBQuery.slow_db_query_meta_value -- Limited results in admin context. |
| 1417 | 1564 | $args = array( |
| 1418 | - 'meta_key' => 'vigilante_pending_approval', | |
| 1565 | + 'meta_key' => self::site_user_meta_key( 'vigilante_pending_approval' ), | |
| 1419 | 1566 | 'meta_value' => '1', |
| 1420 | 1567 | 'orderby' => 'registered', |
| 1421 | 1568 | 'order' => 'DESC', |
| 1422 | 1569 | ); |
| @@ -1421,8 +1568,12 @@ | ||
| 1421 | 1568 | 'order' => 'DESC', |
| 1422 | 1569 | ); |
| 1423 | 1570 | // phpcs:enable WordPress.DB.SlowDBQuery.slow_db_query_meta_key, WordPress.DB.SlowDBQuery.slow_db_query_meta_value |
| 1424 | 1571 | |
| 1572 | + if ( is_multisite() ) { | |
| 1573 | + $args['blog_id'] = 0; | |
| 1574 | + } | |
| 1575 | + | |
| 1425 | 1576 | return get_users( $args ); |
| 1426 | 1577 | } |
| 1427 | 1578 | |
| 1428 | 1579 | /** |
| @@ -1434,9 +1585,11 @@ | ||
| 1434 | 1585 | */ |
| 1435 | 1586 | public function approve_user( $user_id, $approved_by = 0 ) { |
| 1436 | 1587 | // Same reasoning as reject_user(): approving an account that never asked |
| 1437 | 1588 | // for approval is a no-op that reports success and writes misleading meta. |
| 1438 | - if ( ! get_user_meta( $user_id, 'vigilante_pending_approval', true ) ) { | |
| 1589 | + // Only this site's flag counts, so approving never clears the queue of | |
| 1590 | + // another site of the network (see site_user_meta_key()). | |
| 1591 | + if ( ! get_user_meta( $user_id, self::site_user_meta_key( 'vigilante_pending_approval' ), true ) ) { | |
| 1439 | 1592 | return false; |
| 1440 | 1593 | } |
| 1441 | 1594 | |
| 1442 | 1595 | $user = get_userdata( $user_id ); |
| @@ -1443,12 +1596,15 @@ | ||
| 1443 | 1596 | if ( ! $user ) { |
| 1444 | 1597 | return false; |
| 1445 | 1598 | } |
| 1446 | 1599 | |
| 1447 | - delete_user_meta( $user_id, 'vigilante_pending_approval' ); | |
| 1448 | - delete_user_meta( $user_id, 'vigilante_pending_since' ); | |
| 1449 | - update_user_meta( $user_id, 'vigilante_approved_by', $approved_by ); | |
| 1450 | - update_user_meta( $user_id, 'vigilante_approved_date', time() ); | |
| 1600 | + delete_user_meta( $user_id, self::site_user_meta_key( 'vigilante_pending_approval' ) ); | |
| 1601 | + delete_user_meta( $user_id, self::site_user_meta_key( 'vigilante_pending_since' ) ); | |
| 1602 | + // Por sitio como las dos de arriba: quien aprueba y cuando es un hecho de | |
| 1603 | + // la cola de ESTE sitio, y dejarlas globales hacia que una aprobacion | |
| 1604 | + // pisara el registro de otro (cierra B4 de la revision cruzada). | |
| 1605 | + update_user_meta( $user_id, self::site_user_meta_key( 'vigilante_approved_by' ), $approved_by ); | |
| 1606 | + update_user_meta( $user_id, self::site_user_meta_key( 'vigilante_approved_date' ), time() ); | |
| 1451 | 1607 | |
| 1452 | 1608 | // Log |
| 1453 | 1609 | if ( $this->activity_log ) { |
| 1454 | 1610 | $admin = $approved_by ? get_userdata( $approved_by ) : null; |
| @@ -1489,9 +1645,9 @@ | ||
| 1489 | 1645 | // Only an account actually waiting for approval may be rejected. Without |
| 1490 | 1646 | // this the handler deletes any user id it is given, and wp_delete_user() |
| 1491 | 1647 | // with no reassignment takes their posts with them, skipping the dialog |
| 1492 | 1648 | // core always shows. Deleting a member is the Users screen's job. |
| 1493 | - if ( ! get_user_meta( $user_id, 'vigilante_pending_approval', true ) ) { | |
| 1649 | + if ( ! get_user_meta( $user_id, self::site_user_meta_key( 'vigilante_pending_approval' ), true ) ) { | |
| 1494 | 1650 | return false; |
| 1495 | 1651 | } |
| 1496 | 1652 | |
| 1497 | 1653 | // Log before deletion |
| @@ -1518,8 +1674,21 @@ | ||
| 1518 | 1674 | |
| 1519 | 1675 | // Send rejection email before deleting |
| 1520 | 1676 | $this->send_rejection_email( $user, $reason ); |
| 1521 | 1677 | |
| 1678 | + /* | |
| 1679 | + * The mark goes first, because on a network the account may well survive | |
| 1680 | + * the deletion: wp_delete_user() only calls remove_user_from_blog() there | |
| 1681 | + * (wp-admin/includes/user.php:440-442), which clears the role and nothing | |
| 1682 | + * of this plugin's own meta. Leaving it behind made Reject a loop with no | |
| 1683 | + * way out: the account stayed blocked on every site of the network, the | |
| 1684 | + * row never left the queue (which since 2.11.10 no longer hides accounts | |
| 1685 | + * without a role), and pressing Reject again sent the rejection email once | |
| 1686 | + * more and reported success. Found by the third cross review of 2.11.10. | |
| 1687 | + */ | |
| 1688 | + delete_user_meta( $user_id, self::site_user_meta_key( 'vigilante_pending_approval' ) ); | |
| 1689 | + delete_user_meta( $user_id, self::site_user_meta_key( 'vigilante_pending_since' ) ); | |
| 1690 | + | |
| 1522 | 1691 | // Delete user |
| 1523 | 1692 | require_once ABSPATH . 'wp-admin/includes/user.php'; |
| 1524 | 1693 | return wp_delete_user( $user_id ); |
| 1525 | 1694 | } |
| @@ -1901,8 +2070,31 @@ | ||
| 1901 | 2070 | return max( 0, $count ); |
| 1902 | 2071 | } |
| 1903 | 2072 | |
| 1904 | 2073 | /** |
| 2074 | + * Whether the session store this limit would act on belongs to a whole network | |
| 2075 | + * | |
| 2076 | + * WP_Session_Tokens keeps session_tokens in the usermeta table, which is | |
| 2077 | + * network wide, while this limit is configured per site. So on a network a | |
| 2078 | + * site administrator setting a low limit would count, and with close_oldest | |
| 2079 | + * close, the sessions the same user opened on other sites, including an | |
| 2080 | + * administrator session elsewhere; and block_new would refuse a login over | |
| 2081 | + * sessions that have nothing to do with this site. Reported by the wp.org | |
| 2082 | + * automated review of 2.11.9, on the close_oldest half. | |
| 2083 | + * | |
| 2084 | + * Until the network-wide policy of 3.1.0, the limit simply does not apply on | |
| 2085 | + * a network, and the settings screen says so. On a single site nothing | |
| 2086 | + * changes: there the session store and the setting cover the same thing. | |
| 2087 | + * | |
| 2088 | + * @since 2.11.10 | |
| 2089 | + * | |
| 2090 | + * @return bool | |
| 2091 | + */ | |
| 2092 | + public static function session_limit_is_network_wide() { | |
| 2093 | + return is_multisite(); | |
| 2094 | + } | |
| 2095 | + | |
| 2096 | + /** | |
| 1905 | 2097 | * Check session limit before login completes (for block_new behavior) |
| 1906 | 2098 | * |
| 1907 | 2099 | * @param WP_User $user User object. |
| 1908 | 2100 | * @param string $password Password. |
| @@ -1912,8 +2104,12 @@ | ||
| 1912 | 2104 | if ( is_wp_error( $user ) ) { |
| 1913 | 2105 | return $user; |
| 1914 | 2106 | } |
| 1915 | 2107 | |
| 2108 | + if ( self::session_limit_is_network_wide() ) { | |
| 2109 | + return $user; | |
| 2110 | + } | |
| 2111 | + | |
| 1916 | 2112 | $settings = $this->options['session_limits'] ?? array(); |
| 1917 | 2113 | $max_sessions = absint( $settings['max_sessions'] ?? 3 ); |
| 1918 | 2114 | $exclude_admins = ! empty( $settings['exclude_admins'] ); |
| 1919 | 2115 | |
| @@ -1943,11 +2139,8 @@ | ||
| 1943 | 2139 | 'warning' |
| 1944 | 2140 | ); |
| 1945 | 2141 | } |
| 1946 | 2142 | |
| 1947 | - // Mark this as a controlled rejection (not a brute force attempt) | |
| 1948 | - add_filter( 'vigilante_skip_failed_login_count', '__return_true' ); | |
| 1949 | - | |
| 1950 | 2143 | return new WP_Error( |
| 1951 | 2144 | 'session_limit_exceeded', |
| 1952 | 2145 | sprintf( |
| 1953 | 2146 | /* translators: %d: Maximum sessions allowed */ |
| @@ -1966,8 +2159,12 @@ | ||
| 1966 | 2159 | * @param string $user_login Username. |
| 1967 | 2160 | * @param WP_User $user User object. |
| 1968 | 2161 | */ |
| 1969 | 2162 | public function enforce_session_limit( $user_login, $user ) { |
| 2163 | + if ( self::session_limit_is_network_wide() ) { | |
| 2164 | + return; | |
| 2165 | + } | |
| 2166 | + | |
| 1970 | 2167 | $settings = $this->options['session_limits'] ?? array(); |
| 1971 | 2168 | $max_sessions = absint( $settings['max_sessions'] ?? 3 ); |
| 1972 | 2169 | $behavior = $settings['behavior'] ?? 'block_new'; |
| 1973 | 2170 | $exclude_admins = ! empty( $settings['exclude_admins'] ); |
| @@ -2687,8 +2884,21 @@ | ||
| 2687 | 2884 | * |
| 2688 | 2885 | * @param int $user_id User ID. |
| 2689 | 2886 | */ |
| 2690 | 2887 | public function send_verification_email( $user_id ) { |
| 2888 | + /* | |
| 2889 | + * Never send an account that is already verified back to pending. The | |
| 2890 | + * resend link below reaches this, and while the pending value was | |
| 2891 | + * unreadable (see the note on the meta write) that was harmless; with | |
| 2892 | + * the check working, resending for a verified account would lock its | |
| 2893 | + * owner out of their own site. | |
| 2894 | + */ | |
| 2895 | + if ( metadata_exists( 'user', $user_id, 'vigilante_email_verified' ) | |
| 2896 | + && get_user_meta( $user_id, 'vigilante_email_verified', true ) | |
| 2897 | + ) { | |
| 2898 | + return; | |
| 2899 | + } | |
| 2900 | + | |
| 2691 | 2901 | $user = get_userdata( $user_id ); |
| 2692 | 2902 | if ( ! $user ) { |
| 2693 | 2903 | return; |
| 2694 | 2904 | } |
| @@ -2703,10 +2913,22 @@ | ||
| 2703 | 2913 | |
| 2704 | 2914 | // Store token |
| 2705 | 2915 | update_user_meta( $user_id, 'vigilante_verification_token', $token_hash ); |
| 2706 | 2916 | update_user_meta( $user_id, 'vigilante_verification_expires', $expires ); |
| 2707 | - update_user_meta( $user_id, 'vigilante_email_verified', false ); | |
| 2708 | 2917 | |
| 2918 | + /* | |
| 2919 | + * '0' and not false. update_user_meta() stores false as an empty string | |
| 2920 | + * (maybe_serialize() returns it unchanged and wpdb writes it with %s), and | |
| 2921 | + * an empty string is what get_user_meta() also returns when there is no | |
| 2922 | + * row at all. So from the moment this feature existed until 2.11.10 the | |
| 2923 | + * value written to mean "not verified yet" was read back as "this account | |
| 2924 | + * predates the feature, let it in", and the branch that blocks the login | |
| 2925 | + * was unreachable. Found by the file-by-file review of 2.11.10. '0' is | |
| 2926 | + * falsy in PHP and survives the round trip, and the readers below tell an | |
| 2927 | + * absent row from a stored one with metadata_exists(). | |
| 2928 | + */ | |
| 2929 | + update_user_meta( $user_id, 'vigilante_email_verified', '0' ); | |
| 2930 | + | |
| 2709 | 2931 | // Build verification URL |
| 2710 | 2932 | $verify_url = add_query_arg( |
| 2711 | 2933 | array( |
| 2712 | 2934 | 'vigilante_verify' => '1', |
| @@ -2780,16 +3002,21 @@ | ||
| 2780 | 3002 | if ( is_wp_error( $user ) ) { |
| 2781 | 3003 | return $user; |
| 2782 | 3004 | } |
| 2783 | 3005 | |
| 2784 | - // Check if email is verified | |
| 2785 | - $verified = get_user_meta( $user->ID, 'vigilante_email_verified', true ); | |
| 2786 | - | |
| 2787 | - // If no meta exists, user was created before this feature - allow | |
| 2788 | - if ( '' === $verified ) { | |
| 3006 | + /* | |
| 3007 | + * Only a row that does not exist means "created before this feature". | |
| 3008 | + * An existing row holding an empty string is an account that older | |
| 3009 | + * versions marked as pending, and it has to be blocked like any other: | |
| 3010 | + * reading both the same way is what made this check let everyone in | |
| 3011 | + * (see send_verification_email()). | |
| 3012 | + */ | |
| 3013 | + if ( ! metadata_exists( 'user', $user->ID, 'vigilante_email_verified' ) ) { | |
| 2789 | 3014 | return $user; |
| 2790 | 3015 | } |
| 2791 | 3016 | |
| 3017 | + $verified = get_user_meta( $user->ID, 'vigilante_email_verified', true ); | |
| 3018 | + | |
| 2792 | 3019 | if ( ! $verified ) { |
| 2793 | 3020 | $settings = $this->options['email_verification'] ?? array(); |
| 2794 | 3021 | $allow_resend = ! empty( $settings['allow_resend'] ); |
| 2795 | 3022 | |
| @@ -2831,10 +3058,19 @@ | ||
| 2831 | 3058 | |
| 2832 | 3059 | // Verify nonce to prevent CSRF and user-ID probing. |
| 2833 | 3060 | if ( ! isset( $_GET['_vigilante_nonce'] ) || |
| 2834 | 3061 | ! wp_verify_nonce( sanitize_text_field( wp_unslash( $_GET['_vigilante_nonce'] ) ), 'vigilante_resend_verification_' . $user_id ) ) { |
| 2835 | - wp_safe_redirect( add_query_arg( 'vigilante_message', 'invalid', wp_login_url() ) ); | |
| 2836 | - exit; | |
| 3062 | + /* | |
| 3063 | + * Nothing is redirected to the login page until the request | |
| 3064 | + * has proved something, and a bad nonce proves nothing. It | |
| 3065 | + * used to answer with a redirect to wp_login_url(), which | |
| 3066 | + * under a custom login URL IS the secret address, so any | |
| 3067 | + * visitor could read it out of the Location header of a | |
| 3068 | + * request carrying garbage. Found by the third cross review | |
| 3069 | + * of 2.11.10. Returning leaves the request to render the page | |
| 3070 | + * it asked for, which tells nobody anything. | |
| 3071 | + */ | |
| 3072 | + return; | |
| 2837 | 3073 | } |
| 2838 | 3074 | |
| 2839 | 3075 | // Rate limiting: allow 1 resend every 5 minutes per user to prevent email spam. |
| 2840 | 3076 | $transient_key = 'vigilante_resend_' . $user_id; |
| @@ -2855,11 +3091,12 @@ | ||
| 2855 | 3091 | $user_id = isset( $_GET['user_id'] ) ? absint( $_GET['user_id'] ) : 0; |
| 2856 | 3092 | // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- token-based verification below. |
| 2857 | 3093 | $token = isset( $_GET['token'] ) ? sanitize_text_field( wp_unslash( $_GET['token'] ) ) : ''; |
| 2858 | 3094 | |
| 3095 | + // Same as the resend above: no proof, no redirect, so the Location | |
| 3096 | + // header cannot be used to read the custom login URL. | |
| 2859 | 3097 | if ( ! $user_id || ! $token ) { |
| 2860 | - wp_safe_redirect( add_query_arg( 'vigilante_message', 'invalid', wp_login_url() ) ); | |
| 2861 | - exit; | |
| 3098 | + return; | |
| 2862 | 3099 | } |
| 2863 | 3100 | |
| 2864 | 3101 | $stored_hash = (string) get_user_meta( $user_id, 'vigilante_verification_token', true ); |
| 2865 | 3102 | $expires = (int) get_user_meta( $user_id, 'vigilante_verification_expires', true ); |
| @@ -2868,10 +3105,9 @@ | ||
| 2868 | 3105 | // any account with no verification pending and "invalid" for one waiting, |
| 2869 | 3106 | // so a wrong link revealed which user ids were waiting (2.11.8). Only the |
| 2870 | 3107 | // holder of the right token learns that it expired. |
| 2871 | 3108 | if ( '' === $stored_hash || ! hash_equals( $stored_hash, wp_hash( $token ) ) ) { |
| 2872 | - wp_safe_redirect( add_query_arg( 'vigilante_message', 'invalid', wp_login_url() ) ); | |
| 2873 | - exit; | |
| 3109 | + return; | |
| 2874 | 3110 | } |
| 2875 | 3111 | |
| 2876 | 3112 | if ( time() > $expires ) { |
| 2877 | 3113 | wp_safe_redirect( add_query_arg( 'vigilante_message', 'expired', wp_login_url() ) ); |
| @@ -2899,12 +3135,11 @@ | ||
| 2899 | 3135 | 'info' |
| 2900 | 3136 | ); |
| 2901 | 3137 | } |
| 2902 | 3138 | |
| 2903 | - // Check if user still needs approval | |
| 2904 | - $is_pending = get_user_meta( $user_id, 'vigilante_pending_approval', true ); | |
| 2905 | - | |
| 2906 | - if ( $is_pending ) { | |
| 3139 | + // Anywhere on the network, so the message matches what will actually | |
| 3140 | + // happen at the login: that is what blocks (see is_pending_anywhere()). | |
| 3141 | + if ( self::is_pending_anywhere( $user_id ) ) { | |
| 2907 | 3142 | // User verified but still pending approval |
| 2908 | 3143 | wp_safe_redirect( |
| 2909 | 3144 | add_query_arg( |
| 2910 | 3145 | array( |
| @@ -2965,14 +3200,16 @@ | ||
| 2965 | 3200 | * @param int $user_id User ID. |
| 2966 | 3201 | * @return bool |
| 2967 | 3202 | */ |
| 2968 | 3203 | public function is_email_verified( $user_id ) { |
| 2969 | - $verified = get_user_meta( $user_id, 'vigilante_email_verified', true ); | |
| 2970 | - | |
| 2971 | - // If no meta exists, consider verified (old users) | |
| 2972 | - if ( '' === $verified ) { | |
| 3204 | + // Same reading as block_unverified_user_login(): only an absent row means | |
| 3205 | + // the account predates the feature. A stored empty string is an account | |
| 3206 | + // an older version left pending. | |
| 3207 | + if ( ! metadata_exists( 'user', $user_id, 'vigilante_email_verified' ) ) { | |
| 2973 | 3208 | return true; |
| 2974 | 3209 | } |
| 3210 | + | |
| 3211 | + $verified = get_user_meta( $user_id, 'vigilante_email_verified', true ); | |
| 2975 | 3212 | |
| 2976 | 3213 | return (bool) $verified; |
| 2977 | 3214 | } |
| 2978 | 3215 | |
