diff --git a/models/fixtures/org_user.yml b/models/fixtures/org_user.yml index d08f695799..8d58169a32 100644 --- a/models/fixtures/org_user.yml +++ b/models/fixtures/org_user.yml @@ -81,3 +81,21 @@ uid: 5 org_id: 23 is_public: false + +- + id: 15 + uid: 1 + org_id: 35 + is_public: true + +- + id: 16 + uid: 1 + org_id: 36 + is_public: true + +- + id: 17 + uid: 5 + org_id: 36 + is_public: true diff --git a/models/fixtures/team.yml b/models/fixtures/team.yml index aa3b36e644..65326eedbf 100644 --- a/models/fixtures/team.yml +++ b/models/fixtures/team.yml @@ -184,3 +184,36 @@ num_members: 1 includes_all_repositories: false can_create_org_repo: true + +- + id: 18 + org_id: 35 + lower_name: owners + name: Owners + authorize: 4 # owner + num_repos: 0 + num_members: 1 + includes_all_repositories: false + can_create_org_repo: true + +- + id: 19 + org_id: 36 + lower_name: owners + name: Owners + authorize: 4 # owner + num_repos: 0 + num_members: 1 + includes_all_repositories: false + can_create_org_repo: true + +- + id: 20 + org_id: 36 + lower_name: team20writepackage + name: team20writepackage + authorize: 1 + num_repos: 0 + num_members: 1 + includes_all_repositories: false + can_create_org_repo: true diff --git a/models/fixtures/team_unit.yml b/models/fixtures/team_unit.yml index 5257d2c385..5d2ba2fb6c 100644 --- a/models/fixtures/team_unit.yml +++ b/models/fixtures/team_unit.yml @@ -273,4 +273,10 @@ id: 46 team_id: 17 type: 9 # package - access_mode: 0 + access_mode: 2 + +- + id: 47 + team_id: 20 + type: 9 # package + access_mode: 2 diff --git a/models/fixtures/team_user.yml b/models/fixtures/team_user.yml index b95f76c723..feace5f2a5 100644 --- a/models/fixtures/team_user.yml +++ b/models/fixtures/team_user.yml @@ -105,3 +105,21 @@ org_id: 23 team_id: 17 uid: 5 + +- + id: 19 + org_id: 35 + team_id: 18 + uid: 1 + +- + id: 20 + org_id: 36 + team_id: 19 + uid: 1 + +- + id: 21 + org_id: 36 + team_id: 20 + uid: 5 diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml index eba33a7c36..26bb7a9f4b 100644 --- a/models/fixtures/user.yml +++ b/models/fixtures/user.yml @@ -1258,3 +1258,77 @@ repo_admin_change_team_access: false theme: "" keep_activity_private: false + +- + id: 35 + lower_name: private_org35 + name: private_org35 + full_name: Private Org 35 + email: private_org35@example.com + keep_email_private: false + email_notifications_preference: enabled + passwd: ZogKvWdyEx:password + passwd_hash_algo: dummy + must_change_password: false + login_source: 0 + login_name: private_org35 + type: 1 + salt: ZogKvWdyEx + max_repo_creation: -1 + is_active: true + is_admin: false + is_restricted: false + allow_git_hook: false + allow_import_local: false + allow_create_organization: true + prohibit_login: false + avatar: avatar35 + avatar_email: private_org35@example.com + use_custom_avatar: false + num_followers: 0 + num_following: 0 + num_stars: 0 + num_repos: 0 + num_teams: 1 + num_members: 1 + visibility: 2 + repo_admin_change_team_access: false + theme: "" + keep_activity_private: false + +- + id: 36 + lower_name: limited_org36 + name: limited_org36 + full_name: Limited Org 36 + email: limited_org36@example.com + keep_email_private: false + email_notifications_preference: enabled + passwd: ZogKvWdyEx:password + passwd_hash_algo: dummy + must_change_password: false + login_source: 0 + login_name: limited_org36 + type: 1 + salt: ZogKvWdyEx + max_repo_creation: -1 + is_active: true + is_admin: false + is_restricted: false + allow_git_hook: false + allow_import_local: false + allow_create_organization: true + prohibit_login: false + avatar: avatar22 + avatar_email: limited_org36@example.com + use_custom_avatar: false + num_followers: 0 + num_following: 0 + num_stars: 0 + num_repos: 0 + num_teams: 2 + num_members: 2 + visibility: 1 + repo_admin_change_team_access: false + theme: "" + keep_activity_private: false diff --git a/modules/context/package.go b/modules/context/package.go index 8e80fa66ec..be50e0a991 100644 --- a/modules/context/package.go +++ b/modules/context/package.go @@ -108,18 +108,28 @@ func determineAccessMode(ctx *Base, pkg *Package, doer *user_model.User) (perm.A if doer != nil && !doer.IsGhost() { // 1. If user is logged in, check all team packages permissions - teams, err := organization.GetUserOrgTeams(ctx, org.ID, doer.ID) + var err error + accessMode, err = org.GetOrgUserMaxAuthorizeLevel(doer.ID) if err != nil { return accessMode, err } - for _, t := range teams { - perm := t.UnitAccessMode(ctx, unit.TypePackages) - if accessMode < perm { - accessMode = perm + // If access mode is less than write check every team for more permissions + // The minimum possible access mode is read for org members + if accessMode < perm.AccessModeWrite { + teams, err := organization.GetUserOrgTeams(ctx, org.ID, doer.ID) + if err != nil { + return accessMode, err + } + for _, t := range teams { + perm := t.UnitAccessMode(ctx, unit.TypePackages) + if accessMode < perm { + accessMode = perm + } } } - } else if organization.HasOrgOrUserVisible(ctx, pkg.Owner, doer) { - // 2. If user is non-login, check if org is visible to non-login user + } + if accessMode == perm.AccessModeNone && organization.HasOrgOrUserVisible(ctx, pkg.Owner, doer) { + // 2. If user is unauthorized or no org member, check if org is visible accessMode = perm.AccessModeRead } } else { diff --git a/tests/integration/api_org_test.go b/tests/integration/api_org_test.go index edbf576b9e..83a101716b 100644 --- a/tests/integration/api_org_test.go +++ b/tests/integration/api_org_test.go @@ -170,9 +170,9 @@ func TestAPIGetAll(t *testing.T) { var apiOrgList []*api.Organization DecodeJSON(t, resp, &apiOrgList) - assert.Len(t, apiOrgList, 9) - assert.Equal(t, "org25", apiOrgList[1].FullName) - assert.Equal(t, "public", apiOrgList[1].Visibility) + assert.Len(t, apiOrgList, 11) + assert.Equal(t, "Limited Org 36", apiOrgList[1].FullName) + assert.Equal(t, "limited", apiOrgList[1].Visibility) // accessing without a token will return only public orgs req = NewRequestf(t, "GET", "/api/v1/orgs") diff --git a/tests/integration/api_packages_test.go b/tests/integration/api_packages_test.go index 84733f683b..cd981e9c73 100644 --- a/tests/integration/api_packages_test.go +++ b/tests/integration/api_packages_test.go @@ -157,29 +157,227 @@ 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}) + limitedUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 33}) + privateUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 31}) + privateOrgMember := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 23}) // user has package write access + limitedOrgMember := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 36}) // user has package write access + publicOrgMember := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 25}) // user has package read access + privateOrgNoMember := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 35}) + limitedOrgNoMember := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 22}) + publicOrgNoMember := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 17}) - uploadPackage := func(doer, owner *user_model.User, expectedStatus int) { - url := fmt.Sprintf("/api/packages/%s/generic/test-package/1.0/file.bin", owner.Name) + uploadPackage := func(doer, owner *user_model.User, filename string, expectedStatus int) { + url := fmt.Sprintf("/api/packages/%s/generic/test-package/1.0/%s.bin", owner.Name, filename) req := NewRequestWithBody(t, "PUT", url, bytes.NewReader([]byte{1})) - AddBasicAuthHeader(req, doer.Name) + if doer != nil { + AddBasicAuthHeader(req, doer.Name) + } MakeRequest(t, req, expectedStatus) } - uploadPackage(user, inactive, http.StatusUnauthorized) - uploadPackage(inactive, inactive, http.StatusUnauthorized) - uploadPackage(inactive, user, http.StatusUnauthorized) - uploadPackage(admin, inactive, http.StatusCreated) - uploadPackage(admin, user, http.StatusCreated) + downloadPackage := func(doer, owner *user_model.User, expectedStatus int) { + url := fmt.Sprintf("/api/packages/%s/generic/test-package/1.0/admin.bin", owner.Name) + req := NewRequest(t, "GET", url) + if doer != nil { + AddBasicAuthHeader(req, doer.Name) + } + MakeRequest(t, req, expectedStatus) + } - // 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) + type Target struct { + Owner *user_model.User + ExpectedStatus int + } - 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) + t.Run("Upload", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + cases := []struct { + Doer *user_model.User + Filename string + Targets []Target + }{ + { // Admins can upload to every owner + Doer: admin, + Filename: "admin", + Targets: []Target{ + {admin, http.StatusCreated}, + {inactive, http.StatusCreated}, + {user, http.StatusCreated}, + {limitedUser, http.StatusCreated}, + {privateUser, http.StatusCreated}, + {privateOrgMember, http.StatusCreated}, + {limitedOrgMember, http.StatusCreated}, + {publicOrgMember, http.StatusCreated}, + {privateOrgNoMember, http.StatusCreated}, + {limitedOrgNoMember, http.StatusCreated}, + {publicOrgNoMember, http.StatusCreated}, + }, + }, + { // Without credentials no upload should be possible + Doer: nil, + Filename: "nil", + Targets: []Target{ + {admin, http.StatusUnauthorized}, + {inactive, http.StatusUnauthorized}, + {user, http.StatusUnauthorized}, + {limitedUser, http.StatusUnauthorized}, + {privateUser, http.StatusUnauthorized}, + {privateOrgMember, http.StatusUnauthorized}, + {limitedOrgMember, http.StatusUnauthorized}, + {publicOrgMember, http.StatusUnauthorized}, + {privateOrgNoMember, http.StatusUnauthorized}, + {limitedOrgNoMember, http.StatusUnauthorized}, + {publicOrgNoMember, http.StatusUnauthorized}, + }, + }, + { // Inactive users can't upload anywhere + Doer: inactive, + Filename: "inactive", + Targets: []Target{ + {admin, http.StatusUnauthorized}, + {inactive, http.StatusUnauthorized}, + {user, http.StatusUnauthorized}, + {limitedUser, http.StatusUnauthorized}, + {privateUser, http.StatusUnauthorized}, + {privateOrgMember, http.StatusUnauthorized}, + {limitedOrgMember, http.StatusUnauthorized}, + {publicOrgMember, http.StatusUnauthorized}, + {privateOrgNoMember, http.StatusUnauthorized}, + {limitedOrgNoMember, http.StatusUnauthorized}, + {publicOrgNoMember, http.StatusUnauthorized}, + }, + }, + { // Normal users can upload to self and orgs in which they are members and have package write access + Doer: user, + Filename: "user", + Targets: []Target{ + {admin, http.StatusUnauthorized}, + {inactive, http.StatusUnauthorized}, + {user, http.StatusCreated}, + {limitedUser, http.StatusUnauthorized}, + {privateUser, http.StatusUnauthorized}, + {privateOrgMember, http.StatusCreated}, + {limitedOrgMember, http.StatusCreated}, + {publicOrgMember, http.StatusUnauthorized}, + {privateOrgNoMember, http.StatusUnauthorized}, + {limitedOrgNoMember, http.StatusUnauthorized}, + {publicOrgNoMember, http.StatusUnauthorized}, + }, + }, + } + + for _, c := range cases { + for _, t := range c.Targets { + uploadPackage(c.Doer, t.Owner, c.Filename, t.ExpectedStatus) + } + } + }) + + t.Run("Download", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + cases := []struct { + Doer *user_model.User + Filename string + Targets []Target + }{ + { // Admins can access everything + Doer: admin, + Targets: []Target{ + {admin, http.StatusOK}, + {inactive, http.StatusOK}, + {user, http.StatusOK}, + {limitedUser, http.StatusOK}, + {privateUser, http.StatusOK}, + {privateOrgMember, http.StatusOK}, + {limitedOrgMember, http.StatusOK}, + {publicOrgMember, http.StatusOK}, + {privateOrgNoMember, http.StatusOK}, + {limitedOrgNoMember, http.StatusOK}, + {publicOrgNoMember, http.StatusOK}, + }, + }, + { // Without credentials only public owners are accessible + Doer: nil, + Targets: []Target{ + {admin, http.StatusOK}, + {inactive, http.StatusOK}, + {user, http.StatusOK}, + {limitedUser, http.StatusUnauthorized}, + {privateUser, http.StatusUnauthorized}, + {privateOrgMember, http.StatusUnauthorized}, + {limitedOrgMember, http.StatusUnauthorized}, + {publicOrgMember, http.StatusOK}, + {privateOrgNoMember, http.StatusUnauthorized}, + {limitedOrgNoMember, http.StatusUnauthorized}, + {publicOrgNoMember, http.StatusOK}, + }, + }, + { // Inactive users have no access + Doer: inactive, + Targets: []Target{ + {admin, http.StatusUnauthorized}, + {inactive, http.StatusUnauthorized}, + {user, http.StatusUnauthorized}, + {limitedUser, http.StatusUnauthorized}, + {privateUser, http.StatusUnauthorized}, + {privateOrgMember, http.StatusUnauthorized}, + {limitedOrgMember, http.StatusUnauthorized}, + {publicOrgMember, http.StatusUnauthorized}, + {privateOrgNoMember, http.StatusUnauthorized}, + {limitedOrgNoMember, http.StatusUnauthorized}, + {publicOrgNoMember, http.StatusUnauthorized}, + }, + }, + { // Normal users can access self, public or limited users/orgs and private orgs in which they are members + Doer: user, + Targets: []Target{ + {admin, http.StatusOK}, + {inactive, http.StatusOK}, + {user, http.StatusOK}, + {limitedUser, http.StatusOK}, + {privateUser, http.StatusUnauthorized}, + {privateOrgMember, http.StatusOK}, + {limitedOrgMember, http.StatusOK}, + {publicOrgMember, http.StatusOK}, + {privateOrgNoMember, http.StatusUnauthorized}, + {limitedOrgNoMember, http.StatusOK}, + {publicOrgNoMember, http.StatusOK}, + }, + }, + } + + for _, c := range cases { + for _, target := range c.Targets { + downloadPackage(c.Doer, target.Owner, target.ExpectedStatus) + } + } + }) + + t.Run("API", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() + + session := loginUser(t, user.Name) + tokenReadPackage := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadPackage) + + for _, target := range []Target{ + {admin, http.StatusOK}, + {inactive, http.StatusOK}, + {user, http.StatusOK}, + {limitedUser, http.StatusOK}, + {privateUser, http.StatusForbidden}, + {privateOrgMember, http.StatusOK}, + {limitedOrgMember, http.StatusOK}, + {publicOrgMember, http.StatusOK}, + {privateOrgNoMember, http.StatusForbidden}, + {limitedOrgNoMember, http.StatusOK}, + {publicOrgNoMember, http.StatusOK}, + } { + req := NewRequest(t, "GET", fmt.Sprintf("/api/v1/packages/%s?token=%s", target.Owner.Name, tokenReadPackage)) + MakeRequest(t, req, target.ExpectedStatus) + } + }) } func TestPackageQuota(t *testing.T) {