diff --git a/backend/onedrive/metadata.go b/backend/onedrive/metadata.go index d69cdc155..c6a67b132 100644 --- a/backend/onedrive/metadata.go +++ b/backend/onedrive/metadata.go @@ -613,7 +613,7 @@ func (o *Object) tryGetBtime(modTime time.Time) time.Time { } // adds metadata (except permissions) if --metadata is in use -func (o *Object) fetchMetadataForCreate(ctx context.Context, src fs.ObjectInfo, options []fs.OpenOption, modTime time.Time) (createRequest api.CreateUploadRequest, err error) { +func (o *Object) fetchMetadataForCreate(ctx context.Context, src fs.ObjectInfo, options []fs.OpenOption, modTime time.Time) (createRequest api.CreateUploadRequest, metadata fs.Metadata, err error) { createRequest = api.CreateUploadRequest{ // we set mtime no matter what Item: api.Metadata{ FileSystemInfo: &api.FileSystemInfoFacet{ @@ -625,10 +625,10 @@ func (o *Object) fetchMetadataForCreate(ctx context.Context, src fs.ObjectInfo, meta, err := fs.GetMetadataOptions(ctx, o.fs, src, options) if err != nil { - return createRequest, fmt.Errorf("failed to read metadata from source object: %w", err) + return createRequest, nil, fmt.Errorf("failed to read metadata from source object: %w", err) } if meta == nil { - return createRequest, nil // no metadata or --metadata not in use, so just return mtime + return createRequest, nil, nil // no metadata or --metadata not in use, so just return mtime } if o.meta == nil { o.meta = o.fs.newMetadata(o.Remote()) @@ -636,13 +636,13 @@ func (o *Object) fetchMetadataForCreate(ctx context.Context, src fs.ObjectInfo, o.meta.mtime = modTime numSet, err := o.meta.Set(ctx, meta) if err != nil { - return createRequest, err + return createRequest, meta, err } if numSet == 0 { - return createRequest, nil + return createRequest, meta, nil } createRequest.Item = o.meta.toAPIMetadata() - return createRequest, nil + return createRequest, meta, nil } // Fetch metadata and update updateInfo if --metadata is in use @@ -665,27 +665,6 @@ func (f *Fs) fetchAndUpdateMetadata(ctx context.Context, src fs.ObjectInfo, opti return newInfo, err } -// Fetch and update permissions if --metadata is in use -// This is similar to fetchAndUpdateMetadata, except it does NOT set modtime or other metadata if there are no permissions to set. -// This is intended for cases where metadata may already have been set during upload and an extra step is needed only for permissions. -func (f *Fs) fetchAndUpdatePermissions(ctx context.Context, src fs.ObjectInfo, options []fs.OpenOption, updateInfo *Object) (info *api.Item, err error) { - meta, err := fs.GetMetadataOptions(ctx, f, src, options) - if err != nil { - return nil, fmt.Errorf("failed to read metadata from source object: %w", err) - } - if meta == nil || !f.needsUpdatePermissions(meta) { - return nil, nil // no metadata, --metadata not in use, or wrong flags - } - if updateInfo.meta == nil { - updateInfo.meta = f.newMetadata(updateInfo.Remote()) - } - newInfo, err := updateInfo.updateMetadata(ctx, meta) - if newInfo == nil { - return info, err - } - return newInfo, err -} - // updateMetadata calls Get, Set, and Write func (o *Object) updateMetadata(ctx context.Context, meta fs.Metadata) (info *api.Item, err error) { _, err = o.meta.Get(ctx) // refresh permissions diff --git a/backend/onedrive/metadata.md b/backend/onedrive/metadata.md index 7c81ca14b..bd057690a 100644 --- a/backend/onedrive/metadata.md +++ b/backend/onedrive/metadata.md @@ -4,11 +4,11 @@ differences between OneDrive Personal and Business (see table below for details). Permissions are also supported, if `--onedrive-metadata-permissions` is set. The -accepted values for `--onedrive-metadata-permissions` are `read`, `write`, -`read,write`, and `off` (the default). `write` supports adding new permissions, +accepted values for `--onedrive-metadata-permissions` are "`read`", "`write`", +"`read,write`", and "`off`" (the default). "`write`" supports adding new permissions, updating the "role" of existing permissions, and removing permissions. Updating and removing require the Permission ID to be known, so it is recommended to use -`read,write` instead of `write` if you wish to update/remove permissions. +"`read,write`" instead of "`write`" if you wish to update/remove permissions. Permissions are read/written in JSON format using the same schema as the [OneDrive API](https://learn.microsoft.com/en-us/onedrive/developer/rest-api/resources/permission?view=odsp-graph-online), @@ -92,31 +92,14 @@ 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: +Example request to add a "read" permission with `--metadata-mapper`: ```json -[ - { - "id": "", - "grantedTo": { - "user": {}, - "application": {}, - "device": {} - }, - "grantedToIdentities": [ - { - "user": { - "id": "ryan@contoso.com" - }, - "application": {}, - "device": {} - } - ], - "roles": [ - "read" - ] - } -] +{ + "Metadata": { + "permissions": "[{\"grantedToIdentities\":[{\"user\":{\"id\":\"ryan@contoso.com\"}}],\"roles\":[\"read\"]}]" + } +} ``` Note that adding a permission can fail if a conflicting permission already diff --git a/backend/onedrive/onedrive.go b/backend/onedrive/onedrive.go index c1da57a60..d7d3c2c37 100644 --- a/backend/onedrive/onedrive.go +++ b/backend/onedrive/onedrive.go @@ -2304,11 +2304,11 @@ func (o *Object) Open(ctx context.Context, options ...fs.OpenOption) (in io.Read } // createUploadSession creates an upload session for the object -func (o *Object) createUploadSession(ctx context.Context, src fs.ObjectInfo, modTime time.Time) (response *api.CreateUploadResponse, err error) { +func (o *Object) createUploadSession(ctx context.Context, src fs.ObjectInfo, modTime time.Time) (response *api.CreateUploadResponse, metadata fs.Metadata, err error) { opts := o.fs.newOptsCallWithPath(ctx, o.remote, "POST", "/createUploadSession") - createRequest, err := o.fetchMetadataForCreate(ctx, src, opts.Options, modTime) + createRequest, metadata, err := o.fetchMetadataForCreate(ctx, src, opts.Options, modTime) if err != nil { - return nil, err + return nil, metadata, err } var resp *http.Response err = o.fs.pacer.Call(func() (bool, error) { @@ -2321,7 +2321,7 @@ func (o *Object) createUploadSession(ctx context.Context, src fs.ObjectInfo, mod } return shouldRetry(ctx, resp, err) }) - return response, err + return response, metadata, err } // getPosition gets the current position in a multipart upload @@ -2437,7 +2437,7 @@ func (o *Object) uploadMultipart(ctx context.Context, in io.Reader, src fs.Objec // Create upload session fs.Debugf(o, "Starting multipart upload") - session, err := o.createUploadSession(ctx, src, modTime) + session, metadata, err := o.createUploadSession(ctx, src, modTime) if err != nil { return nil, err } @@ -2474,10 +2474,10 @@ func (o *Object) uploadMultipart(ctx context.Context, in io.Reader, src fs.Objec if err != nil { return info, err } - if !o.fs.opt.MetadataPermissions.IsSet(rwWrite) { + if metadata == nil || !o.fs.needsUpdatePermissions(metadata) { return info, err } - info, err = o.fs.fetchAndUpdatePermissions(ctx, src, options, o) // for permissions, which can't be set during original upload + info, err = o.updateMetadata(ctx, metadata) // for permissions, which can't be set during original upload if info == nil { return nil, err } diff --git a/backend/onedrive/onedrive_internal_test.go b/backend/onedrive/onedrive_internal_test.go index 2cfa85e7c..f8e54940f 100644 --- a/backend/onedrive/onedrive_internal_test.go +++ b/backend/onedrive/onedrive_internal_test.go @@ -323,6 +323,49 @@ func (f *Fs) TestServerSideCopyMove(t *testing.T, r *fstest.Run) { f.compareMeta(t, expectedMeta, actualMeta, true) } +// TestMetadataMapper tests adding permissions with the --metadata-mapper +func (f *Fs) TestMetadataMapper(t *testing.T, r *fstest.Run) { + // setup + ctx, ci := fs.AddConfig(ctx) + ci.Metadata = true + _ = f.opt.MetadataPermissions.Set("read,write") + file1 := r.WriteFile(randomFilename(), content, t2) + + const blob = `{"Metadata":{"permissions":"[{\"grantedToIdentities\":[{\"user\":{\"id\":\"ryan@contoso.com\"}}],\"roles\":[\"read\"]}]"}}` + + // Copy + ci.MetadataMapper = []string{"echo", blob} + require.NoError(t, ci.Dump.Set("mapper")) + obj1, err := r.Flocal.NewObject(ctx, file1.Path) + assert.NoError(t, err) + obj2, err := operations.Copy(ctx, f, nil, randomFilename(), obj1) + assert.NoError(t, err) + actualMeta, err := fs.GetMetadata(ctx, obj2) + assert.NoError(t, err) + + actualP := unmarshalPerms(t, actualMeta["permissions"]) + found := false + foundCount := 0 + for _, p := range actualP { + for _, identity := range p.GrantedToIdentities { + 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 + assert.Equal(t, []api.Role{api.ReadRole}, p.Roles) + found = true + foundCount++ + } + } + } + assert.True(t, found, fmt.Sprintf("no permission found with expected role (want: \n\n%v \n\ngot: \n\n%v\n\n)", blob, actualMeta)) + assert.Equal(t, 1, foundCount, "expected to find exactly 1 match") +} + // helper function to put an object with metadata and permissions func (f *Fs) putWithMeta(ctx context.Context, t *testing.T, file *fstest.Item, perms []*api.PermissionsType) (expectedMeta, actualMeta fs.Metadata) { t.Helper() @@ -459,6 +502,8 @@ func (f *Fs) InternalTest(t *testing.T) { testF, r = newTestF() t.Run("TestServerSideCopyMove", func(t *testing.T) { testF.TestServerSideCopyMove(t, r) }) testF.resetTestDefaults(r) + t.Run("TestMetadataMapper", func(t *testing.T) { testF.TestMetadataMapper(t, r) }) + testF.resetTestDefaults(r) } var _ fstests.InternalTester = (*Fs)(nil) diff --git a/fs/log.go b/fs/log.go index e13c948a8..d84458a37 100644 --- a/fs/log.go +++ b/fs/log.go @@ -203,7 +203,7 @@ func PrettyPrint(in any, label string, level LogLevel) { return } inBytes, err := json.MarshalIndent(in, "", "\t") - if err != nil { + if err != nil || string(inBytes) == "{}" || string(inBytes) == "[]" { LogPrintf(level, label, "\n%+v\n", in) return }