Skip to content

Commit

Permalink
VP-1048: Fix order prices visibility (#122)
Browse files Browse the repository at this point in the history 
* VP-1048: Fix order prices visibility

- Fixed the case when order prices are 0 for admin with some order permissions, but without "order:read_prices";
- Added unit tests;
- Made methods that need get user async;
  • Loading branch information
@yecli
yecli authored Dec 3, 2019
1 parent d415839 commit cc4a7c9
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 35 deletions.
94 changes: 89 additions & 5 deletions VirtoCommerce.OrderModule.Test/CheckingResponseGroup.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace VirtoCommerce.OrderModule.Test
[CLSCompliant(false)]
public class CheckingResponseGroup
{
private static Permission[] PreparePermissions(bool withPrices = false)
private static Permission[] PreparePermissions(bool withPrices)
{
var permissions = new List<Permission>
{
Expand Down Expand Up @@ -43,8 +43,15 @@ private static Permission[] PreparePermissions(bool withPrices = false)
[InlineData("Default", "WithPrices")]
public void CanCheckPermissionsWithNoPrices(string expected, string respGroup)
{
var permissions = PreparePermissions();
Assert.Equal(expected, OrderReadPricesPermission.ApplyResponseGroupFiltering(permissions, respGroup));
// Arrange
var permissions = PreparePermissions(false);
var user = new ApplicationUserExtended();

// Act
var result = OrderReadPricesPermission.ApplyResponseGroupFiltering(user, permissions, respGroup);

// Assert
Assert.Equal(expected, result);
}

[Theory]
Expand All @@ -54,8 +61,15 @@ public void CanCheckPermissionsWithNoPrices(string expected, string respGroup)
[InlineData("scope1", "scope1")]
public void CanCheckPermissionsWithPrices(string expected, string respGroup)
{
// Arrange
var permissions = PreparePermissions(true);
Assert.Equal(expected, OrderReadPricesPermission.ApplyResponseGroupFiltering(permissions, respGroup));
var user = new ApplicationUserExtended();

// Act
var result = OrderReadPricesPermission.ApplyResponseGroupFiltering(user, permissions, respGroup);

// Assert
Assert.Equal(expected, result);
}

[Theory]
Expand All @@ -65,8 +79,78 @@ public void CanCheckPermissionsWithPrices(string expected, string respGroup)
[InlineData("scope1", "scope1")]
public void CanCheckPermissionsNoPermissions(string expected, string respGroup)
{
// Arrange
var permissions = new Permission[0];
var user = new ApplicationUserExtended();

// Act
var result = OrderReadPricesPermission.ApplyResponseGroupFiltering(user, permissions, respGroup);

// Assert
Assert.Equal(expected, result);
}

[Theory]
[InlineData(null, null)]
[InlineData("scope1,scope2", "scope1,scope2")]
[InlineData("WithPrices,scope1,scope2", "WithPrices,scope1,scope2")]
[InlineData("scope1", "scope1")]
public void ApplyResponseGroupFiltering_AdminWithOrderPermissionNoReadPrices_NoChangesInResponseGroup(string expected, string respGroup)
{
// Arrange
var permissions = PreparePermissions(false);
var user = new ApplicationUserExtended()
{
IsAdministrator = true,
};

// Act
var result = OrderReadPricesPermission.ApplyResponseGroupFiltering(user, permissions, respGroup);

// Assert
Assert.Equal(expected, result);
}

[Theory]
[InlineData(null, null)]
[InlineData("scope1,scope2", "scope1,scope2")]
[InlineData("WithPrices,scope1,scope2", "WithPrices,scope1,scope2")]
[InlineData("scope1", "scope1")]
public void ApplyResponseGroupFiltering_AdminWithOrderPermissionWithReadPrices_NoChangesInResponseGroup(string expected, string respGroup)
{
// Arrange
var permissions = PreparePermissions(true);
var user = new ApplicationUserExtended()
{
IsAdministrator = true,
};

// Act
var result = OrderReadPricesPermission.ApplyResponseGroupFiltering(user, permissions, respGroup);

// Assert
Assert.Equal(expected, result);
}

[Theory]
[InlineData(null, null)]
[InlineData("scope1,scope2", "scope1,scope2")]
[InlineData("WithPrices,scope1,scope2", "WithPrices,scope1,scope2")]
[InlineData("scope1", "scope1")]
public void ApplyResponseGroupFiltering_AdminNoPermissions_NoChangesInResponseGroup(string expected, string respGroup)
{
// Arrange
var permissions = new Permission[0];
Assert.Equal(expected, OrderReadPricesPermission.ApplyResponseGroupFiltering(permissions, respGroup));
var user = new ApplicationUserExtended()
{
IsAdministrator = true,
};

// Act
var result = OrderReadPricesPermission.ApplyResponseGroupFiltering(user, permissions, respGroup);

// Assert
Assert.Equal(expected, result);
}
}
}
62 changes: 34 additions & 28 deletions VirtoCommerce.OrderModule.Web/Controllers/Api/OrderModuleController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,10 @@ public OrderModuleController(
[HttpPost]
[Route("search")]
[ResponseType(typeof(webModel.CustomerOrderSearchResult))]
public IHttpActionResult Search(CustomerOrderSearchCriteria criteria)
public async Task<IHttpActionResult> Search(CustomerOrderSearchCriteria criteria)
{
//Scope bound ACL filtration
criteria = FilterOrderSearchCriteria(HttpContext.Current.User.Identity.Name, criteria);
criteria = await FilterOrderSearchCriteria(HttpContext.Current.User.Identity.Name, criteria);

var result = _searchService.SearchCustomerOrders(criteria);
var retVal = new CustomerOrderSearchResult
Expand All @@ -116,11 +116,15 @@ public IHttpActionResult Search(CustomerOrderSearchCriteria criteria)
[HttpGet]
[Route("number/{number}")]
[ResponseType(typeof(CustomerOrder))]
public IHttpActionResult GetByNumber(string number, [FromUri] string respGroup = null)
public async Task<IHttpActionResult> GetByNumber(string number, [FromUri] string respGroup = null)
{
var searchCriteria = AbstractTypeFactory<CustomerOrderSearchCriteria>.TryCreateInstance();
searchCriteria.Number = number;
searchCriteria.ResponseGroup = OrderReadPricesPermission.ApplyResponseGroupFiltering(_securityService.GetUserPermissions(User.Identity.Name), respGroup);

var userName = User.Identity.Name;
var user = await _securityService.FindByNameAsync(userName, UserDetails.Reduced);

searchCriteria.ResponseGroup = OrderReadPricesPermission.ApplyResponseGroupFiltering(user, _securityService.GetUserPermissions(userName), respGroup);

var result = _searchService.SearchCustomerOrders(searchCriteria);

Expand Down Expand Up @@ -149,28 +153,32 @@ public IHttpActionResult GetByNumber(string number, [FromUri] string respGroup =
[HttpGet]
[Route("{id}")]
[ResponseType(typeof(CustomerOrder))]
public IHttpActionResult GetById(string id, [FromUri] string respGroup = null)
public async Task<IHttpActionResult> GetById(string id, [FromUri] string respGroup = null)
{
var retVal = _customerOrderService.GetByIds(new[] { id }, OrderReadPricesPermission.ApplyResponseGroupFiltering(_securityService.GetUserPermissions(User.Identity.Name), respGroup))
.FirstOrDefault();
var userName = User.Identity.Name;
var user = await _securityService.FindByNameAsync(userName, UserDetails.Reduced);

if (retVal == null)
respGroup = OrderReadPricesPermission.ApplyResponseGroupFiltering(user, _securityService.GetUserPermissions(userName), respGroup);

var result = _customerOrderService.GetByIds(new[] { id }, respGroup).FirstOrDefault();

if (result == null)
{
return NotFound();
}

//Scope bound security check
var scopes = _permissionScopeService.GetObjectPermissionScopeStrings(retVal).ToArray();
var scopes = _permissionScopeService.GetObjectPermissionScopeStrings(result).ToArray();

if (!_securityService.UserHasAnyPermission(User.Identity.Name, scopes, OrderPredefinedPermissions.Read))
{
throw new HttpResponseException(HttpStatusCode.Unauthorized);
}

//Set scopes for UI scope bounded ACL checking
retVal.Scopes = scopes;
result.Scopes = scopes;

return Ok(retVal);
return Ok(result);
}

/// <summary>
Expand Down Expand Up @@ -333,13 +341,13 @@ public IHttpActionResult GetNewShipment(string id)
return Ok(retVal);

////Detect not whole shipped items
////TODO: LineItem partial shipping
//var shippedLineItemIds = order.Shipments.SelectMany(x => x.Items).Select(x => x.LineItemId);
////TechDebt: LineItem partial shipping
////var shippedLineItemIds = order.Shipments.SelectMany(x => x.Items).Select(x => x.LineItemId);

////TODO Add check for digital products (don't add to shipment)
////TechDebt: Add check for digital products (don't add to shipment)
//retVal.Items = order.Items.Where(x => !shippedLineItemIds.Contains(x.Id))
// .Select(x => new coreModel.ShipmentItem(x)).ToList();
//return Ok(retVal.ToWebModel());
//// .Select(x => new coreModel.ShipmentItem(x)).ToList();
////return Ok(retVal.ToWebModel());
}

return NotFound();
Expand Down Expand Up @@ -503,12 +511,16 @@ public IHttpActionResult PostProcessPayment(webModel.PaymentCallbackParameters c
[HttpGet]
[Route("invoice/{orderNumber}")]
[SwaggerFileResponse]
public IHttpActionResult GetInvoicePdf(string orderNumber)
public async Task<IHttpActionResult> GetInvoicePdf(string orderNumber)
{
var searchCriteria = AbstractTypeFactory<CustomerOrderSearchCriteria>.TryCreateInstance();
var userName = User.Identity.Name;
var user = await _securityService.FindByNameAsync(userName, UserDetails.Reduced);
var responseGroup = OrderReadPricesPermission.ApplyResponseGroupFiltering(user, _securityService.GetUserPermissions(userName), null);

searchCriteria.Number = orderNumber;
searchCriteria.Take = 1;
searchCriteria.ResponseGroup = OrderReadPricesPermission.ApplyResponseGroupFiltering(_securityService.GetUserPermissions(User.Identity.Name), null);
searchCriteria.ResponseGroup = responseGroup;

var order = _searchService.SearchCustomerOrders(searchCriteria).Results.FirstOrDefault();

Expand Down Expand Up @@ -554,10 +566,11 @@ public IHttpActionResult GetOrderChanges(string id)
return Ok(result);
}

private CustomerOrderSearchCriteria FilterOrderSearchCriteria(string userName,
CustomerOrderSearchCriteria criteria)
private async Task<CustomerOrderSearchCriteria> FilterOrderSearchCriteria(string userName, CustomerOrderSearchCriteria criteria)
{
var user = _securityService.FindByNameAsync(userName, UserDetails.Reduced).Result;
var user = await _securityService.FindByNameAsync(userName, UserDetails.Reduced);

criteria.ResponseGroup = OrderReadPricesPermission.ApplyResponseGroupFiltering(user, _securityService.GetUserPermissions(userName), criteria.ResponseGroup);

if (!_securityService.UserHasAnyPermission(userName, null, OrderPredefinedPermissions.Read))
{
Expand All @@ -582,13 +595,6 @@ private CustomerOrderSearchCriteria FilterOrderSearchCriteria(string userName,
}
}

if (!user.IsAdministrator)
{
// ResponseGroup
criteria.ResponseGroup = OrderReadPricesPermission.ApplyResponseGroupFiltering(
_securityService.GetUserPermissions(User.Identity.Name), criteria.ResponseGroup);
}

return criteria;
}
}
Expand Down
4 changes: 2 additions & 2 deletions VirtoCommerce.OrderModule.Web/Security/OrderReadPricesPermission.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ namespace VirtoCommerce.OrderModule.Web.Security
{
public static class OrderReadPricesPermission
{
public static string ApplyResponseGroupFiltering(Permission[] permissions, string respGroup)
public static string ApplyResponseGroupFiltering(ApplicationUserExtended user, Permission[] permissions, string respGroup)
{
var result = respGroup;

var needRestrict = permissions.Any() && !permissions.Any(x => x.Id == OrderPredefinedPermissions.ReadPrices);
var needRestrict = !user.IsAdministrator && permissions.Any() && !permissions.Any(x => x.Id == OrderPredefinedPermissions.ReadPrices);

if (needRestrict && string.IsNullOrWhiteSpace(respGroup))
{
Expand Down

0 comments on commit cc4a7c9

Please sign in to comment.