Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug - Shop manager can see packages not given permission for. #3541

Closed
Akarshit opened this issue Jan 22, 2018 · 7 comments · Fixed by #3645
Closed

Bug - Shop manager can see packages not given permission for. #3541

Akarshit opened this issue Jan 22, 2018 · 7 comments · Fixed by #3645
Assignees
Labels
bug For issues that describe a defect or regression in the released software

Comments

@Akarshit
Copy link
Contributor

Expected behavior

Users in the Shop Manager group should only be able to see the packages they have been given permission for.

Actual behavior

Users in the Shop Manager group can see all the packages(irrespective of if they have been given permission for it or not)

Steps to reproduce the behavior

  1. Run reaction
  2. Invite a user to Shop Manager group.
  3. Login as that user in incognito mode and check that the user can see all shortcutBar icons
  4. Toggle off the following permissions from the shop manager group: Orders, Email, Tax, Shipping
  5. Check that the user can still see those shortcutBar icons.

Cause

The cause of the issue is that the Shop Manager has admin access(here and here).
And here we say that admin can see all the packages. So even if some permission is not given to the Shop Manager, he still has the admin permission and thus would be able to see everything.

Solutions

  1. Remove the admin role from Shop Manager group
  2. Only allow admin users to see packages they have permission for.
@impactmass
Copy link
Contributor

While defining the "Shop Manager" default group in #2183 & #2184, we only specified that it should not have the "owner" role.

From @Akarshit 's research, this issue is happening because the group has "admin" role (see the "Cause" section of the issue detail).

An immediate solution is to remove the "admin" role, but I think we also need to review the entire Shop Manager default group. What it stands for and it's permissions list. @spencern what are your thoughts on this?

@spencern
Copy link
Contributor

@impactmass @Akarshit
Right now admin acts as a kind of global permission for the shop, similar to owner.
For users that should not have access to certain dashboards/"packages" I think we'll need to remove the admin role and assign specific roles.

Let's considering removing the admin role from the Shop Manager group by default, identifying what problems it solves and any new issues it may create. I think that is probably the right direction, but we need to make sure that any additional issues it creates are accounted for.

I think restricting admin users to seeing only packages they have permission for might defeat the purpose of that role, but I can also see why that role is confusing from the UI as it appears that certain privileges are revoked, when a user will still have access because of their admin role.

@Akarshit Akarshit self-assigned this Jan 23, 2018
@impactmass
Copy link
Contributor

noted @spencern, so we'll go ahead and remove the admin role from the shop manager default group, but @Akarshit can we make a note of actions that a user in this group will NOT be able to perform by removing "admin" from it?

@brent-hoover brent-hoover added bug For issues that describe a defect or regression in the released software security labels Jan 24, 2018
@spencern spencern mentioned this issue Feb 5, 2018
@spencern spencern added this to the Bugfix Sprint 1 milestone Feb 9, 2018
@spencern
Copy link
Contributor

@Akarshit do you have any progress or updates for this issue?

@Akarshit
Copy link
Contributor Author

@spencern The fix for the issue has already been merged in release-1.8.0.

@brent-hoover
Copy link
Collaborator

brent-hoover commented Feb 13, 2018

@spencern it looks to me like this ticket was up to date? We were just waiting for release-1.8.0 to be ready for it to be closed? Not sure what else we could have done to keep this updated.

@spencern
Copy link
Contributor

@Akarshit @zenweasel thanks for the update, I didn't see that there was an attached PR and it was in the "in progress" column, so I wasn't sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug For issues that describe a defect or regression in the released software
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants