Default to authenticated user on codespace delete (#6944)

When a username option is not provided for the `gh codespace delete`
command, we will use the authenticated user's login as the default to
avoid deleting anyone else's codespace by mistake.

Prior to this change, running `gh codespace delete --org MYORG --all`
would fetch all of the codespacese associated with the org regardless of
user and then only delete the ones associated with the authenticated
user, which would lead to 404 errors when MYORG had codespaces owned by
members other than the authenticated member.

Co-authored-by: Victoria Dye <vdye@github.com>
Co-authored-by: Lessley Dennington <ldennington@github.com>

pkg/cmd/codespace/common.go

@@ -80,6 +80,7 @@ func startLiveShareSession(ctx context.Context, codespace *api.Codespace, a *App
 
 //go:generate moq -fmt goimports -rm -skip-ensure -out mock_api.go . apiClient
 type apiClient interface {
+	GetUser(ctx context.Context) (*api.User, error)
 	GetCodespace(ctx context.Context, name string, includeConnection bool) (*api.Codespace, error)
 	GetOrgMemberCodespace(ctx context.Context, orgName string, userName string, codespaceName string) (*api.Codespace, error)
 	ListCodespaces(ctx context.Context, opts api.ListCodespacesOptions) ([]*api.Codespace, error)

pkg/cmd/codespace/delete.go

@@ -84,7 +84,15 @@ func (a *App) Delete(ctx context.Context, opts deleteOptions) (err error) {
 	nameFilter := opts.codespaceName
 	if nameFilter == "" {
 		a.StartProgressIndicatorWithLabel("Fetching codespaces")
-		codespaces, err = a.apiClient.ListCodespaces(ctx, api.ListCodespacesOptions{OrgName: opts.orgName, UserName: opts.userName})
+		userName := opts.userName
+		if userName == "" && opts.orgName != "" {
+			currentUser, err := a.apiClient.GetUser(ctx)
+			if err != nil {
+				return err
+			}
+			userName = currentUser.Login
+		}
+		codespaces, err = a.apiClient.ListCodespaces(ctx, api.ListCodespacesOptions{OrgName: opts.orgName, UserName: userName})
 		a.StopProgressIndicator()
 		if err != nil {
 			return fmt.Errorf("error getting codespaces: %w", err)

pkg/cmd/codespace/delete_test.go

@@ -202,10 +202,28 @@ func TestDelete(t *testing.T) {
 			wantStdout:  "",
 			wantErr:     true,
 		},
+		{
+			name: "deletion for org codespace succeeds without username",
+			opts: deleteOptions{
+				deleteAll: true,
+				orgName:   "bookish",
+			},
+			codespaces: []*api.Codespace{
+				{
+					Name:  "monalisa-spoonknife-123",
+					Owner: api.User{Login: "monalisa"},
+				},
+			},
+			wantDeleted: []string{"monalisa-spoonknife-123"},
+			wantStdout:  "",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			apiMock := &apiClientMock{
+				GetUserFunc: func(_ context.Context) (*api.User, error) {
+					return &api.User{Login: "monalisa"}, nil
+				},
 				DeleteCodespaceFunc: func(_ context.Context, name string, orgName string, userName string) error {
 					if tt.deleteErr != nil {
 						return tt.deleteErr
@@ -253,6 +271,11 @@ func TestDelete(t *testing.T) {
 			if (err != nil) != tt.wantErr {
 				t.Errorf("delete() error = %v, wantErr %v", err, tt.wantErr)
 			}
+			for _, listArgs := range apiMock.ListCodespacesCalls() {
+				if listArgs.Opts.OrgName != "" && listArgs.Opts.UserName == "" {
+					t.Errorf("ListCodespaces() expected username option to be set")
+				}
+			}
 			var gotDeleted []string
 			for _, delArgs := range apiMock.DeleteCodespaceCalls() {
 				gotDeleted = append(gotDeleted, delArgs.Name)

pkg/cmd/codespace/mock_api.go

@@ -46,6 +46,9 @@ import (
 //			GetRepositoryFunc: func(ctx context.Context, nwo string) (*api.Repository, error) {
 //				panic("mock out the GetRepository method")
 //			},
+//			GetUserFunc: func(ctx context.Context) (*api.User, error) {
+//				panic("mock out the GetUser method")
+//			},
 //			ListCodespacesFunc: func(ctx context.Context, opts api.ListCodespacesOptions) ([]*api.Codespace, error) {
 //				panic("mock out the ListCodespaces method")
 //			},
@@ -95,6 +98,9 @@ type apiClientMock struct {
 	// GetRepositoryFunc mocks the GetRepository method.
 	GetRepositoryFunc func(ctx context.Context, nwo string) (*api.Repository, error)
 
+	// GetUserFunc mocks the GetUser method.
+	GetUserFunc func(ctx context.Context) (*api.User, error)
+
 	// ListCodespacesFunc mocks the ListCodespaces method.
 	ListCodespacesFunc func(ctx context.Context, opts api.ListCodespacesOptions) ([]*api.Codespace, error)
 
@@ -201,6 +207,11 @@ type apiClientMock struct {
 			// Nwo is the nwo argument value.
 			Nwo string
 		}
+		// GetUser holds details about calls to the GetUser method.
+		GetUser []struct {
+			// Ctx is the ctx argument value.
+			Ctx context.Context
+		}
 		// ListCodespaces holds details about calls to the ListCodespaces method.
 		ListCodespaces []struct {
 			// Ctx is the ctx argument value.
@@ -248,6 +259,7 @@ type apiClientMock struct {
 	lockGetCodespacesMachines          sync.RWMutex
 	lockGetOrgMemberCodespace          sync.RWMutex
 	lockGetRepository                  sync.RWMutex
+	lockGetUser                        sync.RWMutex
 	lockListCodespaces                 sync.RWMutex
 	lockListDevContainers              sync.RWMutex
 	lockStartCodespace                 sync.RWMutex
@@ -658,6 +670,38 @@ func (mock *apiClientMock) GetRepositoryCalls() []struct {
 	return calls
 }
 
+// GetUser calls GetUserFunc.
+func (mock *apiClientMock) GetUser(ctx context.Context) (*api.User, error) {
+	if mock.GetUserFunc == nil {
+		panic("apiClientMock.GetUserFunc: method is nil but apiClient.GetUser was just called")
+	}
+	callInfo := struct {
+		Ctx context.Context
+	}{
+		Ctx: ctx,
+	}
+	mock.lockGetUser.Lock()
+	mock.calls.GetUser = append(mock.calls.GetUser, callInfo)
+	mock.lockGetUser.Unlock()
+	return mock.GetUserFunc(ctx)
+}
+
+// GetUserCalls gets all the calls that were made to GetUser.
+// Check the length with:
+//
+//	len(mockedapiClient.GetUserCalls())
+func (mock *apiClientMock) GetUserCalls() []struct {
+	Ctx context.Context
+} {
+	var calls []struct {
+		Ctx context.Context
+	}
+	mock.lockGetUser.RLock()
+	calls = mock.calls.GetUser
+	mock.lockGetUser.RUnlock()
+	return calls
+}
+
 // ListCodespaces calls ListCodespacesFunc.
 func (mock *apiClientMock) ListCodespaces(ctx context.Context, opts api.ListCodespacesOptions) ([]*api.Codespace, error) {
 	if mock.ListCodespacesFunc == nil {