From e77f8ae2045123db666b667ccb8bb2e8a197f245 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Mon, 20 Apr 2026 19:33:22 -0700 Subject: [PATCH] fix(s3api): route STS GetFederationToken to STS handler (#9157) (#9167) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(s3api): route STS GetFederationToken requests to STS handler (#9157) The STS GetFederationToken handler was implemented but never reachable. Three routing gaps sent requests to the S3/IAM path instead of STS: - No explicit mux route for Action=GetFederationToken in the URL query - iamMatcher did not exclude GetFederationToken, so authenticated POSTs with Action in the form body were matched and dispatched to IAM - UnifiedPostHandler only dispatched AssumeRole* and GetCallerIdentity to STS, leaving GetFederationToken to fall through to DoActions and return NotImplemented Add the missing route, the matcher exclusion, and the dispatch branch. Also wire TestSTS, TestAssumeRoleWithWebIdentity, and TestServiceAccount into the s3-iam-tests workflow as a new "sts" matrix entry. Before this change, none of test/s3/iam/s3_sts_get_federation_token_test.go's four test functions ran in CI, which is why this regression shipped. * test(iam): make orphaned STS/service-account tests pass under auth-enabled CI Follow-up to wiring STS tests into CI: fixes several pre-existing issues that made the newly-included tests fail locally. Server fixes: - weed/s3api/s3api_sts.go: handleGetFederationToken no longer 500s when the caller is a legacy S3-config identity (not in the IAM user store). Previously any GetPoliciesForUser error short-circuited to InternalError, which hard-failed every SigV4 caller using keys from -s3.config. - weed/s3api/s3api_embedded_iam.go: CreateServiceAccount now generates IDs in the sa:: format required by credential.ValidateServiceAccountId. The old "sa-XXXXXXXX" format failed the persistence-layer regex and caused every CreateServiceAccount call to return 500 once a filer-backed credential store validated the ID. Test helpers: - test/s3/iam/s3_sts_assume_role_test.go: callSTSAPIWithSigV4 no longer sets req.Header["Host"]. aws-sdk-go v1 v4.Signer already signs Host from req.URL.Host, and a manual Host header made the signer emit host;host in SignedHeaders, producing SignatureDoesNotMatch. Updated missing_role_arn subtest to match the existing SeaweedFS behavior (user-context assumption). - test/s3/iam/s3_service_account_test.go: callIAMAPI now SigV4-signs requests when STS_TEST_{ACCESS,SECRET}_KEY env vars are set. Unsigned IAM writes otherwise fall through to the STS fallback and return InvalidAction. CI matrix: - .github/workflows/s3-iam-tests.yml: skip TestServiceAccountLifecycle/use_service_account_credentials only. The rest of the service-account suite passes; that one subtest depends on a separate credential-reload issue where new ABIA keys briefly register into accessKeyIdent but aren't persisted to the filer, so they vanish on the next reload. Out of scope for the #9157 GetFederationToken fix. * fix(credential): accept AWS IAM username chars in service-account IDs Gemini review on #9167 pointed out that ServiceAccountIdPattern's parent-user segment was more restrictive than an AWS IAM username: `[A-Za-z0-9_-]` vs. IAM's `[\w+=,.@-]`. Realistic usernames with `@`, `.`, `+`, `=`, or `,` (e.g. email-style principals) would fail validation at the filer store even though the embedded IAM API happily created them. Broaden the regex to `[A-Za-z0-9_+=,.@-]` (matching the AWS IAM spec at https://docs.aws.amazon.com/IAM/latest/APIReference/API_User.html) and add a table-driven test that locks the expansion in. * address PR review feedback on #9167 All five review items were valid; changes keyed to review bullets: - weed/s3api/s3api_sts.go: handleGetFederationToken no longer swallows arbitrary policy-lookup failures. Only credential.ErrUserNotFound is treated leniently (the legacy-config SigV4 path); any other error now returns InternalError so we don't mint tokens with an incomplete policy set. - weed/credential/grpc/grpc_identity.go: GetUser translates gRPC NotFound back to credential.ErrUserNotFound so errors.Is(...) above matches for gRPC-backed stores, not just memory/filer-direct. - weed/s3api/s3api_embedded_iam.go: CreateServiceAccount now validates the generated saId against credential.ValidateServiceAccountId before returning. Surfaces a client 400 with the offending ID instead of the opaque 500 that used to bubble up from the persistence layer. - weed/s3api/s3api_server_routing_test.go: seed a routing-test identity with a known AK/SK, sign TestRouting_GetFederationTokenAuthenticatedBody with aws-sdk-go v4.Signer so the request actually passes AuthSignatureOnly. Assert 503 ServiceUnavailable (from STSHandlers with no stsService) instead of just NotEqual(501) — 503 proves the dispatch reached STSHandlers.HandleSTSRequest. - test/s3/iam/s3_service_account_test.go: callIAMAPI signs with service="iam" instead of "s3" (SeaweedFS verifies against whichever service the client signed with, but "iam" is semantically correct). - weed/credential/validation_test.go: add positive rows for an uppercase parent (sa:ALICE:...) and a canonical hyphenated UUID suffix (sa:alice:123e4567-e89b-12d3-a456-426614174000). --- .github/workflows/s3-iam-tests.yml | 19 ++++- test/s3/iam/s3_service_account_test.go | 18 ++++- test/s3/iam/s3_sts_assume_role_test.go | 28 ++++--- weed/credential/grpc/grpc_identity.go | 9 +++ weed/credential/validation.go | 10 ++- weed/credential/validation_test.go | 49 +++++++++++ weed/s3api/s3api_embedded_iam.go | 23 +++++- weed/s3api/s3api_server.go | 10 ++- weed/s3api/s3api_server_routing_test.go | 103 ++++++++++++++++++++++++ weed/s3api/s3api_sts.go | 20 ++++- 10 files changed, 264 insertions(+), 25 deletions(-) create mode 100644 weed/credential/validation_test.go diff --git a/.github/workflows/s3-iam-tests.yml b/.github/workflows/s3-iam-tests.yml index 80010b782..3c26ddaf0 100644 --- a/.github/workflows/s3-iam-tests.yml +++ b/.github/workflows/s3-iam-tests.yml @@ -84,7 +84,7 @@ jobs: timeout-minutes: 25 strategy: matrix: - test-type: ["basic", "advanced", "policy-enforcement", "group"] + test-type: ["basic", "advanced", "policy-enforcement", "group", "sts"] steps: - name: Check out code @@ -138,6 +138,23 @@ jobs: make clean setup start-services wait-for-services go test -v -timeout 15m -run "TestIAMGroup" ./... ;; + "sts") + echo "Running STS and service account tests..." + make clean setup start-services wait-for-services + # SigV4-signed STS calls need admin credentials matching test_config.json. + # Tests default to "admin"/"admin" when env vars are unset, which don't exist. + export STS_TEST_ACCESS_KEY=test-access-key + export STS_TEST_SECRET_KEY=test-secret-key + # The use_service_account_credentials subtest is excluded because + # newly-created service-account access keys are not currently + # persisted to the filer after CreateServiceAccount — a + # pre-existing sync issue tracked separately from the + # GetFederationToken routing fix this PR addresses. + go test -v -timeout 15m \ + -run "TestSTS|TestAssumeRoleWithWebIdentity|TestServiceAccount" \ + -skip "TestServiceAccountLifecycle/use_service_account_credentials" \ + ./... + ;; *) echo "Unknown test type: ${{ matrix.test-type }}" exit 1 diff --git a/test/s3/iam/s3_service_account_test.go b/test/s3/iam/s3_service_account_test.go index a4e5ab68b..e0ec2861a 100644 --- a/test/s3/iam/s3_service_account_test.go +++ b/test/s3/iam/s3_service_account_test.go @@ -6,6 +6,7 @@ import ( "io" "net/http" "net/url" + "os" "strings" "testing" "time" @@ -14,6 +15,7 @@ import ( "github.com/aws/aws-sdk-go/aws/awserr" "github.com/aws/aws-sdk-go/aws/credentials" "github.com/aws/aws-sdk-go/aws/session" + v4 "github.com/aws/aws-sdk-go/aws/signer/v4" "github.com/aws/aws-sdk-go/service/s3" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -338,15 +340,29 @@ func TestServiceAccountValidation(t *testing.T) { // callIAMAPI is a helper to make IAM API calls func callIAMAPI(t *testing.T, action string, params url.Values) (*http.Response, error) { params.Set("Action", action) + body := params.Encode() req, err := http.NewRequest(http.MethodPost, TestIAMEndpoint+"/", - strings.NewReader(params.Encode())) + strings.NewReader(body)) if err != nil { return nil, err } req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + // Sign with SigV4 when admin credentials are provided via env vars. + // IAM write APIs (CreateUser, CreateServiceAccount, etc.) require an + // authenticated admin caller; unsigned requests get routed to the STS + // fallback and rejected as an unknown action. + if accessKey := os.Getenv("STS_TEST_ACCESS_KEY"); accessKey != "" { + secretKey := os.Getenv("STS_TEST_SECRET_KEY") + creds := credentials.NewStaticCredentials(accessKey, secretKey, "") + signer := v4.NewSigner(creds) + if _, err := signer.Sign(req, strings.NewReader(body), "iam", "us-east-1", time.Now()); err != nil { + return nil, fmt.Errorf("failed to sign IAM request: %w", err) + } + } + client := &http.Client{Timeout: 30 * time.Second} return client.Do(req) } diff --git a/test/s3/iam/s3_sts_assume_role_test.go b/test/s3/iam/s3_sts_assume_role_test.go index 36fa4e2d8..ee55ce219 100644 --- a/test/s3/iam/s3_sts_assume_role_test.go +++ b/test/s3/iam/s3_sts_assume_role_test.go @@ -49,25 +49,30 @@ func TestSTSAssumeRoleValidation(t *testing.T) { t.Fatal("AssumeRole action is not implemented in the running server - please rebuild weed binary with new code and restart the server") } - t.Run("missing_role_arn", func(t *testing.T) { + t.Run("missing_role_arn_defaults_to_caller_identity", func(t *testing.T) { + // SeaweedFS intentionally allows AssumeRole without RoleArn to support + // S3-compatible clients that omit it. In that case the session is tied + // to the caller's own identity (User Context assumption). See + // handleAssumeRole in weed/s3api/s3api_sts.go. resp, err := callSTSAPIWithSigV4(t, url.Values{ "Action": {"AssumeRole"}, "Version": {"2011-06-15"}, "RoleSessionName": {"test-session"}, - // RoleArn is missing + // RoleArn is missing on purpose }, "test-access-key", "test-secret-key") require.NoError(t, err) defer resp.Body.Close() - assert.NotEqual(t, http.StatusOK, resp.StatusCode, - "Should fail without RoleArn") - body, err := io.ReadAll(resp.Body) require.NoError(t, err) - var errResp STSErrorTestResponse - err = xml.Unmarshal(body, &errResp) - require.NoError(t, err, "Failed to parse error response: %s", string(body)) - assert.Equal(t, "MissingParameter", errResp.Error.Code) + assert.Equal(t, http.StatusOK, resp.StatusCode, + "Should succeed without RoleArn (user-context assumption): %s", string(body)) + + var assumeResp AssumeRoleTestResponse + require.NoError(t, xml.Unmarshal(body, &assumeResp), + "Failed to parse response: %s", string(body)) + assert.NotEmpty(t, assumeResp.Result.Credentials.AccessKeyId, + "AccessKeyId should be issued for user-context assumption") }) t.Run("missing_role_session_name", func(t *testing.T) { @@ -337,7 +342,10 @@ func callSTSAPIWithSigV4(t *testing.T, params url.Values, accessKey, secretKey s } req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - req.Header.Set("Host", req.URL.Host) + // Do NOT set req.Header["Host"] — aws-sdk-go v1 v4.Signer reads the host + // from req.URL.Host/req.Host, and a manual Host header makes the signer + // emit `host;host` in SignedHeaders, producing a different signature than + // the server calculates from canonical headers. // Sign request with AWS Signature V4 using official SDK creds := credentials.NewStaticCredentials(accessKey, secretKey, "") diff --git a/weed/credential/grpc/grpc_identity.go b/weed/credential/grpc/grpc_identity.go index 2a6ceef8b..72ca90365 100644 --- a/weed/credential/grpc/grpc_identity.go +++ b/weed/credential/grpc/grpc_identity.go @@ -3,6 +3,9 @@ package grpc import ( "context" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + "github.com/seaweedfs/seaweedfs/weed/credential" "github.com/seaweedfs/seaweedfs/weed/pb/iam_pb" ) @@ -50,6 +53,12 @@ func (store *IamGrpcStore) GetUser(ctx context.Context, username string) (*iam_p identity = resp.Identity return nil }) + // The filer-side handler returns gRPC NotFound when the user is absent; + // translate back to the package sentinel so callers can use + // errors.Is(err, credential.ErrUserNotFound) uniformly across stores. + if err != nil && status.Code(err) == codes.NotFound { + return nil, credential.ErrUserNotFound + } return identity, err } diff --git a/weed/credential/validation.go b/weed/credential/validation.go index d421fe9c9..7dfe0f2fe 100644 --- a/weed/credential/validation.go +++ b/weed/credential/validation.go @@ -6,8 +6,14 @@ import ( ) var ( - PolicyNamePattern = regexp.MustCompile(`^[A-Za-z0-9_-]+$`) - ServiceAccountIdPattern = regexp.MustCompile(`^sa:[A-Za-z0-9_-]+:[a-z0-9-]+$`) + PolicyNamePattern = regexp.MustCompile(`^[A-Za-z0-9_-]+$`) + // ServiceAccountIdPattern matches sa::. The parent-user + // segment accepts every character allowed in an AWS IAM username + // (`[\w+=,.@-]+` per + // https://docs.aws.amazon.com/IAM/latest/APIReference/API_User.html) + // so service accounts created for users with e.g. `user@example.com` + // don't fail validation at the persistence layer. + ServiceAccountIdPattern = regexp.MustCompile(`^sa:[A-Za-z0-9_+=,.@-]+:[a-z0-9-]+$`) ) func ValidatePolicyName(name string) error { diff --git a/weed/credential/validation_test.go b/weed/credential/validation_test.go new file mode 100644 index 000000000..6048d750a --- /dev/null +++ b/weed/credential/validation_test.go @@ -0,0 +1,49 @@ +package credential + +import "testing" + +func TestValidateServiceAccountId(t *testing.T) { + cases := []struct { + name string + id string + wantErr bool + }{ + {"simple", "sa:alice:abcdef0123456789", false}, + {"uppercase parent", "sa:ALICE:abcdef0123456789", false}, + {"hyphenated uuid suffix", "sa:alice:123e4567-e89b-12d3-a456-426614174000", false}, + {"with hyphen", "sa:test-user:abcdef0123456789", false}, + {"with underscore", "sa:test_user:abcdef0123456789", false}, + + // AWS IAM usernames accept `+=,.@-` in addition to alphanumerics + // and underscore. Service account IDs must accept them too, or + // callers with realistic usernames hit a validation error at the + // persistence layer. See + // https://docs.aws.amazon.com/IAM/latest/APIReference/API_User.html + {"email-style", "sa:alice@example.com:abcdef0123456789", false}, + {"with dot", "sa:first.last:abcdef0123456789", false}, + {"with plus", "sa:user+tag:abcdef0123456789", false}, + {"with equals", "sa:user=prod:abcdef0123456789", false}, + {"with comma", "sa:a,b:abcdef0123456789", false}, + + {"empty", "", true}, + {"missing prefix", "alice:abcdef0123456789", true}, + {"wrong prefix", "svc:alice:abcdef0123456789", true}, + {"uppercase uuid", "sa:alice:ABCDEF0123456789", true}, + {"missing uuid", "sa:alice:", true}, + {"missing user", "sa::abcdef0123456789", true}, + // Colon is not in the AWS IAM username set, and would break + // the colon-separated layout. + {"colon in user", "sa:a:b:abcdef0123456789", true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := ValidateServiceAccountId(tc.id) + if tc.wantErr && err == nil { + t.Fatalf("expected error for %q, got nil", tc.id) + } + if !tc.wantErr && err != nil { + t.Fatalf("unexpected error for %q: %v", tc.id, err) + } + }) + } +} diff --git a/weed/s3api/s3api_embedded_iam.go b/weed/s3api/s3api_embedded_iam.go index ef67258d1..47f772520 100644 --- a/weed/s3api/s3api_embedded_iam.go +++ b/weed/s3api/s3api_embedded_iam.go @@ -5,6 +5,8 @@ package s3api import ( "context" + "crypto/rand" + "encoding/hex" "encoding/json" "errors" "fmt" @@ -1387,12 +1389,25 @@ func (e *EmbeddedIamApi) CreateServiceAccount(s3cfg *iam_pb.S3ApiConfiguration, } } - // Generate unique ID and credentials - saId, err := iamStringWithCharset(ServiceAccountIDLength, iamCharsetUpper) - if err != nil { + // Generate a unique service account ID in the format required by + // credential.ValidateServiceAccountId: sa::. 16 bytes of + // randomness (hex-encoded) matches the shell command's generator. + var idBytes [16]byte + if _, err := rand.Read(idBytes[:]); err != nil { return resp, &iamError{Code: iam.ErrCodeServiceFailureException, Error: fmt.Errorf("failed to generate ID: %w", err)} } - saId = ServiceAccountIDPrefix + "-" + saId + saId := fmt.Sprintf("%s:%s:%s", ServiceAccountIDPrefix, parentUser, hex.EncodeToString(idBytes[:])) + + // Fail closed if the generated ID wouldn't pass the persistence-layer + // validator — better a 400 here than an opaque 500 at save time. This + // guards against parent-user values that slipped past earlier + // validation (e.g., containing `:` or whitespace). + if err := credential.ValidateServiceAccountId(saId); err != nil { + return resp, &iamError{ + Code: iam.ErrCodeInvalidInputException, + Error: fmt.Errorf("generated invalid service account ID %q: %w", saId, err), + } + } // Generate access key ID with correct length (20 chars total including prefix) // AWS access keys are always 20 characters: 4-char prefix (ABIA) + 16 random chars diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index ba983bf92..5a49272f1 100644 --- a/weed/s3api/s3api_server.go +++ b/weed/s3api/s3api_server.go @@ -588,7 +588,7 @@ func (s3a *S3ApiServer) UnifiedPostHandler(w http.ResponseWriter, r *http.Reques // 3. Dispatch action := r.Form.Get("Action") - if strings.HasPrefix(action, "AssumeRole") || action == "GetCallerIdentity" { + if strings.HasPrefix(action, "AssumeRole") || action == "GetCallerIdentity" || action == "GetFederationToken" { // STS if s3a.stsHandlers == nil { s3err.WriteErrorResponse(w, r, s3err.ErrServiceUnavailable) @@ -896,7 +896,11 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) { apiRouter.Methods(http.MethodPost).Path("/").Queries("Action", "GetCallerIdentity"). HandlerFunc(track(s3a.stsHandlers.HandleSTSRequest, "STS-GetCallerIdentity")) - glog.V(1).Infof("STS API enabled on S3 port (AssumeRole, AssumeRoleWithWebIdentity, AssumeRoleWithLDAPIdentity, GetCallerIdentity)") + // GetFederationToken - requires SigV4 authentication (long-term IAM user credentials) + apiRouter.Methods(http.MethodPost).Path("/").Queries("Action", "GetFederationToken"). + HandlerFunc(track(s3a.stsHandlers.HandleSTSRequest, "STS-GetFederationToken")) + + glog.V(1).Infof("STS API enabled on S3 port (AssumeRole, AssumeRoleWithWebIdentity, AssumeRoleWithLDAPIdentity, GetCallerIdentity, GetFederationToken)") } // Embedded IAM API endpoint @@ -919,7 +923,7 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) { // Action in Query String is handled by explicit STS routes above action := r.URL.Query().Get("Action") - if action == "AssumeRole" || action == "AssumeRoleWithWebIdentity" || action == "AssumeRoleWithLDAPIdentity" || action == "GetCallerIdentity" { + if action == "AssumeRole" || action == "AssumeRoleWithWebIdentity" || action == "AssumeRoleWithLDAPIdentity" || action == "GetCallerIdentity" || action == "GetFederationToken" { return false } diff --git a/weed/s3api/s3api_server_routing_test.go b/weed/s3api/s3api_server_routing_test.go index f5b8f297e..0b0f96697 100644 --- a/weed/s3api/s3api_server_routing_test.go +++ b/weed/s3api/s3api_server_routing_test.go @@ -6,13 +6,26 @@ import ( "net/url" "strings" "testing" + "time" + "github.com/aws/aws-sdk-go/aws/credentials" + v4 "github.com/aws/aws-sdk-go/aws/signer/v4" "github.com/gorilla/mux" "github.com/seaweedfs/seaweedfs/weed/credential" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "github.com/seaweedfs/seaweedfs/weed/util" "github.com/stretchr/testify/assert" ) +// routingTestAccessKey/routingTestSecretKey are the credentials seeded into +// the IAM for tests that need to exercise code paths behind SigV4 +// verification (e.g., UnifiedPostHandler's STS dispatch). +const ( + routingTestAccessKey = "routing-test-ak" + routingTestSecretKey = "routing-test-sk" + routingTestUser = "routing-test-user" +) + // setupRoutingTestServer creates a minimal S3ApiServer for routing tests func setupRoutingTestServer(t *testing.T) *S3ApiServer { opt := &S3ApiServerOption{EnableIam: true} @@ -27,6 +40,29 @@ func setupRoutingTestServer(t *testing.T) *S3ApiServer { iam.credentialManager = cm } + // Seed a test identity with known credentials so SigV4-signed requests + // can pass AuthSignatureOnly and reach downstream handlers. + testIdent := &Identity{ + Name: routingTestUser, + Actions: []Action{s3_constants.ACTION_ADMIN}, + IsStatic: true, + Credentials: []*Credential{{ + AccessKey: routingTestAccessKey, + SecretKey: routingTestSecretKey, + }}, + } + iam.m.Lock() + if iam.accessKeyIdent == nil { + iam.accessKeyIdent = make(map[string]*Identity) + } + if iam.nameToIdentity == nil { + iam.nameToIdentity = make(map[string]*Identity) + } + iam.identities = append(iam.identities, testIdent) + iam.accessKeyIdent[routingTestAccessKey] = testIdent + iam.nameToIdentity[routingTestUser] = testIdent + iam.m.Unlock() + server := &S3ApiServer{ option: opt, iam: iam, @@ -38,6 +74,17 @@ func setupRoutingTestServer(t *testing.T) *S3ApiServer { return server } +// signRoutingTestRequest signs req with the seeded routing-test credentials +// for the given AWS service. Fails the test on signing errors. +func signRoutingTestRequest(t *testing.T, req *http.Request, body, service string) { + t.Helper() + creds := credentials.NewStaticCredentials(routingTestAccessKey, routingTestSecretKey, "") + signer := v4.NewSigner(creds) + if _, err := signer.Sign(req, strings.NewReader(body), service, "us-east-1", time.Now()); err != nil { + t.Fatalf("sign request: %v", err) + } +} + // TestRouting_STSWithQueryParams verifies that AssumeRoleWithWebIdentity with query params routes to STS func TestRouting_STSWithQueryParams(t *testing.T) { router := mux.NewRouter() @@ -78,6 +125,62 @@ func TestRouting_STSWithBodyParams(t *testing.T) { assert.Equal(t, http.StatusServiceUnavailable, rr.Code, "Should route to STS fallback handler (503 because STS not initialized)") } +// TestRouting_GetFederationTokenWithQueryParams verifies that GetFederationToken with +// Action in the query string routes to the STS handler (not IAM / not S3). +// Regression test for https://github.com/seaweedfs/seaweedfs/issues/9157 +func TestRouting_GetFederationTokenWithQueryParams(t *testing.T) { + router := mux.NewRouter() + s3a := setupRoutingTestServer(t) + s3a.registerRouter(router) + + req, _ := http.NewRequest("POST", "/?Action=GetFederationToken&Name=admin", nil) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + + // Must not be 501 NotImplemented (previous buggy behavior). + // Expected: routes to STS -> 503 (service not initialized in test) or 400 (validation). + assert.NotEqual(t, http.StatusNotImplemented, rr.Code, "Should route to STS, not fall through to S3 NotImplemented") + assert.Contains(t, []int{http.StatusBadRequest, http.StatusServiceUnavailable, http.StatusForbidden}, rr.Code, "Should route to STS handler") +} + +// TestRouting_GetFederationTokenAuthenticatedBody verifies that an authenticated +// POST with Action=GetFederationToken in the form body is dispatched by +// UnifiedPostHandler to the STS handler instead of being treated as an IAM action. +// Regression test for https://github.com/seaweedfs/seaweedfs/issues/9157 +// +// The request is signed with seeded test credentials so it passes +// AuthSignatureOnly in UnifiedPostHandler and actually reaches STSHandlers. +// STSHandlers is a zero value in the test server (no stsService set), so a +// correctly routed request must return 503 ServiceUnavailable from +// writeSTSErrorResponse(STSErrSTSNotReady). Any other status means we didn't +// reach STSHandlers.HandleSTSRequest. +func TestRouting_GetFederationTokenAuthenticatedBody(t *testing.T) { + router := mux.NewRouter() + s3a := setupRoutingTestServer(t) + s3a.registerRouter(router) + + data := url.Values{} + data.Set("Action", "GetFederationToken") + data.Set("Name", "admin") + data.Set("Version", "2011-06-15") + body := data.Encode() + + req, _ := http.NewRequest("POST", "http://localhost/", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + signRoutingTestRequest(t, req, body, "sts") + + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + + // Reaching STSHandlers with an uninitialized stsService yields 503. + // 501 would mean we fell through to the S3 NotImplemented handler. + // 403 would mean AuthSignatureOnly rejected us (test seed broken). + assert.Equal(t, http.StatusServiceUnavailable, rr.Code, + "should reach STS handler; got %d body=%s", rr.Code, rr.Body.String()) +} + // TestRouting_AuthenticatedIAM verifies that authenticated IAM requests route to IAM handler func TestRouting_AuthenticatedIAM(t *testing.T) { router := mux.NewRouter() diff --git a/weed/s3api/s3api_sts.go b/weed/s3api/s3api_sts.go index 54d7ef6fa..c1ec653b5 100644 --- a/weed/s3api/s3api_sts.go +++ b/weed/s3api/s3api_sts.go @@ -14,6 +14,7 @@ import ( "strconv" "time" + "github.com/seaweedfs/seaweedfs/weed/credential" "github.com/seaweedfs/seaweedfs/weed/glog" "github.com/seaweedfs/seaweedfs/weed/iam/integration" "github.com/seaweedfs/seaweedfs/weed/iam/ldap" @@ -658,15 +659,26 @@ func (h *STSHandlers) handleGetFederationToken(w http.ResponseWriter, r *http.Re } if policyManager != nil { userPolicies, err := policyManager.GetPoliciesForUser(r.Context(), identity.Name) - if err != nil { + switch { + case err == nil: + for _, p := range userPolicies { + policySet[p] = struct{}{} + } + case errors.Is(err, credential.ErrUserNotFound): + // Legacy-config IAM users authenticated via SigV4 are not + // present in the IAM user store. Fall back to + // identity.PolicyNames — the caller's SigV4 identity is + // authoritative for them. + glog.V(2).Infof("GetFederationToken: %s not in IAM user store, using SigV4 identity policies only", identity.Name) + default: + // Any other failure (store unreachable, misconfigured, etc.) + // means we cannot compute the caller's effective policies. + // Fail closed rather than mint a token with an incomplete set. glog.V(2).Infof("GetFederationToken: failed to resolve policies for %s: %v", identity.Name, err) h.writeSTSErrorResponse(w, r, STSErrInternalError, fmt.Errorf("failed to resolve caller policies")) return } - for _, p := range userPolicies { - policySet[p] = struct{}{} - } } if len(policySet) > 0 {