Backup owner and empty permissions (#2833)

Backup owner and empty permissions, we cannot restore them though (refer https://github.com/alcionai/corso/issues/2789#issuecomment-1471787829) . Previous we were just skipping even backing them up. It might be good idea to have this data in case we find that we are able to restore them in the future.

---

#### Does this PR need a docs update or release note?

- [x]  Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [ ]  No

#### Type of change

<!--- Please check the type of change your PR introduces: --->
- [x] 🌻 Feature
- [ ] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Test
- [ ] 💻 CI/Deployment
- [ ] 🧹 Tech Debt/Cleanup

#### Issue(s)

<!-- Can reference multiple issues. Use one of the following "magic words" - "closes, fixes" to auto-close the Github issue. -->
* closes https://github.com/alcionai/corso/issues/2789

#### Test Plan

<!-- How will this be tested prior to merging.-->
- [ ] 💪 Manual
- [x]  Unit test
- [ ] 💚 E2E
This commit is contained in:
Abin Simon 2023-03-17 15:12:23 +05:30 committed by GitHub
parent 9fd3b81f6e
commit 0e1064078a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 67 additions and 20 deletions

View File

@ -15,6 +15,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed ### Fixed
- Fix repo connect not working without a config file - Fix repo connect not working without a config file
- Fix item re-download on expired links silently being skipped - 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 ## [v0.5.0] (beta) - 2023-03-13

View File

@ -785,10 +785,19 @@ func compareOneDriveItem(
return true 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( testElementsMatch(
t, t,
expectedMeta.Permissions, expectedMeta.Permissions,
itemMeta.Permissions, itemPerms,
permissionEqual, permissionEqual,
) )

View File

@ -149,6 +149,7 @@ var (
fileDData = []byte(strings.Repeat("d", 257)) fileDData = []byte(strings.Repeat("d", 257))
fileEData = []byte(strings.Repeat("e", 257)) fileEData = []byte(strings.Repeat("e", 257))
// Cannot restore owner or empty permissions and so not testing them
writePerm = []string{"write"} writePerm = []string{"write"}
readPerm = []string{"read"} readPerm = []string{"read"}
) )

View File

@ -256,18 +256,15 @@ func filterUserPermissions(ctx context.Context, perms []models.Permissionable) [
} }
gv2 := p.GetGrantedToV2() gv2 := p.GetGrantedToV2()
roles := []string{}
for _, r := range p.GetRoles() { // Below are the mapping from roles to "Advanced" permissions
// Skip if the only role available in owner // screen entries:
if r != "owner" { //
roles = append(roles, r) // owner - Full Control
} // write - Design | Edit | Contribute (no difference in /permissions api)
} // read - Read
// empty - Restricted View
if len(roles) == 0 { roles := p.GetRoles()
continue
}
entityID := "" entityID := ""
if gv2.GetUser() != nil { if gv2.GetUser() != nil {
@ -275,7 +272,7 @@ func filterUserPermissions(ctx context.Context, perms []models.Permissionable) [
} else if gv2.GetGroup() != nil { } else if gv2.GetGroup() != nil {
entityID = ptr.Val(gv2.GetGroup().GetId()) entityID = ptr.Val(gv2.GetGroup().GetId())
} else { } 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/ // https://devblogs.microsoft.com/microsoft365dev/controlling-app-access-on-specific-sharepoint-site-collections/
logm := logger.Ctx(ctx) logm := logger.Ctx(ctx)
if gv2.GetApplication() != nil { if gv2.GetApplication() != nil {

View File

@ -291,6 +291,7 @@ func (suite *ItemUnitTestSuite) TestOneDrivePermissionsFilter() {
userID := "fakeuser@provider.com" userID := "fakeuser@provider.com"
userID2 := "fakeuser2@provider.com" userID2 := "fakeuser2@provider.com"
userOwnerPerm, userOwnerUperm := getPermsUperms(permID, userID, "user", []string{"owner"})
userReadPerm, userReadUperm := getPermsUperms(permID, userID, "user", []string{"read"}) userReadPerm, userReadUperm := getPermsUperms(permID, userID, "user", []string{"read"})
userReadWritePerm, userReadWriteUperm := getPermsUperms(permID, userID2, "user", []string{"read", "write"}) userReadWritePerm, userReadWriteUperm := getPermsUperms(permID, userID2, "user", []string{"read", "write"})
@ -322,6 +323,11 @@ func (suite *ItemUnitTestSuite) TestOneDrivePermissionsFilter() {
graphPermissions: []models.Permissionable{userReadPerm}, graphPermissions: []models.Permissionable{userReadPerm},
parsedPermissions: []UserPermission{userReadUperm}, parsedPermissions: []UserPermission{userReadUperm},
}, },
{
name: "user with owner permissions",
graphPermissions: []models.Permissionable{userOwnerPerm},
parsedPermissions: []UserPermission{userOwnerUperm},
},
{ {
name: "user with read and write permissions", name: "user with read and write permissions",
graphPermissions: []models.Permissionable{userReadWritePerm}, graphPermissions: []models.Permissionable{userReadWritePerm},

View File

@ -7,6 +7,7 @@ import (
msdrive "github.com/microsoftgraph/msgraph-sdk-go/drive" msdrive "github.com/microsoftgraph/msgraph-sdk-go/drive"
"github.com/microsoftgraph/msgraph-sdk-go/models" "github.com/microsoftgraph/msgraph-sdk-go/models"
"github.com/pkg/errors" "github.com/pkg/errors"
"golang.org/x/exp/slices"
"github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/common/ptr"
"github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/connector/graph"
@ -90,20 +91,25 @@ func getCollectionMetadata(
func createRestoreFoldersWithPermissions( func createRestoreFoldersWithPermissions(
ctx context.Context, ctx context.Context,
service graph.Servicer, service graph.Servicer,
driveID string, drivePath *path.DrivePath,
restoreFolders []string, restoreFolders []string,
folderMetadata Metadata, folderMetadata Metadata,
permissionIDMappings map[string]string, permissionIDMappings map[string]string,
) (string, error) { ) (string, error) {
id, err := CreateRestoreFolders(ctx, service, driveID, restoreFolders) id, err := CreateRestoreFolders(ctx, service, drivePath.DriveID, restoreFolders)
if err != nil { if err != nil {
return "", err return "", err
} }
if len(drivePath.Folders) == 0 {
// No permissions for root folder
return id, nil
}
err = restorePermissions( err = restorePermissions(
ctx, ctx,
service, service,
driveID, drivePath.DriveID,
id, id,
folderMetadata, folderMetadata,
permissionIDMappings) permissionIDMappings)
@ -111,6 +117,14 @@ func createRestoreFoldersWithPermissions(
return id, err 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( func diffPermissions(
before, after []UserPermission, before, after []UserPermission,
permissionIDMappings map[string]string, permissionIDMappings map[string]string,
@ -124,7 +138,8 @@ func diffPermissions(
found := false found := false
for _, pp := range before { for _, pp := range before {
if cp.ID == permissionIDMappings[pp.ID] { if isSame(cp.Roles, pp.Roles) &&
cp.EntityID == pp.EntityID {
found = true found = true
break break
} }
@ -139,7 +154,8 @@ func diffPermissions(
found := false found := false
for _, cp := range after { for _, cp := range after {
if permissionIDMappings[pp.ID] == cp.ID { if isSame(cp.Roles, pp.Roles) &&
cp.EntityID == pp.EntityID {
found = true found = true
break break
} }
@ -190,8 +206,22 @@ func restorePermissions(
} }
for _, p := range permAdded { 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 := msdrive.NewItemsItemInvitePostRequestBody()
pbody.SetRoles(p.Roles) pbody.SetRoles(roles)
if p.Expiration != nil { if p.Expiration != nil {
expiry := p.Expiration.String() expiry := p.Expiration.String()

View File

@ -183,7 +183,7 @@ func RestoreCollection(
restoreFolderID, err := createRestoreFoldersWithPermissions( restoreFolderID, err := createRestoreFoldersWithPermissions(
ctx, ctx,
service, service,
drivePath.DriveID, drivePath,
restoreFolderElements, restoreFolderElements,
colMeta, colMeta,
permissionIDMappings, permissionIDMappings,