Fix permissions for items shown in admin dashboard (#2145)

* Initial test of filtering pkgs

* Filter showing packages by permissions

* Resolve issue with hasAccess check on registry items

* Comment

* Comment edit

* Call packages coll without filter

* Move audience addition to ReactionApps

* Better comment

* Check not needed again.Apps come based on access level

* space fix

* Test: Permissions on toolbar component

* hasProduct access on toolbar

* Fix data

* Use createProduct as filter

* Refactor toolbar permission check

* Improve Comment

* Add missing perms in pub

* remove comment

* Add comment on filter line

client/modules/accounts/templates/dropdown/dropdown.html

@@ -35,7 +35,7 @@
   <ul class="user-accounts-dropdown-apps">
     {{> userAccountsDropdown}}
     <!--administrative shortcut icons -->
-    {{#each reactionApps reactionAppsOptions}}
+    {{#each reactionApps provides="shortcut" enabled=true}}
       <li class="dropdown-apps-icon">
         <a href={{pathFor name}} id="dropdown-apps-{{name}}" title="{{label}}">
           <i class="{{icon}}"></i>

client/modules/accounts/templates/dropdown/dropdown.js

@@ -72,16 +72,3 @@ Template.loginDropdown.events({
     template.$(".dropdown-toggle").dropdown("toggle");
   }
 });
-
-Template.accountsDropdownApps.helpers({
-  reactionAppsOptions() {
-    // get shortcuts with audience permissions based on user roles
-    const roles = Roles.getRolesForUser(Meteor.userId(), Reaction.getShopId());
-
-    return {
-      provides: "shortcut",
-      enabled: true,
-      audience: roles
-    };
-  }
-});

client/modules/accounts/templates/members/member.js

@@ -80,8 +80,11 @@ Template.memberSettings.helpers({
           // Get all permissions, add them to an array
           if (registryItem.permissions) {
             for (const permission of registryItem.permissions) {
-              permission.shopId = shopId;
-              permissions.push(permission);
+              // check needed because of non-object perms in the permissions array (e.g "admin", "owner")
+              if (typeof permission === "object") {
+                permission.shopId = shopId;
+                permissions.push(permission);
+              }
             }
           }
 

client/modules/core/helpers/apps.js

@@ -47,11 +47,6 @@ export function Apps(optionHash) {
   const reactionApps = [];
   let options = {};
 
-  // remove audience permissions for owner
-  if (Reaction.hasOwnerAccess() && optionHash.audience) {
-    delete optionHash.audience;
-  }
-
   // allow for object or option.hash
   if (optionHash) {
     if (optionHash.hash) {
@@ -66,6 +61,14 @@ export function Apps(optionHash) {
     options.shopId = Reaction.getShopId();
   }
 
+  // make sure audience is used for all calls to ReactionApps
+  options.audience = Roles.getRolesForUser(Meteor.userId(), Reaction.getShopId());
+
+  // remove audience permissions for owner (still needed here for older/legacy calls)
+  if (Reaction.hasOwnerAccess() && options.audience) {
+    delete options.audience;
+  }
+
   //
   // build filter to only get matching registry elements
   //
@@ -89,24 +92,33 @@ export function Apps(optionHash) {
     }
   }
 
-  // fetch the packages
+  delete filter["registry.audience"]; // Temporarily remove "audience" key (see comment below)
+
+  // TODO: Review fix for filter on Packages.find(filter)
+  // The current "filter" setup uses "audience" field which is not present in the registry array in most (if not all) docs
+  // in the Packages coll.
+  // For now, the audience checks (after the Package.find call) filters out the registry items based on permissions. But
+  // part of the filtering should have been handled by the Package.find call, if the "audience" filter works as it should.
   Packages.find(filter).forEach((app) => {
     const matchingRegistry = _.filter(app.registry, function (item) {
-      const itemFilter = registryFilter;
+      const itemFilter = _.cloneDeep(registryFilter);
 
       // check audience permissions only if they exist as part of optionHash and are part of the registry item
       // ideally all routes should use it, safe for backwards compatibility though
       // owner bypasses permissions
-      if (!Reaction.hasOwnerAccess() && item.audience && registryFilter.audience) {
+      if (!Reaction.hasOwnerAccess() && item.permissions && registryFilter.audience) {
         let hasAccess;
 
         for (const permission of registryFilter.audience) {
-          if (item.audience.indexOf(permission) > -1) {
-            hasAccess = true;
-          }
-          // make sure user also has audience perms
-          if (Roles.userIsInRole(Meteor.userId(), permission, Reaction.getShopId())) {
+          // This checks that the registry item contains a permissions matches with the user's permission for the shop
+          const hasPermissionToRegistryItem = item.permissions.indexOf(permission) > -1;
+          // This checks that the user's permission set have the right value that is on the registry item
+          const hasRoleAccessForShop = Roles.userIsInRole(Meteor.userId(), permission, Reaction.getShopId());
+
+          // both checks must pass for access to be granted
+          if (hasPermissionToRegistryItem && hasRoleAccessForShop) {
             hasAccess = true;
+            break;
           }
         }
 

imports/plugins/core/dashboard/client/components/toolbar.js

@@ -16,6 +16,7 @@ class PublishControls extends Component {
     dashboardHeaderTemplate: PropTypes.oneOfType([PropTypes.func, PropTypes.node, PropTypes.string]),
     documentIds: PropTypes.arrayOf(PropTypes.string),
     documents: PropTypes.arrayOf(PropTypes.object),
+    hasCreateProductAccess: PropTypes.bool,
     isEnabled: PropTypes.bool,
     isPreview: PropTypes.bool,
     onAddProduct: PropTypes.func,
@@ -61,16 +62,20 @@ class PublishControls extends Component {
   }
 
   renderVisibilitySwitch() {
-    return (
-      <Switch
-        i18nKeyLabel="app.editMode"
-        i18nKeyOnLabel="app.editMode"
-        label={"Edit Mode"}
-        onLabel={"Edit Mode"}
-        checked={!this.props.isPreview}
-        onChange={this.onViewContextChange}
-      />
-    );
+    if (this.props.hasCreateProductAccess) {
+      return (
+        <Switch
+          i18nKeyLabel="app.editMode"
+          i18nKeyOnLabel="app.editMode"
+          label={"Edit Mode"}
+          onLabel={"Edit Mode"}
+          checked={!this.props.isPreview}
+          onChange={this.onViewContextChange}
+        />
+      );
+    }
+
+    return null;
   }
 
   renderAdminButton() {
@@ -90,14 +95,18 @@ class PublishControls extends Component {
   }
 
   renderAddButton() {
-    return (
-      <FlatButton
-        i18nKeyTooltip="app.shortcut.addProductLabel"
-        icon={"fa fa-plus"}
-        tooltip={"Add Product"}
-        onClick={this.props.onAddProduct}
-      />
-    );
+    if (this.props.hasCreateProductAccess) {
+      return (
+        <FlatButton
+          i18nKeyTooltip="app.shortcut.addProductLabel"
+          icon={"fa fa-plus"}
+          tooltip={"Add Product"}
+          onClick={this.props.onAddProduct}
+        />
+      );
+    }
+
+    return null;
   }
 
   renderPackageButons() {
@@ -113,7 +122,7 @@ class PublishControls extends Component {
   }
 
   renderCustomControls() {
-    if (this.props.dashboardHeaderTemplate) {
+    if (this.props.dashboardHeaderTemplate && this.props.hasCreateProductAccess) {
       if (this.props.isEnabled) {
         return [
           <VerticalDivider key="customControlsVerticaldivider" />,

imports/plugins/core/dashboard/client/containers/toolbarContainer.js

@@ -88,6 +88,7 @@ function composer(props, onData) {
     isEnabled: isRevisionControlEnabled(),
     isActionViewAtRootView: Reaction.isActionViewAtRootView(),
     actionViewIsOpen: Reaction.isActionViewOpen(),
+    hasCreateProductAccess: Reaction.hasPermission("createProduct", Meteor.userId(), Reaction.shopId),
 
     // Callbacks
     onAddProduct: handleAddProduct,

server/publications/collections/packages.js

@@ -35,9 +35,11 @@ function transform(doc, userId) {
       registry.shopId = doc.shopId;
       registry.packageName = registry.packageName || doc.name;
       registry.settingsKey = (registry.name || doc.name).split("/").splice(-1)[0];
-
       // check and set package enabled state
-      // todo could add audience permissions to registry
+      registry.permissions = [...permissions];
+      if (registry.route) {
+        registry.permissions.push(registry.name || doc.name + "/" + registry.template);
+      }
       if (doc.settings && doc.settings[registry.settingsKey]) {
         registry.enabled = !!doc.settings[registry.settingsKey].enabled;
       } else {