Skip to content

Commit c895a8e

Browse files
michaelstinglrhafer
authored andcommitted
feat(tus): return etag and permissions after the last TUS chunk
The finalizing PATCH of a chunked TUS upload returned only the file id, so a client had to issue a follow-up PROPFIND to learn the new etag and permissions, an extra round-trip per upload. The creation-with-upload path already returns them: the ocdav gateway stats the resource and sets OC-ETag/ETag/OC-Perm. This registers tusd's PreFinishResponseCallback in the dataprovider tus handler and attaches the same headers to the chunked finalize. The finalized resource is resolved by the storage driver through a new UploadSessionResolver interface, so the generic datatx layer does not reconstruct the upload's executant: decomposedfs resolves the finished upload as its own executant (the data path's transfer token carries no user identity) and returns the resource info. The session is rebuilt from the in-hand tus info, so it still resolves after a synchronous upload has cleaned up its session. The lookup is best-effort: on a stat error, or a driver that does not implement the interface, it sets no headers and leaves the PROPFIND fallback intact. The WebDAV-permission derivation is shared with the gateway's creation-with-upload path via net.WebDAVPermissions, so the two cannot drift; the propfind handler keeps its own share-types-aware variant. Fixes: opencloud-eu/opencloud#2409
1 parent e87f7a0 commit c895a8e

12 files changed

Lines changed: 390 additions & 20 deletions

File tree

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// Copyright 2018-2026 CERN
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
//
15+
// In applying this license, CERN does not waive the privileges and immunities
16+
// granted to it by virtue of its status as an Intergovernmental Organization
17+
// or submit itself to any jurisdiction.
18+
19+
package net
20+
21+
import (
22+
"context"
23+
"encoding/json"
24+
25+
link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1"
26+
provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
27+
28+
"github.com/opencloud-eu/reva/v2/pkg/conversions"
29+
)
30+
31+
// WebDAVPermissions derives the WebDAV permissions string (the OC-Perm header value) for a
32+
// resource. It is shared by the ocdav gateway's creation-with-upload response and the
33+
// dataprovider's chunked TUS finalize response, so the two report the same permissions.
34+
func WebDAVPermissions(ctx context.Context, ri *provider.ResourceInfo) string {
35+
isPublic := false
36+
if o := ri.GetOpaque(); o != nil && o.Map != nil {
37+
if e := o.Map["link-share"]; e != nil && e.Decoder == "json" {
38+
ls := &link.PublicShare{}
39+
_ = json.Unmarshal(e.Value, ls)
40+
isPublic = ls != nil
41+
}
42+
}
43+
isShared := !IsCurrentUserOwnerOrManager(ctx, ri.GetOwner(), ri)
44+
role := conversions.RoleFromResourcePermissions(ri.GetPermissionSet(), isPublic)
45+
return role.WebDAVPermissions(
46+
ri.GetType() == provider.ResourceType_RESOURCE_TYPE_CONTAINER,
47+
isShared,
48+
false,
49+
isPublic,
50+
)
51+
}
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
// Copyright 2018-2026 CERN
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
//
15+
// In applying this license, CERN does not waive the privileges and immunities
16+
// granted to it by virtue of its status as an Intergovernmental Organization
17+
// or submit itself to any jurisdiction.
18+
19+
package net_test
20+
21+
import (
22+
"context"
23+
"encoding/json"
24+
25+
userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
26+
link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1"
27+
provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
28+
typespb "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
29+
"github.com/opencloud-eu/reva/v2/internal/http/services/owncloud/ocdav/net"
30+
ctxpkg "github.com/opencloud-eu/reva/v2/pkg/ctx"
31+
32+
. "github.com/onsi/ginkgo/v2"
33+
. "github.com/onsi/gomega"
34+
)
35+
36+
var _ = Describe("WebDAVPermissions", func() {
37+
var (
38+
owner = &userpb.User{Id: &userpb.UserId{OpaqueId: "owner"}, Username: "owner"}
39+
other = &userpb.User{Id: &userpb.UserId{OpaqueId: "other"}, Username: "other"}
40+
41+
// a resource the user may edit, owned by "owner"
42+
resource = func(t provider.ResourceType, opaque *typespb.Opaque) *provider.ResourceInfo {
43+
return &provider.ResourceInfo{
44+
Type: t,
45+
Owner: owner.Id,
46+
Opaque: opaque,
47+
PermissionSet: &provider.ResourcePermissions{
48+
Stat: true, ListContainer: true, InitiateFileDownload: true,
49+
InitiateFileUpload: true, Move: true, Delete: true,
50+
},
51+
}
52+
}
53+
54+
ownerCtx = ctxpkg.ContextSetUser(context.Background(), owner)
55+
otherCtx = ctxpkg.ContextSetUser(context.Background(), other)
56+
)
57+
58+
It("omits the shared marker when the current user owns the resource", func() {
59+
Expect(net.WebDAVPermissions(ownerCtx, resource(provider.ResourceType_RESOURCE_TYPE_FILE, nil))).ToNot(HavePrefix("S"))
60+
})
61+
62+
It("leads with the shared marker when the resource is not owned by the current user", func() {
63+
Expect(net.WebDAVPermissions(otherCtx, resource(provider.ResourceType_RESOURCE_TYPE_FILE, nil))).To(HavePrefix("S"))
64+
})
65+
66+
It("returns a permission string for a container", func() {
67+
perms := net.WebDAVPermissions(otherCtx, resource(provider.ResourceType_RESOURCE_TYPE_CONTAINER, nil))
68+
Expect(perms).ToNot(BeEmpty())
69+
Expect(perms).To(HavePrefix("S"))
70+
})
71+
72+
It("handles a public-link resource via the link-share opaque", func() {
73+
b, err := json.Marshal(&link.PublicShare{Id: &link.PublicShareId{OpaqueId: "share-1"}})
74+
Expect(err).ToNot(HaveOccurred())
75+
opaque := &typespb.Opaque{Map: map[string]*typespb.OpaqueEntry{
76+
"link-share": {Decoder: "json", Value: b},
77+
}}
78+
Expect(net.WebDAVPermissions(ownerCtx, resource(provider.ResourceType_RESOURCE_TYPE_FILE, opaque))).ToNot(BeEmpty())
79+
})
80+
})

internal/http/services/owncloud/ocdav/tus.go

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ package ocdav
2020

2121
import (
2222
"context"
23-
"encoding/json"
2423
"io"
2524
"net/http"
2625
"path"
@@ -30,14 +29,12 @@ import (
3029
"time"
3130

3231
rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
33-
link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1"
3432
provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
3533
typespb "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
3634
"github.com/opencloud-eu/reva/v2/internal/http/services/owncloud/ocdav/errors"
3735
"github.com/opencloud-eu/reva/v2/internal/http/services/owncloud/ocdav/net"
3836
"github.com/opencloud-eu/reva/v2/internal/http/services/owncloud/ocdav/spacelookup"
3937
"github.com/opencloud-eu/reva/v2/pkg/appctx"
40-
"github.com/opencloud-eu/reva/v2/pkg/conversions"
4138
"github.com/opencloud-eu/reva/v2/pkg/rhttp"
4239
"github.com/opencloud-eu/reva/v2/pkg/storagespace"
4340
"github.com/opencloud-eu/reva/v2/pkg/utils"
@@ -374,23 +371,7 @@ func (s *svc) handleTusPost(ctx context.Context, w http.ResponseWriter, r *http.
374371
return
375372
}
376373

377-
// get WebDav permissions for file
378-
isPublic := false
379-
if info.Opaque != nil && info.Opaque.Map != nil {
380-
if info.Opaque.Map["link-share"] != nil && info.Opaque.Map["link-share"].Decoder == "json" {
381-
ls := &link.PublicShare{}
382-
_ = json.Unmarshal(info.Opaque.Map["link-share"].Value, ls)
383-
isPublic = ls != nil
384-
}
385-
}
386-
isShared := !net.IsCurrentUserOwnerOrManager(ctx, info.Owner, info)
387-
role := conversions.RoleFromResourcePermissions(info.PermissionSet, isPublic)
388-
permissions := role.WebDAVPermissions(
389-
info.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER,
390-
isShared,
391-
false,
392-
isPublic,
393-
)
374+
permissions := net.WebDAVPermissions(ctx, info)
394375

395376
w.Header().Set(net.HeaderContentType, info.MimeType)
396377
w.Header().Set(net.HeaderOCFileID, storagespace.FormatResourceID(info.Id))

pkg/rhttp/datatx/manager/tus/tus.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,9 @@ func (m *manager) Handler(fs storage.FS) (http.Handler, error) {
108108
StoreComposer: composer,
109109
NotifyCompleteUploads: true,
110110
Logger: slog.New(tusdLogger{log: m.log}),
111+
// Add the finalized resource's etag and permissions to the last chunk's response
112+
// (see preFinishResponseCallback). https://github.com/opencloud-eu/opencloud/issues/2409
113+
PreFinishResponseCallback: m.preFinishResponseCallback(fs),
111114
}
112115

113116
if m.conf.CorsEnabled {
@@ -207,6 +210,42 @@ func (m *manager) Handler(fs storage.FS) (http.Handler, error) {
207210
return h, nil
208211
}
209212

213+
// preFinishResponseCallback returns the tusd callback that runs when an upload completes. Before
214+
// the final response is written, it looks up the finalized resource and attaches its etag and
215+
// WebDAV permissions to the response headers, so clients do not need a follow-up PROPFIND after
216+
// the last chunk. It mirrors the etag and permissions the ocdav gateway already produces for the
217+
// creation-with-upload path. The storage driver resolves the resource as the upload's executant
218+
// (the data path's transfer token carries no user identity), so this layer does not reconstruct
219+
// the user. It is best effort: it logs any lookup failure and still completes the upload (the
220+
// client falls back to a PROPFIND). https://github.com/opencloud-eu/opencloud/issues/2409
221+
func (m *manager) preFinishResponseCallback(fs storage.FS) func(tusd.HookEvent) (tusd.HTTPResponse, error) {
222+
resolver, ok := fs.(storage.UploadSessionResolver)
223+
return func(hook tusd.HookEvent) (tusd.HTTPResponse, error) {
224+
resp := tusd.HTTPResponse{Header: tusd.HTTPHeader{}}
225+
if !ok {
226+
// the storage driver cannot resolve the upload; the client falls back to a PROPFIND
227+
return resp, nil
228+
}
229+
230+
ri, ctx, err := resolver.ResolveUpload(hook.Context, hook.Upload)
231+
if err != nil || ri == nil {
232+
// The stat can fail for a finished upload (for example an expired upload token or a
233+
// removed node); the client recovers with a PROPFIND, so this is a warning, not an error.
234+
m.log.Warn().Err(err).Str("uploadid", hook.Upload.ID).Msg("could not stat finished upload, not setting etag/permission headers")
235+
return resp, nil
236+
}
237+
238+
if etag := ri.GetEtag(); etag != "" {
239+
resp.Header[net.HeaderOCETag] = etag
240+
resp.Header[net.HeaderETag] = etag
241+
}
242+
243+
resp.Header[net.HeaderOCPermissions] = net.WebDAVPermissions(ctx, ri)
244+
245+
return resp, nil
246+
}
247+
}
248+
210249
func setHeaders(fs storage.FS, w http.ResponseWriter, r *http.Request) {
211250
ctx := r.Context()
212251
id := path.Base(r.URL.Path)
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
// Copyright 2026 OpenCloud GmbH <mail@opencloud.eu>
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package tus
5+
6+
import (
7+
"context"
8+
"errors"
9+
10+
userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
11+
provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
12+
"github.com/rs/zerolog"
13+
tusd "github.com/tus/tusd/v2/pkg/handler"
14+
15+
. "github.com/onsi/ginkgo/v2"
16+
. "github.com/onsi/gomega"
17+
18+
"github.com/opencloud-eu/reva/v2/internal/http/services/owncloud/ocdav/net"
19+
ctxpkg "github.com/opencloud-eu/reva/v2/pkg/ctx"
20+
"github.com/opencloud-eu/reva/v2/pkg/storage"
21+
)
22+
23+
// fakeResolver implements storage.UploadSessionResolver. Embedding storage.FS satisfies the
24+
// rest of that interface (those methods are never called by the callback under test). It
25+
// records the upload info it was called with and returns a preconfigured resource, context
26+
// and error, so the tests can drive the callback without a real storage backend.
27+
type fakeResolver struct {
28+
storage.FS
29+
ri *provider.ResourceInfo
30+
ctx context.Context
31+
err error
32+
gotInfo tusd.FileInfo
33+
}
34+
35+
func (f *fakeResolver) ResolveUpload(ctx context.Context, info tusd.FileInfo) (*provider.ResourceInfo, context.Context, error) {
36+
f.gotInfo = info
37+
rctx := f.ctx
38+
if rctx == nil {
39+
rctx = ctx
40+
}
41+
return f.ri, rctx, f.err
42+
}
43+
44+
// plainFS implements only storage.FS, not the resolver, so the callback takes its no-op path.
45+
type plainFS struct{ storage.FS }
46+
47+
// testManager builds a manager with a no-op logger, the way New sets one in production, so the
48+
// callback's warn path does not dereference a nil logger.
49+
func testManager() *manager {
50+
l := zerolog.Nop()
51+
return &manager{log: &l}
52+
}
53+
54+
var _ = Describe("preFinishResponseCallback", func() {
55+
executant := &userpb.UserId{Idp: "https://idp.example", OpaqueId: "executant-1"}
56+
asExecutant := ctxpkg.ContextSetUser(context.Background(), &userpb.User{Id: executant})
57+
58+
// a resource the executant may edit; owned by someone else unless overridden
59+
editable := func(etag, ownerID string) *provider.ResourceInfo {
60+
return &provider.ResourceInfo{
61+
Etag: etag,
62+
Type: provider.ResourceType_RESOURCE_TYPE_FILE,
63+
Owner: &userpb.UserId{Idp: "https://idp.example", OpaqueId: ownerID},
64+
PermissionSet: &provider.ResourcePermissions{
65+
Stat: true, ListContainer: true, InitiateFileDownload: true,
66+
InitiateFileUpload: true, Move: true, Delete: true,
67+
},
68+
}
69+
}
70+
71+
hook := tusd.HookEvent{
72+
Context: context.Background(),
73+
Upload: tusd.FileInfo{ID: "upload-1", Storage: map[string]string{"NodeId": "node-1"}},
74+
}
75+
76+
It("resolves the finished upload and sets etag and permissions", func() {
77+
fs := &fakeResolver{ri: editable(`"abc123"`, "other-owner"), ctx: asExecutant}
78+
79+
resp, err := testManager().preFinishResponseCallback(fs)(hook)
80+
81+
Expect(err).ToNot(HaveOccurred())
82+
// the in-hand tus info is handed to the resolver, not looked up by id in the session store
83+
Expect(fs.gotInfo.ID).To(Equal("upload-1"))
84+
// etag mirrored into both headers
85+
Expect(resp.Header[net.HeaderOCETag]).To(Equal(`"abc123"`))
86+
Expect(resp.Header[net.HeaderETag]).To(Equal(`"abc123"`))
87+
// resolved as a user who does not own the resource -> shared -> permissions lead with S
88+
Expect(resp.Header[net.HeaderOCPermissions]).To(HavePrefix("S"))
89+
})
90+
91+
It("omits the shared marker when the resource is owned by the resolving user", func() {
92+
fs := &fakeResolver{ri: editable(`"abc123"`, "executant-1"), ctx: asExecutant}
93+
94+
resp, err := testManager().preFinishResponseCallback(fs)(hook)
95+
96+
Expect(err).ToNot(HaveOccurred())
97+
Expect(resp.Header[net.HeaderOCPermissions]).ToNot(BeEmpty())
98+
Expect(resp.Header[net.HeaderOCPermissions]).ToNot(HavePrefix("S"))
99+
})
100+
101+
It("is best-effort: no headers and no error when the resolve fails", func() {
102+
fs := &fakeResolver{err: errors.New("not found"), ctx: asExecutant}
103+
104+
resp, err := testManager().preFinishResponseCallback(fs)(hook)
105+
106+
Expect(err).ToNot(HaveOccurred())
107+
Expect(resp.Header).ToNot(HaveKey(net.HeaderOCETag))
108+
Expect(resp.Header).ToNot(HaveKey(net.HeaderETag))
109+
Expect(resp.Header).ToNot(HaveKey(net.HeaderOCPermissions))
110+
})
111+
112+
It("is best-effort: no headers and no error when the resolve returns no resource", func() {
113+
fs := &fakeResolver{ri: nil, ctx: asExecutant}
114+
115+
resp, err := testManager().preFinishResponseCallback(fs)(hook)
116+
117+
Expect(err).ToNot(HaveOccurred())
118+
Expect(resp.Header).ToNot(HaveKey(net.HeaderOCPermissions))
119+
Expect(resp.Header).ToNot(HaveKey(net.HeaderOCETag))
120+
})
121+
122+
It("omits the etag header when the resource has none", func() {
123+
fs := &fakeResolver{ri: editable("", "other-owner"), ctx: asExecutant}
124+
125+
resp, err := testManager().preFinishResponseCallback(fs)(hook)
126+
127+
Expect(err).ToNot(HaveOccurred())
128+
Expect(resp.Header).ToNot(HaveKey(net.HeaderOCETag))
129+
Expect(resp.Header).To(HaveKey(net.HeaderOCPermissions))
130+
})
131+
132+
It("does nothing when the storage driver cannot resolve uploads", func() {
133+
resp, err := testManager().preFinishResponseCallback(plainFS{})(hook)
134+
135+
Expect(err).ToNot(HaveOccurred())
136+
Expect(resp.Header).ToNot(HaveKey(net.HeaderOCETag))
137+
Expect(resp.Header).ToNot(HaveKey(net.HeaderOCPermissions))
138+
})
139+
})
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Copyright 2026 OpenCloud GmbH <mail@opencloud.eu>
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package tus
5+
6+
import (
7+
"testing"
8+
9+
. "github.com/onsi/ginkgo/v2"
10+
. "github.com/onsi/gomega"
11+
)
12+
13+
func TestTus(t *testing.T) {
14+
RegisterFailHandler(Fail)
15+
RunSpecs(t, "Tus Suite")
16+
}

0 commit comments

Comments
 (0)