From dad1159864269ec0ed55b488ba9a25ce2dfc6c09 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Thu, 28 May 2026 14:11:34 +0200 Subject: [PATCH] Add Dav Signing Functionality WebDAV needs to sign URLs against two different endpoints: one for internal consumers (Diego cells via BOSH network) and one for external consumers (CAPI downloads via gorouter). Adds public_endpoint to DAV config, SignInternal and SignPublic methods to the DAV client, and sign-internal/sign-public CLI commands via a type assertion so the cross-backend Storager interface stays unchanged. --- dav/client/client.go | 36 ++++ dav/client/client_test.go | 136 +++++++++++++++ dav/client/clientfakes/fake_storage_client.go | 162 ++++++++++++++++++ dav/client/storage_client.go | 24 ++- dav/config/config.go | 13 +- storage/commandexecuter.go | 36 ++++ 6 files changed, 397 insertions(+), 10 deletions(-) diff --git a/dav/client/client.go b/dav/client/client.go index 0a015c4..d9d5913 100644 --- a/dav/client/client.go +++ b/dav/client/client.go @@ -138,6 +138,42 @@ func (d *DavBlobstore) Sign(dest string, action string, expiration time.Duration } } +func (d *DavBlobstore) SignInternal(dest string, action string, expiration time.Duration) (string, error) { + slog.Info("signing internal url for webdav", "dest", dest, "action", action, "expiration", expiration) + if err := validateBlobID(dest); err != nil { + return "", err + } + action = strings.ToUpper(action) + switch action { + case "GET", "PUT": + signedURL, err := d.storageClient.SignInternal(dest, action, expiration) + if err != nil { + return "", fmt.Errorf("failed to sign internal URL: %w", err) + } + return signedURL, nil + default: + return "", fmt.Errorf("action not implemented: %s", action) + } +} + +func (d *DavBlobstore) SignPublic(dest string, action string, expiration time.Duration) (string, error) { + slog.Info("signing public url for webdav", "dest", dest, "action", action, "expiration", expiration) + if err := validateBlobID(dest); err != nil { + return "", err + } + action = strings.ToUpper(action) + switch action { + case "GET", "PUT": + signedURL, err := d.storageClient.SignPublic(dest, action, expiration) + if err != nil { + return "", fmt.Errorf("failed to sign public URL: %w", err) + } + return signedURL, nil + default: + return "", fmt.Errorf("action not implemented: %s", action) + } +} + func (d *DavBlobstore) DeleteRecursive(prefix string) error { slog.Info("deleting blobs recursively from webdav", "prefix", prefix) return d.storageClient.DeleteRecursive(prefix) diff --git a/dav/client/client_test.go b/dav/client/client_test.go index 23efc5b..c507749 100644 --- a/dav/client/client_test.go +++ b/dav/client/client_test.go @@ -185,6 +185,142 @@ var _ = Describe("Client", func() { }) }) + Context("SignInternal", func() { + var expiry = 100 * time.Second + + It("forwards args to the storage client and returns the signed URL", func() { + fakeStorageClient := &clientfakes.FakeStorageClient{} + fakeStorageClient.SignInternalReturns("https://internal-url", nil) + + davBlobstore := client.NewWithStorageClient(fakeStorageClient) + url, err := davBlobstore.SignInternal("blob/path", "get", expiry) + + Expect(err).NotTo(HaveOccurred()) + Expect(url).To(Equal("https://internal-url")) + + Expect(fakeStorageClient.SignInternalCallCount()).To(Equal(1)) + object, action, expiration := fakeStorageClient.SignInternalArgsForCall(0) + Expect(object).To(Equal("blob/path")) + Expect(action).To(Equal("GET")) + Expect(expiration).To(Equal(expiry)) + }) + + It("uppercases the action before forwarding", func() { + fakeStorageClient := &clientfakes.FakeStorageClient{} + fakeStorageClient.SignInternalReturns("https://internal-url", nil) + + davBlobstore := client.NewWithStorageClient(fakeStorageClient) + _, err := davBlobstore.SignInternal("blob/path", "put", expiry) + Expect(err).NotTo(HaveOccurred()) + + _, action, _ := fakeStorageClient.SignInternalArgsForCall(0) + Expect(action).To(Equal("PUT")) + }) + + It("rejects an invalid blob ID without calling the storage client", func() { + fakeStorageClient := &clientfakes.FakeStorageClient{} + + davBlobstore := client.NewWithStorageClient(fakeStorageClient) + url, err := davBlobstore.SignInternal("", "get", expiry) + + Expect(err).To(HaveOccurred()) + Expect(url).To(Equal("")) + Expect(fakeStorageClient.SignInternalCallCount()).To(Equal(0)) + }) + + It("rejects unknown actions without calling the storage client", func() { + fakeStorageClient := &clientfakes.FakeStorageClient{} + + davBlobstore := client.NewWithStorageClient(fakeStorageClient) + url, err := davBlobstore.SignInternal("blob/path", "delete", expiry) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("action not implemented")) + Expect(url).To(Equal("")) + Expect(fakeStorageClient.SignInternalCallCount()).To(Equal(0)) + }) + + It("propagates errors from the storage client", func() { + fakeStorageClient := &clientfakes.FakeStorageClient{} + fakeStorageClient.SignInternalReturns("", fmt.Errorf("internal-boom")) + + davBlobstore := client.NewWithStorageClient(fakeStorageClient) + url, err := davBlobstore.SignInternal("blob/path", "get", expiry) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("internal-boom")) + Expect(url).To(Equal("")) + }) + }) + + Context("SignPublic", func() { + var expiry = 100 * time.Second + + It("forwards args to the storage client and returns the signed URL", func() { + fakeStorageClient := &clientfakes.FakeStorageClient{} + fakeStorageClient.SignPublicReturns("https://public-url", nil) + + davBlobstore := client.NewWithStorageClient(fakeStorageClient) + url, err := davBlobstore.SignPublic("blob/path", "get", expiry) + + Expect(err).NotTo(HaveOccurred()) + Expect(url).To(Equal("https://public-url")) + + Expect(fakeStorageClient.SignPublicCallCount()).To(Equal(1)) + object, action, expiration := fakeStorageClient.SignPublicArgsForCall(0) + Expect(object).To(Equal("blob/path")) + Expect(action).To(Equal("GET")) + Expect(expiration).To(Equal(expiry)) + }) + + It("uppercases the action before forwarding", func() { + fakeStorageClient := &clientfakes.FakeStorageClient{} + fakeStorageClient.SignPublicReturns("https://public-url", nil) + + davBlobstore := client.NewWithStorageClient(fakeStorageClient) + _, err := davBlobstore.SignPublic("blob/path", "put", expiry) + Expect(err).NotTo(HaveOccurred()) + + _, action, _ := fakeStorageClient.SignPublicArgsForCall(0) + Expect(action).To(Equal("PUT")) + }) + + It("rejects an invalid blob ID without calling the storage client", func() { + fakeStorageClient := &clientfakes.FakeStorageClient{} + + davBlobstore := client.NewWithStorageClient(fakeStorageClient) + url, err := davBlobstore.SignPublic("", "get", expiry) + + Expect(err).To(HaveOccurred()) + Expect(url).To(Equal("")) + Expect(fakeStorageClient.SignPublicCallCount()).To(Equal(0)) + }) + + It("rejects unknown actions without calling the storage client", func() { + fakeStorageClient := &clientfakes.FakeStorageClient{} + + davBlobstore := client.NewWithStorageClient(fakeStorageClient) + url, err := davBlobstore.SignPublic("blob/path", "delete", expiry) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("action not implemented")) + Expect(url).To(Equal("")) + Expect(fakeStorageClient.SignPublicCallCount()).To(Equal(0)) + }) + + It("propagates errors from the storage client", func() { + fakeStorageClient := &clientfakes.FakeStorageClient{} + fakeStorageClient.SignPublicReturns("", fmt.Errorf("public-boom")) + + davBlobstore := client.NewWithStorageClient(fakeStorageClient) + url, err := davBlobstore.SignPublic("blob/path", "get", expiry) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("public-boom")) + Expect(url).To(Equal("")) + }) + }) + Context("Copy", func() { It("forwards source and destination to the storage client", func() { fakeStorageClient := &clientfakes.FakeStorageClient{} diff --git a/dav/client/clientfakes/fake_storage_client.go b/dav/client/clientfakes/fake_storage_client.go index 6d4cf55..17262c8 100644 --- a/dav/client/clientfakes/fake_storage_client.go +++ b/dav/client/clientfakes/fake_storage_client.go @@ -132,6 +132,36 @@ type FakeStorageClient struct { result1 string result2 error } + SignInternalStub func(string, string, time.Duration) (string, error) + signInternalMutex sync.RWMutex + signInternalArgsForCall []struct { + arg1 string + arg2 string + arg3 time.Duration + } + signInternalReturns struct { + result1 string + result2 error + } + signInternalReturnsOnCall map[int]struct { + result1 string + result2 error + } + SignPublicStub func(string, string, time.Duration) (string, error) + signPublicMutex sync.RWMutex + signPublicArgsForCall []struct { + arg1 string + arg2 string + arg3 time.Duration + } + signPublicReturns struct { + result1 string + result2 error + } + signPublicReturnsOnCall map[int]struct { + result1 string + result2 error + } invocations map[string][][]interface{} invocationsMutex sync.RWMutex } @@ -755,6 +785,138 @@ func (fake *FakeStorageClient) SignReturnsOnCall(i int, result1 string, result2 }{result1, result2} } +func (fake *FakeStorageClient) SignInternal(arg1 string, arg2 string, arg3 time.Duration) (string, error) { + fake.signInternalMutex.Lock() + ret, specificReturn := fake.signInternalReturnsOnCall[len(fake.signInternalArgsForCall)] + fake.signInternalArgsForCall = append(fake.signInternalArgsForCall, struct { + arg1 string + arg2 string + arg3 time.Duration + }{arg1, arg2, arg3}) + stub := fake.SignInternalStub + fakeReturns := fake.signInternalReturns + fake.recordInvocation("SignInternal", []interface{}{arg1, arg2, arg3}) + fake.signInternalMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeStorageClient) SignInternalCallCount() int { + fake.signInternalMutex.RLock() + defer fake.signInternalMutex.RUnlock() + return len(fake.signInternalArgsForCall) +} + +func (fake *FakeStorageClient) SignInternalCalls(stub func(string, string, time.Duration) (string, error)) { + fake.signInternalMutex.Lock() + defer fake.signInternalMutex.Unlock() + fake.SignInternalStub = stub +} + +func (fake *FakeStorageClient) SignInternalArgsForCall(i int) (string, string, time.Duration) { + fake.signInternalMutex.RLock() + defer fake.signInternalMutex.RUnlock() + argsForCall := fake.signInternalArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 +} + +func (fake *FakeStorageClient) SignInternalReturns(result1 string, result2 error) { + fake.signInternalMutex.Lock() + defer fake.signInternalMutex.Unlock() + fake.SignInternalStub = nil + fake.signInternalReturns = struct { + result1 string + result2 error + }{result1, result2} +} + +func (fake *FakeStorageClient) SignInternalReturnsOnCall(i int, result1 string, result2 error) { + fake.signInternalMutex.Lock() + defer fake.signInternalMutex.Unlock() + fake.SignInternalStub = nil + if fake.signInternalReturnsOnCall == nil { + fake.signInternalReturnsOnCall = make(map[int]struct { + result1 string + result2 error + }) + } + fake.signInternalReturnsOnCall[i] = struct { + result1 string + result2 error + }{result1, result2} +} + +func (fake *FakeStorageClient) SignPublic(arg1 string, arg2 string, arg3 time.Duration) (string, error) { + fake.signPublicMutex.Lock() + ret, specificReturn := fake.signPublicReturnsOnCall[len(fake.signPublicArgsForCall)] + fake.signPublicArgsForCall = append(fake.signPublicArgsForCall, struct { + arg1 string + arg2 string + arg3 time.Duration + }{arg1, arg2, arg3}) + stub := fake.SignPublicStub + fakeReturns := fake.signPublicReturns + fake.recordInvocation("SignPublic", []interface{}{arg1, arg2, arg3}) + fake.signPublicMutex.Unlock() + if stub != nil { + return stub(arg1, arg2, arg3) + } + if specificReturn { + return ret.result1, ret.result2 + } + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeStorageClient) SignPublicCallCount() int { + fake.signPublicMutex.RLock() + defer fake.signPublicMutex.RUnlock() + return len(fake.signPublicArgsForCall) +} + +func (fake *FakeStorageClient) SignPublicCalls(stub func(string, string, time.Duration) (string, error)) { + fake.signPublicMutex.Lock() + defer fake.signPublicMutex.Unlock() + fake.SignPublicStub = stub +} + +func (fake *FakeStorageClient) SignPublicArgsForCall(i int) (string, string, time.Duration) { + fake.signPublicMutex.RLock() + defer fake.signPublicMutex.RUnlock() + argsForCall := fake.signPublicArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2, argsForCall.arg3 +} + +func (fake *FakeStorageClient) SignPublicReturns(result1 string, result2 error) { + fake.signPublicMutex.Lock() + defer fake.signPublicMutex.Unlock() + fake.SignPublicStub = nil + fake.signPublicReturns = struct { + result1 string + result2 error + }{result1, result2} +} + +func (fake *FakeStorageClient) SignPublicReturnsOnCall(i int, result1 string, result2 error) { + fake.signPublicMutex.Lock() + defer fake.signPublicMutex.Unlock() + fake.SignPublicStub = nil + if fake.signPublicReturnsOnCall == nil { + fake.signPublicReturnsOnCall = make(map[int]struct { + result1 string + result2 error + }) + } + fake.signPublicReturnsOnCall[i] = struct { + result1 string + result2 error + }{result1, result2} +} + func (fake *FakeStorageClient) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() diff --git a/dav/client/storage_client.go b/dav/client/storage_client.go index f77d196..c8071ef 100644 --- a/dav/client/storage_client.go +++ b/dav/client/storage_client.go @@ -26,6 +26,8 @@ type StorageClient interface { Delete(path string) (err error) DeleteRecursive(prefix string) error Sign(objectID, action string, duration time.Duration) (string, error) + SignInternal(objectID, action string, duration time.Duration) (string, error) + SignPublic(objectID, action string, duration time.Duration) (string, error) Copy(srcBlob, dstBlob string) error List(prefix string) ([]string, error) Properties(path string) error @@ -189,15 +191,29 @@ func (c *storageClient) Delete(path string) error { return nil } +// Sign delegates to SignInternal; kept for callers that don't need the public/private distinction. func (c *storageClient) Sign(blobID, action string, duration time.Duration) (string, error) { - signer := URLsigner.NewSigner(c.config.Secret) - signTime := time.Now() + return c.SignInternal(blobID, action, duration) +} - signedURL, err := signer.GenerateSignedURL(c.config.Endpoint, blobID, action, signTime, duration) +func (c *storageClient) SignInternal(blobID, action string, duration time.Duration) (string, error) { + return c.signAgainst(c.config.Endpoint, blobID, action, duration) +} + +func (c *storageClient) SignPublic(blobID, action string, duration time.Duration) (string, error) { + endpoint := c.config.PublicEndpoint + if endpoint == "" { + endpoint = c.config.Endpoint + } + return c.signAgainst(endpoint, blobID, action, duration) +} + +func (c *storageClient) signAgainst(endpoint, blobID, action string, duration time.Duration) (string, error) { + signer := URLsigner.NewSigner(c.config.Secret) + signedURL, err := signer.GenerateSignedURL(endpoint, blobID, action, time.Now(), duration) if err != nil { return "", fmt.Errorf("pre-signing the url: %w", err) } - return signedURL, nil } diff --git a/dav/config/config.go b/dav/config/config.go index 40711ac..34c8442 100644 --- a/dav/config/config.go +++ b/dav/config/config.go @@ -6,12 +6,13 @@ import ( ) type Config struct { - User string - Password string - Endpoint string - RetryAttempts uint - TLS TLS - Secret string + User string + Password string + Endpoint string + PublicEndpoint string `json:"public_endpoint"` + RetryAttempts uint + TLS TLS + Secret string } type TLS struct { diff --git a/storage/commandexecuter.go b/storage/commandexecuter.go index 179a8e4..4cc693b 100644 --- a/storage/commandexecuter.go +++ b/storage/commandexecuter.go @@ -107,6 +107,42 @@ func (sty *CommandExecuter) Execute(cmd string, nonFlagArgs []string) error { } fmt.Print(signedURL) + case "sign-internal", "sign-public": + if len(nonFlagArgs) != 3 { + return fmt.Errorf("%s method expects 3 arguments got %d", cmd, len(nonFlagArgs)) + } + + objectID, action := nonFlagArgs[0], nonFlagArgs[1] + action = strings.ToLower(action) + if action != "get" && action != "put" { + return fmt.Errorf("action not implemented: %s. Available actions are 'get' and 'put'", action) + } + + expiration, err := time.ParseDuration(nonFlagArgs[2]) + if err != nil { + return fmt.Errorf("expiration should be in the format of a duration i.e. 1h, 60m, 3600s. Got: %s", nonFlagArgs[2]) + } + + // DAV-specific: type-assert rather than pollute the cross-backend Storager interface. + dualSigner, ok := sty.str.(interface { + SignInternal(string, string, time.Duration) (string, error) + SignPublic(string, string, time.Duration) (string, error) + }) + if !ok { + return fmt.Errorf("%s is only supported by storage backends with separate internal/public endpoints (currently: dav)", cmd) + } + + var signedURL string + if cmd == "sign-internal" { + signedURL, err = dualSigner.SignInternal(objectID, action, expiration) + } else { + signedURL, err = dualSigner.SignPublic(objectID, action, expiration) + } + if err != nil { + return fmt.Errorf("failed to %s request: %w", cmd, err) + } + fmt.Print(signedURL) + case "list": var prefix string if len(nonFlagArgs) > 1 {