From 949d3fe056af7b302bbe603cb1cd57ccf257452a Mon Sep 17 00:00:00 2001 From: Gusted Date: Sat, 21 Jan 2023 12:08:31 +0100 Subject: [PATCH] Don't return duplicated users who can create org repo (##22560) - Backport of #22560 - Currently the function `GetUsersWhoCanCreateOrgRepo` uses a query that is able to have duplicated users in the result, this is can happen under the condition that a user is in team that either is the owner team or has permission to create organization repositories. - Add test code to simulate the above condition for user 3, [`TestGetUsersWhoCanCreateOrgRepo`](https://github.com/go-gitea/gitea/blob/a1fcb1cfb84fd6b36c8fe9fd56588119fa4377bc/models/organization/org_test.go#L435) is the test function that tests for this. - The fix is quite trivial, which is adding `DISTINCT` keyword to `` `user`.id ``. --- models/fixtures/team.yml | 11 +++++++++++ models/fixtures/team_user.yml | 6 ++++++ models/fixtures/user.yml | 2 +- models/organization/org.go | 5 ++++- models/organization/org_test.go | 7 ++++--- 5 files changed, 26 insertions(+), 5 deletions(-) diff --git a/models/fixtures/team.yml b/models/fixtures/team.yml index ea47a33f1c4db..dd434d78a980c 100644 --- a/models/fixtures/team.yml +++ b/models/fixtures/team.yml @@ -140,3 +140,14 @@ num_members: 1 includes_all_repositories: false can_create_org_repo: false + +- + id: 14 + org_id: 3 + lower_name: teamcreaterepo + name: teamCreateRepo + authorize: 2 # write + num_repos: 0 + num_members: 1 + includes_all_repositories: false + can_create_org_repo: true diff --git a/models/fixtures/team_user.yml b/models/fixtures/team_user.yml index 845741effddf8..de4f29d977cbb 100644 --- a/models/fixtures/team_user.yml +++ b/models/fixtures/team_user.yml @@ -93,3 +93,9 @@ org_id: 19 team_id: 6 uid: 31 + +- + id: 17 + org_id: 3 + team_id: 14 + uid: 2 diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml index 0e3348e146a82..36bc049ca43b4 100644 --- a/models/fixtures/user.yml +++ b/models/fixtures/user.yml @@ -104,7 +104,7 @@ num_following: 0 num_stars: 0 num_repos: 3 - num_teams: 4 + num_teams: 5 num_members: 3 visibility: 0 repo_admin_change_team_access: false diff --git a/models/organization/org.go b/models/organization/org.go index 217fa623bf8f0..fbf986dc14215 100644 --- a/models/organization/org.go +++ b/models/organization/org.go @@ -402,7 +402,10 @@ func GetUsersWhoCanCreateOrgRepo(ctx context.Context, orgID int64) ([]*user_mode Join("INNER", "`team_user`", "`team_user`.uid=`user`.id"). Join("INNER", "`team`", "`team`.id=`team_user`.team_id"). Where(builder.Eq{"team.can_create_org_repo": true}.Or(builder.Eq{"team.authorize": perm.AccessModeOwner})). - And("team_user.org_id = ?", orgID).Asc("`user`.name").Find(&users) + And("team_user.org_id = ?", orgID). + Asc("`user`.name"). + Distinct("`user`.id"). + Find(&users) } // SearchOrganizationsOptions options to filter organizations diff --git a/models/organization/org_test.go b/models/organization/org_test.go index 0fba6e25925cc..0b9c611b61b1a 100644 --- a/models/organization/org_test.go +++ b/models/organization/org_test.go @@ -92,11 +92,12 @@ func TestUser_GetTeams(t *testing.T) { org := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 3}) teams, err := org.LoadTeams() assert.NoError(t, err) - if assert.Len(t, teams, 4) { + if assert.Len(t, teams, 5) { assert.Equal(t, int64(1), teams[0].ID) assert.Equal(t, int64(2), teams[1].ID) assert.Equal(t, int64(12), teams[2].ID) - assert.Equal(t, int64(7), teams[3].ID) + assert.Equal(t, int64(14), teams[3].ID) + assert.Equal(t, int64(7), teams[4].ID) } } @@ -293,7 +294,7 @@ func TestUser_GetUserTeamIDs(t *testing.T) { assert.NoError(t, err) assert.Equal(t, expected, teamIDs) } - testSuccess(2, []int64{1, 2}) + testSuccess(2, []int64{1, 2, 14}) testSuccess(4, []int64{2}) testSuccess(unittest.NonexistentID, []int64{}) }