From e928ebca155e3f2676958d12d0fd4892d34de336 Mon Sep 17 00:00:00 2001 From: Andrej Mickov Date: Sat, 16 May 2026 10:24:32 +0200 Subject: [PATCH] Defer multi-delete authorization to handler Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- api/multi_delete_auth_test.go | 134 ++++++++++++++++++++++++++++++++-- auth/action.go | 11 +++ auth/service.go | 7 ++ 3 files changed, 144 insertions(+), 8 deletions(-) diff --git a/api/multi_delete_auth_test.go b/api/multi_delete_auth_test.go index 495f504..1bdb770 100644 --- a/api/multi_delete_auth_test.go +++ b/api/multi_delete_auth_test.go @@ -3,13 +3,18 @@ package api import ( "bytes" "context" + "crypto/hmac" + "crypto/sha256" "encoding/base64" + "encoding/hex" "errors" "io" "log/slog" "net/http" "net/http/httptest" + "net/url" "path/filepath" + "sort" "strings" "testing" "time" @@ -78,19 +83,24 @@ func withAuthContext(req *http.Request, accessKeyID string) *http.Request { } func createDeleteUser(t *testing.T, authSvc *auth.Service, prefix string) { + t.Helper() + createDeleteUserWithStatements(t, authSvc, []models.AuthPolicyStatement{ + { + Effect: "allow", + Actions: []string{"s3:DeleteObject"}, + Bucket: "test-bucket", + Prefix: prefix, + }, + }) +} + +func createDeleteUserWithStatements(t *testing.T, authSvc *auth.Service, statements []models.AuthPolicyStatement) { t.Helper() _, err := authSvc.CreateUser(auth.CreateUserInput{ AccessKeyID: "delete-user", SecretKey: "delete-secret-1", Policy: models.AuthPolicy{ - Statements: []models.AuthPolicyStatement{ - { - Effect: "allow", - Actions: []string{"s3:DeleteObject"}, - Bucket: "test-bucket", - Prefix: prefix, - }, - }, + Statements: statements, }, }) if err != nil { @@ -163,3 +173,111 @@ func TestMultiDeleteAllowsScopedKeys(t *testing.T) { t.Fatalf("allowed object should be deleted, got err=%v", err) } } + +func TestMultiDeleteRouteAuthorizesKeysAfterMiddleware(t *testing.T) { + handler, svc, authSvc := newAuthorizedDeleteHandler(t) + handler.setupRoutes() + if err := svc.CreateBucket("test-bucket"); err != nil { + t.Fatalf("create bucket: %v", err) + } + createDeleteUserWithStatements(t, authSvc, []models.AuthPolicyStatement{ + {Effect: "allow", Actions: []string{"s3:DeleteObject"}, Bucket: "test-bucket", Prefix: "allowed/"}, + {Effect: "deny", Actions: []string{"s3:DeleteObject"}, Bucket: "test-bucket", Prefix: "private/"}, + }) + putTestObject(t, svc, "allowed/file.txt") + putTestObject(t, svc, "private/file.txt") + + body := `allowed/file.txtprivate/file.txt` + req := httptest.NewRequest(http.MethodPost, "/test-bucket?delete", strings.NewReader(body)) + signTestSigV4Request(t, req, "delete-user", "delete-secret-1") + rec := httptest.NewRecorder() + + handler.router.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("unexpected status: got %d body=%s", rec.Code, rec.Body.String()) + } + responseBody := rec.Body.String() + if !strings.Contains(responseBody, "allowed/file.txt") || !strings.Contains(responseBody, "") { + t.Fatalf("expected allowed key deletion, body=%s", responseBody) + } + if !strings.Contains(responseBody, "private/file.txt") || !strings.Contains(responseBody, "AccessDenied") { + t.Fatalf("expected per-key AccessDenied, body=%s", responseBody) + } + if _, err := svc.HeadObject("test-bucket", "allowed/file.txt"); !errors.Is(err, metadata.ErrObjectNotFound) { + t.Fatalf("allowed object should be deleted, got err=%v", err) + } + if _, err := svc.HeadObject("test-bucket", "private/file.txt"); err != nil { + t.Fatalf("private object should remain: %v", err) + } +} + +func signTestSigV4Request(t *testing.T, req *http.Request, accessKeyID, secretKey string) { + t.Helper() + + amzDate := time.Now().UTC().Format("20060102T150405Z") + date := amzDate[:8] + region := "us-east-1" + serviceName := "s3" + scope := strings.Join([]string{date, region, serviceName, "aws4_request"}, "/") + signedHeaders := []string{"host", "x-amz-content-sha256", "x-amz-date"} + signedHeadersRaw := strings.Join(signedHeaders, ";") + payloadHash := "UNSIGNED-PAYLOAD" + + req.Header.Set("x-amz-date", amzDate) + req.Header.Set("x-amz-content-sha256", payloadHash) + canonicalRequest := strings.Join([]string{ + req.Method, + req.URL.EscapedPath(), + canonicalTestQuery(req.URL.RawQuery), + "host:" + strings.TrimSpace(req.Host) + "\n" + + "x-amz-content-sha256:" + payloadHash + "\n" + + "x-amz-date:" + amzDate + "\n", + signedHeadersRaw, + payloadHash, + }, "\n") + canonicalHash := sha256.Sum256([]byte(canonicalRequest)) + stringToSign := strings.Join([]string{ + "AWS4-HMAC-SHA256", + amzDate, + scope, + hex.EncodeToString(canonicalHash[:]), + }, "\n") + signingKey := testHMAC(testHMAC(testHMAC(testHMAC([]byte("AWS4"+secretKey), date), region), serviceName), "aws4_request") + signature := hex.EncodeToString(testHMAC(signingKey, stringToSign)) + + req.Header.Set("Authorization", "AWS4-HMAC-SHA256 "+ + "Credential="+accessKeyID+"/"+scope+", "+ + "SignedHeaders="+signedHeadersRaw+", "+ + "Signature="+signature) +} + +func canonicalTestQuery(rawQuery string) string { + values, _ := url.ParseQuery(rawQuery) + pairs := make([]string, 0) + for key, valueList := range values { + if len(valueList) == 0 { + pairs = append(pairs, awsTestQueryEscape(key)+"=") + continue + } + for _, value := range valueList { + pairs = append(pairs, awsTestQueryEscape(key)+"="+awsTestQueryEscape(value)) + } + } + sort.Strings(pairs) + return strings.Join(pairs, "&") +} + +func awsTestQueryEscape(value string) string { + encoded := url.QueryEscape(value) + encoded = strings.ReplaceAll(encoded, "+", "%20") + encoded = strings.ReplaceAll(encoded, "*", "%2A") + encoded = strings.ReplaceAll(encoded, "%7E", "~") + return encoded +} + +func testHMAC(key []byte, value string) []byte { + mac := hmac.New(sha256.New, key) + _, _ = mac.Write([]byte(value)) + return mac.Sum(nil) +} diff --git a/auth/action.go b/auth/action.go index 988029f..45251cc 100644 --- a/auth/action.go +++ b/auth/action.go @@ -30,6 +30,17 @@ type RequestTarget struct { Prefix string } +func RequiresHandlerAuthorization(r *http.Request) bool { + if r == nil || r.URL == nil { + return false + } + if r.Method == http.MethodPost { + _, isDelete := r.URL.Query()["delete"] + return isDelete + } + return false +} + func resolveTarget(r *http.Request) RequestTarget { path := strings.TrimPrefix(r.URL.Path, "/") if path == "" { diff --git a/auth/service.go b/auth/service.go index 2bb0eee..9eb08b5 100644 --- a/auth/service.go +++ b/auth/service.go @@ -188,6 +188,13 @@ func (s *Service) AuthenticateRequest(r *http.Request) (RequestContext, error) { AuthType: authType, }, nil } + if RequiresHandlerAuthorization(r) { + return RequestContext{ + Authenticated: true, + AccessKeyID: identity.AccessKeyID, + AuthType: authType, + }, nil + } policy, err := s.store.GetAuthPolicy(identity.AccessKeyID) if err != nil {