larryh/corso
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Filter out non existent entities in permissions and link shares (#4955)

Browse Source 
<!-- PR description-->

---

#### 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
- [ ] 🤖 Supportability/Tests
- [ ] 💻 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.-->
- [ ] 💪 Manual
- [x] ⚡ Unit test
- [ ] 💚 E2E
This commit is contained in:
Abin Simon 2024-01-08 19:14:25 +05:30 committed by GitHub
parent 8067c72904
commit ca50b1e908
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 467 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

100
src/internal/m365/collection/drive/permission.go
View File

@ -397,7 +397,7 @@ func UpdateLinkShares(
newLS, err := upils.PostItemLinkShareUpdate(ictx, driveID, itemID, lsbody)
if graph.IsErrUsersCannotBeResolved(err) {
oldLinkShareIDToNewID.Store(ls.ID, "") // empty to signify that we could not restore
oldLinkShareIDToNewID.Store(ls.ID, nonRestorablePermission)
logger.CtxErr(ictx, err).Info("unable to restore link share")
continue
@ -449,6 +449,102 @@ func UpdateLinkShares(
return alreadyDeleted, nil
}
func filterUnavailableEntitiesInLinkShare(
ctx context.Context,
linkShares []metadata.LinkShare,
availableEntities ResourceIDNames,
oldLinkShareIDToNewID syncd.MapTo[string],
) []metadata.LinkShare {
filtered := []metadata.LinkShare{}
if availableEntities.Users == nil || availableEntities.Groups == nil {
// This should not be happening unless we missed to fill in the caches
logger.Ctx(ctx).Info("no available entities, not filtering link shares")
return linkShares
}
for _, p := range linkShares {
entities := []metadata.Entity{}
for _, e := range p.Entities {
available := false
switch e.EntityType {
case metadata.GV2User:
_, ok := availableEntities.Users.NameOf(e.ID)
available = available || ok
case metadata.GV2Group:
_, ok := availableEntities.Groups.NameOf(e.ID)
available = available || ok
default:
// We only know about users and groups
available = true
}
if available {
entities = append(entities, e)
}
}
if len(entities) > 0 {
p.Entities = entities
filtered = append(filtered, p)
continue
}
// If we have no entities, we can't restore the link share
// and so we have to mark it as not restored.
// This is done to make sure we don't try to delete it later.
oldLinkShareIDToNewID.Store(p.ID, "")
}
return filtered
}
func filterUnavailableEntitiesInPermissions(
ctx context.Context,
perms []metadata.Permission,
availableEntities ResourceIDNames,
oldPermIDToNewID syncd.MapTo[string],
) []metadata.Permission {
if availableEntities.Users == nil || availableEntities.Groups == nil {
// This should not be happening unless we missed to fill in the caches
logger.Ctx(ctx).Info("no available entities, not filtering link shares")
return perms
}
filtered := []metadata.Permission{}
for _, p := range perms {
available := false
switch p.EntityType {
case metadata.GV2User:
_, ok := availableEntities.Users.NameOf(p.EntityID)
available = available || ok
case metadata.GV2Group:
_, ok := availableEntities.Groups.NameOf(p.EntityID)
available = available || ok
default:
// We only know about users and groups
// TODO: extend the check to other entity types
available = true
}
if available {
filtered = append(filtered, p)
continue
}
// If we have no entities, we can't restore the permission
// and so we have to mark it as not restored.
oldPermIDToNewID.Store(p.ID, "")
}
return filtered
}
// RestorePermissions takes in the permissions of an item, computes
// what permissions need to added and removed based on the parent
// folder metas and uses that to add/remove the necessary permissions
@ -478,6 +574,7 @@ func RestorePermissions(
if previousLinkShares != nil {
lsAdded, lsRemoved := metadata.DiffLinkShares(previousLinkShares, current.LinkShares)
lsAdded = filterUnavailableEntitiesInLinkShare(ctx, lsAdded, caches.AvailableEntities, caches.OldLinkShareIDToNewID)
// Link shares have to be updated before permissions as we have to
// use the information about if we had to reset the inheritance to
@ -503,6 +600,7 @@ func RestorePermissions(
}
permAdded, permRemoved := metadata.DiffPermissions(previous.Permissions, current.Permissions)
permAdded = filterUnavailableEntitiesInPermissions(ctx, permAdded, caches.AvailableEntities, caches.OldPermIDToNewID)
if didReset {
// In case we did a reset of permissions when restoring link

258
src/internal/m365/collection/drive/permission_test.go
View File

@ -2,6 +2,7 @@ package drive
import (
"context"
"fmt"
"strings"
"testing"
@ -11,6 +12,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
"github.com/alcionai/corso/src/internal/common/idname"
"github.com/alcionai/corso/src/internal/common/ptr"
"github.com/alcionai/corso/src/internal/common/syncd"
"github.com/alcionai/corso/src/internal/m365/collection/drive/metadata"
@ -326,3 +328,259 @@ func (suite *PermissionsUnitTestSuite) TestLinkShareRestoreNonExistentUser() {
expectedSuccessValues := []bool{true, false, false, false}
assert.Equal(t, expectedSuccessValues, successValues)
}
func (suite *PermissionsUnitTestSuite) TestFilterUnavailableEntitiesInPermissions() {
table := []struct {
name string
permissions []metadata.Permission
expected []metadata.Permission
availableEntities ResourceIDNames
skippedPermissions []string
}{
{
name: "single item",
permissions: []metadata.Permission{
{ID: "p1", EntityID: "e1", EntityType: metadata.GV2User},
},
expected: []metadata.Permission{
{ID: "p1", EntityID: "e1", EntityType: metadata.GV2User},
},
availableEntities: ResourceIDNames{
Users: idname.NewCache(map[string]string{"e1": "e1"}),
Groups: idname.NewCache(map[string]string{}),
},
},
{
name: "single item with missing entity",
permissions: []metadata.Permission{
{ID: "p1", EntityID: "e1", EntityType: metadata.GV2User},
},
expected: []metadata.Permission{},
availableEntities: ResourceIDNames{
Users: idname.NewCache(map[string]string{}),
Groups: idname.NewCache(map[string]string{}),
},
skippedPermissions: []string{"p1"},
},
{
name: "multiple items",
permissions: []metadata.Permission{
{ID: "p1", EntityID: "e1", EntityType: metadata.GV2User},
{ID: "p2", EntityID: "e2", EntityType: metadata.GV2User},
{ID: "p3", EntityID: "e3", EntityType: metadata.GV2User},
},
expected: []metadata.Permission{
{ID: "p1", EntityID: "e1", EntityType: metadata.GV2User},
{ID: "p2", EntityID: "e2", EntityType: metadata.GV2User},
{ID: "p3", EntityID: "e3", EntityType: metadata.GV2User},
},
availableEntities: ResourceIDNames{
Users: idname.NewCache(map[string]string{"e1": "e1", "e2": "e2", "e3": "e3"}),
Groups: idname.NewCache(map[string]string{}),
},
},
{
name: "multiple items with missing entity",
permissions: []metadata.Permission{
{ID: "p1", EntityID: "e1", EntityType: metadata.GV2User},
{ID: "p2", EntityID: "e2", EntityType: metadata.GV2User},
{ID: "p3", EntityID: "e3", EntityType: metadata.GV2Group},
},
expected: []metadata.Permission{
{ID: "p1", EntityID: "e1", EntityType: metadata.GV2User},
{ID: "p3", EntityID: "e3", EntityType: metadata.GV2Group},
},
availableEntities: ResourceIDNames{
Users: idname.NewCache(map[string]string{"e1": "e1"}),
Groups: idname.NewCache(map[string]string{"e3": "e3"}),
},
skippedPermissions: []string{"p2"},
},
{
name: "multiple items with missing entity and multiple types",
permissions: []metadata.Permission{
{ID: "p1", EntityID: "e1", EntityType: metadata.GV2User},
{ID: "p2", EntityID: "e2", EntityType: metadata.GV2User},
{ID: "p3", EntityID: "e3", EntityType: metadata.GV2Group},
{ID: "p4", EntityID: "e4", EntityType: metadata.GV2Group},
{ID: "p5", EntityID: "e5", EntityType: metadata.GV2User},
},
expected: []metadata.Permission{
{ID: "p1", EntityID: "e1", EntityType: metadata.GV2User},
{ID: "p3", EntityID: "e3", EntityType: metadata.GV2Group},
{ID: "p5", EntityID: "e5", EntityType: metadata.GV2User},
},
availableEntities: ResourceIDNames{
Users: idname.NewCache(map[string]string{"e1": "e1", "e5": "e5"}),
Groups: idname.NewCache(map[string]string{"e3": "e3"}),
},
skippedPermissions: []string{"p2", "p4"},
},
{
name: "single item different type",
permissions: []metadata.Permission{
{ID: "p1", EntityID: "e1", EntityType: metadata.GV2Group},
},
expected: []metadata.Permission{},
availableEntities: ResourceIDNames{
Users: idname.NewCache(map[string]string{"e1": "e1"}),
Groups: idname.NewCache(map[string]string{}),
},
skippedPermissions: []string{"p1"},
},
{
name: "unhandled types",
permissions: []metadata.Permission{
{ID: "p1", EntityID: "e1", EntityType: metadata.GV2Device},
{ID: "p2", EntityID: "e2", EntityType: metadata.GV2App},
{ID: "p3", EntityID: "e3", EntityType: metadata.GV2SiteUser},
{ID: "p4", EntityID: "e4", EntityType: metadata.GV2SiteGroup},
},
expected: []metadata.Permission{
{ID: "p1", EntityID: "e1", EntityType: metadata.GV2Device},
{ID: "p2", EntityID: "e2", EntityType: metadata.GV2App},
{ID: "p3", EntityID: "e3", EntityType: metadata.GV2SiteUser},
{ID: "p4", EntityID: "e4", EntityType: metadata.GV2SiteGroup},
},
availableEntities: ResourceIDNames{
// these are users and not what we have
Users: idname.NewCache(map[string]string{"e1": "e1", "e2": "e2", "e3": "e3", "e4": "e4"}),
Groups: idname.NewCache(map[string]string{}),
},
skippedPermissions: []string{},
},
}
for _, test := range table {
suite.Run(test.name, func() {
t := suite.T()
ctx, flush := tester.NewContext(t)
defer flush()
oldToNew := syncd.NewMapTo[string]()
filtered := filterUnavailableEntitiesInPermissions(ctx, test.permissions, test.availableEntities, oldToNew)
assert.Equal(t, test.expected, filtered, "filtered permissions")
for _, id := range test.skippedPermissions {
_, ok := oldToNew.Load(id)
assert.True(t, ok, fmt.Sprintf("skipped id %s", id))
}
})
}
}
type eidtype struct {
id string
etype metadata.GV2Type
}
func (suite *PermissionsUnitTestSuite) TestFilterUnavailableEntitiesInLinkShare() {
ls := func(lsid string, entities []eidtype) metadata.LinkShare {
ents := []metadata.Entity{}
for _, e := range entities {
ents = append(ents, metadata.Entity{ID: e.id, EntityType: e.etype})
}
return metadata.LinkShare{
ID: lsid,
Entities: ents,
}
}
ae := func(uids, gids []string) ResourceIDNames {
users := map[string]string{}
for _, id := range uids {
users[id] = id
}
groups := map[string]string{}
for _, id := range gids {
groups[id] = id
}
return ResourceIDNames{
Users: idname.NewCache(users),
Groups: idname.NewCache(groups),
}
}
table := []struct {
name string
linkShares []metadata.LinkShare
expected []metadata.LinkShare
availableEntities ResourceIDNames
skippedLinkShares []string
}{
{
name: "single item, single available entity",
linkShares: []metadata.LinkShare{ls("ls1", []eidtype{{"e1", metadata.GV2User}})},
expected: []metadata.LinkShare{ls("ls1", []eidtype{{"e1", metadata.GV2User}})},
availableEntities: ae([]string{"e1"}, []string{}),
},
{
name: "single item, single missing entity",
linkShares: []metadata.LinkShare{ls("ls1", []eidtype{{"e1", metadata.GV2User}})},
expected: []metadata.LinkShare{},
availableEntities: ae([]string{}, []string{}),
skippedLinkShares: []string{"ls1"},
},
{
name: "multiple items, multiple available entities",
linkShares: []metadata.LinkShare{
ls("ls1", []eidtype{{"e1", metadata.GV2User}, {"e2", metadata.GV2User}}),
ls("ls2", []eidtype{{"e3", metadata.GV2User}, {"e4", metadata.GV2User}}),
},
expected: []metadata.LinkShare{
ls("ls1", []eidtype{{"e1", metadata.GV2User}, {"e2", metadata.GV2User}}),
ls("ls2", []eidtype{{"e3", metadata.GV2User}, {"e4", metadata.GV2User}}),
},
availableEntities: ae([]string{"e1", "e2", "e3", "e4"}, []string{}),
},
{
name: "multiple items, missing entities",
linkShares: []metadata.LinkShare{
ls("ls1", []eidtype{{"e1", metadata.GV2User}, {"e2", metadata.GV2Group}}),
ls("ls2", []eidtype{{"e3", metadata.GV2User}, {"e4", metadata.GV2Group}}),
},
expected: []metadata.LinkShare{
ls("ls1", []eidtype{{"e1", metadata.GV2User}}),
ls("ls2", []eidtype{{"e4", metadata.GV2Group}}),
},
availableEntities: ae([]string{"e1"}, []string{"e4"}),
skippedLinkShares: []string{},
},
{
name: "unhandled items",
linkShares: []metadata.LinkShare{
ls("ls1", []eidtype{{"e1", metadata.GV2Device}, {"e2", metadata.GV2App}}),
ls("ls2", []eidtype{{"e3", metadata.GV2SiteUser}, {"e4", metadata.GV2SiteGroup}}),
},
expected: []metadata.LinkShare{
ls("ls1", []eidtype{{"e1", metadata.GV2Device}, {"e2", metadata.GV2App}}),
ls("ls2", []eidtype{{"e3", metadata.GV2SiteUser}, {"e4", metadata.GV2SiteGroup}}),
},
availableEntities: ae([]string{}, []string{}),
},
}
for _, test := range table {
suite.Run(test.name, func() {
t := suite.T()
ctx, flush := tester.NewContext(t)
defer flush()
oldToNew := syncd.NewMapTo[string]()
filtered := filterUnavailableEntitiesInLinkShare(ctx, test.linkShares, test.availableEntities, oldToNew)
assert.Equal(t, test.expected, filtered, "filtered link shares")
for _, id := range test.skippedLinkShares {
_, ok := oldToNew.Load(id)
assert.True(t, ok, fmt.Sprintf("skipped id %s", id))
}
})
}
}

26
src/internal/m365/collection/drive/restore_caches.go
View File

@ -11,6 +11,7 @@ import (
"github.com/alcionai/corso/src/internal/common/ptr"
"github.com/alcionai/corso/src/internal/common/syncd"
"github.com/alcionai/corso/src/internal/m365/collection/drive/metadata"
"github.com/alcionai/corso/src/pkg/fault"
"github.com/alcionai/corso/src/pkg/services/m365/api"
"github.com/alcionai/corso/src/pkg/services/m365/api/graph"
)
@ -21,6 +22,11 @@ type driveInfo struct {
rootFolderID string
}
type ResourceIDNames struct {
Users idname.Cacher
Groups idname.Cacher
}
type restoreCaches struct {
BackupDriveIDName idname.Cacher
collisionKeyToItemID map[string]api.DriveItemIDType
@ -30,6 +36,7 @@ type restoreCaches struct {
OldLinkShareIDToNewID syncd.MapTo[string]
OldPermIDToNewID syncd.MapTo[string]
ParentDirToMeta syncd.MapTo[metadata.Metadata]
AvailableEntities ResourceIDNames
pool sync.Pool
}
@ -59,12 +66,18 @@ func (rc *restoreCaches) AddDrive(
return nil
}
type GetAllIDsAndNameser interface {
GetAllIDsAndNames(ctx context.Context, errs *fault.Bus) (idname.Cacher, error)
}
// Populate looks up drive items available to the protectedResource
// and adds their info to the caches.
func (rc *restoreCaches) Populate(
ctx context.Context,
usersGetter, groupsGetter GetAllIDsAndNameser,
gdparf GetDrivePagerAndRootFolderer,
protectedResourceID string,
errs *fault.Bus,
) error {
drives, err := api.GetAllDrives(
ctx,
@ -79,6 +92,19 @@ func (rc *restoreCaches) Populate(
}
}
users, err := usersGetter.GetAllIDsAndNames(ctx, errs)
if err != nil {
return clues.Wrap(err, "getting users")
}
groups, err := groupsGetter.GetAllIDsAndNames(ctx, errs)
if err != nil {
return clues.Wrap(err, "getting groups")
}
rc.AvailableEntities.Users = users
rc.AvailableEntities.Groups = groups
return nil
}

63
src/internal/m365/collection/drive/restore_test.go
View File

@ -428,6 +428,17 @@ func (m *mockGDPARF) NewDrivePager(
return m.pager
}
type mockAllIDsAndNamesGetter struct {
kvs map[string]string
}
func (m mockAllIDsAndNamesGetter) GetAllIDsAndNames(
context.Context,
*fault.Bus,
) (idname.Cacher, error) {
return idname.NewCache(m.kvs), nil
}
func (suite *RestoreUnitSuite) TestRestoreCaches_Populate() {
rfID := "this-is-id"
driveID := "another-id"
@ -443,12 +454,14 @@ func (suite *RestoreUnitSuite) TestRestoreCaches_Populate() {
table := []struct {
name string
mock *apiMock.Pager[models.Driveable]
users map[string]string
groups map[string]string
expectErr require.ErrorAssertionFunc
expectLen int
checkValues bool
}{
{
name: "no results",
name: "no drive results",
mock: &apiMock.Pager[models.Driveable]{
ToReturn: []apiMock.PagerResult[models.Driveable]{
{Values: []models.Driveable{}},
@ -458,7 +471,7 @@ func (suite *RestoreUnitSuite) TestRestoreCaches_Populate() {
expectLen: 0,
},
{
name: "one result",
name: "one drive result",
mock: &apiMock.Pager[models.Driveable]{
ToReturn: []apiMock.PagerResult[models.Driveable]{
{Values: []models.Driveable{md}},
@ -478,6 +491,32 @@ func (suite *RestoreUnitSuite) TestRestoreCaches_Populate() {
expectErr: require.Error,
expectLen: 0,
},
{
name: "multiple users",
mock: &apiMock.Pager[models.Driveable]{
ToReturn: []apiMock.PagerResult[models.Driveable]{
{Values: []models.Driveable{}},
},
},
users: map[string]string{
"1": "one",
"2": "two",
},
expectErr: require.NoError,
},
{
name: "multiple groups",
mock: &apiMock.Pager[models.Driveable]{
ToReturn: []apiMock.PagerResult[models.Driveable]{
{Values: []models.Driveable{}},
},
},
groups: map[string]string{
"1": "one",
"2": "two",
},
expectErr: require.NoError,
},
}
for _, test := range table {
suite.Run(test.name, func() {
@ -491,8 +530,16 @@ func (suite *RestoreUnitSuite) TestRestoreCaches_Populate() {
pager: test.mock,
}
errs := fault.New(true)
rc := NewRestoreCaches(nil)
err := rc.Populate(ctx, gdparf, "shmoo")
err := rc.Populate(
ctx,
mockAllIDsAndNamesGetter{test.users},
mockAllIDsAndNamesGetter{test.groups},
gdparf,
"shmoo",
errs)
test.expectErr(t, err, clues.ToCore(err))
assert.Equal(t, rc.DriveIDToDriveInfo.Size(), test.expectLen)
@ -509,6 +556,16 @@ func (suite *RestoreUnitSuite) TestRestoreCaches_Populate() {
assert.Equal(t, name, nameResult.name, "drive name")
assert.Equal(t, rfID, nameResult.rootFolderID, "root folder id")
}
for key, val := range test.users {
name, _ := rc.AvailableEntities.Users.NameOf(key)
assert.Equal(t, name, val, "user")
}
for key, val := range test.groups {
name, _ := rc.AvailableEntities.Groups.NameOf(key)
assert.Equal(t, name, val, "group")
}
})
}
}

2
src/internal/m365/collection/site/restore.go
View File

@ -46,7 +46,7 @@ func ConsumeRestoreCollections(
el = errs.Local()
)
err := caches.Populate(ctx, lrh, rcc.ProtectedResource.ID())
err := caches.Populate(ctx, ac.Users(), ac.Groups(), lrh, rcc.ProtectedResource.ID(), errs)
if err != nil {
return nil, clues.Wrap(err, "initializing restore caches")
}

2
src/internal/m365/service/groups/restore.go
View File

@ -109,7 +109,7 @@ func (h *groupsHandler) ConsumeRestoreCollections(
Selector: rcc.Selector,
}
err = caches.Populate(ictx, lrh, srcc.ProtectedResource.ID())
err = caches.Populate(ictx, h.apiClient.Users(), h.apiClient.Groups(), lrh, srcc.ProtectedResource.ID(), errs)
if err != nil {
return nil, nil, clues.Wrap(err, "initializing restore caches")
}

2
src/internal/m365/service/onedrive/restore.go
View File

@ -53,7 +53,7 @@ func (h *onedriveHandler) ConsumeRestoreCollections(
ctx = clues.Add(ctx, "backup_version", rcc.BackupVersion)
err := caches.Populate(ctx, rh, rcc.ProtectedResource.ID())
err := caches.Populate(ctx, h.apiClient.Users(), h.apiClient.Groups(), rh, rcc.ProtectedResource.ID(), errs)
if err != nil {
return nil, nil, clues.Wrap(err, "initializing restore caches")
}

2
src/internal/m365/service/sharepoint/restore.go
View File

@ -81,7 +81,7 @@ func (h *sharepointHandler) ConsumeRestoreCollections(
switch dc.FullPath().Category() {
case path.LibrariesCategory:
err = caches.Populate(ctx, lrh, rcc.ProtectedResource.ID())
err = caches.Populate(ctx, h.apiClient.Users(), h.apiClient.Groups(), lrh, rcc.ProtectedResource.ID(), errs)
if err != nil {
return nil, nil, clues.Wrap(err, "initializing restore caches")
}

20
src/pkg/services/m365/api/groups.go
View File

@ -12,6 +12,7 @@ import (
"github.com/microsoftgraph/msgraph-sdk-go/groups"
"github.com/microsoftgraph/msgraph-sdk-go/models"
"github.com/alcionai/corso/src/internal/common/idname"
"github.com/alcionai/corso/src/internal/common/ptr"
"github.com/alcionai/corso/src/internal/common/str"
"github.com/alcionai/corso/src/internal/common/tform"
@ -51,6 +52,25 @@ func (c Groups) GetAll(
return getGroups(ctx, errs, service)
}
// GetAllIDsAndNames retrieves all groups in the tenant and returns them in an idname.Cacher
func (c Groups) GetAllIDsAndNames(ctx context.Context, errs *fault.Bus) (idname.Cacher, error) {
all, err := c.GetAll(ctx, errs)
if err != nil {
return nil, clues.Wrap(err, "getting all users")
}
idToName := make(map[string]string, len(all))
for _, g := range all {
id := strings.ToLower(ptr.Val(g.GetId()))
name := ptr.Val(g.GetDisplayName())
idToName[id] = name
}
return idname.NewCache(idToName), nil
}
// GetAll retrieves all groups.
func getGroups(
ctx context.Context,