Audit permissions for accounts in user accounts dashboard

[kieckhafer]

1. Go through all account permissions and make sure toggling is correctly granting admin and client control / access
2. Fix all labels to make sure permissions show an appropriate title / label
3. Add "Groups", which allow you to select a type of user (i.e. Admin, Accounts, Products, etc), and automatically grants you the appropriate access

[aaronjudd]

Also see related permissions issues, covering changes in permissions being made now for marketplace #1591 #1622. A recent bug reported as well with #1641 (which this issue covers), and a legacy issue #797, which also covers testing.

[aaronjudd]

Related to #1642

[impactmass]

I'm testing this out on wip-release-0.20.0. What I see is when I add more permissions to a new account, and I go to that account, the extended privileges don't kick-in. It still behaves as a normal account without the extra permissions (which I believe is #1 point mentioned above)
That's what I'm taking a look into first.

[impactmass]

An update on step 1 mentioned on this ticket:

One case where granted permissions are not currently working as I think it should is being able to access to menu items on nav dropdown, after the right permission is given to an account. That is:

Expected behavior: After giving an account the access to "Accounts", "Dashboard" etc (screenshot). The account should be able to see these options when the nav dropdown is clicked.

Actual Behavior: The new access doesn't reflect in the options as shown

Steps to Reproduce the Behavior: Follow as shown in the first screenshot.

I've traced this to the fact that the filter used when querying for matching Packaging e.g

{
  "registry.provides": "shortcut",
  "enabled": true,
  "registry.audience": {
    "$in": [
      "account/profile",
      "guest",
      "product",
      "tag",
      ...
    ]
  },
  "shopId": "xxxxxx"
}

does not match the way the data is stored. I currently haven't seen a Package with a registry array filed that contains an audience array.

[impactmass]

Status is being tracked on WIP PR here #2145

[impactmass]

Status on the different parts of this:

1. The first issue: Granting access to admin dashboard immediately opens all packages/settings to a non-admin. This should be fixed after (WIP) Fix permissions for items shown in admin dashboard #2145 passes review and gets merged. It's still WIP because there are some parts needing clarification (see PR).

2. While working on *1 above, I started an audit of all dashboard permissions toggling. Current status is reported on Fix permissions for items shown in admin dashboard #2145 as well. Issues discovered can be split to another issue.
   (Update: Moving the audit of the perms to the next comment below on this issue, instead of being on the PR).

3. Ensuring appropriate label for all permissions. All the labels seem fine at the moment.

4. "Groups" (mentioned in this issue's description). Yet to be started.

[impactmass]

Audit status (see *2 in comment above)

Confirming Top-Level & Sub-level Toggling of permissions
(Checked items works. Unchecked items - yet to confirm. Other comments are for issues spotted)

• Accounts ("reaction-accounts")
• Accounts ("accounts") [Access to the whole accounts sidebar]
• Account Settings ("reaction-accounts/accountsSettings") [can't figure out what this should do]
• Accounts ("dashboard/accounts") [can't figure out what this should do]
• Profile ("account/profile")
• Catalog (gives access to catalog section of dashboard)
• Checkout ("reaction-checkout")
• Checkout ("cart/checkout")
• /Cart/Completed/:_id? ("cart/completed") [Able to see the page without access given]
• Dashboard ("reaction-dashboard")
• Dashboard ("dashboard")
• Shop Settings ("shopSettings") [Doesn't show/hide shop settings] (Should we even have this? The "Accounts" counterpart doesn't)
• Discounts ("reaction-discounts") [Has access to "Codes" under Payments tab; regardless of access level. Gets access denied when creating code every time] [Update: confirmed it works]
• Email
• I18n
• Layout ("reaction-layout") [Haven't figure out what this does]
• Logging ("reaction-logging") [This is a mostly server-side pkg. Should probably not show up in the list?]
• Orders ("reaction-orders")
• Orders ("orders") [Issue: When with access to the Orders panel...the count (new, processing, completed) shows 0, but Orders are shown in the sections if available... possibly caused by the publication]
• Orders ("dashboard/orders")
• /Dashboard/Pdf/Orders/:Id ("dashboard/pdf/orders") [Not working. User gets access to this page regardless if it's on/off]
• Payments ("reaction-payments")
• Revisions ("reaction-revisions")
• Shipping ("reaction-shipping")
• Shipping ("shipping")
• Tax ("reaction-taxes")
• Templates
• Ui ("reaction-ui"), Themes ("reaction-ui/uiDashboard"), /Dashboard/Ui/:Id ("dashboard/uiThemeDetails"), Ui Grid, Ui Navbar, Ui Tagnav, Ui Search & Migrations. All these don't have admin functions?
• Analytics ("reaction-analytics")
• Analytics Settings ("reaction-analytics/reactionAnalyticsSettings") Confirm difference between this and above
• Discount Codes

Other questions:

• Confirm this behavior: Removing access to Profile under Accounts, still shows the profile shortcut icon. Although when it is clicked, a You don't have permission to view this content message is shown.
• Should removing access to Accounts (or more specifically Profile under Accounts) also takes away username beside avatar?

[aaronjudd]

• Account Settings ("reaction-accounts/accountsSettings") [can't figure out what this should do]
  I'm not sure either. it might be redundant old registry entries.

• Accounts ("dashboard/accounts") [can't figure out what this should do]

• Shop Settings ("shopSettings") [Doesn't show/hide shop settings] (Should we even have this? The "Accounts" counterpart doesn't)

Yes, shop settings is the default screen that all shop admins should configure (name,etc). This should be visible.

• Discounts ("reaction-discounts") [Has access to "Codes" under Payments tab; regardless of access level. Gets access denied when creating code every time]

If "Codes" is enabled, they should always be visible (to all users). I think that I made a special case for this (but not sure). ie, you would always want users to be able to input codes, but not create them.

• Layout ("reaction-layout") [Haven't figure out what this does]

I think this can be removed from the registry. We were going to build a UI to allow selection of layouts/themes. On hold for a bit...

• Logging ("reaction-logging") [This is a mostly server-side pkg. Should probably not show up in the list?]

See #2035

Feel like this was [also] supposed to be for loggly configuration @jshimko? The credentials have to be configured as ENV variables, but this should have settings. This was mentioned / implemented in #1574. I created #2165 to track the UI component.

• Ui ("reaction-ui"), Themes ("reaction-ui/uiDashboard"), /Dashboard/Ui/:Id ("dashboard/uiThemeDetails"), Ui Grid, Ui Navbar, Ui Tagnav, Ui Search & Migrations. All these don't have admin functions?

These are all components @mikemurray .. some of these should respect permissions I would think. however.. maybe these should be ignored for now as well?

• Analytics Settings ("reaction-analytics/reactionAnalyticsSettings") Confirm difference between this and above

Hmmm.. I don't see analytics in admin or guest view. I'll have to dig in there.

Some of these labels could be cleaned up in #1877.

[aaronjudd]

• Removing access to Profile under Accounts, still shows the profile shortcut icon. Although when it is clicked, a You don't have permission to view this content message is shown.
  

If profile is enabled, you should be able both see the link as well as view the accounts page.

It feels as though both of these "Accounts", that they're actually misconfigured parents, and the children are supposed to be "Profile" and "Account Settings".

[impactmass]

Status: PR review in progress (#2145)

[kieckhafer]

@impactmass do you think all your permissions updates took care of this?

[impactmass]

The permissions epic (update #2183) took care of mostly point 2 and 3 mentioned on the description of this ticket. There is more work to be done around point 1. I'll go through the issues and write up a new ticket that will take over (and close this off).

[spencern]

@mikemurray and I discovered today during our final review of 1.5 that the dashboard does does not take permissions into account when determining which modules are available to the user. It appears that the dashboard permission is the only one that mattered.

[aaronjudd]

@impactmass @spencern please update status on this issue.

[impactmass]

Point 1 and part of 2 was covered in #2145 #2235. Point 3 (Groups) was completed as part of #2183.
There is now a UI/UX epic issue for accounts admin, and that can cover the labels display. It's better to work on them there because the labels at the time this issue was created is different from what it is now.

@spencern re: your last comment.... please confirm if the issue you mentioned is still happening. I currently don't see it. (also after the fix done in #2183 and more recently #3505).
I think we can close this ticket it that is working correctly.

[spencern]

@impactmass as that comment is from October and you're not seeing it, I think we can close.

[aaronjudd]

I was seeing issues while testing master (1.7.0) - most likely new issues should be created. Specifically some changes did not appear to take affect. Some gave errors, and I also "changed owner" and hit the "cancel" and the owner changed regardless of cancel, etc. these could all be new issues -> but am suggesting another review is required.

[impactmass]

Went through as mentioned by @aaronjudd. I found and create issues: #3648 and #3649.
Both related to the group permissions setup in #2183 and #2184 (more recent tickets than this one). We can close this in favor of those.

[brent-hoover]

@impactmass So is this ready to close?

[impactmass]

yes can be closed, in favor of the new issues created above, plus the UI epic (#2922)