ACL system allows admin users to add themselves to roles of greater seniority (2.50)

Fixed. Please see changeset 4297607a1c47

a.m. wrote:

Excellent, this solution makes the most sense to me as it falls in line with how the rest of the permissions function.  Thank you very much!

Aladino,

  How did you address this item?  My approach was to let the admin be just that. I created a new role (manager) who manages the site and customers. I only allow the admin to manage the roles.

I am now trying to prevent the manager/users from viewing roles that are higher that his credentials thus preventing them from adding themselves to a higher role.

I know this thread is old, but I thought I would add some info since I just delt with this issue today.

Version 3.1

First will explain what I did, then I will mention how it can be (I think) added.  Note...the main reason aside from higher roles being possible to attain from a lesser role is that if manage customer roles is disabled, but manage customers is enabled...you don't have enough permission to create a user as you can't assign a default role.  So even trying to create a simple guest or registered role with manage customers enabled...not possible unless both permissions are enabled. So...brings us back to the same challenge. Here is what I did:

My objective: create a new rep role that can only see customers that belong to its affiliate in the backend customer list (ie... the rep customer is linked to an affiliate as it is the affiliate in my case).  Then any customers they create (simple registered) would automatically get the affiliate id of the rep added to its customer record.  (for commission).  
Change a system role like content managers to be able to create new rep accounts (ie. create affiliate and new customer with role type Rep and join/merge to the affiliate).  Now, there is a lot of code about the affiliate which I will leave out of below as its out of context in this thread.

Step 1, use Content Managers role as a test

In CustomerControllers, I changed the Create and Edit functions which are used to generate the lists to below (snippet)

//****New Custom *****************
            //customer roles
            Customer loggedCustomer = _customerService.GetCustomerByEmail(HttpContext.User.Identity.Name);

            if (loggedCustomer.IsInCustomerRole("Administrator"))
            {
                model.AvailableCustomerRoles = _customerService
                .GetAllCustomerRoles(true)
                .Select(cr => cr.ToModel())
                .ToList();
                model.SelectedCustomerRoleIds = customer.CustomerRoles.Select(cr => cr.Id).ToArray();
                model.AllowManagingCustomerRoles = _permissionService.Authorize(StandardPermissionProvider.ManageCustomerRoles);
            }
            else if (loggedCustomer.IsInCustomerRole("Content Owners"))
            {
                model.AvailableCustomerRoles = _customerService
                .GetBottom3Roles(true)
                .Select(cr => cr.ToModel())
                .ToList();
                model.SelectedCustomerRoleIds = customer.CustomerRoles.Select(cr => cr.Id).ToArray();
                model.AllowManagingCustomerRoles = _permissionService.Authorize(StandardPermissionProvider.ManageCustomerRoles);
            }
            else
            {
                model.AvailableCustomerRoles = _customerService
                .GetBottom2Roles(true)
                .Select(cr => cr.ToModel())
                .ToList();
                model.SelectedCustomerRoleIds = customer.CustomerRoles.Select(cr => cr.Id).ToArray();
                model.AllowManagingCustomerRoles = _permissionService.Authorize(StandardPermissionProvider.ManageCustomerRoles);
            }

Same idea as above for the create function

Now you notice I created 2 new roles called GetBottom3Roles and GetBottom2Roles...will get to that in a min.

Now, over to the CustomerRoleController,

//***New Custom **********
            Customer loggedCustomer = _customerService.GetCustomerByEmail(HttpContext.User.Identity.Name);

            if (loggedCustomer.IsInCustomerRole("Administrator"))
            {
                var customerRoles = _customerService.GetAllCustomerRoles(true);

                var gridModel = new GridModel<CustomerRoleModel>
                {
                    Data = customerRoles.Select(x => x.ToModel()),
                    Total = customerRoles.Count()
                };
                return new JsonResult
                {
                    Data = gridModel
                };
            }
            else if (loggedCustomer.IsInCustomerRole("Content Owners"))
            {
                //Only bring back a subset of roles (bottom 3 roles)
                var customerRoles = _customerService.GetBottom3Roles(false);

                var gridModel = new GridModel<CustomerRoleModel>
                {
                    Data = customerRoles.Select(x => x.ToModel()),
                    Total = customerRoles.Count()
                };
                return new JsonResult
                {
                    Data = gridModel
                };
            }
            else
            {
                var customerRoles = _customerService.GetBottom2Roles(false);

                var gridModel = new GridModel<CustomerRoleModel>
                {
                    Data = customerRoles.Select(x => x.ToModel()),
                    Total = customerRoles.Count()
                };
                return new JsonResult
                {
                    Data = gridModel
                };
            }


Note: You if your going to use this code, you don't need to repeat the gridModel section, but I was rushing when I did this.

ok, now to take a closer look at GetBottom2Roles and GetBottom3Roles
For that were heading to the customerService in the services project.

//****New Custom ********************
        public virtual IList<CustomerRole> GetBottom2Roles(bool showHidden = false)
        {
            string key = string.Format(CUSTOMERROLES_ALL_KEY, showHidden);
            return _cacheManager.Get(key, () =>
            {
                var query = from cr in _customerRoleRepository.Table
                            orderby cr.Name
                            where (showHidden || cr.Active) & (cr.SystemName.Equals("Registered") | cr.SystemName.Equals("Guests"))
                            select cr;
                var customerRoles = query.ToList();
                return customerRoles;
            });
        }
        public virtual IList<CustomerRole> GetBottom3Roles(bool showHidden = false)
        {
            string key = string.Format(CUSTOMERROLES_ALL_KEY, showHidden);
            return _cacheManager.Get(key, () =>
            {
                var query = from cr in _customerRoleRepository.Table
                            orderby cr.Name
                            where (showHidden || cr.Active) & (cr.SystemName.Equals("Registered") | cr.SystemName.Equals("Guests") | cr.SystemName.Equals("Rep"))
                            select cr;
                var customerRoles = query.ToList();
                return customerRoles;
            });
        }

and that is it. So now for a some explanation and then I will explain how to make this generic for nop 3.2
Basically, I wanted a way for a Rep (custom role) to be able to create customers, but just basic shoppers (so registered or guests)...and I wanted my Content Owners to be able to create a customer with the rep role...but have no other options in either scenario. This is why I built out 2 methods and hard coded my role names... but after you read what my objective was, it made sense as the shortest route to arrive at solution when I had only an hour to put this together.

So...if I were to do this in 3.2... I would keep the GetAllCustomerRoles, but add 1 overloaded function which takes a comma delimitted string of role names.  So as Administrator is a system role, keep the default GetAllCustomerRoles for that role...but any other role, use a lookup table and pick from a list...that easy.

eg.
public virtual IList<CustomerRole> GetRoleSubset(bool showHidden = false, string roles)
        {
            string key = string.Format(CUSTOMERROLES_ALL_KEY, showHidden);
            return _cacheManager.Get(key, () =>
            {
                var query = from cr in _customerRoleRepository.Table
                            orderby cr.Name
                            where (showHidden || cr.Active) & (roles.Contains(roles))
                            select cr;
                var customerRoles = query.ToList();
                return customerRoles;
            });
        }

I might actually change my initial prototype implementation to this when I get some time...but maybe this approach will make it into Nop 3.2.  Hope I gave enough details for someone to do their own verison.


Andrei, as this is an old thread, not sure if your going to be reading this or not...but the function above, using the same approach in the customer controller leveraging a lookup table could be done.  And it keeps the logic clean.  What do you think?


ChuckR

ChuckR wrote:
Hi Chuck,

That's for reporting. Agree. I've just created a work item. Although it'll be fixed a bit another way.
1. You should not hard-code customer role names
2. Too complex validations
3. etc

I think it's easier and better to:
1. Merge "Manage customer" and "Manage customer role" into one single permission as it was.
2. Validate "Administrators" role when editing a customer. If a current customer is not "Administrators", then do not allow adding to this role.

a.m. wrote:

Thanks Andrei!

I have posted my revised idea into the work item. Now that I had time to sleep on it, thought of another approach.  I will followup in the work item thread.

Thanks again,
Chuck

Thanks. Please see my comment on codeplex and the changeset 8ea8390b9281

a.m. wrote:

Thanks Andrei,

So I took a look at your changes in the changeset window you posted on codeplex an hour ago and altered my custom solution to match.  Then I extended to my use case which requires multi-level role assignment and came up with a "starter governance" model.  I am just explaining this quick in case someone else sees a better way to cover my requirement in future.
Same concept as DisplayOrder in most of your tables, I just added 1 more integer value to the role table called (Rank).  For administrator I assigned 0, then content managers I assigned the value 5 (leaving space in the middle for new custom roles between), then my CustomRep the value 15.  In essence this very simply defines levels.
So in your new published code where you check for Administrator... all I did was change this to check for rank...getRolesWithAHigherRankThanMe function.
Basically the function gets the lowest numeric value of the logged in user (basically 0 is the best, and the higher the number goes, the less permissions) and gets all Roles with a higher rank value...so for me Rank=25 is guest.
Once it has that, it runs the function above and gets all roles with a rank value higher for display.

Administrator-Rank 0
Some power use role - Rank 3
Content Manager - Rank 5
...etc
Custom Rep - Rank 15
Registered - Rank 25
Guest - 25

logic... Get All roles with a rank equal to my own(logged in user) or lower.

Note: I am keeping "Manage custom roles" alive in my version so that I can control the role screen where this new rank value (implemented identical to display order in other areas) gets managed.

Thanks
Chuck

I am using version 3.30 but don't have the option to restrict by role. I created a customer with limited rights and customer access but was able to change the role access even without being admin. is there a fix for version 3.3?

Josephsawyer wrote:
Please do not duplicate forum questions (deleted).

Please provide a complete list of steps to reproduce this issue on version 3.30