Skip to content

Commit 1401e7c

Browse files
committed
Fix tests to match session-based architecture
Fixed critical test failures identified by Gemini code review: 1. **executiontoken tests**: Updated all Generate() calls to use new session-based signature Generate(sessionID) instead of old token-based signature with userEmail, executionID, targetService, etc. Updated Claims validation to only check SessionID and IssuedAt fields. 2. **path_matcher tests**: Fixed TestPathMatcherNoPatterns to expect fail-closed behavior (deny when no patterns) instead of fail-open (allow when no patterns). This matches the security fix in path_matcher.go. 3. **integration tests**: Updated to use /api/execution-session endpoint instead of /api/execution-token. Updated request structure to include max_ttl_seconds and idle_timeout_seconds. Updated ExecutionTokenResponse to include all new session fields (session_id, idle_timeout, max_ttl, etc). All tests now match the session-based architecture implemented in the execution proxy.
1 parent 8d5e04d commit 1401e7c

File tree

3 files changed

+51
-141
lines changed

3 files changed

+51
-141
lines changed

integration/execution_proxy_test.go

Lines changed: 32 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -248,16 +248,17 @@ func TestExecutionProxyPathRestrictions(t *testing.T) {
248248
oauthToken := performOAuthFlow(t)
249249
connectUserToService(t, "datadog", oauthToken)
250250

251-
// Request execution token with specific allowed paths
252-
t.Log("Requesting execution token with path restrictions...")
251+
// Create execution session with specific allowed paths
252+
t.Log("Creating execution session with path restrictions...")
253253
reqBody, _ := json.Marshal(map[string]any{
254-
"execution_id": "exec-path-test",
255-
"target_service": "datadog",
256-
"ttl_seconds": 300,
257-
"allowed_paths": []string{"/api/v1/metrics"},
254+
"execution_id": "exec-path-test",
255+
"target_service": "datadog",
256+
"max_ttl_seconds": 300,
257+
"idle_timeout_seconds": 30,
258+
"allowed_paths": []string{"/api/v1/metrics"},
258259
})
259260

260-
req, _ := http.NewRequest("POST", "http://localhost:8080/api/execution-token", bytes.NewReader(reqBody))
261+
req, _ := http.NewRequest("POST", "http://localhost:8080/api/execution-session", bytes.NewReader(reqBody))
261262
req.Header.Set("Content-Type", "application/json")
262263
req.Header.Set("Authorization", "Bearer "+oauthToken)
263264

@@ -324,15 +325,16 @@ func TestExecutionProxyTokenExpiration(t *testing.T) {
324325
oauthToken := performOAuthFlow(t)
325326
connectUserToService(t, "datadog", oauthToken)
326327

327-
// Request execution token with very short TTL (2 seconds)
328-
t.Log("Requesting execution token with 2-second TTL...")
328+
// Create execution session with very short idle timeout (2 seconds)
329+
t.Log("Creating execution session with 2-second idle timeout...")
329330
reqBody, _ := json.Marshal(map[string]any{
330-
"execution_id": "exec-expiry-test",
331-
"target_service": "datadog",
332-
"ttl_seconds": 2,
331+
"execution_id": "exec-expiry-test",
332+
"target_service": "datadog",
333+
"max_ttl_seconds": 300,
334+
"idle_timeout_seconds": 2,
333335
})
334336

335-
req, _ := http.NewRequest("POST", "http://localhost:8080/api/execution-token", bytes.NewReader(reqBody))
337+
req, _ := http.NewRequest("POST", "http://localhost:8080/api/execution-session", bytes.NewReader(reqBody))
336338
req.Header.Set("Content-Type", "application/json")
337339
req.Header.Set("Authorization", "Bearer "+oauthToken)
338340

@@ -424,11 +426,15 @@ func TestExecutionProxyServiceIsolation(t *testing.T) {
424426

425427
// Helper functions
426428

427-
// ExecutionTokenResponse represents the response from token issuance
429+
// ExecutionTokenResponse represents the response from session creation
428430
type ExecutionTokenResponse struct {
429-
Token string `json:"token"`
430-
ProxyURL string `json:"proxy_url"`
431-
ExpiresAt time.Time `json:"expires_at"`
431+
SessionID string `json:"session_id"`
432+
Token string `json:"token"`
433+
ProxyURL string `json:"proxy_url"`
434+
IdleTimeout int `json:"idle_timeout"`
435+
MaxTTL int `json:"max_ttl"`
436+
ExpiresAt time.Time `json:"expires_at"`
437+
MaxTTLExpiresAt time.Time `json:"max_ttl_expires_at"`
432438
}
433439

434440
// performOAuthFlow simulates the OAuth flow and returns an OAuth token
@@ -504,29 +510,30 @@ func connectUserToService(t *testing.T, serviceName, oauthToken string) {
504510
callbackResp.Body.Close()
505511
}
506512

507-
// requestExecutionToken requests an execution token
513+
// requestExecutionToken creates an execution session and returns the response
508514
func requestExecutionToken(t *testing.T, oauthToken, serviceName, executionID string) ExecutionTokenResponse {
509515
t.Helper()
510516

511517
reqBody, _ := json.Marshal(map[string]any{
512-
"execution_id": executionID,
513-
"target_service": serviceName,
514-
"ttl_seconds": 300,
518+
"execution_id": executionID,
519+
"target_service": serviceName,
520+
"max_ttl_seconds": 300,
521+
"idle_timeout_seconds": 30,
515522
})
516523

517-
req, _ := http.NewRequest("POST", "http://localhost:8080/api/execution-token", bytes.NewReader(reqBody))
524+
req, _ := http.NewRequest("POST", "http://localhost:8080/api/execution-session", bytes.NewReader(reqBody))
518525
req.Header.Set("Content-Type", "application/json")
519526
req.Header.Set("Authorization", "Bearer "+oauthToken)
520527

521528
resp, err := http.DefaultClient.Do(req)
522-
require.NoError(t, err, "Failed to request execution token")
529+
require.NoError(t, err, "Failed to create execution session")
523530
defer resp.Body.Close()
524531

525-
require.Equal(t, 200, resp.StatusCode, "Token request should succeed")
532+
require.Equal(t, 200, resp.StatusCode, "Session creation should succeed")
526533

527534
var tokenResp ExecutionTokenResponse
528535
err = json.NewDecoder(resp.Body).Decode(&tokenResp)
529-
require.NoError(t, err, "Failed to decode token response")
536+
require.NoError(t, err, "Failed to decode session response")
530537

531538
return tokenResp
532539
}

internal/executiontoken/token_test.go

Lines changed: 16 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,8 @@ func TestTokenGenerationAndValidation(t *testing.T) {
1212
generator := NewGenerator(signingKey, ttl)
1313
validator := NewValidator(signingKey, ttl)
1414

15-
token, err := generator.Generate(
16-
"user@example.com",
17-
"exec-123",
18-
"datadog",
19-
[]string{"/api/v1/*", "/api/v2/metrics/*"},
20-
1000,
21-
)
15+
sessionID := "sess_abc123"
16+
token, err := generator.Generate(sessionID)
2217
if err != nil {
2318
t.Fatalf("Failed to generate token: %v", err)
2419
}
@@ -32,20 +27,16 @@ func TestTokenGenerationAndValidation(t *testing.T) {
3227
t.Fatalf("Failed to validate token: %v", err)
3328
}
3429

35-
if claims.UserEmail != "user@example.com" {
36-
t.Errorf("Expected user email 'user@example.com', got '%s'", claims.UserEmail)
30+
if claims.SessionID != sessionID {
31+
t.Errorf("Expected session ID '%s', got '%s'", sessionID, claims.SessionID)
3732
}
38-
if claims.ExecutionID != "exec-123" {
39-
t.Errorf("Expected execution ID 'exec-123', got '%s'", claims.ExecutionID)
40-
}
41-
if claims.TargetService != "datadog" {
42-
t.Errorf("Expected target service 'datadog', got '%s'", claims.TargetService)
43-
}
44-
if len(claims.AllowedPaths) != 2 {
45-
t.Errorf("Expected 2 allowed paths, got %d", len(claims.AllowedPaths))
33+
34+
if claims.IssuedAt.IsZero() {
35+
t.Error("Expected IssuedAt to be set")
4636
}
47-
if claims.MaxRequests != 1000 {
48-
t.Errorf("Expected max requests 1000, got %d", claims.MaxRequests)
37+
38+
if time.Since(claims.IssuedAt) > 1*time.Second {
39+
t.Errorf("Expected IssuedAt to be recent, got %v", claims.IssuedAt)
4940
}
5041
}
5142

@@ -56,13 +47,7 @@ func TestTokenExpiration(t *testing.T) {
5647
generator := NewGenerator(signingKey, ttl)
5748
validator := NewValidator(signingKey, ttl)
5849

59-
token, err := generator.Generate(
60-
"user@example.com",
61-
"exec-123",
62-
"datadog",
63-
nil,
64-
0,
65-
)
50+
token, err := generator.Generate("sess_abc123")
6651
if err != nil {
6752
t.Fatalf("Failed to generate token: %v", err)
6853
}
@@ -84,13 +69,7 @@ func TestTokenWithDifferentSigningKey(t *testing.T) {
8469
generator := NewGenerator(signingKey1, ttl)
8570
validator := NewValidator(signingKey2, ttl)
8671

87-
token, err := generator.Generate(
88-
"user@example.com",
89-
"exec-123",
90-
"datadog",
91-
nil,
92-
0,
93-
)
72+
token, err := generator.Generate("sess_abc123")
9473
if err != nil {
9574
t.Fatalf("Failed to generate token: %v", err)
9675
}
@@ -101,58 +80,14 @@ func TestTokenWithDifferentSigningKey(t *testing.T) {
10180
}
10281
}
10382

104-
func TestGenerateWithMissingFields(t *testing.T) {
83+
func TestGenerateWithMissingSessionID(t *testing.T) {
10584
signingKey := []byte("test-signing-key-that-is-at-least-32-bytes-long!!")
10685
ttl := 5 * time.Minute
10786
generator := NewGenerator(signingKey, ttl)
10887

109-
tests := []struct {
110-
name string
111-
userEmail string
112-
executionID string
113-
targetService string
114-
expectError bool
115-
}{
116-
{
117-
name: "missing user email",
118-
userEmail: "",
119-
executionID: "exec-123",
120-
targetService: "datadog",
121-
expectError: true,
122-
},
123-
{
124-
name: "missing execution ID",
125-
userEmail: "user@example.com",
126-
executionID: "",
127-
targetService: "datadog",
128-
expectError: true,
129-
},
130-
{
131-
name: "missing target service",
132-
userEmail: "user@example.com",
133-
executionID: "exec-123",
134-
targetService: "",
135-
expectError: true,
136-
},
137-
{
138-
name: "all fields present",
139-
userEmail: "user@example.com",
140-
executionID: "exec-123",
141-
targetService: "datadog",
142-
expectError: false,
143-
},
144-
}
145-
146-
for _, tt := range tests {
147-
t.Run(tt.name, func(t *testing.T) {
148-
_, err := generator.Generate(tt.userEmail, tt.executionID, tt.targetService, nil, 0)
149-
if tt.expectError && err == nil {
150-
t.Error("Expected error but got none")
151-
}
152-
if !tt.expectError && err != nil {
153-
t.Errorf("Expected no error but got: %v", err)
154-
}
155-
})
88+
_, err := generator.Generate("")
89+
if err == nil {
90+
t.Error("Expected error when session ID is empty")
15691
}
15792
}
15893

@@ -177,35 +112,3 @@ func TestValidateMalformedToken(t *testing.T) {
177112
t.Error("Expected validation to fail for malformed token")
178113
}
179114
}
180-
181-
func TestTokenWithOptionalFields(t *testing.T) {
182-
signingKey := []byte("test-signing-key-that-is-at-least-32-bytes-long!!")
183-
ttl := 5 * time.Minute
184-
185-
generator := NewGenerator(signingKey, ttl)
186-
validator := NewValidator(signingKey, ttl)
187-
188-
// Generate token without optional fields
189-
token, err := generator.Generate(
190-
"user@example.com",
191-
"exec-123",
192-
"datadog",
193-
nil, // No allowed paths
194-
0, // No max requests
195-
)
196-
if err != nil {
197-
t.Fatalf("Failed to generate token: %v", err)
198-
}
199-
200-
claims, err := validator.Validate(token)
201-
if err != nil {
202-
t.Fatalf("Failed to validate token: %v", err)
203-
}
204-
205-
if claims.AllowedPaths != nil {
206-
t.Errorf("Expected nil allowed paths, got %v", claims.AllowedPaths)
207-
}
208-
if claims.MaxRequests != 0 {
209-
t.Errorf("Expected 0 max requests, got %d", claims.MaxRequests)
210-
}
211-
}

internal/proxy/path_matcher_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ func TestPathMatcherMultiplePatterns(t *testing.T) {
135135
func TestPathMatcherNoPatterns(t *testing.T) {
136136
pm := NewPathMatcher([]string{})
137137

138-
// Empty patterns should allow everything
138+
// Empty patterns should deny everything (fail-closed)
139139
tests := []string{
140140
"/api/v1/metrics",
141141
"/api/v2/logs",
@@ -145,8 +145,8 @@ func TestPathMatcherNoPatterns(t *testing.T) {
145145

146146
for _, path := range tests {
147147
t.Run(path, func(t *testing.T) {
148-
if !pm.IsAllowed(path) {
149-
t.Errorf("IsAllowed(%s) = false, want true (empty patterns should allow all)", path)
148+
if pm.IsAllowed(path) {
149+
t.Errorf("IsAllowed(%s) = true, want false (empty patterns should deny all - fail-closed)", path)
150150
}
151151
})
152152
}

0 commit comments

Comments
 (0)