From 481323104a4222d7346040257e7e9a8916c92faa Mon Sep 17 00:00:00 2001 From: Max Date: Wed, 15 Oct 2025 19:32:40 +0800 Subject: [PATCH] Implement deep copy for entry configuration to prevent global config corruption - Added a new function, createPublicEntryConfig, to create a deep copy of EntryConfig without sensitive data, ensuring that global configuration remains intact when returning public config. - Updated entry verification logic to utilize EntryVerificationStatus for clearer status handling during user verification. - Enhanced tests to verify that the global config is not modified after multiple requests for public entry configuration, addressing a critical bug related to captcha secret exposure. --- openapi/tests/user/entry_test.go | 141 ++++++++++++++++++++++++++ openapi/user/entry.go | 168 +++++++++++++++++++++++++++---- openapi/user/types.go | 24 +++-- 3 files changed, 305 insertions(+), 28 deletions(-) diff --git a/openapi/tests/user/entry_test.go b/openapi/tests/user/entry_test.go index dc253030..e9864a48 100644 --- a/openapi/tests/user/entry_test.go +++ b/openapi/tests/user/entry_test.go @@ -516,3 +516,144 @@ func createUserWithMobile(t *testing.T, userID, mobile string) { t.Logf("Created user with mobile: user_id=%s, mobile=%s", createdUserID, mobile) } + +// TestEntryConfigDeepCopy tests that getting public entry config doesn't modify global config +// This test verifies the fix for the bug where captcha secret was deleted from global config +// when returning public config to the frontend. +func TestEntryConfigDeepCopy(t *testing.T) { + serverURL := testutils.Prepare(t) + defer testutils.Clean() + + // Get base URL from server config + baseURL := "" + if openapi.Server != nil && openapi.Server.Config != nil { + baseURL = openapi.Server.Config.BaseURL + } + + // Use UUID to ensure unique identifiers + testUUID := strings.ReplaceAll(uuid.New().String(), "-", "")[:8] + testEmail := fmt.Sprintf("test_%s@example.com", testUUID) + + t.Run("GetEntryConfig_Multiple_Times_Should_Not_Corrupt_Global_Config", func(t *testing.T) { + // Step 1: Get entry config (first time) + resp1, err := http.Get(serverURL + baseURL + "/user/entry") + assert.NoError(t, err, "First GET /user/entry should succeed") + defer resp1.Body.Close() + + assert.Equal(t, http.StatusOK, resp1.StatusCode, "First request should return 200 OK") + + // Parse response to verify secret is not exposed + var config1 map[string]interface{} + err = json.NewDecoder(resp1.Body).Decode(&config1) + assert.NoError(t, err, "Should decode first config response") + + // Verify captcha secret is not exposed in public config + if form, ok := config1["form"].(map[string]interface{}); ok { + if captcha, ok := form["captcha"].(map[string]interface{}); ok { + if options, ok := captcha["options"].(map[string]interface{}); ok { + _, hasSecret := options["secret"] + assert.False(t, hasSecret, "Public config should NOT expose captcha secret") + t.Logf("✓ First request: captcha secret properly hidden from public config") + } + } + } + + // Step 2: Get entry config (second time) - this should still work + resp2, err := http.Get(serverURL + baseURL + "/user/entry") + assert.NoError(t, err, "Second GET /user/entry should succeed") + defer resp2.Body.Close() + + assert.Equal(t, http.StatusOK, resp2.StatusCode, "Second request should return 200 OK") + + // Parse second response + var config2 map[string]interface{} + err = json.NewDecoder(resp2.Body).Decode(&config2) + assert.NoError(t, err, "Should decode second config response") + t.Logf("✓ Second request: config retrieved successfully") + + // Step 3: Get captcha for entry verification + captchaID, captchaAnswer := getCaptcha(t, serverURL, baseURL, "turnstile") + t.Logf("✓ Got captcha: ID=%s", captchaID) + + // Step 4: Call entry verify endpoint - this should NOT fail with "Turnstile secret not configured" + // This is the critical test: if global config was corrupted, this will fail + verifyData := map[string]interface{}{ + "username": testEmail, + "captcha": captchaAnswer, + } + + verifyJSON, err := json.Marshal(verifyData) + assert.NoError(t, err, "Should marshal verify request") + + verifyResp, err := http.Post( + serverURL+baseURL+"/user/entry/verify", + "application/json", + bytes.NewReader(verifyJSON), + ) + assert.NoError(t, err, "POST /user/entry/verify should succeed") + defer verifyResp.Body.Close() + + // Read response body for debugging + verifyBody, err := io.ReadAll(verifyResp.Body) + assert.NoError(t, err, "Should read verify response body") + + // Parse response + var verifyResult map[string]interface{} + err = json.Unmarshal(verifyBody, &verifyResult) + assert.NoError(t, err, "Should decode verify response") + + // The key assertion: verify should NOT fail with "Turnstile secret not configured" + if verifyResp.StatusCode != http.StatusOK { + // Check if it's the bug we're testing for + if errorDesc, ok := verifyResult["error_description"].(string); ok { + assert.NotContains(t, errorDesc, "Turnstile secret not configured", + "CRITICAL BUG: Global config was corrupted! Captcha secret was deleted from global config when returning public config") + t.Logf("Error (expected for new user): %s", errorDesc) + } + } else { + t.Logf("✓ Entry verify succeeded: %v", verifyResult) + } + + // Additional verification: Get config third time and verify again + resp3, err := http.Get(serverURL + baseURL + "/user/entry") + assert.NoError(t, err, "Third GET /user/entry should succeed") + defer resp3.Body.Close() + assert.Equal(t, http.StatusOK, resp3.StatusCode, "Third request should return 200 OK") + t.Logf("✓ Third request: config still works after verify") + + // Final verify to ensure global config is still intact + verifyData2 := map[string]interface{}{ + "username": testEmail, + "captcha": captchaAnswer, + } + + verifyJSON2, err := json.Marshal(verifyData2) + assert.NoError(t, err, "Should marshal second verify request") + + verifyResp2, err := http.Post( + serverURL+baseURL+"/user/entry/verify", + "application/json", + bytes.NewReader(verifyJSON2), + ) + assert.NoError(t, err, "Second POST /user/entry/verify should succeed") + defer verifyResp2.Body.Close() + + verifyBody2, err := io.ReadAll(verifyResp2.Body) + assert.NoError(t, err, "Should read second verify response body") + + var verifyResult2 map[string]interface{} + err = json.Unmarshal(verifyBody2, &verifyResult2) + assert.NoError(t, err, "Should decode second verify response") + + // Final critical assertion + if verifyResp2.StatusCode != http.StatusOK { + if errorDesc, ok := verifyResult2["error_description"].(string); ok { + assert.NotContains(t, errorDesc, "Turnstile secret not configured", + "CRITICAL BUG STILL EXISTS: Global config was corrupted on second verify!") + } + } + + t.Log("✅ SUCCESS: Deep copy fix is working correctly!") + t.Log("✅ Global config is NOT corrupted after multiple public config requests") + }) +} diff --git a/openapi/user/entry.go b/openapi/user/entry.go index 7b218ed9..4d314fa0 100644 --- a/openapi/user/entry.go +++ b/openapi/user/entry.go @@ -42,21 +42,8 @@ func getEntryConfig(c *gin.Context) { return } - // Create public config without sensitive data - publicConfig := *config - publicConfig.ClientSecret = "" // Remove sensitive data - - // Remove captcha secret from public config - if publicConfig.Form != nil && publicConfig.Form.Captcha != nil && publicConfig.Form.Captcha.Options != nil { - // Create a copy of captcha options without the secret - captchaOptions := make(map[string]interface{}) - for k, v := range publicConfig.Form.Captcha.Options { - if k != "secret" { - captchaOptions[k] = v - } - } - publicConfig.Form.Captcha.Options = captchaOptions - } + // Create public config without sensitive data (deep copy to avoid modifying global config) + publicConfig := createPublicEntryConfig(config) // Return the entry configuration response.RespondWithSuccess(c, response.StatusOK, publicConfig) @@ -190,13 +177,13 @@ func GinEntryVerify(c *gin.Context) { // If user exists: return login status if userExists { - verifyResp.Status = "login" + verifyResp.Status = EntryVerificationStatusLogin response.RespondWithSuccess(c, response.StatusOK, verifyResp) return } // User doesn't exist: send verification code and return register status - verifyResp.Status = "register" + verifyResp.Status = EntryVerificationStatusRegister // Send verification code asynchronously go func() { @@ -394,10 +381,10 @@ func sendEntryVerificationCode(ctx context.Context, config *EntryConfig, usernam // Prepare template data templateData := messengertypes.TemplateData{ - "to": username, - "verification_code": verificationCode, - "expires_in": "10", // 10 minutes - "locale": locale, + "to": username, + "code": verificationCode, // Variable name matches template: {{ code }} + "expires_in": "10", // 10 minutes + "locale": locale, } // Send verification code @@ -408,3 +395,142 @@ func sendEntryVerificationCode(ctx context.Context, config *EntryConfig, usernam return nil } + +// createPublicEntryConfig creates a deep copy of EntryConfig without sensitive data +// This prevents modifying the global config when removing secrets +func createPublicEntryConfig(config *EntryConfig) *EntryConfig { + if config == nil { + return nil + } + + // Create a new config instance + publicConfig := &EntryConfig{ + Title: config.Title, + Description: config.Description, + Default: config.Default, + SuccessURL: config.SuccessURL, + FailureURL: config.FailureURL, + LogoutRedirect: config.LogoutRedirect, + ClientID: config.ClientID, + ClientSecret: "", // Remove sensitive data + AutoLogin: config.AutoLogin, + Role: config.Role, + Type: config.Type, + InviteRequired: config.InviteRequired, + } + + // Deep copy Form config + if config.Form != nil { + publicConfig.Form = &FormConfig{ + ForgotPasswordLink: config.Form.ForgotPasswordLink, + RememberMe: config.Form.RememberMe, + RegisterLink: config.Form.RegisterLink, + LoginLink: config.Form.LoginLink, + TermsOfServiceLink: config.Form.TermsOfServiceLink, + PrivacyPolicyLink: config.Form.PrivacyPolicyLink, + } + + // Deep copy Username config + if config.Form.Username != nil { + publicConfig.Form.Username = &UsernameConfig{ + Placeholder: config.Form.Username.Placeholder, + } + if config.Form.Username.Fields != nil { + publicConfig.Form.Username.Fields = make([]string, len(config.Form.Username.Fields)) + copy(publicConfig.Form.Username.Fields, config.Form.Username.Fields) + } + } + + // Deep copy Password config + if config.Form.Password != nil { + publicConfig.Form.Password = &PasswordConfig{ + Placeholder: config.Form.Password.Placeholder, + } + } + + // Deep copy ConfirmPassword config + if config.Form.ConfirmPassword != nil { + publicConfig.Form.ConfirmPassword = &PasswordConfig{ + Placeholder: config.Form.ConfirmPassword.Placeholder, + } + } + + // Deep copy Captcha config (WITHOUT secret) + if config.Form.Captcha != nil { + publicConfig.Form.Captcha = &CaptchaConfig{ + Type: config.Form.Captcha.Type, + } + + // Deep copy Options, excluding "secret" + if config.Form.Captcha.Options != nil { + publicConfig.Form.Captcha.Options = make(map[string]interface{}) + for k, v := range config.Form.Captcha.Options { + if k != "secret" { + publicConfig.Form.Captcha.Options[k] = v + } + } + } + } + } + + // Deep copy Token config + if config.Token != nil { + publicConfig.Token = &TokenConfig{ + ExpiresIn: config.Token.ExpiresIn, + RememberMeExpiresIn: config.Token.RememberMeExpiresIn, + } + } + + // Deep copy Messenger config (without sensitive data) + if config.Messenger != nil { + publicConfig.Messenger = &MessengerConfig{} + + if config.Messenger.Mail != nil { + publicConfig.Messenger.Mail = &MessengerChannelConfig{ + Channel: config.Messenger.Mail.Channel, + Template: config.Messenger.Mail.Template, + } + } + + if config.Messenger.SMS != nil { + publicConfig.Messenger.SMS = &MessengerChannelConfig{ + Channel: config.Messenger.SMS.Channel, + Template: config.Messenger.SMS.Template, + } + } + } + + // Deep copy ThirdParty config + if config.ThirdParty != nil { + publicConfig.ThirdParty = &ThirdParty{} + + if config.ThirdParty.Providers != nil { + publicConfig.ThirdParty.Providers = make([]*Provider, len(config.ThirdParty.Providers)) + for i, provider := range config.ThirdParty.Providers { + if provider != nil { + // Create a copy of provider without sensitive data + publicConfig.ThirdParty.Providers[i] = &Provider{ + ID: provider.ID, + Label: provider.Label, + Title: provider.Title, + Logo: provider.Logo, + Color: provider.Color, + TextColor: provider.TextColor, + ClientID: provider.ClientID, + ResponseMode: provider.ResponseMode, + // ClientSecret is intentionally omitted for security + // ClientSecretGenerator is intentionally omitted for security + } + + // Copy scopes if present + if provider.Scopes != nil { + publicConfig.ThirdParty.Providers[i].Scopes = make([]string, len(provider.Scopes)) + copy(publicConfig.ThirdParty.Providers[i].Scopes, provider.Scopes) + } + } + } + } + } + + return publicConfig +} diff --git a/openapi/user/types.go b/openapi/user/types.go index 0d759a11..f76e9770 100644 --- a/openapi/user/types.go +++ b/openapi/user/types.go @@ -7,6 +7,9 @@ import ( // LoginStatus represents the login status type LoginStatus string +// EntryVerificationStatus represents the entry verification status +type EntryVerificationStatus string + const ( // LoginStatusSuccess is the success status LoginStatusSuccess LoginStatus = "ok" @@ -16,6 +19,13 @@ const ( LoginStatusTeamSelection LoginStatus = "team_selection_required" ) +const ( + // EntryVerificationStatusLogin is the login status + EntryVerificationStatusLogin EntryVerificationStatus = "login" + // EntryVerificationStatusRegister is the register status + EntryVerificationStatusRegister EntryVerificationStatus = "register" +) + const ( // ScopeMFAVerification is the MFA verification scope for temporary access token ScopeMFAVerification = "mfa_verification" @@ -244,13 +254,13 @@ type EntryVerifyRequest struct { // EntryVerifyResponse represents the response for entry verification type EntryVerifyResponse struct { - Status string `json:"status"` // "login" or "register" - AccessToken string `json:"access_token"` // Temporary token for next step - ExpiresIn int `json:"expires_in"` // Token expiration in seconds - TokenType string `json:"token_type"` // Token type (Bearer) - Scope string `json:"scope"` // Token scope - UserExists bool `json:"user_exists"` // Whether user exists - VerificationSent bool `json:"verification_sent,omitempty"` // Whether verification code was sent (for register) + Status EntryVerificationStatus `json:"status"` // "login" or "register" + AccessToken string `json:"access_token"` // Temporary token for next step + ExpiresIn int `json:"expires_in"` // Token expiration in seconds + TokenType string `json:"token_type"` // Token type (Bearer) + Scope string `json:"scope"` // Token scope + UserExists bool `json:"user_exists"` // Whether user exists + VerificationSent bool `json:"verification_sent,omitempty"` // Whether verification code was sent (for register) } // Built-in preset mapping types