From 2fea795673e6c11d2a4ef18ffce47e6967b8b007 Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Mon, 15 Jun 2026 23:26:41 +0600 Subject: [PATCH 1/9] fix(customer): restore storefront login and session auth (#285) Unify token/session binding after login and registration, restore SSR auth from cookie without minting guest tokens, and align email/password handling between manager and storefront. --- .../minishop3/js/web/core/ApiClient.js | 13 +++ .../components/minishop3/js/web/ui/AuthUI.js | 7 +- .../Api/Manager/CustomersController.php | 15 ++- .../Controllers/Auth/PasswordAuthProvider.php | 10 +- .../src/Middleware/TokenMiddleware.php | 6 +- .../src/Processors/Api/Customer/Login.php | 73 ++------------ .../src/Processors/Api/Customer/Register.php | 68 ++----------- .../src/Services/Customer/AuthManager.php | 63 ++++++++++++ .../Services/Customer/CustomerPageService.php | 11 +++ .../src/Services/Customer/RegisterService.php | 2 +- .../minishop3/src/Services/TokenService.php | 97 ++++++++++++------- .../minishop3/src/Utils/SessionHelper.php | 16 +++ 12 files changed, 211 insertions(+), 170 deletions(-) create mode 100644 core/components/minishop3/src/Utils/SessionHelper.php diff --git a/assets/components/minishop3/js/web/core/ApiClient.js b/assets/components/minishop3/js/web/core/ApiClient.js index 1bbad2b8..a654cc26 100644 --- a/assets/components/minishop3/js/web/core/ApiClient.js +++ b/assets/components/minishop3/js/web/core/ApiClient.js @@ -89,6 +89,19 @@ class ApiClient { return tokenErrors.includes(result.message) } + /** + * Web API returns payload in `data`; legacy processors used `object`. + * + * @param {Object|null|undefined} result - API response + * @returns {Object|null} + */ + static getPayload (result) { + if (!result) { + return null + } + return result.data ?? result.object ?? null + } + /** * GET request * diff --git a/assets/components/minishop3/js/web/ui/AuthUI.js b/assets/components/minishop3/js/web/ui/AuthUI.js index a4810100..07cd42db 100644 --- a/assets/components/minishop3/js/web/ui/AuthUI.js +++ b/assets/components/minishop3/js/web/ui/AuthUI.js @@ -109,7 +109,7 @@ class AuthUI { if (result.success) { this.showMessage('login-messages', this.t('ms3_customer_login_success'), 'success') setTimeout(() => { - this.handleRedirect(result.object) + this.handleRedirect(ApiClient.getPayload(result)) }, 1000) } else { this.showMessage('login-messages', result.message || this.t('ms3_err_unknown'), 'danger') @@ -177,9 +177,10 @@ class AuthUI { 'success' ) - if (result.object && result.object.token) { + const payload = ApiClient.getPayload(result) + if (payload && payload.token) { setTimeout(() => { - this.handleRedirect(result.object) + this.handleRedirect(payload) }, 1500) } else { setTimeout(() => { diff --git a/core/components/minishop3/src/Controllers/Api/Manager/CustomersController.php b/core/components/minishop3/src/Controllers/Api/Manager/CustomersController.php index bf009ff2..1c4ffe77 100644 --- a/core/components/minishop3/src/Controllers/Api/Manager/CustomersController.php +++ b/core/components/minishop3/src/Controllers/Api/Manager/CustomersController.php @@ -2,6 +2,7 @@ namespace MiniShop3\Controllers\Api\Manager; +use MiniShop3\Controllers\Auth\PasswordAuthProvider; use MiniShop3\Model\msCustomer; use MiniShop3\Router\HttpStatus; use MiniShop3\Router\Response; @@ -177,13 +178,19 @@ public function update(array $data = []): array foreach ($allowedFields as $field) { if (isset($data[$field])) { - $customer->set($field, $data[$field]); + $value = $data[$field]; + if ($field === 'email' && is_string($value)) { + $value = PasswordAuthProvider::normalizeEmail($value); + } + $customer->set($field, $value); } } - if (!empty($data['password'])) { - $hashedPassword = password_hash($data['password'], PASSWORD_DEFAULT); - $customer->set('password', $hashedPassword); + if (isset($data['password'])) { + $password = trim((string)$data['password']); + if ($password !== '') { + $customer->set('password', PasswordAuthProvider::hashPassword($password)); + } } if (!$customer->save()) { diff --git a/core/components/minishop3/src/Controllers/Auth/PasswordAuthProvider.php b/core/components/minishop3/src/Controllers/Auth/PasswordAuthProvider.php index 513d1b72..516f7485 100644 --- a/core/components/minishop3/src/Controllers/Auth/PasswordAuthProvider.php +++ b/core/components/minishop3/src/Controllers/Auth/PasswordAuthProvider.php @@ -40,6 +40,14 @@ public function __construct(modX $modx) $this->modx = $modx; } + /** + * Normalize email for lookup and storage. + */ + public static function normalizeEmail(string $email): string + { + return strtolower(trim($email)); + } + /** * Authenticate by email and password * @@ -48,7 +56,7 @@ public function __construct(modX $modx) */ public function authenticate(array $credentials): ?msCustomer { - $email = trim($credentials['email'] ?? ''); + $email = self::normalizeEmail($credentials['email'] ?? ''); $password = $credentials['password'] ?? ''; if (empty($email) || empty($password)) { diff --git a/core/components/minishop3/src/Middleware/TokenMiddleware.php b/core/components/minishop3/src/Middleware/TokenMiddleware.php index 46a0c6cd..e67cd950 100644 --- a/core/components/minishop3/src/Middleware/TokenMiddleware.php +++ b/core/components/minishop3/src/Middleware/TokenMiddleware.php @@ -7,6 +7,7 @@ use MiniShop3\Router\Response; use MiniShop3\Services\TokenService; use MiniShop3\Utils\CookieHelper; +use MiniShop3\Utils\SessionHelper; use MODX\Revolution\modX; /** @@ -71,10 +72,7 @@ public function handle(array $params) $uri = $_SERVER['REQUEST_URI'] ?? '/'; $isPublic = $this->isPublicRoute($uri); - // Ensure session is active - if (session_status() !== PHP_SESSION_ACTIVE) { - session_start(); - } + SessionHelper::ensureActive(); // For non-public routes: check session first if (!$isPublic && !empty($_SESSION['ms3']['customer_id'])) { diff --git a/core/components/minishop3/src/Processors/Api/Customer/Login.php b/core/components/minishop3/src/Processors/Api/Customer/Login.php index a602f80e..31461242 100644 --- a/core/components/minishop3/src/Processors/Api/Customer/Login.php +++ b/core/components/minishop3/src/Processors/Api/Customer/Login.php @@ -2,11 +2,9 @@ namespace MiniShop3\Processors\Api\Customer; -use MiniShop3\Model\msCustomerToken; +use MiniShop3\Controllers\Auth\PasswordAuthProvider; use MiniShop3\Services\Customer\AuthManager; use MiniShop3\Services\Customer\RateLimiter; -use MiniShop3\Services\Order\OrderDraftManager; -use MiniShop3\Utils\CookieHelper; use MODX\Revolution\Processors\Processor; /** @@ -27,10 +25,10 @@ public function process() { $this->modx->lexicon->load('minishop3:customer'); - $email = trim($this->getProperty('email', '')); + $email = PasswordAuthProvider::normalizeEmail($this->getProperty('email', '')); $password = $this->getProperty('password', ''); - if (empty($email) || empty($password)) { + if ($email === '' || $password === '') { return $this->failure($this->modx->lexicon('ms3_customer_err_login_required')); } @@ -71,66 +69,11 @@ public function process() $rateLimiter->reset('login', $ip); - // Use existing token from cookie/session instead of creating new one - $currentToken = CookieHelper::getTokenFromCookie(); - if (empty($currentToken)) { - $currentToken = $_SESSION['ms3']['customer_token'] ?? ''; + $session = $authManager->establishCustomerSession($customer); + if (!$session) { + return $this->failure($this->modx->lexicon('ms3_customer_err_token_create')); } - $tokenObj = null; - $tokenString = ''; - $expiresAt = ''; - - if (!empty($currentToken)) { - // Find existing token in DB - $tokenObj = $this->modx->getObject(msCustomerToken::class, [ - 'token' => $currentToken, - 'type' => msCustomerToken::TYPE_API, - ]); - - if ($tokenObj) { - // Bind customer to existing token - $tokenObj->set('customer_id', $customer->id); - - // Extend TTL - $ttl = (int)$this->modx->getOption('ms3_customer_token_ttl', null, 604800); - $tokenObj->set('expires_at', date('Y-m-d H:i:s', time() + $ttl)); - $tokenObj->save(); - - $tokenString = $tokenObj->get('token'); - $expiresAt = $tokenObj->get('expires_at'); - - // Bind draft order to customer - /** @var OrderDraftManager $draftManager */ - $draftManager = $this->modx->services->get('ms3_order_draft_manager'); - $draftManager->bindDraftToCustomer($tokenString, $customer->id); - } - } - - // Edge case: no valid existing token — create new one - if (!$tokenObj) { - $ttl = (int)$this->modx->getOption('ms3_customer_token_ttl', null, 604800); - $tokenObj = $authManager->createToken($customer, 'api', $ttl); - - if (!$tokenObj) { - return $this->failure($this->modx->lexicon('ms3_customer_err_token_create')); - } - - $tokenString = $tokenObj->get('token'); - $expiresAt = $tokenObj->get('expires_at'); - - // Set cookie for new token - CookieHelper::setTokenCookie($this->modx, $tokenString); - } - - // Update session - if (!isset($_SESSION['ms3'])) { - $_SESSION['ms3'] = []; - } - $_SESSION['ms3']['customer_id'] = $customer->id; - $_SESSION['ms3']['customer_token'] = $tokenString; - $_SESSION['ms3']['customer_token_expires'] = strtotime($expiresAt); - $redirectPageId = (int)$this->getProperty('redirect_page_id', 0); if (!$redirectPageId) { $redirectPageId = (int)$this->modx->getOption('ms3_customer_redirect_after_login', null, 0); @@ -150,8 +93,8 @@ public function process() 'phone' => $customer->get('phone'), 'email_verified' => !empty($customer->get('email_verified_at')), ], - 'token' => $tokenString, - 'expires_at' => $expiresAt, + 'token' => $session['token'], + 'expires_at' => $session['expires_at'], 'redirect_url' => $redirectUrl, ]); } diff --git a/core/components/minishop3/src/Processors/Api/Customer/Register.php b/core/components/minishop3/src/Processors/Api/Customer/Register.php index 76252f16..499b3fac 100644 --- a/core/components/minishop3/src/Processors/Api/Customer/Register.php +++ b/core/components/minishop3/src/Processors/Api/Customer/Register.php @@ -2,13 +2,11 @@ namespace MiniShop3\Processors\Api\Customer; -use MiniShop3\Model\msCustomerToken; +use MiniShop3\Controllers\Auth\PasswordAuthProvider; use MiniShop3\Services\Customer\AuthManager; use MiniShop3\Services\Customer\EmailVerificationService; use MiniShop3\Services\Customer\RateLimiter; use MiniShop3\Services\Customer\RegisterService; -use MiniShop3\Services\Order\OrderDraftManager; -use MiniShop3\Utils\CookieHelper; use MODX\Revolution\Processors\Processor; /** @@ -29,18 +27,18 @@ public function process() { $this->modx->lexicon->load('minishop3:customer'); - $email = trim($this->getProperty('email', '')); + $email = PasswordAuthProvider::normalizeEmail($this->getProperty('email', '')); $password = $this->getProperty('password', ''); $firstName = trim($this->getProperty('first_name', '')); $lastName = trim($this->getProperty('last_name', '')); $phone = trim($this->getProperty('phone', '')); $privacyAccepted = (bool)$this->getProperty('privacy_accepted', false); - if (empty($email)) { + if ($email === '') { return $this->failure($this->modx->lexicon('ms3_customer_err_email_required')); } - if (empty($password)) { + if ($password === '') { return $this->failure($this->modx->lexicon('ms3_customer_err_password_required')); } @@ -86,59 +84,13 @@ public function process() $expiresAt = null; if ($autoLogin && !$requireEmailVerification) { - // Use existing token from cookie/session instead of creating new one - $currentToken = CookieHelper::getTokenFromCookie(); - if (empty($currentToken)) { - $currentToken = $_SESSION['ms3']['customer_token'] ?? ''; - } - - $tokenObj = null; - - if (!empty($currentToken)) { - $tokenObj = $this->modx->getObject(msCustomerToken::class, [ - 'token' => $currentToken, - 'type' => msCustomerToken::TYPE_API, - ]); - - if ($tokenObj) { - // Bind customer to existing token - $tokenObj->set('customer_id', $customer->id); - - $ttl = (int)$this->modx->getOption('ms3_customer_token_ttl', null, 604800); - $tokenObj->set('expires_at', date('Y-m-d H:i:s', time() + $ttl)); - $tokenObj->save(); - - $tokenString = $tokenObj->get('token'); - $expiresAt = $tokenObj->get('expires_at'); - - // Bind draft order to customer - /** @var OrderDraftManager $draftManager */ - $draftManager = $this->modx->services->get('ms3_order_draft_manager'); - $draftManager->bindDraftToCustomer($tokenString, $customer->id); - } - } - - // Edge case: no valid existing token - if (!$tokenObj) { - /** @var AuthManager $authManager */ - $authManager = $this->modx->services->get('ms3_auth_manager'); - $ttl = (int)$this->modx->getOption('ms3_customer_token_ttl', null, 604800); - $tokenObj = $authManager->createToken($customer, 'api', $ttl); - - if ($tokenObj) { - $tokenString = $tokenObj->get('token'); - $expiresAt = $tokenObj->get('expires_at'); - CookieHelper::setTokenCookie($this->modx, $tokenString); - } - } + /** @var AuthManager $authManager */ + $authManager = $this->modx->services->get('ms3_auth_manager'); + $session = $authManager->establishCustomerSession($customer); - if ($tokenString) { - if (!isset($_SESSION['ms3'])) { - $_SESSION['ms3'] = []; - } - $_SESSION['ms3']['customer_id'] = $customer->id; - $_SESSION['ms3']['customer_token'] = $tokenString; - $_SESSION['ms3']['customer_token_expires'] = strtotime($expiresAt); + if ($session) { + $tokenString = $session['token']; + $expiresAt = $session['expires_at']; } } diff --git a/core/components/minishop3/src/Services/Customer/AuthManager.php b/core/components/minishop3/src/Services/Customer/AuthManager.php index 8a80bbac..7a839a8e 100644 --- a/core/components/minishop3/src/Services/Customer/AuthManager.php +++ b/core/components/minishop3/src/Services/Customer/AuthManager.php @@ -6,6 +6,9 @@ use MiniShop3\Controllers\Auth\PasswordAuthProvider; use MiniShop3\Model\msCustomer; use MiniShop3\Model\msCustomerToken; +use MiniShop3\Services\Order\OrderDraftManager; +use MiniShop3\Utils\CookieHelper; +use MiniShop3\Utils\SessionHelper; use MODX\Revolution\modX; /** @@ -157,6 +160,66 @@ public function authenticate(array $credentials): ?msCustomer return null; } + /** + * Bind authenticated customer to API token, session, cookie, and draft order. + * + * Reuses guest token from cookie/session when possible so cart is preserved. + * + * @return array{token: string, expires_at: string}|null + */ + public function establishCustomerSession(msCustomer $customer): ?array + { + SessionHelper::ensureActive(); + + $ttl = (int)$this->modx->getOption('ms3_customer_token_ttl', null, 604800); + $currentToken = CookieHelper::getTokenFromCookie(); + if ($currentToken === '') { + $currentToken = $_SESSION['ms3']['customer_token'] ?? ''; + } + + $tokenObj = null; + if ($currentToken !== '') { + $tokenObj = $this->modx->getObject(msCustomerToken::class, [ + 'token' => $currentToken, + 'type' => msCustomerToken::TYPE_API, + ]); + } + + if ($tokenObj) { + $tokenObj->set('customer_id', $customer->id); + $tokenObj->set('expires_at', date('Y-m-d H:i:s', time() + $ttl)); + if (!$tokenObj->save()) { + return null; + } + } else { + $tokenObj = $this->createToken($customer, msCustomerToken::TYPE_API, $ttl); + if (!$tokenObj) { + return null; + } + } + + $tokenString = $tokenObj->get('token'); + $expiresAt = $tokenObj->get('expires_at'); + + CookieHelper::setTokenCookie($this->modx, $tokenString); + + /** @var OrderDraftManager $draftManager */ + $draftManager = $this->modx->services->get('ms3_order_draft_manager'); + $draftManager->bindDraftToCustomer($tokenString, $customer->id); + + if (!isset($_SESSION['ms3'])) { + $_SESSION['ms3'] = []; + } + $_SESSION['ms3']['customer_id'] = $customer->id; + $_SESSION['ms3']['customer_token'] = $tokenString; + $_SESSION['ms3']['customer_token_expires'] = strtotime($expiresAt); + + return [ + 'token' => $tokenString, + 'expires_at' => $expiresAt, + ]; + } + /** * Create token for customer * diff --git a/core/components/minishop3/src/Services/Customer/CustomerPageService.php b/core/components/minishop3/src/Services/Customer/CustomerPageService.php index d7f9792e..0e17f131 100644 --- a/core/components/minishop3/src/Services/Customer/CustomerPageService.php +++ b/core/components/minishop3/src/Services/Customer/CustomerPageService.php @@ -4,6 +4,8 @@ use MiniShop3\MiniShop3; use MiniShop3\Model\msCustomer; +use MiniShop3\Services\TokenService; +use MiniShop3\Utils\SessionHelper; use MODX\Revolution\modX; use ModxPro\PdoTools\Fetch; @@ -66,6 +68,15 @@ public function __construct(modX $modx, MiniShop3 $ms3, array $scriptProperties */ public function checkAuth(): bool { + if ($this->modx->services->has('ms3_token_service')) { + /** @var TokenService $tokenService */ + $tokenService = $this->modx->services->get('ms3_token_service'); + $tokenService->ensureSessionActive(); + $tokenService->restoreSessionFromCookie(); + } else { + SessionHelper::ensureActive(); + } + if (empty($_SESSION['ms3']['customer_id'])) { $this->modx->log( modX::LOG_LEVEL_DEBUG, diff --git a/core/components/minishop3/src/Services/Customer/RegisterService.php b/core/components/minishop3/src/Services/Customer/RegisterService.php index 546e12f7..b9b0b7d2 100644 --- a/core/components/minishop3/src/Services/Customer/RegisterService.php +++ b/core/components/minishop3/src/Services/Customer/RegisterService.php @@ -82,7 +82,7 @@ public function setEmailVerification(EmailVerificationService $service): void */ public function register(array $data): array { - $email = trim($data['email'] ?? ''); + $email = PasswordAuthProvider::normalizeEmail($data['email'] ?? ''); if (empty($email)) { return [ diff --git a/core/components/minishop3/src/Services/TokenService.php b/core/components/minishop3/src/Services/TokenService.php index caf850bb..fb994138 100644 --- a/core/components/minishop3/src/Services/TokenService.php +++ b/core/components/minishop3/src/Services/TokenService.php @@ -103,6 +103,65 @@ public function generateCustomerToken(?int $ttl = null): array ]; } + /** + * Ensure PHP session is started before reading or writing $_SESSION. + */ + public function ensureSessionActive(): void + { + if (session_status() !== PHP_SESSION_ACTIVE) { + session_start(); + } + } + + /** + * Restore session from httpOnly ms3_token cookie without minting a new token. + * + * @return bool True when a valid cookie token was found and session was hydrated + */ + public function restoreSessionFromCookie(): bool + { + if ($this->getCustomerToken()) { + return true; + } + + $cookieToken = CookieHelper::getTokenFromCookie(); + if ($cookieToken === '') { + return false; + } + + $tokenObj = $this->modx->getObject(msCustomerToken::class, [ + 'token' => $cookieToken, + 'type' => msCustomerToken::TYPE_API, + ]); + + if (!$tokenObj) { + return false; + } + + if ($tokenObj->isExpired()) { + $ttl = (int)$this->modx->getOption('ms3_customer_token_ttl', null, 604800); + $tokenObj->set('expires_at', date('Y-m-d H:i:s', time() + $ttl)); + $tokenObj->save(); + } + + $this->ensureSessionActive(); + + if (!isset($_SESSION['ms3'])) { + $_SESSION['ms3'] = []; + } + $_SESSION['ms3']['customer_token'] = $cookieToken; + $_SESSION['ms3']['customer_token_expires'] = strtotime($tokenObj->get('expires_at')); + + $customerId = (int)$tokenObj->get('customer_id'); + if ($customerId > 0) { + $_SESSION['ms3']['customer_id'] = $customerId; + } + + CookieHelper::setTokenCookie($this->modx, $cookieToken); + + return true; + } + /** * Resolve existing token or create new one * @@ -115,49 +174,19 @@ public function generateCustomerToken(?int $ttl = null): array */ public function resolveOrCreateToken(): string { - // 1. Check session $sessionToken = $this->getCustomerToken(); if ($sessionToken) { CookieHelper::setTokenCookie($this->modx, $sessionToken); return $sessionToken; } - // 2. Check cookie - $cookieToken = CookieHelper::getTokenFromCookie(); - if (!empty($cookieToken)) { - $tokenObj = $this->modx->getObject(msCustomerToken::class, [ - 'token' => $cookieToken, - 'type' => msCustomerToken::TYPE_API, - ]); - - if ($tokenObj) { - // Auto-renew expired token - if ($tokenObj->isExpired()) { - $ttl = (int)$this->modx->getOption('ms3_customer_token_ttl', null, 604800); - $tokenObj->set('expires_at', date('Y-m-d H:i:s', time() + $ttl)); - $tokenObj->save(); - } - - // Restore session - if (!isset($_SESSION['ms3'])) { - $_SESSION['ms3'] = []; - } - $_SESSION['ms3']['customer_token'] = $cookieToken; - $_SESSION['ms3']['customer_token_expires'] = strtotime($tokenObj->get('expires_at')); - - $customerId = (int)$tokenObj->get('customer_id'); - if ($customerId > 0) { - $_SESSION['ms3']['customer_id'] = $customerId; - } - - // Refresh cookie TTL - CookieHelper::setTokenCookie($this->modx, $cookieToken); - - return $cookieToken; + if ($this->restoreSessionFromCookie()) { + $restoredToken = $this->getCustomerToken(); + if ($restoredToken) { + return $restoredToken; } } - // 3. Generate new token $result = $this->generateCustomerToken(); return $result['token']; } diff --git a/core/components/minishop3/src/Utils/SessionHelper.php b/core/components/minishop3/src/Utils/SessionHelper.php new file mode 100644 index 00000000..e4007706 --- /dev/null +++ b/core/components/minishop3/src/Utils/SessionHelper.php @@ -0,0 +1,16 @@ + Date: Thu, 16 Jul 2026 11:50:08 +0600 Subject: [PATCH 2/9] fix(customer): harden session restore after storefront login Prevent guest token mint and middleware from wiping an authenticated customer session, fail register auto-login when session bind fails, and add focused regression tests for email normalize and API payload shape. --- .../chunks/ms3_customer_unauthorized.tpl | 6 + .../minishop3/lexicon/en/customer.inc.php | 1 + .../minishop3/lexicon/ru/customer.inc.php | 1 + .../src/Controllers/Customer/Customer.php | 66 +++------ .../src/Middleware/TokenMiddleware.php | 21 +-- .../src/Processors/Api/Customer/Login.php | 6 +- .../src/Processors/Api/Customer/Register.php | 8 +- .../src/Services/Customer/AuthManager.php | 75 ++++++++--- .../minishop3/src/Services/TokenService.php | 127 +++++++++++------- .../tests/ApiClientGetPayloadTest.php | 54 ++++++++ .../tests/CustomerAuthNormalizeEmailTest.php | 31 +++++ 11 files changed, 260 insertions(+), 136 deletions(-) create mode 100644 core/components/minishop3/tests/ApiClientGetPayloadTest.php create mode 100644 core/components/minishop3/tests/CustomerAuthNormalizeEmailTest.php diff --git a/core/components/minishop3/elements/chunks/ms3_customer_unauthorized.tpl b/core/components/minishop3/elements/chunks/ms3_customer_unauthorized.tpl index 81c233a8..fda92998 100644 --- a/core/components/minishop3/elements/chunks/ms3_customer_unauthorized.tpl +++ b/core/components/minishop3/elements/chunks/ms3_customer_unauthorized.tpl @@ -35,6 +35,7 @@ {'ms3_customer_email' | lexicon} * @@ -43,6 +44,7 @@ {'ms3_customer_password' | lexicon} * @@ -79,6 +81,7 @@ {'ms3_customer_email' | lexicon} * @@ -112,6 +115,7 @@ {'ms3_customer_password' | lexicon} * {'ms3_customer_password_hint' | lexicon} @@ -123,6 +127,7 @@ {'ms3_customer_password_confirm' | lexicon} * @@ -151,4 +156,5 @@ window.ms3Lexicon.ms3_customer_err_password_mismatch = '{'ms3_customer_err_passw window.ms3Lexicon.ms3_customer_err_privacy_required = '{'ms3_customer_err_privacy_required' | lexicon}'; window.ms3Lexicon.ms3_customer_register_success = '{'ms3_customer_register_success' | lexicon}'; window.ms3Lexicon.ms3_err_unknown = '{'ms3_err_unknown' | lexicon}'; +window.ms3Lexicon.ms3_customer_password_recovery_not_available = '{'ms3_customer_password_recovery_not_available' | lexicon}'; diff --git a/core/components/minishop3/lexicon/en/customer.inc.php b/core/components/minishop3/lexicon/en/customer.inc.php index 4bf69e7e..a5cb3908 100644 --- a/core/components/minishop3/lexicon/en/customer.inc.php +++ b/core/components/minishop3/lexicon/en/customer.inc.php @@ -36,6 +36,7 @@ $_lang['ms3_customer_password_confirm'] = 'Confirm Password'; $_lang['ms3_customer_register_success'] = 'Registration successful'; $_lang['ms3_customer_login_success'] = 'You have successfully logged in'; +$_lang['ms3_customer_password_recovery_not_available'] = 'Password recovery is not available yet'; $_lang['ms3_customer_logout'] = 'Logout'; $_lang['ms3_customer_logout_success'] = 'You have been logged out'; $_lang['ms3_customer_logout_confirm'] = 'Are you sure you want to logout?'; diff --git a/core/components/minishop3/lexicon/ru/customer.inc.php b/core/components/minishop3/lexicon/ru/customer.inc.php index bcc67993..146df2c3 100644 --- a/core/components/minishop3/lexicon/ru/customer.inc.php +++ b/core/components/minishop3/lexicon/ru/customer.inc.php @@ -36,6 +36,7 @@ $_lang['ms3_customer_password_confirm'] = 'Подтверждение пароля'; $_lang['ms3_customer_register_success'] = 'Регистрация прошла успешно'; $_lang['ms3_customer_login_success'] = 'Вы успешно вошли в систему'; +$_lang['ms3_customer_password_recovery_not_available'] = 'Восстановление пароля пока недоступно'; $_lang['ms3_customer_logout'] = 'Выход'; $_lang['ms3_customer_logout_success'] = 'Вы вышли из системы'; $_lang['ms3_customer_logout_confirm'] = 'Вы действительно хотите выйти?'; diff --git a/core/components/minishop3/src/Controllers/Customer/Customer.php b/core/components/minishop3/src/Controllers/Customer/Customer.php index 1157ee57..de4e4342 100644 --- a/core/components/minishop3/src/Controllers/Customer/Customer.php +++ b/core/components/minishop3/src/Controllers/Customer/Customer.php @@ -6,11 +6,11 @@ require_once($autoload); +use MiniShop3\Controllers\Auth\PasswordAuthProvider; use MiniShop3\MiniShop3; use MiniShop3\Model\msCustomer; -use MiniShop3\Model\msCustomerToken; +use MiniShop3\Services\Customer\AuthManager; use MiniShop3\Services\Customer\CustomerAddressManager; -use MiniShop3\Utils\CookieHelper; use MODX\Revolution\modX; use Rakit\Validation\Validator; @@ -66,6 +66,9 @@ public function generateToken(): array $tokenService = $this->modx->services->get('ms3_token_service'); $result = $tokenService->generateCustomerToken(); + if ($result['token'] === '') { + return $this->error('ms3_err_token'); + } return $this->success('', [ 'token' => $result['token'], @@ -456,43 +459,6 @@ public function getOrCreate(?array $orderData = null): int return 0; } - /** - * Auto-login customer after order creation - * - * Binds existing msCustomerToken to customer and sets session + cookie. - * - * @param msCustomer $msCustomer Customer to login - */ - protected function autoLoginCustomer(msCustomer $msCustomer): void - { - if (!isset($_SESSION['ms3'])) { - $_SESSION['ms3'] = []; - } - $_SESSION['ms3']['customer_id'] = $msCustomer->id; - - // Resolve current token: cookie → session → controller token - // ($this->token comes from $_REQUEST via middleware cookie injection) - $currentToken = CookieHelper::getTokenFromCookie(); - if (empty($currentToken)) { - $currentToken = $_SESSION['ms3']['customer_token'] ?? $this->token; - } - - if (!empty($currentToken)) { - $tokenObj = $this->modx->getObject(msCustomerToken::class, [ - 'token' => $currentToken, - 'type' => msCustomerToken::TYPE_API, - ]); - - if ($tokenObj) { - $tokenObj->set('customer_id', $msCustomer->id); - $tokenObj->save(); - } - - $_SESSION['ms3']['customer_token'] = $currentToken; - CookieHelper::setTokenCookie($this->modx, $currentToken); - } - } - /** * Find customer by email * @@ -501,7 +467,8 @@ protected function autoLoginCustomer(msCustomer $msCustomer): void */ protected function findByEmail(string $email): ?msCustomer { - if (empty($email)) { + $email = PasswordAuthProvider::normalizeEmail($email); + if ($email === '') { return null; } @@ -550,20 +517,12 @@ protected function createFromOrderData(array $orderData): ?msCustomer if ($registerResult['success']) { $msCustomer = $registerResult['customer']; - - if ($autoLogin) { - $this->autoLoginCustomer($msCustomer); - } } else { $msCustomer = $this->findByEmail($email); if ($msCustomer) { $msCustomer->set('token', $this->token); $msCustomer->save(); - - if ($autoLogin) { - $this->autoLoginCustomer($msCustomer); - } } } } @@ -579,9 +538,16 @@ protected function createFromOrderData(array $orderData): ?msCustomer ]; $msCustomer = $this->create($customerData); + } - if ($msCustomer && $autoLogin) { - $this->autoLoginCustomer($msCustomer); + if ($msCustomer && $autoLogin) { + /** @var AuthManager $authManager */ + $authManager = $this->modx->services->get('ms3_auth_manager'); + if (!$authManager->establishCustomerSession($msCustomer)) { + $this->modx->log( + modX::LOG_LEVEL_ERROR, + "[Customer] establishCustomerSession failed for customer #{$msCustomer->id}" + ); } } diff --git a/core/components/minishop3/src/Middleware/TokenMiddleware.php b/core/components/minishop3/src/Middleware/TokenMiddleware.php index e67cd950..8d541d35 100644 --- a/core/components/minishop3/src/Middleware/TokenMiddleware.php +++ b/core/components/minishop3/src/Middleware/TokenMiddleware.php @@ -93,24 +93,9 @@ public function handle(array $params) ]); if ($tokenObj) { - // Auto-renew expired token - if ($tokenObj->isExpired()) { - $ttl = (int)$this->modx->getOption('ms3_customer_token_ttl', null, 604800); - $newExpiresAt = date('Y-m-d H:i:s', time() + $ttl); - $tokenObj->set('expires_at', $newExpiresAt); - $tokenObj->save(); - } - - // Save to session - if (!isset($_SESSION['ms3'])) { - $_SESSION['ms3'] = []; - } - $_SESSION['ms3']['customer_token'] = $token; - $_SESSION['ms3']['customer_id'] = $tokenObj->get('customer_id'); - $_SESSION['ms3']['customer_token_expires'] = strtotime($tokenObj->get('expires_at')); - - // Refresh cookie - CookieHelper::setTokenCookie($this->modx, $token); + /** @var TokenService $tokenService */ + $tokenService = $this->modx->services->get('ms3_token_service'); + $tokenService->syncSessionFromToken($tokenObj); // Ensure $_REQUEST has the token for controllers $_REQUEST['ms3_token'] = $token; diff --git a/core/components/minishop3/src/Processors/Api/Customer/Login.php b/core/components/minishop3/src/Processors/Api/Customer/Login.php index 31461242..b5eb3344 100644 --- a/core/components/minishop3/src/Processors/Api/Customer/Login.php +++ b/core/components/minishop3/src/Processors/Api/Customer/Login.php @@ -11,7 +11,7 @@ * Login - customer login processor * * Authenticates customer and binds existing session token to customer. - * Token does NOT change on login — guest cart is preserved. + * Token reuses guest cart token when possible; otherwise mints a new API token. * Protected from brute-force via RateLimiter. * * @package MiniShop3\Processors\Api\Customer @@ -59,6 +59,10 @@ public function process() ]); if (!$customer) { + if ($authManager->getLastAuthFailure() === 'invalid_credentials') { + $authManager->handleFailedLoginByEmail($email); + } + $this->modx->log( \MODX\Revolution\modX::LOG_LEVEL_WARN, "[Login] Failed login attempt for email: {$email} from IP: {$ip}" diff --git a/core/components/minishop3/src/Processors/Api/Customer/Register.php b/core/components/minishop3/src/Processors/Api/Customer/Register.php index 499b3fac..454bbc6b 100644 --- a/core/components/minishop3/src/Processors/Api/Customer/Register.php +++ b/core/components/minishop3/src/Processors/Api/Customer/Register.php @@ -88,10 +88,12 @@ public function process() $authManager = $this->modx->services->get('ms3_auth_manager'); $session = $authManager->establishCustomerSession($customer); - if ($session) { - $tokenString = $session['token']; - $expiresAt = $session['expires_at']; + if (!$session) { + return $this->failure($this->modx->lexicon('ms3_customer_err_token_create')); } + + $tokenString = $session['token']; + $expiresAt = $session['expires_at']; } $rateLimiter->reset('login', $ip); diff --git a/core/components/minishop3/src/Services/Customer/AuthManager.php b/core/components/minishop3/src/Services/Customer/AuthManager.php index 7a839a8e..881ce71c 100644 --- a/core/components/minishop3/src/Services/Customer/AuthManager.php +++ b/core/components/minishop3/src/Services/Customer/AuthManager.php @@ -7,7 +7,7 @@ use MiniShop3\Model\msCustomer; use MiniShop3\Model\msCustomerToken; use MiniShop3\Services\Order\OrderDraftManager; -use MiniShop3\Utils\CookieHelper; +use MiniShop3\Services\TokenService; use MiniShop3\Utils\SessionHelper; use MODX\Revolution\modX; @@ -46,6 +46,9 @@ class AuthManager /** @var AuthProviderInterface[] Registered authentication providers */ protected array $providers = []; + /** Last authenticate() failure: invalid_credentials|blocked|inactive|none */ + protected string $lastAuthFailure = 'none'; + /** * @param modX $modx */ @@ -56,6 +59,14 @@ public function __construct(modX $modx) $this->registerProvider(new PasswordAuthProvider($modx)); } + /** + * Reason of the last failed authenticate() call. + */ + public function getLastAuthFailure(): string + { + return $this->lastAuthFailure; + } + /** * Register authentication provider * @@ -105,6 +116,8 @@ public function getProviders(): array */ public function authenticate(array $credentials): ?msCustomer { + $this->lastAuthFailure = 'invalid_credentials'; + foreach ($this->providers as $provider) { if ($provider->supports($credentials)) { $this->modx->log( @@ -118,6 +131,7 @@ public function authenticate(array $credentials): ?msCustomer if ($customer->get('is_blocked')) { $blockedUntil = $customer->get('blocked_until'); if ($blockedUntil && strtotime($blockedUntil) > time()) { + $this->lastAuthFailure = 'blocked'; $this->modx->log( modX::LOG_LEVEL_WARN, "[AuthManager] Customer #{$customer->id} is blocked until {$blockedUntil}" @@ -131,6 +145,7 @@ public function authenticate(array $credentials): ?msCustomer } if (!$customer->get('is_active')) { + $this->lastAuthFailure = 'inactive'; $this->modx->log( modX::LOG_LEVEL_WARN, "[AuthManager] Customer #{$customer->id} is not active" @@ -142,6 +157,7 @@ public function authenticate(array $credentials): ?msCustomer $customer->set('failed_login_attempts', 0); $customer->save(); + $this->lastAuthFailure = 'none'; $this->modx->log( modX::LOG_LEVEL_INFO, "[AuthManager] Customer #{$customer->id} authenticated via {$provider->getName()}" @@ -172,10 +188,10 @@ public function establishCustomerSession(msCustomer $customer): ?array SessionHelper::ensureActive(); $ttl = (int)$this->modx->getOption('ms3_customer_token_ttl', null, 604800); - $currentToken = CookieHelper::getTokenFromCookie(); - if ($currentToken === '') { - $currentToken = $_SESSION['ms3']['customer_token'] ?? ''; - } + + /** @var TokenService $tokenService */ + $tokenService = $this->modx->services->get('ms3_token_service'); + $currentToken = $tokenService->getBindableTokenString(); $tokenObj = null; if ($currentToken !== '') { @@ -185,7 +201,12 @@ public function establishCustomerSession(msCustomer $customer): ?array ]); } - if ($tokenObj) { + // Rebind only guest tokens (preserve cart). Never hijack another customer's token. + $canReuse = $tokenObj + && ((int)$tokenObj->get('customer_id') === 0 + || (int)$tokenObj->get('customer_id') === (int)$customer->id); + + if ($canReuse) { $tokenObj->set('customer_id', $customer->id); $tokenObj->set('expires_at', date('Y-m-d H:i:s', time() + $ttl)); if (!$tokenObj->save()) { @@ -198,25 +219,26 @@ public function establishCustomerSession(msCustomer $customer): ?array } } - $tokenString = $tokenObj->get('token'); - $expiresAt = $tokenObj->get('expires_at'); - - CookieHelper::setTokenCookie($this->modx, $tokenString); + $tokenString = (string)$tokenObj->get('token'); /** @var OrderDraftManager $draftManager */ $draftManager = $this->modx->services->get('ms3_order_draft_manager'); - $draftManager->bindDraftToCustomer($tokenString, $customer->id); + if (!$draftManager->bindDraftToCustomer($tokenString, $customer->id)) { + $this->modx->log( + modX::LOG_LEVEL_WARN, + "[AuthManager] bindDraftToCustomer failed for customer #{$customer->id}" + ); + } - if (!isset($_SESSION['ms3'])) { - $_SESSION['ms3'] = []; + $tokenService->syncSessionFromToken($tokenObj); + + if (session_status() === PHP_SESSION_ACTIVE) { + session_regenerate_id(true); } - $_SESSION['ms3']['customer_id'] = $customer->id; - $_SESSION['ms3']['customer_token'] = $tokenString; - $_SESSION['ms3']['customer_token_expires'] = strtotime($expiresAt); return [ 'token' => $tokenString, - 'expires_at' => $expiresAt, + 'expires_at' => $tokenObj->get('expires_at'), ]; } @@ -233,7 +255,7 @@ public function createToken(msCustomer $customer, string $type = 'api', int $ttl /** @var msCustomerToken $token */ $token = $this->modx->newObject(msCustomerToken::class); - $tokenString = bin2hex(random_bytes(64)); + $tokenString = bin2hex(random_bytes(32)); $expiresAt = date('Y-m-d H:i:s', time() + $ttl); $token->set('customer_id', $customer->id); @@ -407,4 +429,21 @@ public function handleFailedLogin(msCustomer $customer): void $customer->save(); } + + /** + * Record failed login by normalized email when the account exists. + */ + public function handleFailedLoginByEmail(string $email): void + { + $email = PasswordAuthProvider::normalizeEmail($email); + if ($email === '') { + return; + } + + /** @var msCustomer|null $customer */ + $customer = $this->modx->getObject(msCustomer::class, ['email' => $email]); + if ($customer) { + $this->handleFailedLogin($customer); + } + } } diff --git a/core/components/minishop3/src/Services/TokenService.php b/core/components/minishop3/src/Services/TokenService.php index fb994138..b0e3090a 100644 --- a/core/components/minishop3/src/Services/TokenService.php +++ b/core/components/minishop3/src/Services/TokenService.php @@ -4,6 +4,7 @@ use MiniShop3\Model\msCustomerToken; use MiniShop3\Utils\CookieHelper; +use MiniShop3\Utils\SessionHelper; use MODX\Revolution\modX; /** @@ -45,12 +46,11 @@ public function __construct(modX $modx) */ public function generateCustomerToken(?int $ttl = null): array { - $existingToken = $this->getCustomerToken(); - if ($existingToken) { - $expires = $_SESSION['ms3']['customer_token_expires'] ?? (time() + 86400); + $existingToken = $this->ensureCustomerTokenLoaded(); + if ($existingToken !== null) { + $expires = (int)($_SESSION['ms3']['customer_token_expires'] ?? (time() + 86400)); $lifetime = max(0, $expires - time()); - // Refresh cookie TTL CookieHelper::setTokenCookie($this->modx, $existingToken); return [ @@ -108,9 +108,7 @@ public function generateCustomerToken(?int $ttl = null): array */ public function ensureSessionActive(): void { - if (session_status() !== PHP_SESSION_ACTIVE) { - session_start(); - } + SessionHelper::ensureActive(); } /** @@ -138,57 +136,44 @@ public function restoreSessionFromCookie(): bool return false; } - if ($tokenObj->isExpired()) { - $ttl = (int)$this->modx->getOption('ms3_customer_token_ttl', null, 604800); - $tokenObj->set('expires_at', date('Y-m-d H:i:s', time() + $ttl)); - $tokenObj->save(); - } - - $this->ensureSessionActive(); - - if (!isset($_SESSION['ms3'])) { - $_SESSION['ms3'] = []; - } - $_SESSION['ms3']['customer_token'] = $cookieToken; - $_SESSION['ms3']['customer_token_expires'] = strtotime($tokenObj->get('expires_at')); - - $customerId = (int)$tokenObj->get('customer_id'); - if ($customerId > 0) { - $_SESSION['ms3']['customer_id'] = $customerId; - } - + $this->renewTokenIfExpired($tokenObj); + $this->applyTokenToSession($tokenObj); CookieHelper::setTokenCookie($this->modx, $cookieToken); return true; } /** - * Resolve existing token or create new one - * - * Resolution chain: - * 1. Session token → if valid, return - * 2. Cookie token → verify in DB (msCustomerToken type=api), restore session, return - * 3. Generate new token → set cookie + session, return - * - * @return string Token string + * Resolve existing token or create new one (session → cookie restore → mint). */ public function resolveOrCreateToken(): string { - $sessionToken = $this->getCustomerToken(); - if ($sessionToken) { - CookieHelper::setTokenCookie($this->modx, $sessionToken); - return $sessionToken; - } + return $this->generateCustomerToken()['token']; + } - if ($this->restoreSessionFromCookie()) { - $restoredToken = $this->getCustomerToken(); - if ($restoredToken) { - return $restoredToken; - } + /** + * Token string for binding an authenticated customer (cookie → session, no minting). + */ + public function getBindableTokenString(): string + { + SessionHelper::ensureActive(); + + $cookieToken = CookieHelper::getTokenFromCookie(); + if ($cookieToken !== '') { + return $cookieToken; } - $result = $this->generateCustomerToken(); - return $result['token']; + return (string)($_SESSION['ms3']['customer_token'] ?? ''); + } + + /** + * Renew expired API token TTL and hydrate $_SESSION from DB row. + */ + public function syncSessionFromToken(msCustomerToken $tokenObj): void + { + $this->renewTokenIfExpired($tokenObj); + $this->applyTokenToSession($tokenObj); + CookieHelper::setTokenCookie($this->modx, $tokenObj->get('token')); } /** @@ -405,4 +390,54 @@ public function clearSnippetCache(?string $token = null): bool return $this->modx->cacheManager->clean($options); } } + + /** + * Load valid session/cookie token without minting a new one. + */ + private function ensureCustomerTokenLoaded(): ?string + { + SessionHelper::ensureActive(); + + $token = $this->getCustomerToken(); + if ($token !== null) { + return $token; + } + + $this->restoreSessionFromCookie(); + + return $this->getCustomerToken(); + } + + private function renewTokenIfExpired(msCustomerToken $tokenObj): void + { + if (!$tokenObj->isExpired()) { + return; + } + + $ttl = (int)$this->modx->getOption('ms3_customer_token_ttl', null, 604800); + $tokenObj->set('expires_at', date('Y-m-d H:i:s', time() + $ttl)); + $tokenObj->save(); + } + + /** + * Write token row into $_SESSION without overwriting an authenticated customer with guest token. + */ + private function applyTokenToSession(msCustomerToken $tokenObj): void + { + SessionHelper::ensureActive(); + + if (!isset($_SESSION['ms3'])) { + $_SESSION['ms3'] = []; + } + + $_SESSION['ms3']['customer_token'] = $tokenObj->get('token'); + $_SESSION['ms3']['customer_token_expires'] = strtotime($tokenObj->get('expires_at')); + + $tokenCustomerId = (int)$tokenObj->get('customer_id'); + if ($tokenCustomerId > 0) { + $_SESSION['ms3']['customer_id'] = $tokenCustomerId; + } elseif (empty($_SESSION['ms3']['customer_id'])) { + $_SESSION['ms3']['customer_id'] = 0; + } + } } diff --git a/core/components/minishop3/tests/ApiClientGetPayloadTest.php b/core/components/minishop3/tests/ApiClientGetPayloadTest.php new file mode 100644 index 00000000..33c0099f --- /dev/null +++ b/core/components/minishop3/tests/ApiClientGetPayloadTest.php @@ -0,0 +1,54 @@ + true, + 'data' => ['token' => 'abc', 'redirect_url' => '/profile'], + 'object' => null, +]); +if (!is_array($web) || ($web['redirect_url'] ?? '') !== '/profile') { + $fail('Web API data payload must expose redirect_url'); +} + +$legacy = $getPayload([ + 'success' => true, + 'object' => ['token' => 'xyz', 'redirect_url' => '/cabinet'], +]); +if (!is_array($legacy) || ($legacy['redirect_url'] ?? '') !== '/cabinet') { + $fail('Legacy object payload must expose redirect_url'); +} + +$preferData = $getPayload([ + 'data' => ['redirect_url' => '/from-data'], + 'object' => ['redirect_url' => '/from-object'], +]); +if (($preferData['redirect_url'] ?? '') !== '/from-data') { + $fail('data must win over object'); +} + +if ($getPayload(null) !== null) { + $fail('null input must return null'); +} + +fwrite(STDOUT, "OK ApiClientGetPayloadTest\n"); +exit(0); diff --git a/core/components/minishop3/tests/CustomerAuthNormalizeEmailTest.php b/core/components/minishop3/tests/CustomerAuthNormalizeEmailTest.php new file mode 100644 index 00000000..19f90e19 --- /dev/null +++ b/core/components/minishop3/tests/CustomerAuthNormalizeEmailTest.php @@ -0,0 +1,31 @@ + Date: Thu, 16 Jul 2026 11:56:44 +0600 Subject: [PATCH 3/9] fix(customer): unify logout so cookie restore cannot re-auth Snippet and API logout now revoke tokens, mint a guest cookie, and extend TTL in DB on refresh. Registration keeps the account if auto-login session bind fails and points the user to the login page. --- .../elements/snippets/ms3_customer.php | 16 +---- .../minishop3/lexicon/en/customer.inc.php | 1 + .../minishop3/lexicon/ru/customer.inc.php | 1 + .../src/Processors/Api/Customer/Logout.php | 52 ++------------- .../src/Processors/Api/Customer/Register.php | 27 ++++++-- .../src/Services/Customer/AuthManager.php | 65 +++++++++++++++++++ .../minishop3/src/Services/TokenService.php | 23 ++++++- 7 files changed, 117 insertions(+), 68 deletions(-) diff --git a/core/components/minishop3/elements/snippets/ms3_customer.php b/core/components/minishop3/elements/snippets/ms3_customer.php index 591b8f7e..9adba9dc 100644 --- a/core/components/minishop3/elements/snippets/ms3_customer.php +++ b/core/components/minishop3/elements/snippets/ms3_customer.php @@ -26,19 +26,9 @@ $modx->lexicon->load('minishop3:cart'); // For order details template if (isset($_GET['action']) && $_GET['action'] === 'logout') { - if (!empty($_SESSION['ms3']['customer_token'])) { - $token = $_SESSION['ms3']['customer_token']; - $tokenObj = $modx->getObject(\MiniShop3\Model\msCustomerToken::class, ['token' => $token]); - if ($tokenObj) { - $tokenObj->remove(); - } - } - - if (isset($_SESSION['ms3'])) { - unset($_SESSION['ms3']['customer_id']); - unset($_SESSION['ms3']['customer_token']); - unset($_SESSION['ms3']['customer_token_expires']); - } + /** @var \MiniShop3\Services\Customer\AuthManager $authManager */ + $authManager = $modx->services->get('ms3_auth_manager'); + $authManager->logoutCurrentCustomer(); $loginPageId = $modx->getOption('ms3_customer_login_page_id', null, 1); $modx->sendRedirect($modx->makeUrl($loginPageId)); diff --git a/core/components/minishop3/lexicon/en/customer.inc.php b/core/components/minishop3/lexicon/en/customer.inc.php index a5cb3908..a9775c44 100644 --- a/core/components/minishop3/lexicon/en/customer.inc.php +++ b/core/components/minishop3/lexicon/en/customer.inc.php @@ -35,6 +35,7 @@ $_lang['ms3_customer_password'] = 'Password'; $_lang['ms3_customer_password_confirm'] = 'Confirm Password'; $_lang['ms3_customer_register_success'] = 'Registration successful'; +$_lang['ms3_customer_register_success_login_required'] = 'Registration successful. Please sign in with your email and password.'; $_lang['ms3_customer_login_success'] = 'You have successfully logged in'; $_lang['ms3_customer_password_recovery_not_available'] = 'Password recovery is not available yet'; $_lang['ms3_customer_logout'] = 'Logout'; diff --git a/core/components/minishop3/lexicon/ru/customer.inc.php b/core/components/minishop3/lexicon/ru/customer.inc.php index 146df2c3..9154c6f6 100644 --- a/core/components/minishop3/lexicon/ru/customer.inc.php +++ b/core/components/minishop3/lexicon/ru/customer.inc.php @@ -35,6 +35,7 @@ $_lang['ms3_customer_password'] = 'Пароль'; $_lang['ms3_customer_password_confirm'] = 'Подтверждение пароля'; $_lang['ms3_customer_register_success'] = 'Регистрация прошла успешно'; +$_lang['ms3_customer_register_success_login_required'] = 'Регистрация прошла успешно. Войдите с email и паролем.'; $_lang['ms3_customer_login_success'] = 'Вы успешно вошли в систему'; $_lang['ms3_customer_password_recovery_not_available'] = 'Восстановление пароля пока недоступно'; $_lang['ms3_customer_logout'] = 'Выход'; diff --git a/core/components/minishop3/src/Processors/Api/Customer/Logout.php b/core/components/minishop3/src/Processors/Api/Customer/Logout.php index cec37eb9..55af67c3 100644 --- a/core/components/minishop3/src/Processors/Api/Customer/Logout.php +++ b/core/components/minishop3/src/Processors/Api/Customer/Logout.php @@ -2,16 +2,13 @@ namespace MiniShop3\Processors\Api\Customer; -use MiniShop3\Model\msCustomer; -use MiniShop3\Model\msCustomerToken; use MiniShop3\Services\Customer\AuthManager; -use MiniShop3\Utils\CookieHelper; use MODX\Revolution\Processors\Processor; /** * Logout - customer logout processor * - * Revokes all customer tokens and creates a fresh anonymous token. + * Revokes all customer API tokens and creates a fresh anonymous token. * New cookie is set — guest cart starts fresh (security: old token invalidated). * * @package MiniShop3\Processors\Api\Customer @@ -23,52 +20,15 @@ class Logout extends Processor */ public function process() { - $customerId = $_SESSION['ms3']['customer_id'] ?? null; + $this->modx->lexicon->load('minishop3:customer'); - if (!$customerId) { - return $this->success($this->modx->lexicon('ms3_customer_logout_success')); - } - - /** @var msCustomer $customer */ - $customer = $this->modx->getObject(msCustomer::class, $customerId); - - if ($customer) { - /** @var AuthManager $authManager */ - $authManager = $this->modx->services->get('ms3_auth_manager'); + /** @var AuthManager $authManager */ + $authManager = $this->modx->services->get('ms3_auth_manager'); - // Revoke ALL api tokens for this customer (including current) - $authManager->revokeTokens($customer, 'api'); - - $this->modx->log( - \MODX\Revolution\modX::LOG_LEVEL_INFO, - "[Logout] Customer #{$customer->id} logged out" - ); + if (!$authManager->logoutCurrentCustomer()) { + return $this->failure($this->modx->lexicon('ms3_customer_err_token_create')); } - // Generate fresh anonymous token - $ttl = (int)$this->modx->getOption('ms3_customer_token_ttl', null, 604800); - $newToken = bin2hex(random_bytes(32)); - $expiresAt = date('Y-m-d H:i:s', time() + $ttl); - - $tokenObj = $this->modx->newObject(msCustomerToken::class); - $tokenObj->set('token', $newToken); - $tokenObj->set('type', msCustomerToken::TYPE_API); - $tokenObj->set('customer_id', 0); - $tokenObj->set('expires_at', $expiresAt); - $tokenObj->set('created_at', date('Y-m-d H:i:s')); - $tokenObj->save(); - - // Update session with new anonymous token - if (!isset($_SESSION['ms3'])) { - $_SESSION['ms3'] = []; - } - unset($_SESSION['ms3']['customer_id']); - $_SESSION['ms3']['customer_token'] = $newToken; - $_SESSION['ms3']['customer_token_expires'] = time() + $ttl; - - // Set new cookie - CookieHelper::setTokenCookie($this->modx, $newToken); - return $this->success($this->modx->lexicon('ms3_customer_logout_success')); } } diff --git a/core/components/minishop3/src/Processors/Api/Customer/Register.php b/core/components/minishop3/src/Processors/Api/Customer/Register.php index 454bbc6b..3723f1cb 100644 --- a/core/components/minishop3/src/Processors/Api/Customer/Register.php +++ b/core/components/minishop3/src/Processors/Api/Customer/Register.php @@ -88,18 +88,21 @@ public function process() $authManager = $this->modx->services->get('ms3_auth_manager'); $session = $authManager->establishCustomerSession($customer); - if (!$session) { - return $this->failure($this->modx->lexicon('ms3_customer_err_token_create')); + if ($session) { + $tokenString = $session['token']; + $expiresAt = $session['expires_at']; + } else { + $this->modx->log( + \MODX\Revolution\modX::LOG_LEVEL_ERROR, + "[Register] Customer #{$customer->id} created but establishCustomerSession failed" + ); } - - $tokenString = $session['token']; - $expiresAt = $session['expires_at']; } $rateLimiter->reset('login', $ip); $redirectUrl = ''; - if ($autoLogin && !$requireEmailVerification) { + if ($tokenString !== null) { $redirectPageId = (int)$this->getProperty('redirect_page_id', 0); if (!$redirectPageId) { $redirectPageId = (int)$this->modx->getOption('ms3_customer_redirect_after_login', null, 0); @@ -108,9 +111,18 @@ public function process() if ($redirectPageId > 0) { $redirectUrl = $this->modx->makeUrl($redirectPageId, '', '', 'full'); } + } elseif ($autoLogin && !$requireEmailVerification) { + $loginPageId = (int)$this->modx->getOption('ms3_customer_login_page_id', null, 0); + if ($loginPageId > 0) { + $redirectUrl = $this->modx->makeUrl($loginPageId, '', '', 'full'); + } } - return $this->success($this->modx->lexicon('ms3_customer_register_success'), [ + $messageKey = ($autoLogin && !$requireEmailVerification && $tokenString === null) + ? 'ms3_customer_register_success_login_required' + : 'ms3_customer_register_success'; + + return $this->success($this->modx->lexicon($messageKey), [ 'customer' => [ 'id' => $customer->id, 'email' => $customer->get('email'), @@ -123,6 +135,7 @@ public function process() 'expires_at' => $expiresAt, 'email_verification_required' => $requireEmailVerification, 'redirect_url' => $redirectUrl, + 'auto_login' => $tokenString !== null, ]); } diff --git a/core/components/minishop3/src/Services/Customer/AuthManager.php b/core/components/minishop3/src/Services/Customer/AuthManager.php index 881ce71c..0bab3a2b 100644 --- a/core/components/minishop3/src/Services/Customer/AuthManager.php +++ b/core/components/minishop3/src/Services/Customer/AuthManager.php @@ -8,6 +8,7 @@ use MiniShop3\Model\msCustomerToken; use MiniShop3\Services\Order\OrderDraftManager; use MiniShop3\Services\TokenService; +use MiniShop3\Utils\CookieHelper; use MiniShop3\Utils\SessionHelper; use MODX\Revolution\modX; @@ -242,6 +243,70 @@ public function establishCustomerSession(msCustomer $customer): ?array ]; } + /** + * End storefront session: revoke API tokens, mint guest token, refresh session id. + * + * Used by Web API Logout and snippet `?action=logout` so cookie restore cannot re-auth. + * + * @return bool false when anonymous token could not be persisted + */ + public function logoutCurrentCustomer(): bool + { + SessionHelper::ensureActive(); + + /** @var TokenService $tokenService */ + $tokenService = $this->modx->services->get('ms3_token_service'); + $tokenService->restoreSessionFromCookie(); + + $customerId = (int)($_SESSION['ms3']['customer_id'] ?? 0); + if ($customerId > 0) { + /** @var msCustomer|null $customer */ + $customer = $this->modx->getObject(msCustomer::class, $customerId); + if ($customer) { + $this->revokeTokens($customer, msCustomerToken::TYPE_API); + $this->modx->log( + modX::LOG_LEVEL_INFO, + "[AuthManager] Customer #{$customer->id} logged out" + ); + } + } else { + $orphanToken = $tokenService->getBindableTokenString(); + if ($orphanToken !== '') { + $tokenObj = $this->modx->getObject(msCustomerToken::class, [ + 'token' => $orphanToken, + 'type' => msCustomerToken::TYPE_API, + ]); + if ($tokenObj) { + $tokenObj->remove(); + } + } + } + + if (isset($_SESSION['ms3'])) { + unset( + $_SESSION['ms3']['customer_id'], + $_SESSION['ms3']['customer_token'], + $_SESSION['ms3']['customer_token_expires'] + ); + } + CookieHelper::clearTokenCookie($this->modx); + + $guest = $tokenService->generateCustomerToken(); + if ($guest['token'] === '') { + $this->modx->log( + modX::LOG_LEVEL_ERROR, + '[AuthManager] logoutCurrentCustomer failed to mint anonymous token' + ); + return false; + } + + if (session_status() === PHP_SESSION_ACTIVE) { + session_regenerate_id(true); + } + + return true; + } + /** * Create token for customer * diff --git a/core/components/minishop3/src/Services/TokenService.php b/core/components/minishop3/src/Services/TokenService.php index b0e3090a..13287a68 100644 --- a/core/components/minishop3/src/Services/TokenService.php +++ b/core/components/minishop3/src/Services/TokenService.php @@ -193,10 +193,29 @@ public function updateCustomerToken(string $token, ?int $ttl = null): array $ttl = (int)$this->modx->getOption('ms3_customer_token_ttl', null, 604800); } + SessionHelper::ensureActive(); + + $tokenObj = $this->modx->getObject(msCustomerToken::class, [ + 'token' => $token, + 'type' => msCustomerToken::TYPE_API, + ]); + + if (!$tokenObj) { + return $this->generateCustomerToken($ttl); + } + $expires = time() + $ttl; + $tokenObj->set('expires_at', date('Y-m-d H:i:s', $expires)); + if (!$tokenObj->save()) { + $this->modx->log( + modX::LOG_LEVEL_ERROR, + '[TokenService] Failed to extend token TTL in database' + ); + return ['token' => '', 'expires' => 0, 'lifetime' => 0]; + } - $_SESSION['ms3']['customer_token'] = $token; - $_SESSION['ms3']['customer_token_expires'] = $expires; + $this->applyTokenToSession($tokenObj); + CookieHelper::setTokenCookie($this->modx, $token); $this->modx->log( modX::LOG_LEVEL_DEBUG, From ef7e3daf8dd93da6b67221f60b0698b6b154e09f Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Thu, 16 Jul 2026 11:59:40 +0600 Subject: [PATCH 4/9] fix(customer): close auth silent-failure review findings Fail register auto-login when session bind fails, clear customer_id on guest token hydrate, enforce active/blocked checks in TokenMiddleware, and harden save/TTL/session observability around auth flows. --- .../Controllers/Auth/PasswordAuthProvider.php | 17 +++++--- .../src/Controllers/Customer/Customer.php | 10 ++++- .../src/Middleware/TokenMiddleware.php | 39 ++++++++++++++++++- .../src/Processors/Api/Customer/Register.php | 27 ++++--------- .../src/Services/Customer/AuthManager.php | 21 ++++++++-- .../src/Services/Order/OrderDraftManager.php | 26 +++++++++---- .../minishop3/src/Services/TokenService.php | 24 +++++++++--- .../minishop3/src/Utils/SessionHelper.php | 8 +++- 8 files changed, 127 insertions(+), 45 deletions(-) diff --git a/core/components/minishop3/src/Controllers/Auth/PasswordAuthProvider.php b/core/components/minishop3/src/Controllers/Auth/PasswordAuthProvider.php index 516f7485..8c9e79cc 100644 --- a/core/components/minishop3/src/Controllers/Auth/PasswordAuthProvider.php +++ b/core/components/minishop3/src/Controllers/Auth/PasswordAuthProvider.php @@ -104,12 +104,17 @@ public function authenticate(array $credentials): ?msCustomer if (password_needs_rehash($hashedPassword, PASSWORD_BCRYPT)) { $newHash = password_hash($password, PASSWORD_BCRYPT); $customer->set('password', $newHash); - $customer->save(); - - $this->modx->log( - modX::LOG_LEVEL_INFO, - "[PasswordAuthProvider] Password rehashed for customer #{$customer->id}" - ); + if ($customer->save()) { + $this->modx->log( + modX::LOG_LEVEL_INFO, + "[PasswordAuthProvider] Password rehashed for customer #{$customer->id}" + ); + } else { + $this->modx->log( + modX::LOG_LEVEL_WARN, + "[PasswordAuthProvider] Failed to persist rehashed password for customer #{$customer->id}" + ); + } } $this->modx->log( diff --git a/core/components/minishop3/src/Controllers/Customer/Customer.php b/core/components/minishop3/src/Controllers/Customer/Customer.php index de4e4342..278efcb3 100644 --- a/core/components/minishop3/src/Controllers/Customer/Customer.php +++ b/core/components/minishop3/src/Controllers/Customer/Customer.php @@ -86,6 +86,9 @@ public function updateToken(string $token = ''): array $tokenService = $this->modx->services->get('ms3_token_service'); $result = $tokenService->updateCustomerToken($token); + if ($result['token'] === '') { + return $this->error('ms3_err_token'); + } return $this->success('', [ 'token' => $result['token'], @@ -522,7 +525,12 @@ protected function createFromOrderData(array $orderData): ?msCustomer if ($msCustomer) { $msCustomer->set('token', $this->token); - $msCustomer->save(); + if (!$msCustomer->save()) { + $this->modx->log( + modX::LOG_LEVEL_ERROR, + "[Customer] Failed to update token for customer #{$msCustomer->id}" + ); + } } } } diff --git a/core/components/minishop3/src/Middleware/TokenMiddleware.php b/core/components/minishop3/src/Middleware/TokenMiddleware.php index 8d541d35..e1acf36b 100644 --- a/core/components/minishop3/src/Middleware/TokenMiddleware.php +++ b/core/components/minishop3/src/Middleware/TokenMiddleware.php @@ -77,9 +77,11 @@ public function handle(array $params) // For non-public routes: check session first if (!$isPublic && !empty($_SESSION['ms3']['customer_id'])) { $customer = $this->modx->getObject(\MiniShop3\Model\msCustomer::class, $_SESSION['ms3']['customer_id']); - if ($customer) { + if ($customer && $this->isCustomerSessionAllowed($customer)) { return null; } + + unset($_SESSION['ms3']['customer_id']); } // Resolve token from multiple sources @@ -100,6 +102,20 @@ public function handle(array $params) // Ensure $_REQUEST has the token for controllers $_REQUEST['ms3_token'] = $token; + $tokenCustomerId = (int)$tokenObj->get('customer_id'); + if ($tokenCustomerId > 0) { + $customer = $this->modx->getObject( + \MiniShop3\Model\msCustomer::class, + $tokenCustomerId + ); + if (!$customer || !$this->isCustomerSessionAllowed($customer)) { + unset($_SESSION['ms3']['customer_id']); + if (!$isPublic) { + return Response::error('ms3_err_token_invalid', HttpStatus::UNAUTHORIZED); + } + } + } + return null; } @@ -157,6 +173,27 @@ private function resolveToken(): string return $_REQUEST['ms3_token'] ?? $_REQUEST['token'] ?? ''; } + /** + * Session shortcut is valid only for active, non-blocked customers. + */ + private function isCustomerSessionAllowed(\MiniShop3\Model\msCustomer $customer): bool + { + if (!$customer->get('is_active')) { + return false; + } + + if (!$customer->get('is_blocked')) { + return true; + } + + $blockedUntil = $customer->get('blocked_until'); + if ($blockedUntil && strtotime((string)$blockedUntil) > time()) { + return false; + } + + return true; + } + /** * Check if route is public * diff --git a/core/components/minishop3/src/Processors/Api/Customer/Register.php b/core/components/minishop3/src/Processors/Api/Customer/Register.php index 3723f1cb..454bbc6b 100644 --- a/core/components/minishop3/src/Processors/Api/Customer/Register.php +++ b/core/components/minishop3/src/Processors/Api/Customer/Register.php @@ -88,21 +88,18 @@ public function process() $authManager = $this->modx->services->get('ms3_auth_manager'); $session = $authManager->establishCustomerSession($customer); - if ($session) { - $tokenString = $session['token']; - $expiresAt = $session['expires_at']; - } else { - $this->modx->log( - \MODX\Revolution\modX::LOG_LEVEL_ERROR, - "[Register] Customer #{$customer->id} created but establishCustomerSession failed" - ); + if (!$session) { + return $this->failure($this->modx->lexicon('ms3_customer_err_token_create')); } + + $tokenString = $session['token']; + $expiresAt = $session['expires_at']; } $rateLimiter->reset('login', $ip); $redirectUrl = ''; - if ($tokenString !== null) { + if ($autoLogin && !$requireEmailVerification) { $redirectPageId = (int)$this->getProperty('redirect_page_id', 0); if (!$redirectPageId) { $redirectPageId = (int)$this->modx->getOption('ms3_customer_redirect_after_login', null, 0); @@ -111,18 +108,9 @@ public function process() if ($redirectPageId > 0) { $redirectUrl = $this->modx->makeUrl($redirectPageId, '', '', 'full'); } - } elseif ($autoLogin && !$requireEmailVerification) { - $loginPageId = (int)$this->modx->getOption('ms3_customer_login_page_id', null, 0); - if ($loginPageId > 0) { - $redirectUrl = $this->modx->makeUrl($loginPageId, '', '', 'full'); - } } - $messageKey = ($autoLogin && !$requireEmailVerification && $tokenString === null) - ? 'ms3_customer_register_success_login_required' - : 'ms3_customer_register_success'; - - return $this->success($this->modx->lexicon($messageKey), [ + return $this->success($this->modx->lexicon('ms3_customer_register_success'), [ 'customer' => [ 'id' => $customer->id, 'email' => $customer->get('email'), @@ -135,7 +123,6 @@ public function process() 'expires_at' => $expiresAt, 'email_verification_required' => $requireEmailVerification, 'redirect_url' => $redirectUrl, - 'auto_login' => $tokenString !== null, ]); } diff --git a/core/components/minishop3/src/Services/Customer/AuthManager.php b/core/components/minishop3/src/Services/Customer/AuthManager.php index 0bab3a2b..f7abe609 100644 --- a/core/components/minishop3/src/Services/Customer/AuthManager.php +++ b/core/components/minishop3/src/Services/Customer/AuthManager.php @@ -142,7 +142,12 @@ public function authenticate(array $credentials): ?msCustomer $customer->set('is_blocked', false); $customer->set('blocked_until', null); $customer->set('failed_login_attempts', 0); - $customer->save(); + if (!$customer->save()) { + $this->modx->log( + modX::LOG_LEVEL_ERROR, + "[AuthManager] Failed to clear block flags for customer #{$customer->id}" + ); + } } if (!$customer->get('is_active')) { @@ -156,7 +161,12 @@ public function authenticate(array $credentials): ?msCustomer $customer->set('last_login_at', date('Y-m-d H:i:s')); $customer->set('failed_login_attempts', 0); - $customer->save(); + if (!$customer->save()) { + $this->modx->log( + modX::LOG_LEVEL_ERROR, + "[AuthManager] Failed to persist last_login for customer #{$customer->id}" + ); + } $this->lastAuthFailure = 'none'; $this->modx->log( @@ -492,7 +502,12 @@ public function handleFailedLogin(msCustomer $customer): void ); } - $customer->save(); + if (!$customer->save()) { + $this->modx->log( + modX::LOG_LEVEL_ERROR, + "[AuthManager] Failed to persist failed_login_attempts for customer #{$customer->id}" + ); + } } /** diff --git a/core/components/minishop3/src/Services/Order/OrderDraftManager.php b/core/components/minishop3/src/Services/Order/OrderDraftManager.php index f7450608..3ce7f73b 100644 --- a/core/components/minishop3/src/Services/Order/OrderDraftManager.php +++ b/core/components/minishop3/src/Services/Order/OrderDraftManager.php @@ -506,18 +506,28 @@ public function bindDraftToCustomer(string $token, int $customerId, string $ctx 'context' => $ctx, ]); - if ($draft && empty($draft->get('customer_id'))) { - $draft->set('customer_id', $customerId); - $draft->save(); + if (!$draft) { + return true; + } + + if (!empty($draft->get('customer_id'))) { + return true; + } + $draft->set('customer_id', $customerId); + if (!$draft->save()) { $this->modx->log( - modX::LOG_LEVEL_INFO, - "[OrderDraftManager] Bound draft #{$draft->get('id')} to customer #{$customerId}" + modX::LOG_LEVEL_ERROR, + "[OrderDraftManager] Failed to bind draft #{$draft->get('id')} to customer #{$customerId}" ); - - return true; + return false; } - return false; + $this->modx->log( + modX::LOG_LEVEL_INFO, + "[OrderDraftManager] Bound draft #{$draft->get('id')} to customer #{$customerId}" + ); + + return true; } } diff --git a/core/components/minishop3/src/Services/TokenService.php b/core/components/minishop3/src/Services/TokenService.php index 13287a68..e65c5a01 100644 --- a/core/components/minishop3/src/Services/TokenService.php +++ b/core/components/minishop3/src/Services/TokenService.php @@ -427,19 +427,33 @@ private function ensureCustomerTokenLoaded(): ?string return $this->getCustomerToken(); } - private function renewTokenIfExpired(msCustomerToken $tokenObj): void + /** + * @return bool false when the row is expired and TTL could not be persisted + */ + private function renewTokenIfExpired(msCustomerToken $tokenObj): bool { if (!$tokenObj->isExpired()) { - return; + return true; } $ttl = (int)$this->modx->getOption('ms3_customer_token_ttl', null, 604800); + $previousExpires = $tokenObj->get('expires_at'); $tokenObj->set('expires_at', date('Y-m-d H:i:s', time() + $ttl)); - $tokenObj->save(); + + if (!$tokenObj->save()) { + $tokenObj->set('expires_at', $previousExpires); + $this->modx->log( + modX::LOG_LEVEL_ERROR, + '[TokenService] Failed to renew expired token TTL in database' + ); + return false; + } + + return true; } /** - * Write token row into $_SESSION without overwriting an authenticated customer with guest token. + * Write token row into $_SESSION. Guest tokens clear authenticated customer_id. */ private function applyTokenToSession(msCustomerToken $tokenObj): void { @@ -455,7 +469,7 @@ private function applyTokenToSession(msCustomerToken $tokenObj): void $tokenCustomerId = (int)$tokenObj->get('customer_id'); if ($tokenCustomerId > 0) { $_SESSION['ms3']['customer_id'] = $tokenCustomerId; - } elseif (empty($_SESSION['ms3']['customer_id'])) { + } else { $_SESSION['ms3']['customer_id'] = 0; } } diff --git a/core/components/minishop3/src/Utils/SessionHelper.php b/core/components/minishop3/src/Utils/SessionHelper.php index e4007706..bdd9ee96 100644 --- a/core/components/minishop3/src/Utils/SessionHelper.php +++ b/core/components/minishop3/src/Utils/SessionHelper.php @@ -9,8 +9,14 @@ class SessionHelper { public static function ensureActive(): void { + if (session_status() === PHP_SESSION_ACTIVE) { + return; + } + + session_start(); + if (session_status() !== PHP_SESSION_ACTIVE) { - session_start(); + error_log('[MiniShop3] SessionHelper: session_start() failed to activate PHP session'); } } } From 1e5d05e0420bab648eda1f57541638b63314d0f3 Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Thu, 16 Jul 2026 12:02:07 +0600 Subject: [PATCH 5/9] refactor(customer): single API token persist and email normalize path Route login/logout/guest mint through TokenService::persistApiToken so session/cookie hydrate from one DB row, move normalizeEmail to AuthManager, and drop the misplaced PHP ApiClient payload test. --- .../Api/Manager/CustomersController.php | 3 +- .../Controllers/Auth/PasswordAuthProvider.php | 5 +- .../src/Controllers/Customer/Customer.php | 3 +- .../src/Processors/Api/Customer/Login.php | 3 +- .../src/Processors/Api/Customer/Register.php | 3 +- .../src/Services/Customer/AuthManager.php | 41 +++++---- .../src/Services/Customer/RegisterService.php | 2 +- .../minishop3/src/Services/TokenService.php | 84 ++++++++++++------- .../tests/ApiClientGetPayloadTest.php | 54 ------------ .../tests/CustomerAuthNormalizeEmailTest.php | 13 ++- 10 files changed, 99 insertions(+), 112 deletions(-) delete mode 100644 core/components/minishop3/tests/ApiClientGetPayloadTest.php diff --git a/core/components/minishop3/src/Controllers/Api/Manager/CustomersController.php b/core/components/minishop3/src/Controllers/Api/Manager/CustomersController.php index 1c4ffe77..49207a0f 100644 --- a/core/components/minishop3/src/Controllers/Api/Manager/CustomersController.php +++ b/core/components/minishop3/src/Controllers/Api/Manager/CustomersController.php @@ -6,6 +6,7 @@ use MiniShop3\Model\msCustomer; use MiniShop3\Router\HttpStatus; use MiniShop3\Router\Response; +use MiniShop3\Services\Customer\AuthManager; use MODX\Revolution\modX; /** @@ -180,7 +181,7 @@ public function update(array $data = []): array if (isset($data[$field])) { $value = $data[$field]; if ($field === 'email' && is_string($value)) { - $value = PasswordAuthProvider::normalizeEmail($value); + $value = AuthManager::normalizeEmail($value); } $customer->set($field, $value); } diff --git a/core/components/minishop3/src/Controllers/Auth/PasswordAuthProvider.php b/core/components/minishop3/src/Controllers/Auth/PasswordAuthProvider.php index 8c9e79cc..e5c04210 100644 --- a/core/components/minishop3/src/Controllers/Auth/PasswordAuthProvider.php +++ b/core/components/minishop3/src/Controllers/Auth/PasswordAuthProvider.php @@ -3,6 +3,7 @@ namespace MiniShop3\Controllers\Auth; use MiniShop3\Model\msCustomer; +use MiniShop3\Services\Customer\AuthManager; use MODX\Revolution\modX; /** @@ -41,11 +42,11 @@ public function __construct(modX $modx) } /** - * Normalize email for lookup and storage. + * Normalize email for lookup and storage (delegates to AuthManager). */ public static function normalizeEmail(string $email): string { - return strtolower(trim($email)); + return AuthManager::normalizeEmail($email); } /** diff --git a/core/components/minishop3/src/Controllers/Customer/Customer.php b/core/components/minishop3/src/Controllers/Customer/Customer.php index 278efcb3..cf8d8e34 100644 --- a/core/components/minishop3/src/Controllers/Customer/Customer.php +++ b/core/components/minishop3/src/Controllers/Customer/Customer.php @@ -6,7 +6,6 @@ require_once($autoload); -use MiniShop3\Controllers\Auth\PasswordAuthProvider; use MiniShop3\MiniShop3; use MiniShop3\Model\msCustomer; use MiniShop3\Services\Customer\AuthManager; @@ -470,7 +469,7 @@ public function getOrCreate(?array $orderData = null): int */ protected function findByEmail(string $email): ?msCustomer { - $email = PasswordAuthProvider::normalizeEmail($email); + $email = AuthManager::normalizeEmail($email); if ($email === '') { return null; } diff --git a/core/components/minishop3/src/Processors/Api/Customer/Login.php b/core/components/minishop3/src/Processors/Api/Customer/Login.php index b5eb3344..9f5bb521 100644 --- a/core/components/minishop3/src/Processors/Api/Customer/Login.php +++ b/core/components/minishop3/src/Processors/Api/Customer/Login.php @@ -2,7 +2,6 @@ namespace MiniShop3\Processors\Api\Customer; -use MiniShop3\Controllers\Auth\PasswordAuthProvider; use MiniShop3\Services\Customer\AuthManager; use MiniShop3\Services\Customer\RateLimiter; use MODX\Revolution\Processors\Processor; @@ -25,7 +24,7 @@ public function process() { $this->modx->lexicon->load('minishop3:customer'); - $email = PasswordAuthProvider::normalizeEmail($this->getProperty('email', '')); + $email = AuthManager::normalizeEmail($this->getProperty('email', '')); $password = $this->getProperty('password', ''); if ($email === '' || $password === '') { diff --git a/core/components/minishop3/src/Processors/Api/Customer/Register.php b/core/components/minishop3/src/Processors/Api/Customer/Register.php index 454bbc6b..99cdb8bf 100644 --- a/core/components/minishop3/src/Processors/Api/Customer/Register.php +++ b/core/components/minishop3/src/Processors/Api/Customer/Register.php @@ -2,7 +2,6 @@ namespace MiniShop3\Processors\Api\Customer; -use MiniShop3\Controllers\Auth\PasswordAuthProvider; use MiniShop3\Services\Customer\AuthManager; use MiniShop3\Services\Customer\EmailVerificationService; use MiniShop3\Services\Customer\RateLimiter; @@ -27,7 +26,7 @@ public function process() { $this->modx->lexicon->load('minishop3:customer'); - $email = PasswordAuthProvider::normalizeEmail($this->getProperty('email', '')); + $email = AuthManager::normalizeEmail($this->getProperty('email', '')); $password = $this->getProperty('password', ''); $firstName = trim($this->getProperty('first_name', '')); $lastName = trim($this->getProperty('last_name', '')); diff --git a/core/components/minishop3/src/Services/Customer/AuthManager.php b/core/components/minishop3/src/Services/Customer/AuthManager.php index f7abe609..819ffc48 100644 --- a/core/components/minishop3/src/Services/Customer/AuthManager.php +++ b/core/components/minishop3/src/Services/Customer/AuthManager.php @@ -217,17 +217,13 @@ public function establishCustomerSession(msCustomer $customer): ?array && ((int)$tokenObj->get('customer_id') === 0 || (int)$tokenObj->get('customer_id') === (int)$customer->id); - if ($canReuse) { - $tokenObj->set('customer_id', $customer->id); - $tokenObj->set('expires_at', date('Y-m-d H:i:s', time() + $ttl)); - if (!$tokenObj->save()) { - return null; - } - } else { - $tokenObj = $this->createToken($customer, msCustomerToken::TYPE_API, $ttl); - if (!$tokenObj) { - return null; - } + $tokenObj = $tokenService->persistApiToken( + (int)$customer->id, + $canReuse ? $currentToken : null, + $ttl + ); + if (!$tokenObj) { + return null; } $tokenString = (string)$tokenObj->get('token'); @@ -241,8 +237,6 @@ public function establishCustomerSession(msCustomer $customer): ?array ); } - $tokenService->syncSessionFromToken($tokenObj); - if (session_status() === PHP_SESSION_ACTIVE) { session_regenerate_id(true); } @@ -301,8 +295,8 @@ public function logoutCurrentCustomer(): bool } CookieHelper::clearTokenCookie($this->modx); - $guest = $tokenService->generateCustomerToken(); - if ($guest['token'] === '') { + $guest = $tokenService->persistApiToken(0); + if (!$guest) { $this->modx->log( modX::LOG_LEVEL_ERROR, '[AuthManager] logoutCurrentCustomer failed to mint anonymous token' @@ -317,6 +311,14 @@ public function logoutCurrentCustomer(): bool return true; } + /** + * Canonical email normalization for lookup and storage. + */ + public static function normalizeEmail(string $email): string + { + return strtolower(trim($email)); + } + /** * Create token for customer * @@ -327,6 +329,13 @@ public function logoutCurrentCustomer(): bool */ public function createToken(msCustomer $customer, string $type = 'api', int $ttl = 86400): ?msCustomerToken { + if ($type === msCustomerToken::TYPE_API) { + /** @var TokenService $tokenService */ + $tokenService = $this->modx->services->get('ms3_token_service'); + + return $tokenService->persistApiToken((int)$customer->id, null, $ttl); + } + /** @var msCustomerToken $token */ $token = $this->modx->newObject(msCustomerToken::class); @@ -515,7 +524,7 @@ public function handleFailedLogin(msCustomer $customer): void */ public function handleFailedLoginByEmail(string $email): void { - $email = PasswordAuthProvider::normalizeEmail($email); + $email = AuthManager::normalizeEmail($email); if ($email === '') { return; } diff --git a/core/components/minishop3/src/Services/Customer/RegisterService.php b/core/components/minishop3/src/Services/Customer/RegisterService.php index b9b0b7d2..3ec123fd 100644 --- a/core/components/minishop3/src/Services/Customer/RegisterService.php +++ b/core/components/minishop3/src/Services/Customer/RegisterService.php @@ -82,7 +82,7 @@ public function setEmailVerification(EmailVerificationService $service): void */ public function register(array $data): array { - $email = PasswordAuthProvider::normalizeEmail($data['email'] ?? ''); + $email = AuthManager::normalizeEmail($data['email'] ?? ''); if (empty($email)) { return [ diff --git a/core/components/minishop3/src/Services/TokenService.php b/core/components/minishop3/src/Services/TokenService.php index e65c5a01..e56b4c53 100644 --- a/core/components/minishop3/src/Services/TokenService.php +++ b/core/components/minishop3/src/Services/TokenService.php @@ -62,45 +62,75 @@ public function generateCustomerToken(?int $ttl = null): array $customerId = (int)($_SESSION['ms3']['customer_id'] ?? 0); - $token = bin2hex(random_bytes(32)); + $tokenObj = $this->persistApiToken($customerId, null, $ttl); + if (!$tokenObj) { + return ['token' => '', 'expires' => 0, 'lifetime' => 0]; + } + + $expires = (int)strtotime((string)$tokenObj->get('expires_at')); + + $this->modx->log( + modX::LOG_LEVEL_INFO, + "[TokenService] Generated customer token for customer_id={$customerId}, expires: " + . $tokenObj->get('expires_at') + ); + + return [ + 'token' => (string)$tokenObj->get('token'), + 'expires' => $expires, + 'lifetime' => max(0, $expires - time()) * 1000, + ]; + } + + /** + * Persist an API token row and hydrate session/cookie from that DB row. + * + * Single mint/update path for guest, login bind, and logout anonymous token. + * + * @param int $customerId 0 = guest + * @param string|null $reuseToken update this token when present in DB; otherwise mint + */ + public function persistApiToken(int $customerId, ?string $reuseToken = null, ?int $ttl = null): ?msCustomerToken + { + SessionHelper::ensureActive(); if ($ttl === null) { $ttl = (int)$this->modx->getOption('ms3_customer_token_ttl', null, 604800); } $expiresAt = date('Y-m-d H:i:s', time() + $ttl); + $tokenObj = null; - $tokenObj = $this->modx->newObject(\MiniShop3\Model\msCustomerToken::class); - $tokenObj->set('customer_id', $customerId); - $tokenObj->set('token', $token); - $tokenObj->set('type', \MiniShop3\Model\msCustomerToken::TYPE_API); - $tokenObj->set('expires_at', $expiresAt); - $tokenObj->set('created_at', date('Y-m-d H:i:s')); + if ($reuseToken !== null && $reuseToken !== '') { + $tokenObj = $this->modx->getObject(msCustomerToken::class, [ + 'token' => $reuseToken, + 'type' => msCustomerToken::TYPE_API, + ]); + } + + if ($tokenObj) { + $tokenObj->set('customer_id', $customerId); + $tokenObj->set('expires_at', $expiresAt); + } else { + $tokenObj = $this->modx->newObject(msCustomerToken::class); + $tokenObj->set('customer_id', $customerId); + $tokenObj->set('token', bin2hex(random_bytes(32))); + $tokenObj->set('type', msCustomerToken::TYPE_API); + $tokenObj->set('expires_at', $expiresAt); + $tokenObj->set('created_at', date('Y-m-d H:i:s')); + } if (!$tokenObj->save()) { $this->modx->log( modX::LOG_LEVEL_ERROR, - "[TokenService] Failed to save token to database" + '[TokenService] Failed to persist API token' ); - return ['token' => '', 'expires' => 0, 'lifetime' => 0]; + return null; } - $_SESSION['ms3']['customer_token'] = $token; - $_SESSION['ms3']['customer_token_expires'] = time() + $ttl; + $this->syncSessionFromToken($tokenObj); - // Set httpOnly cookie - CookieHelper::setTokenCookie($this->modx, $token); - - $this->modx->log( - modX::LOG_LEVEL_INFO, - "[TokenService] Generated customer token for customer_id={$customerId}, expires: " . $expiresAt - ); - - return [ - 'token' => $token, - 'expires' => time() + $ttl, - 'lifetime' => $ttl * 1000, - ]; + return $tokenObj; } /** @@ -136,15 +166,13 @@ public function restoreSessionFromCookie(): bool return false; } - $this->renewTokenIfExpired($tokenObj); - $this->applyTokenToSession($tokenObj); - CookieHelper::setTokenCookie($this->modx, $cookieToken); + $this->syncSessionFromToken($tokenObj); return true; } /** - * Resolve existing token or create new one (session → cookie restore → mint). + * Resolve existing token or mint one (session → cookie restore → persistApiToken). */ public function resolveOrCreateToken(): string { diff --git a/core/components/minishop3/tests/ApiClientGetPayloadTest.php b/core/components/minishop3/tests/ApiClientGetPayloadTest.php deleted file mode 100644 index 33c0099f..00000000 --- a/core/components/minishop3/tests/ApiClientGetPayloadTest.php +++ /dev/null @@ -1,54 +0,0 @@ - true, - 'data' => ['token' => 'abc', 'redirect_url' => '/profile'], - 'object' => null, -]); -if (!is_array($web) || ($web['redirect_url'] ?? '') !== '/profile') { - $fail('Web API data payload must expose redirect_url'); -} - -$legacy = $getPayload([ - 'success' => true, - 'object' => ['token' => 'xyz', 'redirect_url' => '/cabinet'], -]); -if (!is_array($legacy) || ($legacy['redirect_url'] ?? '') !== '/cabinet') { - $fail('Legacy object payload must expose redirect_url'); -} - -$preferData = $getPayload([ - 'data' => ['redirect_url' => '/from-data'], - 'object' => ['redirect_url' => '/from-object'], -]); -if (($preferData['redirect_url'] ?? '') !== '/from-data') { - $fail('data must win over object'); -} - -if ($getPayload(null) !== null) { - $fail('null input must return null'); -} - -fwrite(STDOUT, "OK ApiClientGetPayloadTest\n"); -exit(0); diff --git a/core/components/minishop3/tests/CustomerAuthNormalizeEmailTest.php b/core/components/minishop3/tests/CustomerAuthNormalizeEmailTest.php index 19f90e19..3a564672 100644 --- a/core/components/minishop3/tests/CustomerAuthNormalizeEmailTest.php +++ b/core/components/minishop3/tests/CustomerAuthNormalizeEmailTest.php @@ -10,7 +10,7 @@ require __DIR__ . '/../vendor/autoload.php'; -use MiniShop3\Controllers\Auth\PasswordAuthProvider; +use MiniShop3\Services\Customer\AuthManager; $fail = static function (string $message): never { fwrite(STDERR, "FAIL: {$message}\n"); @@ -23,9 +23,14 @@ } }; -$assertSame('user@example.com', PasswordAuthProvider::normalizeEmail('User@Example.COM'), 'lowercases email'); -$assertSame('user@example.com', PasswordAuthProvider::normalizeEmail(' user@example.com '), 'trims email'); -$assertSame('', PasswordAuthProvider::normalizeEmail(' '), 'blank becomes empty'); +$assertSame('user@example.com', AuthManager::normalizeEmail('User@Example.COM'), 'lowercases email'); +$assertSame('user@example.com', AuthManager::normalizeEmail(' user@example.com '), 'trims email'); +$assertSame('', AuthManager::normalizeEmail(' '), 'blank becomes empty'); +$assertSame( + 'user@example.com', + \MiniShop3\Controllers\Auth\PasswordAuthProvider::normalizeEmail('User@Example.COM'), + 'PasswordAuthProvider delegates to AuthManager' +); fwrite(STDOUT, "OK CustomerAuthNormalizeEmailTest\n"); exit(0); From fa15199585761a313a0b79bc8d32486aacb2a861 Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Thu, 16 Jul 2026 12:04:54 +0600 Subject: [PATCH 6/9] fix(customer): clarify login failures and cover session sticky policy Return distinct blocked/inactive login messages without lockout increment, add legacy email lookup with soft normalize on success, drop dead remember checkbox, and add smoke tests for guest wipe and token reuse guards. --- .../chunks/ms3_customer_unauthorized.tpl | 7 -- .../minishop3/lexicon/en/customer.inc.php | 2 + .../minishop3/lexicon/ru/customer.inc.php | 2 + .../Controllers/Auth/PasswordAuthProvider.php | 23 +++++- .../src/Controllers/Customer/Customer.php | 17 ++++- .../src/Processors/Api/Customer/Login.php | 15 +++- .../src/Services/Customer/AuthManager.php | 57 ++++++++++++--- .../minishop3/src/Services/TokenService.php | 14 ++-- .../CustomerAuthSessionSemanticsTest.php | 73 +++++++++++++++++++ 9 files changed, 179 insertions(+), 31 deletions(-) create mode 100644 core/components/minishop3/tests/CustomerAuthSessionSemanticsTest.php diff --git a/core/components/minishop3/elements/chunks/ms3_customer_unauthorized.tpl b/core/components/minishop3/elements/chunks/ms3_customer_unauthorized.tpl index fda92998..392e35e4 100644 --- a/core/components/minishop3/elements/chunks/ms3_customer_unauthorized.tpl +++ b/core/components/minishop3/elements/chunks/ms3_customer_unauthorized.tpl @@ -48,13 +48,6 @@ placeholder="{'ms3_customer_password_placeholder' | lexicon}" required> -
- - -
- diff --git a/core/components/minishop3/lexicon/en/customer.inc.php b/core/components/minishop3/lexicon/en/customer.inc.php index a9775c44..14a93af0 100644 --- a/core/components/minishop3/lexicon/en/customer.inc.php +++ b/core/components/minishop3/lexicon/en/customer.inc.php @@ -45,6 +45,8 @@ // Errors - Authentication $_lang['ms3_customer_err_login_required'] = 'Please provide email and password'; $_lang['ms3_customer_err_login_invalid'] = 'Invalid email or password'; +$_lang['ms3_customer_err_login_blocked'] = 'This account is temporarily blocked. Try again later.'; +$_lang['ms3_customer_err_login_inactive'] = 'This account is inactive. Contact the store administrator.'; $_lang['ms3_customer_err_login_rate_limit'] = 'Too many login attempts ({attempts}/{max}). Try again in {minutes} minutes.'; $_lang['ms3_customer_err_email_required'] = 'Email is required'; $_lang['ms3_customer_err_email_invalid'] = 'Invalid email format'; diff --git a/core/components/minishop3/lexicon/ru/customer.inc.php b/core/components/minishop3/lexicon/ru/customer.inc.php index 9154c6f6..a9111b0f 100644 --- a/core/components/minishop3/lexicon/ru/customer.inc.php +++ b/core/components/minishop3/lexicon/ru/customer.inc.php @@ -45,6 +45,8 @@ // Errors - Authentication $_lang['ms3_customer_err_login_required'] = 'Укажите email и пароль'; $_lang['ms3_customer_err_login_invalid'] = 'Неверный email или пароль'; +$_lang['ms3_customer_err_login_blocked'] = 'Аккаунт временно заблокирован. Попробуйте позже.'; +$_lang['ms3_customer_err_login_inactive'] = 'Аккаунт неактивен. Обратитесь к администратору магазина.'; $_lang['ms3_customer_err_login_rate_limit'] = 'Превышен лимит попыток входа ({attempts}/{max}). Попробуйте через {minutes} минут.'; $_lang['ms3_customer_err_email_required'] = 'Email обязателен для заполнения'; $_lang['ms3_customer_err_email_invalid'] = 'Указан некорректный email'; diff --git a/core/components/minishop3/src/Controllers/Auth/PasswordAuthProvider.php b/core/components/minishop3/src/Controllers/Auth/PasswordAuthProvider.php index e5c04210..806691ef 100644 --- a/core/components/minishop3/src/Controllers/Auth/PasswordAuthProvider.php +++ b/core/components/minishop3/src/Controllers/Auth/PasswordAuthProvider.php @@ -68,11 +68,19 @@ public function authenticate(array $credentials): ?msCustomer return null; } - // Find customer by email - /** @var msCustomer $customer */ + // Normalized lookup, then legacy mixed-case exact match (utf8mb4_bin / old rows). + /** @var msCustomer|null $customer */ $customer = $this->modx->getObject(msCustomer::class, [ 'email' => $email, ]); + if (!$customer) { + $raw = trim((string)($credentials['email'] ?? '')); + if ($raw !== '' && $raw !== $email) { + $customer = $this->modx->getObject(msCustomer::class, [ + 'email' => $raw, + ]); + } + } if (!$customer) { $this->modx->log( @@ -101,6 +109,17 @@ public function authenticate(array $credentials): ?msCustomer return null; } + // Soft-migrate legacy mixed-case emails to canonical lowercase. + if ((string)$customer->get('email') !== $email) { + $customer->set('email', $email); + if (!$customer->save()) { + $this->modx->log( + modX::LOG_LEVEL_WARN, + "[PasswordAuthProvider] Failed to normalize email for customer #{$customer->id}" + ); + } + } + // Check if password hash needs to be updated (if bcrypt settings changed) if (password_needs_rehash($hashedPassword, PASSWORD_BCRYPT)) { $newHash = password_hash($password, PASSWORD_BCRYPT); diff --git a/core/components/minishop3/src/Controllers/Customer/Customer.php b/core/components/minishop3/src/Controllers/Customer/Customer.php index cf8d8e34..96e00884 100644 --- a/core/components/minishop3/src/Controllers/Customer/Customer.php +++ b/core/components/minishop3/src/Controllers/Customer/Customer.php @@ -469,12 +469,23 @@ public function getOrCreate(?array $orderData = null): int */ protected function findByEmail(string $email): ?msCustomer { - $email = AuthManager::normalizeEmail($email); - if ($email === '') { + $normalized = AuthManager::normalizeEmail($email); + if ($normalized === '') { return null; } - return $this->modx->getObject(msCustomer::class, ['email' => $email]); + /** @var msCustomer|null $customer */ + $customer = $this->modx->getObject(msCustomer::class, ['email' => $normalized]); + if ($customer) { + return $customer; + } + + $raw = trim($email); + if ($raw !== '' && $raw !== $normalized) { + return $this->modx->getObject(msCustomer::class, ['email' => $raw]) ?: null; + } + + return null; } /** diff --git a/core/components/minishop3/src/Processors/Api/Customer/Login.php b/core/components/minishop3/src/Processors/Api/Customer/Login.php index 9f5bb521..2cbe6354 100644 --- a/core/components/minishop3/src/Processors/Api/Customer/Login.php +++ b/core/components/minishop3/src/Processors/Api/Customer/Login.php @@ -58,16 +58,25 @@ public function process() ]); if (!$customer) { - if ($authManager->getLastAuthFailure() === 'invalid_credentials') { + // Only wrong password / unknown email increment lockout counter. + // blocked / inactive keep lastAuthFailure and must not call handleFailedLoginByEmail. + $failure = $authManager->getLastAuthFailure(); + if ($failure === 'invalid_credentials') { $authManager->handleFailedLoginByEmail($email); } $this->modx->log( \MODX\Revolution\modX::LOG_LEVEL_WARN, - "[Login] Failed login attempt for email: {$email} from IP: {$ip}" + "[Login] Failed login attempt for email: {$email} from IP: {$ip} (reason: {$failure})" ); - return $this->failure($this->modx->lexicon('ms3_customer_err_login_invalid')); + $messageKey = match ($failure) { + 'blocked' => 'ms3_customer_err_login_blocked', + 'inactive' => 'ms3_customer_err_login_inactive', + default => 'ms3_customer_err_login_invalid', + }; + + return $this->failure($this->modx->lexicon($messageKey)); } $rateLimiter->reset('login', $ip); diff --git a/core/components/minishop3/src/Services/Customer/AuthManager.php b/core/components/minishop3/src/Services/Customer/AuthManager.php index 819ffc48..b22cfe60 100644 --- a/core/components/minishop3/src/Services/Customer/AuthManager.php +++ b/core/components/minishop3/src/Services/Customer/AuthManager.php @@ -213,9 +213,10 @@ public function establishCustomerSession(msCustomer $customer): ?array } // Rebind only guest tokens (preserve cart). Never hijack another customer's token. - $canReuse = $tokenObj - && ((int)$tokenObj->get('customer_id') === 0 - || (int)$tokenObj->get('customer_id') === (int)$customer->id); + $canReuse = self::canReuseApiToken( + $tokenObj ? (int)$tokenObj->get('customer_id') : -1, + (int)$customer->id + ); $tokenObj = $tokenService->persistApiToken( (int)$customer->id, @@ -311,6 +312,20 @@ public function logoutCurrentCustomer(): bool return true; } + /** + * Whether an existing API token row may be rebound to the logging-in customer. + * + * @param int $tokenCustomerId -1 when no token row exists + */ + public static function canReuseApiToken(int $tokenCustomerId, int $loggingInCustomerId): bool + { + if ($tokenCustomerId < 0) { + return false; + } + + return $tokenCustomerId === 0 || $tokenCustomerId === $loggingInCustomerId; + } + /** * Canonical email normalization for lookup and storage. */ @@ -319,6 +334,32 @@ public static function normalizeEmail(string $email): string return strtolower(trim($email)); } + /** + * Lookup customer by normalized email, then legacy mixed-case exact match. + */ + public function findCustomerByEmail(string $email): ?msCustomer + { + $normalized = self::normalizeEmail($email); + if ($normalized === '') { + return null; + } + + /** @var msCustomer|null $customer */ + $customer = $this->modx->getObject(msCustomer::class, ['email' => $normalized]); + if ($customer) { + return $customer; + } + + $raw = trim($email); + if ($raw !== '' && $raw !== $normalized) { + /** @var msCustomer|null $legacy */ + $legacy = $this->modx->getObject(msCustomer::class, ['email' => $raw]); + return $legacy ?: null; + } + + return null; + } + /** * Create token for customer * @@ -520,17 +561,11 @@ public function handleFailedLogin(msCustomer $customer): void } /** - * Record failed login by normalized email when the account exists. + * Record failed login by normalized/legacy email when the account exists. */ public function handleFailedLoginByEmail(string $email): void { - $email = AuthManager::normalizeEmail($email); - if ($email === '') { - return; - } - - /** @var msCustomer|null $customer */ - $customer = $this->modx->getObject(msCustomer::class, ['email' => $email]); + $customer = $this->findCustomerByEmail($email); if ($customer) { $this->handleFailedLogin($customer); } diff --git a/core/components/minishop3/src/Services/TokenService.php b/core/components/minishop3/src/Services/TokenService.php index e56b4c53..33257133 100644 --- a/core/components/minishop3/src/Services/TokenService.php +++ b/core/components/minishop3/src/Services/TokenService.php @@ -495,10 +495,14 @@ private function applyTokenToSession(msCustomerToken $tokenObj): void $_SESSION['ms3']['customer_token_expires'] = strtotime($tokenObj->get('expires_at')); $tokenCustomerId = (int)$tokenObj->get('customer_id'); - if ($tokenCustomerId > 0) { - $_SESSION['ms3']['customer_id'] = $tokenCustomerId; - } else { - $_SESSION['ms3']['customer_id'] = 0; - } + $_SESSION['ms3']['customer_id'] = self::sessionCustomerIdFromTokenRow($tokenCustomerId); + } + + /** + * Session customer_id derived from an API token row (guest clears auth). + */ + public static function sessionCustomerIdFromTokenRow(int $tokenCustomerId): int + { + return $tokenCustomerId > 0 ? $tokenCustomerId : 0; } } diff --git a/core/components/minishop3/tests/CustomerAuthSessionSemanticsTest.php b/core/components/minishop3/tests/CustomerAuthSessionSemanticsTest.php new file mode 100644 index 00000000..7323301f --- /dev/null +++ b/core/components/minishop3/tests/CustomerAuthSessionSemanticsTest.php @@ -0,0 +1,73 @@ + Date: Thu, 16 Jul 2026 12:06:58 +0600 Subject: [PATCH 7/9] fix(customer): rotate API token on login to stop session fixation Mint a fresh token on establishCustomerSession, transfer guest/own draft cart then revoke the old token, require session token ownership in middleware/checkAuth, and clear local session after password reset revoke. --- .../src/Middleware/TokenMiddleware.php | 35 ++++++- .../Processors/Api/Customer/ResetPassword.php | 1 + .../src/Services/Customer/AuthManager.php | 92 ++++++++++++++----- .../Services/Customer/CustomerPageService.php | 61 ++++++++++++ .../src/Services/Order/OrderDraftManager.php | 60 ++++++++++++ .../CustomerAuthSessionSemanticsTest.php | 20 ++-- 6 files changed, 236 insertions(+), 33 deletions(-) diff --git a/core/components/minishop3/src/Middleware/TokenMiddleware.php b/core/components/minishop3/src/Middleware/TokenMiddleware.php index e1acf36b..427a162a 100644 --- a/core/components/minishop3/src/Middleware/TokenMiddleware.php +++ b/core/components/minishop3/src/Middleware/TokenMiddleware.php @@ -74,14 +74,22 @@ public function handle(array $params) SessionHelper::ensureActive(); - // For non-public routes: check session first + // For non-public routes: session shortcut only when token still belongs to customer if (!$isPublic && !empty($_SESSION['ms3']['customer_id'])) { $customer = $this->modx->getObject(\MiniShop3\Model\msCustomer::class, $_SESSION['ms3']['customer_id']); - if ($customer && $this->isCustomerSessionAllowed($customer)) { + if ( + $customer + && $this->isCustomerSessionAllowed($customer) + && $this->sessionTokenMatchesCustomer((int)$customer->id) + ) { return null; } - unset($_SESSION['ms3']['customer_id']); + unset( + $_SESSION['ms3']['customer_id'], + $_SESSION['ms3']['customer_token'], + $_SESSION['ms3']['customer_token_expires'] + ); } // Resolve token from multiple sources @@ -194,6 +202,27 @@ private function isCustomerSessionAllowed(\MiniShop3\Model\msCustomer $customer) return true; } + /** + * Ensure PHP session / cookie API token still maps to this customer in DB. + */ + private function sessionTokenMatchesCustomer(int $customerId): bool + { + $token = (string)($_SESSION['ms3']['customer_token'] ?? ''); + if ($token === '') { + $token = CookieHelper::getTokenFromCookie(); + } + if ($token === '') { + return false; + } + + $tokenObj = $this->modx->getObject(\MiniShop3\Model\msCustomerToken::class, [ + 'token' => $token, + 'type' => \MiniShop3\Model\msCustomerToken::TYPE_API, + ]); + + return $tokenObj && (int)$tokenObj->get('customer_id') === $customerId; + } + /** * Check if route is public * diff --git a/core/components/minishop3/src/Processors/Api/Customer/ResetPassword.php b/core/components/minishop3/src/Processors/Api/Customer/ResetPassword.php index 34e55fa5..3a175d6c 100644 --- a/core/components/minishop3/src/Processors/Api/Customer/ResetPassword.php +++ b/core/components/minishop3/src/Processors/Api/Customer/ResetPassword.php @@ -68,6 +68,7 @@ public function process() } $authManager->revokeTokens($customer); + $authManager->invalidateLocalSessionForCustomer($customer); $this->modx->log( \MODX\Revolution\modX::LOG_LEVEL_INFO, diff --git a/core/components/minishop3/src/Services/Customer/AuthManager.php b/core/components/minishop3/src/Services/Customer/AuthManager.php index b22cfe60..649e067c 100644 --- a/core/components/minishop3/src/Services/Customer/AuthManager.php +++ b/core/components/minishop3/src/Services/Customer/AuthManager.php @@ -188,9 +188,11 @@ public function authenticate(array $credentials): ?msCustomer } /** - * Bind authenticated customer to API token, session, cookie, and draft order. + * Bind authenticated customer to a fresh API token, session, cookie, and draft order. * - * Reuses guest token from cookie/session when possible so cart is preserved. + * Always mints a new token (anti session-fixation). Guest/own previous token may + * transfer the draft cart, then the old token row is removed so a planted cookie + * cannot keep access after login. * * @return array{token: string, expires_at: string}|null */ @@ -202,27 +204,18 @@ public function establishCustomerSession(msCustomer $customer): ?array /** @var TokenService $tokenService */ $tokenService = $this->modx->services->get('ms3_token_service'); - $currentToken = $tokenService->getBindableTokenString(); + $previousToken = $tokenService->getBindableTokenString(); - $tokenObj = null; - if ($currentToken !== '') { - $tokenObj = $this->modx->getObject(msCustomerToken::class, [ - 'token' => $currentToken, + $previousTokenObj = null; + if ($previousToken !== '') { + $previousTokenObj = $this->modx->getObject(msCustomerToken::class, [ + 'token' => $previousToken, 'type' => msCustomerToken::TYPE_API, ]); } - // Rebind only guest tokens (preserve cart). Never hijack another customer's token. - $canReuse = self::canReuseApiToken( - $tokenObj ? (int)$tokenObj->get('customer_id') : -1, - (int)$customer->id - ); - - $tokenObj = $tokenService->persistApiToken( - (int)$customer->id, - $canReuse ? $currentToken : null, - $ttl - ); + // Always rotate API token on login/register (do not upgrade a planted guest token). + $tokenObj = $tokenService->persistApiToken((int)$customer->id, null, $ttl); if (!$tokenObj) { return null; } @@ -231,7 +224,27 @@ public function establishCustomerSession(msCustomer $customer): ?array /** @var OrderDraftManager $draftManager */ $draftManager = $this->modx->services->get('ms3_order_draft_manager'); - if (!$draftManager->bindDraftToCustomer($tokenString, $customer->id)) { + + $previousCustomerId = $previousTokenObj + ? (int)$previousTokenObj->get('customer_id') + : -1; + + if ( + $previousToken !== '' + && $previousToken !== $tokenString + && self::canTransferCartFromToken($previousCustomerId, (int)$customer->id) + ) { + if (!$draftManager->transferDraftToToken($previousToken, $tokenString, (int)$customer->id)) { + $this->modx->log( + modX::LOG_LEVEL_WARN, + "[AuthManager] transferDraftToToken failed for customer #{$customer->id}" + ); + } + + if ($previousTokenObj) { + $previousTokenObj->remove(); + } + } elseif (!$draftManager->bindDraftToCustomer($tokenString, (int)$customer->id)) { $this->modx->log( modX::LOG_LEVEL_WARN, "[AuthManager] bindDraftToCustomer failed for customer #{$customer->id}" @@ -313,11 +326,40 @@ public function logoutCurrentCustomer(): bool } /** - * Whether an existing API token row may be rebound to the logging-in customer. + * After API tokens were revoked, drop local PHP session/cookie if it belonged to that customer. + */ + public function invalidateLocalSessionForCustomer(msCustomer $customer): void + { + SessionHelper::ensureActive(); + + if ((int)($_SESSION['ms3']['customer_id'] ?? 0) !== (int)$customer->id) { + return; + } + + if (isset($_SESSION['ms3'])) { + unset( + $_SESSION['ms3']['customer_id'], + $_SESSION['ms3']['customer_token'], + $_SESSION['ms3']['customer_token_expires'] + ); + } + CookieHelper::clearTokenCookie($this->modx); + + /** @var TokenService $tokenService */ + $tokenService = $this->modx->services->get('ms3_token_service'); + $tokenService->persistApiToken(0); + + if (session_status() === PHP_SESSION_ACTIVE) { + session_regenerate_id(true); + } + } + + /** + * Whether cart/draft may be moved from a previous API token onto the new login token. * * @param int $tokenCustomerId -1 when no token row exists */ - public static function canReuseApiToken(int $tokenCustomerId, int $loggingInCustomerId): bool + public static function canTransferCartFromToken(int $tokenCustomerId, int $loggingInCustomerId): bool { if ($tokenCustomerId < 0) { return false; @@ -326,6 +368,14 @@ public static function canReuseApiToken(int $tokenCustomerId, int $loggingInCust return $tokenCustomerId === 0 || $tokenCustomerId === $loggingInCustomerId; } + /** + * @deprecated Use canTransferCartFromToken(); kept for call-site compatibility in tests. + */ + public static function canReuseApiToken(int $tokenCustomerId, int $loggingInCustomerId): bool + { + return self::canTransferCartFromToken($tokenCustomerId, $loggingInCustomerId); + } + /** * Canonical email normalization for lookup and storage. */ diff --git a/core/components/minishop3/src/Services/Customer/CustomerPageService.php b/core/components/minishop3/src/Services/Customer/CustomerPageService.php index 0e17f131..8201b670 100644 --- a/core/components/minishop3/src/Services/Customer/CustomerPageService.php +++ b/core/components/minishop3/src/Services/Customer/CustomerPageService.php @@ -5,6 +5,7 @@ use MiniShop3\MiniShop3; use MiniShop3\Model\msCustomer; use MiniShop3\Services\TokenService; +use MiniShop3\Utils\CookieHelper; use MiniShop3\Utils\SessionHelper; use MODX\Revolution\modX; use ModxPro\PdoTools\Fetch; @@ -87,6 +88,16 @@ public function checkAuth(): bool $this->customerId = (int)$_SESSION['ms3']['customer_id']; + if (!$this->sessionTokenMatchesCustomer($this->customerId)) { + $this->clearAuthSession(); + $this->modx->log( + modX::LOG_LEVEL_WARN, + "[CustomerPageService] Session customer #{$this->customerId} does not match API token" + ); + $this->customerId = null; + return false; + } + $this->customer = $this->modx->getObject(msCustomer::class, $this->customerId); if (!$this->customer) { @@ -94,12 +105,62 @@ public function checkAuth(): bool modX::LOG_LEVEL_WARN, "[CustomerPageService] Customer #{$this->customerId} not found in database" ); + $this->clearAuthSession(); + $this->customerId = null; return false; } + if (!$this->customer->get('is_active')) { + $this->clearAuthSession(); + $this->customerId = null; + $this->customer = null; + return false; + } + + if ($this->customer->get('is_blocked')) { + $blockedUntil = $this->customer->get('blocked_until'); + if ($blockedUntil && strtotime((string)$blockedUntil) > time()) { + $this->clearAuthSession(); + $this->customerId = null; + $this->customer = null; + return false; + } + } + return true; } + private function sessionTokenMatchesCustomer(int $customerId): bool + { + $token = (string)($_SESSION['ms3']['customer_token'] ?? ''); + if ($token === '') { + $token = CookieHelper::getTokenFromCookie(); + } + if ($token === '') { + return false; + } + + $tokenObj = $this->modx->getObject(\MiniShop3\Model\msCustomerToken::class, [ + 'token' => $token, + 'type' => \MiniShop3\Model\msCustomerToken::TYPE_API, + ]); + + return $tokenObj && (int)$tokenObj->get('customer_id') === $customerId; + } + + private function clearAuthSession(): void + { + if (!isset($_SESSION['ms3'])) { + return; + } + + unset( + $_SESSION['ms3']['customer_id'], + $_SESSION['ms3']['customer_token'], + $_SESSION['ms3']['customer_token_expires'] + ); + } + /** * Get authenticated customer ID * diff --git a/core/components/minishop3/src/Services/Order/OrderDraftManager.php b/core/components/minishop3/src/Services/Order/OrderDraftManager.php index 3ce7f73b..61f689ca 100644 --- a/core/components/minishop3/src/Services/Order/OrderDraftManager.php +++ b/core/components/minishop3/src/Services/Order/OrderDraftManager.php @@ -530,4 +530,64 @@ public function bindDraftToCustomer(string $token, int $customerId, string $ctx return true; } + + /** + * Move a draft order from one session token to another and bind customer_id. + * + * Used after login token rotation so the cart survives while the old token is revoked. + */ + public function transferDraftToToken( + string $fromToken, + string $toToken, + int $customerId, + string $ctx = 'web' + ): bool { + if ($fromToken === '' || $toToken === '' || $customerId <= 0) { + return false; + } + + if ($fromToken === $toToken) { + return $this->bindDraftToCustomer($toToken, $customerId, $ctx); + } + + $statusDraft = (int)$this->modx->getOption('ms3_status_draft', null, 1) ?: 1; + + $draft = $this->modx->getObject(msOrder::class, [ + 'token' => $fromToken, + 'status_id' => $statusDraft, + 'context' => $ctx, + ]); + + if (!$draft) { + return true; + } + + $existingCustomerId = (int)$draft->get('customer_id'); + if ($existingCustomerId > 0 && $existingCustomerId !== $customerId) { + $this->modx->log( + modX::LOG_LEVEL_WARN, + "[OrderDraftManager] Refused draft transfer #{$draft->get('id')} " + . "(owned by customer #{$existingCustomerId})" + ); + return false; + } + + $draft->set('customer_id', $customerId); + $draft->set('token', $toToken); + if (!$draft->save()) { + $this->modx->log( + modX::LOG_LEVEL_ERROR, + "[OrderDraftManager] Failed to transfer draft #{$draft->get('id')} " + . "to token for customer #{$customerId}" + ); + return false; + } + + $this->modx->log( + modX::LOG_LEVEL_INFO, + "[OrderDraftManager] Transferred draft #{$draft->get('id')} to customer #{$customerId}" + ); + + return true; + } } diff --git a/core/components/minishop3/tests/CustomerAuthSessionSemanticsTest.php b/core/components/minishop3/tests/CustomerAuthSessionSemanticsTest.php index 7323301f..0158f8df 100644 --- a/core/components/minishop3/tests/CustomerAuthSessionSemanticsTest.php +++ b/core/components/minishop3/tests/CustomerAuthSessionSemanticsTest.php @@ -3,9 +3,9 @@ /** * Session/token policy smoke checks for #285 (without MODX bootstrap). * - * Covers the sticky-auth contracts that broke storefront login: - * - guest token must clear session customer_id - * - login must not rebind another customer's API token + * Covers sticky-auth + anti-fixation contracts: + * - guest token must clear session customer_id on hydrate + * - login rotates token; cart may transfer only from guest/own token * - failed-login lockout applies only to invalid_credentials * * Run: php tests/CustomerAuthSessionSemanticsTest.php @@ -46,7 +46,6 @@ $assertSame(0, TokenService::sessionCustomerIdFromTokenRow(0), 'guest token clears customer_id'); $assertSame(0, TokenService::sessionCustomerIdFromTokenRow(-1), 'invalid token customer_id clears auth'); -// Simulate reload: after login session has customer_id; guest hydrate must wipe it. $sessionCustomerId = 7; $sessionCustomerId = TokenService::sessionCustomerIdFromTokenRow(0); $assertSame(0, $sessionCustomerId, 'reload with guest cookie must not keep auth session'); @@ -54,11 +53,14 @@ $sessionCustomerId = TokenService::sessionCustomerIdFromTokenRow(7); $assertSame(7, $sessionCustomerId, 'reload with auth cookie restores customer_id'); -// --- establishCustomerSession reuse guard --- -$assertFalse(AuthManager::canReuseApiToken(-1, 5), 'missing token cannot reuse'); -$assertTrue(AuthManager::canReuseApiToken(0, 5), 'guest token may rebind'); -$assertTrue(AuthManager::canReuseApiToken(5, 5), 'own token may rebind'); -$assertFalse(AuthManager::canReuseApiToken(9, 5), 'other customer token must mint new'); +// --- login token rotation: cart transfer eligibility (old token is never upgraded in place) --- +$assertFalse(AuthManager::canTransferCartFromToken(-1, 5), 'missing token cannot transfer cart'); +$assertTrue(AuthManager::canTransferCartFromToken(0, 5), 'guest token may transfer cart then be revoked'); +$assertTrue(AuthManager::canTransferCartFromToken(5, 5), 'own token may transfer cart then be revoked'); +$assertFalse(AuthManager::canTransferCartFromToken(9, 5), 'other customer token must not transfer cart'); + +// Alias kept for older call sites / docs +$assertTrue(AuthManager::canReuseApiToken(0, 5), 'canReuseApiToken aliases transfer policy'); // --- lockout reasons: only invalid_credentials should trigger handleFailedLoginByEmail --- $shouldIncrementLockout = static function (string $lastAuthFailure): bool { From 152fdc4e02cdea9e95c50e9ce9287d2343719f3b Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Thu, 16 Jul 2026 12:14:10 +0600 Subject: [PATCH 8/9] fix(customer): address post-rotation auth review findings Pass raw email into authenticate/register for legacy case lookup, always revoke the previous browser token after login rotation, enforce token expiry in session ownership checks, and harden logout/clearAuthSession. --- .../elements/snippets/ms3_customer.php | 12 +++++- .../src/Middleware/TokenMiddleware.php | 28 +++---------- .../src/Processors/Api/Customer/Login.php | 12 +++--- .../src/Processors/Api/Customer/Register.php | 5 ++- .../src/Services/Customer/AuthManager.php | 9 ++-- .../Services/Customer/CustomerPageService.php | 39 ++++++----------- .../src/Services/Customer/RegisterService.php | 10 ++++- .../minishop3/src/Services/TokenService.php | 42 ++++++++++++++++++- 8 files changed, 93 insertions(+), 64 deletions(-) diff --git a/core/components/minishop3/elements/snippets/ms3_customer.php b/core/components/minishop3/elements/snippets/ms3_customer.php index 9adba9dc..5820972c 100644 --- a/core/components/minishop3/elements/snippets/ms3_customer.php +++ b/core/components/minishop3/elements/snippets/ms3_customer.php @@ -28,7 +28,17 @@ if (isset($_GET['action']) && $_GET['action'] === 'logout') { /** @var \MiniShop3\Services\Customer\AuthManager $authManager */ $authManager = $modx->services->get('ms3_auth_manager'); - $authManager->logoutCurrentCustomer(); + if (!$authManager->logoutCurrentCustomer()) { + $modx->log( + \MODX\Revolution\modX::LOG_LEVEL_ERROR, + '[ms3_customer] logoutCurrentCustomer failed; forcing guest token mint' + ); + if ($modx->services->has('ms3_token_service')) { + /** @var \MiniShop3\Services\TokenService $tokenService */ + $tokenService = $modx->services->get('ms3_token_service'); + $tokenService->persistApiToken(0); + } + } $loginPageId = $modx->getOption('ms3_customer_login_page_id', null, 1); $modx->sendRedirect($modx->makeUrl($loginPageId)); diff --git a/core/components/minishop3/src/Middleware/TokenMiddleware.php b/core/components/minishop3/src/Middleware/TokenMiddleware.php index 427a162a..0d02193a 100644 --- a/core/components/minishop3/src/Middleware/TokenMiddleware.php +++ b/core/components/minishop3/src/Middleware/TokenMiddleware.php @@ -76,11 +76,14 @@ public function handle(array $params) // For non-public routes: session shortcut only when token still belongs to customer if (!$isPublic && !empty($_SESSION['ms3']['customer_id'])) { - $customer = $this->modx->getObject(\MiniShop3\Model\msCustomer::class, $_SESSION['ms3']['customer_id']); + $customerId = (int)$_SESSION['ms3']['customer_id']; + $customer = $this->modx->getObject(\MiniShop3\Model\msCustomer::class, $customerId); + /** @var TokenService $tokenService */ + $tokenService = $this->modx->services->get('ms3_token_service'); if ( $customer && $this->isCustomerSessionAllowed($customer) - && $this->sessionTokenMatchesCustomer((int)$customer->id) + && $tokenService->sessionTokenBelongsToCustomer($customerId) ) { return null; } @@ -202,27 +205,6 @@ private function isCustomerSessionAllowed(\MiniShop3\Model\msCustomer $customer) return true; } - /** - * Ensure PHP session / cookie API token still maps to this customer in DB. - */ - private function sessionTokenMatchesCustomer(int $customerId): bool - { - $token = (string)($_SESSION['ms3']['customer_token'] ?? ''); - if ($token === '') { - $token = CookieHelper::getTokenFromCookie(); - } - if ($token === '') { - return false; - } - - $tokenObj = $this->modx->getObject(\MiniShop3\Model\msCustomerToken::class, [ - 'token' => $token, - 'type' => \MiniShop3\Model\msCustomerToken::TYPE_API, - ]); - - return $tokenObj && (int)$tokenObj->get('customer_id') === $customerId; - } - /** * Check if route is public * diff --git a/core/components/minishop3/src/Processors/Api/Customer/Login.php b/core/components/minishop3/src/Processors/Api/Customer/Login.php index 2cbe6354..6e8545a5 100644 --- a/core/components/minishop3/src/Processors/Api/Customer/Login.php +++ b/core/components/minishop3/src/Processors/Api/Customer/Login.php @@ -9,8 +9,8 @@ /** * Login - customer login processor * - * Authenticates customer and binds existing session token to customer. - * Token reuses guest cart token when possible; otherwise mints a new API token. + * Authenticates customer and establishes a rotated API session token. + * Guest cart may transfer from the previous token; the previous token is revoked. * Protected from brute-force via RateLimiter. * * @package MiniShop3\Processors\Api\Customer @@ -24,7 +24,8 @@ public function process() { $this->modx->lexicon->load('minishop3:customer'); - $email = AuthManager::normalizeEmail($this->getProperty('email', '')); + $emailRaw = trim((string)$this->getProperty('email', '')); + $email = AuthManager::normalizeEmail($emailRaw); $password = $this->getProperty('password', ''); if ($email === '' || $password === '') { @@ -52,8 +53,9 @@ public function process() /** @var AuthManager $authManager */ $authManager = $this->modx->services->get('ms3_auth_manager'); + // Pass raw email so PasswordAuthProvider can resolve legacy mixed-case rows. $customer = $authManager->authenticate([ - 'email' => $email, + 'email' => $emailRaw, 'password' => $password, ]); @@ -62,7 +64,7 @@ public function process() // blocked / inactive keep lastAuthFailure and must not call handleFailedLoginByEmail. $failure = $authManager->getLastAuthFailure(); if ($failure === 'invalid_credentials') { - $authManager->handleFailedLoginByEmail($email); + $authManager->handleFailedLoginByEmail($emailRaw); } $this->modx->log( diff --git a/core/components/minishop3/src/Processors/Api/Customer/Register.php b/core/components/minishop3/src/Processors/Api/Customer/Register.php index 99cdb8bf..295b5323 100644 --- a/core/components/minishop3/src/Processors/Api/Customer/Register.php +++ b/core/components/minishop3/src/Processors/Api/Customer/Register.php @@ -26,7 +26,8 @@ public function process() { $this->modx->lexicon->load('minishop3:customer'); - $email = AuthManager::normalizeEmail($this->getProperty('email', '')); + $emailRaw = trim((string)$this->getProperty('email', '')); + $email = AuthManager::normalizeEmail($emailRaw); $password = $this->getProperty('password', ''); $firstName = trim($this->getProperty('first_name', '')); $lastName = trim($this->getProperty('last_name', '')); @@ -62,7 +63,7 @@ public function process() $registerService->setEmailVerification($emailVerification); $result = $registerService->register([ - 'email' => $email, + 'email' => $emailRaw, 'password' => $password, 'first_name' => $firstName, 'last_name' => $lastName, diff --git a/core/components/minishop3/src/Services/Customer/AuthManager.php b/core/components/minishop3/src/Services/Customer/AuthManager.php index 649e067c..0447e315 100644 --- a/core/components/minishop3/src/Services/Customer/AuthManager.php +++ b/core/components/minishop3/src/Services/Customer/AuthManager.php @@ -240,10 +240,6 @@ public function establishCustomerSession(msCustomer $customer): ?array "[AuthManager] transferDraftToToken failed for customer #{$customer->id}" ); } - - if ($previousTokenObj) { - $previousTokenObj->remove(); - } } elseif (!$draftManager->bindDraftToCustomer($tokenString, (int)$customer->id)) { $this->modx->log( modX::LOG_LEVEL_WARN, @@ -251,6 +247,11 @@ public function establishCustomerSession(msCustomer $customer): ?array ); } + // Always drop the previous browser token so a planted/old cookie cannot keep access. + if ($previousTokenObj && $previousToken !== $tokenString) { + $previousTokenObj->remove(); + } + if (session_status() === PHP_SESSION_ACTIVE) { session_regenerate_id(true); } diff --git a/core/components/minishop3/src/Services/Customer/CustomerPageService.php b/core/components/minishop3/src/Services/Customer/CustomerPageService.php index 8201b670..38a55518 100644 --- a/core/components/minishop3/src/Services/Customer/CustomerPageService.php +++ b/core/components/minishop3/src/Services/Customer/CustomerPageService.php @@ -88,7 +88,12 @@ public function checkAuth(): bool $this->customerId = (int)$_SESSION['ms3']['customer_id']; - if (!$this->sessionTokenMatchesCustomer($this->customerId)) { + /** @var TokenService $tokenService */ + $tokenService = $this->modx->services->has('ms3_token_service') + ? $this->modx->services->get('ms3_token_service') + : null; + + if (!$tokenService || !$tokenService->sessionTokenBelongsToCustomer($this->customerId)) { $this->clearAuthSession(); $this->modx->log( modX::LOG_LEVEL_WARN, @@ -130,35 +135,17 @@ public function checkAuth(): bool return true; } - private function sessionTokenMatchesCustomer(int $customerId): bool - { - $token = (string)($_SESSION['ms3']['customer_token'] ?? ''); - if ($token === '') { - $token = CookieHelper::getTokenFromCookie(); - } - if ($token === '') { - return false; - } - - $tokenObj = $this->modx->getObject(\MiniShop3\Model\msCustomerToken::class, [ - 'token' => $token, - 'type' => \MiniShop3\Model\msCustomerToken::TYPE_API, - ]); - - return $tokenObj && (int)$tokenObj->get('customer_id') === $customerId; - } - private function clearAuthSession(): void { - if (!isset($_SESSION['ms3'])) { - return; + if (isset($_SESSION['ms3'])) { + unset( + $_SESSION['ms3']['customer_id'], + $_SESSION['ms3']['customer_token'], + $_SESSION['ms3']['customer_token_expires'] + ); } - unset( - $_SESSION['ms3']['customer_id'], - $_SESSION['ms3']['customer_token'], - $_SESSION['ms3']['customer_token_expires'] - ); + CookieHelper::clearTokenCookie($this->modx); } /** diff --git a/core/components/minishop3/src/Services/Customer/RegisterService.php b/core/components/minishop3/src/Services/Customer/RegisterService.php index 3ec123fd..1c98c1a9 100644 --- a/core/components/minishop3/src/Services/Customer/RegisterService.php +++ b/core/components/minishop3/src/Services/Customer/RegisterService.php @@ -100,7 +100,15 @@ public function register(array $data): array ]; } - $existing = $this->modx->getObject(msCustomer::class, ['email' => $email]); + $existing = null; + if ($this->modx->services->has('ms3_auth_manager')) { + /** @var AuthManager $authManager */ + $authManager = $this->modx->services->get('ms3_auth_manager'); + $existing = $authManager->findCustomerByEmail($data['email'] ?? $email); + } + if (!$existing) { + $existing = $this->modx->getObject(msCustomer::class, ['email' => $email]); + } if ($existing) { return [ 'success' => false, diff --git a/core/components/minishop3/src/Services/TokenService.php b/core/components/minishop3/src/Services/TokenService.php index 33257133..45ed5c7c 100644 --- a/core/components/minishop3/src/Services/TokenService.php +++ b/core/components/minishop3/src/Services/TokenService.php @@ -277,8 +277,11 @@ public function getCustomerToken(): ?string "[TokenService] Customer token expired, clearing session" ); - unset($_SESSION['ms3']['customer_token']); - unset($_SESSION['ms3']['customer_token_expires']); + unset( + $_SESSION['ms3']['customer_token'], + $_SESSION['ms3']['customer_token_expires'], + $_SESSION['ms3']['customer_id'] + ); return null; } @@ -455,6 +458,41 @@ private function ensureCustomerTokenLoaded(): ?string return $this->getCustomerToken(); } + /** + * Whether session/cookie API token belongs to customer and is not expired. + * Renews TTL in DB when the row is past expires_at. + */ + public function sessionTokenBelongsToCustomer(int $customerId): bool + { + SessionHelper::ensureActive(); + + $token = (string)($_SESSION['ms3']['customer_token'] ?? ''); + if ($token === '') { + $token = CookieHelper::getTokenFromCookie(); + } + if ($token === '' || $customerId <= 0) { + return false; + } + + $tokenObj = $this->modx->getObject(msCustomerToken::class, [ + 'token' => $token, + 'type' => msCustomerToken::TYPE_API, + ]); + + if (!$tokenObj || (int)$tokenObj->get('customer_id') !== $customerId) { + return false; + } + + if ($tokenObj->isExpired() && !$this->renewTokenIfExpired($tokenObj)) { + return false; + } + + $this->applyTokenToSession($tokenObj); + CookieHelper::setTokenCookie($this->modx, (string)$tokenObj->get('token')); + + return true; + } + /** * @return bool false when the row is expired and TTL could not be persisted */ From 52a5b4ee8212b6253b452723f445884bcfc803d7 Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Thu, 16 Jul 2026 12:41:58 +0600 Subject: [PATCH 9/9] fix(customer): add web API logout route for storefront auth Expose POST /api/v1/customer/logout so AuthUI and API clients can revoke the session token after login; closes the 404 gap found during #285 QA. --- core/components/minishop3/config/routes/web.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/core/components/minishop3/config/routes/web.php b/core/components/minishop3/config/routes/web.php index a3381528..42bb9933 100644 --- a/core/components/minishop3/config/routes/web.php +++ b/core/components/minishop3/config/routes/web.php @@ -155,6 +155,18 @@ return Response::success($response->getObject(), $response->getMessage()); }); + $router->post('/logout', function($params) use ($modx) { + $response = $modx->runProcessor( + 'MiniShop3\Processors\Api\Customer\Logout', + [] + ); + + if ($response->isError()) { + return Response::error($response->getMessage(), HttpStatus::BAD_REQUEST); + } + + return Response::success($response->getObject() ?: [], $response->getMessage()); + }, [$tokenMiddleware]); $router->post('/register', function($params) use ($modx) { $input = file_get_contents('php://input'); $data = json_decode($input, true) ?: [];