diff --git a/weed/iam/helpers.go b/weed/iam/helpers.go index 0e8016433..a80ecf994 100644 --- a/weed/iam/helpers.go +++ b/weed/iam/helpers.go @@ -136,7 +136,7 @@ var baseS3ActionMap = map[string]string{ "DeleteBucketCors": s3_constants.ACTION_WRITE, "GetBucketNotification": s3_constants.ACTION_READ, "PutBucketNotification": s3_constants.ACTION_WRITE, - "GetBucketObjectLockConfiguration": s3_constants.ACTION_READ, + "GetBucketObjectLockConfiguration": s3_constants.ACTION_GET_BUCKET_OBJECT_LOCK_CONFIG, "PutBucketObjectLockConfiguration": s3_constants.ACTION_PUT_BUCKET_OBJECT_LOCK_CONFIG, // Multipart upload operations "CreateMultipartUpload": s3_constants.ACTION_WRITE, @@ -146,11 +146,11 @@ var baseS3ActionMap = map[string]string{ "ListMultipartUploads": s3_constants.ACTION_WRITE, "ListParts": s3_constants.ACTION_WRITE, // Retention and legal hold operations - "GetObjectRetention": s3_constants.ACTION_READ, - "PutObjectRetention": s3_constants.ACTION_WRITE, - "GetObjectLegalHold": s3_constants.ACTION_READ, - "PutObjectLegalHold": s3_constants.ACTION_WRITE, - "BypassGovernanceRetention": s3_constants.ACTION_WRITE, + "GetObjectRetention": s3_constants.ACTION_GET_OBJECT_RETENTION, + "PutObjectRetention": s3_constants.ACTION_PUT_OBJECT_RETENTION, + "GetObjectLegalHold": s3_constants.ACTION_GET_OBJECT_LEGAL_HOLD, + "PutObjectLegalHold": s3_constants.ACTION_PUT_OBJECT_LEGAL_HOLD, + "BypassGovernanceRetention": s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION, } func init() { @@ -191,6 +191,18 @@ func MapToIdentitiesAction(action string) string { return StatementActionTagging case s3_constants.ACTION_DELETE_BUCKET: return StatementActionDelete + case s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION: + return "BypassGovernanceRetention" + case s3_constants.ACTION_GET_OBJECT_RETENTION: + return "GetObjectRetention" + case s3_constants.ACTION_PUT_OBJECT_RETENTION: + return "PutObjectRetention" + case s3_constants.ACTION_GET_OBJECT_LEGAL_HOLD: + return "GetObjectLegalHold" + case s3_constants.ACTION_PUT_OBJECT_LEGAL_HOLD: + return "PutObjectLegalHold" + case s3_constants.ACTION_GET_BUCKET_OBJECT_LOCK_CONFIG: + return "GetBucketObjectLockConfiguration" case s3_constants.ACTION_PUT_BUCKET_OBJECT_LOCK_CONFIG: return "PutBucketObjectLockConfiguration" case s3_constants.ACTION_PUT_BUCKET_POLICY: diff --git a/weed/s3api/s3api_governance_permissions_test.go b/weed/s3api/s3api_governance_permissions_test.go index 22403ddaf..9c05fbcd1 100644 --- a/weed/s3api/s3api_governance_permissions_test.go +++ b/weed/s3api/s3api_governance_permissions_test.go @@ -179,19 +179,15 @@ func TestCheckGovernanceBypassPermissionIntegrationBehavior(t *testing.T) { // This test documents the expected behavior when checkGovernanceBypassPermission // is called with a full IAM system: // - // 1. Function calls s3a.iam.authRequest() with the bypass action + // 1. Function calls s3a.iam.authRequest() with the base bypass action; + // authRequest derives the bucket and object resource from the request. // 2. If authRequest returns errCode != s3err.ErrNone, function returns false - // 3. If authRequest succeeds, function checks identity.CanDo() with the bypass action - // 4. If CanDo() returns true, function returns true - // 5. If bypass permission fails, function checks admin action with identity.CanDo() - // 6. If admin action succeeds, function returns true and logs admin access - // 7. If all checks fail, function returns false + // 3. If authRequest succeeds through a legacy action, IAM/STS policy, bucket + // policy, or Admin grant, function returns true without a second legacy check. // // The function correctly uses: // - s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION for bypass permission - // - s3_constants.ACTION_ADMIN for admin permission - // - Proper resource path generation with bucket/object format - // - Trimming of leading slashes from object names + // - The normal authRequest resource and Admin semantics } // TestGovernanceBypassPermission was removed because it tested the old diff --git a/weed/s3api/s3api_object_lock_actions_authz_test.go b/weed/s3api/s3api_object_lock_actions_authz_test.go new file mode 100644 index 000000000..a7ac525e4 --- /dev/null +++ b/weed/s3api/s3api_object_lock_actions_authz_test.go @@ -0,0 +1,147 @@ +package s3api + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/gorilla/mux" + iamlib "github.com/seaweedfs/seaweedfs/weed/iam" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" + "github.com/stretchr/testify/require" +) + +func TestObjectLockRoutesUseDedicatedActions(t *testing.T) { + bindings := handlerActionBindings(t) + for _, tc := range []struct{ handler, want string }{ + {"GetObjectRetentionHandler", "ACTION_GET_OBJECT_RETENTION"}, + {"PutObjectRetentionHandler", "ACTION_PUT_OBJECT_RETENTION"}, + {"GetObjectLegalHoldHandler", "ACTION_GET_OBJECT_LEGAL_HOLD"}, + {"PutObjectLegalHoldHandler", "ACTION_PUT_OBJECT_LEGAL_HOLD"}, + {"GetObjectLockConfigurationHandler", "ACTION_GET_BUCKET_OBJECT_LOCK_CONFIG"}, + {"PutObjectLockConfigurationHandler", "ACTION_PUT_BUCKET_OBJECT_LOCK_CONFIG"}, + } { + if got := bindings[tc.handler]; got != tc.want { + t.Errorf("%s is gated on %s, want %s", tc.handler, got, tc.want) + } + } +} + +func TestGovernanceBypassUsesDedicatedAuthorization(t *testing.T) { + for _, tc := range []struct { + name string + action Action + allowed bool + }{ + {"dedicated permission", Action(s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION + ":test-bucket/*"), true}, + {"coarse write", Action(s3_constants.ACTION_WRITE + ":test-bucket/*"), false}, + {"admin", Action(s3_constants.ACTION_ADMIN), true}, + } { + t.Run(tc.name, func(t *testing.T) { + iam := newTestIAM() + iam.identities[0].Actions = []Action{tc.action} + iam.identities[0].Account = &Account{Id: "test-account"} + s3a := &S3ApiServer{iam: iam} + req := httptest.NewRequest(http.MethodDelete, "http://localhost:8333/test-bucket/test-object", nil) + req = mux.SetURLVars(req, map[string]string{"bucket": "test-bucket", "object": "test-object"}) + require.NoError(t, signRawHTTPRequest(context.Background(), req, + "AKIAIOSFODNN7EXAMPLE", "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", "us-east-1")) + + if got := s3a.checkGovernanceBypassPermission(req, "test-bucket", "/test-object"); got != tc.allowed { + t.Errorf("checkGovernanceBypassPermission() = %v, want %v", got, tc.allowed) + } + }) + } +} + +func TestGovernanceBypassUsesBodyObjectKey(t *testing.T) { + iam := newTestIAM() + iam.identities[0].Actions = []Action{ + Action(s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION + ":test-bucket/allowed/*"), + } + iam.identities[0].Account = &Account{Id: "test-account"} + s3a := &S3ApiServer{iam: iam} + + req := httptest.NewRequest(http.MethodPost, "http://localhost:8333/test-bucket?delete", nil) + req = mux.SetURLVars(req, map[string]string{"bucket": "test-bucket"}) + req = req.WithContext(s3_constants.SetIdentityInContext(req.Context(), iam.identities[0])) + + if !s3a.checkGovernanceBypassPermission(req, "test-bucket", "allowed/object") { + t.Fatal("object-scoped bypass grant did not authorize a DeleteObjects body key") + } + if s3a.checkGovernanceBypassPermission(req, "test-bucket", "denied/object") { + t.Fatal("object-scoped bypass grant authorized a key outside its prefix") + } +} + +func TestGovernanceBypassNormalizesBodyObjectKey(t *testing.T) { + iam := newTestIAM() + iam.identities[0].Account = &Account{Id: "test-account"} + s3a := &S3ApiServer{iam: iam} + + req := httptest.NewRequest(http.MethodPost, "http://localhost:8333/test-bucket?delete", nil) + req = mux.SetURLVars(req, map[string]string{"bucket": "test-bucket"}) + req = req.WithContext(s3_constants.SetIdentityInContext(req.Context(), iam.identities[0])) + + iam.identities[0].Actions = []Action{ + Action(s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION + ":test-bucket/allowed/o?ject"), + } + if !s3a.checkGovernanceBypassPermission(req, "test-bucket", "//allowed//object") { + t.Fatal("canonical object grant did not authorize the equivalent noncanonical body key") + } + + iam.identities[0].Actions = []Action{ + Action(s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION + ":test-bucket/allowed//o?ject"), + } + if s3a.checkGovernanceBypassPermission(req, "test-bucket", "//allowed//object") { + t.Fatal("noncanonical alias grant authorized the canonical mutation target") + } +} + +func TestCoarseReadWriteDoNotGrantObjectLockActions(t *testing.T) { + identity := &Identity{ + Name: "object-reader-writer", + Actions: []Action{ + Action(s3_constants.ACTION_READ + ":test-bucket"), + Action(s3_constants.ACTION_WRITE + ":test-bucket"), + }, + } + + for _, action := range []string{ + s3_constants.ACTION_GET_OBJECT_RETENTION, + s3_constants.ACTION_PUT_OBJECT_RETENTION, + s3_constants.ACTION_GET_OBJECT_LEGAL_HOLD, + s3_constants.ACTION_PUT_OBJECT_LEGAL_HOLD, + s3_constants.ACTION_GET_BUCKET_OBJECT_LOCK_CONFIG, + s3_constants.ACTION_PUT_BUCKET_OBJECT_LOCK_CONFIG, + s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION, + } { + if identity.CanDo(Action(action), "test-bucket", "some/key") { + t.Errorf("coarse Read/Write unexpectedly granted %s", action) + } + } +} + +func TestObjectLockActionsRoundTrip(t *testing.T) { + for _, tc := range []struct{ policyAction, identityAction string }{ + {"GetObjectRetention", s3_constants.ACTION_GET_OBJECT_RETENTION}, + {"PutObjectRetention", s3_constants.ACTION_PUT_OBJECT_RETENTION}, + {"GetObjectLegalHold", s3_constants.ACTION_GET_OBJECT_LEGAL_HOLD}, + {"PutObjectLegalHold", s3_constants.ACTION_PUT_OBJECT_LEGAL_HOLD}, + {"GetBucketObjectLockConfiguration", s3_constants.ACTION_GET_BUCKET_OBJECT_LOCK_CONFIG}, + {"PutBucketObjectLockConfiguration", s3_constants.ACTION_PUT_BUCKET_OBJECT_LOCK_CONFIG}, + {"BypassGovernanceRetention", s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION}, + } { + t.Run(tc.policyAction, func(t *testing.T) { + for _, action := range []string{tc.policyAction, "s3:" + tc.policyAction} { + if got := iamlib.MapToStatementAction(action); got != tc.identityAction { + t.Errorf("MapToStatementAction(%q) = %q, want %q", action, got, tc.identityAction) + } + } + if got := iamlib.MapToIdentitiesAction(tc.identityAction); got != tc.policyAction { + t.Errorf("MapToIdentitiesAction(%q) = %q, want %q", tc.identityAction, got, tc.policyAction) + } + }) + } +} diff --git a/weed/s3api/s3api_object_retention.go b/weed/s3api/s3api_object_retention.go index 66b82cade..2d1e8f4ff 100644 --- a/weed/s3api/s3api_object_retention.go +++ b/weed/s3api/s3api_object_retention.go @@ -6,7 +6,6 @@ import ( "fmt" "net/http" "strconv" - "strings" "time" "github.com/seaweedfs/seaweedfs/weed/glog" @@ -521,33 +520,26 @@ func (s3a *S3ApiServer) getLegalHoldFromEntry(entry *filer_pb.Entry) (*ObjectLeg // checkGovernanceBypassPermission checks if user has permission to bypass governance retention func (s3a *S3ApiServer) checkGovernanceBypassPermission(request *http.Request, bucket, object string) bool { - // Use the existing IAM auth system to check the specific permission - // Create the governance bypass action with proper bucket/object concatenation - // Note: path.Join would drop bucket if object has leading slash, so use explicit formatting - resource := fmt.Sprintf("%s/%s", bucket, strings.TrimPrefix(object, "/")) - action := Action(fmt.Sprintf("%s:%s", s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION, resource)) + identity, _ := s3_constants.GetIdentityFromContext(request).(*Identity) + if identity == nil { + // Most handlers arrive through Auth and carry the authenticated identity + // in context. Keep direct callers correct without authorizing against the + // request URL, which is bucket-only for DeleteObjects. + var errCode s3err.ErrorCode + identity, errCode, _ = s3a.iam.authenticateRequestInternal(request) + if errCode != s3err.ErrNone { + glog.V(3).Infof("IAM authentication failed for governance bypass: %v", errCode) + return false + } + } - // Use the IAM system to authenticate and authorize this specific action - identity, errCode := s3a.iam.authRequest(request, action) + errCode := s3a.iam.authorizeObjectKeyAction(request, identity, request.Method, + s3_constants.ACTION_BYPASS_GOVERNANCE_RETENTION, bucket, s3_constants.NormalizeObjectKey(object), request.URL.Query().Get("versionId")) if errCode != s3err.ErrNone { - glog.V(3).Infof("IAM auth failed for governance bypass: %v", errCode) + glog.V(3).Infof("IAM authorization failed for governance bypass on %s/%s: %v", bucket, object, errCode) return false } - - // Verify that the authenticated identity can perform this action - if identity != nil && identity.CanDo(action, bucket, object) { - return true - } - - // Additional check: allow users with Admin action to bypass governance retention - // Use the proper S3 Admin action constant instead of generic isAdmin() method - adminAction := Action(fmt.Sprintf("%s:%s", s3_constants.ACTION_ADMIN, resource)) - if identity != nil && identity.CanDo(adminAction, bucket, object) { - glog.V(2).Infof("Admin user %s granted governance bypass permission for %s/%s", identity.Name, bucket, object) - return true - } - - return false + return true } // evaluateGovernanceBypassRequest evaluates if governance bypass is requested and permitted diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index c8c3cd328..8e415eee6 100644 --- a/weed/s3api/s3api_server.go +++ b/weed/s3api/s3api_server.go @@ -870,16 +870,16 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) { // PutObjectACL bucket.Methods(http.MethodPut).Path(objectPath).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.PutObjectAclHandler, ACTION_WRITE_ACP)), "PUT")).Queries("acl", "") // PutObjectRetention - bucket.Methods(http.MethodPut).Path(objectPath).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.PutObjectRetentionHandler, ACTION_WRITE)), "PUT")).Queries("retention", "") + bucket.Methods(http.MethodPut).Path(objectPath).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.PutObjectRetentionHandler, ACTION_PUT_OBJECT_RETENTION)), "PUT")).Queries("retention", "") // PutObjectLegalHold - bucket.Methods(http.MethodPut).Path(objectPath).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.PutObjectLegalHoldHandler, ACTION_WRITE)), "PUT")).Queries("legal-hold", "") + bucket.Methods(http.MethodPut).Path(objectPath).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.PutObjectLegalHoldHandler, ACTION_PUT_OBJECT_LEGAL_HOLD)), "PUT")).Queries("legal-hold", "") // GetObjectACL bucket.Methods(http.MethodGet).Path(objectPath).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetObjectAclHandler, ACTION_READ_ACP)), "GET")).Queries("acl", "") // GetObjectRetention - bucket.Methods(http.MethodGet).Path(objectPath).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetObjectRetentionHandler, ACTION_READ)), "GET")).Queries("retention", "") + bucket.Methods(http.MethodGet).Path(objectPath).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetObjectRetentionHandler, ACTION_GET_OBJECT_RETENTION)), "GET")).Queries("retention", "") // GetObjectLegalHold - bucket.Methods(http.MethodGet).Path(objectPath).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetObjectLegalHoldHandler, ACTION_READ)), "GET")).Queries("legal-hold", "") + bucket.Methods(http.MethodGet).Path(objectPath).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetObjectLegalHoldHandler, ACTION_GET_OBJECT_LEGAL_HOLD)), "GET")).Queries("legal-hold", "") // objects with query @@ -964,7 +964,7 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) { bucket.Methods(http.MethodPut).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.PutBucketVersioningHandler, ACTION_WRITE)), "PUT")).Queries("versioning", "") // GetObjectLockConfiguration / PutObjectLockConfiguration (bucket-level operations) - bucket.Methods(http.MethodGet).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetObjectLockConfigurationHandler, ACTION_READ)), "GET")).Queries("object-lock", "") + bucket.Methods(http.MethodGet).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetObjectLockConfigurationHandler, ACTION_GET_BUCKET_OBJECT_LOCK_CONFIG)), "GET")).Queries("object-lock", "") bucket.Methods(http.MethodPut).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.PutObjectLockConfigurationHandler, ACTION_PUT_BUCKET_OBJECT_LOCK_CONFIG)), "PUT")).Queries("object-lock", "") // GetBucketTagging