From 18037f530e053e3727d1afe6ef1374432b84ec84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Mon, 26 Aug 2024 16:42:59 +0200 Subject: [PATCH 1/4] fix listing ocm properties MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../service/v0/api_driveitem_permissions.go | 2 +- services/graph/pkg/service/v0/base.go | 4 +- services/graph/pkg/service/v0/utils.go | 60 ++++++++++++------- services/graph/pkg/unifiedrole/unifiedrole.go | 32 ++++++---- .../graph/pkg/unifiedrole/unifiedrole_test.go | 2 +- 5 files changed, 64 insertions(+), 36 deletions(-) diff --git a/services/graph/pkg/service/v0/api_driveitem_permissions.go b/services/graph/pkg/service/v0/api_driveitem_permissions.go index 292a098b51..591d72948c 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 6c8a58ecd6..534cf0321d 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 d08a05567c..c8a45a85e2 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 cd40dcd18d..d20d1bd19e 100644 --- a/services/graph/pkg/unifiedrole/unifiedrole.go +++ b/services/graph/pkg/unifiedrole/unifiedrole.go @@ -527,28 +527,36 @@ 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 { - actionSet := map[string]struct{}{} - for _, action := range CS3ResourcePermissionsToLibregraphActions(p) { - actionSet[action] = struct{}{} - } +func CS3ResourcePermissionsToUnifiedRole(p *provider.ResourcePermissions, constraints string, listFederatedRoles bool) *libregraph.UnifiedRoleDefinition { + actions := CS3ResourcePermissionsToLibregraphActions(p) var res *libregraph.UnifiedRoleDefinition for _, uRole := range GetBuiltinRoleDefinitionList() { - matchFound := false + definitionMatch := false for _, uPerm := range uRole.GetRolePermissions() { - if uPerm.GetCondition() != constraints { - // the requested constraints don't match, this isn't our role + + // 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(uPerm.GetCondition(), UnifiedRoleConditionFederatedUser) + switch { + case !strings.Contains(uPerm.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 - break + for i, action := range uPerm.GetAllowedResourceActions() { + if !slices.Contains(actions, action) { + break + } + if i == len(uPerm.GetAllowedResourceActions())-1 { + definitionMatch = true + } } } - if matchFound { + if definitionMatch { res = uRole break } diff --git a/services/graph/pkg/unifiedrole/unifiedrole_test.go b/services/graph/pkg/unifiedrole/unifiedrole_test.go index 145dd66c62..d43898f732 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())) }, From 93c2e33b1e958a7e64db027338f6f3e247887774 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Mon, 26 Aug 2024 16:43:35 +0200 Subject: [PATCH 2/4] bump reva MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- go.mod | 2 +- go.sum | 4 ++-- .../github.com/cs3org/reva/v2/pkg/ocm/storage/received/ocm.go | 1 + vendor/modules.txt | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index 8e745fb65b..cb404a5d21 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 80e745ce62..19e0ac46f3 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/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 ae7147a69e..0b1fea976d 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 f991003070..b2bd3589e4 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 From 6c13e5fc285893b59eba060a54d575252c4892ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Mon, 26 Aug 2024 16:45:23 +0200 Subject: [PATCH 3/4] bump reva & changelog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- changelog/unreleased/bump-reva.md | 1 + changelog/unreleased/ocm-listing-fixes.md | 5 +++++ 2 files changed, 6 insertions(+) create mode 100644 changelog/unreleased/ocm-listing-fixes.md diff --git a/changelog/unreleased/bump-reva.md b/changelog/unreleased/bump-reva.md index eb8a031538..70745801d0 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 0000000000..ffff55eab6 --- /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 From e933299c8d21a6ff80989597a0c77baac03fdc6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Mon, 26 Aug 2024 17:17:51 +0200 Subject: [PATCH 4/4] all actions must be part of the role MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- services/graph/pkg/unifiedrole/unifiedrole.go | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/services/graph/pkg/unifiedrole/unifiedrole.go b/services/graph/pkg/unifiedrole/unifiedrole.go index d20d1bd19e..5f33950409 100644 --- a/services/graph/pkg/unifiedrole/unifiedrole.go +++ b/services/graph/pkg/unifiedrole/unifiedrole.go @@ -528,17 +528,20 @@ 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, listFederatedRoles bool) *libregraph.UnifiedRoleDefinition { - actions := CS3ResourcePermissionsToLibregraphActions(p) + actionSet := map[string]struct{}{} + for _, action := range CS3ResourcePermissionsToLibregraphActions(p) { + actionSet[action] = struct{}{} + } var res *libregraph.UnifiedRoleDefinition for _, uRole := range GetBuiltinRoleDefinitionList() { definitionMatch := false - for _, uPerm := range uRole.GetRolePermissions() { + 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(uPerm.GetCondition(), UnifiedRoleConditionFederatedUser) + isFederatedRole := strings.Contains(permission.GetCondition(), UnifiedRoleConditionFederatedUser) switch { - case !strings.Contains(uPerm.GetCondition(), constraints): + case !strings.Contains(permission.GetCondition(), constraints): continue case listFederatedRoles && !isFederatedRole: continue @@ -547,13 +550,9 @@ func CS3ResourcePermissionsToUnifiedRole(p *provider.ResourcePermissions, constr } // if the actions converted from the ResourcePermissions equal the action the defined for the role, we have match - for i, action := range uPerm.GetAllowedResourceActions() { - if !slices.Contains(actions, action) { - break - } - if i == len(uPerm.GetAllowedResourceActions())-1 { - definitionMatch = true - } + if resourceActionsEqual(actionSet, permission.GetAllowedResourceActions()) { + definitionMatch = true + break } } if definitionMatch {