Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

REST API - Authenticated user has ability to read/write others users data #9494

Closed
jordan-enev opened this issue Nov 2, 2015 · 8 comments
Closed

Comments

@jordan-enev
Copy link

The scenario is as follows:
I have an eCommerce website and these days I will start develop a mobile app (it will represent the website - registered users makes orders). The mobile app will communicate with the website with the Woocommerce REST API. For authentication I'll use Auth Basic via HTTPS.
When an user tries to login from the mobile app - the request will be sent to a custom REST endpoint (at website) and there will be generated and returned back to the user - Consumer Key and Consumer Secret. So the user will use them for future REST requests.

The problem that I'm facing is:
An authenticated user (with Consumer Key and Consumer Secret) has ability to read/write others users data.
For an example:
An authenticated user can create an order for another customer - only have to change customer_id value in the request.

So is this desired behavior of the REST API? If it is, how can I restrict one user to change only his data?

Thank you in advance,
Jordan

@claudiosanches
Copy link
Contributor

It works the same way as in WordPress, an administrator user can edit other users, but a customer user is not able to do that.

We use current_user_can( 'edit_users' ):

https://github.com/woothemes/woocommerce/blob/master/includes/api/class-wc-api-customers.php#L693-L750

@jordan-enev
Copy link
Author

Nope, it doesn't work as in Wordpress.
When a customer is logged in to the Wordpress, he has ability to read and write only to his profile data, so :

  • The customer can't create an order for another user;
  • The customer can't edit another user's profile;

At REST API a logged in user to change his profile has to have edit_user permission, but this permission gives ability to change all users data, right?

According your answer I understand that this is the normal behavior.

So is there a way an user to change only his data via REST API?

Thanks,
Jordan

@mikejolley
Copy link
Member

I don't follow your use case @jordantoen - this API isn't for customers to edit and write data. It's for store owners/apps to create things on their behalf. Customers don't have access to API key creation.

@jordan-enev
Copy link
Author

Thank you for the answers.
@mikejolley your comment makes sense to me and clears the whole picture.

While I researched about the case, I found that there are a lot of people who try to use the API in that way.

I think it is worth the API to support eCommerce for customers via mobile apps.

@ar-ml
Copy link

ar-ml commented Nov 22, 2017

@jordan-enev @mikejolley Is this situation still the case by November 2017?
I am searching for a way to retrieve user specific order information via an API from woocommerce, coming from an Android app.
I would like to READ ONLY orders and subscriptions the specific user has made. Only the orders from one user, not from all users.
Woocommerce support chat just told me, this could be done via WC REST API, but if I get this right, this way would open up potential access to all orders from all customers and therefore be a security / privacy hole. So the guy from WC support chat must be wrong.
My question is: Has anyone found a secure way to get customer specific order information (like the information the user sees when logging into his account on our website) via an API or extension or plugin?

@jordan-enev
Copy link
Author

jordan-enev commented Nov 22, 2017

At 2015 I made a custom filter / validation in my Back-end API layer, in order to read the data for a specific user only. From that time, I haven't touched any WooCommerce project, so maybe there is a better way of doing it.

@ar-ml
Copy link

ar-ml commented Nov 22, 2017

@jordan-enev Thanks for your reply. Could you post the code of your custom filter here? Maybe it would help me and other people. Thanks!

@jordan-enev
Copy link
Author

Please follow the code comments:

<?php


/**
 * Class App_API_Validation
 *
 * It is responsible for validating REST API's requests.
 * WooCommerce REST API doesn't ensure that one user has ability to CRUD only his data.
 * More info about the problem we are fixing here: https://github.com/woothemes/woocommerce/issues/9494
 */
class App_API_Validation {

	/**
	 * Validate REST API's request
	 *
	 * It is hooked on 'woocommerce_api_dispatch_args' filter.
	 * With our custom validation we ensure that user has access
	 * only to his resources, which means:
	 * #1 - User doesn't have access ability to make CRUD operations to other users profiles
	 * #2 - User doesn't have access to others users resources (we are validating ownership)
	 * #3 - User doesn't have access to any resource list - for example to get all customers or orders.
	 *
	 * @param array $args Data sent by REST API client
	 * @param array $callback Function to process REST API's request
	 *
	 * @return WP_Error|array
	 */
	public function validate_request( $args, $callback ) {
		$class_name  = get_class( $callback[0] );
		$method_name = $callback[1];
		$id          = ( isset( $args['id'] ) ? $args['id'] : false );

		$are_resources_valid = true;
		if ( $id ) {
			if ( $this->has_to_trigger_resources_user_validation( $class_name, $method_name ) ) {
				// Does user access his own profile ?
				$are_resources_valid = $this->validate_resources_user( $id );
			} else {
				// Does user has ownership of the requested resource ?
				$are_resources_valid = $this->validate_resources_ownership( $id, $class_name );
			}
		} else {
			// Does user try to get a list of resources,
			// for example - all customers or orders
			$are_resources_valid = $this->validate_resources_list( $class_name, $method_name );
		}

		// If requested resources validation fails trigger error
		if ( ! $are_resources_valid ) {
			$args = $this->get_error();
		}

		return $args;
	}

	/**
	 * Filter resource data on create
	 *
	 * We ensure whether a customer creates resource for himself
	 */
	public function validate_resource_creation() {
		add_filter( 'woocommerce_api_create_order_data', array( $this, 'validate_customer_on_resource_creation' ), 1 );
	}

	/**
	 * Validate customer on resource creation
	 *
	 * Do not allow one customer to create resource of another customer
	 *
	 * @param array $data Resource data
	 *
	 * @return array $data
	 * @throws WC_API_Exception
	 */
	public function validate_customer_on_resource_creation( $data ) {
		$customer_id                           = $data['customer_id'];
		$does_user_create_resource_for_himself = $this->validate_resources_user( $customer_id );
		if ( ! $does_user_create_resource_for_himself ) {
			$this->get_error( true );
		}

		return $data;
	}

	/**
	 * Get or throw an error message if validation fails
	 *
	 * @param bool|false $throw Whether to throw new error or not
	 *
	 * @return WP_Error
	 * @throws WC_API_Exception
	 */
	public function get_error( $throw = false ) {
		$code    = 'woocommerce_app_api_insufficient_permissions';
		$message = __( 'You don\'t have sufficient permissions', 'app-translations' );
		$status  = 401;

		if ( $throw ) {
			throw new WC_API_Exception( $code, $message, $status );
		}

		return new WP_Error( $code, $message, array( 'status' => $status ) );
	}

	/**
	 * Check whether or not to trigger user validation
	 *
	 * User validation is triggered when one of listed method is called
	 *
	 * @param string $class_name
	 * @param string $method_name
	 *
	 * @return bool
	 */
	public function has_to_trigger_resources_user_validation( $class_name, $method_name ) {
		$resources_user = array(
			'WC_API_Customers' => array(
				'get_customer',
			),
			'WC_API_Customers_Extended' => array(
				'get_customer_addresses'
			),
		);

		return $this->is_method_exists( $resources_user, $class_name, $method_name );
	}

	/**
	 * Check if a method exists in array of classes (resources)
	 *
	 * @param $resources
	 * @param $class_name
	 * @param $method_name
	 *
	 * @return bool
	 */
	private function is_method_exists( $resources, $class_name, $method_name ) {
		$is_class_exist  = array_key_exists( $class_name, $resources );
		$is_method_exist = ( $is_class_exist && in_array( $method_name, $resources[ $class_name ] ) );

		return $is_method_exist;
	}

	/**
	 * Validate resource list
	 *
	 * If the customer tries to access list of resource,
	 * we are restrict his access.
	 *
	 * @param $class_name
	 * @param $method_name
	 *
	 * @return bool
	 */
	public function validate_resources_list( $class_name, $method_name ) {
		$resources_list = array(
			'WC_API_Customers' => array(
				'get_customers'
			),
			'WC_API_Addresses' => array(
				'get_addresses'
			)
		);

		// If the method exist, then it is forbidden for access
		$condition = ( ! $this->is_method_exists( $resources_list, $class_name, $method_name ) );

		return $condition;
	}

	/**
	 * Does user access his own profile
	 *
	 * We are checking whether current user (by his customer key)
	 * has the same id according requested $id
	 *
	 * @param int $id ID of the requested customer
	 *
	 * @return bool
	 */
	public function validate_resources_user( $id ) {
		$current_user = wp_get_current_user();
		// Check if the user tries to access his own resources
		$does_user_access_his_resource = ( $id == $current_user->ID );

		return $does_user_access_his_resource;
	}


	/**
	 * Check whether requested resource id is owned by current user
	 *
	 * At $resources_ownership array we describe all resources these require
	 * user to has ownership of them.
	 *
	 * @param int    $id ID of requested resource
	 * @param string $class_name Class that represent requested resource
	 *
	 * @return bool
	 */
	public function validate_resources_ownership( $id, $class_name ) {
		// Classes that require user to have ownership of requested resource
		$resources_ownership = array(
			'WC_API_Addresses',
			'WC_API_Orders',
		);

		$is_class_exist = in_array( $class_name, $resources_ownership );
		if ( ! $is_class_exist ) {
			return true;
		}

		$current_user = wp_get_current_user();
		$model_user   = new User( $current_user->ID );
		switch ( $class_name ) {
			case 'WC_API_Addresses':
				$has_user_ownership_of_object = $model_user->has_user_address( $id );
				break;
			case 'WC_API_Orders':
				$has_user_ownership_of_object = $model_user->has_user_order( $id );
				break;
		}

		return $has_user_ownership_of_object;
	}

}

@woocommerce woocommerce locked and limited conversation to collaborators Nov 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants