Merge pull request #1 from rclone/master

update

.github/workflows/build.yml

@@ -231,48 +231,66 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
+      - name: Get runner parameters
+        id: get-runner-parameters
+        shell: bash
+        run: |
+          echo "year-week=$(/bin/date -u "+%Y%V")" >> $GITHUB_OUTPUT
+          echo "runner-os-version=$ImageOS" >> $GITHUB_OUTPUT
+
       - name: Checkout
         uses: actions/checkout@v4
 
       - name: Install Go
+        id: setup-go
         uses: actions/setup-go@v5
         with:
           go-version: '>=1.22.0-rc.1'
           check-latest: true
           cache: false
 
+      - name: Cache
+        uses: actions/cache@v4
+        with:
+          path: |
+            ~/go/pkg/mod
+            ~/.cache/go-build
+            ~/.cache/golangci-lint
+          key: golangci-lint-${{ steps.get-runner-parameters.outputs.runner-os-version }}-go${{ steps.setup-go.outputs.go-version }}-${{ steps.get-runner-parameters.outputs.year-week }}-${{ hashFiles('go.sum') }}
+          restore-keys: golangci-lint-${{ steps.get-runner-parameters.outputs.runner-os-version }}-go${{ steps.setup-go.outputs.go-version }}-${{ steps.get-runner-parameters.outputs.year-week }}-
+
       - name: Code quality test (Linux)
-        uses: golangci/golangci-lint-action@v4
+        uses: golangci/golangci-lint-action@v6
         with:
           version: latest
-          skip-cache: false # Caching enabled (which is default) on this first lint step only, it handles complete cache of build, go modules and golangci-lint analysis which was necessary to get all lint steps to properly take advantage of it
+          skip-cache: true
 
       - name: Code quality test (Windows)
-        uses: golangci/golangci-lint-action@v4
+        uses: golangci/golangci-lint-action@v6
         env:
           GOOS: "windows"
         with:
           version: latest
           skip-cache: true
 
       - name: Code quality test (macOS)
-        uses: golangci/golangci-lint-action@v4
+        uses: golangci/golangci-lint-action@v6
         env:
           GOOS: "darwin"
         with:
           version: latest
           skip-cache: true
 
       - name: Code quality test (FreeBSD)
-        uses: golangci/golangci-lint-action@v4
+        uses: golangci/golangci-lint-action@v6
         env:
           GOOS: "freebsd"
         with:
           version: latest
           skip-cache: true
 
       - name: Code quality test (OpenBSD)
-        uses: golangci/golangci-lint-action@v4
+        uses: golangci/golangci-lint-action@v6
         env:
           GOOS: "openbsd"
         with:

.github/workflows/notify.yml

@@ -0,0 +1,14 @@
+name: Notify users based on issue labels
+
+on:
+  issues:
+      types: [labeled]
+
+jobs:
+  notify:
+    runs-on: ubuntu-latest
+    steps:
+        - uses: jenschelkopf/issue-label-notification-action@1.3
+          with:
+             recipients: |
+                  Support Contract=@rclone/support

backend/drive/metadata.go

@@ -152,7 +152,7 @@ func (f *Fs) setPermissions(ctx context.Context, info *drive.File, permissions [
 			return f.shouldRetry(ctx, err)
 		})
 		if err != nil {
-			fs.Errorf(f, "Failed to set permission: %v", err)
+			fs.Errorf(f, "Failed to set permission %s for %q: %v", perm.Role, perm.EmailAddress, err)
 			errs.Add(err)
 		}
 	}

backend/onedrive/api/types.go

@@ -42,6 +42,8 @@ var _ error = (*Error)(nil)
 type Identity struct {
 	DisplayName string `json:"displayName,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
@@ -51,6 +53,9 @@ type IdentitySet struct {
 	User        Identity `json:"user,omitempty"`
 	Application Identity `json:"application,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.
@@ -215,14 +220,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 +599,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
+}

backend/onedrive/metadata.go

@@ -396,7 +396,7 @@ func (m *Metadata) sortPermissions() (add, update, remove []*api.PermissionsType
 		if n.ID != "" {
 			// sanity check: ensure there's a matching "old" id with a non-matching role
 			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)
 				continue
@@ -418,6 +418,10 @@ func (m *Metadata) sortPermissions() (add, update, remove []*api.PermissionsType
 		}
 	}
 	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 {
 			if n == nil || n.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.
-// 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".
-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 != ""
@@ -487,7 +491,10 @@ func fillRecipients(p *api.PermissionsType) (recipients []api.DriveRecipient) {
 		r := api.DriveRecipient{}
 
 		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
 			r.Email = id
 		} else if strings.ContainsRune(identity.User.DisplayName, '@') {
@@ -503,12 +510,31 @@ func fillRecipients(p *api.PermissionsType) (recipients []api.DriveRecipient) {
 		ids[id] = struct{}{}
 		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
 }
 
@@ -518,7 +544,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,
 	}

backend/onedrive/metadata.md

@@ -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.
 
 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
 you don't need to read or write permissions it is recommended to omit

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,15 +350,15 @@ 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
 				foundCount++
 			}
 		}
 		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},
 	}}
 }