From 784423bd9d68e8ab973bc7c684853aa7ea5a1633 Mon Sep 17 00:00:00 2001 From: bircni Date: Sun, 11 Oct 2026 21:56:32 +0200 Subject: [PATCH] fix(team): merge team edit --unit into the current unit policy Gitea replaces a team's whole unit policy on edit, so `tea team edit 5 --unit repo.wiki=write` revoked code, issue and pull request access. team edit now merges --unit into the current policy; --unit-replace keeps the old replacing behavior. Signed-off-by: bircni --- internal/app/teams.go | 13 +++++++++++-- internal/app/teams_test.go | 26 ++++++++++++++++++++++++++ internal/services/teams.go | 24 ++++++++++++++++++------ internal/services/teams_test.go | 28 ++++++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 8 deletions(-) diff --git a/internal/app/teams.go b/internal/app/teams.go index 5a189dc1..09de536e 100644 --- a/internal/app/teams.go +++ b/internal/app/teams.go @@ -35,7 +35,7 @@ func (a *application) teamCommand() *cobra.Command { group.AddCommand(list) for _, verb := range []string{"view", "create", "edit", "delete"} { var name, description, permission string - var createRepo, allRepos, force bool + var createRepo, allRepos, force, replaceUnits bool var units map[string]string command := &cobra.Command{Use: verb + " ", Short: titleCase(verb) + " an organization team", Args: argsExact(1)} switch verb { @@ -49,7 +49,12 @@ func (a *application) teamCommand() *cobra.Command { command.Flags().StringVar(&name, "name", "", "Team name") command.Flags().StringVar(&description, "description", "", "Team description") command.Flags().StringVar(&permission, "permission", "read", "Default access: read, write, admin") - command.Flags().StringToStringVar(&units, "unit", nil, "Unit permissions, for example repo.code=write,repo.issues=read (replaces unit policy)") + unitUsage := "Unit permissions, for example repo.code=write,repo.issues=read" + if verb == "edit" { + unitUsage = "Unit permissions to change, for example repo.wiki=write; merged into the current policy" + command.Flags().BoolVar(&replaceUnits, "unit-replace", false, "Replace the whole unit policy with --unit; omitted units lose access") + } + command.Flags().StringToStringVar(&units, "unit", nil, unitUsage) command.Flags().BoolVar(&createRepo, "can-create-repo", false, "Allow creating organization repositories") command.Flags().BoolVar(&allRepos, "all-repositories", false, "Access every organization repository") } @@ -68,6 +73,9 @@ func (a *application) teamCommand() *cobra.Command { if err := oneOf("permission", permission, "read", "write", "admin"); err != nil { return err } + if replaceUnits && !cmd.Flags().Changed("unit") { + return clierr.Wrap(clierr.Usage, errors.New("--unit-replace requires --unit")) + } if cmd.Flags().Changed("unit") && (len(units) == 0 || permission == "admin") { return clierr.Wrap(clierr.Usage, errors.New("--unit needs a nonempty policy and cannot restrict administrator access")) } @@ -106,6 +114,7 @@ func (a *application) teamCommand() *cobra.Command { } if cmd.Flags().Changed("unit") { opts.Units = units + opts.ReplaceUnits = replaceUnits } if cmd.Flags().Changed("can-create-repo") { opts.CanCreateRepo = &createRepo diff --git a/internal/app/teams_test.go b/internal/app/teams_test.go index e61cb92a..d3648f10 100644 --- a/internal/app/teams_test.go +++ b/internal/app/teams_test.go @@ -5,6 +5,7 @@ package app import ( "context" + "encoding/json" "net/http" "testing" @@ -52,3 +53,28 @@ func TestTeamDeleteNamesTheTeamAndProtectsTheOwnerTeam(t *testing.T) { require.Equal(t, []string{`Delete team "devs" (ID 5) from acme?`}, prompt.confirmations) require.Equal(t, []string{"/api/v1/teams/3", "/api/v1/teams/5"}, deleted) } + +func TestTeamEditMergesUnitsIntoTheCurrentPolicy(t *testing.T) { + var sent []map[string]any + server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, "/api/v1/teams/5", r.URL.Path) + if r.Method == http.MethodPatch { + var body map[string]any + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + sent = append(sent, body) + } + _, _ = w.Write([]byte(`{"id":5,"name":"devs","permission":"write","units_map":{"repo.code":"write","repo.issues":"write","repo.pulls":"write"}}`)) + })) + defer server.Close() + deps, _, _ := serverDeps(t, server.URL) + require.NoError(t, Execute(context.Background(), deps, []string{"team", "edit", "5", "--unit", "repo.wiki=write", "--json"})) + require.NoError(t, Execute(context.Background(), deps, []string{"team", "edit", "5", "--unit", "repo.wiki=write", "--unit-replace", "--json"})) + require.Equal(t, []map[string]any{ + {"units_map": map[string]any{"repo.code": "write", "repo.issues": "write", "repo.pulls": "write", "repo.wiki": "write"}}, + {"units_map": map[string]any{"repo.wiki": "write"}}, + }, sent) + err := Execute(context.Background(), deps, []string{"team", "edit", "5", "--unit-replace", "--name", "devs"}) + require.Equal(t, clierr.Usage, clierr.ExitCode(err), "%v", err) + require.ErrorContains(t, err, "--unit-replace requires --unit") + require.Len(t, sent, 2) +} diff --git a/internal/services/teams.go b/internal/services/teams.go index 84d0bbe5..9623aaa7 100644 --- a/internal/services/teams.go +++ b/internal/services/teams.go @@ -6,6 +6,7 @@ package services import ( "context" "fmt" + "maps" "net/http" "strings" @@ -47,6 +48,8 @@ type TeamOptions struct { CanCreateRepo *bool `json:"can_create_org_repo,omitempty"` AllRepositories *bool `json:"includes_all_repositories,omitempty"` Units map[string]string `json:"units_map,omitempty"` + // ReplaceUnits makes Edit replace the unit policy with Units instead of merging Units into it. + ReplaceUnits bool `json:"-"` } // Teams implements team, member and repository access operations. @@ -101,19 +104,28 @@ func (s Teams) Create(ctx context.Context, org string, opts TeamOptions) (Team, return teamFrom(team), nil } -// Edit preserves unit permissions when changing access to all repositories. +// Edit merges Units into the current unit policy unless ReplaceUnits is set, and preserves unit +// permissions when changing access to all repositories. Gitea replaces the whole policy on edit. func (s Teams) Edit(ctx context.Context, id int64, opts TeamOptions) (Team, error) { - if opts.AllRepositories != nil && opts.Permission == nil { + repositoryPolicy := opts.AllRepositories != nil && opts.Permission == nil + if repositoryPolicy || (opts.Units != nil && !opts.ReplaceUnits) { current, err := s.View(ctx, id) if err != nil { return Team{}, err } - if current.Permission == "owner" { - return Team{}, &CapabilityError{Feature: "owner-team repository policy editing", Reason: "owner teams already manage all organization repositories"} + if repositoryPolicy { + if current.Permission == "owner" { + return Team{}, &CapabilityError{Feature: "owner-team repository policy editing", Reason: "owner teams already manage all organization repositories"} + } + opts.Permission = ¤t.Permission } - opts.Permission = ¤t.Permission - if opts.Units == nil { + switch { + case opts.Units == nil: opts.Units = current.Units + case !opts.ReplaceUnits: + units := maps.Clone(current.Units) + maps.Copy(units, opts.Units) + opts.Units = units } } var team gitea.Team diff --git a/internal/services/teams_test.go b/internal/services/teams_test.go index ceca6e03..7ea6f9c6 100644 --- a/internal/services/teams_test.go +++ b/internal/services/teams_test.go @@ -37,6 +37,34 @@ func TestTeamRepositoryPolicyPreservesUnitPermissions(t *testing.T) { require.Equal(t, 1, mutations) } +func TestTeamEditMergesUnitsUnlessReplaced(t *testing.T) { + for _, test := range []struct { + replace bool + reads int + want map[string]any + }{ + {false, 1, map[string]any{"repo.code": "write", "repo.issues": "read", "repo.wiki": "write"}}, + {true, 0, map[string]any{"repo.wiki": "write"}}, + } { + reads := 0 + client := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.Equal(t, "/api/v1/teams/5", r.URL.Path) + if r.Method == http.MethodGet { + reads++ + _, _ = w.Write([]byte(`{"id":5,"permission":"write","units_map":{"repo.code":"write","repo.issues":"read","repo.wiki":"none"}}`)) + return + } + body := requestJSON(t, r) + require.Equal(t, test.want, body["units_map"]) + require.NotContains(t, body, "permission") + _, _ = w.Write([]byte(`{"id":5}`)) + })) + _, err := (Teams{Client: client}).Edit(context.Background(), 5, TeamOptions{Units: map[string]string{"repo.wiki": "write"}, ReplaceUnits: test.replace}) + require.NoError(t, err) + require.Equal(t, test.reads, reads) + } +} + func TestTeamCreateSendsPermissionAndUnits(t *testing.T) { client := newTestClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { require.Equal(t, http.MethodPost, r.Method)