Files
seaweedfs/test/s3/iam/s3_service_account_test.go
Chris Lu e77f8ae204 fix(s3api): route STS GetFederationToken to STS handler (#9157) (#9167)
* 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:<parent>:<uuid> 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).
2026-04-20 19:33:22 -07:00

380 lines
12 KiB
Go

package iam
import (
"encoding/xml"
"fmt"
"io"
"net/http"
"net/url"
"os"
"strings"
"testing"
"time"
"github.com/aws/aws-sdk-go/aws"
"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"
)
// Service Account API test constants
const (
TestIAMEndpoint = "http://localhost:8333"
)
// ServiceAccountInfo represents the response structure for service account operations
type ServiceAccountInfo struct {
ServiceAccountId string `xml:"ServiceAccountId"`
ParentUser string `xml:"ParentUser"`
Description string `xml:"Description,omitempty"`
AccessKeyId string `xml:"AccessKeyId"`
SecretAccessKey string `xml:"SecretAccessKey,omitempty"`
Status string `xml:"Status"`
Expiration string `xml:"Expiration,omitempty"`
CreateDate string `xml:"CreateDate"`
}
// CreateServiceAccountResponse represents the response for CreateServiceAccount
type CreateServiceAccountResponse struct {
XMLName xml.Name `xml:"CreateServiceAccountResponse"`
CreateServiceAccountResult struct {
ServiceAccount ServiceAccountInfo `xml:"ServiceAccount"`
} `xml:"CreateServiceAccountResult"`
}
// ListServiceAccountsResponse represents the response for ListServiceAccounts
type ListServiceAccountsResponse struct {
XMLName xml.Name `xml:"ListServiceAccountsResponse"`
ListServiceAccountsResult struct {
ServiceAccounts []ServiceAccountInfo `xml:"ServiceAccounts>member"`
IsTruncated bool `xml:"IsTruncated"`
} `xml:"ListServiceAccountsResult"`
}
// GetServiceAccountResponse represents the response for GetServiceAccount
type GetServiceAccountResponse struct {
XMLName xml.Name `xml:"GetServiceAccountResponse"`
GetServiceAccountResult struct {
ServiceAccount ServiceAccountInfo `xml:"ServiceAccount"`
} `xml:"GetServiceAccountResult"`
}
// TestServiceAccountLifecycle tests the complete lifecycle of service accounts
// This is a high-value test covering Create, Get, List, Update, Delete operations
func TestServiceAccountLifecycle(t *testing.T) {
if testing.Short() {
t.Skip("Skipping integration test in short mode")
}
// Check if SeaweedFS is running
if !isSeaweedFSRunning(t) {
t.Skip("SeaweedFS is not running at", TestIAMEndpoint)
}
// First, ensure the parent user exists
parentUserName := fmt.Sprintf("testuser-%d", time.Now().UnixNano())
t.Run("create_parent_user", func(t *testing.T) {
resp, err := callIAMAPI(t, "CreateUser", url.Values{
"UserName": {parentUserName},
})
require.NoError(t, err)
defer resp.Body.Close()
assert.Equal(t, http.StatusOK, resp.StatusCode, "CreateUser should succeed")
})
// Store service account IDs for cleanup
var createdServiceAccounts []string
defer func() {
// Cleanup: delete all service accounts first, then parent user
for _, saId := range createdServiceAccounts {
resp, _ := callIAMAPI(t, "DeleteServiceAccount", url.Values{
"ServiceAccountId": {saId},
})
if resp != nil {
resp.Body.Close()
}
}
// Now delete the parent user
resp, _ := callIAMAPI(t, "DeleteUser", url.Values{
"UserName": {parentUserName},
})
if resp != nil {
resp.Body.Close()
}
}()
var createdSAId string
var createdAccessKeyId string
var createdSecretAccessKey string
t.Run("create_service_account", func(t *testing.T) {
resp, err := callIAMAPI(t, "CreateServiceAccount", url.Values{
"ParentUser": {parentUserName},
"Description": {"Test service account for CI/CD"},
})
require.NoError(t, err)
defer resp.Body.Close()
assert.Equal(t, http.StatusOK, resp.StatusCode, "CreateServiceAccount should succeed")
body, err := io.ReadAll(resp.Body)
require.NoError(t, err)
var createResp CreateServiceAccountResponse
err = xml.Unmarshal(body, &createResp)
require.NoError(t, err)
sa := createResp.CreateServiceAccountResult.ServiceAccount
createdSAId = sa.ServiceAccountId
createdAccessKeyId = sa.AccessKeyId
createdSecretAccessKey = sa.SecretAccessKey
// Add to cleanup list
createdServiceAccounts = append(createdServiceAccounts, createdSAId)
assert.NotEmpty(t, createdSAId, "ServiceAccountId should not be empty")
assert.Equal(t, parentUserName, sa.ParentUser, "ParentUser should match")
assert.Equal(t, "Test service account for CI/CD", sa.Description)
assert.Equal(t, "Active", sa.Status)
assert.NotEmpty(t, sa.AccessKeyId, "AccessKeyId should not be empty")
assert.NotEmpty(t, sa.SecretAccessKey, "SecretAccessKey should be returned on create")
assert.True(t, strings.HasPrefix(sa.AccessKeyId, "ABIA"),
"Service account AccessKeyId should have ABIA prefix")
t.Logf("Created service account: ID=%s, AccessKeyId=%s", createdSAId, createdAccessKeyId)
})
t.Run("get_service_account", func(t *testing.T) {
require.NotEmpty(t, createdSAId, "Service account should have been created")
resp, err := callIAMAPI(t, "GetServiceAccount", url.Values{
"ServiceAccountId": {createdSAId},
})
require.NoError(t, err)
defer resp.Body.Close()
assert.Equal(t, http.StatusOK, resp.StatusCode)
body, err := io.ReadAll(resp.Body)
require.NoError(t, err)
var getResp GetServiceAccountResponse
err = xml.Unmarshal(body, &getResp)
require.NoError(t, err)
sa := getResp.GetServiceAccountResult.ServiceAccount
assert.Equal(t, createdSAId, sa.ServiceAccountId)
assert.Equal(t, parentUserName, sa.ParentUser)
assert.Empty(t, sa.SecretAccessKey, "SecretAccessKey should not be returned on Get")
})
t.Run("list_service_accounts", func(t *testing.T) {
resp, err := callIAMAPI(t, "ListServiceAccounts", url.Values{
"ParentUser": {parentUserName},
})
require.NoError(t, err)
defer resp.Body.Close()
assert.Equal(t, http.StatusOK, resp.StatusCode)
body, err := io.ReadAll(resp.Body)
require.NoError(t, err)
var listResp ListServiceAccountsResponse
err = xml.Unmarshal(body, &listResp)
require.NoError(t, err)
assert.GreaterOrEqual(t, len(listResp.ListServiceAccountsResult.ServiceAccounts), 1,
"Should have at least one service account for the parent user")
})
t.Run("update_service_account_status", func(t *testing.T) {
require.NotEmpty(t, createdSAId)
// Disable the service account
resp, err := callIAMAPI(t, "UpdateServiceAccount", url.Values{
"ServiceAccountId": {createdSAId},
"Status": {"Inactive"},
})
require.NoError(t, err)
defer resp.Body.Close()
assert.Equal(t, http.StatusOK, resp.StatusCode)
// Verify it's now inactive
getResp, err := callIAMAPI(t, "GetServiceAccount", url.Values{
"ServiceAccountId": {createdSAId},
})
require.NoError(t, err)
defer getResp.Body.Close()
body, err := io.ReadAll(getResp.Body)
require.NoError(t, err)
var result GetServiceAccountResponse
err = xml.Unmarshal(body, &result)
require.NoError(t, err, "Failed to parse response: %s", string(body))
assert.Equal(t, "Inactive", result.GetServiceAccountResult.ServiceAccount.Status)
})
// Test that credentials could be used (verify they work with AWS SDK)
// This must run BEFORE delete_service_account to use valid credentials
t.Run("use_service_account_credentials", func(t *testing.T) {
require.NotEmpty(t, createdAccessKeyId)
require.NotEmpty(t, createdSecretAccessKey)
sess, err := session.NewSession(&aws.Config{
Region: aws.String("us-east-1"),
Endpoint: aws.String(TestIAMEndpoint), // IAM and S3 usually on same port in mini-seaweed
Credentials: credentials.NewStaticCredentials(
createdAccessKeyId,
createdSecretAccessKey,
"",
),
DisableSSL: aws.Bool(true),
S3ForcePathStyle: aws.Bool(true),
})
require.NoError(t, err)
s3Client := s3.New(sess)
_, err = s3Client.ListBuckets(&s3.ListBucketsInput{})
// Note: we don't necessarily expect success if no buckets/permissions
// but we expect it not to fail with "InvalidAccessKeyId" or "SignatureDoesNotMatch"
if err != nil {
if aerr, ok := err.(awserr.Error); ok {
assert.NotEqual(t, "InvalidAccessKeyId", aerr.Code(), "Credentials should be valid")
assert.NotEqual(t, "SignatureDoesNotMatch", aerr.Code(), "Signature should be valid")
}
}
})
t.Run("delete_service_account", func(t *testing.T) {
require.NotEmpty(t, createdSAId)
resp, err := callIAMAPI(t, "DeleteServiceAccount", url.Values{
"ServiceAccountId": {createdSAId},
})
require.NoError(t, err)
defer resp.Body.Close()
assert.Equal(t, http.StatusOK, resp.StatusCode)
// Verify it no longer exists
getResp, err := callIAMAPI(t, "GetServiceAccount", url.Values{
"ServiceAccountId": {createdSAId},
})
require.NoError(t, err)
defer getResp.Body.Close()
// Should return an error (not found)
assert.NotEqual(t, http.StatusOK, getResp.StatusCode,
"GetServiceAccount should fail after deletion")
})
}
// TestServiceAccountValidation tests validation of service account operations
func TestServiceAccountValidation(t *testing.T) {
if testing.Short() {
t.Skip("Skipping integration test in short mode")
}
if !isSeaweedFSRunning(t) {
t.Skip("SeaweedFS is not running at", TestIAMEndpoint)
}
t.Run("create_without_parent_user", func(t *testing.T) {
resp, err := callIAMAPI(t, "CreateServiceAccount", url.Values{
"Description": {"Test without parent"},
})
require.NoError(t, err)
defer resp.Body.Close()
assert.NotEqual(t, http.StatusOK, resp.StatusCode,
"CreateServiceAccount without ParentUser should fail")
})
t.Run("create_with_nonexistent_parent", func(t *testing.T) {
resp, err := callIAMAPI(t, "CreateServiceAccount", url.Values{
"ParentUser": {"nonexistent-user-12345"},
"Description": {"Test with nonexistent parent"},
})
require.NoError(t, err)
defer resp.Body.Close()
assert.NotEqual(t, http.StatusOK, resp.StatusCode,
"CreateServiceAccount with nonexistent parent should fail")
})
t.Run("get_nonexistent_service_account", func(t *testing.T) {
resp, err := callIAMAPI(t, "GetServiceAccount", url.Values{
"ServiceAccountId": {"sa-NONEXISTENT123"},
})
require.NoError(t, err)
defer resp.Body.Close()
assert.NotEqual(t, http.StatusOK, resp.StatusCode,
"GetServiceAccount for nonexistent ID should fail")
})
t.Run("delete_nonexistent_service_account", func(t *testing.T) {
resp, err := callIAMAPI(t, "DeleteServiceAccount", url.Values{
"ServiceAccountId": {"sa-NONEXISTENT123"},
})
require.NoError(t, err)
defer resp.Body.Close()
assert.NotEqual(t, http.StatusOK, resp.StatusCode,
"DeleteServiceAccount for nonexistent ID should fail")
})
}
// 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(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)
}
// isSeaweedFSRunning checks if SeaweedFS S3 API is running
func isSeaweedFSRunning(t *testing.T) bool {
client := &http.Client{Timeout: 2 * time.Second}
resp, err := client.Get(TestIAMEndpoint + "/status")
if err != nil {
return false
}
defer resp.Body.Close()
return resp.StatusCode == http.StatusOK
}