diff --git a/models/organization/team_fix.go b/models/organization/team_fix.go new file mode 100644 index 00000000000..42f852167d3 --- /dev/null +++ b/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 +} diff --git a/services/doctor/dbconsistency.go b/services/doctor/dbconsistency.go index 7cb7445148a..1527a462866 100644 --- a/services/doctor/dbconsistency.go +++ b/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 diff --git a/tests/integration/api_team_test.go b/tests/integration/api_team_test.go index d14c66ff2ca..f7bfd4e06a2 100644 --- a/tests/integration/api_team_test.go +++ b/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" - editFalse := false - teamToEdit := &api.EditTeamOption{ + editDescription = "team 1" + editFalse = false + teamToEdit = &api.EditTeamOption{ Name: "teamone", Description: &editDescription, Permission: "admin", diff --git a/tests/integration/org_test.go b/tests/integration/org_test.go index ef4ef2bb9b4..b5186cb70aa 100644 --- a/tests/integration/org_test.go +++ b/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) + } +}