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

allow admin user to copy dashboards #13605

Open
wants to merge 2 commits into
base: 3.x-dev
from

Conversation

Projects
None yet
3 participants
@tsteur
Member

tsteur commented Oct 14, 2018

fix #13567

@tsteur tsteur added this to the 3.7.0 milestone Oct 14, 2018

}
if (!$userFound) {
throw new \Exception(Piwik::translate('Cannot copy dashboard to user %s, user not found or user does not have access to same site.', $copyToUser));

This comment has been minimized.

@sgiehl

sgiehl Oct 14, 2018

Member

Guess this should be moved to a translation key, or the Piwik::translate can be removed

This comment has been minimized.

@tsteur

tsteur Oct 15, 2018

Member

didn't create translation key similar to the end of the API method where it is also not translated as when triggered through the UI this cannot happen... will remove the piwik translate

@diosmosis

This comment has been minimized.

Member

diosmosis commented Oct 15, 2018

Testing locally it seems that even for an admin user it shows the entire list of users in the "Copy dashboard to user" modal. Think this is a security risk.

@tsteur

This comment has been minimized.

Member

tsteur commented Oct 15, 2018

I tested locally here and it didn't. Did you test if the users you see are in the same sites as the admin user?

@diosmosis

This comment has been minimized.

Member

diosmosis commented Oct 15, 2018

I had a single site every user had access to, w/o that site it works (though some users still result in the "Cannot copy dashboard to user" error; they probably don't have access to the site).

@tsteur

This comment has been minimized.

Member

tsteur commented Oct 15, 2018

When every user has access to this site, and the user is an admin user, then the user can see all the other users. That's expected. I just tested here again and it works as expected. As admin users you can see all other users that have access to the same site.

say you have

  • site 1 admin access
  • site 2 write access
  • site 3 view access
  • site 4 admin access
  • site 5 admin access
  • site 6 view access

Then you are allowed to see a list of all users that have access to site 1, 4 and 5.

The method UsersManager.getUsers is used to show the list of available users and is also used to validate in the backend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment