Fix test

models/organization/team_fix.go

@@ -0,0 +1,72 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package organization
+
+import (
+	"context"
+	"strings"
+
+	"code.gitea.io/gitea/models/db"
+	"code.gitea.io/gitea/models/perm"
+	"code.gitea.io/gitea/models/unit"
+
+	"xorm.io/builder"
+)
+
+// CountInconsistentOwnerTeams returns the amount of owner teams that any of
+// their access modes not set to the right value. For external trackers and external wikis
+// it should be Read and otherwise it should be Owner.
+func CountInconsistentOwnerTeams(ctx context.Context) (int64, error) {
+	cnt1, err := db.GetEngine(ctx).Table("team").
+		Join("INNER", "team_unit", "`team`.id = `team_unit`.team_id").
+		Where("`team`.lower_name = ?", strings.ToLower(OwnerTeamName)).
+		And(builder.Or(builder.Eq{"`team_unit`.`type`": unit.TypeExternalTracker}, builder.Eq{"`team_unit`.`type`": unit.TypeExternalWiki})).
+		And("`team_unit`.`access_mode` <> ?", perm.AccessModeRead).
+		GroupBy("`team`.`id`").
+		Count()
+	if err != nil {
+		return 0, err
+	}
+
+	cnt2, err := db.GetEngine(ctx).Table("team").
+		Join("INNER", "team_unit", "`team`.id = `team_unit`.team_id").
+		Where("`team`.lower_name = ?", strings.ToLower(OwnerTeamName)).
+		And(builder.And(builder.Neq{"`team_unit`.`type`": unit.TypeExternalTracker}, builder.Neq{"`team_unit`.`type`": unit.TypeExternalWiki})).
+		And("`team_unit`.`access_mode` <> ?", perm.AccessModeOwner).
+		GroupBy("`team_unit`.team_id").
+		Count()
+	if err != nil {
+		return 0, err
+	}
+	return cnt1 + cnt2, nil
+}
+
+// FixInconsistentOwnerTeams fixes inconsistent owner teams that  of
+// their access modes not set to the right value. For external trackers and external wikis
+// it should be Read and otherwise it should be Owner.
+func FixInconsistentOwnerTeams(ctx context.Context) (int64, error) {
+	subQuery := builder.Select("id").From("team").Where(builder.Eq{"lower_name": strings.ToLower(OwnerTeamName)})
+	updated1, err := db.GetEngine(ctx).Table("team_unit").
+		Where(builder.Or(builder.Eq{"`team_unit`.`type`": unit.TypeExternalTracker}, builder.Eq{"`team_unit`.`type`": unit.TypeExternalWiki})).
+		In("team_id", subQuery).
+		Cols("access_mode").
+		Update(&TeamUnit{
+			AccessMode: perm.AccessModeRead,
+		})
+	if err != nil {
+		return 0, err
+	}
+
+	updated2, err := db.GetEngine(ctx).Table("team_unit").
+		Where(builder.And(builder.Neq{"`team_unit`.`type`": unit.TypeExternalTracker}, builder.Neq{"`team_unit`.`type`": unit.TypeExternalWiki})).
+		In("team_id", subQuery).
+		Cols("access_mode").
+		Update(&TeamUnit{
+			AccessMode: perm.AccessModeOwner,
+		})
+	if err != nil {
+		return 0, err
+	}
+	return updated1 + updated2, nil
+}

services/doctor/dbconsistency.go

@@ -11,6 +11,7 @@ import (
 	"code.gitea.io/gitea/models/db"
 	issues_model "code.gitea.io/gitea/models/issues"
 	"code.gitea.io/gitea/models/migrations"
+	org_model "code.gitea.io/gitea/models/organization"
 	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/setting"
@@ -164,6 +165,12 @@ func prepareDBConsistencyChecks() []consistencyCheck {
 			Fixer:        repo_model.DeleteOrphanedTopics,
 			FixedMessage: "Removed",
 		},
+		{
+			Name:         "Owner teams with wrong admin access unit",
+			Counter:      org_model.CountInconsistentOwnerTeams,
+			Fixer:        org_model.FixInconsistentOwnerTeams,
+			FixedMessage: "Fixed",
+		},
 	}
 
 	// TODO: function to recalc all counters

tests/integration/api_team_test.go

@@ -12,6 +12,7 @@ import (
 	auth_model "code.gitea.io/gitea/models/auth"
 	"code.gitea.io/gitea/models/db"
 	"code.gitea.io/gitea/models/organization"
+	org_model "code.gitea.io/gitea/models/organization"
 	"code.gitea.io/gitea/models/perm"
 	"code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/models/unit"
@@ -42,6 +43,7 @@ func TestAPITeam(t *testing.T) {
 	DecodeJSON(t, resp, &apiTeam)
 	assert.EqualValues(t, team.ID, apiTeam.ID)
 	assert.Equal(t, team.Name, apiTeam.Name)
+	assert.Equal(t, team.Description, apiTeam.Description)
 	assert.EqualValues(t, convert.ToOrganization(db.DefaultContext, org), apiTeam.Organization)
 
 	// non team member user will not access the teams details
@@ -59,10 +61,30 @@ func TestAPITeam(t *testing.T) {
 
 	// Get an admin user able to create, update and delete teams.
 	user = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
+	org = unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 6})
 	session = loginUser(t, user.Name)
 	token = getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteOrganization)
 
-	org = unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 6})
+	// edit owner team
+	editDescription := "this is owner team"
+	editFalse := false
+	teamToEdit := &api.EditTeamOption{
+		Name:                    "not_owner_team", // should be ignored
+		Description:             &editDescription,
+		Permission:              "admin",                                              // should be ignored
+		IncludesAllRepositories: &editFalse,                                           // should be ignored
+		Units:                   []string{"repo.code", "repo.pulls", "repo.releases"}, // should be ignored
+	}
+
+	req = NewRequestWithJSON(t, "PATCH", fmt.Sprintf("/api/v1/teams/%d", team.ID), teamToEdit).
+		AddTokenAuth(token)
+	resp = MakeRequest(t, req, http.StatusOK)
+	apiTeam = api.Team{}
+	DecodeJSON(t, resp, &apiTeam)
+	checkTeamResponse(t, "Edit Owner Team", &apiTeam, org_model.OwnerTeamName, *teamToEdit.Description, true,
+		"owner", unit.AllUnitKeyNames(), nil)
+	checkTeamBean(t, apiTeam.ID, org_model.OwnerTeamName, *teamToEdit.Description, true,
+		"owner", unit.AllUnitKeyNames(), nil)
 
 	// Create team.
 	teamToCreate := &api.CreateTeamOption{
@@ -84,9 +106,9 @@ func TestAPITeam(t *testing.T) {
 	teamID := apiTeam.ID
 
 	// Edit team.
-	editDescription := "team 1"
+	editDescription = "team 1"
-	editFalse := false
+	editFalse = false
-	teamToEdit := &api.EditTeamOption{
+	teamToEdit = &api.EditTeamOption{
 		Name:                    "teamone",
 		Description:             &editDescription,
 		Permission:              "admin",

tests/integration/org_test.go

@@ -10,6 +10,10 @@ import (
 	"testing"
 
 	auth_model "code.gitea.io/gitea/models/auth"
+	"code.gitea.io/gitea/models/db"
+	"code.gitea.io/gitea/models/organization"
+	org_model "code.gitea.io/gitea/models/organization"
+	"code.gitea.io/gitea/models/perm"
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
 	api "code.gitea.io/gitea/modules/structs"
@@ -218,3 +222,39 @@ func TestTeamSearch(t *testing.T) {
 	req = NewRequestf(t, "GET", "/org/%s/teams/-/search?q=%s", org.Name, "team")
 	session.MakeRequest(t, req, http.StatusNotFound)
 }
+
+func TestOwnerTeamsEdit(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+
+	user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
+	org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3, Type: user_model.UserTypeOrganization})
+	session := loginUser(t, user1.Name)
+
+	team := unittest.AssertExistsAndLoadBean(t, &organization.Team{OrgID: org.ID, Name: org_model.OwnerTeamName})
+	assert.EqualValues(t, perm.AccessModeOwner, team.AccessMode)
+	assert.EqualValues(t, "", team.Description)
+	assert.NoError(t, team.LoadUnits(db.DefaultContext))
+	assert.EqualValues(t, 7, len(team.Units))
+	for _, unit := range team.Units {
+		assert.EqualValues(t, perm.AccessModeOwner, unit.AccessMode)
+	}
+
+	req := NewRequestWithValues(t, "POST", fmt.Sprintf("/org/%s/teams/owners/edit", org.Name), map[string]string{
+		"_csrf":       GetUserCSRFToken(t, session),
+		"team_name":   "Not Owners", // This change should be ignored
+		"description": "Just a description",
+	})
+	session.MakeRequest(t, req, http.StatusSeeOther)
+
+	// reload team
+	team = unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: team.ID})
+	assert.EqualValues(t, perm.AccessModeOwner, team.AccessMode)
+	assert.EqualValues(t, org_model.OwnerTeamName, team.Name)
+	assert.EqualValues(t, "Just a description", team.Description)
+
+	assert.NoError(t, team.LoadUnits(db.DefaultContext))
+	assert.EqualValues(t, 7, len(team.Units))
+	for _, unit := range team.Units {
+		assert.EqualValues(t, perm.AccessModeOwner, unit.AccessMode)
+	}
+}