Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Require at least one unit to be enabled #24189

Merged
merged 17 commits into from
May 6, 2023
Merged

Require at least one unit to be enabled #24189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
5bf9752
Release unit now could be disabled globally
lunny Apr 18, 2023
8d5c8aa
configuration could not disable all the units
lunny Apr 18, 2023
b5cf290
Remove unused functions
lunny Apr 22, 2023
dc362c6
Add empty default test
lunny Apr 22, 2023
b84f8f6
Merge branch 'main' into lunny/release_disable
lunny Apr 22, 2023
3dd3125
hide code pages when code is disabled globally
lunny Apr 22, 2023
a7b2f85
Fix test
lunny Apr 27, 2023
a009d7e
Merge branch 'main' into lunny/release_disable
lunny Apr 27, 2023
dafab3c
Follow yp05327's suggestion
lunny May 4, 2023
21774f7
Merge branch 'main' into lunny/release_disable
lunny May 4, 2023
cd22bf0
Update options/locale/locale_en-US.ini
lunny May 4, 2023
5460744
Update templates/explore/navbar.tmpl
lunny May 4, 2023
b082965
Follow yp05327's suggestion
lunny May 4, 2023
a3f5819
Merge branch 'lunny/release_disable' of github.com:lunny/gitea into l…
lunny May 4, 2023
c70f297
Merge branch 'main' into lunny/release_disable
GiteaBot May 6, 2023
2dddf93
Merge branch 'main' into lunny/release_disable
GiteaBot May 6, 2023
832aebe
Merge branch 'main' into lunny/release_disable
GiteaBot May 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion models/repo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
47 changes: 11 additions & 36 deletions models/unit/unit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package unit

import (
"errors"
"fmt"
"strings"

Expand Down Expand Up @@ -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{}
)
Expand All @@ -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)
}
}

Expand All @@ -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, ", "))
lunny marked this conversation as resolved.
Show resolved Hide resolved
}
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)
lunny marked this conversation as resolved.
Show resolved Hide resolved
if len(DefaultForkRepoUnits) == 0 {
return errors.New("no default fork repository units found")
}
return nil
}

// UnitGlobalDisabled checks if unit type is global disabled
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down
76 changes: 59 additions & 17 deletions models/unit/unit_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
1 change: 1 addition & 0 deletions options/locale/locale_en-US.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 5 additions & 3 deletions routers/api/v1/repo/repo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 6 additions & 0 deletions routers/web/repo/setting.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

lunny marked this conversation as resolved.
Show resolved Hide resolved
if err := repo_model.UpdateRepositoryUnits(repo, units, deleteUnitTypes); err != nil {
ctx.ServerError("UpdateRepositoryUnits", err)
return
Expand Down
38 changes: 22 additions & 16 deletions routers/web/web.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Copy link
Member

@silverwind silverwind Jun 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lunny I can no longer access /explore/code page on my local instance because of this change, despite being an admin. What is the reason for this change and where can one grant access to this global code search?

Isn't this "unit access" thing something that applies only to repos, not global?

Copy link
Member Author

@lunny lunny Jun 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, it should be a bug. #25173 should fix it. Now every unit could be disabled globally. The bug is it should not check ContextUser because this router maybe allowed to visit anonymously.

m.Get("/topics/search", explore.TopicSearch)
}, ignExploreSignIn)
m.Group("/issues", func() {
Expand Down Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion templates/explore/navbar.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion templates/user/profile.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down