poc: Use share manager for space membership by rhafer · Pull Request #2492 · opencloud-eu/opencloud

rhafer · 2026-03-17T15:29:09Z

This is a proof of concept implementation for #2319. (Persisting Space-Membership in the share-manager)

Requires an adjustment to the CS3 API. Basically just an addtional sharefilter-type: rhafer/cs3apis@6333ce5

The required reva changes are here: https://github.com/rhafer/reva/tree/spaces-in-sharemanager. The rest is mainly accomplished by settting use_common_space_root_share_logic to true. If we decide to move forward with this I'd suggest to remove that option and make the behavior the default. Should get us rid of a bit more code in reva.

As you can see, the changes simplifies the sharing code in the graph service quite a bit as it removes quite a few of the "is this a space root" special cases.

rhafer · 2026-03-17T15:31:13Z

cc @aduffeck @butonic @micbar

aduffeck

This looks really nice as a first step.

I suppose in the future we might want to retrieve the list of shared spaces from the space manager too instead of relying on the grants using ListStorageSpaces (analogous to what we do with "regular" shares), but that's something we can discuss later.

aduffeck · 2026-03-18T07:44:52Z

 					"ocmproviderauthorizersvc":      cfg.OCMEndpoint,
 					"ocmcoresvc":                    cfg.OCMEndpoint,
-					"commit_share_to_storage_grant": cfg.CommitShareToStorageGrant,
+					"use_common_space_root_share_logic": true,


Is this actually being used? I didn't find a reference to it in reva.

It's used in the gateway:

https://github.com/opencloud-eu/reva/blob/main/internal/grpc/services/gateway/gateway.go#L79

We should probably remove that config switch, if we decide to switch to the share-manager.

Ah, I thought it was a new flag being introduced with your change. Thanks.

rhafer · 2026-03-26T12:36:44Z

Update, I've addressed many of the failing tests. However there is a few things I can't really fix myself:

Some of the test still fail because they assume a specific format of the permissionId on the SpaceRoots. Up to now for space roots that format was something like u:<grantee-userid> or g:<grantee-groupid> that was however just a crutch and has change now. The permissionId should always be treated as opaque Id. I already fixed that in a lot of places, but the remaining places exceed my php skills
The more difficult part is probably all the failing tests that still use the ocs endpoints. Before diving into fixing ocs we should probably finally decide how to move forward with ocs for sharing (especially for managing space membership). Maybe it is about time to just let is rest in peace. (@micbar @aduffeck @dragotin opinions?) AFAIK we don't have any client that is still using ocs for managing space memberships.

rhafer · 2026-04-09T13:54:54Z

2. The more difficult part is probably all the failing tests that still use the `ocs` endpoints.

The ocs tests are green now. The ocs API is now able to handle the space memberships when they are stored in the ShareProvider.

rhafer · 2026-04-09T14:28:38Z

The tests that are still failing are:

apiSharingNg1/removeAccessToDrive.feature:132
apiSharingNg1/removeAccessToDrive.feature:133
apiSharingNg1/removeAccessToDrive.feature:134
apiSharingNg1/removeAccessToDrive.feature:137
apiSharingNg1/removeAccessToDrive.feature:145
apiSharingNg1/removeAccessToDriveItem.feature:123
apiSharingNg1/removeAccessToDriveItem.feature:124
apiSharingNg1/removeAccessToDriveItem.feature:125
apiSharingNg1/removeAccessToDriveItem.feature:143
apiSharingNg1/removeAccessToDriveItem.feature:144
apiSharingNg1/removeAccessToDriveItem.feature:145

As already mentioned, all of them are failing because the test suite is making wrong assumptions about the format of the permissionid for permissions on Spaces. With this change the permissionId for a space membership does not longer have the format u:<userid> or g:<groupid>. The IDs should just be treated as opaque ids (similar to permissions on folder and files).

So to remove the access of a specific user from a space. One needs to first list the permissions on that space and then pick the permission id of the permission that belongs to the user.

@ScharfViktor would be cool if you could take a look into this. This is a bit above my PHP level. For many of the other failures I've worked around this by just recording the id using shareNgAddToCreatedUserGroupShares maybe that could work here as well, I am unsure.

ScharfViktor · 2026-04-13T11:15:51Z

apiSharingNg1/removeAccessToDrive.feature:137
apiSharingNg1/removeAccessToDrive.feature:145

still failed because user can remove own access even he is the last space manager

Scenario: user cannot remove himself from the project space if he is the last manager using root endpoint # /Users/v.scharf/Work/opencloud/opencloud/tests/acceptance/features/apiSharingNg1/removeAccessToDrive.feature:137
    Given the administrator has assigned the role "Space Admin" to user "Alice" using the Graph API         # GraphContext::theAdministratorHasGivenTheRoleUsingTheGraphApi()
    And user "Alice" has created a space "NewSpace" with the default quota using the Graph API              # SpacesContext::theUserHasCreatedASpaceByDefaultUsingTheGraphApi()
    When user "Alice" tries to remove own access from space "NewSpace" using root endpoint of the Graph API # SharingNgContext::userRemovesOwnAccessFromSpaceUsingGraphAPI()
    Then the HTTP status code should be "403"                                                               # FeatureContext::thenTheHTTPStatusCodeShouldBe()
      HTTP status code 204 is not the expected value 403
      Failed asserting that 204 matches expected '403'.
    And the user "Alice" should have a space called "NewSpace"

…sponse

This gets us rid of quite a bit of special casing for space permission. Also provides us with "real" permission IDs instead of those faked "u:<userid>" ones.

The `id` property of the `permissions` on a space root does not longer have that special `u:<userid>` format any. It now has the same format as the permission id on "normal" driveItems.

rhafer · 2026-04-13T15:39:46Z

-			default => throw new Exception("shareType '$shareType' does not match user|group|link "),
-		};
+		// if recipient is not provided, it means user tries to remove own access, then we need to get the user permission id
+		if (!isset($recipient)) {


@ScharfViktor This breaks userRemovesLinkFromSpaceUsingRootEndpointOfGraphAPI() I think.
It calls $this->removeAccessToSpace($sharer, 'link', $space) (so $recipient is unset). But it would need $permissionID be set to $this->featureContext->shareNgGetLastCreatedLinkShareID() because of $shareType='link'

rhafer · 2026-04-13T15:54:25Z

apiSharingNg1/removeAccessToDrive.feature:137
apiSharingNg1/removeAccessToDrive.feature:145

still failed because user can remove own access even he is the last space manager

Scenario: user cannot remove himself from the project space if he is the last manager using root endpoint # /Users/v.scharf/Work/opencloud/opencloud/tests/acceptance/features/apiSharingNg1/removeAccessToDrive.feature:137
    Given the administrator has assigned the role "Space Admin" to user "Alice" using the Graph API         # GraphContext::theAdministratorHasGivenTheRoleUsingTheGraphApi()
    And user "Alice" has created a space "NewSpace" with the default quota using the Graph API              # SpacesContext::theUserHasCreatedASpaceByDefaultUsingTheGraphApi()
    When user "Alice" tries to remove own access from space "NewSpace" using root endpoint of the Graph API # SharingNgContext::userRemovesOwnAccessFromSpaceUsingGraphAPI()
    Then the HTTP status code should be "403"                                                               # FeatureContext::thenTheHTTPStatusCodeShouldBe()
      HTTP status code 204 is not the expected value 403
      Failed asserting that 204 matches expected '403'.
    And the user "Alice" should have a space called "NewSpace"

I've bumped reva to opencloud-eu/reva@90717df that should have fixed it.

ScharfViktor · 2026-04-13T16:40:40Z

I've bumped reva to opencloud-eu/reva@90717df that should have fixed it.

I can confirm, that it fixed

We now report a quota of -3 for unlimited quota instead of 0, which clients interpreted as a quota of 0.

sonarqubecloud · 2026-04-14T06:11:43Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

rhafer changed the title ~~poc: User share manager for space membershop~~ poc: User share manager for space membership Mar 17, 2026

rhafer mentioned this pull request Mar 17, 2026

[research/poc] Use share-manager for managing space memberships #2319

Open

1 task

aduffeck reviewed Mar 18, 2026

View reviewed changes

rhafer force-pushed the share-manager-for-spaces branch 3 times, most recently from 1460b5f to b0c5b1a Compare March 26, 2026 12:26

rhafer force-pushed the share-manager-for-spaces branch from b0c5b1a to aec01d9 Compare April 8, 2026 14:40

rhafer changed the title ~~poc: User share manager for space membership~~ poc: Use share manager for space membership Apr 8, 2026

rhafer force-pushed the share-manager-for-spaces branch 2 times, most recently from 82a3c5a to cdebaae Compare April 9, 2026 12:50

rhafer force-pushed the share-manager-for-spaces branch from cdebaae to bfcd472 Compare April 9, 2026 14:09

ScharfViktor self-assigned this Apr 11, 2026

rhafer added 3 commits April 13, 2026 15:35

Use filter to avoid including space membership in the sharedWithMe re…

de4b2cf

…sponse

graph: use share manager for managing space permissions

8a9c121

This gets us rid of quite a bit of special casing for space permission. Also provides us with "real" permission IDs instead of those faked "u:<userid>" ones.

tests: Adjust acceptance test for recent Space sharing changes

6df6c2e

The `id` property of the `permissions` on a space root does not longer have that special `u:<userid>` format any. It now has the same format as the permission id on "normal" driveItems.

rhafer commented Apr 13, 2026

View reviewed changes

rhafer and others added 2 commits April 13, 2026 17:45

bump reva to personal fork

dd94e6b

adjust tests

1f91075

rhafer force-pushed the share-manager-for-spaces branch from 64bb3f5 to 1f91075 Compare April 13, 2026 15:46

adjust test

804fc8d

Adjust quota test to fixed behavior

a01121b

We now report a quota of -3 for unlimited quota instead of 0, which clients interpreted as a quota of 0.

rhafer mentioned this pull request Apr 14, 2026

Implement a migration for importing Space Memberships into Share Manager #2612

Open

5 tasks

Conversation

rhafer commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rhafer commented Mar 17, 2026

Uh oh!

aduffeck left a comment

Choose a reason for hiding this comment

Uh oh!

aduffeck Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

rhafer Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

aduffeck Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

rhafer commented Mar 26, 2026

Uh oh!

rhafer commented Apr 9, 2026

Uh oh!

rhafer commented Apr 9, 2026

Uh oh!

ScharfViktor commented Apr 13, 2026

Uh oh!

rhafer Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

rhafer commented Apr 13, 2026

Uh oh!

ScharfViktor commented Apr 13, 2026

Uh oh!

sonarqubecloud Bot commented Apr 14, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rhafer commented Mar 17, 2026 •

edited

Loading