fix: share jail usage for the listSharedWithMe endpoint

[fschade]

Description

The graph beta ListSharedWithMe endpoint was rebuilt and now uses the share jail.
since the output highly relies on the share state, e.g.

• SHARE_STATE_ACCEPTED
• SHARE_STATE_PENDING
• SHARE_STATE_REJECTED

the implementation looks complicated at first glance which is due to the different states.
The code is intended to serve as a basis for discussion and primarily show whether the right thing is happening here, since the cs3 stat does not return jailed references.

@butonic, If you find time, please take a look at the code to see if it does the right thing.
@individual-it, In the meantime, you can test with the php sdk to see if everything works.

the unit tests will fail at the moment and need to be updated, same for the libregraph api clients (due to missing remoteItem permissions)

Related Issue

• fixes [sharing-ng] sharedWithMe endpoint shows : and not ! and $ as separator in id #8000
• needed for [QA][Sharing NG] ApiTest: list sharedWithMe #7826
• part of Sharing NG #6993
• needs enhancement: extend remoteItem to contain permissions libre-graph-api#153

Motivation and Context

The previous implementation of the graph beta ListSharedWithMe had not used the required share jail

How Has This Been Tested?

• local installation
• ci test suite

Output

{
  "value": [
    {
      "createdBy": {
        "user": {
          "displayName": "Admin",
          "id": "33c4dae1-084a-4b1a-af16-62dd499a43db"
        }
      },
      "id": "82a26d6a-acc2-4074-92ec-4f6d9d0c8e03$33c4dae1-084a-4b1a-af16-62dd499a43db!db7c807c-55f2-48a6-98be-580d695c5921",
      "lastModifiedDateTime": "2023-12-18T16:54:09.932155608+01:00",
      "name": "REJECTED",
      "parentReference": {
        "driveId": "82a26d6a-acc2-4074-92ec-4f6d9d0c8e03$33c4dae1-084a-4b1a-af16-62dd499a43db!33c4dae1-084a-4b1a-af16-62dd499a43db",
        "driveType": "personal"
      },
      "permissions": [
        {
          "@libre.graph.permissions.actions": [
            "libre.graph/driveItem/permissions/create",
            "libre.graph/driveItem/path/read",
            "libre.graph/driveItem/quota/read",
            "libre.graph/driveItem/content/read",
            "libre.graph/driveItem/children/read",
            "libre.graph/driveItem/deleted/read",
            "libre.graph/driveItem/basic/read"
          ],
          "grantedToV2": {
            "user": {
              "displayName": "",
              "id": "f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c"
            }
          },
          "id": "82a26d6a-acc2-4074-92ec-4f6d9d0c8e03:33c4dae1-084a-4b1a-af16-62dd499a43db:7caf033f-bbdc-4705-b18f-d60f4782030f",
          "roles": [
            "b1e2218d-eef8-4d4c-b82d-0f1a1b48f3b5"
          ]
        }
      ]
    },
    {
      "createdBy": {
        "user": {
          "displayName": "Admin",
          "id": "33c4dae1-084a-4b1a-af16-62dd499a43db"
        }
      },
      "createdDateTime": "2023-12-18T16:54:16.068284+01:00",
      "id": "a0ca6a90-a365-4782-871e-d44447bbc668$a0ca6a90-a365-4782-871e-d44447bbc668!82a26d6a-acc2-4074-92ec-4f6d9d0c8e03:33c4dae1-084a-4b1a-af16-62dd499a43db:9a47489b-9824-4694-94f2-17e70183d989",
      "lastModifiedDateTime": "2023-12-18T16:54:16.068284+01:00",
      "name": "ACCEPTED",
      "parentReference": {
        "driveId": "82a26d6a-acc2-4074-92ec-4f6d9d0c8e03$33c4dae1-084a-4b1a-af16-62dd499a43db!33c4dae1-084a-4b1a-af16-62dd499a43db",
        "driveType": "personal"
      },
      "remoteItem": {
        "createdBy": {
          "user": {
            "displayName": "Admin",
            "id": "33c4dae1-084a-4b1a-af16-62dd499a43db"
          }
        },
        "eTag": "44c83f7d54cf706425776424adc8c48c",
        "folder": {},
        "id": "82a26d6a-acc2-4074-92ec-4f6d9d0c8e03$33c4dae1-084a-4b1a-af16-62dd499a43db!0fceb583-b3ac-4fb0-b87e-d683292a377e",
        "lastModifiedDateTime": "2023-12-18T16:53:50.087649795+01:00",
        "name": "ACCEPTED",
        "permissions": [
          {
            "@libre.graph.permissions.actions": [
              "libre.graph/driveItem/permissions/create",
              "libre.graph/driveItem/path/read",
              "libre.graph/driveItem/quota/read",
              "libre.graph/driveItem/content/read",
              "libre.graph/driveItem/children/read",
              "libre.graph/driveItem/deleted/read",
              "libre.graph/driveItem/basic/read"
            ],
            "grantedToV2": {
              "user": {
                "displayName": "",
                "id": "f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c"
              }
            },
            "id": "82a26d6a-acc2-4074-92ec-4f6d9d0c8e03:33c4dae1-084a-4b1a-af16-62dd499a43db:9a47489b-9824-4694-94f2-17e70183d989",
            "roles": [
              "b1e2218d-eef8-4d4c-b82d-0f1a1b48f3b5"
            ]
          }
        ],
        "shared": {
          "owner": {
            "user": {
              "displayName": "Admin",
              "id": "33c4dae1-084a-4b1a-af16-62dd499a43db"
            }
          },
          "sharedBy": {
            "user": {
              "displayName": "Admin",
              "id": "33c4dae1-084a-4b1a-af16-62dd499a43db"
            }
          },
          "sharedDateTime": "2023-12-18T16:54:16.068284+01:00"
        }
      }
    }
  ]
}

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• [] Tests only (no source changes)

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised:

[update-docs]

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.

[rhafer]

I am still a bit unsure about the partent references we're setting. In case of an accepted share we now return this driveItem wrapping a remoteItem (shortened):

{
      "id": "a0ca6a90-a365-4782-871e-d44447bbc668$a0ca6a90-a365-4782-871e-d44447bbc668!82a26d6a-acc2-4074-92ec-4f6d9d0c8e03:33c4dae1-084a-4b1a-af16-62dd499a43db:9a47489b-9824-4694-94f2-17e70183d989",
      "parentReference": {
        "driveId": "82a26d6a-acc2-4074-92ec-4f6d9d0c8e03$33c4dae1-084a-4b1a-af16-62dd499a43db!33c4dae1-084a-4b1a-af16-62dd499a43db",
        "driveType": "personal"
      },
      "remoteItem": {
        "id": "82a26d6a-acc2-4074-92ec-4f6d9d0c8e03$33c4dae1-084a-4b1a-af16-62dd499a43db!0fceb583-b3ac-4fb0-b87e-d683292a377e",
        ...
      },
...
}

• The id of the outer driveItem is basically composed of the storageId and spaceId of the virtual share jail and the share id. (Which is correct I think).
• But what is the correct parentReference for the outer `driveItem? It then example it is bascially the spacedId of the space containing the original resource. Is that correct?
• The inner remoteItem does currently not have parentReference at all. Do we need to set one? If yes, which one?

@micbar or @butonic I think we need your input here.

[individual-it]

Trying to build this locally fails with ../services/graph/pkg/service/v0/sharedwithme.go:239:17: remoteItem.SetPermissions undefined (type *libregraph.RemoteItem has no field or method SetPermissions)

[fschade]

Trying to build this locally fails with ../services/graph/pkg/service/v0/sharedwithme.go:239:17: remoteItem.SetPermissions undefined (type *libregraph.RemoteItem has no field or method SetPermissions)

thanks, please try again, should be fixed now

[individual-it]

The parentReference.driveId does not match what I get from /me/drives I think its too long

Expected :'b23d095d-d752-4b4b-9b3b-eb06f8f1eb28$fac58de5-a5d7-4137-9db8-743d86d4496d'
Actual   :'b23d095d-d752-4b4b-9b3b-eb06f8f1eb28$fac58de5-a5d7-4137-9db8-743d86d4496d!fac58de5-a5d7-4137-9db8-743d86d4496d'

The part after $ is repeated also after the !
Looks like that is the user-id of the sharer

[individual-it]

So to detect if a share is accepted or declined we would have to check if remoteItem exists?

The docs say:
The driveItems returned from the sharedWithMe method always include the remoteItem facet that indicates they are items from a different drive.

So with this PR that is not true anymore.

[fschade]

parentReference.driveId is statResponse.GetInfo().GetSpace().GetRoot()

the remote item is only attached if the share is accepted, e.g. ShareState_SHARE_STATE_ACCEPTED but not for ShareState_SHARE_STATE_PENDING and ShareState_SHARE_STATE_REJECTED

[fschade]

@individual-it, it still misses the @Client.Synchronize property, ill add it tomorrow, can you have a look in the meantime and check if it works for you that way?

[individual-it]

Works generally well. id and etag are not present when the share is not accepted and that is the way we discussed it yesterday 👍

[individual-it]

PR to test this changes in ocis-php-sdk: owncloud/ocis-php-sdk#166

[fschade]

@individual-it, contains @Client.Synchronize now but you need to link following pr to build:
owncloud/libre-graph-api#156

i keep it in the current state now to discussion it on tuesday with @micbar

[fschade]

@individual-it we had another conversation this morning regarding the permission.client.synced property. The conclusion was that if ok for you we skip adding that and use the drive id instead:

• id: mounted
• no id: not mounted

can you please check if thats ok for you and the sdk youre working on?

[fschade]

i leave this pr in draft state for today to have room for discussion since the topic seems to be controversial.
With the current status we save a stat and permission identities now contain a displayName.

If a drive item is shared to the same grantee, first via group and second directly to the user identity, the share aka driveItem does not get normalized and appears multiple times instead.

in a future pr we should only list one drive item and multiple permission entries instead. FollowUp PR needed.

At the moment the tests are still disabled, as soon as we have agreed on a status I will adapt all tests

FYI: @micbar, @individual-it, @butonic, @rhafer

[rhafer]

• Currently both, the outer driveItem and the wrapped remoteItem, contain a permissions property. Apart from the duplication, I think it's wrong. Only the inner remoteItem should have that property.
• Also I think moving the ui.hidden and @client.synchronize flags from the permission property into the the remoteItem is wrong. Both flags are share specific properties in the CS3 API. If you create a share with a user and that user hides the share and after the same item is share to that user via group membership, the item will be visible again as it is a different share. (We basically need to implement it the same on the libregraph level for permission, as we can't really set a correct ui.hidden value on the remoteitem in cases where multiple shares are received for the same it item.
• When one of the received shares is a share from a project space the API will just return a 404 (it seems the owner of the shared item is not a valid userid but the project space id). -> reported as sharing-ng: Emtpy sharedWithMe response when received shares contain share from project space #8215

{
	"error": {
		"code": "itemNotFound",
		"innererror": {
			"date": "2024-01-10T16:06:33Z",
			"request-id": "23a8b0919805/ArGUHkKdIn-000002"
		},
		"message": "not found"
	}
}

[rhafer]

If a drive item is shared to the same grantee, first via group and second directly to the user identity, the share aka driveItem does not get normalized and appears multiple times instead.

in a future pr we should only list one drive item and multiple permission entries instead. FollowUp PR needed.

How would that work with the hidden and synchronized flag now being part of the remoteItem. These flags can differ between multiple shares received for the same resource. Also, what would be the correct id value for the wrapping driveItem. IIRC that id is derived from the shareId/permissionId.

[micbar]

How would that work with the hidden and synchronized flag now being part of the remoteItem.

The hidden and synchronized flag need to be part of the permission.

[rhafer]

I am still a bit unsure about the partent references we're setting.

• But what is the correct parentReference for the outer `driveItem? It then example it is bascially the spacedId of the space containing the original resource. Is that correct?

According to #6993 (comment) the parentReference in the outer driveItem should point to the id of the drive containing the mountpoint. To my understand that means it should basically be the id of the share jail.

• The inner remoteItem does currently not have parentReference at all. Do we need to set one? If yes, which one?

Also according to the above linked issue, the id of the remoteItems parentReference should NOT be set (because the received is not allowed to see the parent). The driveType and driveId values should be set to the spaceType/spaceId values of the space containing the shared resource.

I'll rework this PR accordingly.

[rhafer]

I am moving this out of draft now. I think it is ready for review. There is still the know bug with shares created from space, but in order to unblock @individual-it from making progress on the php sdk I'd rather handle that in a separate PR.

remaining issue will be handled in a separate PR

[sonarcloud]

Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

4 New issues
0 Security Hotspots
77.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[micbar]

LGTM so far.
Will do some testing after we build an rc.2