fix(repo): centralize repository-scoped authorization (#39063)

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

routers/api/v1/api.go

@@ -427,6 +427,15 @@ func reqOwner() func(ctx *context.APIContext) {
 	}
 }
 
+func reqRepoDangerZone() func(ctx *context.APIContext) {
+	return func(ctx *context.APIContext) {
+		if !access_model.CanDoerManageRepoDangerZone(ctx, ctx.Doer, ctx.Repo.Repository, &ctx.Repo.Permission) {
+			ctx.APIError(http.StatusForbidden, "user has no permission to manage the danger zone")
+			return
+		}
+	}
+}
+
 // reqSelfOrAdmin doer should be the same as the contextUser or site admin
 func reqSelfOrAdmin() func(ctx *context.APIContext) {
 	return func(ctx *context.APIContext) {
@@ -1305,11 +1314,11 @@ func Routes() *web.Router {
 				m.Get("/compare/*", reqRepoReader(unit.TypeCode), repo.CompareDiff)
 
 				m.Combo("").Get(reqAnyRepoReader(), repo.Get).
-					Delete(reqToken(), reqOwner(), repo.Delete).
+					Delete(reqToken(), reqRepoDangerZone(), repo.Delete).
 					Patch(reqToken(), reqAdmin(), bind(api.EditRepoOption{}), repo.Edit)
 				m.Post("/generate", reqToken(), reqRepoReader(unit.TypeCode), bind(api.GenerateRepoOption{}), repo.Generate)
 				m.Group("/transfer", func() {
-					m.Post("", reqOwner(), bind(api.TransferRepoOption{}), repo.Transfer)
+					m.Post("", reqRepoDangerZone(), bind(api.TransferRepoOption{}), repo.Transfer)
 					m.Post("/accept", repo.AcceptTransfer)
 					m.Post("/reject", repo.RejectTransfer)
 				}, reqToken())

routers/api/v1/org/team.go

@@ -613,7 +613,7 @@ func GetTeamRepo(ctx *context.APIContext) {
 	//   "404":
 	//     "$ref": "#/responses/notFound"
 
-	repo := getRepositoryByParams(ctx)
+	repo, permission := getRepositoryByParams(ctx)
 	if ctx.Written() {
 		return
 	}
@@ -629,11 +629,6 @@ func GetTeamRepo(ctx *context.APIContext) {
 		return
 	}
 
-	permission, err := access_model.GetDoerRepoPermission(ctx, repo, ctx.Doer)
-	if err != nil {
-		ctx.APIErrorInternal(err)
-		return
-	}
 	// The team may be reachable by a non-team-member via its visibility tier;
 	// don't confirm the existence of a repo the doer cannot access.
 	if !permission.HasAnyUnitAccessOrPublicAccess() {
@@ -641,31 +636,28 @@ func GetTeamRepo(ctx *context.APIContext) {
 		return
 	}
 
-	ctx.JSON(http.StatusOK, convert.ToRepo(ctx, repo, permission))
+	ctx.JSON(http.StatusOK, convert.ToRepo(ctx, repo, *permission))
 }
 
 // getRepositoryByParams get repository by a team's organization ID and repo name
-func getRepositoryByParams(ctx *context.APIContext) *repo_model.Repository {
+func getRepositoryByParams(ctx *context.APIContext) (*repo_model.Repository, *access_model.Permission) {
 	repo, err := repo_model.GetRepositoryByName(ctx, ctx.Org.Team.OrgID, ctx.PathParam("reponame"))
 	if err != nil {
-		if repo_model.IsErrRepoNotExist(err) {
-			ctx.APIErrorNotFound()
-		} else {
-			ctx.APIErrorInternal(err)
-		}
-		return nil
+		ctx.APIErrorAuto(err)
+		return nil, nil
 	}
-	return repo
+	perm, err := access_model.GetDoerRepoPermission(ctx, repo, ctx.Doer)
+	if err != nil {
+		ctx.APIErrorAuto(err)
+		return nil, nil
+	}
+	return repo, &perm
 }
 
-func canChangeTeamRepository(ctx *context.APIContext) bool {
-	canChange, err := ctx.Org.Organization.CanChangeRepoTeamAccess(ctx, ctx.Doer)
-	if err != nil {
-		ctx.APIErrorInternal(err)
-		return false
-	}
+func canManageRepoCollaboratorTeam(ctx *context.APIContext, repo *repo_model.Repository, perm *access_model.Permission) bool {
+	canChange := access_model.CanDoerManageOrgRepoCollaboratorTeam(ctx, repo, perm)
 	if !canChange {
-		ctx.APIError(http.StatusForbidden, "Must be an organization owner")
+		ctx.APIError(http.StatusForbidden, "Must have permission to manage team repository access")
 		return false
 	}
 	return true
@@ -703,18 +695,11 @@ func AddTeamRepository(ctx *context.APIContext) {
 	//   "404":
 	//     "$ref": "#/responses/notFound"
 
-	repo := getRepositoryByParams(ctx)
+	repo, perm := getRepositoryByParams(ctx)
 	if ctx.Written() {
 		return
 	}
-	if !canChangeTeamRepository(ctx) {
-		return
-	}
-	if access, err := access_model.AccessLevel(ctx, ctx.Doer, repo); err != nil {
-		ctx.APIErrorInternal(err)
-		return
-	} else if access < perm.AccessModeAdmin {
-		ctx.APIError(http.StatusForbidden, "Must have admin-level access to the repository")
+	if !canManageRepoCollaboratorTeam(ctx, repo, perm) {
 		return
 	}
 	if err := repo_service.TeamAddRepository(ctx, ctx.Org.Team, repo); err != nil {
@@ -758,18 +743,11 @@ func RemoveTeamRepository(ctx *context.APIContext) {
 	//   "404":
 	//     "$ref": "#/responses/notFound"
 
-	repo := getRepositoryByParams(ctx)
+	repo, perm := getRepositoryByParams(ctx)
 	if ctx.Written() {
 		return
 	}
-	if !canChangeTeamRepository(ctx) {
-		return
-	}
-	if access, err := access_model.AccessLevel(ctx, ctx.Doer, repo); err != nil {
-		ctx.APIErrorInternal(err)
-		return
-	} else if access < perm.AccessModeAdmin {
-		ctx.APIError(http.StatusForbidden, "Must have admin-level access to the repository")
+	if !canManageRepoCollaboratorTeam(ctx, repo, perm) {
 		return
 	}
 	if err := repo_service.RemoveRepositoryFromTeam(ctx, ctx.Org.Team, repo.ID); err != nil {

routers/api/v1/repo/repo.go

@@ -1154,15 +1154,6 @@ func Delete(ctx *context.APIContext) {
 	owner := ctx.Repo.Owner
 	repo := ctx.Repo.Repository
 
-	canDelete, err := repo_module.CanUserDelete(ctx, repo, ctx.Doer)
-	if err != nil {
-		ctx.APIErrorInternal(err)
-		return
-	} else if !canDelete {
-		ctx.APIError(http.StatusForbidden, "Given user is not owner of organization.")
-		return
-	}
-
 	if ctx.Repo.GitRepo != nil {
 		ctx.Repo.GitRepo.Close()
 	}

routers/api/v1/repo/teams.go

@@ -8,6 +8,7 @@ import (
 	"net/http"
 
 	"gitea.dev/models/organization"
+	access_model "gitea.dev/models/perm/access"
 	"gitea.dev/services/context"
 	"gitea.dev/services/convert"
 	repo_service "gitea.dev/services/repository"
@@ -188,11 +189,7 @@ func DeleteTeam(ctx *context.APIContext) {
 }
 
 func changeRepoTeam(ctx *context.APIContext, add bool) {
-	if !ctx.Repo.Owner.IsOrganization() {
-		ctx.APIError(http.StatusMethodNotAllowed, "repo is not owned by an organization")
-		return
-	}
-	if !canChangeRepoTeam(ctx) {
+	if !canChangeOrgRepoTeam(ctx) {
 		return
 	}
 
@@ -224,14 +221,10 @@ func changeRepoTeam(ctx *context.APIContext, add bool) {
 	ctx.Status(http.StatusNoContent)
 }
 
-func canChangeRepoTeam(ctx *context.APIContext) bool {
-	canChange, err := organization.OrgFromUser(ctx.Repo.Owner).CanChangeRepoTeamAccess(ctx, ctx.Doer)
-	if err != nil {
-		ctx.APIErrorInternal(err)
-		return false
-	}
+func canChangeOrgRepoTeam(ctx *context.APIContext) bool {
+	canChange := access_model.CanDoerManageOrgRepoCollaboratorTeam(ctx, ctx.Repo.Repository, &ctx.Repo.Permission)
 	if !canChange {
-		ctx.APIError(http.StatusForbidden, "Must be an organization owner")
+		ctx.APIError(http.StatusForbidden, "No permission to change organization repository's team")
 		return false
 	}
 	return true

routers/web/repo/issue_view.go

@@ -74,11 +74,7 @@ func roleDescriptor(ctx *context.Context, repo *repo_model.Repository, poster *u
 			return roleDesc, nil
 		}
 		// Otherwise (poster is site admin), check if poster is the real repo admin.
-		isRealRepoAdmin, err := access_model.IsUserRealRepoAdmin(ctx, repo, poster)
-		if err != nil {
-			return roleDesc, err
-		}
-		if isRealRepoAdmin {
+		if access_model.IsUserRealRepoAdmin(ctx, repo, poster) {
 			roleDesc.RoleInRepo = issues_model.RoleRepoOwner
 			return roleDesc, nil
 		}

routers/web/repo/setting/collaboration.go

@@ -10,6 +10,7 @@ import (
 
 	"gitea.dev/models/organization"
 	"gitea.dev/models/perm"
+	"gitea.dev/models/perm/access"
 	repo_model "gitea.dev/models/repo"
 	unit_model "gitea.dev/models/unit"
 	user_model "gitea.dev/models/user"
@@ -43,14 +44,7 @@ func Collaboration(ctx *context.Context) {
 	ctx.Data["OrgName"] = ctx.Repo.Repository.OwnerName
 	ctx.Data["Org"] = ctx.Repo.Repository.Owner
 	ctx.Data["Units"] = unit_model.Units
-	if ctx.Repo.Owner.IsOrganization() {
-		ctx.Data["CanChangeRepoTeamAccess"], err = organization.OrgFromUser(ctx.Repo.Owner).CanChangeRepoTeamAccess(ctx, ctx.Doer)
-		if err != nil {
-			ctx.ServerError("CanChangeRepoTeamAccess", err)
-			return
-		}
-	}
-
+	ctx.Data["CanChangeRepoTeamAccess"] = access.CanDoerManageOrgRepoCollaboratorTeam(ctx, ctx.Repo.Repository, &ctx.Repo.Permission)
 	ctx.HTML(http.StatusOK, tplCollaboration)
 }
 
@@ -162,7 +156,7 @@ func DeleteCollaboration(ctx *context.Context) {
 
 // AddTeamPost response for adding a team to a repository
 func AddTeamPost(ctx *context.Context) {
-	if !canChangeRepoTeamAccess(ctx) {
+	if !canManageRepoCollaboratorTeam(ctx) {
 		return
 	}
 
@@ -206,7 +200,7 @@ func AddTeamPost(ctx *context.Context) {
 
 // DeleteTeam response for deleting a team from a repository
 func DeleteTeam(ctx *context.Context) {
-	if !canChangeRepoTeamAccess(ctx) {
+	if !canManageRepoCollaboratorTeam(ctx) {
 		return
 	}
 
@@ -225,12 +219,8 @@ func DeleteTeam(ctx *context.Context) {
 	ctx.JSONRedirect(ctx.Repo.RepoLink + "/settings/collaboration")
 }
 
-func canChangeRepoTeamAccess(ctx *context.Context) bool {
-	canChange, err := organization.OrgFromUser(ctx.Repo.Owner).CanChangeRepoTeamAccess(ctx, ctx.Doer)
-	if err != nil {
-		ctx.ServerError("CanChangeRepoTeamAccess", err)
-		return false
-	}
+func canManageRepoCollaboratorTeam(ctx *context.Context) bool {
+	canChange := access.CanDoerManageOrgRepoCollaboratorTeam(ctx, ctx.Repo.Repository, &ctx.Repo.Permission)
 	if !canChange {
 		ctx.Flash.Error(ctx.Tr("repo.settings.change_team_access_not_allowed"))
 		ctx.Redirect(ctx.Repo.RepoLink + "/settings/collaboration")

routers/web/repo/setting/setting.go

@@ -13,6 +13,7 @@ import (
 
 	"gitea.dev/models/db"
 	"gitea.dev/models/organization"
+	access_model "gitea.dev/models/perm/access"
 	repo_model "gitea.dev/models/repo"
 	unit_model "gitea.dev/models/unit"
 	user_model "gitea.dev/models/user"
@@ -55,6 +56,14 @@ type selectOption struct {
 	Selected bool
 }
 
+func canManageRepoDangerZone(ctx *context.Context) bool {
+	if !access_model.CanDoerManageRepoDangerZone(ctx, ctx.Doer, ctx.Repo.Repository, &ctx.Repo.Permission) {
+		ctx.JSONErrorNotFound()
+		return false
+	}
+	return true
+}
+
 // SettingsCtxData is a middleware that sets all the general context data for the
 // settings template.
 func SettingsCtxData(ctx *context.Context) {
@@ -67,6 +76,7 @@ func SettingsCtxData(ctx *context.Context) {
 	ctx.Data["DefaultMirrorInterval"] = setting.Mirror.DefaultInterval
 	ctx.Data["MinimumMirrorInterval"] = setting.Mirror.MinInterval
 	ctx.Data["CanConvertFork"] = ctx.Repo.Repository.IsFork && ctx.Doer.CanCreateRepoIn(ctx.Repo.Repository.Owner)
+	ctx.Data["CanManagerDangerZone"] = access_model.CanDoerManageRepoDangerZone(ctx, ctx.Doer, ctx.Repo.Repository, &ctx.Repo.Permission)
 
 	signing, _ := git.GetSigningKey(ctx)
 	ctx.Data["SigningKeyAvailable"] = signing != nil
@@ -774,12 +784,12 @@ func handleSettingsPostAdminIndex(ctx *context.Context) {
 }
 
 func handleSettingsPostConvert(ctx *context.Context) {
-	form := web.GetForm[*forms.RepoSettingForm](ctx)
-	repo := ctx.Repo.Repository
-	if !ctx.Repo.Permission.IsOwner() {
-		ctx.JSONErrorNotFound()
+	if !canManageRepoDangerZone(ctx) {
 		return
 	}
+
+	form := web.GetForm[*forms.RepoSettingForm](ctx)
+	repo := ctx.Repo.Repository
 	if repo.Name != form.RepoName {
 		ctx.JSONError(ctx.Tr("form.enterred_invalid_repo_name"))
 		return
@@ -804,12 +814,12 @@ func handleSettingsPostConvert(ctx *context.Context) {
 }
 
 func handleSettingsPostConvertFork(ctx *context.Context) {
-	form := web.GetForm[*forms.RepoSettingForm](ctx)
-	repo := ctx.Repo.Repository
-	if !ctx.Repo.Permission.IsOwner() {
-		ctx.JSONErrorNotFound()
+	if !canManageRepoDangerZone(ctx) {
 		return
 	}
+
+	form := web.GetForm[*forms.RepoSettingForm](ctx)
+	repo := ctx.Repo.Repository
 	if err := repo.LoadOwner(ctx); err != nil {
 		ctx.ServerError("Convert Fork", err)
 		return
@@ -844,12 +854,12 @@ func handleSettingsPostConvertFork(ctx *context.Context) {
 }
 
 func handleSettingsPostTransfer(ctx *context.Context) {
-	form := web.GetForm[*forms.RepoSettingForm](ctx)
-	repo := ctx.Repo.Repository
-	if !ctx.Repo.Permission.IsOwner() {
-		ctx.JSONErrorNotFound()
+	if !canManageRepoDangerZone(ctx) {
 		return
 	}
+
+	form := web.GetForm[*forms.RepoSettingForm](ctx)
+	repo := ctx.Repo.Repository
 	if repo.Name != form.RepoName {
 		ctx.JSONError(ctx.Tr("form.enterred_invalid_repo_name"))
 		return
@@ -908,12 +918,11 @@ func handleSettingsPostTransfer(ctx *context.Context) {
 }
 
 func handleSettingsPostCancelTransfer(ctx *context.Context) {
-	repo := ctx.Repo.Repository
-	if !ctx.Repo.Permission.IsOwner() {
-		ctx.HTTPError(http.StatusNotFound)
+	if !canManageRepoDangerZone(ctx) {
 		return
 	}
 
+	repo := ctx.Repo.Repository
 	repoTransfer, err := repo_model.GetPendingRepositoryTransfer(ctx, ctx.Repo.Repository)
 	if err != nil {
 		if repo_model.IsErrNoPendingTransfer(err) {
@@ -936,12 +945,12 @@ func handleSettingsPostCancelTransfer(ctx *context.Context) {
 }
 
 func handleSettingsPostDelete(ctx *context.Context) {
-	form := web.GetForm[*forms.RepoSettingForm](ctx)
-	repo := ctx.Repo.Repository
-	if !ctx.Repo.Permission.IsOwner() {
-		ctx.JSONErrorNotFound()
+	if !canManageRepoDangerZone(ctx) {
 		return
 	}
+
+	form := web.GetForm[*forms.RepoSettingForm](ctx)
+	repo := ctx.Repo.Repository
 	if repo.Name != form.RepoName {
 		ctx.JSONError(ctx.Tr("form.enterred_invalid_repo_name"))
 		return
@@ -963,12 +972,11 @@ func handleSettingsPostDelete(ctx *context.Context) {
 }
 
 func handleSettingsPostDeleteWiki(ctx *context.Context) {
-	form := web.GetForm[*forms.RepoSettingForm](ctx)
-	repo := ctx.Repo.Repository
-	if !ctx.Repo.Permission.IsOwner() {
-		ctx.JSONErrorNotFound()
+	if !canManageRepoDangerZone(ctx) {
 		return
 	}
+	form := web.GetForm[*forms.RepoSettingForm](ctx)
+	repo := ctx.Repo.Repository
 	if repo.Name != form.RepoName {
 		ctx.JSONError(ctx.Tr("form.enterred_invalid_repo_name"))
 		return
@@ -985,12 +993,11 @@ func handleSettingsPostDeleteWiki(ctx *context.Context) {
 }
 
 func handleSettingsPostArchive(ctx *context.Context) {
-	repo := ctx.Repo.Repository
-	if !ctx.Repo.Permission.IsOwner() {
-		ctx.HTTPError(http.StatusForbidden)
+	if !canManageRepoDangerZone(ctx) {
 		return
 	}
 
+	repo := ctx.Repo.Repository
 	if repo.IsMirror {
 		ctx.Flash.Error(ctx.Tr("repo.settings.archive.error_ismirror"))
 		ctx.Redirect(ctx.Repo.RepoLink + "/settings")
@@ -1018,12 +1025,11 @@ func handleSettingsPostArchive(ctx *context.Context) {
 }
 
 func handleSettingsPostUnarchive(ctx *context.Context) {
-	repo := ctx.Repo.Repository
-	if !ctx.Repo.Permission.IsOwner() {
-		ctx.HTTPError(http.StatusForbidden)
+	if !canManageRepoDangerZone(ctx) {
 		return
 	}
 
+	repo := ctx.Repo.Repository
 	if err := repo_model.SetArchiveRepoState(ctx, repo, false); err != nil {
 		log.Error("Tried to unarchive a repo: %s", err)
 		ctx.Flash.Error(ctx.Tr("repo.settings.unarchive.error"))
@@ -1047,6 +1053,10 @@ func handleSettingsPostUnarchive(ctx *context.Context) {
 }
 
 func handleSettingsPostVisibility(ctx *context.Context) {
+	if !canManageRepoDangerZone(ctx) {
+		return
+	}
+
 	repo := ctx.Repo.Repository
 	if repo.IsFork {
 		ctx.JSONError(ctx.Tr("repo.settings.visibility.fork_error"))
@@ -1055,7 +1065,7 @@ func handleSettingsPostVisibility(ctx *context.Context) {
 
 	private := ctx.FormOptionalBool("private").ValueOrDefault(true) // default to true for privacy & safety
 
-	// when ForcePrivate enabled, you could change public repo to private, but only admin users can change private to public
+	// when ForcePrivate enabled, you could change public repo to private, only site admin users can change private to public
 	if !private && setting.Repository.ForcePrivate && !ctx.Doer.IsAdmin {
 		ctx.JSONError(ctx.Tr("form.repository_force_private"))
 		return

routers/web/repo/setting/settings_test.go

@@ -11,6 +11,7 @@ import (
 	asymkey_model "gitea.dev/models/asymkey"
 	"gitea.dev/models/organization"
 	"gitea.dev/models/perm"
+	access_model "gitea.dev/models/perm/access"
 	repo_model "gitea.dev/models/repo"
 	"gitea.dev/models/unittest"
 	user_model "gitea.dev/models/user"
@@ -173,143 +174,48 @@ func TestCollaborationPost_NonExistentUser(t *testing.T) {
 
 func TestAddTeamPost(t *testing.T) {
 	unittest.PrepareTestEnv(t)
-	ctx, _ := contexttest.MockContext(t, "org26/repo43")
+	org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 26})
+	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 43})
+	team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 11})
+	repo.Owner = org
 
-	ctx.Req.Form.Set("team", "team11")
-
-	org := &user_model.User{
-		LowerName: "org26",
-		Type:      user_model.UserTypeOrganization,
+	testAddTeamPost := func(t *testing.T, teamName string, repoAdminChangeTeamAccess bool) *context.Context {
+		ctx, _ := contexttest.MockContext(t, "org26/repo43")
+		ctx.Req.Form.Set("team", teamName)
+		org.RepoAdminChangeTeamAccess = repoAdminChangeTeamAccess
+		ctx.Repo = &context.Repository{
+			Permission: access_model.Permission{AccessMode: perm.AccessModeAdmin},
+			Owner:      repo.Owner,
+			Repository: repo,
+		}
+		ctx.Doer = &user_model.User{ID: 1, IsAdmin: true}
+		AddTeamPost(ctx)
+		return ctx
 	}
 
-	team := &organization.Team{
-		ID:    11,
-		OrgID: 26,
-	}
-
-	re := &repo_model.Repository{
-		ID:      43,
-		Owner:   org,
-		OwnerID: 26,
-	}
-
-	repo := &context.Repository{
-		Owner: &user_model.User{
-			ID:                        26,
-			LowerName:                 "org26",
-			RepoAdminChangeTeamAccess: true,
-		},
-		Repository: re,
-	}
-
-	ctx.Repo = repo
-
-	AddTeamPost(ctx)
-
-	assert.True(t, repo_service.HasRepository(t.Context(), team, re.ID))
-	assert.Equal(t, http.StatusSeeOther, ctx.Resp.WrittenStatus())
-	assert.Empty(t, ctx.Flash.ErrorMsg)
-}
-
-func TestAddTeamPost_NotAllowed(t *testing.T) {
-	unittest.PrepareTestEnv(t)
-	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 32})
-	require.NoError(t, repo.LoadOwner(t.Context()))
-	adminTeam := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 12})
-	targetTeam := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 2})
-	require.NoError(t, repo_service.TeamAddRepository(t.Context(), adminTeam, repo))
-	doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 28})
-	repoContext := &context.Repository{Owner: repo.Owner, Repository: repo}
-	renderCtx, _ := contexttest.MockContext(t, repo.Link()+"/settings/collaboration")
-	renderCtx.Repo = repoContext
-	renderCtx.Doer = doer
-	Collaboration(renderCtx)
-	assert.Equal(t, false, renderCtx.Data["CanChangeRepoTeamAccess"])
-
-	ctx, _ := contexttest.MockContext(t, repo.Link()+"/settings/collaboration")
-	ctx.Req.Form.Set("team", targetTeam.Name)
-	ctx.Repo = repoContext
-	ctx.Doer = doer
-
-	AddTeamPost(ctx)
-
-	assert.False(t, repo_service.HasRepository(t.Context(), targetTeam, repo.ID))
-	assert.Equal(t, http.StatusSeeOther, ctx.Resp.WrittenStatus())
-	assert.NotEmpty(t, ctx.Flash.ErrorMsg)
-}
-
-func TestAddTeamPost_AddTeamTwice(t *testing.T) {
-	unittest.PrepareTestEnv(t)
-	ctx, _ := contexttest.MockContext(t, "org26/repo43")
-
-	ctx.Req.Form.Set("team", "team11")
-
-	org := &user_model.User{
-		LowerName: "org26",
-		Type:      user_model.UserTypeOrganization,
-	}
-
-	team := &organization.Team{
-		ID:    11,
-		OrgID: 26,
-	}
-
-	re := &repo_model.Repository{
-		ID:      43,
-		Owner:   org,
-		OwnerID: 26,
-	}
-
-	repo := &context.Repository{
-		Owner: &user_model.User{
-			ID:                        26,
-			LowerName:                 "org26",
-			RepoAdminChangeTeamAccess: true,
-		},
-		Repository: re,
-	}
-
-	ctx.Repo = repo
-
-	AddTeamPost(ctx)
-
-	AddTeamPost(ctx)
-	assert.True(t, repo_service.HasRepository(t.Context(), team, re.ID))
-	assert.Equal(t, http.StatusSeeOther, ctx.Resp.WrittenStatus())
-	assert.NotEmpty(t, ctx.Flash.ErrorMsg)
-}
-
-func TestAddTeamPost_NonExistentTeam(t *testing.T) {
-	unittest.PrepareTestEnv(t)
-	ctx, _ := contexttest.MockContext(t, "org26/repo43")
-
-	ctx.Req.Form.Set("team", "team-non-existent")
-
-	org := &user_model.User{
-		LowerName: "org26",
-		Type:      user_model.UserTypeOrganization,
-	}
-
-	re := &repo_model.Repository{
-		ID:      43,
-		Owner:   org,
-		OwnerID: 26,
-	}
-
-	repo := &context.Repository{
-		Owner: &user_model.User{
-			ID:                        26,
-			LowerName:                 "org26",
-			RepoAdminChangeTeamAccess: true,
-		},
-		Repository: re,
-	}
-
-	ctx.Repo = repo
-
-	AddTeamPost(ctx)
-	assert.Equal(t, http.StatusSeeOther, ctx.Resp.WrittenStatus())
-	assert.NotEmpty(t, ctx.Flash.ErrorMsg)
+	t.Run("NonExisting", func(t *testing.T) {
+		ctx := testAddTeamPost(t, "team-not-exist", true)
+		assert.Equal(t, http.StatusSeeOther, ctx.Resp.WrittenStatus())
+		assert.Contains(t, ctx.Flash.ErrorMsg, "form.team_not_exist")
+	})
+	t.Run("NotAllowed", func(t *testing.T) {
+		ctx := testAddTeamPost(t, team.Name, false)
+		assert.False(t, repo_service.HasRepository(t.Context(), team, repo.ID))
+		assert.Equal(t, http.StatusSeeOther, ctx.Resp.WrittenStatus())
+		assert.Contains(t, ctx.Flash.ErrorMsg, "repo.settings.change_team_access_not_allowed")
+	})
+	t.Run("Allowed", func(t *testing.T) {
+		ctx := testAddTeamPost(t, team.Name, true)
+		assert.True(t, repo_service.HasRepository(t.Context(), team, repo.ID))
+		assert.Equal(t, http.StatusSeeOther, ctx.Resp.WrittenStatus())
+		assert.Empty(t, ctx.Flash.ErrorMsg)
+		t.Run("Twice", func(t *testing.T) {
+			ctx := testAddTeamPost(t, team.Name, true)
+			assert.True(t, repo_service.HasRepository(t.Context(), team, repo.ID))
+			assert.Equal(t, http.StatusSeeOther, ctx.Resp.WrittenStatus())
+			assert.Contains(t, ctx.Flash.ErrorMsg, "repo.settings.add_team_duplicate")
+		})
+	})
 }
 
 func TestDeleteTeam(t *testing.T) {
@@ -317,37 +223,21 @@ func TestDeleteTeam(t *testing.T) {
 	ctx, _ := contexttest.MockContext(t, "org3/team1/repo3")
 
 	ctx.Req.Form.Set("id", "2")
-
-	org := &user_model.User{
-		LowerName: "org3",
-		Type:      user_model.UserTypeOrganization,
+	org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3})
+	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3})
+	team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 2})
+	repo.Owner = org
+	org.RepoAdminChangeTeamAccess = true
+	ctx.Repo = &context.Repository{
+		Permission: access_model.Permission{AccessMode: perm.AccessModeAdmin},
+		Owner:      repo.Owner,
+		Repository: repo,
 	}
+	ctx.Doer = &user_model.User{ID: 1, IsAdmin: true}
 
-	team := &organization.Team{
-		ID:    2,
-		OrgID: 3,
-	}
-
-	re := &repo_model.Repository{
-		ID:      3,
-		Owner:   org,
-		OwnerID: 3,
-	}
-
-	repo := &context.Repository{
-		Owner: &user_model.User{
-			ID:                        3,
-			LowerName:                 "org3",
-			RepoAdminChangeTeamAccess: true,
-		},
-		Repository: re,
-	}
-
-	ctx.Repo = repo
-
+	assert.True(t, repo_service.HasRepository(t.Context(), team, repo.ID))
 	DeleteTeam(ctx)
-
-	assert.False(t, repo_service.HasRepository(t.Context(), team, re.ID))
+	assert.False(t, repo_service.HasRepository(t.Context(), team, repo.ID))
 }
 
 func TestHandleSettingsPostMirrorPreservesExistingUsername(t *testing.T) {

routers/web/web.go

@@ -987,11 +987,9 @@ func registerWebRoutes(m *web.Router, webAuth *AuthMiddleware) {
 			m.Post("/teams/{team}/action/repo/{action}", org.TeamsRepoAction)
 		}, context.OrgAssignment(context.OrgAssignmentOptions{RequireMember: true, RequireTeamMember: true}))
 
-		// require member/team-admin permission (old logic is: requireMember=true, requireTeamAdmin=true)
-		// but it doesn't seem right: requireTeamAdmin does nothing
 		m.Group("/{org}", func() {
 			m.Get("/teams/-/search", org.SearchTeam)
-		}, context.OrgAssignment(context.OrgAssignmentOptions{RequireMember: true, RequireTeamAdmin: true}))
+		}, context.OrgAssignment(context.OrgAssignmentOptions{RequireMember: true}))
 
 		// require owner permission
 		m.Group("/{org}", func() {