You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@answer.apache.org by li...@apache.org on 2026/06/15 06:44:42 UTC
(answer) 02/02: fix(site): require explicit email verification setting
This is an automated email from the ASF dual-hosted git repository.
LinkinStars pushed a commit to branch dev
in repository https://gitbox.apache.org/repos/asf/answer.git
commit 3b6f981b81ca4376138734e4039eab59b1fea63c
Author: Artur Iusupov <iu...@phystech.edu>
AuthorDate: Tue Jun 9 01:39:27 2026 +0400
fix(site): require explicit email verification setting
Replace OptionalBool with an explicit require_email_verification value for the login settings save request while keeping legacy read defaults intact.
Use positive RequireEmailVerification naming through the registration flow and inline the site setting mapping.
Add validation, save-path, and registration coverage for the explicit email verification setting.
---
docs/docs.go | 3 +
docs/swagger.json | 3 +
docs/swagger.yaml | 2 +
internal/controller/user_controller.go | 6 +-
internal/controller/user_controller_test.go | 37 -----------
internal/migrations/v34.go | 2 +
internal/schema/siteinfo_schema.go | 37 ++---------
internal/schema/siteinfo_schema_test.go | 72 ++++++++++++++++++++++
internal/schema/user_schema.go | 14 ++---
internal/service/content/user_service.go | 6 +-
internal/service/content/user_service_test.go | 20 +++---
internal/service/siteinfo/siteinfo_service.go | 15 +----
internal/service/siteinfo/siteinfo_service_test.go | 55 ++++++++---------
13 files changed, 135 insertions(+), 137 deletions(-)
diff --git a/docs/docs.go b/docs/docs.go
index 29e3a962..4e48c88d 100644
--- a/docs/docs.go
+++ b/docs/docs.go
@@ -12081,6 +12081,9 @@ const docTemplate = `{
},
"schema.SiteLoginReq": {
"type": "object",
+ "required": [
+ "require_email_verification"
+ ],
"properties": {
"allow_email_domains": {
"type": "array",
diff --git a/docs/swagger.json b/docs/swagger.json
index 2cc8f130..a075dfe4 100644
--- a/docs/swagger.json
+++ b/docs/swagger.json
@@ -12054,6 +12054,9 @@
},
"schema.SiteLoginReq": {
"type": "object",
+ "required": [
+ "require_email_verification"
+ ],
"properties": {
"allow_email_domains": {
"type": "array",
diff --git a/docs/swagger.yaml b/docs/swagger.yaml
index 2f7e3754..b3416a10 100644
--- a/docs/swagger.yaml
+++ b/docs/swagger.yaml
@@ -2518,6 +2518,8 @@ definitions:
type: boolean
require_email_verification:
type: boolean
+ required:
+ - require_email_verification
type: object
schema.SiteLoginResp:
properties:
diff --git a/internal/controller/user_controller.go b/internal/controller/user_controller.go
index 9552182e..531d88b4 100644
--- a/internal/controller/user_controller.go
+++ b/internal/controller/user_controller.go
@@ -279,7 +279,7 @@ func (uc *UserController) UserRegisterByEmail(ctx *gin.Context) {
handler.HandleResponse(ctx, errors.BadRequest(reason.EmailIllegalDomainError), nil)
return
}
- applyEmailVerificationSetting(req, siteInfo)
+ req.RequireEmailVerification = siteInfo.RequireEmailVerification
req.IP = ctx.ClientIP()
isAdmin := middleware.GetUserIsAdminModerator(ctx)
if !isAdmin {
@@ -306,10 +306,6 @@ func (uc *UserController) UserRegisterByEmail(ctx *gin.Context) {
}
}
-func applyEmailVerificationSetting(req *schema.UserRegisterReq, siteInfo *schema.SiteLoginResp) {
- req.SkipEmailVerification = !siteInfo.RequireEmailVerification
-}
-
// UserVerifyEmail godoc
// @Summary UserVerifyEmail
// @Description UserVerifyEmail
diff --git a/internal/controller/user_controller_test.go b/internal/controller/user_controller_test.go
deleted file mode 100644
index e48dc863..00000000
--- a/internal/controller/user_controller_test.go
+++ /dev/null
@@ -1,37 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-package controller
-
-import (
- "testing"
-
- "github.com/apache/answer/internal/schema"
- "github.com/stretchr/testify/assert"
-)
-
-func TestApplyEmailVerificationSetting(t *testing.T) {
- req := &schema.UserRegisterReq{}
- applyEmailVerificationSetting(req, &schema.SiteLoginResp{RequireEmailVerification: false})
- assert.True(t, req.SkipEmailVerification)
-
- req = &schema.UserRegisterReq{}
- applyEmailVerificationSetting(req, &schema.SiteLoginResp{RequireEmailVerification: true})
- assert.False(t, req.SkipEmailVerification)
-}
diff --git a/internal/migrations/v34.go b/internal/migrations/v34.go
index 1cc1895b..245bb976 100644
--- a/internal/migrations/v34.go
+++ b/internal/migrations/v34.go
@@ -67,6 +67,8 @@ func backfillRequireEmailVerification(content string) (string, error) {
}
requireEmailVerification, exists := loginConfig["require_email_verification"]
+ // Legacy configs that predate this setting should keep the safer behavior.
+ // Treat a missing or null value as requiring email verification.
if !exists || bytes.Equal(bytes.TrimSpace(requireEmailVerification), []byte("null")) {
loginConfig["require_email_verification"] = json.RawMessage("true")
} else {
diff --git a/internal/schema/siteinfo_schema.go b/internal/schema/siteinfo_schema.go
index 3cd27873..1d0b27ff 100644
--- a/internal/schema/siteinfo_schema.go
+++ b/internal/schema/siteinfo_schema.go
@@ -21,7 +21,6 @@ package schema
import (
"context"
- "encoding/json"
"fmt"
"net/mail"
"net/url"
@@ -217,39 +216,13 @@ type SiteUsersReq struct {
AllowUpdateLocation bool `json:"allow_update_location"`
}
-// OptionalBool preserves whether a JSON boolean field was omitted, set to null,
-// or set to a concrete boolean value.
-type OptionalBool struct {
- Set bool `json:"-"`
- Null bool `json:"-"`
- Value bool `json:"-"`
-}
-
-func (b *OptionalBool) UnmarshalJSON(data []byte) error {
- b.Set = true
- if string(data) == "null" {
- b.Null = true
- b.Value = false
- return nil
- }
- b.Null = false
- return json.Unmarshal(data, &b.Value)
-}
-
-func (b OptionalBool) MarshalJSON() ([]byte, error) {
- if !b.Set || b.Null {
- return []byte("null"), nil
- }
- return json.Marshal(b.Value)
-}
-
// SiteLoginReq site login request
type SiteLoginReq struct {
- AllowNewRegistrations bool `json:"allow_new_registrations"`
- AllowEmailRegistrations bool `json:"allow_email_registrations"`
- AllowPasswordLogin bool `json:"allow_password_login"`
- AllowEmailDomains []string `json:"allow_email_domains"`
- RequireEmailVerification OptionalBool `json:"require_email_verification" swaggertype:"boolean"`
+ AllowNewRegistrations bool `json:"allow_new_registrations"`
+ AllowEmailRegistrations bool `json:"allow_email_registrations"`
+ AllowPasswordLogin bool `json:"allow_password_login"`
+ AllowEmailDomains []string `json:"allow_email_domains"`
+ RequireEmailVerification *bool `validate:"required" json:"require_email_verification" swaggertype:"boolean"`
}
// SiteLoginResp site login response
diff --git a/internal/schema/siteinfo_schema_test.go b/internal/schema/siteinfo_schema_test.go
new file mode 100644
index 00000000..e5413f4a
--- /dev/null
+++ b/internal/schema/siteinfo_schema_test.go
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package schema
+
+import (
+ "encoding/json"
+ "testing"
+
+ "github.com/apache/answer/internal/base/validator"
+ "github.com/segmentfault/pacman/i18n"
+ "github.com/stretchr/testify/require"
+)
+
+func TestSiteLoginReqRequireEmailVerificationValidation(t *testing.T) {
+ tests := []struct {
+ name string
+ payload string
+ expectError bool
+ }{
+ {
+ name: "omitted is invalid",
+ payload: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"allow_email_domains":[]}`,
+ expectError: true,
+ },
+ {
+ name: "null is invalid",
+ payload: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"allow_email_domains":[],"require_email_verification":null}`,
+ expectError: true,
+ },
+ {
+ name: "false is valid",
+ payload: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"allow_email_domains":[],"require_email_verification":false}`,
+ expectError: false,
+ },
+ {
+ name: "true is valid",
+ payload: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"allow_email_domains":[],"require_email_verification":true}`,
+ expectError: false,
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ req := &SiteLoginReq{}
+ require.NoError(t, json.Unmarshal([]byte(tt.payload), req))
+
+ _, err := validator.GetValidatorByLang(i18n.DefaultLanguage).Check(req)
+ if tt.expectError {
+ require.Error(t, err)
+ } else {
+ require.NoError(t, err)
+ }
+ })
+ }
+}
diff --git a/internal/schema/user_schema.go b/internal/schema/user_schema.go
index 7d0c55a5..0683a5af 100644
--- a/internal/schema/user_schema.go
+++ b/internal/schema/user_schema.go
@@ -219,13 +219,13 @@ type UserEmailLoginReq struct {
// UserRegisterReq user register request
type UserRegisterReq struct {
- Name string `validate:"required,gte=2,lte=30" json:"name"`
- Email string `validate:"required,email,gt=0,lte=500" json:"e_mail" `
- Pass string `validate:"required,gte=8,lte=32" json:"pass"`
- CaptchaID string `json:"captcha_id"`
- CaptchaCode string `json:"captcha_code"`
- IP string `json:"-" `
- SkipEmailVerification bool `json:"-"`
+ Name string `validate:"required,gte=2,lte=30" json:"name"`
+ Email string `validate:"required,email,gt=0,lte=500" json:"e_mail" `
+ Pass string `validate:"required,gte=8,lte=32" json:"pass"`
+ CaptchaID string `json:"captcha_id"`
+ CaptchaCode string `json:"captcha_code"`
+ IP string `json:"-" `
+ RequireEmailVerification bool `json:"-"`
}
func (u *UserRegisterReq) Check() (errFields []*validator.FormErrorField, err error) {
diff --git a/internal/service/content/user_service.go b/internal/service/content/user_service.go
index 1d7866f3..d0ebe875 100644
--- a/internal/service/content/user_service.go
+++ b/internal/service/content/user_service.go
@@ -518,7 +518,7 @@ func (us *UserService) UserRegisterByEmail(ctx context.Context, registerUserInfo
log.Errorf("set default user notification config failed, err: %v", err)
}
- err = applyRegistrationVerification(userInfo, registerUserInfo.SkipEmailVerification, registrationVerificationActions{
+ err = applyRegistrationVerification(userInfo, registerUserInfo.RequireEmailVerification, registrationVerificationActions{
sendActivationEmail: func() error {
return us.sendRegistrationActivationEmail(ctx, userInfo)
},
@@ -569,10 +569,10 @@ type registrationVerificationActions struct {
}
func applyRegistrationVerification(
- userInfo *entity.User, skipEmailVerification bool, actions registrationVerificationActions,
+ userInfo *entity.User, requireEmailVerification bool, actions registrationVerificationActions,
) error {
userInfo.MailStatus = entity.EmailStatusToBeVerified
- if !skipEmailVerification {
+ if requireEmailVerification {
return actions.sendActivationEmail()
}
diff --git a/internal/service/content/user_service_test.go b/internal/service/content/user_service_test.go
index 0d77b383..d77a1c44 100644
--- a/internal/service/content/user_service_test.go
+++ b/internal/service/content/user_service_test.go
@@ -29,11 +29,11 @@ import (
)
func TestApplyRegistrationVerification(t *testing.T) {
- t.Run("enabled sends activation email and leaves user inactive", func(t *testing.T) {
+ t.Run("required sends activation email and leaves email pending", func(t *testing.T) {
userInfo := &entity.User{}
calls := map[string]int{}
- err := applyRegistrationVerification(userInfo, false, registrationVerificationActions{
+ err := applyRegistrationVerification(userInfo, true, registrationVerificationActions{
sendActivationEmail: func() error {
calls["sendActivationEmail"]++
return nil
@@ -55,11 +55,11 @@ func TestApplyRegistrationVerification(t *testing.T) {
assert.Zero(t, calls["markEmailAvailable"])
})
- t.Run("disabled activates once and marks email available", func(t *testing.T) {
+ t.Run("not required activates once and marks email available", func(t *testing.T) {
userInfo := &entity.User{}
calls := map[string]int{}
- err := applyRegistrationVerification(userInfo, true, registrationVerificationActions{
+ err := applyRegistrationVerification(userInfo, false, registrationVerificationActions{
sendActivationEmail: func() error {
calls["sendActivationEmail"]++
return nil
@@ -81,11 +81,11 @@ func TestApplyRegistrationVerification(t *testing.T) {
assert.Equal(t, 1, calls["markEmailAvailable"])
})
- t.Run("disabled user activation failure falls back to email verification", func(t *testing.T) {
+ t.Run("not required user activation failure falls back to email verification", func(t *testing.T) {
userInfo := &entity.User{}
calls := map[string]int{}
- err := applyRegistrationVerification(userInfo, true, registrationVerificationActions{
+ err := applyRegistrationVerification(userInfo, false, registrationVerificationActions{
sendActivationEmail: func() error {
calls["sendActivationEmail"]++
return nil
@@ -107,11 +107,11 @@ func TestApplyRegistrationVerification(t *testing.T) {
assert.Zero(t, calls["markEmailAvailable"])
})
- t.Run("disabled email status failure falls back to email verification", func(t *testing.T) {
+ t.Run("not required email status failure falls back to email verification", func(t *testing.T) {
userInfo := &entity.User{}
calls := map[string]int{}
- err := applyRegistrationVerification(userInfo, true, registrationVerificationActions{
+ err := applyRegistrationVerification(userInfo, false, registrationVerificationActions{
sendActivationEmail: func() error {
calls["sendActivationEmail"]++
return nil
@@ -133,11 +133,11 @@ func TestApplyRegistrationVerification(t *testing.T) {
assert.Equal(t, 1, calls["markEmailAvailable"])
})
- t.Run("fallback email failure returns before active status", func(t *testing.T) {
+ t.Run("fallback email failure returns before available email status", func(t *testing.T) {
userInfo := &entity.User{}
expectedErr := errors.New("email failed")
- err := applyRegistrationVerification(userInfo, true, registrationVerificationActions{
+ err := applyRegistrationVerification(userInfo, false, registrationVerificationActions{
sendActivationEmail: func() error {
return expectedErr
},
diff --git a/internal/service/siteinfo/siteinfo_service.go b/internal/service/siteinfo/siteinfo_service.go
index 7656a140..8b32b722 100644
--- a/internal/service/siteinfo/siteinfo_service.go
+++ b/internal/service/siteinfo/siteinfo_service.go
@@ -291,17 +291,8 @@ func (s *SiteInfoService) SaveSiteSecurity(ctx context.Context, req *schema.Site
// SaveSiteLogin save site legal configuration
func (s *SiteInfoService) SaveSiteLogin(ctx context.Context, req *schema.SiteLoginReq) (err error) {
- requireEmailVerification := true
- if req.RequireEmailVerification.Set {
- if !req.RequireEmailVerification.Null {
- requireEmailVerification = req.RequireEmailVerification.Value
- }
- } else {
- currentLogin, err := s.GetSiteLogin(ctx)
- if err != nil {
- return err
- }
- requireEmailVerification = currentLogin.RequireEmailVerification
+ if req.RequireEmailVerification == nil {
+ return errors.BadRequest(reason.RequestFormatError)
}
loginConfig := &schema.SiteLoginResp{
@@ -309,7 +300,7 @@ func (s *SiteInfoService) SaveSiteLogin(ctx context.Context, req *schema.SiteLog
AllowEmailRegistrations: req.AllowEmailRegistrations,
AllowPasswordLogin: req.AllowPasswordLogin,
AllowEmailDomains: req.AllowEmailDomains,
- RequireEmailVerification: requireEmailVerification,
+ RequireEmailVerification: *req.RequireEmailVerification,
}
content, _ := json.Marshal(loginConfig)
data := &entity.SiteInfo{
diff --git a/internal/service/siteinfo/siteinfo_service_test.go b/internal/service/siteinfo/siteinfo_service_test.go
index fc4bbdf7..a3f834a5 100644
--- a/internal/service/siteinfo/siteinfo_service_test.go
+++ b/internal/service/siteinfo/siteinfo_service_test.go
@@ -28,7 +28,6 @@ import (
"github.com/apache/answer/internal/entity"
"github.com/apache/answer/internal/schema"
"github.com/apache/answer/internal/service/mock"
- "github.com/apache/answer/internal/service/siteinfo_common"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/mock/gomock"
@@ -37,33 +36,17 @@ import (
func TestSiteInfoService_SaveSiteLoginRequireEmailVerification(t *testing.T) {
tests := []struct {
name string
- currentContent string
- requestPayload string
- expectGet bool
+ requireEmail bool
expectedRequire bool
}{
{
- name: "omitted preserves normalized default",
- currentContent: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true}`,
- requestPayload: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"allow_email_domains":[]}`,
- expectGet: true,
- expectedRequire: true,
- },
- {
- name: "omitted preserves current false",
- currentContent: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"require_email_verification":false}`,
- requestPayload: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"allow_email_domains":[]}`,
- expectGet: true,
- expectedRequire: false,
- },
- {
- name: "null normalizes true",
- requestPayload: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"allow_email_domains":[],"require_email_verification":null}`,
+ name: "explicit true persists true",
+ requireEmail: true,
expectedRequire: true,
},
{
name: "explicit false persists false",
- requestPayload: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"allow_email_domains":[],"require_email_verification":false}`,
+ requireEmail: false,
expectedRequire: false,
},
}
@@ -74,11 +57,6 @@ func TestSiteInfoService_SaveSiteLoginRequireEmailVerification(t *testing.T) {
defer ctl.Finish()
repo := mock.NewMockSiteInfoRepo(ctl)
- if tt.expectGet {
- repo.EXPECT().GetByType(gomock.Any(), constant.SiteTypeLogin).
- Return(&entity.SiteInfo{Content: tt.currentContent}, true, nil)
- }
-
var savedContent string
repo.EXPECT().SaveByType(gomock.Any(), constant.SiteTypeLogin, gomock.Any()).
DoAndReturn(func(_ context.Context, _ string, data *entity.SiteInfo) error {
@@ -86,12 +64,15 @@ func TestSiteInfoService_SaveSiteLoginRequireEmailVerification(t *testing.T) {
return nil
})
- req := &schema.SiteLoginReq{}
- require.NoError(t, json.Unmarshal([]byte(tt.requestPayload), req))
-
service := &SiteInfoService{
- siteInfoRepo: repo,
- siteInfoCommonService: siteinfo_common.NewSiteInfoCommonService(repo),
+ siteInfoRepo: repo,
+ }
+ req := &schema.SiteLoginReq{
+ AllowNewRegistrations: true,
+ AllowEmailRegistrations: true,
+ AllowPasswordLogin: true,
+ AllowEmailDomains: []string{},
+ RequireEmailVerification: &tt.requireEmail,
}
require.NoError(t, service.SaveSiteLogin(context.TODO(), req))
assert.NotContains(t, savedContent, `"require_email_verification":null`)
@@ -102,3 +83,15 @@ func TestSiteInfoService_SaveSiteLoginRequireEmailVerification(t *testing.T) {
})
}
}
+
+func TestSiteInfoService_SaveSiteLoginRequiresEmailVerificationValue(t *testing.T) {
+ service := &SiteInfoService{}
+ req := &schema.SiteLoginReq{
+ AllowNewRegistrations: true,
+ AllowEmailRegistrations: true,
+ AllowPasswordLogin: true,
+ AllowEmailDomains: []string{},
+ }
+
+ require.Error(t, service.SaveSiteLogin(context.TODO(), req))
+}