From 1b630ff7cdbb2ec48b67f8e3295c142f5ad77180 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Tue, 20 Sep 2022 09:59:20 +0200 Subject: [PATCH] Fix user visible check (#21210) Fixes #21206 If user and viewer are equal the method should return true. Also the common organization check was wrong as `count` can never be less then 0. Co-authored-by: 6543 <6543@obermui.de> Co-authored-by: Lunny Xiao --- models/fixtures/access.yml | 12 ++++++ models/fixtures/follow.yml | 5 +++ models/fixtures/org_user.yml | 6 +++ models/fixtures/team.yml | 2 +- models/fixtures/team_user.yml | 6 +++ models/fixtures/user.yml | 24 +++++++++++- models/user/user.go | 4 +- models/user/user_test.go | 53 ++++++++++++++++++++++++++ tests/integration/api_nodeinfo_test.go | 2 +- 9 files changed, 109 insertions(+), 5 deletions(-) diff --git a/models/fixtures/access.yml b/models/fixtures/access.yml index 4027e5fe9..446502843 100644 --- a/models/fixtures/access.yml +++ b/models/fixtures/access.yml @@ -124,3 +124,15 @@ repo_id: 24 mode: 1 +- + id: 22 + user_id: 31 + repo_id: 27 + mode: 4 + +- + id: 23 + user_id: 31 + repo_id: 28 + mode: 4 + diff --git a/models/fixtures/follow.yml b/models/fixtures/follow.yml index 480fa065c..b8d35828b 100644 --- a/models/fixtures/follow.yml +++ b/models/fixtures/follow.yml @@ -12,3 +12,8 @@ id: 3 user_id: 2 follow_id: 8 + +- + id: 4 + user_id: 31 + follow_id: 33 diff --git a/models/fixtures/org_user.yml b/models/fixtures/org_user.yml index e06d94cfc..d6bbdaa9d 100644 --- a/models/fixtures/org_user.yml +++ b/models/fixtures/org_user.yml @@ -69,3 +69,9 @@ uid: 2 org_id: 17 is_public: true + +- + id: 13 + uid: 31 + org_id: 19 + is_public: true diff --git a/models/fixtures/team.yml b/models/fixtures/team.yml index f6dfd1e9d..880f49dc9 100644 --- a/models/fixtures/team.yml +++ b/models/fixtures/team.yml @@ -55,7 +55,7 @@ name: Owners authorize: 4 # owner num_repos: 2 - num_members: 1 + num_members: 2 can_create_org_repo: true - diff --git a/models/fixtures/team_user.yml b/models/fixtures/team_user.yml index 8f21164df..845741eff 100644 --- a/models/fixtures/team_user.yml +++ b/models/fixtures/team_user.yml @@ -87,3 +87,9 @@ org_id: 17 team_id: 9 uid: 29 + +- + id: 16 + org_id: 19 + team_id: 6 + uid: 31 diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml index 87405bfd2..790156189 100644 --- a/models/fixtures/user.yml +++ b/models/fixtures/user.yml @@ -345,7 +345,7 @@ avatar_email: user19@example.com num_repos: 2 is_active: true - num_members: 1 + num_members: 2 num_teams: 1 - @@ -572,6 +572,8 @@ avatar: avatar31 avatar_email: user31@example.com num_repos: 0 + num_followers: 0 + num_following: 1 is_active: true - @@ -590,3 +592,23 @@ avatar_email: user30@example.com num_repos: 0 is_active: true + +- + id: 33 + lower_name: user33 + name: user33 + login_name: user33 + full_name: User 33 (Limited Visibility) + email: user33@example.com + passwd_hash_algo: argon2 + passwd: a3d5fcd92bae586c2e3dbe72daea7a0d27833a8d0227aa1704f4bbd775c1f3b03535b76dd93b0d4d8d22a519dca47df1547b # password + type: 0 # individual + salt: ZogKvWdyEx + is_admin: false + visibility: 1 + avatar: avatar33 + avatar_email: user33@example.com + num_repos: 0 + num_followers: 1 + num_following: 0 + is_active: true diff --git a/models/user/user.go b/models/user/user.go index f1df335eb..32484a487 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -1267,7 +1267,7 @@ func isUserVisibleToViewerCond(viewer *User) builder.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 { + if viewer != nil && (viewer.IsAdmin || viewer.ID == u.ID) { return true } @@ -1306,7 +1306,7 @@ func IsUserVisibleToViewer(ctx context.Context, u, viewer *User) bool { return false } - if count < 0 { + if count == 0 { // No common organization return false } diff --git a/models/user/user_test.go b/models/user/user_test.go index 940382cda..848c978a9 100644 --- a/models/user/user_test.go +++ b/models/user/user_test.go @@ -400,3 +400,56 @@ func TestUnfollowUser(t *testing.T) { unittest.CheckConsistencyFor(t, &user_model.User{}) } + +func TestIsUserVisibleToViewer(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) // admin, public + user4 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) // normal, public + user20 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 20}) // public, same team as user31 + user29 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 29}) // public, is restricted + user31 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 31}) // private, same team as user20 + user33 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 33}) // limited, follows 31 + + test := func(u, viewer *user_model.User, expected bool) { + name := func(u *user_model.User) string { + if u == nil { + return "" + } + return u.Name + } + assert.Equal(t, expected, user_model.IsUserVisibleToViewer(db.DefaultContext, u, viewer), "user %v should be visible to viewer %v: %v", name(u), name(viewer), expected) + } + + // admin viewer + test(user1, user1, true) + test(user20, user1, true) + test(user31, user1, true) + test(user33, user1, true) + + // non admin viewer + test(user4, user4, true) + test(user20, user4, true) + test(user31, user4, false) + test(user33, user4, true) + test(user4, nil, true) + + // public user + test(user4, user20, true) + test(user4, user31, true) + test(user4, user33, true) + + // limited user + test(user33, user33, true) + test(user33, user4, true) + test(user33, user29, false) + test(user33, nil, false) + + // private user + test(user31, user31, true) + test(user31, user4, false) + test(user31, user20, true) + test(user31, user29, false) + test(user31, user33, true) + test(user31, nil, false) +} diff --git a/tests/integration/api_nodeinfo_test.go b/tests/integration/api_nodeinfo_test.go index 76f9105a5..2446acec9 100644 --- a/tests/integration/api_nodeinfo_test.go +++ b/tests/integration/api_nodeinfo_test.go @@ -32,7 +32,7 @@ func TestNodeinfo(t *testing.T) { DecodeJSON(t, resp, &nodeinfo) assert.True(t, nodeinfo.OpenRegistrations) assert.Equal(t, "gitea", nodeinfo.Software.Name) - assert.Equal(t, 23, nodeinfo.Usage.Users.Total) + assert.Equal(t, 24, nodeinfo.Usage.Users.Total) assert.Equal(t, 17, nodeinfo.Usage.LocalPosts) assert.Equal(t, 2, nodeinfo.Usage.LocalComments) })