STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Move create codespace permission opt out behind else to fix token permissions

Browse source
This commit is contained in:
David Gardiner 2024-01-12 12:16:38 -08:00
parent 72b6dc5d8c
commit 20273d1f17
4 changed files with 147 additions and 16 deletions
Download patch file Download diff file Expand all files Collapse all files

11
pkg/cmd/codespace/common.go
View file

@ -114,10 +114,11 @@ func chooseCodespaceFromList(ctx context.Context, codespaces []*api.Codespace, i
},
}
prompter := &Prompter{}
var answers struct {
Codespace int
}
if err := ask(csSurvey, &answers); err != nil {
if err := prompter.Ask(csSurvey, &answers); err != nil {
return nil, fmt.Errorf("error getting answers: %w", err)
}
@ -145,9 +146,15 @@ func safeClose(closer io.Closer, err *error) {
// It is not portable to assume stdin/stdout are fds 0 and 1.
var hasTTY = term.IsTerminal(int(os.Stdin.Fd())) && term.IsTerminal(int(os.Stdout.Fd()))
type SurveyPrompter interface {
Ask(qs []*survey.Question, response interface{}) error
}
type Prompter struct{}
// ask asks survey questions on the terminal, using standard options.
// It fails unless hasTTY, but ideally callers should avoid calling it in that case.
func ask(qs []*survey.Question, response interface{}) error {
func (p *Prompter) Ask(qs []*survey.Question, response interface{}) error {
if !hasTTY {
return fmt.Errorf("no terminal")
}

27
pkg/cmd/codespace/create.go
View file

@ -140,6 +140,7 @@ func (a *App) Create(ctx context.Context, opts createOptions) error {
return a.browser.Browse(fmt.Sprintf("%s/codespaces/new", a.apiClient.ServerURL()))
}
prompter := &Prompter{}
promptForRepoAndBranch := userInputs.Repository == "" && !opts.useWeb
if promptForRepoAndBranch {
var defaultRepo string
@ -167,7 +168,7 @@ func (a *App) Create(ctx context.Context, opts createOptions) error {
Validate: survey.Required,
},
}
if err := ask(repoQuestions, &userInputs); err != nil {
if err := prompter.Ask(repoQuestions, &userInputs); err != nil {
return fmt.Errorf("failed to prompt: %w", err)
}
}
@ -212,7 +213,7 @@ func (a *App) Create(ctx context.Context, opts createOptions) error {
},
}
if err := ask(branchQuestions, &userInputs); err != nil {
if err := prompter.Ask(branchQuestions, &userInputs); err != nil {
return fmt.Errorf("failed to prompt: %w", err)
}
}
@ -259,7 +260,7 @@ func (a *App) Create(ctx context.Context, opts createOptions) error {
},
}
if err := ask([]*survey.Question{devContainerPathQuestion}, &devContainerPath); err != nil {
if err := prompter.Ask([]*survey.Question{devContainerPathQuestion}, &devContainerPath); err != nil {
return fmt.Errorf("failed to prompt: %w", err)
}
}
@ -277,7 +278,7 @@ func (a *App) Create(ctx context.Context, opts createOptions) error {
// web UI also provide a way to select machine type
// therefore we let the user choose from the web UI instead of prompting from CLI
if !(opts.useWeb && opts.machine == "") {
machine, err = getMachineName(ctx, a.apiClient, repository.ID, opts.machine, branch, userInputs.Location, devContainerPath)
machine, err = getMachineName(ctx, a.apiClient, prompter, repository.ID, opts.machine, branch, userInputs.Location, devContainerPath)
if err != nil {
return fmt.Errorf("error getting machine type: %w", err)
}
@ -320,7 +321,7 @@ func (a *App) Create(ctx context.Context, opts createOptions) error {
return fmt.Errorf("error creating codespace: %w", err)
}
codespace, err = a.handleAdditionalPermissions(ctx, createParams, aerr.AllowPermissionsURL)
codespace, err = a.handleAdditionalPermissions(ctx, prompter, createParams, aerr.AllowPermissionsURL)
if err != nil {
// this error could be a cmdutil.SilentError (in the case that the user opened the browser) so we don't want to wrap it
return err
@ -344,7 +345,7 @@ func (a *App) Create(ctx context.Context, opts createOptions) error {
return nil
}
func (a *App) handleAdditionalPermissions(ctx context.Context, createParams *api.CreateCodespaceParams, allowPermissionsURL string) (*api.Codespace, error) {
func (a *App) handleAdditionalPermissions(ctx context.Context, prompter SurveyPrompter, createParams *api.CreateCodespaceParams, allowPermissionsURL string) (*api.Codespace, error) {
var (
isInteractive = a.io.CanPrompt()
cs = a.io.ColorScheme()
@ -379,7 +380,7 @@ func (a *App) handleAdditionalPermissions(ctx context.Context, createParams *api
Accept string
}
if err := ask(permsSurvey, &answers); err != nil {
if err := prompter.Ask(permsSurvey, &answers); err != nil {
return nil, fmt.Errorf("error getting answers: %w", err)
}
@ -393,12 +394,12 @@ func (a *App) handleAdditionalPermissions(ctx context.Context, createParams *api
if err := a.pollForPermissions(ctx, createParams); err != nil {
return nil, fmt.Errorf("error polling for permissions: %w", err)
}
} else {
// If the user chose to create the codespace without the permissions,
// we can continue with the create opting out of the additional permissions
createParams.PermissionsOptOut = true
}
// if the user chose to create the codespace without the permissions,
// we can continue with the create opting out of the additional permissions
createParams.PermissionsOptOut = true
var codespace *api.Codespace
err := a.RunWithProgress("Creating codespace", func() (err error) {
codespace, err = a.apiClient.CreateCodespace(ctx, createParams)
@ -510,7 +511,7 @@ func (a *App) showStatus(ctx context.Context, codespace *api.Codespace) error {
}
// getMachineName prompts the user to select the machine type, or validates the machine if non-empty.
func getMachineName(ctx context.Context, apiClient apiClient, repoID int, machine, branch, location string, devcontainerPath string) (string, error) {
func getMachineName(ctx context.Context, apiClient apiClient, prompter SurveyPrompter, repoID int, machine, branch, location string, devcontainerPath string) (string, error) {
machines, err := apiClient.GetCodespacesMachines(ctx, repoID, branch, location, devcontainerPath)
if err != nil {
return "", fmt.Errorf("error requesting machine instance types: %w", err)
@ -561,7 +562,7 @@ func getMachineName(ctx context.Context, apiClient apiClient, repoID int, machin
}
var machineAnswers struct{ Machine string }
if err := ask(machineSurvey, &machineAnswers); err != nil {
if err := prompter.Ask(machineSurvey, &machineAnswers); err != nil {
return "", fmt.Errorf("error getting machine: %w", err)
}

122
pkg/cmd/codespace/create_test.go
View file

@ -7,6 +7,7 @@ import (
"testing"
"time"
"github.com/AlecAivazis/survey/v2"
"github.com/cli/cli/v2/internal/browser"
"github.com/cli/cli/v2/internal/codespaces/api"
"github.com/cli/cli/v2/pkg/cmdutil"
@ -703,6 +704,127 @@ func TestBuildDisplayName(t *testing.T) {
}
}
type MockSurveyPrompter struct {
AskFunc func(qs []*survey.Question, response interface{}) error
}
func (m *MockSurveyPrompter) Ask(qs []*survey.Question, response interface{}) error {
return m.AskFunc(qs, response)
}
type MockBrowser struct {
Err error
}
func (b *MockBrowser) Browse(url string) error {
if b.Err != nil {
return b.Err
}
return nil
}
func TestHandleAdditionalPermissions(t *testing.T) {
tests := []struct {
name string
isInteractive bool
accept string
permissionsOptOut bool
browserErr error
pollForPermissionsErr error
createCodespaceErr error
wantErr bool
}{
{
name: "non-interactive",
isInteractive: false,
permissionsOptOut: false,
wantErr: true,
},
{
name: "interactive, continue in browser, browser error",
isInteractive: true,
accept: "Continue in browser to review and authorize additional permissions (Recommended)",
permissionsOptOut: false,
browserErr: fmt.Errorf("browser error"),
wantErr: true,
},
{
name: "interactive, continue in browser, poll for permissions error",
isInteractive: true,
accept: "Continue in browser to review and authorize additional permissions (Recommended)",
permissionsOptOut: false,
pollForPermissionsErr: fmt.Errorf("poll for permissions error"),
wantErr: true,
},
{
name: "interactive, continue in browser, create codespace error",
isInteractive: true,
accept: "Continue in browser to review and authorize additional permissions (Recommended)",
permissionsOptOut: false,
createCodespaceErr: fmt.Errorf("create codespace error"),
wantErr: true,
},
{
name: "interactive, continue without authorizing",
isInteractive: true,
accept: "Continue without authorizing additional permissions",
permissionsOptOut: true,
createCodespaceErr: fmt.Errorf("create codespace error"),
wantErr: true,
},
{
name: "interactive, continue without authorizing, create codespace success",
isInteractive: true,
accept: "Continue without authorizing additional permissions",
permissionsOptOut: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ios, _, _, _ := iostreams.Test()
a := &App{
io: ios,
browser: &MockBrowser{
Err: tt.browserErr,
},
apiClient: &apiClientMock{
CreateCodespaceFunc: func(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) {
return nil, tt.createCodespaceErr
},
GetCodespacesPermissionsCheckFunc: func(ctx context.Context, repoID int, branch string, devcontainerPath string) (bool, error) {
if tt.pollForPermissionsErr != nil {
return false, tt.pollForPermissionsErr
}
return true, nil
},
},
}
if tt.isInteractive {
a.io.SetStdinTTY(true)
a.io.SetStdoutTTY(true)
a.io.SetStderrTTY(true)
}
params := &api.CreateCodespaceParams{}
_, err := a.handleAdditionalPermissions(context.Background(), &MockSurveyPrompter{
AskFunc: func(qs []*survey.Question, response interface{}) error {
*response.(*struct{ Accept string }) = struct{ Accept string }{Accept: tt.accept}
return nil
},
}, params, "http://example.com")
if (err != nil) != tt.wantErr {
t.Errorf("handleAdditionalPermissions() error = %v, wantErr %v", err, tt.wantErr)
}
if tt.permissionsOptOut != params.PermissionsOptOut {
t.Errorf("handleAdditionalPermissions() permissionsOptOut = %v, want %v", params.PermissionsOptOut, tt.permissionsOptOut)
}
})
}
}
func apiCreateDefaults(c *apiClientMock) *apiClientMock {
if c.GetRepositoryFunc == nil {
c.GetRepositoryFunc = func(ctx context.Context, nwo string) (*api.Repository, error) {

3
pkg/cmd/codespace/delete.go
View file

@ -220,6 +220,7 @@ func confirmDeletion(p prompter, apiCodespace *api.Codespace, isInteractive bool
type surveyPrompter struct{}
func (p *surveyPrompter) Confirm(message string) (bool, error) {
prompter := &Prompter{}
var confirmed struct {
Confirmed bool
}
@ -231,7 +232,7 @@ func (p *surveyPrompter) Confirm(message string) (bool, error) {
},
},
}
if err := ask(q, &confirmed); err != nil {
if err := prompter.Ask(q, &confirmed); err != nil {
return false, fmt.Errorf("failed to prompt: %w", err)
}