From bbfd3c51c7e868b0efa8814ba79bc289058ea6f7 Mon Sep 17 00:00:00 2001 From: Azareal Date: Sat, 19 Oct 2019 20:33:59 +1000 Subject: [PATCH] Stop blocked users making profile comments too. Hide the Send Message option on profiles for blocked users. Move the profile reply routes to their own file. Remove a redundant user initialisation. Shorten things. --- common/forum.go | 4 +- common/likes.go | 16 ++-- common/pages.go | 2 + common/relations.go | 13 ++-- common/template_init.go | 2 +- common/topic.go | 10 +-- common/user.go | 10 +-- routes/profile.go | 13 ++-- routes/profile_reply.go | 131 ++++++++++++++++++++++++++++++++ routes/reply.go | 164 ++++++---------------------------------- templates/profile.html | 8 +- 11 files changed, 196 insertions(+), 177 deletions(-) create mode 100644 routes/profile_reply.go diff --git a/common/forum.go b/common/forum.go index cd627223..0a239aa3 100644 --- a/common/forum.go +++ b/common/forum.go @@ -95,9 +95,9 @@ func (f *Forum) Update(name string, desc string, active bool, preset string) err } func (f *Forum) SetPreset(preset string, gid int) error { - fperms, changed := GroupForumPresetToForumPerms(preset) + fp, changed := GroupForumPresetToForumPerms(preset) if changed { - return f.SetPerms(fperms, preset, gid) + return f.SetPerms(fp, preset, gid) } return nil } diff --git a/common/likes.go b/common/likes.go index 1e40a23f..fce98326 100644 --- a/common/likes.go +++ b/common/likes.go @@ -1,7 +1,10 @@ package common -import "database/sql" -import "github.com/Azareal/Gosora/query_gen" +import ( + "database/sql" + + qgen "github.com/Azareal/Gosora/query_gen" +) var Likes LikeStore @@ -22,7 +25,7 @@ func NewDefaultLikeStore(acc *qgen.Accumulator) (*DefaultLikeStore, error) { // TODO: Write a test for this func (s *DefaultLikeStore) BulkExists(ids []int, sentBy int, targetType string) (eids []int, err error) { - rows, err := qgen.NewAcc().Select("likes").Columns("targetItem").Where("sentBy = ? AND targetType = ?").In("targetItem", ids).Query(sentBy,targetType) + rows, err := qgen.NewAcc().Select("likes").Columns("targetItem").Where("sentBy = ? AND targetType = ?").In("targetItem", ids).Query(sentBy, targetType) if err == sql.ErrNoRows { return nil, nil } else if err != nil { @@ -32,11 +35,10 @@ func (s *DefaultLikeStore) BulkExists(ids []int, sentBy int, targetType string) var id int for rows.Next() { - err = rows.Scan(&id) - if err != nil { + if err := rows.Scan(&id); err != nil { return nil, err } - eids = append(eids,id) + eids = append(eids, id) } return eids, rows.Err() } @@ -49,4 +51,4 @@ func (s *DefaultLikeStore) Count() (count int) { LogError(err) } return count -} \ No newline at end of file +} diff --git a/common/pages.go b/common/pages.go index 00d56058..89180a94 100644 --- a/common/pages.go +++ b/common/pages.go @@ -208,6 +208,8 @@ type ProfilePage struct { CurrentScore int NextScore int Blocked bool + CanMessage bool + CanComment bool } type CreateTopicPage struct { diff --git a/common/relations.go b/common/relations.go index c1702165..9307582f 100644 --- a/common/relations.go +++ b/common/relations.go @@ -23,19 +23,20 @@ type DefaultBlockStore struct { } func NewDefaultBlockStore(acc *qgen.Accumulator) (*DefaultBlockStore, error) { + ub := "users_blocks" return &DefaultBlockStore{ - isBlocked: acc.Select("users_blocks").Cols("blocker").Where("blocker = ? AND blockedUser = ?").Prepare(), - add: acc.Insert("users_blocks").Columns("blocker,blockedUser").Fields("?,?").Prepare(), - remove: acc.Delete("users_blocks").Where("blocker = ? AND blockedUser = ?").Prepare(), + isBlocked: acc.Select(ub).Cols("blocker").Where("blocker = ? AND blockedUser = ?").Prepare(), + add: acc.Insert(ub).Columns("blocker,blockedUser").Fields("?,?").Prepare(), + remove: acc.Delete(ub).Where("blocker = ? AND blockedUser = ?").Prepare(), }, acc.FirstError() } func (s *DefaultBlockStore) IsBlockedBy(blocker, blockee int) (bool, error) { err := s.isBlocked.QueryRow(blocker, blockee).Scan(&blocker) - if err != nil && err != ErrNoRows { - return false, err + if err == ErrNoRows { + return false, nil } - return err != ErrNoRows, nil + return err == nil, err } func (s *DefaultBlockStore) Add(blocker, blockee int) error { diff --git a/common/template_init.go b/common/template_init.go index 136c13e9..0c9294f9 100644 --- a/common/template_init.go +++ b/common/template_init.go @@ -296,7 +296,7 @@ func compileTemplates(wg *sync.WaitGroup, c *tmpl.CTemplateSet, themeName string return err } - ppage := ProfilePage{htitle("User 526"), replyList, user, 0, 0, false} // TODO: Use the score from user to generate the currentScore and nextScore + ppage := ProfilePage{htitle("User 526"), replyList, user, 0, 0, false,false,false} // TODO: Use the score from user to generate the currentScore and nextScore t.Add("profile", "c.ProfilePage", ppage) var topicsList []*TopicsRow diff --git a/common/topic.go b/common/topic.go index d4234eaa..988c5dae 100644 --- a/common/topic.go +++ b/common/topic.go @@ -242,9 +242,8 @@ func init() { // Flush the topic out of the cache // ? - We do a CacheRemove() here instead of mutating the pointer to avoid creating a race condition func (t *Topic) cacheRemove() { - tcache := Topics.GetCache() - if tcache != nil { - tcache.Remove(t.ID) + if tc := Topics.GetCache(); tc != nil { + tc.Remove(t.ID) } TopicListThaw.Thaw() } @@ -565,11 +564,10 @@ func (t *TopicUser) Replies(offset int, pFrag int, user *User) (rlist []*ReplyUs if err != nil { return nil, "", err } - - err = reply.Init() - if err != nil { + if err := reply.Init(); err != nil { return nil, "", err } + reply.ContentHtml = ParseMessage(reply.Content, t.ParentID, "forums") // TODO: This doesn't work properly so pick the first one instead? diff --git a/common/user.go b/common/user.go index 443111b8..3eef6284 100644 --- a/common/user.go +++ b/common/user.go @@ -180,9 +180,8 @@ func (u *User) Init() { // TODO: Refactor this idiom into something shorter, maybe with a NullUserCache when one isn't set? func (u *User) CacheRemove() { - ucache := Users.GetCache() - if ucache != nil { - ucache.Remove(u.ID) + if uc := Users.GetCache(); uc != nil { + uc.Remove(u.ID) } TopicListThaw.Thaw() } @@ -336,9 +335,8 @@ func (u *User) ChangeGroup(group int) (err error) { // ! Only updates the database not the *User for safety reasons func (u *User) UpdateIP(host string) error { _, err := userStmts.updateLastIP.Exec(host, u.ID) - ucache := Users.GetCache() - if ucache != nil { - ucache.Remove(u.ID) + if uc := Users.GetCache(); uc != nil { + uc.Remove(u.ID) } return err } diff --git a/routes/profile.go b/routes/profile.go index d476afe9..81607966 100644 --- a/routes/profile.go +++ b/routes/profile.go @@ -59,7 +59,6 @@ func ViewProfile(w http.ResponseWriter, r *http.Request, user c.User, header *c. } else if err != nil { return c.InternalError(err, w, r) } - puser.Init() } header.Title = phrases.GetTitlePhrasef("profile", puser.Name) header.Path = c.BuildProfileURL(c.NameToSlug(puser.Name), puser.ID) @@ -86,13 +85,10 @@ func ViewProfile(w http.ResponseWriter, r *http.Request, user c.User, header *c. if err != nil { return c.InternalError(err, w, r) } - if group.Tag != "" { ru.Tag = group.Tag } else if puser.ID == ru.CreatedBy { ru.Tag = phrases.GetTmplPhrase("profile_owner_tag") - } else { - ru.Tag = "" } // TODO: Add a hook here @@ -111,6 +107,13 @@ func ViewProfile(w http.ResponseWriter, r *http.Request, user c.User, header *c. return c.InternalError(err, w, r) } - ppage := c.ProfilePage{header, replyList, *puser, currentScore, nextScore, blocked} + blockedInv, err := c.UserBlocks.IsBlockedBy(puser.ID, user.ID) + if err != nil { + return c.InternalError(err, w, r) + } + canMessage := (!blockedInv && user.Perms.UseConvos) || user.IsSuperMod + canComment := !blockedInv && user.Perms.ViewTopic && user.Perms.CreateReply + + ppage := c.ProfilePage{header, replyList, *puser, currentScore, nextScore, blocked, canMessage, canComment} return renderTemplate("profile", w, r, header, ppage) } diff --git a/routes/profile_reply.go b/routes/profile_reply.go new file mode 100644 index 00000000..a4595194 --- /dev/null +++ b/routes/profile_reply.go @@ -0,0 +1,131 @@ +package routes + +import ( + "database/sql" + "net/http" + "strconv" + + c "github.com/Azareal/Gosora/common" + "github.com/Azareal/Gosora/common/counters" +) + +func ProfileReplyCreateSubmit(w http.ResponseWriter, r *http.Request, user c.User) c.RouteError { + if !user.Perms.ViewTopic || !user.Perms.CreateReply { + return c.NoPermissions(w, r, user) + } + uid, err := strconv.Atoi(r.PostFormValue("uid")) + if err != nil { + return c.LocalError("Invalid UID", w, r, user) + } + + profileOwner, err := c.Users.Get(uid) + if err == sql.ErrNoRows { + return c.LocalError("The profile you're trying to post on doesn't exist.", w, r, user) + } else if err != nil { + return c.InternalError(err, w, r) + } + + blocked, err := c.UserBlocks.IsBlockedBy(profileOwner.ID, user.ID) + if err != nil { + return c.InternalError(err, w, r) + } + // Supermods can bypass blocks so they can tell people off when they do something stupid or have to convey important information + if blocked && !user.IsSuperMod { + return c.LocalError("You don't have permission to send messages to one of these users.", w, r, user) + } + + content := c.PreparseMessage(r.PostFormValue("content")) + if len(content) == 0 { + return c.LocalError("You can't make a blank post", w, r, user) + } + // TODO: Fully parse the post and store it in the parsed column + _, err = c.Prstore.Create(profileOwner.ID, content, user.ID, user.LastIP) + if err != nil { + return c.InternalError(err, w, r) + } + + // ! Be careful about leaking per-route permission state with &user + alert := c.Alert{ActorID: user.ID, TargetUserID: profileOwner.ID, Event: "reply", ElementType: "user", ElementID: profileOwner.ID, Actor: &user} + err = c.AddActivityAndNotifyTarget(alert) + if err != nil { + return c.InternalError(err, w, r) + } + + counters.PostCounter.Bump() + http.Redirect(w, r, "/user/"+strconv.Itoa(uid), http.StatusSeeOther) + return nil +} + +func ProfileReplyEditSubmit(w http.ResponseWriter, r *http.Request, user c.User, srid string) c.RouteError { + js := r.PostFormValue("js") == "1" + rid, err := strconv.Atoi(srid) + if err != nil { + return c.LocalErrorJSQ("The provided Reply ID is not a valid number.", w, r, user, js) + } + + reply, err := c.Prstore.Get(rid) + if err == sql.ErrNoRows { + return c.PreErrorJSQ("The target reply doesn't exist.", w, r, js) + } else if err != nil { + return c.InternalErrorJSQ(err, w, r, js) + } + + creator, err := c.Users.Get(reply.CreatedBy) + if err != nil { + return c.InternalErrorJSQ(err, w, r, js) + } + // ? Does the admin understand that this group perm affects this? + if user.ID != creator.ID && !user.Perms.EditReply { + return c.NoPermissionsJSQ(w, r, user, js) + } + + // TODO: Stop blocked users from modifying profile replies? + + err = reply.SetBody(r.PostFormValue("edit_item")) + if err != nil { + return c.InternalErrorJSQ(err, w, r, js) + } + + if !js { + http.Redirect(w, r, "/user/"+strconv.Itoa(creator.ID)+"#reply-"+strconv.Itoa(rid), http.StatusSeeOther) + } else { + w.Write(successJSONBytes) + } + return nil +} + +func ProfileReplyDeleteSubmit(w http.ResponseWriter, r *http.Request, user c.User, srid string) c.RouteError { + js := r.PostFormValue("js") == "1" + rid, err := strconv.Atoi(srid) + if err != nil { + return c.LocalErrorJSQ("The provided Reply ID is not a valid number.", w, r, user, js) + } + + reply, err := c.Prstore.Get(rid) + if err == sql.ErrNoRows { + return c.PreErrorJSQ("The target reply doesn't exist.", w, r, js) + } else if err != nil { + return c.InternalErrorJSQ(err, w, r, js) + } + + creator, err := c.Users.Get(reply.CreatedBy) + if err != nil { + return c.InternalErrorJSQ(err, w, r, js) + } + if user.ID != creator.ID && !user.Perms.DeleteReply { + return c.NoPermissionsJSQ(w, r, user, js) + } + + err = reply.Delete() + if err != nil { + return c.InternalErrorJSQ(err, w, r, js) + } + //log.Printf("The profile post '%d' was deleted by c.User #%d", reply.ID, user.ID) + + if !js { + //http.Redirect(w,r, "/user/" + strconv.Itoa(creator.ID), http.StatusSeeOther) + } else { + w.Write(successJSONBytes) + } + return nil +} diff --git a/routes/reply.go b/routes/reply.go index cc6bc778..71b349b1 100644 --- a/routes/reply.go +++ b/routes/reply.go @@ -92,26 +92,27 @@ func CreateReplySubmit(w http.ResponseWriter, r *http.Request, user c.User) c.Ro //c.DebugDetail("key: ", key) //c.DebugDetailf("values: %+v\n", values) for _, value := range values { - if strings.HasPrefix(key, "pollinputitem[") { - halves := strings.Split(key, "[") - if len(halves) != 2 { - return c.LocalErrorJSQ("Malformed pollinputitem", w, r, user, js) - } - halves[1] = strings.TrimSuffix(halves[1], "]") + if !strings.HasPrefix(key, "pollinputitem[") { + continue + } + halves := strings.Split(key, "[") + if len(halves) != 2 { + return c.LocalErrorJSQ("Malformed pollinputitem", w, r, user, js) + } + halves[1] = strings.TrimSuffix(halves[1], "]") - index, err := strconv.Atoi(halves[1]) - if err != nil { - return c.LocalErrorJSQ("Malformed pollinputitem", w, r, user, js) - } + index, err := strconv.Atoi(halves[1]) + if err != nil { + return c.LocalErrorJSQ("Malformed pollinputitem", w, r, user, js) + } - // If there are duplicates, then something has gone horribly wrong, so let's ignore them, this'll likely happen during an attack - _, exists := pollInputItems[index] - // TODO: Should we use SanitiseBody instead to keep the newlines? - if !exists && len(c.SanitiseSingleLine(value)) != 0 { - pollInputItems[index] = c.SanitiseSingleLine(value) - if len(pollInputItems) >= maxPollOptions { - break - } + // If there are duplicates, then something has gone horribly wrong, so let's ignore them, this'll likely happen during an attack + _, exists := pollInputItems[index] + // TODO: Should we use SanitiseBody instead to keep the newlines? + if !exists && len(c.SanitiseSingleLine(value)) != 0 { + pollInputItems[index] = c.SanitiseSingleLine(value) + if len(pollInputItems) >= maxPollOptions { + break } } } @@ -140,8 +141,7 @@ func CreateReplySubmit(w http.ResponseWriter, r *http.Request, user c.User) c.Ro return c.InternalErrorJSQ(err, w, r, js) } - wcount := c.WordCount(content) - err = user.IncreasePostStats(wcount, false) + err = user.IncreasePostStats(c.WordCount(content), false) if err != nil { return c.InternalErrorJSQ(err, w, r, js) } @@ -164,14 +164,12 @@ func CreateReplySubmit(w http.ResponseWriter, r *http.Request, user c.User) c.Ro var rids []int for rows.Next() { var rid int - err := rows.Scan(&rid) - if err != nil { + if err := rows.Scan(&rid); err != nil { return c.InternalErrorJSQ(err, w, r, js) } rids = append(rids, rid) } - err = rows.Err() - if err != nil { + if err := rows.Err(); err != nil { return c.InternalErrorJSQ(err, w, r, js) } if len(rids) == 0 { @@ -312,9 +310,7 @@ func ReplyDeleteSubmit(w http.ResponseWriter, r *http.Request, user c.User, srid return c.NoPermissionsJSQ(w, r, user, js) } } - - err = reply.Delete() - if err != nil { + if err := reply.Delete(); err != nil { return c.InternalErrorJSQ(err, w, r, js) } @@ -333,8 +329,7 @@ func ReplyDeleteSubmit(w http.ResponseWriter, r *http.Request, user c.User, srid // ? - What happens if an error fires after a redirect...? replyCreator, err := c.Users.Get(reply.CreatedBy) if err == nil { - wcount := c.WordCount(reply.Content) - err = replyCreator.DecreasePostStats(wcount, false) + err = replyCreator.DecreasePostStats(c.WordCount(reply.Content), false) if err != nil { return c.InternalErrorJSQ(err, w, r, js) } @@ -463,117 +458,6 @@ func RemoveAttachFromReplySubmit(w http.ResponseWriter, r *http.Request, user c. return nil } -// TODO: Move the profile reply routes to their own file? -func ProfileReplyCreateSubmit(w http.ResponseWriter, r *http.Request, user c.User) c.RouteError { - if !user.Perms.ViewTopic || !user.Perms.CreateReply { - return c.NoPermissions(w, r, user) - } - uid, err := strconv.Atoi(r.PostFormValue("uid")) - if err != nil { - return c.LocalError("Invalid UID", w, r, user) - } - - profileOwner, err := c.Users.Get(uid) - if err == sql.ErrNoRows { - return c.LocalError("The profile you're trying to post on doesn't exist.", w, r, user) - } else if err != nil { - return c.InternalError(err, w, r) - } - - content := c.PreparseMessage(r.PostFormValue("content")) - if len(content) == 0 { - return c.LocalError("You can't make a blank post", w, r, user) - } - // TODO: Fully parse the post and store it in the parsed column - _, err = c.Prstore.Create(profileOwner.ID, content, user.ID, user.LastIP) - if err != nil { - return c.InternalError(err, w, r) - } - - // ! Be careful about leaking per-route permission state with &user - alert := c.Alert{ActorID: user.ID, TargetUserID: profileOwner.ID, Event: "reply", ElementType: "user", ElementID: profileOwner.ID, Actor: &user} - err = c.AddActivityAndNotifyTarget(alert) - if err != nil { - return c.InternalError(err, w, r) - } - - counters.PostCounter.Bump() - http.Redirect(w, r, "/user/"+strconv.Itoa(uid), http.StatusSeeOther) - return nil -} - -func ProfileReplyEditSubmit(w http.ResponseWriter, r *http.Request, user c.User, srid string) c.RouteError { - js := r.PostFormValue("js") == "1" - rid, err := strconv.Atoi(srid) - if err != nil { - return c.LocalErrorJSQ("The provided Reply ID is not a valid number.", w, r, user, js) - } - - reply, err := c.Prstore.Get(rid) - if err == sql.ErrNoRows { - return c.PreErrorJSQ("The target reply doesn't exist.", w, r, js) - } else if err != nil { - return c.InternalErrorJSQ(err, w, r, js) - } - - creator, err := c.Users.Get(reply.CreatedBy) - if err != nil { - return c.InternalErrorJSQ(err, w, r, js) - } - // ? Does the admin understand that this group perm affects this? - if user.ID != creator.ID && !user.Perms.EditReply { - return c.NoPermissionsJSQ(w, r, user, js) - } - - err = reply.SetBody(r.PostFormValue("edit_item")) - if err != nil { - return c.InternalErrorJSQ(err, w, r, js) - } - - if !js { - http.Redirect(w, r, "/user/"+strconv.Itoa(creator.ID)+"#reply-"+strconv.Itoa(rid), http.StatusSeeOther) - } else { - w.Write(successJSONBytes) - } - return nil -} - -func ProfileReplyDeleteSubmit(w http.ResponseWriter, r *http.Request, user c.User, srid string) c.RouteError { - js := r.PostFormValue("js") == "1" - rid, err := strconv.Atoi(srid) - if err != nil { - return c.LocalErrorJSQ("The provided Reply ID is not a valid number.", w, r, user, js) - } - - reply, err := c.Prstore.Get(rid) - if err == sql.ErrNoRows { - return c.PreErrorJSQ("The target reply doesn't exist.", w, r, js) - } else if err != nil { - return c.InternalErrorJSQ(err, w, r, js) - } - - creator, err := c.Users.Get(reply.CreatedBy) - if err != nil { - return c.InternalErrorJSQ(err, w, r, js) - } - if user.ID != creator.ID && !user.Perms.DeleteReply { - return c.NoPermissionsJSQ(w, r, user, js) - } - - err = reply.Delete() - if err != nil { - return c.InternalErrorJSQ(err, w, r, js) - } - //log.Printf("The profile post '%d' was deleted by c.User #%d", reply.ID, user.ID) - - if !js { - //http.Redirect(w,r, "/user/" + strconv.Itoa(creator.ID), http.StatusSeeOther) - } else { - w.Write(successJSONBytes) - } - return nil -} - func ReplyLikeSubmit(w http.ResponseWriter, r *http.Request, user c.User, srid string) c.RouteError { js := r.PostFormValue("js") == "1" rid, err := strconv.Atoi(srid) diff --git a/templates/profile.html b/templates/profile.html index fb3966c2..9c8058cd 100644 --- a/templates/profile.html +++ b/templates/profile.html @@ -27,13 +27,13 @@ {{if not .CurrentUser.Loggedin}}
{{lang "profile_login_for_options"}}
{{else}} -
+ {{if .CanMessage}}
{{lang "profile_send_message"}} -
+
{{end}} - {{if (.CurrentUser.IsSuperMod) and not (.ProfileOwner.IsSuperMod) }}
+ {{if (.CurrentUser.IsSuperMod) and not (.ProfileOwner.IsSuperMod)}}
{{if .ProfileOwner.IsBanned}}{{lang "profile_unban"}} {{else}}{{lang "profile_ban"}}{{end}}
{{end}} @@ -97,7 +97,7 @@
{{template "profile_comments_row.html" . }}
{{if .CurrentUser.Loggedin}} -{{if not .CurrentUser.IsBanned}} +{{if .CanComment}}