| @@ -14,9 +14,11 @@ | ||
| 14 | 14 | |
| 15 | 15 | class AuthController extends Controller |
| 16 | 16 | { |
| 17 | 17 | /** |
| 18 | - * signUp method will create new user submitted data from sign up form | |
| 18 | + * Two steps: the first submit emails a verification code, the second | |
| 19 | + * verifies it and creates the user. | |
| 20 | + * | |
| 19 | 21 | * @param Request $request |
| 20 | 22 | * @return \WP_REST_Response |
| 21 | 23 | * @throws \FluentSupport\Framework\Validator\ValidationException |
| 22 | 24 | */ |
| @@ -28,8 +30,14 @@ | ||
| 28 | 30 | 'message' => __('You are not allowed to signup using this form', 'fluent-support') |
| 29 | 31 | ]); |
| 30 | 32 | } |
| 31 | 33 | |
| 34 | + if (!Helper::isSignupEnabled()) { | |
| 35 | + return $this->sendError([ | |
| 36 | + 'message' => __('User registration is not enabled on this site.', 'fluent-support') | |
| 37 | + ], 403); | |
| 38 | + } | |
| 39 | + | |
| 32 | 40 | if (!wp_verify_nonce($request->getSafe('_fsupport_signup_nonce', 'sanitize_text_field'), 'fluent_support_signup_nonce')) { |
| 33 | 41 | return $this->sendError([ |
| 34 | 42 | 'message' => __('Security verification failed. Please try again', 'fluent-support') |
| 35 | 43 | ]); |
| @@ -56,12 +64,10 @@ | ||
| 56 | 64 | * @param array $formData |
| 57 | 65 | */ |
| 58 | 66 | do_action('fluent_support/before_signup_validation', $formData); |
| 59 | 67 | |
| 60 | - // the verification code is only submitted on the second step, after the customer has | |
| 61 | - // already passed the captcha and received the email; that step validates the hash | |
| 62 | - // against an issued record below, so the captcha can be skipped there. The first | |
| 63 | - // submit (which sends the verification email) always has to pass the captcha. | |
| 68 | + // The captcha was already passed on the first step, and the second step is | |
| 69 | + // gated by the issued verification record below. | |
| 64 | 70 | $isVerificationStep = !empty($formData['_email_verification_token']); |
| 65 | 71 | |
| 66 | 72 | $checkRecaptchaAvailability = ReCaptchaHandler::isRecaptchaApplicable('signup_form'); |
| 67 | 73 | if ($checkRecaptchaAvailability && !$isVerificationStep) { |
| @@ -102,9 +108,8 @@ | ||
| 102 | 108 | 'message' => __('Please provide a valid verification code that was sent to your email address', 'fluent-support') |
| 103 | 109 | ], 422); |
| 104 | 110 | } |
| 105 | 111 | |
| 106 | - // the code must still be unused and must not have been consumed by a prior request | |
| 107 | 112 | if (($logHash['status'] ?? '') !== 'issued') { |
| 108 | 113 | wp_send_json([ |
| 109 | 114 | 'message' => __('Your verification code has already been used. Please try again', 'fluent-support') |
| 110 | 115 | ], 422); |
| @@ -109,10 +114,9 @@ | ||
| 109 | 114 | 'message' => __('Your verification code has already been used. Please try again', 'fluent-support') |
| 110 | 115 | ], 422); |
| 111 | 116 | } |
| 112 | 117 | |
| 113 | - // records created before the email-binding fix (or any other legacy/malformed record) | |
| 114 | - // have no bound email; treat them as invalid rather than proceeding with a null email | |
| 118 | + // Old or malformed records have no bound email. | |
| 115 | 119 | if (empty($logHash['email'])) { |
| 116 | 120 | wp_send_json([ |
| 117 | 121 | 'message' => __('Your verification code has expired. Please request a new one', 'fluent-support') |
| 118 | 122 | ], 422); |
| @@ -117,14 +121,10 @@ | ||
| 117 | 121 | 'message' => __('Your verification code has expired. Please request a new one', 'fluent-support') |
| 118 | 122 | ], 422); |
| 119 | 123 | } |
| 120 | 124 | |
| 121 | - // check if it got expired or not | |
| 122 | - // valid_till is written via gmdate() on a current_time('timestamp') basis (always | |
| 123 | - // UTC-equivalent, since WP resets the runtime timezone to UTC on every request), so | |
| 124 | - // it must be parsed back as UTC here - otherwise strtotime() would silently reinterpret | |
| 125 | - // it under whatever timezone a plugin/theme may have switched to via | |
| 126 | - // date_default_timezone_set() without restoring it, causing false expiry (or non-expiry) | |
| 125 | + // valid_till is written with gmdate(). Parse it as UTC explicitly, since a plugin | |
| 126 | + // may have changed the PHP default timezone. | |
| 127 | 127 | $validTill = $logHash['valid_till'] ?? ''; |
| 128 | 128 | if (($logHash['used_count'] ?? 0) > 5 || ($validTill && strtotime($validTill . ' UTC') < current_time('timestamp'))) { |
| 129 | 129 | wp_send_json([ |
| 130 | 130 | 'message' => __('Your verification code has been expired. Please try again', 'fluent-support') |
| @@ -142,10 +142,10 @@ | ||
| 142 | 142 | 'message' => __('Please provide a valid verification code that was sent to your email address', 'fluent-support') |
| 143 | 143 | ], 422); |
| 144 | 144 | } |
| 145 | 145 | |
| 146 | - // atomically consume the code: only succeeds if the record is still in the exact | |
| 147 | - // state we just read, closing the race where two requests both pass the checks above | |
| 146 | + // Consume atomically: the update only matches if the record is unchanged since | |
| 147 | + // we read it, so two concurrent requests can't both succeed. | |
| 148 | 148 | $consumed = Meta::where('key', $logHash['login_hash']) |
| 149 | 149 | ->where('object_type', 'fs_login_hashes') |
| 150 | 150 | ->where('value', $logHashMeta->value) |
| 151 | 151 | ->update([ |
| @@ -160,11 +160,9 @@ | ||
| 160 | 160 | 'message' => __('Your verification code has already been used. Please try again', 'fluent-support') |
| 161 | 161 | ], 422); |
| 162 | 162 | } |
| 163 | 163 | |
| 164 | - // the email is now server-verified for this code; ignore whatever the client | |
| 165 | - // submitted and use the address the code was actually issued to, so the signup | |
| 166 | - // can never be completed against a different (e.g. victim's) email address | |
| 164 | + // Use the address the code was issued to, not the submitted one. | |
| 167 | 165 | $formData['email'] = $logHash['email']; |
| 168 | 166 | } |
| 169 | 167 | |
| 170 | 168 | /* |
| @@ -176,13 +174,13 @@ | ||
| 176 | 174 | do_action('fluent_support/after_signup_validation', $formData); |
| 177 | 175 | |
| 178 | 176 | $userId = $this->createUser($formData); |
| 179 | 177 | |
| 180 | - if (is_wp_error($userId)) { | |
| 178 | + if (!$userId) { | |
| 181 | 179 | return $this->response( |
| 182 | 180 | apply_filters( |
| 183 | 181 | 'fluent_support/signup_create_user_error', |
| 184 | - ['error' => $userId->get_error_message()] | |
| 182 | + ['error' => __('Could not create your account. The username or email address may already be registered.', 'fluent-support')] | |
| 185 | 183 | ), 423); |
| 186 | 184 | } |
| 187 | 185 | |
| 188 | 186 | /* |
| @@ -188,9 +186,8 @@ | ||
| 188 | 186 | /* |
| 189 | 187 | * Action After creating WP user from ticket sign up form |
| 190 | 188 | * |
| 191 | 189 | * @since v1.0.0 |
| 192 | - * @param array $formData | |
| 193 | 190 | */ |
| 194 | 191 | do_action('fluent_support/after_creating_user'); |
| 195 | 192 | |
| 196 | 193 | $this->maybeUpdateUser($userId, $formData); |
| @@ -208,13 +205,18 @@ | ||
| 208 | 205 | 'message' => __('Successfully registered to the site.', 'fluent-support'), |
| 209 | 206 | 'redirect' => Arr::get($formData, '__redirect_to', Helper::getPortalBaseUrl()) |
| 210 | 207 | ]); |
| 211 | 208 | |
| 209 | + // The browser navigates to this by script; keep it on this site. | |
| 210 | + if (!empty($response['redirect'])) { | |
| 211 | + $portalUrl = Helper::getPortalBaseUrl(); | |
| 212 | + $response['redirect'] = wp_validate_redirect(esc_url_raw($response['redirect']), $portalUrl) ?: $portalUrl; | |
| 213 | + } | |
| 214 | + | |
| 212 | 215 | return $this->response($response); |
| 213 | 216 | } |
| 214 | 217 | |
| 215 | 218 | /** |
| 216 | - * handleLogin method will perform login functionality and redirect | |
| 217 | 219 | * @param Request $request |
| 218 | 220 | * @return \WP_REST_Response |
| 219 | 221 | */ |
| 220 | 222 | public function handleLogin(Request $request) |
| @@ -253,9 +255,9 @@ | ||
| 253 | 255 | if ($redirect = $request->getSafe('redirect_to', 'sanitize_text_field')) { |
| 254 | 256 | $redirectUrl = wp_validate_redirect($redirect, $redirectUrl); |
| 255 | 257 | } |
| 256 | 258 | |
| 257 | - if (get_current_user_id()) { // user already registered | |
| 259 | + if (get_current_user_id()) { // already logged in | |
| 258 | 260 | return $this->sendSuccess([ |
| 259 | 261 | 'redirect' => $redirectUrl |
| 260 | 262 | ]); |
| 261 | 263 | } |
| @@ -268,9 +270,9 @@ | ||
| 268 | 270 | } else { |
| 269 | 271 | $user = get_user_by('login', $email); |
| 270 | 272 | } |
| 271 | 273 | |
| 272 | - // Rate limiting: per-IP bucket (5 attempts) + per-account bucket (20 attempts) | |
| 274 | + // 5 attempts per account+IP, 20 per account. | |
| 273 | 275 | $ip = Helper::getIp(); |
| 274 | 276 | $ipKey = $user |
| 275 | 277 | ? 'fs_login_ip_' . wp_hash($user->ID . '|' . $ip) |
| 276 | 278 | : 'fs_login_ip_' . wp_hash(strtolower($email) . '|' . $ip); |
| @@ -323,9 +325,8 @@ | ||
| 323 | 325 | 'message' => $user->get_error_message() |
| 324 | 326 | ], 403); |
| 325 | 327 | } |
| 326 | 328 | |
| 327 | - // Clear rate limits for the authenticated user | |
| 328 | 329 | $authIpKey = 'fs_login_ip_' . wp_hash($user->ID . '|' . $ip); |
| 329 | 330 | $authAccountKey = 'fs_login_act_' . wp_hash($user->ID); |
| 330 | 331 | delete_transient($authIpKey); |
| 331 | 332 | delete_transient($authAccountKey); |
| @@ -386,9 +387,9 @@ | ||
| 386 | 387 | } |
| 387 | 388 | |
| 388 | 389 | $user_signon = wp_signon($info, $secure_cookie); |
| 389 | 390 | |
| 390 | - // Note: No sanitization needed here as we're only checking emptiness, not using the cookie value | |
| 391 | + // Only checked for emptiness, so no sanitizing needed. | |
| 391 | 392 | if (!is_wp_error($user_signon) && empty($_COOKIE[LOGGED_IN_COOKIE])) { |
| 392 | 393 | if (headers_sent()) { |
| 393 | 394 | return $this->response([ |
| 394 | 395 | // translators: %1$s is the URL to WordPress cookies documentation, %2$s is the URL to WordPress support forums |
| @@ -427,9 +428,8 @@ | ||
| 427 | 428 | ]); |
| 428 | 429 | } |
| 429 | 430 | |
| 430 | 431 | /** |
| 431 | - * getRules method will prepare the rules for the input field | |
| 432 | 432 | * @param array $fields |
| 433 | 433 | * @return mixed |
| 434 | 434 | */ |
| 435 | 435 | protected function getRules($fields = []) |
| @@ -479,16 +479,14 @@ | ||
| 479 | 479 | $usernameOrEmail = trim(wp_unslash($request->getSafe('user_login', 'sanitize_text_field'))); |
| 480 | 480 | |
| 481 | 481 | if (!$usernameOrEmail) { |
| 482 | 482 | return $this->sendError([ |
| 483 | - 'message' => 'Username or email is required' | |
| 483 | + 'message' => __('Username or email is required.', 'fluent-support') | |
| 484 | 484 | ]); |
| 485 | 485 | } |
| 486 | 486 | |
| 487 | - // IP bucket is a generous volumetric backstop (shared office/NAT IPs can have many | |
| 488 | - // unrelated users). It runs before the account lookup so that probes for accounts | |
| 489 | - // that don't exist are throttled too. Keyed on the IP only, so a 429 here reveals | |
| 490 | - // nothing about whether any given account exists. | |
| 487 | + // Generous because of shared NAT IPs. Runs before the account lookup so probes for | |
| 488 | + // missing accounts are throttled too, and a 429 reveals nothing about existence. | |
| 491 | 489 | if (Helper::hitRateLimit('fs_reset_pass_ip_' . wp_hash(Helper::getIp()), 20)) { |
| 492 | 490 | return $this->sendError([ |
| 493 | 491 | 'message' => __('Too many password reset requests. Please try again after 15 minutes.', 'fluent-support') |
| 494 | 492 | ], 429); |
| @@ -542,13 +540,10 @@ | ||
| 542 | 540 | */ |
| 543 | 541 | // translators: %s is the site name |
| 544 | 542 | $linkText = apply_filters("fluent_support/reset_password_link", sprintf(__('Reset your password for %s', 'fluent-support'), get_bloginfo('name'))); |
| 545 | 543 | |
| 546 | - // Issuance cooldown. get_password_reset_key() rotates the stored key, invalidating | |
| 547 | - // any link already sitting in the account owner's inbox, so an unthrottled caller | |
| 548 | - // could deny password recovery indefinitely. Suppressing the duplicate issuance is | |
| 549 | - // safe: reset mail only ever goes to the account owner, so whoever triggered the | |
| 550 | - // first send has already put a working link in that inbox. | |
| 544 | + // get_password_reset_key() invalidates the previous link, so repeated requests could | |
| 545 | + // block recovery. The owner already has a working link from the first send. | |
| 551 | 546 | $cooldownKey = 'fs_reset_pass_sent_' . wp_hash($user_data->ID); |
| 552 | 547 | |
| 553 | 548 | if (get_transient($cooldownKey)) { |
| 554 | 549 | return $this->sendResetPassResponse(); |
| @@ -598,14 +593,10 @@ | ||
| 598 | 593 | return $this->sendResetPassResponse(); |
| 599 | 594 | } |
| 600 | 595 | |
| 601 | 596 | /** |
| 602 | - * Single response for every password reset outcome. | |
| 597 | + * Same response for every outcome, so the form can't confirm which accounts exist. | |
| 603 | 598 | * |
| 604 | - * Whether the account exists, is disallowed, is not a member of this site or is | |
| 605 | - * inside the resend cooldown, the caller sees the same thing — otherwise the form | |
| 606 | - * confirms which usernames and email addresses are real. | |
| 607 | - * | |
| 608 | 599 | * @return mixed |
| 609 | 600 | */ |
| 610 | 601 | protected function sendResetPassResponse() |
| 611 | 602 | { |
| @@ -614,9 +605,8 @@ | ||
| 614 | 605 | ]); |
| 615 | 606 | } |
| 616 | 607 | |
| 617 | 608 | /** |
| 618 | - * getMessages message will return the validation message regarding sign up or sign in | |
| 619 | 609 | * @param array $rules |
| 620 | 610 | * @return mixed |
| 621 | 611 | */ |
| 622 | 612 | protected function getMessages($rules = []) |
| @@ -631,11 +621,10 @@ | ||
| 631 | 621 | return apply_filters('fluent_support/signup_validation_messages', [], $rules); |
| 632 | 622 | } |
| 633 | 623 | |
| 634 | 624 | /** |
| 635 | - * createUser method will create new user | |
| 636 | 625 | * @param array $formData |
| 637 | - * @return mixed | |
| 626 | + * @return int|false | |
| 638 | 627 | */ |
| 639 | 628 | public function createUser($formData = []) |
| 640 | 629 | { |
| 641 | 630 | /* |
| @@ -688,14 +677,19 @@ | ||
| 688 | 677 | |
| 689 | 678 | } |
| 690 | 679 | |
| 691 | 680 | /** |
| 692 | - * maybeUpdateUser method will update user information if exists | |
| 693 | - * @param $userId | |
| 694 | - * @param $formData | |
| 681 | + * Sets the name fields from first/last name, if given. | |
| 682 | + * | |
| 683 | + * @param int $userId | |
| 684 | + * @param array $formData | |
| 695 | 685 | */ |
| 696 | 686 | public function maybeUpdateUser($userId, $formData) |
| 697 | 687 | { |
| 688 | + if (!$userId) { | |
| 689 | + return; | |
| 690 | + } | |
| 691 | + | |
| 698 | 692 | $firstName = Arr::get($formData, 'first_name', ''); |
| 699 | 693 | $lastName = Arr::get($formData, 'last_name', ''); |
| 700 | 694 | $name = trim($firstName . ' ' . $lastName); |
| 701 | 695 | |
| @@ -735,9 +729,9 @@ | ||
| 735 | 729 | } |
| 736 | 730 | } |
| 737 | 731 | |
| 738 | 732 | public function addUserMetaData($userId, $formData) { |
| 739 | - $customFieldsKey = apply_filters('fluent_support/custom_registration_form_fields_key', Helper::getBusinessSettings('custom_registration_form_field')); | |
| 733 | + $customFieldsKey = Helper::getCustomRegistrationFieldKeys(); | |
| 740 | 734 | |
| 741 | 735 | if (empty($customFieldsKey)) { |
| 742 | 736 | return; |
| 743 | 737 | } |
| @@ -742,18 +736,16 @@ | ||
| 742 | 736 | return; |
| 743 | 737 | } |
| 744 | 738 | |
| 745 | 739 | foreach ($customFieldsKey as $key) { |
| 746 | - if (isset($formData[$key])) { | |
| 747 | - $fieldValue = $formData[$key]; | |
| 748 | - update_user_meta($userId, $key, $fieldValue); | |
| 740 | + if (isset($formData[$key]) && is_scalar($formData[$key])) { | |
| 741 | + update_user_meta($userId, $key, sanitize_text_field($formData[$key])); | |
| 749 | 742 | } |
| 750 | 743 | } |
| 751 | 744 | } |
| 752 | 745 | |
| 753 | 746 | /** |
| 754 | - * assignRole method will assign role to a given user id | |
| 755 | - * @param $userId | |
| 747 | + * @param int $userId | |
| 756 | 748 | */ |
| 757 | 749 | protected function assignRole($userId) |
| 758 | 750 | { |
| 759 | 751 | $user = new \WP_User($userId); |
| @@ -761,9 +753,9 @@ | ||
| 761 | 753 | /* |
| 762 | 754 | * Action before assigning role to registered user |
| 763 | 755 | * |
| 764 | 756 | * @since v1.0.0 |
| 765 | - * @param array $data | |
| 757 | + * @param \WP_User $user | |
| 766 | 758 | */ |
| 767 | 759 | do_action('fluent_support/before_assigning_role', $user); |
| 768 | 760 | /* |
| 769 | 761 | * Filter user assignable role after signup |
| @@ -777,9 +769,9 @@ | ||
| 777 | 769 | /* |
| 778 | 770 | * Action after assigning role to registered user |
| 779 | 771 | * |
| 780 | 772 | * @since v1.0.0 |
| 781 | - * @param array $data | |
| 773 | + * @param \WP_User $user | |
| 782 | 774 | */ |
| 783 | 775 | do_action('fluent_support/after_assigning_role', $user); |
| 784 | 776 | } |
| 785 | 777 | |
| @@ -784,10 +776,9 @@ | ||
| 784 | 776 | } |
| 785 | 777 | |
| 786 | 778 | |
| 787 | 779 | /** |
| 788 | - * login method will clear existing cookies and set new cookie for a given user id | |
| 789 | - * @param $userId | |
| 780 | + * @param int $userId | |
| 790 | 781 | */ |
| 791 | 782 | protected function login($userId) |
| 792 | 783 | { |
| 793 | 784 | /* |
