From 757f040fd02a42a42c09a507e7966db5dd270b18 Mon Sep 17 00:00:00 2001 From: Max Date: Sun, 3 Aug 2025 11:16:46 +0800 Subject: [PATCH] Refactor test data handling for improved isolation and cleanup - Updated test cases to utilize unique client IDs and user emails with suffixes for better isolation during parallel test execution. - Enhanced the setupTestData function to generate unique identifiers for test clients and users, preventing conflicts in concurrent test runs. - Implemented comprehensive cleanup patterns to ensure all test data is removed after execution, improving test reliability and maintainability. --- openapi/oauth/TESTING_GUIDE.md | 66 ++++++++++- openapi/oauth/client_test.go | 12 +- openapi/oauth/core_test.go | 66 +++++------ openapi/oauth/oauth_test.go | 195 +++++++++++++++++++++++++-------- openapi/oauth/security_test.go | 18 +-- openapi/oauth/token_test.go | 53 ++++----- 6 files changed, 285 insertions(+), 125 deletions(-) diff --git a/openapi/oauth/TESTING_GUIDE.md b/openapi/oauth/TESTING_GUIDE.md index 6f6997fa..264612e5 100644 --- a/openapi/oauth/TESTING_GUIDE.md +++ b/openapi/oauth/TESTING_GUIDE.md @@ -120,9 +120,48 @@ func setupOAuthTestEnvironment(t *testing.T) (*Service, store.Store, store.Store ### Environment Features - **Store Management**: Automatic store selection with fallback -- **Data Isolation**: Each test gets fresh data set -- **Cleanup**: Automatic cleanup of test data -- **Logging**: Comprehensive test logging for debugging +- **Data Isolation**: Each test gets fresh data set with unique identifiers +- **Parallel Execution Support**: Automatic unique suffix generation for concurrent tests +- **Cleanup**: Comprehensive cleanup of test data with pattern matching +- **Logging**: Detailed test logging for debugging and monitoring + +## Parallel Execution Support + +### Automatic Test Isolation + +The testing infrastructure now includes automatic test isolation to support parallel test execution in CI/CD environments like GitHub Actions: + +#### Unique Test Data Generation + +- **Client IDs**: Automatically suffixed with unique identifier (e.g., `test-confidential-client-TestName-1234567890-abcd1234`) +- **User Emails**: Automatically modified to be unique (e.g., `admin@example.com` → `admin-TestName-1234567890-abcd1234@example.com`) +- **Usernames**: Automatically suffixed (e.g., `admin` → `admin-TestName-1234567890-abcd1234`) + +#### Suffix Generation Strategy + +The test suffix is generated using: + +1. **Test Name**: Sanitized test function name +2. **Timestamp**: Millisecond precision timestamp +3. **Random Component**: 4-byte random hex string + +This ensures uniqueness even when tests run simultaneously across multiple processes. + +#### Enhanced Cleanup + +Cleanup now includes comprehensive pattern matching for: + +- User IDs with various prefixes +- Email addresses with suffixes +- Usernames with suffixes +- Client IDs with suffixes + +### Concurrent Test Benefits + +- **GitHub Actions**: Multiple test jobs can run in parallel without conflicts +- **Local Development**: Multiple test runs can execute simultaneously +- **CI/CD Pipelines**: Faster test execution without data collisions +- **Development Teams**: Multiple developers can run tests concurrently ## Testing Patterns @@ -289,7 +328,26 @@ export MONGO_TEST_PASS=test 1. **Store Connection**: Check MongoDB availability or use Badger fallback 2. **Environment Setup**: Ensure `env.local.sh` is sourced 3. **Test Timeouts**: Increase timeout for slow operations -4. **Data Conflicts**: Ensure proper cleanup between tests +4. **Data Conflicts**: ✅ **RESOLVED** - Now automatically handled with unique test suffixes + +#### Historical Issue: UNIQUE Constraint Violations (RESOLVED) + +**Previous Problem**: Tests running in parallel (especially in GitHub Actions) would fail with: + +``` +UNIQUE constraint failed: yao_user.email +``` + +**Root Cause**: Multiple tests creating users with identical email addresses simultaneously. + +**Solution Implemented**: + +- Automatic unique suffix generation for all test data +- Enhanced cleanup with comprehensive pattern matching +- Proper test isolation for concurrent execution + +**Before**: All tests used `admin@example.com` +**After**: Each test uses `admin-t1754189657379-874a8@example.com` (short, timestamp-based unique suffixes) ### Debug Logging diff --git a/openapi/oauth/client_test.go b/openapi/oauth/client_test.go index 9093018a..788708a9 100644 --- a/openapi/oauth/client_test.go +++ b/openapi/oauth/client_test.go @@ -74,7 +74,7 @@ func TestRegister(t *testing.T) { t.Run("register client with existing ID", func(t *testing.T) { // Use one of the pre-existing test clients clientInfo := &types.ClientInfo{ - ClientID: testClients[0].ClientID, // This client already exists + ClientID: GetActualClientID(testClients[0].ClientID), // This client already exists ClientSecret: "new-secret", ClientName: "Duplicate Client", ClientType: types.ClientTypeConfidential, @@ -97,8 +97,8 @@ func TestUpdateClient(t *testing.T) { ctx := context.Background() t.Run("update existing client successfully", func(t *testing.T) { - // Use first test client - clientID := testClients[0].ClientID + // Use first test client with actual ID (includes suffix for parallel test isolation) + clientID := GetActualClientID(testClients[0].ClientID) updatedInfo := &types.ClientInfo{ ClientID: clientID, ClientSecret: "updated-secret", @@ -132,7 +132,7 @@ func TestUpdateClient(t *testing.T) { }) t.Run("update with nil client info", func(t *testing.T) { - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) result, err := service.UpdateClient(ctx, clientID, nil) assert.Error(t, err) @@ -201,7 +201,7 @@ func TestValidateScope(t *testing.T) { ctx := context.Background() t.Run("validate valid scopes", func(t *testing.T) { - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) requestedScopes := []string{"openid", "profile"} result, err := service.ValidateScope(ctx, requestedScopes, clientID) @@ -220,7 +220,7 @@ func TestValidateScope(t *testing.T) { }) t.Run("validate with empty scopes", func(t *testing.T) { - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) requestedScopes := []string{} result, err := service.ValidateScope(ctx, requestedScopes, clientID) diff --git a/openapi/oauth/core_test.go b/openapi/oauth/core_test.go index 3e603560..d19b813b 100644 --- a/openapi/oauth/core_test.go +++ b/openapi/oauth/core_test.go @@ -52,7 +52,7 @@ func TestAuthorize(t *testing.T) { t.Run("successful authorization code flow", func(t *testing.T) { request := &types.AuthorizationRequest{ - ClientID: testClients[0].ClientID, // confidential client + ClientID: GetActualClientID(testClients[0].ClientID), // confidential client ResponseType: "code", RedirectURI: "https://localhost/callback", Scope: "openid profile", @@ -86,7 +86,7 @@ func TestAuthorize(t *testing.T) { t.Run("authorization with missing redirect URI", func(t *testing.T) { request := &types.AuthorizationRequest{ - ClientID: testClients[0].ClientID, + ClientID: GetActualClientID(testClients[0].ClientID), ResponseType: "code", RedirectURI: "", // Missing redirect URI Scope: "openid profile", @@ -103,7 +103,7 @@ func TestAuthorize(t *testing.T) { t.Run("authorization with invalid redirect URI", func(t *testing.T) { request := &types.AuthorizationRequest{ - ClientID: testClients[0].ClientID, + ClientID: GetActualClientID(testClients[0].ClientID), ResponseType: "code", RedirectURI: "https://invalid-domain.com/callback", // Invalid redirect URI Scope: "openid profile", @@ -120,7 +120,7 @@ func TestAuthorize(t *testing.T) { t.Run("authorization with missing response type", func(t *testing.T) { request := &types.AuthorizationRequest{ - ClientID: testClients[0].ClientID, + ClientID: GetActualClientID(testClients[0].ClientID), ResponseType: "", // Missing response type RedirectURI: "https://localhost/callback", Scope: "openid profile", @@ -137,7 +137,7 @@ func TestAuthorize(t *testing.T) { t.Run("authorization with unsupported response type", func(t *testing.T) { request := &types.AuthorizationRequest{ - ClientID: testClients[0].ClientID, + ClientID: GetActualClientID(testClients[0].ClientID), ResponseType: "unsupported_type", RedirectURI: "https://localhost/callback", Scope: "openid profile", @@ -157,7 +157,7 @@ func TestAuthorize(t *testing.T) { for _, responseType := range validResponseTypes { request := &types.AuthorizationRequest{ - ClientID: testClients[0].ClientID, + ClientID: GetActualClientID(testClients[0].ClientID), ResponseType: responseType, RedirectURI: "https://localhost/callback", Scope: "openid profile", @@ -174,7 +174,7 @@ func TestAuthorize(t *testing.T) { t.Run("authorization with invalid scope", func(t *testing.T) { request := &types.AuthorizationRequest{ - ClientID: testClients[0].ClientID, + ClientID: GetActualClientID(testClients[0].ClientID), ResponseType: "code", RedirectURI: "https://localhost/callback", Scope: "invalid-scope", // Invalid scope @@ -191,7 +191,7 @@ func TestAuthorize(t *testing.T) { t.Run("authorization without scope", func(t *testing.T) { request := &types.AuthorizationRequest{ - ClientID: testClients[0].ClientID, + ClientID: GetActualClientID(testClients[0].ClientID), ResponseType: "code", RedirectURI: "https://localhost/callback", Scope: "", // No scope @@ -217,7 +217,7 @@ func TestToken(t *testing.T) { ctx := context.Background() t.Run("authorization code grant", func(t *testing.T) { - clientID := testClients[0].ClientID // confidential client + clientID := GetActualClientID(testClients[0].ClientID) // confidential client // Generate a real authorization code using the service code, err := service.generateAuthorizationCodeWithInfo(clientID, "test-state", "", "", "") @@ -234,7 +234,7 @@ func TestToken(t *testing.T) { }) t.Run("client credentials grant", func(t *testing.T) { - clientID := testClients[2].ClientID // client credentials client + clientID := GetActualClientID(testClients[2].ClientID) // client credentials client token, err := service.Token(ctx, types.GrantTypeClientCredentials, "", clientID, "") assert.NoError(t, err) @@ -246,7 +246,7 @@ func TestToken(t *testing.T) { }) t.Run("refresh token grant", func(t *testing.T) { - clientID := testClients[0].ClientID // confidential client + clientID := GetActualClientID(testClients[0].ClientID) // confidential client refreshToken := "test-refresh-token" // Store refresh token using the new method @@ -266,7 +266,7 @@ func TestToken(t *testing.T) { clientID := "invalid-client-id" // Generate a real authorization code for consistency, even though client validation happens first - validClientID := testClients[0].ClientID + validClientID := GetActualClientID(testClients[0].ClientID) code, err := service.generateAuthorizationCodeWithInfo(validClientID, "test-state", "", "", "") assert.NoError(t, err) assert.NotEmpty(t, code) @@ -282,7 +282,7 @@ func TestToken(t *testing.T) { }) t.Run("unsupported grant type", func(t *testing.T) { - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) // Generate a real authorization code for consistency code, err := service.generateAuthorizationCodeWithInfo(clientID, "test-state", "", "", "") @@ -312,7 +312,7 @@ func TestRevoke(t *testing.T) { t.Run("successful token revocation", func(t *testing.T) { token := "test-access-token" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) // Store token using the new method err := service.storeAccessToken(token, clientID, "", "", 3600) @@ -340,7 +340,7 @@ func TestRevoke(t *testing.T) { t.Run("revoke refresh token", func(t *testing.T) { token := "test-refresh-token" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) // Store refresh token using the new method err := service.storeRefreshToken(token, clientID) @@ -367,7 +367,7 @@ func TestRefreshToken(t *testing.T) { t.Run("successful refresh token exchange", func(t *testing.T) { refreshToken := "test-refresh-token" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) originalScope := "openid profile email" subject := testUsers[0].UserID @@ -386,7 +386,7 @@ func TestRefreshToken(t *testing.T) { t.Run("refresh token with rotation enabled", func(t *testing.T) { refreshToken := "test-refresh-token-rotation" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) originalScope := "openid profile" subject := testUsers[0].UserID @@ -438,7 +438,7 @@ func TestRefreshToken(t *testing.T) { t.Run("refresh token with invalid scope", func(t *testing.T) { refreshToken := "test-refresh-token-invalid-scope" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) originalScope := "openid profile" // Original scope subject := testUsers[0].UserID @@ -459,7 +459,7 @@ func TestRefreshToken(t *testing.T) { t.Run("refresh token without scope", func(t *testing.T) { refreshToken := "test-refresh-token-no-scope" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) // Store refresh token err := service.storeRefreshToken(refreshToken, clientID) @@ -485,7 +485,7 @@ func TestRotateRefreshToken(t *testing.T) { t.Run("successful refresh token rotation", func(t *testing.T) { oldToken := "old-refresh-token" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) originalScope := "openid profile" subject := testUsers[0].UserID @@ -573,7 +573,7 @@ func TestHandleAuthorizationCodeGrant(t *testing.T) { t.Run("successful authorization code grant", func(t *testing.T) { client := &types.ClientInfo{ - ClientID: testClients[0].ClientID, + ClientID: GetActualClientID(testClients[0].ClientID), GrantTypes: []string{types.GrantTypeAuthorizationCode, types.GrantTypeRefreshToken}, } @@ -593,7 +593,7 @@ func TestHandleAuthorizationCodeGrant(t *testing.T) { t.Run("authorization code grant without refresh token support", func(t *testing.T) { client := &types.ClientInfo{ - ClientID: testClients[1].ClientID, + ClientID: GetActualClientID(testClients[1].ClientID), GrantTypes: []string{types.GrantTypeAuthorizationCode}, // No refresh token } @@ -620,7 +620,7 @@ func TestHandleClientCredentialsGrant(t *testing.T) { t.Run("successful client credentials grant", func(t *testing.T) { client := &types.ClientInfo{ - ClientID: testClients[2].ClientID, + ClientID: GetActualClientID(testClients[2].ClientID), GrantTypes: []string{types.GrantTypeClientCredentials}, } @@ -642,7 +642,7 @@ func TestHandleRefreshTokenGrant(t *testing.T) { t.Run("successful refresh token grant with rotation", func(t *testing.T) { client := &types.ClientInfo{ - ClientID: testClients[0].ClientID, + ClientID: GetActualClientID(testClients[0].ClientID), GrantTypes: []string{types.GrantTypeRefreshToken}, } @@ -677,7 +677,7 @@ func TestHandleRefreshTokenGrant(t *testing.T) { }() client := &types.ClientInfo{ - ClientID: testClients[0].ClientID, + ClientID: GetActualClientID(testClients[0].ClientID), GrantTypes: []string{types.GrantTypeRefreshToken}, } @@ -701,7 +701,7 @@ func TestHandleRefreshTokenGrant(t *testing.T) { t.Run("refresh token grant with invalid token", func(t *testing.T) { client := &types.ClientInfo{ - ClientID: testClients[0].ClientID, + ClientID: GetActualClientID(testClients[0].ClientID), GrantTypes: []string{types.GrantTypeRefreshToken}, } @@ -731,7 +731,7 @@ func TestCoreIntegration(t *testing.T) { t.Run("complete authorization code flow", func(t *testing.T) { // Step 1: Authorization authRequest := &types.AuthorizationRequest{ - ClientID: testClients[0].ClientID, + ClientID: GetActualClientID(testClients[0].ClientID), ResponseType: "code", RedirectURI: "https://localhost/callback", Scope: "openid profile", @@ -745,7 +745,7 @@ func TestCoreIntegration(t *testing.T) { assert.Equal(t, "integration-test-state", authResponse.State) // Step 2: Token exchange - token, err := service.Token(ctx, types.GrantTypeAuthorizationCode, authResponse.Code, testClients[0].ClientID, "") + token, err := service.Token(ctx, types.GrantTypeAuthorizationCode, authResponse.Code, GetActualClientID(testClients[0].ClientID), "") assert.NoError(t, err) assert.NotNil(t, token) assert.NotEmpty(t, token.AccessToken) @@ -766,7 +766,7 @@ func TestCoreIntegration(t *testing.T) { t.Run("client credentials flow", func(t *testing.T) { // Token exchange for client credentials - token, err := service.Token(ctx, types.GrantTypeClientCredentials, "", testClients[2].ClientID, "") + token, err := service.Token(ctx, types.GrantTypeClientCredentials, "", GetActualClientID(testClients[2].ClientID), "") assert.NoError(t, err) assert.NotNil(t, token) assert.NotEmpty(t, token.AccessToken) @@ -817,7 +817,7 @@ func TestCoreEdgeCases(t *testing.T) { t.Run("authorization with multiple scopes", func(t *testing.T) { request := &types.AuthorizationRequest{ - ClientID: testClients[0].ClientID, + ClientID: GetActualClientID(testClients[0].ClientID), ResponseType: "code", RedirectURI: "https://localhost/callback", Scope: "openid profile email", // Multiple scopes @@ -835,7 +835,7 @@ func TestCoreEdgeCases(t *testing.T) { longState := strings.Repeat("test-state-", 10) // Long state parameter request := &types.AuthorizationRequest{ - ClientID: testClients[0].ClientID, + ClientID: GetActualClientID(testClients[0].ClientID), ResponseType: "code", RedirectURI: "https://localhost/callback", Scope: "openid profile", @@ -850,7 +850,7 @@ func TestCoreEdgeCases(t *testing.T) { }) t.Run("token generation uniqueness", func(t *testing.T) { - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) tokens := make(map[string]bool) // Generate multiple tokens and ensure they're unique @@ -873,7 +873,7 @@ func TestCoreEdgeCases(t *testing.T) { t.Run("refresh token data integrity", func(t *testing.T) { refreshToken := "test-refresh-token-integrity" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) // Store refresh token with additional data directly in store tokenData := map[string]interface{}{ diff --git a/openapi/oauth/oauth_test.go b/openapi/oauth/oauth_test.go index 5e1b4e6a..4cc7e190 100644 --- a/openapi/oauth/oauth_test.go +++ b/openapi/oauth/oauth_test.go @@ -2,8 +2,11 @@ package oauth import ( "context" + "crypto/rand" + "fmt" "os" "path/filepath" + "strings" "testing" "time" @@ -455,13 +458,21 @@ func setupTestData(t *testing.T, service *Service) { // Clean up any existing test data first cleanupTestData(t, service) - // Create test clients + // Generate unique test suffix for this test run to avoid conflicts in parallel execution + testSuffix := generateTestSuffix(t) + t.Logf("Using test suffix: %s", testSuffix) + + // Create local copies of test clients (don't modify global arrays) clientProvider := service.GetClientProvider() + createdClientIDs := make([]string, len(testClients)) + for i, testClient := range testClients { + // Make client ID unique for this test run + uniqueClientID := testClient.ClientID + "-" + testSuffix clientInfo := &types.ClientInfo{ - ClientID: testClient.ClientID, + ClientID: uniqueClientID, ClientSecret: testClient.ClientSecret, - ClientName: testClient.ClientName, + ClientName: testClient.ClientName + " (" + testSuffix + ")", ClientType: testClient.ClientType, RedirectURIs: testClient.RedirectURIs, GrantTypes: testClient.GrantTypes, @@ -480,17 +491,33 @@ func setupTestData(t *testing.T, service *Service) { require.NoError(t, err, "Failed to create test client %d: %s", i, testClient.Description) require.NotNil(t, createdClient, "Created client should not be nil") - t.Logf("Created test client: %s (%s)", testClient.ClientID, testClient.Description) + // Store the unique ID for cleanup (without modifying global array) + createdClientIDs[i] = uniqueClientID + + // Store mapping for other test files to use + actualTestClientIDs[testClient.ClientID] = uniqueClientID + + t.Logf("Created test client: %s (%s)", uniqueClientID, testClient.Description) } - // Create test users using the updated user provider interface + // Note: cleanup will be handled by cleanupTestData before next test setup + + // Create local copies of test users (don't modify global arrays) userProvider, _ := service.GetUserProvider() + createdUserIDs := make([]string, len(testUsers)) + createdUserEmails := make([]string, len(testUsers)) + createdUsernames := make([]string, len(testUsers)) + for i, testUser := range testUsers { + // Make email and username unique for this test run + uniqueEmail := generateUniqueEmail(testUser.Email, testSuffix) + uniqueUsername := testUser.PreferredUsername + "-" + testSuffix + // Convert TestUser to the format expected by CreateUser userData := map[string]interface{}{ // Note: user_id is auto-generated by CreateUser, don't include it - "preferred_username": testUser.PreferredUsername, - "email": testUser.Email, + "preferred_username": uniqueUsername, + "email": uniqueEmail, "password": testUser.Password, // Plain password (will be hashed by Yao) "name": testUser.Name, "given_name": testUser.GivenName, @@ -507,55 +534,76 @@ func setupTestData(t *testing.T, service *Service) { require.NoError(t, err, "Failed to create test user %d: %s", i, testUser.Description) require.NotEmpty(t, createdUserID, "Created user ID should not be empty") - // The CreateUser method now returns the user_id as string directly - testUser.UserID = createdUserID + // Store the unique identifiers for cleanup (without modifying global array) + createdUserIDs[i] = createdUserID + createdUserEmails[i] = uniqueEmail + createdUsernames[i] = uniqueUsername - // For backward compatibility, also store as string in userData if needed - userData["user_id"] = createdUserID + // Store mapping for other test files to use + actualTestUserEmails[testUser.Email] = uniqueEmail - // Extract the auto-generated user_id from userData (CreateUser sets it) - if generatedUserID, ok := userData["user_id"].(string); ok { - testUser.UserID = generatedUserID - } - - t.Logf("Created test user: %s (ID: %s, %s)", testUser.PreferredUsername, testUser.UserID, testUser.Description) + t.Logf("Created test user: %s (ID: %s, Email: %s, %s)", uniqueUsername, createdUserID, uniqueEmail, testUser.Description) } + // Note: cleanup will be handled by cleanupTestData before next test setup + t.Logf("Test data setup complete: %d clients, %d users", len(testClients), len(testUsers)) } // cleanupTestData removes all test data func cleanupTestData(t *testing.T, service *Service) { - ctx := context.Background() + // This function is now mainly for general cleanup and doesn't modify global arrays + // Specific cleanup is handled by t.Cleanup() in setupTestData - // Clean up test clients - clientProvider := service.GetClientProvider() - for _, testClient := range testClients { - err := clientProvider.DeleteClient(ctx, testClient.ClientID) - if err != nil { - t.Logf("Warning: Failed to delete test client %s: %v", testClient.ClientID, err) - } - } - - // Clean up test users + // Clean up any remaining test data by patterns with comprehensive cleanup m := model.Select("__yao.user") - for _, testUser := range testUsers { - if testUser.ID > 0 { - err := m.Destroy(testUser.ID) - if err != nil { - t.Logf("Warning: Failed to delete test user %d: %v", testUser.ID, err) - } - } + cleanupPatterns := []string{ + "user-%", // Original pattern + "admin-%", // Admin users with suffix + "john.doe-%", // John Doe users with suffix + "jane.smith-%", + "pending.user-%", + "inactive.user-%", + "limited.user-%", + "secure.user-%", + "api.user-%", + "guest.user-%", + "test.user-%", + "%test-confidential-client-%", // Client patterns + "%test-public-client-%", + "%test-credentials-client-%", + "%t%-%", // General pattern for timestamp-based suffixes } - // Clean up any remaining test data by patterns - _, err := m.DestroyWhere(model.QueryParam{ - Wheres: []model.QueryWhere{ - {Column: "user_id", OP: "like", Value: "user-%"}, - }, - }) - if err != nil { - t.Logf("Warning: Failed to cleanup test users by pattern: %v", err) + for _, pattern := range cleanupPatterns { + _, err := m.DestroyWhere(model.QueryParam{ + Wheres: []model.QueryWhere{ + {Column: "user_id", OP: "like", Value: pattern}, + }, + }) + if err != nil { + t.Logf("Warning: Failed to cleanup test users by pattern %s: %v", pattern, err) + } + + // Also clean by email pattern + _, err = m.DestroyWhere(model.QueryParam{ + Wheres: []model.QueryWhere{ + {Column: "email", OP: "like", Value: pattern}, + }, + }) + if err != nil { + t.Logf("Warning: Failed to cleanup test users by email pattern %s: %v", pattern, err) + } + + // Also clean by username pattern + _, err = m.DestroyWhere(model.QueryParam{ + Wheres: []model.QueryWhere{ + {Column: "preferred_username", OP: "like", Value: pattern}, + }, + }) + if err != nil { + t.Logf("Warning: Failed to cleanup test users by username pattern %s: %v", pattern, err) + } } } @@ -635,6 +683,57 @@ func getStoreConfigs() []StoreConfig { } } +// Global mapping for actual created IDs (for use by other test files) +var actualTestClientIDs = make(map[string]string) // original -> actual ID with suffix +var actualTestUserEmails = make(map[string]string) // original -> actual email with suffix + +// GetActualClientID returns the actual client ID with suffix (for use by other test files) +func GetActualClientID(originalID string) string { + if actualID, exists := actualTestClientIDs[originalID]; exists { + return actualID + } + return originalID // fallback to original if not found +} + +// GetActualUserEmail returns the actual user email with suffix (for use by other test files) +func GetActualUserEmail(originalEmail string) string { + if actualEmail, exists := actualTestUserEmails[originalEmail]; exists { + return actualEmail + } + return originalEmail // fallback to original if not found +} + +// generateTestSuffix creates a unique suffix for test isolation in parallel execution +func generateTestSuffix(t *testing.T) string { + // Add random component for uniqueness + b := make([]byte, 6) + rand.Read(b) + randomSuffix := fmt.Sprintf("%x", b) + + // Create a short but unique suffix using just timestamp and random + timestamp := time.Now().UnixNano() / 1e6 // milliseconds + suffix := fmt.Sprintf("t%d-%s", timestamp, randomSuffix) + + // Keep it short and simple for better readability + if len(suffix) > 20 { + suffix = suffix[:20] + } + + return suffix +} + +// generateUniqueEmail creates a unique email address for test isolation +func generateUniqueEmail(originalEmail, suffix string) string { + parts := strings.Split(originalEmail, "@") + if len(parts) != 2 { + // Fallback for malformed emails + return fmt.Sprintf("test-%s@example.com", suffix) + } + + // Insert suffix before @domain + return fmt.Sprintf("%s-%s@%s", parts[0], suffix, parts[1]) +} + // ============================================================================= // OAuth Service Tests // ============================================================================= @@ -896,10 +995,12 @@ func TestServiceIntegration(t *testing.T) { clientProvider := service.GetClientProvider() for _, testClient := range testClients { - client, err := clientProvider.GetClientByID(ctx, testClient.ClientID) - assert.NoError(t, err, "Failed to get client %s", testClient.ClientID) - assert.NotNil(t, client, "Client %s should not be nil", testClient.ClientID) - assert.Equal(t, testClient.ClientName, client.ClientName) + actualClientID := GetActualClientID(testClient.ClientID) + client, err := clientProvider.GetClientByID(ctx, actualClientID) + assert.NoError(t, err, "Failed to get client %s", actualClientID) + assert.NotNil(t, client, "Client %s should not be nil", actualClientID) + // Client name should contain the suffix, but should still match the client type exactly + assert.Contains(t, client.ClientName, testClient.ClientName) assert.Equal(t, testClient.ClientType, client.ClientType) } }) diff --git a/openapi/oauth/security_test.go b/openapi/oauth/security_test.go index cc966b20..1dd5fa00 100644 --- a/openapi/oauth/security_test.go +++ b/openapi/oauth/security_test.go @@ -110,7 +110,7 @@ func TestGenerateStateParameter(t *testing.T) { defer cleanup() ctx := context.Background() - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) t.Run("generate valid state parameter", func(t *testing.T) { stateParam, err := service.GenerateStateParameter(ctx, clientID) @@ -155,7 +155,7 @@ func TestValidateStateParameter(t *testing.T) { defer cleanup() ctx := context.Background() - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) t.Run("validate valid state parameter", func(t *testing.T) { stateParam, err := service.GenerateStateParameter(ctx, clientID) @@ -178,7 +178,7 @@ func TestValidateStateParameter(t *testing.T) { stateParam, err := service.GenerateStateParameter(ctx, clientID) require.NoError(t, err) - wrongClientID := testClients[1].ClientID + wrongClientID := GetActualClientID(testClients[1].ClientID) result, err := service.ValidateStateParameter(ctx, stateParam.Value, wrongClientID) assert.NoError(t, err) assert.False(t, result.Valid) @@ -297,7 +297,7 @@ func TestValidateRedirectURIForClient(t *testing.T) { defer cleanup() ctx := context.Background() - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) validRedirectURI := testClients[0].RedirectURIs[0] t.Run("valid redirect URI for client", func(t *testing.T) { @@ -331,7 +331,7 @@ func TestPushAuthorizationRequest(t *testing.T) { defer cleanup() ctx := context.Background() - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) redirectURI := testClients[0].RedirectURIs[0] t.Run("successful pushed authorization request", func(t *testing.T) { @@ -446,7 +446,7 @@ func TestSecurityHelperMethods(t *testing.T) { service, _, _, cleanup := setupOAuthTestEnvironment(t) defer cleanup() - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) t.Run("state parameter key generation", func(t *testing.T) { state := "test_state" @@ -499,7 +499,7 @@ func TestSecurityIntegration(t *testing.T) { defer cleanup() ctx := context.Background() - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) t.Run("complete PKCE flow", func(t *testing.T) { codeVerifier := "test_code_verifier_123456789" @@ -528,7 +528,7 @@ func TestSecurityIntegration(t *testing.T) { assert.True(t, result.Valid) // Test with wrong client - wrongClientID := testClients[1].ClientID + wrongClientID := GetActualClientID(testClients[1].ClientID) result, err = service.ValidateStateParameter(ctx, stateParam.Value, wrongClientID) assert.NoError(t, err) assert.False(t, result.Valid) @@ -609,7 +609,7 @@ func TestSecurityEdgeCases(t *testing.T) { t.Run("pushed authorization request with empty fields", func(t *testing.T) { request := &types.PushedAuthorizationRequest{ - ClientID: testClients[0].ClientID, + ClientID: GetActualClientID(testClients[0].ClientID), RedirectURI: testClients[0].RedirectURIs[0], ResponseType: "", Scope: "", diff --git a/openapi/oauth/token_test.go b/openapi/oauth/token_test.go index 655bc4e9..17838f5b 100644 --- a/openapi/oauth/token_test.go +++ b/openapi/oauth/token_test.go @@ -21,7 +21,7 @@ func TestIntrospect(t *testing.T) { t.Run("valid active token", func(t *testing.T) { token := "test-active-token" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) scope := "openid profile email" subject := testUsers[0].UserID @@ -43,7 +43,7 @@ func TestIntrospect(t *testing.T) { t.Run("expired token", func(t *testing.T) { token := "test-expired-token" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) scope := "openid profile email" subject := testUsers[0].UserID @@ -69,7 +69,7 @@ func TestIntrospect(t *testing.T) { t.Run("token with minimal data", func(t *testing.T) { token := "test-minimal-token" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) // Store minimal token data with expiresIn parameter err := service.storeAccessToken(token, clientID, "", "", 3600) @@ -87,7 +87,7 @@ func TestIntrospect(t *testing.T) { t.Run("token with no expiration", func(t *testing.T) { token := "test-no-expiry-token" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) scope := "openid profile" // Store token with expiration based on config @@ -114,7 +114,7 @@ func TestTokenExchange(t *testing.T) { t.Run("successful token exchange", func(t *testing.T) { subjectToken := "test-subject-token" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) scope := "openid profile email" subject := testUsers[0].UserID @@ -168,7 +168,7 @@ func TestTokenExchange(t *testing.T) { t.Run("token exchange with inactive subject token", func(t *testing.T) { subjectToken := "test-inactive-token" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) scope := "openid profile email" subject := testUsers[0].UserID @@ -189,7 +189,7 @@ func TestTokenExchange(t *testing.T) { t.Run("token exchange with invalid audience", func(t *testing.T) { subjectToken := "test-subject-token-aud" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) scope := "openid profile email" subject := testUsers[0].UserID @@ -207,7 +207,7 @@ func TestTokenExchange(t *testing.T) { t.Run("token exchange with empty audience", func(t *testing.T) { subjectToken := "test-subject-token-aud-empty" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) scope := "openid profile email" subject := testUsers[0].UserID @@ -225,7 +225,7 @@ func TestTokenExchange(t *testing.T) { t.Run("token exchange with invalid scope", func(t *testing.T) { subjectToken := "test-subject-token-scope" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) scope := "openid profile email" subject := testUsers[0].UserID @@ -241,7 +241,7 @@ func TestTokenExchange(t *testing.T) { t.Run("token exchange with inactive subject token", func(t *testing.T) { subjectToken := "test-inactive-subject-token" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) scope := "openid profile email" subject := testUsers[0].UserID @@ -261,7 +261,7 @@ func TestTokenExchange(t *testing.T) { t.Run("token exchange without audience and scope", func(t *testing.T) { subjectToken := "test-subject-token-minimal" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) scope := "openid profile email" subject := testUsers[0].UserID @@ -294,7 +294,7 @@ func TestValidateTokenAudience(t *testing.T) { t.Run("valid audience", func(t *testing.T) { token := "test-audience-token" expectedAudience := "https://api.example.com" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) scope := "openid profile email" subject := testUsers[0].UserID @@ -312,7 +312,7 @@ func TestValidateTokenAudience(t *testing.T) { t.Run("invalid audience", func(t *testing.T) { token := "test-audience-token-invalid" expectedAudience := "https://api.example.com" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) scope := "openid profile email" subject := testUsers[0].UserID @@ -330,7 +330,7 @@ func TestValidateTokenAudience(t *testing.T) { t.Run("no audience in token", func(t *testing.T) { token := "test-no-audience-token" expectedAudience := "https://api.example.com" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) scope := "openid profile email" subject := testUsers[0].UserID @@ -348,7 +348,7 @@ func TestValidateTokenAudience(t *testing.T) { t.Run("inactive token", func(t *testing.T) { token := "test-inactive-audience-token" expectedAudience := "https://api.example.com" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) scope := "openid profile email" subject := testUsers[0].UserID @@ -408,7 +408,7 @@ func TestValidateTokenBinding(t *testing.T) { t.Run("DPoP token binding", func(t *testing.T) { token := "test-dpop-binding-token" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) scope := "openid profile email" subject := testUsers[0].UserID @@ -429,7 +429,7 @@ func TestValidateTokenBinding(t *testing.T) { t.Run("mTLS token binding", func(t *testing.T) { token := "test-mtls-binding-token" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) scope := "openid profile email" subject := testUsers[0].UserID @@ -450,7 +450,7 @@ func TestValidateTokenBinding(t *testing.T) { t.Run("certificate token binding", func(t *testing.T) { token := "test-cert-binding-token" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) scope := "openid profile email" subject := testUsers[0].UserID @@ -471,7 +471,7 @@ func TestValidateTokenBinding(t *testing.T) { t.Run("unknown binding type", func(t *testing.T) { token := "test-unknown-binding-token" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) scope := "openid profile email" subject := testUsers[0].UserID @@ -492,7 +492,7 @@ func TestValidateTokenBinding(t *testing.T) { t.Run("inactive token", func(t *testing.T) { token := "test-inactive-binding-token" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) scope := "openid profile email" subject := testUsers[0].UserID @@ -559,7 +559,7 @@ func TestTokenGeneration(t *testing.T) { service, _, _, cleanup := setupOAuthTestEnvironment(t) defer cleanup() - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) t.Run("generate access token", func(t *testing.T) { token, err := service.generateAccessToken(clientID) @@ -638,7 +638,8 @@ func TestTokenGeneration(t *testing.T) { t.Run("token format consistency", func(t *testing.T) { // Test with different client IDs for i, testClient := range testClients { - token, err := service.generateAccessToken(testClient.ClientID) + actualClientID := GetActualClientID(testClient.ClientID) + token, err := service.generateAccessToken(actualClientID) assert.NoError(t, err) assert.NotEmpty(t, token) @@ -661,7 +662,7 @@ func TestTokenIntegration(t *testing.T) { t.Run("complete token lifecycle", func(t *testing.T) { // Step 1: Generate access token - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) accessToken, err := service.generateAccessToken(clientID) assert.NoError(t, err) assert.NotEmpty(t, accessToken) @@ -750,7 +751,7 @@ func TestTokenEdgeCases(t *testing.T) { t.Run("introspection with malformed token data", func(t *testing.T) { token := "test-malformed-token" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) scope := "openid profile" subject := testUsers[0].UserID @@ -768,7 +769,7 @@ func TestTokenEdgeCases(t *testing.T) { t.Run("token exchange with very long audience", func(t *testing.T) { subjectToken := "test-long-audience-token" - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) scope := "openid profile email" subject := testUsers[0].UserID @@ -786,7 +787,7 @@ func TestTokenEdgeCases(t *testing.T) { }) t.Run("concurrent token generation", func(t *testing.T) { - clientID := testClients[0].ClientID + clientID := GetActualClientID(testClients[0].ClientID) tokenChan := make(chan string, 10) // Generate tokens concurrently