Skip to content

Commit f604144

Browse files
wxiaoguangwxiaoguang
authored
Refactor FindOrgOptions to use enum instead of bool, fix membership visibility (#34629)
1 parent 1fe652c commit f604144

File tree

12 files changed

+74
-51
lines changed

12 files changed

+74
-51
lines changed

‎models/activities/statistic.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"code.gitea.io/gitea/models/webhook"
2020
"code.gitea.io/gitea/modules/optional"
2121
"code.gitea.io/gitea/modules/setting"
22+
"code.gitea.io/gitea/modules/structs"
2223
)
2324

2425
// Statistic contains the database statistics
@@ -68,7 +69,7 @@ func GetStatistic(ctx context.Context) (stats Statistic) {
6869
}
6970
stats.Counter.UsersNotActive = user_model.CountUsers(ctx, &usersNotActiveOpts)
7071

71-
stats.Counter.Org, _ = db.Count[organization.Organization](ctx, organization.FindOrgOptions{IncludePrivate: true})
72+
stats.Counter.Org, _ = db.Count[organization.Organization](ctx, organization.FindOrgOptions{IncludeVisibility: structs.VisibleTypePrivate})
7273
stats.Counter.PublicKey, _ = e.Count(new(asymkey_model.PublicKey))
7374
stats.Counter.Repo, _ = repo_model.CountRepositories(ctx, repo_model.CountRepositoryOptions{})
7475
stats.Counter.Watch, _ = e.Count(new(repo_model.Watch))

‎models/organization/org_list.go

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ type SearchOrganizationsOptions struct {
5050
// FindOrgOptions finds orgs options
5151
type FindOrgOptions struct {
5252
db.ListOptions
53-
UserID int64
54-
IncludePrivate bool
53+
UserID int64
54+
IncludeVisibility structs.VisibleType
5555
}
5656

5757
func queryUserOrgIDs(userID int64, includePrivate bool) *builder.Builder {
@@ -65,18 +65,27 @@ func queryUserOrgIDs(userID int64, includePrivate bool) *builder.Builder {
6565
func (opts FindOrgOptions) ToConds() builder.Cond {
6666
var cond builder.Cond = builder.Eq{"`user`.`type`": user_model.UserTypeOrganization}
6767
if opts.UserID > 0 {
68-
cond = cond.And(builder.In("`user`.`id`", queryUserOrgIDs(opts.UserID, opts.IncludePrivate)))
69-
}
70-
if !opts.IncludePrivate {
71-
cond = cond.And(builder.Eq{"`user`.visibility": structs.VisibleTypePublic})
68+
cond = cond.And(builder.In("`user`.`id`", queryUserOrgIDs(opts.UserID, opts.IncludeVisibility == structs.VisibleTypePrivate)))
7269
}
70+
// public=0, limited=1, private=2
71+
cond = cond.And(builder.Lte{"`user`.visibility": opts.IncludeVisibility})
7372
return cond
7473
}
7574

7675
func (opts FindOrgOptions) ToOrders() string {
7776
return "`user`.lower_name ASC"
7877
}
7978

79+
func DoerViewOtherVisibility(doer, other *user_model.User) structs.VisibleType {
80+
if doer == nil || other == nil {
81+
return structs.VisibleTypePublic
82+
}
83+
if doer.IsAdmin || doer.ID == other.ID {
84+
return structs.VisibleTypePrivate
85+
}
86+
return structs.VisibleTypeLimited
87+
}
88+
8089
// GetOrgsCanCreateRepoByUserID returns a list of organizations where given user ID
8190
// are allowed to create repos.
8291
func GetOrgsCanCreateRepoByUserID(ctx context.Context, userID int64) ([]*Organization, error) {

‎models/organization/org_list_test.go

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,48 +10,53 @@ import (
1010
"code.gitea.io/gitea/models/organization"
1111
"code.gitea.io/gitea/models/unittest"
1212
user_model "code.gitea.io/gitea/models/user"
13+
"code.gitea.io/gitea/modules/structs"
1314

1415
"github.com/stretchr/testify/assert"
1516
)
1617

17-
func TestCountOrganizations(t *testing.T) {
18+
func TestOrgList(t *testing.T) {
1819
assert.NoError(t, unittest.PrepareTestDatabase())
20+
t.Run("CountOrganizations", testCountOrganizations)
21+
t.Run("FindOrgs", testFindOrgs)
22+
t.Run("GetUserOrgsList", testGetUserOrgsList)
23+
t.Run("LoadOrgListTeams", testLoadOrgListTeams)
24+
t.Run("DoerViewOtherVisibility", testDoerViewOtherVisibility)
25+
}
26+
27+
func testCountOrganizations(t *testing.T) {
1928
expected, err := db.GetEngine(db.DefaultContext).Where("type=?", user_model.UserTypeOrganization).Count(&organization.Organization{})
2029
assert.NoError(t, err)
21-
cnt, err := db.Count[organization.Organization](db.DefaultContext, organization.FindOrgOptions{IncludePrivate: true})
30+
cnt, err := db.Count[organization.Organization](db.DefaultContext, organization.FindOrgOptions{IncludeVisibility: structs.VisibleTypePrivate})
2231
assert.NoError(t, err)
2332
assert.Equal(t, expected, cnt)
2433
}
2534

26-
func TestFindOrgs(t *testing.T) {
27-
assert.NoError(t, unittest.PrepareTestDatabase())
28-
35+
func testFindOrgs(t *testing.T) {
2936
orgs, err := db.Find[organization.Organization](db.DefaultContext, organization.FindOrgOptions{
30-
UserID: 4,
31-
IncludePrivate: true,
37+
UserID: 4,
38+
IncludeVisibility: structs.VisibleTypePrivate,
3239
})
3340
assert.NoError(t, err)
3441
if assert.Len(t, orgs, 1) {
3542
assert.EqualValues(t, 3, orgs[0].ID)
3643
}
3744

3845
orgs, err = db.Find[organization.Organization](db.DefaultContext, organization.FindOrgOptions{
39-
UserID: 4,
40-
IncludePrivate: false,
46+
UserID: 4,
4147
})
4248
assert.NoError(t, err)
4349
assert.Empty(t, orgs)
4450

4551
total, err := db.Count[organization.Organization](db.DefaultContext, organization.FindOrgOptions{
46-
UserID: 4,
47-
IncludePrivate: true,
52+
UserID: 4,
53+
IncludeVisibility: structs.VisibleTypePrivate,
4854
})
4955
assert.NoError(t, err)
5056
assert.EqualValues(t, 1, total)
5157
}
5258

53-
func TestGetUserOrgsList(t *testing.T) {
54-
assert.NoError(t, unittest.PrepareTestDatabase())
59+
func testGetUserOrgsList(t *testing.T) {
5560
orgs, err := organization.GetUserOrgsList(db.DefaultContext, &user_model.User{ID: 4})
5661
assert.NoError(t, err)
5762
if assert.Len(t, orgs, 1) {
@@ -61,8 +66,7 @@ func TestGetUserOrgsList(t *testing.T) {
6166
}
6267
}
6368

64-
func TestLoadOrgListTeams(t *testing.T) {
65-
assert.NoError(t, unittest.PrepareTestDatabase())
69+
func testLoadOrgListTeams(t *testing.T) {
6670
orgs, err := organization.GetUserOrgsList(db.DefaultContext, &user_model.User{ID: 4})
6771
assert.NoError(t, err)
6872
assert.Len(t, orgs, 1)
@@ -71,3 +75,10 @@ func TestLoadOrgListTeams(t *testing.T) {
7175
assert.Len(t, teamsMap, 1)
7276
assert.Len(t, teamsMap[3], 5)
7377
}
78+
79+
func testDoerViewOtherVisibility(t *testing.T) {
80+
assert.Equal(t, structs.VisibleTypePublic, organization.DoerViewOtherVisibility(nil, nil))
81+
assert.Equal(t, structs.VisibleTypeLimited, organization.DoerViewOtherVisibility(&user_model.User{ID: 1}, &user_model.User{ID: 2}))
82+
assert.Equal(t, structs.VisibleTypePrivate, organization.DoerViewOtherVisibility(&user_model.User{ID: 1}, &user_model.User{ID: 1}))
83+
assert.Equal(t, structs.VisibleTypePrivate, organization.DoerViewOtherVisibility(&user_model.User{ID: 1, IsAdmin: true}, &user_model.User{ID: 2}))
84+
}

‎routers/api/v1/org/org.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,10 @@ import (
2626

2727
func listUserOrgs(ctx *context.APIContext, u *user_model.User) {
2828
listOptions := utils.GetListOptions(ctx)
29-
showPrivate := ctx.IsSigned && (ctx.Doer.IsAdmin || ctx.Doer.ID == u.ID)
30-
3129
opts := organization.FindOrgOptions{
32-
ListOptions: listOptions,
33-
UserID: u.ID,
34-
IncludePrivate: showPrivate,
30+
ListOptions: listOptions,
31+
UserID: u.ID,
32+
IncludeVisibility: organization.DoerViewOtherVisibility(ctx.Doer, u),
3533
}
3634
orgs, maxResults, err := db.FindAndCount[organization.Organization](ctx, opts)
3735
if err != nil {

‎routers/web/admin/users.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"code.gitea.io/gitea/modules/log"
2222
"code.gitea.io/gitea/modules/optional"
2323
"code.gitea.io/gitea/modules/setting"
24+
"code.gitea.io/gitea/modules/structs"
2425
"code.gitea.io/gitea/modules/templates"
2526
"code.gitea.io/gitea/modules/web"
2627
"code.gitea.io/gitea/routers/web/explore"
@@ -292,9 +293,9 @@ func ViewUser(ctx *context.Context) {
292293
ctx.Data["EmailsTotal"] = len(emails)
293294

294295
orgs, err := db.Find[org_model.Organization](ctx, org_model.FindOrgOptions{
295-
ListOptions: db.ListOptionsAll,
296-
UserID: u.ID,
297-
IncludePrivate: true,
296+
ListOptions: db.ListOptionsAll,
297+
UserID: u.ID,
298+
IncludeVisibility: structs.VisibleTypePrivate,
298299
})
299300
if err != nil {
300301
ctx.ServerError("FindOrgs", err)

‎routers/web/shared/user/header.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,13 +47,12 @@ func prepareContextForProfileBigAvatar(ctx *context.Context) {
4747
ctx.Data["RenderedDescription"] = content
4848
}
4949

50-
showPrivate := ctx.IsSigned && (ctx.Doer.IsAdmin || ctx.Doer.ID == ctx.ContextUser.ID)
5150
orgs, err := db.Find[organization.Organization](ctx, organization.FindOrgOptions{
52-
UserID: ctx.ContextUser.ID,
53-
IncludePrivate: showPrivate,
51+
UserID: ctx.ContextUser.ID,
52+
IncludeVisibility: organization.DoerViewOtherVisibility(ctx.Doer, ctx.ContextUser),
5453
ListOptions: db.ListOptions{
5554
Page: 1,
56-
// query one more results (without a separate counting) to see whether we need to add the "show more orgs" link
55+
// query one more result (without a separate counting) to see whether we need to add the "show more orgs" link
5756
PageSize: setting.UI.User.OrgPagingNum + 1,
5857
},
5958
})

‎routers/web/user/profile.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,7 @@ func userProfile(ctx *context.Context) {
7676

7777
profileDbRepo, profileReadmeBlob := shared_user.FindOwnerProfileReadme(ctx, ctx.Doer)
7878

79-
showPrivate := ctx.IsSigned && (ctx.Doer.IsAdmin || ctx.Doer.ID == ctx.ContextUser.ID)
80-
prepareUserProfileTabData(ctx, showPrivate, profileDbRepo, profileReadmeBlob)
79+
prepareUserProfileTabData(ctx, profileDbRepo, profileReadmeBlob)
8180

8281
// prepare the user nav header data after "prepareUserProfileTabData" to avoid re-querying the NumFollowers & NumFollowing
8382
// because ctx.Data["NumFollowers"] and "NumFollowing" logic duplicates in both of them
@@ -90,7 +89,7 @@ func userProfile(ctx *context.Context) {
9089
ctx.HTML(http.StatusOK, tplProfile)
9190
}
9291

93-
func prepareUserProfileTabData(ctx *context.Context, showPrivate bool, profileDbRepo *repo_model.Repository, profileReadme *git.Blob) {
92+
func prepareUserProfileTabData(ctx *context.Context, profileDbRepo *repo_model.Repository, profileReadme *git.Blob) {
9493
// if there is a profile readme, default to "overview" page, otherwise, default to "repositories" page
9594
// if there is not a profile readme, the overview tab should be treated as the repositories tab
9695
tab := ctx.FormString("tab")
@@ -175,6 +174,7 @@ func prepareUserProfileTabData(ctx *context.Context, showPrivate bool, profileDb
175174
case "activity":
176175
date := ctx.FormString("date")
177176
pagingNum = setting.UI.FeedPagingNum
177+
showPrivate := ctx.IsSigned && (ctx.Doer.IsAdmin || ctx.Doer.ID == ctx.ContextUser.ID)
178178
items, count, err := feed_service.GetFeeds(ctx, activities_model.GetFeedsOptions{
179179
RequestedUser: ctx.ContextUser,
180180
Actor: ctx.Doer,
@@ -265,8 +265,8 @@ func prepareUserProfileTabData(ctx *context.Context, showPrivate bool, profileDb
265265
}
266266
case "organizations":
267267
orgs, count, err := db.FindAndCount[organization.Organization](ctx, organization.FindOrgOptions{
268-
UserID: ctx.ContextUser.ID,
269-
IncludePrivate: showPrivate,
268+
UserID: ctx.ContextUser.ID,
269+
IncludeVisibility: organization.DoerViewOtherVisibility(ctx.Doer, ctx.ContextUser),
270270
ListOptions: db.ListOptions{
271271
Page: page,
272272
PageSize: pagingNum,

‎routers/web/user/setting/profile.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"code.gitea.io/gitea/modules/log"
2323
"code.gitea.io/gitea/modules/optional"
2424
"code.gitea.io/gitea/modules/setting"
25+
"code.gitea.io/gitea/modules/structs"
2526
"code.gitea.io/gitea/modules/templates"
2627
"code.gitea.io/gitea/modules/translation"
2728
"code.gitea.io/gitea/modules/typesniffer"
@@ -206,8 +207,8 @@ func Organization(ctx *context.Context) {
206207
PageSize: setting.UI.Admin.UserPagingNum,
207208
Page: ctx.FormInt("page"),
208209
},
209-
UserID: ctx.Doer.ID,
210-
IncludePrivate: ctx.IsSigned,
210+
UserID: ctx.Doer.ID,
211+
IncludeVisibility: structs.VisibleTypePrivate,
211212
}
212213

213214
if opts.Page <= 0 {

‎services/oauth2_provider/access_token.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@ import (
1616
user_model "code.gitea.io/gitea/models/user"
1717
"code.gitea.io/gitea/modules/log"
1818
"code.gitea.io/gitea/modules/setting"
19+
api "code.gitea.io/gitea/modules/structs"
1920
"code.gitea.io/gitea/modules/timeutil"
21+
"code.gitea.io/gitea/modules/util"
2022

2123
"github.com/golang-jwt/jwt/v5"
2224
)
@@ -231,12 +233,11 @@ func NewAccessTokenResponse(ctx context.Context, grant *auth.OAuth2Grant, server
231233
}, nil
232234
}
233235

234-
// returns a list of "org" and "org:team" strings,
235-
// that the given user is a part of.
236+
// GetOAuthGroupsForUser returns a list of "org" and "org:team" strings, that the given user is a part of.
236237
func GetOAuthGroupsForUser(ctx context.Context, user *user_model.User, onlyPublicGroups bool) ([]string, error) {
237238
orgs, err := db.Find[org_model.Organization](ctx, org_model.FindOrgOptions{
238-
UserID: user.ID,
239-
IncludePrivate: !onlyPublicGroups,
239+
UserID: user.ID,
240+
IncludeVisibility: util.Iif(onlyPublicGroups, api.VisibleTypePublic, api.VisibleTypePrivate),
240241
})
241242
if err != nil {
242243
return nil, fmt.Errorf("GetUserOrgList: %w", err)

‎services/user/user.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"code.gitea.io/gitea/modules/log"
2121
"code.gitea.io/gitea/modules/setting"
2222
"code.gitea.io/gitea/modules/storage"
23+
"code.gitea.io/gitea/modules/structs"
2324
"code.gitea.io/gitea/modules/util"
2425
"code.gitea.io/gitea/services/agit"
2526
asymkey_service "code.gitea.io/gitea/services/asymkey"
@@ -177,8 +178,8 @@ func DeleteUser(ctx context.Context, u *user_model.User, purge bool) error {
177178
PageSize: repo_model.RepositoryListDefaultPageSize,
178179
Page: 1,
179180
},
180-
UserID: u.ID,
181-
IncludePrivate: true,
181+
UserID: u.ID,
182+
IncludeVisibility: structs.VisibleTypePrivate,
182183
})
183184
if err != nil {
184185
return fmt.Errorf("unable to find org list for %s[%d]. Error: %w", u.Name, u.ID, err)

‎tests/integration/auth_ldap_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"code.gitea.io/gitea/models/unittest"
1616
user_model "code.gitea.io/gitea/models/user"
1717
"code.gitea.io/gitea/modules/optional"
18+
"code.gitea.io/gitea/modules/structs"
1819
"code.gitea.io/gitea/modules/test"
1920
"code.gitea.io/gitea/modules/translation"
2021
"code.gitea.io/gitea/modules/util"
@@ -437,8 +438,8 @@ func TestLDAPGroupTeamSyncAddMember(t *testing.T) {
437438
Name: gitLDAPUser.UserName,
438439
})
439440
usersOrgs, err := db.Find[organization.Organization](db.DefaultContext, organization.FindOrgOptions{
440-
UserID: user.ID,
441-
IncludePrivate: true,
441+
UserID: user.ID,
442+
IncludeVisibility: structs.VisibleTypePrivate,
442443
})
443444
assert.NoError(t, err)
444445
allOrgTeams, err := organization.GetUserOrgTeams(db.DefaultContext, org.ID, user.ID)

‎tests/integration/org_count_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,8 @@ func doCheckOrgCounts(username string, orgCounts map[string]int, strict bool, ca
120120
})
121121

122122
orgs, err := db.Find[organization.Organization](db.DefaultContext, organization.FindOrgOptions{
123-
UserID: user.ID,
124-
IncludePrivate: true,
123+
UserID: user.ID,
124+
IncludeVisibility: api.VisibleTypePrivate,
125125
})
126126
assert.NoError(t, err)
127127

0 commit comments

Comments
 (0)