Fix some edge cases around OneDrive permissions backup (#2370)

## Description

This fixes tying to parse link shares as permissions and some error/retry handling.

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

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

## Type of change

<!--- Please check the type of change your PR introduces: --->
- [ ] 🌻 Feature
- [x] 🐛 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. -->
* #<issue>

## Test Plan

<!-- How will this be tested prior to merging.-->
- [x] 💪 Manual
- [x]  Unit test
- [ ] 💚 E2E
This commit is contained in:
Abin Simon 2023-02-03 10:38:28 +05:30 committed by GitHub
parent 9da0a7878b
commit 76c2ac628b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 90 additions and 5 deletions

View File

@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- When the same user has permissions to a file and the containing - When the same user has permissions to a file and the containing
folder, we only restore folder level permissions for the user and no folder, we only restore folder level permissions for the user and no
separate file only permission is restored. separate file only permission is restored.
- Link shares are not restored
## [v0.2.0] (alpha) - 2023-1-29 ## [v0.2.0] (alpha) - 2023-1-29

View File

@ -292,7 +292,9 @@ func (oc *Collection) populateItems(ctx context.Context) {
itemMeta, itemMetaSize, err = oc.itemMetaReader(ctx, oc.service, oc.driveID, item) itemMeta, itemMetaSize, err = oc.itemMetaReader(ctx, oc.service, oc.driveID, item)
// retry on Timeout type errors, break otherwise. // retry on Timeout type errors, break otherwise.
if err == nil || !graph.IsErrTimeout(err) { if err == nil ||
!graph.IsErrTimeout(err) ||
!graph.IsInternalServerError(err) {
break break
} }
@ -302,7 +304,7 @@ func (oc *Collection) populateItems(ctx context.Context) {
} }
if err != nil { if err != nil {
errUpdater(*item.GetId(), err) errUpdater(*item.GetId(), errors.Wrap(err, "failed to get item permissions"))
return return
} }
} }

View File

@ -191,12 +191,24 @@ func oneDriveItemMetaInfo(
perm, err := service.Client().DrivesById(driveID).ItemsById(*itemID).Permissions().Get(ctx, nil) perm, err := service.Client().DrivesById(driveID).ItemsById(*itemID).Permissions().Get(ctx, nil)
if err != nil { if err != nil {
return Metadata{}, errors.Wrapf(err, "failed to get item permissions %s", *itemID) return Metadata{}, err
} }
uperms := filterUserPermissions(perm.GetValue())
return Metadata{Permissions: uperms}, nil
}
func filterUserPermissions(perms []models.Permissionable) []UserPermission {
up := []UserPermission{} up := []UserPermission{}
for _, p := range perm.GetValue() { for _, p := range perms {
if p.GetGrantedToV2() == nil {
// For link shares, we get permissions without a user
// specified
continue
}
roles := []string{} roles := []string{}
for _, r := range p.GetRoles() { for _, r := range p.GetRoles() {
@ -218,7 +230,7 @@ func oneDriveItemMetaInfo(
}) })
} }
return Metadata{Permissions: up}, nil return up
} }
// sharePointItemInfo will populate a details.SharePointInfo struct // sharePointItemInfo will populate a details.SharePointInfo struct

View File

@ -8,6 +8,7 @@ import (
msgraphsdk "github.com/microsoftgraph/msgraph-sdk-go" msgraphsdk "github.com/microsoftgraph/msgraph-sdk-go"
"github.com/microsoftgraph/msgraph-sdk-go/models" "github.com/microsoftgraph/msgraph-sdk-go/models"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite" "github.com/stretchr/testify/suite"
@ -257,3 +258,72 @@ func (suite *ItemIntegrationSuite) TestDriveGetFolder() {
}) })
} }
} }
func getPermsUperms(permID, userID string, scopes []string) (models.Permissionable, UserPermission) {
identity := models.NewIdentity()
identity.SetAdditionalData(map[string]any{"email": &userID})
sharepointIdentity := models.NewSharePointIdentitySet()
sharepointIdentity.SetUser(identity)
perm := models.NewPermission()
perm.SetId(&permID)
perm.SetRoles([]string{"read"})
perm.SetGrantedToV2(sharepointIdentity)
uperm := UserPermission{
ID: permID,
Roles: []string{"read"},
Email: userID,
}
return perm, uperm
}
func TestOneDrivePermissionsFilter(t *testing.T) {
permID := "fakePermId"
userID := "fakeuser@provider.com"
userID2 := "fakeuser2@provider.com"
readPerm, readUperm := getPermsUperms(permID, userID, []string{"read"})
readWritePerm, readWriteUperm := getPermsUperms(permID, userID2, []string{"read", "write"})
noPerm, _ := getPermsUperms(permID, userID, []string{"read"})
noPerm.SetGrantedToV2(nil) // eg: link shares
cases := []struct {
name string
graphPermissions []models.Permissionable
parsedPermissions []UserPermission
}{
{
name: "no perms",
graphPermissions: []models.Permissionable{},
parsedPermissions: []UserPermission{},
},
{
name: "no user bound to perms",
graphPermissions: []models.Permissionable{noPerm},
parsedPermissions: []UserPermission{},
},
{
name: "user with read permissions",
graphPermissions: []models.Permissionable{readPerm},
parsedPermissions: []UserPermission{readUperm},
},
{
name: "user with read and write permissions",
graphPermissions: []models.Permissionable{readWritePerm},
parsedPermissions: []UserPermission{readWriteUperm},
},
{
name: "multiple users with separate permissions",
graphPermissions: []models.Permissionable{readPerm, readWritePerm},
parsedPermissions: []UserPermission{readUperm, readWriteUperm},
},
}
for _, tc := range cases {
actual := filterUserPermissions(tc.graphPermissions)
assert.ElementsMatch(t, tc.parsedPermissions, actual)
}
}