From dcdbfd81d48af254540ea73dd98593843ac05630 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Thu, 14 Mar 2024 17:53:47 +0100 Subject: [PATCH] chore: rework cs3ReceivedSharesToDriveItems to make linter happy --- services/graph/pkg/service/v0/utils.go | 195 +++++++++--------- services/graph/pkg/unifiedrole/unifiedrole.go | 5 +- 2 files changed, 99 insertions(+), 101 deletions(-) diff --git a/services/graph/pkg/service/v0/utils.go b/services/graph/pkg/service/v0/utils.go index 8618f1602c..ee5ccd03e9 100644 --- a/services/graph/pkg/service/v0/utils.go +++ b/services/graph/pkg/service/v0/utils.go @@ -153,25 +153,6 @@ func cs3ReceivedSharesToDriveItems(ctx context.Context, resharing bool, receivedShares []*collaboration.ReceivedShare) ([]libregraph.DriveItem, error) { - // doStat is a helper function that stat a resource. - doStat := func(resourceId *storageprovider.ResourceId) (*storageprovider.StatResponse, error) { - shareStat, err := gatewayClient.Stat(ctx, &storageprovider.StatRequest{ - Ref: &storageprovider.Reference{ResourceId: resourceId}, - }) - switch errCode := errorcode.FromCS3Status(shareStat.GetStatus(), err); { - case errCode == nil: - break - // skip ItemNotFound shares, they might have been deleted in the meantime or orphans. - case errCode.GetCode() == errorcode.ItemNotFound: - return nil, nil - default: - logger.Error().Err(errCode).Msg("could not stat") - return nil, errCode - } - - return shareStat, nil - } - ch := make(chan libregraph.DriveItem) group := new(errgroup.Group) // Set max concurrency @@ -188,80 +169,29 @@ func cs3ReceivedSharesToDriveItems(ctx context.Context, group.Go(func() error { var err error // redeclare - shareStat, err := doStat(receivedShares[0].GetShare().GetResourceId()) - if shareStat == nil || err != nil { + shareStat, err := gatewayClient.Stat(ctx, &storageprovider.StatRequest{ + Ref: &storageprovider.Reference{ + ResourceId: receivedShares[0].GetShare().GetResourceId(), + }, + }) + + switch errCode := errorcode.FromCS3Status(shareStat.GetStatus(), err); { + case errCode == nil: + break + // skip ItemNotFound shares, they might have been deleted in the meantime or orphans. + case errCode.GetCode() == errorcode.ItemNotFound: + return nil + default: + logger.Error().Err(errCode).Msg("could not stat") + return errCode + } + + driveItem, err := fillDriveItemPropertiesFromReceivedShare(ctx, logger, identityCache, + resharing, receivedShares) + if err != nil { return err } - driveItem := libregraph.NewDriveItem() - - permissions := make([]libregraph.Permission, 0, len(receivedShares)) - - var oldestReceivedShare *collaboration.ReceivedShare - for _, receivedShare := range receivedShares { - switch { - case oldestReceivedShare == nil: - fallthrough - case utils.TSToTime(receivedShare.GetShare().GetCtime()).Before(utils.TSToTime(oldestReceivedShare.GetShare().GetCtime())): - oldestReceivedShare = receivedShare - } - - permission, err := cs3ReceivedShareToLibreGraphPermissions(ctx, logger, identityCache, resharing, receivedShare) - if err != nil { - return err - } - - // If at least one of the shares was accepted, we consider the driveItem's synchronized - // flag enabled. - // Also we use the Mountpoint name of the first accepted mountpoint as the name of - // of the driveItem - if receivedShare.GetState() == collaboration.ShareState_SHARE_STATE_ACCEPTED { - driveItem.SetClientSynchronize(true) - if name := receivedShare.GetMountPoint().GetPath(); name != "" && driveItem.GetName() == "" { - driveItem.SetName(receivedShare.GetMountPoint().GetPath()) - } - } - - // if at least one share is marked as hidden, consider the whole driveItem to be hidden - if receivedShare.GetHidden() { - driveItem.SetUIHidden(true) - } - - if userID := receivedShare.GetShare().GetCreator(); userID != nil { - identity, err := cs3UserIdToIdentity(ctx, identityCache, userID) - if err != nil { - logger.Warn().Err(err).Str("userid", userID.String()).Msg("could not get creator of the share") - } - - permission.SetInvitation( - libregraph.SharingInvitation{ - InvitedBy: &libregraph.IdentitySet{ - User: &identity, - }, - }, - ) - } - permissions = append(permissions, *permission) - - } - - // To stay compatible with the usershareprovider and the webdav - // service the id of the driveItem is composed of the StorageID and - // SpaceID of the sharestorage appended with the opaque ID of - // the oldest share for the resource: - // '$! - // Note: This means that the driveitem ID will change when the oldest - // share is removed. It would be good to have are more stable ID here (e.g. - // derived from the shared resource's ID. But as we need to use the same - // ID across all services this means we needed to make similar adjustments - // to the sharejail (usershareprovider, webdav). Which we can't currently do - // as some clients rely on the IDs used there having a special format. - driveItem.SetId(storagespace.FormatResourceID(storageprovider.ResourceId{ - StorageId: utils.ShareStorageProviderID, - OpaqueId: oldestReceivedShare.GetShare().GetId().GetOpaqueId(), - SpaceId: utils.ShareStorageSpaceID, - })) - if !driveItem.HasUIHidden() { driveItem.SetUIHidden(false) } @@ -272,7 +202,7 @@ func cs3ReceivedSharesToDriveItems(ctx context.Context, } } - remoteItem := libregraph.NewRemoteItem() + remoteItem := driveItem.RemoteItem { if id := shareStat.GetInfo().GetId(); id != nil { remoteItem.SetId(storagespace.FormatResourceID(*id)) @@ -369,14 +299,6 @@ func cs3ReceivedSharesToDriveItems(ctx context.Context, remoteItem.File = file driveItem.File = file } - - if len(permissions) > 0 { - remoteItem.Permissions = permissions - } - - if !reflect.ValueOf(*remoteItem).IsZero() { - driveItem.RemoteItem = remoteItem - } } ch <- *driveItem @@ -400,6 +322,81 @@ func cs3ReceivedSharesToDriveItems(ctx context.Context, return driveItems, err } +func fillDriveItemPropertiesFromReceivedShare(ctx context.Context, logger *log.Logger, + identityCache identity.IdentityCache, resharing bool, + receivedShares []*collaboration.ReceivedShare) (*libregraph.DriveItem, error) { + + driveItem := libregraph.NewDriveItem() + permissions := make([]libregraph.Permission, 0, len(receivedShares)) + + var oldestReceivedShare *collaboration.ReceivedShare + for _, receivedShare := range receivedShares { + switch { + case oldestReceivedShare == nil: + fallthrough + case utils.TSToTime(receivedShare.GetShare().GetCtime()).Before(utils.TSToTime(oldestReceivedShare.GetShare().GetCtime())): + oldestReceivedShare = receivedShare + } + + permission, err := cs3ReceivedShareToLibreGraphPermissions(ctx, logger, identityCache, resharing, receivedShare) + if err != nil { + return driveItem, err + } + + // If at least one of the shares was accepted, we consider the driveItem's synchronized + // flag enabled. + // Also we use the Mountpoint name of the first accepted mountpoint as the name for + // the driveItem + if receivedShare.GetState() == collaboration.ShareState_SHARE_STATE_ACCEPTED { + driveItem.SetClientSynchronize(true) + if name := receivedShare.GetMountPoint().GetPath(); name != "" && driveItem.GetName() == "" { + driveItem.SetName(receivedShare.GetMountPoint().GetPath()) + } + } + + // if at least one share is marked as hidden, consider the whole driveItem to be hidden + if receivedShare.GetHidden() { + driveItem.SetUIHidden(true) + } + + if userID := receivedShare.GetShare().GetCreator(); userID != nil { + identity, err := cs3UserIdToIdentity(ctx, identityCache, userID) + if err != nil { + logger.Warn().Err(err).Str("userid", userID.String()).Msg("could not get creator of the share") + } + + permission.SetInvitation( + libregraph.SharingInvitation{ + InvitedBy: &libregraph.IdentitySet{ + User: &identity, + }, + }, + ) + } + permissions = append(permissions, *permission) + // To stay compatible with the usershareprovider and the webdav + // service the id of the driveItem is composed of the StorageID and + // SpaceID of the sharestorage appended with the opaque ID of + // the oldest share for the resource: + // '$! + // Note: This means that the driveitem ID will change when the oldest + // share is removed. It would be good to have are more stable ID here (e.g. + // derived from the shared resource's ID. But as we need to use the same + // ID across all services this means we needed to make similar adjustments + // to the sharejail (usershareprovider, webdav). Which we can't currently do + // as some clients rely on the IDs used there having a special format. + driveItem.SetId(storagespace.FormatResourceID(storageprovider.ResourceId{ + StorageId: utils.ShareStorageProviderID, + OpaqueId: oldestReceivedShare.GetShare().GetId().GetOpaqueId(), + SpaceId: utils.ShareStorageSpaceID, + })) + + } + driveItem.RemoteItem = libregraph.NewRemoteItem() + driveItem.RemoteItem.Permissions = permissions + return driveItem, nil +} + func cs3ReceivedShareToLibreGraphPermissions(ctx context.Context, logger *log.Logger, identityCache identity.IdentityCache, resharing bool, receivedShare *collaboration.ReceivedShare) (*libregraph.Permission, error) { permission := libregraph.NewPermission() diff --git a/services/graph/pkg/unifiedrole/unifiedrole.go b/services/graph/pkg/unifiedrole/unifiedrole.go index 795c7ddcb4..2926299ea8 100644 --- a/services/graph/pkg/unifiedrole/unifiedrole.go +++ b/services/graph/pkg/unifiedrole/unifiedrole.go @@ -215,9 +215,10 @@ func GetBuiltinRoleDefinitionList(resharing bool) []*libregraph.UnifiedRoleDefin // GetApplicableRoleDefinitionsForActions returns a list of role definitions // that match the provided actions and constraints func GetApplicableRoleDefinitionsForActions(actions []string, constraints string, resharing, descending bool) []*libregraph.UnifiedRoleDefinition { - var definitions []*libregraph.UnifiedRoleDefinition + builtin := GetBuiltinRoleDefinitionList(resharing) + definitions := make([]*libregraph.UnifiedRoleDefinition, 0, len(builtin)) - for _, definition := range GetBuiltinRoleDefinitionList(resharing) { + for _, definition := range builtin { definitionMatch := true for _, permission := range definition.GetRolePermissions() {