Repository: owncloud/ocis
Pull request #9920 (2024-08-26 14:46:18)

Ocm listing fixes
The libre graph API now returns an etag, the role and the creation time for ocm shares.

related https://github.com/owncloud/ocis/issues/9735


Patch:
diff --git a/changelog/unreleased/bump-reva.md b/changelog/unreleased/bump-reva.md
index eb8a0315381..70745801d02 100644
--- a/changelog/unreleased/bump-reva.md
+++ b/changelog/unreleased/bump-reva.md
@@ -2,5 +2,6 @@ Enhancement: Bump reva
 
 Bumps reva version
 
+https://github.com/owncloud/ocis/pull/9920
 https://github.com/owncloud/ocis/pull/9879
 https://github.com/owncloud/ocis/pull/9860
diff --git a/changelog/unreleased/ocm-listing-fixes.md b/changelog/unreleased/ocm-listing-fixes.md
new file mode 100644
index 00000000000..ffff55eab6c
--- /dev/null
+++ b/changelog/unreleased/ocm-listing-fixes.md
@@ -0,0 +1,5 @@
+Bugfix: add missing properties to when listing ocm shares 
+
+The libre graph API now returns an etag, the role and the creation time for ocm shares.
+
+https://github.com/owncloud/ocis/pull/9920
diff --git a/go.mod b/go.mod
index 8e745fb65b6..cb404a5d216 100644
--- a/go.mod
+++ b/go.mod
@@ -15,7 +15,7 @@ require (
 	github.com/cenkalti/backoff v2.2.1+incompatible
 	github.com/coreos/go-oidc/v3 v3.11.0
 	github.com/cs3org/go-cs3apis v0.0.0-20240724121416-062c4e3046cb
-	github.com/cs3org/reva/v2 v2.23.1-0.20240823142954-51e6e33750e7
+	github.com/cs3org/reva/v2 v2.23.1-0.20240826144102-af5123b523cf
 	github.com/dhowden/tag v0.0.0-20230630033851-978a0926ee25
 	github.com/dutchcoders/go-clamd v0.0.0-20170520113014-b970184f4d9e
 	github.com/egirna/icap-client v0.1.1
diff --git a/go.sum b/go.sum
index 80e745ce62b..19e0ac46f31 100644
--- a/go.sum
+++ b/go.sum
@@ -255,8 +255,8 @@ github.com/crewjam/saml v0.4.14 h1:g9FBNx62osKusnFzs3QTN5L9CVA/Egfgm+stJShzw/c=
 github.com/crewjam/saml v0.4.14/go.mod h1:UVSZCf18jJkk6GpWNVqcyQJMD5HsRugBPf4I1nl2mME=
 github.com/cs3org/go-cs3apis v0.0.0-20240724121416-062c4e3046cb h1:KmYZDReplv/yfwc1LNYpDcVhVujC3Pasv6WjXx1haSU=
 github.com/cs3org/go-cs3apis v0.0.0-20240724121416-062c4e3046cb/go.mod h1:yyP8PRo0EZou3nSH7H4qjlzQwaydPeIRNgX50npQHpE=
-github.com/cs3org/reva/v2 v2.23.1-0.20240823142954-51e6e33750e7 h1:q5U8sebSA3VqeLuf8Xhg1bVRxc8oJuRjQCjkl8xQPaI=
-github.com/cs3org/reva/v2 v2.23.1-0.20240823142954-51e6e33750e7/go.mod h1:p7CHBXcg6sSqB+0JMNDfC1S7TSh9FghXkw1kTV3KcJI=
+github.com/cs3org/reva/v2 v2.23.1-0.20240826144102-af5123b523cf h1:VMg9uATNCBjJhU0dJJ5wgchLCCkZr/6IxjJxY+8hAAs=
+github.com/cs3org/reva/v2 v2.23.1-0.20240826144102-af5123b523cf/go.mod h1:p7CHBXcg6sSqB+0JMNDfC1S7TSh9FghXkw1kTV3KcJI=
 github.com/cyberdelia/templates v0.0.0-20141128023046-ca7fffd4298c/go.mod h1:GyV+0YP4qX0UQ7r2MoYZ+AvYDp12OF5yg4q8rGnyNh4=
 github.com/cyphar/filepath-securejoin v0.2.4 h1:Ugdm7cg7i6ZK6x3xDF1oEu1nfkyfH53EtKeQYTC3kyg=
 github.com/cyphar/filepath-securejoin v0.2.4/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4=
diff --git a/services/graph/pkg/service/v0/api_driveitem_permissions.go b/services/graph/pkg/service/v0/api_driveitem_permissions.go
index 292a098b516..591d72948cc 100644
--- a/services/graph/pkg/service/v0/api_driveitem_permissions.go
+++ b/services/graph/pkg/service/v0/api_driveitem_permissions.go
@@ -124,7 +124,7 @@ func (s DriveItemPermissionsService) Invite(ctx context.Context, resourceId *sto
 	cs3ResourcePermissions := unifiedrole.PermissionsToCS3ResourcePermissions(unifiedRolePermissions)
 
 	permission := &libregraph.Permission{}
-	if role := unifiedrole.CS3ResourcePermissionsToUnifiedRole(cs3ResourcePermissions, condition); role != nil {
+	if role := unifiedrole.CS3ResourcePermissionsToUnifiedRole(cs3ResourcePermissions, condition, false); role != nil {
 		permission.Roles = []string{role.GetId()}
 	}
 
diff --git a/services/graph/pkg/service/v0/base.go b/services/graph/pkg/service/v0/base.go
index 6c8a58ecd6f..534cf0321d6 100644
--- a/services/graph/pkg/service/v0/base.go
+++ b/services/graph/pkg/service/v0/base.go
@@ -191,7 +191,7 @@ func (g BaseGraphService) cs3SpacePermissionsToLibreGraph(ctx context.Context, s
 			p.SetExpirationDateTime(time.Unix(int64(exp.GetSeconds()), int64(exp.GetNanos())))
 		}
 
-		if role := unifiedrole.CS3ResourcePermissionsToUnifiedRole(perm, unifiedrole.UnifiedRoleConditionDrive); role != nil {
+		if role := unifiedrole.CS3ResourcePermissionsToUnifiedRole(perm, unifiedrole.UnifiedRoleConditionDrive, false); role != nil {
 			switch apiVersion {
 			case APIVersion_1:
 				if r := unifiedrole.GetLegacyName(*role); r != "" {
@@ -459,6 +459,7 @@ func (g BaseGraphService) cs3UserShareToPermission(ctx context.Context, share *c
 	role := unifiedrole.CS3ResourcePermissionsToUnifiedRole(
 		share.GetPermissions().GetPermissions(),
 		roleCondition,
+		false,
 	)
 	if role != nil {
 		perm.SetRoles([]string{role.GetId()})
@@ -542,6 +543,7 @@ func (g BaseGraphService) cs3OCMShareToPermission(ctx context.Context, share *oc
 	role := unifiedrole.CS3ResourcePermissionsToUnifiedRole(
 		permissions,
 		roleCondition,
+		true,
 	)
 	if role != nil {
 		perm.SetRoles([]string{role.GetId()})
diff --git a/services/graph/pkg/service/v0/utils.go b/services/graph/pkg/service/v0/utils.go
index d08a05567c4..c8a45a85e28 100644
--- a/services/graph/pkg/service/v0/utils.go
+++ b/services/graph/pkg/service/v0/utils.go
@@ -445,7 +445,7 @@ func cs3ReceivedShareToLibreGraphPermissions(ctx context.Context, logger *log.Lo
 		if err != nil {
 			return nil, err
 		}
-		role := unifiedrole.CS3ResourcePermissionsToUnifiedRole(permissionSet, condition)
+		role := unifiedrole.CS3ResourcePermissionsToUnifiedRole(permissionSet, condition, false)
 
 		if role != nil {
 			permission.SetRoles([]string{role.GetId()})
@@ -491,6 +491,17 @@ func roleConditionForResourceType(ri *storageprovider.ResourceInfo) (string, err
 	}
 }
 
+func federatedRoleConditionForResourceType(ri *storageprovider.ResourceInfo) (string, error) {
+	switch {
+	case ri.Type == storageprovider.ResourceType_RESOURCE_TYPE_CONTAINER:
+		return unifiedrole.UnifiedRoleConditionFolderFederatedUser, nil
+	case ri.Type == storageprovider.ResourceType_RESOURCE_TYPE_FILE:
+		return unifiedrole.UnifiedRoleConditionFileFederatedUser, nil
+	default:
+		return "", errorcode.New(errorcode.InvalidRequest, "unsupported resource type for federated role")
+	}
+}
+
 // ExtractShareIdFromResourceId is a bit of a hack.
 // We should not rely on a specific format of the item id.
 // But currently there is no other way to get the ShareID.
@@ -764,36 +775,43 @@ func fillDriveItemPropertiesFromReceivedOCMShare(ctx context.Context, logger *lo
 
 func cs3ReceivedOCMShareToLibreGraphPermissions(ctx context.Context, logger *log.Logger,
 	identityCache identity.IdentityCache, receivedShare *ocm.ReceivedShare,
-	_ *storageprovider.ResourceInfo) (*libregraph.Permission, error) {
+	resourceInfo *storageprovider.ResourceInfo) (*libregraph.Permission, error) {
 	permission := libregraph.NewPermission()
 	if id := receivedShare.GetId().GetOpaqueId(); id != "" {
 		permission.SetId(id)
 	}
 
+	if cTime := receivedShare.GetCtime(); cTime != nil {
+		permission.SetCreatedDateTime(cs3TimestampToTime(cTime))
+	}
+
 	if expiration := receivedShare.GetExpiration(); expiration != nil {
 		permission.SetExpirationDateTime(cs3TimestampToTime(expiration))
 	}
 
-	/*
-		if permissionSet := receivedShare.GetShare().GetPermissions().GetPermissions(); permissionSet != nil {
-			condition, err := roleConditionForResourceType(resourceInfo)
-			if err != nil {
-				return nil, err
-			}
-			role := unifiedrole.CS3ResourcePermissionsToUnifiedRole(*permissionSet, condition)
-
-			if role != nil {
-				permission.SetRoles([]string{role.GetId()})
-			}
-
-			actions := unifiedrole.CS3ResourcePermissionsToLibregraphActions(*permissionSet)
-
-			// actions only make sense if no role is set
-			if role == nil && len(actions) > 0 {
-				permission.SetLibreGraphPermissionsActions(actions)
-			}
+	var permissions *storageprovider.ResourcePermissions
+	for _, protocol := range receivedShare.GetProtocols() {
+		if protocol.GetWebdavOptions().GetPermissions() != nil {
+			permissions = protocol.GetWebdavOptions().GetPermissions().GetPermissions()
 		}
-	*/
+	}
+	condition, err := federatedRoleConditionForResourceType(resourceInfo)
+	if err != nil {
+		return nil, err
+	}
+	role := unifiedrole.CS3ResourcePermissionsToUnifiedRole(
+		permissions,
+		condition,
+		true,
+	)
+	if role != nil {
+		permission.SetRoles([]string{role.GetId()})
+	} else {
+		actions := unifiedrole.CS3ResourcePermissionsToLibregraphActions(permissions)
+		permission.SetLibreGraphPermissionsActions(actions)
+		permission.SetRoles(nil)
+	}
+
 	switch grantee := receivedShare.GetGrantee(); {
 	case grantee.GetType() == storageprovider.GranteeType_GRANTEE_TYPE_USER:
 		user, err := cs3UserIdToIdentity(ctx, identityCache, grantee.GetUserId())
diff --git a/services/graph/pkg/unifiedrole/unifiedrole.go b/services/graph/pkg/unifiedrole/unifiedrole.go
index cd40dcd18d6..5f33950409d 100644
--- a/services/graph/pkg/unifiedrole/unifiedrole.go
+++ b/services/graph/pkg/unifiedrole/unifiedrole.go
@@ -527,7 +527,7 @@ func GetLegacyName(role libregraph.UnifiedRoleDefinition) string {
 
 // CS3ResourcePermissionsToUnifiedRole tries to find the UnifiedRoleDefinition that matches the supplied
 // CS3 ResourcePermissions and constraints.
-func CS3ResourcePermissionsToUnifiedRole(p *provider.ResourcePermissions, constraints string) *libregraph.UnifiedRoleDefinition {
+func CS3ResourcePermissionsToUnifiedRole(p *provider.ResourcePermissions, constraints string, listFederatedRoles bool) *libregraph.UnifiedRoleDefinition {
 	actionSet := map[string]struct{}{}
 	for _, action := range CS3ResourcePermissionsToLibregraphActions(p) {
 		actionSet[action] = struct{}{}
@@ -535,20 +535,27 @@ func CS3ResourcePermissionsToUnifiedRole(p *provider.ResourcePermissions, constr
 
 	var res *libregraph.UnifiedRoleDefinition
 	for _, uRole := range GetBuiltinRoleDefinitionList() {
-		matchFound := false
-		for _, uPerm := range uRole.GetRolePermissions() {
-			if uPerm.GetCondition() != constraints {
-				// the requested constraints don't match, this isn't our role
+		definitionMatch := false
+
+		for _, permission := range uRole.GetRolePermissions() {
+			// this is a dirty comparison because we are not really parsing the SDDL, but as long as we && the conditions we are good
+			isFederatedRole := strings.Contains(permission.GetCondition(), UnifiedRoleConditionFederatedUser)
+			switch {
+			case !strings.Contains(permission.GetCondition(), constraints):
+				continue
+			case listFederatedRoles && !isFederatedRole:
+				continue
+			case !listFederatedRoles && isFederatedRole:
 				continue
 			}
 
 			// if the actions converted from the ResourcePermissions equal the action the defined for the role, we have match
-			if resourceActionsEqual(actionSet, uPerm.GetAllowedResourceActions()) {
-				matchFound = true
+			if resourceActionsEqual(actionSet, permission.GetAllowedResourceActions()) {
+				definitionMatch = true
 				break
 			}
 		}
-		if matchFound {
+		if definitionMatch {
 			res = uRole
 			break
 		}
diff --git a/vendor/github.com/cs3org/reva/v2/pkg/ocm/storage/received/ocm.go b/vendor/github.com/cs3org/reva/v2/pkg/ocm/storage/received/ocm.go
index ae7147a69ef..0b1fea976db 100644
--- a/vendor/github.com/cs3org/reva/v2/pkg/ocm/storage/received/ocm.go
+++ b/vendor/github.com/cs3org/reva/v2/pkg/ocm/storage/received/ocm.go
@@ -285,6 +285,7 @@ func convertStatToResourceInfo(ref *provider.Reference, f fs.FileInfo, share *oc
 		Mtime: &typepb.Timestamp{
 			Seconds: uint64(f.ModTime().Unix()),
 		},
+		Etag:          webdavFile.ETag(),
 		Owner:         share.Creator,
 		PermissionSet: webdavProtocol.Permissions.Permissions,
 		Checksum: &provider.ResourceChecksum{
diff --git a/vendor/modules.txt b/vendor/modules.txt
index f991003070a..b2bd3589e42 100644
--- a/vendor/modules.txt
+++ b/vendor/modules.txt
@@ -367,7 +367,7 @@ github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1
 github.com/cs3org/go-cs3apis/cs3/storage/registry/v1beta1
 github.com/cs3org/go-cs3apis/cs3/tx/v1beta1
 github.com/cs3org/go-cs3apis/cs3/types/v1beta1
-# github.com/cs3org/reva/v2 v2.23.1-0.20240823142954-51e6e33750e7
+# github.com/cs3org/reva/v2 v2.23.1-0.20240826144102-af5123b523cf
 ## explicit; go 1.21
 github.com/cs3org/reva/v2/cmd/revad/internal/grace
 github.com/cs3org/reva/v2/cmd/revad/runtime


Test patch:
diff --git a/services/graph/pkg/unifiedrole/unifiedrole_test.go b/services/graph/pkg/unifiedrole/unifiedrole_test.go
index 145dd66c627..d43898f7321 100644
--- a/services/graph/pkg/unifiedrole/unifiedrole_test.go
+++ b/services/graph/pkg/unifiedrole/unifiedrole_test.go
@@ -19,7 +19,7 @@ var _ = Describe("unifiedroles", func() {
 		func(legacyRole *rConversions.Role, unifiedRole *libregraph.UnifiedRoleDefinition, constraints string) {
 			cs3perm := legacyRole.CS3ResourcePermissions()
 
-			r := unifiedrole.CS3ResourcePermissionsToUnifiedRole(cs3perm, constraints)
+			r := unifiedrole.CS3ResourcePermissionsToUnifiedRole(cs3perm, constraints, false)
 			Expect(r.GetId()).To(Equal(unifiedRole.GetId()))
 
 		},