diff --git a/weed/s3api/iceberg/server.go b/weed/s3api/iceberg/server.go index fe280d829..043b20096 100644 --- a/weed/s3api/iceberg/server.go +++ b/weed/s3api/iceberg/server.go @@ -46,6 +46,13 @@ type Server struct { // NewServer creates a new Iceberg REST Catalog server. func NewServer(filerClient FilerClient, authenticator S3Authenticator) *Server { manager := s3tables.NewManager() + // Mirror the S3 port: fall open by default only when the gateway itself is + // open. With auth configured, an authenticated catalog caller must pass the + // normal permission check instead of being allowed because no policy denied + // it — even if the full identity struct ever fails to reach the handler. + if authenticator != nil { + manager.SetDefaultAllow(authenticator.DefaultAllow()) + } return &Server{ filerClient: filerClient, tablesManager: manager, diff --git a/weed/s3api/s3tables/handler.go b/weed/s3api/s3tables/handler.go index 2572a49c3..60386da83 100644 --- a/weed/s3api/s3tables/handler.go +++ b/weed/s3api/s3tables/handler.go @@ -48,6 +48,7 @@ type S3TablesHandler struct { region string accountID string defaultAllow bool // Whether to allow access by default (for zero-config IAM) + trusted bool // Trusted local tooling (shell/admin) bypasses authorization iamAuthorizer IAMAuthorizer } @@ -79,6 +80,12 @@ func (h *S3TablesHandler) SetDefaultAllow(allow bool) { h.defaultAllow = allow } +// SetTrusted lets local tooling that talks to the filer directly (shell, admin +// console) bypass authorization. HTTP-facing callers must not set it. +func (h *S3TablesHandler) SetTrusted(trusted bool) { + h.trusted = trusted +} + // FilerClient interface for filer operations type FilerClient interface { WithFilerClient(streamingMode bool, fn func(client filer_pb.SeaweedFilerClient) error) error @@ -212,7 +219,11 @@ func (h *S3TablesHandler) getAccountID(r *http.Request) string { idField := accountVal.FieldByName("Id") if idField.IsValid() && idField.Kind() == reflect.String { if principal := normalizePrincipalID(idField.String()); principal != "" { - return principal + // Account-less identities default to the admin account; only + // keep it for real admins, else use the unique identity name. + if principal != s3_constants.AccountAdminId || hasAdminAction(getIdentityActions(r)) { + return principal + } } } } diff --git a/weed/s3api/s3tables/handler_bucket_create.go b/weed/s3api/s3tables/handler_bucket_create.go index 9e7f4e283..37d692780 100644 --- a/weed/s3api/s3tables/handler_bucket_create.go +++ b/weed/s3api/s3tables/handler_bucket_create.go @@ -53,7 +53,7 @@ func (h *S3TablesHandler) handleCreateTableBucket(w http.ResponseWriter, r *http } if !CheckPermissionWithContext("CreateTableBucket", principal, owner, "", "", &PolicyContext{ IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to create table buckets") return NewAuthError("CreateTableBucket", principal, "not authorized to create table buckets") diff --git a/weed/s3api/s3tables/handler_bucket_get_list_delete.go b/weed/s3api/s3tables/handler_bucket_get_list_delete.go index 0bed2943b..18be86bf9 100644 --- a/weed/s3api/s3tables/handler_bucket_get_list_delete.go +++ b/weed/s3api/s3tables/handler_bucket_get_list_delete.go @@ -72,7 +72,7 @@ func (h *S3TablesHandler) handleGetTableBucket(w http.ResponseWriter, r *http.Re if !CheckPermissionWithContext("GetTableBucket", principal, metadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{ TableBucketName: bucketName, IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to get table bucket details") return ErrAccessDenied @@ -97,16 +97,9 @@ func (h *S3TablesHandler) handleListTableBuckets(w http.ResponseWriter, r *http. return err } - principal := h.getAccountID(r) + // No account-level gate: visibility is enforced per bucket below, so an + // owner can always list its own buckets and others are filtered out. accountID := h.getAccountID(r) - identityActions := getIdentityActions(r) - if !CheckPermissionWithContext("ListTableBuckets", principal, accountID, "", "", &PolicyContext{ - IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, - }) { - h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to list table buckets") - return NewAuthError("ListTableBuckets", principal, "not authorized to list table buckets") - } maxBuckets := req.MaxBuckets if maxBuckets <= 0 { @@ -200,7 +193,7 @@ func (h *S3TablesHandler) handleListTableBuckets(w http.ResponseWriter, r *http. if !CheckPermissionWithContext("GetTableBucket", accountID, metadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{ TableBucketName: entry.Entry.Name, IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) { continue } @@ -303,7 +296,7 @@ func (h *S3TablesHandler) handleDeleteTableBucket(w http.ResponseWriter, r *http if !CheckPermissionWithContext("DeleteTableBucket", principal, metadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{ TableBucketName: bucketName, IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) { return NewAuthError("DeleteTableBucket", principal, fmt.Sprintf("not authorized to delete bucket %s", bucketName)) } diff --git a/weed/s3api/s3tables/handler_identity_test.go b/weed/s3api/s3tables/handler_identity_test.go index 1e928a054..12122a81e 100644 --- a/weed/s3api/s3tables/handler_identity_test.go +++ b/weed/s3api/s3tables/handler_identity_test.go @@ -16,7 +16,9 @@ type testIdentityAccount struct { } type testIdentity struct { + Name string Account *testIdentityAccount + Actions []string Claims map[string]interface{} } @@ -112,6 +114,53 @@ func TestGetAccountIDFallsBackToAccountID(t *testing.T) { assert.Equal(t, "my-account-id", h.getAccountID(req), "expected Account.Id to be returned when claims are missing") } +func TestGetAccountIDNonAdminDoesNotInheritAdminAccount(t *testing.T) { + h := NewS3TablesHandler() + id := &testIdentity{ + Account: &testIdentityAccount{Id: s3_constants.AccountAdminId}, + Actions: []string{"Read", "List"}, + } + req := httptest.NewRequest(http.MethodGet, "/", nil) + req = req.WithContext(s3_constants.SetIdentityInContext(req.Context(), id)) + req = req.WithContext(s3_constants.SetIdentityNameInContext(req.Context(), "readonly")) + + assert.Equal(t, "readonly", h.getAccountID(req), "a non-admin identity must not inherit the shared admin account") +} + +func TestGetAccountIDAdminActionKeepsAdminAccount(t *testing.T) { + h := NewS3TablesHandler() + id := &testIdentity{ + Account: &testIdentityAccount{Id: s3_constants.AccountAdminId}, + Actions: []string{s3_constants.ACTION_ADMIN}, + } + req := httptest.NewRequest(http.MethodGet, "/", nil) + req = req.WithContext(s3_constants.SetIdentityInContext(req.Context(), id)) + req = req.WithContext(s3_constants.SetIdentityNameInContext(req.Context(), "root")) + + assert.Equal(t, s3_constants.AccountAdminId, h.getAccountID(req), "an admin identity keeps the admin account as principal") +} + +func TestDefaultAllowForOnlyAppliesToUnauthenticatedOrAnonymous(t *testing.T) { + h := NewS3TablesHandler() + h.SetDefaultAllow(true) + + noIdentity := httptest.NewRequest(http.MethodGet, "/", nil) + assert.True(t, h.defaultAllowFor(noIdentity), "zero-config requests with no identity keep the open default") + + anon := httptest.NewRequest(http.MethodGet, "/", nil) + anon = anon.WithContext(s3_constants.SetIdentityInContext(anon.Context(), + &testIdentity{Name: s3_constants.AccountAnonymousId})) + assert.True(t, h.defaultAllowFor(anon), "anonymous requests keep the open default") + + authed := httptest.NewRequest(http.MethodGet, "/", nil) + authed = authed.WithContext(s3_constants.SetIdentityInContext(authed.Context(), + &testIdentity{Name: "readonly", Account: &testIdentityAccount{Id: s3_constants.AccountAdminId}, Actions: []string{"Read"}})) + assert.False(t, h.defaultAllowFor(authed), "an authenticated identity must not benefit from the open default") + + h.SetDefaultAllow(false) + assert.False(t, h.defaultAllowFor(noIdentity), "default-allow disabled is never open") +} + func TestGetAccountIDNormalizesAccountIDARN(t *testing.T) { h := NewS3TablesHandler() id := &testIdentity{ diff --git a/weed/s3api/s3tables/handler_namespace.go b/weed/s3api/s3tables/handler_namespace.go index fe622daa7..61b4ae896 100644 --- a/weed/s3api/s3tables/handler_namespace.go +++ b/weed/s3api/s3tables/handler_namespace.go @@ -118,7 +118,7 @@ func (h *S3TablesHandler) handleCreateNamespace(w http.ResponseWriter, r *http.R Namespace: namespaceName, TableBucketTags: bucketTags, IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) { glog.Infof("S3Tables: Permission denied for CreateNamespace - principal=%s, owner=%s", principal, bucketMetadata.OwnerAccountID) h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to create namespace in this bucket") @@ -259,7 +259,7 @@ func (h *S3TablesHandler) handleGetNamespace(w http.ResponseWriter, r *http.Requ Namespace: namespaceName, TableBucketTags: bucketTags, IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) { h.writeError(w, http.StatusNotFound, ErrCodeNoSuchNamespace, "namespace not found") return ErrAccessDenied @@ -346,7 +346,7 @@ func (h *S3TablesHandler) handleListNamespaces(w http.ResponseWriter, r *http.Re TableBucketName: bucketName, TableBucketTags: bucketTags, IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) { h.writeError(w, http.StatusNotFound, ErrCodeNoSuchBucket, fmt.Sprintf("table bucket %s not found", bucketName)) return ErrAccessDenied @@ -531,7 +531,7 @@ func (h *S3TablesHandler) handleDeleteNamespace(w http.ResponseWriter, r *http.R Namespace: namespaceName, TableBucketTags: bucketTags, IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) { h.writeError(w, http.StatusNotFound, ErrCodeNoSuchNamespace, "namespace not found") return ErrAccessDenied diff --git a/weed/s3api/s3tables/handler_policy.go b/weed/s3api/s3tables/handler_policy.go index b2c5adbd8..c749412ef 100644 --- a/weed/s3api/s3tables/handler_policy.go +++ b/weed/s3api/s3tables/handler_policy.go @@ -94,7 +94,7 @@ func (h *S3TablesHandler) handlePutTableBucketPolicy(w http.ResponseWriter, r *h if !CheckPermissionWithContext("PutTableBucketPolicy", principal, bucketMetadata.OwnerAccountID, "", bucketARN, &PolicyContext{ TableBucketName: bucketName, IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to put table bucket policy") return NewAuthError("PutTableBucketPolicy", principal, "not authorized to put table bucket policy") @@ -172,7 +172,7 @@ func (h *S3TablesHandler) handleGetTableBucketPolicy(w http.ResponseWriter, r *h if !CheckPermissionWithContext("GetTableBucketPolicy", principal, bucketMetadata.OwnerAccountID, string(policy), bucketARN, &PolicyContext{ TableBucketName: bucketName, IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to get table bucket policy") return NewAuthError("GetTableBucketPolicy", principal, "not authorized to get table bucket policy") @@ -248,7 +248,7 @@ func (h *S3TablesHandler) handleDeleteTableBucketPolicy(w http.ResponseWriter, r if !CheckPermissionWithContext("DeleteTableBucketPolicy", principal, bucketMetadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{ TableBucketName: bucketName, IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to delete table bucket policy") return NewAuthError("DeleteTableBucketPolicy", principal, "not authorized to delete table bucket policy") @@ -349,7 +349,7 @@ func (h *S3TablesHandler) handlePutTablePolicy(w http.ResponseWriter, r *http.Re Namespace: namespaceName, TableName: tableName, IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to put table policy") return NewAuthError("PutTablePolicy", principal, "not authorized to put table policy") @@ -457,7 +457,7 @@ func (h *S3TablesHandler) handleGetTablePolicy(w http.ResponseWriter, r *http.Re Namespace: namespaceName, TableName: tableName, IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to get table policy") return NewAuthError("GetTablePolicy", principal, "not authorized to get table policy") @@ -547,7 +547,7 @@ func (h *S3TablesHandler) handleDeleteTablePolicy(w http.ResponseWriter, r *http Namespace: namespaceName, TableName: tableName, IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to delete table policy") return NewAuthError("DeleteTablePolicy", principal, "not authorized to delete table policy") @@ -646,7 +646,7 @@ func (h *S3TablesHandler) handleTagResource(w http.ResponseWriter, r *http.Reque TagKeys: requestTagKeys, ResourceTags: existingTags, IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) { return NewAuthError("TagResource", principal, "not authorized to tag resource") } @@ -764,7 +764,7 @@ func (h *S3TablesHandler) handleListTagsForResource(w http.ResponseWriter, r *ht TableBucketTags: bucketTags, ResourceTags: tags, IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) { return NewAuthError("ListTagsForResource", principal, "not authorized to list tags for resource") } @@ -872,7 +872,7 @@ func (h *S3TablesHandler) handleUntagResource(w http.ResponseWriter, r *http.Req TagKeys: req.TagKeys, ResourceTags: tags, IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) { return NewAuthError("UntagResource", principal, "not authorized to untag resource") } diff --git a/weed/s3api/s3tables/handler_table.go b/weed/s3api/s3tables/handler_table.go index 0b5b50932..ecfe3eafb 100644 --- a/weed/s3api/s3tables/handler_table.go +++ b/weed/s3api/s3tables/handler_table.go @@ -143,7 +143,7 @@ func (h *S3TablesHandler) handleCreateTable(w http.ResponseWriter, r *http.Reque TagKeys: mapKeys(req.Tags), TableBucketTags: bucketTags, IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) bucketAllowed := CheckPermissionWithContext("CreateTable", accountID, bucketMetadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{ TableBucketName: bucketName, @@ -153,7 +153,7 @@ func (h *S3TablesHandler) handleCreateTable(w http.ResponseWriter, r *http.Reque TagKeys: mapKeys(req.Tags), TableBucketTags: bucketTags, IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) if !nsAllowed && !bucketAllowed { @@ -386,7 +386,7 @@ func (h *S3TablesHandler) handleGetTable(w http.ResponseWriter, r *http.Request, TableBucketTags: bucketTags, ResourceTags: tableTags, IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) bucketAllowed := CheckPermissionWithContext("GetTable", accountID, bucketMetadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{ TableBucketName: bucketName, @@ -395,7 +395,7 @@ func (h *S3TablesHandler) handleGetTable(w http.ResponseWriter, r *http.Request, TableBucketTags: bucketTags, ResourceTags: tableTags, IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) if !tableAllowed && !bucketAllowed { @@ -525,14 +525,14 @@ func (h *S3TablesHandler) handleListTables(w http.ResponseWriter, r *http.Reques Namespace: namespaceName, TableBucketTags: bucketTags, IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) bucketAllowed := CheckPermissionWithContext("ListTables", accountID, bucketMeta.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{ TableBucketName: bucketName, Namespace: namespaceName, TableBucketTags: bucketTags, IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) if !nsAllowed && !bucketAllowed { return ErrAccessDenied @@ -577,7 +577,7 @@ func (h *S3TablesHandler) handleListTables(w http.ResponseWriter, r *http.Reques TableBucketName: bucketName, TableBucketTags: bucketTags, IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) { return ErrAccessDenied } @@ -916,7 +916,7 @@ func (h *S3TablesHandler) handleDeleteTable(w http.ResponseWriter, r *http.Reque TableBucketTags: bucketTags, ResourceTags: tableTags, IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) bucketAllowed := CheckPermissionWithContext("DeleteTable", principal, bucketMetadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{ TableBucketName: bucketName, @@ -925,7 +925,7 @@ func (h *S3TablesHandler) handleDeleteTable(w http.ResponseWriter, r *http.Reque TableBucketTags: bucketTags, ResourceTags: tableTags, IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) if !tableAllowed && !bucketAllowed { h.writeError(w, http.StatusForbidden, ErrCodeAccessDenied, "not authorized to delete table") @@ -1058,7 +1058,7 @@ func (h *S3TablesHandler) handleUpdateTable(w http.ResponseWriter, r *http.Reque TableBucketTags: bucketTags, ResourceTags: tableTags, IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) bucketAllowed := CheckPermissionWithContext("UpdateTable", principal, bucketMetadata.OwnerAccountID, bucketPolicy, bucketARN, &PolicyContext{ TableBucketName: bucketName, @@ -1067,7 +1067,7 @@ func (h *S3TablesHandler) handleUpdateTable(w http.ResponseWriter, r *http.Reque TableBucketTags: bucketTags, ResourceTags: tableTags, IdentityActions: identityActions, - DefaultAllow: h.defaultAllow, + DefaultAllow: h.defaultAllowFor(r), }) if !tableAllowed && !bucketAllowed { diff --git a/weed/s3api/s3tables/iam.go b/weed/s3api/s3tables/iam.go index aa63b6f3c..c5cbef59d 100644 --- a/weed/s3api/s3tables/iam.go +++ b/weed/s3api/s3tables/iam.go @@ -48,6 +48,24 @@ func (h *S3TablesHandler) shouldUseIAM(r *http.Request, identityActions, identit return len(identityPolicyNames) > 0 } +// defaultAllowFor reports whether the open-by-default fallback applies: only for +// trusted tooling or unauthenticated/anonymous access. An authenticated principal +// must pass an explicit check. +func (h *S3TablesHandler) defaultAllowFor(r *http.Request) bool { + if h.trusted { + return true + } + if !h.defaultAllow { + return false + } + // The Manager path forwards only the identity name, so a name alone (no + // identity object) still counts as an authenticated principal. + if s3_constants.GetIdentityFromContext(r) == nil && s3_constants.GetIdentityNameFromContext(r) == "" { + return true + } + return isAnonymousIdentity(r) +} + func isAnonymousIdentity(r *http.Request) bool { val, ok := getIdentityStructValue(r) if !ok { diff --git a/weed/s3api/s3tables/manager.go b/weed/s3api/s3tables/manager.go index c3dcbc1ed..10d7856ff 100644 --- a/weed/s3api/s3tables/manager.go +++ b/weed/s3api/s3tables/manager.go @@ -41,6 +41,11 @@ func (m *Manager) SetDefaultAllow(allow bool) { m.handler.SetDefaultAllow(allow) } +// SetTrusted lets trusted local tooling (shell, admin console) bypass authorization. +func (m *Manager) SetTrusted(trusted bool) { + m.handler.SetTrusted(trusted) +} + // Execute runs an S3 Tables operation and decodes the response into resp (if provided). func (m *Manager) Execute(ctx context.Context, filerClient FilerClient, operation string, req interface{}, resp interface{}, identity string) error { body, err := json.Marshal(req) diff --git a/weed/s3api/s3tables/manager_test.go b/weed/s3api/s3tables/manager_test.go new file mode 100644 index 000000000..6d7ec2075 --- /dev/null +++ b/weed/s3api/s3tables/manager_test.go @@ -0,0 +1,99 @@ +package s3tables + +import ( + "context" + "errors" + "testing" + + "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +var errFilerReached = errors.New("filer reached") + +// recordingFilerClient reports whether the handler reached the filer. The +// CreateTableBucket handler authorizes before touching the filer, so "filer +// reached" is a proxy for "authorization passed". +type recordingFilerClient struct { + called bool +} + +func (c *recordingFilerClient) WithFilerClient(streamingMode bool, fn func(client filer_pb.SeaweedFilerClient) error) error { + c.called = true + return errFilerReached +} + +// The Manager path (used by the Iceberg catalog) enforces authorization for +// authenticated callers and only falls open for trusted/zero-config access. +func TestManagerCreateTableBucketAuthorization(t *testing.T) { + lowPriv := &testIdentity{ + Name: "alice", + Account: &testIdentityAccount{Id: s3_constants.AccountAdminId}, + Actions: []string{"Read"}, + } + + cases := []struct { + name string + defaultAllow bool + trusted bool + ctx context.Context + identity string + wantFiler bool // true => authorization passed (filer reached) + }{ + { + name: "authenticated identity struct is enforced", + defaultAllow: true, + ctx: s3_constants.SetIdentityInContext(context.Background(), lowPriv), + identity: "alice", + wantFiler: false, + }, + { + name: "secured manager denies a name without struct", + defaultAllow: false, + ctx: context.Background(), + identity: "alice", + wantFiler: false, + }, + { + name: "untrusted name without struct is enforced", + defaultAllow: true, + ctx: context.Background(), + identity: "alice", + wantFiler: false, + }, + { + name: "trusted manager allows a name without struct", + defaultAllow: true, + trusted: true, + ctx: context.Background(), + identity: "alice", + wantFiler: true, + }, + { + name: "admin principal is allowed", + defaultAllow: false, + ctx: context.Background(), + identity: s3_constants.AccountAdminId, + wantFiler: true, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + m := NewManager() + m.SetDefaultAllow(tc.defaultAllow) + m.SetTrusted(tc.trusted) + fc := &recordingFilerClient{} + err := m.Execute(tc.ctx, fc, "CreateTableBucket", &CreateTableBucketRequest{Name: "testbucket"}, nil, tc.identity) + require.Error(t, err) // denied (403) or the filer sentinel (reached); never nil here + assert.Equal(t, tc.wantFiler, fc.called, "filer reached should equal authorization passed") + if !tc.wantFiler { + var s3Err *S3TablesError + require.ErrorAs(t, err, &s3Err) + assert.Equal(t, ErrCodeAccessDenied, s3Err.Type) + } + }) + } +} diff --git a/weed/s3api/s3tables/permissions.go b/weed/s3api/s3tables/permissions.go index e5cb45a01..08a666c83 100644 --- a/weed/s3api/s3tables/permissions.go +++ b/weed/s3api/s3tables/permissions.go @@ -193,16 +193,14 @@ func hasIdentityPermission(operation string, ctx *PolicyContext) bool { if !strings.Contains(operation, ":") { fullAction = "s3tables:" + operation } + if hasAdminAction(ctx.IdentityActions) { + return true + } candidates := []string{operation, fullAction} if ctx.TableBucketName != "" { candidates = append(candidates, operation+":"+ctx.TableBucketName, fullAction+":"+ctx.TableBucketName) } for _, action := range ctx.IdentityActions { - // Legacy static identities may still use broad admin markers or s3 wildcards. - // s3:* is treated as s3tables:* so shared admin policies still permit table access. - if action == "*" || action == string(s3_constants.ACTION_ADMIN) || action == "s3:*" || action == "s3tables:*" { - return true - } for _, candidate := range candidates { if action == candidate { return true @@ -215,6 +213,18 @@ func hasIdentityPermission(operation string, ctx *PolicyContext) bool { return false } +// hasAdminAction reports whether the action list grants blanket admin access. +// Legacy static identities may use broad markers or s3 wildcards; s3:* is treated +// as s3tables:* so shared admin policies still permit table access. +func hasAdminAction(actions []string) bool { + for _, action := range actions { + if action == "*" || action == s3_constants.ACTION_ADMIN || action == "s3:*" || action == "s3tables:*" { + return true + } + } + return false +} + // matchesPrincipal checks if the principal matches the statement's principal func matchesPrincipal(principalSpec interface{}, principal string) bool { if principalSpec == nil { diff --git a/weed/shell/s3tables_helpers.go b/weed/shell/s3tables_helpers.go index 94dab9d92..d595a5af7 100644 --- a/weed/shell/s3tables_helpers.go +++ b/weed/shell/s3tables_helpers.go @@ -28,6 +28,8 @@ func executeS3Tables(commandEnv *CommandEnv, operation string, req interface{}, defer cancel() return withFilerClient(commandEnv, func(client filer_pb.SeaweedFilerClient) error { manager := s3tables.NewManager() + // The shell talks to the filer directly with no S3 auth, so it is trusted. + manager.SetTrusted(true) mgrClient := s3tables.NewManagerClient(client) return manager.Execute(ctx, mgrClient, operation, req, resp, accountID) })