mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-11-10 05:48:19 +01:00
fix(s3): preserve exact policy document in embedded IAM put/get-user-policy (#9025)
* fix(s3): preserve exact policy document in embedded IAM PutUserPolicy/GetUserPolicy (#9008) The embedded IAM implementation (used when IAM requests go through the S3 gateway) discarded the original policy document on PutUserPolicy, storing only the lossy ident.Actions representation. GetUserPolicy then reconstructed the document from these coarse-grained actions, producing wildcard-expanded actions (s3:GetObject → s3:Get*), duplicates, and collapsed resources (array → single string). PR #9009 fixed this in the standalone IAM server (weed/iamapi/) but the embedded IAM (weed/s3api/) — which is the code path most users hit — had the same bugs. Changes: - Add InlinePolicyStore optional interface to credential store, with implementations for FilerEtcStore (uses existing PoliciesCollection), MemoryStore, and PropagatingCredentialStore. - Embedded IAM PutUserPolicy now persists the original policy document via CredentialManager.PutUserInlinePolicy for lossless round-trips. - Embedded IAM GetUserPolicy first tries the stored inline policy; only falls back to lossy reconstruction from ident.Actions when no stored document exists (e.g. policies created before this fix). - Fix the fallback reconstruction: add action deduplication and preserve resource paths verbatim (no more spurious /* appending). - Update DeleteUserPolicy/ListUserPolicies to use stored inline policies. * fix(s3): address PR review feedback for embedded IAM inline policies - Validate PolicyName is non-empty in PutUserPolicy and DeleteUserPolicy - Add recomputeActions() to aggregate ident.Actions from ALL stored inline policies on put/delete, fixing the issue where a second PutUserPolicy would overwrite the first policy's enforcement - Log errors from GetUserInlinePolicy in the GetUserPolicy fallback instead of silently ignoring them - Add initialization guards to MemoryStore GetUserInlinePolicy and ListUserInlinePolicies for consistency with other read methods * fix(s3): make inline policy persistence fatal and propagate recompute errors Address second round of review feedback: - recomputeActions() now returns ([]string, error) so callers can distinguish store failures from "no stored policies" and abort the mutation on transient errors instead of silently falling back. - PutUserInlinePolicy and DeleteUserInlinePolicy failures are now fatal: the API call returns ServiceFailure instead of logging and continuing, keeping ident.Actions and stored policy state in sync. * chore: gofmt weed/s3api/iceberg/handlers_oauth.go Pre-existing formatting issue from #9017; fixes S3 Tables Format Check CI.
This commit is contained in:
1 parent
e648c76bcf
commit
c390448906
8 files changed
+429
-24
No files matched your search
@@ -271,6 +271,40 @@ func (cm *CredentialManager) UpdatePolicy(ctx context.Context, name string, docu
|
||||
return cm.Store.PutPolicy(ctx, name, document)
|
||||
}
|
||||
|
||||
// PutUserInlinePolicy stores a per-user inline policy document.
|
||||
// Returns nil without error if the underlying store does not support inline policies.
|
||||
func (cm *CredentialManager) PutUserInlinePolicy(ctx context.Context, userName, policyName string, document policy_engine.PolicyDocument) error {
|
||||
if store, ok := cm.Store.(InlinePolicyStore); ok {
|
||||
return store.PutUserInlinePolicy(ctx, userName, policyName, document)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// GetUserInlinePolicy retrieves a per-user inline policy document.
|
||||
// Returns nil without error if the underlying store does not support inline policies.
|
||||
func (cm *CredentialManager) GetUserInlinePolicy(ctx context.Context, userName, policyName string) (*policy_engine.PolicyDocument, error) {
|
||||
if store, ok := cm.Store.(InlinePolicyStore); ok {
|
||||
return store.GetUserInlinePolicy(ctx, userName, policyName)
|
||||
}
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
// DeleteUserInlinePolicy removes a per-user inline policy document.
|
||||
func (cm *CredentialManager) DeleteUserInlinePolicy(ctx context.Context, userName, policyName string) error {
|
||||
if store, ok := cm.Store.(InlinePolicyStore); ok {
|
||||
return store.DeleteUserInlinePolicy(ctx, userName, policyName)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// ListUserInlinePolicies returns the names of all inline policies for a user.
|
||||
func (cm *CredentialManager) ListUserInlinePolicies(ctx context.Context, userName string) ([]string, error) {
|
||||
if store, ok := cm.Store.(InlinePolicyStore); ok {
|
||||
return store.ListUserInlinePolicies(ctx, userName)
|
||||
}
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
// LoadS3ConfigFile reads a static S3 identity config file and registers
|
||||
// the identities so they appear in LoadConfiguration and listing results.
|
||||
func (cm *CredentialManager) LoadS3ConfigFile(path string) error {
|
||||
|
||||
@@ -137,5 +137,16 @@ type PolicyManager interface {
|
||||
GetPolicy(ctx context.Context, name string) (*policy_engine.PolicyDocument, error)
|
||||
}
|
||||
|
||||
// InlinePolicyStore is an optional interface for credential stores that support
|
||||
// per-user inline policy storage. Stores that implement this interface preserve
|
||||
// the exact policy document submitted via PutUserPolicy, enabling lossless
|
||||
// round-trips through GetUserPolicy.
|
||||
type InlinePolicyStore interface {
|
||||
PutUserInlinePolicy(ctx context.Context, userName, policyName string, document policy_engine.PolicyDocument) error
|
||||
GetUserInlinePolicy(ctx context.Context, userName, policyName string) (*policy_engine.PolicyDocument, error)
|
||||
DeleteUserInlinePolicy(ctx context.Context, userName, policyName string) error
|
||||
ListUserInlinePolicies(ctx context.Context, userName string) ([]string, error)
|
||||
}
|
||||
|
||||
// Stores holds all available credential store implementations
|
||||
var Stores []CredentialStore
|
||||
@@ -316,6 +316,69 @@ func (store *FilerEtcStore) GetPolicy(ctx context.Context, name string) (*policy
|
||||
return nil, nil // Policy not found
|
||||
}
|
||||
|
||||
// PutUserInlinePolicy stores a per-user inline policy document.
|
||||
func (store *FilerEtcStore) PutUserInlinePolicy(ctx context.Context, userName, policyName string, document policy_engine.PolicyDocument) error {
|
||||
store.policyMu.Lock()
|
||||
defer store.policyMu.Unlock()
|
||||
|
||||
policiesCollection, _, err := store.loadLegacyPoliciesCollection(ctx)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if policiesCollection.InlinePolicies[userName] == nil {
|
||||
policiesCollection.InlinePolicies[userName] = make(map[string]policy_engine.PolicyDocument)
|
||||
}
|
||||
policiesCollection.InlinePolicies[userName][policyName] = document
|
||||
return store.saveLegacyPoliciesCollection(ctx, policiesCollection)
|
||||
}
|
||||
|
||||
// GetUserInlinePolicy retrieves a per-user inline policy document.
|
||||
func (store *FilerEtcStore) GetUserInlinePolicy(ctx context.Context, userName, policyName string) (*policy_engine.PolicyDocument, error) {
|
||||
policiesCollection, _, err := store.loadLegacyPoliciesCollection(ctx)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if userPolicies := policiesCollection.InlinePolicies[userName]; userPolicies != nil {
|
||||
if doc, exists := userPolicies[policyName]; exists {
|
||||
return &doc, nil
|
||||
}
|
||||
}
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
// DeleteUserInlinePolicy removes a per-user inline policy document.
|
||||
func (store *FilerEtcStore) DeleteUserInlinePolicy(ctx context.Context, userName, policyName string) error {
|
||||
store.policyMu.Lock()
|
||||
defer store.policyMu.Unlock()
|
||||
|
||||
policiesCollection, _, err := store.loadLegacyPoliciesCollection(ctx)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if userPolicies := policiesCollection.InlinePolicies[userName]; userPolicies != nil {
|
||||
delete(userPolicies, policyName)
|
||||
if len(userPolicies) == 0 {
|
||||
delete(policiesCollection.InlinePolicies, userName)
|
||||
}
|
||||
}
|
||||
return store.saveLegacyPoliciesCollection(ctx, policiesCollection)
|
||||
}
|
||||
|
||||
// ListUserInlinePolicies returns the names of all inline policies for a user.
|
||||
func (store *FilerEtcStore) ListUserInlinePolicies(ctx context.Context, userName string) ([]string, error) {
|
||||
policiesCollection, _, err := store.loadLegacyPoliciesCollection(ctx)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
userPolicies := policiesCollection.InlinePolicies[userName]
|
||||
names := make([]string, 0, len(userPolicies))
|
||||
for name := range userPolicies {
|
||||
names = append(names, name)
|
||||
}
|
||||
return names, nil
|
||||
}
|
||||
|
||||
// ListPolicyNames returns all managed policy names stored in the filer.
|
||||
func (store *FilerEtcStore) ListPolicyNames(ctx context.Context) ([]string, error) {
|
||||
names := make([]string, 0)
|
||||
|
||||
@@ -105,3 +105,71 @@ func (store *MemoryStore) DeletePolicy(ctx context.Context, name string) error {
|
||||
delete(store.policies, name)
|
||||
return nil
|
||||
}
|
||||
|
||||
// PutUserInlinePolicy stores a per-user inline policy document.
|
||||
func (store *MemoryStore) PutUserInlinePolicy(ctx context.Context, userName, policyName string, document policy_engine.PolicyDocument) error {
|
||||
store.mu.Lock()
|
||||
defer store.mu.Unlock()
|
||||
|
||||
if !store.initialized {
|
||||
return fmt.Errorf("store not initialized")
|
||||
}
|
||||
|
||||
if store.inlinePolicies[userName] == nil {
|
||||
store.inlinePolicies[userName] = make(map[string]policy_engine.PolicyDocument)
|
||||
}
|
||||
store.inlinePolicies[userName][policyName] = document
|
||||
return nil
|
||||
}
|
||||
|
||||
// GetUserInlinePolicy retrieves a per-user inline policy document.
|
||||
func (store *MemoryStore) GetUserInlinePolicy(ctx context.Context, userName, policyName string) (*policy_engine.PolicyDocument, error) {
|
||||
store.mu.RLock()
|
||||
defer store.mu.RUnlock()
|
||||
|
||||
if !store.initialized {
|
||||
return nil, fmt.Errorf("store not initialized")
|
||||
}
|
||||
|
||||
if userPolicies := store.inlinePolicies[userName]; userPolicies != nil {
|
||||
if doc, exists := userPolicies[policyName]; exists {
|
||||
return &doc, nil
|
||||
}
|
||||
}
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
// DeleteUserInlinePolicy removes a per-user inline policy document.
|
||||
func (store *MemoryStore) DeleteUserInlinePolicy(ctx context.Context, userName, policyName string) error {
|
||||
store.mu.Lock()
|
||||
defer store.mu.Unlock()
|
||||
|
||||
if !store.initialized {
|
||||
return fmt.Errorf("store not initialized")
|
||||
}
|
||||
|
||||
if userPolicies := store.inlinePolicies[userName]; userPolicies != nil {
|
||||
delete(userPolicies, policyName)
|
||||
if len(userPolicies) == 0 {
|
||||
delete(store.inlinePolicies, userName)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// ListUserInlinePolicies returns the names of all inline policies for a user.
|
||||
func (store *MemoryStore) ListUserInlinePolicies(ctx context.Context, userName string) ([]string, error) {
|
||||
store.mu.RLock()
|
||||
defer store.mu.RUnlock()
|
||||
|
||||
if !store.initialized {
|
||||
return nil, fmt.Errorf("store not initialized")
|
||||
}
|
||||
|
||||
userPolicies := store.inlinePolicies[userName]
|
||||
names := make([]string, 0, len(userPolicies))
|
||||
for name := range userPolicies {
|
||||
names = append(names, name)
|
||||
}
|
||||
return names, nil
|
||||
}
|
||||
@@ -18,12 +18,13 @@ func init() {
|
||||
// This is primarily intended for testing purposes
|
||||
type MemoryStore struct {
|
||||
mu sync.RWMutex
|
||||
users map[string]*iam_pb.Identity // username -> identity
|
||||
accessKeys map[string]string // access_key -> username
|
||||
serviceAccounts map[string]*iam_pb.ServiceAccount // id -> service_account
|
||||
serviceAccountAccessKeys map[string]string // access_key -> id
|
||||
policies map[string]policy_engine.PolicyDocument // policy_name -> policy_document
|
||||
groups map[string]*iam_pb.Group // group_name -> group
|
||||
users map[string]*iam_pb.Identity // username -> identity
|
||||
accessKeys map[string]string // access_key -> username
|
||||
serviceAccounts map[string]*iam_pb.ServiceAccount // id -> service_account
|
||||
serviceAccountAccessKeys map[string]string // access_key -> id
|
||||
policies map[string]policy_engine.PolicyDocument // policy_name -> policy_document
|
||||
inlinePolicies map[string]map[string]policy_engine.PolicyDocument // username -> policy_name -> document
|
||||
groups map[string]*iam_pb.Group // group_name -> group
|
||||
initialized bool
|
||||
}
|
||||
|
||||
@@ -44,6 +45,7 @@ func (store *MemoryStore) Initialize(configuration util.Configuration, prefix st
|
||||
store.serviceAccounts = make(map[string]*iam_pb.ServiceAccount)
|
||||
store.serviceAccountAccessKeys = make(map[string]string)
|
||||
store.policies = make(map[string]policy_engine.PolicyDocument)
|
||||
store.inlinePolicies = make(map[string]map[string]policy_engine.PolicyDocument)
|
||||
store.groups = make(map[string]*iam_pb.Group)
|
||||
store.initialized = true
|
||||
|
||||
@@ -59,6 +61,7 @@ func (store *MemoryStore) Shutdown() {
|
||||
store.serviceAccounts = nil
|
||||
store.serviceAccountAccessKeys = nil
|
||||
store.policies = nil
|
||||
store.inlinePolicies = nil
|
||||
store.groups = nil
|
||||
store.initialized = false
|
||||
}
|
||||
@@ -74,6 +77,7 @@ func (store *MemoryStore) Reset() {
|
||||
store.serviceAccounts = make(map[string]*iam_pb.ServiceAccount)
|
||||
store.serviceAccountAccessKeys = make(map[string]string)
|
||||
store.policies = make(map[string]policy_engine.PolicyDocument)
|
||||
store.inlinePolicies = make(map[string]map[string]policy_engine.PolicyDocument)
|
||||
store.groups = make(map[string]*iam_pb.Group)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -280,6 +280,34 @@ func (s *PropagatingCredentialStore) LoadInlinePolicies(ctx context.Context) (ma
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
func (s *PropagatingCredentialStore) PutUserInlinePolicy(ctx context.Context, userName, policyName string, document policy_engine.PolicyDocument) error {
|
||||
if store, ok := s.CredentialStore.(InlinePolicyStore); ok {
|
||||
return store.PutUserInlinePolicy(ctx, userName, policyName, document)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (s *PropagatingCredentialStore) GetUserInlinePolicy(ctx context.Context, userName, policyName string) (*policy_engine.PolicyDocument, error) {
|
||||
if store, ok := s.CredentialStore.(InlinePolicyStore); ok {
|
||||
return store.GetUserInlinePolicy(ctx, userName, policyName)
|
||||
}
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
func (s *PropagatingCredentialStore) DeleteUserInlinePolicy(ctx context.Context, userName, policyName string) error {
|
||||
if store, ok := s.CredentialStore.(InlinePolicyStore); ok {
|
||||
return store.DeleteUserInlinePolicy(ctx, userName, policyName)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (s *PropagatingCredentialStore) ListUserInlinePolicies(ctx context.Context, userName string) ([]string, error) {
|
||||
if store, ok := s.CredentialStore.(InlinePolicyStore); ok {
|
||||
return store.ListUserInlinePolicies(ctx, userName)
|
||||
}
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
func (s *PropagatingCredentialStore) CreatePolicy(ctx context.Context, name string, document policy_engine.PolicyDocument) error {
|
||||
if pm, ok := s.CredentialStore.(PolicyManager); ok {
|
||||
if err := pm.CreatePolicy(ctx, name, document); err != nil {
|
||||
|
||||
Reference in new issue
Block a user