Commit Graph

26 Commits

Author SHA1 Message Date
Chris Lu
a4217dff5f s3tables: enhance DeleteTable authorization with policy checking
Fetch and evaluate table policies in DeleteTable handler to support policy-based
delegation. Aligns authorization behavior with GetTable and ListTables handlers
instead of only checking ownership.
2026-01-28 19:42:12 -08:00
Chris Lu
78c00e313a go fmt 2026-01-28 18:34:32 -08:00
Chris Lu
f5d26b803b s3tables: Fix ListTables authorization and policy parsing
Make ListTables authorization consistent with GetTable/CreateTable:

1. ListTables authorization now evaluates policies instead of owner-only checks:
   - For namespace listing: checks namespace policy AND bucket policy
   - For bucket-wide listing: checks bucket policy
   - Uses CanListTables permission framework

2. Remove owner-only filter in listTablesWithClient that prevented policy-based
   sharing of tables. Authorization is now enforced at the handler level, so all
   tables in the namespace/bucket are returned to authorized callers (who have
   access either via ownership or policy).

3. Add flexible PolicyDocument.UnmarshalJSON to support both single-object and
   array forms of Statement field:
   - Handles: {"Statement": {...}}
   - Handles: {"Statement": [{...}, {...}]}
   - Improves AWS IAM compatibility

This ensures cross-account table listing works when delegated via bucket/namespace
policies, consistent with the authorization model for other operations.
2026-01-28 18:27:37 -08:00
Chris Lu
25b0f86bda s3tables: Fix ownership consistency across handlers
Address three related ownership consistency issues:

1. CreateNamespace now sets OwnerAccountID to bucketMetadata.OwnerAccountID
   instead of request principal. This prevents namespaces created by
   delegated callers (via bucket policy) from becoming unmanageable, since
   ListNamespaces filters by bucket owner.

2. CreateTable now:
   - Fetches bucket metadata to use correct owner for bucket policy evaluation
   - Uses namespaceMetadata.OwnerAccountID for namespace policy checks
   - Uses bucketMetadata.OwnerAccountID for bucket policy checks
   - Sets table OwnerAccountID to namespaceMetadata.OwnerAccountID (inherited)

3. GetTable now:
   - Fetches bucket metadata to use correct owner for bucket policy evaluation
   - Uses metadata.OwnerAccountID for table policy checks
   - Uses bucketMetadata.OwnerAccountID for bucket policy checks

This ensures:
- Bucket owner retains implicit "owner always allowed" behavior even when
  evaluating bucket policies
- Ownership hierarchy is consistent (namespace owned by bucket, table owned by namespace)
- Cross-principal delegation via policies doesn't break ownership chains
2026-01-28 18:03:47 -08:00
Chris Lu
b7bba7e7dc s3tables: Generate ARNs using resource owner account ID
Change ARN generation to use resource OwnerAccountID instead of caller
identity (h.getAccountID(r)). This ensures ARNs are stable and consistent
regardless of which principal accesses the resource.

Updated generateTableBucketARN and generateTableARN function signatures
to accept ownerAccountID parameter. All call sites updated to pass the
resource owner's account ID from metadata.

This prevents ARN inconsistency issues when multiple principals have
access to the same resource via policies.
2026-01-28 17:38:22 -08:00
Chris Lu
e7b2869aa9 s3tables: Use policy framework for GetTable authorization
Replace strict ownership check with policy-based authorization in GetTable.
Now checks both table and bucket policies for GetTable permission, allowing
authorized non-owners to read table metadata.

Authorization logic:
- Table policy grants GetTable → allowed
- Bucket policy grants GetTable → allowed
- Otherwise → 404 NotFound (no access disclosed)

Maintains security through policy evaluation while enabling read delegation.
2026-01-28 17:37:12 -08:00
Chris Lu
bea0f8eda0 s3tables: Use policy framework for table creation authorization
Replace strict ownership check in CreateTable with policy-based authorization.
Now checks both namespace and bucket policies for CreateTable permission,
allowing delegation via resource policies while still respecting owner bypass.

Authorization logic:
- Namespace policy grants CreateTable → allowed
- Bucket policy grants CreateTable → allowed
- Otherwise → denied (even if same owner)

This enables cross-principal table creation via policies while maintaining
security through explicit allow/deny semantics.
2026-01-28 17:36:53 -08:00
Chris Lu
08bd1e2563 s3tables: Pre-validate namespace to return 400 instead of 500
Move validateNamespace call outside of filerClient.WithFilerClient closure
so that validation errors return HTTP 400 (InvalidRequest) instead of 500
(InternalError).

Before: Validation error inside closure → treated as internal error → 500
After: Validation error before closure → handled as bad request → 400

This provides correct error semantics: namespace validation is an input
validation issue, not a server error.
2026-01-28 17:03:04 -08:00
Chris Lu
e0da63fd0a s3tables: Add upper bound validation for MaxTables parameter
MaxTables is user-controlled and influences gRPC ListEntries limits via
uint32(maxTables*2). Without an upper bound, very large values can overflow
uint32 or cause excessively large directory scans. Cap MaxTables to 1000 and
return InvalidRequest for out-of-range values, consistent with S3 MaxKeys
handling.
2026-01-28 16:20:32 -08:00
Chris Lu
a6c3e96f7b s3tables: fix double-write issue in handleListTables
Remove premature HTTP error writes from within WithFilerClient closure
to prevent duplicate status code responses. Error handling is now
consistently performed at the top level using isAuthError.
2026-01-28 14:41:14 -08:00
Chris Lu
4d4af0589b s3tables: standardize access denied errors using ErrAccessDenied constant 2026-01-28 14:33:01 -08:00
Chris Lu
0d65daad4c s3tables: improve pagination robustness and error handling in table listing handlers 2026-01-28 14:04:09 -08:00
Chris Lu
d4ebafbacd s3tables: enforce strict resource ownership and implement result filtering for tables 2026-01-28 13:59:28 -08:00
Chris Lu
1697ec862f ownerAccountID 2026-01-28 13:54:49 -08:00
Chris Lu
78a007d42b s3tables: refactor permission checks to use resource owner in table handlers 2026-01-28 13:50:19 -08:00
Chris Lu
f17ec59d46 s3tables: implement optimistic concurrency for table deletion
Added VersionToken validation to handleDeleteTable. Refactored table
listing to use request context for accurate ARN generation and fixed
cross-namespace pagination issues.
2026-01-28 13:25:32 -08:00
Chris Lu
5c43f1e6a4 s3tables: fix cross-namespace pagination in listTablesInAllNamespaces 2026-01-28 12:46:21 -08:00
Chris Lu
47ef8c3cce s3tables: add table name validation and cleanup duplicated logic in table handlers 2026-01-28 12:30:32 -08:00
Chris Lu
f13e250fc3 S3 Tables: fix gRPC stream loop handling for list operations
- Correctly handle io.EOF to terminate loops gracefully.
- Propagate other errors to prevent silent failures.
- Ensure all list results are processed effectively.
2026-01-28 12:09:04 -08:00
Chris Lu
dc4c62e742 s3tables: harden auth and error handling
- Add authorization checks to all S3 Tables handlers (policy, table ops) to enforce security
- Improve error handling to distinguish between NotFound (404) and InternalError (500)
- Fix directory FileMode usage in filer_ops
- Improve test randomness for version tokens
- Update permissions comments to acknowledge IAM gaps
2026-01-28 11:49:57 -08:00
Chris Lu
62a1178a0b s3tables: improve robustness, security, and error propagation in handlers
- Implement strict table name validation (prevention of path traversal and character enforcement)
- Add nil checks for entry.Entry in all listing loops to prevent panics
- Propagate backend errors instead of swallowing them or assuming 404
- Correctly map filer_pb.ErrNotFound to appropriate S3 error codes
- Standardize existence checks across bucket, namespace, and table handlers
2026-01-28 11:37:02 -08:00
Chris Lu
2c551dad5d s3tables: fix pagination and enhance error handling in list/delete operations
- Fix InclusiveStartFrom logic to ensure exclusive start on continued pages
- Prevent duplicates in bucket, namespace, and table listings
- Fail fast on listing errors during bucket and namespace deletion
- Stop swallowing errors in handleListTables and return proper HTTP error responses
2026-01-28 10:36:28 -08:00
Chris Lu
33da87452b Refine S3 Tables implementation to address code review feedback
- Standardize namespace representation to []string
- Improve listing logic with pagination and StartFromFileName
- Enhance error handling with sentinel errors and robust checks
- Add JSON encoding error logging
- Fix CI workflow to use gofmt -l
- Standardize timestamps in directory creation
- Validate single-level namespaces
2026-01-28 10:04:27 -08:00
Chris Lu
b30631c3b5 s3tables: propagate request context to filer operations 2026-01-28 09:38:01 -08:00
Chris Lu
ef3873b616 s3tables: add error handling for json.Marshal calls
- Add error handling in handler_namespace.go (metadata marshaling)
- Add error handling in handler_table.go (metadata and tags marshaling)
- Add error handling in handler_policy.go (tag marshaling in TagResource and UntagResource)
- Return proper errors with context instead of silently ignoring failures
2026-01-28 01:13:42 -08:00
Chris Lu
3b1920cf43 s3tables: add handler_ prefix to operation handler files
- Rename bucket_create.go → handler_bucket_create.go
- Rename bucket_get_list_delete.go → handler_bucket_get_list_delete.go
- Rename namespace.go → handler_namespace.go
- Rename table.go → handler_table.go
- Rename policy.go → handler_policy.go

Improves file organization by clearly identifying handler implementations.
No code changes, refactoring only.
2026-01-28 01:00:00 -08:00