From bb77f9a91a47fffcf85962e996b329dab4e609b2 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Mon, 3 Apr 2023 00:51:17 +0000 Subject: [PATCH 1/9] fix --- modules/context/package.go | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/modules/context/package.go b/modules/context/package.go index 2a55db3a77fa..e955c26993f3 100644 --- a/modules/context/package.go +++ b/modules/context/package.go @@ -96,29 +96,20 @@ func determineAccessMode(ctx *Context) (perm.AccessMode, error) { if ctx.Package.Owner.IsOrganization() { org := organization.OrgFromUser(ctx.Package.Owner) - // 1. Get user max authorize level for the org (may be none, if user is not member of the org) - if ctx.Doer != nil { - var err error - accessMode, err = org.GetOrgUserMaxAuthorizeLevel(ctx.Doer.ID) + if ctx.Doer != nil && !ctx.Doer.IsGhost() { + // 1. If user is logined, check all team packages permissions + teams, err := organization.GetUserOrgTeams(ctx, org.ID, ctx.Doer.ID) if err != nil { return accessMode, err } - // If access mode is less than write check every team for more permissions - if accessMode < perm.AccessModeWrite { - teams, err := organization.GetUserOrgTeams(ctx, org.ID, ctx.Doer.ID) - if err != nil { - return accessMode, err - } - for _, t := range teams { - perm := t.UnitAccessMode(ctx, unit.TypePackages) - if accessMode < perm { - accessMode = perm - } + for _, t := range teams { + perm := t.UnitAccessMode(ctx, unit.TypePackages) + if accessMode < perm { + accessMode = perm } } - } - // 2. If authorize level is none, check if org is visible to user - if accessMode == perm.AccessModeNone && organization.HasOrgOrUserVisible(ctx, ctx.Package.Owner, ctx.Doer) { + } else if organization.HasOrgOrUserVisible(ctx, ctx.Package.Owner, ctx.Doer) { + // 2. If user is non-login, check if org is visible to non-login user accessMode = perm.AccessModeRead } } else { From 0615bad957e0e5c0b950ae6fa56c125d1e54136a Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Mon, 3 Apr 2023 00:52:10 +0000 Subject: [PATCH 2/9] add todo --- modules/context/package.go | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/context/package.go b/modules/context/package.go index e955c26993f3..2cb2f1efc776 100644 --- a/modules/context/package.go +++ b/modules/context/package.go @@ -92,6 +92,7 @@ func determineAccessMode(ctx *Context) (perm.AccessMode, error) { return perm.AccessModeNone, nil } + // TODO: ActionUser permission check accessMode := perm.AccessModeNone if ctx.Package.Owner.IsOrganization() { org := organization.OrgFromUser(ctx.Package.Owner) From 7cf4b570dc4c93b3e2c0a1e37ad83df1ca06aecf Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Tue, 4 Apr 2023 07:23:34 +0000 Subject: [PATCH 3/9] add test --- models/fixtures/org_user.yml | 6 ++++++ models/fixtures/team.yml | 11 +++++++++++ models/fixtures/team_unit.yml | 6 ++++++ models/fixtures/team_user.yml | 6 ++++++ tests/integration/api_packages_test.go | 10 ++++++++++ 5 files changed, 39 insertions(+) diff --git a/models/fixtures/org_user.yml b/models/fixtures/org_user.yml index d6bbdaa9da26..784949241863 100644 --- a/models/fixtures/org_user.yml +++ b/models/fixtures/org_user.yml @@ -75,3 +75,9 @@ uid: 31 org_id: 19 is_public: true + +- + id: 14 + uid: 5 + org_id: 23 + is_public: false \ No newline at end of file diff --git a/models/fixtures/team.yml b/models/fixtures/team.yml index be988b8fce12..b89f10a5e448 100644 --- a/models/fixtures/team.yml +++ b/models/fixtures/team.yml @@ -172,4 +172,15 @@ num_repos: 0 num_members: 0 includes_all_repositories: false + can_create_org_repo: true + +- + id: 17 + org_id: 23 + lower_name: team14writeauth + name: team14WriteAuth + authorize: 2 # write + num_repos: 0 + num_members: 1 + includes_all_repositories: false can_create_org_repo: true \ No newline at end of file diff --git a/models/fixtures/team_unit.yml b/models/fixtures/team_unit.yml index 2e23a6312919..ee33b1639204 100644 --- a/models/fixtures/team_unit.yml +++ b/models/fixtures/team_unit.yml @@ -268,3 +268,9 @@ team_id: 9 type: 1 # code access_mode: 1 + +- + id: 46 + team_id: 17 + type: 9 # package + access_mode: 0 \ No newline at end of file diff --git a/models/fixtures/team_user.yml b/models/fixtures/team_user.yml index de4f29d977cb..fd99bfd411b9 100644 --- a/models/fixtures/team_user.yml +++ b/models/fixtures/team_user.yml @@ -99,3 +99,9 @@ org_id: 3 team_id: 14 uid: 2 + +- + id: 18 + org_id: 23 + team_id: 17 + uid: 5 \ No newline at end of file diff --git a/tests/integration/api_packages_test.go b/tests/integration/api_packages_test.go index 4228003e2db3..74a7e3c79546 100644 --- a/tests/integration/api_packages_test.go +++ b/tests/integration/api_packages_test.go @@ -157,6 +157,7 @@ func TestPackageAccess(t *testing.T) { admin := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) inactive := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 9}) + privatedOrg := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 23}) uploadPackage := func(doer, owner *user_model.User, expectedStatus int) { url := fmt.Sprintf("/api/packages/%s/generic/test-package/1.0/file.bin", owner.Name) @@ -170,6 +171,15 @@ func TestPackageAccess(t *testing.T) { uploadPackage(inactive, user, http.StatusUnauthorized) uploadPackage(admin, inactive, http.StatusCreated) uploadPackage(admin, user, http.StatusCreated) + + // team.authorize is write, but team_unit.access_mode is none + // so the user can not upload packages or get package list + uploadPackage(user, privatedOrg, http.StatusUnauthorized) + + session := loginUser(t, user.Name) + tokenReadPackage := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadPackage) + req := NewRequest(t, "GET", fmt.Sprintf("/api/v1/packages/%s?token=%s", privatedOrg.Name, tokenReadPackage)) + MakeRequest(t, req, http.StatusForbidden) } func TestPackageQuota(t *testing.T) { From 6e3931c103d5c2c63901e849d04074405e1d9fa4 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Tue, 4 Apr 2023 08:12:32 +0000 Subject: [PATCH 4/9] fix --- models/fixtures/user.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml index ce54defacdc6..3e302dfb9af0 100644 --- a/models/fixtures/user.yml +++ b/models/fixtures/user.yml @@ -844,8 +844,8 @@ num_following: 0 num_stars: 0 num_repos: 2 - num_teams: 1 - num_members: 0 + num_teams: 2 + num_members: 1 visibility: 2 repo_admin_change_team_access: false theme: "" From 7e07bacb008a0d9eb4e262ebe733369e0409c947 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Wed, 5 Apr 2023 01:27:57 +0000 Subject: [PATCH 5/9] fix --- models/organization/org_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/models/organization/org_test.go b/models/organization/org_test.go index cfa304d7b29f..10d9473b5a45 100644 --- a/models/organization/org_test.go +++ b/models/organization/org_test.go @@ -212,7 +212,7 @@ func TestGetOrgUsersByUserID(t *testing.T) { orgUsers, err := organization.GetOrgUsersByUserID(5, &organization.SearchOrganizationsOptions{All: true}) assert.NoError(t, err) - if assert.Len(t, orgUsers, 2) { + if assert.Len(t, orgUsers, 3) { assert.Equal(t, organization.OrgUser{ ID: orgUsers[0].ID, OrgID: 6, @@ -225,6 +225,12 @@ func TestGetOrgUsersByUserID(t *testing.T) { UID: 5, IsPublic: false, }, *orgUsers[1]) + assert.Equal(t, organization.OrgUser{ + ID: orgUsers[2].ID, + OrgID: 23, + UID: 5, + IsPublic: false, + }, *orgUsers[2]) } publicOrgUsers, err := organization.GetOrgUsersByUserID(5, &organization.SearchOrganizationsOptions{All: false}) From 4320930379d3914c896bf8fd49bbe1290ea39022 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Wed, 5 Apr 2023 01:30:52 +0000 Subject: [PATCH 6/9] add new line --- models/fixtures/org_user.yml | 2 +- models/fixtures/team.yml | 2 +- models/fixtures/team_unit.yml | 2 +- models/fixtures/team_user.yml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/models/fixtures/org_user.yml b/models/fixtures/org_user.yml index 784949241863..d08f69579913 100644 --- a/models/fixtures/org_user.yml +++ b/models/fixtures/org_user.yml @@ -80,4 +80,4 @@ id: 14 uid: 5 org_id: 23 - is_public: false \ No newline at end of file + is_public: false diff --git a/models/fixtures/team.yml b/models/fixtures/team.yml index b89f10a5e448..aa3b36e64439 100644 --- a/models/fixtures/team.yml +++ b/models/fixtures/team.yml @@ -183,4 +183,4 @@ num_repos: 0 num_members: 1 includes_all_repositories: false - can_create_org_repo: true \ No newline at end of file + can_create_org_repo: true diff --git a/models/fixtures/team_unit.yml b/models/fixtures/team_unit.yml index ee33b1639204..5257d2c3856d 100644 --- a/models/fixtures/team_unit.yml +++ b/models/fixtures/team_unit.yml @@ -273,4 +273,4 @@ id: 46 team_id: 17 type: 9 # package - access_mode: 0 \ No newline at end of file + access_mode: 0 diff --git a/models/fixtures/team_user.yml b/models/fixtures/team_user.yml index fd99bfd411b9..b95f76c72376 100644 --- a/models/fixtures/team_user.yml +++ b/models/fixtures/team_user.yml @@ -104,4 +104,4 @@ id: 18 org_id: 23 team_id: 17 - uid: 5 \ No newline at end of file + uid: 5 From c1aad04fc18b79ebcb2a774bfbd833026a24ecc8 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Wed, 5 Apr 2023 02:09:28 +0000 Subject: [PATCH 7/9] fix order --- models/organization/org_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/models/organization/org_test.go b/models/organization/org_test.go index 10d9473b5a45..ab8671f9c93d 100644 --- a/models/organization/org_test.go +++ b/models/organization/org_test.go @@ -215,19 +215,19 @@ func TestGetOrgUsersByUserID(t *testing.T) { if assert.Len(t, orgUsers, 3) { assert.Equal(t, organization.OrgUser{ ID: orgUsers[0].ID, - OrgID: 6, + OrgID: 23, UID: 5, - IsPublic: true, + IsPublic: false, }, *orgUsers[0]) assert.Equal(t, organization.OrgUser{ ID: orgUsers[1].ID, - OrgID: 7, + OrgID: 6, UID: 5, - IsPublic: false, + IsPublic: true, }, *orgUsers[1]) assert.Equal(t, organization.OrgUser{ ID: orgUsers[2].ID, - OrgID: 23, + OrgID: 7, UID: 5, IsPublic: false, }, *orgUsers[2]) From 782a7b33f67ad45d31b223462c5a6dfa13cdba2e Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Wed, 5 Apr 2023 04:27:51 +0000 Subject: [PATCH 8/9] fix index --- models/organization/org_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/organization/org_test.go b/models/organization/org_test.go index ab8671f9c93d..6e583879976a 100644 --- a/models/organization/org_test.go +++ b/models/organization/org_test.go @@ -236,7 +236,7 @@ func TestGetOrgUsersByUserID(t *testing.T) { publicOrgUsers, err := organization.GetOrgUsersByUserID(5, &organization.SearchOrganizationsOptions{All: false}) assert.NoError(t, err) assert.Len(t, publicOrgUsers, 1) - assert.Equal(t, *orgUsers[0], *publicOrgUsers[0]) + assert.Equal(t, *orgUsers[1], *publicOrgUsers[0]) orgUsers, err = organization.GetOrgUsersByUserID(1, &organization.SearchOrganizationsOptions{All: true}) assert.NoError(t, err) From 847af056e240fcf88b40cc247dbe8429b5d1a1c6 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 6 Apr 2023 21:44:45 +0800 Subject: [PATCH 9/9] Update modules/context/package.go Co-authored-by: delvh --- modules/context/package.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/context/package.go b/modules/context/package.go index 2cb2f1efc776..2a0159eb5cdd 100644 --- a/modules/context/package.go +++ b/modules/context/package.go @@ -98,7 +98,7 @@ func determineAccessMode(ctx *Context) (perm.AccessMode, error) { org := organization.OrgFromUser(ctx.Package.Owner) if ctx.Doer != nil && !ctx.Doer.IsGhost() { - // 1. If user is logined, check all team packages permissions + // 1. If user is logged in, check all team packages permissions teams, err := organization.GetUserOrgTeams(ctx, org.ID, ctx.Doer.ID) if err != nil { return accessMode, err