summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLunny Xiao <xiaolunwen@gmail.com>2023-05-06 11:39:06 +0200
committerGitHub <noreply@github.com>2023-05-06 11:39:06 +0200
commite5a8ebc0ed2553a16bac4b54a74cc3e5e8036b32 (patch)
treec26686d3793d3908c9f55a11be242103ff1ea04b
parentFix broken `README` link (#24546) (diff)
downloadforgejo-e5a8ebc0ed2553a16bac4b54a74cc3e5e8036b32.tar.xz
forgejo-e5a8ebc0ed2553a16bac4b54a74cc3e5e8036b32.zip
Require at least one unit to be enabled (#24189)
Don't remember why the previous decision that `Code` and `Release` are non-disable units globally. Since now every unit include `Code` could be disabled, maybe we should have a new rule that the repo should have at least one unit. So any unit could be disabled. Fixes #20960 Fixes #7525 --------- Co-authored-by: delvh <dev.lh@web.de> Co-authored-by: yp05327 <576951401@qq.com>
-rw-r--r--models/repo.go4
-rw-r--r--models/unit/unit.go47
-rw-r--r--models/unit/unit_test.go76
-rw-r--r--options/locale/locale_en-US.ini1
-rw-r--r--routers/api/v1/repo/repo.go8
-rw-r--r--routers/web/repo/setting.go6
-rw-r--r--routers/web/web.go38
-rw-r--r--templates/explore/navbar.tmpl2
-rw-r--r--templates/user/profile.tmpl2
9 files changed, 109 insertions, 75 deletions
diff --git a/models/repo.go b/models/repo.go
index 5a1e2e028e..d2a3311c8b 100644
--- a/models/repo.go
+++ b/models/repo.go
@@ -40,7 +40,9 @@ var ItemsPerPage = 40
// Init initialize model
func Init(ctx context.Context) error {
- unit.LoadUnitConfig()
+ if err := unit.LoadUnitConfig(); err != nil {
+ return err
+ }
return system_model.Init(ctx)
}
diff --git a/models/unit/unit.go b/models/unit/unit.go
index 3d5a8842cd..7cd679116f 100644
--- a/models/unit/unit.go
+++ b/models/unit/unit.go
@@ -4,6 +4,7 @@
package unit
import (
+ "errors"
"fmt"
"strings"
@@ -106,12 +107,6 @@ var (
TypeExternalTracker,
}
- // MustRepoUnits contains the units could not be disabled currently
- MustRepoUnits = []Type{
- TypeCode,
- TypeReleases,
- }
-
// DisabledRepoUnits contains the units that have been globally disabled
DisabledRepoUnits = []Type{}
)
@@ -122,18 +117,13 @@ func validateDefaultRepoUnits(defaultUnits, settingDefaultUnits []Type) []Type {
// Use setting if not empty
if len(settingDefaultUnits) > 0 {
- // MustRepoUnits required as default
- units = make([]Type, len(MustRepoUnits))
- copy(units, MustRepoUnits)
+ units = make([]Type, 0, len(settingDefaultUnits))
for _, settingUnit := range settingDefaultUnits {
if !settingUnit.CanBeDefault() {
log.Warn("Not allowed as default unit: %s", settingUnit.String())
continue
}
- // MustRepoUnits already added
- if settingUnit.CanDisable() {
- units = append(units, settingUnit)
- }
+ units = append(units, settingUnit)
}
}
@@ -150,30 +140,30 @@ func validateDefaultRepoUnits(defaultUnits, settingDefaultUnits []Type) []Type {
}
// LoadUnitConfig load units from settings
-func LoadUnitConfig() {
+func LoadUnitConfig() error {
var invalidKeys []string
DisabledRepoUnits, invalidKeys = FindUnitTypes(setting.Repository.DisabledRepoUnits...)
if len(invalidKeys) > 0 {
log.Warn("Invalid keys in disabled repo units: %s", strings.Join(invalidKeys, ", "))
}
- // Check that must units are not disabled
- for i, disabledU := range DisabledRepoUnits {
- if !disabledU.CanDisable() {
- log.Warn("Not allowed to global disable unit %s", disabledU.String())
- DisabledRepoUnits = append(DisabledRepoUnits[:i], DisabledRepoUnits[i+1:]...)
- }
- }
setDefaultRepoUnits, invalidKeys := FindUnitTypes(setting.Repository.DefaultRepoUnits...)
if len(invalidKeys) > 0 {
log.Warn("Invalid keys in default repo units: %s", strings.Join(invalidKeys, ", "))
}
DefaultRepoUnits = validateDefaultRepoUnits(DefaultRepoUnits, setDefaultRepoUnits)
+ if len(DefaultRepoUnits) == 0 {
+ return errors.New("no default repository units found")
+ }
setDefaultForkRepoUnits, invalidKeys := FindUnitTypes(setting.Repository.DefaultForkRepoUnits...)
if len(invalidKeys) > 0 {
log.Warn("Invalid keys in default fork repo units: %s", strings.Join(invalidKeys, ", "))
}
DefaultForkRepoUnits = validateDefaultRepoUnits(DefaultForkRepoUnits, setDefaultForkRepoUnits)
+ if len(DefaultForkRepoUnits) == 0 {
+ return errors.New("no default fork repository units found")
+ }
+ return nil
}
// UnitGlobalDisabled checks if unit type is global disabled
@@ -186,16 +176,6 @@ func (u Type) UnitGlobalDisabled() bool {
return false
}
-// CanDisable checks if this unit type can be disabled.
-func (u *Type) CanDisable() bool {
- for _, mu := range MustRepoUnits {
- if *u == mu {
- return false
- }
- }
- return true
-}
-
// CanBeDefault checks if the unit type can be a default repo unit
func (u *Type) CanBeDefault() bool {
for _, nadU := range NotAllowedDefaultRepoUnits {
@@ -216,11 +196,6 @@ type Unit struct {
MaxAccessMode perm.AccessMode // The max access mode of the unit. i.e. Read means this unit can only be read.
}
-// CanDisable returns if this unit could be disabled.
-func (u *Unit) CanDisable() bool {
- return u.Type.CanDisable()
-}
-
// IsLessThan compares order of two units
func (u Unit) IsLessThan(unit Unit) bool {
if (u.Type == TypeExternalTracker || u.Type == TypeExternalWiki) && unit.Type != TypeExternalTracker && unit.Type != TypeExternalWiki {
diff --git a/models/unit/unit_test.go b/models/unit/unit_test.go
index 50d7817197..5c50a106bd 100644
--- a/models/unit/unit_test.go
+++ b/models/unit/unit_test.go
@@ -12,42 +12,84 @@ import (
)
func TestLoadUnitConfig(t *testing.T) {
- defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) {
- DisabledRepoUnits = disabledRepoUnits
- DefaultRepoUnits = defaultRepoUnits
- DefaultForkRepoUnits = defaultForkRepoUnits
- }(DisabledRepoUnits, DefaultRepoUnits, DefaultForkRepoUnits)
- defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) {
- setting.Repository.DisabledRepoUnits = disabledRepoUnits
- setting.Repository.DefaultRepoUnits = defaultRepoUnits
- setting.Repository.DefaultForkRepoUnits = defaultForkRepoUnits
- }(setting.Repository.DisabledRepoUnits, setting.Repository.DefaultRepoUnits, setting.Repository.DefaultForkRepoUnits)
-
t.Run("regular", func(t *testing.T) {
+ defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) {
+ DisabledRepoUnits = disabledRepoUnits
+ DefaultRepoUnits = defaultRepoUnits
+ DefaultForkRepoUnits = defaultForkRepoUnits
+ }(DisabledRepoUnits, DefaultRepoUnits, DefaultForkRepoUnits)
+ defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) {
+ setting.Repository.DisabledRepoUnits = disabledRepoUnits
+ setting.Repository.DefaultRepoUnits = defaultRepoUnits
+ setting.Repository.DefaultForkRepoUnits = defaultForkRepoUnits
+ }(setting.Repository.DisabledRepoUnits, setting.Repository.DefaultRepoUnits, setting.Repository.DefaultForkRepoUnits)
+
setting.Repository.DisabledRepoUnits = []string{"repo.issues"}
setting.Repository.DefaultRepoUnits = []string{"repo.code", "repo.releases", "repo.issues", "repo.pulls"}
setting.Repository.DefaultForkRepoUnits = []string{"repo.releases"}
- LoadUnitConfig()
+ assert.NoError(t, LoadUnitConfig())
assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnits)
assert.Equal(t, []Type{TypeCode, TypeReleases, TypePullRequests}, DefaultRepoUnits)
- assert.Equal(t, []Type{TypeCode, TypeReleases}, DefaultForkRepoUnits)
+ assert.Equal(t, []Type{TypeReleases}, DefaultForkRepoUnits)
})
t.Run("invalid", func(t *testing.T) {
+ defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) {
+ DisabledRepoUnits = disabledRepoUnits
+ DefaultRepoUnits = defaultRepoUnits
+ DefaultForkRepoUnits = defaultForkRepoUnits
+ }(DisabledRepoUnits, DefaultRepoUnits, DefaultForkRepoUnits)
+ defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) {
+ setting.Repository.DisabledRepoUnits = disabledRepoUnits
+ setting.Repository.DefaultRepoUnits = defaultRepoUnits
+ setting.Repository.DefaultForkRepoUnits = defaultForkRepoUnits
+ }(setting.Repository.DisabledRepoUnits, setting.Repository.DefaultRepoUnits, setting.Repository.DefaultForkRepoUnits)
+
setting.Repository.DisabledRepoUnits = []string{"repo.issues", "invalid.1"}
setting.Repository.DefaultRepoUnits = []string{"repo.code", "invalid.2", "repo.releases", "repo.issues", "repo.pulls"}
setting.Repository.DefaultForkRepoUnits = []string{"invalid.3", "repo.releases"}
- LoadUnitConfig()
+ assert.NoError(t, LoadUnitConfig())
assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnits)
assert.Equal(t, []Type{TypeCode, TypeReleases, TypePullRequests}, DefaultRepoUnits)
- assert.Equal(t, []Type{TypeCode, TypeReleases}, DefaultForkRepoUnits)
+ assert.Equal(t, []Type{TypeReleases}, DefaultForkRepoUnits)
})
t.Run("duplicate", func(t *testing.T) {
+ defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) {
+ DisabledRepoUnits = disabledRepoUnits
+ DefaultRepoUnits = defaultRepoUnits
+ DefaultForkRepoUnits = defaultForkRepoUnits
+ }(DisabledRepoUnits, DefaultRepoUnits, DefaultForkRepoUnits)
+ defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) {
+ setting.Repository.DisabledRepoUnits = disabledRepoUnits
+ setting.Repository.DefaultRepoUnits = defaultRepoUnits
+ setting.Repository.DefaultForkRepoUnits = defaultForkRepoUnits
+ }(setting.Repository.DisabledRepoUnits, setting.Repository.DefaultRepoUnits, setting.Repository.DefaultForkRepoUnits)
+
setting.Repository.DisabledRepoUnits = []string{"repo.issues", "repo.issues"}
setting.Repository.DefaultRepoUnits = []string{"repo.code", "repo.releases", "repo.issues", "repo.pulls", "repo.code"}
setting.Repository.DefaultForkRepoUnits = []string{"repo.releases", "repo.releases"}
- LoadUnitConfig()
+ assert.NoError(t, LoadUnitConfig())
assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnits)
assert.Equal(t, []Type{TypeCode, TypeReleases, TypePullRequests}, DefaultRepoUnits)
- assert.Equal(t, []Type{TypeCode, TypeReleases}, DefaultForkRepoUnits)
+ assert.Equal(t, []Type{TypeReleases}, DefaultForkRepoUnits)
+ })
+ t.Run("empty_default", func(t *testing.T) {
+ defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []Type) {
+ DisabledRepoUnits = disabledRepoUnits
+ DefaultRepoUnits = defaultRepoUnits
+ DefaultForkRepoUnits = defaultForkRepoUnits
+ }(DisabledRepoUnits, DefaultRepoUnits, DefaultForkRepoUnits)
+ defer func(disabledRepoUnits, defaultRepoUnits, defaultForkRepoUnits []string) {
+ setting.Repository.DisabledRepoUnits = disabledRepoUnits
+ setting.Repository.DefaultRepoUnits = defaultRepoUnits
+ setting.Repository.DefaultForkRepoUnits = defaultForkRepoUnits
+ }(setting.Repository.DisabledRepoUnits, setting.Repository.DefaultRepoUnits, setting.Repository.DefaultForkRepoUnits)
+
+ setting.Repository.DisabledRepoUnits = []string{"repo.issues", "repo.issues"}
+ setting.Repository.DefaultRepoUnits = []string{}
+ setting.Repository.DefaultForkRepoUnits = []string{"repo.releases", "repo.releases"}
+ assert.NoError(t, LoadUnitConfig())
+ assert.Equal(t, []Type{TypeIssues}, DisabledRepoUnits)
+ assert.ElementsMatch(t, []Type{TypeCode, TypePullRequests, TypeReleases, TypeWiki, TypePackages, TypeProjects}, DefaultRepoUnits)
+ assert.Equal(t, []Type{TypeReleases}, DefaultForkRepoUnits)
})
}
diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
index f48ebbdc74..248936e35d 100644
--- a/options/locale/locale_en-US.ini
+++ b/options/locale/locale_en-US.ini
@@ -2004,6 +2004,7 @@ settings.delete_notices_2 = - This operation will permanently delete the <strong
settings.delete_notices_fork_1 = - Forks of this repository will become independent after deletion.
settings.deletion_success = The repository has been deleted.
settings.update_settings_success = The repository settings have been updated.
+settings.update_settings_no_unit = The repository should allow at least some sort of interaction.
settings.confirm_delete = Delete Repository
settings.add_collaborator = Add Collaborator
settings.add_collaborator_success = The collaborator has been added.
diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go
index 48ace3a8e0..36cc03f893 100644
--- a/routers/api/v1/repo/repo.go
+++ b/routers/api/v1/repo/repo.go
@@ -970,9 +970,11 @@ func updateRepoUnits(ctx *context.APIContext, opts api.EditRepoOption) error {
}
}
- if err := repo_model.UpdateRepositoryUnits(repo, units, deleteUnitTypes); err != nil {
- ctx.Error(http.StatusInternalServerError, "UpdateRepositoryUnits", err)
- return err
+ if len(units)+len(deleteUnitTypes) > 0 {
+ if err := repo_model.UpdateRepositoryUnits(repo, units, deleteUnitTypes); err != nil {
+ ctx.Error(http.StatusInternalServerError, "UpdateRepositoryUnits", err)
+ return err
+ }
}
log.Trace("Repository advanced settings updated: %s/%s", owner.Name, repo.Name)
diff --git a/routers/web/repo/setting.go b/routers/web/repo/setting.go
index 0c36503b3c..16b718c919 100644
--- a/routers/web/repo/setting.go
+++ b/routers/web/repo/setting.go
@@ -536,6 +536,12 @@ func SettingsPost(ctx *context.Context) {
deleteUnitTypes = append(deleteUnitTypes, unit_model.TypePullRequests)
}
+ if len(units) == 0 {
+ ctx.Flash.Error(ctx.Tr("repo.settings.update_settings_no_unit"))
+ ctx.Redirect(ctx.Repo.RepoLink + "/settings")
+ return
+ }
+
if err := repo_model.UpdateRepositoryUnits(repo, units, deleteUnitTypes); err != nil {
ctx.ServerError("UpdateRepositoryUnits", err)
return
diff --git a/routers/web/web.go b/routers/web/web.go
index e904db321d..5917c93e22 100644
--- a/routers/web/web.go
+++ b/routers/web/web.go
@@ -261,6 +261,27 @@ func registerRoutes(m *web.Route) {
}
}
+ reqUnitAccess := func(unitType unit.Type, accessMode perm.AccessMode) func(ctx *context.Context) {
+ return func(ctx *context.Context) {
+ if unitType.UnitGlobalDisabled() {
+ ctx.NotFound(unitType.String(), nil)
+ return
+ }
+
+ if ctx.ContextUser == nil {
+ ctx.NotFound(unitType.String(), nil)
+ return
+ }
+
+ if ctx.ContextUser.IsOrganization() {
+ if ctx.Org.Organization.UnitPermission(ctx, ctx.Doer, unitType) < accessMode {
+ ctx.NotFound(unitType.String(), nil)
+ return
+ }
+ }
+ }
+ }
+
addWebhookAddRoutes := func() {
m.Get("/{type}/new", repo.WebhooksNew)
m.Post("/gitea/new", web.Bind(forms.NewWebhookForm{}), repo.GiteaHooksNewPost)
@@ -334,7 +355,7 @@ func registerRoutes(m *web.Route) {
m.Get("/users", explore.Users)
m.Get("/users/sitemap-{idx}.xml", sitemapEnabled, explore.Users)
m.Get("/organizations", explore.Organizations)
- m.Get("/code", explore.Code)
+ m.Get("/code", reqUnitAccess(unit.TypeCode, perm.AccessModeRead), explore.Code)
m.Get("/topics/search", explore.TopicSearch)
}, ignExploreSignIn)
m.Group("/issues", func() {
@@ -649,21 +670,6 @@ func registerRoutes(m *web.Route) {
}
}
- reqUnitAccess := func(unitType unit.Type, accessMode perm.AccessMode) func(ctx *context.Context) {
- return func(ctx *context.Context) {
- if ctx.ContextUser == nil {
- ctx.NotFound(unitType.String(), nil)
- return
- }
- if ctx.ContextUser.IsOrganization() {
- if ctx.Org.Organization.UnitPermission(ctx, ctx.Doer, unitType) < accessMode {
- ctx.NotFound(unitType.String(), nil)
- return
- }
- }
- }
- }
-
// ***** START: Organization *****
m.Group("/org", func() {
m.Group("/{org}", func() {
diff --git a/templates/explore/navbar.tmpl b/templates/explore/navbar.tmpl
index 3a23df6992..3a556812f9 100644
--- a/templates/explore/navbar.tmpl
+++ b/templates/explore/navbar.tmpl
@@ -10,7 +10,7 @@
<a class="{{if .PageIsExploreOrganizations}}active {{end}}item" href="{{AppSubUrl}}/explore/organizations">
{{svg "octicon-organization"}} {{.locale.Tr "explore.organizations"}}
</a>
- {{if .IsRepoIndexerEnabled}}
+ {{if and (not $.UnitTypeCode.UnitGlobalDisabled) .IsRepoIndexerEnabled}}
<a class="{{if .PageIsExploreCode}}active {{end}}item" href="{{AppSubUrl}}/explore/code">
{{svg "octicon-code"}} {{.locale.Tr "explore.code"}}
</a>
diff --git a/templates/user/profile.tmpl b/templates/user/profile.tmpl
index 5463270854..8d48474441 100644
--- a/templates/user/profile.tmpl
+++ b/templates/user/profile.tmpl
@@ -135,7 +135,7 @@
{{svg "octicon-package"}} {{.locale.Tr "packages.title"}}
</a>
{{end}}
- {{if .IsRepoIndexerEnabled}}
+ {{if and (not $.UnitTypeCode.UnitGlobalDisabled) .IsRepoIndexerEnabled}}
<a class='{{if eq .TabName "code"}}active {{end}}item' href="{{.ContextUser.HomeLink}}/-/code">
{{svg "octicon-code"}} {{.locale.Tr "user.code"}}
</a>