Merge pull request #1205 from trheyi/main

Implement deep copy for entry configuration to prevent global config …
This commit is contained in:
Max 2025-10-15 19:33:09 +08:00 committed by GitHub
commit 2f72b19227
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 305 additions and 28 deletions

View file

@ -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")
})
}

View file

@ -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
}

View file

@ -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