Feature #1106: Limit admins to stores PART II#1178
Feature #1106: Limit admins to stores PART II#1178bandieramonte wants to merge 17 commits intosmartstore:mainfrom
Conversation
…to feature/multi-store-customers
…e not authorized to access by means of manipulating URLs.
…to feature/multi-store-customers
…to feature/multi-store-customers
…to feature/product-page-adjustment
…n choose to filter by any store, or to add/edit any entity into any store regardless of which stores they currently have access to.
…to feature/multi-store-customers
…ndieramonte/Smartstore into feature/multi-store-customers
|
@mgesing @Michael-Herzog |
|
I don't mean to be negative here, as you obviously put a lot of effort into developing this feature, but the solution you chose is the wrong one.
For example, it is unclear what this MediaGallery.cshtml customization has to do with the actual topic. I miss a centralized approach in the code to reduce the many permission checks and data filters. I am therefore concerned that the amount of code required to implement this feature consistently will continue to grow significantly. Some entities are covered here, but what about those that the core does not have access to? Permission checks should not be limited to action methods, as entities can also be accessed via import or Web API. An admin should not be limited in his permissions. They should be able to do everything in the store without exception. It would be a breaking change to restrict this. I also don't like the idea of introducing an Admin and a Super-Admin to compete in their permissions. This again leads to unattractive extra code. A super administrator is a special case. A role that I would save for as few, very special cases as possible, and only use when something cannot be implemented via an admin role. For example, we once used a SuperAdmin to control special functions (e.g. executing an automatic database reset) of a demo backend store via a plugin that only exist in this particular demo store. As a solution, I would have pursued an extension of the permission system and introduced a new system |
|
Hi @mgesing, To begin with, MediaGallery.cshtml customization was an accidental commit which I had reversed in this one on Oct 14, 2024. Second, I want to bring to attention that, as stated in this PR's description, this PR is the continuation of PR 1113, which had been merged on June 24, 2024. That PR follows the same principles which were objected to here. Lastly, your feedback about how to properly do this change is very useful and I wish I had known this before working on these changes. I did extend PermissionService by creating the new function CanAccessEntity intending to have a centralized approach of reducing permission checks. However, some entities are built differently and so I was forced to create a few specific checks outside of this class for those cases. If ever I find the time to redo these changes, I will definitely keep in mind your constructive feedback and check in new commits to this PR. |
This PR is the mandatory continuation of PR #1113, and intends to finish implementation of feature #1106.
While the previous PR only added filtering for orders, so that customers can only view orders from stores they have access to, this work was far from complete. This PR completes implementation by adding the same filtering to the rest of the models present in the dashboard:
If a customer has access to all of the existing stores, then nothing is filtered away (it works as it originally did).
In addition, the customer role Super Admin has been further developed, to help manage administer these new store restrictions, as follows:
Finally, this PR introduces the implementation restricting customers to create any of the aforementioned models only to stores it has access to. Super admins are able to create any model to any stores. Likewise, when editing any model, non super admins can only add stores they have access to.