From 663f3d704cffd81718eeb30c19b3fd3282f58191 Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Wed, 29 Jul 2026 08:28:13 +0600 Subject: [PATCH 1/2] feat(web): public Customer DTO and profile field allowlist Replace blacklist-based profile updates with an explicit allowlist and serialize all Web customer responses through CustomerPublicDto. --- .../Api/Web/CustomerEmailController.php | 6 +- .../Api/Web/CustomerProfileController.php | 97 ++++++------ .../src/Processors/Api/Customer/Login.php | 14 +- .../src/Processors/Api/Customer/Register.php | 14 +- .../Processors/Api/Customer/VerifyEmail.php | 11 +- .../Services/Customer/CustomerPublicDto.php | 143 +++++++++++++++++- .../tests/CustomerAuthProcessorsDtoTest.php | 47 ++++++ .../minishop3/tests/CustomerPublicDtoTest.php | 19 ++- .../tests/ProfileQuickUpdateForbiddenTest.php | 62 +++++--- 9 files changed, 316 insertions(+), 97 deletions(-) create mode 100644 core/components/minishop3/tests/CustomerAuthProcessorsDtoTest.php diff --git a/core/components/minishop3/src/Controllers/Api/Web/CustomerEmailController.php b/core/components/minishop3/src/Controllers/Api/Web/CustomerEmailController.php index 89f62f19..eb1c4473 100644 --- a/core/components/minishop3/src/Controllers/Api/Web/CustomerEmailController.php +++ b/core/components/minishop3/src/Controllers/Api/Web/CustomerEmailController.php @@ -5,6 +5,7 @@ use MiniShop3\MiniShop3; use MiniShop3\Model\msCustomer; use MiniShop3\Router\Response; +use MiniShop3\Services\Customer\CustomerPublicDto; use MiniShop3\Services\Customer\EmailVerificationService; use MODX\Revolution\modX; @@ -127,7 +128,10 @@ public function verify(array $params): array|Response return $this->success( $this->modx->lexicon('ms3_customer_email_verify_success'), - ['customer_id' => $customer->id] + [ + 'customer_id' => $customer->id, + 'customer' => CustomerPublicDto::fromCustomer($customer, $this->modx, $this->ms3), + ] ); } diff --git a/core/components/minishop3/src/Controllers/Api/Web/CustomerProfileController.php b/core/components/minishop3/src/Controllers/Api/Web/CustomerProfileController.php index d8bf918c..03cb948a 100644 --- a/core/components/minishop3/src/Controllers/Api/Web/CustomerProfileController.php +++ b/core/components/minishop3/src/Controllers/Api/Web/CustomerProfileController.php @@ -20,31 +20,6 @@ */ class CustomerProfileController { - /** - * Field names never editable via POST /api/v1/customer/add (security & system counters). - * - * @var list - */ - private const PROFILE_QUICK_UPDATE_FORBIDDEN = [ - 'id', - 'token', - 'user_id', - 'password', - 'email_verified_at', - 'is_active', - 'is_blocked', - 'failed_login_attempts', - 'blocked_until', - 'created_at', - 'updated_at', - 'last_login_at', - 'orders_count', - 'total_spent', - 'last_order_at', - 'privacy_accepted_at', - 'privacy_ip', - ]; - /** @var modX */ protected modX $modx; @@ -84,9 +59,26 @@ public function update(array $data): array } $customerId = (int)$customer->get('id'); - $validator = new Validator(); - $validation = $validator->make($data, $this->getProfileFieldRules()); + $this->ms3->loadMap(); + $editableKeys = CustomerPublicDto::editableFieldKeys($this->modx, $this->ms3); + $data = array_intersect_key($data, array_flip($editableKeys)); + + $rules = $this->getProfileFieldRules(); + foreach (array_keys($rules) as $coreKey) { + if (!array_key_exists($coreKey, $data)) { + $_SESSION['ms3']['customer_profile_errors'] = [ + $coreKey => $this->modx->lexicon('ms3_customer_err_validation'), + ]; + return $this->error( + $this->modx->lexicon('ms3_customer_err_validation'), + ['errors' => [$coreKey => $this->modx->lexicon('ms3_customer_err_validation')]] + ); + } + } + + $validator = new Validator(); + $validation = $validator->make($data, $rules); $validation->validate(); if ($validation->fails()) { @@ -99,21 +91,33 @@ public function update(array $data): array ); } - $newEmail = trim($data['email']); - - if (!$this->isEmailAvailable($customer, $newEmail)) { - $_SESSION['ms3']['customer_profile_errors'] = [ - 'email' => $this->modx->lexicon('ms3_customer_err_email_exists') - ]; - return $this->error($this->modx->lexicon('ms3_customer_err_email_exists')); + $fieldMeta = $this->modx->getFieldMeta(msCustomer::class); + if (!is_array($fieldMeta) || $fieldMeta === []) { + return $this->error($this->modx->lexicon('ms3_customer_err_save')); } - $this->resetEmailVerificationIfChanged($customer, $newEmail); + foreach ($data as $key => $rawValue) { + if ($key === 'email') { + $newEmail = trim((string) $rawValue); + if (!$this->isEmailAvailable($customer, $newEmail)) { + $_SESSION['ms3']['customer_profile_errors'] = [ + 'email' => $this->modx->lexicon('ms3_customer_err_email_exists'), + ]; + + return $this->error($this->modx->lexicon('ms3_customer_err_email_exists')); + } + $this->resetEmailVerificationIfChanged($customer, $newEmail); + $customer->set('email', $newEmail); + continue; + } - $customer->set('first_name', trim($data['first_name'])); - $customer->set('last_name', trim($data['last_name'])); - $customer->set('email', $newEmail); - $customer->set('phone', trim($data['phone'])); + if (isset($rules[$key])) { + $customer->set($key, trim((string) $rawValue)); + continue; + } + + $customer->set($key, $this->normalizeQuickProfileValue($rawValue, $fieldMeta[$key])); + } if (!$customer->save()) { return $this->error($this->modx->lexicon('ms3_customer_err_save')); @@ -128,7 +132,7 @@ public function update(array $data): array return $this->success( $this->modx->lexicon('ms3_customer_profile_updated'), - ['customer' => CustomerPublicDto::fromCustomer($customer)] + ['customer' => CustomerPublicDto::fromCustomer($customer, $this->modx, $this->ms3)] ); } @@ -157,16 +161,13 @@ public function updateField(array $data): array } $this->ms3->loadMap(); - $fieldMeta = $this->modx->getFieldMeta(msCustomer::class); - if (!is_array($fieldMeta) || $fieldMeta === []) { + $editableKeys = CustomerPublicDto::editableFieldKeys($this->modx, $this->ms3); + if (!in_array($key, $editableKeys, true)) { return $this->error($this->modx->lexicon('ms3_customer_err_field_not_allowed')); } - if (in_array($key, self::PROFILE_QUICK_UPDATE_FORBIDDEN, true)) { - return $this->error($this->modx->lexicon('ms3_customer_err_field_not_allowed')); - } - - if (!isset($fieldMeta[$key])) { + $fieldMeta = $this->modx->getFieldMeta(msCustomer::class); + if (!is_array($fieldMeta) || !isset($fieldMeta[$key])) { return $this->error($this->modx->lexicon('ms3_customer_err_field_not_allowed')); } @@ -206,7 +207,7 @@ public function updateField(array $data): array $this->modx->lexicon('ms3_customer_profile_updated'), [ $key => $customer->get($key), - 'customer' => CustomerPublicDto::fromCustomer($customer), + 'customer' => CustomerPublicDto::fromCustomer($customer, $this->modx, $this->ms3), ] ); } diff --git a/core/components/minishop3/src/Processors/Api/Customer/Login.php b/core/components/minishop3/src/Processors/Api/Customer/Login.php index 6e8545a5..78206541 100644 --- a/core/components/minishop3/src/Processors/Api/Customer/Login.php +++ b/core/components/minishop3/src/Processors/Api/Customer/Login.php @@ -2,7 +2,9 @@ namespace MiniShop3\Processors\Api\Customer; +use MiniShop3\MiniShop3; use MiniShop3\Services\Customer\AuthManager; +use MiniShop3\Services\Customer\CustomerPublicDto; use MiniShop3\Services\Customer\RateLimiter; use MODX\Revolution\Processors\Processor; @@ -98,15 +100,11 @@ public function process() $redirectUrl = $this->modx->makeUrl($redirectPageId, '', '', 'full'); } + /** @var MiniShop3 $ms3 */ + $ms3 = $this->modx->services->get('ms3'); + return $this->success('', [ - 'customer' => [ - 'id' => $customer->id, - 'email' => $customer->get('email'), - 'first_name' => $customer->get('first_name'), - 'last_name' => $customer->get('last_name'), - 'phone' => $customer->get('phone'), - 'email_verified' => !empty($customer->get('email_verified_at')), - ], + 'customer' => CustomerPublicDto::fromCustomer($customer, $this->modx, $ms3), '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 295b5323..87b895af 100644 --- a/core/components/minishop3/src/Processors/Api/Customer/Register.php +++ b/core/components/minishop3/src/Processors/Api/Customer/Register.php @@ -2,7 +2,9 @@ namespace MiniShop3\Processors\Api\Customer; +use MiniShop3\MiniShop3; use MiniShop3\Services\Customer\AuthManager; +use MiniShop3\Services\Customer\CustomerPublicDto; use MiniShop3\Services\Customer\EmailVerificationService; use MiniShop3\Services\Customer\RateLimiter; use MiniShop3\Services\Customer\RegisterService; @@ -110,15 +112,11 @@ public function process() } } + /** @var MiniShop3 $ms3 */ + $ms3 = $this->modx->services->get('ms3'); + return $this->success($this->modx->lexicon('ms3_customer_register_success'), [ - 'customer' => [ - 'id' => $customer->id, - 'email' => $customer->get('email'), - 'first_name' => $customer->get('first_name'), - 'last_name' => $customer->get('last_name'), - 'phone' => $customer->get('phone'), - 'email_verified' => !empty($customer->get('email_verified_at')), - ], + 'customer' => CustomerPublicDto::fromCustomer($customer, $this->modx, $ms3), 'token' => $tokenString, 'expires_at' => $expiresAt, 'email_verification_required' => $requireEmailVerification, diff --git a/core/components/minishop3/src/Processors/Api/Customer/VerifyEmail.php b/core/components/minishop3/src/Processors/Api/Customer/VerifyEmail.php index 078b7c9a..56c160dd 100644 --- a/core/components/minishop3/src/Processors/Api/Customer/VerifyEmail.php +++ b/core/components/minishop3/src/Processors/Api/Customer/VerifyEmail.php @@ -2,6 +2,8 @@ namespace MiniShop3\Processors\Api\Customer; +use MiniShop3\MiniShop3; +use MiniShop3\Services\Customer\CustomerPublicDto; use MiniShop3\Services\Customer\EmailVerificationService; use MODX\Revolution\Processors\Processor; @@ -39,12 +41,11 @@ public function process() return $this->failure($this->modx->lexicon('ms3_customer_err_email_verification_invalid')); } + /** @var MiniShop3 $ms3 */ + $ms3 = $this->modx->services->get('ms3'); + return $this->success($this->modx->lexicon('ms3_customer_email_verify_success'), [ - 'customer' => [ - 'id' => $customer->id, - 'email' => $customer->get('email'), - 'email_verified' => true, - ], + 'customer' => CustomerPublicDto::fromCustomer($customer, $this->modx, $ms3), ]); } } diff --git a/core/components/minishop3/src/Services/Customer/CustomerPublicDto.php b/core/components/minishop3/src/Services/Customer/CustomerPublicDto.php index 3b43041c..81cc96a9 100644 --- a/core/components/minishop3/src/Services/Customer/CustomerPublicDto.php +++ b/core/components/minishop3/src/Services/Customer/CustomerPublicDto.php @@ -4,15 +4,18 @@ namespace MiniShop3\Services\Customer; +use MiniShop3\MiniShop3; use MiniShop3\Model\msCustomer; +use MiniShop3\Model\msExtraField; +use MODX\Revolution\modX; /** - * Public customer payload for Web API responses (no secrets / lock internals). + * Public customer payload and profile field allowlist for Web API (#424). */ final class CustomerPublicDto { /** - * Fields allowed in public customer JSON (fail-closed for new columns). + * Core fields exposed in public customer JSON (fail-closed for new columns). * * @var list */ @@ -33,20 +36,148 @@ final class CustomerPublicDto 'privacy_accepted_at', ]; + /** + * Base profile fields editable via Web API (Rakit rules apply in controller). + * + * @var list + */ + public const CORE_PROFILE_EDITABLE_FIELDS = [ + 'first_name', + 'last_name', + 'email', + 'phone', + ]; + + /** + * Never writable from Web customer profile/add endpoints. + * + * @var list + */ + public const SYSTEM_NON_EDITABLE_FIELDS = [ + 'id', + 'token', + 'user_id', + 'password', + 'email_verified_at', + 'is_active', + 'is_blocked', + 'failed_login_attempts', + 'blocked_until', + 'created_at', + 'updated_at', + 'last_login_at', + 'orders_count', + 'total_spent', + 'last_order_at', + 'privacy_accepted_at', + 'privacy_ip', + ]; + /** * @param array $customerFields * @return array */ - public static function fromArray(array $customerFields): array + public static function fromArray(array $customerFields, array $extraPublicKeys = []): array { - return array_intersect_key($customerFields, array_flip(self::PUBLIC_FIELDS)); + $allowed = self::publicFieldKeys($extraPublicKeys); + + return array_intersect_key($customerFields, array_flip($allowed)); } /** * @return array */ - public static function fromCustomer(msCustomer $customer): array + public static function fromCustomer(msCustomer $customer, ?modX $modx = null, ?MiniShop3 $ms3 = null): array + { + $extraKeys = ($modx !== null && $ms3 !== null) + ? self::activeCustomerExtraFieldKeys($modx) + : []; + + return self::fromArray($customer->toArray(), $extraKeys); + } + + /** + * @return list + */ + public static function publicFieldKeys(array $extraFieldKeys = []): array + { + $keys = self::PUBLIC_FIELDS; + + foreach ($extraFieldKeys as $key) { + $key = (string) $key; + if ($key === '' || in_array($key, self::SYSTEM_NON_EDITABLE_FIELDS, true)) { + continue; + } + if (!in_array($key, $keys, true)) { + $keys[] = $key; + } + } + + return $keys; + } + + /** + * @param list $extraFieldKeys Active msExtraField keys for msCustomer + * + * @return list + */ + public static function mergeEditableFieldKeys(array $extraFieldKeys): array + { + $allowed = self::CORE_PROFILE_EDITABLE_FIELDS; + + foreach ($extraFieldKeys as $key) { + $key = (string) $key; + if ($key === '' || in_array($key, self::SYSTEM_NON_EDITABLE_FIELDS, true)) { + continue; + } + if (!in_array($key, $allowed, true)) { + $allowed[] = $key; + } + } + + return $allowed; + } + + /** + * @return list + */ + public static function editableFieldKeys(modX $modx, MiniShop3 $ms3): array + { + $ms3->loadMap(); + $fieldMeta = $modx->getFieldMeta(msCustomer::class); + if (!is_array($fieldMeta) || $fieldMeta === []) { + return self::CORE_PROFILE_EDITABLE_FIELDS; + } + + $extraKeys = self::activeCustomerExtraFieldKeys($modx); + $merged = self::mergeEditableFieldKeys($extraKeys); + + return array_values(array_filter( + $merged, + static fn(string $key): bool => isset($fieldMeta[$key]) + )); + } + + public static function isEditableField(string $key, modX $modx, MiniShop3 $ms3): bool { - return self::fromArray($customer->toArray()); + return in_array($key, self::editableFieldKeys($modx, $ms3), true); + } + + /** + * @return list + */ + private static function activeCustomerExtraFieldKeys(modX $modx): array + { + $keys = []; + $iterator = $modx->getIterator(msExtraField::class, [ + 'class' => msCustomer::class, + 'active' => 1, + ]); + + foreach ($iterator as $field) { + $keys[] = (string) $field->get('key'); + } + + return $keys; } } diff --git a/core/components/minishop3/tests/CustomerAuthProcessorsDtoTest.php b/core/components/minishop3/tests/CustomerAuthProcessorsDtoTest.php new file mode 100644 index 00000000..02f6b887 --- /dev/null +++ b/core/components/minishop3/tests/CustomerAuthProcessorsDtoTest.php @@ -0,0 +1,47 @@ +modx, $ms3)')) { + $fail("{$file} must return customer via CustomerPublicDto::fromCustomer"); + } + + if (preg_match("/'email_verified'\\s*=>/", $src)) { + $fail("{$file} must not expose legacy email_verified boolean"); + } + + if (preg_match("/'customer'\\s*=>\\s*\\[/", $src)) { + $fail("{$file} must not build inline customer array"); + } +} + +fwrite(STDOUT, "OK CustomerAuthProcessorsDtoTest\n"); +exit(0); diff --git a/core/components/minishop3/tests/CustomerPublicDtoTest.php b/core/components/minishop3/tests/CustomerPublicDtoTest.php index c8bb1ccf..7c84ab1a 100644 --- a/core/components/minishop3/tests/CustomerPublicDtoTest.php +++ b/core/components/minishop3/tests/CustomerPublicDtoTest.php @@ -75,6 +75,21 @@ } } +$coreEditable = CustomerPublicDto::CORE_PROFILE_EDITABLE_FIELDS; +foreach (['first_name', 'last_name', 'email', 'phone'] as $key) { + if (!in_array($key, $coreEditable, true)) { + $fail("CORE_PROFILE_EDITABLE_FIELDS missing {$key}"); + } +} + +$withExtra = CustomerPublicDto::fromArray( + array_merge($input, ['loyalty_tier' => 'gold']), + ['loyalty_tier'] +); +if (($withExtra['loyalty_tier'] ?? null) !== 'gold') { + $fail('registered extra field must be included in public payload'); +} + $controllerSrc = file_get_contents(__DIR__ . '/../src/Controllers/Api/Web/CustomerProfileController.php'); if ($controllerSrc === false) { $fail('unable to read CustomerProfileController.php'); @@ -82,8 +97,8 @@ if (str_contains($controllerSrc, '$customer->toArray()')) { $fail('CustomerProfileController still serializes full customer via toArray()'); } -if (!str_contains($controllerSrc, 'CustomerPublicDto::fromCustomer')) { - $fail('CustomerProfileController missing CustomerPublicDto::fromCustomer'); +if (!str_contains($controllerSrc, 'CustomerPublicDto::fromCustomer($customer, $this->modx, $this->ms3)')) { + $fail('CustomerProfileController missing CustomerPublicDto::fromCustomer with modx/ms3'); } fwrite(STDOUT, "OK CustomerPublicDtoTest\n"); diff --git a/core/components/minishop3/tests/ProfileQuickUpdateForbiddenTest.php b/core/components/minishop3/tests/ProfileQuickUpdateForbiddenTest.php index a63b6b14..28cc1b73 100644 --- a/core/components/minishop3/tests/ProfileQuickUpdateForbiddenTest.php +++ b/core/components/minishop3/tests/ProfileQuickUpdateForbiddenTest.php @@ -1,47 +1,71 @@ 1, 'company' => 'ACME', 'password' => 'secret'], + ['company'] +); +if (($publicWithExtra['company'] ?? null) !== 'ACME') { + $fail('extra public field must appear when registered in msExtraField keys'); +} +if (array_key_exists('password', $publicWithExtra)) { + $fail('password must never appear in public DTO'); } fwrite(STDOUT, "OK ProfileQuickUpdateForbiddenTest\n"); From 8e62a54ba8cf4403cf5c56f95e7684c5409244b3 Mon Sep 17 00:00:00 2001 From: Ivan Bochkarev Date: Wed, 29 Jul 2026 23:13:32 +0600 Subject: [PATCH 2/2] fix(web): customer DTO registry, required deps, partial profile update Address #424 review findings: - Extract CustomerExtraFieldRegistry from CustomerPublicDto for the msExtraField DB lookup; CustomerPublicDto now delegates to it - fromCustomer() requires modX + ms3 (remove silent null mode that returned no extra keys) - update() supports partial updates: validate only core rules for fields present in $data instead of rejecting when any core field is missing - Replace grep-only CustomerAuthProcessorsDtoTest with a behavior test that exercises fromCustomer() serialization (extra-field allowlist + secret/system fields never leak); update ProfileQuickUpdateForbiddenTest to assert partial-update contract --- .../Api/Web/CustomerProfileController.php | 19 +-- .../Customer/CustomerExtraFieldRegistry.php | 38 +++++ .../Services/Customer/CustomerPublicDto.php | 29 +--- .../tests/CustomerAuthProcessorsDtoTest.php | 137 +++++++++++++++++- .../tests/ProfileQuickUpdateForbiddenTest.php | 7 +- .../minishop3/tests/stubs/StubMsCustomer.php | 37 +++++ 6 files changed, 226 insertions(+), 41 deletions(-) create mode 100644 core/components/minishop3/src/Services/Customer/CustomerExtraFieldRegistry.php create mode 100644 core/components/minishop3/tests/stubs/StubMsCustomer.php diff --git a/core/components/minishop3/src/Controllers/Api/Web/CustomerProfileController.php b/core/components/minishop3/src/Controllers/Api/Web/CustomerProfileController.php index 03cb948a..eeca791a 100644 --- a/core/components/minishop3/src/Controllers/Api/Web/CustomerProfileController.php +++ b/core/components/minishop3/src/Controllers/Api/Web/CustomerProfileController.php @@ -63,20 +63,15 @@ public function update(array $data): array $editableKeys = CustomerPublicDto::editableFieldKeys($this->modx, $this->ms3); $data = array_intersect_key($data, array_flip($editableKeys)); - $rules = $this->getProfileFieldRules(); - foreach (array_keys($rules) as $coreKey) { - if (!array_key_exists($coreKey, $data)) { - $_SESSION['ms3']['customer_profile_errors'] = [ - $coreKey => $this->modx->lexicon('ms3_customer_err_validation'), - ]; - - return $this->error( - $this->modx->lexicon('ms3_customer_err_validation'), - ['errors' => [$coreKey => $this->modx->lexicon('ms3_customer_err_validation')]] - ); - } + if ($data === []) { + return $this->error($this->modx->lexicon('ms3_customer_err_validation')); } + // Partial update: validate only the core profile rules for fields + // actually present in $data. Missing core fields are left untouched + // rather than rejected (#424 review). + $rules = array_intersect_key($this->getProfileFieldRules(), $data); + $validator = new Validator(); $validation = $validator->make($data, $rules); $validation->validate(); diff --git a/core/components/minishop3/src/Services/Customer/CustomerExtraFieldRegistry.php b/core/components/minishop3/src/Services/Customer/CustomerExtraFieldRegistry.php new file mode 100644 index 00000000..dcd98378 --- /dev/null +++ b/core/components/minishop3/src/Services/Customer/CustomerExtraFieldRegistry.php @@ -0,0 +1,38 @@ + + */ + public static function activeKeys(modX $modx): array + { + $keys = []; + $iterator = $modx->getIterator(msExtraField::class, [ + 'class' => msCustomer::class, + 'active' => 1, + ]); + + foreach ($iterator as $field) { + $keys[] = (string) $field->get('key'); + } + + return $keys; + } +} diff --git a/core/components/minishop3/src/Services/Customer/CustomerPublicDto.php b/core/components/minishop3/src/Services/Customer/CustomerPublicDto.php index 81cc96a9..3712b0fc 100644 --- a/core/components/minishop3/src/Services/Customer/CustomerPublicDto.php +++ b/core/components/minishop3/src/Services/Customer/CustomerPublicDto.php @@ -6,7 +6,6 @@ use MiniShop3\MiniShop3; use MiniShop3\Model\msCustomer; -use MiniShop3\Model\msExtraField; use MODX\Revolution\modX; /** @@ -87,13 +86,9 @@ public static function fromArray(array $customerFields, array $extraPublicKeys = /** * @return array */ - public static function fromCustomer(msCustomer $customer, ?modX $modx = null, ?MiniShop3 $ms3 = null): array + public static function fromCustomer(msCustomer $customer, modX $modx, MiniShop3 $ms3): array { - $extraKeys = ($modx !== null && $ms3 !== null) - ? self::activeCustomerExtraFieldKeys($modx) - : []; - - return self::fromArray($customer->toArray(), $extraKeys); + return self::fromArray($customer->toArray(), CustomerExtraFieldRegistry::activeKeys($modx)); } /** @@ -149,7 +144,7 @@ public static function editableFieldKeys(modX $modx, MiniShop3 $ms3): array return self::CORE_PROFILE_EDITABLE_FIELDS; } - $extraKeys = self::activeCustomerExtraFieldKeys($modx); + $extraKeys = CustomerExtraFieldRegistry::activeKeys($modx); $merged = self::mergeEditableFieldKeys($extraKeys); return array_values(array_filter( @@ -162,22 +157,4 @@ public static function isEditableField(string $key, modX $modx, MiniShop3 $ms3): { return in_array($key, self::editableFieldKeys($modx, $ms3), true); } - - /** - * @return list - */ - private static function activeCustomerExtraFieldKeys(modX $modx): array - { - $keys = []; - $iterator = $modx->getIterator(msExtraField::class, [ - 'class' => msCustomer::class, - 'active' => 1, - ]); - - foreach ($iterator as $field) { - $keys[] = (string) $field->get('key'); - } - - return $keys; - } } diff --git a/core/components/minishop3/tests/CustomerAuthProcessorsDtoTest.php b/core/components/minishop3/tests/CustomerAuthProcessorsDtoTest.php index 02f6b887..43e2583b 100644 --- a/core/components/minishop3/tests/CustomerAuthProcessorsDtoTest.php +++ b/core/components/minishop3/tests/CustomerAuthProcessorsDtoTest.php @@ -1,13 +1,26 @@ */ + private array $extraKeys; + + /** + * @param list $extraKeys + */ + public function __construct(array $extraKeys) + { + parent::__construct(); + $this->extraKeys = $extraKeys; + } + + public function getIterator($className, $criteria = null) + { + if ($className !== msExtraField::class) { + return new ArrayIterator([]); + } + + $fields = array_map( + static fn(string $key): object => new class ($key) { + public function __construct(private string $key) + { + } + + public function get(string $k): mixed + { + return $k === 'key' ? $this->key : null; + } + }, + $this->extraKeys + ); + + return new ArrayIterator($fields); + } +}; + +$ms3 = new class extends MiniShop3 { + public function __construct() + { + // Bypass real constructor: fromCustomer() does not use ms3. + } +}; + +$customer = new StubMsCustomer([ + 'id' => 42, + 'user_id' => 7, + 'first_name' => 'Ada', + 'last_name' => 'Lovelace', + 'email' => 'ada@example.com', + 'phone' => '+10000000000', + 'password' => '$2y$10$notarealhash', + 'token' => 'session-or-api-secret', + 'email_verified_at' => '2026-01-02 03:04:05', + 'is_active' => true, + 'is_blocked' => true, + 'privacy_ip' => '203.0.113.10', + 'failed_login_attempts' => 3, + 'blocked_until' => '2026-01-01 00:00:00', + 'created_at' => '2025-01-01 00:00:00', + 'updated_at' => '2025-06-01 00:00:00', + 'last_login_at' => '2025-12-01 00:00:00', + 'orders_count' => 2, + 'total_spent' => 99.5, + 'last_order_at' => '2025-11-01 00:00:00', + 'privacy_accepted_at' => '2025-01-01 00:00:00', + 'loyalty_tier' => 'gold', + 'company' => 'ACME', + 'future_secret_column' => 'must-not-leak', +]); + +$public = CustomerPublicDto::fromCustomer($customer, $modx, $ms3); + +$expected = [ + 'id' => 42, + 'first_name' => 'Ada', + 'last_name' => 'Lovelace', + 'email' => 'ada@example.com', + 'phone' => '+10000000000', + 'email_verified_at' => '2026-01-02 03:04:05', + 'is_active' => true, + 'created_at' => '2025-01-01 00:00:00', + 'updated_at' => '2025-06-01 00:00:00', + 'last_login_at' => '2025-12-01 00:00:00', + 'orders_count' => 2, + 'total_spent' => 99.5, + 'last_order_at' => '2025-11-01 00:00:00', + 'privacy_accepted_at' => '2025-01-01 00:00:00', + 'loyalty_tier' => 'gold', + 'company' => 'ACME', +]; + +if ($public !== $expected) { + $fail(sprintf( + "fromCustomer behavior mismatch:\nexpected: %s\nactual: %s", + json_encode($expected, JSON_UNESCAPED_SLASHES), + json_encode($public, JSON_UNESCAPED_SLASHES) + )); +} + +foreach (['password', 'token', 'privacy_ip', 'failed_login_attempts', 'blocked_until', 'is_blocked', 'user_id', 'future_secret_column'] as $hidden) { + if (array_key_exists($hidden, $public)) { + $fail("fromCustomer leaked secret/system field: {$hidden}"); + } +} + +// Empty extra-field registry → only core public fields, no extras leak. +$emptyModx = new class extends modX { + public function getIterator($className, $criteria = null) + { + return new ArrayIterator([]); + } +}; +$barePublic = CustomerPublicDto::fromCustomer($customer, $emptyModx, $ms3); +if (array_key_exists('loyalty_tier', $barePublic) || array_key_exists('company', $barePublic)) { + $fail('fromCustomer must not expose extra fields when registry has none registered'); +} + fwrite(STDOUT, "OK CustomerAuthProcessorsDtoTest\n"); exit(0); diff --git a/core/components/minishop3/tests/ProfileQuickUpdateForbiddenTest.php b/core/components/minishop3/tests/ProfileQuickUpdateForbiddenTest.php index 28cc1b73..0ec29d37 100644 --- a/core/components/minishop3/tests/ProfileQuickUpdateForbiddenTest.php +++ b/core/components/minishop3/tests/ProfileQuickUpdateForbiddenTest.php @@ -30,8 +30,11 @@ $fail('updateField must enforce CustomerPublicDto allowlist via editableFieldKeys'); } -if (!preg_match('/foreach \\(array_keys\\(\\$rules\\)/', $controllerSrc)) { - $fail('update must require all core profile fields before validation'); +if (preg_match('/foreach \\(array_keys\\(\\$rules\\)/', $controllerSrc)) { + $fail('update must not require all core profile fields — partial updates only (#424)'); +} +if (!preg_match('/array_intersect_key\\(\\$this->getProfileFieldRules\\(\\),\\s*\\$data\\)/', $controllerSrc)) { + $fail('update must restrict validation to core rules for fields present in $data'); } foreach (['privacy_accepted_at', 'privacy_ip', 'password', 'token', 'email_verified_at'] as $key) { diff --git a/core/components/minishop3/tests/stubs/StubMsCustomer.php b/core/components/minishop3/tests/stubs/StubMsCustomer.php new file mode 100644 index 00000000..00013bd4 --- /dev/null +++ b/core/components/minishop3/tests/stubs/StubMsCustomer.php @@ -0,0 +1,37 @@ + */ + private array $fields; + + /** + * @param array $fields + */ + public function __construct(array $fields = []) + { + $this->fields = $fields; + } + + public function get($key) + { + return $this->fields[$key] ?? null; + } + + /** + * @return array + */ + public function toArray(): array + { + return $this->fields; + } +}