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.
This commit is contained in:
committed by
LinkinStars
parent
7123326515
commit
3a5a15fe77
@@ -12081,6 +12081,9 @@ const docTemplate = `{
|
||||
},
|
||||
"schema.SiteLoginReq": {
|
||||
"type": "object",
|
||||
"required": [
|
||||
"require_email_verification"
|
||||
],
|
||||
"properties": {
|
||||
"allow_email_domains": {
|
||||
"type": "array",
|
||||
|
||||
@@ -12054,6 +12054,9 @@
|
||||
},
|
||||
"schema.SiteLoginReq": {
|
||||
"type": "object",
|
||||
"required": [
|
||||
"require_email_verification"
|
||||
],
|
||||
"properties": {
|
||||
"allow_email_domains": {
|
||||
"type": "array",
|
||||
|
||||
@@ -2518,6 +2518,8 @@ definitions:
|
||||
type: boolean
|
||||
require_email_verification:
|
||||
type: boolean
|
||||
required:
|
||||
- require_email_verification
|
||||
type: object
|
||||
schema.SiteLoginResp:
|
||||
properties:
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
@@ -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 {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -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) {
|
||||
|
||||
@@ -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()
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
},
|
||||
|
||||
@@ -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{
|
||||
|
||||
@@ -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))
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user