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

Move roles and group.permissions code from account plugin to simple-auth plugin #6111

Merged
merged 9 commits into from
Feb 28, 2020

Conversation

aldeed
Copy link
Contributor

@aldeed aldeed commented Feb 26, 2020

Impact: minor
Type: feature

Issue

Some code in the account plugin references things that are now the domain of the new simple-authorization plugin. This makes it difficult to remove the simple-authorization plugin and replace it with a different authorization solution.

Solution

  • Move everything related to roles (collection, queries, util fns, etc.) to the simple-authorization plugin.
  • Move everything related to the group.permissions array to the simple-authorization plugin as an extension of the Group type.

Breaking changes

None

Testing

  1. Check out this PR branch and link in the corresponding simple-auth plugin PR.
  2. Verify that createAccountGroup and updateAccountGroup mutations continue to work as expected, and allow you to pass in a permissions array as part of group.
  3. Verify that Query.roles and shop.roles GQL queries continue to work. They should return all the built-in permissions as well as any other strings you pass in the permissions array when calling createAccountGroup or updateAccountGroup mutation.
  4. Verify that you can request the permissions field on Group type in GraphQL.
  5. Clear the database, register a user, and then query the API with that user's token. Verify that both global groups are auto-created and the first account is added to the system-manager group.
  6. Create the first shop. Verify that the "shop manager" and "owner" groups are created for that shop, and the account that created the shop is added to the owner group for that shop.

Testing Queries

createAccountGroup mutation:

mutation createAccountGroup($input: CreateAccountGroupInput!) {
  createAccountGroup(input: $input) {
    group {
      _id
      description
      name
      slug
      permissions
      createdAt
      createdBy {
        primaryEmailAddress
      }
      updatedAt
      shop {
        _id
        name
      }
    }
  }
}

updateAccountGroup mutation:

mutation updateAccountGroup($input: UpdateAccountGroupInput!) {
  updateAccountGroup(input: $input) {
    group {
      _id
      description
      permissions
      name
      slug
      createdAt
      createdBy {
        primaryEmailAddress
      }
      updatedAt
      shop {
        _id
        name
      }
    }
  }
}

Create global group with permissions

The first account, and any account in the "system-manager" group, should be able to create a global group with the above mutation and input like this:

{
  "input": {
    "group": {
      "name": "global test group",
      "permissions": ["reaction:legacy:groups/read"]
    }
  }
}

Update global group permissions

The first account, and any account in the "system-manager" group, should be able to update a global group with the above mutation and input like this:

{
  "input": {
    "groupId": "cmVhY3Rpb24vZ3JvdXA6TjdMdHE4OHJhekpNTWhlazk=",
    "group": {
      "permissions": ["reaction:legacy:groups/read", "reaction:legacy:groups/update"]
    }
  }
}

(Use actual groupId, such as group._id returned by createAccountGroup.)

Create shop group with permissions

Any account in the "shop-manager" group for a shop should be able to create a shop group with the above mutation and input like this:

{
  "input": {
    "group": {
      "name": "shop test group",
      "description": "A test group",
      "permissions": ["reaction:legacy:groups/read"]
    },
    "shopId": "cmVhY3Rpb24vc2hvcDp5YjRTOG1KQ1hmQzJweEFrTg=="
  }
}

Update shop group permissions

The first account, and any account in the "system-manager" group, should be able to update a global group with the above mutation and input like this:

{
  "input": {
    "groupId": "cmVhY3Rpb24vZ3JvdXA6TjdMdHE4OHJhekpNTWhlazk=",
    "group": {
      "permissions": ["reaction:legacy:groups/read", "reaction:legacy:groups/update"]
    },
    "shopId": "cmVhY3Rpb24vc2hvcDp5YjRTOG1KQ1hmQzJweEFrTg=="
  }
}

(Use actual groupId, such as group._id returned by createAccountGroup.)

Get a group with permissions

{
  group(id: "cmVhY3Rpb24vZ3JvdXA6bVlvWnM4eFk5emFRYldlY2M=") {
    _id
    description
    permissions
    name
    slug
    createdAt
    createdBy {
      primaryEmailAddress
    }
    updatedAt
    shop {
      _id
      name
    }
  }
}

Get shop roles

An account in shop manager group should be able to do this query for the shop:

{
  primaryShop {
    roles {
      nodes {
        name
      }
    }
  }
}

And verify the correct roles list comes back.

Get shop roles

An account in shop manager group should be able to do this query for the shop:

{
  roles(shopId: "cmVhY3Rpb24vc2hvcDo5eUhFODd3cUpKRzluSE1QWg==") {
    nodes {
      name
    }
  }
}

And verify the correct roles list comes back.

Also cleanup of createAccountGroup and updateAccountGroup mutations

Signed-off-by: Eric Dobbertin <[email protected]>
@aldeed aldeed force-pushed the fix-aldeed-remove-ensure-roles branch from 91f5b14 to 4cac771 Compare February 26, 2020 01:30
rosshadden
rosshadden previously approved these changes Feb 28, 2020
Signed-off-by: Eric Dobbertin <[email protected]>
Copy link
Member

@kieckhafer kieckhafer left a comment

Choose a reason for hiding this comment

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

Added [email protected].

Works as expected with that release.

@kieckhafer kieckhafer merged commit 31cd565 into trunk Feb 28, 2020
@kieckhafer kieckhafer deleted the fix-aldeed-remove-ensure-roles branch February 28, 2020 23:30
@kieckhafer kieckhafer mentioned this pull request Mar 5, 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.

3 participants