From 23d424248d4ed04141d74b8fdf743cb5eabe3f23 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Mon, 31 Aug 2026 10:22:06 -0700 Subject: [PATCH] sts: session duration no longer clamped to the web identity token exp (#11048) * sts: session duration no longer clamped to the web identity token exp The assumed-role session lifetime is governed by DurationSeconds and the configured tokenDuration/maxSessionLength, matching AWS. Clamping to the already-verified token's exp made short-lived id_tokens (GitLab issues ~2-minute ones) yield unusable sessions regardless of configuration. Claude-Session: https://claude.ai/code/session_01XH7iM88ZqWMEvsLB8tkWPQ * sts: cover session duration against short-lived web identity tokens The mock OIDC provider now carries the token exp through to the identity like the real provider, so the integration test would catch the clamp. Claude-Session: https://claude.ai/code/session_01XH7iM88ZqWMEvsLB8tkWPQ --- weed/iam/integration/iam_integration_test.go | 41 ++++++++++++++++++- weed/iam/oidc/mock_provider.go | 8 +++- weed/iam/sts/sts_service.go | 42 ++++++-------------- weed/iam/sts/sts_service_test.go | 38 ++++++++++++++++++ 4 files changed, 95 insertions(+), 34 deletions(-) create mode 100644 weed/iam/sts/sts_service_test.go diff --git a/weed/iam/integration/iam_integration_test.go b/weed/iam/integration/iam_integration_test.go index bb696597e..30b477187 100644 --- a/weed/iam/integration/iam_integration_test.go +++ b/weed/iam/integration/iam_integration_test.go @@ -429,6 +429,39 @@ func TestSessionExpiration(t *testing.T) { assert.True(t, allowed, "Access should still be allowed since token hasn't naturally expired") } +// TestSessionOutlivesShortLivedToken verifies the session lifetime is not +// clamped to the web identity token's exp: IdPs like GitLab issue ~2-minute +// id_tokens, and the session should still run the configured duration. +func TestSessionOutlivesShortLivedToken(t *testing.T) { + iamManager := setupIntegratedIAMSystem(t) + ctx := context.Background() + + shortLivedJWT := createTestJWT(t, "https://test-issuer.com", "test-user-123", "test-signing-key", + time.Now().Add(3*time.Minute)) + + tests := []struct { + name string + durationSeconds *int64 + wantDuration time.Duration + }{ + {"default duration ignores token exp", nil, time.Hour}, + {"explicit DurationSeconds ignores token exp", int64Ptr(3600), time.Hour}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + response, err := iamManager.AssumeRoleWithWebIdentity(ctx, &sts.AssumeRoleWithWebIdentityRequest{ + RoleArn: "arn:aws:iam::role/S3ReadOnlyRole", + WebIdentityToken: shortLivedJWT, + RoleSessionName: "short-lived-token-test", + DurationSeconds: tc.durationSeconds, + }) + require.NoError(t, err) + assert.WithinDuration(t, time.Now().Add(tc.wantDuration), response.Credentials.Expiration, time.Minute) + }) + } +} + // TestTrustPolicyValidation tests role trust policy validation func TestTrustPolicyValidation(t *testing.T) { iamManager := setupIntegratedIAMSystem(t) @@ -717,12 +750,16 @@ func TestOIDCClaimsTrustPolicy(t *testing.T) { // Helper functions and test setup // createTestJWT creates a test JWT token with the specified issuer, subject and signing key -func createTestJWT(t *testing.T, issuer, subject, signingKey string) string { +func createTestJWT(t *testing.T, issuer, subject, signingKey string, exp ...time.Time) string { + expiresAt := time.Now().Add(time.Hour) + if len(exp) > 0 { + expiresAt = exp[0] + } token := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{ "iss": issuer, "sub": subject, "aud": "test-client-id", - "exp": time.Now().Add(time.Hour).Unix(), + "exp": expiresAt.Unix(), "iat": time.Now().Unix(), // Add claims that trust policy validation expects "idp": "test-oidc", // Identity provider claim for trust policy matching diff --git a/weed/iam/oidc/mock_provider.go b/weed/iam/oidc/mock_provider.go index 3d5f0edab..bc20a5f14 100644 --- a/weed/iam/oidc/mock_provider.go +++ b/weed/iam/oidc/mock_provider.go @@ -61,14 +61,18 @@ func (m *MockOIDCProvider) Authenticate(ctx context.Context, token string) (*pro groups, _ := claims.GetClaimStringSlice("groups") rawRoles, _ := claims.GetClaimStringSlice("roles") - return &providers.ExternalIdentity{ + identity := &providers.ExternalIdentity{ UserID: claims.Subject, Email: email, DisplayName: displayName, Groups: groups, Roles: rawRoles, Provider: m.name, - }, nil + } + if !claims.ExpiresAt.IsZero() { + identity.TokenExpiration = &claims.ExpiresAt + } + return identity, nil } // ValidateToken validates tokens using test data diff --git a/weed/iam/sts/sts_service.go b/weed/iam/sts/sts_service.go index 99aacba5d..6905f12fa 100644 --- a/weed/iam/sts/sts_service.go +++ b/weed/iam/sts/sts_service.go @@ -596,9 +596,8 @@ func (s *STSService) AssumeRoleWithWebIdentity(ctx context.Context, request *Ass } } - // 4. Calculate session duration, capping at the source token's expiration - // This ensures sessions from short-lived tokens (e.g., GitLab CI job tokens) don't outlive their source - sessionDuration := s.calculateSessionDuration(request.DurationSeconds, externalIdentity.TokenExpiration) + // 4. Calculate session duration + sessionDuration := s.calculateSessionDuration(request.DurationSeconds) expiresAt := time.Now().Add(sessionDuration) // 5. Generate session ID and credentials @@ -754,18 +753,17 @@ func (s *STSService) AssumeRoleWithCredentials(ctx context.Context, request *Ass return nil, fmt.Errorf("role assumption denied: %w", err) } - // 4-7. Mint the session. For credential-based auth there is no source token - // with an expiration to cap the duration against. + // 4-7. Mint the session return s.issueSession(request.RoleArn, request.RoleSessionName, sessionPolicy, - request.DurationSeconds, nil, provider.Name(), externalIdentity.UserID) + request.DurationSeconds, provider.Name(), externalIdentity.UserID) } // issueSession mints temporary credentials and the self-contained JWT that // carries the whole session, shared by every assume-role entry point. func (s *STSService) issueSession(roleArn, roleSessionName, sessionPolicy string, - durationSeconds *int64, tokenExpiration *time.Time, providerName, subject string) (*AssumeRoleResponse, error) { + durationSeconds *int64, providerName, subject string) (*AssumeRoleResponse, error) { - sessionDuration := s.calculateSessionDuration(durationSeconds, tokenExpiration) + sessionDuration := s.calculateSessionDuration(durationSeconds) expiresAt := time.Now().Add(sessionDuration) sessionId, err := GenerateSessionId() @@ -882,7 +880,7 @@ func (s *STSService) AssumeRoleForPrincipal(ctx context.Context, request *Assume } return s.issueSession(request.RoleArn, request.RoleSessionName, sessionPolicy, - request.DurationSeconds, nil, request.ProviderName, request.Principal) + request.DurationSeconds, request.ProviderName, request.Principal) } // ValidateSessionToken validates a session token and returns session information @@ -1110,34 +1108,18 @@ func (s *STSService) validateRoleAssumptionForCredentials(ctx context.Context, r return nil } -// calculateSessionDuration calculates the session duration, respecting the source token's expiration -// If the incoming web identity token has an exp claim, the session duration is capped to not exceed it -// This ensures that sessions from short-lived tokens (e.g., GitLab CI job tokens) don't outlive their source -func (s *STSService) calculateSessionDuration(durationSeconds *int64, tokenExpiration *time.Time) time.Duration { +// calculateSessionDuration returns the requested DurationSeconds, or the +// configured TokenDuration default, capped at MaxSessionLength. The source +// token's exp deliberately plays no part: per AWS semantics the session +// outlives the (already verified) web identity token. +func (s *STSService) calculateSessionDuration(durationSeconds *int64) time.Duration { var duration time.Duration if durationSeconds != nil { duration = time.Duration(*durationSeconds) * time.Second } else { - // Use default from config duration = s.Config.TokenDuration.Duration } - // If the source token has an expiration, cap the session duration to not exceed it - // This follows the principle: "if calculated exp > incoming exp claim, then limit outgoing exp to incoming exp" - if tokenExpiration != nil && !tokenExpiration.IsZero() { - timeUntilTokenExpiry := time.Until(*tokenExpiration) - if timeUntilTokenExpiry <= 0 { - // Token already expired - use minimal duration as defense-in-depth - // The token should have been rejected during validation, but we handle this defensively - glog.V(2).Infof("Source token already expired, using minimal session duration") - duration = time.Minute - } else if timeUntilTokenExpiry < duration { - glog.V(2).Infof("Limiting session duration from %v to %v based on source token expiration", - duration, timeUntilTokenExpiry) - duration = timeUntilTokenExpiry - } - } - // Cap at MaxSessionLength if configured if s.Config.MaxSessionLength.Duration > 0 && duration > s.Config.MaxSessionLength.Duration { glog.V(2).Infof("Limiting session duration from %v to %v based on MaxSessionLength config", diff --git a/weed/iam/sts/sts_service_test.go b/weed/iam/sts/sts_service_test.go new file mode 100644 index 000000000..83bb46c89 --- /dev/null +++ b/weed/iam/sts/sts_service_test.go @@ -0,0 +1,38 @@ +package sts + +import ( + "testing" + "time" +) + +func TestCalculateSessionDuration(t *testing.T) { + svc := NewSTSService() + if err := svc.Initialize(&STSConfig{ + TokenDuration: FlexibleDuration{time.Hour}, + MaxSessionLength: FlexibleDuration{12 * time.Hour}, + Issuer: "test-issuer", + SigningKey: []byte("test-signing-key-at-least-32-bytes-long"), + }); err != nil { + t.Fatalf("Initialize() error = %v", err) + } + + seconds := func(v int64) *int64 { return &v } + + tests := []struct { + name string + durationSeconds *int64 + want time.Duration + }{ + {"default from config", nil, time.Hour}, + {"explicit request", seconds(1800), 30 * time.Minute}, + {"capped at MaxSessionLength", seconds(86400), 12 * time.Hour}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := svc.calculateSessionDuration(tc.durationSeconds); got != tc.want { + t.Errorf("calculateSessionDuration(%v) = %v, want %v", tc.durationSeconds, got, tc.want) + } + }) + } +}