From 93cec4c6c5698b6491d835a0093a6b83153c8395 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 3 Jul 2022 22:46:22 +0100 Subject: [PATCH 1/4] Only show Followers that current user can access Users who are following or being followed by a user should only be displayed if the viewing user can see them. Signed-off-by: Andrew Thornton --- models/user/user.go | 63 ++++++++++++++++++++++++++++----- routers/api/v1/user/follower.go | 8 ++--- routers/web/user/profile.go | 8 ++--- 3 files changed, 62 insertions(+), 17 deletions(-) diff --git a/models/user/user.go b/models/user/user.go index 9460bd38fe428..5db76ea7e7b3a 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -316,37 +316,45 @@ func (u *User) GenerateEmailActivateCode(email string) string { } // GetUserFollowers returns range of user's followers. -func GetUserFollowers(u *User, listOptions db.ListOptions) ([]*User, error) { - sess := db.GetEngine(db.DefaultContext). +func GetUserFollowers(ctx context.Context, u, viewer *User, listOptions db.ListOptions) ([]*User, int64, error) { + sess := db.GetEngine(ctx). + Select("`user`.*"). + Join("LEFT", "follow", "`user`.id=follow.user_id"). Where("follow.follow_id=?", u.ID). - Join("LEFT", "follow", "`user`.id=follow.user_id") + And(isUserVisibleToViewerCond(viewer)) if listOptions.Page != 0 { sess = db.SetSessionPagination(sess, &listOptions) users := make([]*User, 0, listOptions.PageSize) - return users, sess.Find(&users) + count, err := sess.FindAndCount(&users) + return users, count, err } users := make([]*User, 0, 8) - return users, sess.Find(&users) + count, err := sess.FindAndCount(&users) + return users, count, err } // GetUserFollowing returns range of user's following. -func GetUserFollowing(u *User, listOptions db.ListOptions) ([]*User, error) { +func GetUserFollowing(ctx context.Context, u, viewer *User, listOptions db.ListOptions) ([]*User, int64, error) { sess := db.GetEngine(db.DefaultContext). + Select("`user`.*"). + Join("LEFT", "follow", "`user`.id=follow.follow_id"). Where("follow.user_id=?", u.ID). - Join("LEFT", "follow", "`user`.id=follow.follow_id") + And(isUserVisibleToViewerCond(viewer)) if listOptions.Page != 0 { sess = db.SetSessionPagination(sess, &listOptions) users := make([]*User, 0, listOptions.PageSize) - return users, sess.Find(&users) + count, err := sess.FindAndCount(&users) + return users, count, err } users := make([]*User, 0, 8) - return users, sess.Find(&users) + count, err := sess.FindAndCount(&users) + return users, count, err } // NewGitSig generates and returns the signature of given user. @@ -1219,6 +1227,43 @@ func GetAdminUser() (*User, error) { return &admin, nil } +func isUserVisibleToViewerCond(viewer *User) builder.Cond { + cond := builder.NewCond() + if viewer != nil && viewer.IsAdmin { + return cond + } + cond = builder.Eq{ + "`user`.Visibility": structs.VisibleTypePublic, + } + + if viewer == nil || viewer.IsRestricted { + return cond + } + + cond = builder.Not{builder.Eq{ + "`user`.Visibility": structs.VisibleTypePrivate, + }}.Or( + builder.In("`user`.id", + builder. + Select("`follow`.user_id"). + From("follow"). + Where(builder.Eq{"`follow`.follow_id": viewer.ID})), + builder.In("`user`.id", + builder. + Select("`team_user`.uid"). + From("team_user"). + Join("INNER", "`team_user` AS t2", "`team_user`.id = `t2`.id"). + Where(builder.Eq{"`t2`.uid": viewer.ID})), + builder.In("`user`.id", + builder. + Select("`team_user`.uid"). + From("team_user"). + Join("INNER", "`team_user` AS t2", "`team_user`.org_id = `t2`.org_id"). + Where(builder.Eq{"`t2`.uid": viewer.ID}))) + + return cond +} + // IsUserVisibleToViewer check if viewer is able to see user profile func IsUserVisibleToViewer(ctx context.Context, u, viewer *User) bool { if viewer != nil && viewer.IsAdmin { diff --git a/routers/api/v1/user/follower.go b/routers/api/v1/user/follower.go index 3c81b27f8dad4..22f8f40e1c810 100644 --- a/routers/api/v1/user/follower.go +++ b/routers/api/v1/user/follower.go @@ -24,13 +24,13 @@ func responseAPIUsers(ctx *context.APIContext, users []*user_model.User) { } func listUserFollowers(ctx *context.APIContext, u *user_model.User) { - users, err := user_model.GetUserFollowers(u, utils.GetListOptions(ctx)) + users, count, err := user_model.GetUserFollowers(ctx, u, ctx.Doer, utils.GetListOptions(ctx)) if err != nil { ctx.Error(http.StatusInternalServerError, "GetUserFollowers", err) return } - ctx.SetTotalCountHeader(int64(u.NumFollowers)) + ctx.SetTotalCountHeader(count) responseAPIUsers(ctx, users) } @@ -86,13 +86,13 @@ func ListFollowers(ctx *context.APIContext) { } func listUserFollowing(ctx *context.APIContext, u *user_model.User) { - users, err := user_model.GetUserFollowing(u, utils.GetListOptions(ctx)) + users, count, err := user_model.GetUserFollowing(ctx, u, ctx.Doer, utils.GetListOptions(ctx)) if err != nil { ctx.Error(http.StatusInternalServerError, "GetUserFollowing", err) return } - ctx.SetTotalCountHeader(int64(u.NumFollowing)) + ctx.SetTotalCountHeader(count) responseAPIUsers(ctx, users) } diff --git a/routers/web/user/profile.go b/routers/web/user/profile.go index 609f3242c65e2..6f23d239e26a5 100644 --- a/routers/web/user/profile.go +++ b/routers/web/user/profile.go @@ -157,7 +157,7 @@ func Profile(ctx *context.Context) { switch tab { case "followers": - items, err := user_model.GetUserFollowers(ctx.ContextUser, db.ListOptions{ + items, count, err := user_model.GetUserFollowers(ctx, ctx.ContextUser, ctx.Doer, db.ListOptions{ PageSize: setting.UI.User.RepoPagingNum, Page: page, }) @@ -167,9 +167,9 @@ func Profile(ctx *context.Context) { } ctx.Data["Cards"] = items - total = ctx.ContextUser.NumFollowers + total = int(count) case "following": - items, err := user_model.GetUserFollowing(ctx.ContextUser, db.ListOptions{ + items, count, err := user_model.GetUserFollowing(ctx, ctx.ContextUser, ctx.Doer, db.ListOptions{ PageSize: setting.UI.User.RepoPagingNum, Page: page, }) @@ -179,7 +179,7 @@ func Profile(ctx *context.Context) { } ctx.Data["Cards"] = items - total = ctx.ContextUser.NumFollowing + total = int(count) case "activity": ctx.Data["Feeds"], err = models.GetFeeds(ctx, models.GetFeedsOptions{ RequestedUser: ctx.ContextUser, From aeff803cfcf69a801822f8630f45581ee67a2549 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 3 Jul 2022 23:06:42 +0100 Subject: [PATCH 2/4] builder.Neq not builder.NEq Signed-off-by: Andrew Thornton --- models/user/user.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/models/user/user.go b/models/user/user.go index 5db76ea7e7b3a..5d8637be1abab 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -1232,17 +1232,16 @@ func isUserVisibleToViewerCond(viewer *User) builder.Cond { if viewer != nil && viewer.IsAdmin { return cond } - cond = builder.Eq{ - "`user`.Visibility": structs.VisibleTypePublic, - } if viewer == nil || viewer.IsRestricted { - return cond + return builder.Eq{ + "`user`.Visibility": structs.VisibleTypePublic, + } } - cond = builder.Not{builder.Eq{ + cond = builder.Neq{ "`user`.Visibility": structs.VisibleTypePrivate, - }}.Or( + }.Or( builder.In("`user`.id", builder. Select("`follow`.user_id"). From 08d46f96f4886fe0666d20d243035873516d6d5c Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 3 Jul 2022 23:08:16 +0100 Subject: [PATCH 3/4] remove common cond as it is confusing people Signed-off-by: Andrew Thornton --- models/user/user.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/models/user/user.go b/models/user/user.go index 5d8637be1abab..6b1c944802f46 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -1228,9 +1228,8 @@ func GetAdminUser() (*User, error) { } func isUserVisibleToViewerCond(viewer *User) builder.Cond { - cond := builder.NewCond() if viewer != nil && viewer.IsAdmin { - return cond + return builder.NewCond() } if viewer == nil || viewer.IsRestricted { @@ -1239,7 +1238,7 @@ func isUserVisibleToViewerCond(viewer *User) builder.Cond { } } - cond = builder.Neq{ + return builder.Neq{ "`user`.Visibility": structs.VisibleTypePrivate, }.Or( builder.In("`user`.id", @@ -1259,8 +1258,6 @@ func isUserVisibleToViewerCond(viewer *User) builder.Cond { From("team_user"). Join("INNER", "`team_user` AS t2", "`team_user`.org_id = `t2`.org_id"). Where(builder.Eq{"`t2`.uid": viewer.ID}))) - - return cond } // IsUserVisibleToViewer check if viewer is able to see user profile From a450abaa1ccda4e12bb7d8bb6f20afca0d3c4e30 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sun, 3 Jul 2022 23:09:17 +0100 Subject: [PATCH 4/4] use lowercase for visibility Signed-off-by: Andrew Thornton --- models/user/user.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/user/user.go b/models/user/user.go index 6b1c944802f46..f487c883bff41 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -1234,12 +1234,12 @@ func isUserVisibleToViewerCond(viewer *User) builder.Cond { if viewer == nil || viewer.IsRestricted { return builder.Eq{ - "`user`.Visibility": structs.VisibleTypePublic, + "`user`.visibility": structs.VisibleTypePublic, } } return builder.Neq{ - "`user`.Visibility": structs.VisibleTypePrivate, + "`user`.visibility": structs.VisibleTypePrivate, }.Or( builder.In("`user`.id", builder.