From 6fa92eed4512371825214bdd5f224d27500f6827 Mon Sep 17 00:00:00 2001 From: Artur Iusupov Date: Fri, 5 Jun 2026 00:42:00 +0400 Subject: [PATCH 1/2] feat(site): allow disabling email verification --- docs/docs.go | 6 + docs/swagger.json | 6 + docs/swagger.yaml | 4 + i18n/en_US.yaml | 5 +- internal/controller/user_controller.go | 5 + internal/controller/user_controller_test.go | 37 ++++ internal/migrations/init.go | 7 +- internal/migrations/migrations.go | 1 + internal/migrations/v30.go | 9 +- internal/migrations/v34.go | 85 +++++++++ internal/migrations/v34_test.go | 172 ++++++++++++++++++ internal/schema/siteinfo_schema.go | 48 ++++- internal/schema/user_schema.go | 13 +- internal/service/content/user_service.go | 61 ++++++- internal/service/content/user_service_test.go | 155 ++++++++++++++++ internal/service/siteinfo/siteinfo_service.go | 22 ++- .../service/siteinfo/siteinfo_service_test.go | 104 +++++++++++ .../siteinfo_common/siteinfo_service.go | 2 +- .../siteinfo_common/siteinfo_service_test.go | 44 +++++ ui/src/common/interface.ts | 1 + ui/src/pages/Admin/Login/index.tsx | 15 ++ .../Register/components/SignUpForm/index.tsx | 13 +- ui/src/pages/Users/Register/index.tsx | 20 +- ui/src/services/common.ts | 5 +- ui/src/stores/loginSetting.ts | 1 + 25 files changed, 797 insertions(+), 44 deletions(-) create mode 100644 internal/controller/user_controller_test.go create mode 100644 internal/migrations/v34.go create mode 100644 internal/migrations/v34_test.go create mode 100644 internal/service/content/user_service_test.go create mode 100644 internal/service/siteinfo/siteinfo_service_test.go diff --git a/docs/docs.go b/docs/docs.go index 03b06701b..29e3a9622 100644 --- a/docs/docs.go +++ b/docs/docs.go @@ -12096,6 +12096,9 @@ const docTemplate = `{ }, "allow_password_login": { "type": "boolean" + }, + "require_email_verification": { + "type": "boolean" } } }, @@ -12116,6 +12119,9 @@ const docTemplate = `{ }, "allow_password_login": { "type": "boolean" + }, + "require_email_verification": { + "type": "boolean" } } }, diff --git a/docs/swagger.json b/docs/swagger.json index 3bfb2e3ce..2cc8f1305 100644 --- a/docs/swagger.json +++ b/docs/swagger.json @@ -12069,6 +12069,9 @@ }, "allow_password_login": { "type": "boolean" + }, + "require_email_verification": { + "type": "boolean" } } }, @@ -12089,6 +12092,9 @@ }, "allow_password_login": { "type": "boolean" + }, + "require_email_verification": { + "type": "boolean" } } }, diff --git a/docs/swagger.yaml b/docs/swagger.yaml index c08f1e8d4..2f7e3754e 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -2516,6 +2516,8 @@ definitions: type: boolean allow_password_login: type: boolean + require_email_verification: + type: boolean type: object schema.SiteLoginResp: properties: @@ -2529,6 +2531,8 @@ definitions: type: boolean allow_password_login: type: boolean + require_email_verification: + type: boolean type: object schema.SiteMCPReq: properties: diff --git a/i18n/en_US.yaml b/i18n/en_US.yaml index 61f496500..5d1faa3e0 100644 --- a/i18n/en_US.yaml +++ b/i18n/en_US.yaml @@ -2241,6 +2241,10 @@ ui: title: Email registration label: Allow email registration text: Turn off to prevent anyone creating new account through email. + email_verification: + title: Email verification + label: Require email verification + text: When enabled, users must verify their email address before using the site. allowed_email_domains: title: Allowed email domains text: Email domains that users must register accounts with. One domain per line. Ignored when empty. @@ -2485,4 +2489,3 @@ ui: copied: Copied external_content_warning: External images/media are not displayed. - diff --git a/internal/controller/user_controller.go b/internal/controller/user_controller.go index 77c806e07..9552182ed 100644 --- a/internal/controller/user_controller.go +++ b/internal/controller/user_controller.go @@ -279,6 +279,7 @@ func (uc *UserController) UserRegisterByEmail(ctx *gin.Context) { handler.HandleResponse(ctx, errors.BadRequest(reason.EmailIllegalDomainError), nil) return } + applyEmailVerificationSetting(req, siteInfo) req.IP = ctx.ClientIP() isAdmin := middleware.GetUserIsAdminModerator(ctx) if !isAdmin { @@ -305,6 +306,10 @@ 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 new file mode 100644 index 000000000..e48dc8636 --- /dev/null +++ b/internal/controller/user_controller_test.go @@ -0,0 +1,37 @@ +/* + * 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/init.go b/internal/migrations/init.go index 9dbe6eb8e..d5098dd0c 100644 --- a/internal/migrations/init.go +++ b/internal/migrations/init.go @@ -226,9 +226,10 @@ func (m *Mentor) initSiteInfoGeneralData() { func (m *Mentor) initSiteInfoLoginConfig() { loginConfig := map[string]any{ - "allow_new_registrations": true, - "allow_email_registrations": true, - "allow_password_login": true, + "allow_new_registrations": true, + "allow_email_registrations": true, + "allow_password_login": true, + "require_email_verification": true, } loginConfigDataBytes, _ := json.Marshal(loginConfig) _, m.err = m.engine.Context(m.ctx).Insert(&entity.SiteInfo{ diff --git a/internal/migrations/migrations.go b/internal/migrations/migrations.go index 7fca2d50d..59fbb7bea 100644 --- a/internal/migrations/migrations.go +++ b/internal/migrations/migrations.go @@ -109,6 +109,7 @@ var migrations = []Migration{ NewMigration("v1.8.1", "ai feat", aiFeat, true), NewMigration("v2.0.1", "change avatar type to text", updateAvatarType, false), NewMigration("v2.0.2", "add reasoning content to ai conversation record", addAIConversationReasoningContent, false), + NewMigration("v2.0.3", "add require email verification login setting", addRequireEmailVerification, true), } func GetMigrations() []Migration { diff --git a/internal/migrations/v30.go b/internal/migrations/v30.go index 72765e372..bf785755a 100644 --- a/internal/migrations/v30.go +++ b/internal/migrations/v30.go @@ -302,10 +302,11 @@ func splitLegalMenu(ctx context.Context, x *xorm.Engine) error { PrivacyPolicyParsedText: oldSiteLegal.PrivacyPolicyParsedText, } siteLogin := &schema.SiteLoginResp{ - AllowNewRegistrations: oldSiteLogin.AllowNewRegistrations, - AllowEmailRegistrations: oldSiteLogin.AllowEmailRegistrations, - AllowPasswordLogin: oldSiteLogin.AllowPasswordLogin, - AllowEmailDomains: oldSiteLogin.AllowEmailDomains, + AllowNewRegistrations: oldSiteLogin.AllowNewRegistrations, + AllowEmailRegistrations: oldSiteLogin.AllowEmailRegistrations, + AllowPasswordLogin: oldSiteLogin.AllowPasswordLogin, + AllowEmailDomains: oldSiteLogin.AllowEmailDomains, + RequireEmailVerification: true, } siteGeneral := &schema.SiteGeneralReq{ Name: oldSiteGeneral.Name, diff --git a/internal/migrations/v34.go b/internal/migrations/v34.go new file mode 100644 index 000000000..1cc1895ba --- /dev/null +++ b/internal/migrations/v34.go @@ -0,0 +1,85 @@ +/* + * 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 migrations + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "strings" + + "github.com/apache/answer/internal/base/constant" + "github.com/apache/answer/internal/entity" + "xorm.io/xorm" +) + +func addRequireEmailVerification(ctx context.Context, x *xorm.Engine) error { + loginSiteInfo := &entity.SiteInfo{} + exist, err := x.Context(ctx).Where("type = ?", constant.SiteTypeLogin).Get(loginSiteInfo) + if err != nil { + return fmt.Errorf("get login config failed: %w", err) + } + if !exist { + return nil + } + + content, err := backfillRequireEmailVerification(loginSiteInfo.Content) + if err != nil { + return fmt.Errorf("backfill login config failed: %w", err) + } + loginSiteInfo.Content = content + _, err = x.Context(ctx).ID(loginSiteInfo.ID).Cols("content").Update(loginSiteInfo) + if err != nil { + return fmt.Errorf("update login config failed: %w", err) + } + return nil +} + +func backfillRequireEmailVerification(content string) (string, error) { + if strings.TrimSpace(content) == "" { + content = "{}" + } + + loginConfig := map[string]json.RawMessage{} + if err := json.Unmarshal([]byte(content), &loginConfig); err != nil { + return "", err + } + if loginConfig == nil { + loginConfig = map[string]json.RawMessage{} + } + + requireEmailVerification, exists := loginConfig["require_email_verification"] + if !exists || bytes.Equal(bytes.TrimSpace(requireEmailVerification), []byte("null")) { + loginConfig["require_email_verification"] = json.RawMessage("true") + } else { + var value bool + if err := json.Unmarshal(requireEmailVerification, &value); err != nil { + return "", err + } + loginConfig["require_email_verification"] = requireEmailVerification + } + + data, err := json.Marshal(loginConfig) + if err != nil { + return "", err + } + return string(data), nil +} diff --git a/internal/migrations/v34_test.go b/internal/migrations/v34_test.go new file mode 100644 index 000000000..e082ed158 --- /dev/null +++ b/internal/migrations/v34_test.go @@ -0,0 +1,172 @@ +/* + * 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 migrations + +import ( + "context" + "encoding/json" + "testing" + + "github.com/apache/answer/internal/base/constant" + "github.com/apache/answer/internal/entity" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "xorm.io/xorm" +) + +func TestBackfillRequireEmailVerification(t *testing.T) { + tests := []struct { + name string + content string + expected bool + }{ + { + name: "adds true for missing key", + content: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true}`, + expected: true, + }, + { + name: "converts null to true", + content: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"require_email_verification":null}`, + expected: true, + }, + { + name: "preserves false", + content: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"require_email_verification":false}`, + expected: false, + }, + { + name: "preserves true", + content: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"require_email_verification":true}`, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + content, err := backfillRequireEmailVerification(tt.content) + require.NoError(t, err) + + var result map[string]bool + require.NoError(t, json.Unmarshal([]byte(content), &result)) + assert.Equal(t, tt.expected, result["require_email_verification"]) + }) + } +} + +func TestAddRequireEmailVerificationAbsentLoginRow(t *testing.T) { + x, err := xorm.NewEngine("sqlite", ":memory:") + require.NoError(t, err) + defer func() { + _ = x.Close() + }() + require.NoError(t, x.Sync(new(entity.SiteInfo))) + + require.NoError(t, addRequireEmailVerification(context.TODO(), x)) +} + +func TestAddRequireEmailVerificationUpdatesLoginRow(t *testing.T) { + x, err := xorm.NewEngine("sqlite", ":memory:") + require.NoError(t, err) + defer func() { + _ = x.Close() + }() + require.NoError(t, x.Sync(new(entity.SiteInfo))) + + _, err = x.Insert(&entity.SiteInfo{ + Type: constant.SiteTypeLogin, + Content: `{"allow_new_registrations":true}`, + Status: 1, + }) + require.NoError(t, err) + + require.NoError(t, addRequireEmailVerification(context.TODO(), x)) + + login := &entity.SiteInfo{} + exist, err := x.Where("type = ?", constant.SiteTypeLogin).Get(login) + require.NoError(t, err) + require.True(t, exist) + + var result struct { + RequireEmailVerification bool `json:"require_email_verification"` + } + require.NoError(t, json.Unmarshal([]byte(login.Content), &result)) + assert.True(t, result.RequireEmailVerification) +} + +func TestSplitLegalMenuKeepsRequireEmailVerificationDefaultTrue(t *testing.T) { + x, err := xorm.NewEngine("sqlite", ":memory:") + require.NoError(t, err) + defer func() { + _ = x.Close() + }() + require.NoError(t, x.Sync(new(entity.SiteInfo))) + + _, err = x.Insert(&entity.SiteInfo{ + Type: constant.SiteTypeLegal, + Content: `{ + "terms_of_service_original_text":"tos", + "terms_of_service_parsed_text":"tos", + "privacy_policy_original_text":"privacy", + "privacy_policy_parsed_text":"privacy", + "external_content_display":"always_display" + }`, + Status: 1, + }) + require.NoError(t, err) + _, err = x.Insert(&entity.SiteInfo{ + Type: constant.SiteTypeLogin, + Content: `{ + "allow_new_registrations":true, + "allow_email_registrations":true, + "allow_password_login":true, + "login_required":false, + "allow_email_domains":[] + }`, + Status: 1, + }) + require.NoError(t, err) + _, err = x.Insert(&entity.SiteInfo{ + Type: constant.SiteTypeGeneral, + Content: `{ + "name":"site", + "short_description":"short", + "description":"description", + "site_url":"https://example.com", + "contact_email":"admin@example.com", + "check_update":true + }`, + Status: 1, + }) + require.NoError(t, err) + + require.NoError(t, splitLegalMenu(context.TODO(), x)) + + login := &entity.SiteInfo{} + exist, err := x.Where("type = ?", constant.SiteTypeLogin).Get(login) + require.NoError(t, err) + require.True(t, exist) + + var result struct { + RequireEmailVerification bool `json:"require_email_verification"` + } + require.NoError(t, json.Unmarshal([]byte(login.Content), &result)) + assert.True(t, result.RequireEmailVerification) +} diff --git a/internal/schema/siteinfo_schema.go b/internal/schema/siteinfo_schema.go index bdf2308d3..3cd27873a 100644 --- a/internal/schema/siteinfo_schema.go +++ b/internal/schema/siteinfo_schema.go @@ -21,6 +21,7 @@ package schema import ( "context" + "encoding/json" "fmt" "net/mail" "net/url" @@ -216,12 +217,48 @@ 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"` + 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"` +} + +// SiteLoginResp site login response +type SiteLoginResp 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 bool `json:"require_email_verification"` } // SiteCustomCssHTMLReq site custom css html @@ -310,9 +347,6 @@ type SiteInterfaceResp SiteInterfaceReq // SiteBrandingResp site branding response type SiteBrandingResp SiteBrandingReq -// SiteLoginResp site login response -type SiteLoginResp SiteLoginReq - // SiteCustomCssHTMLResp site custom css html response type SiteCustomCssHTMLResp SiteCustomCssHTMLReq diff --git a/internal/schema/user_schema.go b/internal/schema/user_schema.go index d209c8f58..7d0c55a58 100644 --- a/internal/schema/user_schema.go +++ b/internal/schema/user_schema.go @@ -219,12 +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:"-" ` + 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:"-"` } 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 42a2efda7..1d7866f35 100644 --- a/internal/service/content/user_service.go +++ b/internal/service/content/user_service.go @@ -518,18 +518,20 @@ func (us *UserService) UserRegisterByEmail(ctx context.Context, registerUserInfo log.Errorf("set default user notification config failed, err: %v", err) } - // send email - data := &schema.EmailCodeContent{ - Email: registerUserInfo.Email, - UserID: userInfo.ID, - } - code := token.GenerateToken() - verifyEmailURL := fmt.Sprintf("%s/users/account-activation?code=%s", us.getSiteUrl(ctx), code) - title, body, err := us.emailService.RegisterTemplate(ctx, verifyEmailURL) + err = applyRegistrationVerification(userInfo, registerUserInfo.SkipEmailVerification, registrationVerificationActions{ + sendActivationEmail: func() error { + return us.sendRegistrationActivationEmail(ctx, userInfo) + }, + activateUser: func() error { + return us.userActivity.UserActive(ctx, userInfo.ID) + }, + markEmailAvailable: func() error { + return us.userRepo.UpdateEmailStatus(ctx, userInfo.ID, entity.EmailStatusAvailable) + }, + }) if err != nil { return nil, nil, err } - go us.emailService.SendAndSaveCode(ctx, userInfo.ID, userInfo.EMail, title, body, code, data.ToJSONString()) roleID, err := us.userRoleService.GetUserRole(ctx, userInfo.ID) if err != nil { @@ -560,6 +562,47 @@ func (us *UserService) UserRegisterByEmail(ctx context.Context, registerUserInfo return resp, nil, nil } +type registrationVerificationActions struct { + sendActivationEmail func() error + activateUser func() error + markEmailAvailable func() error +} + +func applyRegistrationVerification( + userInfo *entity.User, skipEmailVerification bool, actions registrationVerificationActions, +) error { + userInfo.MailStatus = entity.EmailStatusToBeVerified + if !skipEmailVerification { + return actions.sendActivationEmail() + } + + if err := actions.activateUser(); err != nil { + log.Errorf("activate user during registration failed, fallback to email verification, err: %v", err) + return actions.sendActivationEmail() + } + if err := actions.markEmailAvailable(); err != nil { + log.Errorf("mark email available during registration failed, fallback to email verification, err: %v", err) + return actions.sendActivationEmail() + } + userInfo.MailStatus = entity.EmailStatusAvailable + return nil +} + +func (us *UserService) sendRegistrationActivationEmail(ctx context.Context, userInfo *entity.User) error { + data := &schema.EmailCodeContent{ + Email: userInfo.EMail, + UserID: userInfo.ID, + } + code := token.GenerateToken() + verifyEmailURL := fmt.Sprintf("%s/users/account-activation?code=%s", us.getSiteUrl(ctx), code) + title, body, err := us.emailService.RegisterTemplate(ctx, verifyEmailURL) + if err != nil { + return err + } + go us.emailService.SendAndSaveCode(ctx, userInfo.ID, userInfo.EMail, title, body, code, data.ToJSONString()) + return nil +} + func (us *UserService) UserVerifyEmailSend(ctx context.Context, userID string) error { userInfo, has, err := us.userRepo.GetByUserID(ctx, userID) if err != nil { diff --git a/internal/service/content/user_service_test.go b/internal/service/content/user_service_test.go new file mode 100644 index 000000000..0d77b383d --- /dev/null +++ b/internal/service/content/user_service_test.go @@ -0,0 +1,155 @@ +/* + * 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 content + +import ( + "errors" + "testing" + + "github.com/apache/answer/internal/entity" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestApplyRegistrationVerification(t *testing.T) { + t.Run("enabled sends activation email and leaves user inactive", func(t *testing.T) { + userInfo := &entity.User{} + calls := map[string]int{} + + err := applyRegistrationVerification(userInfo, false, registrationVerificationActions{ + sendActivationEmail: func() error { + calls["sendActivationEmail"]++ + return nil + }, + activateUser: func() error { + calls["activateUser"]++ + return nil + }, + markEmailAvailable: func() error { + calls["markEmailAvailable"]++ + return nil + }, + }) + + require.NoError(t, err) + assert.Equal(t, entity.EmailStatusToBeVerified, userInfo.MailStatus) + assert.Equal(t, 1, calls["sendActivationEmail"]) + assert.Zero(t, calls["activateUser"]) + assert.Zero(t, calls["markEmailAvailable"]) + }) + + t.Run("disabled activates once and marks email available", func(t *testing.T) { + userInfo := &entity.User{} + calls := map[string]int{} + + err := applyRegistrationVerification(userInfo, true, registrationVerificationActions{ + sendActivationEmail: func() error { + calls["sendActivationEmail"]++ + return nil + }, + activateUser: func() error { + calls["activateUser"]++ + return nil + }, + markEmailAvailable: func() error { + calls["markEmailAvailable"]++ + return nil + }, + }) + + require.NoError(t, err) + assert.Equal(t, entity.EmailStatusAvailable, userInfo.MailStatus) + assert.Zero(t, calls["sendActivationEmail"]) + assert.Equal(t, 1, calls["activateUser"]) + assert.Equal(t, 1, calls["markEmailAvailable"]) + }) + + t.Run("disabled user activation failure falls back to email verification", func(t *testing.T) { + userInfo := &entity.User{} + calls := map[string]int{} + + err := applyRegistrationVerification(userInfo, true, registrationVerificationActions{ + sendActivationEmail: func() error { + calls["sendActivationEmail"]++ + return nil + }, + activateUser: func() error { + calls["activateUser"]++ + return errors.New("activate failed") + }, + markEmailAvailable: func() error { + calls["markEmailAvailable"]++ + return nil + }, + }) + + require.NoError(t, err) + assert.Equal(t, entity.EmailStatusToBeVerified, userInfo.MailStatus) + assert.Equal(t, 1, calls["sendActivationEmail"]) + assert.Equal(t, 1, calls["activateUser"]) + assert.Zero(t, calls["markEmailAvailable"]) + }) + + t.Run("disabled email status failure falls back to email verification", func(t *testing.T) { + userInfo := &entity.User{} + calls := map[string]int{} + + err := applyRegistrationVerification(userInfo, true, registrationVerificationActions{ + sendActivationEmail: func() error { + calls["sendActivationEmail"]++ + return nil + }, + activateUser: func() error { + calls["activateUser"]++ + return nil + }, + markEmailAvailable: func() error { + calls["markEmailAvailable"]++ + return errors.New("update failed") + }, + }) + + require.NoError(t, err) + assert.Equal(t, entity.EmailStatusToBeVerified, userInfo.MailStatus) + assert.Equal(t, 1, calls["sendActivationEmail"]) + assert.Equal(t, 1, calls["activateUser"]) + assert.Equal(t, 1, calls["markEmailAvailable"]) + }) + + t.Run("fallback email failure returns before active status", func(t *testing.T) { + userInfo := &entity.User{} + expectedErr := errors.New("email failed") + + err := applyRegistrationVerification(userInfo, true, registrationVerificationActions{ + sendActivationEmail: func() error { + return expectedErr + }, + activateUser: func() error { + return errors.New("activate failed") + }, + markEmailAvailable: func() error { + return nil + }, + }) + + require.ErrorIs(t, err, expectedErr) + assert.Equal(t, entity.EmailStatusToBeVerified, userInfo.MailStatus) + }) +} diff --git a/internal/service/siteinfo/siteinfo_service.go b/internal/service/siteinfo/siteinfo_service.go index 1e25cbaa4..7656a1407 100644 --- a/internal/service/siteinfo/siteinfo_service.go +++ b/internal/service/siteinfo/siteinfo_service.go @@ -291,7 +291,27 @@ 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) { - content, _ := json.Marshal(req) + 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 + } + + loginConfig := &schema.SiteLoginResp{ + AllowNewRegistrations: req.AllowNewRegistrations, + AllowEmailRegistrations: req.AllowEmailRegistrations, + AllowPasswordLogin: req.AllowPasswordLogin, + AllowEmailDomains: req.AllowEmailDomains, + RequireEmailVerification: requireEmailVerification, + } + content, _ := json.Marshal(loginConfig) data := &entity.SiteInfo{ Type: constant.SiteTypeLogin, Content: string(content), diff --git a/internal/service/siteinfo/siteinfo_service_test.go b/internal/service/siteinfo/siteinfo_service_test.go new file mode 100644 index 000000000..fc4bbdf70 --- /dev/null +++ b/internal/service/siteinfo/siteinfo_service_test.go @@ -0,0 +1,104 @@ +/* + * 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 siteinfo + +import ( + "context" + "encoding/json" + "testing" + + "github.com/apache/answer/internal/base/constant" + "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" +) + +func TestSiteInfoService_SaveSiteLoginRequireEmailVerification(t *testing.T) { + tests := []struct { + name string + currentContent string + requestPayload string + expectGet 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}`, + 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}`, + expectedRequire: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctl := gomock.NewController(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 { + savedContent = data.Content + return nil + }) + + req := &schema.SiteLoginReq{} + require.NoError(t, json.Unmarshal([]byte(tt.requestPayload), req)) + + service := &SiteInfoService{ + siteInfoRepo: repo, + siteInfoCommonService: siteinfo_common.NewSiteInfoCommonService(repo), + } + require.NoError(t, service.SaveSiteLogin(context.TODO(), req)) + assert.NotContains(t, savedContent, `"require_email_verification":null`) + + saved := &schema.SiteLoginResp{} + require.NoError(t, json.Unmarshal([]byte(savedContent), saved)) + assert.Equal(t, tt.expectedRequire, saved.RequireEmailVerification) + }) + } +} diff --git a/internal/service/siteinfo_common/siteinfo_service.go b/internal/service/siteinfo_common/siteinfo_service.go index 5e3964c0c..752ae0510 100644 --- a/internal/service/siteinfo_common/siteinfo_service.go +++ b/internal/service/siteinfo_common/siteinfo_service.go @@ -235,7 +235,7 @@ func (s *siteInfoCommonService) GetSiteSecurity(ctx context.Context) (resp *sche // GetSiteLogin get site login config func (s *siteInfoCommonService) GetSiteLogin(ctx context.Context) (resp *schema.SiteLoginResp, err error) { - resp = &schema.SiteLoginResp{} + resp = &schema.SiteLoginResp{RequireEmailVerification: true} if err = s.GetSiteInfoByType(ctx, constant.SiteTypeLogin, resp); err != nil { return nil, err } diff --git a/internal/service/siteinfo_common/siteinfo_service_test.go b/internal/service/siteinfo_common/siteinfo_service_test.go index a87d427f2..430f45fff 100644 --- a/internal/service/siteinfo_common/siteinfo_service_test.go +++ b/internal/service/siteinfo_common/siteinfo_service_test.go @@ -50,3 +50,47 @@ func TestSiteInfoCommonService_GetSiteGeneral(t *testing.T) { require.NoError(t, err) assert.Equal(t, "name", resp.Name) } + +func TestSiteInfoCommonService_GetSiteLoginRequireEmailVerification(t *testing.T) { + tests := []struct { + name string + content string + expected bool + }{ + { + name: "missing key defaults true", + content: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true}`, + expected: true, + }, + { + name: "null defaults true", + content: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"require_email_verification":null}`, + expected: true, + }, + { + name: "explicit false is preserved", + content: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"require_email_verification":false}`, + expected: false, + }, + { + name: "explicit true is preserved", + content: `{"allow_new_registrations":true,"allow_email_registrations":true,"allow_password_login":true,"require_email_verification":true}`, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctl := gomock.NewController(t) + defer ctl.Finish() + repo := mock.NewMockSiteInfoRepo(ctl) + repo.EXPECT().GetByType(gomock.Any(), constant.SiteTypeLogin). + Return(&entity.SiteInfo{Content: tt.content}, true, nil) + + siteInfoCommonService := NewSiteInfoCommonService(repo) + resp, err := siteInfoCommonService.GetSiteLogin(context.TODO()) + require.NoError(t, err) + assert.Equal(t, tt.expected, resp.RequireEmailVerification) + }) + } +} diff --git a/ui/src/common/interface.ts b/ui/src/common/interface.ts index dbcec6c04..8ab714230 100644 --- a/ui/src/common/interface.ts +++ b/ui/src/common/interface.ts @@ -488,6 +488,7 @@ export interface AdminSettingsLogin { allow_email_registrations: boolean; allow_email_domains: string[]; allow_password_login: boolean; + require_email_verification: boolean; } /** diff --git a/ui/src/pages/Admin/Login/index.tsx b/ui/src/pages/Admin/Login/index.tsx index a4a61152f..c596fc1c6 100644 --- a/ui/src/pages/Admin/Login/index.tsx +++ b/ui/src/pages/Admin/Login/index.tsx @@ -47,6 +47,12 @@ const Index: FC = () => { description: t('email_registration.text'), default: true, }, + require_email_verification: { + type: 'boolean', + title: t('email_verification.title'), + description: t('email_verification.text'), + default: true, + }, allow_password_login: { type: 'boolean', title: t('password_login.title'), @@ -73,6 +79,12 @@ const Index: FC = () => { label: t('email_registration.label'), }, }, + require_email_verification: { + 'ui:widget': 'switch', + 'ui:options': { + label: t('email_verification.label'), + }, + }, allow_password_login: { 'ui:widget': 'switch', 'ui:options': { @@ -105,6 +117,7 @@ const Index: FC = () => { allow_email_registrations: formData.allow_email_registrations.value, allow_email_domains: allowedEmailDomains, allow_password_login: formData.allow_password_login.value, + require_email_verification: formData.require_email_verification.value, }; putLoginSetting(reqParams) @@ -139,6 +152,8 @@ const Index: FC = () => { setting.allow_email_domains.join('\n'); } formMeta.allow_password_login.value = setting.allow_password_login; + formMeta.require_email_verification.value = + setting.require_email_verification; setFormData({ ...formMeta }); } }); diff --git a/ui/src/pages/Users/Register/components/SignUpForm/index.tsx b/ui/src/pages/Users/Register/components/SignUpForm/index.tsx index bffa44652..8dc30b965 100644 --- a/ui/src/pages/Users/Register/components/SignUpForm/index.tsx +++ b/ui/src/pages/Users/Register/components/SignUpForm/index.tsx @@ -23,14 +23,17 @@ import { Link } from 'react-router-dom'; import { Trans, useTranslation } from 'react-i18next'; import { useCaptchaPlugin } from '@/utils/pluginKit'; -import type { FormDataType, RegisterReqParams } from '@/common/interface'; +import type { + FormDataType, + RegisterReqParams, + UserInfoRes, +} from '@/common/interface'; import { register } from '@/services'; -import userStore from '@/stores/loggedUserInfo'; import { handleFormError, scrollToElementTop } from '@/utils'; import { useLegalClick } from '@/behaviour/useLegalClick'; interface Props { - callback: () => void; + callback: (user: UserInfoRes) => void; } const Index: React.FC = ({ callback }) => { @@ -53,7 +56,6 @@ const Index: React.FC = ({ callback }) => { }, }); - const updateUser = userStore((state) => state.update); const emailCaptcha = useCaptchaPlugin('email'); const nameRegex = /^[\w.-\s]{2,30}$/; @@ -139,8 +141,7 @@ const Index: React.FC = ({ callback }) => { register(reqParams) .then(async (res) => { await emailCaptcha?.close(); - updateUser(res); - callback(); + callback(res); }) .catch((err) => { if (err.isError) { diff --git a/ui/src/pages/Users/Register/index.tsx b/ui/src/pages/Users/Register/index.tsx index 4d894b6a8..0152983e7 100644 --- a/ui/src/pages/Users/Register/index.tsx +++ b/ui/src/pages/Users/Register/index.tsx @@ -20,12 +20,14 @@ import React, { useState } from 'react'; import { Container, Col } from 'react-bootstrap'; import { Trans, useTranslation } from 'react-i18next'; -import { Link } from 'react-router-dom'; +import { Link, useNavigate } from 'react-router-dom'; import { usePageTags } from '@/hooks'; +import type * as Type from '@/common/interface'; import { Unactivate, WelcomeTitle, PluginRender } from '@/components'; import { guard } from '@/utils'; -import { loginSettingStore } from '@/stores'; +import { loggedUserInfoStore, loginSettingStore } from '@/stores'; +import { setupAppTheme } from '@/utils/localize'; import { PluginType } from '@/utils/pluginKit/interface'; import SignUpForm from './components/SignUpForm'; @@ -33,9 +35,17 @@ import SignUpForm from './components/SignUpForm'; const Index: React.FC = () => { const [showForm, setShowForm] = useState(true); const { t } = useTranslation('translation', { keyPrefix: 'login' }); + const navigate = useNavigate(); const loginSetting = loginSettingStore((state) => state.login); - const onStep = () => { - setShowForm((bol) => !bol); + const updateUser = loggedUserInfoStore((state) => state.update); + const onRegister = (user: Type.UserInfoRes) => { + updateUser(user); + setupAppTheme(); + if (user.mail_status === 2) { + setShowForm(false); + return; + } + guard.handleLoginRedirect(navigate); }; usePageTags({ title: t('sign_up', { keyPrefix: 'page_title' }), @@ -60,7 +70,7 @@ const Index: React.FC = () => { slug_name="third_party_connector" className="mb-5" /> - {showSignupForm ? : null} + {showSignupForm ? : null}
Already have an account? Log in diff --git a/ui/src/services/common.ts b/ui/src/services/common.ts index cac34b4ff..f612bac93 100644 --- a/ui/src/services/common.ts +++ b/ui/src/services/common.ts @@ -129,7 +129,10 @@ export const login = (params: Type.LoginReqParams) => { }; export const register = (params: Type.RegisterReqParams) => { - return request.post('/answer/api/v1/user/register/email', params); + return request.post( + '/answer/api/v1/user/register/email', + params, + ); }; export const logout = () => { diff --git a/ui/src/stores/loginSetting.ts b/ui/src/stores/loginSetting.ts index 7acf765ee..f49c394b8 100644 --- a/ui/src/stores/loginSetting.ts +++ b/ui/src/stores/loginSetting.ts @@ -32,6 +32,7 @@ const loginSetting = create((set) => ({ allow_email_registrations: true, allow_email_domains: [], allow_password_login: true, + require_email_verification: true, }, update: (params) => set(() => { From 10f09902cd1d0c7ab342bfb9adff93e784bfc02d Mon Sep 17 00:00:00 2001 From: Artur Iusupov Date: Tue, 9 Jun 2026 01:39:27 +0400 Subject: [PATCH 2/2] 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 +--- .../service/siteinfo/siteinfo_service_test.go | 55 +++++++------- 13 files changed, 135 insertions(+), 137 deletions(-) delete mode 100644 internal/controller/user_controller_test.go create mode 100644 internal/schema/siteinfo_schema_test.go diff --git a/docs/docs.go b/docs/docs.go index 29e3a9622..4e48c88d0 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 2cc8f1305..a075dfe45 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 2f7e3754e..b3416a10e 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 9552182ed..531d88b45 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 e48dc8636..000000000 --- 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 1cc1895ba..245bb976d 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 3cd27873a..1d0b27ff6 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 000000000..e5413f4a9 --- /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 7d0c55a58..0683a5aff 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 1d7866f35..d0ebe8754 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 0d77b383d..d77a1c448 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 7656a1407..8b32b722e 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 fc4bbdf70..a3f834a53 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)) +}