VirtoCommerce/vc-module-order

Add permission which could manage prices visibility

yecli opened this issue ยท 11 comments

yecli commented

We need to add new scope for ''order:read" that could limit returned order data by specifying CustomerOrderResponseGroup's. As a specific case, we want to be able to return orders with no amounts info (prices/discounts/taxes).
image
Prices in selected places should not be shown, API should not return info on them.

Main idea:
If user have scope OrderLimitResponseScope in 'order:read' permission - we return all orders data except response groups specified in the scope.
In case user do not have that scope under 'order:read' permission - all data are returned (means no changes for already existing users).

Technical details:
vc-module-core

vc-module-order

  • When we handling orders API request in OrderModuleController, we need to check for OrderLimitResponseScope scope existence and exclude selected scopes from returned response group;

  • Need to add an ability to nullify amount values for orders that API returns. It could be achieved by one of the following ways:

  1. Create own entities for different order objects amounts (e.g. by using EF ComplexType) and not load/clear them based on ResponseGroup. This would require core models change, also need to support backward compatibility in that case;
  2. In service CustomerOrderServiceImpl we could clear amount values for order objects;
  • When updating entities, we need to skip updating properties for the Response group we do not have rights to. For amounts, it could be done by skipping fields update if we have no data for in Patch methods (e.g. CustomerOrderEntity.Patch). Need to carefully check if we have no data for the object - all non calculated non nullable amount properties (probably decimal or int) should be default value of that type. E.g. for LineItem these amounts are TaxPercentRate, Price, DiscountAmount and Quantity, looking at TotalsCalculator;

UI

  • Add scope OrderLimitResponseScope that allow to choose restricted ResponseGroups (it should be multiselect dictionary as OrderStoreScope based on some of CustomerOrderResponseGroup enum values - now only new WithAmounts is must-have value):
    image

  • Hide amounts on the orders blades and widgets based on the scope value;

[Security issues] Why encrypt/decrypt prices? "Visibility" means we don't need to return any amounts or values in the backend API response at all.
[Compatibility issues] this feature will impact existant solutions because this permission is not by default and prices/amounts will be hidden after update. So custom projects will require to make changes in users permissions.
[Suggestion] Add a scope to order:read permission like "Show Amounts" = true/false (with true by default)
[Note] Make sure not only prices but all order amounts are hidden. So term "Show amounts" is more correct in this case.

@tatarincev please review

yecli commented

Encrypt/decrypt were proposed for saving data from the client - that way we could get prices from encrypted data from the client side. In case we want to avoid these security risks - we could use clearing/loading amounts on API controllers side instead.

Good idea about scope in order:read, maybe it could be something like "HideAmounts" for compatibility (so its absence would leave current behavior).

the feature is about limiting reading, not saving, updating or encrypting data.
"HideAmounts" sounds good.

@IgorisB Encrypt/decrypt is required due of peculiarities of working our update logic (order aggregate is saved with all it properties, no partial update allowed) therefore there are may be situations when user who not have permissions to see a prices, have permissions to update order e.g status or do some other actions that can lead to save order object and empty properties with amounts (removed before on read operation) will replace original values in DB.

@tatarincev @yecli
I agree with @IgorisB we should not return any data if a user doesn't have the permission.

To encrypt/decrypt the value, you would need to change the data model and it will crash the current clients.

I offer

  1. use Data Masking and return for example Zero value
  2. If I could not read the price, I could not update the price
  3. Update admin UI to hide the data if I don't have the permission

Note:
the best approach - don't load this date from DB if I don't have the permisson

Do you any use cases for that situation? I can't imagine situation when customer who can change order itself can't see prices in order

@asvishnyakov I had the discussion with @tatarincev and @IgorisB. @tatarincev will reorganize this ticket to split Price and Total amount int own entities (with back capabilities), add a new response group WithPrice and extend grant read permission with Response groups scope. So, we can limit read access to specific response groups.

yecli commented

Updated description according to the discussion results.

  • Add item WithPrices in the enumeration CustomerOrderResponseGroup. Add to Full. Make PullRequest.

  • Create an OrderLimitResponseScope inherited from PermissionScope
    (VirtoCommerce.OrderModule.Web, Security directory), and register it in module.cs (PostInitialize())

  • Add a list of valid items from the enumeration CustomerOrderResponseGroup to OrderLimitResponseScope

  • In the method GetCustomerOrdersByIds of repository OrderRepositoryImpl.cs reset not calculated base prices if the WithPrices flag in ResponseGroup is not set

  • Add a processing element CustomerOrderResponseGroup.WithPrices to the method
    GetCustomerOrdersByIds of OrderRepositoryImpl.cs repository

  • Change the path method of entities (prices changing is available only if at least one value is not 0)

  • Development of unit tests

  • All the methods for selecting orders in OrderModuleController.cs should work according to the
    OrderLimitResponseScope

  • Add a new OrderLimitResponseScope element to the list for the order:read role

  • Hide price on the LineItemsWidget of the order details blade

  • Hide the CustomerOrderTotalsWidget on the order details blade

  • Hide prices on the OrderOperationDetail widget

  • Hide prices on the ShipmentDetail blade

  • Hide prices on the PaymentDetail blade

  • Hide prices on the OrderLineItems blade

yecli commented

Released in v2.17.23.