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

Switch to using ids for storing permissions (#2674)

Browse Source 
This also enable backing up and restoring permissions for groups and applications.

---

#### 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. -->
* fixes https://github.com/alcionai/corso/issues/2621
* fixes https://github.com/alcionai/corso/issues/2673

#### Test Plan

<!-- How will this be tested prior to merging.-->
- [ ] 💪 Manual
- [ ] ⚡ Unit test
- [x] 💚 E2E
This commit is contained in:
Abin Simon 2023-03-07 09:37:37 +05:30 committed by GitHub
parent 030147ddcb
commit 33ecbd1820
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 208 additions and 100 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added
- Show owner information when doing backup list in json format
- Onedrive files that are flagged as malware get skipped during backup.
- Permissions for groups can now be backed up and restored
### Fixed
- Corso-generated .meta files and permissions no longer appear in the backup details.

127
src/internal/connector/graph_connector_onedrive_test.go
View File

@ -20,17 +20,27 @@ import (
"github.com/alcionai/corso/src/pkg/path"
)
func getMetadata(fileName, user string, roles []string) onedrive.Metadata {
if len(user) == 0 || len(roles) == 0 {
// For any version post this(inclusive), we expect to be using IDs for
// permission instead of email
const versionPermissionSwitchedToID = version.OneDrive4DirIncludesPermissions
func getMetadata(fileName string, perm permData, permUseID bool) onedrive.Metadata {
if len(perm.user) == 0 || len(perm.roles) == 0 {
return onedrive.Metadata{FileName: fileName}
}
id := base64.StdEncoding.EncodeToString([]byte(user + strings.Join(roles, "+")))
id := base64.StdEncoding.EncodeToString([]byte(perm.user + strings.Join(perm.roles, "+")))
uperm := onedrive.UserPermission{ID: id, Roles: perm.roles}
if permUseID {
uperm.EntityID = perm.entityID
} else {
uperm.Email = perm.user
}
testMeta := onedrive.Metadata{
FileName: fileName,
Permissions: []onedrive.UserPermission{
{ID: id, Roles: roles, Email: user},
},
Permissions: []onedrive.UserPermission{uperm},
}
return testMeta
@ -66,12 +76,12 @@ func onedriveItemWithData(
func onedriveMetadata(
t *testing.T,
fileName, itemID string,
user string,
roles []string,
perm permData,
permUseID bool,
) itemInfo {
t.Helper()
testMeta := getMetadata(fileName, user, roles)
testMeta := getMetadata(fileName, perm, permUseID)
testMetaJSON, err := json.Marshal(testMeta)
require.NoError(t, err, "marshalling metadata")
@ -87,7 +97,9 @@ type GraphConnectorOneDriveIntegrationSuite struct {
tester.Suite
connector *GraphConnector
user string
userID string
secondaryUser string
secondaryUserID string
acct account.Account
}
@ -110,6 +122,14 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) SetupSuite() {
suite.secondaryUser = tester.SecondaryM365UserID(suite.T())
suite.acct = tester.NewM365Account(suite.T())
user, err := suite.connector.Owners.Users().GetByID(ctx, suite.user)
require.NoErrorf(suite.T(), err, "fetching user %s", suite.user)
suite.userID = *user.GetId()
secondaryUser, err := suite.connector.Owners.Users().GetByID(ctx, suite.secondaryUser)
require.NoErrorf(suite.T(), err, "fetching user %s", suite.secondaryUser)
suite.secondaryUserID = *secondaryUser.GetId()
tester.LogTimeOfTest(suite.T())
}
@ -157,12 +177,7 @@ func (c onedriveCollection) collection() colInfo {
}
}
func (c *onedriveCollection) withFile(
name string,
fileData []byte,
user string,
roles []string,
) *onedriveCollection {
func (c *onedriveCollection) withFile(name string, fileData []byte, perm permData) *onedriveCollection {
switch c.backupVersion {
case 0:
// Lookups will occur using the most recent version of things so we need
@ -184,8 +199,8 @@ func (c *onedriveCollection) withFile(
c.t,
"",
name+onedrive.MetaFileSuffix,
user,
roles)
perm,
c.backupVersion >= versionPermissionSwitchedToID)
c.items = append(c.items, metadata)
c.aux = append(c.aux, metadata)
@ -196,11 +211,7 @@ func (c *onedriveCollection) withFile(
return c
}
func (c *onedriveCollection) withFolder(
name string,
user string,
roles []string,
) *onedriveCollection {
func (c *onedriveCollection) withFolder(name string, perm permData) *onedriveCollection {
switch c.backupVersion {
case 0, version.OneDrive4DirIncludesPermissions:
return c
@ -212,8 +223,8 @@ func (c *onedriveCollection) withFolder(
c.t,
"",
name+onedrive.DirMetaFileSuffix,
user,
roles))
perm,
c.backupVersion >= versionPermissionSwitchedToID))
default:
assert.FailNowf(c.t, "bad backup version", "version %d", c.backupVersion)
@ -224,10 +235,7 @@ func (c *onedriveCollection) withFolder(
// withPermissions adds permissions to the folder represented by this
// onedriveCollection.
func (c *onedriveCollection) withPermissions(
user string,
roles []string,
) *onedriveCollection {
func (c *onedriveCollection) withPermissions(perm permData) *onedriveCollection {
// These versions didn't store permissions for the folder or didn't store them
// in the folder's collection.
if c.backupVersion < version.OneDrive4DirIncludesPermissions {
@ -244,8 +252,8 @@ func (c *onedriveCollection) withPermissions(
c.t,
name,
name+onedrive.DirMetaFileSuffix,
user,
roles)
perm,
c.backupVersion >= versionPermissionSwitchedToID)
c.items = append(c.items, metadata)
c.aux = append(c.aux, metadata)
@ -254,7 +262,8 @@ func (c *onedriveCollection) withPermissions(
}
type permData struct {
user string
user string // user is only for older versions
entityID string
roles []string
}
@ -285,14 +294,14 @@ func testDataForInfo(t *testing.T, cols []onedriveColInfo, backupVersion int) []
onedriveCol := newOneDriveCollection(t, c.pathElements, backupVersion)
for _, f := range c.files {
onedriveCol.withFile(f.name, f.data, f.perms.user, f.perms.roles)
onedriveCol.withFile(f.name, f.data, f.perms)
}
for _, d := range c.folders {
onedriveCol.withFolder(d.name, d.perms.user, d.perms.roles)
onedriveCol.withFolder(d.name, d.perms)
}
onedriveCol.withPermissions(c.perms.user, c.perms.roles)
onedriveCol.withPermissions(c.perms)
res = append(res, onedriveCol.collection())
}
@ -504,6 +513,7 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsRestoreAndBa
data: fileAData,
perms: permData{
user: suite.secondaryUser,
entityID: suite.secondaryUserID,
roles: writePerm,
},
},
@ -522,6 +532,7 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsRestoreAndBa
name: folderAName,
perms: permData{
user: suite.secondaryUser,
entityID: suite.secondaryUserID,
roles: readPerm,
},
},
@ -529,6 +540,7 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsRestoreAndBa
name: folderCName,
perms: permData{
user: suite.secondaryUser,
entityID: suite.secondaryUserID,
roles: readPerm,
},
},
@ -544,6 +556,7 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsRestoreAndBa
data: fileBData,
perms: permData{
user: suite.secondaryUser,
entityID: suite.secondaryUserID,
roles: readPerm,
},
},
@ -553,6 +566,7 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsRestoreAndBa
name: folderAName,
perms: permData{
user: suite.secondaryUser,
entityID: suite.secondaryUserID,
roles: readPerm,
},
},
@ -568,12 +582,14 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsRestoreAndBa
data: fileDData,
perms: permData{
user: suite.secondaryUser,
entityID: suite.secondaryUserID,
roles: readPerm,
},
},
},
perms: permData{
user: suite.secondaryUser,
entityID: suite.secondaryUserID,
roles: readPerm,
},
},
@ -587,12 +603,14 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsRestoreAndBa
data: fileEData,
perms: permData{
user: suite.secondaryUser,
entityID: suite.secondaryUserID,
roles: writePerm,
},
},
},
perms: permData{
user: suite.secondaryUser,
entityID: suite.secondaryUserID,
roles: readPerm,
},
},
@ -608,6 +626,7 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsRestoreAndBa
},
perms: permData{
user: suite.secondaryUser,
entityID: suite.secondaryUserID,
roles: readPerm,
},
},
@ -619,6 +638,9 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsRestoreAndBa
for vn := test.startVersion; vn <= version.Backup; vn++ {
suite.Run(fmt.Sprintf("Version%d", vn), func() {
t := suite.T()
// Ideally this can always be true or false and still
// work, but limiting older versions to use emails so as
// to validate that flow as well.
input := testDataForInfo(t, test.cols, vn)
testData := restoreBackupInfoMultiVersion{
@ -671,6 +693,7 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsBackupAndNoR
data: fileAData,
perms: permData{
user: suite.secondaryUser,
entityID: suite.secondaryUserID,
roles: writePerm,
},
},
@ -742,6 +765,18 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsRestoreAndNo
suite.user,
)
secondaryUserRead := permData{
user: suite.secondaryUser,
entityID: suite.secondaryUserID,
roles: readPerm,
}
secondaryUserWrite := permData{
user: suite.secondaryUser,
entityID: suite.secondaryUserID,
roles: writePerm,
}
test := restoreBackupInfoMultiVersion{
service: path.OneDriveService,
resource: Users,
@ -759,13 +794,11 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsRestoreAndNo
withFile(
fileName,
fileAData,
suite.secondaryUser,
writePerm,
secondaryUserWrite,
).
withFolder(
folderBName,
suite.secondaryUser,
readPerm,
secondaryUserRead,
).
collection(),
newOneDriveCollection(
@ -781,12 +814,10 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsRestoreAndNo
withFile(
fileName,
fileEData,
suite.secondaryUser,
readPerm,
secondaryUserRead,
).
withPermissions(
suite.secondaryUser,
readPerm,
secondaryUserRead,
).
collection(),
},
@ -803,13 +834,11 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsRestoreAndNo
withFile(
fileName,
fileAData,
"",
nil,
permData{},
).
withFolder(
folderBName,
"",
nil,
permData{},
).
collection(),
newOneDriveCollection(
@ -825,14 +854,12 @@ func (suite *GraphConnectorOneDriveIntegrationSuite) TestPermissionsRestoreAndNo
withFile(
fileName,
fileEData,
"",
nil,
permData{},
).
// Call this to generate a meta file with the folder name that we can
// check.
withPermissions(
"",
nil,
permData{},
).
collection(),
},

3
src/internal/connector/onedrive/collection.go
View File

@ -204,7 +204,8 @@ func (oc Collection) DoNotMergeItems() bool {
type UserPermission struct {
ID string `json:"id,omitempty"`
Roles []string `json:"role,omitempty"`
Email string `json:"email,omitempty"`
Email string `json:"email,omitempty"` // DEPRECATED: Replaced with UserID in newer backups
EntityID string `json:"entityId,omitempty"`
Expiration *time.Time `json:"expiration,omitempty"`
}

32
src/internal/connector/onedrive/item.go
View File

@ -237,12 +237,12 @@ func oneDriveItemPermissionInfo(
return nil, err
}
uperms := filterUserPermissions(perm.GetValue())
uperms := filterUserPermissions(ctx, perm.GetValue())
return uperms, nil
}
func filterUserPermissions(perms []models.Permissionable) []UserPermission {
func filterUserPermissions(ctx context.Context, perms []models.Permissionable) []UserPermission {
up := []UserPermission{}
for _, p := range perms {
@ -252,6 +252,7 @@ func filterUserPermissions(perms []models.Permissionable) []UserPermission {
continue
}
gv2 := p.GetGrantedToV2()
roles := []string{}
for _, r := range p.GetRoles() {
@ -265,10 +266,35 @@ func filterUserPermissions(perms []models.Permissionable) []UserPermission {
continue
}
entityID := ""
if gv2.GetUser() != nil {
entityID = *gv2.GetUser().GetId()
} else if gv2.GetGroup() != nil {
entityID = *gv2.GetGroup().GetId()
} else {
// TODO Add appliction 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 {
logm.With("application_id", *gv2.GetApplication().GetId())
}
if gv2.GetDevice() != nil {
logm.With("application_id", *gv2.GetDevice().GetId())
}
logm.Warn("untracked permission")
}
// Technically GrantedToV2 can also contain devices, but the
// documentation does not mention about devices in permissions
if entityID == "" {
// This should ideally not be hit
continue
}
up = append(up, UserPermission{
ID: ptr.Val(p.GetId()),
Roles: roles,
Email: *p.GetGrantedToV2().GetUser().GetAdditionalData()["email"].(*string),
EntityID: entityID,
Expiration: p.GetExpirationDateTime(),
})
}

74
src/internal/connector/onedrive/item_test.go
View File

@ -235,12 +235,23 @@ func (suite *ItemIntegrationSuite) TestDriveGetFolder() {
}
}
func getPermsUperms(permID, userID string, scopes []string) (models.Permissionable, UserPermission) {
func getPermsUperms(permID, userID, entity string, scopes []string) (models.Permissionable, UserPermission) {
identity := models.NewIdentity()
identity.SetId(&userID)
identity.SetAdditionalData(map[string]any{"email": &userID})
sharepointIdentity := models.NewSharePointIdentitySet()
switch entity {
case "user":
sharepointIdentity.SetUser(identity)
case "group":
sharepointIdentity.SetGroup(identity)
case "application":
sharepointIdentity.SetApplication(identity)
case "device":
sharepointIdentity.SetDevice(identity)
}
perm := models.NewPermission()
perm.SetId(&permID)
@ -250,21 +261,32 @@ func getPermsUperms(permID, userID string, scopes []string) (models.Permissionab
uperm := UserPermission{
ID: permID,
Roles: []string{"read"},
Email: userID,
EntityID: userID,
}
return perm, uperm
}
func TestOneDrivePermissionsFilter(t *testing.T) {
type ItemUnitTestSuite struct {
tester.Suite
}
func TestItemUnitTestSuite(t *testing.T) {
suite.Run(t, &ItemUnitTestSuite{Suite: tester.NewUnitSuite(t)})
}
func (suite *ItemUnitTestSuite) TestOneDrivePermissionsFilter() {
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"})
userReadPerm, userReadUperm := getPermsUperms(permID, userID, "user", []string{"read"})
userReadWritePerm, userReadWriteUperm := getPermsUperms(permID, userID2, "user", []string{"read", "write"})
noPerm, _ := getPermsUperms(permID, userID, []string{"read"})
groupReadPerm, groupReadUperm := getPermsUperms(permID, userID, "group", []string{"read"})
groupReadWritePerm, groupReadWriteUperm := getPermsUperms(permID, userID2, "group", []string{"read", "write"})
noPerm, _ := getPermsUperms(permID, userID, "user", []string{"read"})
noPerm.SetGrantedToV2(nil) // eg: link shares
cases := []struct {
@ -282,24 +304,48 @@ func TestOneDrivePermissionsFilter(t *testing.T) {
graphPermissions: []models.Permissionable{noPerm},
parsedPermissions: []UserPermission{},
},
// user
{
name: "user with read permissions",
graphPermissions: []models.Permissionable{readPerm},
parsedPermissions: []UserPermission{readUperm},
graphPermissions: []models.Permissionable{userReadPerm},
parsedPermissions: []UserPermission{userReadUperm},
},
{
name: "user with read and write permissions",
graphPermissions: []models.Permissionable{readWritePerm},
parsedPermissions: []UserPermission{readWriteUperm},
graphPermissions: []models.Permissionable{userReadWritePerm},
parsedPermissions: []UserPermission{userReadWriteUperm},
},
{
name: "multiple users with separate permissions",
graphPermissions: []models.Permissionable{readPerm, readWritePerm},
parsedPermissions: []UserPermission{readUperm, readWriteUperm},
graphPermissions: []models.Permissionable{userReadPerm, userReadWritePerm},
parsedPermissions: []UserPermission{userReadUperm, userReadWriteUperm},
},
// group
{
name: "group with read permissions",
graphPermissions: []models.Permissionable{groupReadPerm},
parsedPermissions: []UserPermission{groupReadUperm},
},
{
name: "group with read and write permissions",
graphPermissions: []models.Permissionable{groupReadWritePerm},
parsedPermissions: []UserPermission{groupReadWriteUperm},
},
{
name: "multiple groups with separate permissions",
graphPermissions: []models.Permissionable{groupReadPerm, groupReadWritePerm},
parsedPermissions: []UserPermission{groupReadUperm, groupReadWriteUperm},
},
}
for _, tc := range cases {
actual := filterUserPermissions(tc.graphPermissions)
assert.ElementsMatch(t, tc.parsedPermissions, actual)
suite.Run(tc.name, func() {
ctx, flush := tester.NewContext()
defer flush()
actual := filterUserPermissions(ctx, tc.graphPermissions)
assert.ElementsMatch(suite.T(), tc.parsedPermissions, actual)
})
}
}

7
src/internal/connector/onedrive/permission.go
View File

@ -210,7 +210,14 @@ func restorePermissions(
pbody.SetRequireSignIn(&rs)
rec := models.NewDriveRecipient()
if p.EntityID != "" {
rec.SetObjectId(&p.EntityID)
} else {
// Previous versions used to only store email for a
// permissions. Use that if id is not found.
rec.SetEmail(&p.Email)
}
pbody.SetRecipients([]models.DriveRecipientable{rec})
np, err := service.Client().DrivesById(driveID).ItemsById(itemID).Invite().Post(ctx, pbody, nil)