From a3f54686281c361fd58b1ef1c522fbab9dbce35c Mon Sep 17 00:00:00 2001 From: bircni Date: Sun, 11 Oct 2026 21:54:57 +0200 Subject: [PATCH] fix(team): name the team in delete prompts and protect the owner team team delete confirmed with "delete team 3 on " and deleted whatever the ID pointed at, including an organization's owner team. It now looks the team up first and asks to delete team "devs" (ID 5) from acme. Deleting an owner team requires --force and, interactively, typing the team's org/name. Signed-off-by: bircni --- internal/app/teams.go | 39 +++++++++++++++++++++------ internal/app/teams_test.go | 54 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 8 deletions(-) create mode 100644 internal/app/teams_test.go diff --git a/internal/app/teams.go b/internal/app/teams.go index fd2c5dfd..5a189dc1 100644 --- a/internal/app/teams.go +++ b/internal/app/teams.go @@ -5,6 +5,7 @@ package app import ( "errors" + "fmt" "strconv" "strings" @@ -34,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 bool + var createRepo, allRepos, force bool var units map[string]string command := &cobra.Command{Use: verb + " ", Short: titleCase(verb) + " an organization team", Args: argsExact(1)} switch verb { @@ -42,6 +43,7 @@ func (a *application) teamCommand() *cobra.Command { command.Use = "create " case "delete": command.Annotations = textOutput("team deletion does not return a resource") + command.Flags().BoolVar(&force, "force", false, "Allow deleting the organization's owner team") } if verb == "create" || verb == "edit" { command.Flags().StringVar(&name, "name", "", "Team name") @@ -120,12 +122,8 @@ func (a *application) teamCommand() *cobra.Command { return err } service := services.Teams{Client: target.client} - if verb == "delete" || prompted { - subject := args[0] - if verb == "create" { - subject = name + " in " + args[0] - } - if err := a.confirm(cmd, verb+" team "+subject+" on "+target.Host); err != nil { + if prompted { + if err := a.confirm(cmd, "create team "+name+" in "+args[0]+" on "+target.Host); err != nil { return err } } @@ -138,7 +136,7 @@ func (a *application) teamCommand() *cobra.Command { case "edit": item, err = service.Edit(cmd.Context(), id, opts) case "delete": - return service.Delete(cmd.Context(), id) + return a.deleteTeam(cmd, service, id, force) } if err != nil { return err @@ -151,6 +149,31 @@ func (a *application) teamCommand() *cobra.Command { return group } +// deleteTeam names the team in its confirmation and deletes an organization's owner team only +// with --force, after the user types its name. +func (a *application) deleteTeam(cmd *cobra.Command, service services.Teams, id int64, force bool) error { + team, err := service.View(cmd.Context(), id) + if err != nil { + return err + } + subject, answer := fmt.Sprintf("team %q (ID %d)", team.Name, team.ID), team.Name + if team.Organization != "" { + subject += " from " + team.Organization + answer = team.Organization + "/" + team.Name + } + if team.Permission != "owner" { + err = a.confirm(cmd, "Delete "+subject+"?") + } else if !force { + return clierr.Wrap(clierr.Usage, fmt.Errorf("%s is the organization's owner team and its members would lose owner access; pass --force to delete it", subject)) + } else { + err = a.confirmTyped(cmd, "Delete owner "+subject+"? Its members lose owner access.", answer) + } + if err != nil { + return err + } + return service.Delete(cmd.Context(), id) +} + func (a *application) teamAccessCommand(repository bool) *cobra.Command { kind := "member" if repository { diff --git a/internal/app/teams_test.go b/internal/app/teams_test.go new file mode 100644 index 00000000..e61cb92a --- /dev/null +++ b/internal/app/teams_test.go @@ -0,0 +1,54 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package app + +import ( + "context" + "net/http" + "testing" + + "gitea.dev/tea/internal/clierr" + "github.com/stretchr/testify/require" +) + +func TestTeamDeleteNamesTheTeamAndProtectsTheOwnerTeam(t *testing.T) { + var deleted []string + server := newAuthenticatedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.Method + " " + r.URL.Path { + case "GET /api/v1/teams/3": + _, _ = w.Write([]byte(`{"id":3,"name":"Owners","permission":"owner","organization":{"username":"acme"}}`)) + case "GET /api/v1/teams/5": + _, _ = w.Write([]byte(`{"id":5,"name":"devs","permission":"write","organization":{"username":"acme"}}`)) + case "DELETE /api/v1/teams/3", "DELETE /api/v1/teams/5": + deleted = append(deleted, r.URL.Path) + w.WriteHeader(http.StatusNoContent) + default: + t.Errorf("unexpected request %s %s", r.Method, r.URL.Path) + w.WriteHeader(http.StatusTeapot) + } + })) + defer server.Close() + deps, _, _ := serverDeps(t, server.URL) + err := Execute(context.Background(), deps, []string{"team", "delete", "3", "--yes"}) + require.Equal(t, clierr.Usage, clierr.ExitCode(err), "%v", err) + require.ErrorContains(t, err, `team "Owners" (ID 3) from acme is the organization's owner team`) + require.ErrorContains(t, err, "--force") + require.Empty(t, deleted) + + deps.Streams.InTTY, deps.Streams.ErrTTY = true, true + prompt := &scriptedPrompter{inputs: map[string]string{`Delete owner team "Owners" (ID 3) from acme? Its members lose owner access. Type acme/Owners to confirm:`: "Owners"}} + deps.Prompter = prompt + err = Execute(context.Background(), deps, []string{"team", "delete", "3", "--force"}) + require.Equal(t, clierr.Canceled, clierr.ExitCode(err), "%v", err) + require.Len(t, prompt.asked, 1) + require.Empty(t, deleted) + prompt.inputs[prompt.asked[0]] = "acme/Owners" + require.NoError(t, Execute(context.Background(), deps, []string{"team", "delete", "3", "--force"})) + require.Equal(t, []string{"/api/v1/teams/3"}, deleted) + + prompt.accept = true + require.NoError(t, Execute(context.Background(), deps, []string{"team", "delete", "5"})) + 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) +}