fix: migrate broken team authorize access mode (#39579)

* fix:  #39571
* ref: https://github.com/go-gitea/gitea/pull/38938#pullrequestreview-4945228548
* fix the bug in `assignTeamPermissionUnits` which can result in wrong team access

---------

Signed-off-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
wxiaoguang authored and GitHub committed 2026-10-05 09:03:34 +00:00
1 parent b71967b254
commit b0d5a63e7a
7 files changed
+118 -2

No files matched your search

+3 -1
View File
@@ -33,6 +33,7 @@ import (
"gitea.dev/modelmigration/v1_8"
"gitea.dev/modelmigration/v1_9"
"gitea.dev/modelmigration/v28"
"gitea.dev/modelmigration/v29"
"gitea.dev/modules/git"
"gitea.dev/modules/log"
"gitea.dev/modules/setting"
@@ -431,7 +432,8 @@ func prepareMigrationTasks() []*migration {
newMigration(355, "Add AutoMerge merged_commit_id column", v28.AddAutoMergeMergedCommitID),
// Gitea 28.0.0 ends at migration ID number 355 (database version 356)
newMigration(356, "Add index on action_run commit_sha", v28.AddActionRunCommitSHAIndex),
newMigration(356, "Add index on action_run commit_sha", v29.AddActionRunCommitSHAIndex),
newMigration(357, "Normalize legacy team authorize values", v29.NormalizeLegacyTeamAuthorize),
}
return preparedMigrations
}
+14
View File
@@ -0,0 +1,14 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package v29
import (
"testing"
"gitea.dev/modelmigration/migrationtest"
)
func TestMain(m *testing.M) {
migrationtest.MainTest(m)
}
@@ -1,7 +1,7 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package v28
package v29
import (
"context"
+25
View File
@@ -0,0 +1,25 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package v29
import (
"context"
"gitea.dev/modelmigration/base"
)
// NormalizeLegacyTeamAuthorize sets leftover read/write authorize values to none.
// https://github.com/go-gitea/gitea/pull/34128 made non-admin teams use team_unit (authorize=none).
// Any positive authorize now means blanket access on every unit; migrating legacy read/write
// to none preserves their existing team_unit-scoped access.
func NormalizeLegacyTeamAuthorize(_ context.Context, x base.EngineMigration) error {
// AccessModeNone=0, AccessModeRead=1, AccessModeWrite=2, AccessModeAdmin=3
_, err := x.Exec(`
UPDATE team SET authorize = 0
WHERE authorize > 0 AND authorize < 3
AND EXISTS (
SELECT 1 FROM team_unit WHERE team_unit.team_id = team.id
);`)
return err
}
+55
View File
@@ -0,0 +1,55 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package v29
import (
"testing"
"gitea.dev/modelmigration/migrationtest"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestNormalizeLegacyTeamAuthorize(t *testing.T) {
type Team struct {
ID int64 `xorm:"pk"`
Authorize int
}
type TeamUnit struct {
ID int64 `xorm:"pk"`
TeamID int64 `xorm:"INDEX"`
}
x, deferrable := migrationtest.PrepareTestEnv(t, 0, new(Team), new(TeamUnit))
defer deferrable()
if x == nil || t.Failed() {
return
}
_, err := x.Insert(
&Team{ID: 1, Authorize: 4},
&Team{ID: 2, Authorize: 3},
&Team{ID: 3, Authorize: 2},
&Team{ID: 4, Authorize: 1},
&Team{ID: 5, Authorize: 0},
&TeamUnit{TeamID: 3},
)
require.NoError(t, err)
require.NoError(t, NormalizeLegacyTeamAuthorize(t.Context(), x))
get := func(id int64) int {
tBean := &Team{ID: id}
has, err := x.Get(tBean)
require.NoError(t, err)
require.True(t, has)
return tBean.Authorize
}
assert.Equal(t, 4, get(1))
assert.Equal(t, 3, get(2))
assert.Equal(t, 0, get(3)) // has team unit, reset to none
assert.Equal(t, 1, get(4)) // no team unit, kept
assert.Equal(t, 0, get(5))
}
+1
View File
@@ -167,6 +167,7 @@ func assignTeamPermissionUnits(team *organization.Team, permission string, units
oldAccessMode := team.AccessMode
oldUnitPerms := team.GetUnitsMap()
if len(unitsMap) > 0 {
team.AccessMode = perm.AccessModeNone
team.Units = make([]*organization.TeamUnit, 0, len(unitsMap))
for unitKey, p := range unitsMap {
unitType, unitPerm := unit_model.TypeFromKey(unitKey), perm.ParseAccessMode(p)
+19
View File
@@ -109,6 +109,16 @@ func TestAPITeam(t *testing.T) {
checkTeamResponse(t, "EditTeam1_DescOnly", apiTeam, teamToEdit.Name, *teamToEditDesc.Description, *teamToEdit.IncludesAllRepositories, api.AccessLevelName(teamToEdit.Permission), nil)
checkTeamBean(t, apiTeam.ID, teamToEdit.Name, *teamToEditDesc.Description, *teamToEdit.IncludesAllRepositories, api.AccessLevelName(teamToEdit.Permission), nil)
// Edit team to granular permissions, team's permission should be reset to none
req = NewRequestWithJSON(t, "PATCH", fmt.Sprintf("/api/v1/teams/%d", teamID), api.EditTeamOption{
Permission: "read",
Units: []string{"repo.code", "repo.issues"},
}).AddTokenAuth(token)
resp = MakeRequest(t, req, http.StatusOK)
apiTeam = DecodeJSON(t, resp, &api.Team{})
checkTeamResponse(t, "EditTeam1_Granular", apiTeam, teamToEdit.Name, editDescription, editFalse, api.AccessLevelNameNone, expectedTeamUnitsMap)
checkTeamBean(t, teamID, teamToEdit.Name, editDescription, editFalse, api.AccessLevelNameNone, expectedTeamUnitsMap)
// Read team.
teamRead := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: teamID})
assert.NoError(t, teamRead.LoadUnits(t.Context()))
@@ -139,6 +149,15 @@ func TestAPITeam(t *testing.T) {
checkTeamBean(t, apiTeam.ID, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories, api.AccessLevelNameNone, teamToCreate.UnitsMap)
teamID = apiTeam.ID
// Create a team with permission=write, then edit it to granular permissions later, team's permission should be reset to none
req = NewRequestWithJSON(t, "PATCH", fmt.Sprintf("/api/v1/teams/%d", teamID), api.EditTeamOption{
Permission: "write",
}).AddTokenAuth(token)
resp = MakeRequest(t, req, http.StatusOK)
apiTeam = DecodeJSON(t, resp, &api.Team{})
checkTeamResponse(t, "EditTeam2_Write", apiTeam, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories, api.AccessLevelNameWrite, nil)
checkTeamBean(t, teamID, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories, api.AccessLevelNameWrite, nil)
// Edit team.
editDescription = "team 1"
editFalse = false