diff --git a/weed/admin/dash/bucket_management.go b/weed/admin/dash/bucket_management.go index 07e649a50..a799964d0 100644 --- a/weed/admin/dash/bucket_management.go +++ b/weed/admin/dash/bucket_management.go @@ -267,16 +267,22 @@ func (s *AdminServer) ShowBucketPolicy(w http.ResponseWriter, r *http.Request) { return } - policy, err := s.GetBucketPolicy(bucketName) + policy, raw, err := s.GetBucketPolicy(bucketName) if err != nil { writeJSONError(w, bucketPolicyErrorStatus(err), "Failed to get bucket policy: "+err.Error()) return } - writeJSON(w, http.StatusOK, map[string]interface{}{ + resp := map[string]interface{}{ "bucket": bucketName, "policy": policy, - }) + } + if policy == nil && len(raw) > 0 { + // Stored bytes the decoder rejects: hand them to the JSON tab so + // the operator can fix or delete the document. + resp["policy_text"] = string(raw) + } + writeJSON(w, http.StatusOK, resp) } // UpdateBucketPolicy replaces the bucket policy for a bucket. diff --git a/weed/admin/dash/bucket_policy.go b/weed/admin/dash/bucket_policy.go index 0a860f919..31a14f614 100644 --- a/weed/admin/dash/bucket_policy.go +++ b/weed/admin/dash/bucket_policy.go @@ -12,28 +12,26 @@ import ( "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" ) -// MaxBucketPolicySize mirrors AWS S3's 20 KB bucket-policy limit. This is an -// admin-side cap the S3 gateway does not itself enforce; it can only reject -// a policy the gateway would have accepted, never disagree about one -// already stored, so it cannot desync admin and S3 behavior. -const MaxBucketPolicySize = 20 * 1024 - // ErrInvalidBucketPolicy wraps a validation failure from SetBucketPolicy so // callers (the HTTP handler) can map it to 400 instead of 500 without // resorting to matching on the error string. var ErrInvalidBucketPolicy = errors.New("invalid bucket policy") // GetBucketPolicy returns the policy document stored on a bucket's filer -// entry, or (nil, nil) if the bucket has no policy — that is not an error, -// it just means the caller (e.g. the admin UI) should show an empty editor -// instead of special-casing a 404. -func (s *AdminServer) GetBucketPolicy(bucketName string) (*policy_engine.PolicyDocument, error) { +// entry, or (nil, nil, nil) if the bucket has no policy — that is not an +// error, it just means the caller (e.g. the admin UI) should show an empty +// editor instead of special-casing a 404. Stored bytes the current decoder +// rejects come back as (nil, raw, nil): a 500 here would leave the UI +// unable to show, fix, or even delete the one policy an operator most +// needs to remove — and DeleteBucketPolicy never reads the document. +func (s *AdminServer) GetBucketPolicy(bucketName string) (*policy_engine.PolicyDocument, []byte, error) { filerConfig, err := s.getFilerConfig() if err != nil { - return nil, fmt.Errorf("get filer configuration: %w", err) + return nil, nil, fmt.Errorf("get filer configuration: %w", err) } var doc *policy_engine.PolicyDocument + var raw []byte err = s.WithFilerClient(func(client filer_pb.SeaweedFilerClient) error { resp, err := filer_pb.LookupEntry(context.Background(), client, &filer_pb.LookupDirectoryEntryRequest{ Directory: filerConfig.BucketsPath, @@ -50,19 +48,19 @@ func (s *AdminServer) GetBucketPolicy(bucketName string) (*policy_engine.PolicyD if len(policyJSON) == 0 { return nil } + raw = policyJSON var parsed policy_engine.PolicyDocument - if err := json.Unmarshal(policyJSON, &parsed); err != nil { - return fmt.Errorf("parse stored bucket policy: %w", err) + if err := json.Unmarshal(policyJSON, &parsed); err == nil { + doc = &parsed } - doc = &parsed return nil }) if err != nil { - return nil, err + return nil, nil, err } - return doc, nil + return doc, raw, nil } // SetBucketPolicy validates and stores a bucket policy, applying the exact @@ -72,18 +70,11 @@ func (s *AdminServer) GetBucketPolicy(bucketName string) (*policy_engine.PolicyD // // Propagation to every S3 gateway is automatic: writing the // s3-bucket-policy extended attribute drives the filer metadata log, which -// each gateway's onBucketMetadataChange subscription already watches to -// rebuild its bucket policy cache. No separate notify step is needed here. -// -// Note: PutBucketPolicyHandler on the S3 gateway also mirrors the policy -// into the IAM policy store under "bucket-policy:" -// (iam_manager.go's UpdateBucketPolicy), but its delete counterpart -// (removeBucketPolicyFromIAM) is an unimplemented TODO — so that mirror is -// already unreliable after any S3-side DeleteBucketPolicy. The admin write -// path deliberately does not replicate it: doing so would only deepen an -// existing inconsistency, and admin has no handle on the S3 gateway's -// iamManager anyway. See the TODO in -// weed/s3api/s3api_bucket_policy_handlers.go for the follow-up. +// each gateway's onBucketMetadataChange subscription watches to rebuild its +// bucket policy cache and to maintain the advanced-IAM +// "bucket-policy:" mirror (mirrorBucketPolicyToIAM in +// weed/s3api/s3api_bucket_policy_handlers.go). No separate notify step is +// needed here. func (s *AdminServer) SetBucketPolicy(bucketName string, doc *policy_engine.PolicyDocument) error { if err := policy_engine.ValidatePolicy(doc); err != nil { return fmt.Errorf("%w: %w", ErrInvalidBucketPolicy, err) @@ -96,10 +87,24 @@ func (s *AdminServer) SetBucketPolicy(bucketName string, doc *policy_engine.Poli if err != nil { return fmt.Errorf("marshal policy document: %w", err) } - if len(policyJSON) > MaxBucketPolicySize { - return fmt.Errorf("%w: bucket policy is %d bytes, which exceeds the %d byte limit", ErrInvalidBucketPolicy, len(policyJSON), MaxBucketPolicySize) + if len(policyJSON) > policy_engine.MaxBucketPolicySize { + return fmt.Errorf("%w: bucket policy is %d bytes, which exceeds the %d byte limit", ErrInvalidBucketPolicy, len(policyJSON), policy_engine.MaxBucketPolicySize) } + return s.writeBucketPolicy(bucketName, policyJSON) +} + +// DeleteBucketPolicy clears the bucket policy stored on a bucket's filer +// entry. Deleting a policy that doesn't exist is a success, matching +// DeleteBucketLifecycle's idempotent behavior. This cannot go through +// SetBucketPolicy: validation there rejects a nil document. +func (s *AdminServer) DeleteBucketPolicy(bucketName string) error { + return s.writeBucketPolicy(bucketName, nil) +} + +// writeBucketPolicy patches the policy key on the bucket's filer entry; a +// nil policyJSON clears it. +func (s *AdminServer) writeBucketPolicy(bucketName string, policyJSON []byte) error { filerConfig, err := s.getFilerConfig() if err != nil { return fmt.Errorf("get filer configuration: %w", err) @@ -125,46 +130,10 @@ func (s *AdminServer) SetBucketPolicy(bucketName string, doc *policy_engine.Poli Mutations: []*filer_pb.ObjectMutation{bucketPolicyMutation(filerConfig.BucketsPath, bucketName, policyJSON)}, }) if err != nil { - return fmt.Errorf("failed to update bucket policy: %w", err) + return fmt.Errorf("write bucket policy: %w", err) } if resp.Error != "" { - return fmt.Errorf("failed to update bucket policy: %s", resp.Error) - } - return nil - }) -} - -// DeleteBucketPolicy clears the bucket policy stored on a bucket's filer -// entry. Deleting a policy that doesn't exist is a success, matching -// DeleteBucketLifecycle's idempotent behavior. -func (s *AdminServer) DeleteBucketPolicy(bucketName string) error { - filerConfig, err := s.getFilerConfig() - if err != nil { - return fmt.Errorf("get filer configuration: %w", err) - } - - return s.WithFilerClient(func(client filer_pb.SeaweedFilerClient) error { - if _, err := filer_pb.LookupEntry(context.Background(), client, &filer_pb.LookupDirectoryEntryRequest{ - Directory: filerConfig.BucketsPath, - Name: bucketName, - }); err != nil { - if errors.Is(err, filer_pb.ErrNotFound) { - return fmt.Errorf("%w: %s", ErrBucketNotFound, bucketName) - } - return fmt.Errorf("look up bucket %s: %w", bucketName, err) - } - - bucketPath := filerConfig.BucketsPath + "/" + bucketName - resp, err := client.ObjectTransaction(context.Background(), &filer_pb.ObjectTransactionRequest{ - LockKey: bucketPath, - RouteKey: s3_constants.ObjectWriteRouteKeyPrefix + bucketPath, - Mutations: []*filer_pb.ObjectMutation{bucketPolicyMutation(filerConfig.BucketsPath, bucketName, nil)}, - }) - if err != nil { - return fmt.Errorf("failed to delete bucket policy: %w", err) - } - if resp.Error != "" { - return fmt.Errorf("failed to delete bucket policy: %s", resp.Error) + return fmt.Errorf("write bucket policy: %s", resp.Error) } return nil }) diff --git a/weed/admin/dash/bucket_policy_test.go b/weed/admin/dash/bucket_policy_test.go index df8c19ce3..422c35294 100644 --- a/weed/admin/dash/bucket_policy_test.go +++ b/weed/admin/dash/bucket_policy_test.go @@ -128,7 +128,7 @@ func TestSetBucketPolicy_RejectsOversized(t *testing.T) { // A resource list long enough to blow the cap: this must fail before // any filer call, which is what makes it testable without one. doc := validBucketPolicyDoc("mybucket") - doc.Statement[0].Sid = strings.Repeat("x", MaxBucketPolicySize+1) + doc.Statement[0].Sid = strings.Repeat("x", policy_engine.MaxBucketPolicySize+1) err := (&AdminServer{}).SetBucketPolicy("mybucket", doc) if err == nil { diff --git a/weed/admin/dash/s3tables_management.go b/weed/admin/dash/s3tables_management.go index 78c35bb03..0db9e81d0 100644 --- a/weed/admin/dash/s3tables_management.go +++ b/weed/admin/dash/s3tables_management.go @@ -1043,8 +1043,11 @@ func (s *AdminServer) GetS3TablesBucketPolicy(w http.ResponseWriter, r *http.Req getReq := &s3tables.GetTableBucketPolicyRequest{TableBucketARN: bucketArn} var resp s3tables.GetTableBucketPolicyResponse if err := s.executeS3TablesOperation(r.Context(), "GetTableBucketPolicy", getReq, &resp); err != nil { - writeS3TablesError(w, err) - return + // No policy is a normal state for the UI (empty editor), not an error. + if !isS3TablesNoSuchPolicy(err) { + writeS3TablesError(w, err) + return + } } writeJSON(w, http.StatusOK, map[string]interface{}{"policy": resp.ResourcePolicy}) } @@ -1111,8 +1114,10 @@ func (s *AdminServer) GetS3TablesTablePolicy(w http.ResponseWriter, r *http.Requ getReq := &s3tables.GetTablePolicyRequest{TableBucketARN: bucketArn, Namespace: namespaceParts, Name: name} var resp s3tables.GetTablePolicyResponse if err := s.executeS3TablesOperation(r.Context(), "GetTablePolicy", getReq, &resp); err != nil { - writeS3TablesError(w, err) - return + if !isS3TablesNoSuchPolicy(err) { + writeS3TablesError(w, err) + return + } } writeJSON(w, http.StatusOK, map[string]interface{}{"policy": resp.ResourcePolicy}) } @@ -1225,6 +1230,11 @@ func writeS3TablesError(w http.ResponseWriter, err error) { writeJSONError(w, s3TablesErrorStatus(err), parseS3TablesErrorMessage(err)) } +func isS3TablesNoSuchPolicy(err error) bool { + var s3Err *s3tables.S3TablesError + return errors.As(err, &s3Err) && s3Err.Type == s3tables.ErrCodeNoSuchPolicy +} + func s3TablesErrorStatus(err error) int { var s3Err *s3tables.S3TablesError if errors.As(err, &s3Err) { diff --git a/weed/admin/static/js/policy_editor.js b/weed/admin/static/js/policy_editor.js index 0f914ed77..015551e80 100644 --- a/weed/admin/static/js/policy_editor.js +++ b/weed/admin/static/js/policy_editor.js @@ -35,15 +35,24 @@ const POLICY_EDITOR_CONFIG = {}; // resourceDatalistId - id of the shared resource-suggestions . Default: 'policyResourceSuggestions'. // principalDatalistId - id of the shared principal-suggestions . Default: 'policyPrincipalSuggestions'. // requirePrincipal - if true, a new statement seeds Principal with '*' -// instead of leaving it empty. Bucket policies -// require a Principal per statement; IAM policies -// don't. The server remains the source of truth for -// this rule either way - see -// policy_engine.ValidateBucketPolicy. +// instead of leaving it empty, and the NotPrincipal +// mode is not offered (the bucket policy backend +// rejects statements without a Principal). The +// server remains the source of truth for this rule +// either way - see policy_engine.ValidateBucketPolicy. +// allowNegation - if false, the NotResource/NotPrincipal mode +// dropdowns are not offered and a loaded document +// using them falls back to the JSON tab. For +// backends whose evaluator has no Not* fields +// (s3tables silently drops them, turning e.g. +// Allow+NotResource into allow-everything). // bucket - if set, the Resource autocomplete only offers // this bucket's ARN and ARN/*, instead of fetching // every bucket, and a new statement's Resource is // seeded with arn:aws:s3:::/*. +// resourceSuggestions - if set, the Resource autocomplete offers exactly +// these values and never fetches bucket/folder +// names. For non-S3 ARN dialects (s3tables). function registerPolicyEditor(which, config) { POLICY_EDITOR_CONFIG[which] = Object.assign({ textareaId: which + 'PolicyDocument', @@ -55,7 +64,9 @@ function registerPolicyEditor(which, config) { resourceDatalistId: 'policyResourceSuggestions', principalDatalistId: 'policyPrincipalSuggestions', requirePrincipal: false, - bucket: null + allowNegation: true, + bucket: null, + resourceSuggestions: null }, config || {}); } @@ -104,16 +115,43 @@ function policyEditorConfig(which) { return policyEditorConfig(which).textareaId; } + function policyEditorOffersNotPrincipal(which) { + const cfg = policyEditorConfig(which); + return cfg.allowNegation && !cfg.requirePrincipal; + } + + // True when the serialized policy carries at least one statement. Guards + // Save in the consumers: an empty editor commits as {"Statement":[]}, + // which is never what a user wants stored - on backends with no + // server-side validation (s3tables) it evaluates default-deny for every + // non-owner while the UI still shows "Not configured". + function policyTextHasStatements(text) { + try { + const stmts = (JSON.parse(text) || {}).Statement; + return Array.isArray(stmts) ? stmts.length > 0 : !!stmts; + } catch (e) { + return true; + } + } + function policyEditorBodyId(which) { return policyEditorConfig(which).editorBodyId; } function normalizeToStringArray(value) { if (value === undefined || value === null) return []; - // Coerce: these feed escapeHtml, which calls text.replace, and a - // policy is free to carry a number or a boolean here. - if (Array.isArray(value)) return value.map(String); - return [String(value)]; + const values = Array.isArray(value) ? value : [value]; + return values.map(function(v) { + if (v !== null && typeof v === 'object') { + // String(v) would render '[object Object]' (or comma-join a + // nested array) and a later save would persist that coercion; + // send the document to the JSON tab instead. + throw new Error('Policy list entries must be strings (got ' + JSON.stringify(v) + ')'); + } + // Coerce scalars: these feed escapeHtml, which calls text.replace, + // and a policy is free to carry a number or a boolean here. + return String(v); + }); } const POLICY_DOCUMENT_KNOWN_KEYS = ['Version', 'Statement']; @@ -126,21 +164,21 @@ function policyEditorConfig(which) { // tab. The admin API's PolicyDocument only carries Version and Statement, // so such fields are still dropped by the server on save; see // confirmPolicyFieldDiscard, which warns the user before that happens. - function policyDocToEditorState(doc) { - if (doc === null || typeof doc !== 'object') { - // null or a bare scalar (string/number/boolean) can't represent a + function policyDocToEditorState(which, doc) { + const cfg = policyEditorConfig(which); + if (doc === null || typeof doc !== 'object' || Array.isArray(doc)) { + // null, a bare scalar, or an array (typeof [] is 'object', and a + // pasted statement array is a common mistake) can't represent a // policy document; treating it as "zero statements" would hide // from the user that their input wasn't actually a document. throw new Error('Policy document must be a JSON object (got ' + JSON.stringify(doc) + ')'); } const state = { version: doc.Version || '2012-10-17', statements: [], otherFields: {} }; - if (!Array.isArray(doc)) { - Object.keys(doc).forEach(function(key) { - if (POLICY_DOCUMENT_KNOWN_KEYS.indexOf(key) === -1) { - state.otherFields[key] = doc[key]; - } - }); - } + Object.keys(doc).forEach(function(key) { + if (POLICY_DOCUMENT_KNOWN_KEYS.indexOf(key) === -1) { + state.otherFields[key] = doc[key]; + } + }); const rawStatements = doc && doc.Statement ? (Array.isArray(doc.Statement) ? doc.Statement : [doc.Statement]) : []; @@ -161,6 +199,9 @@ function policyEditorConfig(which) { // it in the JSON tab rather than silently picking one. throw new Error('Statement ' + (idx + 1) + ': cannot specify both Resource and NotResource'); } + if (hasNotResource && !cfg.allowNegation) { + throw new Error('Statement ' + (idx + 1) + ': NotResource is not supported for this policy type'); + } const resourceMode = hasNotResource ? 'NotResource' : 'Resource'; const hasPrincipal = Object.prototype.hasOwnProperty.call(stmt, 'Principal'); @@ -168,6 +209,9 @@ function policyEditorConfig(which) { if (hasPrincipal && hasNotPrincipal) { throw new Error('Statement ' + (idx + 1) + ': cannot specify both Principal and NotPrincipal'); } + if (hasNotPrincipal && (!cfg.allowNegation || cfg.requirePrincipal)) { + throw new Error('Statement ' + (idx + 1) + ': NotPrincipal is not supported for this policy type'); + } let principalMode = 'Principal'; let principalValues = []; let principalManaged = false; @@ -221,7 +265,12 @@ function policyEditorConfig(which) { if (!value || typeof value !== 'object' || Array.isArray(value)) return null; const keys = Object.keys(value); if (keys.length !== 1 || keys[0] !== 'AWS') return null; - return normalizeToStringArray(value.AWS); + try { + return normalizeToStringArray(value.AWS); + } catch (e) { + // Non-string entries: not the simple form; fall back to extras. + return null; + } } // Converts editor state back into a policy document. Structured fields @@ -365,24 +414,28 @@ function policyEditorConfig(which) { '' + '' + '
' + - '' + - '' + + '' + + (policyEditorConfig(which).allowNegation + ? '' + : 'Resource') + '' + (stmt.resourceMode === 'NotResource' ? '
The statement applies to every resource except the ones listed.
' : '') + '
' + resourceRows + '
' + '' + '
' + '
' + - '' + - '' + + '' + + (policyEditorOffersNotPrincipal(which) + ? '' + : 'Principal') + '' + - '
Principal / NotPrincipal (AWS account/user ARN, or "*" for everyone). Only the AWS type and "*" are supported here; other forms stay editable via Advanced fields.
' + + '
' + (policyEditorOffersNotPrincipal(which) ? 'Principal / NotPrincipal' : 'Principal') + ' (AWS account/user ARN, or "*" for everyone). Only the AWS type and "*" are supported here; other forms stay editable via Advanced fields.
' + (stmt.hasComplexPrincipal ? '
This statement\'s Principal/NotPrincipal uses a form not supported by this field — see Advanced fields below.
' : '') + '
' + principalRows + '
' + '' + @@ -474,7 +527,7 @@ function policyEditorConfig(which) { } let newState; try { - newState = policyDocToEditorState(doc); + newState = policyDocToEditorState(which, doc); } catch (e) { showAlert(e.message, 'error'); return false; @@ -522,7 +575,7 @@ function policyEditorConfig(which) { } let state; try { - state = policyDocToEditorState(doc); + state = policyDocToEditorState(which, doc); } catch (e) { policyEditors[which] = { version: '2012-10-17', statements: [], otherFields: {}, unparsed: true }; renderPolicyEditor(which); @@ -543,9 +596,13 @@ function policyEditorConfig(which) { } const cfg = policyEditorConfig(which); commitPolicyEditorForm(which); + // Seed Resource with the broadest pinned suggestion, mirroring the + // arn:aws:s3::: seeding cfg.bucket gets. + const seededResources = cfg.bucket ? ['arn:aws:s3:::' + cfg.bucket + '/*'] + : (cfg.resourceSuggestions ? [cfg.resourceSuggestions[cfg.resourceSuggestions.length - 1]] : []); policyEditorState(which).statements.push({ sid: '', effect: 'Allow', actions: [], - resourceMode: 'Resource', resources: cfg.bucket ? ['arn:aws:s3:::' + cfg.bucket + '/*'] : [], + resourceMode: 'Resource', resources: seededResources, principalMode: 'Principal', principalValues: cfg.requirePrincipal ? ['*'] : [], hasComplexPrincipal: false, extras: '' }); @@ -569,21 +626,74 @@ function policyEditorConfig(which) { // The JSON tab is the source of truth right now; parse it back // into the structured editor to keep both in sync, but leave the // textarea's own text untouched. - return commitPolicyTextareaToEditor(which); + const text = document.getElementById(policyTextareaId(which)).value; + if (text && text.trim()) { + let doc; + try { + doc = JSON.parse(text); + } catch (e) { + showAlert('Invalid JSON in policy document: ' + e.message, 'error'); + return false; + } + try { + policyEditors[which] = policyDocToEditorState(which, doc); + } catch (e) { + // Valid JSON the structured editor can't model is still + // saveable from here - exactly the documents the load path + // shunts to this tab. It just stays JSON-tab-only. + policyEditors[which] = { version: '2012-10-17', statements: [], otherFields: {}, unparsed: true }; + } + renderPolicyEditor(which); + } else if (!commitPolicyTextareaToEditor(which)) { + return false; + } + } else { + if (policyEditorState(which).unparsed) { + // The editor never held this document, so serializing it would + // write an empty policy over whatever is in the JSON tab. + showAlert(POLICY_JSON_TAB_ONLY_MESSAGE, 'error'); + return false; + } + try { + commitPolicyEditorToTextarea(which); + } catch (e) { + showAlert(e.message, 'error'); + return false; + } } - if (policyEditorState(which).unparsed) { - // The editor never held this document, so serializing it would - // write an empty policy over whatever is in the JSON tab. - showAlert(POLICY_JSON_TAB_ONLY_MESSAGE, 'error'); + // Final gate over what would actually be saved. The mode dropdowns + // being hidden is not enough: the JSON tab and a statement's + // Advanced-fields box can both carry Not* keys, and where negation + // is disallowed the backend's evaluator silently drops them - + // Allow+NotResource would come back as allow-everything. + const negationError = policyTextDisallowedNegationError(which); + if (negationError) { + showAlert(negationError, 'error'); return false; } + return true; + } + + // Returns an error message if the JSON textarea for `which` holds a + // statement using NotResource/NotPrincipal while the instance disallows + // negation, or null. Unparseable/empty text is left to other checks. + function policyTextDisallowedNegationError(which) { + if (policyEditorConfig(which).allowNegation) return null; + let doc; try { - commitPolicyEditorToTextarea(which); - return true; + doc = JSON.parse(document.getElementById(policyTextareaId(which)).value); } catch (e) { - showAlert(e.message, 'error'); - return false; + return null; } + const stmts = doc && doc.Statement ? (Array.isArray(doc.Statement) ? doc.Statement : [doc.Statement]) : []; + for (let i = 0; i < stmts.length; i++) { + const stmt = stmts[i] || {}; + if (Object.prototype.hasOwnProperty.call(stmt, 'NotResource') || + Object.prototype.hasOwnProperty.call(stmt, 'NotPrincipal')) { + return 'Statement ' + (i + 1) + ': NotResource/NotPrincipal are not supported for this policy type and would be silently ignored. Remove them before saving.'; + } + } + return null; } // The admin API's policy document carries only Version and Statement, so @@ -609,8 +719,11 @@ function policyEditorConfig(which) { const statements = (doc && doc.Statement) || []; for (let i = 0; i < statements.length; i++) { const stmt = statements[i] || {}; - if (stmt.Principal === undefined && stmt.NotPrincipal === undefined) { - return 'Statement ' + (i + 1) + ': a Principal (or NotPrincipal) is required.'; + // Principal specifically: the server rule this front-runs + // (policy_engine.ValidateBucketPolicy) rejects NotPrincipal-only + // statements too. + if (stmt.Principal === undefined) { + return 'Statement ' + (i + 1) + ': a Principal is required.'; } } return null; @@ -805,6 +918,10 @@ function policyEditorConfig(which) { const cfg = policyEditorConfig(which); const datalist = document.getElementById(cfg.resourceDatalistId); if (!datalist) return; + if (cfg.resourceSuggestions) { + renderPolicyDatalistOptions(datalist, cfg.resourceSuggestions); + return; + } const state = policyResourcePathState(inputEl.value); if (state.stage === 'bucket') { @@ -880,7 +997,7 @@ function policyEditorConfig(which) { function insertSamplePolicy(which, sampleDoc) { const doc = sampleDoc || POLICY_SAMPLE_DOCUMENT; - policyEditors[which] = policyDocToEditorState(doc); + policyEditors[which] = policyDocToEditorState(which, doc); renderPolicyEditor(which); document.getElementById(policyTextareaId(which)).value = JSON.stringify(doc, null, 2); } diff --git a/weed/admin/static/js/s3tables.js b/weed/admin/static/js/s3tables.js index f143a71da..182f949fe 100644 --- a/weed/admin/static/js/s3tables.js +++ b/weed/admin/static/js/s3tables.js @@ -32,6 +32,41 @@ let s3tablesTablePolicyLoaded = false; let s3tablesBucketPolicyRequestSeq = 0; let s3tablesTablePolicyRequestSeq = 0; +// True while a policy PUT/DELETE is in flight, so a double-click - or Save +// and Delete fired in quick succession - can't send overlapping mutations. +// Same pattern as the classic bucket modal in s3_buckets.templ. +let s3tablesBucketPolicyMutationInFlight = false; +let s3tablesTablePolicyMutationInFlight = false; + +function setS3TablesBucketPolicyMutationInFlight(inFlight) { + s3tablesBucketPolicyMutationInFlight = inFlight; + const save = document.getElementById('s3tablesBucketPolicySaveBtn'); + const del = document.getElementById('s3tablesBucketPolicyDeleteBtn'); + if (save) save.disabled = inFlight; + if (del) del.disabled = inFlight; +} + +function setS3TablesTablePolicyMutationInFlight(inFlight) { + s3tablesTablePolicyMutationInFlight = inFlight; + const save = document.getElementById('s3tablesTablePolicySaveBtn'); + const del = document.getElementById('s3tablesTablePolicyDeleteBtn'); + if (save) save.disabled = inFlight; + if (del) del.disabled = inFlight; +} + +// The dialog identity captured when a mutation started, so a completion +// that lands after the shared modal moved on to a different resource can't +// alert against, hide, or reload over that other resource's state. +function currentS3TablesBucketPolicyTarget() { + return document.getElementById('s3tablesBucketPolicyArn').value; +} + +function currentS3TablesTablePolicyTarget() { + return document.getElementById('s3tablesTablePolicyBucketArn').value + '\n' + + document.getElementById('s3tablesTablePolicyNamespace').value + '\n' + + document.getElementById('s3tablesTablePolicyName').value; +} + function getCSRFToken() { const tokenMeta = document.querySelector('meta[name="csrf-token"]'); if (!tokenMeta) { @@ -57,13 +92,7 @@ function initS3TablesBuckets() { s3tablesBucketPolicyModal = new bootstrap.Modal(document.getElementById('s3tablesBucketPolicyModal')); s3tablesTagsModal = new bootstrap.Modal(document.getElementById('s3tablesTagsModal')); - // Shared visual policy editor (weed/admin/static/js/policy_editor.js), - // reused here from the bucket policy admin page. Table bucket policies - // aren't validated against policy_engine.PolicyDocument server-side - // (see s3tables/permissions.go's separate PolicyDocument type), so no - // requirePrincipal/bucket config is set - the editor just gives a - // structured view over the same JSON the JSON tab holds. - registerPolicyEditor('s3tablesBucket', { textareaId: 's3tablesBucketPolicyText' }); + registerS3TablesBucketPolicyEditor(''); setupPolicyEditor('s3tablesBucket'); const ownerSelect = document.getElementById('s3tablesBucketOwner'); @@ -107,6 +136,7 @@ function initS3TablesBuckets() { button.addEventListener('click', function () { const bucketArn = this.dataset.bucketArn || ''; document.getElementById('s3tablesBucketPolicyArn').value = bucketArn; + registerS3TablesBucketPolicyEditor(bucketArn); loadS3TablesBucketPolicy(bucketArn); s3tablesBucketPolicyModal.show(); }); @@ -203,36 +233,11 @@ function initS3TablesBuckets() { const policyForm = document.getElementById('s3tablesBucketPolicyForm'); if (policyForm) { - policyForm.addEventListener('submit', async function (e) { - e.preventDefault(); - if (!s3tablesBucketPolicyLoaded) { - alert('The current policy has not finished loading. Close and reopen this dialog before saving.'); - return; - } - if (!commitPolicyActiveTab('s3tablesBucket')) return; - const bucketArn = document.getElementById('s3tablesBucketPolicyArn').value; - const policy = document.getElementById('s3tablesBucketPolicyText').value.trim(); - if (!policy) { - alert('Policy JSON is required'); - return; - } - try { - const response = await fetch(s3tBasePath('/api/s3tables/bucket-policy'), { - method: 'PUT', - headers: s3tWriteHeaders({ 'Content-Type': 'application/json' }), - body: JSON.stringify({ bucket_arn: bucketArn, policy: policy }) - }); - const data = await response.json(); - if (!response.ok) { - alert(data.error || 'Failed to update policy'); - return; - } - alert('Policy updated'); - s3tablesBucketPolicyModal.hide(); - } catch (error) { - alert('Failed to update policy: ' + error.message); - } - }); + // Saves go through the Save button only; implicit form submission + // (Enter in a single-line editor input) must never PUT half-built + // state, which the backend would store verbatim. + policyForm.addEventListener('submit', function (e) { e.preventDefault(); }); + document.getElementById('s3tablesBucketPolicySaveBtn').addEventListener('click', saveS3TablesBucketPolicy); } const tagsForm = document.getElementById('s3tablesTagsForm'); @@ -258,7 +263,7 @@ function initS3TablesTables() { s3tablesTablePolicyModal = new bootstrap.Modal(document.getElementById('s3tablesTablePolicyModal')); s3tablesTagsModal = new bootstrap.Modal(document.getElementById('s3tablesTagsModal')); - registerPolicyEditor('s3tablesTable', { textareaId: 's3tablesTablePolicyText' }); + registerS3TablesTablePolicyEditor('', '', ''); setupPolicyEditor('s3tablesTable'); const dataContainer = document.getElementById('s3tables-tables-content'); @@ -278,6 +283,7 @@ function initS3TablesTables() { document.getElementById('s3tablesTablePolicyBucketArn').value = dataBucketArn; document.getElementById('s3tablesTablePolicyNamespace').value = dataNamespace; document.getElementById('s3tablesTablePolicyName').value = this.dataset.tableName || ''; + registerS3TablesTablePolicyEditor(dataBucketArn, dataNamespace, this.dataset.tableName || ''); loadS3TablesTablePolicy(dataBucketArn, dataNamespace, this.dataset.tableName || ''); s3tablesTablePolicyModal.show(); }); @@ -346,35 +352,9 @@ function initS3TablesTables() { const policyForm = document.getElementById('s3tablesTablePolicyForm'); if (policyForm) { - policyForm.addEventListener('submit', async function (e) { - e.preventDefault(); - if (!s3tablesTablePolicyLoaded) { - alert('The current policy has not finished loading. Close and reopen this dialog before saving.'); - return; - } - if (!commitPolicyActiveTab('s3tablesTable')) return; - const policy = document.getElementById('s3tablesTablePolicyText').value.trim(); - if (!policy) { - alert('Policy JSON is required'); - return; - } - try { - const response = await fetch(s3tBasePath('/api/s3tables/table-policy'), { - method: 'PUT', - headers: s3tWriteHeaders({ 'Content-Type': 'application/json' }), - body: JSON.stringify({ bucket_arn: dataBucketArn, namespace: dataNamespace, name: document.getElementById('s3tablesTablePolicyName').value, policy: policy }) - }); - const data = await response.json(); - if (!response.ok) { - alert(data.error || 'Failed to update policy'); - return; - } - alert('Policy updated'); - s3tablesTablePolicyModal.hide(); - } catch (error) { - alert('Failed to update policy: ' + error.message); - } - }); + // Same Enter-must-not-submit rule as the bucket policy form. + policyForm.addEventListener('submit', function (e) { e.preventDefault(); }); + document.getElementById('s3tablesTablePolicySaveBtn').addEventListener('click', saveS3TablesTablePolicy); } const tagsForm = document.getElementById('s3tablesTagsForm'); @@ -619,6 +599,30 @@ async function deleteS3TablesBucket() { } } +// Shared visual policy editor (weed/admin/static/js/policy_editor.js), +// configured for the S3 Tables policy engine: only s3tables: action +// suggestions, resource suggestions pinned to the open resource's ARN, and +// no NotResource/NotPrincipal modes - the s3tables evaluator has no such +// fields and would silently drop them (see s3tables/permissions.go). +// Re-registered on every dialog open so the suggestions track the resource. +function registerS3TablesBucketPolicyEditor(bucketArn) { + registerPolicyEditor('s3tablesBucket', { + textareaId: 's3tablesBucketPolicyText', + actionDatalistId: 's3tablesPolicyActionSuggestions', + allowNegation: false, + resourceSuggestions: bucketArn ? [bucketArn, bucketArn + '/table/*'] : null + }); +} + +function registerS3TablesTablePolicyEditor(bucketArn, namespace, name) { + registerPolicyEditor('s3tablesTable', { + textareaId: 's3tablesTablePolicyText', + actionDatalistId: 's3tablesPolicyActionSuggestions', + allowNegation: false, + resourceSuggestions: bucketArn && namespace && name ? [bucketArn + '/table/' + namespace + '/' + name] : null + }); +} + async function loadS3TablesBucketPolicy(bucketArn) { const requestSeq = ++s3tablesBucketPolicyRequestSeq; document.getElementById('s3tablesBucketPolicyText').value = ''; @@ -658,25 +662,133 @@ async function loadS3TablesBucketPolicy(bucketArn) { loadPolicyTextareaIntoEditor('s3tablesBucket'); } +async function saveS3TablesBucketPolicy() { + if (s3tablesBucketPolicyMutationInFlight) return; + if (!s3tablesBucketPolicyLoaded) { + alert('The current policy has not finished loading. Close and reopen this dialog before saving.'); + return; + } + if (!commitPolicyActiveTab('s3tablesBucket')) return; + const bucketArn = currentS3TablesBucketPolicyTarget(); + const policy = document.getElementById('s3tablesBucketPolicyText').value.trim(); + if (!policy || !policyTextHasStatements(policy)) { + alert('Add at least one statement, or use Delete Policy to remove the policy.'); + return; + } + setS3TablesBucketPolicyMutationInFlight(true); + try { + const response = await fetch(s3tBasePath('/api/s3tables/bucket-policy'), { + method: 'PUT', + headers: s3tWriteHeaders({ 'Content-Type': 'application/json' }), + body: JSON.stringify({ bucket_arn: bucketArn, policy: policy }) + }); + const data = await response.json(); + setS3TablesBucketPolicyMutationInFlight(false); + const stillCurrent = bucketArn === currentS3TablesBucketPolicyTarget(); + if (!response.ok) { + if (stillCurrent) { + alert(data.error || 'Failed to update policy'); + } else { + console.error('Error saving policy for ' + bucketArn + ' (no longer the open resource): ' + (data.error || 'unknown error')); + } + return; + } + if (!stillCurrent) return; + s3tablesBucketPolicyModal.hide(); + // Reload so the Policy column reflects the change. + setTimeout(() => location.reload(), 500); + } catch (error) { + setS3TablesBucketPolicyMutationInFlight(false); + if (bucketArn === currentS3TablesBucketPolicyTarget()) { + alert('Failed to update policy: ' + error.message); + } else { + console.error('Error saving policy for ' + bucketArn + ': ' + error.message); + } + } +} + +async function saveS3TablesTablePolicy() { + if (s3tablesTablePolicyMutationInFlight) return; + if (!s3tablesTablePolicyLoaded) { + alert('The current policy has not finished loading. Close and reopen this dialog before saving.'); + return; + } + if (!commitPolicyActiveTab('s3tablesTable')) return; + const target = currentS3TablesTablePolicyTarget(); + const policy = document.getElementById('s3tablesTablePolicyText').value.trim(); + if (!policy || !policyTextHasStatements(policy)) { + alert('Add at least one statement, or use Delete Policy to remove the policy.'); + return; + } + setS3TablesTablePolicyMutationInFlight(true); + try { + const response = await fetch(s3tBasePath('/api/s3tables/table-policy'), { + method: 'PUT', + headers: s3tWriteHeaders({ 'Content-Type': 'application/json' }), + body: JSON.stringify({ + bucket_arn: document.getElementById('s3tablesTablePolicyBucketArn').value, + namespace: document.getElementById('s3tablesTablePolicyNamespace').value, + name: document.getElementById('s3tablesTablePolicyName').value, + policy: policy + }) + }); + const data = await response.json(); + setS3TablesTablePolicyMutationInFlight(false); + const stillCurrent = target === currentS3TablesTablePolicyTarget(); + if (!response.ok) { + if (stillCurrent) { + alert(data.error || 'Failed to update policy'); + } else { + console.error('Error saving table policy (no longer the open resource): ' + (data.error || 'unknown error')); + } + return; + } + if (!stillCurrent) return; + s3tablesTablePolicyModal.hide(); + setTimeout(() => location.reload(), 500); + } catch (error) { + setS3TablesTablePolicyMutationInFlight(false); + if (target === currentS3TablesTablePolicyTarget()) { + alert('Failed to update policy: ' + error.message); + } else { + console.error('Error saving table policy: ' + error.message); + } + } +} + async function deleteS3TablesBucketPolicy() { - const bucketArn = document.getElementById('s3tablesBucketPolicyArn').value; + const bucketArn = currentS3TablesBucketPolicyTarget(); if (!bucketArn) return; + if (s3tablesBucketPolicyMutationInFlight) return; if (!s3tablesBucketPolicyLoaded) { alert('The current policy has not finished loading. Close and reopen this dialog before deleting.'); return; } + if (!confirm('Delete this table bucket policy? This cannot be undone.')) return; + setS3TablesBucketPolicyMutationInFlight(true); try { const response = await fetch(s3tBasePath(`/api/s3tables/bucket-policy?bucket=${encodeURIComponent(bucketArn)}`), { method: 'DELETE', headers: s3tWriteHeaders() }); const data = await response.json(); + setS3TablesBucketPolicyMutationInFlight(false); + const stillCurrent = bucketArn === currentS3TablesBucketPolicyTarget(); if (!response.ok) { - alert(data.error || 'Failed to delete policy'); + if (stillCurrent) { + alert(data.error || 'Failed to delete policy'); + } else { + console.error('Error deleting policy for ' + bucketArn + ' (no longer the open resource): ' + (data.error || 'unknown error')); + } return; } - alert('Policy deleted'); - document.getElementById('s3tablesBucketPolicyText').value = ''; - commitPolicyTextareaToEditor('s3tablesBucket'); + if (!stillCurrent) return; + s3tablesBucketPolicyModal.hide(); + setTimeout(() => location.reload(), 500); } catch (error) { - alert('Failed to delete policy: ' + error.message); + setS3TablesBucketPolicyMutationInFlight(false); + if (bucketArn === currentS3TablesBucketPolicyTarget()) { + alert('Failed to delete policy: ' + error.message); + } else { + console.error('Error deleting policy for ' + bucketArn + ': ' + error.message); + } } } @@ -786,26 +898,42 @@ async function loadS3TablesTablePolicy(bucketArn, namespace, name) { } async function deleteS3TablesTablePolicy() { + if (s3tablesTablePolicyMutationInFlight) return; if (!s3tablesTablePolicyLoaded) { alert('The current policy has not finished loading. Close and reopen this dialog before deleting.'); return; } - const dataContainer = document.getElementById('s3tables-tables-content'); - const dataBucketArn = dataContainer.dataset.bucketArn || ''; - const dataNamespace = dataContainer.dataset.namespace || ''; - const query = new URLSearchParams({ bucket: dataBucketArn, namespace: dataNamespace, name: document.getElementById('s3tablesTablePolicyName').value }); + const target = currentS3TablesTablePolicyTarget(); + const query = new URLSearchParams({ + bucket: document.getElementById('s3tablesTablePolicyBucketArn').value, + namespace: document.getElementById('s3tablesTablePolicyNamespace').value, + name: document.getElementById('s3tablesTablePolicyName').value + }); + if (!confirm('Delete the policy for table ' + document.getElementById('s3tablesTablePolicyName').value + '? This cannot be undone.')) return; + setS3TablesTablePolicyMutationInFlight(true); try { const response = await fetch(s3tBasePath(`/api/s3tables/table-policy?${query.toString()}`), { method: 'DELETE', headers: s3tWriteHeaders() }); const data = await response.json(); + setS3TablesTablePolicyMutationInFlight(false); + const stillCurrent = target === currentS3TablesTablePolicyTarget(); if (!response.ok) { - alert(data.error || 'Failed to delete policy'); + if (stillCurrent) { + alert(data.error || 'Failed to delete policy'); + } else { + console.error('Error deleting table policy (no longer the open resource): ' + (data.error || 'unknown error')); + } return; } - alert('Policy deleted'); - document.getElementById('s3tablesTablePolicyText').value = ''; - commitPolicyTextareaToEditor('s3tablesTable'); + if (!stillCurrent) return; + s3tablesTablePolicyModal.hide(); + setTimeout(() => location.reload(), 500); } catch (error) { - alert('Failed to delete policy: ' + error.message); + setS3TablesTablePolicyMutationInFlight(false); + if (target === currentS3TablesTablePolicyTarget()) { + alert('Failed to delete policy: ' + error.message); + } else { + console.error('Error deleting table policy: ' + error.message); + } } } diff --git a/weed/admin/static_gz/js/policy_editor.js.gz b/weed/admin/static_gz/js/policy_editor.js.gz index 5a7183cc2..79922b8d5 100644 Binary files a/weed/admin/static_gz/js/policy_editor.js.gz and b/weed/admin/static_gz/js/policy_editor.js.gz differ diff --git a/weed/admin/static_gz/js/s3tables.js.gz b/weed/admin/static_gz/js/s3tables.js.gz index 2b5cd7e1f..20a0c8cbc 100644 Binary files a/weed/admin/static_gz/js/s3tables.js.gz and b/weed/admin/static_gz/js/s3tables.js.gz differ diff --git a/weed/admin/view/app/policies.templ b/weed/admin/view/app/policies.templ index dd85f75c2..60d34db5a 100644 --- a/weed/admin/view/app/policies.templ +++ b/weed/admin/view/app/policies.templ @@ -172,32 +172,6 @@ templ Policies(data dash.PoliciesData) { - - @PolicyDatalists() @@ -352,6 +326,11 @@ templ Policies(data dash.PoliciesData) { ") + templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 16, "
Create IAM Policy
Enter a unique name for this policy (alphanumeric and underscores only)
Enter the policy document in AWS IAM JSON format
View IAM Policy

Loading policy...

Edit IAM Policy
Policy name cannot be changed
Edit the policy document in AWS IAM JSON format
") if templ_7745c5c3_Err != nil { return templ_7745c5c3_Err } diff --git a/weed/admin/view/app/policy_action_suggestions.go b/weed/admin/view/app/policy_action_suggestions.go index 608bc8eaf..2bb11ef17 100644 --- a/weed/admin/view/app/policy_action_suggestions.go +++ b/weed/admin/view/app/policy_action_suggestions.go @@ -1,6 +1,10 @@ package app -import "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" +import ( + "strings" + + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" +) // PolicyActionSuggestions feeds the used by the structured policy // editor's Action inputs. This is an input-assistance aid, NOT a validation @@ -9,6 +13,18 @@ import "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" // like "s3:Get*", etc.). var PolicyActionSuggestions = buildPolicyActionSuggestions() +// S3TablesPolicyActionSuggestions is the s3tables: subset, for the S3 Tables +// policy editors whose evaluator only matches s3tables actions. +var S3TablesPolicyActionSuggestions = func() []string { + var out []string + for _, a := range PolicyActionSuggestions { + if strings.HasPrefix(a, "s3tables:") { + out = append(out, a) + } + } + return out +}() + func buildPolicyActionSuggestions() []string { return []string{ // s3: actions, sourced from the constants used elsewhere for policy diff --git a/weed/admin/view/app/policy_datalists.templ b/weed/admin/view/app/policy_datalists.templ index 53a916503..d29f0de06 100644 --- a/weed/admin/view/app/policy_datalists.templ +++ b/weed/admin/view/app/policy_datalists.templ @@ -1,14 +1,40 @@ package app -// PolicyDatalists renders the three elements the shared visual -// policy editor (weed/admin/static/js/policy_editor.js) attaches its -// action/resource/principal suggestions to. Any page -// embedding that editor must render this once; its ids -// (policyActionSuggestions, policyResourceSuggestions, -// policyPrincipalSuggestions) are the registerPolicyEditor defaults, so a -// page only needs to override them if it renders more than one instance of -// this component. +// PolicyDatalists renders the elements the shared visual policy +// editor (weed/admin/static/js/policy_editor.js) attaches its +// action/resource/principal suggestions to, plus the +// styles the editor's generated markup depends on. Any page embedding that +// editor must render this once; the ids (policyActionSuggestions, +// policyResourceSuggestions, policyPrincipalSuggestions) are the +// registerPolicyEditor defaults, so a page only needs to override them if +// it renders more than one instance of this component. templ PolicyDatalists() { + + for _, action := range PolicyActionSuggestions { @@ -16,6 +42,14 @@ templ PolicyDatalists() { } + + + for _, action := range S3TablesPolicyActionSuggestions { + + } + + diff --git a/weed/admin/view/app/policy_datalists_templ.go b/weed/admin/view/app/policy_datalists_templ.go index 0116f6845..b0e599dc5 100644 --- a/weed/admin/view/app/policy_datalists_templ.go +++ b/weed/admin/view/app/policy_datalists_templ.go @@ -8,14 +8,14 @@ package app import "github.com/a-h/templ" import templruntime "github.com/a-h/templ/runtime" -// PolicyDatalists renders the three elements the shared visual -// policy editor (weed/admin/static/js/policy_editor.js) attaches its -// action/resource/principal suggestions to. Any page -// embedding that editor must render this once; its ids -// (policyActionSuggestions, policyResourceSuggestions, -// policyPrincipalSuggestions) are the registerPolicyEditor defaults, so a -// page only needs to override them if it renders more than one instance of -// this component. +// PolicyDatalists renders the elements the shared visual policy +// editor (weed/admin/static/js/policy_editor.js) attaches its +// action/resource/principal suggestions to, plus the +// styles the editor's generated markup depends on. Any page embedding that +// editor must render this once; the ids (policyActionSuggestions, +// policyResourceSuggestions, policyPrincipalSuggestions) are the +// registerPolicyEditor defaults, so a page only needs to override them if +// it renders more than one instance of this component. func PolicyDatalists() templ.Component { return templruntime.GeneratedTemplate(func(templ_7745c5c3_Input templruntime.GeneratedComponentInput) (templ_7745c5c3_Err error) { templ_7745c5c3_W, ctx := templ_7745c5c3_Input.Writer, templ_7745c5c3_Input.Context @@ -37,7 +37,7 @@ func PolicyDatalists() templ.Component { templ_7745c5c3_Var1 = templ.NopComponent } ctx = templ.ClearChildren(ctx) - templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 1, "") + templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 1, "") if templ_7745c5c3_Err != nil { return templ_7745c5c3_Err } @@ -49,7 +49,7 @@ func PolicyDatalists() templ.Component { var templ_7745c5c3_Var2 string templ_7745c5c3_Var2, templ_7745c5c3_Err = templ.ResolveAttributeValue(action) if templ_7745c5c3_Err != nil { - return templ.Error{Err: templ_7745c5c3_Err, FileName: `weed/admin/view/app/policy_datalists.templ`, Line: 15, Col: 25} + return templ.Error{Err: templ_7745c5c3_Err, FileName: `weed/admin/view/app/policy_datalists.templ`, Line: 41, Col: 25} } _, templ_7745c5c3_Err = templ_7745c5c3_Buffer.WriteString(templ_7745c5c3_Var2) if templ_7745c5c3_Err != nil { @@ -60,7 +60,30 @@ func PolicyDatalists() templ.Component { return templ_7745c5c3_Err } } - templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 4, "") + templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 4, "") + if templ_7745c5c3_Err != nil { + return templ_7745c5c3_Err + } + for _, action := range S3TablesPolicyActionSuggestions { + templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 5, "") + if templ_7745c5c3_Err != nil { + return templ_7745c5c3_Err + } + } + templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 7, "") if templ_7745c5c3_Err != nil { return templ_7745c5c3_Err } diff --git a/weed/admin/view/app/s3_buckets.templ b/weed/admin/view/app/s3_buckets.templ index fe124cd3a..d76c07124 100644 --- a/weed/admin/view/app/s3_buckets.templ +++ b/weed/admin/view/app/s3_buckets.templ @@ -1410,7 +1410,7 @@ templ S3Buckets(data dash.S3BucketsData) { // Drop responses that arrive after another bucket was opened const requestSeq = ++policyRequestSeq; - fetch(basePath('/api/s3/buckets/' + bucketName + '/policy')) + fetch(basePath('/api/s3/buckets/' + encodeURIComponent(bucketName) + '/policy')) .then(parseLifecycleResponse) .then(({ ok, data }) => { if (requestSeq !== policyRequestSeq) return; @@ -1422,8 +1422,11 @@ templ S3Buckets(data dash.S3BucketsData) { return; } document.getElementById('bucketPolicyEditorWrapper').style.display = ''; + // policy_text carries stored bytes the server-side + // decoder rejects; the JSON tab shows them so they + // can be fixed or deleted. document.getElementById('bucketPolicyDocument').value = - data.policy ? JSON.stringify(data.policy, null, 2) : ''; + data.policy ? JSON.stringify(data.policy, null, 2) : (data.policy_text || ''); bucketPolicyLoaded = true; loadPolicyTextareaIntoEditor('bucket'); }) @@ -1454,8 +1457,8 @@ templ S3Buckets(data dash.S3BucketsData) { if (!commitPolicyActiveTab('bucket')) return; const raw = document.getElementById('bucketPolicyDocument').value.trim(); - if (!raw) { - alert('Enter a policy document, or use Delete to remove the bucket policy.'); + if (!raw || !policyTextHasStatements(raw)) { + alert('Add at least one statement, or use Delete to remove the bucket policy.'); return; } @@ -1471,13 +1474,18 @@ templ S3Buckets(data dash.S3BucketsData) { alert(validationError); return; } + // The admin API decodes only Version and Statement, so warn + // before other top-level fields are silently dropped - the IAM + // policies page already does. (The S3 Tables dialogs don't: + // their backend stores the document verbatim.) + if (!confirmPolicyFieldDiscard('bucket')) return; // Captured now: if the dialog moves on to a different bucket // before this PUT resolves, the completion below must not hide // or reload over whatever that other bucket is now showing. const targetBucket = policyEditorBucket; setBucketPolicyMutationInFlight(true); - fetch(basePath('/api/s3/buckets/' + targetBucket + '/policy'), { + fetch(basePath('/api/s3/buckets/' + encodeURIComponent(targetBucket) + '/policy'), { method: 'PUT', headers: csrfHeaders(), body: JSON.stringify({ policy: policy }) @@ -1524,7 +1532,7 @@ templ S3Buckets(data dash.S3BucketsData) { // hide or reload over whatever that other bucket is now showing. const targetBucket = policyEditorBucket; setBucketPolicyMutationInFlight(true); - fetch(basePath('/api/s3/buckets/' + targetBucket + '/policy'), { + fetch(basePath('/api/s3/buckets/' + encodeURIComponent(targetBucket) + '/policy'), { method: 'DELETE', headers: csrfHeaders() }) diff --git a/weed/admin/view/app/s3_buckets_templ.go b/weed/admin/view/app/s3_buckets_templ.go index 64c1ee16a..cfae7e171 100644 --- a/weed/admin/view/app/s3_buckets_templ.go +++ b/weed/admin/view/app/s3_buckets_templ.go @@ -865,7 +865,7 @@ func S3Buckets(data dash.S3BucketsData) templ.Component { if templ_7745c5c3_Err != nil { return templ_7745c5c3_Err } - templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 99, "
Bucket Policy
Loading bucket policy...

Every statement must specify a Principal, and every Resource must refer to this bucket. Leave the document empty and click Delete to remove the policy.

Enter the bucket policy document as JSON
Manage Bucket Owner
Select the S3 identity that owns this bucket. Non-admin users can only access buckets they own.
Loading users...
") + templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 99, "
Bucket Policy
Loading bucket policy...

Every statement must specify a Principal, and every Resource must refer to this bucket. Leave the document empty and click Delete to remove the policy.

Enter the bucket policy document as JSON
Manage Bucket Owner
Select the S3 identity that owns this bucket. Non-admin users can only access buckets they own.
Loading users...
") if templ_7745c5c3_Err != nil { return templ_7745c5c3_Err } diff --git a/weed/admin/view/app/s3tables_buckets.templ b/weed/admin/view/app/s3tables_buckets.templ index 9530175d0..9ee919646 100644 --- a/weed/admin/view/app/s3tables_buckets.templ +++ b/weed/admin/view/app/s3tables_buckets.templ @@ -449,10 +449,13 @@ dataset = lance.dataset(table.location, storage_options=table.storage_options)`
- -
diff --git a/weed/admin/view/app/s3tables_buckets_templ.go b/weed/admin/view/app/s3tables_buckets_templ.go index 5936465ae..e321473f8 100644 --- a/weed/admin/view/app/s3tables_buckets_templ.go +++ b/weed/admin/view/app/s3tables_buckets_templ.go @@ -690,7 +690,7 @@ dataset = lance.dataset(table.location, storage_options=table.storage_options)`) if templ_7745c5c3_Err != nil { return templ_7745c5c3_Err } - templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 68, "
Table Bucket Policy
Provide a policy JSON; use Delete Policy to remove the policy.
Resource Tags
Loading...
") + templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 68, "
Table Bucket Policy
Provide a policy JSON; use Delete Policy to remove the policy.
Resource Tags
Loading...
") if templ_7745c5c3_Err != nil { return templ_7745c5c3_Err } diff --git a/weed/admin/view/app/s3tables_tables.templ b/weed/admin/view/app/s3tables_tables.templ index 8df58785d..8cc100e11 100644 --- a/weed/admin/view/app/s3tables_tables.templ +++ b/weed/admin/view/app/s3tables_tables.templ @@ -292,10 +292,13 @@ templ S3TablesTables(data dash.S3TablesTablesData) {
- -
diff --git a/weed/admin/view/app/s3tables_tables_templ.go b/weed/admin/view/app/s3tables_tables_templ.go index e8efb6074..f5ad06cec 100644 --- a/weed/admin/view/app/s3tables_tables_templ.go +++ b/weed/admin/view/app/s3tables_tables_templ.go @@ -549,7 +549,7 @@ func S3TablesTables(data dash.S3TablesTablesData) templ.Component { if templ_7745c5c3_Err != nil { return templ_7745c5c3_Err } - templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 56, "
Table Policy
Resource Tags
Loading...
") + templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 56, "
Table Policy
Resource Tags
Loading...
") if templ_7745c5c3_Err != nil { return templ_7745c5c3_Err } diff --git a/weed/iam/integration/iam_manager.go b/weed/iam/integration/iam_manager.go index 6a04e09f9..ef4aae516 100644 --- a/weed/iam/integration/iam_manager.go +++ b/weed/iam/integration/iam_manager.go @@ -865,6 +865,44 @@ func (m *IAMManager) UpdateBucketPolicy(ctx context.Context, bucketName string, return m.policyEngine.AddPolicy(m.getFilerAddress(), policyName, &policyDoc) } +// EnsureBucketPolicy stores the policy for a bucket only when no mirror is +// stored yet, backfilling policies that predate the IAM integration (the +// metadata subscription only sees changes). A present mirror is left alone, +// so repeat calls cost one cached read. Returns whether a write happened, +// so the caller can reconcile a write that raced a concurrent change. +func (m *IAMManager) EnsureBucketPolicy(ctx context.Context, bucketName string, policyJSON []byte) (bool, error) { + if !m.initialized { + return false, fmt.Errorf("IAM manager not initialized") + } + + if bucketName == "" { + return false, fmt.Errorf("bucket name cannot be empty") + } + + if existing, err := m.policyEngine.GetPolicy(ctx, m.getFilerAddress(), "bucket-policy:"+bucketName); err == nil && existing != nil { + return false, nil + } + + if err := m.UpdateBucketPolicy(ctx, bucketName, policyJSON); err != nil { + return false, err + } + return true, nil +} + +// RemoveBucketPolicy deletes the stored policy for a bucket. Removing a +// policy that was never stored is a success. +func (m *IAMManager) RemoveBucketPolicy(ctx context.Context, bucketName string) error { + if !m.initialized { + return fmt.Errorf("IAM manager not initialized") + } + + if bucketName == "" { + return fmt.Errorf("bucket name cannot be empty") + } + + return m.policyEngine.DeletePolicy(ctx, m.getFilerAddress(), "bucket-policy:"+bucketName) +} + // AssumeRoleWithWebIdentity assumes a role using web identity (OIDC) func (m *IAMManager) AssumeRoleWithWebIdentity(ctx context.Context, request *sts.AssumeRoleWithWebIdentityRequest) (*sts.AssumeRoleResponse, error) { if !m.initialized { diff --git a/weed/iam/policy/policy_engine.go b/weed/iam/policy/policy_engine.go index acd7c0a2f..f53768bb4 100644 --- a/weed/iam/policy/policy_engine.go +++ b/weed/iam/policy/policy_engine.go @@ -357,6 +357,20 @@ func (e *PolicyEngine) AddPolicy(filerAddress string, name string, policy *Polic return e.store.StorePolicy(context.Background(), filerAddress, name, policy) } +// GetPolicy returns a stored policy document, or an error when it does not +// exist in the configured store. +func (e *PolicyEngine) GetPolicy(ctx context.Context, filerAddress string, name string) (*PolicyDocument, error) { + if !e.initialized { + return nil, fmt.Errorf("policy engine not initialized") + } + + if name == "" { + return nil, fmt.Errorf("policy name cannot be empty") + } + + return e.store.GetPolicy(ctx, filerAddress, name) +} + // DeletePolicy removes a policy from the configured store. func (e *PolicyEngine) DeletePolicy(ctx context.Context, filerAddress string, name string) error { if !e.initialized { diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index 599fa1923..add4e69f5 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -76,6 +76,11 @@ type IdentityAccessManagement struct { // Bucket policy engine for evaluating bucket policies policyEngine *BucketPolicyEngine + // primeBucketForIAM loads a bucket's config (and with it the advanced-IAM + // bucket-policy mirror backfill) before an IAM authorization that will + // evaluate that mirror. Set by NewS3ApiServer; nil in tests. + primeBucketForIAM func(bucket string) + // Cached policy engine for IAM policy fallback evaluation. // Keyed by policy name, kept in sync by PutPolicy/DeletePolicy. iamPolicyEngine *policy_engine.PolicyEngine @@ -2865,6 +2870,13 @@ func (iam *IdentityAccessManagement) AuthorizeObjectDelete(r *http.Request, iden func (iam *IdentityAccessManagement) authorizeWithIAM(r *http.Request, identity *Identity, action Action, bucket string, object string) s3err.ErrorCode { ctx := r.Context() + // The evaluation below consults the bucket-policy: mirror, so the + // bucket's lazy load (which backfills that mirror) must happen first - + // nothing earlier on a denied request's path would ever trigger it. + if iam.primeBucketForIAM != nil && bucket != "" { + iam.primeBucketForIAM(bucket) + } + // Get session info from request headers // First check for JWT-based authentication headers (SeaweedFSSessionTokenHeader) sessionToken := r.Header.Get(s3_constants.SeaweedFSSessionTokenHeader) diff --git a/weed/s3api/auth_credentials_subscribe.go b/weed/s3api/auth_credentials_subscribe.go index 2f6714839..50be9104e 100644 --- a/weed/s3api/auth_credentials_subscribe.go +++ b/weed/s3api/auth_credentials_subscribe.go @@ -184,6 +184,7 @@ func (s3a *S3ApiServer) onCircuitBreakerConfigChange(dir string, oldEntry *filer func (s3a *S3ApiServer) onBucketMetadataChange(dir string, oldEntry *filer_pb.Entry, newEntry *filer_pb.Entry) error { if dir == s3a.option.BucketsPath { s3a.maintainBucketOwnerIndex(oldEntry, newEntry) + s3a.mirrorBucketPolicyToIAM(oldEntry, newEntry) if newEntry != nil { // Update bucket registry (existing functionality) s3a.bucketRegistry.LoadBucketMetadata(newEntry) diff --git a/weed/s3api/policy_engine/bucket_policy.go b/weed/s3api/policy_engine/bucket_policy.go index b7cf969c9..d909489fd 100644 --- a/weed/s3api/policy_engine/bucket_policy.go +++ b/weed/s3api/policy_engine/bucket_policy.go @@ -5,24 +5,22 @@ import ( "strings" ) +// MaxBucketPolicySize mirrors AWS S3's 20 KB bucket-policy limit, enforced +// by both writers (the S3 gateway's PutBucketPolicy and the admin UI) so +// neither surface can store a document the other refuses to manage. +const MaxBucketPolicySize = 20 * 1024 + // ValidateBucketPolicy performs bucket-specific policy validation, on top of -// the generic structural checks in ValidatePolicy. It enforces the rules -// that make a policy document valid as an S3 *bucket* policy specifically: -// every statement must name a Principal, and every Resource/NotResource/Action -// must scope to the given bucket. +// the generic structural checks in ValidatePolicy - callers run that first, +// so the version and non-empty-statement rules are not re-checked here. It +// enforces the rules that make a policy document valid as an S3 *bucket* +// policy specifically: every statement must name a Principal, and every +// Resource/NotResource/Action must scope to the given bucket. // // This is shared between the S3 gateway's PutBucketPolicy handler // (weed/s3api/s3api_bucket_policy_handlers.go) and the admin UI // (weed/admin/dash) so both enforce identical rules. func ValidateBucketPolicy(policyDoc *PolicyDocument, bucket string) error { - if policyDoc.Version != PolicyVersion2012_10_17 { - return fmt.Errorf("unsupported policy version: %s (must be %s)", policyDoc.Version, PolicyVersion2012_10_17) - } - - if len(policyDoc.Statement) == 0 { - return fmt.Errorf("policy document must contain at least one statement") - } - for i, statement := range policyDoc.Statement { // Bucket policies must have Principal if statement.Principal == nil { diff --git a/weed/s3api/policy_engine/bucket_policy_test.go b/weed/s3api/policy_engine/bucket_policy_test.go index 217e1c8fa..0126850e9 100644 --- a/weed/s3api/policy_engine/bucket_policy_test.go +++ b/weed/s3api/policy_engine/bucket_policy_test.go @@ -54,15 +54,17 @@ func TestValidateBucketPolicy(t *testing.T) { }) t.Run("bad version", func(t *testing.T) { + // Version and non-empty-statement rules live in ValidatePolicy, + // which callers run first. doc := &PolicyDocument{Version: "2008-10-17", Statement: []PolicyStatement{validStatement()}} - if err := ValidateBucketPolicy(doc, bucket); err == nil { + if err := ValidatePolicy(doc); err == nil { t.Error("expected error for bad version") } }) t.Run("zero statements", func(t *testing.T) { doc := &PolicyDocument{Version: PolicyVersion2012_10_17, Statement: []PolicyStatement{}} - if err := ValidateBucketPolicy(doc, bucket); err == nil { + if err := ValidatePolicy(doc); err == nil { t.Error("expected error for zero statements") } }) diff --git a/weed/s3api/s3api_bucket_config.go b/weed/s3api/s3api_bucket_config.go index fdecb24db..9a3a3b02d 100644 --- a/weed/s3api/s3api_bucket_config.go +++ b/weed/s3api/s3api_bucket_config.go @@ -381,6 +381,19 @@ func (s3a *S3ApiServer) getBucketConfig(bucket string) (*BucketConfig, s3err.Err config := s3a.newBucketConfigFromEntry(bucket, entry) + // A cold load is the first time this gateway learns the bucket's policy + // exists, and the metadata subscription only mirrors changes - a policy + // that predates the IAM integration would otherwise never reach the + // advanced-IAM mirror and its grants would not bind on the IAM path. + // Synchronous: the IAM auth path primes the bucket through here before + // evaluating the mirror, so the backfill has to land first - a one-time + // cost on the load that discovers the policy. Raw entry bytes, not the + // parsed document, so the backfill can byte-compare against a later + // entry read when it reconciles. + if policyJSON := entry.Extended[BUCKET_POLICY_METADATA_KEY]; len(policyJSON) > 0 && config.BucketPolicy != nil && s3a.bucketPolicyIAMManager() != nil { + s3a.ensureBucketPolicyInIAM(bucket, policyJSON) + } + // Cache the result s3a.bucketConfigCache.Set(bucket, config) diff --git a/weed/s3api/s3api_bucket_policy_handlers.go b/weed/s3api/s3api_bucket_policy_handlers.go index 9e38fe09f..4842403a3 100644 --- a/weed/s3api/s3api_bucket_policy_handlers.go +++ b/weed/s3api/s3api_bucket_policy_handlers.go @@ -1,6 +1,7 @@ package s3api import ( + "bytes" "context" "encoding/json" "errors" @@ -9,6 +10,7 @@ import ( "net/http" "github.com/seaweedfs/seaweedfs/weed/glog" + "github.com/seaweedfs/seaweedfs/weed/iam/integration" "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" "github.com/seaweedfs/seaweedfs/weed/s3api/policy_engine" "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" @@ -75,7 +77,7 @@ func (s3a *S3ApiServer) PutBucketPolicyHandler(w http.ResponseWriter, r *http.Re glog.V(3).Infof("PutBucketPolicyHandler: bucket=%s", bucket) // Read policy document from request body - body, err := io.ReadAll(r.Body) + body, err := io.ReadAll(io.LimitReader(r.Body, policy_engine.MaxBucketPolicySize+1)) if err != nil { glog.Errorf("Failed to read bucket policy request body: %v", err) s3err.WriteErrorResponse(w, r, s3err.ErrInvalidPolicyDocument) @@ -83,6 +85,11 @@ func (s3a *S3ApiServer) PutBucketPolicyHandler(w http.ResponseWriter, r *http.Re } defer r.Body.Close() + if len(body) > policy_engine.MaxBucketPolicySize { + s3err.WriteErrorResponse(w, r, s3err.ErrPolicyTooLarge) + return + } + // Parse and validate policy document var policyDoc policy_engine.PolicyDocument if err := json.Unmarshal(body, &policyDoc); err != nil { @@ -299,37 +306,157 @@ func (s3a *S3ApiServer) deleteBucketPolicy(bucket string) error { // updateBucketPolicyInIAM updates the IAM system with the new bucket policy func (s3a *S3ApiServer) updateBucketPolicyInIAM(bucket string, policyDoc *policy_engine.PolicyDocument) error { - // Update IAM integration with new bucket policy - if s3a.iam.iamIntegration != nil { - // Type assert to access the concrete implementation which has access to iamManager - if s3Integration, ok := s3a.iam.iamIntegration.(*S3IAMIntegration); ok { - if s3Integration.iamManager != nil { - glog.V(2).Infof("Updated bucket policy for %s in IAM system", bucket) - - policyJSON, err := json.Marshal(policyDoc) - if err != nil { - return fmt.Errorf("failed to marshal policy: %w", err) - } - - return s3Integration.iamManager.UpdateBucketPolicy(context.Background(), bucket, policyJSON) - } - } + iamManager := s3a.bucketPolicyIAMManager() + if iamManager == nil { + return nil } - return nil + policyJSON, err := json.Marshal(policyDoc) + if err != nil { + return fmt.Errorf("failed to marshal policy: %w", err) + } + + glog.V(2).Infof("Updated bucket policy for %s in IAM system", bucket) + return iamManager.UpdateBucketPolicy(context.Background(), bucket, policyJSON) +} + +// ensureBucketPolicyInIAM backfills the IAM mirror for a policy the +// subscription never saw change (one that predates the IAM integration). +// Called from the lazy bucket-config load; a present mirror is left alone. +func (s3a *S3ApiServer) ensureBucketPolicyInIAM(bucket string, policyJSON []byte) { + iamManager := s3a.bucketPolicyIAMManager() + if iamManager == nil { + return + } + + wrote, err := iamManager.EnsureBucketPolicy(context.Background(), bucket, policyJSON) + if err != nil { + glog.Warningf("backfill bucket policy for %s into IAM: %v", bucket, err) + return + } + if !wrote { + return + } + + // The check-then-write above can race a concurrent policy change or + // delete: the event-driven mirror may have landed in between, and this + // write would then have re-stored bytes that are already stale - with + // no later event to heal it. Reconcile against a fresh entry read, + // which is authoritative; anything changing after this read fires its + // own event, and the mirror for it finds this write already present. + entry, err := s3a.getBucketEntry(bucket) + if err != nil { + if errors.Is(err, filer_pb.ErrNotFound) { + if err := s3a.removeBucketPolicyFromIAM(bucket); err != nil { + glog.Warningf("remove bucket policy for %s from IAM: %v", bucket, err) + } + } + return + } + current := entry.Extended[BUCKET_POLICY_METADATA_KEY] + if bytes.Equal(current, policyJSON) { + return + } + if len(current) == 0 { + if err := s3a.removeBucketPolicyFromIAM(bucket); err != nil { + glog.Warningf("remove bucket policy for %s from IAM: %v", bucket, err) + } + return + } + var policyDoc policy_engine.PolicyDocument + if err := json.Unmarshal(current, &policyDoc); err != nil { + glog.Warningf("backfill bucket policy for %s into IAM: parse: %v", bucket, err) + return + } + if err := s3a.updateBucketPolicyInIAM(bucket, &policyDoc); err != nil { + glog.Warningf("backfill bucket policy for %s into IAM: %v", bucket, err) + } } // removeBucketPolicyFromIAM removes the bucket policy from the IAM system func (s3a *S3ApiServer) removeBucketPolicyFromIAM(bucket string) error { - // This would remove the bucket policy from our advanced IAM system + iamManager := s3a.bucketPolicyIAMManager() + if iamManager == nil { + return nil + } + glog.V(2).Infof("Removed bucket policy for %s from IAM system", bucket) + return iamManager.RemoveBucketPolicy(context.Background(), bucket) +} - // TODO: Integrate with IAM manager to remove resource-based policies - // s3a.iam.iamIntegration.iamManager.RemoveBucketPolicy(bucket) - +// bucketPolicyIAMManager returns the advanced-IAM manager the +// "bucket-policy:" mirror lives in, or nil when the integration is +// not enabled. +func (s3a *S3ApiServer) bucketPolicyIAMManager() *integration.IAMManager { + if s3a.iam == nil || s3a.iam.iamIntegration == nil { + return nil + } + if s3Integration, ok := s3a.iam.iamIntegration.(*S3IAMIntegration); ok { + return s3Integration.iamManager + } return nil } +// mirrorBucketPolicyToIAM keeps the "bucket-policy:" IAM mirror in +// sync with the policy stored on a bucket's filer entry. Driven from the +// metadata subscription so it covers every writer - this gateway's own +// PutBucketPolicy, another gateway's, the admin UI, bucket deletion, and +// rename - where the handlers' direct calls only ever covered the first. +func (s3a *S3ApiServer) mirrorBucketPolicyToIAM(oldEntry, newEntry *filer_pb.Entry) { + if s3a.bucketPolicyIAMManager() == nil { + return + } + removeName, updateName, updatePolicy := bucketPolicyMirrorOps(oldEntry, newEntry) + if removeName != "" { + if err := s3a.removeBucketPolicyFromIAM(removeName); err != nil { + glog.Warningf("remove bucket policy for %s from IAM: %v", removeName, err) + } + } + if updateName == "" { + return + } + var policyDoc policy_engine.PolicyDocument + if err := json.Unmarshal(updatePolicy, &policyDoc); err != nil { + glog.Warningf("mirror bucket policy for %s to IAM: parse: %v", updateName, err) + return + } + if err := s3a.updateBucketPolicyInIAM(updateName, &policyDoc); err != nil { + glog.Warningf("mirror bucket policy for %s to IAM: %v", updateName, err) + } +} + +// bucketPolicyMirrorOps computes what a bucket entry change means for the +// IAM mirror: a name whose mirror must be removed, and a (name, policy) to +// write. A rename delivers both entries under different names in one event, +// and the old name's mirror has to move even when the policy bytes are +// unchanged - equality only short-circuits same-name updates. +func bucketPolicyMirrorOps(oldEntry, newEntry *filer_pb.Entry) (removeName, updateName string, updatePolicy []byte) { + var oldName, newName string + var oldPolicy, newPolicy []byte + if oldEntry != nil { + oldName = oldEntry.Name + oldPolicy = oldEntry.Extended[BUCKET_POLICY_METADATA_KEY] + } + if newEntry != nil { + newName = newEntry.Name + newPolicy = newEntry.Extended[BUCKET_POLICY_METADATA_KEY] + } + if oldName != "" && oldName != newName && len(oldPolicy) > 0 { + removeName = oldName + oldPolicy = nil + } + if newName == "" || bytes.Equal(oldPolicy, newPolicy) { + return + } + if len(newPolicy) == 0 { + removeName = newName + return + } + updateName = newName + updatePolicy = newPolicy + return +} + // GetPublicAccessBlockHandler Retrieves the PublicAccessBlock configuration for an S3 bucket // https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetPublicAccessBlock.html func (s3a *S3ApiServer) GetPublicAccessBlockHandler(w http.ResponseWriter, r *http.Request) { diff --git a/weed/s3api/s3api_bucket_policy_mirror_test.go b/weed/s3api/s3api_bucket_policy_mirror_test.go new file mode 100644 index 000000000..b000e3277 --- /dev/null +++ b/weed/s3api/s3api_bucket_policy_mirror_test.go @@ -0,0 +1,114 @@ +package s3api + +import ( + "testing" + + "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" +) + +func bucketEntryWithPolicy(name, policy string) *filer_pb.Entry { + entry := &filer_pb.Entry{Name: name} + if policy != "" { + entry.Extended = map[string][]byte{BUCKET_POLICY_METADATA_KEY: []byte(policy)} + } + return entry +} + +func TestBucketPolicyMirrorOps(t *testing.T) { + policyA := `{"Version":"2012-10-17","Statement":[{"Effect":"Allow"}]}` + policyB := `{"Version":"2012-10-17","Statement":[{"Effect":"Deny"}]}` + + tests := []struct { + name string + oldEntry *filer_pb.Entry + newEntry *filer_pb.Entry + removeName string + updateName string + updatePolicy string + }{ + { + name: "create without policy", + newEntry: bucketEntryWithPolicy("a", ""), + }, + { + name: "policy set", + oldEntry: bucketEntryWithPolicy("a", ""), + newEntry: bucketEntryWithPolicy("a", policyA), + updateName: "a", + updatePolicy: policyA, + }, + { + name: "policy replaced", + oldEntry: bucketEntryWithPolicy("a", policyA), + newEntry: bucketEntryWithPolicy("a", policyB), + updateName: "a", + updatePolicy: policyB, + }, + { + name: "policy unchanged", + oldEntry: bucketEntryWithPolicy("a", policyA), + newEntry: bucketEntryWithPolicy("a", policyA), + }, + { + name: "policy removed", + oldEntry: bucketEntryWithPolicy("a", policyA), + newEntry: bucketEntryWithPolicy("a", ""), + removeName: "a", + }, + { + name: "bucket deleted", + oldEntry: bucketEntryWithPolicy("a", policyA), + removeName: "a", + }, + { + name: "bucket deleted without policy", + oldEntry: bucketEntryWithPolicy("a", ""), + }, + { + // The rename case: same policy bytes on both sides must still + // move the mirror to the new name. + name: "renamed with unchanged policy", + oldEntry: bucketEntryWithPolicy("a", policyA), + newEntry: bucketEntryWithPolicy("b", policyA), + removeName: "a", + updateName: "b", + updatePolicy: policyA, + }, + { + name: "renamed with changed policy", + oldEntry: bucketEntryWithPolicy("a", policyA), + newEntry: bucketEntryWithPolicy("b", policyB), + removeName: "a", + updateName: "b", + updatePolicy: policyB, + }, + { + name: "renamed and policy dropped", + oldEntry: bucketEntryWithPolicy("a", policyA), + newEntry: bucketEntryWithPolicy("b", ""), + removeName: "a", + }, + { + name: "renamed without prior policy", + oldEntry: bucketEntryWithPolicy("a", ""), + newEntry: bucketEntryWithPolicy("b", policyA), + updateName: "b", + updatePolicy: policyA, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + removeName, updateName, updatePolicy := bucketPolicyMirrorOps(tc.oldEntry, tc.newEntry) + if removeName != tc.removeName { + t.Errorf("removeName = %q, want %q", removeName, tc.removeName) + } + if updateName != tc.updateName { + t.Errorf("updateName = %q, want %q", updateName, tc.updateName) + } + if string(updatePolicy) != tc.updatePolicy { + t.Errorf("updatePolicy = %q, want %q", updatePolicy, tc.updatePolicy) + } + }) + } +} diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index dd60b8d59..704200f34 100644 --- a/weed/s3api/s3api_server.go +++ b/weed/s3api/s3api_server.go @@ -358,6 +358,15 @@ func NewS3ApiServerWithStore(router *mux.Router, option *S3ApiServerOption, expl // policy conditions on s3:x-amz-server-side-encryption evaluate correctly. policyEngine.MultipartSSELookup = s3ApiServer.getMultipartSSEAlgorithm + // Advanced-IAM authorization evaluates the bucket-policy: mirror + // before any handler runs, so the auth path has to be what triggers the + // lazy bucket load (and with it the mirror backfill): a grant carried + // only by a not-yet-mirrored policy would otherwise deny forever, and + // the denied request never reaches the handlers that load the bucket. + iam.primeBucketForIAM = func(bucket string) { + s3ApiServer.getBucketConfig(bucket) + } + // Initialize advanced IAM system if config is provided or explicitly enabled if option.IamConfig != "" || option.EnableIam { configSource := "defaults" diff --git a/weed/s3api/s3err/s3api_errors.go b/weed/s3api/s3err/s3api_errors.go index 87ffaeb6f..0d5afa13f 100644 --- a/weed/s3api/s3err/s3api_errors.go +++ b/weed/s3api/s3err/s3api_errors.go @@ -95,6 +95,7 @@ const ( ErrMalformedCredentialDate ErrMalformedPolicy ErrInvalidPolicyDocument + ErrPolicyTooLarge ErrMissingSignHeadersTag ErrMissingSignTag ErrUnsignedHeaders @@ -389,6 +390,11 @@ var errorCodeResponse = map[ErrorCode]APIError{ Description: "Policy has invalid resource.", HTTPStatusCode: http.StatusBadRequest, }, + ErrPolicyTooLarge: { + Code: "PolicyTooLarge", + Description: "Policy exceeds the maximum allowed document size.", + HTTPStatusCode: http.StatusBadRequest, + }, ErrInvalidPolicyDocument: { Code: "InvalidPolicyDocument", Description: "The content of the policy document is invalid.",