diff --git a/CHANGELOG.md b/CHANGELOG.md index b92307c01..266361d50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fix repo connect not working without a config file - Fix item re-download on expired links silently being skipped +- Improved permissions backup and restore for OneDrive + +### Known Issues +- Owner (Full control) or empty (Restricted View) roles cannot be restored for OneDrive ## [v0.5.0] (beta) - 2023-03-13 diff --git a/src/internal/connector/graph_connector_helper_test.go b/src/internal/connector/graph_connector_helper_test.go index baa0bd705..6ae1cb9b4 100644 --- a/src/internal/connector/graph_connector_helper_test.go +++ b/src/internal/connector/graph_connector_helper_test.go @@ -785,10 +785,19 @@ func compareOneDriveItem( return true } + // We cannot restore owner permissions, so skip checking them + itemPerms := []onedrive.UserPermission{} + + for _, p := range itemMeta.Permissions { + if p.Roles[0] != "owner" { + itemPerms = append(itemPerms, p) + } + } + testElementsMatch( t, expectedMeta.Permissions, - itemMeta.Permissions, + itemPerms, permissionEqual, ) diff --git a/src/internal/connector/graph_connector_onedrive_test.go b/src/internal/connector/graph_connector_onedrive_test.go index 127a431c7..ad2f2b875 100644 --- a/src/internal/connector/graph_connector_onedrive_test.go +++ b/src/internal/connector/graph_connector_onedrive_test.go @@ -149,6 +149,7 @@ var ( fileDData = []byte(strings.Repeat("d", 257)) fileEData = []byte(strings.Repeat("e", 257)) + // Cannot restore owner or empty permissions and so not testing them writePerm = []string{"write"} readPerm = []string{"read"} ) diff --git a/src/internal/connector/onedrive/item.go b/src/internal/connector/onedrive/item.go index 096f125d9..2f17f9e81 100644 --- a/src/internal/connector/onedrive/item.go +++ b/src/internal/connector/onedrive/item.go @@ -256,18 +256,15 @@ func filterUserPermissions(ctx context.Context, perms []models.Permissionable) [ } gv2 := p.GetGrantedToV2() - roles := []string{} - for _, r := range p.GetRoles() { - // Skip if the only role available in owner - if r != "owner" { - roles = append(roles, r) - } - } - - if len(roles) == 0 { - continue - } + // Below are the mapping from roles to "Advanced" permissions + // screen entries: + // + // owner - Full Control + // write - Design | Edit | Contribute (no difference in /permissions api) + // read - Read + // empty - Restricted View + roles := p.GetRoles() entityID := "" if gv2.GetUser() != nil { @@ -275,7 +272,7 @@ func filterUserPermissions(ctx context.Context, perms []models.Permissionable) [ } else if gv2.GetGroup() != nil { entityID = ptr.Val(gv2.GetGroup().GetId()) } else { - // TODO Add appliction permissions when adding permissions for SharePoint + // TODO Add application permissions when adding permissions for SharePoint // https://devblogs.microsoft.com/microsoft365dev/controlling-app-access-on-specific-sharepoint-site-collections/ logm := logger.Ctx(ctx) if gv2.GetApplication() != nil { diff --git a/src/internal/connector/onedrive/item_test.go b/src/internal/connector/onedrive/item_test.go index f1b4d1ff6..625553023 100644 --- a/src/internal/connector/onedrive/item_test.go +++ b/src/internal/connector/onedrive/item_test.go @@ -291,6 +291,7 @@ func (suite *ItemUnitTestSuite) TestOneDrivePermissionsFilter() { userID := "fakeuser@provider.com" userID2 := "fakeuser2@provider.com" + userOwnerPerm, userOwnerUperm := getPermsUperms(permID, userID, "user", []string{"owner"}) userReadPerm, userReadUperm := getPermsUperms(permID, userID, "user", []string{"read"}) userReadWritePerm, userReadWriteUperm := getPermsUperms(permID, userID2, "user", []string{"read", "write"}) @@ -322,6 +323,11 @@ func (suite *ItemUnitTestSuite) TestOneDrivePermissionsFilter() { graphPermissions: []models.Permissionable{userReadPerm}, parsedPermissions: []UserPermission{userReadUperm}, }, + { + name: "user with owner permissions", + graphPermissions: []models.Permissionable{userOwnerPerm}, + parsedPermissions: []UserPermission{userOwnerUperm}, + }, { name: "user with read and write permissions", graphPermissions: []models.Permissionable{userReadWritePerm}, diff --git a/src/internal/connector/onedrive/permission.go b/src/internal/connector/onedrive/permission.go index 2fafc1460..6157b03a0 100644 --- a/src/internal/connector/onedrive/permission.go +++ b/src/internal/connector/onedrive/permission.go @@ -7,6 +7,7 @@ import ( msdrive "github.com/microsoftgraph/msgraph-sdk-go/drive" "github.com/microsoftgraph/msgraph-sdk-go/models" "github.com/pkg/errors" + "golang.org/x/exp/slices" "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/connector/graph" @@ -90,20 +91,25 @@ func getCollectionMetadata( func createRestoreFoldersWithPermissions( ctx context.Context, service graph.Servicer, - driveID string, + drivePath *path.DrivePath, restoreFolders []string, folderMetadata Metadata, permissionIDMappings map[string]string, ) (string, error) { - id, err := CreateRestoreFolders(ctx, service, driveID, restoreFolders) + id, err := CreateRestoreFolders(ctx, service, drivePath.DriveID, restoreFolders) if err != nil { return "", err } + if len(drivePath.Folders) == 0 { + // No permissions for root folder + return id, nil + } + err = restorePermissions( ctx, service, - driveID, + drivePath.DriveID, id, folderMetadata, permissionIDMappings) @@ -111,6 +117,14 @@ func createRestoreFoldersWithPermissions( 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, permissionIDMappings map[string]string, @@ -124,7 +138,8 @@ func diffPermissions( found := false for _, pp := range before { - if cp.ID == permissionIDMappings[pp.ID] { + if isSame(cp.Roles, pp.Roles) && + cp.EntityID == pp.EntityID { found = true break } @@ -139,7 +154,8 @@ func diffPermissions( found := false for _, cp := range after { - if permissionIDMappings[pp.ID] == cp.ID { + if isSame(cp.Roles, pp.Roles) && + cp.EntityID == pp.EntityID { found = true break } @@ -190,8 +206,22 @@ func restorePermissions( } for _, p := range permAdded { + // We are not able to restore permissions when there are no + // roles or for owner, this seems to be restriction in graph + roles := []string{} + + for _, r := range p.Roles { + if r != "owner" { + roles = append(roles, r) + } + } + + if len(roles) == 0 { + continue + } + pbody := msdrive.NewItemsItemInvitePostRequestBody() - pbody.SetRoles(p.Roles) + pbody.SetRoles(roles) if p.Expiration != nil { expiry := p.Expiration.String() diff --git a/src/internal/connector/onedrive/restore.go b/src/internal/connector/onedrive/restore.go index 1a483a0d2..cf7c8127e 100644 --- a/src/internal/connector/onedrive/restore.go +++ b/src/internal/connector/onedrive/restore.go @@ -183,7 +183,7 @@ func RestoreCollection( restoreFolderID, err := createRestoreFoldersWithPermissions( ctx, service, - drivePath.DriveID, + drivePath, restoreFolderElements, colMeta, permissionIDMappings,