From 68dc79eddd12b417bcf943a4708d9ffd3224aa10 Mon Sep 17 00:00:00 2001 From: nielash Date: Sun, 21 Apr 2024 21:39:15 -0400 Subject: [PATCH] onedrive: fix references to deprecated permissions properties Before this change, metadata permissions used the `grantedTo` and `grantedToIdentities` properties, which are deprecated on OneDrive Business in favor of `grantedToV2` and `grantedToIdentitiesV2`. After this change, OneDrive Business uses the new V2 versions, while OneDrive Personal still uses the originals, as the V2 versions are not available for OneDrive Personal. (see https://learn.microsoft.com/en-us/answers/questions/1079737/inconsistency-between-grantedtov2-and-grantedto-re) --- backend/onedrive/api/types.go | 40 +++++++++++++++++----- backend/onedrive/metadata.go | 12 +++---- backend/onedrive/onedrive_internal_test.go | 40 ++++++++++++++-------- docs/content/onedrive.md | 14 ++++---- 4 files changed, 71 insertions(+), 35 deletions(-) diff --git a/backend/onedrive/api/types.go b/backend/onedrive/api/types.go index da4fa5a3c..7b7bff5cc 100644 --- a/backend/onedrive/api/types.go +++ b/backend/onedrive/api/types.go @@ -215,14 +215,16 @@ const ( // PermissionsType provides information about a sharing permission granted for a DriveItem resource. // Sharing permissions have a number of different forms. The Permission resource represents these different forms through facets on the resource. type PermissionsType struct { - ID string `json:"id"` // The unique identifier of the permission among all permissions on the item. Read-only. - GrantedTo *IdentitySet `json:"grantedTo,omitempty"` // For user type permissions, the details of the users & applications for this permission. Read-only. - GrantedToIdentities []*IdentitySet `json:"grantedToIdentities,omitempty"` // For link type permissions, the details of the users to whom permission was granted. Read-only. - Invitation *SharingInvitationType `json:"invitation,omitempty"` // Details of any associated sharing invitation for this permission. Read-only. - InheritedFrom *ItemReference `json:"inheritedFrom,omitempty"` // Provides a reference to the ancestor of the current permission, if it is inherited from an ancestor. Read-only. - Link *SharingLinkType `json:"link,omitempty"` // Provides the link details of the current permission, if it is a link type permissions. Read-only. - Roles []Role `json:"roles,omitempty"` // The type of permission (read, write, owner, member). Read-only. - ShareID string `json:"shareId,omitempty"` // A unique token that can be used to access this shared item via the shares API. Read-only. + ID string `json:"id"` // The unique identifier of the permission among all permissions on the item. Read-only. + GrantedTo *IdentitySet `json:"grantedTo,omitempty"` // For user type permissions, the details of the users & applications for this permission. Read-only. Deprecated on OneDrive Business only. + GrantedToIdentities []*IdentitySet `json:"grantedToIdentities,omitempty"` // For link type permissions, the details of the users to whom permission was granted. Read-only. Deprecated on OneDrive Business only. + GrantedToV2 *IdentitySet `json:"grantedToV2,omitempty"` // For user type permissions, the details of the users & applications for this permission. Read-only. Not available for OneDrive Personal. + GrantedToIdentitiesV2 []*IdentitySet `json:"grantedToIdentitiesV2,omitempty"` // For link type permissions, the details of the users to whom permission was granted. Read-only. Not available for OneDrive Personal. + Invitation *SharingInvitationType `json:"invitation,omitempty"` // Details of any associated sharing invitation for this permission. Read-only. + InheritedFrom *ItemReference `json:"inheritedFrom,omitempty"` // Provides a reference to the ancestor of the current permission, if it is inherited from an ancestor. Read-only. + Link *SharingLinkType `json:"link,omitempty"` // Provides the link details of the current permission, if it is a link type permissions. Read-only. + Roles []Role `json:"roles,omitempty"` // The type of permission (read, write, owner, member). Read-only. + ShareID string `json:"shareId,omitempty"` // A unique token that can be used to access this shared item via the shares API. Read-only. } // Role represents the type of permission (read, write, owner, member) @@ -592,3 +594,25 @@ type SiteResource struct { type SiteResponse struct { Sites []SiteResource `json:"value"` } + +// GetGrantedTo returns the GrantedTo property. +// This is to get around the odd problem of +// GrantedTo being deprecated on OneDrive Business, while +// GrantedToV2 is unavailable on OneDrive Personal. +func (p *PermissionsType) GetGrantedTo(driveType string) *IdentitySet { + if driveType == "personal" { + return p.GrantedTo + } + return p.GrantedToV2 +} + +// GetGrantedToIdentities returns the GrantedToIdentities property. +// This is to get around the odd problem of +// GrantedToIdentities being deprecated on OneDrive Business, while +// GrantedToIdentitiesV2 is unavailable on OneDrive Personal. +func (p *PermissionsType) GetGrantedToIdentities(driveType string) []*IdentitySet { + if driveType == "personal" { + return p.GrantedToIdentities + } + return p.GrantedToIdentitiesV2 +} diff --git a/backend/onedrive/metadata.go b/backend/onedrive/metadata.go index 0ebcf2026..a80cbefd7 100644 --- a/backend/onedrive/metadata.go +++ b/backend/onedrive/metadata.go @@ -477,11 +477,11 @@ func (m *Metadata) processPermissions(ctx context.Context, add, update, remove [ // fillRecipients looks for recipients to add from the permission passed in. // It looks for an email address in identity.User.ID and DisplayName, otherwise it uses the identity.User.ID as r.ObjectID. // It considers both "GrantedTo" and "GrantedToIdentities". -func fillRecipients(p *api.PermissionsType) (recipients []api.DriveRecipient) { +func fillRecipients(p *api.PermissionsType, driveType string) (recipients []api.DriveRecipient) { if p == nil { return recipients } - ids := make(map[string]struct{}, len(p.GrantedToIdentities)+1) + ids := make(map[string]struct{}, len(p.GetGrantedToIdentities(driveType))+1) isUnique := func(s string) bool { _, ok := ids[s] return !ok && s != "" @@ -507,11 +507,11 @@ func fillRecipients(p *api.PermissionsType) (recipients []api.DriveRecipient) { ids[id] = struct{}{} recipients = append(recipients, r) } - for _, identity := range p.GrantedToIdentities { + for _, identity := range p.GetGrantedToIdentities(driveType) { addRecipient(identity) } - if p.GrantedTo != nil && p.GrantedTo.User != (api.Identity{}) { - addRecipient(p.GrantedTo) + if p.GetGrantedTo(driveType) != nil && p.GetGrantedTo(driveType).User != (api.Identity{}) { + addRecipient(p.GetGrantedTo(driveType)) } return recipients } @@ -522,7 +522,7 @@ func (m *Metadata) addPermission(ctx context.Context, p *api.PermissionsType) (n opts := m.fs.newOptsCall(m.normalizedID, "POST", "/invite") req := &api.AddPermissionsRequest{ - Recipients: fillRecipients(p), + Recipients: fillRecipients(p, m.fs.driveType), RequireSignIn: m.fs.driveType != driveTypePersonal, // personal and business have conflicting requirements Roles: p.Roles, } diff --git a/backend/onedrive/onedrive_internal_test.go b/backend/onedrive/onedrive_internal_test.go index f8e54940f..8940f538a 100644 --- a/backend/onedrive/onedrive_internal_test.go +++ b/backend/onedrive/onedrive_internal_test.go @@ -50,7 +50,7 @@ func (f *Fs) TestWritePermissions(t *testing.T, r *fstest.Run) { file1 := r.WriteFile(randomFilename(), content, t2) // add a permission with "read" role - permissions := defaultPermissions() + permissions := defaultPermissions(f.driveType) permissions[0].Roles[0] = api.ReadRole expectedMeta, actualMeta := f.putWithMeta(ctx, t, &file1, permissions) f.compareMeta(t, expectedMeta, actualMeta, false) @@ -59,7 +59,7 @@ func (f *Fs) TestWritePermissions(t *testing.T, r *fstest.Run) { found, num := false, 0 foundCount := 0 for i, p := range actualP { - for _, identity := range p.GrantedToIdentities { + for _, identity := range p.GetGrantedToIdentities(f.driveType) { if identity.User.DisplayName == testUserID { // note: expected will always be element 0 here, but actual may be variable based on org settings assert.Equal(t, expectedP[0].Roles, p.Roles) @@ -68,7 +68,7 @@ func (f *Fs) TestWritePermissions(t *testing.T, r *fstest.Run) { } } if f.driveType == driveTypePersonal { - if p.GrantedTo != nil && p.GrantedTo.User != (api.Identity{}) && p.GrantedTo.User.ID == testUserID { // shows up in a different place on biz vs. personal + if p.GetGrantedTo(f.driveType) != nil && p.GetGrantedTo(f.driveType).User != (api.Identity{}) && p.GetGrantedTo(f.driveType).User.ID == testUserID { // shows up in a different place on biz vs. personal assert.Equal(t, expectedP[0].Roles, p.Roles) found, num = true, i foundCount++ @@ -106,7 +106,7 @@ func (f *Fs) TestWritePermissions(t *testing.T, r *fstest.Run) { found = false var foundP *api.PermissionsType for _, p := range actualP { - if p.GrantedTo == nil || p.GrantedTo.User == (api.Identity{}) || p.GrantedTo.User.ID != testUserID { + if p.GetGrantedTo(f.driveType) == nil || p.GetGrantedTo(f.driveType).User == (api.Identity{}) || p.GetGrantedTo(f.driveType).User.ID != testUserID { continue } found = true @@ -134,7 +134,7 @@ func (f *Fs) TestReadPermissions(t *testing.T, r *fstest.Run) { // test that what we got before vs. after is the same _ = f.opt.MetadataPermissions.Set("read") _, expectedMeta := f.putWithMeta(ctx, t, &file1, []*api.PermissionsType{}) // return var intentionally switched here - permissions := defaultPermissions() + permissions := defaultPermissions(f.driveType) _, actualMeta := f.putWithMeta(ctx, t, &file1, permissions) if f.driveType == driveTypePersonal { perms, ok := actualMeta["permissions"] @@ -150,7 +150,7 @@ func (f *Fs) TestReadMetadata(t *testing.T, r *fstest.Run) { ctx, ci := fs.AddConfig(ctx) ci.Metadata = true file1 := r.WriteFile(randomFilename(), "hello", t2) - permissions := defaultPermissions() + permissions := defaultPermissions(f.driveType) _ = f.opt.MetadataPermissions.Set("read,write") _, actualMeta := f.putWithMeta(ctx, t, &file1, permissions) @@ -174,7 +174,7 @@ func (f *Fs) TestDirectoryMetadata(t *testing.T, r *fstest.Run) { ctx, ci := fs.AddConfig(ctx) ci.Metadata = true _ = f.opt.MetadataPermissions.Set("read,write") - permissions := defaultPermissions() + permissions := defaultPermissions(f.driveType) permissions[0].Roles[0] = api.ReadRole expectedMeta := fs.Metadata{ @@ -288,7 +288,7 @@ func (f *Fs) TestServerSideCopyMove(t *testing.T, r *fstest.Run) { file1 := r.WriteFile(randomFilename(), content, t2) // add a permission with "read" role - permissions := defaultPermissions() + permissions := defaultPermissions(f.driveType) permissions[0].Roles[0] = api.ReadRole expectedMeta, actualMeta := f.putWithMeta(ctx, t, &file1, permissions) f.compareMeta(t, expectedMeta, actualMeta, false) @@ -331,7 +331,10 @@ func (f *Fs) TestMetadataMapper(t *testing.T, r *fstest.Run) { _ = f.opt.MetadataPermissions.Set("read,write") file1 := r.WriteFile(randomFilename(), content, t2) - const blob = `{"Metadata":{"permissions":"[{\"grantedToIdentities\":[{\"user\":{\"id\":\"ryan@contoso.com\"}}],\"roles\":[\"read\"]}]"}}` + blob := `{"Metadata":{"permissions":"[{\"grantedToIdentities\":[{\"user\":{\"id\":\"ryan@contoso.com\"}}],\"roles\":[\"read\"]}]"}}` + if f.driveType != driveTypePersonal { + blob = `{"Metadata":{"permissions":"[{\"grantedToIdentitiesV2\":[{\"user\":{\"id\":\"ryan@contoso.com\"}}],\"roles\":[\"read\"]}]"}}` + } // Copy ci.MetadataMapper = []string{"echo", blob} @@ -347,7 +350,7 @@ func (f *Fs) TestMetadataMapper(t *testing.T, r *fstest.Run) { found := false foundCount := 0 for _, p := range actualP { - for _, identity := range p.GrantedToIdentities { + for _, identity := range p.GetGrantedToIdentities(f.driveType) { if identity.User.DisplayName == testUserID { assert.Equal(t, []api.Role{api.ReadRole}, p.Roles) found = true @@ -355,7 +358,7 @@ func (f *Fs) TestMetadataMapper(t *testing.T, r *fstest.Run) { } } if f.driveType == driveTypePersonal { - if p.GrantedTo != nil && p.GrantedTo.User != (api.Identity{}) && p.GrantedTo.User.ID == testUserID { // shows up in a different place on biz vs. personal + if p.GetGrantedTo(f.driveType) != nil && p.GetGrantedTo(f.driveType).User != (api.Identity{}) && p.GetGrantedTo(f.driveType).User.ID == testUserID { // shows up in a different place on biz vs. personal assert.Equal(t, []api.Role{api.ReadRole}, p.Roles) found = true foundCount++ @@ -449,11 +452,18 @@ func indent(t *testing.T, s string) string { return marshalPerms(t, p) } -func defaultPermissions() []*api.PermissionsType { +func defaultPermissions(driveType string) []*api.PermissionsType { + if driveType == driveTypePersonal { + return []*api.PermissionsType{{ + GrantedTo: &api.IdentitySet{User: api.Identity{}}, + GrantedToIdentities: []*api.IdentitySet{{User: api.Identity{ID: testUserID}}}, + Roles: []api.Role{api.WriteRole}, + }} + } return []*api.PermissionsType{{ - GrantedTo: &api.IdentitySet{User: api.Identity{}}, - GrantedToIdentities: []*api.IdentitySet{{User: api.Identity{ID: testUserID}}}, - Roles: []api.Role{api.WriteRole}, + GrantedToV2: &api.IdentitySet{User: api.Identity{}}, + GrantedToIdentitiesV2: []*api.IdentitySet{{User: api.Identity{ID: testUserID}}}, + Roles: []api.Role{api.WriteRole}, }} } diff --git a/docs/content/onedrive.md b/docs/content/onedrive.md index 122a7b1f3..0b55629d6 100644 --- a/docs/content/onedrive.md +++ b/docs/content/onedrive.md @@ -754,7 +754,7 @@ Example for OneDrive Business: [ { "id": "48d31887-5fad-4d73-a9f5-3c356e68a038", - "grantedToIdentities": [ + "grantedToIdentitiesV2": [ { "user": { "displayName": "ryan@contoso.com" @@ -775,7 +775,7 @@ Example for OneDrive Business: }, { "id": "5D33DD65C6932946", - "grantedTo": { + "grantedToV2": { "user": { "displayName": "John Doe", "id": "efee1b77-fb3b-4f65-99d6-274c11914d12" @@ -796,10 +796,12 @@ format. The [`--metadata-mapper`](https://rclone.org/docs/#metadata-mapper) tool be very helpful for this. When adding permissions, an email address can be provided in the `User.ID` or -`DisplayName` properties of `grantedTo` or `grantedToIdentities`. Alternatively, -an ObjectID can be provided in `User.ID`. At least one valid recipient must be -provided in order to add a permission for a user. Creating a Public Link is also -supported, if `Link.Scope` is set to `"anonymous"`. +`DisplayName` properties of `grantedTo` or `grantedToIdentities` (these are +deprecated on OneDrive Business -- instead, use `grantedToV2` and +`grantedToIdentitiesV2`, respectively). Alternatively, an ObjectID can be +provided in `User.ID`. At least one valid recipient must be provided in order to +add a permission for a user. Creating a Public Link is also supported, if +`Link.Scope` is set to `"anonymous"`. Example request to add a "read" permission: