core
/
gitea
3
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Revert package access change from #23879 (#25707) 

Fixes (?) #25538
Fixes https://codeberg.org/forgejo/forgejo/issues/972

Regression #23879

#23879 introduced a change which prevents read access to packages if a
user is not a member of an organization.

That PR also contained a change which disallows package access if the
team unit is configured with "no access" for packages. I don't think
this change makes sense (at the moment). It may be relevant for private
orgs. But for public or limited orgs that's useless because an
unauthorized user would have more access rights than the team member.
This PR restores the old behaviour "If a user has read access for an
owner, they can read packages".

---------

Co-authored-by: Giteabot <teabot@gitea.io>
This commit is contained in:
KN4CK3R 2023-07-09 15:00:07 +02:00 committed by GitHub
parent 819aed35bf
commit 321383db22
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 384 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
models/fixtures/org_user.yml
View File

@ -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

33
models/fixtures/team.yml
View File

@ -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

8
models/fixtures/team_unit.yml
View File

@ -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

18
models/fixtures/team_user.yml
View File

@ -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

74
models/fixtures/user.yml
View File

@ -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

24
modules/context/package.go
View File

@ -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 {

6
tests/integration/api_org_test.go
View File

@ -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")

230
tests/integration/api_packages_test.go
View File

@ -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) {