Browse Source

address comments

pull/7160/head
chrislu 1 month ago
parent
commit
89810dcea3
  1. 3
      test/s3/iam/STS_DISTRIBUTED.md
  2. 10
      weed/iam/sts/sts_service.go
  3. 29
      weed/s3api/s3_iam_middleware.go
  4. 57
      weed/s3api/s3_token_differentiation_test.go

3
test/s3/iam/STS_DISTRIBUTED.md

@ -264,8 +264,7 @@ services:
6. STS generates temporary credentials
- Creates temporary access key, secret key, session token
- Session token is signed JWT with session ID
- Stores session info in distributed session store
- Session token is signed JWT with all session information embedded (stateless)
7. User receives temporary credentials
{

10
weed/iam/sts/sts_service.go

@ -15,7 +15,7 @@ import (
// in JWT tokens, eliminating the need for session storage and enabling true
// distributed operation without shared state
type STSService struct {
config *STSConfig
Config *STSConfig // Public for access by other components
initialized bool
providers map[string]providers.IdentityProvider
issuerToProvider map[string]providers.IdentityProvider // Efficient issuer-based provider lookup
@ -199,7 +199,7 @@ func (s *STSService) Initialize(config *STSConfig) error {
return fmt.Errorf("invalid STS configuration: %w", err)
}
s.config = config
s.Config = config
// Initialize token generator for stateless JWT operations
s.tokenGenerator = NewTokenGenerator(config.SigningKey, config.Issuer)
@ -366,7 +366,7 @@ func (s *STSService) AssumeRoleWithWebIdentity(ctx context.Context, request *Ass
}
// Create rich JWT claims with all session information
sessionClaims := NewSTSSessionClaims(sessionId, s.config.Issuer, expiresAt).
sessionClaims := NewSTSSessionClaims(sessionId, s.Config.Issuer, expiresAt).
WithSessionName(request.RoleSessionName).
WithRoleInfo(request.RoleArn, assumedRoleUser.Arn, assumedRoleUser.Arn).
WithIdentityProvider(provider.Name(), externalIdentity.UserID, "").
@ -445,7 +445,7 @@ func (s *STSService) AssumeRoleWithCredentials(ctx context.Context, request *Ass
}
// Create rich JWT claims with all session information
sessionClaims := NewSTSSessionClaims(sessionId, s.config.Issuer, expiresAt).
sessionClaims := NewSTSSessionClaims(sessionId, s.Config.Issuer, expiresAt).
WithSessionName(request.RoleSessionName).
WithRoleInfo(request.RoleArn, assumedRoleUser.Arn, assumedRoleUser.Arn).
WithIdentityProvider(provider.Name(), externalIdentity.UserID, "").
@ -621,7 +621,7 @@ func (s *STSService) calculateSessionDuration(durationSeconds *int64) time.Durat
}
// Use default from config
return s.config.TokenDuration
return s.Config.TokenDuration
}
// extractSessionIdFromToken extracts session ID from JWT session token

29
weed/s3api/s3_iam_middleware.go

@ -532,31 +532,14 @@ func getProviderNames(providers map[string]providers.IdentityProvider) []string
}
// isSTSIssuer determines if an issuer belongs to the STS service
// This provides more robust token type detection than checking for role claims
// Uses exact match against configured STS issuer for security and correctness
func (s3iam *S3IAMIntegration) isSTSIssuer(issuer string) bool {
if s3iam.stsService == nil {
if s3iam.stsService == nil || s3iam.stsService.Config == nil {
return false
}
// Get the STS configuration to check the issuer
// For now, we'll use a simple heuristic - STS issuers typically contain "sts" or are internal
// In a full implementation, this could check against configured STS issuer values
// Check if issuer contains common STS patterns
stsPatterns := []string{
"sts",
"seaweedfs-sts",
"seaweed",
"localhost", // For development/testing
}
lowerIssuer := strings.ToLower(issuer)
for _, pattern := range stsPatterns {
if strings.Contains(lowerIssuer, pattern) {
return true
}
}
// If no pattern matches, assume it's an external OIDC issuer
return false
// Directly compare with the configured STS issuer for exact match
// This prevents false positives from external OIDC providers that might
// contain STS-related keywords in their issuer URLs
return issuer == s3iam.stsService.Config.Issuer
}

57
weed/s3api/s3_token_differentiation_test.go

@ -1,7 +1,9 @@
package s3api
import (
"strings"
"testing"
"time"
"github.com/seaweedfs/seaweedfs/weed/iam/integration"
"github.com/seaweedfs/seaweedfs/weed/iam/sts"
@ -9,10 +11,23 @@ import (
)
func TestS3IAMIntegration_isSTSIssuer(t *testing.T) {
// Create test STS service
// Create test STS service with configuration
stsService := sts.NewSTSService()
// Create S3IAM integration with STS service
// Set up STS configuration with a specific issuer
testIssuer := "https://seaweedfs-prod.company.com/sts"
stsConfig := &sts.STSConfig{
Issuer: testIssuer,
SigningKey: []byte("test-signing-key-32-characters-long"),
TokenDuration: time.Hour,
MaxSessionLength: 12 * time.Hour, // Required field
}
// Initialize STS service with config (this sets the Config field)
err := stsService.Initialize(stsConfig)
assert.NoError(t, err)
// Create S3IAM integration with configured STS service
s3iam := &S3IAMIntegration{
iamManager: &integration.IAMManager{}, // Mock
stsService: stsService,
@ -25,33 +40,33 @@ func TestS3IAMIntegration_isSTSIssuer(t *testing.T) {
issuer string
expected bool
}{
// STS issuers (should return true)
// Only exact match should return true
{
name: "explicit STS issuer",
issuer: "seaweedfs-sts",
name: "exact match with configured issuer",
issuer: testIssuer,
expected: true,
},
// All other issuers should return false (exact matching)
{
name: "STS in issuer name",
issuer: "https://mycompany-sts.example.com",
expected: true,
name: "similar but not exact issuer",
issuer: "https://seaweedfs-prod.company.com/sts2",
expected: false,
},
{
name: "seaweed in issuer name",
issuer: "https://seaweed-prod.company.com",
expected: true,
name: "substring of configured issuer",
issuer: "seaweedfs-prod.company.com",
expected: false,
},
{
name: "localhost for development",
issuer: "http://localhost:9333/sts",
expected: true,
name: "contains configured issuer as substring",
issuer: "prefix-" + testIssuer + "-suffix",
expected: false,
},
{
name: "case insensitive STS",
issuer: "SEAWEEDFS-STS-PROD",
expected: true,
name: "case sensitive - different case",
issuer: strings.ToUpper(testIssuer),
expected: false,
},
// External OIDC issuers (should return false)
{
name: "Google OIDC",
issuer: "https://accounts.google.com",
@ -73,8 +88,8 @@ func TestS3IAMIntegration_isSTSIssuer(t *testing.T) {
expected: false,
},
{
name: "Generic OIDC provider",
issuer: "https://oidc.provider.com",
name: "Empty string",
issuer: "",
expected: false,
},
}
@ -82,7 +97,7 @@ func TestS3IAMIntegration_isSTSIssuer(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := s3iam.isSTSIssuer(tt.issuer)
assert.Equal(t, tt.expected, result, "isSTSIssuer should correctly identify issuer type")
assert.Equal(t, tt.expected, result, "isSTSIssuer should use exact matching against configured issuer")
})
}
}

Loading…
Cancel
Save