From 1242e483b3a9a592cf920f84a4f02d3fa2f8b04b Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Wed, 4 Sep 2024 16:51:48 +0200 Subject: [PATCH 1/9] improve error handling Signed-off-by: Christian Richter --- services/graph/pkg/service/v0/base.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/services/graph/pkg/service/v0/base.go b/services/graph/pkg/service/v0/base.go index 96cba9b07..26f77c306 100644 --- a/services/graph/pkg/service/v0/base.go +++ b/services/graph/pkg/service/v0/base.go @@ -638,6 +638,10 @@ func (g BaseGraphService) getCS3PublicShareByID(ctx context.Context, permissionI }, }, ) + if err != nil { + return nil, err + } + if err := errorcode.FromCS3Status(getPublicShareResp.GetStatus(), err); err != nil { return nil, err } @@ -661,6 +665,10 @@ func (g BaseGraphService) removePublicShare(ctx context.Context, permissionID st }, }, }) + if err != nil { + return err + } + if err := errorcode.FromCS3Status(removePublicShareResp.GetStatus(), err); err != nil { return err } @@ -685,6 +693,9 @@ func (g BaseGraphService) removeUserShare(ctx context.Context, permissionID stri }, }, }) + if err != nil { + return err + } if err := errorcode.FromCS3Status(removeShareResp.GetStatus(), err); err != nil { return err @@ -714,6 +725,9 @@ func (g BaseGraphService) removeSpacePermission(ctx context.Context, permissionI }, }, }) + if err != nil { + return err + } if err := errorcode.FromCS3Status(removeShareResp.GetStatus(), err); err != nil { return err @@ -747,6 +761,10 @@ func (g BaseGraphService) getCS3UserShareByID(ctx context.Context, permissionID }, }, }) + if err != nil { + return nil, err + } + if err := errorcode.FromCS3Status(getShareResp.GetStatus(), err); err != nil { return nil, err } @@ -897,6 +915,9 @@ func (g BaseGraphService) updateUserShare(ctx context.Context, permissionID stri } updateUserShareResp, err := gatewayClient.UpdateShare(ctx, &cs3UpdateShareReq) + if err != nil { + return nil, err + } if err := errorcode.FromCS3Status(updateUserShareResp.GetStatus(), err); err != nil { return nil, err } From 70a9ce6e7466c67bff50e355d753b16cd13ef4d1 Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Wed, 4 Sep 2024 16:52:29 +0200 Subject: [PATCH 2/9] allow deletion of federated shares Signed-off-by: Christian Richter --- .../service/v0/api_driveitem_permissions.go | 10 ++ services/graph/pkg/service/v0/base.go | 103 ++++++++++++++---- 2 files changed, 94 insertions(+), 19 deletions(-) diff --git a/services/graph/pkg/service/v0/api_driveitem_permissions.go b/services/graph/pkg/service/v0/api_driveitem_permissions.go index 5d35fba3a..abf6b3b56 100644 --- a/services/graph/pkg/service/v0/api_driveitem_permissions.go +++ b/services/graph/pkg/service/v0/api_driveitem_permissions.go @@ -71,6 +71,7 @@ const ( Public User Space + OCM ) // NewDriveItemPermissionsService creates a new DriveItemPermissionsService @@ -463,6 +464,13 @@ func (s DriveItemPermissionsService) DeletePermission(ctx context.Context, itemI } } + if sharedResourceID == nil && s.config.IncludeOCMSharees { + sharedResourceID, err = s.getOCMPermissionResourceID(ctx, permissionID) + if err == nil { + permissionType = OCM + } + } + switch { case err != nil: return err @@ -486,6 +494,8 @@ func (s DriveItemPermissionsService) DeletePermission(ctx context.Context, itemI return s.removePublicShare(ctx, permissionID) case Space: return s.removeSpacePermission(ctx, permissionID, sharedResourceID) + case OCM: + return s.removeOCMPermission(ctx, permissionID) } // This should never be reached diff --git a/services/graph/pkg/service/v0/base.go b/services/graph/pkg/service/v0/base.go index 26f77c306..5e85e7c9c 100644 --- a/services/graph/pkg/service/v0/base.go +++ b/services/graph/pkg/service/v0/base.go @@ -154,17 +154,17 @@ func (g BaseGraphService) cs3SpacePermissionsToLibreGraph(ctx context.Context, s // will have the same id. tmp := id isGroup := false - var identity libregraph.Identity + var cs3Identity libregraph.Identity var err error var p libregraph.Permission if _, ok := groupsMap[id]; ok { - identity, err = groupIdToIdentity(ctx, g.identityCache, tmp) + cs3Identity, err = groupIdToIdentity(ctx, g.identityCache, tmp) if err != nil { g.logger.Warn().Str("groupid", tmp).Msg("Group not found by id") } isGroup = true } else { - identity, err = userIdToIdentity(ctx, g.identityCache, tmp) + cs3Identity, err = userIdToIdentity(ctx, g.identityCache, tmp) if err != nil { g.logger.Warn().Str("userid", tmp).Msg("User not found by id") } @@ -173,17 +173,19 @@ func (g BaseGraphService) cs3SpacePermissionsToLibreGraph(ctx context.Context, s case APIVersion_1: var identitySet libregraph.IdentitySet if isGroup { - identitySet.SetGroup(identity) + identitySet.SetGroup(cs3Identity) } else { - identitySet.SetUser(identity) + identitySet.SetUser(cs3Identity) } + p.SetGrantedToV2(libregraph.SharePointIdentitySet{User: identitySet.User, Group: identitySet.Group}) + // FIXME: needs to be removed p.SetGrantedToIdentities([]libregraph.IdentitySet{identitySet}) case APIVersion_1_Beta_1: var identitySet libregraph.SharePointIdentitySet if isGroup { - identitySet.SetGroup(identity) + identitySet.SetGroup(cs3Identity) } else { - identitySet.SetUser(identity) + identitySet.SetUser(cs3Identity) } p.SetId(identitySetToSpacePermissionID(identitySet)) p.SetGrantedToV2(identitySet) @@ -485,14 +487,14 @@ func (g BaseGraphService) cs3UserShareToPermission(ctx context.Context, share *c } perm.SetGrantedToV2(grantedTo) if share.GetCreator() != nil { - identity, err := cs3UserIdToIdentity(ctx, g.identityCache, share.GetCreator()) + cs3Identity, err := cs3UserIdToIdentity(ctx, g.identityCache, share.GetCreator()) if err != nil { return nil, errorcode.New(errorcode.GeneralException, err.Error()) } perm.SetInvitation( libregraph.SharingInvitation{ InvitedBy: &libregraph.IdentitySet{ - User: &identity, + User: &cs3Identity, }, }, ) @@ -571,14 +573,14 @@ func (g BaseGraphService) cs3OCMShareToPermission(ctx context.Context, share *oc } perm.SetGrantedToV2(grantedTo) if share.GetCreator() != nil { - identity, err := cs3UserIdToIdentity(ctx, g.identityCache, share.GetCreator()) + cs3Identity, err := cs3UserIdToIdentity(ctx, g.identityCache, share.GetCreator()) if err != nil { return nil, errorcode.New(errorcode.GeneralException, err.Error()) } perm.SetInvitation( libregraph.SharingInvitation{ InvitedBy: &libregraph.IdentitySet{ - User: &identity, + User: &cs3Identity, }, }, ) @@ -613,11 +615,11 @@ func (g BaseGraphService) cs3PublicSharesToDriveItems(ctx context.Context, share } func (g BaseGraphService) getLinkPermissionResourceID(ctx context.Context, permissionID string) (*storageprovider.ResourceId, error) { - share, err := g.getCS3PublicShareByID(ctx, permissionID) + cs3Share, err := g.getCS3PublicShareByID(ctx, permissionID) if err != nil { return nil, err } - return share.GetResourceId(), nil + return cs3Share.GetResourceId(), nil } func (g BaseGraphService) getCS3PublicShareByID(ctx context.Context, permissionID string) (*link.PublicShare, error) { @@ -648,6 +650,34 @@ func (g BaseGraphService) getCS3PublicShareByID(ctx context.Context, permissionI return getPublicShareResp.GetShare(), nil } +func (g BaseGraphService) removeOCMPermission(ctx context.Context, permissionID string) error { + gatewayClient, err := g.gatewaySelector.Next() + if err != nil { + g.logger.Debug().Err(err).Msg("selecting gatewaySelector failed") + return err + } + + removePublicShareResp, err := gatewayClient.RemoveOCMShare(ctx, + &ocm.RemoveOCMShareRequest{ + Ref: &ocm.ShareReference{ + Spec: &ocm.ShareReference_Id{ + Id: &ocm.ShareId{ + OpaqueId: permissionID, + }, + }, + }, + }) + if err != nil { + return err + } + + if err := errorcode.FromCS3Status(removePublicShareResp.GetStatus(), err); err != nil { + return err + } + // We need to return an untyped nil here otherwise the error==nil check won't work + return nil +} + func (g BaseGraphService) removePublicShare(ctx context.Context, permissionID string) error { gatewayClient, err := g.gatewaySelector.Next() if err != nil { @@ -736,12 +766,47 @@ func (g BaseGraphService) removeSpacePermission(ctx context.Context, permissionI return nil } -func (g BaseGraphService) getUserPermissionResourceID(ctx context.Context, permissionID string) (*storageprovider.ResourceId, error) { - share, err := g.getCS3UserShareByID(ctx, permissionID) +func (g BaseGraphService) getOCMPermissionResourceID(ctx context.Context, permissionID string) (*storageprovider.ResourceId, error) { + cs3Share, err := g.getCS3OCMShareByID(ctx, permissionID) if err != nil { return nil, err } - return share.GetResourceId(), nil + return cs3Share.GetResourceId(), nil +} + +func (g BaseGraphService) getCS3OCMShareByID(ctx context.Context, permissionID string) (*ocm.Share, error) { + gatewayClient, err := g.gatewaySelector.Next() + if err != nil { + g.logger.Debug().Err(err).Msg("selecting gatewaySelector failed") + return nil, err + } + + getShareResp, err := gatewayClient.GetOCMShare(ctx, + &ocm.GetOCMShareRequest{ + Ref: &ocm.ShareReference{ + Spec: &ocm.ShareReference_Id{ + Id: &ocm.ShareId{ + OpaqueId: permissionID, + }, + }, + }, + }) + if err != nil { + return nil, err + } + + if err := errorcode.FromCS3Status(getShareResp.GetStatus(), err); err != nil { + return nil, err + } + return getShareResp.GetShare(), nil +} + +func (g BaseGraphService) getUserPermissionResourceID(ctx context.Context, permissionID string) (*storageprovider.ResourceId, error) { + cs3Share, err := g.getCS3UserShareByID(ctx, permissionID) + if err != nil { + return nil, err + } + return cs3Share.GetResourceId(), nil } func (g BaseGraphService) getCS3UserShareByID(ctx context.Context, permissionID string) (*collaboration.Share, error) { @@ -806,7 +871,7 @@ func (g BaseGraphService) getPermissionByID(ctx context.Context, permissionID st } case errors.As(err, &errcode) && errcode.GetCode() == errorcode.ItemNotFound: // there is no public link with that id, check if this is a user share - share, err := g.getCS3UserShareByID(ctx, permissionID) + cs3Share, err := g.getCS3UserShareByID(ctx, permissionID) if err != nil { return nil, nil, err } @@ -818,11 +883,11 @@ func (g BaseGraphService) getPermissionByID(ctx context.Context, permissionID st if err != nil { return nil, nil, err } - permission, err := g.cs3UserShareToPermission(ctx, share, condition) + permission, err := g.cs3UserShareToPermission(ctx, cs3Share, condition) if err != nil { return nil, nil, err } - return permission, share.GetResourceId(), nil + return permission, cs3Share.GetResourceId(), nil } return nil, nil, err From e9c6a0a3cd225798bbf54bac7fc7891d812c0b89 Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Thu, 5 Sep 2024 15:49:07 +0200 Subject: [PATCH 3/9] [WIP] Update OCM Shares MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jörn Friedrich Dreyer Signed-off-by: Christian Richter --- .../service/v0/api_driveitem_permissions.go | 24 +++- services/graph/pkg/service/v0/base.go | 128 ++++++++++++++++++ 2 files changed, 148 insertions(+), 4 deletions(-) diff --git a/services/graph/pkg/service/v0/api_driveitem_permissions.go b/services/graph/pkg/service/v0/api_driveitem_permissions.go index abf6b3b56..8510d8608 100644 --- a/services/graph/pkg/service/v0/api_driveitem_permissions.go +++ b/services/graph/pkg/service/v0/api_driveitem_permissions.go @@ -526,7 +526,14 @@ func (s DriveItemPermissionsService) DeleteSpaceRootPermission(ctx context.Conte func (s DriveItemPermissionsService) UpdatePermission(ctx context.Context, itemID *storageprovider.ResourceId, permissionID string, newPermission libregraph.Permission) (libregraph.Permission, error) { oldPermission, sharedResourceID, err := s.getPermissionByID(ctx, permissionID, itemID) if err != nil { - return libregraph.Permission{}, err + if s.config.IncludeOCMSharees { + oldPermission, sharedResourceID, err = s.getOCMPermissionByID(ctx, permissionID, itemID) + if err != nil { + return libregraph.Permission{}, err + } + } else { + return libregraph.Permission{}, err + } } // The resourceID of the shared resource need to match the item ID from the Request Path @@ -547,10 +554,19 @@ func (s DriveItemPermissionsService) UpdatePermission(ctx context.Context, itemI // This is a user share updatedPermission, err := s.updateUserShare(ctx, permissionID, sharedResourceID, &newPermission) - if err != nil { - return libregraph.Permission{}, err + if err == nil { + return *updatedPermission, nil } - return *updatedPermission, nil + + // This is an ocm share + if s.config.IncludeOCMSharees { + updatePermission, err := s.updateOCMPermission(ctx, permissionID, itemID, &newPermission) + if err == nil { + return *updatePermission, err + } + } + return libregraph.Permission{}, err + } // UpdateSpaceRootPermission updates a permission on the root item of a project space diff --git a/services/graph/pkg/service/v0/base.go b/services/graph/pkg/service/v0/base.go index 5e85e7c9c..bf860345a 100644 --- a/services/graph/pkg/service/v0/base.go +++ b/services/graph/pkg/service/v0/base.go @@ -836,6 +836,31 @@ func (g BaseGraphService) getCS3UserShareByID(ctx context.Context, permissionID return getShareResp.GetShare(), nil } +func (g BaseGraphService) getOCMPermissionByID(ctx context.Context, permissionID string, itemID *storageprovider.ResourceId) (*libregraph.Permission, *storageprovider.ResourceId, error) { + gatewayClient, err := g.gatewaySelector.Next() + if err != nil { + g.logger.Debug().Err(err).Msg("selecting gatewaySelevtor failed") + return nil, nil, err + } + ocmShare, err := g.getCS3OCMShareByID(ctx, permissionID) + if err != nil { + return nil, nil, err + } + resourceInfo, err := utils.GetResourceByID(ctx, itemID, gatewayClient) + if err != nil { + return nil, nil, err + } + condition, err := roleConditionForResourceType(resourceInfo) + if err != nil { + return nil, nil, err + } + permission, err := g.cs3OCMShareToPermission(ctx, ocmShare, condition) + if err != nil { + return nil, nil, err + } + return permission, ocmShare.GetResourceId(), nil +} + func (g BaseGraphService) getPermissionByID(ctx context.Context, permissionID string, itemID *storageprovider.ResourceId) (*libregraph.Permission, *storageprovider.ResourceId, error) { var errcode errorcode.Error gatewayClient, err := g.gatewaySelector.Next() @@ -893,6 +918,109 @@ func (g BaseGraphService) getPermissionByID(ctx context.Context, permissionID st return nil, nil, err } +func (g BaseGraphService) updateOCMPermission(ctx context.Context, permissionID string, itemID *storageprovider.ResourceId, newPermission *libregraph.Permission) (*libregraph.Permission, error) { + gatewayClient, err := g.gatewaySelector.Next() + if err != nil { + g.logger.Debug().Err(err).Msg("selecting gatewaySelector failed") + return nil, err + } + + resourceInfo, err := utils.GetResourceByID(ctx, itemID, gatewayClient) + if err != nil { + return nil, err + } + condition, err := roleConditionForResourceType(resourceInfo) + if err != nil { + return nil, err + } + var cs3UpdateOCMShareReq ocm.UpdateOCMShareRequest + cs3UpdateOCMShareReq.Ref = &ocm.ShareReference{ + Spec: &ocm.ShareReference_Id{ + Id: &ocm.ShareId{ + OpaqueId: permissionID, + }, + }, + } + if expiration, ok := newPermission.GetExpirationDateTimeOk(); ok { + cs3UpdateOCMShareReq.Field = append(cs3UpdateOCMShareReq.Field, &ocm.UpdateOCMShareRequest_UpdateField{ + Field: &ocm.UpdateOCMShareRequest_UpdateField_Expiration{ + Expiration: utils.TimeToTS(*expiration), + }, + }, + ) + } + + var allowedResourceActions []string + var permissionsUpdated bool + if roles, ok := newPermission.GetRolesOk(); ok { + if len(roles) > 0 { + for _, roleID := range roles { + role, err := unifiedrole.GetRole(unifiedrole.RoleFilterIDs(roleID)) + if err != nil { + g.logger.Debug().Err(err).Interface("role", role).Msg("unable to convert requested role") + return nil, err + } + + allowedResourceActions = unifiedrole.GetAllowedResourceActions(role, condition) + if len(allowedResourceActions) == 0 { + return nil, errorcode.New(errorcode.InvalidRequest, "role not applicable to this resource") + } + } + permissionsUpdated = true + + } else if allowedResourceActions, ok = newPermission.GetLibreGraphPermissionsActionsOk(); ok && len(allowedResourceActions) > 0 { + permissionsUpdated = true + } + + if permissionsUpdated { + cs3UpdateOCMShareReq.Field = append(cs3UpdateOCMShareReq.Field, &ocm.UpdateOCMShareRequest_UpdateField{ + Field: &ocm.UpdateOCMShareRequest_UpdateField_AccessMethods{ + AccessMethods: &ocm.AccessMethod{ + Term: &ocm.AccessMethod_WebdavOptions{ + WebdavOptions: &ocm.WebDAVAccessMethod{ + Permissions: unifiedrole.PermissionsToCS3ResourcePermissions( + []*libregraph.UnifiedRolePermission{ + { + + AllowedResourceActions: allowedResourceActions, + }, + }, + ), + }, + }, + }, + }, + }) + } + } + + updateOCMShareResp, err := gatewayClient.UpdateOCMShare(ctx, &cs3UpdateOCMShareReq) + if err != nil { + return nil, err + } + if err := errorcode.FromCS3Status(updateOCMShareResp.GetStatus(), err); err != nil { + return nil, err + } + + ocmShareResp, err := gatewayClient.GetOCMShare(ctx, &ocm.GetOCMShareRequest{ + Ref: &ocm.ShareReference{ + Spec: &ocm.ShareReference_Id{ + Id: &ocm.ShareId{ + OpaqueId: permissionID, + }, + }, + }, + }) + if err != nil { + return nil, err + } + permission, err := g.cs3OCMShareToPermission(ctx, ocmShareResp.GetShare(), condition) + if err != nil { + return nil, err + } + return permission, nil +} + func (g BaseGraphService) updateUserShare(ctx context.Context, permissionID string, itemID *storageprovider.ResourceId, newPermission *libregraph.Permission) (*libregraph.Permission, error) { gatewayClient, err := g.gatewaySelector.Next() if err != nil { From 8ee17e7f273cc99fce0bbd8383adbea1e39469f6 Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Fri, 6 Sep 2024 07:02:45 +0200 Subject: [PATCH 4/9] fix invalid check Signed-off-by: Christian Richter --- services/graph/pkg/service/v0/api_driveitem_permissions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/graph/pkg/service/v0/api_driveitem_permissions.go b/services/graph/pkg/service/v0/api_driveitem_permissions.go index 8510d8608..f236a6e76 100644 --- a/services/graph/pkg/service/v0/api_driveitem_permissions.go +++ b/services/graph/pkg/service/v0/api_driveitem_permissions.go @@ -554,7 +554,7 @@ func (s DriveItemPermissionsService) UpdatePermission(ctx context.Context, itemI // This is a user share updatedPermission, err := s.updateUserShare(ctx, permissionID, sharedResourceID, &newPermission) - if err == nil { + if err == nil && updatedPermission != nil { return *updatedPermission, nil } From 01b9521d09c6878a00abd4fc28ee65d72144fa67 Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Fri, 6 Sep 2024 07:05:55 +0200 Subject: [PATCH 5/9] temporarily vendor from fork Signed-off-by: Christian Richter --- go.mod | 2 + go.sum | 4 +- .../pkg/ocm/provider/authorizer/json/json.go | 6 +- .../v2/pkg/ocm/share/repository/json/json.go | 93 ++++++++++++++++++- vendor/modules.txt | 3 +- 5 files changed, 102 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index 5500c4276..2229d150c 100644 --- a/go.mod +++ b/go.mod @@ -365,6 +365,8 @@ replace github.com/unrolled/secure => github.com/DeepDiver1975/secure v0.0.0-202 replace github.com/go-micro/plugins/v4/store/nats-js-kv => github.com/kobergj/plugins/v4/store/nats-js-kv v0.0.0-20240807130109-f62bb67e8c90 +replace github.com/cs3org/reva/v2 => github.com/dragonchaser/reva/v2 v2.4.1-0.20240906050800-d68d37964295 + // exclude the v2 line of go-sqlite3 which was released accidentally and prevents pulling in newer versions of go-sqlite3 // see https://github.com/mattn/go-sqlite3/issues/965 for more details exclude github.com/mattn/go-sqlite3 v2.0.3+incompatible diff --git a/go.sum b/go.sum index a632b1569..ec891731e 100644 --- a/go.sum +++ b/go.sum @@ -255,8 +255,6 @@ 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.20240905133054-2de6ff31c4e3 h1:L1fD7ShX6W17e5YMgWpcmCq2KVHQy48gFrnc261iohQ= -github.com/cs3org/reva/v2 v2.23.1-0.20240905133054-2de6ff31c4e3/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= @@ -291,6 +289,8 @@ github.com/dnaeon/go-vcr v1.0.1/go.mod h1:aBB1+wY4s93YsC3HHjMBMrwTj2R9FHDzUr9KyG github.com/dnsimple/dnsimple-go v0.63.0/go.mod h1:O5TJ0/U6r7AfT8niYNlmohpLbCSG+c71tQlGr9SeGrg= github.com/docker/go-units v0.5.0 h1:69rxXcBk27SvSaaxTtLh/8llcHD8vYHT7WSdRZ/jvr4= github.com/docker/go-units v0.5.0/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDDbaIK4Dk= +github.com/dragonchaser/reva/v2 v2.4.1-0.20240906050800-d68d37964295 h1:/fGWgMqZ6YZ3979MJ3UgQRHNDZIxzCX1vlY2008f3d8= +github.com/dragonchaser/reva/v2 v2.4.1-0.20240906050800-d68d37964295/go.mod h1:p7CHBXcg6sSqB+0JMNDfC1S7TSh9FghXkw1kTV3KcJI= github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk= github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY= github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto= diff --git a/vendor/github.com/cs3org/reva/v2/pkg/ocm/provider/authorizer/json/json.go b/vendor/github.com/cs3org/reva/v2/pkg/ocm/provider/authorizer/json/json.go index fc3eb0c89..dac24f9c9 100644 --- a/vendor/github.com/cs3org/reva/v2/pkg/ocm/provider/authorizer/json/json.go +++ b/vendor/github.com/cs3org/reva/v2/pkg/ocm/provider/authorizer/json/json.go @@ -104,7 +104,11 @@ func normalizeDomain(d string) (string, error) { return "", err } - return u.Host, nil + normalizedDomain := u.Hostname() + if port := u.Port(); port != "" { + normalizedDomain += ":" + port + } + return normalizedDomain, nil } func (a *authorizer) GetInfoByDomain(_ context.Context, domain string) (*ocmprovider.ProviderInfo, error) { diff --git a/vendor/github.com/cs3org/reva/v2/pkg/ocm/share/repository/json/json.go b/vendor/github.com/cs3org/reva/v2/pkg/ocm/share/repository/json/json.go index 139a320b4..234b76553 100644 --- a/vendor/github.com/cs3org/reva/v2/pkg/ocm/share/repository/json/json.go +++ b/vendor/github.com/cs3org/reva/v2/pkg/ocm/share/repository/json/json.go @@ -381,8 +381,97 @@ func receivedShareEqual(ref *ocm.ShareReference, s *ocm.ReceivedShare) bool { return false } -func (m *mgr) UpdateShare(ctx context.Context, user *userpb.User, ref *ocm.ShareReference, f ...*ocm.UpdateOCMShareRequest_UpdateField) (*ocm.Share, error) { - return nil, errtypes.NotSupported("not yet implemented") +// UpdateShare updates the share with the given fields. +func (m *mgr) UpdateShare(ctx context.Context, user *userpb.User, ref *ocm.ShareReference, fields ...*ocm.UpdateOCMShareRequest_UpdateField) (*ocm.Share, error) { + m.Lock() + defer m.Unlock() + if err := m.load(); err != nil { + return nil, err + } + for _, s := range m.model.Shares { + if sharesEqual(ref, s) { + if utils.UserEqual(user.Id, s.Owner) || utils.UserEqual(user.Id, s.Creator) { + + for _, f := range fields { + if exp := f.GetExpiration(); exp != nil { + s.Expiration = exp + } + if am := f.GetAccessMethods(); am != nil { + var ( + webdavOptions *ocm.WebDAVAccessMethod + webappOptions *ocm.WebappAccessMethod + transferOptions *ocm.TransferAccessMethod + // TODO: *AccessMethod_GenericOptions + + newWebdavOptions *ocm.WebDAVAccessMethod + newWebappOptions *ocm.WebappAccessMethod + newTransferOptions *ocm.TransferAccessMethod + // TODO: *AccessMethod_GenericOptions + ) + + for _, sm := range s.GetAccessMethods() { + webdavOptions = sm.GetWebdavOptions() + webappOptions = sm.GetWebappOptions() + transferOptions = sm.GetTransferOptions() + } + + newWebdavOptions = am.GetWebdavOptions() + newWebappOptions = am.GetWebappOptions() + newTransferOptions = am.GetTransferOptions() + + newAccesMethods := []*ocm.AccessMethod{} + + if newWebdavOptions != nil { + newAccesMethods = append(newAccesMethods, &ocm.AccessMethod{ + Term: &ocm.AccessMethod_WebdavOptions{ + WebdavOptions: newWebdavOptions, + }, + }) + } else if webdavOptions != nil { + newAccesMethods = append(newAccesMethods, &ocm.AccessMethod{ + Term: &ocm.AccessMethod_WebdavOptions{ + WebdavOptions: webdavOptions, + }, + }) + } + + if newWebappOptions != nil { + newAccesMethods = append(newAccesMethods, &ocm.AccessMethod{ + Term: &ocm.AccessMethod_WebappOptions{ + WebappOptions: newWebappOptions, + }, + }) + } else if webappOptions != nil { + newAccesMethods = append(newAccesMethods, &ocm.AccessMethod{ + Term: &ocm.AccessMethod_WebappOptions{ + WebappOptions: webappOptions, + }, + }) + } + + if newTransferOptions != nil { + newAccesMethods = append(newAccesMethods, &ocm.AccessMethod{ + Term: &ocm.AccessMethod_TransferOptions{ + TransferOptions: newTransferOptions, + }, + }) + } else if transferOptions != nil { + newAccesMethods = append(newAccesMethods, &ocm.AccessMethod{ + Term: &ocm.AccessMethod_TransferOptions{ + TransferOptions: transferOptions, + }, + }) + } + s.AccessMethods = newAccesMethods + } + } + + return s, nil + } + } + } + + return nil, errtypes.NotFound(ref.String()) } func (m *mgr) ListShares(ctx context.Context, user *userpb.User, filters []*ocm.ListOCMSharesRequest_Filter) ([]*ocm.Share, error) { diff --git a/vendor/modules.txt b/vendor/modules.txt index 927340e68..7b353727f 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.20240905133054-2de6ff31c4e3 +# github.com/cs3org/reva/v2 v2.23.1-0.20240905133054-2de6ff31c4e3 => github.com/dragonchaser/reva/v2 v2.4.1-0.20240906050800-d68d37964295 ## explicit; go 1.21 github.com/cs3org/reva/v2/cmd/revad/internal/grace github.com/cs3org/reva/v2/cmd/revad/runtime @@ -2437,3 +2437,4 @@ stash.kopano.io/kgol/rndm # github.com/egirna/icap-client => github.com/fschade/icap-client v0.0.0-20240802074440-aade4a234387 # github.com/unrolled/secure => github.com/DeepDiver1975/secure v0.0.0-20240611112133-abc838fb797c # github.com/go-micro/plugins/v4/store/nats-js-kv => github.com/kobergj/plugins/v4/store/nats-js-kv v0.0.0-20240807130109-f62bb67e8c90 +# github.com/cs3org/reva/v2 => github.com/dragonchaser/reva/v2 v2.4.1-0.20240906050800-d68d37964295 From b0c23dce6468929d1dc16d1253200ab70231ca65 Mon Sep 17 00:00:00 2001 From: Christian Richter Date: Fri, 6 Sep 2024 08:27:30 +0200 Subject: [PATCH 6/9] fix wrong error return Signed-off-by: Christian Richter --- services/graph/pkg/service/v0/api_driveitem_permissions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/graph/pkg/service/v0/api_driveitem_permissions.go b/services/graph/pkg/service/v0/api_driveitem_permissions.go index f236a6e76..6f843b404 100644 --- a/services/graph/pkg/service/v0/api_driveitem_permissions.go +++ b/services/graph/pkg/service/v0/api_driveitem_permissions.go @@ -562,7 +562,7 @@ func (s DriveItemPermissionsService) UpdatePermission(ctx context.Context, itemI if s.config.IncludeOCMSharees { updatePermission, err := s.updateOCMPermission(ctx, permissionID, itemID, &newPermission) if err == nil { - return *updatePermission, err + return *updatePermission, nil } } return libregraph.Permission{}, err From 1b6c6c1ad32addf3dcb02a4c6cf1d91fe2c70e13 Mon Sep 17 00:00:00 2001 From: Florian Schade Date: Tue, 10 Sep 2024 11:49:37 +0200 Subject: [PATCH 7/9] chore: bump reva --- go.mod | 4 +--- go.sum | 4 ++-- .../cs3org/reva/v2/pkg/ocm/provider/authorizer/json/json.go | 6 +----- vendor/modules.txt | 3 +-- 4 files changed, 5 insertions(+), 12 deletions(-) diff --git a/go.mod b/go.mod index 2229d150c..1351b7e9d 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.20240905133054-2de6ff31c4e3 + github.com/cs3org/reva/v2 v2.23.1-0.20240909172158-5fd1e89e1557 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 @@ -365,8 +365,6 @@ replace github.com/unrolled/secure => github.com/DeepDiver1975/secure v0.0.0-202 replace github.com/go-micro/plugins/v4/store/nats-js-kv => github.com/kobergj/plugins/v4/store/nats-js-kv v0.0.0-20240807130109-f62bb67e8c90 -replace github.com/cs3org/reva/v2 => github.com/dragonchaser/reva/v2 v2.4.1-0.20240906050800-d68d37964295 - // exclude the v2 line of go-sqlite3 which was released accidentally and prevents pulling in newer versions of go-sqlite3 // see https://github.com/mattn/go-sqlite3/issues/965 for more details exclude github.com/mattn/go-sqlite3 v2.0.3+incompatible diff --git a/go.sum b/go.sum index ec891731e..d253454ff 100644 --- a/go.sum +++ b/go.sum @@ -255,6 +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.20240909172158-5fd1e89e1557 h1:ZVjUBlOU4jfRVSW3xCc0GKxXsYJzwZ7LTfKlqnWvXMw= +github.com/cs3org/reva/v2 v2.23.1-0.20240909172158-5fd1e89e1557/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= @@ -289,8 +291,6 @@ github.com/dnaeon/go-vcr v1.0.1/go.mod h1:aBB1+wY4s93YsC3HHjMBMrwTj2R9FHDzUr9KyG github.com/dnsimple/dnsimple-go v0.63.0/go.mod h1:O5TJ0/U6r7AfT8niYNlmohpLbCSG+c71tQlGr9SeGrg= github.com/docker/go-units v0.5.0 h1:69rxXcBk27SvSaaxTtLh/8llcHD8vYHT7WSdRZ/jvr4= github.com/docker/go-units v0.5.0/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDDbaIK4Dk= -github.com/dragonchaser/reva/v2 v2.4.1-0.20240906050800-d68d37964295 h1:/fGWgMqZ6YZ3979MJ3UgQRHNDZIxzCX1vlY2008f3d8= -github.com/dragonchaser/reva/v2 v2.4.1-0.20240906050800-d68d37964295/go.mod h1:p7CHBXcg6sSqB+0JMNDfC1S7TSh9FghXkw1kTV3KcJI= github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk= github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY= github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto= diff --git a/vendor/github.com/cs3org/reva/v2/pkg/ocm/provider/authorizer/json/json.go b/vendor/github.com/cs3org/reva/v2/pkg/ocm/provider/authorizer/json/json.go index dac24f9c9..fc3eb0c89 100644 --- a/vendor/github.com/cs3org/reva/v2/pkg/ocm/provider/authorizer/json/json.go +++ b/vendor/github.com/cs3org/reva/v2/pkg/ocm/provider/authorizer/json/json.go @@ -104,11 +104,7 @@ func normalizeDomain(d string) (string, error) { return "", err } - normalizedDomain := u.Hostname() - if port := u.Port(); port != "" { - normalizedDomain += ":" + port - } - return normalizedDomain, nil + return u.Host, nil } func (a *authorizer) GetInfoByDomain(_ context.Context, domain string) (*ocmprovider.ProviderInfo, error) { diff --git a/vendor/modules.txt b/vendor/modules.txt index 7b353727f..004a2f521 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.20240905133054-2de6ff31c4e3 => github.com/dragonchaser/reva/v2 v2.4.1-0.20240906050800-d68d37964295 +# github.com/cs3org/reva/v2 v2.23.1-0.20240909172158-5fd1e89e1557 ## explicit; go 1.21 github.com/cs3org/reva/v2/cmd/revad/internal/grace github.com/cs3org/reva/v2/cmd/revad/runtime @@ -2437,4 +2437,3 @@ stash.kopano.io/kgol/rndm # github.com/egirna/icap-client => github.com/fschade/icap-client v0.0.0-20240802074440-aade4a234387 # github.com/unrolled/secure => github.com/DeepDiver1975/secure v0.0.0-20240611112133-abc838fb797c # github.com/go-micro/plugins/v4/store/nats-js-kv => github.com/kobergj/plugins/v4/store/nats-js-kv v0.0.0-20240807130109-f62bb67e8c90 -# github.com/cs3org/reva/v2 => github.com/dragonchaser/reva/v2 v2.4.1-0.20240906050800-d68d37964295 From 7c34505f544336d8cdbac0b0da95441f8712382e Mon Sep 17 00:00:00 2001 From: Florian Schade Date: Tue, 10 Sep 2024 13:03:51 +0200 Subject: [PATCH 8/9] fix: use FromCS3Status error helper --- .../service/v0/api_driveitem_permissions.go | 30 ++++--- services/graph/pkg/service/v0/base.go | 85 +++++++++---------- 2 files changed, 56 insertions(+), 59 deletions(-) diff --git a/services/graph/pkg/service/v0/api_driveitem_permissions.go b/services/graph/pkg/service/v0/api_driveitem_permissions.go index 6f843b404..9cdc55772 100644 --- a/services/graph/pkg/service/v0/api_driveitem_permissions.go +++ b/services/graph/pkg/service/v0/api_driveitem_permissions.go @@ -16,15 +16,16 @@ import ( ocm "github.com/cs3org/go-cs3apis/cs3/sharing/ocm/v1beta1" storageprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" + "github.com/go-chi/chi/v5" + "github.com/go-chi/render" + libregraph "github.com/owncloud/libre-graph-api-go" + revactx "github.com/cs3org/reva/v2/pkg/ctx" "github.com/cs3org/reva/v2/pkg/publicshare" "github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool" "github.com/cs3org/reva/v2/pkg/share" "github.com/cs3org/reva/v2/pkg/storagespace" "github.com/cs3org/reva/v2/pkg/utils" - "github.com/go-chi/chi/v5" - "github.com/go-chi/render" - libregraph "github.com/owncloud/libre-graph-api-go" "github.com/owncloud/ocis/v2/ocis-pkg/l10n" l10n_pkg "github.com/owncloud/ocis/v2/services/graph/pkg/l10n" @@ -496,10 +497,10 @@ func (s DriveItemPermissionsService) DeletePermission(ctx context.Context, itemI return s.removeSpacePermission(ctx, permissionID, sharedResourceID) case OCM: return s.removeOCMPermission(ctx, permissionID) + default: + // This should never be reached + return errorcode.New(errorcode.GeneralException, "failed to delete permission") } - - // This should never be reached - return errorcode.New(errorcode.GeneralException, "failed to delete permission") } // DeleteSpaceRootPermission deletes a permission on the root item of a project space @@ -525,15 +526,15 @@ func (s DriveItemPermissionsService) DeleteSpaceRootPermission(ctx context.Conte // UpdatePermission updates a permission on a drive item func (s DriveItemPermissionsService) UpdatePermission(ctx context.Context, itemID *storageprovider.ResourceId, permissionID string, newPermission libregraph.Permission) (libregraph.Permission, error) { oldPermission, sharedResourceID, err := s.getPermissionByID(ctx, permissionID, itemID) + + // try to get the permission from ocm if the permission was not found first place + if err != nil && s.config.IncludeOCMSharees { + oldPermission, sharedResourceID, err = s.getOCMPermissionByID(ctx, permissionID, itemID) + } + + // if we still can't find the permission, return an error if err != nil { - if s.config.IncludeOCMSharees { - oldPermission, sharedResourceID, err = s.getOCMPermissionByID(ctx, permissionID, itemID) - if err != nil { - return libregraph.Permission{}, err - } - } else { - return libregraph.Permission{}, err - } + return libregraph.Permission{}, err } // The resourceID of the shared resource need to match the item ID from the Request Path @@ -565,6 +566,7 @@ func (s DriveItemPermissionsService) UpdatePermission(ctx context.Context, itemI return *updatePermission, nil } } + return libregraph.Permission{}, err } diff --git a/services/graph/pkg/service/v0/base.go b/services/graph/pkg/service/v0/base.go index bf860345a..dcbb3bf2b 100644 --- a/services/graph/pkg/service/v0/base.go +++ b/services/graph/pkg/service/v0/base.go @@ -640,13 +640,10 @@ func (g BaseGraphService) getCS3PublicShareByID(ctx context.Context, permissionI }, }, ) - if err != nil { - return nil, err - } - if err := errorcode.FromCS3Status(getPublicShareResp.GetStatus(), err); err != nil { return nil, err } + return getPublicShareResp.GetShare(), nil } @@ -666,14 +663,12 @@ func (g BaseGraphService) removeOCMPermission(ctx context.Context, permissionID }, }, }, - }) - if err != nil { - return err - } - + }, + ) if err := errorcode.FromCS3Status(removePublicShareResp.GetStatus(), err); err != nil { return err } + // We need to return an untyped nil here otherwise the error==nil check won't work return nil } @@ -694,14 +689,12 @@ func (g BaseGraphService) removePublicShare(ctx context.Context, permissionID st }, }, }, - }) - if err != nil { - return err - } - + }, + ) if err := errorcode.FromCS3Status(removePublicShareResp.GetStatus(), err); err != nil { return err } + // We need to return an untyped nil here otherwise the error==nil check won't work return nil } @@ -722,14 +715,12 @@ func (g BaseGraphService) removeUserShare(ctx context.Context, permissionID stri }, }, }, - }) - if err != nil { - return err - } - + }, + ) if err := errorcode.FromCS3Status(removeShareResp.GetStatus(), err); err != nil { return err } + // We need to return an untyped nil here otherwise the error==nil check won't work return nil } @@ -755,13 +746,10 @@ func (g BaseGraphService) removeSpacePermission(ctx context.Context, permissionI }, }, }) - if err != nil { - return err - } - if err := errorcode.FromCS3Status(removeShareResp.GetStatus(), err); err != nil { return err } + // We need to return an untyped nil here otherwise the error==nil check won't work return nil } @@ -771,6 +759,7 @@ func (g BaseGraphService) getOCMPermissionResourceID(ctx context.Context, permis if err != nil { return nil, err } + return cs3Share.GetResourceId(), nil } @@ -790,14 +779,12 @@ func (g BaseGraphService) getCS3OCMShareByID(ctx context.Context, permissionID s }, }, }, - }) - if err != nil { - return nil, err - } - + }, + ) if err := errorcode.FromCS3Status(getShareResp.GetStatus(), err); err != nil { return nil, err } + return getShareResp.GetShare(), nil } @@ -806,6 +793,7 @@ func (g BaseGraphService) getUserPermissionResourceID(ctx context.Context, permi if err != nil { return nil, err } + return cs3Share.GetResourceId(), nil } @@ -825,14 +813,12 @@ func (g BaseGraphService) getCS3UserShareByID(ctx context.Context, permissionID }, }, }, - }) - if err != nil { - return nil, err - } - + }, + ) if err := errorcode.FromCS3Status(getShareResp.GetStatus(), err); err != nil { return nil, err } + return getShareResp.GetShare(), nil } @@ -842,22 +828,27 @@ func (g BaseGraphService) getOCMPermissionByID(ctx context.Context, permissionID g.logger.Debug().Err(err).Msg("selecting gatewaySelevtor failed") return nil, nil, err } + ocmShare, err := g.getCS3OCMShareByID(ctx, permissionID) if err != nil { return nil, nil, err } + resourceInfo, err := utils.GetResourceByID(ctx, itemID, gatewayClient) if err != nil { return nil, nil, err } + condition, err := roleConditionForResourceType(resourceInfo) if err != nil { return nil, nil, err } + permission, err := g.cs3OCMShareToPermission(ctx, ocmShare, condition) if err != nil { return nil, nil, err } + return permission, ocmShare.GetResourceId(), nil } @@ -900,18 +891,22 @@ func (g BaseGraphService) getPermissionByID(ctx context.Context, permissionID st if err != nil { return nil, nil, err } + resourceInfo, err := utils.GetResourceByID(ctx, itemID, gatewayClient) if err != nil { return nil, nil, err } + condition, err := roleConditionForResourceType(resourceInfo) if err != nil { return nil, nil, err } + permission, err := g.cs3UserShareToPermission(ctx, cs3Share, condition) if err != nil { return nil, nil, err } + return permission, cs3Share.GetResourceId(), nil } @@ -929,10 +924,12 @@ func (g BaseGraphService) updateOCMPermission(ctx context.Context, permissionID if err != nil { return nil, err } + condition, err := roleConditionForResourceType(resourceInfo) if err != nil { return nil, err } + var cs3UpdateOCMShareReq ocm.UpdateOCMShareRequest cs3UpdateOCMShareReq.Ref = &ocm.ShareReference{ Spec: &ocm.ShareReference_Id{ @@ -941,12 +938,15 @@ func (g BaseGraphService) updateOCMPermission(ctx context.Context, permissionID }, }, } + if expiration, ok := newPermission.GetExpirationDateTimeOk(); ok { - cs3UpdateOCMShareReq.Field = append(cs3UpdateOCMShareReq.Field, &ocm.UpdateOCMShareRequest_UpdateField{ - Field: &ocm.UpdateOCMShareRequest_UpdateField_Expiration{ - Expiration: utils.TimeToTS(*expiration), + cs3UpdateOCMShareReq.Field = append( + cs3UpdateOCMShareReq.Field, + &ocm.UpdateOCMShareRequest_UpdateField{ + Field: &ocm.UpdateOCMShareRequest_UpdateField_Expiration{ + Expiration: utils.TimeToTS(*expiration), + }, }, - }, ) } @@ -981,7 +981,6 @@ func (g BaseGraphService) updateOCMPermission(ctx context.Context, permissionID Permissions: unifiedrole.PermissionsToCS3ResourcePermissions( []*libregraph.UnifiedRolePermission{ { - AllowedResourceActions: allowedResourceActions, }, }, @@ -995,9 +994,6 @@ func (g BaseGraphService) updateOCMPermission(ctx context.Context, permissionID } updateOCMShareResp, err := gatewayClient.UpdateOCMShare(ctx, &cs3UpdateOCMShareReq) - if err != nil { - return nil, err - } if err := errorcode.FromCS3Status(updateOCMShareResp.GetStatus(), err); err != nil { return nil, err } @@ -1011,13 +1007,15 @@ func (g BaseGraphService) updateOCMPermission(ctx context.Context, permissionID }, }, }) - if err != nil { + if err := errorcode.FromCS3Status(ocmShareResp.GetStatus(), err); err != nil { return nil, err } + permission, err := g.cs3OCMShareToPermission(ctx, ocmShareResp.GetShare(), condition) if err != nil { return nil, err } + return permission, nil } @@ -1108,9 +1106,6 @@ func (g BaseGraphService) updateUserShare(ctx context.Context, permissionID stri } updateUserShareResp, err := gatewayClient.UpdateShare(ctx, &cs3UpdateShareReq) - if err != nil { - return nil, err - } if err := errorcode.FromCS3Status(updateUserShareResp.GetStatus(), err); err != nil { return nil, err } From 3a4c0f33eab9481ee1451f1df14eed9bcb78ab1c Mon Sep 17 00:00:00 2001 From: Florian Schade Date: Wed, 11 Sep 2024 15:26:27 +0200 Subject: [PATCH 9/9] fix: ocm share update --- .../unreleased/allow-update-ocm-shares.md | 6 ++++++ changelog/unreleased/bump-reva.md | 2 ++ go.mod | 2 +- go.sum | 4 ++-- services/graph/pkg/service/v0/base.go | 4 +++- services/graph/pkg/unifiedrole/roles.go | 4 ---- services/web/pkg/theme/theme.go | 8 -------- .../v2/pkg/ocm/share/repository/json/json.go | 19 +++++++++++++++---- vendor/modules.txt | 2 +- 9 files changed, 30 insertions(+), 21 deletions(-) create mode 100644 changelog/unreleased/allow-update-ocm-shares.md diff --git a/changelog/unreleased/allow-update-ocm-shares.md b/changelog/unreleased/allow-update-ocm-shares.md new file mode 100644 index 000000000..366097858 --- /dev/null +++ b/changelog/unreleased/allow-update-ocm-shares.md @@ -0,0 +1,6 @@ +Bugfix: Allow update of ocm shares + +We fixed a bug that prevented ocm shares to be updated or removed. + +https://github.com/owncloud/ocis/pull/9980 +https://github.com/owncloud/ocis/issues/9926 diff --git a/changelog/unreleased/bump-reva.md b/changelog/unreleased/bump-reva.md index 093e0b5f4..3a457310c 100644 --- a/changelog/unreleased/bump-reva.md +++ b/changelog/unreleased/bump-reva.md @@ -2,6 +2,8 @@ Enhancement: Bump reva Bumps reva version +https://github.com/owncloud/ocis/pull/9980 +https://github.com/owncloud/ocis/pull/9981 https://github.com/owncloud/ocis/pull/9981 https://github.com/owncloud/ocis/pull/9920 https://github.com/owncloud/ocis/pull/9879 diff --git a/go.mod b/go.mod index 1351b7e9d..c9a1c57fd 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.20240909172158-5fd1e89e1557 + github.com/cs3org/reva/v2 v2.24.1-0.20240911132317-de8cea1f9e72 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 d253454ff..b611901a6 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.20240909172158-5fd1e89e1557 h1:ZVjUBlOU4jfRVSW3xCc0GKxXsYJzwZ7LTfKlqnWvXMw= -github.com/cs3org/reva/v2 v2.23.1-0.20240909172158-5fd1e89e1557/go.mod h1:p7CHBXcg6sSqB+0JMNDfC1S7TSh9FghXkw1kTV3KcJI= +github.com/cs3org/reva/v2 v2.24.1-0.20240911132317-de8cea1f9e72 h1:J1CCIbBOKVGEqEng3OwZzeX5jVLb8iTzM251D2C8oyo= +github.com/cs3org/reva/v2 v2.24.1-0.20240911132317-de8cea1f9e72/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/base.go b/services/graph/pkg/service/v0/base.go index dcbb3bf2b..6992a2e99 100644 --- a/services/graph/pkg/service/v0/base.go +++ b/services/graph/pkg/service/v0/base.go @@ -925,7 +925,7 @@ func (g BaseGraphService) updateOCMPermission(ctx context.Context, permissionID return nil, err } - condition, err := roleConditionForResourceType(resourceInfo) + condition, err := federatedRoleConditionForResourceType(resourceInfo) if err != nil { return nil, err } @@ -1030,10 +1030,12 @@ func (g BaseGraphService) updateUserShare(ctx context.Context, permissionID stri if err != nil { return nil, err } + condition, err := roleConditionForResourceType(resourceInfo) if err != nil { return nil, err } + var cs3UpdateShareReq collaboration.UpdateShareRequest // When updating a space root we need to reference the share by resourceId and grantee if IsSpaceRoot(itemID) { diff --git a/services/graph/pkg/unifiedrole/roles.go b/services/graph/pkg/unifiedrole/roles.go index 27d9af8aa..12cf07047 100644 --- a/services/graph/pkg/unifiedrole/roles.go +++ b/services/graph/pkg/unifiedrole/roles.go @@ -38,10 +38,6 @@ const ( UnifiedRoleManagerID = "312c0871-5ef7-4b3a-85b6-0e4074c64049" // UnifiedRoleSecureViewerID Unified role secure viewer id. UnifiedRoleSecureViewerID = "aa97fe03-7980-45ac-9e50-b325749fd7e6" - // UnifiedRoleFederatedViewerID Unified role federated viewer id. - UnifiedRoleFederatedViewerID = "be531789-063c-48bf-a9fe-857e6fbee7da" - // UnifiedRoleFederatedEditorID Unified role federated editor id. - UnifiedRoleFederatedEditorID = "36279a93-e4e3-4bbb-8a23-53b05b560963" // Wile the below conditions follow the SDDL syntax, they are not parsed anywhere. We use them as strings to // represent the constraints that a role definition applies to. For the actual syntax, see the SDDL documentation diff --git a/services/web/pkg/theme/theme.go b/services/web/pkg/theme/theme.go index 0477be28e..35f842206 100644 --- a/services/web/pkg/theme/theme.go +++ b/services/web/pkg/theme/theme.go @@ -65,14 +65,6 @@ var themeDefaults = KV{ "label": "UnifiedRoleSecureView", "iconName": "shield", }, - unifiedrole.UnifiedRoleFederatedViewerID: KV{ - "label": "UnifiedRoleFederatedViewer", - "iconName": "eye", - }, - unifiedrole.UnifiedRoleFederatedEditorID: KV{ - "label": "UnifiedRoleFederatedEditor", - "iconName": "pencil", - }, }, }, } diff --git a/vendor/github.com/cs3org/reva/v2/pkg/ocm/share/repository/json/json.go b/vendor/github.com/cs3org/reva/v2/pkg/ocm/share/repository/json/json.go index 234b76553..007022cd5 100644 --- a/vendor/github.com/cs3org/reva/v2/pkg/ocm/share/repository/json/json.go +++ b/vendor/github.com/cs3org/reva/v2/pkg/ocm/share/repository/json/json.go @@ -31,14 +31,15 @@ import ( ocm "github.com/cs3org/go-cs3apis/cs3/sharing/ocm/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" typespb "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" + "github.com/google/uuid" + "github.com/pkg/errors" + "google.golang.org/genproto/protobuf/field_mask" + "github.com/cs3org/reva/v2/pkg/errtypes" "github.com/cs3org/reva/v2/pkg/ocm/share" "github.com/cs3org/reva/v2/pkg/ocm/share/repository/registry" "github.com/cs3org/reva/v2/pkg/utils" "github.com/cs3org/reva/v2/pkg/utils/cfg" - "github.com/google/uuid" - "github.com/pkg/errors" - "google.golang.org/genproto/protobuf/field_mask" ) func init() { @@ -466,7 +467,17 @@ func (m *mgr) UpdateShare(ctx context.Context, user *userpb.User, ref *ocm.Share } } - return s, nil + clone, err := cloneShare(s) + if err != nil { + return nil, err + } + m.model.Shares[s.Id.OpaqueId] = clone + + if err := m.save(); err != nil { + return nil, errors.Wrap(err, "error saving share") + } + + return clone, nil } } } diff --git a/vendor/modules.txt b/vendor/modules.txt index 004a2f521..61544bf28 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.20240909172158-5fd1e89e1557 +# github.com/cs3org/reva/v2 v2.24.1-0.20240911132317-de8cea1f9e72 ## explicit; go 1.21 github.com/cs3org/reva/v2/cmd/revad/internal/grace github.com/cs3org/reva/v2/cmd/revad/runtime