From 0a4078558ea298bfb90edd3a66028f74aa0cd261 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 6 Oct 2026 13:02:08 +0800 Subject: [PATCH] fix: migrate broken team authorize access mode (#39579) (#39580) backport #39579 --- modelmigration/migrations.go | 5 +++ modelmigration/v29/main_test.go | 14 ++++++++ modelmigration/v29/v356.go | 25 ++++++++++++++ modelmigration/v29/v357.go | 25 ++++++++++++++ modelmigration/v29/v357_test.go | 55 ++++++++++++++++++++++++++++++ models/actions/run.go | 6 ++-- routers/api/v1/org/team.go | 1 + tests/integration/api_team_test.go | 19 +++++++++++ 8 files changed, 147 insertions(+), 3 deletions(-) create mode 100644 modelmigration/v29/main_test.go create mode 100644 modelmigration/v29/v356.go create mode 100644 modelmigration/v29/v357.go create mode 100644 modelmigration/v29/v357_test.go diff --git a/modelmigration/migrations.go b/modelmigration/migrations.go index 694cf0257c6..6db32b6b202 100644 --- a/modelmigration/migrations.go +++ b/modelmigration/migrations.go @@ -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" @@ -429,6 +430,10 @@ func prepareMigrationTasks() []*migration { newMigration(353, "Add audit event table", v28.AddAuditEventTable), newMigration(354, "Add Actions job queue indexes", v28.AddActionQueueIndexes), newMigration(355, "Add AutoMerge merged_commit_id column", v28.AddAutoMergeMergedCommitID), + // Gitea 28.0.0 ends at migration ID number 355 (database version 356) + // HERE: 2 migrations from 29 are added since they don't change database structure + newMigration(356, "Add index on action_run commit_sha", v29.AddActionRunCommitSHAIndex), + newMigration(357, "Normalize legacy team authorize values", v29.NormalizeLegacyTeamAuthorize), } return preparedMigrations } diff --git a/modelmigration/v29/main_test.go b/modelmigration/v29/main_test.go new file mode 100644 index 00000000000..de8cd867d6c --- /dev/null +++ b/modelmigration/v29/main_test.go @@ -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) +} diff --git a/modelmigration/v29/v356.go b/modelmigration/v29/v356.go new file mode 100644 index 00000000000..f85d39a45d2 --- /dev/null +++ b/modelmigration/v29/v356.go @@ -0,0 +1,25 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v29 + +import ( + "context" + + "gitea.dev/modelmigration/base" + + "xorm.io/xorm" +) + +// AddActionRunCommitSHAIndex indexes the runs lookup by commit, which the API `head_sha` filter uses. +func AddActionRunCommitSHAIndex(_ context.Context, x base.EngineMigration) error { + type ActionRun struct { + CommitSHA string `xorm:"index"` + } + + _, err := x.SyncWithOptions(xorm.SyncOptions{ + IgnoreDropIndices: true, + IgnoreConstrains: true, + }, new(ActionRun)) + return err +} diff --git a/modelmigration/v29/v357.go b/modelmigration/v29/v357.go new file mode 100644 index 00000000000..106e411f96f --- /dev/null +++ b/modelmigration/v29/v357.go @@ -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). +// authorize>=write 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 +} diff --git a/modelmigration/v29/v357_test.go b/modelmigration/v29/v357_test.go new file mode 100644 index 00000000000..e0077a8b783 --- /dev/null +++ b/modelmigration/v29/v357_test.go @@ -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)) +} diff --git a/models/actions/run.go b/models/actions/run.go index 2ca797b8f33..e76ccccdb72 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -39,9 +39,9 @@ type ActionRun struct { TriggerUserID int64 `xorm:"index"` TriggerUser *user_model.User `xorm:"-"` ScheduleID int64 - Ref string `xorm:"index"` // the commit/tag/… that caused the run - IsRefDeleted bool `xorm:"-"` - CommitSHA string + Ref string `xorm:"index"` // the commit/tag/… that caused the run + IsRefDeleted bool `xorm:"-"` + CommitSHA string `xorm:"index"` IsForkPullRequest bool // If this is triggered by a PR from a forked repository or an untrusted user, we need to check if it is approved and limit permissions when running the workflow. NeedApproval bool // may need approval if it's a fork pull request ApprovedBy int64 `xorm:"index"` // who approved diff --git a/routers/api/v1/org/team.go b/routers/api/v1/org/team.go index a256520e0da..21a8adba9bd 100644 --- a/routers/api/v1/org/team.go +++ b/routers/api/v1/org/team.go @@ -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) diff --git a/tests/integration/api_team_test.go b/tests/integration/api_team_test.go index ddc0d1eddd3..17a86ede464 100644 --- a/tests/integration/api_team_test.go +++ b/tests/integration/api_team_test.go @@ -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