From 9a58412929e64a9b2111598df11d892f0ad6c644 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 15 Apr 2024 17:19:47 +0800 Subject: [PATCH 01/22] fix --- models/migrations/migrations.go | 2 ++ models/migrations/v1_23/v297.go | 17 +++++++++++ models/organization/team.go | 4 +-- models/perm/access/repo_permission.go | 34 ++++++++++----------- models/perm/access_mode.go | 44 ++++++++++++++++----------- models/perm/access_mode_test.go | 21 +++++++++++++ models/repo/repo_unit.go | 12 +++++--- modules/templates/helper.go | 10 ++++++ options/locale/locale_en-US.ini | 1 + routers/web/repo/setting/setting.go | 8 +++-- services/convert/convert.go | 2 +- services/convert/user.go | 4 +-- services/forms/repo_form.go | 1 + templates/repo/settings/options.tmpl | 15 +++++++-- 14 files changed, 126 insertions(+), 49 deletions(-) create mode 100644 models/migrations/v1_23/v297.go create mode 100644 models/perm/access_mode_test.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 5326d48f901bc..cb3a64f48c633 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -582,6 +582,8 @@ var migrations = []Migration{ NewMigration("Add commit status summary table", v1_23.AddCommitStatusSummary), // v296 -> v297 NewMigration("Add missing field of commit status summary table", v1_23.AddCommitStatusSummary2), + // v297 -> v298 + NewMigration("Add everyone_access_mode for repo_unit", v1_23.AddRepoUnitEveryoneAccessMode), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v1_23/v297.go b/models/migrations/v1_23/v297.go new file mode 100644 index 0000000000000..726672dc7bf50 --- /dev/null +++ b/models/migrations/v1_23/v297.go @@ -0,0 +1,17 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_23 //nolint + +import ( + "xorm.io/xorm" + + "code.gitea.io/gitea/models/perm" +) + +func AddRepoUnitEveryoneAccessMode(x *xorm.Engine) error { + type RepoUnit struct { //revive:disable-line:exported + EveryoneAccessMode perm.AccessMode `xorm:"NOT NULL DEFAULT -1"` + } + return x.Sync(&RepoUnit{}) +} diff --git a/models/organization/team.go b/models/organization/team.go index 17db11c42d1a1..1c4570a7b8918 100644 --- a/models/organization/team.go +++ b/models/organization/team.go @@ -130,11 +130,11 @@ func (t *Team) GetUnitsMap() map[string]string { m := make(map[string]string) if t.AccessMode >= perm.AccessModeAdmin { for _, u := range unit.Units { - m[u.NameKey] = t.AccessMode.String() + m[u.NameKey] = t.AccessMode.ToString() } } else { for _, u := range t.Units { - m[u.Unit().NameKey] = u.AccessMode.String() + m[u.Unit().NameKey] = u.AccessMode.ToString() } } return m diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go index 4175cb9b92661..4011a8e77916f 100644 --- a/models/perm/access/repo_permission.go +++ b/models/perm/access/repo_permission.go @@ -104,7 +104,7 @@ func (p *Permission) CanWriteIssuesOrPulls(isPull bool) bool { func (p *Permission) LogString() string { format := " perm.UnitsMode[u.Type] { + perm.UnitsMode[u.Type] = u.EveryoneAccessMode } - log.Trace("Permission Loaded for %-v in %-v:\nPermissions: %-+v", - user, - repo, - perm) - }() + } } +} + +// GetUserRepoPermission returns the user permissions to the repository +func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, user *user_model.User) (perm Permission, err error) { + defer func() { + applyDefaultUserRepoPermission(user, &perm) + if log.IsTrace() { + log.Trace("Permission Loaded for user %-v in repo %-v, permissions: %-+v", user, repo, perm) + } + }() // anonymous user visit private repo. // TODO: anonymous user visit public unit of private repo??? @@ -152,7 +153,6 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use } var isCollaborator bool - var err error if user != nil { isCollaborator, err = repo_model.IsCollaborator(ctx, repo.ID, user.ID) if err != nil { diff --git a/models/perm/access_mode.go b/models/perm/access_mode.go index a37bc1f0e164e..86cb2e9f14d56 100644 --- a/models/perm/access_mode.go +++ b/models/perm/access_mode.go @@ -5,26 +5,29 @@ package perm import ( "fmt" + "slices" + + "code.gitea.io/gitea/modules/util" ) // AccessMode specifies the users access mode type AccessMode int const ( - // AccessModeNone no access - AccessModeNone AccessMode = iota // 0 - // AccessModeRead read access - AccessModeRead // 1 - // AccessModeWrite write access - AccessModeWrite // 2 - // AccessModeAdmin admin access - AccessModeAdmin // 3 - // AccessModeOwner owner access - AccessModeOwner // 4 + AccessModeUnset AccessMode = -1 + iota // -1: no access mode is set + + AccessModeNone // 0: no access + AccessModeRead // 1: read access + AccessModeWrite // 2: write access + AccessModeAdmin // 3: admin access + AccessModeOwner // 4: owner access ) -func (mode AccessMode) String() string { +// ToString returns the string representation of the access mode, do not make it a Stringer, otherwise it's difficult to render in templates +func (mode AccessMode) ToString() string { switch mode { + case AccessModeUnset: + return "unset" case AccessModeRead: return "read" case AccessModeWrite: @@ -39,19 +42,26 @@ func (mode AccessMode) String() string { } func (mode AccessMode) LogString() string { - return fmt.Sprintf("", mode, mode.String()) + return fmt.Sprintf("", mode, mode.ToString()) } // ParseAccessMode returns corresponding access mode to given permission string. -func ParseAccessMode(permission string) AccessMode { +func ParseAccessMode(permission string, allowed ...AccessMode) AccessMode { + m := AccessModeNone switch permission { + case "unset": + m = AccessModeUnset case "read": - return AccessModeRead + m = AccessModeRead case "write": - return AccessModeWrite + m = AccessModeWrite case "admin": - return AccessModeAdmin + m = AccessModeAdmin default: - return AccessModeNone + // the "owner" access is not really used for user input, it's mainly for checking access level in code, so don't parse it + } + if len(allowed) == 0 { + return m } + return util.Iif(slices.Contains(allowed, m), m, AccessModeNone) } diff --git a/models/perm/access_mode_test.go b/models/perm/access_mode_test.go new file mode 100644 index 0000000000000..d9ec2c419dc80 --- /dev/null +++ b/models/perm/access_mode_test.go @@ -0,0 +1,21 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package perm + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestAccessMode(t *testing.T) { + names := []string{ /*-1*/ "unset", "none", "read", "write", "admin"} + for i, name := range names { + m := ParseAccessMode(name) + assert.Equal(t, AccessMode(i-1), m) + } + assert.Equal(t, "owner", AccessModeOwner.ToString()) + assert.Equal(t, AccessModeNone, ParseAccessMode("owner")) + assert.Equal(t, AccessModeNone, ParseAccessMode("invalid")) +} diff --git a/models/repo/repo_unit.go b/models/repo/repo_unit.go index 5a841f4d312e8..73e1598275f12 100644 --- a/models/repo/repo_unit.go +++ b/models/repo/repo_unit.go @@ -10,6 +10,7 @@ import ( "strings" "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/perm" "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/setting" @@ -41,11 +42,12 @@ func (err ErrUnitTypeNotExist) Unwrap() error { // RepoUnit describes all units of a repository type RepoUnit struct { //revive:disable-line:exported - ID int64 - RepoID int64 `xorm:"INDEX(s)"` - Type unit.Type `xorm:"INDEX(s)"` - Config convert.Conversion `xorm:"TEXT"` - CreatedUnix timeutil.TimeStamp `xorm:"INDEX CREATED"` + ID int64 + RepoID int64 `xorm:"INDEX(s)"` + Type unit.Type `xorm:"INDEX(s)"` + Config convert.Conversion `xorm:"TEXT"` + CreatedUnix timeutil.TimeStamp `xorm:"INDEX CREATED"` + EveryoneAccessMode perm.AccessMode `xorm:"NOT NULL DEFAULT -1"` } func init() { diff --git a/modules/templates/helper.go b/modules/templates/helper.go index 5d2fa79bc524c..aa3872c741b2c 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -34,6 +34,7 @@ func NewFuncMap() template.FuncMap { // ----------------------------------------------------------------- // html/template related functions "dict": dict, // it's lowercase because this name has been widely used. Our other functions should have uppercase names. + "Iif": Iif, "Eval": Eval, "SafeHTML": SafeHTML, "HTMLFormat": HTMLFormat, @@ -238,6 +239,15 @@ func DotEscape(raw string) string { return strings.ReplaceAll(raw, ".", "\u200d.\u200d") } +func Iif(condition bool, vals ...any) any { + if condition { + return vals[0] + } else if len(vals) > 1 { + return vals[1] + } + return nil +} + // Eval the expression and return the result, see the comment of eval.Expr for details. // To use this helper function in templates, pass each token as a separate parameter. // diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 0a3d12d7a40ff..ac7c6004e53f4 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2096,6 +2096,7 @@ settings.advanced_settings = Advanced Settings settings.wiki_desc = Enable Repository Wiki settings.use_internal_wiki = Use Built-In Wiki settings.default_wiki_branch_name = Default Wiki Branch Name +settings.default_wiki_everyone_access = Default Access Permission for every signed-in user: settings.failed_to_change_default_wiki_branch = Failed to change the default wiki branch. settings.use_external_wiki = Use External Wiki settings.external_wiki_url = External Wiki URL diff --git a/routers/web/repo/setting/setting.go b/routers/web/repo/setting/setting.go index 00a5282f34e77..890f4be8ec79e 100644 --- a/routers/web/repo/setting/setting.go +++ b/routers/web/repo/setting/setting.go @@ -16,6 +16,7 @@ import ( actions_model "code.gitea.io/gitea/models/actions" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/organization" + "code.gitea.io/gitea/models/perm" repo_model "code.gitea.io/gitea/models/repo" unit_model "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" @@ -476,9 +477,10 @@ func SettingsPost(ctx *context.Context) { deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeWiki) } else if form.EnableWiki && !form.EnableExternalWiki && !unit_model.TypeWiki.UnitGlobalDisabled() { units = append(units, repo_model.RepoUnit{ - RepoID: repo.ID, - Type: unit_model.TypeWiki, - Config: new(repo_model.UnitConfig), + RepoID: repo.ID, + Type: unit_model.TypeWiki, + Config: new(repo_model.UnitConfig), + EveryoneAccessMode: perm.ParseAccessMode(form.DefaultWikiEveryoneAccess, perm.AccessModeUnset, perm.AccessModeRead, perm.AccessModeWrite), }) deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeExternalWiki) } else { diff --git a/services/convert/convert.go b/services/convert/convert.go index ca3ec32a4042f..b6592f8fa05b2 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -322,7 +322,7 @@ func ToTeams(ctx context.Context, teams []*organization.Team, loadOrgs bool) ([] Description: t.Description, IncludesAllRepositories: t.IncludesAllRepositories, CanCreateOrgRepo: t.CanCreateOrgRepo, - Permission: t.AccessMode.String(), + Permission: t.AccessMode.ToString(), Units: t.GetUnitNames(), UnitsMap: t.GetUnitsMap(), } diff --git a/services/convert/user.go b/services/convert/user.go index 3521dd2f905c3..314713d0c916c 100644 --- a/services/convert/user.go +++ b/services/convert/user.go @@ -102,7 +102,7 @@ func User2UserSettings(user *user_model.User) api.UserSettings { func ToUserAndPermission(ctx context.Context, user, doer *user_model.User, accessMode perm.AccessMode) api.RepoCollaboratorPermission { return api.RepoCollaboratorPermission{ User: ToUser(ctx, user, doer), - Permission: accessMode.String(), - RoleName: accessMode.String(), + Permission: accessMode.ToString(), + RoleName: accessMode.ToString(), } } diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index e45a2a1695522..f49cc2e86bcb1 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -134,6 +134,7 @@ type RepoSettingForm struct { EnableWiki bool EnableExternalWiki bool DefaultWikiBranch string + DefaultWikiEveryoneAccess string ExternalWikiURL string EnableIssues bool EnableExternalTracker bool diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index df6ccbf6bc2fb..c01a5b3db7d81 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -317,7 +317,9 @@ - {{$isWikiEnabled := or (.Repository.UnitEnabled $.Context $.UnitTypeWiki) (.Repository.UnitEnabled $.Context $.UnitTypeExternalWiki)}} + {{$isBulitinWikiEnabled := .Repository.UnitEnabled $.Context $.UnitTypeWiki}} + {{$isExternalWikiEnabled := .Repository.UnitEnabled $.Context $.UnitTypeExternalWiki}} + {{$isWikiEnabled := or $isBulitinWikiEnabled $isExternalWikiEnabled}} {{$isWikiGlobalDisabled := .UnitTypeWiki.UnitGlobalDisabled}} {{$isExternalWikiGlobalDisabled := .UnitTypeExternalWiki.UnitGlobalDisabled}} {{$isBothWikiGlobalDisabled := and $isWikiGlobalDisabled $isExternalWikiGlobalDisabled}} @@ -331,7 +333,7 @@
- +
@@ -339,6 +341,15 @@
+
+ {{$unitBuiltinWiki := .Repository.MustGetUnit ctx $.UnitTypeWiki}} + + +
From 8ba2d63315a29cad8b9bf9788989c3d5c3bf9393 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 15 Apr 2024 17:55:03 +0800 Subject: [PATCH 02/22] fix test --- models/migrations/v1_23/v297.go | 4 +-- models/perm/access/repo_permission.go | 4 +-- models/perm/access/repo_permission_test.go | 34 ++++++++++++++++++++++ options/locale/locale_en-US.ini | 1 + templates/repo/settings/options.tmpl | 6 ++-- tests/integration/api_team_test.go | 4 +-- 6 files changed, 44 insertions(+), 9 deletions(-) create mode 100644 models/perm/access/repo_permission_test.go diff --git a/models/migrations/v1_23/v297.go b/models/migrations/v1_23/v297.go index 726672dc7bf50..3d33ff3fec818 100644 --- a/models/migrations/v1_23/v297.go +++ b/models/migrations/v1_23/v297.go @@ -4,9 +4,9 @@ package v1_23 //nolint import ( - "xorm.io/xorm" - "code.gitea.io/gitea/models/perm" + + "xorm.io/xorm" ) func AddRepoUnitEveryoneAccessMode(x *xorm.Engine) error { diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go index 4011a8e77916f..17c53f9287ec6 100644 --- a/models/perm/access/repo_permission.go +++ b/models/perm/access/repo_permission.go @@ -127,9 +127,9 @@ func (p *Permission) LogString() string { } func applyDefaultUserRepoPermission(user *user_model.User, perm *Permission) { - if user != nil { + if user != nil && user.ID > 0 { for _, u := range perm.Units { - if u.EveryoneAccessMode != perm_model.AccessModeUnset && u.EveryoneAccessMode > perm.UnitsMode[u.Type] { + if u.EveryoneAccessMode > 0 && u.EveryoneAccessMode > perm.UnitsMode[u.Type] { perm.UnitsMode[u.Type] = u.EveryoneAccessMode } } diff --git a/models/perm/access/repo_permission_test.go b/models/perm/access/repo_permission_test.go new file mode 100644 index 0000000000000..37611916edd3a --- /dev/null +++ b/models/perm/access/repo_permission_test.go @@ -0,0 +1,34 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package access + +import ( + "testing" + + perm_model "code.gitea.io/gitea/models/perm" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unit" + user_model "code.gitea.io/gitea/models/user" + + "github.com/stretchr/testify/assert" +) + +func TestApplyDefaultUserRepoPermission(t *testing.T) { + perm := Permission{ + AccessMode: perm_model.AccessModeNone, + UnitsMode: map[unit.Type]perm_model.AccessMode{}, + } + + perm.Units = []*repo_model.RepoUnit{ + {Type: unit.TypeWiki, EveryoneAccessMode: perm_model.AccessModeNone}, + } + applyDefaultUserRepoPermission(nil, &perm) + assert.False(t, perm.CanRead(unit.TypeWiki)) + + perm.Units = []*repo_model.RepoUnit{ + {Type: unit.TypeWiki, EveryoneAccessMode: perm_model.AccessModeRead}, + } + applyDefaultUserRepoPermission(&user_model.User{ID: 1}, &perm) + assert.True(t, perm.CanRead(unit.TypeWiki)) +} diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index ac7c6004e53f4..84416fa64690a 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -885,6 +885,7 @@ repo_and_org_access = Repository and Organization Access permissions_public_only = Public only permissions_access_all = All (public, private, and limited) select_permissions = Select permissions +permission_unset = (unset) permission_no_access = No Access permission_read = Read permission_write = Read and Write diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index c01a5b3db7d81..cef28a6680696 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -345,9 +345,9 @@ {{$unitBuiltinWiki := .Repository.MustGetUnit ctx $.UnitTypeWiki}}
diff --git a/tests/integration/api_team_test.go b/tests/integration/api_team_test.go index 4df545284e97f..d14c66ff2ca64 100644 --- a/tests/integration/api_team_test.go +++ b/tests/integration/api_team_test.go @@ -126,7 +126,7 @@ func TestAPITeam(t *testing.T) { apiTeam = api.Team{} DecodeJSON(t, resp, &apiTeam) checkTeamResponse(t, "ReadTeam1", &apiTeam, teamRead.Name, *teamToEditDesc.Description, teamRead.IncludesAllRepositories, - teamRead.AccessMode.String(), teamRead.GetUnitNames(), teamRead.GetUnitsMap()) + teamRead.AccessMode.ToString(), teamRead.GetUnitNames(), teamRead.GetUnitsMap()) // Delete team. req = NewRequestf(t, "DELETE", "/api/v1/teams/%d", teamID). @@ -197,7 +197,7 @@ func TestAPITeam(t *testing.T) { DecodeJSON(t, resp, &apiTeam) assert.NoError(t, teamRead.LoadUnits(db.DefaultContext)) checkTeamResponse(t, "ReadTeam2", &apiTeam, teamRead.Name, *teamToEditDesc.Description, teamRead.IncludesAllRepositories, - teamRead.AccessMode.String(), teamRead.GetUnitNames(), teamRead.GetUnitsMap()) + teamRead.AccessMode.ToString(), teamRead.GetUnitNames(), teamRead.GetUnitsMap()) // Delete team. req = NewRequestf(t, "DELETE", "/api/v1/teams/%d", teamID). From 69396b5f418787930812fdcd81cb338d5fc9e801 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 15 Apr 2024 18:21:38 +0800 Subject: [PATCH 03/22] always check map length instead of nil --- models/migrations/v1_11/v111.go | 2 +- models/perm/access/repo_permission.go | 27 +++++++++++---------------- services/convert/repository.go | 2 +- 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/models/migrations/v1_11/v111.go b/models/migrations/v1_11/v111.go index d757acb7d2328..1722792a38f82 100644 --- a/models/migrations/v1_11/v111.go +++ b/models/migrations/v1_11/v111.go @@ -336,7 +336,7 @@ func AddBranchProtectionCanPushAndEnableWhitelist(x *xorm.Engine) error { if err != nil { return false, err } - if perm.UnitsMode == nil { + if len(perm.UnitsMode) == 0 { for _, u := range perm.Units { if u.Type == UnitTypeCode { return AccessModeWrite <= perm.AccessMode, nil diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go index 17c53f9287ec6..c5ac28668d925 100644 --- a/models/perm/access/repo_permission.go +++ b/models/perm/access/repo_permission.go @@ -20,7 +20,7 @@ import ( type Permission struct { AccessMode perm_model.AccessMode Units []*repo_model.RepoUnit - UnitsMode map[unit.Type]perm_model.AccessMode + UnitsMode map[unit.Type]perm_model.AccessMode // zero length means use the AccessMode above } // IsOwner returns true if current user is the owner of repository. @@ -33,17 +33,14 @@ func (p *Permission) IsAdmin() bool { return p.AccessMode >= perm_model.AccessModeAdmin } -// HasAccess returns true if the current user has at least read access to any unit of this repository +// HasAccess returns true if the current user might have at least read access to any unit of this repository func (p *Permission) HasAccess() bool { - if p.UnitsMode == nil { - return p.AccessMode >= perm_model.AccessModeRead - } - return len(p.UnitsMode) > 0 + return len(p.UnitsMode) > 0 || p.AccessMode >= perm_model.AccessModeRead } -// UnitAccessMode returns current user accessmode to the specify unit of the repository +// UnitAccessMode returns current user access mode to the specify unit of the repository func (p *Permission) UnitAccessMode(unitType unit.Type) perm_model.AccessMode { - if p.UnitsMode == nil { + if len(p.UnitsMode) == 0 { for _, u := range p.Units { if u.Type == unitType { return p.AccessMode @@ -145,6 +142,12 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use } }() + if err := repo.LoadUnits(ctx); err != nil { + return perm, err + } + perm.Units = repo.Units + perm.UnitsMode = make(map[unit.Type]perm_model.AccessMode) + // anonymous user visit private repo. // TODO: anonymous user visit public unit of private repo??? if user == nil && repo.IsPrivate { @@ -171,12 +174,6 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use return perm, nil } - if err := repo.LoadUnits(ctx); err != nil { - return perm, err - } - - perm.Units = repo.Units - // anonymous visit public repo if user == nil { perm.AccessMode = perm_model.AccessModeRead @@ -202,8 +199,6 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use return perm, nil } - perm.UnitsMode = make(map[unit.Type]perm_model.AccessMode) - // Collaborators on organization if isCollaborator { for _, u := range repo.Units { diff --git a/services/convert/repository.go b/services/convert/repository.go index 39efd304a96ad..2b0f72f21a2df 100644 --- a/services/convert/repository.go +++ b/services/convert/repository.go @@ -25,7 +25,7 @@ func ToRepo(ctx context.Context, repo *repo_model.Repository, permissionInRepo a func innerToRepo(ctx context.Context, repo *repo_model.Repository, permissionInRepo access_model.Permission, isParent bool) *api.Repository { var parent *api.Repository - if permissionInRepo.Units == nil && permissionInRepo.UnitsMode == nil { + if len(permissionInRepo.Units) == 0 && len(permissionInRepo.UnitsMode) == 0 { // If Units and UnitsMode are both nil, it means that it's a hard coded permission, // like access_model.Permission{AccessMode: perm.AccessModeAdmin}. // So we need to load units for the repo, or UnitAccessMode will always return perm.AccessModeNone. From 6664f3ce377ed0bcd825c7d52bdfbabf39187e72 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 15 Apr 2024 19:38:32 +0800 Subject: [PATCH 04/22] more tests --- models/perm/access/repo_permission.go | 9 ++---- models/perm/access/repo_permission_test.go | 35 ++++++++++++++++++++++ 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go index c5ac28668d925..7a51af01d5c9e 100644 --- a/models/perm/access/repo_permission.go +++ b/models/perm/access/repo_permission.go @@ -142,11 +142,11 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use } }() - if err := repo.LoadUnits(ctx); err != nil { + perm.UnitsMode = make(map[unit.Type]perm_model.AccessMode) // always initialize UnitsMode to avoid nil panic + if err = repo.LoadUnits(ctx); err != nil { return perm, err } perm.Units = repo.Units - perm.UnitsMode = make(map[unit.Type]perm_model.AccessMode) // anonymous user visit private repo. // TODO: anonymous user visit public unit of private repo??? @@ -163,7 +163,7 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use } } - if err := repo.LoadOwner(ctx); err != nil { + if err = repo.LoadOwner(ctx); err != nil { return perm, err } @@ -192,9 +192,6 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use return perm, err } - if err := repo.LoadOwner(ctx); err != nil { - return perm, err - } if !repo.Owner.IsOrganization() { return perm, nil } diff --git a/models/perm/access/repo_permission_test.go b/models/perm/access/repo_permission_test.go index 37611916edd3a..2effd58e6449d 100644 --- a/models/perm/access/repo_permission_test.go +++ b/models/perm/access/repo_permission_test.go @@ -32,3 +32,38 @@ func TestApplyDefaultUserRepoPermission(t *testing.T) { applyDefaultUserRepoPermission(&user_model.User{ID: 1}, &perm) assert.True(t, perm.CanRead(unit.TypeWiki)) } + +func TestUnitAccessMode(t *testing.T) { + perm := Permission{ + AccessMode: perm_model.AccessModeNone, + } + assert.Equal(t, perm_model.AccessModeNone, perm.UnitAccessMode(unit.TypeWiki), "no unit or map, use AccessMode") + + perm = Permission{ + AccessMode: perm_model.AccessModeOwner, + Units: []*repo_model.RepoUnit{ + {Type: unit.TypeWiki, EveryoneAccessMode: perm_model.AccessModeRead}, + }, + UnitsMode: map[unit.Type]perm_model.AccessMode{}, + } + assert.Equal(t, perm_model.AccessModeOwner, perm.UnitAccessMode(unit.TypeWiki), "only unit no map, use AccessMode") + + perm = Permission{ + AccessMode: perm_model.AccessModeOwner, + UnitsMode: map[unit.Type]perm_model.AccessMode{ + unit.TypeWiki: perm_model.AccessModeRead, + }, + } + assert.Equal(t, perm_model.AccessModeRead, perm.UnitAccessMode(unit.TypeWiki), "no unit only map, use map") + + perm = Permission{ + AccessMode: perm_model.AccessModeOwner, + Units: []*repo_model.RepoUnit{ + {Type: unit.TypeWiki, EveryoneAccessMode: perm_model.AccessModeWrite}, + }, + UnitsMode: map[unit.Type]perm_model.AccessMode{ + unit.TypeWiki: perm_model.AccessModeRead, + }, + } + assert.Equal(t, perm_model.AccessModeRead, perm.UnitAccessMode(unit.TypeWiki), "has unit and map, use map") +} From 5a4b2866def6cdb9ad6c7bb15593d1e92c97f174 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 15 Apr 2024 21:27:56 +0800 Subject: [PATCH 05/22] u.EveryoneAccessMode >= perm_model.AccessModeRead --- models/perm/access/repo_permission.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go index 7a51af01d5c9e..22b26e8f43989 100644 --- a/models/perm/access/repo_permission.go +++ b/models/perm/access/repo_permission.go @@ -126,7 +126,7 @@ func (p *Permission) LogString() string { func applyDefaultUserRepoPermission(user *user_model.User, perm *Permission) { if user != nil && user.ID > 0 { for _, u := range perm.Units { - if u.EveryoneAccessMode > 0 && u.EveryoneAccessMode > perm.UnitsMode[u.Type] { + if u.EveryoneAccessMode >= perm_model.AccessModeRead && u.EveryoneAccessMode > perm.UnitsMode[u.Type] { perm.UnitsMode[u.Type] = u.EveryoneAccessMode } } From 34650ad1b23e287e447ac0deaf3d93fa8f31357e Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 15 Apr 2024 22:01:00 +0800 Subject: [PATCH 06/22] fix permission check --- models/perm/access/repo_permission.go | 17 ++++++++++------- models/perm/access/repo_permission_test.go | 12 ++++++++++-- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go index 22b26e8f43989..f9d76729725f2 100644 --- a/models/perm/access/repo_permission.go +++ b/models/perm/access/repo_permission.go @@ -14,6 +14,7 @@ import ( "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/util" ) // Permission contains all the permissions related variables to a repository for a user @@ -40,15 +41,17 @@ func (p *Permission) HasAccess() bool { // UnitAccessMode returns current user access mode to the specify unit of the repository func (p *Permission) UnitAccessMode(unitType unit.Type) perm_model.AccessMode { - if len(p.UnitsMode) == 0 { - for _, u := range p.Units { - if u.Type == unitType { - return p.AccessMode - } + // if the units map contains the access mode, use it, but admin/owner mode could override it + if m, ok := p.UnitsMode[unitType]; ok { + return util.Iif(p.AccessMode >= perm_model.AccessModeAdmin, p.AccessMode, m) + } + // if the units map does not contain the access mode, return the default access mode if the unit exists + for _, u := range p.Units { + if u.Type == unitType { + return p.AccessMode } - return perm_model.AccessModeNone } - return p.UnitsMode[unitType] + return perm_model.AccessModeNone } // CanAccess returns true if user has mode access to the unit of the repository diff --git a/models/perm/access/repo_permission_test.go b/models/perm/access/repo_permission_test.go index 2effd58e6449d..f55bf77aa4a14 100644 --- a/models/perm/access/repo_permission_test.go +++ b/models/perm/access/repo_permission_test.go @@ -49,7 +49,15 @@ func TestUnitAccessMode(t *testing.T) { assert.Equal(t, perm_model.AccessModeOwner, perm.UnitAccessMode(unit.TypeWiki), "only unit no map, use AccessMode") perm = Permission{ - AccessMode: perm_model.AccessModeOwner, + AccessMode: perm_model.AccessModeAdmin, + UnitsMode: map[unit.Type]perm_model.AccessMode{ + unit.TypeWiki: perm_model.AccessModeRead, + }, + } + assert.Equal(t, perm_model.AccessModeAdmin, perm.UnitAccessMode(unit.TypeWiki), "no unit only map, admin overrides map") + + perm = Permission{ + AccessMode: perm_model.AccessModeNone, UnitsMode: map[unit.Type]perm_model.AccessMode{ unit.TypeWiki: perm_model.AccessModeRead, }, @@ -57,7 +65,7 @@ func TestUnitAccessMode(t *testing.T) { assert.Equal(t, perm_model.AccessModeRead, perm.UnitAccessMode(unit.TypeWiki), "no unit only map, use map") perm = Permission{ - AccessMode: perm_model.AccessModeOwner, + AccessMode: perm_model.AccessModeNone, Units: []*repo_model.RepoUnit{ {Type: unit.TypeWiki, EveryoneAccessMode: perm_model.AccessModeWrite}, }, From 96d435c65ead29bbc8fb15592ddbb01f5d0f98e8 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 15 Apr 2024 22:08:57 +0800 Subject: [PATCH 07/22] no need to introduce "unset" --- models/migrations/v1_23/v297.go | 2 +- models/perm/access_mode.go | 7 +------ options/locale/locale_en-US.ini | 1 - routers/web/repo/setting/setting.go | 2 +- templates/repo/settings/options.tmpl | 2 +- 5 files changed, 4 insertions(+), 10 deletions(-) diff --git a/models/migrations/v1_23/v297.go b/models/migrations/v1_23/v297.go index 3d33ff3fec818..e79f04cf9c618 100644 --- a/models/migrations/v1_23/v297.go +++ b/models/migrations/v1_23/v297.go @@ -11,7 +11,7 @@ import ( func AddRepoUnitEveryoneAccessMode(x *xorm.Engine) error { type RepoUnit struct { //revive:disable-line:exported - EveryoneAccessMode perm.AccessMode `xorm:"NOT NULL DEFAULT -1"` + EveryoneAccessMode perm.AccessMode `xorm:"NOT NULL DEFAULT 0"` } return x.Sync(&RepoUnit{}) } diff --git a/models/perm/access_mode.go b/models/perm/access_mode.go index 86cb2e9f14d56..0364191e2e9b2 100644 --- a/models/perm/access_mode.go +++ b/models/perm/access_mode.go @@ -14,9 +14,8 @@ import ( type AccessMode int const ( - AccessModeUnset AccessMode = -1 + iota // -1: no access mode is set + AccessModeNone AccessMode = iota // 0: no access - AccessModeNone // 0: no access AccessModeRead // 1: read access AccessModeWrite // 2: write access AccessModeAdmin // 3: admin access @@ -26,8 +25,6 @@ const ( // ToString returns the string representation of the access mode, do not make it a Stringer, otherwise it's difficult to render in templates func (mode AccessMode) ToString() string { switch mode { - case AccessModeUnset: - return "unset" case AccessModeRead: return "read" case AccessModeWrite: @@ -49,8 +46,6 @@ func (mode AccessMode) LogString() string { func ParseAccessMode(permission string, allowed ...AccessMode) AccessMode { m := AccessModeNone switch permission { - case "unset": - m = AccessModeUnset case "read": m = AccessModeRead case "write": diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 84416fa64690a..ac7c6004e53f4 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -885,7 +885,6 @@ repo_and_org_access = Repository and Organization Access permissions_public_only = Public only permissions_access_all = All (public, private, and limited) select_permissions = Select permissions -permission_unset = (unset) permission_no_access = No Access permission_read = Read permission_write = Read and Write diff --git a/routers/web/repo/setting/setting.go b/routers/web/repo/setting/setting.go index 890f4be8ec79e..b55e259e4bea9 100644 --- a/routers/web/repo/setting/setting.go +++ b/routers/web/repo/setting/setting.go @@ -480,7 +480,7 @@ func SettingsPost(ctx *context.Context) { RepoID: repo.ID, Type: unit_model.TypeWiki, Config: new(repo_model.UnitConfig), - EveryoneAccessMode: perm.ParseAccessMode(form.DefaultWikiEveryoneAccess, perm.AccessModeUnset, perm.AccessModeRead, perm.AccessModeWrite), + EveryoneAccessMode: perm.ParseAccessMode(form.DefaultWikiEveryoneAccess, perm.AccessModeNone, perm.AccessModeRead, perm.AccessModeWrite), }) deleteUnitTypes = append(deleteUnitTypes, unit_model.TypeExternalWiki) } else { diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index cef28a6680696..f034ea1747ab6 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -345,7 +345,7 @@ {{$unitBuiltinWiki := .Repository.MustGetUnit ctx $.UnitTypeWiki}} From cbd2e2434d9a5ad99b7ec9dfc205f2542711098d Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 15 Apr 2024 22:18:29 +0800 Subject: [PATCH 08/22] fix permission check in convert function --- services/convert/repository.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/convert/repository.go b/services/convert/repository.go index 2b0f72f21a2df..3ea7cb389f891 100644 --- a/services/convert/repository.go +++ b/services/convert/repository.go @@ -25,10 +25,10 @@ func ToRepo(ctx context.Context, repo *repo_model.Repository, permissionInRepo a func innerToRepo(ctx context.Context, repo *repo_model.Repository, permissionInRepo access_model.Permission, isParent bool) *api.Repository { var parent *api.Repository - if len(permissionInRepo.Units) == 0 && len(permissionInRepo.UnitsMode) == 0 { - // If Units and UnitsMode are both nil, it means that it's a hard coded permission, + if len(permissionInRepo.Units) == 0 { + // If Units is empty, it means that it's a hard coded permission, (TODO: maybe no such problem anymore, because we always load the units now) // like access_model.Permission{AccessMode: perm.AccessModeAdmin}. - // So we need to load units for the repo, or UnitAccessMode will always return perm.AccessModeNone. + // So we need to load units for the repo, otherwise UnitAccessMode will always return perm.AccessModeNone. _ = repo.LoadUnits(ctx) // the error is not important, so ignore it permissionInRepo.Units = repo.Units } From 756bb7944256b33115107ebd8b0f5e96c263d02b Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 15 Apr 2024 22:27:01 +0800 Subject: [PATCH 09/22] fine tune --- models/perm/access/repo_permission.go | 2 +- models/perm/access/repo_permission_test.go | 15 +++++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go index f9d76729725f2..b261b359ed351 100644 --- a/models/perm/access/repo_permission.go +++ b/models/perm/access/repo_permission.go @@ -21,7 +21,7 @@ import ( type Permission struct { AccessMode perm_model.AccessMode Units []*repo_model.RepoUnit - UnitsMode map[unit.Type]perm_model.AccessMode // zero length means use the AccessMode above + UnitsMode map[unit.Type]perm_model.AccessMode } // IsOwner returns true if current user is the owner of repository. diff --git a/models/perm/access/repo_permission_test.go b/models/perm/access/repo_permission_test.go index f55bf77aa4a14..6c67c090bcabd 100644 --- a/models/perm/access/repo_permission_test.go +++ b/models/perm/access/repo_permission_test.go @@ -18,16 +18,19 @@ func TestApplyDefaultUserRepoPermission(t *testing.T) { perm := Permission{ AccessMode: perm_model.AccessModeNone, UnitsMode: map[unit.Type]perm_model.AccessMode{}, - } - - perm.Units = []*repo_model.RepoUnit{ - {Type: unit.TypeWiki, EveryoneAccessMode: perm_model.AccessModeNone}, + Units: []*repo_model.RepoUnit{ + {Type: unit.TypeWiki, EveryoneAccessMode: perm_model.AccessModeNone}, + }, } applyDefaultUserRepoPermission(nil, &perm) assert.False(t, perm.CanRead(unit.TypeWiki)) - perm.Units = []*repo_model.RepoUnit{ - {Type: unit.TypeWiki, EveryoneAccessMode: perm_model.AccessModeRead}, + perm = Permission{ + AccessMode: perm_model.AccessModeNone, + UnitsMode: map[unit.Type]perm_model.AccessMode{}, + Units: []*repo_model.RepoUnit{ + {Type: unit.TypeWiki, EveryoneAccessMode: perm_model.AccessModeRead}, + }, } applyDefaultUserRepoPermission(&user_model.User{ID: 1}, &perm) assert.True(t, perm.CanRead(unit.TypeWiki)) From e8dda7e6b70ab4434b8009c569ac148cc441424e Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 15 Apr 2024 22:31:41 +0800 Subject: [PATCH 10/22] fix model default --- models/repo/repo_unit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/repo/repo_unit.go b/models/repo/repo_unit.go index 73e1598275f12..fd5baa948861c 100644 --- a/models/repo/repo_unit.go +++ b/models/repo/repo_unit.go @@ -47,7 +47,7 @@ type RepoUnit struct { //revive:disable-line:exported Type unit.Type `xorm:"INDEX(s)"` Config convert.Conversion `xorm:"TEXT"` CreatedUnix timeutil.TimeStamp `xorm:"INDEX CREATED"` - EveryoneAccessMode perm.AccessMode `xorm:"NOT NULL DEFAULT -1"` + EveryoneAccessMode perm.AccessMode `xorm:"NOT NULL DEFAULT 0"` } func init() { From 903c8c9564643ac4a7e5fc594d173a4d6ff8fe27 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 15 Apr 2024 22:46:52 +0800 Subject: [PATCH 11/22] fix test --- models/perm/access_mode_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/models/perm/access_mode_test.go b/models/perm/access_mode_test.go index d9ec2c419dc80..982fceee5a538 100644 --- a/models/perm/access_mode_test.go +++ b/models/perm/access_mode_test.go @@ -10,11 +10,12 @@ import ( ) func TestAccessMode(t *testing.T) { - names := []string{ /*-1*/ "unset", "none", "read", "write", "admin"} + names := []string{"none", "read", "write", "admin"} for i, name := range names { m := ParseAccessMode(name) - assert.Equal(t, AccessMode(i-1), m) + assert.Equal(t, AccessMode(i), m) } + assert.Equal(t, AccessMode(4), AccessModeOwner) assert.Equal(t, "owner", AccessModeOwner.ToString()) assert.Equal(t, AccessModeNone, ParseAccessMode("owner")) assert.Equal(t, AccessModeNone, ParseAccessMode("invalid")) From 4aab4ee24b11eaf581f30fd29a82f1c11a1ac561 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 16 Apr 2024 11:51:56 +0800 Subject: [PATCH 12/22] Update options/locale/locale_en-US.ini Co-authored-by: silverwind --- options/locale/locale_en-US.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index ac7c6004e53f4..2c3212807163d 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2096,7 +2096,7 @@ settings.advanced_settings = Advanced Settings settings.wiki_desc = Enable Repository Wiki settings.use_internal_wiki = Use Built-In Wiki settings.default_wiki_branch_name = Default Wiki Branch Name -settings.default_wiki_everyone_access = Default Access Permission for every signed-in user: +settings.default_wiki_everyone_access = Default Access Permission for signed-in users: settings.failed_to_change_default_wiki_branch = Failed to change the default wiki branch. settings.use_external_wiki = Use External Wiki settings.external_wiki_url = External Wiki URL From 7f9b95ec60fbb01353a9ac11865c387b0686bbfc Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 16 Apr 2024 11:56:37 +0800 Subject: [PATCH 13/22] address reviews --- modules/templates/helper.go | 2 ++ templates/repo/settings/options.tmpl | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/modules/templates/helper.go b/modules/templates/helper.go index aa3872c741b2c..e3d9d71419b10 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -239,6 +239,8 @@ func DotEscape(raw string) string { return strings.ReplaceAll(raw, ".", "\u200d.\u200d") } +// Iif is an "inline-if", similar util.Iif[T] but templates need the non-generic version, +// and it could be simpley used as "{{Iif expr trueVal}}" (omit the falseVal). func Iif(condition bool, vals ...any) any { if condition { return vals[0] diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index f034ea1747ab6..a99e24e151c70 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -317,9 +317,9 @@
- {{$isBulitinWikiEnabled := .Repository.UnitEnabled $.Context $.UnitTypeWiki}} + {{$isBuiltinWikiEnabled := .Repository.UnitEnabled $.Context $.UnitTypeWiki}} {{$isExternalWikiEnabled := .Repository.UnitEnabled $.Context $.UnitTypeExternalWiki}} - {{$isWikiEnabled := or $isBulitinWikiEnabled $isExternalWikiEnabled}} + {{$isWikiEnabled := or $isBuiltinWikiEnabled $isExternalWikiEnabled}} {{$isWikiGlobalDisabled := .UnitTypeWiki.UnitGlobalDisabled}} {{$isExternalWikiGlobalDisabled := .UnitTypeExternalWiki.UnitGlobalDisabled}} {{$isBothWikiGlobalDisabled := and $isWikiGlobalDisabled $isExternalWikiGlobalDisabled}} @@ -333,7 +333,7 @@
- +
From e7548e5fa99a72ba2b88d5c8af4eb95d6ddf4c05 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 16 Apr 2024 14:08:04 +0800 Subject: [PATCH 14/22] refactor unit mode --- models/issues/pull_list.go | 3 +- models/perm/access/access.go | 8 ++- models/perm/access/repo_permission.go | 58 ++++++++++++---------- models/perm/access/repo_permission_test.go | 18 +++---- modules/templates/helper.go | 2 +- routers/api/v1/api.go | 6 +-- routers/private/hook_pre_receive.go | 6 +-- services/convert/repository.go | 7 +-- 8 files changed, 53 insertions(+), 55 deletions(-) diff --git a/models/issues/pull_list.go b/models/issues/pull_list.go index de3eceed374d7..ffc0d3cabfbf2 100644 --- a/models/issues/pull_list.go +++ b/models/issues/pull_list.go @@ -62,7 +62,8 @@ func CanMaintainerWriteToBranch(ctx context.Context, p access_model.Permission, return true } - if len(p.Units) < 1 { + // the code below depends on Units to get the repository ID, not ideal but just keep it for now + if len(p.Units) == 0 { return false } diff --git a/models/perm/access/access.go b/models/perm/access/access.go index b422a086149a0..6a0a901f719e7 100644 --- a/models/perm/access/access.go +++ b/models/perm/access/access.go @@ -63,13 +63,11 @@ func accessLevel(ctx context.Context, user *user_model.User, repo *repo_model.Re } func maxAccessMode(modes ...perm.AccessMode) perm.AccessMode { - max := perm.AccessModeNone + maxMode := perm.AccessModeNone for _, mode := range modes { - if mode > max { - max = mode - } + maxMode = max(maxMode, mode) } - return max + return maxMode } type userAccess struct { diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go index b261b359ed351..335a7de0055f2 100644 --- a/models/perm/access/repo_permission.go +++ b/models/perm/access/repo_permission.go @@ -6,6 +6,7 @@ package access import ( "context" "fmt" + "slices" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/organization" @@ -21,7 +22,7 @@ import ( type Permission struct { AccessMode perm_model.AccessMode Units []*repo_model.RepoUnit - UnitsMode map[unit.Type]perm_model.AccessMode + unitsMode map[unit.Type]perm_model.AccessMode } // IsOwner returns true if current user is the owner of repository. @@ -36,22 +37,26 @@ func (p *Permission) IsAdmin() bool { // HasAccess returns true if the current user might have at least read access to any unit of this repository func (p *Permission) HasAccess() bool { - return len(p.UnitsMode) > 0 || p.AccessMode >= perm_model.AccessModeRead + return len(p.unitsMode) > 0 || p.AccessMode >= perm_model.AccessModeRead } // UnitAccessMode returns current user access mode to the specify unit of the repository func (p *Permission) UnitAccessMode(unitType unit.Type) perm_model.AccessMode { // if the units map contains the access mode, use it, but admin/owner mode could override it - if m, ok := p.UnitsMode[unitType]; ok { + if m, ok := p.unitsMode[unitType]; ok { return util.Iif(p.AccessMode >= perm_model.AccessModeAdmin, p.AccessMode, m) } // if the units map does not contain the access mode, return the default access mode if the unit exists + hasUnit := slices.ContainsFunc(p.Units, func(u *repo_model.RepoUnit) bool { return u.Type == unitType }) + return util.Iif(hasUnit, p.AccessMode, perm_model.AccessModeNone) +} + +func (p *Permission) SetUnitsWithDefaultAccessMode(units []*repo_model.RepoUnit, mode perm_model.AccessMode) { + p.Units = units + p.unitsMode = make(map[unit.Type]perm_model.AccessMode) for _, u := range p.Units { - if u.Type == unitType { - return p.AccessMode - } + p.unitsMode[u.Type] = mode } - return perm_model.AccessModeNone } // CanAccess returns true if user has mode access to the unit of the repository @@ -104,21 +109,21 @@ func (p *Permission) CanWriteIssuesOrPulls(isPull bool) bool { func (p *Permission) LogString() string { format := " 0 { for _, u := range perm.Units { - if u.EveryoneAccessMode >= perm_model.AccessModeRead && u.EveryoneAccessMode > perm.UnitsMode[u.Type] { - perm.UnitsMode[u.Type] = u.EveryoneAccessMode + if u.EveryoneAccessMode >= perm_model.AccessModeRead && u.EveryoneAccessMode > perm.unitsMode[u.Type] { + perm.unitsMode[u.Type] = u.EveryoneAccessMode } } } @@ -139,13 +144,12 @@ func applyDefaultUserRepoPermission(user *user_model.User, perm *Permission) { // GetUserRepoPermission returns the user permissions to the repository func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, user *user_model.User) (perm Permission, err error) { defer func() { - applyDefaultUserRepoPermission(user, &perm) + applyEveryoneRepoPermission(user, &perm) if log.IsTrace() { log.Trace("Permission Loaded for user %-v in repo %-v, permissions: %-+v", user, repo, perm) } }() - perm.UnitsMode = make(map[unit.Type]perm_model.AccessMode) // always initialize UnitsMode to avoid nil panic if err = repo.LoadUnits(ctx); err != nil { return perm, err } @@ -199,10 +203,12 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use return perm, nil } + perm.unitsMode = make(map[unit.Type]perm_model.AccessMode) + // Collaborators on organization if isCollaborator { for _, u := range repo.Units { - perm.UnitsMode[u.Type] = perm.AccessMode + perm.unitsMode[u.Type] = perm.AccessMode } } @@ -216,7 +222,7 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use for _, team := range teams { if team.AccessMode >= perm_model.AccessModeAdmin { perm.AccessMode = perm_model.AccessModeOwner - perm.UnitsMode = nil + perm.unitsMode = nil return perm, nil } } @@ -226,9 +232,9 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use for _, team := range teams { teamMode := team.UnitAccessMode(ctx, u.Type) if teamMode > perm_model.AccessModeNone { - m := perm.UnitsMode[u.Type] + m := perm.unitsMode[u.Type] if m < teamMode { - perm.UnitsMode[u.Type] = teamMode + perm.unitsMode[u.Type] = teamMode } found = true } @@ -236,15 +242,15 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use // for a public repo on an organization, a non-restricted user has read permission on non-team defined units. if !found && !repo.IsPrivate && !user.IsRestricted { - if _, ok := perm.UnitsMode[u.Type]; !ok { - perm.UnitsMode[u.Type] = perm_model.AccessModeRead + if _, ok := perm.unitsMode[u.Type]; !ok { + perm.unitsMode[u.Type] = perm_model.AccessModeRead } } } // remove no permission units perm.Units = make([]*repo_model.RepoUnit, 0, len(repo.Units)) - for t := range perm.UnitsMode { + for t := range perm.unitsMode { for _, u := range repo.Units { if u.Type == t { perm.Units = append(perm.Units, u) @@ -329,7 +335,7 @@ func HasAccessUnit(ctx context.Context, user *user_model.User, repo *repo_model. // Currently any write access (code, issues or pr's) is assignable, to match assignee list in user interface. func CanBeAssigned(ctx context.Context, user *user_model.User, repo *repo_model.Repository, _ bool) (bool, error) { if user.IsOrganization() { - return false, fmt.Errorf("Organization can't be added as assignee [user_id: %d, repo_id: %d]", user.ID, repo.ID) + return false, fmt.Errorf("organization can't be added as assignee [user_id: %d, repo_id: %d]", user.ID, repo.ID) } perm, err := GetUserRepoPermission(ctx, repo, user) if err != nil { diff --git a/models/perm/access/repo_permission_test.go b/models/perm/access/repo_permission_test.go index 6c67c090bcabd..8442592b24d5a 100644 --- a/models/perm/access/repo_permission_test.go +++ b/models/perm/access/repo_permission_test.go @@ -14,25 +14,25 @@ import ( "github.com/stretchr/testify/assert" ) -func TestApplyDefaultUserRepoPermission(t *testing.T) { +func TestApplyEveryoneRepoPermission(t *testing.T) { perm := Permission{ AccessMode: perm_model.AccessModeNone, - UnitsMode: map[unit.Type]perm_model.AccessMode{}, + unitsMode: map[unit.Type]perm_model.AccessMode{}, Units: []*repo_model.RepoUnit{ {Type: unit.TypeWiki, EveryoneAccessMode: perm_model.AccessModeNone}, }, } - applyDefaultUserRepoPermission(nil, &perm) + applyEveryoneRepoPermission(nil, &perm) assert.False(t, perm.CanRead(unit.TypeWiki)) perm = Permission{ AccessMode: perm_model.AccessModeNone, - UnitsMode: map[unit.Type]perm_model.AccessMode{}, + unitsMode: map[unit.Type]perm_model.AccessMode{}, Units: []*repo_model.RepoUnit{ {Type: unit.TypeWiki, EveryoneAccessMode: perm_model.AccessModeRead}, }, } - applyDefaultUserRepoPermission(&user_model.User{ID: 1}, &perm) + applyEveryoneRepoPermission(&user_model.User{ID: 1}, &perm) assert.True(t, perm.CanRead(unit.TypeWiki)) } @@ -47,13 +47,13 @@ func TestUnitAccessMode(t *testing.T) { Units: []*repo_model.RepoUnit{ {Type: unit.TypeWiki, EveryoneAccessMode: perm_model.AccessModeRead}, }, - UnitsMode: map[unit.Type]perm_model.AccessMode{}, + unitsMode: map[unit.Type]perm_model.AccessMode{}, } assert.Equal(t, perm_model.AccessModeOwner, perm.UnitAccessMode(unit.TypeWiki), "only unit no map, use AccessMode") perm = Permission{ AccessMode: perm_model.AccessModeAdmin, - UnitsMode: map[unit.Type]perm_model.AccessMode{ + unitsMode: map[unit.Type]perm_model.AccessMode{ unit.TypeWiki: perm_model.AccessModeRead, }, } @@ -61,7 +61,7 @@ func TestUnitAccessMode(t *testing.T) { perm = Permission{ AccessMode: perm_model.AccessModeNone, - UnitsMode: map[unit.Type]perm_model.AccessMode{ + unitsMode: map[unit.Type]perm_model.AccessMode{ unit.TypeWiki: perm_model.AccessModeRead, }, } @@ -72,7 +72,7 @@ func TestUnitAccessMode(t *testing.T) { Units: []*repo_model.RepoUnit{ {Type: unit.TypeWiki, EveryoneAccessMode: perm_model.AccessModeWrite}, }, - UnitsMode: map[unit.Type]perm_model.AccessMode{ + unitsMode: map[unit.Type]perm_model.AccessMode{ unit.TypeWiki: perm_model.AccessModeRead, }, } diff --git a/modules/templates/helper.go b/modules/templates/helper.go index e3d9d71419b10..360b48c59416a 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -240,7 +240,7 @@ func DotEscape(raw string) string { } // Iif is an "inline-if", similar util.Iif[T] but templates need the non-generic version, -// and it could be simpley used as "{{Iif expr trueVal}}" (omit the falseVal). +// and it could be simply used as "{{Iif expr trueVal}}" (omit the falseVal). func Iif(condition bool, vals ...any) any { if condition { return vals[0] diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index e870378c4b247..42a52b8b15aad 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -209,11 +209,7 @@ func repoAssignment() func(ctx *context.APIContext) { ctx.Error(http.StatusInternalServerError, "LoadUnits", err) return } - ctx.Repo.Permission.Units = ctx.Repo.Repository.Units - ctx.Repo.Permission.UnitsMode = make(map[unit.Type]perm.AccessMode) - for _, u := range ctx.Repo.Repository.Units { - ctx.Repo.Permission.UnitsMode[u.Type] = ctx.Repo.Permission.AccessMode - } + ctx.Repo.Permission.SetUnitsWithDefaultAccessMode(ctx.Repo.Repository.Units, ctx.Repo.Permission.AccessMode) } else { ctx.Repo.Permission, err = access_model.GetUserRepoPermission(ctx, repo, ctx.Doer) if err != nil { diff --git a/routers/private/hook_pre_receive.go b/routers/private/hook_pre_receive.go index 32ec3003e265c..4e59237ed3ec6 100644 --- a/routers/private/hook_pre_receive.go +++ b/routers/private/hook_pre_receive.go @@ -481,11 +481,7 @@ func (ctx *preReceiveContext) loadPusherAndPermission() bool { }) return false } - ctx.userPerm.Units = ctx.Repo.Repository.Units - ctx.userPerm.UnitsMode = make(map[unit.Type]perm_model.AccessMode) - for _, u := range ctx.Repo.Repository.Units { - ctx.userPerm.UnitsMode[u.Type] = ctx.userPerm.AccessMode - } + ctx.userPerm.SetUnitsWithDefaultAccessMode(ctx.Repo.Repository.Units, ctx.userPerm.AccessMode) } else { user, err := user_model.GetUserByID(ctx, ctx.opts.UserID) if err != nil { diff --git a/services/convert/repository.go b/services/convert/repository.go index 3ea7cb389f891..b4bc81e6f759a 100644 --- a/services/convert/repository.go +++ b/services/convert/repository.go @@ -26,9 +26,10 @@ func innerToRepo(ctx context.Context, repo *repo_model.Repository, permissionInR var parent *api.Repository if len(permissionInRepo.Units) == 0 { - // If Units is empty, it means that it's a hard coded permission, (TODO: maybe no such problem anymore, because we always load the units now) - // like access_model.Permission{AccessMode: perm.AccessModeAdmin}. - // So we need to load units for the repo, otherwise UnitAccessMode will always return perm.AccessModeNone. + // If Units is empty, it means that it's a hard-coded permission, + // like access_model.Permission{AccessMode: perm.AccessModeAdmin}, or like access_model.Permission{AccessMode: perm.AccessModeNone}. + // So we need to load units for the repo, otherwise UnitAccessMode might return perm.AccessModeNone. + // FIXME: it should figure out why there are "access_model.Permission{AccessMode: perm.AccessModeNone}" in the code. _ = repo.LoadUnits(ctx) // the error is not important, so ignore it permissionInRepo.Units = repo.Units } From e95faa5e04afe747e57b838f5a0cb23adbb747dd Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 17 Apr 2024 16:43:11 +0800 Subject: [PATCH 15/22] fix merge --- models/perm/access/repo_permission.go | 2 +- templates/repo/settings/options.tmpl | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go index d0f7e4d6302e2..d481d331ce819 100644 --- a/models/perm/access/repo_permission.go +++ b/models/perm/access/repo_permission.go @@ -241,7 +241,7 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use var found bool for _, team := range teams { if teamMode, exist := team.UnitAccessModeEx(ctx, u.Type); exist { - perm.UnitsMode[u.Type] = max(perm.UnitsMode[u.Type], teamMode) + perm.unitsMode[u.Type] = max(perm.unitsMode[u.Type], teamMode) found = true } } diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index 1fff210a43c44..c3ebb04f40f5d 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -317,7 +317,9 @@
- {{$isWikiEnabled := or (.Repository.UnitEnabled $.Context ctx.Consts.RepoUnitTypeWiki) (.Repository.UnitEnabled $.Context ctx.Consts.RepoUnitTypeExternalWiki)}} + {{$isBuiltinWikiEnabled := .Repository.UnitEnabled ctx ctx.Consts.RepoUnitTypeWiki}} + {{$isExternalWikiEnabled := .Repository.UnitEnabled ctx ctx.Consts.RepoUnitTypeExternalWiki}} + {{$isWikiEnabled := or $isBuiltinWikiEnabled $isExternalWikiEnabled}} {{$isWikiGlobalDisabled := ctx.Consts.RepoUnitTypeWiki.UnitGlobalDisabled}} {{$isExternalWikiGlobalDisabled := ctx.Consts.RepoUnitTypeExternalWiki.UnitGlobalDisabled}} {{$isBothWikiGlobalDisabled := and $isWikiGlobalDisabled $isExternalWikiGlobalDisabled}} From 2016049c50ddab792d4d357ea95b6c96b3f96c76 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 17 Apr 2024 20:39:54 +0800 Subject: [PATCH 16/22] Update templates/repo/settings/options.tmpl Co-authored-by: Lunny Xiao --- templates/repo/settings/options.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index c3ebb04f40f5d..eebeb8bd17810 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -342,7 +342,7 @@
- {{$unitBuiltinWiki := .Repository.MustGetUnit ctx $.UnitTypeWiki}} + {{$unitBuiltinWiki := .Repository.MustGetUnit ctx ctx.Consts.RepoUnitTypeWiki}} +
-
- - -
-
- {{$unitBuiltinWiki := .Repository.MustGetUnit ctx ctx.Consts.RepoUnitTypeWiki}} - - +
+
+ + +
+
+ {{$unitInternalWiki := .Repository.MustGetUnit ctx ctx.Consts.RepoUnitTypeWiki}} + + +
- +
-
+

{{ctx.Locale.Tr "repo.settings.external_wiki_url_desc"}}

From 06ce4011babd935fc01bfac074a532da6a2458e2 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 17 Apr 2024 21:52:38 +0800 Subject: [PATCH 19/22] fix repo setting ui --- templates/repo/settings/options.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index 61e8fb173b3d7..d3d5e34d3a9e7 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -337,7 +337,7 @@
-
+
From cbbd32112fcc0fb6e410cb0fd47cb97462aeb0de Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 17 Apr 2024 22:06:59 +0800 Subject: [PATCH 20/22] more tests --- models/perm/access/repo_permission_test.go | 37 +++++++++++++++++----- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/models/perm/access/repo_permission_test.go b/models/perm/access/repo_permission_test.go index 5bb29c21ce0c0..aaa53bb24fede 100644 --- a/models/perm/access/repo_permission_test.go +++ b/models/perm/access/repo_permission_test.go @@ -32,21 +32,42 @@ func TestApplyEveryoneRepoPermission(t *testing.T) { } applyEveryoneRepoPermission(&user_model.User{ID: 1}, &perm) assert.True(t, perm.CanRead(unit.TypeWiki)) + + perm = Permission{ + AccessMode: perm_model.AccessModeWrite, + units: []*repo_model.RepoUnit{ + {Type: unit.TypeWiki, EveryoneAccessMode: perm_model.AccessModeRead}, + }, + } + applyEveryoneRepoPermission(&user_model.User{ID: 1}, &perm) + assert.True(t, perm.CanRead(unit.TypeWiki)) + assert.False(t, perm.CanWrite(unit.TypeWiki)) // because there is no unit mode, so the everyone-mode is used as the unit's access mode + + perm = Permission{ + units: []*repo_model.RepoUnit{ + {Type: unit.TypeWiki, EveryoneAccessMode: perm_model.AccessModeRead}, + }, + unitsMode: map[unit.Type]perm_model.AccessMode{ + unit.TypeWiki: perm_model.AccessModeWrite, + }, + } + applyEveryoneRepoPermission(&user_model.User{ID: 1}, &perm) + assert.True(t, perm.CanWrite(unit.TypeWiki)) } func TestUnitAccessMode(t *testing.T) { perm := Permission{ AccessMode: perm_model.AccessModeNone, } - assert.Equal(t, perm_model.AccessModeNone, perm.UnitAccessMode(unit.TypeWiki), "no unit or map, use AccessMode") + assert.Equal(t, perm_model.AccessModeNone, perm.UnitAccessMode(unit.TypeWiki), "no unit, no map, use AccessMode") perm = Permission{ - AccessMode: perm_model.AccessModeOwner, + AccessMode: perm_model.AccessModeRead, units: []*repo_model.RepoUnit{ - {Type: unit.TypeWiki, EveryoneAccessMode: perm_model.AccessModeRead}, + {Type: unit.TypeWiki}, }, } - assert.Equal(t, perm_model.AccessModeOwner, perm.UnitAccessMode(unit.TypeWiki), "only unit no map, use AccessMode") + assert.Equal(t, perm_model.AccessModeRead, perm.UnitAccessMode(unit.TypeWiki), "only unit, no map, use AccessMode") perm = Permission{ AccessMode: perm_model.AccessModeAdmin, @@ -54,7 +75,7 @@ func TestUnitAccessMode(t *testing.T) { unit.TypeWiki: perm_model.AccessModeRead, }, } - assert.Equal(t, perm_model.AccessModeAdmin, perm.UnitAccessMode(unit.TypeWiki), "no unit only map, admin overrides map") + assert.Equal(t, perm_model.AccessModeAdmin, perm.UnitAccessMode(unit.TypeWiki), "no unit, only map, admin overrides map") perm = Permission{ AccessMode: perm_model.AccessModeNone, @@ -62,16 +83,16 @@ func TestUnitAccessMode(t *testing.T) { unit.TypeWiki: perm_model.AccessModeRead, }, } - assert.Equal(t, perm_model.AccessModeRead, perm.UnitAccessMode(unit.TypeWiki), "no unit only map, use map") + assert.Equal(t, perm_model.AccessModeRead, perm.UnitAccessMode(unit.TypeWiki), "no unit, only map, use map") perm = Permission{ AccessMode: perm_model.AccessModeNone, units: []*repo_model.RepoUnit{ - {Type: unit.TypeWiki, EveryoneAccessMode: perm_model.AccessModeWrite}, + {Type: unit.TypeWiki}, }, unitsMode: map[unit.Type]perm_model.AccessMode{ unit.TypeWiki: perm_model.AccessModeRead, }, } - assert.Equal(t, perm_model.AccessModeRead, perm.UnitAccessMode(unit.TypeWiki), "has unit and map, use map") + assert.Equal(t, perm_model.AccessModeRead, perm.UnitAccessMode(unit.TypeWiki), "has unit, and map, use map") } From 0f4dfe39979c8855cda2ef793e0b8bd8839ba8fa Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 17 Apr 2024 22:28:54 +0800 Subject: [PATCH 21/22] use better text for "not set" permission --- options/locale/locale_en-US.ini | 1 + templates/repo/settings/options.tmpl | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 2c3212807163d..575e34cebd0d0 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -885,6 +885,7 @@ repo_and_org_access = Repository and Organization Access permissions_public_only = Public only permissions_access_all = All (public, private, and limited) select_permissions = Select permissions +permission_not_set = Not set permission_no_access = No Access permission_read = Read permission_write = Read and Write diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index d3d5e34d3a9e7..c0411cfc562e9 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -346,7 +346,8 @@ {{$unitInternalWiki := .Repository.MustGetUnit ctx ctx.Consts.RepoUnitTypeWiki}} From fb219731c96c83d22b14982d23428abaeb9ed95f Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 17 Apr 2024 23:21:06 +0800 Subject: [PATCH 22/22] only apply everyone access mode if there is no error --- models/perm/access/repo_permission.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go index fc5ca7e9171b0..9cce95b77648e 100644 --- a/models/perm/access/repo_permission.go +++ b/models/perm/access/repo_permission.go @@ -174,7 +174,9 @@ func applyEveryoneRepoPermission(user *user_model.User, perm *Permission) { // GetUserRepoPermission returns the user permissions to the repository func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, user *user_model.User) (perm Permission, err error) { defer func() { - applyEveryoneRepoPermission(user, &perm) + if err == nil { + applyEveryoneRepoPermission(user, &perm) + } if log.IsTrace() { log.Trace("Permission Loaded for user %-v in repo %-v, permissions: %-+v", user, repo, perm) }