Compare commits

...
Sign in to create a new pull request.

3 commits

Author SHA1 Message Date
nielash
df02292457 onedrive: add support for group permissions
This change adds support for "group" identities, and SharePoint variants
"siteUser" and "siteGroup". It also adds support for using any identity type
(including "application" and "device") as a recipient source when adding
permissions.
2024-04-30 08:50:11 -04:00
nielash
f75929fe3c 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)
2024-04-30 07:30:53 -04:00
nielash
f9d98ffb72 onedrive: skip writing permissions with 'owner' role
The 'owner' role is an implicit role that can't be removed, so don't try to.
2024-04-30 07:30:53 -04:00
5 changed files with 108 additions and 40 deletions

View file

@ -42,6 +42,8 @@ var _ error = (*Error)(nil)
type Identity struct { type Identity struct {
DisplayName string `json:"displayName,omitempty"` DisplayName string `json:"displayName,omitempty"`
ID string `json:"id,omitempty"` ID string `json:"id,omitempty"`
Email string `json:"email,omitempty"` // not officially documented, but seems to sometimes exist
LoginName string `json:"loginName,omitempty"` // SharePoint only
} }
// IdentitySet is a keyed collection of Identity objects. It is used // IdentitySet is a keyed collection of Identity objects. It is used
@ -51,6 +53,9 @@ type IdentitySet struct {
User Identity `json:"user,omitempty"` User Identity `json:"user,omitempty"`
Application Identity `json:"application,omitempty"` Application Identity `json:"application,omitempty"`
Device Identity `json:"device,omitempty"` Device Identity `json:"device,omitempty"`
Group Identity `json:"group,omitempty"`
SiteGroup Identity `json:"siteGroup,omitempty"` // The SharePoint group associated with this action. Optional.
SiteUser Identity `json:"siteUser,omitempty"` // The SharePoint user associated with this action. Optional.
} }
// Quota groups storage space quota-related information on OneDrive into a single structure. // Quota groups storage space quota-related information on OneDrive into a single structure.
@ -215,14 +220,16 @@ const (
// PermissionsType provides information about a sharing permission granted for a DriveItem resource. // 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. // Sharing permissions have a number of different forms. The Permission resource represents these different forms through facets on the resource.
type PermissionsType struct { type PermissionsType struct {
ID string `json:"id"` // The unique identifier of the permission among all permissions on the item. 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. 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. 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.
Invitation *SharingInvitationType `json:"invitation,omitempty"` // Details of any associated sharing invitation for this permission. Read-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.
InheritedFrom *ItemReference `json:"inheritedFrom,omitempty"` // Provides a reference to the ancestor of the current permission, if it is inherited from an ancestor. Read-only. 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.
Link *SharingLinkType `json:"link,omitempty"` // Provides the link details of the current permission, if it is a link type permissions. Read-only. Invitation *SharingInvitationType `json:"invitation,omitempty"` // Details of any associated sharing invitation for this permission. Read-only.
Roles []Role `json:"roles,omitempty"` // The type of permission (read, write, owner, member). 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.
ShareID string `json:"shareId,omitempty"` // A unique token that can be used to access this shared item via the shares API. 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) // Role represents the type of permission (read, write, owner, member)
@ -592,3 +599,25 @@ type SiteResource struct {
type SiteResponse struct { type SiteResponse struct {
Sites []SiteResource `json:"value"` 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
}

View file

@ -396,7 +396,7 @@ func (m *Metadata) sortPermissions() (add, update, remove []*api.PermissionsType
if n.ID != "" { if n.ID != "" {
// sanity check: ensure there's a matching "old" id with a non-matching role // sanity check: ensure there's a matching "old" id with a non-matching role
if !slices.ContainsFunc(old, func(o *api.PermissionsType) bool { if !slices.ContainsFunc(old, func(o *api.PermissionsType) bool {
return o.ID == n.ID && slices.Compare(o.Roles, n.Roles) != 0 && len(o.Roles) > 0 && len(n.Roles) > 0 return o.ID == n.ID && slices.Compare(o.Roles, n.Roles) != 0 && len(o.Roles) > 0 && len(n.Roles) > 0 && !slices.Contains(o.Roles, api.OwnerRole)
}) { }) {
fs.Debugf(m.remote, "skipping update for invalid roles: %v (perm ID: %v)", n.Roles, n.ID) fs.Debugf(m.remote, "skipping update for invalid roles: %v (perm ID: %v)", n.Roles, n.ID)
continue continue
@ -418,6 +418,10 @@ func (m *Metadata) sortPermissions() (add, update, remove []*api.PermissionsType
} }
} }
for _, o := range old { for _, o := range old {
if slices.Contains(o.Roles, api.OwnerRole) {
fs.Debugf(m.remote, "skipping remove permission -- can't remove 'owner' role")
continue
}
newHasOld := slices.ContainsFunc(new, func(n *api.PermissionsType) bool { newHasOld := slices.ContainsFunc(new, func(n *api.PermissionsType) bool {
if n == nil || n.ID == "" { if n == nil || n.ID == "" {
return false // can't remove perms without an ID return false // can't remove perms without an ID
@ -471,13 +475,13 @@ func (m *Metadata) processPermissions(ctx context.Context, add, update, remove [
} }
// fillRecipients looks for recipients to add from the permission passed in. // 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 looks for an email address in identity.User.Email, ID, and DisplayName, otherwise it uses the identity.User.ID as r.ObjectID.
// It considers both "GrantedTo" and "GrantedToIdentities". // 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 { if p == nil {
return recipients 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 { isUnique := func(s string) bool {
_, ok := ids[s] _, ok := ids[s]
return !ok && s != "" return !ok && s != ""
@ -487,7 +491,10 @@ func fillRecipients(p *api.PermissionsType) (recipients []api.DriveRecipient) {
r := api.DriveRecipient{} r := api.DriveRecipient{}
id := "" id := ""
if strings.ContainsRune(identity.User.ID, '@') { if strings.ContainsRune(identity.User.Email, '@') {
id = identity.User.Email
r.Email = id
} else if strings.ContainsRune(identity.User.ID, '@') {
id = identity.User.ID id = identity.User.ID
r.Email = id r.Email = id
} else if strings.ContainsRune(identity.User.DisplayName, '@') { } else if strings.ContainsRune(identity.User.DisplayName, '@') {
@ -503,12 +510,31 @@ func fillRecipients(p *api.PermissionsType) (recipients []api.DriveRecipient) {
ids[id] = struct{}{} ids[id] = struct{}{}
recipients = append(recipients, r) recipients = append(recipients, r)
} }
for _, identity := range p.GrantedToIdentities {
addRecipient(identity) forIdentitySet := func(iSet *api.IdentitySet) {
if iSet == nil {
return
}
iS := *iSet
forIdentity := func(i api.Identity) {
if i != (api.Identity{}) {
iS.User = i
addRecipient(&iS)
}
}
forIdentity(iS.User)
forIdentity(iS.SiteUser)
forIdentity(iS.Group)
forIdentity(iS.SiteGroup)
forIdentity(iS.Application)
forIdentity(iS.Device)
} }
if p.GrantedTo != nil && p.GrantedTo.User != (api.Identity{}) {
addRecipient(p.GrantedTo) for _, identitySet := range p.GetGrantedToIdentities(driveType) {
forIdentitySet(identitySet)
} }
forIdentitySet(p.GetGrantedTo(driveType))
return recipients return recipients
} }
@ -518,7 +544,7 @@ func (m *Metadata) addPermission(ctx context.Context, p *api.PermissionsType) (n
opts := m.fs.newOptsCall(m.normalizedID, "POST", "/invite") opts := m.fs.newOptsCall(m.normalizedID, "POST", "/invite")
req := &api.AddPermissionsRequest{ req := &api.AddPermissionsRequest{
Recipients: fillRecipients(p), Recipients: fillRecipients(p, m.fs.driveType),
RequireSignIn: m.fs.driveType != driveTypePersonal, // personal and business have conflicting requirements RequireSignIn: m.fs.driveType != driveTypePersonal, // personal and business have conflicting requirements
Roles: p.Roles, Roles: p.Roles,
} }

View file

@ -109,7 +109,8 @@ To update an existing permission, include both the Permission ID and the new
`roles` to be assigned. `roles` is the only property that can be changed. `roles` to be assigned. `roles` is the only property that can be changed.
To remove permissions, pass in a blob containing only the permissions you wish To remove permissions, pass in a blob containing only the permissions you wish
to keep (which can be empty, to remove all.) to keep (which can be empty, to remove all.) Note that the `owner` role will be
ignored, as it cannot be removed.
Note that both reading and writing permissions requires extra API calls, so if Note that both reading and writing permissions requires extra API calls, so if
you don't need to read or write permissions it is recommended to omit you don't need to read or write permissions it is recommended to omit

View file

@ -50,7 +50,7 @@ func (f *Fs) TestWritePermissions(t *testing.T, r *fstest.Run) {
file1 := r.WriteFile(randomFilename(), content, t2) file1 := r.WriteFile(randomFilename(), content, t2)
// add a permission with "read" role // add a permission with "read" role
permissions := defaultPermissions() permissions := defaultPermissions(f.driveType)
permissions[0].Roles[0] = api.ReadRole permissions[0].Roles[0] = api.ReadRole
expectedMeta, actualMeta := f.putWithMeta(ctx, t, &file1, permissions) expectedMeta, actualMeta := f.putWithMeta(ctx, t, &file1, permissions)
f.compareMeta(t, expectedMeta, actualMeta, false) f.compareMeta(t, expectedMeta, actualMeta, false)
@ -59,7 +59,7 @@ func (f *Fs) TestWritePermissions(t *testing.T, r *fstest.Run) {
found, num := false, 0 found, num := false, 0
foundCount := 0 foundCount := 0
for i, p := range actualP { for i, p := range actualP {
for _, identity := range p.GrantedToIdentities { for _, identity := range p.GetGrantedToIdentities(f.driveType) {
if identity.User.DisplayName == testUserID { if identity.User.DisplayName == testUserID {
// note: expected will always be element 0 here, but actual may be variable based on org settings // 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) 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 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) assert.Equal(t, expectedP[0].Roles, p.Roles)
found, num = true, i found, num = true, i
foundCount++ foundCount++
@ -106,7 +106,7 @@ func (f *Fs) TestWritePermissions(t *testing.T, r *fstest.Run) {
found = false found = false
var foundP *api.PermissionsType var foundP *api.PermissionsType
for _, p := range actualP { 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 continue
} }
found = true 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 // test that what we got before vs. after is the same
_ = f.opt.MetadataPermissions.Set("read") _ = f.opt.MetadataPermissions.Set("read")
_, expectedMeta := f.putWithMeta(ctx, t, &file1, []*api.PermissionsType{}) // return var intentionally switched here _, 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) _, actualMeta := f.putWithMeta(ctx, t, &file1, permissions)
if f.driveType == driveTypePersonal { if f.driveType == driveTypePersonal {
perms, ok := actualMeta["permissions"] perms, ok := actualMeta["permissions"]
@ -150,7 +150,7 @@ func (f *Fs) TestReadMetadata(t *testing.T, r *fstest.Run) {
ctx, ci := fs.AddConfig(ctx) ctx, ci := fs.AddConfig(ctx)
ci.Metadata = true ci.Metadata = true
file1 := r.WriteFile(randomFilename(), "hello", t2) file1 := r.WriteFile(randomFilename(), "hello", t2)
permissions := defaultPermissions() permissions := defaultPermissions(f.driveType)
_ = f.opt.MetadataPermissions.Set("read,write") _ = f.opt.MetadataPermissions.Set("read,write")
_, actualMeta := f.putWithMeta(ctx, t, &file1, permissions) _, 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) ctx, ci := fs.AddConfig(ctx)
ci.Metadata = true ci.Metadata = true
_ = f.opt.MetadataPermissions.Set("read,write") _ = f.opt.MetadataPermissions.Set("read,write")
permissions := defaultPermissions() permissions := defaultPermissions(f.driveType)
permissions[0].Roles[0] = api.ReadRole permissions[0].Roles[0] = api.ReadRole
expectedMeta := fs.Metadata{ expectedMeta := fs.Metadata{
@ -288,7 +288,7 @@ func (f *Fs) TestServerSideCopyMove(t *testing.T, r *fstest.Run) {
file1 := r.WriteFile(randomFilename(), content, t2) file1 := r.WriteFile(randomFilename(), content, t2)
// add a permission with "read" role // add a permission with "read" role
permissions := defaultPermissions() permissions := defaultPermissions(f.driveType)
permissions[0].Roles[0] = api.ReadRole permissions[0].Roles[0] = api.ReadRole
expectedMeta, actualMeta := f.putWithMeta(ctx, t, &file1, permissions) expectedMeta, actualMeta := f.putWithMeta(ctx, t, &file1, permissions)
f.compareMeta(t, expectedMeta, actualMeta, false) 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") _ = f.opt.MetadataPermissions.Set("read,write")
file1 := r.WriteFile(randomFilename(), content, t2) 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 // Copy
ci.MetadataMapper = []string{"echo", blob} ci.MetadataMapper = []string{"echo", blob}
@ -347,7 +350,7 @@ func (f *Fs) TestMetadataMapper(t *testing.T, r *fstest.Run) {
found := false found := false
foundCount := 0 foundCount := 0
for _, p := range actualP { for _, p := range actualP {
for _, identity := range p.GrantedToIdentities { for _, identity := range p.GetGrantedToIdentities(f.driveType) {
if identity.User.DisplayName == testUserID { if identity.User.DisplayName == testUserID {
assert.Equal(t, []api.Role{api.ReadRole}, p.Roles) assert.Equal(t, []api.Role{api.ReadRole}, p.Roles)
found = true found = true
@ -355,7 +358,7 @@ func (f *Fs) TestMetadataMapper(t *testing.T, r *fstest.Run) {
} }
} }
if f.driveType == driveTypePersonal { 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) assert.Equal(t, []api.Role{api.ReadRole}, p.Roles)
found = true found = true
foundCount++ foundCount++
@ -449,11 +452,18 @@ func indent(t *testing.T, s string) string {
return marshalPerms(t, p) 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{{ return []*api.PermissionsType{{
GrantedTo: &api.IdentitySet{User: api.Identity{}}, GrantedToV2: &api.IdentitySet{User: api.Identity{}},
GrantedToIdentities: []*api.IdentitySet{{User: api.Identity{ID: testUserID}}}, GrantedToIdentitiesV2: []*api.IdentitySet{{User: api.Identity{ID: testUserID}}},
Roles: []api.Role{api.WriteRole}, Roles: []api.Role{api.WriteRole},
}} }}
} }

View file

@ -754,7 +754,7 @@ Example for OneDrive Business:
[ [
{ {
"id": "48d31887-5fad-4d73-a9f5-3c356e68a038", "id": "48d31887-5fad-4d73-a9f5-3c356e68a038",
"grantedToIdentities": [ "grantedToIdentitiesV2": [
{ {
"user": { "user": {
"displayName": "ryan@contoso.com" "displayName": "ryan@contoso.com"
@ -775,7 +775,7 @@ Example for OneDrive Business:
}, },
{ {
"id": "5D33DD65C6932946", "id": "5D33DD65C6932946",
"grantedTo": { "grantedToV2": {
"user": { "user": {
"displayName": "John Doe", "displayName": "John Doe",
"id": "efee1b77-fb3b-4f65-99d6-274c11914d12" "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. be very helpful for this.
When adding permissions, an email address can be provided in the `User.ID` or When adding permissions, an email address can be provided in the `User.ID` or
`DisplayName` properties of `grantedTo` or `grantedToIdentities`. Alternatively, `DisplayName` properties of `grantedTo` or `grantedToIdentities` (these are
an ObjectID can be provided in `User.ID`. At least one valid recipient must be deprecated on OneDrive Business -- instead, use `grantedToV2` and
provided in order to add a permission for a user. Creating a Public Link is also `grantedToIdentitiesV2`, respectively). Alternatively, an ObjectID can be
supported, if `Link.Scope` is set to `"anonymous"`. 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: Example request to add a "read" permission: