| @@ -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 |
| @@ -131,8 +169,16 @@ | ||
| 131 | 169 | if ( ! empty( $password_expiration['enabled'] ) ) { |
| 132 | 170 | add_action( 'wp_login', array( $this, 'check_password_expiration' ), 10, 2 ); |
| 133 | 171 | add_action( 'admin_notices', array( $this, 'show_password_expiration_notice' ) ); |
| 134 | 172 | add_action( 'admin_init', array( $this, 'force_password_change_redirect' ) ); |
| 173 | + // Enforcement beyond wp-admin: REST and the front end, so an expired | |
| 174 | + // password cannot keep operating outside the redirect (2.11.9). | |
| 175 | + add_filter( 'rest_authentication_errors', array( $this, 'block_expired_password_rest' ), 20 ); | |
| 176 | + add_action( 'template_redirect', array( $this, 'force_password_change_frontend' ) ); | |
| 177 | + // The profile is the only screen where the change can be made, and | |
| 178 | + // WooCommerce keeps accounts with no back-end role out of wp-admin. | |
| 179 | + add_filter( 'woocommerce_prevent_admin_access', array( $this, 'allow_profile_for_password_change' ) ); | |
| 180 | + add_filter( 'authenticate', array( $this, 'block_expired_password_xmlrpc' ), 30, 1 ); | |
| 135 | 181 | add_action( 'profile_update', array( $this, 'update_password_change_date' ), 10, 2 ); |
| 136 | 182 | add_action( 'user_register', array( $this, 'set_initial_password_date' ) ); |
| 137 | 183 | add_action( 'user_profile_update_errors', array( $this, 'check_password_history' ), 10, 3 ); |
| 138 | 184 | |
| @@ -160,11 +206,11 @@ | ||
| 160 | 206 | add_filter( 'registration_redirect', array( $this, 'custom_registration_redirect' ) ); |
| 161 | 207 | add_action( 'login_message', array( $this, 'show_registration_pending_message' ) ); |
| 162 | 208 | } |
| 163 | 209 | |
| 164 | - // Force password reset login message (always active, independent of settings) | |
| 165 | - add_filter( 'authenticate', array( $this, 'check_force_reset_on_login' ), 30, 3 ); | |
| 166 | - add_action( 'after_password_reset', array( $this, 'clear_force_reset_meta' ), 10, 1 ); | |
| 210 | + // What enforces marks already written to an account, which stays | |
| 211 | + // registered even with the module off. See init_enforcement_hooks(). | |
| 212 | + $this->init_enforcement_hooks(); | |
| 167 | 213 | } |
| 168 | 214 | |
| 169 | 215 | /** |
| 170 | 216 | * Validate username on profile update |
| @@ -241,9 +287,11 @@ | ||
| 241 | 287 | * @return bool |
| 242 | 288 | */ |
| 243 | 289 | private function is_insecure_username( $username ) { |
| 244 | 290 | $username = strtolower( trim( $username ) ); |
| 245 | - $insecure_usernames = $this->options['insecure_usernames'] ?? array(); | |
| 291 | + // Vigilante_Settings::list_or_shipped(): an empty list of a setting with | |
| 292 | + // no field on screen is a leftover, not a choice. See its docblock. | |
| 293 | + $insecure_usernames = $this->get_insecure_usernames(); | |
| 246 | 294 | |
| 247 | 295 | return in_array( $username, array_map( 'strtolower', $insecure_usernames ), true ); |
| 248 | 296 | } |
| 249 | 297 | |
| @@ -945,9 +993,14 @@ | ||
| 945 | 993 | * |
| 946 | 994 | * @return array |
| 947 | 995 | */ |
| 948 | 996 | public function get_insecure_usernames() { |
| 949 | - return $this->options['insecure_usernames'] ?? array(); | |
| 997 | + // The single reader of this list, so what blocks and what the screen | |
| 998 | + // reports can never disagree. | |
| 999 | + return Vigilante_Settings::list_or_shipped( | |
| 1000 | + $this->options['insecure_usernames'] ?? array(), | |
| 1001 | + array( 'user_security', 'insecure_usernames' ) | |
| 1002 | + ); | |
| 950 | 1003 | } |
| 951 | 1004 | |
| 952 | 1005 | /** |
| 953 | 1006 | * Check for existing insecure admin users |
| @@ -1206,22 +1259,27 @@ | ||
| 1206 | 1259 | * @param string $password Password. |
| 1207 | 1260 | * @return WP_User|WP_Error|null |
| 1208 | 1261 | */ |
| 1209 | 1262 | public function check_force_reset_on_login( $user, $username, $password ) { |
| 1210 | - // Resolve the target user. The flag must be evaluated whether the | |
| 1211 | - // credentials matched (WP_User) or not (WP_Error). | |
| 1212 | - if ( $user instanceof WP_User ) { | |
| 1213 | - $login_user = $user; | |
| 1214 | - } else { | |
| 1215 | - $login_user = get_user_by( 'login', $username ); | |
| 1216 | - if ( ! $login_user ) { | |
| 1217 | - $login_user = get_user_by( 'email', $username ); | |
| 1218 | - } | |
| 1263 | + /* | |
| 1264 | + * Only a login that would otherwise have succeeded is turned into this | |
| 1265 | + * rejection. Wrong credentials are left exactly as WordPress reported | |
| 1266 | + * them, and they are counted like any other failed login. | |
| 1267 | + * | |
| 1268 | + * Until 2.11.12 this ran for a WP_Error too, resolving the account from | |
| 1269 | + * the username, and replaced an incorrect_password with the rejection | |
| 1270 | + * below. A rejection of Vigilant's own is not counted towards the brute | |
| 1271 | + * force lockout, so any account with a pending forced reset could be | |
| 1272 | + * guessed at without limit: measured on 17 Sep 2026 against 2.11.11 and | |
| 1273 | + * against the first build of 2.11.12, six wrong passwords in a row, none | |
| 1274 | + * of them counted and no lockout at the end. The message this function | |
| 1275 | + * exists to show belongs to whoever typed the right password. | |
| 1276 | + */ | |
| 1277 | + if ( ! ( $user instanceof WP_User ) ) { | |
| 1278 | + return $user; | |
| 1219 | 1279 | } |
| 1220 | 1280 | |
| 1221 | - if ( ! $login_user ) { | |
| 1222 | - return $user; | |
| 1223 | - } | |
| 1281 | + $login_user = $user; | |
| 1224 | 1282 | |
| 1225 | 1283 | // Check if this user has a pending forced reset. |
| 1226 | 1284 | $force_reset = get_user_meta( $login_user->ID, 'vigilante_force_reset_pending', true ); |
| 1227 | 1285 | if ( ! $force_reset ) { |
| @@ -1227,17 +1285,11 @@ | ||
| 1227 | 1285 | if ( ! $force_reset ) { |
| 1228 | 1286 | return $user; |
| 1229 | 1287 | } |
| 1230 | 1288 | |
| 1231 | - // If credentials were wrong with an error other than incorrect_password | |
| 1232 | - // (e.g. a Vigilant lockout, pending approval), don't shadow it. | |
| 1233 | - if ( is_wp_error( $user ) && ! in_array( 'incorrect_password', $user->get_error_codes(), true ) ) { | |
| 1234 | - return $user; | |
| 1235 | - } | |
| 1289 | + // Not counted towards the brute force lockout: the rejection is | |
| 1290 | + // recognised by its error code (Vigilante_Login_Security::CONTROLLED_REJECTIONS). | |
| 1236 | 1291 | |
| 1237 | - // Skip brute force counter for this controlled rejection. | |
| 1238 | - add_filter( 'vigilante_skip_failed_login_count', '__return_true' ); | |
| 1239 | - | |
| 1240 | 1292 | // Surface the controlled rejection in the activity log so the admin |
| 1241 | 1293 | // can tell apart "user fails login because they typed wrong password" |
| 1242 | 1294 | // from "user fails login because we are forcing a reset". |
| 1243 | 1295 | if ( $this->activity_log ) { |
| @@ -1299,9 +1351,14 @@ | ||
| 1299 | 1351 | return; |
| 1300 | 1352 | } |
| 1301 | 1353 | |
| 1302 | 1354 | $settings = $this->options['registration_approval'] ?? array(); |
| 1303 | - $affected_roles = $settings['affected_roles'] ?? array( 'subscriber' ); | |
| 1355 | + // Vigilante_Settings::list_or_shipped(): an empty list of a setting with | |
| 1356 | + // no field on screen is a leftover, not a choice. See its docblock. | |
| 1357 | + $affected_roles = Vigilante_Settings::list_or_shipped( | |
| 1358 | + $settings['affected_roles'] ?? array(), | |
| 1359 | + array( 'user_security', 'registration_approval', 'affected_roles' ) | |
| 1360 | + ); | |
| 1304 | 1361 | |
| 1305 | 1362 | // Check if user role requires approval |
| 1306 | 1363 | $user_roles = $user->roles; |
| 1307 | 1364 | $needs_approval = array_intersect( $user_roles, $affected_roles ); |
| @@ -1309,11 +1366,11 @@ | ||
| 1309 | 1366 | if ( empty( $needs_approval ) ) { |
| 1310 | 1367 | return; |
| 1311 | 1368 | } |
| 1312 | 1369 | |
| 1313 | - // Set pending status | |
| 1314 | - update_user_meta( $user_id, 'vigilante_pending_approval', true ); | |
| 1315 | - update_user_meta( $user_id, 'vigilante_pending_since', time() ); | |
| 1370 | + // Set pending status, on this site only (see site_user_meta_key()). | |
| 1371 | + update_user_meta( $user_id, self::site_user_meta_key( 'vigilante_pending_approval' ), true ); | |
| 1372 | + update_user_meta( $user_id, self::site_user_meta_key( 'vigilante_pending_since' ), time() ); | |
| 1316 | 1373 | |
| 1317 | 1374 | // Log |
| 1318 | 1375 | if ( $this->activity_log ) { |
| 1319 | 1376 | $this->activity_log->log( |
| @@ -1346,14 +1403,9 @@ | ||
| 1346 | 1403 | if ( is_wp_error( $user ) ) { |
| 1347 | 1404 | return $user; |
| 1348 | 1405 | } |
| 1349 | 1406 | |
| 1350 | - $is_pending = get_user_meta( $user->ID, 'vigilante_pending_approval', true ); | |
| 1351 | - | |
| 1352 | - if ( $is_pending ) { | |
| 1353 | - // Mark this as a controlled rejection (not a brute force attempt) | |
| 1354 | - add_filter( 'vigilante_skip_failed_login_count', '__return_true' ); | |
| 1355 | - | |
| 1407 | + if ( self::is_pending_anywhere( $user->ID ) ) { | |
| 1356 | 1408 | return new WP_Error( |
| 1357 | 1409 | 'pending_approval', |
| 1358 | 1410 | __( '<strong>Account pending:</strong> Your account is awaiting administrator approval. You will receive an email once approved.', 'vigilante' ) |
| 1359 | 1411 | ); |
| @@ -1393,9 +1445,9 @@ | ||
| 1393 | 1445 | $count, |
| 1394 | 1446 | 'vigilante' |
| 1395 | 1447 | ) ), |
| 1396 | 1448 | absint( $count ), |
| 1397 | - '<a href="' . esc_url( admin_url( 'admin.php?page=vigilante&tab=users' ) ) . '">' . esc_html__( 'Review in Vigilant', 'vigilante' ) . '</a>' | |
| 1449 | + '<a href="' . esc_url( admin_url( 'admin.php?page=vigilante&tab=users#vigilante-section-users-pending' ) ) . '">' . esc_html__( 'Review in Vigilant', 'vigilante' ) . '</a>' | |
| 1398 | 1450 | ); |
| 1399 | 1451 | ?> |
| 1400 | 1452 | </p> |
| 1401 | 1453 | </div> |
| @@ -1402,16 +1454,131 @@ | ||
| 1402 | 1454 | <?php |
| 1403 | 1455 | } |
| 1404 | 1456 | |
| 1405 | 1457 | /** |
| 1458 | + * A user meta key that belongs to one site, even on a network | |
| 1459 | + * | |
| 1460 | + * Registration approval is a per-site setting, but user meta is network | |
| 1461 | + * wide, so a single global key made the pending queue shared: an | |
| 1462 | + * administrator of one site saw, approved and rejected accounts waiting on | |
| 1463 | + * another, and clearing the flag cleared it for the whole network. Reported | |
| 1464 | + * by the wp.org automated review of 2.11.9. | |
| 1465 | + * | |
| 1466 | + * On a network the key carries the blog prefix, the way core does with | |
| 1467 | + * capabilities (wp_2_capabilities), so each site keeps its own queue. On a | |
| 1468 | + * single site the key is returned unchanged, so nothing has to be migrated | |
| 1469 | + * there and the stored data of every existing install keeps working. | |
| 1470 | + * | |
| 1471 | + * @since 2.11.10 | |
| 1472 | + * | |
| 1473 | + * Note the default is null and not 0: wpdb::get_blog_prefix() reads null as | |
| 1474 | + * "the current blog", and 0 as the main site, so passing 0 here gave every | |
| 1475 | + * subsite the key of the main site and kept the queue shared. Caught by | |
| 1476 | + * matriz-red-repaso-21110.sh before this shipped. | |
| 1477 | + * | |
| 1478 | + * @param string $key Base meta key. | |
| 1479 | + * @param int|null $blog_id Blog to build it for. Current blog when null. | |
| 1480 | + * @return string | |
| 1481 | + */ | |
| 1482 | + public static function site_user_meta_key( $key, $blog_id = null ) { | |
| 1483 | + global $wpdb; | |
| 1484 | + | |
| 1485 | + if ( ! is_multisite() ) { | |
| 1486 | + return $key; | |
| 1487 | + } | |
| 1488 | + | |
| 1489 | + return $wpdb->get_blog_prefix( $blog_id ) . $key; | |
| 1490 | + } | |
| 1491 | + | |
| 1492 | + /** | |
| 1493 | + * Whether this account is waiting for approval on ANY site of the network | |
| 1494 | + * | |
| 1495 | + * The queue is per site and stays per site, because approving somebody is a | |
| 1496 | + * decision of the site they signed up to. Blocking them is a different | |
| 1497 | + * question with a different answer, and giving it the same one was a hole: | |
| 1498 | + * the session cookie WordPress issues is valid on every host of the network | |
| 1499 | + * (COOKIE_DOMAIN and COOKIEPATH, wp-includes/ms-default-constants.php:58-59 | |
| 1500 | + * and :84-88), so an account held back on demo1 logged in through the main | |
| 1501 | + * site, where it carried no flag, and walked straight back into demo1 with | |
| 1502 | + * that cookie. Reproduced over HTTP by the second cross review of 2.11.10. | |
| 1503 | + * It is the same reasoning that two_factor_required_for() already applies: | |
| 1504 | + * network-wide cookie, network-wide enforcement. | |
| 1505 | + * | |
| 1506 | + * Read from the account's own meta in one pass rather than by asking site by | |
| 1507 | + * site, so the cost does not grow with the network. The legacy key with no | |
| 1508 | + * prefix is included because the migration that moves it runs on the first | |
| 1509 | + * admin page load and until then a waiting account has to keep being | |
| 1510 | + * blocked; reading both fails closed. | |
| 1511 | + * | |
| 1512 | + * @since 2.11.10 | |
| 1513 | + * | |
| 1514 | + * @param int $user_id User ID. | |
| 1515 | + * @return bool | |
| 1516 | + */ | |
| 1517 | + public static function is_pending_anywhere( $user_id ) { | |
| 1518 | + global $wpdb; | |
| 1519 | + | |
| 1520 | + if ( get_user_meta( $user_id, 'vigilante_pending_approval', true ) ) { | |
| 1521 | + return true; | |
| 1522 | + } | |
| 1523 | + | |
| 1524 | + if ( ! is_multisite() ) { | |
| 1525 | + return false; | |
| 1526 | + } | |
| 1527 | + | |
| 1528 | + $all = get_user_meta( $user_id ); | |
| 1529 | + | |
| 1530 | + if ( ! is_array( $all ) ) { | |
| 1531 | + return false; | |
| 1532 | + } | |
| 1533 | + | |
| 1534 | + $pattern = '/^' . preg_quote( $wpdb->base_prefix, '/' ) . '(\d+_)?vigilante_pending_approval$/'; | |
| 1535 | + | |
| 1536 | + foreach ( $all as $key => $values ) { | |
| 1537 | + if ( ! preg_match( $pattern, $key, $m ) ) { | |
| 1538 | + continue; | |
| 1539 | + } | |
| 1540 | + | |
| 1541 | + /* | |
| 1542 | + * A mark left behind by a site that no longer exists asks nobody for | |
| 1543 | + * anything: deleting a subsite does not touch this plugin's user meta, | |
| 1544 | + * so the account stayed blocked on the whole network with no queue | |
| 1545 | + * anywhere to clear it from, in a plugin whose users have no WP-CLI. | |
| 1546 | + * Found by the third cross review of 2.11.10. get_site() is cached, so | |
| 1547 | + * this costs nothing in the usual case of no leftovers. | |
| 1548 | + */ | |
| 1549 | + if ( ! empty( $m[1] ) && ! get_site( (int) rtrim( $m[1], '_' ) ) ) { | |
| 1550 | + continue; | |
| 1551 | + } | |
| 1552 | + | |
| 1553 | + foreach ( (array) $values as $value ) { | |
| 1554 | + if ( ! empty( $value ) ) { | |
| 1555 | + return true; | |
| 1556 | + } | |
| 1557 | + } | |
| 1558 | + } | |
| 1559 | + | |
| 1560 | + return false; | |
| 1561 | + } | |
| 1562 | + | |
| 1563 | + /** | |
| 1406 | 1564 | * Get pending users |
| 1407 | 1565 | * |
| 1566 | + * The meta key is what scopes this list to the current site, so on a network | |
| 1567 | + * the query deliberately does not add the site's own membership filter on | |
| 1568 | + * top. Core's WP_User_Query turns the default into "{$prefix}capabilities | |
| 1569 | + * EXISTS" (wp-includes/class-wp-user-query.php:598-604), and an account that | |
| 1570 | + * is waiting for approval can perfectly well have no role yet: it then held | |
| 1571 | + * this site's flag, was blocked from logging in, and appeared in no queue at | |
| 1572 | + * all, so nobody could ever approve or reject it. Found by the second cross | |
| 1573 | + * review of 2.11.10. | |
| 1574 | + * | |
| 1408 | 1575 | * @return array Array of pending user objects. |
| 1409 | 1576 | */ |
| 1410 | 1577 | public function get_pending_users() { |
| 1411 | 1578 | // phpcs:disable WordPress.DB.SlowDBQuery.slow_db_query_meta_key, WordPress.DB.SlowDBQuery.slow_db_query_meta_value -- Limited results in admin context. |
| 1412 | 1579 | $args = array( |
| 1413 | - 'meta_key' => 'vigilante_pending_approval', | |
| 1580 | + 'meta_key' => self::site_user_meta_key( 'vigilante_pending_approval' ), | |
| 1414 | 1581 | 'meta_value' => '1', |
| 1415 | 1582 | 'orderby' => 'registered', |
| 1416 | 1583 | 'order' => 'DESC', |
| 1417 | 1584 | ); |
| @@ -1416,8 +1583,12 @@ | ||
| 1416 | 1583 | 'order' => 'DESC', |
| 1417 | 1584 | ); |
| 1418 | 1585 | // phpcs:enable WordPress.DB.SlowDBQuery.slow_db_query_meta_key, WordPress.DB.SlowDBQuery.slow_db_query_meta_value |
| 1419 | 1586 | |
| 1587 | + if ( is_multisite() ) { | |
| 1588 | + $args['blog_id'] = 0; | |
| 1589 | + } | |
| 1590 | + | |
| 1420 | 1591 | return get_users( $args ); |
| 1421 | 1592 | } |
| 1422 | 1593 | |
| 1423 | 1594 | /** |
| @@ -1429,9 +1600,11 @@ | ||
| 1429 | 1600 | */ |
| 1430 | 1601 | public function approve_user( $user_id, $approved_by = 0 ) { |
| 1431 | 1602 | // Same reasoning as reject_user(): approving an account that never asked |
| 1432 | 1603 | // for approval is a no-op that reports success and writes misleading meta. |
| 1433 | - if ( ! get_user_meta( $user_id, 'vigilante_pending_approval', true ) ) { | |
| 1604 | + // Only this site's flag counts, so approving never clears the queue of | |
| 1605 | + // another site of the network (see site_user_meta_key()). | |
| 1606 | + if ( ! get_user_meta( $user_id, self::site_user_meta_key( 'vigilante_pending_approval' ), true ) ) { | |
| 1434 | 1607 | return false; |
| 1435 | 1608 | } |
| 1436 | 1609 | |
| 1437 | 1610 | $user = get_userdata( $user_id ); |
| @@ -1438,12 +1611,15 @@ | ||
| 1438 | 1611 | if ( ! $user ) { |
| 1439 | 1612 | return false; |
| 1440 | 1613 | } |
| 1441 | 1614 | |
| 1442 | - delete_user_meta( $user_id, 'vigilante_pending_approval' ); | |
| 1443 | - delete_user_meta( $user_id, 'vigilante_pending_since' ); | |
| 1444 | - update_user_meta( $user_id, 'vigilante_approved_by', $approved_by ); | |
| 1445 | - update_user_meta( $user_id, 'vigilante_approved_date', time() ); | |
| 1615 | + delete_user_meta( $user_id, self::site_user_meta_key( 'vigilante_pending_approval' ) ); | |
| 1616 | + delete_user_meta( $user_id, self::site_user_meta_key( 'vigilante_pending_since' ) ); | |
| 1617 | + // Por sitio como las dos de arriba: quien aprueba y cuando es un hecho de | |
| 1618 | + // la cola de ESTE sitio, y dejarlas globales hacia que una aprobacion | |
| 1619 | + // pisara el registro de otro (cierra B4 de la revision cruzada). | |
| 1620 | + update_user_meta( $user_id, self::site_user_meta_key( 'vigilante_approved_by' ), $approved_by ); | |
| 1621 | + update_user_meta( $user_id, self::site_user_meta_key( 'vigilante_approved_date' ), time() ); | |
| 1446 | 1622 | |
| 1447 | 1623 | // Log |
| 1448 | 1624 | if ( $this->activity_log ) { |
| 1449 | 1625 | $admin = $approved_by ? get_userdata( $approved_by ) : null; |
| @@ -1484,9 +1660,9 @@ | ||
| 1484 | 1660 | // Only an account actually waiting for approval may be rejected. Without |
| 1485 | 1661 | // this the handler deletes any user id it is given, and wp_delete_user() |
| 1486 | 1662 | // with no reassignment takes their posts with them, skipping the dialog |
| 1487 | 1663 | // core always shows. Deleting a member is the Users screen's job. |
| 1488 | - if ( ! get_user_meta( $user_id, 'vigilante_pending_approval', true ) ) { | |
| 1664 | + if ( ! get_user_meta( $user_id, self::site_user_meta_key( 'vigilante_pending_approval' ), true ) ) { | |
| 1489 | 1665 | return false; |
| 1490 | 1666 | } |
| 1491 | 1667 | |
| 1492 | 1668 | // Log before deletion |
| @@ -1513,8 +1689,21 @@ | ||
| 1513 | 1689 | |
| 1514 | 1690 | // Send rejection email before deleting |
| 1515 | 1691 | $this->send_rejection_email( $user, $reason ); |
| 1516 | 1692 | |
| 1693 | + /* | |
| 1694 | + * The mark goes first, because on a network the account may well survive | |
| 1695 | + * the deletion: wp_delete_user() only calls remove_user_from_blog() there | |
| 1696 | + * (wp-admin/includes/user.php:440-442), which clears the role and nothing | |
| 1697 | + * of this plugin's own meta. Leaving it behind made Reject a loop with no | |
| 1698 | + * way out: the account stayed blocked on every site of the network, the | |
| 1699 | + * row never left the queue (which since 2.11.10 no longer hides accounts | |
| 1700 | + * without a role), and pressing Reject again sent the rejection email once | |
| 1701 | + * more and reported success. Found by the third cross review of 2.11.10. | |
| 1702 | + */ | |
| 1703 | + delete_user_meta( $user_id, self::site_user_meta_key( 'vigilante_pending_approval' ) ); | |
| 1704 | + delete_user_meta( $user_id, self::site_user_meta_key( 'vigilante_pending_since' ) ); | |
| 1705 | + | |
| 1517 | 1706 | // Delete user |
| 1518 | 1707 | require_once ABSPATH . 'wp-admin/includes/user.php'; |
| 1519 | 1708 | return wp_delete_user( $user_id ); |
| 1520 | 1709 | } |
| @@ -1533,9 +1722,9 @@ | ||
| 1533 | 1722 | __( '[%s] New user registration pending approval', 'vigilante' ), |
| 1534 | 1723 | $site_name |
| 1535 | 1724 | ); |
| 1536 | 1725 | |
| 1537 | - $approve_url = admin_url( 'admin.php?page=vigilante&tab=users' ); | |
| 1726 | + $approve_url = admin_url( 'admin.php?page=vigilante&tab=users#vigilante-section-users-pending' ); | |
| 1538 | 1727 | |
| 1539 | 1728 | $body = Vigilante_Email_Template::p( __( 'A new user has registered and is awaiting your approval.', 'vigilante' ) ); |
| 1540 | 1729 | $body .= Vigilante_Email_Template::data_table( array( |
| 1541 | 1730 | __( 'Username', 'vigilante' ) => $user->user_login, |
| @@ -1896,8 +2085,31 @@ | ||
| 1896 | 2085 | return max( 0, $count ); |
| 1897 | 2086 | } |
| 1898 | 2087 | |
| 1899 | 2088 | /** |
| 2089 | + * Whether the session store this limit would act on belongs to a whole network | |
| 2090 | + * | |
| 2091 | + * WP_Session_Tokens keeps session_tokens in the usermeta table, which is | |
| 2092 | + * network wide, while this limit is configured per site. So on a network a | |
| 2093 | + * site administrator setting a low limit would count, and with close_oldest | |
| 2094 | + * close, the sessions the same user opened on other sites, including an | |
| 2095 | + * administrator session elsewhere; and block_new would refuse a login over | |
| 2096 | + * sessions that have nothing to do with this site. Reported by the wp.org | |
| 2097 | + * automated review of 2.11.9, on the close_oldest half. | |
| 2098 | + * | |
| 2099 | + * Until the network-wide policy of 3.1.0, the limit simply does not apply on | |
| 2100 | + * a network, and the settings screen says so. On a single site nothing | |
| 2101 | + * changes: there the session store and the setting cover the same thing. | |
| 2102 | + * | |
| 2103 | + * @since 2.11.10 | |
| 2104 | + * | |
| 2105 | + * @return bool | |
| 2106 | + */ | |
| 2107 | + public static function session_limit_is_network_wide() { | |
| 2108 | + return is_multisite(); | |
| 2109 | + } | |
| 2110 | + | |
| 2111 | + /** | |
| 1900 | 2112 | * Check session limit before login completes (for block_new behavior) |
| 1901 | 2113 | * |
| 1902 | 2114 | * @param WP_User $user User object. |
| 1903 | 2115 | * @param string $password Password. |
| @@ -1907,8 +2119,12 @@ | ||
| 1907 | 2119 | if ( is_wp_error( $user ) ) { |
| 1908 | 2120 | return $user; |
| 1909 | 2121 | } |
| 1910 | 2122 | |
| 2123 | + if ( self::session_limit_is_network_wide() ) { | |
| 2124 | + return $user; | |
| 2125 | + } | |
| 2126 | + | |
| 1911 | 2127 | $settings = $this->options['session_limits'] ?? array(); |
| 1912 | 2128 | $max_sessions = absint( $settings['max_sessions'] ?? 3 ); |
| 1913 | 2129 | $exclude_admins = ! empty( $settings['exclude_admins'] ); |
| 1914 | 2130 | |
| @@ -1938,11 +2154,8 @@ | ||
| 1938 | 2154 | 'warning' |
| 1939 | 2155 | ); |
| 1940 | 2156 | } |
| 1941 | 2157 | |
| 1942 | - // Mark this as a controlled rejection (not a brute force attempt) | |
| 1943 | - add_filter( 'vigilante_skip_failed_login_count', '__return_true' ); | |
| 1944 | - | |
| 1945 | 2158 | return new WP_Error( |
| 1946 | 2159 | 'session_limit_exceeded', |
| 1947 | 2160 | sprintf( |
| 1948 | 2161 | /* translators: %d: Maximum sessions allowed */ |
| @@ -1961,8 +2174,12 @@ | ||
| 1961 | 2174 | * @param string $user_login Username. |
| 1962 | 2175 | * @param WP_User $user User object. |
| 1963 | 2176 | */ |
| 1964 | 2177 | public function enforce_session_limit( $user_login, $user ) { |
| 2178 | + if ( self::session_limit_is_network_wide() ) { | |
| 2179 | + return; | |
| 2180 | + } | |
| 2181 | + | |
| 1965 | 2182 | $settings = $this->options['session_limits'] ?? array(); |
| 1966 | 2183 | $max_sessions = absint( $settings['max_sessions'] ?? 3 ); |
| 1967 | 2184 | $behavior = $settings['behavior'] ?? 'block_new'; |
| 1968 | 2185 | $exclude_admins = ! empty( $settings['exclude_admins'] ); |
| @@ -1981,27 +2198,69 @@ | ||
| 1981 | 2198 | return; |
| 1982 | 2199 | } |
| 1983 | 2200 | |
| 1984 | 2201 | if ( 'close_oldest' === $behavior ) { |
| 1985 | - // Sort by login time and destroy oldest | |
| 1986 | - uasort( $all_sessions, function( $a, $b ) { | |
| 1987 | - return ( $a['login'] ?? 0 ) - ( $b['login'] ?? 0 ); | |
| 2202 | + /* | |
| 2203 | + * Remove the oldest sessions by editing the session store directly. | |
| 2204 | + * | |
| 2205 | + * WP_Session_Tokens::get_all() returns array_values( get_sessions() ), | |
| 2206 | + * so its keys are 0, 1, 2, not tokens, and destroy() expects a raw | |
| 2207 | + * token, which is not stored anywhere and cannot be recovered for a | |
| 2208 | + * session other than the current one. Until 2.11.9 the loop passed | |
| 2209 | + * those numeric keys to destroy(), which hashed them, matched nothing | |
| 2210 | + * and closed no session while still counting and logging success, so | |
| 2211 | + * the cap did nothing under close_oldest. Reported by the wp.org | |
| 2212 | + * automated review of 2.11.8. | |
| 2213 | + * | |
| 2214 | + * The store keeps the sessions as the user meta 'session_tokens', | |
| 2215 | + * keyed by the verifier hash( 'sha256', token ), which is the value | |
| 2216 | + * is_current_session() already compares against. So the oldest are | |
| 2217 | + * removed from that map, keeping the current session whatever its age. | |
| 2218 | + * On a network the meta is global (one finding of the multisite audit, | |
| 2219 | + * to be reworked in 3.1.0); here the fix is only to make the removal | |
| 2220 | + * actually happen. | |
| 2221 | + */ | |
| 2222 | + $stored = get_user_meta( $user->ID, 'session_tokens', true ); | |
| 2223 | + | |
| 2224 | + if ( ! is_array( $stored ) || empty( $stored ) ) { | |
| 2225 | + return; | |
| 2226 | + } | |
| 2227 | + | |
| 2228 | + $now = time(); | |
| 2229 | + $changed = false; | |
| 2230 | + | |
| 2231 | + // Expired sessions are dead weight and count for nothing; drop them first. | |
| 2232 | + foreach ( $stored as $verifier => $session ) { | |
| 2233 | + if ( isset( $session['expiration'] ) && (int) $session['expiration'] < $now ) { | |
| 2234 | + unset( $stored[ $verifier ] ); | |
| 2235 | + $changed = true; | |
| 2236 | + } | |
| 2237 | + } | |
| 2238 | + | |
| 2239 | + // Oldest first, keeping the current session whatever its login time. | |
| 2240 | + uasort( $stored, function ( $a, $b ) { | |
| 2241 | + return ( $a['login'] ?? 0 ) <=> ( $b['login'] ?? 0 ); | |
| 1988 | 2242 | } ); |
| 1989 | 2243 | |
| 1990 | - $sessions_to_remove = $session_count - $max_sessions; | |
| 1991 | - $removed = 0; | |
| 2244 | + $sessions_to_remove = count( $stored ) - $max_sessions; | |
| 2245 | + $removed = 0; | |
| 1992 | 2246 | |
| 1993 | - foreach ( $all_sessions as $token_hash => $session ) { | |
| 2247 | + foreach ( $stored as $verifier => $session ) { | |
| 1994 | 2248 | if ( $removed >= $sessions_to_remove ) { |
| 1995 | 2249 | break; |
| 1996 | 2250 | } |
| 1997 | - // Don't remove current session | |
| 1998 | - if ( ! $this->is_current_session( $token_hash ) ) { | |
| 1999 | - $sessions->destroy( $token_hash ); | |
| 2000 | - $removed++; | |
| 2251 | + if ( $this->is_current_session( $verifier ) ) { | |
| 2252 | + continue; | |
| 2001 | 2253 | } |
| 2254 | + unset( $stored[ $verifier ] ); | |
| 2255 | + $removed++; | |
| 2256 | + $changed = true; | |
| 2002 | 2257 | } |
| 2003 | 2258 | |
| 2259 | + if ( $changed ) { | |
| 2260 | + update_user_meta( $user->ID, 'session_tokens', $stored ); | |
| 2261 | + } | |
| 2262 | + | |
| 2004 | 2263 | // Log |
| 2005 | 2264 | if ( $this->activity_log && $removed > 0 ) { |
| 2006 | 2265 | $this->activity_log->log( |
| 2007 | 2266 | 'user', |
| @@ -2051,17 +2310,25 @@ | ||
| 2051 | 2310 | |
| 2052 | 2311 | // Honor both affected_roles AND the per-user exclusion list, and |
| 2053 | 2312 | // clear stale flags if the user no longer matches the rules. |
| 2054 | 2313 | if ( ! $this->is_password_expiration_applicable( $user_id ) ) { |
| 2055 | - if ( get_user_meta( $user_id, 'vigilante_must_change_password', true ) ) { | |
| 2314 | + // Only on a single site, for the same reason as in | |
| 2315 | + // force_password_change_redirect(): on a network the flag belongs to | |
| 2316 | + // the account, and this site's policy says nothing about the site | |
| 2317 | + // that set it. The 2.11.8 fix only covered that method, and this | |
| 2318 | + // notice cleared the flag anyway on the next admin page; found by | |
| 2319 | + // the cross review of 2.11.8. | |
| 2320 | + if ( ! is_multisite() && get_user_meta( $user_id, 'vigilante_must_change_password', true ) ) { | |
| 2056 | 2321 | delete_user_meta( $user_id, 'vigilante_must_change_password' ); |
| 2057 | 2322 | } |
| 2058 | 2323 | return; |
| 2059 | 2324 | } |
| 2060 | 2325 | |
| 2061 | - // Check if must change password | |
| 2062 | - $must_change = get_user_meta( $user_id, 'vigilante_must_change_password', true ); | |
| 2063 | - if ( $must_change ) { | |
| 2326 | + // Ask what the redirect asks. From 2.11.9 to 3.0.3 this read only the | |
| 2327 | + // flag, which is written at login, while force_password_change_redirect() | |
| 2328 | + // also works the expiry out on the spot: a session still open when the | |
| 2329 | + // password expired was sent to the profile with no notice saying why. | |
| 2330 | + if ( $this->must_change_password( $user_id ) ) { | |
| 2064 | 2331 | global $pagenow; |
| 2065 | 2332 | $on_profile = ( 'profile.php' === $pagenow ); |
| 2066 | 2333 | ?> |
| 2067 | 2334 | <div class="notice notice-error"> |
| @@ -2068,8 +2335,9 @@ | ||
| 2068 | 2335 | <p> |
| 2069 | 2336 | <strong><?php esc_html_e( 'Password change required', 'vigilante' ); ?></strong> |
| 2070 | 2337 | <?php if ( $on_profile ) : ?> |
| 2071 | 2338 | <?php esc_html_e( 'Your password has expired. Set a new password in the section below and save your profile to continue.', 'vigilante' ); ?> |
| 2339 | + <a href="#password"><?php esc_html_e( 'Go to the password field', 'vigilante' ); ?></a> | |
| 2072 | 2340 | <?php else : ?> |
| 2073 | 2341 | <?php |
| 2074 | 2342 | printf( |
| 2075 | 2343 | /* translators: %s: Link to profile */ |
| @@ -2088,28 +2356,40 @@ | ||
| 2088 | 2356 | <?php |
| 2089 | 2357 | return; |
| 2090 | 2358 | } |
| 2091 | 2359 | |
| 2092 | - // Show warning if expiring soon | |
| 2360 | + // Show warning if expiring soon. get_days_until_expiration() rounds down, | |
| 2361 | + // so 0 here is less than a day left: an expired password never gets this | |
| 2362 | + // far, must_change_password() has answered for it above. Until 3.0.3 the | |
| 2363 | + // check asked for more than 0, so the last day showed nothing and the | |
| 2364 | + // next thing the user saw was the forced change. | |
| 2093 | 2365 | $days_left = $this->get_days_until_expiration( $user_id ); |
| 2094 | 2366 | $warning_days = absint( $settings['warning_days'] ?? 14 ); |
| 2095 | 2367 | |
| 2096 | - if ( $days_left > 0 && $days_left <= $warning_days ) { | |
| 2368 | + if ( $warning_days > 0 && $days_left >= 0 && $days_left <= $warning_days ) { | |
| 2097 | 2369 | ?> |
| 2098 | 2370 | <div class="notice notice-warning is-dismissible"> |
| 2099 | 2371 | <p> |
| 2100 | 2372 | <?php |
| 2101 | - printf( | |
| 2102 | - /* translators: 1: Number of days, 2: Link to profile */ | |
| 2103 | - esc_html( _n( | |
| 2104 | - 'Your password will expire in %1$d day. Please %2$s.', | |
| 2105 | - 'Your password will expire in %1$d days. Please %2$s.', | |
| 2106 | - $days_left, | |
| 2107 | - 'vigilante' | |
| 2108 | - ) ), | |
| 2109 | - absint( $days_left ), | |
| 2110 | - '<a href="' . esc_url( admin_url( 'profile.php' ) ) . '">' . esc_html__( 'change it now', 'vigilante' ) . '</a>' | |
| 2111 | - ); | |
| 2373 | + if ( $days_left < 1 ) { | |
| 2374 | + printf( | |
| 2375 | + /* translators: %s: Link to profile */ | |
| 2376 | + esc_html__( 'Your password expires in less than a day. Please %s.', 'vigilante' ), | |
| 2377 | + '<a href="' . esc_url( admin_url( 'profile.php' ) ) . '">' . esc_html__( 'change it now', 'vigilante' ) . '</a>' | |
| 2378 | + ); | |
| 2379 | + } else { | |
| 2380 | + printf( | |
| 2381 | + /* translators: 1: Number of days, 2: Link to profile */ | |
| 2382 | + esc_html( _n( | |
| 2383 | + 'Your password will expire in %1$d day. Please %2$s.', | |
| 2384 | + 'Your password will expire in %1$d days. Please %2$s.', | |
| 2385 | + $days_left, | |
| 2386 | + 'vigilante' | |
| 2387 | + ) ), | |
| 2388 | + absint( $days_left ), | |
| 2389 | + '<a href="' . esc_url( admin_url( 'profile.php' ) ) . '">' . esc_html__( 'change it now', 'vigilante' ) . '</a>' | |
| 2390 | + ); | |
| 2391 | + } | |
| 2112 | 2392 | ?> |
| 2113 | 2393 | </p> |
| 2114 | 2394 | </div> |
| 2115 | 2395 | <?php |
| @@ -2116,44 +2396,279 @@ | ||
| 2116 | 2396 | } |
| 2117 | 2397 | } |
| 2118 | 2398 | |
| 2119 | 2399 | /** |
| 2120 | - * Force redirect to password change page | |
| 2400 | + * Whether this user must change an expired password before doing anything else | |
| 2401 | + * | |
| 2402 | + * The flag alone is not enough: it is re-checked against the current policy, | |
| 2403 | + * because the admin may have taken the user's role out of affected_roles or | |
| 2404 | + * added the user to the exclusion list after it was set, which would | |
| 2405 | + * otherwise lock them in a redirect loop. A stale flag is cleared on a | |
| 2406 | + * single site; on a network the meta is shared by every site and the policy | |
| 2407 | + * checked is only this site's, so it is left alone and simply not enforced | |
| 2408 | + * here (a network-wide rework is the 3.1.0 multisite item). | |
| 2409 | + * | |
| 2410 | + * @since 2.11.9 | |
| 2411 | + * | |
| 2412 | + * @param int $user_id User ID. | |
| 2413 | + * @return bool | |
| 2121 | 2414 | */ |
| 2415 | + private function must_change_password( $user_id ) { | |
| 2416 | + if ( ! $user_id ) { | |
| 2417 | + return false; | |
| 2418 | + } | |
| 2419 | + | |
| 2420 | + $flagged = (bool) get_user_meta( $user_id, 'vigilante_must_change_password', true ); | |
| 2421 | + | |
| 2422 | + if ( ! $this->is_password_expiration_applicable( $user_id ) ) { | |
| 2423 | + if ( $flagged && ! is_multisite() ) { | |
| 2424 | + delete_user_meta( $user_id, 'vigilante_must_change_password' ); | |
| 2425 | + } | |
| 2426 | + return false; | |
| 2427 | + } | |
| 2428 | + | |
| 2429 | + if ( $flagged ) { | |
| 2430 | + return true; | |
| 2431 | + } | |
| 2432 | + | |
| 2433 | + // The flag is set at interactive login (check_password_expiration on | |
| 2434 | + // wp_login). A session that authenticates only through REST, XML-RPC or | |
| 2435 | + // an application password never fires wp_login, so the flag can be | |
| 2436 | + // absent while the password is in fact expired. Compute it on the fly | |
| 2437 | + // too, so a non-interactive route is not a way around the block. The | |
| 2438 | + // computation self-seeds the change date on first sight and never locks | |
| 2439 | + // out a user who has no record yet (see is_password_expired()). | |
| 2440 | + return $this->is_password_expired( $user_id ); | |
| 2441 | + } | |
| 2442 | + | |
| 2443 | + /** | |
| 2444 | + * Whether this request is the user's own profile form and nothing else | |
| 2445 | + * | |
| 2446 | + * A user who must change an expired password is let through to the profile, | |
| 2447 | + * because that is where the change is made. Until 3.0.3 "the profile" meant | |
| 2448 | + * the name of the script, and profile.php can be asked for other things: the | |
| 2449 | + * admin bootstrap runs a plugin page or an importer named in the address | |
| 2450 | + * whatever the script is, and fires the hook of any action it is sent | |
| 2451 | + * (wp-admin/admin.php), and the profile script edits whichever user the | |
| 2452 | + * request names (wp-admin/user-edit.php). The pending change could be put | |
| 2453 | + * off while part of the admin area was still in use. | |
| 2454 | + * | |
| 2455 | + * Two requests are the profile form: the one that shows it, which posts | |
| 2456 | + * nothing, and the one that saves it carrying the nonce WordPress printed | |
| 2457 | + * on this user's own form. Parameters in the address that WordPress does | |
| 2458 | + * nothing with are left alone, so a proxy or a browser extension that adds | |
| 2459 | + * one to every address does not send the user round in circles. | |
| 2460 | + * | |
| 2461 | + * Meant for admin_init. wp-admin/profile.php defines IS_PROFILE_PAGE before | |
| 2462 | + * it loads the admin bootstrap, so the constant is there when that script is | |
| 2463 | + * the one running. The network admin and user dashboard copies load the | |
| 2464 | + * bootstrap first: from them the user is taken to the site profile, where | |
| 2465 | + * the notice is shown. | |
| 2466 | + * | |
| 2467 | + * @return bool | |
| 2468 | + */ | |
| 2469 | + private function is_own_profile_request() { | |
| 2470 | + global $pagenow, $plugin_page; | |
| 2471 | + | |
| 2472 | + if ( 'profile.php' !== $pagenow || ! defined( 'IS_PROFILE_PAGE' ) || ! IS_PROFILE_PAGE ) { | |
| 2473 | + return false; | |
| 2474 | + } | |
| 2475 | + | |
| 2476 | + // A plugin page or an importer named in the address. | |
| 2477 | + if ( isset( $plugin_page ) || isset( $_GET['import'] ) ) { | |
| 2478 | + return false; | |
| 2479 | + } | |
| 2480 | + | |
| 2481 | + // Somebody else's profile. | |
| 2482 | + $own_id = (string) get_current_user_id(); | |
| 2483 | + if ( ! empty( $_REQUEST['user_id'] ) && $own_id !== $_REQUEST['user_id'] ) { | |
| 2484 | + return false; | |
| 2485 | + } | |
| 2486 | + | |
| 2487 | + // No action: the form being shown, which posts nothing. | |
| 2488 | + if ( empty( $_REQUEST['action'] ) ) { | |
| 2489 | + return empty( $_POST ); | |
| 2490 | + } | |
| 2491 | + | |
| 2492 | + // An action: only the form being saved. | |
| 2493 | + return 'update' === $_REQUEST['action'] | |
| 2494 | + && isset( $_REQUEST['_wpnonce'] ) | |
| 2495 | + && false !== wp_verify_nonce( sanitize_text_field( wp_unslash( $_REQUEST['_wpnonce'] ) ), 'update-user_' . $own_id ); | |
| 2496 | + } | |
| 2497 | + | |
| 2498 | + /** | |
| 2499 | + * Force a user with an expired password to change it, on wp-admin and AJAX | |
| 2500 | + * | |
| 2501 | + * Until 2.11.9 this only redirected wp-admin pages and skipped AJAX, so an | |
| 2502 | + * expired-password session kept working through admin-ajax, and the REST API | |
| 2503 | + * and the front end were not covered at all. The wp.org automated review of | |
| 2504 | + * 2.11.8 flagged it: setting a flag on login is not enforcement if the flag | |
| 2505 | + * is only read by one redirect. It is now enforced on every entry point, | |
| 2506 | + * here for wp-admin and AJAX and in the three methods below for REST, the | |
| 2507 | + * front end and XML-RPC. The only thing an affected user can still do is | |
| 2508 | + * change the password on profile.php or log out. From 2.11.9 to 3.0.3 that | |
| 2509 | + * was not quite true, because any request to profile.php was let through: | |
| 2510 | + * see is_own_profile_request(). | |
| 2511 | + */ | |
| 2122 | 2512 | public function force_password_change_redirect() { |
| 2123 | - if ( ! is_user_logged_in() ) { | |
| 2513 | + if ( ! is_user_logged_in() || ! $this->must_change_password( get_current_user_id() ) ) { | |
| 2124 | 2514 | return; |
| 2125 | 2515 | } |
| 2126 | 2516 | |
| 2127 | - // Don't redirect on AJAX or profile page | |
| 2517 | + // AJAX: a redirect is useless, so the request is refused. Changing the | |
| 2518 | + // password is a profile.php form POST, not AJAX, so nothing the user | |
| 2519 | + // needs to fix this is blocked. | |
| 2128 | 2520 | if ( wp_doing_ajax() ) { |
| 2521 | + wp_send_json_error( | |
| 2522 | + array( 'message' => __( 'Your password has expired. Change it in your profile before continuing.', 'vigilante' ) ), | |
| 2523 | + 403 | |
| 2524 | + ); | |
| 2525 | + } | |
| 2526 | + | |
| 2527 | + // The profile form is where the change happens; do not redirect it onto | |
| 2528 | + // itself. Anything else profile.php is asked for goes back to the form. | |
| 2529 | + if ( $this->is_own_profile_request() ) { | |
| 2129 | 2530 | return; |
| 2130 | 2531 | } |
| 2131 | 2532 | |
| 2132 | - global $pagenow; | |
| 2133 | - if ( 'profile.php' === $pagenow ) { | |
| 2533 | + $this->redirect_to_profile(); | |
| 2534 | + } | |
| 2535 | + | |
| 2536 | + /** | |
| 2537 | + * Send a user who must change an expired password to their profile | |
| 2538 | + * | |
| 2539 | + * To the top of it, with no "#password" fragment (it had one from 1.0.0 to | |
| 2540 | + * 3.0.3): the browser jumped straight to the password row and left the | |
| 2541 | + * notice that says why, printed at the top of the page, out of sight. The | |
| 2542 | + * notice carries the link to the field instead. | |
| 2543 | + * | |
| 2544 | + * wp_safe_redirect() answers an address it does not take for the site's | |
| 2545 | + * own with a redirect to admin_url(), the dashboard, and the dashboard sent | |
| 2546 | + * the user here again, and so on until the browser gave up. It only knows | |
| 2547 | + * the home host, so that happened on a site whose WordPress lives on a host | |
| 2548 | + * of its own, and also where the host has non-ASCII letters stored as | |
| 2549 | + * written or is an IPv6 literal. The address sent is built here from the | |
| 2550 | + * site's own settings and carries nothing from the request, so the fallback | |
| 2551 | + * for this one call is that same address: what admin_url() says is as good | |
| 2552 | + * for the profile as it is for the dashboard WordPress falls back to. | |
| 2553 | + */ | |
| 2554 | + private function redirect_to_profile() { | |
| 2555 | + $profile = admin_url( 'profile.php' ); | |
| 2556 | + | |
| 2557 | + add_filter( | |
| 2558 | + 'wp_safe_redirect_fallback', | |
| 2559 | + static function () use ( $profile ) { | |
| 2560 | + return $profile; | |
| 2561 | + } | |
| 2562 | + ); | |
| 2563 | + | |
| 2564 | + wp_safe_redirect( $profile ); | |
| 2565 | + exit; | |
| 2566 | + } | |
| 2567 | + | |
| 2568 | + /** | |
| 2569 | + * Refuse REST API requests from a user whose password has expired | |
| 2570 | + * | |
| 2571 | + * @since 2.11.9 | |
| 2572 | + * | |
| 2573 | + * @param WP_Error|null|true $result Result of the earlier authentication checks. | |
| 2574 | + * @return WP_Error|null|true | |
| 2575 | + */ | |
| 2576 | + public function block_expired_password_rest( $result ) { | |
| 2577 | + // Leave any decision another check already made, and do not act on | |
| 2578 | + // logged-out requests to public endpoints. | |
| 2579 | + if ( null !== $result && false !== $result ) { | |
| 2580 | + return $result; | |
| 2581 | + } | |
| 2582 | + | |
| 2583 | + if ( is_user_logged_in() && $this->must_change_password( get_current_user_id() ) ) { | |
| 2584 | + return new WP_Error( | |
| 2585 | + 'vigilante_password_expired', | |
| 2586 | + __( 'Your password has expired. Change it in your profile before using the REST API.', 'vigilante' ), | |
| 2587 | + array( 'status' => 403 ) | |
| 2588 | + ); | |
| 2589 | + } | |
| 2590 | + | |
| 2591 | + return $result; | |
| 2592 | + } | |
| 2593 | + | |
| 2594 | + /** | |
| 2595 | + * Send a user with an expired password to the change page from the front end | |
| 2596 | + * | |
| 2597 | + * @since 2.11.9 | |
| 2598 | + */ | |
| 2599 | + public function force_password_change_frontend() { | |
| 2600 | + if ( is_admin() || ! is_user_logged_in() || ! $this->must_change_password( get_current_user_id() ) ) { | |
| 2134 | 2601 | return; |
| 2135 | 2602 | } |
| 2136 | 2603 | |
| 2137 | - $user_id = get_current_user_id(); | |
| 2138 | - $must_change = get_user_meta( $user_id, 'vigilante_must_change_password', true ); | |
| 2604 | + $this->redirect_to_profile(); | |
| 2605 | + } | |
| 2139 | 2606 | |
| 2140 | - if ( ! $must_change ) { | |
| 2141 | - return; | |
| 2607 | + /** | |
| 2608 | + * Let a user who must change an expired password reach the profile in a WooCommerce store | |
| 2609 | + * | |
| 2610 | + * WooCommerce sends accounts with no back-end role (a customer, a subscriber) | |
| 2611 | + * from wp-admin to My Account, and force_password_change_frontend() sends | |
| 2612 | + * them from there back to the profile. With one of those roles in Affected | |
| 2613 | + * Roles the two redirects chased each other until the browser gave up, from | |
| 2614 | + * 2.11.9 to 3.0.3, and the account could neither use the store nor change | |
| 2615 | + * the password. | |
| 2616 | + * | |
| 2617 | + * WooCommerce's redirect is lifted on the profile and nowhere else, and only | |
| 2618 | + * while the change is pending: from any other wp-admin screen the account | |
| 2619 | + * still ends up on the profile, and once the new password is saved | |
| 2620 | + * WooCommerce sends it back to My Account as it always does. | |
| 2621 | + * | |
| 2622 | + * What this depends on is not the caller's to choose: the user comes from | |
| 2623 | + * the session, whether the change is pending from must_change_password(), | |
| 2624 | + * and the request has to be the profile form and nothing else, which is | |
| 2625 | + * is_own_profile_request(), the same test the redirect uses. What it gives | |
| 2626 | + * is the user's own profile, which WordPress opens to any account that can | |
| 2627 | + * read (wp-admin/menu.php); WooCommerce's redirect is the store's | |
| 2628 | + * preference, not a permission. | |
| 2629 | + * | |
| 2630 | + * @param bool $prevent_access Whether WooCommerce is about to send this request out of wp-admin. | |
| 2631 | + * @return bool | |
| 2632 | + */ | |
| 2633 | + public function allow_profile_for_password_change( $prevent_access ) { | |
| 2634 | + if ( $prevent_access && is_user_logged_in() && $this->is_own_profile_request() && $this->must_change_password( get_current_user_id() ) ) { | |
| 2635 | + return false; | |
| 2142 | 2636 | } |
| 2143 | 2637 | |
| 2144 | - // Re-validate against current settings: the admin may have removed | |
| 2145 | - // this user's role from affected_roles or added the user to the | |
| 2146 | - // excluded list after the flag was set. Without this check the flag | |
| 2147 | - // outlives the configuration change and locks the user in a redirect | |
| 2148 | - // loop into profile.php. | |
| 2149 | - if ( ! $this->is_password_expiration_applicable( $user_id ) ) { | |
| 2150 | - delete_user_meta( $user_id, 'vigilante_must_change_password' ); | |
| 2151 | - return; | |
| 2638 | + return $prevent_access; | |
| 2639 | + } | |
| 2640 | + | |
| 2641 | + /** | |
| 2642 | + * Refuse XML-RPC calls from a user whose password has expired | |
| 2643 | + * | |
| 2644 | + * The last of the four non-wp-admin entry points. XML-RPC authenticates on | |
| 2645 | + * every call with the account credentials (a password or an application | |
| 2646 | + * password), so a session that never touches wp-admin could keep acting | |
| 2647 | + * through xmlrpc.php while the password sits expired. Scoped to XML-RPC | |
| 2648 | + * requests so an ordinary login, which the user needs to reach profile.php, | |
| 2649 | + * is never blocked here. Runs late on authenticate, after core and the | |
| 2650 | + * application-password handler have resolved the user. | |
| 2651 | + * | |
| 2652 | + * @since 2.11.9 | |
| 2653 | + * | |
| 2654 | + * @param WP_User|WP_Error|null $user Result of the earlier authentication. | |
| 2655 | + * @return WP_User|WP_Error|null | |
| 2656 | + */ | |
| 2657 | + public function block_expired_password_xmlrpc( $user ) { | |
| 2658 | + if ( ! ( defined( 'XMLRPC_REQUEST' ) && XMLRPC_REQUEST ) ) { | |
| 2659 | + return $user; | |
| 2152 | 2660 | } |
| 2153 | 2661 | |
| 2154 | - wp_safe_redirect( admin_url( 'profile.php#password' ) ); | |
| 2155 | - exit; | |
| 2662 | + if ( $user instanceof WP_User && $this->must_change_password( $user->ID ) ) { | |
| 2663 | + return new WP_Error( | |
| 2664 | + 'vigilante_password_expired', | |
| 2665 | + __( 'Your password has expired. Change it in your profile before using XML-RPC.', 'vigilante' ), | |
| 2666 | + array( 'status' => 403 ) | |
| 2667 | + ); | |
| 2668 | + } | |
| 2669 | + | |
| 2670 | + return $user; | |
| 2156 | 2671 | } |
| 2157 | 2672 | |
| 2158 | 2673 | /** |
| 2159 | 2674 | * Whether password expiration rules currently apply to a given user |
| @@ -2520,8 +3035,21 @@ | ||
| 2520 | 3035 | * |
| 2521 | 3036 | * @param int $user_id User ID. |
| 2522 | 3037 | */ |
| 2523 | 3038 | public function send_verification_email( $user_id ) { |
| 3039 | + /* | |
| 3040 | + * Never send an account that is already verified back to pending. The | |
| 3041 | + * resend link below reaches this, and while the pending value was | |
| 3042 | + * unreadable (see the note on the meta write) that was harmless; with | |
| 3043 | + * the check working, resending for a verified account would lock its | |
| 3044 | + * owner out of their own site. | |
| 3045 | + */ | |
| 3046 | + if ( metadata_exists( 'user', $user_id, 'vigilante_email_verified' ) | |
| 3047 | + && get_user_meta( $user_id, 'vigilante_email_verified', true ) | |
| 3048 | + ) { | |
| 3049 | + return; | |
| 3050 | + } | |
| 3051 | + | |
| 2524 | 3052 | $user = get_userdata( $user_id ); |
| 2525 | 3053 | if ( ! $user ) { |
| 2526 | 3054 | return; |
| 2527 | 3055 | } |
| @@ -2536,10 +3064,22 @@ | ||
| 2536 | 3064 | |
| 2537 | 3065 | // Store token |
| 2538 | 3066 | update_user_meta( $user_id, 'vigilante_verification_token', $token_hash ); |
| 2539 | 3067 | update_user_meta( $user_id, 'vigilante_verification_expires', $expires ); |
| 2540 | - update_user_meta( $user_id, 'vigilante_email_verified', false ); | |
| 2541 | 3068 | |
| 3069 | + /* | |
| 3070 | + * '0' and not false. update_user_meta() stores false as an empty string | |
| 3071 | + * (maybe_serialize() returns it unchanged and wpdb writes it with %s), and | |
| 3072 | + * an empty string is what get_user_meta() also returns when there is no | |
| 3073 | + * row at all. So from the moment this feature existed until 2.11.10 the | |
| 3074 | + * value written to mean "not verified yet" was read back as "this account | |
| 3075 | + * predates the feature, let it in", and the branch that blocks the login | |
| 3076 | + * was unreachable. Found by the file-by-file review of 2.11.10. '0' is | |
| 3077 | + * falsy in PHP and survives the round trip, and the readers below tell an | |
| 3078 | + * absent row from a stored one with metadata_exists(). | |
| 3079 | + */ | |
| 3080 | + update_user_meta( $user_id, 'vigilante_email_verified', '0' ); | |
| 3081 | + | |
| 2542 | 3082 | // Build verification URL |
| 2543 | 3083 | $verify_url = add_query_arg( |
| 2544 | 3084 | array( |
| 2545 | 3085 | 'vigilante_verify' => '1', |
| @@ -2613,16 +3153,21 @@ | ||
| 2613 | 3153 | if ( is_wp_error( $user ) ) { |
| 2614 | 3154 | return $user; |
| 2615 | 3155 | } |
| 2616 | 3156 | |
| 2617 | - // Check if email is verified | |
| 2618 | - $verified = get_user_meta( $user->ID, 'vigilante_email_verified', true ); | |
| 2619 | - | |
| 2620 | - // If no meta exists, user was created before this feature - allow | |
| 2621 | - if ( '' === $verified ) { | |
| 3157 | + /* | |
| 3158 | + * Only a row that does not exist means "created before this feature". | |
| 3159 | + * An existing row holding an empty string is an account that older | |
| 3160 | + * versions marked as pending, and it has to be blocked like any other: | |
| 3161 | + * reading both the same way is what made this check let everyone in | |
| 3162 | + * (see send_verification_email()). | |
| 3163 | + */ | |
| 3164 | + if ( ! metadata_exists( 'user', $user->ID, 'vigilante_email_verified' ) ) { | |
| 2622 | 3165 | return $user; |
| 2623 | 3166 | } |
| 2624 | 3167 | |
| 3168 | + $verified = get_user_meta( $user->ID, 'vigilante_email_verified', true ); | |
| 3169 | + | |
| 2625 | 3170 | if ( ! $verified ) { |
| 2626 | 3171 | $settings = $this->options['email_verification'] ?? array(); |
| 2627 | 3172 | $allow_resend = ! empty( $settings['allow_resend'] ); |
| 2628 | 3173 | |
| @@ -2664,10 +3209,19 @@ | ||
| 2664 | 3209 | |
| 2665 | 3210 | // Verify nonce to prevent CSRF and user-ID probing. |
| 2666 | 3211 | if ( ! isset( $_GET['_vigilante_nonce'] ) || |
| 2667 | 3212 | ! wp_verify_nonce( sanitize_text_field( wp_unslash( $_GET['_vigilante_nonce'] ) ), 'vigilante_resend_verification_' . $user_id ) ) { |
| 2668 | - wp_safe_redirect( add_query_arg( 'vigilante_message', 'invalid', wp_login_url() ) ); | |
| 2669 | - exit; | |
| 3213 | + /* | |
| 3214 | + * Nothing is redirected to the login page until the request | |
| 3215 | + * has proved something, and a bad nonce proves nothing. It | |
| 3216 | + * used to answer with a redirect to wp_login_url(), which | |
| 3217 | + * under a custom login URL IS the secret address, so any | |
| 3218 | + * visitor could read it out of the Location header of a | |
| 3219 | + * request carrying garbage. Found by the third cross review | |
| 3220 | + * of 2.11.10. Returning leaves the request to render the page | |
| 3221 | + * it asked for, which tells nobody anything. | |
| 3222 | + */ | |
| 3223 | + return; | |
| 2670 | 3224 | } |
| 2671 | 3225 | |
| 2672 | 3226 | // Rate limiting: allow 1 resend every 5 minutes per user to prevent email spam. |
| 2673 | 3227 | $transient_key = 'vigilante_resend_' . $user_id; |
| @@ -2688,28 +3242,30 @@ | ||
| 2688 | 3242 | $user_id = isset( $_GET['user_id'] ) ? absint( $_GET['user_id'] ) : 0; |
| 2689 | 3243 | // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- token-based verification below. |
| 2690 | 3244 | $token = isset( $_GET['token'] ) ? sanitize_text_field( wp_unslash( $_GET['token'] ) ) : ''; |
| 2691 | 3245 | |
| 3246 | + // Same as the resend above: no proof, no redirect, so the Location | |
| 3247 | + // header cannot be used to read the custom login URL. | |
| 2692 | 3248 | if ( ! $user_id || ! $token ) { |
| 2693 | - wp_safe_redirect( add_query_arg( 'vigilante_message', 'invalid', wp_login_url() ) ); | |
| 2694 | - exit; | |
| 3249 | + return; | |
| 2695 | 3250 | } |
| 2696 | 3251 | |
| 2697 | - $stored_hash = get_user_meta( $user_id, 'vigilante_verification_token', true ); | |
| 2698 | - $expires = get_user_meta( $user_id, 'vigilante_verification_expires', true ); | |
| 3252 | + $stored_hash = (string) get_user_meta( $user_id, 'vigilante_verification_token', true ); | |
| 3253 | + $expires = (int) get_user_meta( $user_id, 'vigilante_verification_expires', true ); | |
| 2699 | 3254 | |
| 2700 | - // Check expiration | |
| 3255 | + // The token first. Checking the expiry before it answered "expired" for | |
| 3256 | + // any account with no verification pending and "invalid" for one waiting, | |
| 3257 | + // so a wrong link revealed which user ids were waiting (2.11.8). Only the | |
| 3258 | + // holder of the right token learns that it expired. | |
| 3259 | + if ( '' === $stored_hash || ! hash_equals( $stored_hash, wp_hash( $token ) ) ) { | |
| 3260 | + return; | |
| 3261 | + } | |
| 3262 | + | |
| 2701 | 3263 | if ( time() > $expires ) { |
| 2702 | 3264 | wp_safe_redirect( add_query_arg( 'vigilante_message', 'expired', wp_login_url() ) ); |
| 2703 | 3265 | exit; |
| 2704 | 3266 | } |
| 2705 | 3267 | |
| 2706 | - // Verify token | |
| 2707 | - if ( ! hash_equals( $stored_hash, wp_hash( $token ) ) ) { | |
| 2708 | - wp_safe_redirect( add_query_arg( 'vigilante_message', 'invalid', wp_login_url() ) ); | |
| 2709 | - exit; | |
| 2710 | - } | |
| 2711 | - | |
| 2712 | 3268 | // Mark as verified |
| 2713 | 3269 | update_user_meta( $user_id, 'vigilante_email_verified', true ); |
| 2714 | 3270 | delete_user_meta( $user_id, 'vigilante_verification_token' ); |
| 2715 | 3271 | delete_user_meta( $user_id, 'vigilante_verification_expires' ); |
| @@ -2730,12 +3286,11 @@ | ||
| 2730 | 3286 | 'info' |
| 2731 | 3287 | ); |
| 2732 | 3288 | } |
| 2733 | 3289 | |
| 2734 | - // Check if user still needs approval | |
| 2735 | - $is_pending = get_user_meta( $user_id, 'vigilante_pending_approval', true ); | |
| 2736 | - | |
| 2737 | - if ( $is_pending ) { | |
| 3290 | + // Anywhere on the network, so the message matches what will actually | |
| 3291 | + // happen at the login: that is what blocks (see is_pending_anywhere()). | |
| 3292 | + if ( self::is_pending_anywhere( $user_id ) ) { | |
| 2738 | 3293 | // User verified but still pending approval |
| 2739 | 3294 | wp_safe_redirect( |
| 2740 | 3295 | add_query_arg( |
| 2741 | 3296 | array( |
| @@ -2796,15 +3351,17 @@ | ||
| 2796 | 3351 | * @param int $user_id User ID. |
| 2797 | 3352 | * @return bool |
| 2798 | 3353 | */ |
| 2799 | 3354 | public function is_email_verified( $user_id ) { |
| 2800 | - $verified = get_user_meta( $user_id, 'vigilante_email_verified', true ); | |
| 2801 | - | |
| 2802 | - // If no meta exists, consider verified (old users) | |
| 2803 | - if ( '' === $verified ) { | |
| 3355 | + // Same reading as block_unverified_user_login(): only an absent row means | |
| 3356 | + // the account predates the feature. A stored empty string is an account | |
| 3357 | + // an older version left pending. | |
| 3358 | + if ( ! metadata_exists( 'user', $user_id, 'vigilante_email_verified' ) ) { | |
| 2804 | 3359 | return true; |
| 2805 | 3360 | } |
| 2806 | 3361 | |
| 3362 | + $verified = get_user_meta( $user_id, 'vigilante_email_verified', true ); | |
| 3363 | + | |
| 2807 | 3364 | return (bool) $verified; |
| 2808 | 3365 | } |
| 2809 | 3366 | |
| 2810 | 3367 | /* ========================================================================= |
| @@ -2826,9 +3383,9 @@ | ||
| 2826 | 3383 | |
| 2827 | 3384 | // Check if this user's role requires approval |
| 2828 | 3385 | $needs_approval = false; |
| 2829 | 3386 | if ( ! empty( $registration_approval['enabled'] ) ) { |
| 2830 | - $affected_roles = $registration_approval['affected_roles'] ?? array( 'subscriber' ); | |
| 3387 | + $affected_roles = Vigilante_Settings::list_or_shipped( $registration_approval['affected_roles'] ?? array(), array( 'user_security', 'registration_approval', 'affected_roles' ) ); | |
| 2831 | 3388 | $needs_approval = ! empty( array_intersect( $user->roles, $affected_roles ) ); |
| 2832 | 3389 | } |
| 2833 | 3390 | |
| 2834 | 3391 | // Check if email verification is enabled |
| @@ -2940,9 +3497,9 @@ | ||
| 2940 | 3497 | if ( empty( $registration_approval['enabled'] ) ) { |
| 2941 | 3498 | return false; |
| 2942 | 3499 | } |
| 2943 | 3500 | |
| 2944 | - $affected_roles = $registration_approval['affected_roles'] ?? array( 'subscriber' ); | |
| 3501 | + $affected_roles = Vigilante_Settings::list_or_shipped( $registration_approval['affected_roles'] ?? array(), array( 'user_security', 'registration_approval', 'affected_roles' ) ); | |
| 2945 | 3502 | return ! empty( array_intersect( $user->roles, $affected_roles ) ); |
| 2946 | 3503 | } |
| 2947 | 3504 | |
| 2948 | 3505 | /** |
