Skip to content

Commit

Permalink
Revert package access change from go-gitea#23879 (go-gitea#25707) (go…
Browse files Browse the repository at this point in the history
…-gitea#25785)

Backport go-gitea#25707 by @KN4CK3R

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

Regression go-gitea#23879

go-gitea#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: KN4CK3R <admin@oldschoolhack.me>
  • Loading branch information
GiteaBot and KN4CK3R authored Jul 9, 2023
1 parent 06bcdfe commit 372b622
Show file tree
Hide file tree
Showing 8 changed files with 385 additions and 28 deletions.
18 changes: 18 additions & 0 deletions models/fixtures/org_user.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 changes: 33 additions & 0 deletions models/fixtures/team.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 changes: 7 additions & 1 deletion models/fixtures/team_unit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 changes: 18 additions & 0 deletions models/fixtures/team_user.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 changes: 74 additions & 0 deletions models/fixtures/user.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 changes: 17 additions & 7 deletions modules/context/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions tests/integration/api_org_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Loading

0 comments on commit 372b622

Please sign in to comment.