Merge pull request 'Disabling Stars should disable the routes too' (#2471) from algernon/forgejo:stars/disable-routes into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/2471
Reviewed-by: Otto <otto@codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>

modules/setting/repository.go

@@ -49,7 +49,7 @@ var (
 		DownloadOrCloneMethods                  []string
 		PrefixArchiveFiles                      bool
 		DisableMigrations                       bool
-		DisableStars                            bool `ini:"DISABLE_STARS"`
+		DisableStars                            bool
 		DisableForks                            bool
 		DefaultBranch                           string
 		AllowAdoptionOfUnadoptedRepositories    bool

routers/api/v1/api.go

@@ -964,7 +964,9 @@ func Routes() *web.Route {
 					m.Get("/{target}", user.CheckFollowing)
 				})
 
+				if !setting.Repository.DisableStars {
 					m.Get("/starred", user.GetStarredRepos)
+				}
 
 				m.Get("/subscriptions", user.GetWatchedRepos)
 			}, context_service.UserAssignmentAPI())
@@ -1039,6 +1041,7 @@ func Routes() *web.Route {
 				Post(bind(api.CreateRepoOption{}), repo.Create)
 
 			// (repo scope)
+			if !setting.Repository.DisableStars {
 				m.Group("/starred", func() {
 					m.Get("", user.GetMyStarredRepos)
 					m.Group("/{username}/{reponame}", func() {
@@ -1047,6 +1050,7 @@ func Routes() *web.Route {
 						m.Delete("", user.Unstar)
 					}, repoAssignment())
 				}, tokenRequiresScopes(auth_model.AccessTokenScopeCategoryRepository))
+			}
 			m.Get("/times", repo.ListMyTrackedTimes)
 			m.Get("/stopwatches", repo.GetStopwatches)
 			m.Get("/subscriptions", user.GetMyWatchedRepos)
@@ -1208,7 +1212,9 @@ func Routes() *web.Route {
 				m.Post("/markup", reqToken(), bind(api.MarkupOption{}), misc.Markup)
 				m.Post("/markdown", reqToken(), bind(api.MarkdownOption{}), misc.Markdown)
 				m.Post("/markdown/raw", reqToken(), misc.MarkdownRaw)
+				if !setting.Repository.DisableStars {
 					m.Get("/stargazers", repo.ListStargazers)
+				}
 				m.Get("/subscribers", repo.ListSubscribers)
 				m.Group("/subscription", func() {
 					m.Get("", user.IsWatching)

routers/web/web.go

@@ -1129,8 +1129,10 @@ func registerRoutes(m *web.Route) {
 		m.Post("/unwatch", repo.ActionWatch(false))
 		m.Post("/accept_transfer", repo.ActionTransfer(true))
 		m.Post("/reject_transfer", repo.ActionTransfer(false))
+		if !setting.Repository.DisableStars {
 			m.Post("/star", repo.ActionStar(true))
 			m.Post("/unstar", repo.ActionStar(false))
+		}
 	}, reqSignIn, context.RepoAssignment, context.UnitTypes())
 
 	// Grouping for those endpoints not requiring authentication (but should respect ignSignIn)
@@ -1359,7 +1361,9 @@ func registerRoutes(m *web.Route) {
 					m.Get("/open.svg", badges.GetOpenPullsBadge)
 					m.Get("/closed.svg", badges.GetClosedPullsBadge)
 				})
+				if !setting.Repository.DisableStars {
 					m.Get("/stars.svg", badges.GetStarsBadge)
+				}
 				m.Get("/release.svg", badges.GetLatestReleaseBadge)
 			})
 		}
@@ -1590,7 +1594,9 @@ func registerRoutes(m *web.Route) {
 	m.Post("/{username}/{reponame}/lastcommit/*", ignSignInAndCsrf, context.RepoAssignment, context.UnitTypes(), context.RepoRefByType(context.RepoRefCommit), reqRepoCodeReader, repo.LastCommit)
 
 	m.Group("/{username}/{reponame}", func() {
+		if !setting.Repository.DisableStars {
 			m.Get("/stars", repo.Stars)
+		}
 		m.Get("/watchers", repo.Watchers)
 		m.Get("/search", reqRepoCodeReader, repo.Search)
 	}, ignSignIn, context.RepoAssignment, context.RepoRef(), context.UnitTypes())

tests/integration/api_user_star_test.go

@@ -9,7 +9,10 @@ import (
 	"testing"
 
 	auth_model "code.gitea.io/gitea/models/auth"
+	"code.gitea.io/gitea/modules/setting"
 	api "code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/modules/test"
+	"code.gitea.io/gitea/routers"
 	"code.gitea.io/gitea/tests"
 
 	"github.com/stretchr/testify/assert"
@@ -25,12 +28,26 @@ func TestAPIStar(t *testing.T) {
 	token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadUser)
 	tokenWithUserScope := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteUser, auth_model.AccessTokenScopeWriteRepository)
 
+	assertDisabledStarsNotFound := func(t *testing.T, req *RequestWrapper) {
+		t.Helper()
+
+		defer tests.PrintCurrentTest(t)()
+		defer test.MockVariableValue(&setting.Repository.DisableStars, true)()
+		defer test.MockVariableValue(&testWebRoutes, routers.NormalRoutes())()
+
+		MakeRequest(t, req, http.StatusNotFound)
+	}
+
 	t.Run("Star", func(t *testing.T) {
 		defer tests.PrintCurrentTest(t)()
 
 		req := NewRequest(t, "PUT", fmt.Sprintf("/api/v1/user/starred/%s", repo)).
 			AddTokenAuth(tokenWithUserScope)
 		MakeRequest(t, req, http.StatusNoContent)
+
+		t.Run("disabled stars", func(t *testing.T) {
+			assertDisabledStarsNotFound(t, req)
+		})
 	})
 
 	t.Run("GetStarredRepos", func(t *testing.T) {
@@ -46,6 +63,10 @@ func TestAPIStar(t *testing.T) {
 		DecodeJSON(t, resp, &repos)
 		assert.Len(t, repos, 1)
 		assert.Equal(t, repo, repos[0].FullName)
+
+		t.Run("disabled stars", func(t *testing.T) {
+			assertDisabledStarsNotFound(t, req)
+		})
 	})
 
 	t.Run("GetMyStarredRepos", func(t *testing.T) {
@@ -61,6 +82,10 @@ func TestAPIStar(t *testing.T) {
 		DecodeJSON(t, resp, &repos)
 		assert.Len(t, repos, 1)
 		assert.Equal(t, repo, repos[0].FullName)
+
+		t.Run("disabled stars", func(t *testing.T) {
+			assertDisabledStarsNotFound(t, req)
+		})
 	})
 
 	t.Run("IsStarring", func(t *testing.T) {
@@ -73,6 +98,10 @@ func TestAPIStar(t *testing.T) {
 		req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/user/starred/%s", repo+"notexisting")).
 			AddTokenAuth(tokenWithUserScope)
 		MakeRequest(t, req, http.StatusNotFound)
+
+		t.Run("disabled stars", func(t *testing.T) {
+			assertDisabledStarsNotFound(t, req)
+		})
 	})
 
 	t.Run("Unstar", func(t *testing.T) {
@@ -81,5 +110,9 @@ func TestAPIStar(t *testing.T) {
 		req := NewRequest(t, "DELETE", fmt.Sprintf("/api/v1/user/starred/%s", repo)).
 			AddTokenAuth(tokenWithUserScope)
 		MakeRequest(t, req, http.StatusNoContent)
+
+		t.Run("disabled stars", func(t *testing.T) {
+			assertDisabledStarsNotFound(t, req)
+		})
 	})
 }

tests/integration/repo_badges_test.go

@@ -16,7 +16,9 @@ import (
 	unit_model "code.gitea.io/gitea/models/unit"
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/test"
+	"code.gitea.io/gitea/routers"
 	files_service "code.gitea.io/gitea/services/repository/files"
 	"code.gitea.io/gitea/tests"
 
@@ -107,6 +109,14 @@ func TestBadges(t *testing.T) {
 			resp := MakeRequest(t, req, http.StatusSeeOther)
 
 			assertBadge(t, resp, "stars-0-blue")
+
+			t.Run("disabled stars", func(t *testing.T) {
+				defer tests.PrintCurrentTest(t)()
+				defer test.MockVariableValue(&setting.Repository.DisableStars, true)()
+				defer test.MockVariableValue(&testWebRoutes, routers.NormalRoutes())()
+
+				MakeRequest(t, req, http.StatusNotFound)
+			})
 		})
 
 		t.Run("Issues", func(t *testing.T) {

tests/integration/repo_starwatch_test.go

@@ -9,6 +9,9 @@ import (
 	"strings"
 	"testing"
 
+	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/test"
+	"code.gitea.io/gitea/routers"
 	"code.gitea.io/gitea/tests"
 
 	"github.com/stretchr/testify/assert"
@@ -80,3 +83,26 @@ func TestRepoStarUnstarUI(t *testing.T) {
 func TestRepoWatchUnwatchUI(t *testing.T) {
 	testRepoStarringOrWatching(t, "watch", "watchers")
 }
+
+func TestDisabledStars(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+	defer test.MockVariableValue(&setting.Repository.DisableStars, true)()
+	defer test.MockVariableValue(&testWebRoutes, routers.NormalRoutes())()
+
+	t.Run("repo star, unstar", func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
+
+		req := NewRequest(t, "POST", "/user2/repo1/action/star")
+		MakeRequest(t, req, http.StatusNotFound)
+
+		req = NewRequest(t, "POST", "/user2/repo1/action/unstar")
+		MakeRequest(t, req, http.StatusNotFound)
+	})
+
+	t.Run("repo stargazers", func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
+
+		req := NewRequest(t, "GET", "/user2/repo1/stars")
+		MakeRequest(t, req, http.StatusNotFound)
+	})
+}