diff --git a/weed/iamapi/iamapi_management_handlers.go b/weed/iamapi/iamapi_management_handlers.go index 1e4b9c404..8411ebb5c 100644 --- a/weed/iamapi/iamapi_management_handlers.go +++ b/weed/iamapi/iamapi_management_handlers.go @@ -458,17 +458,33 @@ func (iama *IamApiServer) GetUserPolicy(s3cfg *iam_pb.S3ApiConfiguration, values policyDocument := policy_engine.PolicyDocument{Version: policyDocumentVersion} statements := make(map[string][]string) + seenAction := make(map[string]map[string]bool) for _, action := range ident.Actions { - // parse "Read:EXAMPLE-BUCKET" - act := strings.Split(action, ":") + // parse "Read:EXAMPLE-BUCKET" or "Read:EXAMPLE-BUCKET/prefix/*" + // Use SplitN so the path component (which may contain ':') is preserved intact. + act := strings.SplitN(action, ":", 2) resource := "*" if len(act) == 2 { - resource = fmt.Sprintf("arn:aws:s3:::%s/*", act[1]) + // Preserve the stored path verbatim so bucket-level and + // object-level resources remain distinguishable. GetActions + // stores the path exactly as parsed from the original ARN + // (e.g. "b-le*" for the bucket, "b-le*/*" for objects), and + // reconstruction should not rewrite one into the other. + resource = fmt.Sprintf("arn:aws:s3:::%s", act[1]) } - statements[resource] = append(statements[resource], - fmt.Sprintf("s3:%s", MapToIdentitiesAction(act[0])), - ) + s3Action := fmt.Sprintf("s3:%s", MapToIdentitiesAction(act[0])) + // Dedupe actions per resource: the Read/Write/List internal verbs map to + // coarse wildcards (s3:Get*, s3:Put*, s3:List*), so multiple distinct + // original actions can collapse to the same reconstructed verb. + if seenAction[resource] == nil { + seenAction[resource] = make(map[string]bool) + } + if seenAction[resource][s3Action] { + continue + } + seenAction[resource][s3Action] = true + statements[resource] = append(statements[resource], s3Action) } for resource, actions := range statements { isEqAction := false diff --git a/weed/iamapi/iamapi_management_handlers_test.go b/weed/iamapi/iamapi_management_handlers_test.go index 507631f46..478bb41c5 100644 --- a/weed/iamapi/iamapi_management_handlers_test.go +++ b/weed/iamapi/iamapi_management_handlers_test.go @@ -186,6 +186,77 @@ func TestPutGetUserPolicyPreservesStatements(t *testing.T) { assert.True(t, deleteObjectFound, "s3:DeleteObject action was lost") } +// TestPutGetUserPolicyIssue9008 is a regression test for +// https://github.com/seaweedfs/seaweedfs/issues/9008: put-user-policy followed +// by get-user-policy must return the same policy document that was submitted, +// with Action and Resource lists intact (no duplication, no collapsing). +func TestPutGetUserPolicyIssue9008(t *testing.T) { + s3cfg := &iam_pb.S3ApiConfiguration{ + Identities: []*iam_pb.Identity{{Name: "steward"}}, + } + policyJSON := `{ + "Version": "2012-10-17", + "Statement": [{ + "Effect": "Allow", + "Action": ["s3:GetObject", "s3:PutObject", "s3:ListBucket"], + "Resource": ["arn:aws:s3:::b-le*", "arn:aws:s3:::b-le*/*"] + }] + }` + + mockCfg := &mockIamS3ApiConfig{} + iama := &IamApiServer{s3ApiConfig: mockCfg} + _, iamErr := iama.PutUserPolicy(s3cfg, url.Values{ + "UserName": []string{"steward"}, + "PolicyName": []string{"steward_policy"}, + "PolicyDocument": []string{policyJSON}, + }) + assert.Nil(t, iamErr) + + // Part 1: verbatim round-trip. GetUserPolicy returns the exact document + // that was persisted, with Action and Resource lists intact. + resp, iamErr := iama.GetUserPolicy(s3cfg, url.Values{ + "UserName": []string{"steward"}, + "PolicyName": []string{"steward_policy"}, + }) + assert.Nil(t, iamErr) + + var got policy_engine.PolicyDocument + assert.NoError(t, json.Unmarshal([]byte(resp.GetUserPolicyResult.PolicyDocument), &got)) + + assert.Equal(t, "2012-10-17", got.Version) + assert.Equal(t, 1, len(got.Statement)) + stmt := got.Statement[0] + assert.Equal(t, policy_engine.PolicyEffectAllow, stmt.Effect) + assert.ElementsMatch(t, []string{"s3:GetObject", "s3:PutObject", "s3:ListBucket"}, stmt.Action.Strings()) + assert.ElementsMatch(t, []string{"arn:aws:s3:::b-le*", "arn:aws:s3:::b-le*/*"}, stmt.Resource.Strings()) + + // Part 2: fallback reconstruction. Clear the persisted inline policy so + // GetUserPolicy must rebuild the document from ident.Actions. The fallback + // is lossy (distinct S3 verbs collapse to wildcards like s3:Get*), but it + // must not duplicate actions nor conflate bucket-level and object-level + // resources. + mockCfg.policies = Policies{} + + resp, iamErr = iama.GetUserPolicy(s3cfg, url.Values{ + "UserName": []string{"steward"}, + "PolicyName": []string{"steward_policy"}, + }) + assert.Nil(t, iamErr) + + var fallback policy_engine.PolicyDocument + assert.NoError(t, json.Unmarshal([]byte(resp.GetUserPolicyResult.PolicyDocument), &fallback)) + assert.Equal(t, 1, len(fallback.Statement), "fallback should merge equal-action statements") + fstmt := fallback.Statement[0] + + // Each coarse verb appears exactly once (no duplication from the + // Read/Write/List -> s3:Get*/s3:Put*/s3:List* expansion). + assert.ElementsMatch(t, []string{"s3:Get*", "s3:Put*", "s3:List*"}, fstmt.Action.Strings()) + + // Bucket-level and object-level resources stay distinct — the bare bucket + // pattern must not be rewritten to an object ARN. + assert.ElementsMatch(t, []string{"arn:aws:s3:::b-le*", "arn:aws:s3:::b-le*/*"}, fstmt.Resource.Strings()) +} + func TestMultipleInlinePoliciesAggregateActions(t *testing.T) { s3cfg := &iam_pb.S3ApiConfiguration{ Identities: []*iam_pb.Identity{{Name: "alice"}},