Validate missing fields in Store API (#45840)

* fix field validation

* add changelog

* fix tests

* add more checks

* fix arrays
This commit is contained in:
Seghir Nadir 2024-03-27 16:50:48 +01:00 committed by GitHub
parent 7d6d2c94dd
commit 3513af5ec8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 101 additions and 86 deletions

View File

@ -0,0 +1,4 @@
Significance: patch
Type: fix
Always validate missing additional fields

View File

@ -129,7 +129,7 @@
</rule>
<!-- Temporary -->
<rule ref="Generic.Arrays.DisallowShortArraySyntax.Found">
<rule ref="Universal.Arrays.DisallowShortArraySyntax.Found">
<exclude-pattern>src/Blocks/</exclude-pattern>
<exclude-pattern>src/StoreApi/</exclude-pattern>
</rule>

View File

@ -24,7 +24,7 @@ class CheckoutFields {
*
* @var array
*/
private $additional_fields = array();
private $additional_fields = [];
/**
* Fields locations.
@ -75,8 +75,8 @@ class CheckoutFields {
*/
public function __construct( AssetDataRegistry $asset_data_registry ) {
$this->asset_data_registry = $asset_data_registry;
$this->core_fields = array(
'email' => array(
$this->core_fields = [
'email' => [
'label' => __( 'Email address', 'woocommerce' ),
'optionalLabel' => __(
'Email address (optional)',
@ -87,8 +87,8 @@ class CheckoutFields {
'autocomplete' => 'email',
'autocapitalize' => 'none',
'index' => 0,
),
'first_name' => array(
],
'first_name' => [
'label' => __( 'First name', 'woocommerce' ),
'optionalLabel' => __(
'First name (optional)',
@ -99,8 +99,8 @@ class CheckoutFields {
'autocomplete' => 'given-name',
'autocapitalize' => 'sentences',
'index' => 10,
),
'last_name' => array(
],
'last_name' => [
'label' => __( 'Last name', 'woocommerce' ),
'optionalLabel' => __(
'Last name (optional)',
@ -111,8 +111,8 @@ class CheckoutFields {
'autocomplete' => 'family-name',
'autocapitalize' => 'sentences',
'index' => 20,
),
'company' => array(
],
'company' => [
'label' => __( 'Company', 'woocommerce' ),
'optionalLabel' => __(
'Company (optional)',
@ -123,8 +123,8 @@ class CheckoutFields {
'autocomplete' => 'organization',
'autocapitalize' => 'sentences',
'index' => 30,
),
'address_1' => array(
],
'address_1' => [
'label' => __( 'Address', 'woocommerce' ),
'optionalLabel' => __(
'Address (optional)',
@ -135,8 +135,8 @@ class CheckoutFields {
'autocomplete' => 'address-line1',
'autocapitalize' => 'sentences',
'index' => 40,
),
'address_2' => array(
],
'address_2' => [
'label' => __( 'Apartment, suite, etc.', 'woocommerce' ),
'optionalLabel' => __(
'Apartment, suite, etc. (optional)',
@ -147,8 +147,8 @@ class CheckoutFields {
'autocomplete' => 'address-line2',
'autocapitalize' => 'sentences',
'index' => 50,
),
'country' => array(
],
'country' => [
'label' => __( 'Country/Region', 'woocommerce' ),
'optionalLabel' => __(
'Country/Region (optional)',
@ -158,8 +158,8 @@ class CheckoutFields {
'hidden' => false,
'autocomplete' => 'country',
'index' => 50,
),
'city' => array(
],
'city' => [
'label' => __( 'City', 'woocommerce' ),
'optionalLabel' => __(
'City (optional)',
@ -170,8 +170,8 @@ class CheckoutFields {
'autocomplete' => 'address-level2',
'autocapitalize' => 'sentences',
'index' => 70,
),
'state' => array(
],
'state' => [
'label' => __( 'State/County', 'woocommerce' ),
'optionalLabel' => __(
'State/County (optional)',
@ -182,8 +182,8 @@ class CheckoutFields {
'autocomplete' => 'address-level1',
'autocapitalize' => 'sentences',
'index' => 80,
),
'postcode' => array(
],
'postcode' => [
'label' => __( 'Postal code', 'woocommerce' ),
'optionalLabel' => __(
'Postal code (optional)',
@ -194,8 +194,8 @@ class CheckoutFields {
'autocomplete' => 'postal-code',
'autocapitalize' => 'characters',
'index' => 90,
),
'phone' => array(
],
'phone' => [
'label' => __( 'Phone', 'woocommerce' ),
'optionalLabel' => __(
'Phone (optional)',
@ -207,15 +207,15 @@ class CheckoutFields {
'autocomplete' => 'tel',
'autocapitalize' => 'characters',
'index' => 100,
),
);
],
];
$this->fields_locations = array(
$this->fields_locations = [
// omit email from shipping and billing fields.
'address' => array_merge( \array_diff_key( array_keys( $this->core_fields ), array( 'email' ) ) ),
'contact' => array( 'email' ),
'additional' => array(),
);
'additional' => [],
];
add_filter( 'woocommerce_get_country_locale_default', array( $this, 'update_default_locale_with_fields' ) );
}
@ -296,7 +296,7 @@ class CheckoutFields {
// The above validate_options function ensures these options are valid. Type might not be supplied but then it defaults to text.
$field_data = wp_parse_args(
$options,
array(
[
'id' => '',
'label' => '',
'optionalLabel' => sprintf(
@ -308,11 +308,11 @@ class CheckoutFields {
'type' => 'text',
'hidden' => false,
'required' => false,
'attributes' => array(),
'attributes' => [],
'show_in_order_confirmation' => true,
'sanitize_callback' => array( $this, 'default_sanitize_callback' ),
'validate_callback' => array( $this, 'default_validate_callback' ),
)
]
);
$field_data['attributes'] = $this->register_field_attributes( $field_data['id'], $field_data['attributes'] );
@ -452,8 +452,8 @@ class CheckoutFields {
_doing_it_wrong( '__experimental_woocommerce_blocks_register_checkout_field', esc_html( $message ), '8.6.0' );
return false;
}
$cleaned_options = array();
$added_values = array();
$cleaned_options = [];
$added_values = [];
// Check all entries in $options['options'] has a key and value member.
foreach ( $options['options'] as $option ) {
@ -474,10 +474,10 @@ class CheckoutFields {
$added_values[] = $sanitized_value;
$cleaned_options[] = array(
$cleaned_options[] = [
'value' => $sanitized_value,
'label' => $sanitized_label,
);
];
}
$field_data['options'] = $cleaned_options;
@ -485,12 +485,12 @@ class CheckoutFields {
// If the field is not required, inject an empty option at the start.
if ( isset( $field_data['required'] ) && false === $field_data['required'] && ! in_array( '', $added_values, true ) ) {
$field_data['options'] = array_merge(
array(
array(
[
[
'value' => '',
'label' => '',
),
),
],
],
$field_data['options']
);
}
@ -542,18 +542,18 @@ class CheckoutFields {
}
// These are formatted in camelCase because React components expect them that way.
$allowed_attributes = array(
$allowed_attributes = [
'maxLength',
'readOnly',
'pattern',
'autocomplete',
'autocapitalize',
'title',
);
];
$valid_attributes = array_filter(
$attributes,
function( $_, $key ) use ( $allowed_attributes ) {
function ( $_, $key ) use ( $allowed_attributes ) {
return in_array( $key, $allowed_attributes, true ) || strpos( $key, 'aria-' ) === 0 || strpos( $key, 'data-' ) === 0;
},
ARRAY_FILTER_USE_BOTH
@ -568,7 +568,7 @@ class CheckoutFields {
// Escape attributes to remove any malicious code and return them.
return array_map(
function( $value ) {
function ( $value ) {
return esc_attr( $value );
},
$valid_attributes
@ -756,7 +756,7 @@ class CheckoutFields {
return array_filter(
$this->get_additional_fields(),
function( $key ) use ( $order_fields_keys ) {
function ( $key ) use ( $order_fields_keys ) {
return in_array( $key, $order_fields_keys, true );
},
ARRAY_FILTER_USE_KEY
@ -923,7 +923,7 @@ class CheckoutFields {
$meta_data = $object->get_meta( $meta_key, true );
if ( ! is_array( $meta_data ) ) {
$meta_data = array();
$meta_data = [];
}
$meta_data[ $key ] = $value;
@ -1001,11 +1001,12 @@ class CheckoutFields {
* @return array An array of fields.
*/
public function get_all_fields_from_customer( $customer, $all = false ) {
$meta_data = array(
'billing' => array(),
'shipping' => array(),
'additional' => array(),
);
$meta_data = [
'billing' => [],
'shipping' => [],
'additional' => [],
];
if ( $customer instanceof WC_Customer ) {
$meta_data['billing'] = $customer->get_meta( self::BILLING_FIELDS_KEY, true );
$meta_data['shipping'] = $customer->get_meta( self::SHIPPING_FIELDS_KEY, true );
@ -1023,11 +1024,12 @@ class CheckoutFields {
* @return array An array of fields.
*/
public function get_all_fields_from_order( $order, $all = false ) {
$meta_data = array(
'billing' => array(),
'shipping' => array(),
'additional' => array(),
);
$meta_data = [
'billing' => [],
'shipping' => [],
'additional' => [],
];
if ( $order instanceof WC_Order ) {
$meta_data['billing'] = $order->get_meta( self::BILLING_FIELDS_KEY, true );
$meta_data['shipping'] = $order->get_meta( self::SHIPPING_FIELDS_KEY, true );
@ -1045,11 +1047,11 @@ class CheckoutFields {
* @return array An array of fields.
*/
private function format_meta_data( $meta, $all = false ) {
$billing_fields = $meta['billing'] ?? array();
$shipping_fields = $meta['shipping'] ?? array();
$additional_fields = $meta['additional'] ?? array();
$billing_fields = $meta['billing'] ?? [];
$shipping_fields = $meta['shipping'] ?? [];
$additional_fields = $meta['additional'] ?? [];
$fields = array();
$fields = [];
if ( is_array( $billing_fields ) ) {
foreach ( $billing_fields as $key => $value ) {
@ -1095,7 +1097,7 @@ class CheckoutFields {
);
return array_filter(
$fields,
function( $key ) use ( $customer_fields_keys ) {
function ( $key ) use ( $customer_fields_keys ) {
if ( 0 === strpos( $key, '/billing/' ) ) {
$key = str_replace( '/billing/', '', $key );
} elseif ( 0 === strpos( $key, '/shipping/' ) ) {
@ -1117,7 +1119,7 @@ class CheckoutFields {
public function filter_fields_for_location( $fields, $location ) {
return array_filter(
$fields,
function( $key ) use ( $location ) {
function ( $key ) use ( $location ) {
if ( 0 === strpos( $key, '/billing/' ) ) {
$key = str_replace( '/billing/', '', $key );
} elseif ( 0 === strpos( $key, '/shipping/' ) ) {
@ -1138,7 +1140,7 @@ class CheckoutFields {
public function filter_fields_for_order_confirmation( $fields ) {
return array_filter(
$fields,
function( $field ) {
function ( $field ) {
return ! empty( $field['show_in_order_confirmation'] );
}
);
@ -1155,7 +1157,7 @@ class CheckoutFields {
*/
public function get_order_additional_fields_with_values( $order, $location, $group = '', $context = 'edit' ) {
$fields = $this->get_fields_for_location( $location );
$fields_with_values = array();
$fields_with_values = [];
foreach ( $fields as $field_key => $field ) {
$value = $this->get_field_from_order( $field_key, $order, $group );

View File

@ -243,7 +243,7 @@ class CheckoutSchema extends AbstractSchema {
*/
protected function prepare_payment_details_for_response( array $payment_details ) {
return array_map(
function( $key, $value ) {
function ( $key, $value ) {
return (object) [
'key' => $key,
'value' => $value,
@ -308,7 +308,7 @@ class CheckoutSchema extends AbstractSchema {
if ( 'select' === $field['type'] ) {
$field_schema['enum'] = array_map(
function( $option ) {
function ( $option ) {
return $option['value'];
},
$field['options']
@ -333,7 +333,7 @@ class CheckoutSchema extends AbstractSchema {
protected function schema_has_required_property( $additional_fields_schema ) {
return array_reduce(
array_keys( $additional_fields_schema ),
function( $carry, $key ) use ( $additional_fields_schema ) {
function ( $carry, $key ) use ( $additional_fields_schema ) {
return $carry || $additional_fields_schema[ $key ]['required'];
},
false
@ -352,7 +352,7 @@ class CheckoutSchema extends AbstractSchema {
$fields = $sanitization_utils->wp_kses_array(
array_reduce(
array_keys( $fields ),
function( $carry, $key ) use ( $fields, $properties ) {
function ( $carry, $key ) use ( $fields, $properties ) {
if ( ! isset( $properties[ $key ] ) ) {
return $carry;
}
@ -383,12 +383,15 @@ class CheckoutSchema extends AbstractSchema {
$fields = $this->sanitize_additional_fields( $fields );
$additional_field_schema = $this->get_additional_fields_schema();
// Validate individual properties first.
foreach ( $fields as $key => $field_value ) {
if ( ! isset( $additional_field_schema[ $key ] ) ) {
// Loop over the schema instead of the fields. This is to ensure missing fields are validated.
foreach ( $additional_field_schema as $key => $schema ) {
if ( ! isset( $fields[ $key ] ) && ! $schema['required'] ) {
// Optional fields can go missing.
continue;
}
$result = rest_validate_value_from_schema( $field_value, $additional_field_schema[ $key ], $key );
$field_value = isset( $fields[ $key ] ) ? $fields[ $key ] : null;
$result = rest_validate_value_from_schema( $field_value, $schema, $key );
// Only allow custom validation on fields that pass the schema validation.
if ( true === $result ) {
@ -422,6 +425,6 @@ class CheckoutSchema extends AbstractSchema {
}
}
return $errors->has_errors( $errors ) ? $errors : true;
return $errors->has_errors() ? $errors : true;
}
}

View File

@ -141,7 +141,8 @@ class AdditionalFields extends MockeryTestCase {
* Unregister fields after testing.
*/
private function unregister_fields() {
array_map( '__internal_woocommerce_blocks_deregister_checkout_field', array_column( $this->fields, 'id' ) );
$fields = $this->controller->get_additional_fields();
array_map( '__internal_woocommerce_blocks_deregister_checkout_field', array_keys( $fields ) );
}
/**
@ -1315,7 +1316,8 @@ class AdditionalFields extends MockeryTestCase {
),
'payment_method' => 'bacs',
'additional_fields' => array(
$id => 'value',
'plugin-namespace/job-function' => 'engineering',
$id => 'value',
),
)
);
@ -1383,7 +1385,8 @@ class AdditionalFields extends MockeryTestCase {
),
'payment_method' => 'bacs',
'additional_fields' => array(
$id => 'invalid',
'plugin-namespace/job-function' => 'engineering',
$id => 'invalid',
),
)
);
@ -1458,7 +1461,8 @@ class AdditionalFields extends MockeryTestCase {
),
'payment_method' => 'bacs',
'additional_fields' => array(
$id => 'value',
'plugin-namespace/job-function' => 'engineering',
$id => 'value',
),
)
);
@ -1533,7 +1537,8 @@ class AdditionalFields extends MockeryTestCase {
),
'payment_method' => 'bacs',
'additional_fields' => array(
$id => 'invalid',
'plugin-namespace/job-function' => 'engineering',
$id => 'invalid',
),
)
);
@ -1597,7 +1602,9 @@ class AdditionalFields extends MockeryTestCase {
'plugin-namespace/gov-id' => 'gov id',
),
'payment_method' => 'bacs',
'additional_fields' => array(),
'additional_fields' => array(
'plugin-namespace/job-function' => 'engineering',
),
)
);
$response = rest_get_server()->dispatch( $request );
@ -1616,13 +1623,11 @@ class AdditionalFields extends MockeryTestCase {
* Ensures an error is returned when required fields in Contact are missing.
*/
public function test_place_order_required_contact_field() {
$this->markTestSkipped( 'Additional fields aren\'t validated due to a bug #45496.' );
$id = 'plugin-namespace/my-required-contact-field';
$label = 'My Required Field';
$id = 'plugin-namespace/my-required-contact-field';
\__experimental_woocommerce_blocks_register_checkout_field(
array(
'id' => $id,
'label' => $label,
'label' => 'My Required Field',
'location' => 'contact',
'type' => 'text',
'required' => true,
@ -1661,15 +1666,16 @@ class AdditionalFields extends MockeryTestCase {
'plugin-namespace/gov-id' => 'gov id',
),
'payment_method' => 'bacs',
'additional_fields' => array(),
'additional_fields' => array(
'plugin-namespace/job-function' => 'engineering',
),
)
);
$response = rest_get_server()->dispatch( $request );
$data = $response->get_data();
$this->assertEquals( 400, $response->get_status(), print_r( $data, true ) );
// Error should be updated once we got this working.
$this->assertEquals( \sprintf( 'There was a problem with the provided shipping address: %s is required', $label ), $data['message'], print_r( $data, true ) );
$this->assertEquals( \sprintf( '%s is not of type string.', $id ), $data['data']['params']['additional_fields'], print_r( $data, true ) );
\__internal_woocommerce_blocks_deregister_checkout_field( $id );