Skip to content

Commit

Permalink
Revert package access change from #23879 (#25707)
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
KN4CK3R and GiteaBot authored Jul 9, 2023
1 parent 819aed3 commit 321383d
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: [email protected]
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: [email protected]
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: [email protected]
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: [email protected]
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 321383d

Please sign in to comment.