Repository: owncloud/ocis
Pull request #8384 (2024-02-07 08:33:56)

fix: signed url expiry check and signature validation - stable-4.0
<!--
Thanks for submitting a change to ownCloud!

This is the bug tracker for the oCIS component. Find other components at https://github.com/owncloud/core/blob/master/.github/CONTRIBUTING.md#guidelines

For fixing potential security issues please see https://owncloud.org/security/

To make it possible for us to get your change reviewed and merged please carefully fill out the requested information below.

Please note that any kind of change needs first be submitted to the master branch which holds the next version of oCIS.

Please set the following labels:

- Set label "Status:Needs-Review" for review or "Status:In-Progress" if the PR still has open tasks.
- Assignment: assign to self
- Reviewers: pick at least one
-->

## Description
<!--- Describe your changes in detail -->

## Related Issue
<!--- This project only accepts pull requests related to open issues -->
<!--- If suggesting a new feature or change, please discuss it in an issue first -->
<!--- If fixing a bug, there should be an issue describing it with steps to reproduce -->
<!--- Please link to the issue here: -->
- Fixes <issue_link>

## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->

## How Has This Been Tested?
<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran to -->
<!--- see how your change affects other areas of the code, etc. -->
- test environment:
- test case 1:
- test case 2:
- ...

## Screenshots (if appropriate):

## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] Technical debt
- [ ] Tests only (no source changes)

## Checklist:
<!-- Tick the checkboxes when done. -->
<!-- Raise documentation ticket in owncloud.github.io/ -->
- [ ] Code changes
- [ ] Unit tests added
- [ ] Acceptance tests added
- [ ] Documentation ticket raised: <link> 


update-docs[bot]: Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a [changelog](https://github.com/owncloud/ocis/blob/master/changelog/README.md) item based on your changes.

sonarcloud[bot]: ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=owncloud_ocis&pullRequest=8384) **Quality Gate passed**  
The SonarCloud Quality Gate passed, but some issues were introduced.

[5 New issues](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=8384&resolved=false&inNewCodePeriod=true)  
[0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=owncloud_ocis&pullRequest=8384&resolved=false&inNewCodePeriod=true)  
[85.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=owncloud_ocis&pullRequest=8384&metric=new_coverage&view=list)  
[0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=owncloud_ocis&pullRequest=8384&metric=new_duplicated_lines_density&view=list)  
  
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=owncloud_ocis&pullRequest=8384)


sonarcloud[bot]: ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=owncloud_ocis&pullRequest=8384) **Quality Gate passed**  
The SonarCloud Quality Gate passed, but some issues were introduced.

[5 New issues](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=8384&resolved=false&inNewCodePeriod=true)  
[0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=owncloud_ocis&pullRequest=8384&resolved=false&inNewCodePeriod=true)  
[85.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=owncloud_ocis&pullRequest=8384&metric=new_coverage&view=list)  
[0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=owncloud_ocis&pullRequest=8384&metric=new_duplicated_lines_density&view=list)  
  
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=owncloud_ocis&pullRequest=8384)


sonarcloud[bot]: ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=owncloud_ocis&pullRequest=8384) **Quality Gate passed**  
The SonarCloud Quality Gate passed, but some issues were introduced.

[6 New issues](https://sonarcloud.io/project/issues?id=owncloud_ocis&pullRequest=8384&resolved=false&inNewCodePeriod=true)  
[0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=owncloud_ocis&pullRequest=8384&resolved=false&inNewCodePeriod=true)  
[92.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=owncloud_ocis&pullRequest=8384&metric=new_coverage&view=list)  
[0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=owncloud_ocis&pullRequest=8384&metric=new_duplicated_lines_density&view=list)  
  
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=owncloud_ocis&pullRequest=8384)



Patch:
diff --git a/changelog/unreleased/fix-signed-url-expiry.md b/changelog/unreleased/fix-signed-url-expiry.md
new file mode 100644
index 00000000000..9b0cdf3da2a
--- /dev/null
+++ b/changelog/unreleased/fix-signed-url-expiry.md
@@ -0,0 +1,5 @@
+Bugfix: signed url verification
+
+Signed urls now expire properly
+
+https://github.com/owncloud/ocis/pull/8385
diff --git a/services/proxy/pkg/command/server.go b/services/proxy/pkg/command/server.go
index ffaa6abef35..83131093155 100644
--- a/services/proxy/pkg/command/server.go
+++ b/services/proxy/pkg/command/server.go
@@ -369,6 +369,7 @@ func loadMiddlewares(ctx context.Context, logger log.Logger, cfg *config.Config,
 		UserProvider:       userProvider,
 		UserRoleAssigner:   roleAssigner,
 		Store:              storeClient,
+		Now:                time.Now,
 	})
 
 	return alice.New(
diff --git a/services/proxy/pkg/middleware/signed_url_auth.go b/services/proxy/pkg/middleware/signed_url_auth.go
index a6bc3a703e3..b3c842a26f9 100644
--- a/services/proxy/pkg/middleware/signed_url_auth.go
+++ b/services/proxy/pkg/middleware/signed_url_auth.go
@@ -27,6 +27,7 @@ const (
 	_paramOCDate       = "OC-Date"
 	_paramOCExpires    = "OC-Expires"
 	_paramOCVerb       = "OC-Verb"
+	_paramOCAlgo       = "OC-Algo"
 )
 
 var (
@@ -46,6 +47,7 @@ type SignedURLAuthenticator struct {
 	UserProvider       backend.UserBackend
 	UserRoleAssigner   userroles.UserRoleAssigner
 	Store              storesvc.StoreService
+	Now                func() time.Time
 }
 
 func (m SignedURLAuthenticator) shouldServe(req *http.Request) bool {
@@ -58,30 +60,30 @@ func (m SignedURLAuthenticator) shouldServe(req *http.Request) bool {
 func (m SignedURLAuthenticator) validate(req *http.Request) (err error) {
 	query := req.URL.Query()
 
-	if ok, err := m.allRequiredParametersArePresent(query); !ok {
+	if err := m.allRequiredParametersArePresent(query); err != nil {
 		return err
 	}
 
-	if ok, err := m.requestMethodMatches(req.Method, query); !ok {
+	if err := m.requestMethodMatches(req.Method, query); err != nil {
 		return err
 	}
 
-	if ok, err := m.requestMethodIsAllowed(req.Method); !ok {
+	if err := m.requestMethodIsAllowed(req.Method); err != nil {
 		return err
 	}
 
-	if expired, err := m.urlIsExpired(query, time.Now); expired {
+	if err = m.urlIsExpired(query); err != nil {
 		return err
 	}
 
-	if ok, err := m.signatureIsValid(req); !ok {
+	if err := m.signatureIsValid(req); err != nil {
 		return err
 	}
 
 	return nil
 }
 
-func (m SignedURLAuthenticator) allRequiredParametersArePresent(query url.Values) (ok bool, err error) {
+func (m SignedURLAuthenticator) allRequiredParametersArePresent(query url.Values) (err error) {
 	// check if required query parameters exist in given request query parameters
 	// OC-Signature - the computed signature - server will verify the request upon this REQUIRED
 	// OC-Credential - defines the user scope (shall we use the owncloud user id here - this might leak internal data ....) REQUIRED
@@ -90,23 +92,23 @@ func (m SignedURLAuthenticator) allRequiredParametersArePresent(query url.Values
 	// TODO OC-Verb - defines for which http verb the request is valid - defaults to GET OPTIONAL
 	for _, p := range _requiredParams {
 		if query.Get(p) == "" {
-			return false, fmt.Errorf("required %s parameter not found", p)
+			return fmt.Errorf("required %s parameter not found", p)
 		}
 	}
 
-	return true, nil
+	return nil
 }
 
-func (m SignedURLAuthenticator) requestMethodMatches(meth string, query url.Values) (ok bool, err error) {
+func (m SignedURLAuthenticator) requestMethodMatches(meth string, query url.Values) (err error) {
 	// check if given url query parameter OC-Verb matches given request method
 	if !strings.EqualFold(meth, query.Get(_paramOCVerb)) {
-		return false, errors.New("required OC-Verb parameter did not match request method")
+		return errors.New("required OC-Verb parameter did not match request method")
 	}
 
-	return true, nil
+	return nil
 }
 
-func (m SignedURLAuthenticator) requestMethodIsAllowed(meth string) (ok bool, err error) {
+func (m SignedURLAuthenticator) requestMethodIsAllowed(meth string) (err error) {
 	//  check if given request method is allowed
 	methodIsAllowed := false
 	for _, am := range m.PreSignedURLConfig.AllowedHTTPMethods {
@@ -117,50 +119,81 @@ func (m SignedURLAuthenticator) requestMethodIsAllowed(meth string) (ok bool, er
 	}
 
 	if !methodIsAllowed {
-		return false, errors.New("request method is not listed in PreSignedURLConfig AllowedHTTPMethods")
+		return errors.New("request method is not listed in PreSignedURLConfig AllowedHTTPMethods")
 	}
 
-	return true, nil
+	return nil
 }
 
-func (m SignedURLAuthenticator) urlIsExpired(query url.Values, now func() time.Time) (expired bool, err error) {
+func (m SignedURLAuthenticator) urlIsExpired(query url.Values) (err error) {
 	// check if url is expired by checking if given date (OC-Date) + expires in seconds (OC-Expires) is after now
 	validFrom, err := time.Parse(time.RFC3339, query.Get(_paramOCDate))
 	if err != nil {
-		return true, err
+		return err
 	}
 
 	requestExpiry, err := time.ParseDuration(query.Get(_paramOCExpires) + "s")
 	if err != nil {
-		return true, err
+		return err
 	}
 
 	validTo := validFrom.Add(requestExpiry)
+	if !(m.Now().Before(validTo)) {
+		return errors.New("URL is expired")
+	}
 
-	return !(now().After(validFrom) && now().Before(validTo)), nil
+	return nil
 }
 
-func (m SignedURLAuthenticator) signatureIsValid(req *http.Request) (ok bool, err error) {
-	u := revactx.ContextMustGetUser(req.Context())
-	signingKey, err := m.getSigningKey(req.Context(), u.Id.OpaqueId)
+func (m SignedURLAuthenticator) signatureIsValid(req *http.Request) (err error) {
+	c := revactx.ContextMustGetUser(req.Context())
+	signingKey, err := m.getSigningKey(req.Context(), c.Id.OpaqueId)
 	if err != nil {
 		m.Logger.Error().Err(err).Msg("could not retrieve signing key")
-		return false, err
+		return err
 	}
 	if len(signingKey) == 0 {
 		m.Logger.Error().Err(err).Msg("signing key empty")
-		return false, err
+		return err
+	}
+	u := m.buildUrlToSign(req)
+	computedSignature := m.createSignature(u, signingKey)
+	signatureInURL := req.URL.Query().Get(_paramOCSignature)
+	if computedSignature == signatureInURL {
+		return nil
 	}
+	return fmt.Errorf("signature mismatch: expected %s != actual %s", computedSignature, signatureInURL)
+}
+
+func (m SignedURLAuthenticator) buildUrlToSign(req *http.Request) string {
 	q := req.URL.Query()
-	signature := q.Get(_paramOCSignature)
+
+	// only params required for signing
+	signParameters := make(url.Values)
+	signParameters.Add(_paramOCCredential, q.Get(_paramOCCredential))
+	signParameters.Add(_paramOCDate, q.Get(_paramOCDate))
+	signParameters.Add(_paramOCExpires, q.Get(_paramOCExpires))
+	signParameters.Add(_paramOCVerb, q.Get(_paramOCVerb))
+
+	// remaining query params
+	q.Del(_paramOCAlgo)
+	q.Del(_paramOCCredential)
+	q.Del(_paramOCDate)
+	q.Del(_paramOCExpires)
 	q.Del(_paramOCSignature)
-	req.URL.RawQuery = q.Encode()
-	url := req.URL.String()
-	if !req.URL.IsAbs() {
-		url = "https://" + req.Host + url // TODO where do we get the scheme from
-	}
+	q.Del(_paramOCVerb)
 
-	return m.createSignature(url, signingKey) == signature, nil
+	url := *req.URL
+	if len(q) == 0 {
+		url.RawQuery = signParameters.Encode()
+	} else {
+		url.RawQuery = strings.Join([]string{q.Encode(), signParameters.Encode()}, "&")
+	}
+	u := url.String()
+	if !url.IsAbs() {
+		u = "https://" + req.Host + u // TODO where do we get the scheme
+	}
+	return u
 }
 
 func (m SignedURLAuthenticator) createSignature(url string, signingKey []byte) string {
@@ -223,10 +256,12 @@ func (m SignedURLAuthenticator) Authenticate(r *http.Request) (*http.Request, bo
 			Err(err).
 			Str("authenticator", "signed_url").
 			Str("path", r.URL.Path).
+			Str("url", r.URL.String()).
 			Msg("Could not get user by claim")
 		return nil, false
 	}
 
+	// TODO: set user in context
 	m.Logger.Debug().
 		Str("authenticator", "signed_url").
 		Str("path", r.URL.Path).


Test patch:
diff --git a/services/proxy/pkg/middleware/signed_url_auth_test.go b/services/proxy/pkg/middleware/signed_url_auth_test.go
index 35f84e66559..a14edc01f42 100644
--- a/services/proxy/pkg/middleware/signed_url_auth_test.go
+++ b/services/proxy/pkg/middleware/signed_url_auth_test.go
@@ -1,6 +1,14 @@
 package middleware
 
 import (
+	"context"
+	userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
+	revactx "github.com/cs3org/reva/v2/pkg/ctx"
+	storemsg "github.com/owncloud/ocis/v2/protogen/gen/ocis/messages/store/v0"
+	v0 "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/store/v0"
+	"github.com/owncloud/ocis/v2/services/proxy/pkg/config"
+	"github.com/stretchr/testify/assert"
+	"go-micro.dev/v4/client"
 	"net/http/httptest"
 	"testing"
 	"time"
@@ -34,21 +42,23 @@ func TestSignedURLAuth_allRequiredParametersPresent(t *testing.T) {
 	pua := SignedURLAuthenticator{}
 	baseURL := "https://example.com/example.jpg?"
 	tests := []struct {
-		params   string
-		expected bool
+		params       string
+		errorMessage string
 	}{
-		{"OC-Signature=something&OC-Credential=something&OC-Date=something&OC-Expires=something&OC-Verb=something", true},
-		{"OC-Credential=something&OC-Date=something&OC-Expires=something&OC-Verb=something", false},
-		{"OC-Signature=something&OC-Date=something&OC-Expires=something&OC-Verb=something", false},
-		{"OC-Signature=something&OC-Credential=something&OC-Expires=something&OC-Verb=something", false},
-		{"OC-Signature=something&OC-Credential=something&OC-Date=something&OC-Verb=something", false},
-		{"OC-Signature=something&OC-Credential=something&OC-Date=something&OC-Expires=something", false},
+		{"OC-Signature=something&OC-Credential=something&OC-Date=something&OC-Expires=something&OC-Verb=something", ""},
+		{"OC-Credential=something&OC-Date=something&OC-Expires=something&OC-Verb=something", "required OC-Signature parameter not found"},
+		{"OC-Signature=something&OC-Date=something&OC-Expires=something&OC-Verb=something", "required OC-Credential parameter not found"},
+		{"OC-Signature=something&OC-Credential=something&OC-Expires=something&OC-Verb=something", "required OC-Date parameter not found"},
+		{"OC-Signature=something&OC-Credential=something&OC-Date=something&OC-Verb=something", "required OC-Expires parameter not found"},
+		{"OC-Signature=something&OC-Credential=something&OC-Date=something&OC-Expires=something", "required OC-Verb parameter not found"},
 	}
 	for _, tt := range tests {
 		r := httptest.NewRequest("", baseURL+tt.params, nil)
-		ok, _ := pua.allRequiredParametersArePresent(r.URL.Query())
-		if ok != tt.expected {
-			t.Errorf("with %s expected %t got %t", tt.params, tt.expected, ok)
+		err := pua.allRequiredParametersArePresent(r.URL.Query())
+		if tt.errorMessage != "" {
+			assert.EqualError(t, err, tt.errorMessage, tt.params)
+		} else {
+			assert.Nil(t, err)
 		}
 	}
 }
@@ -56,20 +66,22 @@ func TestSignedURLAuth_allRequiredParametersPresent(t *testing.T) {
 func TestSignedURLAuth_requestMethodMatches(t *testing.T) {
 	pua := SignedURLAuthenticator{}
 	tests := []struct {
-		method   string
-		url      string
-		expected bool
+		method       string
+		url          string
+		errorMessage string
 	}{
-		{"GET", "https://example.com/example.jpg?OC-Verb=GET", true},
-		{"GET", "https://example.com/example.jpg?OC-Verb=get", true},
-		{"POST", "https://example.com/example.jpg?OC-Verb=GET", false},
+		{"GET", "https://example.com/example.jpg?OC-Verb=GET", ""},
+		{"GET", "https://example.com/example.jpg?OC-Verb=get", ""},
+		{"POST", "https://example.com/example.jpg?OC-Verb=GET", "required OC-Verb parameter did not match request method"},
 	}
 
 	for _, tt := range tests {
 		r := httptest.NewRequest(tt.method, tt.url, nil)
-		ok, _ := pua.requestMethodMatches(r.Method, r.URL.Query())
-		if ok != tt.expected {
-			t.Errorf("with method %s and url %s expected %t got %t", tt.method, tt.url, tt.expected, ok)
+		err := pua.requestMethodMatches(r.Method, r.URL.Query())
+		if tt.errorMessage != "" {
+			assert.EqualError(t, err, tt.errorMessage, tt.url)
+		} else {
+			assert.Nil(t, err)
 		}
 	}
 }
@@ -77,50 +89,62 @@ func TestSignedURLAuth_requestMethodMatches(t *testing.T) {
 func TestSignedURLAuth_requestMethodIsAllowed(t *testing.T) {
 	pua := SignedURLAuthenticator{}
 	tests := []struct {
-		method   string
-		allowed  []string
-		expected bool
+		method       string
+		allowed      []string
+		errorMessage string
 	}{
-		{"GET", []string{}, false},
-		{"GET", []string{"POST"}, false},
-		{"GET", []string{"GET"}, true},
-		{"GET", []string{"get"}, true},
-		{"GET", []string{"POST", "GET"}, true},
+		{"GET", []string{}, "request method is not listed in PreSignedURLConfig AllowedHTTPMethods"},
+		{"GET", []string{"POST"}, "request method is not listed in PreSignedURLConfig AllowedHTTPMethods"},
+		{"GET", []string{"GET"}, ""},
+		{"GET", []string{"get"}, ""},
+		{"GET", []string{"POST", "GET"}, ""},
 	}
 
 	for _, tt := range tests {
 		pua.PreSignedURLConfig.AllowedHTTPMethods = tt.allowed
-		ok, _ := pua.requestMethodIsAllowed(tt.method)
-
-		if ok != tt.expected {
-			t.Errorf("with method %s and allowed methods %s expected %t got %t", tt.method, tt.allowed, tt.expected, ok)
+		err := pua.requestMethodIsAllowed(tt.method)
+		if tt.errorMessage != "" {
+			assert.EqualError(t, err, tt.errorMessage)
+		} else {
+			assert.Nil(t, err)
 		}
 	}
 }
 
 func TestSignedURLAuth_urlIsExpired(t *testing.T) {
-	pua := SignedURLAuthenticator{}
 	nowFunc := func() time.Time {
 		t, _ := time.Parse(time.RFC3339, "2020-02-02T12:30:00.000Z")
 		return t
 	}
+	pua := SignedURLAuthenticator{
+		Now: nowFunc,
+	}
 
 	tests := []struct {
-		url       string
-		isExpired bool
+		url          string
+		errorMessage string
 	}{
-		{"http://example.com/example.jpg?OC-Date=2020-02-02T12:29:00.000Z&OC-Expires=61", false},
-		{"http://example.com/example.jpg?OC-Date=2020-02-02T12:29:00.000Z&OC-Expires=invalid", true},
-		{"http://example.com/example.jpg?OC-Date=2020-02-02T12:29:00.000Z&OC-Expires=59", true},
-		{"http://example.com/example.jpg?OC-Date=2020-02-03T12:29:00.000Z&OC-Expires=59", true},
-		{"http://example.com/example.jpg?OC-Date=2020-02-01T12:29:00.000Z&OC-Expires=59", true},
+		// a valid signed url
+		{"http://example.com/example.jpg?OC-Date=2020-02-02T12:29:00.000Z&OC-Expires=61", ""},
+		// invalid expiry
+		{"http://example.com/example.jpg?OC-Date=2020-02-02T12:29:00.000Z&OC-Expires=invalid", "time: invalid duration \"invalids\""},
+		// wrong date format on OC-Date
+		{"http://example.com/example.jpg?OC-Date=2020-02-02TTT12:29:00.000Z&OC-Expires=5", "parsing time \"2020-02-02TTT12:29:00.000Z\" as \"2006-01-02T15:04:05Z07:00\": cannot parse \"TT12:29:00.000Z\" as \"15\""},
+		// expired - 12:29:00 + 59s < 12:30
+		{"http://example.com/example.jpg?OC-Date=2020-02-02T12:29:00.000Z&OC-Expires=59", "URL is expired"},
+		// expired - basically url was created yesterday
+		{"http://example.com/example.jpg?OC-Date=2020-02-01T12:29:00.000Z&OC-Expires=59", "URL is expired"},
+		// future OC-Date - also valid now
+		{"http://example.com/example.jpg?OC-Date=2020-02-03T12:29:00.000Z&OC-Expires=59", ""},
 	}
 
 	for _, tt := range tests {
 		r := httptest.NewRequest("", tt.url, nil)
-		expired, _ := pua.urlIsExpired(r.URL.Query(), nowFunc)
-		if expired != tt.isExpired {
-			t.Errorf("with %s expected %t got %t", tt.url, tt.isExpired, expired)
+		err := pua.urlIsExpired(r.URL.Query())
+		if tt.errorMessage != "" {
+			assert.EqualError(t, err, tt.errorMessage, tt.url)
+		} else {
+			assert.Nil(t, err)
 		}
 	}
 }
@@ -134,3 +158,85 @@ func TestSignedURLAuth_createSignature(t *testing.T) {
 		t.Fail()
 	}
 }
+
+type MyStoreService struct {
+}
+
+func (s MyStoreService) Read(_ context.Context, _ *v0.ReadRequest, _ ...client.CallOption) (*v0.ReadResponse, error) {
+	r := v0.ReadResponse{
+		Records: []*storemsg.Record{{
+			Key:      "foo",
+			Value:    []byte("1234567890"),
+			Expiry:   0,
+			Metadata: nil,
+		}},
+	}
+	return &r, nil
+}
+func (s MyStoreService) Write(_ context.Context, _ *v0.WriteRequest, _ ...client.CallOption) (*v0.WriteResponse, error) {
+	return nil, nil
+}
+func (s MyStoreService) Delete(_ context.Context, _ *v0.DeleteRequest, _ ...client.CallOption) (*v0.DeleteResponse, error) {
+	return nil, nil
+}
+func (s MyStoreService) List(_ context.Context, _ *v0.ListRequest, _ ...client.CallOption) (v0.Store_ListService, error) {
+	return nil, nil
+}
+func (s MyStoreService) Databases(_ context.Context, _ *v0.DatabasesRequest, _ ...client.CallOption) (*v0.DatabasesResponse, error) {
+	return nil, nil
+}
+func (s MyStoreService) Tables(_ context.Context, _ *v0.TablesRequest, _ ...client.CallOption) (*v0.TablesResponse, error) {
+	return nil, nil
+}
+
+func TestSignedURLAuth_validate(t *testing.T) {
+	nowFunc := func() time.Time {
+		t, _ := time.Parse(time.RFC3339, "2020-02-02T12:30:00.000Z")
+		return t
+	}
+	cfg := config.PreSignedURL{
+		AllowedHTTPMethods: []string{"get"},
+		Enabled:            true,
+	}
+	store := MyStoreService{}
+	pua := SignedURLAuthenticator{
+		PreSignedURLConfig: cfg,
+		Store:              store,
+		Now:                nowFunc,
+	}
+
+	tests := []struct {
+		now          string
+		url          string
+		errorMessage string
+	}{
+		{"2020-02-02T12:30:00.000Z", "http://example.com/example.jpg?OC-Date=2020-02-02T12:29:00.000Z&OC-Expires=invalid", "required OC-Signature parameter not found"},
+		{"2020-02-02T12:30:00.000Z", "http://cloud.example.net/?OC-Credential=alice&OC-Date=2019-05-14T11%3A01%3A58.135Z&OC-Expires=1200&OC-Verb=GET&OC-Signature=f9e53a1ee23caef10f72ec392c1b537317491b687bfdd224c782be197d9ca2b6", "URL is expired"},
+		{"2019-05-14T11:02:00.000Z", "http://cloud.example.net/?OC-Credential=alice&OC-Date=2019-05-14T11%3A01%3A58.135Z&OC-Expires=1200&OC-Verb=GET&OC-Signature=f9e53a1ee23caef10f72ec392c1b537317491b687bfdd224c782be197d9ca2b", "signature mismatch: expected f9e53a1ee23caef10f72ec392c1b537317491b687bfdd224c782be197d9ca2b6 != actual f9e53a1ee23caef10f72ec392c1b537317491b687bfdd224c782be197d9ca2b"},
+		{"2019-05-14T11:02:00.000Z", "http://cloud.example.net/?OC-Credential=alice&OC-Date=2019-05-14T11%3A01%3A58.135Z&OC-Expires=1200&OC-Verb=GET&OC-Signature=f9e53a1ee23caef10f72ec392c1b537317491b687bfdd224c782be197d9ca2b6", ""},
+		{"2019-05-14T11:02:00.000Z", "http://cloud.example.net/?OC-Date=2019-05-14T11%3A01%3A58.135Z&OC-Expires=1200&OC-Verb=GET&OC-Credential=alice&OC-Signature=f9e53a1ee23caef10f72ec392c1b537317491b687bfdd224c782be197d9ca2b6", ""},
+		{"2019-05-14T11:02:00.000Z", "http://cloud.example.net/?OC-Algo=PBKDF2%2F10000-SHA512&OC-Date=2019-05-14T11%3A01%3A58.135Z&OC-Expires=1200&OC-Verb=GET&OC-Credential=alice&OC-Signature=f9e53a1ee23caef10f72ec392c1b537317491b687bfdd224c782be197d9ca2b6", ""},
+		{"2024-02-07T12:03:11.966Z", "http://localhost:33001/try?id=1&id=2&OC-Credential=user&OC-Date=2024-02-07T12%3A03%3A11.966Z&OC-Expires=2&OC-Verb=GET&OC-Algo=PBKDF2%2F10000-SHA512&OC-Signature=86e21a1efbf0be989a206109cfedf70a22f338dc8995e849ce002032bc6741c5", ""},
+	}
+
+	for _, tt := range tests {
+		u := userpb.User{
+			Id:          &userpb.UserId{OpaqueId: "useri"},
+			DisplayName: "Test User",
+		}
+		ctx := revactx.ContextSetUser(context.Background(), &u)
+
+		pua.Now = func() time.Time {
+			t, _ := time.Parse(time.RFC3339, tt.now)
+			return t
+		}
+
+		r := httptest.NewRequest("", tt.url, nil).WithContext(ctx)
+		err := pua.validate(r)
+		if tt.errorMessage == "" {
+			assert.Nil(t, err)
+		} else {
+			assert.EqualError(t, err, tt.errorMessage)
+		}
+	}
+}