From d2be6b220eef1ec1e2fe80b4ab991c683db3bab9 Mon Sep 17 00:00:00 2001 From: Azareal Date: Wed, 30 Oct 2019 08:13:45 +1000 Subject: [PATCH] Support for optional emails. Reduce boilerplate and allocations. Fix the error shown on AccountEditEmailTokenSubmit when there aren't any emails rows. Add register_account_email_optional phrase. Add account_email_none phrase. --- common/email_store.go | 2 +- common/group_store.go | 33 +++++++------- common/page_store.go | 31 ++++++------- common/template_init.go | 2 +- common/utils.go | 35 ++++++++++----- langs/english.json | 2 + routes/account.go | 57 +++++++++++------------- templates/account_own_edit_email.html | 3 +- templates/account_own_edit_password.html | 8 ++-- templates/guilds_view_guild.html | 14 +++--- templates/register.html | 8 ++-- 11 files changed, 103 insertions(+), 92 deletions(-) diff --git a/common/email_store.go b/common/email_store.go index c1bc3f51..db7c132d 100644 --- a/common/email_store.go +++ b/common/email_store.go @@ -20,7 +20,7 @@ type DefaultEmailStore struct { func NewDefaultEmailStore(acc *qgen.Accumulator) (*DefaultEmailStore, error) { return &DefaultEmailStore{ - getEmailsByUser: acc.Select("emails").Columns("email, validated, token").Where("uid = ?").Prepare(), + getEmailsByUser: acc.Select("emails").Columns("email,validated,token").Where("uid=?").Prepare(), // Need to fix this: Empty string isn't working, it gets set to 1 instead x.x -- Has this been fixed? verifyEmail: acc.Update("emails").Set("validated = 1, token = ''").Where("email = ?").Prepare(), diff --git a/common/group_store.go b/common/group_store.go index febff38d..71f5a753 100644 --- a/common/group_store.go +++ b/common/group_store.go @@ -91,7 +91,7 @@ func (s *MemoryGroupStore) LoadGroups() error { s.groupCount = i DebugLog("Binding the Not Loggedin Group") - GuestPerms = s.dirtyGetUnsafe(6).Perms + GuestPerms = s.dirtyGetUnsafe(6).Perms // ! Race? TopicListThaw.Thaw() return nil } @@ -164,30 +164,30 @@ func (s *MemoryGroupStore) Reload(id int) error { return nil } -func (s *MemoryGroupStore) initGroup(group *Group) error { - err := json.Unmarshal(group.PermissionsText, &group.Perms) +func (s *MemoryGroupStore) initGroup(g *Group) error { + err := json.Unmarshal(g.PermissionsText, &g.Perms) if err != nil { - log.Printf("group: %+v\n", group) - log.Print("bad group perms: ", group.PermissionsText) + log.Printf("group: %+v\n", g) + log.Print("bad group perms: ", g.PermissionsText) return err } - DebugLogf(group.Name+": %+v\n", group.Perms) + DebugLogf(g.Name+": %+v\n", g.Perms) - err = json.Unmarshal(group.PluginPermsText, &group.PluginPerms) + err = json.Unmarshal(g.PluginPermsText, &g.PluginPerms) if err != nil { - log.Printf("group: %+v\n", group) - log.Print("bad group plugin perms: ", group.PluginPermsText) + log.Printf("group: %+v\n", g) + log.Print("bad group plugin perms: ", g.PluginPermsText) return err } - DebugLogf(group.Name+": %+v\n", group.PluginPerms) + DebugLogf(g.Name+": %+v\n", g.PluginPerms) //group.Perms.ExtData = make(map[string]bool) // TODO: Can we optimise the bit where this cascades down to the user now? - if group.IsAdmin || group.IsMod { - group.IsBanned = false + if g.IsAdmin || g.IsMod { + g.IsBanned = false } - err = s.userCount.QueryRow(group.ID).Scan(&group.UserCount) + err = s.userCount.QueryRow(g.ID).Scan(&g.UserCount) if err != sql.ErrNoRows { return err } @@ -209,9 +209,9 @@ func (s *MemoryGroupStore) SetCanSee(gid int, canSee []int) error { return nil } -func (s *MemoryGroupStore) CacheSet(group *Group) error { +func (s *MemoryGroupStore) CacheSet(g *Group) error { s.Lock() - s.groups[group.ID] = group + s.groups[g.ID] = g s.Unlock() return nil } @@ -234,7 +234,7 @@ func (s *MemoryGroupStore) Create(name string, tag string, isAdmin bool, isMod b } defer tx.Rollback() - insertTx, err := qgen.Builder.SimpleInsertTx(tx, "users_groups", "name, tag, is_admin, is_mod, is_banned, permissions, plugin_perms", "?,?,?,?,?,?,'{}'") + insertTx, err := qgen.Builder.SimpleInsertTx(tx, "users_groups", "name,tag,is_admin,is_mod,is_banned,permissions,plugin_perms", "?,?,?,?,?,?,'{}'") if err != nil { return 0, err } @@ -242,7 +242,6 @@ func (s *MemoryGroupStore) Create(name string, tag string, isAdmin bool, isMod b if err != nil { return 0, err } - gid64, err := res.LastInsertId() if err != nil { return 0, err diff --git a/common/page_store.go b/common/page_store.go index e9e887e0..74017d76 100644 --- a/common/page_store.go +++ b/common/page_store.go @@ -90,12 +90,13 @@ type DefaultPageStore struct { } func NewDefaultPageStore(acc *qgen.Accumulator) (*DefaultPageStore, error) { + pa := "pages" return &DefaultPageStore{ - get: acc.Select("pages").Columns("name, title, body, allowedGroups, menuID").Where("pid = ?").Prepare(), - getByName: acc.Select("pages").Columns("pid, name, title, body, allowedGroups, menuID").Where("name = ?").Prepare(), - getOffset: acc.Select("pages").Columns("pid, name, title, body, allowedGroups, menuID").Orderby("pid DESC").Limit("?,?").Prepare(), - count: acc.Count("pages").Prepare(), - delete: acc.Delete("pages").Where("pid = ?").Prepare(), + get: acc.Select(pa).Columns("name, title, body, allowedGroups, menuID").Where("pid = ?").Prepare(), + getByName: acc.Select(pa).Columns("pid, name, title, body, allowedGroups, menuID").Where("name = ?").Prepare(), + getOffset: acc.Select(pa).Columns("pid, name, title, body, allowedGroups, menuID").Orderby("pid DESC").Limit("?,?").Prepare(), + count: acc.Count(pa).Prepare(), + delete: acc.Delete(pa).Where("pid = ?").Prepare(), }, acc.FirstError() } @@ -122,23 +123,23 @@ func (s *DefaultPageStore) parseAllowedGroups(raw string, page *CustomPage) erro } func (s *DefaultPageStore) Get(id int) (*CustomPage, error) { - page := &CustomPage{ID: id} + p := &CustomPage{ID: id} rawAllowedGroups := "" - err := s.get.QueryRow(id).Scan(&page.Name, &page.Title, &page.Body, &rawAllowedGroups, &page.MenuID) + err := s.get.QueryRow(id).Scan(&p.Name, &p.Title, &p.Body, &rawAllowedGroups, &p.MenuID) if err != nil { return nil, err } - return page, s.parseAllowedGroups(rawAllowedGroups, page) + return p, s.parseAllowedGroups(rawAllowedGroups, p) } func (s *DefaultPageStore) GetByName(name string) (*CustomPage, error) { - page := BlankCustomPage() + p := BlankCustomPage() rawAllowedGroups := "" - err := s.getByName.QueryRow(name).Scan(&page.ID, &page.Name, &page.Title, &page.Body, &rawAllowedGroups, &page.MenuID) + err := s.getByName.QueryRow(name).Scan(&p.ID, &p.Name, &p.Title, &p.Body, &rawAllowedGroups, &p.MenuID) if err != nil { return nil, err } - return page, s.parseAllowedGroups(rawAllowedGroups, page) + return p, s.parseAllowedGroups(rawAllowedGroups, p) } func (s *DefaultPageStore) GetOffset(offset int, perPage int) (pages []*CustomPage, err error) { @@ -149,17 +150,17 @@ func (s *DefaultPageStore) GetOffset(offset int, perPage int) (pages []*CustomPa defer rows.Close() for rows.Next() { - page := &CustomPage{ID: 0} + p := &CustomPage{ID: 0} rawAllowedGroups := "" - err := rows.Scan(&page.ID, &page.Name, &page.Title, &page.Body, &rawAllowedGroups, &page.MenuID) + err := rows.Scan(&p.ID, &p.Name, &p.Title, &p.Body, &rawAllowedGroups, &p.MenuID) if err != nil { return pages, err } - err = s.parseAllowedGroups(rawAllowedGroups, page) + err = s.parseAllowedGroups(rawAllowedGroups, p) if err != nil { return pages, err } - pages = append(pages, page) + pages = append(pages, p) } return pages, rows.Err() } diff --git a/common/template_init.go b/common/template_init.go index 0c9294f9..bc782c8a 100644 --- a/common/template_init.go +++ b/common/template_init.go @@ -339,7 +339,7 @@ func compileTemplates(wg *sync.WaitGroup, c *tmpl.CTemplateSet, themeName string } t.AddStd("login", "c.Page", Page{htitle("Login Page"), tList, nil}) - t.AddStd("register", "c.Page", Page{htitle("Registration Page"), tList, "nananana"}) + t.AddStd("register", "c.Page", Page{htitle("Registration Page"), tList, false}) t.AddStd("error", "c.ErrorPage", ErrorPage{htitle("Error"), "A problem has occurred in the system."}) ipSearchPage := IPSearchPage{htitle("IP Search"), map[int]*User{1: &user2}, "::1"} diff --git a/common/utils.go b/common/utils.go index 9a41af6a..63ff83f9 100644 --- a/common/utils.go +++ b/common/utils.go @@ -319,9 +319,7 @@ func HasSuspiciousEmail(email string) bool { return true } - var dotCount int - var shortBits int - var currentSegmentLength int + var dotCount, shortBits, currentSegmentLength int for _, char := range lowEmail { if char == '.' { dotCount++ @@ -337,25 +335,40 @@ func HasSuspiciousEmail(email string) bool { return dotCount > 7 || shortBits > 2 } +var weakPassStrings = []string{"test", "123","6969","password", "qwerty", "fuck", "love"} + // TODO: Write a test for this func WeakPassword(password string, username string, email string) error { lowPassword := strings.ToLower(password) switch { case password == "": return errors.New("You didn't put in a password.") - case strings.Contains(lowPassword, strings.ToLower(username)) && len(username) > 3: - return errors.New("You can't use your username in your password.") - case strings.Contains(lowPassword, strings.ToLower(email)): - return errors.New("You can't use your email in your password.") case len(password) < 8: return errors.New("Your password needs to be at-least eight characters long") + case strings.Contains(lowPassword, strings.ToLower(username)) && len(username) > 3: + return errors.New("You can't use your username in your password.") + case email != "" && strings.Contains(lowPassword, strings.ToLower(email)): + return errors.New("You can't use your email in your password.") } - if strings.Contains(lowPassword, "test") || strings.Contains(password, "123") || strings.Contains(lowPassword, "password") || strings.Contains(lowPassword, "qwerty") || strings.Contains(lowPassword, "fuck") || strings.Contains(lowPassword, "love") { - return errors.New("You may not have 'test', '123', 'password', 'qwerty', 'love' or 'fuck' in your password") + for _, passBit := range weakPassStrings { + if strings.Contains(lowPassword, passBit) { + s := "You may not have " + for i, passBit := range weakPassStrings { + if i > 0 { + if i == len(weakPassStrings)-1 { + s += " or " + } else { + s += ", " + } + } + s += "'" + passBit + "'" + } + return errors.New(s + " in your password") + } } - var charMap = make(map[rune]int) + charMap := make(map[rune]int) var numbers, symbols, upper, lower int for _, char := range password { charItem, ok := charMap[char] @@ -535,4 +548,4 @@ func BuildSlug(slug string, id int) string { return strconv.Itoa(id) } return slug + "." + strconv.Itoa(id) -} \ No newline at end of file +} diff --git a/langs/english.json b/langs/english.json index 939a9f0e..eeea400a 100644 --- a/langs/english.json +++ b/langs/english.json @@ -474,6 +474,7 @@ "register_head":"Create Account", "register_account_name":"Account Name", "register_account_email":"Email", + "register_account_email_optional":"Email (optional)", "register_account_password":"Password", "register_account_confirm_password":"Confirm Password", "register_account_anti_spam":"Ar​e y​ou a sp​am​bo​t?", @@ -515,6 +516,7 @@ "account_email_secondary":"Secondary", "account_email_verified":"Verified", "account_email_resend_email":"Resend Verification Email", + "account_email_none":"No email addresses found.", "account_password_head":"Edit Password", "account_password_current_password":"Current Password", diff --git a/routes/account.go b/routes/account.go index 981c98fe..e8af7562 100644 --- a/routes/account.go +++ b/routes/account.go @@ -166,7 +166,7 @@ func AccountLoginMFAVerifySubmit(w http.ResponseWriter, r *http.Request, user c. if !mfaVerifySession(provSession, signedSession, uid) { return c.LocalError("Invalid session", w, r, user) } - var token = r.PostFormValue("mfa_token") + token := r.PostFormValue("mfa_token") err = c.Auth.ValidateMFAToken(token, uid) if err != nil { @@ -188,7 +188,7 @@ func AccountRegister(w http.ResponseWriter, r *http.Request, user c.User, h *c.H } h.Title = p.GetTitlePhrase("register") h.AddScriptAsync("register.js") - return renderTemplate("register", w, r, h, c.Page{h, tList, nil}) + return renderTemplate("register", w, r, h, c.Page{h, tList, h.Settings["activation_type"] != 2}) } func isNumeric(data string) (numeric bool) { @@ -227,19 +227,19 @@ func AccountRegisterSubmit(w http.ResponseWriter, r *http.Request, user c.User) } } - username := c.SanitiseSingleLine(r.PostFormValue("username")) - // TODO: Add a dedicated function for validating emails - email := c.SanitiseSingleLine(r.PostFormValue("email")) - if username == "" { + name := c.SanitiseSingleLine(r.PostFormValue("name")) + if name == "" { regError(p.GetErrorPhrase("register_need_username"), "no-username") } - if email == "" { + // TODO: Add a dedicated function for validating emails + email := c.SanitiseSingleLine(r.PostFormValue("email")) + if headerLite.Settings["activation_type"] == 2 && email == "" { regError(p.GetErrorPhrase("register_need_email"), "no-email") } // This is so a numeric name won't interfere with mentioning a user by ID, there might be a better way of doing this like perhaps !@ to mean IDs and @ to mean usernames in the pre-parser - usernameBits := strings.Split(username, " ") - if isNumeric(usernameBits[0]) { + nameBits := strings.Split(name, " ") + if isNumeric(nameBits[0]) { regError(p.GetErrorPhrase("register_first_word_numeric"), "numeric-name") } @@ -249,7 +249,7 @@ func AccountRegisterSubmit(w http.ResponseWriter, r *http.Request, user c.User) password := r.PostFormValue("password") // ? Move this into Create()? What if we want to programatically set weak passwords for tests? - err := c.WeakPassword(password, username, email) + err := c.WeakPassword(password, name, email) if err != nil { regError(err.Error(), "weak-password") } else { @@ -260,7 +260,7 @@ func AccountRegisterSubmit(w http.ResponseWriter, r *http.Request, user c.User) } } - regLog := c.RegLogItem{Username: username, Email: email, FailureReason: regErrReason, Success: regSuccess, IP: user.LastIP} + regLog := c.RegLogItem{Username: name, Email: email, FailureReason: regErrReason, Success: regSuccess, IP: user.LastIP} _, err = regLog.Create() if err != nil { return c.InternalError(err, w, r) @@ -280,7 +280,7 @@ func AccountRegisterSubmit(w http.ResponseWriter, r *http.Request, user c.User) } // TODO: Do the registration attempt logging a little less messily (without having to amend the result after the insert) - uid, err := c.Users.Create(username, password, email, group, active) + uid, err := c.Users.Create(name, password, email, group, active) if err != nil { regLog.Success = false if err == c.ErrAccountExists { @@ -320,12 +320,12 @@ func AccountRegisterSubmit(w http.ResponseWriter, r *http.Request, user c.User) } // TODO: Add an EmailStore and move this there - _, err = qgen.NewAcc().Insert("emails").Columns("email, uid, validated, token").Fields("?,?,?,?").Exec(email, uid, 0, token) + _, err = qgen.NewAcc().Insert("emails").Columns("email,uid,validated,token").Fields("?,?,?,?").Exec(email, uid, 0, token) if err != nil { return c.InternalError(err, w, r) } - err = c.SendValidationEmail(username, email, token) + err = c.SendValidationEmail(name, email, token) if err != nil { return c.LocalError(p.GetErrorPhrase("register_email_fail"), w, r, user) } @@ -345,7 +345,6 @@ func accountEditHead(titlePhrase string, w http.ResponseWriter, r *http.Request, func AccountEdit(w http.ResponseWriter, r *http.Request, user c.User, header *c.Header) c.RouteError { accountEditHead("account", w, r, &user, header) - if r.FormValue("avatar_updated") == "1" { header.AddNotice("account_avatar_updated") } else if r.FormValue("username_updated") == "1" { @@ -387,12 +386,12 @@ func AccountEditPasswordSubmit(w http.ResponseWriter, r *http.Request, user c.Us } var realPassword, salt string - currentPassword := r.PostFormValue("account-current-password") - newPassword := r.PostFormValue("account-new-password") - confirmPassword := r.PostFormValue("account-confirm-password") + currentPassword := r.PostFormValue("current-password") + newPassword := r.PostFormValue("new-password") + confirmPassword := r.PostFormValue("confirm-password") // TODO: Use a reusable statement - err := qgen.NewAcc().Select("users").Columns("password, salt").Where("uid = ?").QueryRow(user.ID).Scan(&realPassword, &salt) + err := qgen.NewAcc().Select("users").Columns("password,salt").Where("uid=?").QueryRow(user.ID).Scan(&realPassword, &salt) if err == sql.ErrNoRows { return c.LocalError("Your account no longer exists.", w, r, user) } else if err != nil { @@ -429,7 +428,6 @@ func AccountEditAvatarSubmit(w http.ResponseWriter, r *http.Request, user c.User if ferr != nil { return ferr } - ferr = c.ChangeAvatar("." + ext, w, r, user) if ferr != nil { return ferr @@ -584,7 +582,7 @@ func AccountEditEmail(w http.ResponseWriter, r *http.Request, user c.User, h *c. // Was this site migrated from another forum software? Most of them don't have multiple emails for a single user. // This also applies when the admin switches site.EnableEmails on after having it off for a while. - if len(emails) == 0 { + if len(emails) == 0 && user.Email != "" { emails = append(emails, c.Email{UserID: user.ID, Email: user.Email, Validated: false, Primary: true}) } @@ -612,7 +610,10 @@ func AccountEditEmailTokenSubmit(w http.ResponseWriter, r *http.Request, user c. targetEmail := c.Email{UserID: user.ID} emails, err := c.Emails.GetEmailsByUser(&user) - if err != nil { + if err == sql.ErrNoRows { + return c.LocalError("A verification email was never sent for you!", w, r, user) + } else if err != nil { + // TODO: Better error if we don't have an email or it's not in the emails table for some reason return c.LocalError("You are not logged in", w, r, user) } for _, email := range emails { @@ -635,8 +636,7 @@ func AccountEditEmailTokenSubmit(w http.ResponseWriter, r *http.Request, user c. // If Email Activation is on, then activate the account while we're here if header.Settings["activation_type"] == 2 { - err = user.Activate() - if err != nil { + if err = user.Activate(); err != nil { return c.InternalError(err, w, r) } } @@ -646,10 +646,9 @@ func AccountEditEmailTokenSubmit(w http.ResponseWriter, r *http.Request, user c. func AccountLogins(w http.ResponseWriter, r *http.Request, user c.User, h *c.Header) c.RouteError { accountEditHead("account_logins", w, r, &user, h) - logCount := c.LoginLogs.CountUser(user.ID) page, _ := strconv.Atoi(r.FormValue("page")) perPage := 12 - offset, page, lastPage := c.PageOffset(logCount, page, perPage) + offset, page, lastPage := c.PageOffset(c.LoginLogs.CountUser(user.ID), page, perPage) logs, err := c.LoginLogs.GetOffset(user.ID, offset, perPage) if err != nil { @@ -779,12 +778,11 @@ func AccountPasswordResetToken(w http.ResponseWriter, r *http.Request, user c.Us header.AddNotice("password_reset_token_token_verified") }*/ - token := r.FormValue("token") uid, err := strconv.Atoi(r.FormValue("uid")) if err != nil { return c.LocalError("Invalid uid", w, r, user) } - + token := r.FormValue("token") err = c.PasswordResetter.ValidateToken(uid, token) if err == sql.ErrNoRows || err == c.ErrBadResetToken { return c.LocalError("This reset token has expired.", w, r, user) @@ -814,8 +812,7 @@ func AccountPasswordResetTokenSubmit(w http.ResponseWriter, r *http.Request, use return c.LocalError("This reset token has expired.", w, r, user) } - token := r.FormValue("token") - err = c.PasswordResetter.ValidateToken(uid, token) + err = c.PasswordResetter.ValidateToken(uid, r.FormValue("token")) if err == sql.ErrNoRows || err == c.ErrBadResetToken { return c.LocalError("This reset token has expired.", w, r, user) } else if err != nil { diff --git a/templates/account_own_edit_email.html b/templates/account_own_edit_email.html index 96639c00..d38685fb 100644 --- a/templates/account_own_edit_email.html +++ b/templates/account_own_edit_email.html @@ -2,7 +2,6 @@

{{lang "account_email_head"}}

- {{range .ItemList}}
{{.Email}} @@ -11,5 +10,5 @@ {{if .Validated}}{{lang "account_email_verified"}}{{else}}{{lang "account_email_resend_email"}}{{end}}
- {{end}} + {{else}}
{{lang "account_email_none"}}
{{end}}
\ No newline at end of file diff --git a/templates/account_own_edit_password.html b/templates/account_own_edit_password.html index 21f2bd27..06a5dab3 100644 --- a/templates/account_own_edit_password.html +++ b/templates/account_own_edit_password.html @@ -1,6 +1,6 @@ {{template "header.html" . }}
- {{template "account_menu.html" . }} + {{template "account_menu.html" .}}

{{lang "account_password_head"}}

@@ -9,15 +9,15 @@
{{lang "account_password_current_password"}}
-
+
{{lang "account_password_new_password"}}
-
+
{{lang "account_password_confirm_password"}}
-
+
diff --git a/templates/guilds_view_guild.html b/templates/guilds_view_guild.html index d71a44ff..58c9d3b0 100644 --- a/templates/guilds_view_guild.html +++ b/templates/guilds_view_guild.html @@ -18,24 +18,24 @@
-
- {{range .ItemList}}
- +
+ {{range .ItemList}}
+ {{.PostCount}} replies
{{.LastReplyAt}} {{.Title}}
Starter: {{.Creator.Name}} - {{if .IsClosed}} | 🔒︎{{end}} + {{if .IsClosed}} | 🔒︎{{end}}
-
+
- {{.LastUser.Name}}
+ {{.LastUser.Name}}
Last: {{.LastReplyAt}}
{{else}}
There aren't any topics in here yet.{{if .CurrentUser.Perms.CreateTopic}} Start one?{{end}}
{{end}}
-{{template "footer.html" . }} +{{template "footer.html" . }} \ No newline at end of file diff --git a/templates/register.html b/templates/register.html index ff7112c3..87aa1e82 100644 --- a/templates/register.html +++ b/templates/register.html @@ -6,12 +6,12 @@
-
{{lang "register_account_name"}}
-
+
{{lang "register_account_name"}}
+
-
{{lang "register_account_email"}}
-
+
{{if not .Something}}{{lang "register_account_email"}}{{else}}{{lang "register_account_email_optional"}}{{end}}
+
{{lang "register_account_password"}}