From 3039ef90f221f40b62e6896041d0d1ab13ea384a Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 13 Nov 2023 18:21:37 +0100 Subject: [PATCH 1/8] dont leak users via rss --- routers/web/user/home.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/routers/web/user/home.go b/routers/web/user/home.go index 8b9a4cd224..f9dc93dc7d 100644 --- a/routers/web/user/home.go +++ b/routers/web/user/home.go @@ -822,6 +822,11 @@ func UsernameSubRoute(ctx *context.Context) { reloadParam := func(suffix string) (success bool) { ctx.SetParams("username", strings.TrimSuffix(username, suffix)) context_service.UserAssignmentWeb()(ctx) + // check view permissions + if ctx.ContextUser.IsIndividual() && !user_model.IsUserVisibleToViewer(ctx, ctx.ContextUser, ctx.Doer) { + ctx.NotFound("user", fmt.Errorf(ctx.ContextUser.Name)) + return false + } return !ctx.Written() } switch { From c1517bdfa417b9efac3ddceece8e44800020d3c6 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 13 Nov 2023 18:37:30 +0100 Subject: [PATCH 2/8] make private users who follow you visible --- models/user/search.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/models/user/search.go b/models/user/search.go index 0fa278c257..98d78e5298 100644 --- a/models/user/search.go +++ b/models/user/search.go @@ -157,6 +157,10 @@ func BuildCanSeeUserCondition(actor *User) builder.Cond { if !actor.IsRestricted { // Not-Restricted users can see public and limited users/organizations cond = cond.Or(builder.In("`user`.visibility", structs.VisibleTypePublic, structs.VisibleTypeLimited)) + // or private users who do follow them + cond = cond.Or(builder.Eq{ + "`user`.visibility": structs.VisibleTypePrivate, + "`user`.id": builder.Select("follow.user_id").From("follow").Where(builder.Eq{"follow.follow_id": actor.ID})}) } // Don't forget about self return cond.Or(builder.Eq{"`user`.id": actor.ID}) From 1a378f47d7ae5df2681a4c1858a91fc4f0e4d471 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 13 Nov 2023 19:47:27 +0100 Subject: [PATCH 3/8] add test --- models/user/search_test.go | 66 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 models/user/search_test.go diff --git a/models/user/search_test.go b/models/user/search_test.go new file mode 100644 index 0000000000..676ed1a04d --- /dev/null +++ b/models/user/search_test.go @@ -0,0 +1,66 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package user_test + +import ( + "testing" + + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/models/user" + + "github.com/stretchr/testify/assert" + "xorm.io/builder" +) + +func TestBuildCanSeeUserCondition(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + getIDs := func(cond builder.Cond) (ids []int64) { + db.GetEngine(db.DefaultContext).Select("id").Table(`user`). + Where(builder.Eq{"is_active": true}.And(cond)).Asc("id").Find(&ids) + return ids + } + + getUser := func(t *testing.T, id int64) *user.User { + user, err := user.GetUserByID(db.DefaultContext, id) + assert.NoError(t, err) + if !assert.NotNil(t, user) { + t.FailNow() + } + return user + } + + // no login + cond := user.BuildCanSeeUserCondition(nil) + assert.NotNil(t, cond) + ids := getIDs(cond) + assert.Len(t, ids, 24) + assert.EqualValues(t, []int64([]int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 24, 27, 28, 29, 30, 32, 34}), ids) + + // normal user + cond = user.BuildCanSeeUserCondition(getUser(t, 5)) + ids = getIDs(cond) + assert.Len(t, ids, 28) + assert.EqualValues(t, []int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 27, 28, 29, 30, 32, 33, 34, 36}, ids) + + // admin + cond = user.BuildCanSeeUserCondition(getUser(t, 1)) + assert.Nil(t, cond) + ids = getIDs(cond) + assert.Len(t, ids, 30) + assert.EqualValues(t, []int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36}, ids) + + // private user + cond = user.BuildCanSeeUserCondition(getUser(t, 31)) + ids = getIDs(cond) + assert.Len(t, ids, 28) + assert.EqualValues(t, []int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 24, 27, 28, 29, 30, 31, 32, 33, 34, 36}, ids) + + // restricted user + cond = user.BuildCanSeeUserCondition(getUser(t, 29)) + ids = getIDs(cond) + assert.Len(t, ids, 2) + assert.EqualValues(t, []int64{17, 29}, ids) +} From 0c655c4ff072ee842e9c4787c2e278ec84a04acf Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 13 Nov 2023 19:51:16 +0100 Subject: [PATCH 4/8] add testcase for fix --- models/user/search_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/models/user/search_test.go b/models/user/search_test.go index 676ed1a04d..fda12a3ca9 100644 --- a/models/user/search_test.go +++ b/models/user/search_test.go @@ -58,6 +58,12 @@ func TestBuildCanSeeUserCondition(t *testing.T) { assert.Len(t, ids, 28) assert.EqualValues(t, []int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 24, 27, 28, 29, 30, 31, 32, 33, 34, 36}, ids) + // limited user who is followed by private user + cond = user.BuildCanSeeUserCondition(getUser(t, 33)) + ids = getIDs(cond) + assert.Len(t, ids, 28) + assert.EqualValues(t, []int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 24, 27, 28, 29, 30, 31, 32, 33, 34, 36}, ids) + // restricted user cond = user.BuildCanSeeUserCondition(getUser(t, 29)) ids = getIDs(cond) From 5bc3b8655ca2207d26008f0a1000b8c550105bac Mon Sep 17 00:00:00 2001 From: "m.huber" Date: Mon, 13 Nov 2023 20:06:14 +0100 Subject: [PATCH 5/8] restricted users only see repos in orgs witch there team was assigned to --- models/repo/repo_list.go | 10 +++++----- models/user/search.go | 3 ++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/models/repo/repo_list.go b/models/repo/repo_list.go index 1668c23c77..533ca5251f 100644 --- a/models/repo/repo_list.go +++ b/models/repo/repo_list.go @@ -652,12 +652,12 @@ func AccessibleRepositoryCondition(user *user_model.User, unitType unit.Type) bu userOrgTeamUnitRepoCond("`repository`.id", user.ID, unitType), ) } - cond = cond.Or( - // 4. Repositories that we directly own - builder.Eq{"`repository`.owner_id": user.ID}, + // 4. Repositories that we directly own + cond = cond.Or(builder.Eq{"`repository`.owner_id": user.ID}) + if !user.IsRestricted { // 5. Be able to see all public repos in private organizations that we are an org_user of - userOrgPublicRepoCond(user.ID), - ) + cond = cond.Or(userOrgPublicRepoCond(user.ID)) + } } return cond diff --git a/models/user/search.go b/models/user/search.go index 98d78e5298..59143d746e 100644 --- a/models/user/search.go +++ b/models/user/search.go @@ -160,7 +160,8 @@ func BuildCanSeeUserCondition(actor *User) builder.Cond { // or private users who do follow them cond = cond.Or(builder.Eq{ "`user`.visibility": structs.VisibleTypePrivate, - "`user`.id": builder.Select("follow.user_id").From("follow").Where(builder.Eq{"follow.follow_id": actor.ID})}) + "`user`.id": builder.Select("follow.user_id").From("follow").Where(builder.Eq{"follow.follow_id": actor.ID}), + }) } // Don't forget about self return cond.Or(builder.Eq{"`user`.id": actor.ID}) From 4bc1ce0a1d531696235a3f5755944064252ec0c9 Mon Sep 17 00:00:00 2001 From: "m.huber" Date: Mon, 13 Nov 2023 20:07:05 +0100 Subject: [PATCH 6/8] clean unrelated --- routers/web/user/home.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/routers/web/user/home.go b/routers/web/user/home.go index f9dc93dc7d..8b9a4cd224 100644 --- a/routers/web/user/home.go +++ b/routers/web/user/home.go @@ -822,11 +822,6 @@ func UsernameSubRoute(ctx *context.Context) { reloadParam := func(suffix string) (success bool) { ctx.SetParams("username", strings.TrimSuffix(username, suffix)) context_service.UserAssignmentWeb()(ctx) - // check view permissions - if ctx.ContextUser.IsIndividual() && !user_model.IsUserVisibleToViewer(ctx, ctx.ContextUser, ctx.Doer) { - ctx.NotFound("user", fmt.Errorf(ctx.ContextUser.Name)) - return false - } return !ctx.Written() } switch { From 499d1470551a9903a7122f8ea25bfd93034ca99e Mon Sep 17 00:00:00 2001 From: "m.huber" Date: Mon, 13 Nov 2023 20:09:43 +0100 Subject: [PATCH 7/8] split --- models/repo/repo_list.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/models/repo/repo_list.go b/models/repo/repo_list.go index 533ca5251f..1668c23c77 100644 --- a/models/repo/repo_list.go +++ b/models/repo/repo_list.go @@ -652,12 +652,12 @@ func AccessibleRepositoryCondition(user *user_model.User, unitType unit.Type) bu userOrgTeamUnitRepoCond("`repository`.id", user.ID, unitType), ) } - // 4. Repositories that we directly own - cond = cond.Or(builder.Eq{"`repository`.owner_id": user.ID}) - if !user.IsRestricted { + cond = cond.Or( + // 4. Repositories that we directly own + builder.Eq{"`repository`.owner_id": user.ID}, // 5. Be able to see all public repos in private organizations that we are an org_user of - cond = cond.Or(userOrgPublicRepoCond(user.ID)) - } + userOrgPublicRepoCond(user.ID), + ) } return cond From 713b4dbfcb5cec00b2309314b2995814db89203f Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 13 Nov 2023 20:23:26 +0100 Subject: [PATCH 8/8] fix-lint --- models/user/search_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/user/search_test.go b/models/user/search_test.go index fda12a3ca9..6699f88cde 100644 --- a/models/user/search_test.go +++ b/models/user/search_test.go @@ -37,7 +37,7 @@ func TestBuildCanSeeUserCondition(t *testing.T) { assert.NotNil(t, cond) ids := getIDs(cond) assert.Len(t, ids, 24) - assert.EqualValues(t, []int64([]int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 24, 27, 28, 29, 30, 32, 34}), ids) + assert.EqualValues(t, []int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 24, 27, 28, 29, 30, 32, 34}, ids) // normal user cond = user.BuildCanSeeUserCondition(getUser(t, 5))