mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-09-08 15:41:15 +02:00
* 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).
380 lines
12 KiB
Go
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
|
|
}
|