diff --git a/src/internal/connector/graph_connector_onedrive_test.go b/src/internal/connector/graph_connector_onedrive_test.go index 242681b84..71fc264c6 100644 --- a/src/internal/connector/graph_connector_onedrive_test.go +++ b/src/internal/connector/graph_connector_onedrive_test.go @@ -2,13 +2,13 @@ package connector import ( "context" - "encoding/base64" "encoding/json" "fmt" "strings" "testing" "github.com/alcionai/clues" + "github.com/google/uuid" "github.com/microsoftgraph/msgraph-sdk-go/models" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -37,7 +37,10 @@ func getMetadata(fileName string, perm permData, permUseID bool) onedrive.Metada } } - id := base64.StdEncoding.EncodeToString([]byte(perm.user + strings.Join(perm.roles, "+"))) + // In case of permissions, the id will usually be same for same + // user/role combo unless deleted and readded, but we have to do + // this as we only have two users of which one is already taken. + id := uuid.NewString() uperm := onedrive.UserPermission{ID: id, Roles: perm.roles} if permUseID { diff --git a/src/internal/connector/onedrive/permission.go b/src/internal/connector/onedrive/permission.go index 938f6d1f5..8c0fca2b7 100644 --- a/src/internal/connector/onedrive/permission.go +++ b/src/internal/connector/onedrive/permission.go @@ -6,8 +6,8 @@ import ( "github.com/alcionai/clues" msdrive "github.com/microsoftgraph/msgraph-sdk-go/drive" "github.com/microsoftgraph/msgraph-sdk-go/models" - "golang.org/x/exp/slices" + "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/version" @@ -93,7 +93,11 @@ func createRestoreFoldersWithPermissions( service graph.Servicer, drivePath *path.DrivePath, restoreFolders []string, + folderPath path.Path, folderMetadata Metadata, + folderMetas map[string]Metadata, + permissionIDMappings map[string]string, + restorePerms bool, ) (string, error) { id, err := CreateRestoreFolders(ctx, service, drivePath.DriveID, restoreFolders) if err != nil { @@ -105,28 +109,25 @@ func createRestoreFoldersWithPermissions( return id, nil } + if !restorePerms { + return id, nil + } + err = RestorePermissions( ctx, creds, service, drivePath.DriveID, id, - folderMetadata) + folderPath, + folderMetadata, + folderMetas, + permissionIDMappings) return id, err } -// isSame checks equality of two string slices -func isSame(first, second []string) bool { - slices.Sort(first) - slices.Sort(second) - - return slices.Equal(first, second) -} - -func diffPermissions( - before, after []UserPermission, -) ([]UserPermission, []UserPermission) { +func diffPermissions(before, after []UserPermission) ([]UserPermission, []UserPermission) { var ( added = []UserPermission{} removed = []UserPermission{} @@ -136,8 +137,7 @@ func diffPermissions( found := false for _, pp := range before { - if isSame(cp.Roles, pp.Roles) && - cp.EntityID == pp.EntityID { + if pp.ID == cp.ID { found = true break } @@ -152,8 +152,7 @@ func diffPermissions( found := false for _, cp := range after { - if isSame(cp.Roles, pp.Roles) && - cp.EntityID == pp.EntityID { + if pp.ID == cp.ID { found = true break } @@ -167,31 +166,58 @@ func diffPermissions( return added, removed } -// RestorePermissions takes in the permissions that were added and the -// removed(ones present in parent but not in child) and adds/removes -// the necessary permissions on onedrive objects. -func RestorePermissions( +// computeParentPermissions computes the parent permissions by +// traversing folderMetas and finding the first item with custom +// permissions. folderMetas is expected to have all the parent +// directory metas for this to work. +func computeParentPermissions(itemPath path.Path, folderMetas map[string]Metadata) (Metadata, error) { + var ( + parent path.Path + meta Metadata + + err error + ok bool + ) + + parent = itemPath + + for { + parent, err = parent.Dir() + if err != nil { + return Metadata{}, clues.New("getting parent") + } + + onedrivePath, err := path.ToOneDrivePath(parent) + if err != nil { + return Metadata{}, clues.New("get parent path") + } + + if len(onedrivePath.Folders) == 0 { + return Metadata{}, nil + } + + meta, ok = folderMetas[parent.String()] + if !ok { + return Metadata{}, clues.New("no parent meta") + } + + if meta.SharingMode == SharingModeCustom { + return meta, nil + } + } +} + +// UpdatePermissions takes in the set of permission to be added and +// removed from an item to bring it to the desired state. +func UpdatePermissions( ctx context.Context, creds account.M365Config, service graph.Servicer, driveID string, itemID string, - meta Metadata, + permAdded, permRemoved []UserPermission, + permissionIDMappings map[string]string, ) error { - if meta.SharingMode == SharingModeInherited { - return nil - } - - ctx = clues.Add(ctx, "permission_item_id", itemID) - - // TODO(meain): Compute this from the data that we have instead of fetching from graph - currentPermissions, err := driveItemPermissionInfo(ctx, service, driveID, itemID) - if err != nil { - return graph.Wrap(ctx, err, "fetching current permissions") - } - - permAdded, permRemoved := diffPermissions(currentPermissions, meta.Permissions) - for _, p := range permRemoved { // deletes require unique http clients // https://github.com/alcionai/corso/issues/2707 @@ -202,11 +228,16 @@ func RestorePermissions( return graph.Wrap(ctx, err, "creating delete client") } + pid, ok := permissionIDMappings[p.ID] + if !ok { + return clues.New("no new permission id").WithClues(ctx) + } + err = graph.NewService(a). Client(). DrivesById(driveID). ItemsById(itemID). - PermissionsById(p.ID). + PermissionsById(pid). Delete(ctx, nil) if err != nil { return graph.Wrap(ctx, err, "removing permissions") @@ -253,11 +284,44 @@ func RestorePermissions( pbody.SetRecipients([]models.DriveRecipientable{rec}) - _, err := service.Client().DrivesById(driveID).ItemsById(itemID).Invite().Post(ctx, pbody, nil) + np, err := service.Client().DrivesById(driveID).ItemsById(itemID).Invite().Post(ctx, pbody, nil) if err != nil { return graph.Wrap(ctx, err, "setting permissions") } + + permissionIDMappings[p.ID] = ptr.Val(np.GetValue()[0].GetId()) } return nil } + +// RestorePermissions takes in the permissions of an item, computes +// what permissions need to added and removed based on the parent +// folder metas and uses that to add/remove the necessary permissions +// on onedrive items. +func RestorePermissions( + ctx context.Context, + creds account.M365Config, + service graph.Servicer, + driveID string, + itemID string, + itemPath path.Path, + meta Metadata, + folderMetas map[string]Metadata, + permissionIDMappings map[string]string, +) error { + if meta.SharingMode == SharingModeInherited { + return nil + } + + ctx = clues.Add(ctx, "permission_item_id", itemID) + + parentPermissions, err := computeParentPermissions(itemPath, folderMetas) + if err != nil { + return clues.Wrap(err, "parent permissions").WithClues(ctx) + } + + permAdded, permRemoved := diffPermissions(parentPermissions.Permissions, meta.Permissions) + + return UpdatePermissions(ctx, creds, service, driveID, itemID, permAdded, permRemoved, permissionIDMappings) +} diff --git a/src/internal/connector/onedrive/permission_test.go b/src/internal/connector/onedrive/permission_test.go new file mode 100644 index 000000000..a729ca24a --- /dev/null +++ b/src/internal/connector/onedrive/permission_test.go @@ -0,0 +1,153 @@ +package onedrive + +import ( + "fmt" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" + + "github.com/alcionai/corso/src/internal/tester" + "github.com/alcionai/corso/src/pkg/path" +) + +type PermissionsUnitTestSuite struct { + tester.Suite +} + +func TestPermissionsUnitTestSuite(t *testing.T) { + suite.Run(t, &PermissionsUnitTestSuite{Suite: tester.NewUnitSuite(t)}) +} + +func (suite *PermissionsUnitTestSuite) TestComputeParentPermissions() { + entryPath := fmt.Sprintf(rootDrivePattern, "drive-id") + "/level0/level1/level2/entry" + rootEntryPath := fmt.Sprintf(rootDrivePattern, "drive-id") + "/entry" + + entry, err := path.Build( + "tenant", + "user", + path.OneDriveService, + path.FilesCategory, + false, + strings.Split(entryPath, "/")..., + ) + require.NoError(suite.T(), err, "creating path") + + rootEntry, err := path.Build( + "tenant", + "user", + path.OneDriveService, + path.FilesCategory, + false, + strings.Split(rootEntryPath, "/")..., + ) + require.NoError(suite.T(), err, "creating path") + + level2, err := entry.Dir() + require.NoError(suite.T(), err, "level2 path") + + level1, err := level2.Dir() + require.NoError(suite.T(), err, "level1 path") + + level0, err := level1.Dir() + require.NoError(suite.T(), err, "level0 path") + + metadata := Metadata{ + SharingMode: SharingModeCustom, + Permissions: []UserPermission{ + { + Roles: []string{"write"}, + EntityID: "user-id", + }, + }, + } + + metadata2 := Metadata{ + SharingMode: SharingModeCustom, + Permissions: []UserPermission{ + { + Roles: []string{"read"}, + EntityID: "user-id", + }, + }, + } + + inherited := Metadata{ + SharingMode: SharingModeInherited, + Permissions: []UserPermission{}, + } + + table := []struct { + name string + item path.Path + meta Metadata + parentPerms map[string]Metadata + }{ + { + name: "root level entry", + item: rootEntry, + meta: Metadata{}, + parentPerms: map[string]Metadata{}, + }, + { + name: "root level directory", + item: level0, + meta: Metadata{}, + parentPerms: map[string]Metadata{}, + }, + { + name: "direct parent perms", + item: entry, + meta: metadata, + parentPerms: map[string]Metadata{ + level2.String(): metadata, + }, + }, + { + name: "top level parent perms", + item: entry, + meta: metadata, + parentPerms: map[string]Metadata{ + level2.String(): inherited, + level1.String(): inherited, + level0.String(): metadata, + }, + }, + { + name: "all inherited", + item: entry, + meta: Metadata{}, + parentPerms: map[string]Metadata{ + level2.String(): inherited, + level1.String(): inherited, + level0.String(): inherited, + }, + }, + { + name: "multiple custom permission", + item: entry, + meta: metadata, + parentPerms: map[string]Metadata{ + level2.String(): inherited, + level1.String(): metadata, + level0.String(): metadata2, + }, + }, + } + + for _, test := range table { + suite.Run(test.name, func() { + _, flush := tester.NewContext() + defer flush() + + t := suite.T() + + m, err := computeParentPermissions(test.item, test.parentPerms) + require.NoError(t, err, "compute permissions") + + assert.Equal(t, m, test.meta) + }) + } +} diff --git a/src/internal/connector/onedrive/restore.go b/src/internal/connector/onedrive/restore.go index 4f35267af..ff3db56cb 100644 --- a/src/internal/connector/onedrive/restore.go +++ b/src/internal/connector/onedrive/restore.go @@ -46,7 +46,11 @@ func RestoreCollections( var ( restoreMetrics support.CollectionMetrics metrics support.CollectionMetrics - folderMetas map[string]Metadata + folderMetas = map[string]Metadata{} + + // permissionIDMappings is used to map between old and new id + // of permissions as we restore them + permissionIDMappings = map[string]string{} ) ctx = clues.Add( @@ -60,10 +64,7 @@ func RestoreCollections( return dcs[i].FullPath().String() < dcs[j].FullPath().String() }) - var ( - el = errs.Local() - parentMetas = map[string]Metadata{} - ) + el := errs.Local() // Iterate through the data collections and restore the contents of each for _, dc := range dcs { @@ -80,13 +81,14 @@ func RestoreCollections( "path", dc.FullPath()) ) - metrics, folderMetas, err = RestoreCollection( + metrics, err = RestoreCollection( ictx, creds, backupVersion, service, dc, - parentMetas, + folderMetas, + permissionIDMappings, OneDriveSource, dest.ContainerName, deets, @@ -96,10 +98,6 @@ func RestoreCollections( el.AddRecoverable(err) } - for k, v := range folderMetas { - parentMetas[k] = v - } - restoreMetrics = support.CombineMetrics(restoreMetrics, metrics) if errors.Is(err, context.Canceled) { @@ -128,19 +126,19 @@ func RestoreCollection( backupVersion int, service graph.Servicer, dc data.RestoreCollection, - parentMetas map[string]Metadata, + folderMetas map[string]Metadata, + permissionIDMappings map[string]string, source driveSource, restoreContainerName string, deets *details.Builder, restorePerms bool, errs *fault.Bus, -) (support.CollectionMetrics, map[string]Metadata, error) { +) (support.CollectionMetrics, error) { var ( - metrics = support.CollectionMetrics{} - copyBuffer = make([]byte, copyBufferSize) - directory = dc.FullPath() - folderMetas = map[string]Metadata{} - el = errs.Local() + metrics = support.CollectionMetrics{} + copyBuffer = make([]byte, copyBufferSize) + directory = dc.FullPath() + el = errs.Local() ) ctx, end := diagnostics.Span(ctx, "gc:oneDrive:restoreCollection", diagnostics.Label("path", directory)) @@ -148,7 +146,7 @@ func RestoreCollection( drivePath, err := path.ToOneDrivePath(directory) if err != nil { - return metrics, folderMetas, clues.Wrap(err, "creating drive path").WithClues(ctx) + return metrics, clues.Wrap(err, "creating drive path").WithClues(ctx) } // Assemble folder hierarchy we're going to restore into (we recreate the folder hierarchy @@ -171,11 +169,11 @@ func RestoreCollection( ctx, drivePath, dc, - parentMetas, + folderMetas, backupVersion, restorePerms) if err != nil { - return metrics, folderMetas, clues.Wrap(err, "getting permissions").WithClues(ctx) + return metrics, clues.Wrap(err, "getting permissions").WithClues(ctx) } // Create restore folders and get the folder ID of the folder the data stream will be restored in @@ -185,11 +183,16 @@ func RestoreCollection( service, drivePath, restoreFolderElements, - colMeta) + dc.FullPath(), + colMeta, + folderMetas, + permissionIDMappings, + restorePerms) if err != nil { - return metrics, folderMetas, clues.Wrap(err, "creating folders for restore") + return metrics, clues.Wrap(err, "creating folders for restore") } + folderMetas[dc.FullPath().String()] = colMeta items := dc.Items(ctx, errs) for { @@ -199,11 +202,11 @@ func RestoreCollection( select { case <-ctx.Done(): - return metrics, folderMetas, err + return metrics, err case itemData, ok := <-items: if !ok { - return metrics, folderMetas, nil + return metrics, nil } itemPath, err := dc.FullPath().Append(itemData.UUID(), true) @@ -223,6 +226,7 @@ func RestoreCollection( restoreFolderID, copyBuffer, folderMetas, + permissionIDMappings, restorePerms, itemData, itemPath) @@ -259,7 +263,7 @@ func RestoreCollection( } } - return metrics, folderMetas, el.Failure() + return metrics, el.Failure() } // restores an item, according to correct backup version behavior. @@ -275,6 +279,7 @@ func restoreItem( restoreFolderID string, copyBuffer []byte, folderMetas map[string]Metadata, + permissionIDMappings map[string]string, restorePerms bool, itemData data.Stream, itemPath path.Path, @@ -341,6 +346,9 @@ func restoreItem( restoreFolderID, copyBuffer, restorePerms, + folderMetas, + permissionIDMappings, + itemPath, itemData) if err != nil { return details.ItemInfo{}, false, clues.Wrap(err, "v1 restore") @@ -361,6 +369,9 @@ func restoreItem( restoreFolderID, copyBuffer, restorePerms, + folderMetas, + permissionIDMappings, + itemPath, itemData) if err != nil { return details.ItemInfo{}, false, clues.Wrap(err, "v6 restore") @@ -408,6 +419,9 @@ func restoreV1File( restoreFolderID string, copyBuffer []byte, restorePerms bool, + folderMetas map[string]Metadata, + permissionIDMappings map[string]string, + itemPath path.Path, itemData data.Stream, ) (details.ItemInfo, error) { trimmedName := strings.TrimSuffix(itemData.UUID(), DataFileSuffix) @@ -445,7 +459,10 @@ func restoreV1File( service, drivePath.DriveID, itemID, - meta) + itemPath, + meta, + folderMetas, + permissionIDMappings) if err != nil { return details.ItemInfo{}, clues.Wrap(err, "restoring item permissions") } @@ -463,6 +480,9 @@ func restoreV6File( restoreFolderID string, copyBuffer []byte, restorePerms bool, + folderMetas map[string]Metadata, + permissionIDMappings map[string]string, + itemPath path.Path, itemData data.Stream, ) (details.ItemInfo, error) { trimmedName := strings.TrimSuffix(itemData.UUID(), DataFileSuffix) @@ -511,7 +531,10 @@ func restoreV6File( service, drivePath.DriveID, itemID, - meta) + itemPath, + meta, + folderMetas, + permissionIDMappings) if err != nil { return details.ItemInfo{}, clues.Wrap(err, "restoring item permissions") } diff --git a/src/internal/connector/sharepoint/restore.go b/src/internal/connector/sharepoint/restore.go index 1f77b7b7a..39aa85cff 100644 --- a/src/internal/connector/sharepoint/restore.go +++ b/src/internal/connector/sharepoint/restore.go @@ -67,13 +67,14 @@ func RestoreCollections( switch dc.FullPath().Category() { case path.LibrariesCategory: - metrics, _, err = onedrive.RestoreCollection( + metrics, err = onedrive.RestoreCollection( ictx, creds, backupVersion, service, dc, map[string]onedrive.Metadata{}, // Currently permission data is not stored for sharepoint + map[string]string{}, onedrive.SharePointSource, dest.ContainerName, deets, diff --git a/src/internal/operations/backup_integration_test.go b/src/internal/operations/backup_integration_test.go index d4560033c..c2e664dc8 100644 --- a/src/internal/operations/backup_integration_test.go +++ b/src/internal/operations/backup_integration_test.go @@ -1264,6 +1264,13 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_oneDriveIncrementals() { var ( newFile models.DriveItemable newFileName = "new_file.txt" + + permissionIDMappings = map[string]string{} + writePerm = onedrive.UserPermission{ + ID: "perm-id", + Roles: []string{"write"}, + EntityID: suite.user, + } ) // Although established as a table, these tests are not isolated from each other. @@ -1307,21 +1314,16 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_oneDriveIncrementals() { driveItem := models.NewDriveItem() driveItem.SetName(&newFileName) driveItem.SetFile(models.NewFile()) - err = onedrive.RestorePermissions( + err = onedrive.UpdatePermissions( ctx, creds, gc.Service, driveID, *newFile.GetId(), - onedrive.Metadata{ - SharingMode: onedrive.SharingModeCustom, - Permissions: []onedrive.UserPermission{ - { - Roles: []string{"write"}, - EntityID: suite.user, - }, - }, - }) + []onedrive.UserPermission{writePerm}, + []onedrive.UserPermission{}, + permissionIDMappings, + ) require.NoErrorf(t, err, "add permission to file %v", clues.ToCore(err)) }, itemsRead: 1, // .data file for newitem @@ -1333,16 +1335,16 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_oneDriveIncrementals() { driveItem := models.NewDriveItem() driveItem.SetName(&newFileName) driveItem.SetFile(models.NewFile()) - err = onedrive.RestorePermissions( + err = onedrive.UpdatePermissions( ctx, creds, gc.Service, driveID, *newFile.GetId(), - onedrive.Metadata{ - SharingMode: onedrive.SharingModeCustom, - Permissions: []onedrive.UserPermission{}, - }) + []onedrive.UserPermission{}, + []onedrive.UserPermission{writePerm}, + permissionIDMappings, + ) require.NoError(t, err, "add permission to file", clues.ToCore(err)) }, itemsRead: 1, // .data file for newitem @@ -1355,21 +1357,16 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_oneDriveIncrementals() { driveItem := models.NewDriveItem() driveItem.SetName(&newFileName) driveItem.SetFile(models.NewFile()) - err = onedrive.RestorePermissions( + err = onedrive.UpdatePermissions( ctx, creds, gc.Service, driveID, targetContainer, - onedrive.Metadata{ - SharingMode: onedrive.SharingModeCustom, - Permissions: []onedrive.UserPermission{ - { - Roles: []string{"write"}, - EntityID: suite.user, - }, - }, - }) + []onedrive.UserPermission{writePerm}, + []onedrive.UserPermission{}, + permissionIDMappings, + ) require.NoError(t, err, "add permission to file", clues.ToCore(err)) }, itemsRead: 0, @@ -1382,16 +1379,16 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_oneDriveIncrementals() { driveItem := models.NewDriveItem() driveItem.SetName(&newFileName) driveItem.SetFile(models.NewFile()) - err = onedrive.RestorePermissions( + err = onedrive.UpdatePermissions( ctx, creds, gc.Service, driveID, targetContainer, - onedrive.Metadata{ - SharingMode: onedrive.SharingModeCustom, - Permissions: []onedrive.UserPermission{}, - }) + []onedrive.UserPermission{}, + []onedrive.UserPermission{writePerm}, + permissionIDMappings, + ) require.NoError(t, err, "add permission to file", clues.ToCore(err)) }, itemsRead: 0,