From 72ee7f3b006f8cb5c1031a5607c87618e0609242 Mon Sep 17 00:00:00 2001 From: Gusted Date: Wed, 19 Mar 2025 16:45:42 +0000 Subject: [PATCH] fix: consider issues in repository accessible via `access` table (#7270) - Consider the following scenario: a private repository in an organization with a team that has no specific access to that repository. Members of that team are still able to visit the repository because of entries in the `access` table. - Consider this specific scenario for the gathering of issues for project tables. - Unit test added - Resolves forgejo/forgejo#7217 - Ref: forgejo/forgejo#6843 Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7270 Reviewed-by: Earl Warren Reviewed-by: 0ko <0ko@noreply.codeberg.org> Co-authored-by: Gusted Co-committed-by: Gusted --- .../TestPrivateRepoProjects/access.yml | 5 ++ .../TestPrivateRepoProjects/project.yml | 11 ++++ .../TestPrivateRepoProjects/project_board.yml | 8 +++ .../TestPrivateRepoProjects/project_issue.yml | 11 ++++ models/issues/issue_project_test.go | 54 +++++++++++++++++++ models/issues/issue_search.go | 3 ++ 6 files changed, 92 insertions(+) create mode 100644 models/fixtures/TestPrivateRepoProjects/access.yml create mode 100644 models/fixtures/TestPrivateRepoProjects/project.yml create mode 100644 models/fixtures/TestPrivateRepoProjects/project_board.yml create mode 100644 models/fixtures/TestPrivateRepoProjects/project_issue.yml diff --git a/models/fixtures/TestPrivateRepoProjects/access.yml b/models/fixtures/TestPrivateRepoProjects/access.yml new file mode 100644 index 0000000000..4149e34b0b --- /dev/null +++ b/models/fixtures/TestPrivateRepoProjects/access.yml @@ -0,0 +1,5 @@ +- + id: 1001 + user_id: 29 + repo_id: 3 + mode: 1 diff --git a/models/fixtures/TestPrivateRepoProjects/project.yml b/models/fixtures/TestPrivateRepoProjects/project.yml new file mode 100644 index 0000000000..f66e4c8676 --- /dev/null +++ b/models/fixtures/TestPrivateRepoProjects/project.yml @@ -0,0 +1,11 @@ +- + id: 1001 + title: Org project that contains private issues + owner_id: 3 + repo_id: 0 + is_closed: false + creator_id: 2 + board_type: 1 + type: 3 + created_unix: 1738000000 + updated_unix: 1738000000 diff --git a/models/fixtures/TestPrivateRepoProjects/project_board.yml b/models/fixtures/TestPrivateRepoProjects/project_board.yml new file mode 100644 index 0000000000..9829cf7e27 --- /dev/null +++ b/models/fixtures/TestPrivateRepoProjects/project_board.yml @@ -0,0 +1,8 @@ +- + id: 1001 + project_id: 1001 + title: Triage + creator_id: 2 + default: true + created_unix: 1738000000 + updated_unix: 1738000000 diff --git a/models/fixtures/TestPrivateRepoProjects/project_issue.yml b/models/fixtures/TestPrivateRepoProjects/project_issue.yml new file mode 100644 index 0000000000..3e8c1dca9e --- /dev/null +++ b/models/fixtures/TestPrivateRepoProjects/project_issue.yml @@ -0,0 +1,11 @@ +- + id: 1001 + issue_id: 6 + project_id: 1001 + project_board_id: 1001 + +- + id: 1002 + issue_id: 15 + project_id: 1001 + project_board_id: 1001 diff --git a/models/issues/issue_project_test.go b/models/issues/issue_project_test.go index e28b52128e..d724922946 100644 --- a/models/issues/issue_project_test.go +++ b/models/issues/issue_project_test.go @@ -117,3 +117,57 @@ func TestPrivateIssueProjects(t *testing.T) { }) }) } + +func TestPrivateRepoProjects(t *testing.T) { + defer tests.AddFixtures("models/fixtures/TestPrivateRepoProjects/")() + require.NoError(t, unittest.PrepareTestDatabase()) + + org := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 3}) + orgProject := unittest.AssertExistsAndLoadBean(t, &project.Project{ID: 1001, OwnerID: org.ID}) + column := unittest.AssertExistsAndLoadBean(t, &project.Column{ID: 1001, ProjectID: orgProject.ID}) + + t.Run("Partial access", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + user29 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 29}) + + issueList, err := issues.LoadIssuesFromColumn(db.DefaultContext, column, user29, org, optional.None[bool]()) + require.NoError(t, err) + assert.Len(t, issueList, 1) + assert.EqualValues(t, 6, issueList[0].ID) + + issuesNum, err := issues.NumIssuesInProject(db.DefaultContext, orgProject, user29, org, optional.None[bool]()) + require.NoError(t, err) + assert.EqualValues(t, 1, issuesNum) + + issuesNum, err = issues.NumIssuesInProject(db.DefaultContext, orgProject, user29, org, optional.Some(true)) + require.NoError(t, err) + assert.EqualValues(t, 0, issuesNum) + + issuesNum, err = issues.NumIssuesInProject(db.DefaultContext, orgProject, user29, org, optional.Some(false)) + require.NoError(t, err) + assert.EqualValues(t, 1, issuesNum) + }) + + t.Run("Full access", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + issueList, err := issues.LoadIssuesFromColumn(db.DefaultContext, column, user2, org, optional.None[bool]()) + require.NoError(t, err) + assert.Len(t, issueList, 2) + assert.EqualValues(t, 15, issueList[0].ID) + assert.EqualValues(t, 6, issueList[1].ID) + + issuesNum, err := issues.NumIssuesInProject(db.DefaultContext, orgProject, user2, org, optional.None[bool]()) + require.NoError(t, err) + assert.EqualValues(t, 2, issuesNum) + + issuesNum, err = issues.NumIssuesInProject(db.DefaultContext, orgProject, user2, org, optional.Some(true)) + require.NoError(t, err) + assert.EqualValues(t, 0, issuesNum) + + issuesNum, err = issues.NumIssuesInProject(db.DefaultContext, orgProject, user2, org, optional.Some(false)) + require.NoError(t, err) + assert.EqualValues(t, 2, issuesNum) + }) +} diff --git a/models/issues/issue_search.go b/models/issues/issue_search.go index 56c219af39..6592f3708c 100644 --- a/models/issues/issue_search.go +++ b/models/issues/issue_search.go @@ -341,6 +341,9 @@ func issuePullAccessibleRepoCond(repoIDstr string, userID int64, org *organizati builder.Or( repo_model.UserOrgUnitRepoCond(repoIDstr, userID, org.ID, unitType), // team member repos repo_model.UserOrgPublicUnitRepoCond(userID, org.ID), // user org public non-member repos, TODO: check repo has issues + builder.And( + builder.In("issue.repo_id", builder.Select("id").From("repository").Where(builder.Eq{"owner_id": org.ID})), + repo_model.UserAccessRepoCond(repoIDstr, userID)), // user can access org repo in a unit independent way ), ) }