move permissions into metadata (#3216)

Moves the UserPermissions struct into onedrive/metadata as
the Permission struct, for better shared support across onedrive,
sharepoint, and any other packages that reference it.

---

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

- [x] ⛔ No

#### Type of change

- [x] 🧹 Tech Debt/Cleanup

#### Issue(s)

* #3135

#### Test Plan

- [x] ⚡ Unit test
- [x] 💚 E2E

src/cmd/factory/impl/common.go

@@ -20,7 +20,6 @@ import (
 	"github.com/alcionai/corso/src/internal/common/ptr"
 	"github.com/alcionai/corso/src/internal/connector"
 	exchMock "github.com/alcionai/corso/src/internal/connector/exchange/mock"
-	"github.com/alcionai/corso/src/internal/connector/onedrive"
 	"github.com/alcionai/corso/src/internal/connector/onedrive/metadata"
 	"github.com/alcionai/corso/src/internal/data"
 	"github.com/alcionai/corso/src/internal/tester"
@@ -204,7 +203,7 @@ type permData struct {
 	user        string // user is only for older versions
 	entityID    string
 	roles       []string
-	sharingMode onedrive.SharingMode
+	sharingMode metadata.SharingMode
 }
 
 type itemData struct {
@@ -672,10 +671,10 @@ func onedriveMetadata(
 	}
 }
 
-func getMetadata(fileName string, perm permData, permUseID bool) onedrive.Metadata {
+func getMetadata(fileName string, perm permData, permUseID bool) metadata.Metadata {
 	if len(perm.user) == 0 || len(perm.roles) == 0 ||
-		perm.sharingMode != onedrive.SharingModeCustom {
+		perm.sharingMode != metadata.SharingModeCustom {
-		return onedrive.Metadata{
+		return metadata.Metadata{
 			FileName:    fileName,
 			SharingMode: perm.sharingMode,
 		}
@@ -685,7 +684,7 @@ func getMetadata(fileName string, perm permData, permUseID bool) onedrive.Metada
 	// 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}
+	uperm := metadata.Permission{ID: id, Roles: perm.roles}
 
 	if permUseID {
 		uperm.EntityID = perm.entityID
@@ -693,9 +692,9 @@ func getMetadata(fileName string, perm permData, permUseID bool) onedrive.Metada
 		uperm.Email = perm.user
 	}
 
-	meta := onedrive.Metadata{
+	meta := metadata.Metadata{
 		FileName:    fileName,
-		Permissions: []onedrive.UserPermission{uperm},
+		Permissions: []metadata.Permission{uperm},
 	}
 
 	return meta

src/internal/connector/graph/metadata/metadata.go

@@ -12,6 +12,7 @@ func IsMetadataFile(p path.Path) bool {
 
 	case path.SharePointService:
 		return p.Category() == path.LibrariesCategory && metadata.HasMetaSuffix(p.Item())
+
 	default:
 		return false
 	}

src/internal/connector/graph_connector_helper_test.go

@@ -693,7 +693,7 @@ func compareExchangeEvent(
 	checkEvent(t, expectedEvent, itemEvent)
 }
 
-func permissionEqual(expected onedrive.UserPermission, got onedrive.UserPermission) bool {
+func permissionEqual(expected metadata.Permission, got metadata.Permission) bool {
 	if !strings.EqualFold(expected.Email, got.Email) {
 		return false
 	}
@@ -769,8 +769,8 @@ func compareOneDriveItem(
 
 	if isMeta {
 		var (
-			itemMeta     onedrive.Metadata
+			itemMeta     metadata.Metadata
-			expectedMeta onedrive.Metadata
+			expectedMeta metadata.Metadata
 		)
 
 		err = json.Unmarshal(buf, &itemMeta)
@@ -812,7 +812,7 @@ func compareOneDriveItem(
 		}
 
 		// We cannot restore owner permissions, so skip checking them
-		itemPerms := []onedrive.UserPermission{}
+		itemPerms := []metadata.Permission{}
 
 		for _, p := range itemMeta.Permissions {
 			if p.Roles[0] != "owner" {

src/internal/connector/graph_connector_onedrive_test.go

@@ -16,7 +16,6 @@ import (
 
 	"github.com/alcionai/corso/src/internal/common/ptr"
 	"github.com/alcionai/corso/src/internal/connector/graph"
-	"github.com/alcionai/corso/src/internal/connector/onedrive"
 	"github.com/alcionai/corso/src/internal/connector/onedrive/metadata"
 	"github.com/alcionai/corso/src/internal/tester"
 	"github.com/alcionai/corso/src/internal/version"
@@ -29,10 +28,10 @@ import (
 // permission instead of email
 const versionPermissionSwitchedToID = version.OneDrive4DirIncludesPermissions
 
-func getMetadata(fileName string, perm permData, permUseID bool) onedrive.Metadata {
+func getMetadata(fileName string, perm permData, permUseID bool) metadata.Metadata {
 	if len(perm.user) == 0 || len(perm.roles) == 0 ||
-		perm.sharingMode != onedrive.SharingModeCustom {
+		perm.sharingMode != metadata.SharingModeCustom {
-		return onedrive.Metadata{
+		return metadata.Metadata{
 			FileName:    fileName,
 			SharingMode: perm.sharingMode,
 		}
@@ -42,7 +41,7 @@ func getMetadata(fileName string, perm permData, permUseID bool) onedrive.Metada
 	// 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}
+	uperm := metadata.Permission{ID: id, Roles: perm.roles}
 
 	if permUseID {
 		uperm.EntityID = perm.entityID
@@ -50,9 +49,9 @@ func getMetadata(fileName string, perm permData, permUseID bool) onedrive.Metada
 		uperm.Email = perm.user
 	}
 
-	testMeta := onedrive.Metadata{
+	testMeta := metadata.Metadata{
 		FileName:    fileName,
-		Permissions: []onedrive.UserPermission{uperm},
+		Permissions: []metadata.Permission{uperm},
 	}
 
 	return testMeta
@@ -278,7 +277,7 @@ type permData struct {
 	user        string // user is only for older versions
 	entityID    string
 	roles       []string
-	sharingMode onedrive.SharingMode
+	sharingMode metadata.SharingMode
 }
 
 type itemData struct {
@@ -1114,21 +1113,21 @@ func testPermissionsInheritanceRestoreAndBackup(suite oneDriveSuite, startVersio
 				user:        secondaryUserName,
 				entityID:    secondaryUserID,
 				roles:       writePerm,
-				sharingMode: onedrive.SharingModeCustom,
+				sharingMode: metadata.SharingModeCustom,
 			},
 		},
 		{
 			name: "file-inherited",
 			data: fileAData,
 			perms: permData{
-				sharingMode: onedrive.SharingModeInherited,
+				sharingMode: metadata.SharingModeInherited,
 			},
 		},
 		{
 			name: "file-empty",
 			data: fileAData,
 			perms: permData{
-				sharingMode: onedrive.SharingModeCustom,
+				sharingMode: metadata.SharingModeCustom,
 			},
 		},
 	}
@@ -1180,21 +1179,21 @@ func testPermissionsInheritanceRestoreAndBackup(suite oneDriveSuite, startVersio
 				user:        tertiaryUserName,
 				entityID:    tertiaryUserID,
 				roles:       writePerm,
-				sharingMode: onedrive.SharingModeCustom,
+				sharingMode: metadata.SharingModeCustom,
 			},
 		},
 		{
 			pathElements: subfolderABPath,
 			files:        fileSet,
 			perms: permData{
-				sharingMode: onedrive.SharingModeInherited,
+				sharingMode: metadata.SharingModeInherited,
 			},
 		},
 		{
 			pathElements: subfolderACPath,
 			files:        fileSet,
 			perms: permData{
-				sharingMode: onedrive.SharingModeCustom,
+				sharingMode: metadata.SharingModeCustom,
 			},
 		},
 	}

src/internal/connector/onedrive/collection.go

@@ -41,13 +41,6 @@ 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
@@ -306,27 +299,6 @@ func (oc Collection) DoNotMergeItems() bool {
 	return oc.doNotMergeItems
 }
 
-// FilePermission is used to store permissions of a specific user to a
-// OneDrive item.
-type UserPermission struct {
-	ID         string     `json:"id,omitempty"`
-	Roles      []string   `json:"role,omitempty"`
-	Email      string     `json:"email,omitempty"` // DEPRECATED: Replaced with UserID in newer backups
-	EntityID   string     `json:"entityId,omitempty"`
-	Expiration *time.Time `json:"expiration,omitempty"`
-}
-
-// ItemMeta contains metadata about the Item. It gets stored in a
-// separate file in kopia
-type Metadata struct {
-	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"`
-}
-
 // Item represents a single item retrieved from OneDrive
 type Item struct {
 	id   string

src/internal/connector/onedrive/collection_test.go

@@ -68,14 +68,16 @@ func (suite *CollectionUnitTestSuite) TestCollection() {
 		testItemName = "itemName"
 		testItemData = []byte("testdata")
 		now          = time.Now()
-		testItemMeta = Metadata{Permissions: []UserPermission{
+		testItemMeta = metadata.Metadata{
+			Permissions: []metadata.Permission{
 				{
 					ID:         "testMetaID",
 					Roles:      []string{"read", "write"},
 					Email:      "email@provider.com",
 					Expiration: &now,
 				},
-		}}
+			},
+		}
 	)
 
 	type nst struct {
@@ -291,7 +293,6 @@ func (suite *CollectionUnitTestSuite) TestCollection() {
 			assert.Equal(t, testItemName, name)
 			assert.Equal(t, driveFolderPath, parentPath)
 
-			if test.source == OneDriveSource {
 			readItemMeta := readItems[1]
 
 			assert.Equal(t, testItemID+metadata.MetaFileSuffix, readItemMeta.UUID())
@@ -305,7 +306,6 @@ func (suite *CollectionUnitTestSuite) TestCollection() {
 			}
 
 			assert.Equal(t, tm, readMetaData)
-			}
 		})
 	}
 }
@@ -516,6 +516,10 @@ func (suite *CollectionUnitTestSuite) TestCollectionPermissionBackupLatestModTim
 			name:   "oneDrive",
 			source: OneDriveSource,
 		},
+		{
+			name:   "sharePoint",
+			source: SharePointSource,
+		},
 	}
 	for _, test := range table {
 		suite.Run(test.name, func() {

src/internal/connector/onedrive/item.go

@@ -15,6 +15,7 @@ import (
 	"github.com/alcionai/corso/src/internal/common/ptr"
 	"github.com/alcionai/corso/src/internal/connector/graph"
 	"github.com/alcionai/corso/src/internal/connector/onedrive/api"
+	"github.com/alcionai/corso/src/internal/connector/onedrive/metadata"
 	"github.com/alcionai/corso/src/internal/connector/uploadsession"
 	"github.com/alcionai/corso/src/pkg/backup/details"
 	"github.com/alcionai/corso/src/pkg/logger"
@@ -73,18 +74,18 @@ func baseItemMetaReader(
 	item models.DriveItemable,
 ) (io.ReadCloser, int, error) {
 	var (
-		perms []UserPermission
+		perms []metadata.Permission
 		err   error
-		meta  = Metadata{FileName: ptr.Val(item.GetName())}
+		meta  = metadata.Metadata{FileName: ptr.Val(item.GetName())}
 	)
 
 	if item.GetShared() == nil {
-		meta.SharingMode = SharingModeInherited
+		meta.SharingMode = metadata.SharingModeInherited
 	} else {
-		meta.SharingMode = SharingModeCustom
+		meta.SharingMode = metadata.SharingModeCustom
 	}
 
-	if meta.SharingMode == SharingModeCustom {
+	if meta.SharingMode == metadata.SharingModeCustom {
 		perms, err = driveItemPermissionInfo(ctx, service, driveID, ptr.Val(item.GetId()))
 		if err != nil {
 			return nil, 0, err
@@ -211,7 +212,7 @@ func driveItemPermissionInfo(
 	service graph.Servicer,
 	driveID string,
 	itemID string,
-) ([]UserPermission, error) {
+) ([]metadata.Permission, error) {
 	perm, err := api.GetItemPermission(ctx, service, driveID, itemID)
 	if err != nil {
 		return nil, err
@@ -222,8 +223,8 @@ func driveItemPermissionInfo(
 	return uperms, nil
 }
 
-func filterUserPermissions(ctx context.Context, perms []models.Permissionable) []UserPermission {
+func filterUserPermissions(ctx context.Context, perms []models.Permissionable) []metadata.Permission {
-	up := []UserPermission{}
+	up := []metadata.Permission{}
 
 	for _, p := range perms {
 		if p.GetGrantedToV2() == nil {
@@ -268,7 +269,7 @@ func filterUserPermissions(ctx context.Context, perms []models.Permissionable) [
 			continue
 		}
 
-		up = append(up, UserPermission{
+		up = append(up, metadata.Permission{
 			ID:         ptr.Val(p.GetId()),
 			Roles:      roles,
 			EntityID:   entityID,

src/internal/connector/onedrive/item_test.go

@@ -16,6 +16,7 @@ import (
 	"github.com/alcionai/corso/src/internal/common/ptr"
 	"github.com/alcionai/corso/src/internal/connector/graph"
 	"github.com/alcionai/corso/src/internal/connector/onedrive/api"
+	"github.com/alcionai/corso/src/internal/connector/onedrive/metadata"
 	"github.com/alcionai/corso/src/internal/tester"
 	"github.com/alcionai/corso/src/pkg/fault"
 )
@@ -247,7 +248,7 @@ func (suite *ItemIntegrationSuite) TestDriveGetFolder() {
 	}
 }
 
-func getPermsUperms(permID, userID, entity string, scopes []string) (models.Permissionable, UserPermission) {
+func getPermsUperms(permID, userID, entity string, scopes []string) (models.Permissionable, metadata.Permission) {
 	identity := models.NewIdentity()
 	identity.SetId(&userID)
 	identity.SetAdditionalData(map[string]any{"email": &userID})
@@ -270,7 +271,7 @@ func getPermsUperms(permID, userID, entity string, scopes []string) (models.Perm
 	perm.SetRoles([]string{"read"})
 	perm.SetGrantedToV2(sharepointIdentity)
 
-	uperm := UserPermission{
+	uperm := metadata.Permission{
 		ID:       permID,
 		Roles:    []string{"read"},
 		EntityID: userID,
@@ -305,56 +306,56 @@ func (suite *ItemUnitTestSuite) TestOneDrivePermissionsFilter() {
 	cases := []struct {
 		name              string
 		graphPermissions  []models.Permissionable
-		parsedPermissions []UserPermission
+		parsedPermissions []metadata.Permission
 	}{
 		{
 			name:              "no perms",
 			graphPermissions:  []models.Permissionable{},
-			parsedPermissions: []UserPermission{},
+			parsedPermissions: []metadata.Permission{},
 		},
 		{
 			name:              "no user bound to perms",
 			graphPermissions:  []models.Permissionable{noPerm},
-			parsedPermissions: []UserPermission{},
+			parsedPermissions: []metadata.Permission{},
 		},
 
 		// user
 		{
 			name:              "user with read permissions",
 			graphPermissions:  []models.Permissionable{userReadPerm},
-			parsedPermissions: []UserPermission{userReadUperm},
+			parsedPermissions: []metadata.Permission{userReadUperm},
 		},
 		{
 			name:              "user with owner permissions",
 			graphPermissions:  []models.Permissionable{userOwnerPerm},
-			parsedPermissions: []UserPermission{userOwnerUperm},
+			parsedPermissions: []metadata.Permission{userOwnerUperm},
 		},
 		{
 			name:              "user with read and write permissions",
 			graphPermissions:  []models.Permissionable{userReadWritePerm},
-			parsedPermissions: []UserPermission{userReadWriteUperm},
+			parsedPermissions: []metadata.Permission{userReadWriteUperm},
 		},
 		{
 			name:              "multiple users with separate permissions",
 			graphPermissions:  []models.Permissionable{userReadPerm, userReadWritePerm},
-			parsedPermissions: []UserPermission{userReadUperm, userReadWriteUperm},
+			parsedPermissions: []metadata.Permission{userReadUperm, userReadWriteUperm},
 		},
 
 		// group
 		{
 			name:              "group with read permissions",
 			graphPermissions:  []models.Permissionable{groupReadPerm},
-			parsedPermissions: []UserPermission{groupReadUperm},
+			parsedPermissions: []metadata.Permission{groupReadUperm},
 		},
 		{
 			name:              "group with read and write permissions",
 			graphPermissions:  []models.Permissionable{groupReadWritePerm},
-			parsedPermissions: []UserPermission{groupReadWriteUperm},
+			parsedPermissions: []metadata.Permission{groupReadWriteUperm},
 		},
 		{
 			name:              "multiple groups with separate permissions",
 			graphPermissions:  []models.Permissionable{groupReadPerm, groupReadWritePerm},
-			parsedPermissions: []UserPermission{groupReadUperm, groupReadWriteUperm},
+			parsedPermissions: []metadata.Permission{groupReadUperm, groupReadWriteUperm},
 		},
 	}
 	for _, tc := range cases {

src/internal/connector/onedrive/metadata/metadata.go

@@ -0,0 +1,12 @@
+package metadata
+
+// ItemMeta contains metadata about the Item. It gets stored in a
+// separate file in kopia
+type Metadata struct {
+	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 []Permission `json:"permissions,omitempty"`
+}

src/internal/connector/onedrive/metadata/permissions.go

@@ -0,0 +1,90 @@
+package metadata
+
+import (
+	"time"
+
+	"golang.org/x/exp/slices"
+)
+
+type SharingMode int
+
+const (
+	SharingModeCustom = SharingMode(iota)
+	SharingModeInherited
+)
+
+// FilePermission is used to store permissions of a specific resource owner
+// to a drive item.
+type Permission struct {
+	ID         string     `json:"id,omitempty"`
+	Roles      []string   `json:"role,omitempty"`
+	Email      string     `json:"email,omitempty"`    // DEPRECATED: Replaced with EntityID in newer backups
+	EntityID   string     `json:"entityId,omitempty"` // this is the resource owner's ID
+	Expiration *time.Time `json:"expiration,omitempty"`
+}
+
+// isSamePermission checks equality of two UserPermission objects
+func (p Permission) Equals(other Permission) bool {
+	// EntityID can be empty for older backups and Email can be empty
+	// for newer ones. It is not possible for both to be empty.  Also,
+	// if EntityID/Email for one is not empty then the other will also
+	// have EntityID/Email as we backup permissions for all the
+	// parents and children when we have a change in permissions.
+	if p.EntityID != "" && p.EntityID != other.EntityID {
+		return false
+	}
+
+	if p.Email != "" && p.Email != other.Email {
+		return false
+	}
+
+	p1r := p.Roles
+	p2r := other.Roles
+
+	slices.Sort(p1r)
+	slices.Sort(p2r)
+
+	return slices.Equal(p1r, p2r)
+}
+
+// DiffPermissions compares the before and after set, returning
+// the permissions that were added and removed (in that order)
+// in the after set.
+func DiffPermissions(before, after []Permission) ([]Permission, []Permission) {
+	var (
+		added   = []Permission{}
+		removed = []Permission{}
+	)
+
+	for _, cp := range after {
+		found := false
+
+		for _, pp := range before {
+			if cp.Equals(pp) {
+				found = true
+				break
+			}
+		}
+
+		if !found {
+			added = append(added, cp)
+		}
+	}
+
+	for _, pp := range before {
+		found := false
+
+		for _, cp := range after {
+			if cp.Equals(pp) {
+				found = true
+				break
+			}
+		}
+
+		if !found {
+			removed = append(removed, pp)
+		}
+	}
+
+	return added, removed
+}

src/internal/connector/onedrive/metadata/permissions_test.go

@@ -0,0 +1,149 @@
+package metadata
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/suite"
+
+	"github.com/alcionai/corso/src/internal/tester"
+)
+
+type PermissionsUnitTestSuite struct {
+	tester.Suite
+}
+
+func TestPermissionsUnitTestSuite(t *testing.T) {
+	suite.Run(t, &PermissionsUnitTestSuite{Suite: tester.NewUnitSuite(t)})
+}
+
+func (suite *PermissionsUnitTestSuite) TestDiffPermissions() {
+	perm1 := Permission{
+		ID:       "id1",
+		Roles:    []string{"read"},
+		EntityID: "user-id1",
+	}
+
+	perm2 := Permission{
+		ID:       "id2",
+		Roles:    []string{"write"},
+		EntityID: "user-id2",
+	}
+
+	perm3 := Permission{
+		ID:       "id3",
+		Roles:    []string{"write"},
+		EntityID: "user-id3",
+	}
+
+	// The following two permissions have same id and user but
+	// different roles, this is a valid scenario for permissions.
+	sameidperm1 := Permission{
+		ID:       "id0",
+		Roles:    []string{"write"},
+		EntityID: "user-id0",
+	}
+	sameidperm2 := Permission{
+		ID:       "id0",
+		Roles:    []string{"read"},
+		EntityID: "user-id0",
+	}
+
+	emailperm1 := Permission{
+		ID:    "id1",
+		Roles: []string{"read"},
+		Email: "email1@provider.com",
+	}
+
+	emailperm2 := Permission{
+		ID:    "id1",
+		Roles: []string{"read"},
+		Email: "email2@provider.com",
+	}
+
+	table := []struct {
+		name    string
+		before  []Permission
+		after   []Permission
+		added   []Permission
+		removed []Permission
+	}{
+		{
+			name:    "single permission added",
+			before:  []Permission{},
+			after:   []Permission{perm1},
+			added:   []Permission{perm1},
+			removed: []Permission{},
+		},
+		{
+			name:    "single permission removed",
+			before:  []Permission{perm1},
+			after:   []Permission{},
+			added:   []Permission{},
+			removed: []Permission{perm1},
+		},
+		{
+			name:    "multiple permission added",
+			before:  []Permission{},
+			after:   []Permission{perm1, perm2},
+			added:   []Permission{perm1, perm2},
+			removed: []Permission{},
+		},
+		{
+			name:    "single permission removed",
+			before:  []Permission{perm1, perm2},
+			after:   []Permission{},
+			added:   []Permission{},
+			removed: []Permission{perm1, perm2},
+		},
+		{
+			name:    "extra permissions",
+			before:  []Permission{perm1, perm2},
+			after:   []Permission{perm1, perm2, perm3},
+			added:   []Permission{perm3},
+			removed: []Permission{},
+		},
+		{
+			name:    "less permissions",
+			before:  []Permission{perm1, perm2, perm3},
+			after:   []Permission{perm1, perm2},
+			added:   []Permission{},
+			removed: []Permission{perm3},
+		},
+		{
+			name:    "same id different role",
+			before:  []Permission{sameidperm1},
+			after:   []Permission{sameidperm2},
+			added:   []Permission{sameidperm2},
+			removed: []Permission{sameidperm1},
+		},
+		{
+			name:    "email based extra permissions",
+			before:  []Permission{emailperm1},
+			after:   []Permission{emailperm1, emailperm2},
+			added:   []Permission{emailperm2},
+			removed: []Permission{},
+		},
+		{
+			name:    "email based less permissions",
+			before:  []Permission{emailperm1, emailperm2},
+			after:   []Permission{emailperm1},
+			added:   []Permission{},
+			removed: []Permission{emailperm2},
+		},
+	}
+
+	for _, test := range table {
+		suite.Run(test.name, func() {
+			_, flush := tester.NewContext()
+			defer flush()
+
+			t := suite.T()
+
+			added, removed := DiffPermissions(test.before, test.after)
+
+			assert.Equal(t, added, test.added, "added permissions")
+			assert.Equal(t, removed, test.removed, "removed permissions")
+		})
+	}
+}

src/internal/connector/onedrive/permission.go

@@ -6,7 +6,6 @@ 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"
@@ -19,20 +18,20 @@ import (
 
 func getParentMetadata(
 	parentPath path.Path,
-	metas map[string]Metadata,
+	metas map[string]metadata.Metadata,
-) (Metadata, error) {
+) (metadata.Metadata, error) {
 	parentMeta, ok := metas[parentPath.String()]
 	if !ok {
 		onedrivePath, err := path.ToOneDrivePath(parentPath)
 		if err != nil {
-			return Metadata{}, clues.Wrap(err, "invalid restore path")
+			return metadata.Metadata{}, clues.Wrap(err, "invalid restore path")
 		}
 
 		if len(onedrivePath.Folders) != 0 {
-			return Metadata{}, clues.Wrap(err, "computing item permissions")
+			return metadata.Metadata{}, clues.Wrap(err, "computing item permissions")
 		}
 
-		parentMeta = Metadata{}
+		parentMeta = metadata.Metadata{}
 	}
 
 	return parentMeta, nil
@@ -42,12 +41,12 @@ func getCollectionMetadata(
 	ctx context.Context,
 	drivePath *path.DrivePath,
 	dc data.RestoreCollection,
-	metas map[string]Metadata,
+	metas map[string]metadata.Metadata,
 	backupVersion int,
 	restorePerms bool,
-) (Metadata, error) {
+) (metadata.Metadata, error) {
 	if !restorePerms || backupVersion < version.OneDrive1DataAndMetaFiles {
-		return Metadata{}, nil
+		return metadata.Metadata{}, nil
 	}
 
 	var (
@@ -57,13 +56,13 @@ func getCollectionMetadata(
 
 	if len(drivePath.Folders) == 0 {
 		// No permissions for root folder
-		return Metadata{}, nil
+		return metadata.Metadata{}, nil
 	}
 
 	if backupVersion < version.OneDrive4DirIncludesPermissions {
 		colMeta, err := getParentMetadata(collectionPath, metas)
 		if err != nil {
-			return Metadata{}, clues.Wrap(err, "collection metadata")
+			return metadata.Metadata{}, clues.Wrap(err, "collection metadata")
 		}
 
 		return colMeta, nil
@@ -79,83 +78,20 @@ func getCollectionMetadata(
 
 	meta, err := fetchAndReadMetadata(ctx, dc, metaName)
 	if err != nil {
-		return Metadata{}, clues.Wrap(err, "collection metadata")
+		return metadata.Metadata{}, clues.Wrap(err, "collection metadata")
 	}
 
 	return meta, nil
 }
 
-// isSamePermission checks equality of two UserPermission objects
-func isSamePermission(p1, p2 UserPermission) bool {
-	// EntityID can be empty for older backups and Email can be empty
-	// for newer ones. It is not possible for both to be empty.  Also,
-	// if EntityID/Email for one is not empty then the other will also
-	// have EntityID/Email as we backup permissions for all the
-	// parents and children when we have a change in permissions.
-	if p1.EntityID != "" && p1.EntityID != p2.EntityID {
-		return false
-	}
-
-	if p1.Email != "" && p1.Email != p2.Email {
-		return false
-	}
-
-	p1r := p1.Roles
-	p2r := p2.Roles
-
-	slices.Sort(p1r)
-	slices.Sort(p2r)
-
-	return slices.Equal(p1r, p2r)
-}
-
-func diffPermissions(before, after []UserPermission) ([]UserPermission, []UserPermission) {
-	var (
-		added   = []UserPermission{}
-		removed = []UserPermission{}
-	)
-
-	for _, cp := range after {
-		found := false
-
-		for _, pp := range before {
-			if isSamePermission(cp, pp) {
-				found = true
-				break
-			}
-		}
-
-		if !found {
-			added = append(added, cp)
-		}
-	}
-
-	for _, pp := range before {
-		found := false
-
-		for _, cp := range after {
-			if isSamePermission(cp, pp) {
-				found = true
-				break
-			}
-		}
-
-		if !found {
-			removed = append(removed, pp)
-		}
-	}
-
-	return added, removed
-}
-
 // 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) {
+func computeParentPermissions(itemPath path.Path, folderMetas map[string]metadata.Metadata) (metadata.Metadata, error) {
 	var (
 		parent path.Path
-		meta   Metadata
+		meta   metadata.Metadata
 
 		err error
 		ok  bool
@@ -166,24 +102,24 @@ func computeParentPermissions(itemPath path.Path, folderMetas map[string]Metadat
 	for {
 		parent, err = parent.Dir()
 		if err != nil {
-			return Metadata{}, clues.New("getting parent")
+			return metadata.Metadata{}, clues.New("getting parent")
 		}
 
 		onedrivePath, err := path.ToOneDrivePath(parent)
 		if err != nil {
-			return Metadata{}, clues.New("get parent path")
+			return metadata.Metadata{}, clues.New("get parent path")
 		}
 
 		if len(onedrivePath.Folders) == 0 {
-			return Metadata{}, nil
+			return metadata.Metadata{}, nil
 		}
 
 		meta, ok = folderMetas[parent.String()]
 		if !ok {
-			return Metadata{}, clues.New("no parent meta")
+			return metadata.Metadata{}, clues.New("no parent meta")
 		}
 
-		if meta.SharingMode == SharingModeCustom {
+		if meta.SharingMode == metadata.SharingModeCustom {
 			return meta, nil
 		}
 	}
@@ -197,7 +133,7 @@ func UpdatePermissions(
 	service graph.Servicer,
 	driveID string,
 	itemID string,
-	permAdded, permRemoved []UserPermission,
+	permAdded, permRemoved []metadata.Permission,
 	permissionIDMappings map[string]string,
 ) error {
 	// The ordering of the operations is important here. We first
@@ -290,11 +226,11 @@ func RestorePermissions(
 	driveID string,
 	itemID string,
 	itemPath path.Path,
-	meta Metadata,
+	meta metadata.Metadata,
-	folderMetas map[string]Metadata,
+	folderMetas map[string]metadata.Metadata,
 	permissionIDMappings map[string]string,
 ) error {
-	if meta.SharingMode == SharingModeInherited {
+	if meta.SharingMode == metadata.SharingModeInherited {
 		return nil
 	}
 
@@ -305,7 +241,7 @@ func RestorePermissions(
 		return clues.Wrap(err, "parent permissions").WithClues(ctx)
 	}
 
-	permAdded, permRemoved := diffPermissions(parentPermissions.Permissions, meta.Permissions)
+	permAdded, permRemoved := metadata.DiffPermissions(parentPermissions.Permissions, meta.Permissions)
 
 	return UpdatePermissions(ctx, creds, service, driveID, itemID, permAdded, permRemoved, permissionIDMappings)
 }

src/internal/connector/onedrive/permission_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 
+	"github.com/alcionai/corso/src/internal/connector/onedrive/metadata"
 	"github.com/alcionai/corso/src/internal/tester"
 	"github.com/alcionai/corso/src/pkg/path"
 )
@@ -54,9 +55,9 @@ func (suite *PermissionsUnitTestSuite) TestComputeParentPermissions() {
 	level0, err := level1.Dir()
 	require.NoError(suite.T(), err, "level0 path")
 
-	metadata := Metadata{
+	md := metadata.Metadata{
-		SharingMode: SharingModeCustom,
+		SharingMode: metadata.SharingModeCustom,
-		Permissions: []UserPermission{
+		Permissions: []metadata.Permission{
 			{
 				Roles:    []string{"write"},
 				EntityID: "user-id",
@@ -64,9 +65,9 @@ func (suite *PermissionsUnitTestSuite) TestComputeParentPermissions() {
 		},
 	}
 
-	metadata2 := Metadata{
+	metadata2 := metadata.Metadata{
-		SharingMode: SharingModeCustom,
+		SharingMode: metadata.SharingModeCustom,
-		Permissions: []UserPermission{
+		Permissions: []metadata.Permission{
 			{
 				Roles:    []string{"read"},
 				EntityID: "user-id",
@@ -74,52 +75,52 @@ func (suite *PermissionsUnitTestSuite) TestComputeParentPermissions() {
 		},
 	}
 
-	inherited := Metadata{
+	inherited := metadata.Metadata{
-		SharingMode: SharingModeInherited,
+		SharingMode: metadata.SharingModeInherited,
-		Permissions: []UserPermission{},
+		Permissions: []metadata.Permission{},
 	}
 
 	table := []struct {
 		name        string
 		item        path.Path
-		meta        Metadata
+		meta        metadata.Metadata
-		parentPerms map[string]Metadata
+		parentPerms map[string]metadata.Metadata
 	}{
 		{
 			name:        "root level entry",
 			item:        rootEntry,
-			meta:        Metadata{},
+			meta:        metadata.Metadata{},
-			parentPerms: map[string]Metadata{},
+			parentPerms: map[string]metadata.Metadata{},
 		},
 		{
 			name:        "root level directory",
 			item:        level0,
-			meta:        Metadata{},
+			meta:        metadata.Metadata{},
-			parentPerms: map[string]Metadata{},
+			parentPerms: map[string]metadata.Metadata{},
 		},
 		{
 			name: "direct parent perms",
 			item: entry,
-			meta: metadata,
+			meta: md,
-			parentPerms: map[string]Metadata{
+			parentPerms: map[string]metadata.Metadata{
-				level2.String(): metadata,
+				level2.String(): md,
 			},
 		},
 		{
 			name: "top level parent perms",
 			item: entry,
-			meta: metadata,
+			meta: md,
-			parentPerms: map[string]Metadata{
+			parentPerms: map[string]metadata.Metadata{
 				level2.String(): inherited,
 				level1.String(): inherited,
-				level0.String(): metadata,
+				level0.String(): md,
 			},
 		},
 		{
 			name: "all inherited",
 			item: entry,
-			meta: Metadata{},
+			meta: metadata.Metadata{},
-			parentPerms: map[string]Metadata{
+			parentPerms: map[string]metadata.Metadata{
 				level2.String(): inherited,
 				level1.String(): inherited,
 				level0.String(): inherited,
@@ -128,10 +129,10 @@ func (suite *PermissionsUnitTestSuite) TestComputeParentPermissions() {
 		{
 			name: "multiple custom permission",
 			item: entry,
-			meta: metadata,
+			meta: md,
-			parentPerms: map[string]Metadata{
+			parentPerms: map[string]metadata.Metadata{
 				level2.String(): inherited,
-				level1.String(): metadata,
+				level1.String(): md,
 				level0.String(): metadata2,
 			},
 		},
@@ -151,134 +152,3 @@ func (suite *PermissionsUnitTestSuite) TestComputeParentPermissions() {
 		})
 	}
 }
-
-func (suite *PermissionsUnitTestSuite) TestDiffPermissions() {
-	perm1 := UserPermission{
-		ID:       "id1",
-		Roles:    []string{"read"},
-		EntityID: "user-id1",
-	}
-
-	perm2 := UserPermission{
-		ID:       "id2",
-		Roles:    []string{"write"},
-		EntityID: "user-id2",
-	}
-
-	perm3 := UserPermission{
-		ID:       "id3",
-		Roles:    []string{"write"},
-		EntityID: "user-id3",
-	}
-
-	// The following two permissions have same id and user but
-	// different roles, this is a valid scenario for permissions.
-	sameidperm1 := UserPermission{
-		ID:       "id0",
-		Roles:    []string{"write"},
-		EntityID: "user-id0",
-	}
-	sameidperm2 := UserPermission{
-		ID:       "id0",
-		Roles:    []string{"read"},
-		EntityID: "user-id0",
-	}
-
-	emailperm1 := UserPermission{
-		ID:    "id1",
-		Roles: []string{"read"},
-		Email: "email1@provider.com",
-	}
-
-	emailperm2 := UserPermission{
-		ID:    "id1",
-		Roles: []string{"read"},
-		Email: "email2@provider.com",
-	}
-
-	table := []struct {
-		name    string
-		before  []UserPermission
-		after   []UserPermission
-		added   []UserPermission
-		removed []UserPermission
-	}{
-		{
-			name:    "single permission added",
-			before:  []UserPermission{},
-			after:   []UserPermission{perm1},
-			added:   []UserPermission{perm1},
-			removed: []UserPermission{},
-		},
-		{
-			name:    "single permission removed",
-			before:  []UserPermission{perm1},
-			after:   []UserPermission{},
-			added:   []UserPermission{},
-			removed: []UserPermission{perm1},
-		},
-		{
-			name:    "multiple permission added",
-			before:  []UserPermission{},
-			after:   []UserPermission{perm1, perm2},
-			added:   []UserPermission{perm1, perm2},
-			removed: []UserPermission{},
-		},
-		{
-			name:    "single permission removed",
-			before:  []UserPermission{perm1, perm2},
-			after:   []UserPermission{},
-			added:   []UserPermission{},
-			removed: []UserPermission{perm1, perm2},
-		},
-		{
-			name:    "extra permissions",
-			before:  []UserPermission{perm1, perm2},
-			after:   []UserPermission{perm1, perm2, perm3},
-			added:   []UserPermission{perm3},
-			removed: []UserPermission{},
-		},
-		{
-			name:    "less permissions",
-			before:  []UserPermission{perm1, perm2, perm3},
-			after:   []UserPermission{perm1, perm2},
-			added:   []UserPermission{},
-			removed: []UserPermission{perm3},
-		},
-		{
-			name:    "same id different role",
-			before:  []UserPermission{sameidperm1},
-			after:   []UserPermission{sameidperm2},
-			added:   []UserPermission{sameidperm2},
-			removed: []UserPermission{sameidperm1},
-		},
-		{
-			name:    "email based extra permissions",
-			before:  []UserPermission{emailperm1},
-			after:   []UserPermission{emailperm1, emailperm2},
-			added:   []UserPermission{emailperm2},
-			removed: []UserPermission{},
-		},
-		{
-			name:    "email based less permissions",
-			before:  []UserPermission{emailperm1, emailperm2},
-			after:   []UserPermission{emailperm1},
-			added:   []UserPermission{},
-			removed: []UserPermission{emailperm2},
-		},
-	}
-
-	for _, test := range table {
-		suite.Run(test.name, func() {
-			_, flush := tester.NewContext()
-			defer flush()
-
-			t := suite.T()
-
-			added, removed := diffPermissions(test.before, test.after)
-
-			assert.Equal(t, added, test.added, "added permissions")
-			assert.Equal(t, removed, test.removed, "removed permissions")
-		})
-	}
-}

src/internal/connector/onedrive/restore.go

@@ -48,7 +48,7 @@ func RestoreCollections(
 	var (
 		restoreMetrics support.CollectionMetrics
 		metrics        support.CollectionMetrics
-		folderMetas    = map[string]Metadata{}
+		folderMetas    = map[string]metadata.Metadata{}
 
 		// permissionIDMappings is used to map between old and new id
 		// of permissions as we restore them
@@ -132,7 +132,7 @@ func RestoreCollection(
 	backupVersion int,
 	service graph.Servicer,
 	dc data.RestoreCollection,
-	folderMetas map[string]Metadata,
+	folderMetas map[string]metadata.Metadata,
 	permissionIDMappings map[string]string,
 	fc *folderCache,
 	rootIDCache map[string]string, // map of drive id -> root folder ID
@@ -298,7 +298,7 @@ func restoreItem(
 	drivePath *path.DrivePath,
 	restoreFolderID string,
 	copyBuffer []byte,
-	folderMetas map[string]Metadata,
+	folderMetas map[string]metadata.Metadata,
 	permissionIDMappings map[string]string,
 	restorePerms bool,
 	itemData data.Stream,
@@ -439,7 +439,7 @@ func restoreV1File(
 	restoreFolderID string,
 	copyBuffer []byte,
 	restorePerms bool,
-	folderMetas map[string]Metadata,
+	folderMetas map[string]metadata.Metadata,
 	permissionIDMappings map[string]string,
 	itemPath path.Path,
 	itemData data.Stream,
@@ -500,7 +500,7 @@ func restoreV6File(
 	restoreFolderID string,
 	copyBuffer []byte,
 	restorePerms bool,
-	folderMetas map[string]Metadata,
+	folderMetas map[string]metadata.Metadata,
 	permissionIDMappings map[string]string,
 	itemPath path.Path,
 	itemData data.Stream,
@@ -575,8 +575,8 @@ func createRestoreFoldersWithPermissions(
 	driveRootID string,
 	restoreFolders *path.Builder,
 	folderPath path.Path,
-	folderMetadata Metadata,
+	folderMetadata metadata.Metadata,
-	folderMetas map[string]Metadata,
+	folderMetas map[string]metadata.Metadata,
 	fc *folderCache,
 	permissionIDMappings map[string]string,
 	restorePerms bool,
@@ -741,11 +741,11 @@ func fetchAndReadMetadata(
 	ctx context.Context,
 	fetcher fileFetcher,
 	metaName string,
-) (Metadata, error) {
+) (metadata.Metadata, error) {
 	metaFile, err := fetcher.Fetch(ctx, metaName)
 	if err != nil {
 		err = clues.Wrap(err, "getting item metadata").With("meta_file_name", metaName)
-		return Metadata{}, err
+		return metadata.Metadata{}, err
 	}
 
 	metaReader := metaFile.ToReader()
@@ -754,25 +754,25 @@ func fetchAndReadMetadata(
 	meta, err := getMetadata(metaReader)
 	if err != nil {
 		err = clues.Wrap(err, "deserializing item metadata").With("meta_file_name", metaName)
-		return Metadata{}, err
+		return metadata.Metadata{}, err
 	}
 
 	return meta, nil
 }
 
 // getMetadata read and parses the metadata info for an item
-func getMetadata(metar io.ReadCloser) (Metadata, error) {
+func getMetadata(metar io.ReadCloser) (metadata.Metadata, error) {
-	var meta Metadata
+	var meta metadata.Metadata
 	// `metar` will be nil for the top level container folder
 	if metar != nil {
 		metaraw, err := io.ReadAll(metar)
 		if err != nil {
-			return Metadata{}, err
+			return metadata.Metadata{}, err
 		}
 
 		err = json.Unmarshal(metaraw, &meta)
 		if err != nil {
-			return Metadata{}, err
+			return metadata.Metadata{}, err
 		}
 	}
 

src/internal/connector/sharepoint/restore.go

@@ -12,6 +12,7 @@ import (
 	"github.com/alcionai/corso/src/internal/common/ptr"
 	"github.com/alcionai/corso/src/internal/connector/graph"
 	"github.com/alcionai/corso/src/internal/connector/onedrive"
+	"github.com/alcionai/corso/src/internal/connector/onedrive/metadata"
 	"github.com/alcionai/corso/src/internal/connector/sharepoint/api"
 	"github.com/alcionai/corso/src/internal/connector/support"
 	"github.com/alcionai/corso/src/internal/data"
@@ -74,7 +75,7 @@ func RestoreCollections(
 				backupVersion,
 				service,
 				dc,
-				map[string]onedrive.Metadata{}, // Currently permission data is not stored for sharepoint
+				map[string]metadata.Metadata{}, // Currently permission data is not stored for sharepoint
 				map[string]string{},
 				driveFolderCache,
 				nil,

src/internal/operations/backup_integration_test.go

@@ -27,6 +27,7 @@ import (
 	"github.com/alcionai/corso/src/internal/connector/graph"
 	"github.com/alcionai/corso/src/internal/connector/mock"
 	"github.com/alcionai/corso/src/internal/connector/onedrive"
+	"github.com/alcionai/corso/src/internal/connector/onedrive/metadata"
 	"github.com/alcionai/corso/src/internal/connector/support"
 	"github.com/alcionai/corso/src/internal/data"
 	"github.com/alcionai/corso/src/internal/events"
@@ -1168,11 +1169,13 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_oneDriveIncrementals() {
 	runDriveIncrementalTest(
 		suite,
 		suite.user,
+		suite.user,
 		connector.Users,
 		path.OneDriveService,
 		path.FilesCategory,
 		ic,
-		gtdi)
+		gtdi,
+		false)
 }
 
 func (suite *BackupOpIntegrationSuite) TestBackup_Run_sharePointIncrementals() {
@@ -1205,21 +1208,24 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_sharePointIncrementals() {
 	runDriveIncrementalTest(
 		suite,
 		suite.site,
+		suite.user,
 		connector.Sites,
 		path.SharePointService,
 		path.LibrariesCategory,
 		ic,
-		gtdi)
+		gtdi,
+		true)
 }
 
 func runDriveIncrementalTest(
 	suite *BackupOpIntegrationSuite,
-	owner string,
+	owner, permissionsUser string,
 	resource connector.Resource,
 	service path.ServiceType,
 	category path.CategoryType,
 	includeContainers func([]string) selectors.Selector,
 	getTestDriveID func(*testing.T, context.Context, graph.Servicer) string,
+	skipPermissionsTests bool,
 ) {
 	ctx, flush := tester.NewContext()
 	defer flush()
@@ -1310,10 +1316,10 @@ func runDriveIncrementalTest(
 		newFileName = "new_file.txt"
 
 		permissionIDMappings = map[string]string{}
-		writePerm            = onedrive.UserPermission{
+		writePerm            = metadata.Permission{
 			ID:       "perm-id",
 			Roles:    []string{"write"},
-			EntityID: owner,
+			EntityID: permissionsUser,
 		}
 	)
 
@@ -1348,14 +1354,14 @@ func runDriveIncrementalTest(
 					driveID,
 					targetContainer,
 					driveItem)
-				require.NoError(t, err, "creating new file", clues.ToCore(err))
+				require.NoErrorf(t, err, "creating new file %v", clues.ToCore(err))
 			},
 			itemsRead:    1, // .data file for newitem
 			itemsWritten: 3, // .data and .meta for newitem, .dirmeta for parent
 		},
 		{
 			name: "add permission to new file",
-			skip: service == path.SharePointService,
+			skip: skipPermissionsTests,
 			updateFiles: func(t *testing.T) {
 				driveItem := models.NewDriveItem()
 				driveItem.SetName(&newFileName)
@@ -1366,18 +1372,17 @@ func runDriveIncrementalTest(
 					gc.Service,
 					driveID,
 					*newFile.GetId(),
-					[]onedrive.UserPermission{writePerm},
+					[]metadata.Permission{writePerm},
-					[]onedrive.UserPermission{},
+					[]metadata.Permission{},
-					permissionIDMappings,
+					permissionIDMappings)
-				)
+				require.NoErrorf(t, err, "adding permission to file %v", clues.ToCore(err))
-				require.NoErrorf(t, err, "add permission to file %v", clues.ToCore(err))
 			},
 			itemsRead:    1, // .data file for newitem
 			itemsWritten: 2, // .meta for newitem, .dirmeta for parent (.data is not written as it is not updated)
 		},
 		{
 			name: "remove permission from new file",
-			skip: service == path.SharePointService,
+			skip: skipPermissionsTests,
 			updateFiles: func(t *testing.T) {
 				driveItem := models.NewDriveItem()
 				driveItem.SetName(&newFileName)
@@ -1388,18 +1393,17 @@ func runDriveIncrementalTest(
 					gc.Service,
 					driveID,
 					*newFile.GetId(),
-					[]onedrive.UserPermission{},
+					[]metadata.Permission{},
-					[]onedrive.UserPermission{writePerm},
+					[]metadata.Permission{writePerm},
-					permissionIDMappings,
+					permissionIDMappings)
-				)
+				require.NoErrorf(t, err, "adding permission to file %v", clues.ToCore(err))
-				require.NoError(t, err, "add permission to file", clues.ToCore(err))
 			},
 			itemsRead:    1, // .data file for newitem
 			itemsWritten: 2, // .meta for newitem, .dirmeta for parent (.data is not written as it is not updated)
 		},
 		{
 			name: "add permission to container",
-			skip: service == path.SharePointService,
+			skip: skipPermissionsTests,
 			updateFiles: func(t *testing.T) {
 				targetContainer := containerIDs[container1]
 				driveItem := models.NewDriveItem()
@@ -1411,18 +1415,17 @@ func runDriveIncrementalTest(
 					gc.Service,
 					driveID,
 					targetContainer,
-					[]onedrive.UserPermission{writePerm},
+					[]metadata.Permission{writePerm},
-					[]onedrive.UserPermission{},
+					[]metadata.Permission{},
-					permissionIDMappings,
+					permissionIDMappings)
-				)
+				require.NoErrorf(t, err, "adding permission to file %v", clues.ToCore(err))
-				require.NoError(t, err, "add permission to file", clues.ToCore(err))
 			},
 			itemsRead:    0,
 			itemsWritten: 1, // .dirmeta for collection
 		},
 		{
 			name: "remove permission from container",
-			skip: service == path.SharePointService,
+			skip: skipPermissionsTests,
 			updateFiles: func(t *testing.T) {
 				targetContainer := containerIDs[container1]
 				driveItem := models.NewDriveItem()
@@ -1434,11 +1437,10 @@ func runDriveIncrementalTest(
 					gc.Service,
 					driveID,
 					targetContainer,
-					[]onedrive.UserPermission{},
+					[]metadata.Permission{},
-					[]onedrive.UserPermission{writePerm},
+					[]metadata.Permission{writePerm},
-					permissionIDMappings,
+					permissionIDMappings)
-				)
+				require.NoErrorf(t, err, "adding permission to file %v", clues.ToCore(err))
-				require.NoError(t, err, "add permission to file", clues.ToCore(err))
 			},
 			itemsRead:    0,
 			itemsWritten: 1, // .dirmeta for collection
@@ -1452,7 +1454,7 @@ func runDriveIncrementalTest(
 					ItemsById(ptr.Val(newFile.GetId())).
 					Content().
 					Put(ctx, []byte("new content"), nil)
-				require.NoError(t, err, "updating file content")
+				require.NoErrorf(t, err, "updating file contents: %v", clues.ToCore(err))
 			},
 			itemsRead:    1, // .data file for newitem
 			itemsWritten: 3, // .data and .meta for newitem, .dirmeta for parent
@@ -1474,7 +1476,7 @@ func runDriveIncrementalTest(
 					DrivesById(driveID).
 					ItemsById(ptr.Val(newFile.GetId())).
 					Patch(ctx, driveItem, nil)
-				require.NoError(t, err, "renaming file", clues.ToCore(err))
+				require.NoError(t, err, "renaming file %v", clues.ToCore(err))
 			},
 			itemsRead:    1, // .data file for newitem
 			itemsWritten: 3, // .data and .meta for newitem, .dirmeta for parent
@@ -1495,7 +1497,7 @@ func runDriveIncrementalTest(
 					DrivesById(driveID).
 					ItemsById(ptr.Val(newFile.GetId())).
 					Patch(ctx, driveItem, nil)
-				require.NoError(t, err, "moving file between folders", clues.ToCore(err))
+				require.NoErrorf(t, err, "moving file between folders %v", clues.ToCore(err))
 			},
 			itemsRead:    1, // .data file for newitem
 			itemsWritten: 3, // .data and .meta for newitem, .dirmeta for parent
@@ -1510,7 +1512,7 @@ func runDriveIncrementalTest(
 					DrivesById(driveID).
 					ItemsById(ptr.Val(newFile.GetId())).
 					Delete(ctx, nil)
-				require.NoError(t, err, "deleting file", clues.ToCore(err))
+				require.NoErrorf(t, err, "deleting file %v", clues.ToCore(err))
 			},
 			itemsRead:    0,
 			itemsWritten: 0,
@@ -1609,9 +1611,8 @@ func runDriveIncrementalTest(
 	}
 	for _, test := range table {
 		suite.Run(test.name, func() {
-			// TODO(rkeepers): remove when sharepoint supports permission.
 			if test.skip {
-				return
+				suite.T().Skip("flagged to skip")
 			}
 
 			cleanGC, err := connector.NewGraphConnector(ctx, acct, resource)