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

Remove space members #2524

Merged
merged 6 commits into from
Feb 15, 2022
Merged

Remove space members #2524

merged 6 commits into from
Feb 15, 2022

Conversation

C0rby
Copy link
Contributor

@C0rby C0rby commented Feb 9, 2022

  • Removed owners from project spaces
  • Prevent deletion of last space manager
  • Viewers and editors can always be deleted
  • Managers can only be deleted when there will be at least one remaining
  • Fixed a bug that prevented adding new space managers

The delete request looks like this:

curl -k -s -u admin:admin -X DELETE 'https://localhost:9200/ocs/v2.php/apps/files_sharing/api/v1/shares/24387715-32d0
-4a91-8fe9-e4950c2e6c3f?shareWith=richard'

@update-docs
Copy link

update-docs bot commented Feb 9, 2022

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@C0rby C0rby force-pushed the remove-space-members branch 4 times, most recently from db4a1ef to 3562d92 Compare February 11, 2022 11:05
@C0rby C0rby marked this pull request as ready for review February 11, 2022 11:34
Copy link
Contributor

@kobergj kobergj left a comment

Choose a reason for hiding this comment

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

LG just small suggestions

pkg/storage/utils/decomposedfs/spaces.go Outdated Show resolved Hide resolved
pkg/storage/utils/decomposedfs/node/permissions.go Outdated Show resolved Hide resolved
David Christofas added 6 commits February 14, 2022 14:34
The last manager of a space can not be removed.
If the user should be removed then a new manager need to be set.
After that the user can be removed.
The integration tests needed to be updated since I removed the owner from project spaces.
Now the tests don't depend on an owner being set.
@@ -803,16 +806,18 @@ func (n *Node) ReadUserPermissions(ctx context.Context, u *userpb.User) (ap prov
o, err := n.Owner()
if err != nil {
// TODO check if a parent folder has the owner set?
appctx.GetLogger(ctx).Error().Err(err).Interface("node", n).Msg("could not determine owner, returning default permissions")
appctx.GetLogger(ctx).Error().Err(err).Str("node", n.ID).Msg("could not determine owner, returning default permissions")
Copy link
Contributor

Choose a reason for hiding this comment

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

I see what you did there ;-)

Copy link
Contributor

@butonic butonic left a comment

Choose a reason for hiding this comment

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

#nice

@butonic butonic merged commit df1264d into cs3org:edge Feb 15, 2022
dragotin pushed a commit to butonic/reva that referenced this pull request Feb 17, 2022
* remove owner from project spaces

* block removal of last space manager

The last manager of a space can not be removed.
If the user should be removed then a new manager need to be set.
After that the user can be removed.

* check node properties instead of create-space permission

* update integration tests

The integration tests needed to be updated since I removed the owner from project spaces.
Now the tests don't depend on an owner being set.

* add changelog

* add constants for space types and root node id
dragotin added a commit to butonic/reva that referenced this pull request Feb 17, 2022
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