Servoy / svySecurity

All-purpose security module for Servoy
MIT License
0 stars 2 forks source link

SVYX-355 - allow properties to be solution specific #35

Open SteveHawes opened 3 years ago

SteveHawes commented 3 years ago

This pull request implements the feature requested in SVYX-355.

The changes will now allow properties to be specified as follows:

  1. By user_name, tenant_name and solution_name
  2. By tenant_name and solution_name
  3. By solution_name
  4. Global

The getUserProperty, getUserPropertyValue, getTenantProperty and getTenantPropertyValue methods have been updated so that if a property is requested via these methods and there is no matching record found that includes the solution name, the method will then look for a matching property without a specified solution_name. If this is found then a new property is created that matches the tenant_name and user_name but now with the solution_name added as well and this property is returned. This allows for user and tenant properties that were created prior to this change to be migrated silently in the background and also allows a mix of solutions that both include and exclude this change using the same DB.

paronne commented 3 years ago

Hi Steve, thank you for submitting your contribution. Looking at the proposed feature; to get the functionality you describe i am wondering if introducing a new field to store the solution name is truly necessary.

Using the propertyKey/propertyType fields you can include the solutionName into the property key. Example store your property such as

In your use case, when getting the property you first have to look for the com.my-solution.my-property-name, if NULL look for the Global property. It feels it can all be managed in your own scope to be used to decorate the svyProperties with the solutionName concept

e.g.

scopes.myScope.getProperty(propertyKey, propertyType, activeTenantName, activeUserName, activeSolutionName) {
   if (activeSolutionName) propertyKey = activeSolutionName + '.' + propertyKey;
   scopes.svyProperties.getProperty(propertyKey, propertyType, activeTenantName, activeUserName)
}
SteveHawes commented 3 years ago

Hi Paolo,

I did consider that as a possible solution but that makes it difficult to filter out the values that are for another solution, for example in a maintenance program. This means that you can see the property values for solutions that you may not have access to.

By adding the field it is possible to use a DB, table or foundset filter to easily restrict the values available to the user.

Also, each developer would need to implement their own scope for setting and getting the properties (and their values) and accidentally making a call to the core svyProperties scope could break the security of the solution. The way I have implemented it, it is universal for all developers and is difficult to get wrong.

However, all that said, I do understand if this PR is rejected in favour of the developer implementing something in their own scope.

Thanks Steve

paronne commented 3 years ago

Hi Steve,

thanks for your response. I understand your use case. We will keep this request open and further re-asses it later; however indeed there is a good chance it will get rejected. As per the foundset filter. if you use a good naming convention you should still be able to filter the properties by solution name; Servoy even allow a query builder (QBSelect) to be used as a filter, i am sure there is a way to filter these properties as desired.

Regards, Paolo