From c1636ac41c9cab7ae1ce548a27888ec1582c129a Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Sun, 14 Jun 2026 13:55:11 -0700 Subject: [PATCH] s3: give STS sessions a distinct owner account instead of admin (#9963) * s3: give STS sessions a distinct owner account, not admin STS sessions were built with Account: &AccountAdmin, so every assumed-role session shared the admin account for ownership and ACL checks. Use the assumed-role user as the account id instead, matching the JWT auth path. Session permissions are unchanged: they come from the session policies, and admin is granted only through Actions. * s3: resolve STS session identity to the OIDC subject Use sessionInfo.Subject (falling back to the assumed-role user when absent) for the session identity name and account id, so the SigV4 and JWT auth paths resolve the same session to the same identity instead of diverging on AssumedRoleUser vs Subject. * s3: trim verbose comments --- weed/s3api/auth_signature_v4.go | 18 ++++---- weed/s3api/auth_signature_v4_sts_test.go | 52 ++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 9 deletions(-) diff --git a/weed/s3api/auth_signature_v4.go b/weed/s3api/auth_signature_v4.go index a6a763f78..79e19c33a 100644 --- a/weed/s3api/auth_signature_v4.go +++ b/weed/s3api/auth_signature_v4.go @@ -433,22 +433,22 @@ func (iam *IdentityAccessManagement) validateSTSSessionToken(r *http.Request, se claims[k] = v } - // Create an identity for the STS session - // The identity represents the assumed role user + // Key the principal on the OIDC subject so a session resolves to the same + // identity on the SigV4 and JWT paths (see s3_iam_middleware.go); fall back to + // the assumed-role user when absent. Distinct per principal, not shared admin. + principal := sessionInfo.Subject + if principal == "" { + principal = sessionInfo.AssumedRoleUser + } identity := &Identity{ - Name: sessionInfo.AssumedRoleUser, // Use the assumed role user as the identity name - Account: &AccountAdmin, // STS sessions use admin account + Name: principal, + Account: &Account{Id: principal, DisplayName: sessionInfo.SessionName, EmailAddress: principal + "@seaweedfs.local"}, Credentials: []*Credential{cred}, PrincipalArn: sessionInfo.Principal, PolicyNames: sessionInfo.Policies, // Populate PolicyNames for IAM authorization Claims: claims, // Populate Claims for policy variable substitution } - // Restore admin privileges if the session was created by an admin - // if isAdmin, ok := claims["is_admin"].(bool); ok && isAdmin { - // identity.Actions = append(identity.Actions, s3_constants.ACTION_ADMIN) - // } - glog.V(2).Infof("Successfully validated STS session token for principal: %s, assumed role user: %s", sessionInfo.Principal, sessionInfo.AssumedRoleUser) return identity, cred, s3err.ErrNone diff --git a/weed/s3api/auth_signature_v4_sts_test.go b/weed/s3api/auth_signature_v4_sts_test.go index 1e372b199..24a1c3051 100644 --- a/weed/s3api/auth_signature_v4_sts_test.go +++ b/weed/s3api/auth_signature_v4_sts_test.go @@ -4,6 +4,7 @@ import ( "context" "net/http" "testing" + "time" "github.com/gorilla/mux" "github.com/stretchr/testify/assert" @@ -14,11 +15,59 @@ import ( "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" ) +// An STS session owns resources as its own principal (OIDC subject, or the +// assumed-role user when absent), not the shared admin account. +func TestValidateSTSSessionTokenAssignsDistinctAccount(t *testing.T) { + cases := []struct { + name string + subject string + assumedRoleUser string + wantPrincipal string + }{ + {"prefers the subject", "alice", "ReadOnlyRole/alice", "alice"}, + {"falls back to the assumed-role user", "", "ReadOnlyRole/bob", "ReadOnlyRole/bob"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + iam := &IdentityAccessManagement{ + iamIntegration: &MockIAMIntegration{ + validateSessionFunc: func(ctx context.Context, token string) (*sts.SessionInfo, error) { + return &sts.SessionInfo{ + AssumedRoleUser: tc.assumedRoleUser, + Principal: "arn:aws:sts:::assumed-role/" + tc.assumedRoleUser, + Subject: tc.subject, + SessionName: "sess", + Credentials: &sts.Credentials{ + AccessKeyId: "AKIATEST", + SecretAccessKey: "secret", + }, + ExpiresAt: time.Now().Add(time.Hour), + Policies: []string{"ReadOnly"}, + }, nil + }, + }, + } + + req, err := http.NewRequest(http.MethodGet, "http://s3/test", nil) + require.NoError(t, err) + + identity, _, errCode := iam.validateSTSSessionToken(req, "session-token", "AKIATEST") + require.Equal(t, s3err.ErrNone, errCode) + require.NotNil(t, identity.Account) + assert.Equal(t, tc.wantPrincipal, identity.Account.Id, "STS session owns resources as its principal") + assert.Equal(t, tc.wantPrincipal, identity.Name) + assert.NotEqual(t, AccountAdmin.Id, identity.Account.Id, "STS session must not share the admin account") + assert.False(t, identity.isAdmin(), "an STS session has no admin action") + }) + } +} + // MockIAMIntegration is a mock implementation of IAM integration for testing type MockIAMIntegration struct { authenticateJWTFunc func(ctx context.Context, r *http.Request) (*IAMIdentity, s3err.ErrorCode) authorizeFunc func(ctx context.Context, identity *IAMIdentity, action Action, bucket, object string, r *http.Request) s3err.ErrorCode validateTrustPolicyFunc func(ctx context.Context, roleArn, principalArn string) error + validateSessionFunc func(ctx context.Context, token string) (*sts.SessionInfo, error) authCalled bool } @@ -38,6 +87,9 @@ func (m *MockIAMIntegration) AuthenticateJWT(ctx context.Context, r *http.Reques } func (m *MockIAMIntegration) ValidateSessionToken(ctx context.Context, token string) (*sts.SessionInfo, error) { + if m.validateSessionFunc != nil { + return m.validateSessionFunc(ctx, token) + } return nil, nil // Not needed for these tests }