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.
This commit is contained in:
parent
b2dbe7ac51
commit
481323104a
3 changed files with 305 additions and 28 deletions
|
|
@ -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)
|
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")
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -42,21 +42,8 @@ func getEntryConfig(c *gin.Context) {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
// Create public config without sensitive data
|
// Create public config without sensitive data (deep copy to avoid modifying global config)
|
||||||
publicConfig := *config
|
publicConfig := createPublicEntryConfig(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
|
|
||||||
}
|
|
||||||
|
|
||||||
// Return the entry configuration
|
// Return the entry configuration
|
||||||
response.RespondWithSuccess(c, response.StatusOK, publicConfig)
|
response.RespondWithSuccess(c, response.StatusOK, publicConfig)
|
||||||
|
|
@ -190,13 +177,13 @@ func GinEntryVerify(c *gin.Context) {
|
||||||
|
|
||||||
// If user exists: return login status
|
// If user exists: return login status
|
||||||
if userExists {
|
if userExists {
|
||||||
verifyResp.Status = "login"
|
verifyResp.Status = EntryVerificationStatusLogin
|
||||||
response.RespondWithSuccess(c, response.StatusOK, verifyResp)
|
response.RespondWithSuccess(c, response.StatusOK, verifyResp)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
// User doesn't exist: send verification code and return register status
|
// User doesn't exist: send verification code and return register status
|
||||||
verifyResp.Status = "register"
|
verifyResp.Status = EntryVerificationStatusRegister
|
||||||
|
|
||||||
// Send verification code asynchronously
|
// Send verification code asynchronously
|
||||||
go func() {
|
go func() {
|
||||||
|
|
@ -395,7 +382,7 @@ func sendEntryVerificationCode(ctx context.Context, config *EntryConfig, usernam
|
||||||
// Prepare template data
|
// Prepare template data
|
||||||
templateData := messengertypes.TemplateData{
|
templateData := messengertypes.TemplateData{
|
||||||
"to": username,
|
"to": username,
|
||||||
"verification_code": verificationCode,
|
"code": verificationCode, // Variable name matches template: {{ code }}
|
||||||
"expires_in": "10", // 10 minutes
|
"expires_in": "10", // 10 minutes
|
||||||
"locale": locale,
|
"locale": locale,
|
||||||
}
|
}
|
||||||
|
|
@ -408,3 +395,142 @@ func sendEntryVerificationCode(ctx context.Context, config *EntryConfig, usernam
|
||||||
|
|
||||||
return nil
|
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
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -7,6 +7,9 @@ import (
|
||||||
// LoginStatus represents the login status
|
// LoginStatus represents the login status
|
||||||
type LoginStatus string
|
type LoginStatus string
|
||||||
|
|
||||||
|
// EntryVerificationStatus represents the entry verification status
|
||||||
|
type EntryVerificationStatus string
|
||||||
|
|
||||||
const (
|
const (
|
||||||
// LoginStatusSuccess is the success status
|
// LoginStatusSuccess is the success status
|
||||||
LoginStatusSuccess LoginStatus = "ok"
|
LoginStatusSuccess LoginStatus = "ok"
|
||||||
|
|
@ -16,6 +19,13 @@ const (
|
||||||
LoginStatusTeamSelection LoginStatus = "team_selection_required"
|
LoginStatusTeamSelection LoginStatus = "team_selection_required"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
const (
|
||||||
|
// EntryVerificationStatusLogin is the login status
|
||||||
|
EntryVerificationStatusLogin EntryVerificationStatus = "login"
|
||||||
|
// EntryVerificationStatusRegister is the register status
|
||||||
|
EntryVerificationStatusRegister EntryVerificationStatus = "register"
|
||||||
|
)
|
||||||
|
|
||||||
const (
|
const (
|
||||||
// ScopeMFAVerification is the MFA verification scope for temporary access token
|
// ScopeMFAVerification is the MFA verification scope for temporary access token
|
||||||
ScopeMFAVerification = "mfa_verification"
|
ScopeMFAVerification = "mfa_verification"
|
||||||
|
|
@ -244,7 +254,7 @@ type EntryVerifyRequest struct {
|
||||||
|
|
||||||
// EntryVerifyResponse represents the response for entry verification
|
// EntryVerifyResponse represents the response for entry verification
|
||||||
type EntryVerifyResponse struct {
|
type EntryVerifyResponse struct {
|
||||||
Status string `json:"status"` // "login" or "register"
|
Status EntryVerificationStatus `json:"status"` // "login" or "register"
|
||||||
AccessToken string `json:"access_token"` // Temporary token for next step
|
AccessToken string `json:"access_token"` // Temporary token for next step
|
||||||
ExpiresIn int `json:"expires_in"` // Token expiration in seconds
|
ExpiresIn int `json:"expires_in"` // Token expiration in seconds
|
||||||
TokenType string `json:"token_type"` // Token type (Bearer)
|
TokenType string `json:"token_type"` // Token type (Bearer)
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue