From b99a0c3ca2e5486340ef3debe3cee2fae420f73a Mon Sep 17 00:00:00 2001 From: Yaodong Yu <2814461814@qq.com> Date: Thu, 6 Apr 2023 23:06:18 +0800 Subject: [PATCH] feat: optimize the "forget password" page (#1709) --- authz/authz.go | 1 + controllers/auth.go | 30 +++------------ controllers/user.go | 42 +++++++++++++-------- controllers/verification.go | 67 +++++++++++++++++++++++++++++++--- ldap/util.go | 2 +- object/check.go | 4 +- object/verification.go | 14 +++++++ routers/router.go | 1 + util/string.go | 9 +++++ web/src/auth/ForgetPage.js | 19 ++++------ web/src/backend/UserBackend.js | 17 ++++++++- 11 files changed, 145 insertions(+), 61 deletions(-) diff --git a/authz/authz.go b/authz/authz.go index 7c9db0fe..e6af5ce7 100644 --- a/authz/authz.go +++ b/authz/authz.go @@ -108,6 +108,7 @@ p, *, *, POST, /api/set-password, *, * p, *, *, POST, /api/send-verification-code, *, * p, *, *, GET, /api/get-captcha, *, * p, *, *, POST, /api/verify-captcha, *, * +p, *, *, POST, /api/verify-code, *, * p, *, *, POST, /api/reset-email-or-phone, *, * p, *, *, POST, /api/upload-resource, *, * p, *, *, GET, /.well-known/openid-configuration, *, * diff --git a/controllers/auth.go b/controllers/auth.go index 92e03bfb..5569caae 100644 --- a/controllers/auth.go +++ b/controllers/auth.go @@ -246,33 +246,14 @@ func (c *ApiController) Login() { var msg string if form.Password == "" { - var verificationCodeType string - var checkResult string - - if form.Name != "" { - user = object.GetUserByFields(form.Organization, form.Name) - } - - // check result through Email or Phone - var checkDest string - if strings.Contains(form.Username, "@") { - verificationCodeType = "email" - if user != nil && util.GetMaskedEmail(user.Email) == form.Username { - form.Username = user.Email - } - checkDest = form.Username - } else { - verificationCodeType = "phone" - if user != nil && util.GetMaskedPhone(user.Phone) == form.Username { - form.Username = user.Phone - } - } - if user = object.GetUserByFields(form.Organization, form.Username); user == nil { c.ResponseError(fmt.Sprintf(c.T("general:The user: %s doesn't exist"), util.GetId(form.Organization, form.Username))) return } - if verificationCodeType == "phone" { + + verificationCodeType := object.GetVerifyType(form.Username) + var checkDest string + if verificationCodeType == object.VerifyTypePhone { form.CountryCode = user.GetCountryCode(form.CountryCode) var ok bool if checkDest, ok = util.GetE164Number(form.Username, form.CountryCode); !ok { @@ -281,7 +262,8 @@ func (c *ApiController) Login() { } } - checkResult = object.CheckSigninCode(user, checkDest, form.Code, c.GetAcceptLanguage()) + // check result through Email or Phone + checkResult := object.CheckSigninCode(user, checkDest, form.Code, c.GetAcceptLanguage()) if len(checkResult) != 0 { c.ResponseError(fmt.Sprintf("%s - %s", verificationCodeType, checkResult)) return diff --git a/controllers/user.go b/controllers/user.go index 08608c74..3a5a6bbf 100644 --- a/controllers/user.go +++ b/controllers/user.go @@ -95,13 +95,13 @@ func (c *ApiController) GetUser() { owner := c.Input().Get("owner") if owner == "" { - owner, _ = util.GetOwnerAndNameFromId(id) + owner = util.GetOwnerFromId(id) } organization := object.GetOrganization(fmt.Sprintf("%s/%s", "admin", owner)) if !organization.IsProfilePublic { requestUserId := c.GetSessionUsername() - hasPermission, err := object.CheckUserPermission(requestUserId, id, owner, false, c.GetAcceptLanguage()) + hasPermission, err := object.CheckUserPermission(requestUserId, id, false, c.GetAcceptLanguage()) if !hasPermission { c.ResponseError(err.Error()) return @@ -276,14 +276,34 @@ func (c *ApiController) SetPassword() { userName := c.Ctx.Request.Form.Get("userName") oldPassword := c.Ctx.Request.Form.Get("oldPassword") newPassword := c.Ctx.Request.Form.Get("newPassword") + code := c.Ctx.Request.Form.Get("code") + + if strings.Contains(newPassword, " ") { + c.ResponseError(c.T("user:New password cannot contain blank space.")) + return + } + if len(newPassword) <= 5 { + c.ResponseError(c.T("user:New password must have at least 6 characters")) + return + } - requestUserId := c.GetSessionUsername() userId := util.GetId(userOwner, userName) - hasPermission, err := object.CheckUserPermission(requestUserId, userId, userOwner, true, c.GetAcceptLanguage()) - if !hasPermission { - c.ResponseError(err.Error()) + requestUserId := c.GetSessionUsername() + if requestUserId == "" && code == "" { return + } else if code == "" { + hasPermission, err := object.CheckUserPermission(requestUserId, userId, true, c.GetAcceptLanguage()) + if !hasPermission { + c.ResponseError(err.Error()) + return + } + } else { + if code != c.GetSession("verifiedCode") { + c.ResponseError("") + return + } + c.SetSession("verifiedCode", "") } targetUser := object.GetUser(userId) @@ -296,16 +316,6 @@ func (c *ApiController) SetPassword() { } } - if strings.Contains(newPassword, " ") { - c.ResponseError(c.T("user:New password cannot contain blank space.")) - return - } - - if len(newPassword) <= 5 { - c.ResponseError(c.T("user:New password must have at least 6 characters")) - return - } - targetUser.Password = newPassword object.SetUserField(targetUser, "password", targetUser.Password) c.ResponseOk() diff --git a/controllers/verification.go b/controllers/verification.go index f54767fe..e04b6332 100644 --- a/controllers/verification.go +++ b/controllers/verification.go @@ -15,6 +15,7 @@ package controllers import ( + "encoding/json" "errors" "fmt" "strings" @@ -110,7 +111,7 @@ func (c *ApiController) SendVerificationCode() { sendResp := errors.New("invalid dest type") switch destType { - case "email": + case object.VerifyTypeEmail: if !util.IsEmailValid(dest) { c.ResponseError(c.T("check:Email is invalid")) return @@ -132,7 +133,7 @@ func (c *ApiController) SendVerificationCode() { provider := application.GetEmailProvider() sendResp = object.SendVerificationCodeToEmail(organization, user, provider, remoteAddr, dest) - case "phone": + case object.VerifyTypePhone: if method == LoginVerification || method == ForgetVerification { if user != nil && util.GetMaskedPhone(user.Phone) == dest { dest = user.Phone @@ -187,7 +188,7 @@ func (c *ApiController) ResetEmailOrPhone() { checkDest := dest organization := object.GetOrganizationByUser(user) - if destType == "phone" { + if destType == object.VerifyTypePhone { if object.HasUserByField(user.Owner, "phone", dest) { c.ResponseError(c.T("check:Phone already exists")) return @@ -207,7 +208,7 @@ func (c *ApiController) ResetEmailOrPhone() { c.ResponseError(fmt.Sprintf(c.T("verification:Phone number is invalid in your region %s"), user.CountryCode)) return } - } else if destType == "email" { + } else if destType == object.VerifyTypeEmail { if object.HasUserByField(user.Owner, "email", dest) { c.ResponseError(c.T("check:Email already exists")) return @@ -230,10 +231,10 @@ func (c *ApiController) ResetEmailOrPhone() { } switch destType { - case "email": + case object.VerifyTypeEmail: user.Email = dest object.SetUserField(user, "email", user.Email) - case "phone": + case object.VerifyTypePhone: user.Phone = dest object.SetUserField(user, "phone", user.Phone) default: @@ -245,6 +246,60 @@ func (c *ApiController) ResetEmailOrPhone() { c.ResponseOk() } +// VerifyCode +// @Tag Account API +// @Title VerifyCode +// @router /api/verify-code [post] +func (c *ApiController) VerifyCode() { + var form RequestForm + err := json.Unmarshal(c.Ctx.Input.RequestBody, &form) + if err != nil { + c.ResponseError(err.Error()) + return + } + + var user *object.User + if form.Name != "" { + user = object.GetUserByFields(form.Organization, form.Name) + } + + var checkDest string + if strings.Contains(form.Username, "@") { + if user != nil && util.GetMaskedEmail(user.Email) == form.Username { + form.Username = user.Email + } + checkDest = form.Username + } else { + if user != nil && util.GetMaskedPhone(user.Phone) == form.Username { + form.Username = user.Phone + } + } + + if user = object.GetUserByFields(form.Organization, form.Username); user == nil { + c.ResponseError(fmt.Sprintf(c.T("general:The user: %s doesn't exist"), util.GetId(form.Organization, form.Username))) + return + } + + verificationCodeType := object.GetVerifyType(form.Username) + if verificationCodeType == object.VerifyTypePhone { + form.CountryCode = user.GetCountryCode(form.CountryCode) + var ok bool + if checkDest, ok = util.GetE164Number(form.Username, form.CountryCode); !ok { + c.ResponseError(fmt.Sprintf(c.T("verification:Phone number is invalid in your region %s"), form.CountryCode)) + return + } + } + + if result := object.CheckVerificationCode(checkDest, form.Code, c.GetAcceptLanguage()); result.Code != object.VerificationSuccess { + c.ResponseError(result.Msg) + return + } + object.DisableVerificationCode(checkDest) + c.SetSession("verifiedCode", form.Code) + + c.ResponseOk() +} + // VerifyCaptcha ... // @Title VerifyCaptcha // @Tag Verification API diff --git a/ldap/util.go b/ldap/util.go index 640bc929..1674f425 100644 --- a/ldap/util.go +++ b/ldap/util.go @@ -85,7 +85,7 @@ func GetFilteredUsers(m *ldap.Message) (filteredUsers []*object.User, code int) return nil, ldap.LDAPResultInsufficientAccessRights } } else { - hasPermission, err := object.CheckUserPermission(fmt.Sprintf("%s/%s", m.Client.OrgName, m.Client.UserName), fmt.Sprintf("%s/%s", org, name), org, true, "en") + hasPermission, err := object.CheckUserPermission(fmt.Sprintf("%s/%s", m.Client.OrgName, m.Client.UserName), fmt.Sprintf("%s/%s", org, name), true, "en") if !hasPermission { log.Printf("ErrMsg = %v", err.Error()) return nil, ldap.LDAPResultInsufficientAccessRights diff --git a/object/check.go b/object/check.go index 7be4e628..7416764b 100644 --- a/object/check.go +++ b/object/check.go @@ -250,11 +250,13 @@ func filterField(field string) bool { return reFieldWhiteList.MatchString(field) } -func CheckUserPermission(requestUserId, userId, userOwner string, strict bool, lang string) (bool, error) { +func CheckUserPermission(requestUserId, userId string, strict bool, lang string) (bool, error) { if requestUserId == "" { return false, fmt.Errorf(i18n.Translate(lang, "general:Please login first")) } + userOwner := util.GetOwnerFromId(userId) + if userId != "" { targetUser := GetUser(userId) if targetUser == nil { diff --git a/object/verification.go b/object/verification.go index b037bc20..6e63f8a0 100644 --- a/object/verification.go +++ b/object/verification.go @@ -18,6 +18,7 @@ import ( "errors" "fmt" "math/rand" + "strings" "time" "github.com/casdoor/casdoor/conf" @@ -38,6 +39,11 @@ const ( timeoutError = 3 ) +const ( + VerifyTypePhone = "phone" + VerifyTypeEmail = "email" +) + type VerificationRecord struct { Owner string `xorm:"varchar(100) notnull pk" json:"owner"` Name string `xorm:"varchar(100) notnull pk" json:"name"` @@ -213,6 +219,14 @@ func CheckSigninCode(user *User, dest, code, lang string) string { } } +func GetVerifyType(username string) (verificationCodeType string) { + if strings.Contains(username, "@") { + return VerifyTypeEmail + } else { + return VerifyTypeEmail + } +} + // From Casnode/object/validateCode.go line 116 var stdNums = []byte("0123456789") diff --git a/routers/router.go b/routers/router.go index ae5bcee8..2cf47866 100644 --- a/routers/router.go +++ b/routers/router.go @@ -115,6 +115,7 @@ func initAPI() { beego.Router("/api/check-user-password", &controllers.ApiController{}, "POST:CheckUserPassword") beego.Router("/api/get-email-and-phone", &controllers.ApiController{}, "GET:GetEmailAndPhone") beego.Router("/api/send-verification-code", &controllers.ApiController{}, "POST:SendVerificationCode") + beego.Router("/api/verify-code", &controllers.ApiController{}, "POST:VerifyCode") beego.Router("/api/verify-captcha", &controllers.ApiController{}, "POST:VerifyCaptcha") beego.Router("/api/reset-email-or-phone", &controllers.ApiController{}, "POST:ResetEmailOrPhone") beego.Router("/api/get-captcha", &controllers.ApiController{}, "GET:GetCaptcha") diff --git a/util/string.go b/util/string.go index a38a80ed..22e4541f 100644 --- a/util/string.go +++ b/util/string.go @@ -95,6 +95,15 @@ func GetOwnerAndNameFromId(id string) (string, string) { return tokens[0], tokens[1] } +func GetOwnerFromId(id string) string { + tokens := strings.Split(id, "/") + if len(tokens) != 2 { + panic(errors.New("GetOwnerAndNameFromId() error, wrong token count for ID: " + id)) + } + + return tokens[0] +} + func GetOwnerAndNameFromIdNoCheck(id string) (string, string) { tokens := strings.SplitN(id, "/", 2) return tokens[0], tokens[1] diff --git a/web/src/auth/ForgetPage.js b/web/src/auth/ForgetPage.js index dedc27bc..5f18aab3 100644 --- a/web/src/auth/ForgetPage.js +++ b/web/src/auth/ForgetPage.js @@ -33,9 +33,8 @@ class ForgetPage extends React.Component { classes: props, applicationName: props.applicationName ?? props.match.params?.applicationName, msg: null, - userId: "", - username: "", name: "", + username: "", phone: "", email: "", dest: "", @@ -86,7 +85,7 @@ class ForgetPage extends React.Component { const phone = res.data.phone; const email = res.data.email; - if (phone === "" && email === "") { + if (!phone && !email) { Setting.showMessage("error", "no verification method!"); } else { this.setState({ @@ -124,18 +123,16 @@ class ForgetPage extends React.Component { }); break; case "step2": - const oAuthParams = Util.getOAuthGetParameters(); - - AuthBackend.login({ + UserBackend.verifyCode({ application: forms.step2.getFieldValue("application"), organization: forms.step2.getFieldValue("organization"), username: forms.step2.getFieldValue("dest"), name: this.state.name, code: forms.step2.getFieldValue("code"), type: "login", - }, oAuthParams).then(res => { + }).then(res => { if (res.status === "ok") { - this.setState({current: 2, userId: res.data}); + this.setState({current: 2, code: forms.step2.getFieldValue("code")}); } else { Setting.showMessage("error", res.msg); } @@ -150,7 +147,7 @@ class ForgetPage extends React.Component { onFinish(values) { values.username = this.state.name; values.userOwner = this.getApplicationObj()?.organizationObj.name; - UserBackend.setPassword(values.userOwner, values.username, "", values?.newPassword).then(res => { + UserBackend.setPassword(values.userOwner, values.username, "", values?.newPassword, this.state.code).then(res => { if (res.status === "ok") { Setting.redirectToLoginPage(this.getApplicationObj(), this.props.history); } else { @@ -387,7 +384,6 @@ class ForgetPage extends React.Component { hasFeedback > } placeholder={i18next.t("general:Password")} /> @@ -414,14 +410,13 @@ class ForgetPage extends React.Component { ]} > } placeholder={i18next.t("signup:Confirm")} />
diff --git a/web/src/backend/UserBackend.js b/web/src/backend/UserBackend.js index 50d306ee..53cef1c6 100644 --- a/web/src/backend/UserBackend.js +++ b/web/src/backend/UserBackend.js @@ -93,12 +93,16 @@ export function getAffiliationOptions(url, code) { }).then(res => res.json()); } -export function setPassword(userOwner, userName, oldPassword, newPassword) { +export function setPassword(userOwner, userName, oldPassword, newPassword, code = "") { const formData = new FormData(); formData.append("userOwner", userOwner); formData.append("userName", userName); formData.append("oldPassword", oldPassword); formData.append("newPassword", newPassword); + if (code) { + formData.append("code", code); + } + return fetch(`${Setting.ServerUrl}/api/set-password`, { method: "POST", credentials: "include", @@ -188,3 +192,14 @@ export function getCaptcha(owner, name, isCurrentProvider) { }, }).then(res => res.json()).then(res => res.data); } + +export function verifyCode(values) { + return fetch(`${Setting.ServerUrl}/api/verify-code`, { + method: "POST", + credentials: "include", + body: JSON.stringify(values), + headers: { + "Accept-Language": Setting.getAcceptLanguage(), + }, + }).then(res => res.json()); +}