s3: route per-key object authorization through a shared helper (#11072)

* s3: share the per-key object authorization across copy and delete

AuthorizeCopySource and AuthorizeObjectDelete both authorize a key the request
URL does not name by evaluating the bucket policy and IAM against a synthetic
per-key request; only the method and action differed. Extract that into
authorizeObjectKeyAction and make the two callers thin wrappers. No behavior
change.

Claude-Session: https://claude.ai/code/session_01Qo7p6VsoWxMo8816ogJFk5

* s3: route POST Object uploads through the shared object authorization

POST Object uploads (presigned-POST / HTML form) authorized the write with only
the coarse per-identity Write action, unlike the other write paths which also
check the resolved object against the bucket policy and IAM. Route POST through
authorizeObjectKeyAction via a new AuthorizeObjectWrite so it is authorized like
the equivalent PUT.

Claude-Session: https://claude.ai/code/session_01Qo7p6VsoWxMo8816ogJFk5

* s3: test POST Object per-key authorization

Drives a signed POST upload and checks the per-key authorization decision for a
denied, permitted, and admin caller.

Claude-Session: https://claude.ai/code/session_01Qo7p6VsoWxMo8816ogJFk5

weed/s3api/auth_credentials.go

@@ -2758,23 +2758,16 @@ func (iam *IdentityAccessManagement) bucketsNamedByAttachedPolicies(r *http.Requ
 	return true, granted
 }
 
-// AuthorizeCopySource verifies the caller is allowed to read the CopyObject /
-// UploadPartCopy source. The Auth middleware only checks the destination
-// (s3:PutObject) because routing keys on the request URL; without this call,
-// an STS session token scoped to a prefix could copy from any other prefix in
-// the same bucket.
-//
-// The source path is checked against both bucket policy and IAM/identity
-// permissions, mirroring the normal request-routed flow but with a synthetic
-// GetObject request so action resolution and ARN building target the source.
-// Returns s3err.ErrNone when allowed or when auth is disabled.
-func (iam *IdentityAccessManagement) AuthorizeCopySource(r *http.Request, identity *Identity, srcBucket, srcObject, srcVersionId string) s3err.ErrorCode {
-	if !iam.isEnabled() {
-		return s3err.ErrNone
-	}
-	if srcBucket == "" {
-		return s3err.ErrNone
-	}
+// authorizeObjectKeyAction authorizes action on bucket/objectKey for an
+// already-authenticated identity, for a key the request URL does not name: a
+// copy/rename source, a DeleteObjects body key, or a POST Object form key. It
+// evaluates the bucket policy (an explicit Deny wins) and then IAM/identity
+// against a synthetic <method> /<bucket>/<objectKey> request, so ResolveS3Action
+// and buildResourceARN target the object rather than whatever the real URL and
+// its query describe. versionId, when set, and the STS session token ride along
+// for policy conditions. Returns ErrNone when auth is disabled (checked by the
+// callers) or the identity is an admin.
+func (iam *IdentityAccessManagement) authorizeObjectKeyAction(r *http.Request, identity *Identity, method string, action Action, bucket, objectKey, versionId string) s3err.ErrorCode {
 	if identity == nil {
 		return s3err.ErrAccessDenied
 	}
@@ -2782,72 +2775,8 @@ func (iam *IdentityAccessManagement) AuthorizeCopySource(r *http.Request, identi
 		return s3err.ErrNone
 	}
 
-	srcReq := r.Clone(r.Context())
-	srcURL := &url.URL{
-		Scheme: r.URL.Scheme,
-		Host:   r.URL.Host,
-		Path:   "/" + srcBucket + "/" + srcObject,
-	}
-	// Build the synthetic source query from scratch so leftover params like
-	// uploadId/partNumber on UploadPartCopy do not steer ResolveS3Action away
-	// from s3:GetObject. The session token must still flow through for
-	// presigned URLs that carry STS credentials in the query string.
-	srcQuery := make(url.Values)
-	if token := r.URL.Query().Get("X-Amz-Security-Token"); token != "" {
-		srcQuery.Set("X-Amz-Security-Token", token)
-	}
-	if srcVersionId != "" {
-		srcQuery.Set("versionId", srcVersionId)
-	}
-	if len(srcQuery) > 0 {
-		srcURL.RawQuery = srcQuery.Encode()
-	}
-	srcReq.URL = srcURL
-	srcReq.Method = http.MethodGet
-	srcReq.RequestURI = ""
-	srcReq.Body = nil
-	srcReq.GetBody = nil
-	srcReq.ContentLength = 0
-
-	action := s3_constants.ACTION_READ
-
-	if iam.policyEngine != nil {
-		principal := buildPrincipalARN(identity, srcReq)
-		allowed, evaluated, err := iam.policyEngine.EvaluatePolicy(srcBucket, srcObject, action, principal, srcReq, identity.Claims, nil)
-		if err != nil {
-			glog.Errorf("CopyObject source policy evaluation failed for %s/%s: %v - denying", srcBucket, srcObject, err)
-			return s3err.ErrAccessDenied
-		}
-		if evaluated {
-			if allowed {
-				return s3err.ErrNone
-			}
-			return s3err.ErrAccessDenied
-		}
-	}
-
-	return iam.VerifyActionPermission(srcReq, identity, Action(action), srcBucket, srcObject)
-}
-
-// AuthorizeObjectDelete authorizes removing one key the request URL does not
-// name: a key from a DeleteObjects body, or the source of a RenameObject. It is
-// checked against a synthetic DELETE /<bucket>/<key> so that ResolveS3Action and
-// buildResourceARN target the object. Mirrors AuthorizeCopySource.
-func (iam *IdentityAccessManagement) AuthorizeObjectDelete(r *http.Request, identity *Identity, bucket, objectKey, versionId string) s3err.ErrorCode {
-	if !iam.isEnabled() {
-		return s3err.ErrNone
-	}
-	if bucket == "" || objectKey == "" {
-		return s3err.ErrNone
-	}
-	if identity == nil {
-		return s3err.ErrAccessDenied
-	}
-	if identity.isAdmin() {
-		return s3err.ErrNone
-	}
-
-	// Shallow copy: authorization only reads headers, and this runs once per key.
+	// Shallow copy: authorization only reads headers, so sharing the header map
+	// with the original request is safe, and this can run once per key.
 	keyReq := new(http.Request)
 	*keyReq = *r
 	keyURL := &url.URL{
@@ -2855,8 +2784,9 @@ func (iam *IdentityAccessManagement) AuthorizeObjectDelete(r *http.Request, iden
 		Host:   r.URL.Host,
 		Path:   "/" + bucket + "/" + objectKey,
 	}
-	// Build the query from scratch so the envelope's "delete" param can't steer
-	// ResolveS3Action; keep the STS token and per-key versionId for policy eval.
+	// Build the query from scratch so a param on the real request (delete,
+	// uploadId, partNumber, ...) cannot steer ResolveS3Action off the intended
+	// action; keep the STS token and per-key versionId for policy conditions.
 	keyQuery := make(url.Values)
 	if versionId != "" {
 		keyQuery.Set("versionId", versionId)
@@ -2870,19 +2800,17 @@ func (iam *IdentityAccessManagement) AuthorizeObjectDelete(r *http.Request, iden
 		keyURL.RawQuery = keyQuery.Encode()
 	}
 	keyReq.URL = keyURL
-	keyReq.Method = http.MethodDelete
+	keyReq.Method = method
 	keyReq.RequestURI = ""
 	keyReq.Body = nil
 	keyReq.GetBody = nil
 	keyReq.ContentLength = 0
 
-	action := s3_constants.ACTION_WRITE
-
 	if iam.policyEngine != nil {
 		principal := buildPrincipalARN(identity, keyReq)
-		allowed, evaluated, err := iam.policyEngine.EvaluatePolicy(bucket, objectKey, action, principal, keyReq, identity.Claims, nil)
+		allowed, evaluated, err := iam.policyEngine.EvaluatePolicy(bucket, objectKey, string(action), principal, keyReq, identity.Claims, nil)
 		if err != nil {
-			glog.Errorf("DeleteObjects key policy evaluation failed for %s/%s: %v - denying", bucket, objectKey, err)
+			glog.Errorf("policy evaluation failed for %s %s/%s: %v - denying", action, bucket, objectKey, err)
 			return s3err.ErrAccessDenied
 		}
 		if evaluated {
@@ -2893,7 +2821,48 @@ func (iam *IdentityAccessManagement) AuthorizeObjectDelete(r *http.Request, iden
 		}
 	}
 
-	return iam.VerifyActionPermission(keyReq, identity, Action(action), bucket, objectKey)
+	return iam.VerifyActionPermission(keyReq, identity, action, bucket, objectKey)
+}
+
+// AuthorizeCopySource verifies the caller is allowed to read the CopyObject /
+// UploadPartCopy source. The Auth middleware only checks the destination
+// (s3:PutObject) because routing keys on the request URL; without this call,
+// an STS session token scoped to a prefix could copy from any other prefix in
+// the same bucket. Returns s3err.ErrNone when allowed or when auth is disabled.
+func (iam *IdentityAccessManagement) AuthorizeCopySource(r *http.Request, identity *Identity, srcBucket, srcObject, srcVersionId string) s3err.ErrorCode {
+	if !iam.isEnabled() {
+		return s3err.ErrNone
+	}
+	if srcBucket == "" {
+		return s3err.ErrNone
+	}
+	return iam.authorizeObjectKeyAction(r, identity, http.MethodGet, s3_constants.ACTION_READ, srcBucket, srcObject, srcVersionId)
+}
+
+// AuthorizeObjectDelete authorizes removing one key the request URL does not
+// name: a key from a DeleteObjects body, or the source of a RenameObject.
+func (iam *IdentityAccessManagement) AuthorizeObjectDelete(r *http.Request, identity *Identity, bucket, objectKey, versionId string) s3err.ErrorCode {
+	if !iam.isEnabled() {
+		return s3err.ErrNone
+	}
+	if bucket == "" || objectKey == "" {
+		return s3err.ErrNone
+	}
+	return iam.authorizeObjectKeyAction(r, identity, http.MethodDelete, s3_constants.ACTION_WRITE, bucket, objectKey, versionId)
+}
+
+// AuthorizeObjectWrite authorizes writing one key the request URL does not name:
+// a POST Object (presigned-POST / HTML-form) upload carries its key in the
+// multipart form, so the Auth middleware only checked the coarse bucket-level
+// Write action. This runs the same per-object authorization the PUT path applies.
+func (iam *IdentityAccessManagement) AuthorizeObjectWrite(r *http.Request, identity *Identity, bucket, objectKey string) s3err.ErrorCode {
+	if !iam.isEnabled() {
+		return s3err.ErrNone
+	}
+	if bucket == "" || objectKey == "" {
+		return s3err.ErrNone
+	}
+	return iam.authorizeObjectKeyAction(r, identity, http.MethodPut, s3_constants.ACTION_WRITE, bucket, objectKey, "")
 }
 
 // authorizeWithIAM authorizes requests using the IAM integration policy engine

weed/s3api/s3api_object_handlers_postpolicy.go

@@ -139,6 +139,12 @@ func (s3a *S3ApiServer) PostPolicyBucketHandler(w http.ResponseWriter, r *http.R
 	// Forward validated POST form fields to the underlying PUT as headers.
 	applyPostPolicyFormHeaders(r, formValues)
 
+	// Authorize the object like the PUT path; the coarse Write check above does not consult the bucket policy.
+	if errCode := s3a.iam.AuthorizeObjectWrite(r, identity, bucket, object); errCode != s3err.ErrNone {
+		s3err.WriteErrorResponse(w, r, errCode)
+		return
+	}
+
 	// Use fileSize, not r.ContentLength: the multipart body wrapping form
 	// fields and boundaries inflates ContentLength relative to the
 	// object body, which would mis-evaluate any size-filtered rule.

weed/s3api/s3api_object_handlers_postpolicy_authz_test.go

@@ -0,0 +1,172 @@
+package s3api
+
+import (
+	"bytes"
+	"encoding/base64"
+	"fmt"
+	"mime/multipart"
+	"net/http"
+	"net/http/httptest"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/gorilla/mux"
+	"github.com/seaweedfs/seaweedfs/weed/pb/iam_pb"
+	"github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants"
+	"github.com/seaweedfs/seaweedfs/weed/s3api/s3err"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+const (
+	postAuthzAccessKey = "AKIATESTTESTTEST"
+	postAuthzSecretKey = "secret-key-for-tests"
+	postAuthzRegion    = "us-east-1"
+	postAuthzService   = "s3"
+	postAuthzBucket    = "testbucket"
+	postAuthzPrincipal = "arn:aws:iam::000000000000:user/tester"
+)
+
+// newPostAuthzServer builds an S3ApiServer whose non-admin "tester" identity
+// holds the coarse Write action on the bucket and whose bucket policy denies it
+// s3:PutObject under denied/: a writer confined by a bucket policy to part of a
+// shared bucket.
+func newPostAuthzServer(t *testing.T) *S3ApiServer {
+	t.Helper()
+
+	iam := &IdentityAccessManagement{
+		hashes:       make(map[string]*sync.Pool),
+		hashCounters: make(map[string]*int32),
+	}
+	err := iam.loadS3ApiConfiguration(&iam_pb.S3ApiConfiguration{
+		Identities: []*iam_pb.Identity{{
+			Name:        "tester",
+			Account:     &iam_pb.Account{Id: "000000000000", DisplayName: "tester"},
+			Credentials: []*iam_pb.Credential{{AccessKey: postAuthzAccessKey, SecretKey: postAuthzSecretKey}},
+			Actions:     []string{"Read:" + postAuthzBucket, "Write:" + postAuthzBucket, "List:" + postAuthzBucket},
+		}},
+	})
+	require.NoError(t, err)
+
+	policyEngine := NewBucketPolicyEngine()
+	err = policyEngine.engine.SetBucketPolicy(postAuthzBucket, fmt.Sprintf(`{
+		"Version": "2012-10-17",
+		"Statement": [{
+			"Sid": "DenyTesterDeniedPrefix",
+			"Effect": "Deny",
+			"Principal": {"AWS": "%s"},
+			"Action": ["s3:PutObject"],
+			"Resource": ["arn:aws:s3:::%s/denied/*"]
+		}]
+	}`, postAuthzPrincipal, postAuthzBucket))
+	require.NoError(t, err)
+	iam.policyEngine = policyEngine
+
+	s3a := &S3ApiServer{
+		option:       &S3ApiServerOption{BucketsPath: "/buckets"},
+		iam:          iam,
+		policyEngine: policyEngine,
+	}
+	// Pre-populate the bucket registry so validateTableBucketObjectPath sees a
+	// non-table bucket without needing a live filer connection.
+	s3a.bucketRegistry = NewBucketRegistry(s3a)
+	s3a.bucketRegistry.setMetadataCache(&BucketMetaData{Name: postAuthzBucket, IsTableBucket: false})
+	return s3a
+}
+
+// newSignedPostRequest builds a signed multipart POST Object request whose POST
+// policy conditions are satisfied by the form, so the handler passes signature
+// and CheckPostPolicy verification and reaches the write-authorization stage.
+func newSignedPostRequest(t *testing.T, key string) *http.Request {
+	t.Helper()
+
+	now := time.Now().UTC()
+	amzDate := now.Format(iso8601Format)
+	yyyymmddStr := now.Format(yyyymmdd)
+	credential := fmt.Sprintf("%s/%s/%s/%s/aws4_request", postAuthzAccessKey, yyyymmddStr, postAuthzRegion, postAuthzService)
+	expiration := now.Add(1 * time.Hour).Format("2006-01-02T15:04:05.000Z")
+
+	policyJSON := fmt.Sprintf(
+		`{"expiration":"%s","conditions":[`+
+			`["eq","$bucket","%s"],`+
+			`["eq","$key","%s"],`+
+			`["eq","$x-amz-credential","%s"],`+
+			`["eq","$x-amz-algorithm","AWS4-HMAC-SHA256"],`+
+			`["eq","$x-amz-date","%s"]`+
+			`]}`,
+		expiration, postAuthzBucket, key, credential, amzDate,
+	)
+	encodedPolicy := base64.StdEncoding.EncodeToString([]byte(policyJSON))
+	signature := getSignature(getSigningKey(postAuthzSecretKey, yyyymmddStr, postAuthzRegion, postAuthzService), encodedPolicy)
+
+	var buf bytes.Buffer
+	writer := multipart.NewWriter(&buf)
+	require.NoError(t, writer.WriteField("bucket", postAuthzBucket))
+	require.NoError(t, writer.WriteField("key", key))
+	require.NoError(t, writer.WriteField("x-amz-credential", credential))
+	require.NoError(t, writer.WriteField("x-amz-algorithm", "AWS4-HMAC-SHA256"))
+	require.NoError(t, writer.WriteField("x-amz-date", amzDate))
+	require.NoError(t, writer.WriteField("policy", encodedPolicy))
+	require.NoError(t, writer.WriteField("x-amz-signature", signature))
+	filePart, err := writer.CreateFormFile("file", "payload.txt")
+	require.NoError(t, err)
+	_, err = filePart.Write([]byte("payload"))
+	require.NoError(t, err)
+	require.NoError(t, writer.Close())
+
+	req := httptest.NewRequest(http.MethodPost, "/"+postAuthzBucket, &buf)
+	req.Header.Set("Content-Type", writer.FormDataContentType())
+	return mux.SetURLVars(req, map[string]string{"bucket": postAuthzBucket})
+}
+
+// TestPostPolicyBucketHandlerHonorsBucketPolicyDeny drives a signed POST Object
+// upload whose key a bucket policy denies the caller, and asserts the handler
+// rejects it with 403 -- the same result the equivalent PUT gives, now that POST
+// runs the shared per-key authorization instead of only the coarse Write check.
+func TestPostPolicyBucketHandlerHonorsBucketPolicyDeny(t *testing.T) {
+	s3a := newPostAuthzServer(t)
+
+	// Precondition: the policy really denies this principal the denied/ key, so a
+	// non-403 below is the POST path not consulting it, not a mis-configured policy.
+	putReq := httptest.NewRequest(http.MethodPut, "/"+postAuthzBucket+"/denied/secret.txt", nil)
+	allowed, evaluated, err := s3a.iam.policyEngine.EvaluatePolicy(
+		postAuthzBucket, "denied/secret.txt", s3_constants.ACTION_WRITE, postAuthzPrincipal, putReq, nil, nil)
+	require.NoError(t, err)
+	require.True(t, evaluated, "policy must match the denied key for the tester principal")
+	require.False(t, allowed, "policy must deny the tester principal on the denied key")
+
+	rec := httptest.NewRecorder()
+	s3a.PostPolicyBucketHandler(rec, newSignedPostRequest(t, "denied/secret.txt"))
+
+	assert.Equal(t, http.StatusForbidden, rec.Code,
+		"POST Object into a bucket-policy-denied key must be rejected, body: %s", rec.Body.String())
+	assert.Contains(t, rec.Body.String(), "AccessDenied",
+		"response should identify AccessDenied, body: %s", rec.Body.String())
+}
+
+// TestAuthorizeObjectWrite covers the write-authorization decision the POST
+// handler defers to: a bucket-policy Deny blocks the key, a key the policy does
+// not deny clears (the coarse Write action still grants it), and admins are
+// unaffected. Driving the full handler for the allowed case is not hermetic --
+// past authorization it reaches the lifecycle/write path that needs a filer --
+// so the counter-cases are checked at the authorization boundary directly.
+func TestAuthorizeObjectWrite(t *testing.T) {
+	s3a := newPostAuthzServer(t)
+	tester, _, found := s3a.iam.lookupByAccessKey(postAuthzAccessKey)
+	require.True(t, found)
+
+	req := httptest.NewRequest(http.MethodPost, "/"+postAuthzBucket, nil)
+
+	assert.Equal(t, s3err.ErrAccessDenied,
+		s3a.iam.AuthorizeObjectWrite(req, tester, postAuthzBucket, "denied/secret.txt"),
+		"a bucket-policy-denied key must be denied")
+	assert.Equal(t, s3err.ErrNone,
+		s3a.iam.AuthorizeObjectWrite(req, tester, postAuthzBucket, "allowed/report.txt"),
+		"a key the policy does not deny must clear on the coarse Write grant")
+
+	admin := &Identity{Name: "admin", Actions: []Action{s3_constants.ACTION_ADMIN}}
+	assert.Equal(t, s3err.ErrNone,
+		s3a.iam.AuthorizeObjectWrite(req, admin, postAuthzBucket, "denied/secret.txt"),
+		"an admin is not confined by the bucket policy")
+}