diff --git a/src/internal/connector/graph_connector_onedrive_test.go b/src/internal/connector/graph_connector_onedrive_test.go index 0d3e3db6e..1d002d716 100644 --- a/src/internal/connector/graph_connector_onedrive_test.go +++ b/src/internal/connector/graph_connector_onedrive_test.go @@ -25,8 +25,12 @@ import ( const versionPermissionSwitchedToID = version.OneDrive4DirIncludesPermissions func getMetadata(fileName string, perm permData, permUseID bool) onedrive.Metadata { - if len(perm.user) == 0 || len(perm.roles) == 0 { - return onedrive.Metadata{FileName: fileName} + if len(perm.user) == 0 || len(perm.roles) == 0 || + perm.sharingMode != onedrive.SharingModeCustom { + return onedrive.Metadata{ + FileName: fileName, + SharingMode: perm.sharingMode, + } } id := base64.StdEncoding.EncodeToString([]byte(perm.user + strings.Join(perm.roles, "+"))) @@ -262,9 +266,10 @@ func (c *onedriveCollection) withPermissions(perm permData) *onedriveCollection } type permData struct { - user string // user is only for older versions - entityID string - roles []string + user string // user is only for older versions + entityID string + roles []string + sharingMode onedrive.SharingMode } type itemData struct { @@ -877,3 +882,159 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsRestoreAndNo }, ) } + +// This is similar to TestPermissionsRestoreAndBackup but tests purely +// for inheritance and that too only with newer versions +func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsInheritanceRestoreAndBackup() { + ctx, flush := tester.NewContext() + defer flush() + + // Get the default drive ID for the test user. + driveID := mustGetDefaultDriveID( + suite.T(), + ctx, + suite.connector.Service, + suite.user, + ) + + folderAName := "custom" + folderBName := "inherited" + + rootPath := []string{ + "drives", + driveID, + "root:", + } + folderAPath := []string{ + "drives", + driveID, + "root:", + folderAName, + } + subfolderAPath := []string{ + "drives", + driveID, + "root:", + folderAName, + folderAName, + } + subfolderBPath := []string{ + "drives", + driveID, + "root:", + folderAName, + folderBName, + } + + fileSet := []itemData{ + { + name: "file-custom", + data: fileAData, + perms: permData{ + user: suite.secondaryUser, + entityID: suite.secondaryUserID, + roles: writePerm, + sharingMode: onedrive.SharingModeCustom, + }, + }, + { + name: "file-inherited", + data: fileAData, + perms: permData{ + sharingMode: onedrive.SharingModeInherited, + }, + }, + } + + // Here is what this test is testing + // - custom-permission-folder + // - custom-permission-file + // - inherted-permission-file + // - custom-permission-folder + // - custom-permission-file + // - inherted-permission-file + // - inherted-permission-folder + // - custom-permission-file + // - inherted-permission-file + + // No reason why it couldn't work with previous versions, but this + // is when it got introduced. + startVersion := version.OneDrive4DirIncludesPermissions + + test := onedriveTest{ + startVersion: startVersion, + cols: []onedriveColInfo{ + { + pathElements: rootPath, + files: []itemData{}, + folders: []itemData{ + { + name: folderAName, + }, + }, + }, + { + pathElements: folderAPath, + files: fileSet, + folders: []itemData{ + {name: folderAName}, + {name: folderBName}, + }, + perms: permData{ + user: suite.secondaryUser, + entityID: suite.secondaryUserID, + roles: readPerm, + }, + }, + { + pathElements: subfolderAPath, + files: fileSet, + perms: permData{ + user: suite.secondaryUser, + entityID: suite.secondaryUserID, + roles: writePerm, + sharingMode: onedrive.SharingModeCustom, + }, + }, + { + pathElements: subfolderBPath, + files: fileSet, + perms: permData{ + sharingMode: onedrive.SharingModeInherited, + }, + }, + }, + } + + expected := testDataForInfo(suite.T(), test.cols, version.Backup) + + for vn := test.startVersion; vn <= version.Backup; vn++ { + suite.Run(fmt.Sprintf("Version%d", vn), func() { + t := suite.T() + // Ideally this can always be true or false and still + // work, but limiting older versions to use emails so as + // to validate that flow as well. + input := testDataForInfo(t, test.cols, vn) + + testData := restoreBackupInfoMultiVersion{ + service: path.OneDriveService, + resource: Users, + backupVersion: vn, + collectionsPrevious: input, + collectionsLatest: expected, + } + + runRestoreBackupTestVersions( + t, + suite.acct, + testData, + suite.connector.tenant, + []string{suite.user}, + control.Options{ + RestorePermissions: true, + ToggleFeatures: control.Toggles{EnablePermissionsBackup: true}, + }, + ) + }) + } +} diff --git a/src/internal/connector/onedrive/api/drive.go b/src/internal/connector/onedrive/api/drive.go index 0982664a8..743f29fa2 100644 --- a/src/internal/connector/onedrive/api/drive.go +++ b/src/internal/connector/onedrive/api/drive.go @@ -3,8 +3,10 @@ package api import ( "context" "fmt" + "strings" "github.com/alcionai/clues" + abstractions "github.com/microsoft/kiota-abstractions-go" msdrives "github.com/microsoftgraph/msgraph-sdk-go/drives" "github.com/microsoftgraph/msgraph-sdk-go/models" mssites "github.com/microsoftgraph/msgraph-sdk-go/sites" @@ -40,7 +42,18 @@ func NewItemPager( fields []string, ) *driveItemPager { pageCount := pageSize + + headers := abstractions.NewRequestHeaders() + preferHeaderItems := []string{ + "deltashowremovedasdeleted", + "deltatraversepermissiongaps", + "deltashowsharingchanges", + "hierarchicalsharing", + } + headers.Add("Prefer", strings.Join(preferHeaderItems, ",")) + requestConfig := &msdrives.ItemRootDeltaRequestBuilderGetRequestConfiguration{ + Headers: headers, QueryParameters: &msdrives.ItemRootDeltaRequestBuilderGetQueryParameters{ Top: &pageCount, Select: fields, diff --git a/src/internal/connector/onedrive/collection.go b/src/internal/connector/onedrive/collection.go index 2ff31c173..3e928a099 100644 --- a/src/internal/connector/onedrive/collection.go +++ b/src/internal/connector/onedrive/collection.go @@ -53,6 +53,13 @@ var ( _ data.StreamModTime = &metadataItem{} ) +type SharingMode int + +const ( + SharingModeCustom = SharingMode(iota) + SharingModeInherited +) + // Collection represents a set of OneDrive objects retrieved from M365 type Collection struct { // configured to handle large item downloads @@ -213,7 +220,11 @@ type UserPermission struct { // ItemMeta contains metadata about the Item. It gets stored in a // separate file in kopia type Metadata struct { - FileName string `json:"filename,omitempty"` + FileName string `json:"filename,omitempty"` + // SharingMode denotes what the current mode of sharing is for the object. + // - inherited: permissions same as parent permissions (no "shared" in delta) + // - custom: use Permissions to set correct permissions ("shared" has value in delta) + SharingMode SharingMode `json:"permissionMode,omitempty"` Permissions []UserPermission `json:"permissions,omitempty"` } diff --git a/src/internal/connector/onedrive/collections.go b/src/internal/connector/onedrive/collections.go index 9952b6f25..594d6b448 100644 --- a/src/internal/connector/onedrive/collections.go +++ b/src/internal/connector/onedrive/collections.go @@ -622,6 +622,14 @@ func (c *Collections) UpdateCollections( isFolder = item.GetFolder() != nil || item.GetPackage() != nil ) + // TODO(meain): Use `@microsoft.graph.sharedChanged` in + // item.GetAdditionalData() to optimize fetching + // permissions. When permissions change, this flags is + // present, if only data changes, it is not present. Have to + // add `oneDrive.sharedChanged` in `$select` in delta + // https://learn.microsoft.com/en-us/onedrive/developer/rest-api + // /concepts/scan-guidance#scanning-permissions-hierarchies + // Deleted file or folder. if item.GetDeleted() != nil { if err := c.handleDelete( diff --git a/src/internal/connector/onedrive/drive.go b/src/internal/connector/onedrive/drive.go index d33c332fa..a5f5ab29d 100644 --- a/src/internal/connector/onedrive/drive.go +++ b/src/internal/connector/onedrive/drive.go @@ -173,6 +173,7 @@ func defaultItemPager( "size", "deleted", "malware", + "shared", }, ) } diff --git a/src/internal/connector/onedrive/item.go b/src/internal/connector/onedrive/item.go index 76f66b1f6..ae7d82735 100644 --- a/src/internal/connector/onedrive/item.go +++ b/src/internal/connector/onedrive/item.go @@ -73,14 +73,27 @@ func oneDriveItemMetaReader( FileName: *item.GetName(), } - perms, err := oneDriveItemPermissionInfo(ctx, service, driveID, item, fetchPermissions) - if err != nil { - // Keep this in an if-block because if it's not then we have a weird issue - // of having no value in error but golang thinking it's non nil because of - // the way interfaces work. - err = clues.Wrap(err, "fetching item permissions") + if item.GetShared() == nil { + meta.SharingMode = SharingModeInherited } else { - meta.Permissions = perms + meta.SharingMode = SharingModeCustom + } + + var ( + perms []UserPermission + err error + ) + + if meta.SharingMode == SharingModeCustom && fetchPermissions { + perms, err = oneDriveItemPermissionInfo(ctx, service, driveID, ptr.Val(item.GetId())) + if err != nil { + // Keep this in an if-block because if it's not then we have a weird issue + // of having no value in error but golang thinking it's non nil because of + // the way interfaces work. + err = clues.Wrap(err, "fetching item permissions") + } else { + meta.Permissions = perms + } } metaJSON, serializeErr := json.Marshal(meta) @@ -219,29 +232,22 @@ func oneDriveItemInfo(di models.DriveItemable, itemSize int64) *details.OneDrive } } -// oneDriveItemPermissionInfo will fetch the permission information for a drive -// item. +// OneDriveItemPermissionInfo will fetch the permission information +// for a drive item given a drive and item id. func oneDriveItemPermissionInfo( ctx context.Context, service graph.Servicer, driveID string, - di models.DriveItemable, - fetchPermissions bool, + itemID string, ) ([]UserPermission, error) { - if !fetchPermissions { - return nil, nil - } - - id := ptr.Val(di.GetId()) - perm, err := service. Client(). DrivesById(driveID). - ItemsById(id). + ItemsById(itemID). Permissions(). Get(ctx, nil) if err != nil { - return nil, graph.Wrap(ctx, err, "getting item metadata").With("item_id", id) + return nil, graph.Wrap(ctx, err, "getting item metadata").With("item_id", itemID) } uperms := filterUserPermissions(ctx, perm.GetValue()) diff --git a/src/internal/connector/onedrive/permission.go b/src/internal/connector/onedrive/permission.go index f1dd982d7..bb880defd 100644 --- a/src/internal/connector/onedrive/permission.go +++ b/src/internal/connector/onedrive/permission.go @@ -14,80 +14,70 @@ import ( "github.com/alcionai/corso/src/pkg/path" ) -func getParentPermissions( +func getParentMetadata( parentPath path.Path, - parentPermissions map[string][]UserPermission, -) ([]UserPermission, error) { - parentPerms, ok := parentPermissions[parentPath.String()] + metas map[string]Metadata, +) (Metadata, error) { + parentMeta, ok := metas[parentPath.String()] if !ok { onedrivePath, err := path.ToOneDrivePath(parentPath) if err != nil { - return nil, errors.Wrap(err, "invalid restore path") + return Metadata{}, errors.Wrap(err, "invalid restore path") } if len(onedrivePath.Folders) != 0 { - return nil, errors.Wrap(err, "computing item permissions") + return Metadata{}, errors.Wrap(err, "computing item permissions") } - parentPerms = []UserPermission{} + parentMeta = Metadata{} } - return parentPerms, nil + return parentMeta, nil } -func getParentAndCollectionPermissions( +func getCollectionMetadata( ctx context.Context, drivePath *path.DrivePath, dc data.RestoreCollection, - permissions map[string][]UserPermission, + metas map[string]Metadata, backupVersion int, restorePerms bool, -) ([]UserPermission, []UserPermission, error) { +) (Metadata, error) { if !restorePerms || backupVersion < version.OneDrive1DataAndMetaFiles { - return nil, nil, nil + return Metadata{}, nil } var ( err error - parentPerms []UserPermission - colPerms []UserPermission collectionPath = dc.FullPath() ) - // Only get parent permissions if we're not restoring the root. - if len(drivePath.Folders) > 0 { - parentPath, err := collectionPath.Dir() - if err != nil { - return nil, nil, clues.Wrap(err, "getting parent path") - } - - parentPerms, err = getParentPermissions(parentPath, permissions) - if err != nil { - return nil, nil, clues.Wrap(err, "getting parent permissions") - } + if len(drivePath.Folders) == 0 { + // No permissions for root folder + return Metadata{}, nil } if backupVersion < version.OneDrive4DirIncludesPermissions { - colPerms, err = getParentPermissions(collectionPath, permissions) + colMeta, err := getParentMetadata(collectionPath, metas) if err != nil { - return nil, nil, clues.Wrap(err, "getting collection permissions") - } - } else if len(drivePath.Folders) > 0 { - // Root folder doesn't have a metadata file associated with it. - folders := collectionPath.Folders() - - meta, err := fetchAndReadMetadata( - ctx, - dc, - folders[len(folders)-1]+DirMetaFileSuffix) - if err != nil { - return nil, nil, clues.Wrap(err, "collection permissions") + return Metadata{}, clues.Wrap(err, "collection metadata") } - colPerms = meta.Permissions + return colMeta, nil } - return parentPerms, colPerms, nil + // Root folder doesn't have a metadata file associated with it. + folders := collectionPath.Folders() + + meta, err := fetchAndReadMetadata( + ctx, + dc, + folders[len(folders)-1]+DirMetaFileSuffix) + if err != nil { + return Metadata{}, clues.Wrap(err, "collection metadata") + } + + return meta, nil } // createRestoreFoldersWithPermissions creates the restore folder hierarchy in @@ -99,8 +89,7 @@ func createRestoreFoldersWithPermissions( service graph.Servicer, driveID string, restoreFolders []string, - parentPermissions []UserPermission, - folderPermissions []UserPermission, + folderMetadata Metadata, permissionIDMappings map[string]string, ) (string, error) { id, err := CreateRestoreFolders(ctx, service, driveID, restoreFolders) @@ -113,58 +102,52 @@ func createRestoreFoldersWithPermissions( service, driveID, id, - parentPermissions, - folderPermissions, + folderMetadata, permissionIDMappings) return id, err } -// getChildPermissions is to filter out permissions present in the -// parent from the ones that are available for child. This is -// necessary as we store the nested permissions in the child. We -// cannot avoid storing the nested permissions as it is possible that -// a file in a folder can remove the nested permission that is present -// on itself. -func getChildPermissions( - childPermissions, parentPermissions []UserPermission, +func diffPermissions( + before, after []UserPermission, + permissionIDMappings map[string]string, ) ([]UserPermission, []UserPermission) { var ( - addedPermissions = []UserPermission{} - removedPermissions = []UserPermission{} + added = []UserPermission{} + removed = []UserPermission{} ) - for _, cp := range childPermissions { + for _, cp := range after { found := false - for _, pp := range parentPermissions { - if cp.ID == pp.ID { + for _, pp := range before { + if cp.ID == permissionIDMappings[pp.ID] { found = true break } } if !found { - addedPermissions = append(addedPermissions, cp) + added = append(added, cp) } } - for _, pp := range parentPermissions { + for _, pp := range before { found := false - for _, cp := range childPermissions { - if pp.ID == cp.ID { + for _, cp := range after { + if permissionIDMappings[pp.ID] == cp.ID { found = true break } } if !found { - removedPermissions = append(removedPermissions, pp) + removed = append(removed, pp) } } - return addedPermissions, removedPermissions + return added, removed } // restorePermissions takes in the permissions that were added and the @@ -175,19 +158,28 @@ func restorePermissions( service graph.Servicer, driveID string, itemID string, - parentPerms []UserPermission, - childPerms []UserPermission, + meta Metadata, permissionIDMappings map[string]string, ) error { - permAdded, permRemoved := getChildPermissions(childPerms, parentPerms) + 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 := oneDriveItemPermissionInfo(ctx, service, driveID, itemID) + if err != nil { + return graph.Wrap(ctx, err, "fetching current permissions") + } + + permAdded, permRemoved := diffPermissions(currentPermissions, meta.Permissions, permissionIDMappings) + for _, p := range permRemoved { err := service.Client(). DrivesById(driveID). ItemsById(itemID). - PermissionsById(permissionIDMappings[p.ID]). + PermissionsById(p.ID). Delete(ctx, nil) if err != nil { return graph.Wrap(ctx, err, "removing permissions") diff --git a/src/internal/connector/onedrive/restore.go b/src/internal/connector/onedrive/restore.go index 4c5615c06..ea34387b9 100644 --- a/src/internal/connector/onedrive/restore.go +++ b/src/internal/connector/onedrive/restore.go @@ -44,7 +44,7 @@ func RestoreCollections( var ( restoreMetrics support.CollectionMetrics metrics support.CollectionMetrics - folderPerms map[string][]UserPermission + folderMetas map[string]Metadata // permissionIDMappings is used to map between old and new id // of permissions as we restore them @@ -63,8 +63,8 @@ func RestoreCollections( }) var ( - el = errs.Local() - parentPermissions = map[string][]UserPermission{} + el = errs.Local() + parentMetas = map[string]Metadata{} ) // Iterate through the data collections and restore the contents of each @@ -82,12 +82,12 @@ func RestoreCollections( "path", dc.FullPath()) // TODO: pii ) - metrics, folderPerms, permissionIDMappings, err = RestoreCollection( + metrics, folderMetas, permissionIDMappings, err = RestoreCollection( ictx, backupVersion, service, dc, - parentPermissions, + parentMetas, OneDriveSource, dest.ContainerName, deets, @@ -98,8 +98,8 @@ func RestoreCollections( el.AddRecoverable(err) } - for k, v := range folderPerms { - parentPermissions[k] = v + for k, v := range folderMetas { + parentMetas[k] = v } restoreMetrics = support.CombineMetrics(restoreMetrics, metrics) @@ -128,20 +128,20 @@ func RestoreCollection( backupVersion int, service graph.Servicer, dc data.RestoreCollection, - parentPermissions map[string][]UserPermission, + parentMetas map[string]Metadata, source driveSource, restoreContainerName string, deets *details.Builder, permissionIDMappings map[string]string, restorePerms bool, errs *fault.Bus, -) (support.CollectionMetrics, map[string][]UserPermission, map[string]string, error) { +) (support.CollectionMetrics, map[string]Metadata, map[string]string, error) { var ( metrics = support.CollectionMetrics{} copyBuffer = make([]byte, copyBufferSize) directory = dc.FullPath() itemInfo details.ItemInfo - folderPerms = map[string][]UserPermission{} + folderMetas = map[string]Metadata{} ) ctx, end := D.Span(ctx, "gc:oneDrive:restoreCollection", D.Label("path", directory)) @@ -149,7 +149,7 @@ func RestoreCollection( drivePath, err := path.ToOneDrivePath(directory) if err != nil { - return metrics, folderPerms, permissionIDMappings, clues.Wrap(err, "creating drive path").WithClues(ctx) + return metrics, folderMetas, permissionIDMappings, clues.Wrap(err, "creating drive path").WithClues(ctx) } // Assemble folder hierarchy we're going to restore into (we recreate the folder hierarchy @@ -168,15 +168,15 @@ func RestoreCollection( trace.Log(ctx, "gc:oneDrive:restoreCollection", directory.String()) logger.Ctx(ctx).Info("restoring onedrive collection") - parentPerms, colPerms, err := getParentAndCollectionPermissions( + colMeta, err := getCollectionMetadata( ctx, drivePath, dc, - parentPermissions, + parentMetas, backupVersion, restorePerms) if err != nil { - return metrics, folderPerms, permissionIDMappings, clues.Wrap(err, "getting permissions").WithClues(ctx) + return metrics, folderMetas, permissionIDMappings, 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,12 +185,11 @@ func RestoreCollection( service, drivePath.DriveID, restoreFolderElements, - parentPerms, - colPerms, + colMeta, permissionIDMappings, ) if err != nil { - return metrics, folderPerms, permissionIDMappings, clues.Wrap(err, "creating folders for restore") + return metrics, folderMetas, permissionIDMappings, clues.Wrap(err, "creating folders for restore") } var ( @@ -205,11 +204,11 @@ func RestoreCollection( select { case <-ctx.Done(): - return metrics, folderPerms, permissionIDMappings, err + return metrics, folderMetas, permissionIDMappings, err case itemData, ok := <-items: if !ok { - return metrics, folderPerms, permissionIDMappings, nil + return metrics, folderMetas, permissionIDMappings, nil } itemPath, err := dc.FullPath().Append(itemData.UUID(), true) @@ -239,7 +238,6 @@ func RestoreCollection( dc, restoreFolderID, copyBuffer, - colPerms, permissionIDMappings, restorePerms, itemData, @@ -253,7 +251,6 @@ func RestoreCollection( dc, restoreFolderID, copyBuffer, - colPerms, permissionIDMappings, restorePerms, itemData, @@ -296,7 +293,7 @@ func RestoreCollection( } trimmedPath := strings.TrimSuffix(itemPath.String(), DirMetaFileSuffix) - folderPerms[trimmedPath] = meta.Permissions + folderMetas[trimmedPath] = meta } } else { @@ -330,7 +327,7 @@ func RestoreCollection( } } - return metrics, folderPerms, permissionIDMappings, el.Failure() + return metrics, folderMetas, permissionIDMappings, el.Failure() } type fileFetcher interface { @@ -345,7 +342,6 @@ func restoreV1File( fetcher fileFetcher, restoreFolderID string, copyBuffer []byte, - parentPerms []UserPermission, permissionIDMappings map[string]string, restorePerms bool, itemData data.Stream, @@ -384,8 +380,7 @@ func restoreV1File( service, drivePath.DriveID, itemID, - parentPerms, - meta.Permissions, + meta, permissionIDMappings, ) if err != nil { @@ -403,7 +398,6 @@ func restoreV2File( fetcher fileFetcher, restoreFolderID string, copyBuffer []byte, - parentPerms []UserPermission, permissionIDMappings map[string]string, restorePerms bool, itemData data.Stream, @@ -453,8 +447,7 @@ func restoreV2File( service, drivePath.DriveID, itemID, - parentPerms, - meta.Permissions, + meta, permissionIDMappings, ) if err != nil { diff --git a/src/internal/connector/sharepoint/restore.go b/src/internal/connector/sharepoint/restore.go index fa8a7e269..30535bed1 100644 --- a/src/internal/connector/sharepoint/restore.go +++ b/src/internal/connector/sharepoint/restore.go @@ -71,7 +71,7 @@ func RestoreCollections( backupVersion, service, dc, - map[string][]onedrive.UserPermission{}, // Currently permission data is not stored for sharepoint + map[string]onedrive.Metadata{}, // Currently permission data is not stored for sharepoint onedrive.SharePointSource, dest.ContainerName, deets,