Skip to content

feat: enable basic group crud operations#1189

Draft
keithmanville wants to merge 3 commits intodevfrom
groups-crud
Draft

feat: enable basic group crud operations#1189
keithmanville wants to merge 3 commits intodevfrom
groups-crud

Conversation

@keithmanville
Copy link
Collaborator

This commits adds basic group CRUD operations to the REST API:

  • POST /api/v1/groups/
  • PUT /api/v1/groups/{id}
  • DELETE /api/v1/groups/{id}

By default, only the current user is added to a new group. The group is populated with the built-in plugin parameter types.

The default user group (public) is protected from deletion or modification by any user.

Group deletion removes all members. The constraint for ensuring a user has at least one group is maintained by preventing deletion of the public group that all users are part of.

This commit adds checks for all resource endpoints to ensure users cannot access, modify, or delete resources they don't have the correct permissions for. There are not currently protections in place to prevent users from deleting or renaming groups they are not a member of.

It makes use of the existing UserNotInGroupError when a user attempts to access a resource from a group they do not belong to, and adds the UserPermissionsError for cases where a user is in the group but doesn't have the proper permissions to perform an action.

It adds new tests to verify users can not get or delete resources belonging to a group they do not access to. Modification and permissions checkes remain untested.

Some open questions:

  • should flask_login.current_user not go in repository? (note many repo tests fail because they can't get the current user id, but all client-based tests pass) If so need to refactor by passing user into all repo calls from service layer
  • should deleting a resource twice result in success both times or should it be a not found the second time
  • do we we want to distinguish between not being in a group and read access?
  • if you can be in a group without read can you only have write?

This commits adds basic group CRUD operations to the REST API:
- POST /api/v1/groups/
- PUT /api/v1/groups/{id}
- DELETE /api/v1/groups/{id}

By default, only the current user is added to a new group. The group is populated with the
built-in plugin parameter types.

The default user group (public) is protected from deletion or modification by any user.

Group deletion removes all members. The constraint for ensuring a user has at least one group is
maintained by preventing deletion of the public group that all users are part of.

This commit adds checks for all resource endpoints to ensure users cannot access, modify, or delete
resources they don't have the correct permissions for. There are not currently protections in place
to prevent users from deleting or renaming groups they are not a member of.

It makes use of the existing UserNotInGroupError when a user attempts to access a resource from a
group they do not belong to, and adds the UserPermissionsError for cases where a user is in the
group but doesn't have the proper permissions to perform an action.

It adds new tests to verify users can not get or delete resources belonging to a group they do not
access to. Modification and permissions checkes remain untested.
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.

1 participant