From b7221bec34fd49495234a18c26e4f5d81483e102 Mon Sep 17 00:00:00 2001 From: yp05327 <576951401@qq.com> Date: Fri, 14 Apr 2023 04:06:10 +0900 Subject: [PATCH] Fix admin team access mode value in team_unit table (#24012) Same as https://github.com/go-gitea/gitea/pull/23675 Feedback: https://github.com/go-gitea/gitea/pull/23879#issuecomment-1500923636 --- models/migrations/migrations.go | 2 + models/migrations/v1_20/v252.go | 47 +++++++++++++++ routers/api/v1/org/team.go | 19 ++++++ routers/web/org/teams.go | 93 ++++++++++++++++-------------- templates/org/team/new.tmpl | 4 +- tests/integration/api_team_test.go | 31 ++++++++++ 6 files changed, 151 insertions(+), 45 deletions(-) create mode 100644 models/migrations/v1_20/v252.go diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 07240c8e6..35a18fb7f 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -481,6 +481,8 @@ var migrations = []Migration{ NewMigration("Change Container Metadata", v1_20.ChangeContainerMetadataMultiArch), // v251 -> v252 NewMigration("Fix incorrect owner team unit access mode", v1_20.FixIncorrectOwnerTeamUnitAccessMode), + // v252 -> v253 + NewMigration("Fix incorrect admin team unit access mode", v1_20.FixIncorrectAdminTeamUnitAccessMode), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v1_20/v252.go b/models/migrations/v1_20/v252.go new file mode 100644 index 000000000..ab61cd9b8 --- /dev/null +++ b/models/migrations/v1_20/v252.go @@ -0,0 +1,47 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_20 //nolint + +import ( + "code.gitea.io/gitea/modules/log" + + "xorm.io/xorm" +) + +func FixIncorrectAdminTeamUnitAccessMode(x *xorm.Engine) error { + type UnitType int + type AccessMode int + + type TeamUnit struct { + ID int64 `xorm:"pk autoincr"` + OrgID int64 `xorm:"INDEX"` + TeamID int64 `xorm:"UNIQUE(s)"` + Type UnitType `xorm:"UNIQUE(s)"` + AccessMode AccessMode + } + + const ( + // AccessModeAdmin admin access + AccessModeAdmin = 3 + ) + + sess := x.NewSession() + defer sess.Close() + + if err := sess.Begin(); err != nil { + return err + } + + count, err := sess.Table("team_unit"). + Where("team_id IN (SELECT id FROM team WHERE authorize = ?)", AccessModeAdmin). + Update(&TeamUnit{ + AccessMode: AccessModeAdmin, + }) + if err != nil { + return err + } + log.Debug("Updated %d admin team unit access mode to belong to admin instead of none", count) + + return sess.Commit() +} diff --git a/routers/api/v1/org/team.go b/routers/api/v1/org/team.go index 50439251c..024fee346 100644 --- a/routers/api/v1/org/team.go +++ b/routers/api/v1/org/team.go @@ -166,6 +166,21 @@ func attachTeamUnitsMap(team *organization.Team, unitsMap map[string]string) { } } +func attachAdminTeamUnits(team *organization.Team) { + team.Units = make([]*organization.TeamUnit, 0, len(unit_model.AllRepoUnitTypes)) + for _, ut := range unit_model.AllRepoUnitTypes { + up := perm.AccessModeAdmin + if ut == unit_model.TypeExternalTracker || ut == unit_model.TypeExternalWiki { + up = perm.AccessModeRead + } + team.Units = append(team.Units, &organization.TeamUnit{ + OrgID: team.OrgID, + Type: ut, + AccessMode: up, + }) + } +} + // CreateTeam api for create a team func CreateTeam(ctx *context.APIContext) { // swagger:operation POST /orgs/{org}/teams organization orgCreateTeam @@ -213,6 +228,8 @@ func CreateTeam(ctx *context.APIContext) { ctx.Error(http.StatusInternalServerError, "getTeamUnits", errors.New("units permission should not be empty")) return } + } else { + attachAdminTeamUnits(team) } if err := models.NewTeam(team); err != nil { @@ -300,6 +317,8 @@ func EditTeam(ctx *context.APIContext) { } else if len(form.Units) > 0 { attachTeamUnits(team, form.Units) } + } else { + attachAdminTeamUnits(team) } if err := models.UpdateTeam(team, isAuthChanged, isIncludeAllChanged); err != nil { diff --git a/routers/web/org/teams.go b/routers/web/org/teams.go index 1ed798014..e2ec6d878 100644 --- a/routers/web/org/teams.go +++ b/routers/web/org/teams.go @@ -5,6 +5,7 @@ package org import ( + "fmt" "net/http" "net/url" "path" @@ -264,14 +265,26 @@ func NewTeam(ctx *context.Context) { ctx.HTML(http.StatusOK, tplTeamNew) } -func getUnitPerms(forms url.Values) map[unit_model.Type]perm.AccessMode { +func getUnitPerms(forms url.Values, teamPermission perm.AccessMode) map[unit_model.Type]perm.AccessMode { unitPerms := make(map[unit_model.Type]perm.AccessMode) - for k, v := range forms { - if strings.HasPrefix(k, "unit_") { - t, _ := strconv.Atoi(k[5:]) - if t > 0 { - vv, _ := strconv.Atoi(v[0]) - unitPerms[unit_model.Type(t)] = perm.AccessMode(vv) + for _, ut := range unit_model.AllRepoUnitTypes { + // Default accessmode is none + unitPerms[ut] = perm.AccessModeNone + + v, ok := forms[fmt.Sprintf("unit_%d", ut)] + if ok { + vv, _ := strconv.Atoi(v[0]) + if teamPermission >= perm.AccessModeAdmin { + unitPerms[ut] = teamPermission + // Don't allow `TypeExternal{Tracker,Wiki}` to influence this as they can only be set to READ perms. + if ut == unit_model.TypeExternalTracker || ut == unit_model.TypeExternalWiki { + unitPerms[ut] = perm.AccessModeRead + } + } else { + unitPerms[ut] = perm.AccessMode(vv) + if unitPerms[ut] >= perm.AccessModeAdmin { + unitPerms[ut] = perm.AccessModeWrite + } } } } @@ -282,8 +295,8 @@ func getUnitPerms(forms url.Values) map[unit_model.Type]perm.AccessMode { func NewTeamPost(ctx *context.Context) { form := web.GetForm(ctx).(*forms.CreateTeamForm) includesAllRepositories := form.RepoAccess == "all" - unitPerms := getUnitPerms(ctx.Req.Form) p := perm.ParseAccessMode(form.Permission) + unitPerms := getUnitPerms(ctx.Req.Form, p) if p < perm.AccessModeAdmin { // if p is less than admin accessmode, then it should be general accessmode, // so we should calculate the minial accessmode from units accessmodes. @@ -299,17 +312,15 @@ func NewTeamPost(ctx *context.Context) { CanCreateOrgRepo: form.CanCreateOrgRepo, } - if t.AccessMode < perm.AccessModeAdmin { - units := make([]*org_model.TeamUnit, 0, len(unitPerms)) - for tp, perm := range unitPerms { - units = append(units, &org_model.TeamUnit{ - OrgID: ctx.Org.Organization.ID, - Type: tp, - AccessMode: perm, - }) - } - t.Units = units + units := make([]*org_model.TeamUnit, 0, len(unitPerms)) + for tp, perm := range unitPerms { + units = append(units, &org_model.TeamUnit{ + OrgID: ctx.Org.Organization.ID, + Type: tp, + AccessMode: perm, + }) } + t.Units = units ctx.Data["Title"] = ctx.Org.Organization.FullName ctx.Data["PageIsOrgTeams"] = true @@ -422,8 +433,11 @@ func SearchTeam(ctx *context.Context) { func EditTeam(ctx *context.Context) { ctx.Data["Title"] = ctx.Org.Organization.FullName ctx.Data["PageIsOrgTeams"] = true - ctx.Data["team_name"] = ctx.Org.Team.Name - ctx.Data["desc"] = ctx.Org.Team.Description + if err := ctx.Org.Team.LoadUnits(ctx); err != nil { + ctx.ServerError("LoadUnits", err) + return + } + ctx.Data["Team"] = ctx.Org.Team ctx.Data["Units"] = unit_model.Units ctx.HTML(http.StatusOK, tplTeamNew) } @@ -432,7 +446,13 @@ func EditTeam(ctx *context.Context) { func EditTeamPost(ctx *context.Context) { form := web.GetForm(ctx).(*forms.CreateTeamForm) t := ctx.Org.Team - unitPerms := getUnitPerms(ctx.Req.Form) + newAccessMode := perm.ParseAccessMode(form.Permission) + unitPerms := getUnitPerms(ctx.Req.Form, newAccessMode) + if newAccessMode < perm.AccessModeAdmin { + // if newAccessMode is less than admin accessmode, then it should be general accessmode, + // so we should calculate the minial accessmode from units accessmodes. + newAccessMode = unit_model.MinUnitAccessMode(unitPerms) + } isAuthChanged := false isIncludeAllChanged := false includesAllRepositories := form.RepoAccess == "all" @@ -443,14 +463,6 @@ func EditTeamPost(ctx *context.Context) { ctx.Data["Units"] = unit_model.Units if !t.IsOwnerTeam() { - // Validate permission level. - newAccessMode := perm.ParseAccessMode(form.Permission) - if newAccessMode < perm.AccessModeAdmin { - // if p is less than admin accessmode, then it should be general accessmode, - // so we should calculate the minial accessmode from units accessmodes. - newAccessMode = unit_model.MinUnitAccessMode(unitPerms) - } - t.Name = form.TeamName if t.AccessMode != newAccessMode { isAuthChanged = true @@ -467,21 +479,16 @@ func EditTeamPost(ctx *context.Context) { } t.Description = form.Description - if t.AccessMode < perm.AccessModeAdmin { - units := make([]org_model.TeamUnit, 0, len(unitPerms)) - for tp, perm := range unitPerms { - units = append(units, org_model.TeamUnit{ - OrgID: t.OrgID, - TeamID: t.ID, - Type: tp, - AccessMode: perm, - }) - } - if err := org_model.UpdateTeamUnits(t, units); err != nil { - ctx.Error(http.StatusInternalServerError, "UpdateTeamUnits", err.Error()) - return - } + units := make([]*org_model.TeamUnit, 0, len(unitPerms)) + for tp, perm := range unitPerms { + units = append(units, &org_model.TeamUnit{ + OrgID: t.OrgID, + TeamID: t.ID, + Type: tp, + AccessMode: perm, + }) } + t.Units = units if ctx.HasError() { ctx.HTML(http.StatusOK, tplTeamNew) diff --git a/templates/org/team/new.tmpl b/templates/org/team/new.tmpl index 195f8bdcd..2e65d6358 100644 --- a/templates/org/team/new.tmpl +++ b/templates/org/team/new.tmpl @@ -109,7 +109,7 @@
- +
@@ -137,7 +137,7 @@ {{else}} {{if not (eq .Team.LowerName "owners")}} - + {{end}} {{end}} diff --git a/tests/integration/api_team_test.go b/tests/integration/api_team_test.go index 2801fdc98..934e6bf23 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" + "code.gitea.io/gitea/models/perm" "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" "code.gitea.io/gitea/models/unittest" @@ -189,6 +190,36 @@ func TestAPITeam(t *testing.T) { req = NewRequestf(t, "DELETE", "/api/v1/teams/%d?token="+token, teamID) MakeRequest(t, req, http.StatusNoContent) unittest.AssertNotExistsBean(t, &organization.Team{ID: teamID}) + + // Create admin team + teamToCreate = &api.CreateTeamOption{ + Name: "teamadmin", + Description: "team admin", + IncludesAllRepositories: true, + Permission: "admin", + } + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/orgs/%s/teams?token=%s", org.Name, token), teamToCreate) + resp = MakeRequest(t, req, http.StatusCreated) + apiTeam = api.Team{} + DecodeJSON(t, resp, &apiTeam) + for _, ut := range unit.AllRepoUnitTypes { + up := perm.AccessModeAdmin + if ut == unit.TypeExternalTracker || ut == unit.TypeExternalWiki { + up = perm.AccessModeRead + } + unittest.AssertExistsAndLoadBean(t, &organization.TeamUnit{ + OrgID: org.ID, + TeamID: apiTeam.ID, + Type: ut, + AccessMode: up, + }) + } + teamID = apiTeam.ID + + // Delete team. + req = NewRequestf(t, "DELETE", "/api/v1/teams/%d?token="+token, teamID) + MakeRequest(t, req, http.StatusNoContent) + unittest.AssertNotExistsBean(t, &organization.Team{ID: teamID}) } func checkTeamResponse(t *testing.T, testName string, apiTeam *api.Team, name, description string, includesAllRepositories bool, permission string, units []string, unitsMap map[string]string) {