From f6ee6d848190116a7f3e685f2ee4807688551bfe Mon Sep 17 00:00:00 2001 From: Gusted Date: Mon, 27 Jun 2022 15:14:12 +0200 Subject: [PATCH 1/3] Only show users that the doer has access to - Don't show users to the doer hasn't access to(user's visibility set to private or limited). - This PR fixes this problem for User's followers and User's followings and uses a unified option struct(given only one condition changes, it's not worth the duplicated code). --- models/user/user.go | 82 +++++++++++++++++++++++++++------ routers/api/v1/user/follower.go | 12 ++++- routers/web/user/profile.go | 14 +++--- 3 files changed, 87 insertions(+), 21 deletions(-) diff --git a/models/user/user.go b/models/user/user.go index 9460bd38fe428..ca434b07997c6 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -316,15 +316,16 @@ 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). - Where("follow.follow_id=?", u.ID). +func GetUserFollowers(ctx context.Context, opts GetUserFollowOptions) ([]*User, error) { + opts.checkFollowers = true + + sess := db.GetEngine(ctx).Where(opts.toCond()). Join("LEFT", "follow", "`user`.id=follow.user_id") - if listOptions.Page != 0 { - sess = db.SetSessionPagination(sess, &listOptions) + if opts.Page != 0 { + sess = db.SetSessionPagination(sess, &opts.ListOptions) - users := make([]*User, 0, listOptions.PageSize) + users := make([]*User, 0, opts.PageSize) return users, sess.Find(&users) } @@ -332,16 +333,71 @@ func GetUserFollowers(u *User, listOptions db.ListOptions) ([]*User, error) { return users, sess.Find(&users) } +// GetFeedsOptions options for getting the user's followers or followings. +type GetUserFollowOptions struct { + db.ListOptions + checkFollowers bool // Specify if we check for followers or followings. + Actor *User // the user viewing the followings + RequestedUser *User // the user we want followings for +} + +func (opts GetUserFollowOptions) toCond() builder.Cond { + cond := builder.NewCond() + + if opts.checkFollowers { + cond = cond.And(builder.Eq{"`follow`.follow_id": opts.RequestedUser.ID}) + } else { + cond = cond.And(builder.Eq{"`follow`.user_id": opts.RequestedUser.ID}) + } + + // If the actor is not signed in. Only show users that have their visibility + // set to public. + if opts.Actor == nil { + return cond.And(builder.Eq{"`user`.visibility": 0}) + } + + // Fast path for admins. + if opts.Actor.IsAdmin { + return cond + } + + // If the actor is signed in, then we allow all limited & public accounts to be seen. + // However the actor can also see private accounts if they have are in the same organisation + // as that user. + cond = cond.And( + builder.Or( + // Include all limited & public accounts. + builder.Lte{"`user`.visibility": structs.VisibleTypeLimited}, + // Check which private users can be included. + // Get the actor's orginisations and check if the private + // users are in those orgs. + builder.And( + // Specify all private users. + builder.Eq{"`user`.visibility": structs.VisibleTypePrivate}, + // Is the user's id in the organisation that the actor is in? + builder.In("`user`.id", + builder.Select("uid").From("org_user").Where( + builder.In("org_id", + // Get the actor's orginaisations. + builder.Select("org_id").From("org_user").Where(builder.Eq{"uid": opts.Actor.ID})), + )), + ), + ), + ) + + return cond +} + // GetUserFollowing returns range of user's following. -func GetUserFollowing(u *User, listOptions db.ListOptions) ([]*User, error) { - sess := db.GetEngine(db.DefaultContext). - Where("follow.user_id=?", u.ID). - Join("LEFT", "follow", "`user`.id=follow.follow_id") +func GetUserFollowing(ctx context.Context, opts GetUserFollowOptions) ([]*User, error) { + sess := db.GetEngine(ctx). + Join("LEFT", "follow", "`user`.id=follow.follow_id"). + Where(opts.toCond()) - if listOptions.Page != 0 { - sess = db.SetSessionPagination(sess, &listOptions) + if opts.Page != 0 { + sess = db.SetSessionPagination(sess, &opts.ListOptions) - users := make([]*User, 0, listOptions.PageSize) + users := make([]*User, 0, opts.PageSize) return users, sess.Find(&users) } diff --git a/routers/api/v1/user/follower.go b/routers/api/v1/user/follower.go index 3c81b27f8dad4..e76e57732c800 100644 --- a/routers/api/v1/user/follower.go +++ b/routers/api/v1/user/follower.go @@ -24,7 +24,11 @@ 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, err := user_model.GetUserFollowers(ctx, user_model.GetUserFollowOptions{ + RequestedUser: u, + Actor: ctx.Doer, + ListOptions: utils.GetListOptions(ctx), + }) if err != nil { ctx.Error(http.StatusInternalServerError, "GetUserFollowers", err) return @@ -86,7 +90,11 @@ func ListFollowers(ctx *context.APIContext) { } func listUserFollowing(ctx *context.APIContext, u *user_model.User) { - users, err := user_model.GetUserFollowing(u, utils.GetListOptions(ctx)) + users, err := user_model.GetUserFollowing(ctx, user_model.GetUserFollowOptions{ + Actor: ctx.Doer, + RequestedUser: u, + ListOptions: utils.GetListOptions(ctx), + }) if err != nil { ctx.Error(http.StatusInternalServerError, "GetUserFollowing", err) return diff --git a/routers/web/user/profile.go b/routers/web/user/profile.go index 609f3242c65e2..bf99bb172d822 100644 --- a/routers/web/user/profile.go +++ b/routers/web/user/profile.go @@ -157,9 +157,10 @@ func Profile(ctx *context.Context) { switch tab { case "followers": - items, err := user_model.GetUserFollowers(ctx.ContextUser, db.ListOptions{ - PageSize: setting.UI.User.RepoPagingNum, - Page: page, + items, err := user_model.GetUserFollowers(ctx, user_model.GetUserFollowOptions{ + RequestedUser: ctx.ContextUser, + Actor: ctx.Doer, + ListOptions: db.ListOptions{PageSize: setting.UI.User.RepoPagingNum, Page: page}, }) if err != nil { ctx.ServerError("GetUserFollowers", err) @@ -169,9 +170,10 @@ func Profile(ctx *context.Context) { total = ctx.ContextUser.NumFollowers case "following": - items, err := user_model.GetUserFollowing(ctx.ContextUser, db.ListOptions{ - PageSize: setting.UI.User.RepoPagingNum, - Page: page, + items, err := user_model.GetUserFollowing(ctx, user_model.GetUserFollowOptions{ + Actor: ctx.Doer, + RequestedUser: ctx.ContextUser, + ListOptions: db.ListOptions{PageSize: setting.UI.User.RepoPagingNum, Page: page}, }) if err != nil { ctx.ServerError("GetUserFollowing", err) From 54a6f56789946ed3791fd9820801181cce8f1c48 Mon Sep 17 00:00:00 2001 From: Gusted Date: Mon, 27 Jun 2022 15:22:54 +0200 Subject: [PATCH 2/3] Use correct total count. --- routers/api/v1/user/follower.go | 4 ++-- routers/web/user/profile.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/routers/api/v1/user/follower.go b/routers/api/v1/user/follower.go index e76e57732c800..06fb4a7e945a3 100644 --- a/routers/api/v1/user/follower.go +++ b/routers/api/v1/user/follower.go @@ -34,7 +34,7 @@ func listUserFollowers(ctx *context.APIContext, u *user_model.User) { return } - ctx.SetTotalCountHeader(int64(u.NumFollowers)) + ctx.SetTotalCountHeader(int64(len(users))) responseAPIUsers(ctx, users) } @@ -100,7 +100,7 @@ func listUserFollowing(ctx *context.APIContext, u *user_model.User) { return } - ctx.SetTotalCountHeader(int64(u.NumFollowing)) + ctx.SetTotalCountHeader(int64(len(users))) responseAPIUsers(ctx, users) } diff --git a/routers/web/user/profile.go b/routers/web/user/profile.go index bf99bb172d822..cab5c02432e46 100644 --- a/routers/web/user/profile.go +++ b/routers/web/user/profile.go @@ -168,7 +168,7 @@ func Profile(ctx *context.Context) { } ctx.Data["Cards"] = items - total = ctx.ContextUser.NumFollowers + total = len(items) case "following": items, err := user_model.GetUserFollowing(ctx, user_model.GetUserFollowOptions{ Actor: ctx.Doer, @@ -181,7 +181,7 @@ func Profile(ctx *context.Context) { } ctx.Data["Cards"] = items - total = ctx.ContextUser.NumFollowing + total = len(items) case "activity": ctx.Data["Feeds"], err = models.GetFeeds(ctx, models.GetFeedsOptions{ RequestedUser: ctx.ContextUser, From 70d7ff58769102c5baeb5fb12792e29b1cdc7e41 Mon Sep 17 00:00:00 2001 From: Gusted Date: Wed, 29 Jun 2022 19:49:17 +0200 Subject: [PATCH 3/3] Apply suggested changes by KN4CK3R --- models/user/user.go | 33 +++++++++++++++++---------------- routers/api/v1/user/follower.go | 8 ++++---- routers/web/user/profile.go | 8 ++++---- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/models/user/user.go b/models/user/user.go index ca434b07997c6..3ef8dda985647 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -316,35 +316,34 @@ func (u *User) GenerateEmailActivateCode(email string) string { } // GetUserFollowers returns range of user's followers. -func GetUserFollowers(ctx context.Context, opts GetUserFollowOptions) ([]*User, error) { - opts.checkFollowers = true - - sess := db.GetEngine(ctx).Where(opts.toCond()). +func GetUserFollowers(ctx context.Context, opts GetUserFollowOptions) ([]*User, int64, error) { + sess := db.GetEngine(ctx).Where(opts.toCond(true)). Join("LEFT", "follow", "`user`.id=follow.user_id") if opts.Page != 0 { sess = db.SetSessionPagination(sess, &opts.ListOptions) users := make([]*User, 0, opts.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 } // GetFeedsOptions options for getting the user's followers or followings. type GetUserFollowOptions struct { db.ListOptions - checkFollowers bool // Specify if we check for followers or followings. - Actor *User // the user viewing the followings - RequestedUser *User // the user we want followings for + Actor *User // the user viewing the followings + RequestedUser *User // the user we want followings for } -func (opts GetUserFollowOptions) toCond() builder.Cond { +func (opts GetUserFollowOptions) toCond(checkFollowers bool) builder.Cond { cond := builder.NewCond() - if opts.checkFollowers { + if checkFollowers { cond = cond.And(builder.Eq{"`follow`.follow_id": opts.RequestedUser.ID}) } else { cond = cond.And(builder.Eq{"`follow`.user_id": opts.RequestedUser.ID}) @@ -353,7 +352,7 @@ func (opts GetUserFollowOptions) toCond() builder.Cond { // If the actor is not signed in. Only show users that have their visibility // set to public. if opts.Actor == nil { - return cond.And(builder.Eq{"`user`.visibility": 0}) + return cond.And(builder.Eq{"`user`.visibility": structs.VisibleTypePublic}) } // Fast path for admins. @@ -389,20 +388,22 @@ func (opts GetUserFollowOptions) toCond() builder.Cond { } // GetUserFollowing returns range of user's following. -func GetUserFollowing(ctx context.Context, opts GetUserFollowOptions) ([]*User, error) { +func GetUserFollowing(ctx context.Context, opts GetUserFollowOptions) ([]*User, int64, error) { sess := db.GetEngine(ctx). Join("LEFT", "follow", "`user`.id=follow.follow_id"). - Where(opts.toCond()) + Where(opts.toCond(false)) if opts.Page != 0 { sess = db.SetSessionPagination(sess, &opts.ListOptions) users := make([]*User, 0, opts.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. diff --git a/routers/api/v1/user/follower.go b/routers/api/v1/user/follower.go index 06fb4a7e945a3..343ff05d7faef 100644 --- a/routers/api/v1/user/follower.go +++ b/routers/api/v1/user/follower.go @@ -24,7 +24,7 @@ func responseAPIUsers(ctx *context.APIContext, users []*user_model.User) { } func listUserFollowers(ctx *context.APIContext, u *user_model.User) { - users, err := user_model.GetUserFollowers(ctx, user_model.GetUserFollowOptions{ + users, total, err := user_model.GetUserFollowers(ctx, user_model.GetUserFollowOptions{ RequestedUser: u, Actor: ctx.Doer, ListOptions: utils.GetListOptions(ctx), @@ -34,7 +34,7 @@ func listUserFollowers(ctx *context.APIContext, u *user_model.User) { return } - ctx.SetTotalCountHeader(int64(len(users))) + ctx.SetTotalCountHeader(total) responseAPIUsers(ctx, users) } @@ -90,7 +90,7 @@ func ListFollowers(ctx *context.APIContext) { } func listUserFollowing(ctx *context.APIContext, u *user_model.User) { - users, err := user_model.GetUserFollowing(ctx, user_model.GetUserFollowOptions{ + users, total, err := user_model.GetUserFollowing(ctx, user_model.GetUserFollowOptions{ Actor: ctx.Doer, RequestedUser: u, ListOptions: utils.GetListOptions(ctx), @@ -100,7 +100,7 @@ func listUserFollowing(ctx *context.APIContext, u *user_model.User) { return } - ctx.SetTotalCountHeader(int64(len(users))) + ctx.SetTotalCountHeader(total) responseAPIUsers(ctx, users) } diff --git a/routers/web/user/profile.go b/routers/web/user/profile.go index cab5c02432e46..a9fe996bfa4ff 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, user_model.GetUserFollowOptions{ + items, amountFollowers, err := user_model.GetUserFollowers(ctx, user_model.GetUserFollowOptions{ RequestedUser: ctx.ContextUser, Actor: ctx.Doer, ListOptions: db.ListOptions{PageSize: setting.UI.User.RepoPagingNum, Page: page}, @@ -168,9 +168,9 @@ func Profile(ctx *context.Context) { } ctx.Data["Cards"] = items - total = len(items) + total = int(amountFollowers) case "following": - items, err := user_model.GetUserFollowing(ctx, user_model.GetUserFollowOptions{ + items, amountFollowings, err := user_model.GetUserFollowing(ctx, user_model.GetUserFollowOptions{ Actor: ctx.Doer, RequestedUser: ctx.ContextUser, ListOptions: db.ListOptions{PageSize: setting.UI.User.RepoPagingNum, Page: page}, @@ -181,7 +181,7 @@ func Profile(ctx *context.Context) { } ctx.Data["Cards"] = items - total = len(items) + total = int(amountFollowings) case "activity": ctx.Data["Feeds"], err = models.GetFeeds(ctx, models.GetFeedsOptions{ RequestedUser: ctx.ContextUser,