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

Fix user roles and owner permission checks #6008

Merged
merged 7 commits into from
Jan 14, 2020

Conversation

aldeed
Copy link
Contributor

@aldeed aldeed commented Jan 7, 2020

Resolves #6002
Impact: minor
Type: bugfix

Issue

  • addAccountToGroup mutation doesn't properly handle user doc that has no roles property
  • check for global ownership in createAccount isn't awaited and is therefore always truthy
  • When account.shopId is null, which is usually the case now in 3.0.0 release, an account was not initially added to any groups.

Solution

  • addAccountToGroup mutation will not fail if user.roles array is missing
  • check for global ownership in createAccount is awaited so only the first user created will end up in the "owner" group
  • When account.shopId is null and a primary shop exists, the account is associated with the correct group for the primary shop

Additionally removed addRolesToGroups support for registerPlugin config, which is no longer needed. The roles list is currently stored in the reaction-admin app for all plugins. This will be implemented in a better way with the new authorization service.

Breaking changes

If your plugin passes addRolesToGroups option to registerPlugin, it's now ignored.

Testing

I suggest just code review plus testing the basic user reg/login/logout flows in conjunction with testing reactioncommerce/reaction-admin#171

Signed-off-by: Eric Dobbertin <eric@dairystatedesigns.com>
Signed-off-by: Eric Dobbertin <eric@dairystatedesigns.com>
Signed-off-by: Eric Dobbertin <eric@dairystatedesigns.com>
Signed-off-by: Eric Dobbertin <eric@dairystatedesigns.com>
Copy link
Member

@mikemurray mikemurray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@mikemurray mikemurray merged commit 461f3c8 into release-3.0.0 Jan 14, 2020
@mikemurray mikemurray deleted the fix-aldeed-6002-roles-and-owner-permissions branch January 14, 2020 20:36
@kieckhafer kieckhafer mentioned this pull request Jan 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants