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

Merge branch 'trunk' into jungaretti/fix-jupyter-spinner

Browse source
This commit is contained in:
JP Ungaretti 2023-03-07 18:43:34 +00:00
commit 5a7f779cf7
119 changed files with 2365 additions and 1226 deletions
Download patch file Download diff file Expand all files Collapse all files

2
.goreleaser.yml
View file

@ -18,7 +18,7 @@ builds:
- -s -w -X github.com/cli/cli/v2/internal/build.Version={{.Version}} -X github.com/cli/cli/v2/internal/build.Date={{time "2006-01-02"}}
id: macos
goos: [darwin]
goarch: [amd64]
goarch: [amd64, arm64]
- <<: *build_defaults
id: linux

16
api/export_pr.go
View file

@ -19,6 +19,14 @@ func (issue *Issue) ExportData(fields []string) map[string]interface{} {
data[f] = issue.Labels.Nodes
case "projectCards":
data[f] = issue.ProjectCards.Nodes
case "projectItems":
items := make([]map[string]interface{}, 0, len(issue.ProjectItems.Nodes))
for _, n := range issue.ProjectItems.Nodes {
items = append(items, map[string]interface{}{
"title": n.Project.Title,
})
}
data[f] = items
default:
sf := fieldByName(v, f)
data[f] = sf.Interface()
@ -96,6 +104,14 @@ func (pr *PullRequest) ExportData(fields []string) map[string]interface{} {
data[f] = pr.Labels.Nodes
case "projectCards":
data[f] = pr.ProjectCards.Nodes
case "projectItems":
items := make([]map[string]interface{}, 0, len(pr.ProjectItems.Nodes))
for _, n := range pr.ProjectItems.Nodes {
items = append(items, map[string]interface{}{
"title": n.Project.Title,
})
}
data[f] = items
case "reviews":
data[f] = pr.Reviews.Nodes
case "latestReviews":

24
api/export_pr_test.go
View file

@ -75,6 +75,30 @@ func TestIssue_ExportData(t *testing.T) {
}
`),
},
{
name: "project items",
fields: []string{"projectItems"},
inputJSON: heredoc.Doc(`
{ "projectItems": { "nodes": [
{
"id": "PVTI_id",
"project": {
"id": "PVT_id",
"title": "Some Project"
}
}
] } }
`),
outputJSON: heredoc.Doc(`
{
"projectItems": [
{
"title": "Some Project"
}
]
}
`),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {

4
api/http_client.go
View file

@ -14,7 +14,7 @@ import (
)
type tokenGetter interface {
AuthToken(string) (string, string)
Token(string) (string, string)
}
type HTTPClientOptions struct {
@ -93,7 +93,7 @@ func AddAuthTokenHeader(rt http.RoundTripper, cfg tokenGetter) http.RoundTripper
// If the header is already set in the request, don't overwrite it.
if req.Header.Get(authorization) == "" {
hostname := ghinstance.NormalizeHostname(getHost(req))
if token, _ := cfg.AuthToken(hostname); token != "" {
if token, _ := cfg.Token(hostname); token != "" {
req.Header.Set(authorization, fmt.Sprintf("token %s", token))
}
}

2
api/http_client_test.go
View file

@ -202,7 +202,7 @@ func TestNewHTTPClient(t *testing.T) {
type tinyConfig map[string]string
func (c tinyConfig) AuthToken(host string) (string, string) {
func (c tinyConfig) Token(host string) (string, string) {
return c[fmt.Sprintf("%s:%s", host, "oauth_token")], "oauth_token"
}

39
api/queries_org.go
View file

@ -42,45 +42,6 @@ func OrganizationProjects(client *Client, repo ghrepo.Interface) ([]RepoProject,
return projects, nil
}
// OrganizationProjectsV2 fetches all open projectsV2 for an organization.
func OrganizationProjectsV2(client *Client, repo ghrepo.Interface) ([]RepoProjectV2, error) {
type responseData struct {
Organization struct {
ProjectsV2 struct {
Nodes []RepoProjectV2
PageInfo struct {
HasNextPage bool
EndCursor string
}
} `graphql:"projectsV2(first: 100, orderBy: {field: TITLE, direction: ASC}, after: $endCursor, query: $query)"`
} `graphql:"organization(login: $owner)"`
}
variables := map[string]interface{}{
"owner": githubv4.String(repo.RepoOwner()),
"endCursor": (*githubv4.String)(nil),
"query": githubv4.String("is:open"),
}
var projectsV2 []RepoProjectV2
for {
var query responseData
err := client.Query(repo.RepoHost(), "OrganizationProjectV2List", &query, variables)
if err != nil {
return nil, err
}
projectsV2 = append(projectsV2, query.Organization.ProjectsV2.Nodes...)
if !query.Organization.ProjectsV2.PageInfo.HasNextPage {
break
}
variables["endCursor"] = githubv4.String(query.Organization.ProjectsV2.PageInfo.EndCursor)
}
return projectsV2, nil
}
type OrgTeam struct {
ID string
Slug string

127
api/queries_projects_v2.go
View file

@ -10,12 +10,21 @@ import (
const (
errorProjectsV2ReadScope = "field requires one of the following scopes: ['read:project']"
errorProjectsV2UserField = "Field 'projectsV2' doesn't exist on type 'User'"
errorProjectsV2RepositoryField = "Field 'projectsV2' doesn't exist on type 'Repository'"
errorProjectsV2OrganizationField = "Field 'projectsV2' doesn't exist on type 'Organization'"
errorProjectsV2IssueField = "Field 'projectItems' doesn't exist on type 'Issue'"
errorProjectsV2PullRequestField = "Field 'projectItems' doesn't exist on type 'PullRequest'"
)
type ProjectV2 struct {
ID string `json:"id"`
Title string `json:"title"`
Number int `json:"number"`
ResourcePath string `json:"resourcePath"`
Closed bool `json:"closed"`
}
// UpdateProjectV2Items uses the addProjectV2ItemById and the deleteProjectV2Item mutations
// to add and delete items from projects. The addProjectItems and deleteProjectItems arguments are
// mappings between a project and an item. This function can be used across multiple projects
@ -126,6 +135,123 @@ func ProjectsV2ItemsForPullRequest(client *Client, repo ghrepo.Interface, pr *Pu
return nil
}
// OrganizationProjectsV2 fetches all open projectsV2 for an organization.
func OrganizationProjectsV2(client *Client, repo ghrepo.Interface) ([]ProjectV2, error) {
type responseData struct {
Organization struct {
ProjectsV2 struct {
Nodes []ProjectV2
PageInfo struct {
HasNextPage bool
EndCursor string
}
} `graphql:"projectsV2(first: 100, orderBy: {field: TITLE, direction: ASC}, after: $endCursor, query: $query)"`
} `graphql:"organization(login: $owner)"`
}
variables := map[string]interface{}{
"owner": githubv4.String(repo.RepoOwner()),
"endCursor": (*githubv4.String)(nil),
"query": githubv4.String("is:open"),
}
var projectsV2 []ProjectV2
for {
var query responseData
err := client.Query(repo.RepoHost(), "OrganizationProjectV2List", &query, variables)
if err != nil {
return nil, err
}
projectsV2 = append(projectsV2, query.Organization.ProjectsV2.Nodes...)
if !query.Organization.ProjectsV2.PageInfo.HasNextPage {
break
}
variables["endCursor"] = githubv4.String(query.Organization.ProjectsV2.PageInfo.EndCursor)
}
return projectsV2, nil
}
// RepoProjectsV2 fetches all open projectsV2 for a repository.
func RepoProjectsV2(client *Client, repo ghrepo.Interface) ([]ProjectV2, error) {
type responseData struct {
Repository struct {
ProjectsV2 struct {
Nodes []ProjectV2
PageInfo struct {
HasNextPage bool
EndCursor string
}
} `graphql:"projectsV2(first: 100, orderBy: {field: TITLE, direction: ASC}, after: $endCursor, query: $query)"`
} `graphql:"repository(owner: $owner, name: $name)"`
}
variables := map[string]interface{}{
"owner": githubv4.String(repo.RepoOwner()),
"name": githubv4.String(repo.RepoName()),
"endCursor": (*githubv4.String)(nil),
"query": githubv4.String("is:open"),
}
var projectsV2 []ProjectV2
for {
var query responseData
err := client.Query(repo.RepoHost(), "RepositoryProjectV2List", &query, variables)
if err != nil {
return nil, err
}
projectsV2 = append(projectsV2, query.Repository.ProjectsV2.Nodes...)
if !query.Repository.ProjectsV2.PageInfo.HasNextPage {
break
}
variables["endCursor"] = githubv4.String(query.Repository.ProjectsV2.PageInfo.EndCursor)
}
return projectsV2, nil
}
// CurrentUserProjectsV2 fetches all open projectsV2 for current user.
func CurrentUserProjectsV2(client *Client, hostname string) ([]ProjectV2, error) {
type responseData struct {
Viewer struct {
ProjectsV2 struct {
Nodes []ProjectV2
PageInfo struct {
HasNextPage bool
EndCursor string
}
} `graphql:"projectsV2(first: 100, orderBy: {field: TITLE, direction: ASC}, after: $endCursor, query: $query)"`
} `graphql:"viewer"`
}
variables := map[string]interface{}{
"endCursor": (*githubv4.String)(nil),
"query": githubv4.String("is:open"),
}
var projectsV2 []ProjectV2
for {
var query responseData
err := client.Query(hostname, "UserProjectV2List", &query, variables)
if err != nil {
return nil, err
}
projectsV2 = append(projectsV2, query.Viewer.ProjectsV2.Nodes...)
if !query.Viewer.ProjectsV2.PageInfo.HasNextPage {
break
}
variables["endCursor"] = githubv4.String(query.Viewer.ProjectsV2.PageInfo.EndCursor)
}
return projectsV2, nil
}
// When querying ProjectsV2 fields we generally dont want to show the user
// scope errors and field does not exist errors. ProjectsV2IgnorableError
// checks against known error strings to see if an error can be safely ignored.
@ -134,6 +260,7 @@ func ProjectsV2ItemsForPullRequest(client *Client, repo ghrepo.Interface, pr *Pu
func ProjectsV2IgnorableError(err error) bool {
msg := err.Error()
if strings.Contains(msg, errorProjectsV2ReadScope) ||
strings.Contains(msg, errorProjectsV2UserField) ||
strings.Contains(msg, errorProjectsV2RepositoryField) ||
strings.Contains(msg, errorProjectsV2OrganizationField) ||
strings.Contains(msg, errorProjectsV2IssueField) ||

280
api/queries_repo.go
View file

@ -20,6 +20,10 @@ import (
"github.com/shurcooL/githubv4"
)
const (
errorResolvingOrganization = "Could not resolve to an Organization"
)
// Repository contains information about a GitHub repo
type Repository struct {
ID string
@ -653,7 +657,7 @@ type RepoMetadataResult struct {
AssignableUsers []RepoAssignee
Labels []RepoLabel
Projects []RepoProject
ProjectsV2 []RepoProjectV2
ProjectsV2 []ProjectV2
Milestones []RepoMilestone
Teams []OrgTeam
}
@ -758,17 +762,17 @@ func (m *RepoMetadataResult) projectV2TitleToID(projectTitle string) (string, bo
return "", false
}
func ProjectsToPaths(projects []RepoProject, projectsV2 []RepoProjectV2, names []string) ([]string, error) {
func ProjectsToPaths(projects []RepoProject, projectsV2 []ProjectV2, names []string) ([]string, error) {
var paths []string
for _, projectName := range names {
found := false
for _, p := range projects {
if strings.EqualFold(projectName, p.Name) {
// format of ResourcePath: /OWNER/REPO/projects/PROJECT_NUMBER or /orgs/ORG/projects/PROJECT_NUMBER
// required format of path: OWNER/REPO/PROJECT_NUMBER or ORG/PROJECT_NUMBER
// format of ResourcePath: /OWNER/REPO/projects/PROJECT_NUMBER or /orgs/ORG/projects/PROJECT_NUMBER or /users/USER/projects/PROJECT_NUBER
// required format of path: OWNER/REPO/PROJECT_NUMBER or ORG/PROJECT_NUMBER or USER/PROJECT_NUMBER
var path string
pathParts := strings.Split(p.ResourcePath, "/")
if pathParts[1] == "orgs" {
if pathParts[1] == "orgs" || pathParts[1] == "users" {
path = fmt.Sprintf("%s/%s", pathParts[2], pathParts[4])
} else {
path = fmt.Sprintf("%s/%s/%s", pathParts[1], pathParts[2], pathParts[4])
@ -783,11 +787,11 @@ func ProjectsToPaths(projects []RepoProject, projectsV2 []RepoProjectV2, names [
}
for _, p := range projectsV2 {
if strings.EqualFold(projectName, p.Title) {
// format of ResourcePath: /OWNER/REPO/projects/PROJECT_NUMBER or /orgs/ORG/projects/PROJECT_NUMBER
// required format of path: OWNER/REPO/PROJECT_NUMBER or ORG/PROJECT_NUMBER
// format of ResourcePath: /OWNER/REPO/projects/PROJECT_NUMBER or /orgs/ORG/projects/PROJECT_NUMBER or /users/USER/projects/PROJECT_NUBER
// required format of path: OWNER/REPO/PROJECT_NUMBER or ORG/PROJECT_NUMBER or USER/PROJECT_NUMBER
var path string
pathParts := strings.Split(p.ResourcePath, "/")
if pathParts[1] == "orgs" {
if pathParts[1] == "orgs" || pathParts[1] == "users" {
path = fmt.Sprintf("%s/%s", pathParts[2], pathParts[4])
} else {
path = fmt.Sprintf("%s/%s/%s", pathParts[1], pathParts[2], pathParts[4])
@ -845,100 +849,74 @@ type RepoMetadataInput struct {
// RepoMetadata pre-fetches the metadata for attaching to issues and pull requests
func RepoMetadata(client *Client, repo ghrepo.Interface, input RepoMetadataInput) (*RepoMetadataResult, error) {
result := RepoMetadataResult{}
errc := make(chan error)
count := 0
var result RepoMetadataResult
var g errgroup.Group
if input.Assignees || input.Reviewers {
count++
go func() {
g.Go(func() error {
users, err := RepoAssignableUsers(client, repo)
if err != nil {
err = fmt.Errorf("error fetching assignees: %w", err)
}
result.AssignableUsers = users
errc <- err
}()
return err
})
}
if input.Reviewers {
count++
go func() {
g.Go(func() error {
teams, err := OrganizationTeams(client, repo)
// TODO: better detection of non-org repos
if err != nil && !strings.Contains(err.Error(), "Could not resolve to an Organization") {
errc <- fmt.Errorf("error fetching organization teams: %w", err)
return
if err != nil && !strings.Contains(err.Error(), errorResolvingOrganization) {
err = fmt.Errorf("error fetching organization teams: %w", err)
return err
}
result.Teams = teams
errc <- nil
}()
return nil
})
}
if input.Reviewers {
count++
go func() {
g.Go(func() error {
login, err := CurrentLoginName(client, repo.RepoHost())
if err != nil {
err = fmt.Errorf("error fetching current login: %w", err)
}
result.CurrentLogin = login
errc <- err
}()
return err
})
}
if input.Labels {
count++
go func() {
g.Go(func() error {
labels, err := RepoLabels(client, repo)
if err != nil {
err = fmt.Errorf("error fetching labels: %w", err)
}
result.Labels = labels
errc <- err
}()
return err
})
}
if input.Projects {
count++
go func() {
projects, err := RepoAndOrgProjects(client, repo)
if err != nil {
errc <- err
return
}
result.Projects = projects
errc <- nil
}()
}
if input.Projects {
count++
go func() {
projectsV2, err := RepoAndOrgProjectsV2(client, repo)
if err != nil {
errc <- err
return
}
result.ProjectsV2 = projectsV2
errc <- nil
}()
g.Go(func() error {
var err error
result.Projects, result.ProjectsV2, err = relevantProjects(client, repo)
return err
})
}
if input.Milestones {
count++
go func() {
g.Go(func() error {
milestones, err := RepoMilestones(client, repo, "open")
if err != nil {
err = fmt.Errorf("error fetching milestones: %w", err)
}
result.Milestones = milestones
errc <- err
}()
return err
})
}
var err error
for i := 0; i < count; i++ {
if e := <-errc; e != nil {
err = e
}
if err := g.Wait(); err != nil {
return nil, err
}
return &result, err
return &result, nil
}
type RepoResolveInput struct {
@ -1050,14 +1028,6 @@ type RepoProject struct {
ResourcePath string `json:"resourcePath"`
}
type RepoProjectV2 struct {
ID string `json:"id"`
Title string `json:"title"`
Number int `json:"number"`
ResourcePath string `json:"resourcePath"`
Closed bool `json:"closed"`
}
// RepoProjects fetches all open projects for a repository.
func RepoProjects(client *Client, repo ghrepo.Interface) ([]RepoProject, error) {
type responseData struct {
@ -1096,87 +1066,6 @@ func RepoProjects(client *Client, repo ghrepo.Interface) ([]RepoProject, error)
return projects, nil
}
// RepoProjectsV2 fetches all open projectsV2 for a repository.
func RepoProjectsV2(client *Client, repo ghrepo.Interface) ([]RepoProjectV2, error) {
type responseData struct {
Repository struct {
ProjectsV2 struct {
Nodes []RepoProjectV2
PageInfo struct {
HasNextPage bool
EndCursor string
}
} `graphql:"projectsV2(first: 100, orderBy: {field: TITLE, direction: ASC}, after: $endCursor, query: $query)"`
} `graphql:"repository(owner: $owner, name: $name)"`
}
variables := map[string]interface{}{
"owner": githubv4.String(repo.RepoOwner()),
"name": githubv4.String(repo.RepoName()),
"endCursor": (*githubv4.String)(nil),
"query": githubv4.String("is:open"),
}
var projectsV2 []RepoProjectV2
for {
var query responseData
err := client.Query(repo.RepoHost(), "RepositoryProjectV2List", &query, variables)
if err != nil {
return nil, err
}
projectsV2 = append(projectsV2, query.Repository.ProjectsV2.Nodes...)
if !query.Repository.ProjectsV2.PageInfo.HasNextPage {
break
}
variables["endCursor"] = githubv4.String(query.Repository.ProjectsV2.PageInfo.EndCursor)
}
return projectsV2, nil
}
// RepoAndOrgProjects fetches all open projects for a repository and its organization.
func RepoAndOrgProjects(client *Client, repo ghrepo.Interface) ([]RepoProject, error) {
projects, err := RepoProjects(client, repo)
if err != nil {
return nil, fmt.Errorf("error fetching projects: %w", err)
}
orgProjects, err := OrganizationProjects(client, repo)
// TODO: Better detection of non-org repos.
if err != nil && !strings.Contains(err.Error(), "Could not resolve to an Organization") {
return nil, fmt.Errorf("error fetching organization projects: %w", err)
}
projects = append(projects, orgProjects...)
return projects, nil
}
// RepoAndOrgProjectsV2 fetches all open projectsV2 for a repository and its organization.
// Note: If the auth token does not have sufficient scopes or projectsV2 is not supported
// on the host then those errors are swallowed and nil is returned.
func RepoAndOrgProjectsV2(client *Client, repo ghrepo.Interface) ([]RepoProjectV2, error) {
projectsV2, err := RepoProjectsV2(client, repo)
if err != nil {
if ProjectsV2IgnorableError(err) {
return nil, nil
}
return nil, fmt.Errorf("error fetching projectsV2: %w", err)
}
orgProjectsV2, err := OrganizationProjectsV2(client, repo)
if err != nil && !strings.Contains(err.Error(), "Could not resolve to an Organization") {
return nil, fmt.Errorf("error fetching organization projectsV2: %w", err)
}
projectsV2 = append(projectsV2, orgProjectsV2...)
return projectsV2, nil
}
type RepoAssignee struct {
ID string
Login string
@ -1329,27 +1218,100 @@ func RepoMilestones(client *Client, repo ghrepo.Interface, state string) ([]Repo
}
func ProjectNamesToPaths(client *Client, repo ghrepo.Interface, projectNames []string) ([]string, error) {
projects, projectsV2, err := relevantProjects(client, repo)
if err != nil {
return nil, err
}
return ProjectsToPaths(projects, projectsV2, projectNames)
}
// RelevantProjects retrieves set of Projects and ProjectsV2 relevant to given repository:
// - Projects for repository
// - Projects for repository organization, if it belongs to one
// - ProjectsV2 owned by current user
// - ProjectsV2 linked to repository
// - ProjectsV2 owned by repository organization, if it belongs to one
func relevantProjects(client *Client, repo ghrepo.Interface) ([]RepoProject, []ProjectV2, error) {
var repoProjects []RepoProject
var orgProjects []RepoProject
var userProjectsV2 []ProjectV2
var repoProjectsV2 []ProjectV2
var orgProjectsV2 []ProjectV2
g, _ := errgroup.WithContext(context.Background())
var projects []RepoProject
var projectsV2 []RepoProjectV2
g.Go(func() error {
var err error
projects, err = RepoAndOrgProjects(client, repo)
repoProjects, err = RepoProjects(client, repo)
if err != nil {
err = fmt.Errorf("error fetching repo projects (classic): %w", err)
}
return err
})
g.Go(func() error {
var err error
projectsV2, err = RepoAndOrgProjectsV2(client, repo)
return err
orgProjects, err = OrganizationProjects(client, repo)
if err != nil && !strings.Contains(err.Error(), errorResolvingOrganization) {
err = fmt.Errorf("error fetching organization projects (classic): %w", err)
return err
}
return nil
})
g.Go(func() error {
var err error
userProjectsV2, err = CurrentUserProjectsV2(client, repo.RepoHost())
if err != nil && !ProjectsV2IgnorableError(err) {
err = fmt.Errorf("error fetching user projects: %w", err)
return err
}
return nil
})
g.Go(func() error {
var err error
repoProjectsV2, err = RepoProjectsV2(client, repo)
if err != nil && !ProjectsV2IgnorableError(err) {
err = fmt.Errorf("error fetching repo projects: %w", err)
return err
}
return nil
})
g.Go(func() error {
var err error
orgProjectsV2, err = OrganizationProjectsV2(client, repo)
if err != nil &&
!ProjectsV2IgnorableError(err) &&
!strings.Contains(err.Error(), errorResolvingOrganization) {
err = fmt.Errorf("error fetching organization projects: %w", err)
return err
}
return nil
})
if err := g.Wait(); err != nil {
return nil, err
return nil, nil, err
}
return ProjectsToPaths(projects, projectsV2, projectNames)
projects := make([]RepoProject, 0, len(repoProjects)+len(orgProjects))
projects = append(projects, repoProjects...)
projects = append(projects, orgProjects...)
// ProjectV2 might appear across multiple queries so use a map to keep them deduplicated.
m := make(map[string]ProjectV2, len(userProjectsV2)+len(repoProjectsV2)+len(orgProjectsV2))
for _, p := range userProjectsV2 {
m[p.ID] = p
}
for _, p := range repoProjectsV2 {
m[p.ID] = p
}
for _, p := range orgProjectsV2 {
m[p.ID] = p
}
projectsV2 := make([]ProjectV2, 0, len(m))
for _, p := range m {
projectsV2 = append(projectsV2, p)
}
return projects, projectsV2, nil
}
func CreateRepoTransformToV4(apiClient *Client, hostname string, method string, path string, body io.Reader) (*Repository, error) {

30
api/queries_repo_test.go
View file

@ -120,6 +120,16 @@ func Test_RepoMetadata(t *testing.T) {
"pageInfo": { "hasNextPage": false }
} } } }
`))
http.Register(
httpmock.GraphQL(`query UserProjectV2List\b`),
httpmock.StringResponse(`
{ "data": { "viewer": { "projectsV2": {
"nodes": [
{ "title": "MonalisaV2", "id": "MONALISAV2ID" }
],
"pageInfo": { "hasNextPage": false }
} } } }
`))
http.Register(
httpmock.GraphQL(`query OrganizationTeamList\b`),
httpmock.StringResponse(`
@ -170,8 +180,8 @@ func Test_RepoMetadata(t *testing.T) {
}
expectedProjectIDs := []string{"TRIAGEID", "ROADMAPID"}
expectedProjectV2IDs := []string{"TRIAGEV2ID", "ROADMAPV2ID"}
projectIDs, projectV2IDs, err := result.ProjectsToIDs([]string{"triage", "roadmap", "triagev2", "roadmapv2"})
expectedProjectV2IDs := []string{"TRIAGEV2ID", "ROADMAPV2ID", "MONALISAV2ID"}
projectIDs, projectV2IDs, err := result.ProjectsToIDs([]string{"triage", "roadmap", "triagev2", "roadmapv2", "monalisav2"})
if err != nil {
t.Errorf("error resolving projects: %v", err)
}
@ -204,7 +214,7 @@ func Test_ProjectsToPaths(t *testing.T) {
{ID: "id2", Name: "Org Project", ResourcePath: "/orgs/ORG/projects/PROJECT_NUMBER"},
{ID: "id3", Name: "Project", ResourcePath: "/orgs/ORG/projects/PROJECT_NUMBER_2"},
}
projectsV2 := []RepoProjectV2{
projectsV2 := []ProjectV2{
{ID: "id4", Title: "My Project V2", ResourcePath: "/OWNER/REPO/projects/PROJECT_NUMBER_2"},
{ID: "id5", Title: "Org Project V2", ResourcePath: "/orgs/ORG/projects/PROJECT_NUMBER_3"},
}
@ -267,13 +277,23 @@ func Test_ProjectNamesToPaths(t *testing.T) {
"pageInfo": { "hasNextPage": false }
} } } }
`))
http.Register(
httpmock.GraphQL(`query UserProjectV2List\b`),
httpmock.StringResponse(`
{ "data": { "viewer": { "projectsV2": {
"nodes": [
{ "title": "MonalisaV2", "id": "MONALISAV2ID", "resourcePath": "/users/MONALISA/projects/5" }
],
"pageInfo": { "hasNextPage": false }
} } } }
`))
projectPaths, err := ProjectNamesToPaths(client, repo, []string{"Triage", "Roadmap", "TriageV2", "RoadmapV2"})
projectPaths, err := ProjectNamesToPaths(client, repo, []string{"Triage", "Roadmap", "TriageV2", "RoadmapV2", "MonalisaV2"})
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
expectedProjectPaths := []string{"ORG/1", "OWNER/REPO/2", "ORG/2", "OWNER/REPO/4"}
expectedProjectPaths := []string{"ORG/1", "OWNER/REPO/2", "ORG/2", "OWNER/REPO/4", "MONALISA/5"}
if !sliceEqual(projectPaths, expectedProjectPaths) {
t.Errorf("expected projects paths %v, got %v", expectedProjectPaths, projectPaths)
}

1
api/query_builder.go
View file

@ -221,6 +221,7 @@ var IssueFields = []string{
"milestone",
"number",
"projectCards",
"projectItems",
"reactionGroups",
"state",
"title",

129
api/sanitize_ascii.go
View file

@ -11,13 +11,15 @@ import (
var jsonTypeRE = regexp.MustCompile(`[/+]json($|;)`)
// GitHub servers return non-printable characters as their unicode code point values.
// The values of \u0000 to \u001F represent C0 ASCII control characters and
// the values of \u0080 to \u009F represent C1 ASCII control characters. These control
// characters will be interpreted by the terminal, this behaviour can be used maliciously
// as an attack vector, especially the control character \u001B. This function wraps
// JSON response bodies in a ReadCloser that transforms C0 and C1 control characters
// to their caret and hex notations respectively so that the terminal will not interpret them.
// GitHub servers do not sanitize their API output for terminal display
// and leave in unescaped ASCII control characters.
// C0 control characters are represented in their unicode code point form ranging from \u0000 to \u001F.
// C1 control characters are represented in two bytes, the first being 0xC2 and the second ranging from 0x80 to 0x9F.
// These control characters will be interpreted by the terminal, this behaviour can be
// used maliciously as an attack vector, especially the control characters \u001B and \u009B.
// This function wraps JSON response bodies in a ReadCloser that transforms C0 and C1
// control characters to their caret notations respectively so that the terminal will not
// interpret them.
func AddASCIISanitizer(rt http.RoundTripper) http.RoundTripper {
return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) {
res, err := rt.RoundTrip(req)
@ -64,12 +66,28 @@ func (s *sanitizeASCIIReadCloser) Read(out []byte) (int, error) {
for bufIndex < bufLen-6 && outIndex < outLen {
window := buf[bufIndex : bufIndex+6]
if bytes.HasPrefix(window, []byte(`\u00`)) {
repl, _ := mapControlCharacterToCaret(window)
if s.addEscape {
repl = append([]byte{'\\'}, repl...)
s.addEscape = false
// Replace C1 Control Characters
if window[0] == 0xC2 {
repl, _ := mapC1ToCaret(window[:2])
for j := 0; j < len(repl); j++ {
if outIndex < outLen {
out[outIndex] = repl[j]
outIndex++
} else {
s.remainder = append(s.remainder, repl[j])
}
}
bufIndex += 2
continue
}
// Replace C0 Control Characters
if bytes.HasPrefix(window, []byte(`\u00`)) {
repl, found := mapC0ToCaret(window)
if s.addEscape && found {
repl = append([]byte{'\\'}, repl...)
}
s.addEscape = false
for j := 0; j < len(repl); j++ {
if outIndex < outLen {
out[outIndex] = repl[j]
@ -118,10 +136,8 @@ func (s *sanitizeASCIIReadCloser) Read(out []byte) (int, error) {
return outIndex, readErr
}
// mapControlCharacterToCaret maps C0 control sequences to caret notation
// and C1 control sequences to hex notation. C1 control sequences do not
// have caret notation representation.
func mapControlCharacterToCaret(b []byte) ([]byte, bool) {
// mapC0ToCaret maps C0 control sequences to caret notation.
func mapC0ToCaret(b []byte) ([]byte, bool) {
m := map[string]string{
`\u0000`: `^@`,
`\u0001`: `^A`,
@ -155,41 +171,58 @@ func mapControlCharacterToCaret(b []byte) ([]byte, bool) {
`\u001d`: `^]`,
`\u001e`: `^^`,
`\u001f`: `^_`,
`\u0080`: `\\200`,
`\u0081`: `\\201`,
`\u0082`: `\\202`,
`\u0083`: `\\203`,
`\u0084`: `\\204`,
`\u0085`: `\\205`,
`\u0086`: `\\206`,
`\u0087`: `\\207`,
`\u0088`: `\\210`,
`\u0089`: `\\211`,
`\u008a`: `\\212`,
`\u008b`: `\\213`,
`\u008c`: `\\214`,
`\u008d`: `\\215`,
`\u008e`: `\\216`,
`\u008f`: `\\217`,
`\u0090`: `\\220`,
`\u0091`: `\\221`,
`\u0092`: `\\222`,
`\u0093`: `\\223`,
`\u0094`: `\\224`,
`\u0095`: `\\225`,
`\u0096`: `\\226`,
`\u0097`: `\\227`,
`\u0098`: `\\230`,
`\u0099`: `\\231`,
`\u009a`: `\\232`,
`\u009b`: `\\233`,
`\u009c`: `\\234`,
`\u009d`: `\\235`,
`\u009e`: `\\236`,
`\u009f`: `\\237`,
}
if c, ok := m[strings.ToLower(string(b))]; ok {
return []byte(c), true
}
return b, false
}
// mapC1ToCaret maps C1 control sequences to caret notation.
// C1 control sequences are two bytes and start with 0xC2.
func mapC1ToCaret(b []byte) ([]byte, bool) {
if len(b) != 2 {
return b, false
}
if b[0] != 0xC2 {
return b, false
}
m := map[byte]string{
128: `^@`,
129: `^A`,
130: `^B`,
131: `^C`,
132: `^D`,
133: `^E`,
134: `^F`,
135: `^G`,
136: `^H`,
137: `^I`,
138: `^J`,
139: `^K`,
140: `^L`,
141: `^M`,
142: `^N`,
143: `^O`,
144: `^P`,
145: `^Q`,
146: `^R`,
147: `^S`,
148: `^T`,
149: `^U`,
150: `^V`,
151: `^W`,
152: `^X`,
153: `^Y`,
154: `^Z`,
155: `^[`,
156: `^\\`,
157: `^]`,
158: `^^`,
159: `^_`,
}
if c, ok := m[b[1]]; ok {
return []byte(c), true
}
return b, false
}

37
api/sanitize_ascii_test.go
View file

@ -14,7 +14,7 @@ import (
"github.com/stretchr/testify/require"
)
func TestHTTPClient_SanitizeASCIIControlCharacters(t *testing.T) {
func TestHTTPClientSanitizeASCIIControlCharactersC0(t *testing.T) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
issue := Issue{
Title: "\u001B[31mRed Title\u001B[0m",
@ -51,6 +51,41 @@ func TestHTTPClient_SanitizeASCIIControlCharacters(t *testing.T) {
assert.Equal(t, "Escaped ^[ \\^[ \\^[ \\\\^[", issue.ActiveLockReason)
}
func TestHTTPClientSanitizeASCIIControlCharactersC1(t *testing.T) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
issue := Issue{
Title: "\xC2\x9B[31mRed Title\xC2\x9B[0m",
Body: "80\xC2\x80 81\xC2\x81 82\xC2\x82 83\xC2\x83 84\xC2\x84 85\xC2\x85 86\xC2\x86 87\xC2\x87 88\xC2\x88 89\xC2\x89 8A\xC2\x8A 8B\xC2\x8B 8C\xC2\x8C 8D\xC2\x8D 8E\xC2\x8E 8F\xC2\x8F",
Author: Author{
ID: "1",
Name: "90\xC2\x90 91\xC2\x91 92\xC2\x92 93\xC2\x93 94\xC2\x94 95\xC2\x95 96\xC2\x96 97\xC2\x97 98\xC2\x98 99\xC2\x99 9A\xC2\x9A 9B\xC2\x9B 9C\xC2\x9C 9D\xC2\x9D 9E\xC2\x9E 9F\xC2\x9F",
Login: "monalisa\xC2\xA1",
},
}
responseData, _ := json.Marshal(issue)
w.Header().Set("Content-Type", "application/json; charset=utf-8")
fmt.Fprint(w, string(responseData))
}))
defer ts.Close()
client, err := NewHTTPClient(HTTPClientOptions{})
require.NoError(t, err)
req, err := http.NewRequest("GET", ts.URL, nil)
require.NoError(t, err)
res, err := client.Do(req)
require.NoError(t, err)
body, err := io.ReadAll(res.Body)
res.Body.Close()
require.NoError(t, err)
var issue Issue
err = json.Unmarshal(body, &issue)
require.NoError(t, err)
assert.Equal(t, "^[[31mRed Title^[[0m", issue.Title)
assert.Equal(t, "80^@ 81^A 82^B 83^C 84^D 85^E 86^F 87^G 88^H 89^I 8A^J 8B^K 8C^L 8D^M 8E^N 8F^O", issue.Body)
assert.Equal(t, "90^P 91^Q 92^R 93^S 94^T 95^U 96^V 97^W 98^X 99^Y 9A^Z 9B^[ 9C^\\ 9D^] 9E^^ 9F^_", issue.Author.Name)
assert.Equal(t, "monalisa¡", issue.Author.Login)
}
func TestSanitizeASCIIReadCloser(t *testing.T) {
data := []byte(`"Assign},"L`)
var r io.Reader = bytes.NewReader(data)

2
docs/working-with-us.md
View file

@ -48,6 +48,8 @@ If after all of this consideration you think your feature should be merged, plea
Once we've signed off, open up a pull request in [cli/cli](https://github.com/cli/cli) adding your command. Since we make use of `go-gh` within our code already, it shouldn't be too onerous to make your extension merge-able. Link to the issue you opened in step 3 so we have some context on the pull request.
Keep in mind that our expectation of non-trivial commands that end up merged into `cli/cli` is that your team will continue to maintain what they merged over time. We can help redirect issues your way as part of our first responder rotation, but are unable to take on the full support burden for your new command.
## Other considerations
- If you have a high need for secrecy until the point of release, let us know in [#cli on slack](https://github.slack.com/archives/CLLG3RMAR). We'll come up with a solution to work on merging your command in private.

1
git/client.go
View file

@ -142,6 +142,7 @@ func (c *Client) CurrentBranch(ctx context.Context) (string, error) {
if err != nil {
var gitErr *GitError
if ok := errors.As(err, &gitErr); ok && len(gitErr.Stderr) == 0 {
gitErr.err = ErrNotOnAnyBranch
gitErr.Stderr = "not on any branch"
return "", gitErr
}

16
go.mod
View file

@ -6,10 +6,10 @@ require (
github.com/AlecAivazis/survey/v2 v2.3.6
github.com/MakeNowJust/heredoc v1.0.0
github.com/briandowns/spinner v1.18.1
github.com/cenkalti/backoff/v4 v4.1.3
github.com/cenkalti/backoff/v4 v4.2.0
github.com/charmbracelet/glamour v0.5.1-0.20220727184942-e70ff2d969da
github.com/charmbracelet/lipgloss v0.5.0
github.com/cli/go-gh v1.0.0
github.com/cli/go-gh v1.2.1
github.com/cli/oauth v1.0.1
github.com/cli/safeexec v1.0.1
github.com/cpuguy83/go-md2man/v2 v2.0.2
@ -35,10 +35,11 @@ require (
github.com/spf13/cobra v1.6.1
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.7.5
github.com/zalando/go-keyring v0.2.2
golang.org/x/crypto v0.0.0-20210921155107-089bfa567519
golang.org/x/sync v0.1.0
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211
golang.org/x/text v0.5.0
golang.org/x/term v0.5.0
golang.org/x/text v0.7.0
google.golang.org/grpc v1.49.0
google.golang.org/protobuf v1.27.1
gopkg.in/yaml.v3 v3.0.1
@ -46,13 +47,16 @@ require (
require (
github.com/alecthomas/chroma v0.10.0 // indirect
github.com/alessio/shellescape v1.4.1 // indirect
github.com/aymerick/douceur v0.2.0 // indirect
github.com/cli/browser v1.1.0 // indirect
github.com/cli/shurcooL-graphql v0.0.2 // indirect
github.com/danieljoos/wincred v1.1.2 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/dlclark/regexp2 v1.4.0 // indirect
github.com/fatih/color v1.7.0 // indirect
github.com/gdamore/encoding v1.0.0 // indirect
github.com/godbus/dbus/v5 v5.1.0 // indirect
github.com/golang/protobuf v1.5.2 // indirect
github.com/gorilla/css v1.0.0 // indirect
github.com/hashicorp/errwrap v1.0.0 // indirect
@ -73,9 +77,9 @@ require (
github.com/thlib/go-timezone-local v0.0.0-20210907160436-ef149e42d28e // indirect
github.com/yuin/goldmark v1.4.13 // indirect
github.com/yuin/goldmark-emoji v1.0.1 // indirect
golang.org/x/net v0.0.0-20220923203811-8be639271d50 // indirect
golang.org/x/net v0.7.0 // indirect
golang.org/x/oauth2 v0.0.0-20220309155454-6242fa91716a // indirect
golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab // indirect
golang.org/x/sys v0.5.0 // indirect
google.golang.org/genproto v0.0.0-20200825200019-8632dd797987 // indirect
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect
)

29
go.sum
View file

@ -41,12 +41,14 @@ github.com/Netflix/go-expect v0.0.0-20220104043353-73e0943537d2 h1:+vx7roKuyA63n
github.com/Netflix/go-expect v0.0.0-20220104043353-73e0943537d2/go.mod h1:HBCaDeC1lPdgDeDbhX8XFpy1jqjK0IBG8W5K+xYqA0w=
github.com/alecthomas/chroma v0.10.0 h1:7XDcGkCQopCNKjZHfYrNLraA+M7e0fMiJ/Mfikbfjek=
github.com/alecthomas/chroma v0.10.0/go.mod h1:jtJATyUxlIORhUOFNA9NZDWGAQ8wpxQQqNSB4rjA/1s=
github.com/alessio/shellescape v1.4.1 h1:V7yhSDDn8LP4lc4jS8pFkt0zCnzVJlG5JXy9BVKJUX0=
github.com/alessio/shellescape v1.4.1/go.mod h1:PZAiSCk0LJaZkiCSkPv8qIobYglO3FPpyFjDCtHLS30=
github.com/aymerick/douceur v0.2.0 h1:Mv+mAeH1Q+n9Fr+oyamOlAkUNPWPlA8PPGR0QAaYuPk=
github.com/aymerick/douceur v0.2.0/go.mod h1:wlT5vV2O3h55X9m7iVYN0TBM0NH/MmbLnd30/FjWUq4=
github.com/briandowns/spinner v1.18.1 h1:yhQmQtM1zsqFsouh09Bk/jCjd50pC3EOGsh28gLVvwY=
github.com/briandowns/spinner v1.18.1/go.mod h1:mQak9GHqbspjC/5iUx3qMlIho8xBS/ppAL/hX5SmPJU=
github.com/cenkalti/backoff/v4 v4.1.3 h1:cFAlzYUlVYDysBEH2T5hyJZMh3+5+WCBvSnK6Q8UtC4=
github.com/cenkalti/backoff/v4 v4.1.3/go.mod h1:scbssz8iZGpm3xbr14ovlUdkxfGXNInqkPWOWmG2CLw=
github.com/cenkalti/backoff/v4 v4.2.0 h1:HN5dHm3WBOgndBH6E8V0q2jIYIR3s9yglV8k/+MN3u4=
github.com/cenkalti/backoff/v4 v4.2.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE=
github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
github.com/charmbracelet/glamour v0.5.1-0.20220727184942-e70ff2d969da h1:FGz53GWQRiKQ/5xUsoCCkewSQIC7u81Scaxx2nUy3nM=
github.com/charmbracelet/glamour v0.5.1-0.20220727184942-e70ff2d969da/go.mod h1:HXz79SMFnF9arKxqeoHWxmo1BhplAH7wehlRhKQIL94=
@ -60,8 +62,8 @@ github.com/cli/browser v1.1.0 h1:xOZBfkfY9L9vMBgqb1YwRirGu6QFaQ5dP/vXt5ENSOY=
github.com/cli/browser v1.1.0/go.mod h1:HKMQAt9t12kov91Mn7RfZxyJQQgWgyS/3SZswlZ5iTI=
github.com/cli/crypto v0.0.0-20210929142629-6be313f59b03 h1:3f4uHLfWx4/WlnMPXGai03eoWAI+oGHJwr+5OXfxCr8=
github.com/cli/crypto v0.0.0-20210929142629-6be313f59b03/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
github.com/cli/go-gh v1.0.0 h1:zE1YUAUYqGXNZuICEBeOkIMJ5F50BS0ftvtoWGlsEFI=
github.com/cli/go-gh v1.0.0/go.mod h1:bqxLdCoTZ73BuiPEJx4olcO/XKhVZaFDchFagYRBweE=
github.com/cli/go-gh v1.2.1 h1:xFrjejSsgPiwXFP6VYynKWwxLQcNJy3Twbu82ZDlR/o=
github.com/cli/go-gh v1.2.1/go.mod h1:Jxk8X+TCO4Ui/GarwY9tByWm/8zp4jJktzVZNlTW5VM=
github.com/cli/oauth v1.0.1 h1:pXnTFl/qUegXHK531Dv0LNjW4mLx626eS42gnzfXJPA=
github.com/cli/oauth v1.0.1/go.mod h1:qd/FX8ZBD6n1sVNQO3aIdRxeu5LGw9WhKnYhIIoC2A4=
github.com/cli/safeexec v1.0.0/go.mod h1:Z/D4tTN8Vs5gXYHDCbaM1S/anmEDnJb1iW0+EJ5zx3Q=
@ -76,6 +78,8 @@ github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46t
github.com/creack/pty v1.1.17/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4=
github.com/creack/pty v1.1.18 h1:n56/Zwd5o6whRC5PMGretI4IdRLlmBXYNjScPaBgsbY=
github.com/creack/pty v1.1.18/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4=
github.com/danieljoos/wincred v1.1.2 h1:QLdCxFs1/Yl4zduvBdcHB8goaYk9RARS2SgLLRuAyr0=
github.com/danieljoos/wincred v1.1.2/go.mod h1:GijpziifJoIBfYh+S7BbkdUTU4LfM+QnGqR5Vl2tAx0=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
@ -96,6 +100,8 @@ github.com/gdamore/tcell/v2 v2.5.4/go.mod h1:dZgRy5v4iMobMEcWNYBtREnDZAT9DYmfqIk
github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU=
github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8=
github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8=
github.com/godbus/dbus/v5 v5.1.0 h1:4KLkAxT3aOY8Li4FRJe/KvhoNFFxo0m6fNuFUO8QJUk=
github.com/godbus/dbus/v5 v5.1.0/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=
github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=
github.com/golang/groupcache v0.0.0-20190702054246-869f871628b6/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=
github.com/golang/groupcache v0.0.0-20191227052852-215e87163ea7/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=
@ -265,6 +271,8 @@ github.com/yuin/goldmark v1.4.13 h1:fVcFKWvrslecOb/tg+Cc05dkeYx540o0FuFt3nUVDoE=
github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
github.com/yuin/goldmark-emoji v1.0.1 h1:ctuWEyzGBwiucEqxzwe0SOYDXPAucOrE9NQC18Wa1os=
github.com/yuin/goldmark-emoji v1.0.1/go.mod h1:2w1E6FEWLcDQkoTE+7HU6QF1F6SLlNGjRIBbIZQFqkQ=
github.com/zalando/go-keyring v0.2.2 h1:f0xmpYiSrHtSNAVgwip93Cg8tuF45HJM6rHq/A5RI/4=
github.com/zalando/go-keyring v0.2.2/go.mod h1:sI3evg9Wvpw3+n4SqplGSJUMwtDeROfD4nsFz4z9PG0=
go.opencensus.io v0.21.0/go.mod h1:mSImk1erAIZhrmZN+AvHh14ztQfjbGwt4TtuofqLduU=
go.opencensus.io v0.22.0/go.mod h1:+kGneAE2xo2IficOXnaByMWTGM9T73dGwxeWcUqIpI8=
go.opencensus.io v0.22.2/go.mod h1:yxeiOL68Rb0Xd1ddK5vPZ/oVn4vY4Ynel7k9FzqtOIw=
@ -333,8 +341,9 @@ golang.org/x/net v0.0.0-20211015210444-4f30a5c0130f/go.mod h1:9nx3DQGgdP8bBQD5qx
golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk=
golang.org/x/net v0.0.0-20220624214902-1bab6f366d9e/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c=
golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c=
golang.org/x/net v0.0.0-20220923203811-8be639271d50 h1:vKyz8L3zkd+xrMeIaBsQ/MNVPVFSffdaU3ZyYlBGFnI=
golang.org/x/net v0.0.0-20220923203811-8be639271d50/go.mod h1:YDH+HFinaLZZlnHAfSS6ZXJJ9M9t4Dl22yv3iI2vPwk=
golang.org/x/net v0.7.0 h1:rJrUqqhjsgNp7KqAIc25s9pZnjU7TUcSY7HcVZjdn1g=
golang.org/x/net v0.7.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs=
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
@ -385,6 +394,7 @@ golang.org/x/sys v0.0.0-20210319071255-635bc2c9138d/go.mod h1:h1NjWce9XRLGQEsW7w
golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20210819135213-f52c844e1c1c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20210831042530-f4d43177bf5e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20211019181941-9d821ace8654/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
@ -393,12 +403,14 @@ golang.org/x/sys v0.0.0-20220422013727-9388b58f7150/go.mod h1:oPkhp1MJrh7nUepCBc
golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220728004956-3c1f35247d10/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab h1:2QkjZIsXupsJbJIdSjjUOgWK3aEtzyuh2mPt3l/CkeU=
golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.5.0 h1:MUK/U/4lj1t1oPg0HfuXDN/Z1wv31ZJ/YcPiGccS4DU=
golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/term v0.0.0-20210503060354-a79de5458b56/go.mod h1:tfny5GFUkzUvx4ps4ajbZsCe5lw1metzhBm9T3x7oIY=
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 h1:JGgROgKl9N8DuW20oFS5gxc+lE67/N3FcwmBPMe7ArY=
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
golang.org/x/term v0.5.0 h1:n2a8QNdAb0sZNpU9R1ALUXBbY+w51fCQDN+7EdxNBsY=
golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k=
golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
@ -406,8 +418,9 @@ golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk=
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
golang.org/x/text v0.5.0 h1:OLmvp0KP+FVG99Ct/qFiL/Fhk4zp4QQnZ7b2U+5piUM=
golang.org/x/text v0.5.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
golang.org/x/text v0.7.0 h1:4BRB4x83lYWy72KwLD/qYDuTu7q9PjSagHvijDw7cLo=
golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=

42
internal/authflow/flow.go
View file

@ -6,7 +6,6 @@ import (
"io"
"net/http"
"net/url"
"os"
"regexp"
"strings"
@ -28,37 +27,7 @@ var (
jsonTypeRE = regexp.MustCompile(`[/+]json($|;)`)
)
type iconfig interface {
Get(string, string) (string, error)
Set(string, string, string)
Write() error
}
func AuthFlowWithConfig(cfg iconfig, IO *iostreams.IOStreams, hostname, notice string, additionalScopes []string, isInteractive bool) (string, error) {
// TODO this probably shouldn't live in this package. It should probably be in a new package that
// depends on both iostreams and config.
// FIXME: this duplicates `factory.browserLauncher()`
browserLauncher := os.Getenv("GH_BROWSER")
if browserLauncher == "" {
browserLauncher, _ = cfg.Get("", "browser")
}
if browserLauncher == "" {
browserLauncher = os.Getenv("BROWSER")
}
token, userLogin, err := authFlow(hostname, IO, notice, additionalScopes, isInteractive, browserLauncher)
if err != nil {
return "", err
}
cfg.Set(hostname, "user", userLogin)
cfg.Set(hostname, "oauth_token", token)
return token, cfg.Write()
}
func authFlow(oauthHost string, IO *iostreams.IOStreams, notice string, additionalScopes []string, isInteractive bool, browserLauncher string) (string, string, error) {
func AuthFlow(oauthHost string, IO *iostreams.IOStreams, notice string, additionalScopes []string, isInteractive bool, b browser.Browser) (string, string, error) {
w := IO.ErrOut
cs := IO.ColorScheme()
@ -106,7 +75,6 @@ func authFlow(oauthHost string, IO *iostreams.IOStreams, notice string, addition
fmt.Fprintf(w, "%s to open %s in your browser... ", cs.Bold("Press Enter"), oauthHost)
_ = waitForEnter(IO.In)
b := browser.New(browserLauncher, IO.Out, IO.ErrOut)
if err := b.Browse(authURL); err != nil {
fmt.Fprintf(w, "%s Failed opening a web browser at %s\n", cs.Red("!"), authURL)
fmt.Fprintf(w, " %s\n", err)
@ -138,16 +106,16 @@ func authFlow(oauthHost string, IO *iostreams.IOStreams, notice string, addition
}
type cfg struct {
authToken string
token string
}
func (c cfg) AuthToken(hostname string) (string, string) {
return c.authToken, "oauth_token"
func (c cfg) Token(hostname string) (string, string) {
return c.token, "oauth_token"
}
func getViewer(hostname, token string, logWriter io.Writer) (string, error) {
opts := api.HTTPClientOptions{
Config: cfg{authToken: token},
Config: cfg{token: token},
Log: logWriter,
}
client, err := api.NewHTTPClient(opts)

17
internal/codespaces/api/api.go
View file

@ -40,6 +40,7 @@ import (
"strings"
"time"
"github.com/cenkalti/backoff/v4"
"github.com/cli/cli/v2/api"
"github.com/opentracing/opentracing-go"
)
@ -1082,16 +1083,16 @@ func (a *API) setHeaders(req *http.Request) {
// withRetry takes a generic function that sends an http request and retries
// only when the returned response has a >=500 status code.
func (a *API) withRetry(f func() (*http.Response, error)) (resp *http.Response, err error) {
for i := 0; i < 5; i++ {
resp, err = f()
func (a *API) withRetry(f func() (*http.Response, error)) (*http.Response, error) {
bo := backoff.NewConstantBackOff(a.retryBackoff)
return backoff.RetryWithData(func() (*http.Response, error) {
resp, err := f()
if err != nil {
return nil, err
return nil, backoff.Permanent(err)
}
if resp.StatusCode < 500 {
break
return resp, nil
}
time.Sleep(a.retryBackoff * (time.Duration(i) + 1))
}
return resp, err
return nil, errors.New("retry")
}, backoff.WithMaxRetries(bo, 3))
}

5
internal/codespaces/api/api_test.go
View file

@ -185,8 +185,9 @@ func TestCreateCodespaces_Pending(t *testing.T) {
defer svr.Close()
api := API{
githubAPI: svr.URL,
client: &http.Client{},
githubAPI: svr.URL,
client: &http.Client{},
retryBackoff: 0,
}
ctx := context.TODO()

38
internal/codespaces/codespaces.go
View file

@ -37,7 +37,7 @@ type logger interface {
// ConnectToLiveshare waits for a Codespace to become running,
// and connects to it using a Live Share session.
func ConnectToLiveshare(ctx context.Context, progress progressIndicator, sessionLogger logger, apiClient apiClient, codespace *api.Codespace) (sess *liveshare.Session, err error) {
func ConnectToLiveshare(ctx context.Context, progress progressIndicator, sessionLogger logger, apiClient apiClient, codespace *api.Codespace) (*liveshare.Session, error) {
if codespace.State != api.CodespaceStateAvailable {
progress.StartProgressIndicatorWithLabel("Starting codespace")
defer progress.StopProgressIndicator()
@ -45,25 +45,33 @@ func ConnectToLiveshare(ctx context.Context, progress progressIndicator, session
return nil, fmt.Errorf("error starting codespace: %w", err)
}
}
expBackoff := backoff.NewExponentialBackOff()
expBackoff.Multiplier = 1.1
expBackoff.MaxInterval = 10 * time.Second
expBackoff.MaxElapsedTime = 5 * time.Minute
if !connectionReady(codespace) {
expBackoff := backoff.NewExponentialBackOff()
expBackoff.Multiplier = 1.1
expBackoff.MaxInterval = 10 * time.Second
expBackoff.MaxElapsedTime = 5 * time.Minute
for retries := 0; !connectionReady(codespace); retries++ {
if retries > 1 {
duration := expBackoff.NextBackOff()
time.Sleep(duration)
}
err := backoff.Retry(func() error {
var err error
codespace, err = apiClient.GetCodespace(ctx, codespace.Name, true)
if err != nil {
return backoff.Permanent(fmt.Errorf("error getting codespace: %w", err))
}
if expBackoff.GetElapsedTime() >= expBackoff.MaxElapsedTime {
return nil, errors.New("timed out while waiting for the codespace to start")
}
if connectionReady(codespace) {
return nil
}
codespace, err = apiClient.GetCodespace(ctx, codespace.Name, true)
return errors.New("codespace not ready yet")
}, backoff.WithContext(expBackoff, ctx))
if err != nil {
return nil, fmt.Errorf("error getting codespace: %w", err)
var permErr *backoff.PermanentError
if errors.As(err, &permErr) {
return nil, err
}
return nil, errors.New("timed out while waiting for the codespace to start")
}
}

163
internal/config/config.go
View file

@ -6,26 +6,26 @@ import (
ghAuth "github.com/cli/go-gh/pkg/auth"
ghConfig "github.com/cli/go-gh/pkg/config"
"github.com/zalando/go-keyring"
)
const (
hosts = "hosts"
aliases = "aliases"
aliases = "aliases"
hosts = "hosts"
oauthToken = "oauth_token"
)
// This interface describes interacting with some persistent configuration for gh.
//
//go:generate moq -rm -out config_mock.go . Config
type Config interface {
AuthToken(string) (string, string)
Get(string, string) (string, error)
GetOrDefault(string, string) (string, error)
Set(string, string, string)
UnsetHost(string)
Hosts() []string
DefaultHost() (string, string)
Aliases() *AliasConfig
Write() error
Aliases() *AliasConfig
Authentication() *AuthConfig
}
func NewConfig() (Config, error) {
@ -41,10 +41,6 @@ type cfg struct {
cfg *ghConfig.Config
}
func (c *cfg) AuthToken(hostname string) (string, string) {
return ghAuth.TokenForHost(hostname)
}
func (c *cfg) Get(hostname, key string) (string, error) {
if hostname != "" {
val, err := c.cfg.Get([]string{hosts, hostname, key})
@ -86,27 +82,16 @@ func (c *cfg) Set(hostname, key, value string) {
c.cfg.Set([]string{hosts, hostname, key}, value)
}
func (c *cfg) UnsetHost(hostname string) {
if hostname == "" {
return
}
_ = c.cfg.Remove([]string{hosts, hostname})
}
func (c *cfg) Hosts() []string {
return ghAuth.KnownHosts()
}
func (c *cfg) DefaultHost() (string, string) {
return ghAuth.DefaultHost()
func (c *cfg) Write() error {
return ghConfig.Write(c.cfg)
}
func (c *cfg) Aliases() *AliasConfig {
return &AliasConfig{cfg: c.cfg}
}
func (c *cfg) Write() error {
return ghConfig.Write(c.cfg)
func (c *cfg) Authentication() *AuthConfig {
return &AuthConfig{cfg: c.cfg}
}
func defaultFor(key string) string {
@ -127,6 +112,132 @@ func defaultExists(key string) bool {
return false
}
// AuthConfig is used for interacting with some persistent configuration for gh,
// with knowledge on how to access encrypted storage when neccesarry.
// Behavior is scoped to authentication specific tasks.
type AuthConfig struct {
cfg *ghConfig.Config
defaultHostOverride func() (string, string)
hostsOverride func() []string
tokenOverride func(string) (string, string)
}
// Token will retrieve the auth token for the given hostname,
// searching environment variables, plain text config, and
// lastly encypted storage.
func (c *AuthConfig) Token(hostname string) (string, string) {
if c.tokenOverride != nil {
return c.tokenOverride(hostname)
}
token, source := ghAuth.TokenFromEnvOrConfig(hostname)
if token == "" {
var err error
token, err = c.TokenFromKeyring(hostname)
if err == nil {
source = "keyring"
}
}
return token, source
}
// SetToken will override any token resolution and return the given
// token and source for all calls to Token. Use for testing purposes only.
func (c *AuthConfig) SetToken(token, source string) {
c.tokenOverride = func(_ string) (string, string) {
return token, source
}
}
// TokenFromKeyring will retrieve the auth token for the given hostname,
// only searching in encrypted storage.
func (c *AuthConfig) TokenFromKeyring(hostname string) (string, error) {
return keyring.Get(keyringServiceName(hostname), "")
}
// User will retrieve the username for the logged in user at the given hostname.
func (c *AuthConfig) User(hostname string) (string, error) {
return c.cfg.Get([]string{hosts, hostname, "user"})
}
// GitProtocol will retrieve the git protocol for the logged in user at the given hostname.
// If none is set it will return the default value.
func (c *AuthConfig) GitProtocol(hostname string) (string, error) {
key := "git_protocol"
val, err := c.cfg.Get([]string{hosts, hostname, key})
if err == nil {
return val, err
}
return defaultFor(key), nil
}
func (c *AuthConfig) Hosts() []string {
if c.hostsOverride != nil {
return c.hostsOverride()
}
return ghAuth.KnownHosts()
}
// SetHosts will override any hosts resolution and return the given
// hosts for all calls to Hosts. Use for testing purposes only.
func (c *AuthConfig) SetHosts(hosts []string) {
c.hostsOverride = func() []string {
return hosts
}
}
func (c *AuthConfig) DefaultHost() (string, string) {
if c.defaultHostOverride != nil {
return c.defaultHostOverride()
}
return ghAuth.DefaultHost()
}
// SetDefaultHost will override any host resolution and return the given
// host and source for all calls to DefaultHost. Use for testing purposes only.
func (c *AuthConfig) SetDefaultHost(host, source string) {
c.defaultHostOverride = func() (string, string) {
return host, source
}
}
// Login will set user, git protocol, and auth token for the given hostname.
// If the encrypt option is specified it will first try to store the auth token
// in encrypted storage and will fall back to the plain text config file.
func (c *AuthConfig) Login(hostname, username, token, gitProtocol string, secureStorage bool) error {
var setErr error
if secureStorage {
if setErr = keyring.Set(keyringServiceName(hostname), "", token); setErr == nil {
// Clean up the previous oauth_token from the config file.
_ = c.cfg.Remove([]string{hosts, hostname, oauthToken})
}
}
if !secureStorage || setErr != nil {
c.cfg.Set([]string{hosts, hostname, oauthToken}, token)
}
if username != "" {
c.cfg.Set([]string{hosts, hostname, "user"}, username)
}
if gitProtocol != "" {
c.cfg.Set([]string{hosts, hostname, "git_protocol"}, gitProtocol)
}
return ghConfig.Write(c.cfg)
}
// Logout will remove user, git protocol, and auth token for the given hostname.
// It will remove the auth token from the encrypted storage if it exists there.
func (c *AuthConfig) Logout(hostname string) error {
if hostname == "" {
return nil
}
_ = c.cfg.Remove([]string{hosts, hostname})
_ = keyring.Delete(keyringServiceName(hostname), "")
return ghConfig.Write(c.cfg)
}
func keyringServiceName(hostname string) string {
return "gh:" + hostname
}
type AliasConfig struct {
cfg *ghConfig.Config
}

177
internal/config/config_mock.go
View file

@ -20,11 +20,8 @@ var _ Config = &ConfigMock{}
// AliasesFunc: func() *AliasConfig {
// panic("mock out the Aliases method")
// },
// AuthTokenFunc: func(s string) (string, string) {
// panic("mock out the AuthToken method")
// },
// DefaultHostFunc: func() (string, string) {
// panic("mock out the DefaultHost method")
// AuthenticationFunc: func() *AuthConfig {
// panic("mock out the Authentication method")
// },
// GetFunc: func(s1 string, s2 string) (string, error) {
// panic("mock out the Get method")
@ -32,15 +29,9 @@ var _ Config = &ConfigMock{}
// GetOrDefaultFunc: func(s1 string, s2 string) (string, error) {
// panic("mock out the GetOrDefault method")
// },
// HostsFunc: func() []string {
// panic("mock out the Hosts method")
// },
// SetFunc: func(s1 string, s2 string, s3 string) {
// panic("mock out the Set method")
// },
// UnsetHostFunc: func(s string) {
// panic("mock out the UnsetHost method")
// },
// WriteFunc: func() error {
// panic("mock out the Write method")
// },
@ -54,11 +45,8 @@ type ConfigMock struct {
// AliasesFunc mocks the Aliases method.
AliasesFunc func() *AliasConfig
// AuthTokenFunc mocks the AuthToken method.
AuthTokenFunc func(s string) (string, string)
// DefaultHostFunc mocks the DefaultHost method.
DefaultHostFunc func() (string, string)
// AuthenticationFunc mocks the Authentication method.
AuthenticationFunc func() *AuthConfig
// GetFunc mocks the Get method.
GetFunc func(s1 string, s2 string) (string, error)
@ -66,15 +54,9 @@ type ConfigMock struct {
// GetOrDefaultFunc mocks the GetOrDefault method.
GetOrDefaultFunc func(s1 string, s2 string) (string, error)
// HostsFunc mocks the Hosts method.
HostsFunc func() []string
// SetFunc mocks the Set method.
SetFunc func(s1 string, s2 string, s3 string)
// UnsetHostFunc mocks the UnsetHost method.
UnsetHostFunc func(s string)
// WriteFunc mocks the Write method.
WriteFunc func() error
@ -83,13 +65,8 @@ type ConfigMock struct {
// Aliases holds details about calls to the Aliases method.
Aliases []struct {
}
// AuthToken holds details about calls to the AuthToken method.
AuthToken []struct {
// S is the s argument value.
S string
}
// DefaultHost holds details about calls to the DefaultHost method.
DefaultHost []struct {
// Authentication holds details about calls to the Authentication method.
Authentication []struct {
}
// Get holds details about calls to the Get method.
Get []struct {
@ -105,9 +82,6 @@ type ConfigMock struct {
// S2 is the s2 argument value.
S2 string
}
// Hosts holds details about calls to the Hosts method.
Hosts []struct {
}
// Set holds details about calls to the Set method.
Set []struct {
// S1 is the s1 argument value.
@ -117,24 +91,16 @@ type ConfigMock struct {
// S3 is the s3 argument value.
S3 string
}
// UnsetHost holds details about calls to the UnsetHost method.
UnsetHost []struct {
// S is the s argument value.
S string
}
// Write holds details about calls to the Write method.
Write []struct {
}
}
lockAliases sync.RWMutex
lockAuthToken sync.RWMutex
lockDefaultHost sync.RWMutex
lockGet sync.RWMutex
lockGetOrDefault sync.RWMutex
lockHosts sync.RWMutex
lockSet sync.RWMutex
lockUnsetHost sync.RWMutex
lockWrite sync.RWMutex
lockAliases sync.RWMutex
lockAuthentication sync.RWMutex
lockGet sync.RWMutex
lockGetOrDefault sync.RWMutex
lockSet sync.RWMutex
lockWrite sync.RWMutex
}
// Aliases calls AliasesFunc.
@ -164,62 +130,30 @@ func (mock *ConfigMock) AliasesCalls() []struct {
return calls
}
// AuthToken calls AuthTokenFunc.
func (mock *ConfigMock) AuthToken(s string) (string, string) {
if mock.AuthTokenFunc == nil {
panic("ConfigMock.AuthTokenFunc: method is nil but Config.AuthToken was just called")
}
callInfo := struct {
S string
}{
S: s,
}
mock.lockAuthToken.Lock()
mock.calls.AuthToken = append(mock.calls.AuthToken, callInfo)
mock.lockAuthToken.Unlock()
return mock.AuthTokenFunc(s)
}
// AuthTokenCalls gets all the calls that were made to AuthToken.
// Check the length with:
//
// len(mockedConfig.AuthTokenCalls())
func (mock *ConfigMock) AuthTokenCalls() []struct {
S string
} {
var calls []struct {
S string
}
mock.lockAuthToken.RLock()
calls = mock.calls.AuthToken
mock.lockAuthToken.RUnlock()
return calls
}
// DefaultHost calls DefaultHostFunc.
func (mock *ConfigMock) DefaultHost() (string, string) {
if mock.DefaultHostFunc == nil {
panic("ConfigMock.DefaultHostFunc: method is nil but Config.DefaultHost was just called")
// Authentication calls AuthenticationFunc.
func (mock *ConfigMock) Authentication() *AuthConfig {
if mock.AuthenticationFunc == nil {
panic("ConfigMock.AuthenticationFunc: method is nil but Config.Authentication was just called")
}
callInfo := struct {
}{}
mock.lockDefaultHost.Lock()
mock.calls.DefaultHost = append(mock.calls.DefaultHost, callInfo)
mock.lockDefaultHost.Unlock()
return mock.DefaultHostFunc()
mock.lockAuthentication.Lock()
mock.calls.Authentication = append(mock.calls.Authentication, callInfo)
mock.lockAuthentication.Unlock()
return mock.AuthenticationFunc()
}
// DefaultHostCalls gets all the calls that were made to DefaultHost.
// AuthenticationCalls gets all the calls that were made to Authentication.
// Check the length with:
//
// len(mockedConfig.DefaultHostCalls())
func (mock *ConfigMock) DefaultHostCalls() []struct {
// len(mockedConfig.AuthenticationCalls())
func (mock *ConfigMock) AuthenticationCalls() []struct {
} {
var calls []struct {
}
mock.lockDefaultHost.RLock()
calls = mock.calls.DefaultHost
mock.lockDefaultHost.RUnlock()
mock.lockAuthentication.RLock()
calls = mock.calls.Authentication
mock.lockAuthentication.RUnlock()
return calls
}
@ -295,33 +229,6 @@ func (mock *ConfigMock) GetOrDefaultCalls() []struct {
return calls
}
// Hosts calls HostsFunc.
func (mock *ConfigMock) Hosts() []string {
if mock.HostsFunc == nil {
panic("ConfigMock.HostsFunc: method is nil but Config.Hosts was just called")
}
callInfo := struct {
}{}
mock.lockHosts.Lock()
mock.calls.Hosts = append(mock.calls.Hosts, callInfo)
mock.lockHosts.Unlock()
return mock.HostsFunc()
}
// HostsCalls gets all the calls that were made to Hosts.
// Check the length with:
//
// len(mockedConfig.HostsCalls())
func (mock *ConfigMock) HostsCalls() []struct {
} {
var calls []struct {
}
mock.lockHosts.RLock()
calls = mock.calls.Hosts
mock.lockHosts.RUnlock()
return calls
}
// Set calls SetFunc.
func (mock *ConfigMock) Set(s1 string, s2 string, s3 string) {
if mock.SetFunc == nil {
@ -362,38 +269,6 @@ func (mock *ConfigMock) SetCalls() []struct {
return calls
}
// UnsetHost calls UnsetHostFunc.
func (mock *ConfigMock) UnsetHost(s string) {
if mock.UnsetHostFunc == nil {
panic("ConfigMock.UnsetHostFunc: method is nil but Config.UnsetHost was just called")
}
callInfo := struct {
S string
}{
S: s,
}
mock.lockUnsetHost.Lock()
mock.calls.UnsetHost = append(mock.calls.UnsetHost, callInfo)
mock.lockUnsetHost.Unlock()
mock.UnsetHostFunc(s)
}
// UnsetHostCalls gets all the calls that were made to UnsetHost.
// Check the length with:
//
// len(mockedConfig.UnsetHostCalls())
func (mock *ConfigMock) UnsetHostCalls() []struct {
S string
} {
var calls []struct {
S string
}
mock.lockUnsetHost.RLock()
calls = mock.calls.UnsetHost
mock.lockUnsetHost.RUnlock()
return calls
}
// Write calls WriteFunc.
func (mock *ConfigMock) Write() error {
if mock.WriteFunc == nil {

32
internal/config/stub.go
View file

@ -34,10 +34,6 @@ func NewFromString(cfgStr string) *ConfigMock {
c := ghConfig.ReadFromString(cfgStr)
cfg := cfg{c}
mock := &ConfigMock{}
mock.AuthTokenFunc = func(host string) (string, string) {
token, _ := c.Get([]string{"hosts", host, "oauth_token"})
return token, "oauth_token"
}
mock.GetFunc = func(host, key string) (string, error) {
return cfg.Get(host, key)
}
@ -47,21 +43,27 @@ func NewFromString(cfgStr string) *ConfigMock {
mock.SetFunc = func(host, key, value string) {
cfg.Set(host, key, value)
}
mock.UnsetHostFunc = func(host string) {
cfg.UnsetHost(host)
}
mock.HostsFunc = func() []string {
keys, _ := c.Keys([]string{"hosts"})
return keys
}
mock.DefaultHostFunc = func() (string, string) {
return "github.com", "default"
mock.WriteFunc = func() error {
return cfg.Write()
}
mock.AliasesFunc = func() *AliasConfig {
return &AliasConfig{cfg: c}
}
mock.WriteFunc = func() error {
return cfg.Write()
mock.AuthenticationFunc = func() *AuthConfig {
return &AuthConfig{
cfg: c,
defaultHostOverride: func() (string, string) {
return "github.com", "default"
},
hostsOverride: func() []string {
keys, _ := c.Keys([]string{"hosts"})
return keys
},
tokenOverride: func(hostname string) (string, string) {
token, _ := c.Get([]string{hosts, hostname, oauthToken})
return token, "oauth_token"
},
}
}
return mock
}

2
pkg/cmd/api/api.go
View file

@ -309,7 +309,7 @@ func apiRun(opts *ApiOptions) error {
return err
}
host, _ := cfg.DefaultHost()
host, _ := cfg.Authentication().DefaultHost()
if opts.Hostname != "" {
host = opts.Hostname

16
pkg/cmd/auth/gitcredential/helper.go
View file

@ -14,8 +14,8 @@ import (
const tokenUser = "x-access-token"
type config interface {
AuthToken(string) (string, string)
Get(string, string) (string, error)
Token(string) (string, string)
User(string) (string, error)
}
type CredentialOptions struct {
@ -29,7 +29,11 @@ func NewCmdCredential(f *cmdutil.Factory, runF func(*CredentialOptions) error) *
opts := &CredentialOptions{
IO: f.IOStreams,
Config: func() (config, error) {
return f.Config()
cfg, err := f.Config()
if err != nil {
return nil, err
}
return cfg.Authentication(), nil
},
}
@ -108,16 +112,16 @@ func helperRun(opts *CredentialOptions) error {
lookupHost := wants["host"]
var gotUser string
gotToken, source := cfg.AuthToken(lookupHost)
gotToken, source := cfg.Token(lookupHost)
if gotToken == "" && strings.HasPrefix(lookupHost, "gist.") {
lookupHost = strings.TrimPrefix(lookupHost, "gist.")
gotToken, source = cfg.AuthToken(lookupHost)
gotToken, source = cfg.Token(lookupHost)
}
if strings.HasSuffix(source, "_TOKEN") {
gotUser = tokenUser
} else {
gotUser, _ = cfg.Get(lookupHost, "user")
gotUser, _ = cfg.User(lookupHost)
if gotUser == "" {
gotUser = tokenUser
}

7
pkg/cmd/auth/gitcredential/helper_test.go
View file

@ -8,15 +8,14 @@ import (
"github.com/cli/cli/v2/pkg/iostreams"
)
// why not just use the config stub argh
type tinyConfig map[string]string
func (c tinyConfig) AuthToken(host string) (string, string) {
func (c tinyConfig) Token(host string) (string, string) {
return c[fmt.Sprintf("%s:%s", host, "oauth_token")], c["_source"]
}
func (c tinyConfig) Get(host, key string) (string, error) {
return c[fmt.Sprintf("%s:%s", host, key)], nil
func (c tinyConfig) User(host string) (string, error) {
return c[fmt.Sprintf("%s:%s", host, "user")], nil
}
func Test_helperRun(t *testing.T) {

57
pkg/cmd/auth/login/login.go
View file

@ -8,6 +8,7 @@ import (
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/git"
"github.com/cli/cli/v2/internal/browser"
"github.com/cli/cli/v2/internal/config"
"github.com/cli/cli/v2/internal/ghinstance"
"github.com/cli/cli/v2/pkg/cmd/auth/shared"
@ -23,16 +24,18 @@ type LoginOptions struct {
HttpClient func() (*http.Client, error)
GitClient *git.Client
Prompter shared.Prompt
Browser browser.Browser
MainExecutable string
Interactive bool
Hostname string
Scopes []string
Token string
Web bool
GitProtocol string
Hostname string
Scopes []string
Token string
Web bool
GitProtocol string
SecureStorage bool
}
func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Command {
@ -42,6 +45,7 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm
HttpClient: f.HttpClient,
GitClient: f.GitClient,
Prompter: f.Prompter,
Browser: f.Browser,
}
var tokenStdin bool
@ -120,6 +124,7 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm
cmd.Flags().BoolVar(&tokenStdin, "with-token", false, "Read token from standard input")
cmd.Flags().BoolVarP(&opts.Web, "web", "w", false, "Open a browser to authenticate")
cmdutil.StringEnumFlag(cmd, &opts.GitProtocol, "git-protocol", "p", "", []string{"ssh", "https"}, "The protocol to use for git operations")
cmd.Flags().BoolVarP(&opts.SecureStorage, "secure-storage", "", false, "Save authentication credentials in secure credential store")
return cmd
}
@ -129,6 +134,12 @@ func loginRun(opts *LoginOptions) error {
if err != nil {
return err
}
authCfg := cfg.Authentication()
if opts.SecureStorage {
cs := opts.IO.ColorScheme()
fmt.Fprintf(opts.IO.ErrOut, "%s Using secure storage could break installed extensions\n", cs.WarningIcon())
}
hostname := opts.Hostname
if opts.Interactive && hostname == "" {
@ -139,7 +150,7 @@ func loginRun(opts *LoginOptions) error {
}
}
if src, writeable := shared.AuthTokenWriteable(cfg, hostname); !writeable {
if src, writeable := shared.AuthTokenWriteable(authCfg, hostname); !writeable {
fmt.Fprintf(opts.IO.ErrOut, "The value of the %s environment variable is being used for authentication.\n", src)
fmt.Fprint(opts.IO.ErrOut, "To have GitHub CLI store credentials instead, first clear the value from the environment.\n")
return cmdutil.SilentError
@ -151,18 +162,14 @@ func loginRun(opts *LoginOptions) error {
}
if opts.Token != "" {
cfg.Set(hostname, "oauth_token", opts.Token)
if err := shared.HasMinimumScopes(httpClient, hostname, opts.Token); err != nil {
return fmt.Errorf("error validating token: %w", err)
}
if opts.GitProtocol != "" {
cfg.Set(hostname, "git_protocol", opts.GitProtocol)
}
return cfg.Write()
// Adding a user key ensures that a nonempty host section gets written to the config file.
return authCfg.Login(hostname, "x-access-token", opts.Token, opts.GitProtocol, opts.SecureStorage)
}
existingToken, _ := cfg.AuthToken(hostname)
existingToken, _ := authCfg.Token(hostname)
if existingToken != "" && opts.Interactive {
if err := shared.HasMinimumScopes(httpClient, hostname, existingToken); err == nil {
keepGoing, err := opts.Prompter.Confirm(fmt.Sprintf("You're already logged into %s. Do you want to re-authenticate?", hostname), false)
@ -176,17 +183,19 @@ func loginRun(opts *LoginOptions) error {
}
return shared.Login(&shared.LoginOptions{
IO: opts.IO,
Config: cfg,
HTTPClient: httpClient,
Hostname: hostname,
Interactive: opts.Interactive,
Web: opts.Web,
Scopes: opts.Scopes,
Executable: opts.MainExecutable,
GitProtocol: opts.GitProtocol,
Prompter: opts.Prompter,
GitClient: opts.GitClient,
IO: opts.IO,
Config: authCfg,
HTTPClient: httpClient,
Hostname: hostname,
Interactive: opts.Interactive,
Web: opts.Web,
Scopes: opts.Scopes,
Executable: opts.MainExecutable,
GitProtocol: opts.GitProtocol,
Prompter: opts.Prompter,
GitClient: opts.GitClient,
Browser: opts.Browser,
SecureStorage: opts.SecureStorage,
})
}

181
pkg/cmd/auth/login/login_test.go
View file

@ -17,6 +17,7 @@ import (
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/google/shlex"
"github.com/stretchr/testify/assert"
"github.com/zalando/go-keyring"
)
func stubHomeDir(t *testing.T, dir string) {
@ -172,6 +173,23 @@ func Test_NewCmdLogin(t *testing.T) {
Interactive: true,
},
},
{
name: "tty secure-storage",
stdinTTY: true,
cli: "--secure-storage",
wants: LoginOptions{
Interactive: true,
SecureStorage: true,
},
},
{
name: "nontty secure-storage",
cli: "--secure-storage",
wants: LoginOptions{
Hostname: "github.com",
SecureStorage: true,
},
},
}
for _, tt := range tests {
@ -223,13 +241,14 @@ func Test_NewCmdLogin(t *testing.T) {
func Test_loginRun_nontty(t *testing.T) {
tests := []struct {
name string
opts *LoginOptions
httpStubs func(*httpmock.Registry)
cfgStubs func(*config.ConfigMock)
wantHosts string
wantErr string
wantStderr string
name string
opts *LoginOptions
httpStubs func(*httpmock.Registry)
cfgStubs func(*config.ConfigMock)
wantHosts string
wantErr string
wantStderr string
wantSecureToken string
}{
{
name: "with token",
@ -240,7 +259,7 @@ func Test_loginRun_nontty(t *testing.T) {
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.REST("GET", ""), httpmock.ScopesResponder("repo,read:org"))
},
wantHosts: "github.com:\n oauth_token: abc123\n",
wantHosts: "github.com:\n oauth_token: abc123\n user: x-access-token\n",
},
{
name: "with token and https git-protocol",
@ -252,7 +271,7 @@ func Test_loginRun_nontty(t *testing.T) {
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.REST("GET", ""), httpmock.ScopesResponder("repo,read:org"))
},
wantHosts: "github.com:\n oauth_token: abc123\n git_protocol: https\n",
wantHosts: "github.com:\n oauth_token: abc123\n user: x-access-token\n git_protocol: https\n",
},
{
name: "with token and non-default host",
@ -263,7 +282,7 @@ func Test_loginRun_nontty(t *testing.T) {
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.REST("GET", "api/v3/"), httpmock.ScopesResponder("repo,read:org"))
},
wantHosts: "albert.wesker:\n oauth_token: abc123\n",
wantHosts: "albert.wesker:\n oauth_token: abc123\n user: x-access-token\n",
},
{
name: "missing repo scope",
@ -296,7 +315,7 @@ func Test_loginRun_nontty(t *testing.T) {
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.REST("GET", ""), httpmock.ScopesResponder("repo,admin:org"))
},
wantHosts: "github.com:\n oauth_token: abc456\n",
wantHosts: "github.com:\n oauth_token: abc456\n user: x-access-token\n",
},
{
name: "github.com token from environment",
@ -305,8 +324,10 @@ func Test_loginRun_nontty(t *testing.T) {
Token: "abc456",
},
cfgStubs: func(c *config.ConfigMock) {
c.AuthTokenFunc = func(string) (string, string) {
return "value_from_env", "GH_TOKEN"
authCfg := c.Authentication()
authCfg.SetToken("value_from_env", "GH_TOKEN")
c.AuthenticationFunc = func() *config.AuthConfig {
return authCfg
}
},
wantErr: "SilentError",
@ -322,8 +343,10 @@ func Test_loginRun_nontty(t *testing.T) {
Token: "abc456",
},
cfgStubs: func(c *config.ConfigMock) {
c.AuthTokenFunc = func(string) (string, string) {
return "value_from_env", "GH_ENTERPRISE_TOKEN"
authCfg := c.Authentication()
authCfg.SetToken("value_from_env", "GH_ENTERPRISE_TOKEN")
c.AuthenticationFunc = func() *config.AuthConfig {
return authCfg
}
},
wantErr: "SilentError",
@ -332,15 +355,30 @@ func Test_loginRun_nontty(t *testing.T) {
To have GitHub CLI store credentials instead, first clear the value from the environment.
`),
},
{
name: "with token and secure storage",
opts: &LoginOptions{
Hostname: "github.com",
Token: "abc123",
SecureStorage: true,
},
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.REST("GET", ""), httpmock.ScopesResponder("repo,read:org"))
},
wantHosts: "github.com:\n user: x-access-token\n",
wantSecureToken: "abc123",
wantStderr: "! Using secure storage could break installed extensions\n",
},
}
for _, tt := range tests {
ios, _, stdout, stderr := iostreams.Test()
ios.SetStdinTTY(false)
ios.SetStdoutTTY(false)
tt.opts.IO = ios
t.Run(tt.name, func(t *testing.T) {
ios, _, stdout, stderr := iostreams.Test()
ios.SetStdinTTY(false)
ios.SetStdoutTTY(false)
tt.opts.IO = ios
keyring.MockInit()
readConfigs := config.StubWriteConfig(t)
cfg := config.NewBlankConfig()
if tt.cfgStubs != nil {
@ -351,6 +389,7 @@ func Test_loginRun_nontty(t *testing.T) {
}
reg := &httpmock.Registry{}
defer reg.Verify(t)
tt.opts.HttpClient = func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
}
@ -371,11 +410,12 @@ func Test_loginRun_nontty(t *testing.T) {
mainBuf := bytes.Buffer{}
hostsBuf := bytes.Buffer{}
readConfigs(&mainBuf, &hostsBuf)
secureToken, _ := cfg.Authentication().TokenFromKeyring(tt.opts.Hostname)
assert.Equal(t, "", stdout.String())
assert.Equal(t, tt.wantStderr, stderr.String())
assert.Equal(t, tt.wantHosts, hostsBuf.String())
reg.Verify(t)
assert.Equal(t, tt.wantSecureToken, secureToken)
})
}
}
@ -384,14 +424,15 @@ func Test_loginRun_Survey(t *testing.T) {
stubHomeDir(t, t.TempDir())
tests := []struct {
name string
opts *LoginOptions
httpStubs func(*httpmock.Registry)
prompterStubs func(*prompter.PrompterMock)
runStubs func(*run.CommandStubber)
wantHosts string
wantErrOut *regexp.Regexp
cfgStubs func(*config.ConfigMock)
name string
opts *LoginOptions
httpStubs func(*httpmock.Registry)
prompterStubs func(*prompter.PrompterMock)
runStubs func(*run.CommandStubber)
cfgStubs func(*config.ConfigMock)
wantHosts string
wantErrOut *regexp.Regexp
wantSecureToken string
}{
{
name: "already authenticated",
@ -399,8 +440,10 @@ func Test_loginRun_Survey(t *testing.T) {
Interactive: true,
},
cfgStubs: func(c *config.ConfigMock) {
c.AuthTokenFunc = func(h string) (string, string) {
return "ghi789", "oauth_token"
authCfg := c.Authentication()
authCfg.SetToken("ghi789", "oauth_token")
c.AuthenticationFunc = func() *config.AuthConfig {
return authCfg
}
},
httpStubs: func(reg *httpmock.Registry) {
@ -547,32 +590,62 @@ func Test_loginRun_Survey(t *testing.T) {
},
wantErrOut: regexp.MustCompile("Tip: you can generate a Personal Access Token here https://github.com/settings/tokens"),
},
// TODO how to test browser auth?
{
name: "secure storage",
opts: &LoginOptions{
Hostname: "github.com",
Interactive: true,
SecureStorage: true,
},
prompterStubs: func(pm *prompter.PrompterMock) {
pm.SelectFunc = func(prompt, _ string, opts []string) (int, error) {
switch prompt {
case "What is your preferred protocol for Git operations?":
return prompter.IndexFor(opts, "HTTPS")
case "How would you like to authenticate GitHub CLI?":
return prompter.IndexFor(opts, "Paste an authentication token")
}
return -1, prompter.NoSuchPromptErr(prompt)
}
},
runStubs: func(rs *run.CommandStubber) {
rs.Register(`git config credential\.https:/`, 1, "")
rs.Register(`git config credential\.helper`, 1, "")
},
wantHosts: heredoc.Doc(`
github.com:
user: jillv
git_protocol: https
`),
wantErrOut: regexp.MustCompile("! Using secure storage could break installed extensions"),
wantSecureToken: "def456",
},
}
for _, tt := range tests {
if tt.opts == nil {
tt.opts = &LoginOptions{}
}
ios, _, _, stderr := iostreams.Test()
ios.SetStdinTTY(true)
ios.SetStderrTTY(true)
ios.SetStdoutTTY(true)
tt.opts.IO = ios
readConfigs := config.StubWriteConfig(t)
cfg := config.NewBlankConfig()
if tt.cfgStubs != nil {
tt.cfgStubs(cfg)
}
tt.opts.Config = func() (config.Config, error) {
return cfg, nil
}
t.Run(tt.name, func(t *testing.T) {
if tt.opts == nil {
tt.opts = &LoginOptions{}
}
ios, _, _, stderr := iostreams.Test()
ios.SetStdinTTY(true)
ios.SetStderrTTY(true)
ios.SetStdoutTTY(true)
tt.opts.IO = ios
keyring.MockInit()
readConfigs := config.StubWriteConfig(t)
cfg := config.NewBlankConfig()
if tt.cfgStubs != nil {
tt.cfgStubs(cfg)
}
tt.opts.Config = func() (config.Config, error) {
return cfg, nil
}
reg := &httpmock.Registry{}
tt.opts.HttpClient = func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
@ -614,8 +687,10 @@ func Test_loginRun_Survey(t *testing.T) {
mainBuf := bytes.Buffer{}
hostsBuf := bytes.Buffer{}
readConfigs(&mainBuf, &hostsBuf)
secureToken, _ := cfg.Authentication().TokenFromKeyring(tt.opts.Hostname)
assert.Equal(t, tt.wantHosts, hostsBuf.String())
assert.Equal(t, tt.wantSecureToken, secureToken)
if tt.wantErrOut == nil {
assert.Equal(t, "", stderr.String())
} else {

11
pkg/cmd/auth/logout/logout.go
View file

@ -69,8 +69,9 @@ func logoutRun(opts *LogoutOptions) error {
if err != nil {
return err
}
authCfg := cfg.Authentication()
candidates := cfg.Hosts()
candidates := authCfg.Hosts()
if len(candidates) == 0 {
return fmt.Errorf("not logged in to any hosts")
}
@ -100,7 +101,7 @@ func logoutRun(opts *LogoutOptions) error {
}
}
if src, writeable := shared.AuthTokenWriteable(cfg, hostname); !writeable {
if src, writeable := shared.AuthTokenWriteable(authCfg, hostname); !writeable {
fmt.Fprintf(opts.IO.ErrOut, "The value of the %s environment variable is being used for authentication.\n", src)
fmt.Fprint(opts.IO.ErrOut, "To erase credentials stored in GitHub CLI, first clear the value from the environment.\n")
return cmdutil.SilentError
@ -116,7 +117,7 @@ func logoutRun(opts *LogoutOptions) error {
if err != nil {
// suppressing; the user is trying to delete this token and it might be bad.
// we'll see if the username is in the config and fall back to that.
username, _ = cfg.Get(hostname, "user")
username, _ = authCfg.User(hostname)
}
usernameStr := ""
@ -124,9 +125,7 @@ func logoutRun(opts *LogoutOptions) error {
usernameStr = fmt.Sprintf(" account '%s'", username)
}
cfg.UnsetHost(hostname)
err = cfg.Write()
if err != nil {
if err := authCfg.Logout(hostname); err != nil {
return fmt.Errorf("failed to write config, authentication configuration not updated: %w", err)
}

57
pkg/cmd/auth/logout/logout_test.go
View file

@ -2,6 +2,7 @@ package logout
import (
"bytes"
"fmt"
"net/http"
"regexp"
"testing"
@ -13,6 +14,7 @@ import (
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/google/shlex"
"github.com/stretchr/testify/assert"
"github.com/zalando/go-keyring"
)
func Test_NewCmdLogout(t *testing.T) {
@ -96,6 +98,7 @@ func Test_logoutRun_tty(t *testing.T) {
opts *LogoutOptions
prompterStubs func(*prompter.PrompterMock)
cfgHosts []string
secureStorage bool
wantHosts string
wantErrOut *regexp.Regexp
wantErr string
@ -133,14 +136,31 @@ func Test_logoutRun_tty(t *testing.T) {
wantHosts: "github.com:\n oauth_token: abc123\n",
wantErrOut: regexp.MustCompile(`Logged out of cheryl.mason account 'cybilb'`),
},
{
name: "secure storage",
secureStorage: true,
opts: &LogoutOptions{
Hostname: "github.com",
},
cfgHosts: []string{"github.com"},
wantHosts: "{}\n",
wantErrOut: regexp.MustCompile(`Logged out of github.com account 'cybilb'`),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
keyring.MockInit()
readConfigs := config.StubWriteConfig(t)
cfg := config.NewFromString("")
for _, hostname := range tt.cfgHosts {
cfg.Set(hostname, "oauth_token", "abc123")
if tt.secureStorage {
cfg.Set(hostname, "user", "monalisa")
_ = keyring.Set(fmt.Sprintf("gh:%s", hostname), "", "abc123")
cfg.Authentication().SetToken("abc123", "keyring")
} else {
cfg.Set(hostname, "oauth_token", "abc123")
}
}
tt.opts.Config = func() (config.Config, error) {
return cfg, nil
@ -183,8 +203,10 @@ func Test_logoutRun_tty(t *testing.T) {
mainBuf := bytes.Buffer{}
hostsBuf := bytes.Buffer{}
readConfigs(&mainBuf, &hostsBuf)
secureToken, _ := cfg.Authentication().TokenFromKeyring(tt.opts.Hostname)
assert.Equal(t, tt.wantHosts, hostsBuf.String())
assert.Equal(t, "", secureToken)
reg.Verify(t)
})
}
@ -192,12 +214,13 @@ func Test_logoutRun_tty(t *testing.T) {
func Test_logoutRun_nontty(t *testing.T) {
tests := []struct {
name string
opts *LogoutOptions
cfgHosts []string
wantHosts string
wantErr string
ghtoken string
name string
opts *LogoutOptions
cfgHosts []string
secureStorage bool
ghtoken string
wantHosts string
wantErr string
}{
{
name: "hostname, one host",
@ -222,14 +245,30 @@ func Test_logoutRun_nontty(t *testing.T) {
},
wantErr: `not logged in to any hosts`,
},
{
name: "secure storage",
secureStorage: true,
opts: &LogoutOptions{
Hostname: "harry.mason",
},
cfgHosts: []string{"harry.mason"},
wantHosts: "{}\n",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
keyring.MockInit()
readConfigs := config.StubWriteConfig(t)
cfg := config.NewFromString("")
for _, hostname := range tt.cfgHosts {
cfg.Set(hostname, "oauth_token", "abc123")
if tt.secureStorage {
cfg.Set(hostname, "user", "monalisa")
_ = keyring.Set(fmt.Sprintf("gh:%s", hostname), "", "abc123")
cfg.Authentication().SetToken("abc123", "keyring")
} else {
cfg.Set(hostname, "oauth_token", "abc123")
}
}
tt.opts.Config = func() (config.Config, error) {
return cfg, nil
@ -257,8 +296,10 @@ func Test_logoutRun_nontty(t *testing.T) {
mainBuf := bytes.Buffer{}
hostsBuf := bytes.Buffer{}
readConfigs(&mainBuf, &hostsBuf)
secureToken, _ := cfg.Authentication().TokenFromKeyring(tt.opts.Hostname)
assert.Equal(t, tt.wantHosts, hostsBuf.String())
assert.Equal(t, "", secureToken)
reg.Verify(t)
})
}

34
pkg/cmd/auth/refresh/refresh.go
View file

@ -26,18 +26,26 @@ type RefreshOptions struct {
Hostname string
Scopes []string
AuthFlow func(config.Config, *iostreams.IOStreams, string, []string, bool) error
AuthFlow func(*config.AuthConfig, *iostreams.IOStreams, string, []string, bool, bool) error
Interactive bool
Interactive bool
SecureStorage bool
}
func NewCmdRefresh(f *cmdutil.Factory, runF func(*RefreshOptions) error) *cobra.Command {
opts := &RefreshOptions{
IO: f.IOStreams,
Config: f.Config,
AuthFlow: func(cfg config.Config, io *iostreams.IOStreams, hostname string, scopes []string, interactive bool) error {
_, err := authflow.AuthFlowWithConfig(cfg, io, hostname, "", scopes, interactive)
return err
AuthFlow: func(authCfg *config.AuthConfig, io *iostreams.IOStreams, hostname string, scopes []string, interactive, secureStorage bool) error {
if secureStorage {
cs := io.ColorScheme()
fmt.Fprintf(io.ErrOut, "%s Using secure storage could break installed extensions", cs.WarningIcon())
}
token, username, err := authflow.AuthFlow(hostname, io, "", scopes, interactive, f.Browser)
if err != nil {
return err
}
return authCfg.Login(hostname, username, token, "", secureStorage)
},
HttpClient: &http.Client{},
GitClient: f.GitClient,
@ -77,6 +85,7 @@ func NewCmdRefresh(f *cmdutil.Factory, runF func(*RefreshOptions) error) *cobra.
cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "The GitHub host to use for authentication")
cmd.Flags().StringSliceVarP(&opts.Scopes, "scopes", "s", nil, "Additional authentication scopes for gh to have")
cmd.Flags().BoolVarP(&opts.SecureStorage, "secure-storage", "", false, "Save authentication credentials in secure credential store")
return cmd
}
@ -86,8 +95,9 @@ func refreshRun(opts *RefreshOptions) error {
if err != nil {
return err
}
authCfg := cfg.Authentication()
candidates := cfg.Hosts()
candidates := authCfg.Hosts()
if len(candidates) == 0 {
return fmt.Errorf("not logged in to any hosts. Use 'gh auth login' to authenticate with a host")
}
@ -117,14 +127,14 @@ func refreshRun(opts *RefreshOptions) error {
}
}
if src, writeable := shared.AuthTokenWriteable(cfg, hostname); !writeable {
if src, writeable := shared.AuthTokenWriteable(authCfg, hostname); !writeable {
fmt.Fprintf(opts.IO.ErrOut, "The value of the %s environment variable is being used for authentication.\n", src)
fmt.Fprint(opts.IO.ErrOut, "To refresh credentials stored in GitHub CLI, first clear the value from the environment.\n")
return cmdutil.SilentError
}
var additionalScopes []string
if oldToken, _ := cfg.AuthToken(hostname); oldToken != "" {
if oldToken, _ := authCfg.Token(hostname); oldToken != "" {
if oldScopes, err := shared.GetScopes(opts.HttpClient, hostname, oldToken); err == nil {
for _, s := range strings.Split(oldScopes, ",") {
s = strings.TrimSpace(s)
@ -140,7 +150,7 @@ func refreshRun(opts *RefreshOptions) error {
Prompter: opts.Prompter,
GitClient: opts.GitClient,
}
gitProtocol, _ := cfg.GetOrDefault(hostname, "git_protocol")
gitProtocol, _ := authCfg.GitProtocol(hostname)
if opts.Interactive && gitProtocol == "https" {
if err := credentialFlow.Prompt(hostname); err != nil {
return err
@ -148,7 +158,7 @@ func refreshRun(opts *RefreshOptions) error {
additionalScopes = append(additionalScopes, credentialFlow.Scopes()...)
}
if err := opts.AuthFlow(cfg, opts.IO, hostname, append(opts.Scopes, additionalScopes...), opts.Interactive); err != nil {
if err := opts.AuthFlow(authCfg, opts.IO, hostname, append(opts.Scopes, additionalScopes...), opts.Interactive, opts.SecureStorage); err != nil {
return err
}
@ -156,8 +166,8 @@ func refreshRun(opts *RefreshOptions) error {
fmt.Fprintf(opts.IO.ErrOut, "%s Authentication complete.\n", cs.SuccessIcon())
if credentialFlow.ShouldSetup() {
username, _ := cfg.Get(hostname, "user")
password, _ := cfg.AuthToken(hostname)
username, _ := authCfg.User(hostname)
password, _ := authCfg.Token(hostname)
if err := credentialFlow.Setup(hostname, username, password); err != nil {
return err
}

36
pkg/cmd/auth/refresh/refresh_test.go
View file

@ -85,6 +85,14 @@ func Test_NewCmdRefresh(t *testing.T) {
Scopes: []string{"repo:invite", "read:public_key"},
},
},
{
name: "secure storage",
tty: true,
cli: "--secure-storage",
wants: RefreshOptions{
SecureStorage: true,
},
},
}
for _, tt := range tests {
@ -126,8 +134,10 @@ func Test_NewCmdRefresh(t *testing.T) {
}
type authArgs struct {
hostname string
scopes []string
hostname string
scopes []string
interactive bool
secureStorage bool
}
func Test_refreshRun(t *testing.T) {
@ -230,17 +240,33 @@ func Test_refreshRun(t *testing.T) {
scopes: []string{"repo:invite", "public_key:read", "delete_repo", "codespace"},
},
},
{
name: "secure storage",
cfgHosts: []string{
"obed.morton",
},
opts: &RefreshOptions{
Hostname: "obed.morton",
SecureStorage: true,
},
wantAuthArgs: authArgs{
hostname: "obed.morton",
scopes: nil,
secureStorage: true,
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
aa := authArgs{}
tt.opts.AuthFlow = func(_ config.Config, _ *iostreams.IOStreams, hostname string, scopes []string, interactive bool) error {
tt.opts.AuthFlow = func(_ *config.AuthConfig, _ *iostreams.IOStreams, hostname string, scopes []string, interactive, secureStorage bool) error {
aa.hostname = hostname
aa.scopes = scopes
aa.interactive = interactive
aa.secureStorage = secureStorage
return nil
}
_ = config.StubWriteConfig(t)
cfg := config.NewFromString("")
for _, hostname := range tt.cfgHosts {
cfg.Set(hostname, "oauth_token", "abc123")
@ -291,6 +317,8 @@ func Test_refreshRun(t *testing.T) {
assert.Equal(t, tt.wantAuthArgs.hostname, aa.hostname)
assert.Equal(t, tt.wantAuthArgs.scopes, aa.scopes)
assert.Equal(t, tt.wantAuthArgs.interactive, aa.interactive)
assert.Equal(t, tt.wantAuthArgs.secureStorage, aa.secureStorage)
})
}
}

3
pkg/cmd/auth/setupgit/setupgit.go
View file

@ -54,8 +54,9 @@ func setupGitRun(opts *SetupGitOptions) error {
if err != nil {
return err
}
authCfg := cfg.Authentication()
hostnames := cfg.Hosts()
hostnames := authCfg.Hosts()
stderr := opts.IO.ErrOut
cs := opts.IO.ColorScheme()

30
pkg/cmd/auth/setupgit/setupgit_test.go
View file

@ -38,8 +38,10 @@ func Test_setupGitRun(t *testing.T) {
opts: &SetupGitOptions{
Config: func() (config.Config, error) {
cfg := &config.ConfigMock{}
cfg.HostsFunc = func() []string {
return []string{}
cfg.AuthenticationFunc = func() *config.AuthConfig {
authCfg := &config.AuthConfig{}
authCfg.SetHosts([]string{})
return authCfg
}
return cfg, nil
},
@ -53,8 +55,10 @@ func Test_setupGitRun(t *testing.T) {
Hostname: "foo",
Config: func() (config.Config, error) {
cfg := &config.ConfigMock{}
cfg.HostsFunc = func() []string {
return []string{"bar"}
cfg.AuthenticationFunc = func() *config.AuthConfig {
authCfg := &config.AuthConfig{}
authCfg.SetHosts([]string{"bar"})
return authCfg
}
return cfg, nil
},
@ -70,8 +74,10 @@ func Test_setupGitRun(t *testing.T) {
},
Config: func() (config.Config, error) {
cfg := &config.ConfigMock{}
cfg.HostsFunc = func() []string {
return []string{"bar"}
cfg.AuthenticationFunc = func() *config.AuthConfig {
authCfg := &config.AuthConfig{}
authCfg.SetHosts([]string{"bar"})
return authCfg
}
return cfg, nil
},
@ -85,8 +91,10 @@ func Test_setupGitRun(t *testing.T) {
gitConfigure: &mockGitConfigurer{},
Config: func() (config.Config, error) {
cfg := &config.ConfigMock{}
cfg.HostsFunc = func() []string {
return []string{"bar"}
cfg.AuthenticationFunc = func() *config.AuthConfig {
authCfg := &config.AuthConfig{}
authCfg.SetHosts([]string{"bar"})
return authCfg
}
return cfg, nil
},
@ -99,8 +107,10 @@ func Test_setupGitRun(t *testing.T) {
gitConfigure: &mockGitConfigurer{},
Config: func() (config.Config, error) {
cfg := &config.ConfigMock{}
cfg.HostsFunc = func() []string {
return []string{"bar", "yes"}
cfg.AuthenticationFunc = func() *config.AuthConfig {
authCfg := &config.AuthConfig{}
authCfg.SetHosts([]string{"bar", "yes"})
return authCfg
}
return cfg, nil
},

50
pkg/cmd/auth/shared/login_flow.go
View file

@ -10,6 +10,7 @@ import (
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/git"
"github.com/cli/cli/v2/internal/authflow"
"github.com/cli/cli/v2/internal/browser"
"github.com/cli/cli/v2/internal/ghinstance"
"github.com/cli/cli/v2/pkg/cmd/ssh-key/add"
"github.com/cli/cli/v2/pkg/iostreams"
@ -19,23 +20,23 @@ import (
const defaultSSHKeyTitle = "GitHub CLI"
type iconfig interface {
Get(string, string) (string, error)
Set(string, string, string)
Write() error
Login(string, string, string, string, bool) error
}
type LoginOptions struct {
IO *iostreams.IOStreams
Config iconfig
HTTPClient *http.Client
GitClient *git.Client
Hostname string
Interactive bool
Web bool
Scopes []string
Executable string
GitProtocol string
Prompter Prompt
IO *iostreams.IOStreams
Config iconfig
HTTPClient *http.Client
GitClient *git.Client
Hostname string
Interactive bool
Web bool
Scopes []string
Executable string
GitProtocol string
Prompter Prompt
Browser browser.Browser
SecureStorage bool
sshContext ssh.Context
}
@ -145,16 +146,15 @@ func Login(opts *LoginOptions) error {
}
var authToken string
userValidated := false
var username string
if authMode == 0 {
var err error
authToken, err = authflow.AuthFlowWithConfig(cfg, opts.IO, hostname, "", append(opts.Scopes, additionalScopes...), opts.Interactive)
authToken, username, err = authflow.AuthFlow(hostname, opts.IO, "", append(opts.Scopes, additionalScopes...), opts.Interactive, opts.Browser)
if err != nil {
return fmt.Errorf("failed to authenticate via web browser: %w", err)
}
fmt.Fprintf(opts.IO.ErrOut, "%s Authentication complete.\n", cs.SuccessIcon())
userValidated = true
} else {
minimumScopes := append([]string{"repo", "read:org"}, additionalScopes...)
fmt.Fprint(opts.IO.ErrOut, heredoc.Docf(`
@ -162,7 +162,8 @@ func Login(opts *LoginOptions) error {
The minimum required scopes are %s.
`, hostname, scopesSentence(minimumScopes, ghinstance.IsEnterprise(hostname))))
authToken, err := opts.Prompter.AuthToken()
var err error
authToken, err = opts.Prompter.AuthToken()
if err != nil {
return err
}
@ -170,32 +171,23 @@ func Login(opts *LoginOptions) error {
if err := HasMinimumScopes(httpClient, hostname, authToken); err != nil {
return fmt.Errorf("error validating token: %w", err)
}
cfg.Set(hostname, "oauth_token", authToken)
}
var username string
if userValidated {
username, _ = cfg.Get(hostname, "user")
} else {
if username == "" {
apiClient := api.NewClientFromHTTP(httpClient)
var err error
username, err = api.CurrentLoginName(apiClient, hostname)
if err != nil {
return fmt.Errorf("error using api: %w", err)
}
cfg.Set(hostname, "user", username)
}
if gitProtocol != "" {
fmt.Fprintf(opts.IO.ErrOut, "- gh config set -h %s git_protocol %s\n", hostname, gitProtocol)
cfg.Set(hostname, "git_protocol", gitProtocol)
fmt.Fprintf(opts.IO.ErrOut, "%s Configured git protocol\n", cs.SuccessIcon())
}
err := cfg.Write()
if err != nil {
if err := cfg.Login(hostname, username, authToken, gitProtocol, opts.SecureStorage); err != nil {
return err
}

13
pkg/cmd/auth/shared/login_flow_test.go
View file

@ -18,15 +18,10 @@ import (
type tinyConfig map[string]string
func (c tinyConfig) Get(host, key string) (string, error) {
return c[fmt.Sprintf("%s:%s", host, key)], nil
}
func (c tinyConfig) Set(host string, key string, value string) {
c[fmt.Sprintf("%s:%s", host, key)] = value
}
func (c tinyConfig) Write() error {
func (c tinyConfig) Login(host, username, token, gitProtocol string, encrypt bool) error {
c[fmt.Sprintf("%s:%s", host, "user")] = username
c[fmt.Sprintf("%s:%s", host, "oauth_token")] = token
c[fmt.Sprintf("%s:%s", host, "git_protocol")] = gitProtocol
return nil
}

12
pkg/cmd/auth/shared/writeable.go
View file

@ -1,14 +1,12 @@
package shared
import (
"strings"
"github.com/cli/cli/v2/internal/config"
)
const (
oauthToken = "oauth_token"
)
func AuthTokenWriteable(cfg config.Config, hostname string) (string, bool) {
token, src := cfg.AuthToken(hostname)
return src, (token == "" || src == oauthToken)
func AuthTokenWriteable(authCfg *config.AuthConfig, hostname string) (string, bool) {
token, src := authCfg.Token(hostname)
return src, (token == "" || !strings.HasSuffix(src, "_TOKEN"))
}

9
pkg/cmd/auth/status/status.go
View file

@ -61,6 +61,7 @@ func statusRun(opts *StatusOptions) error {
if err != nil {
return err
}
authCfg := cfg.Authentication()
// TODO check tty
@ -70,7 +71,7 @@ func statusRun(opts *StatusOptions) error {
statusInfo := map[string][]string{}
hostnames := cfg.Hosts()
hostnames := authCfg.Hosts()
if len(hostnames) == 0 {
fmt.Fprintf(stderr,
"You are not logged into any GitHub hosts. Run %s to authenticate.\n", cs.Bold("gh auth login"))
@ -91,13 +92,13 @@ func statusRun(opts *StatusOptions) error {
}
isHostnameFound = true
token, tokenSource := cfg.AuthToken(hostname)
token, tokenSource := authCfg.Token(hostname)
if tokenSource == "oauth_token" {
// The go-gh function TokenForHost returns this value as source for tokens read from the
// config file, but we want the file path instead. This attempts to reconstruct it.
tokenSource = filepath.Join(config.ConfigDir(), "hosts.yml")
}
_, tokenIsWriteable := shared.AuthTokenWriteable(cfg, hostname)
_, tokenIsWriteable := shared.AuthTokenWriteable(authCfg, hostname)
statusInfo[hostname] = []string{}
addMsg := func(x string, ys ...interface{}) {
@ -138,7 +139,7 @@ func statusRun(opts *StatusOptions) error {
}
addMsg("%s Logged in to %s as %s (%s)", cs.SuccessIcon(), hostname, cs.Bold(username), tokenSource)
proto, _ := cfg.GetOrDefault(hostname, "git_protocol")
proto, _ := authCfg.GitProtocol(hostname)
if proto != "" {
addMsg("%s Git operations for %s configured to use %s protocol.",
cs.SuccessIcon(), hostname, cs.Bold(proto))

24
pkg/cmd/auth/token/token.go
View file

@ -4,7 +4,6 @@ import (
"fmt"
"github.com/cli/cli/v2/internal/config"
"github.com/cli/cli/v2/internal/ghinstance"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/spf13/cobra"
@ -14,7 +13,8 @@ type TokenOptions struct {
IO *iostreams.IOStreams
Config func() (config.Config, error)
Hostname string
Hostname string
SecureStorage bool
}
func NewCmdToken(f *cmdutil.Factory, runF func(*TokenOptions) error) *cobra.Command {
@ -37,22 +37,30 @@ func NewCmdToken(f *cmdutil.Factory, runF func(*TokenOptions) error) *cobra.Comm
}
cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "The hostname of the GitHub instance authenticated with")
cmd.Flags().BoolVarP(&opts.SecureStorage, "secure-storage", "", false, "Search only secure credential store for authentication token")
_ = cmd.Flags().MarkHidden("secure-storeage")
return cmd
}
func tokenRun(opts *TokenOptions) error {
hostname := opts.Hostname
if hostname == "" {
hostname = ghinstance.Default()
}
cfg, err := opts.Config()
if err != nil {
return err
}
authCfg := cfg.Authentication()
val, _ := cfg.AuthToken(hostname)
hostname := opts.Hostname
if hostname == "" {
hostname, _ = authCfg.DefaultHost()
}
var val string
if opts.SecureStorage {
val, _ = authCfg.TokenFromKeyring(hostname)
} else {
val, _ = authCfg.Token(hostname)
}
if val == "" {
return fmt.Errorf("no oauth token")
}

93
pkg/cmd/auth/token/token_test.go
View file

@ -9,6 +9,7 @@ import (
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/google/shlex"
"github.com/stretchr/testify/assert"
"github.com/zalando/go-keyring"
)
func TestNewCmdToken(t *testing.T) {
@ -34,6 +35,11 @@ func TestNewCmdToken(t *testing.T) {
input: "-h github.mycompany.com",
output: TokenOptions{Hostname: "github.mycompany.com"},
},
{
name: "with secure-storage",
input: "--secure-storage",
output: TokenOptions{SecureStorage: true},
},
}
for _, tt := range tests {
@ -71,11 +77,12 @@ func TestNewCmdToken(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, tt.output.Hostname, cmdOpts.Hostname)
assert.Equal(t, tt.output.SecureStorage, cmdOpts.SecureStorage)
})
}
}
func Test_tokenRun(t *testing.T) {
func TestTokenRun(t *testing.T) {
tests := []struct {
name string
opts TokenOptions
@ -118,20 +125,96 @@ func Test_tokenRun(t *testing.T) {
wantErr: true,
wantErrMsg: "no oauth token",
},
{
name: "uses default host when one is not provided",
opts: TokenOptions{
Config: func() (config.Config, error) {
cfg := config.NewBlankConfig()
cfg.AuthenticationFunc = func() *config.AuthConfig {
authCfg := &config.AuthConfig{}
authCfg.SetDefaultHost("github.mycompany.com", "GH_HOST")
authCfg.SetToken("gho_1234567", "default")
return authCfg
}
return cfg, nil
},
},
wantStdout: "gho_1234567\n",
},
}
for _, tt := range tests {
ios, _, stdout, _ := iostreams.Test()
tt.opts.IO = ios
t.Run(tt.name, func(t *testing.T) {
ios, _, stdout, _ := iostreams.Test()
tt.opts.IO = ios
err := tokenRun(&tt.opts)
if tt.wantErr {
assert.Error(t, err)
assert.EqualError(t, err, tt.wantErrMsg)
return
}
assert.NoError(t, err)
assert.Equal(t, tt.wantStdout, stdout.String())
})
}
}
func TestTokenRunSecureStorage(t *testing.T) {
tests := []struct {
name string
opts TokenOptions
wantStdout string
wantErr bool
wantErrMsg string
}{
{
name: "token",
opts: TokenOptions{
Config: func() (config.Config, error) {
cfg := config.NewBlankConfig()
_ = keyring.Set("gh:github.com", "", "gho_ABCDEFG")
return cfg, nil
},
},
wantStdout: "gho_ABCDEFG\n",
},
{
name: "token by hostname",
opts: TokenOptions{
Config: func() (config.Config, error) {
cfg := config.NewBlankConfig()
_ = keyring.Set("gh:mycompany.com", "", "gho_1234567")
return cfg, nil
},
Hostname: "mycompany.com",
},
wantStdout: "gho_1234567\n",
},
{
name: "no token",
opts: TokenOptions{
Config: func() (config.Config, error) {
cfg := config.NewBlankConfig()
return cfg, nil
},
},
wantErr: true,
wantErrMsg: "no oauth token",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
keyring.MockInit()
ios, _, stdout, _ := iostreams.Test()
tt.opts.IO = ios
tt.opts.SecureStorage = true
err := tokenRun(&tt.opts)
if tt.wantErr {
assert.Error(t, err)
assert.EqualError(t, err, tt.wantErrMsg)
return
}
assert.NoError(t, err)
assert.Equal(t, tt.wantStdout, stdout.String())
})

2
pkg/cmd/browse/browse.go
View file

@ -174,7 +174,7 @@ func parseSection(baseRepo ghrepo.Interface, opts *BrowseOptions) (string, error
}
}
if isNumber(opts.SelectorArg) {
if !opts.CommitFlag && isNumber(opts.SelectorArg) {
return fmt.Sprintf("issues/%s", strings.TrimPrefix(opts.SelectorArg, "#")), nil
}

20
pkg/cmd/browse/browse_test.go
View file

@ -123,6 +123,15 @@ func TestNewCmdBrowse(t *testing.T) {
},
wantsErr: false,
},
{
name: "commit hash flag",
cli: "-c 123",
wants: BrowseOptions{
CommitFlag: true,
SelectorArg: "123",
},
wantsErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@ -395,6 +404,17 @@ func Test_runBrowse(t *testing.T) {
wantsErr: false,
expectedURL: "https://github.com/vilmibm/gh-user-status/tree/6f1a2405cace1633d89a79c74c65f22fe78f9659/main.go",
},
{
name: "open number only commit hash",
opts: BrowseOptions{
CommitFlag: true,
SelectorArg: "1234567890",
GitClient: &testGitClient{},
},
baseRepo: ghrepo.New("yanskun", "ILoveGitHub"),
wantsErr: false,
expectedURL: "https://github.com/yanskun/ILoveGitHub/commit/1234567890",
},
{
name: "relative path from browse_test.go",
opts: BrowseOptions{

13
pkg/cmd/codespace/code.go
View file

@ -10,7 +10,7 @@ import (
func newCodeCmd(app *App) *cobra.Command {
var (
codespace string
selector *CodespaceSelector
useInsiders bool
useWeb bool
)
@ -20,11 +20,12 @@ func newCodeCmd(app *App) *cobra.Command {
Short: "Open a codespace in Visual Studio Code",
Args: noArgsConstraint,
RunE: func(cmd *cobra.Command, args []string) error {
return app.VSCode(cmd.Context(), codespace, useInsiders, useWeb)
return app.VSCode(cmd.Context(), selector, useInsiders, useWeb)
},
}
codeCmd.Flags().StringVarP(&codespace, "codespace", "c", "", "Name of the codespace")
selector = AddCodespaceSelector(codeCmd, app.apiClient)
codeCmd.Flags().BoolVar(&useInsiders, "insiders", false, "Use the insiders version of Visual Studio Code")
codeCmd.Flags().BoolVarP(&useWeb, "web", "w", false, "Use the web version of Visual Studio Code")
@ -32,8 +33,8 @@ func newCodeCmd(app *App) *cobra.Command {
}
// VSCode opens a codespace in the local VS VSCode application.
func (a *App) VSCode(ctx context.Context, codespaceName string, useInsiders bool, useWeb bool) error {
codespace, err := getOrChooseCodespace(ctx, a.apiClient, codespaceName)
func (a *App) VSCode(ctx context.Context, selector *CodespaceSelector, useInsiders bool, useWeb bool) error {
codespace, err := selector.Select(ctx)
if err != nil {
return err
}
@ -65,5 +66,5 @@ func vscodeProtocolURL(codespaceName string, useInsiders bool) string {
if useInsiders {
application = "vscode-insiders"
}
return fmt.Sprintf("%s://github.codespaces/connect?name=%s", application, url.QueryEscape(codespaceName))
return fmt.Sprintf("%s://github.codespaces/connect?name=%s&windowId=_blank", application, url.QueryEscape(codespaceName))
}

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

@ -28,7 +28,7 @@ func TestApp_VSCode(t *testing.T) {
useInsiders: false,
},
wantErr: false,
wantURL: "vscode://github.codespaces/connect?name=monalisa-cli-cli-abcdef",
wantURL: "vscode://github.codespaces/connect?name=monalisa-cli-cli-abcdef&windowId=_blank",
},
{
name: "open VS Code Insiders",
@ -37,7 +37,7 @@ func TestApp_VSCode(t *testing.T) {
useInsiders: true,
},
wantErr: false,
wantURL: "vscode-insiders://github.codespaces/connect?name=monalisa-cli-cli-abcdef",
wantURL: "vscode-insiders://github.codespaces/connect?name=monalisa-cli-cli-abcdef&windowId=_blank",
},
{
name: "open VS Code web",
@ -69,7 +69,9 @@ func TestApp_VSCode(t *testing.T) {
apiClient: testCodeApiMock(),
io: ios,
}
if err := a.VSCode(context.Background(), tt.args.codespaceName, tt.args.useInsiders, tt.args.useWeb); (err != nil) != tt.wantErr {
selector := &CodespaceSelector{api: a.apiClient, codespaceName: tt.args.codespaceName}
if err := a.VSCode(context.Background(), selector, tt.args.useInsiders, tt.args.useWeb); (err != nil) != tt.wantErr {
t.Errorf("App.VSCode() error = %v, wantErr %v", err, tt.wantErr)
}
b.Verify(t, tt.wantURL)
@ -85,8 +87,9 @@ func TestApp_VSCode(t *testing.T) {
func TestPendingOperationDisallowsCode(t *testing.T) {
app := testingCodeApp()
selector := &CodespaceSelector{api: app.apiClient, codespaceName: "disabledCodespace"}
if err := app.VSCode(context.Background(), "disabledCodespace", false, false); err != nil {
if err := app.VSCode(context.Background(), selector, false, false); err != nil {
if err.Error() != "codespace is disabled while it has a pending operation: Some pending operation" {
t.Errorf("expected pending operation error, but got: %v", err)
}

123
pkg/cmd/codespace/codespace_selector.go Normal file
View file

@ -0,0 +1,123 @@
package codespace
import (
"context"
"errors"
"fmt"
"strings"
"github.com/cli/cli/v2/internal/codespaces/api"
"github.com/spf13/cobra"
)
type CodespaceSelector struct {
api apiClient
repoName string
codespaceName string
}
var errNoFilteredCodespaces = errors.New("you have no codespaces meeting the filter criteria")
// AddCodespaceSelector adds persistent flags for selecting a codespace to the given command and returns a CodespaceSelector which applies them
func AddCodespaceSelector(cmd *cobra.Command, api apiClient) *CodespaceSelector {
cs := &CodespaceSelector{api: api}
cmd.PersistentFlags().StringVarP(&cs.codespaceName, "codespace", "c", "", "Name of the codespace")
cmd.PersistentFlags().StringVarP(&cs.repoName, "repo", "R", "", "Filter codespace selection by repository name (user/repo)")
cmd.MarkFlagsMutuallyExclusive("codespace", "repo")
return cs
}
func (cs *CodespaceSelector) Select(ctx context.Context) (codespace *api.Codespace, err error) {
if cs.codespaceName != "" {
codespace, err = cs.api.GetCodespace(ctx, cs.codespaceName, true)
if err != nil {
return nil, fmt.Errorf("getting full codespace details: %w", err)
}
} else {
codespaces, err := cs.fetchCodespaces(ctx)
if err != nil {
return nil, err
}
codespace, err = cs.chooseCodespace(ctx, codespaces)
if err != nil {
return nil, err
}
}
if codespace.PendingOperation {
return nil, fmt.Errorf(
"codespace is disabled while it has a pending operation: %s",
codespace.PendingOperationDisabledReason,
)
}
return codespace, nil
}
func (cs *CodespaceSelector) SelectName(ctx context.Context) (string, error) {
if cs.codespaceName != "" {
return cs.codespaceName, nil
}
codespaces, err := cs.fetchCodespaces(ctx)
if err != nil {
return "", err
}
codespace, err := cs.chooseCodespace(ctx, codespaces)
if err != nil {
return "", err
}
return codespace.Name, nil
}
func (cs *CodespaceSelector) fetchCodespaces(ctx context.Context) (codespaces []*api.Codespace, err error) {
codespaces, err = cs.api.ListCodespaces(ctx, api.ListCodespacesOptions{})
if err != nil {
return nil, fmt.Errorf("error getting codespaces: %w", err)
}
if len(codespaces) == 0 {
return nil, errNoCodespaces
}
// Note that repo filtering done here can also be done in api.ListCodespaces.
// We do it here instead so that we can differentiate no codespaces in general vs. none after filtering.
if cs.repoName != "" {
var filteredCodespaces []*api.Codespace
for _, c := range codespaces {
if !strings.EqualFold(c.Repository.FullName, cs.repoName) {
continue
}
filteredCodespaces = append(filteredCodespaces, c)
}
codespaces = filteredCodespaces
}
if len(codespaces) == 0 {
return nil, errNoFilteredCodespaces
}
return codespaces, err
}
func (cs *CodespaceSelector) chooseCodespace(ctx context.Context, codespaces []*api.Codespace) (codespace *api.Codespace, err error) {
skipPromptForSingleOption := cs.repoName != ""
codespace, err = chooseCodespaceFromList(ctx, codespaces, false, skipPromptForSingleOption)
if err != nil {
if err == errNoCodespaces {
return nil, err
}
return nil, fmt.Errorf("choosing codespace: %w", err)
}
return codespace, nil
}

137
pkg/cmd/codespace/codespace_selector_test.go Normal file
View file

@ -0,0 +1,137 @@
package codespace
import (
"context"
"fmt"
"testing"
"github.com/cli/cli/v2/internal/codespaces/api"
)
func TestSelectWithCodespaceName(t *testing.T) {
wantName := "mock-codespace"
api := &apiClientMock{
GetCodespaceFunc: func(ctx context.Context, name string, includeConnection bool) (*api.Codespace, error) {
if name != wantName {
t.Errorf("incorrect name: want %s, got %s", wantName, name)
}
return &api.Codespace{}, nil
},
}
cs := &CodespaceSelector{api: api, codespaceName: wantName}
_, err := cs.Select(context.Background())
if err != nil {
t.Errorf("unexpected error: %v", err)
}
}
func TestSelectNameWithCodespaceName(t *testing.T) {
wantName := "mock-codespace"
cs := &CodespaceSelector{codespaceName: wantName}
name, err := cs.SelectName(context.Background())
if name != wantName {
t.Errorf("incorrect name: want %s, got %s", wantName, name)
}
if err != nil {
t.Errorf("unexpected error: %v", err)
}
}
func TestFetchCodespaces(t *testing.T) {
var (
repoA1 = &api.Codespace{Name: "1", Repository: api.Repository{FullName: "mock/A"}}
repoA2 = &api.Codespace{Name: "2", Repository: api.Repository{FullName: "mock/A"}}
repoB1 = &api.Codespace{Name: "1", Repository: api.Repository{FullName: "mock/B"}}
)
tests := []struct {
tName string
apiCodespaces []*api.Codespace
repoName string
wantCodespaces []*api.Codespace
wantErr error
}{
// Empty case
{
"empty", nil, "", nil, errNoCodespaces,
},
// Tests with no filtering
{
"no filtering, single codespace",
[]*api.Codespace{repoA1},
"",
[]*api.Codespace{repoA1},
nil,
},
{
"no filtering, multiple codespaces",
[]*api.Codespace{repoA1, repoA2, repoB1},
"",
[]*api.Codespace{repoA1, repoA2, repoB1},
nil,
},
// Test repo filtering
{
"repo filtering, single codespace",
[]*api.Codespace{repoA1},
"mock/A",
[]*api.Codespace{repoA1},
nil,
},
{
"repo filtering, multiple codespaces",
[]*api.Codespace{repoA1, repoA2, repoB1},
"mock/A",
[]*api.Codespace{repoA1, repoA2},
nil,
},
{
"repo filtering, multiple codespaces 2",
[]*api.Codespace{repoA1, repoA2, repoB1},
"mock/B",
[]*api.Codespace{repoB1},
nil,
},
{
"repo filtering, no matches",
[]*api.Codespace{repoA1, repoA2, repoB1},
"mock/C",
nil,
errNoFilteredCodespaces,
},
}
for _, tt := range tests {
t.Run(tt.tName, func(t *testing.T) {
api := &apiClientMock{
ListCodespacesFunc: func(ctx context.Context, opts api.ListCodespacesOptions) ([]*api.Codespace, error) {
return tt.apiCodespaces, nil
},
}
cs := &CodespaceSelector{api: api, repoName: tt.repoName}
codespaces, err := cs.fetchCodespaces(context.Background())
if err != tt.wantErr {
t.Errorf("expected error to be %v, got %v", tt.wantErr, err)
}
if fmt.Sprintf("%v", tt.wantCodespaces) != fmt.Sprintf("%v", codespaces) {
t.Errorf("expected codespaces to be %v, got %v", tt.wantCodespaces, codespaces)
}
})
}
}

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

@ -106,21 +106,17 @@ type apiClient interface {
var errNoCodespaces = errors.New("you have no codespaces")
func chooseCodespace(ctx context.Context, apiClient apiClient) (*api.Codespace, error) {
codespaces, err := apiClient.ListCodespaces(ctx, api.ListCodespacesOptions{})
if err != nil {
return nil, fmt.Errorf("error getting codespaces: %w", err)
}
return chooseCodespaceFromList(ctx, codespaces, false)
}
// chooseCodespaceFromList returns the codespace that the user has interactively selected from the list, or
// an error if there are no codespaces.
func chooseCodespaceFromList(ctx context.Context, codespaces []*api.Codespace, includeOwner bool) (*api.Codespace, error) {
func chooseCodespaceFromList(ctx context.Context, codespaces []*api.Codespace, includeOwner bool, skipPromptForSingleOption bool) (*api.Codespace, error) {
if len(codespaces) == 0 {
return nil, errNoCodespaces
}
if skipPromptForSingleOption && len(codespaces) == 1 {
return codespaces[0], nil
}
sortedCodespaces := codespaces
sort.Slice(sortedCodespaces, func(i, j int) bool {
return sortedCodespaces[i].CreatedAt > sortedCodespaces[j].CreatedAt
@ -158,35 +154,6 @@ func formatCodespacesForSelect(codespaces []*api.Codespace, includeOwner bool) [
return names
}
// getOrChooseCodespace prompts the user to choose a codespace if the codespaceName is empty.
// It then fetches the codespace record with full connection details.
// TODO(josebalius): accept a progress indicator or *App and show progress when fetching.
func getOrChooseCodespace(ctx context.Context, apiClient apiClient, codespaceName string) (codespace *api.Codespace, err error) {
if codespaceName == "" {
codespace, err = chooseCodespace(ctx, apiClient)
if err != nil {
if err == errNoCodespaces {
return nil, err
}
return nil, fmt.Errorf("choosing codespace: %w", err)
}
} else {
codespace, err = apiClient.GetCodespace(ctx, codespaceName, true)
if err != nil {
return nil, fmt.Errorf("getting full codespace details: %w", err)
}
}
if codespace.PendingOperation {
return nil, fmt.Errorf(
"codespace is disabled while it has a pending operation: %s",
codespace.PendingOperationDisabledReason,
)
}
return codespace, nil
}
func safeClose(closer io.Closer, err *error) {
if closeErr := closer.Close(); *err == nil {
*err = closeErr
@ -293,5 +260,9 @@ func addDeprecatedRepoShorthand(cmd *cobra.Command, target *string) error {
return fmt.Errorf("error marking `-r` shorthand as deprecated: %w", err)
}
if cmd.Flag("codespace") != nil {
cmd.MarkFlagsMutuallyExclusive("codespace", "repo-deprecated")
}
return nil
}

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

@ -41,6 +41,8 @@ func newDeleteCmd(app *App) *cobra.Command {
prompter: &surveyPrompter{},
}
var selector *CodespaceSelector
deleteCmd := &cobra.Command{
Use: "delete",
Short: "Delete codespaces",
@ -54,6 +56,11 @@ func newDeleteCmd(app *App) *cobra.Command {
`),
Args: noArgsConstraint,
RunE: func(cmd *cobra.Command, args []string) error {
// TODO: ideally we would use the selector directly, but the logic here is too intertwined with other flags to do so elegantly
// After the admin subcommand is added (see https://github.com/cli/cli/pull/6944#issuecomment-1419553639) we can revisit this.
opts.codespaceName = selector.codespaceName
opts.repoFilter = selector.repoName
if opts.deleteAll && opts.repoFilter != "" {
return cmdutil.FlagErrorf("both `--all` and `--repo` is not supported")
}
@ -64,13 +71,12 @@ func newDeleteCmd(app *App) *cobra.Command {
},
}
deleteCmd.Flags().StringVarP(&opts.codespaceName, "codespace", "c", "", "Name of the codespace")
deleteCmd.Flags().BoolVar(&opts.deleteAll, "all", false, "Delete all codespaces")
deleteCmd.Flags().StringVarP(&opts.repoFilter, "repo", "R", "", "Delete codespaces for a `repository`")
if err := addDeprecatedRepoShorthand(deleteCmd, &opts.repoFilter); err != nil {
selector = AddCodespaceSelector(deleteCmd, app.apiClient)
if err := addDeprecatedRepoShorthand(deleteCmd, &selector.repoName); err != nil {
fmt.Fprintf(app.io.ErrOut, "%v\n", err)
}
deleteCmd.Flags().BoolVar(&opts.deleteAll, "all", false, "Delete all codespaces")
deleteCmd.Flags().BoolVarP(&opts.skipConfirm, "force", "f", false, "Skip confirmation for codespaces that contain unsaved changes")
deleteCmd.Flags().Uint16Var(&opts.keepDays, "days", 0, "Delete codespaces older than `N` days")
deleteCmd.Flags().StringVarP(&opts.orgName, "org", "o", "", "The `login` handle of the organization (admin-only)")
@ -102,7 +108,7 @@ func (a *App) Delete(ctx context.Context, opts deleteOptions) (err error) {
if !opts.deleteAll && opts.repoFilter == "" {
includeUsername := opts.orgName != ""
c, err := chooseCodespaceFromList(ctx, codespaces, includeUsername)
c, err := chooseCodespaceFromList(ctx, codespaces, includeUsername, false)
if err != nil {
return fmt.Errorf("error choosing codespace: %w", err)
}

25
pkg/cmd/codespace/edit.go
View file

@ -2,6 +2,7 @@ package codespace
import (
"context"
"errors"
"fmt"
"github.com/cli/cli/v2/internal/codespaces/api"
@ -10,9 +11,9 @@ import (
)
type editOptions struct {
codespaceName string
displayName string
machine string
selector *CodespaceSelector
displayName string
machine string
}
func newEditCmd(app *App) *cobra.Command {
@ -31,7 +32,7 @@ func newEditCmd(app *App) *cobra.Command {
},
}
editCmd.Flags().StringVarP(&opts.codespaceName, "codespace", "c", "", "Name of the codespace")
opts.selector = AddCodespaceSelector(editCmd, app.apiClient)
editCmd.Flags().StringVarP(&opts.displayName, "display-name", "d", "", "Set the display name")
editCmd.Flags().StringVar(&opts.displayName, "displayName", "", "display name")
if err := editCmd.Flags().MarkDeprecated("displayName", "use `--display-name` instead"); err != nil {
@ -44,17 +45,13 @@ func newEditCmd(app *App) *cobra.Command {
// Edits a codespace
func (a *App) Edit(ctx context.Context, opts editOptions) error {
codespaceName := opts.codespaceName
if codespaceName == "" {
selectedCodespace, err := chooseCodespace(ctx, a.apiClient)
if err != nil {
if err == errNoCodespaces {
return err
}
return fmt.Errorf("error choosing codespace: %w", err)
codespaceName, err := opts.selector.SelectName(ctx)
if err != nil {
// TODO: is there a cleaner way to do this?
if errors.Is(err, errNoCodespaces) || errors.Is(err, errNoFilteredCodespaces) {
return err
}
codespaceName = selectedCodespace.Name
return fmt.Errorf("error choosing codespace: %w", err)
}
err := a.RunWithProgress("Editing codespace", func() (err error) {

17
pkg/cmd/codespace/edit_test.go
View file

@ -23,9 +23,9 @@ func TestEdit(t *testing.T) {
{
name: "edit codespace display name",
opts: editOptions{
codespaceName: "hubot",
displayName: "hubot-changed",
machine: "",
selector: &CodespaceSelector{codespaceName: "hubot"},
displayName: "hubot-changed",
machine: "",
},
wantEdits: &api.EditCodespaceParams{
DisplayName: "hubot-changed",
@ -54,9 +54,9 @@ func TestEdit(t *testing.T) {
{
name: "edit codespace machine",
opts: editOptions{
codespaceName: "hubot",
displayName: "",
machine: "machine",
selector: &CodespaceSelector{codespaceName: "hubot"},
displayName: "",
machine: "machine",
},
wantEdits: &api.EditCodespaceParams{
Machine: "machine",
@ -92,6 +92,11 @@ func TestEdit(t *testing.T) {
var err error
if tt.cliArgs == nil {
if tt.opts.selector == nil {
t.Fatalf("selector must be set in opts if cliArgs are not provided")
}
tt.opts.selector.api = apiMock
err = a.Edit(context.Background(), tt.opts)
} else {
cmd := newEditCmd(a)

10
pkg/cmd/codespace/jupyter.go
View file

@ -13,28 +13,28 @@ import (
)
func newJupyterCmd(app *App) *cobra.Command {
var codespace string
var selector *CodespaceSelector
jupyterCmd := &cobra.Command{
Use: "jupyter",
Short: "Open a codespace in JupyterLab",
Args: noArgsConstraint,
RunE: func(cmd *cobra.Command, args []string) error {
return app.Jupyter(cmd.Context(), codespace)
return app.Jupyter(cmd.Context(), selector)
},
}
jupyterCmd.Flags().StringVarP(&codespace, "codespace", "c", "", "Name of the codespace")
selector = AddCodespaceSelector(jupyterCmd, app.apiClient)
return jupyterCmd
}
func (a *App) Jupyter(ctx context.Context, codespaceName string) (err error) {
func (a *App) Jupyter(ctx context.Context, selector *CodespaceSelector) (err error) {
// Ensure all child tasks (e.g. port forwarding) terminate before return.
ctx, cancel := context.WithCancel(ctx)
defer cancel()
codespace, err := getOrChooseCodespace(ctx, a.apiClient, codespaceName)
codespace, err := selector.Select(ctx)
if err != nil {
return err
}

13
pkg/cmd/codespace/logs.go
View file

@ -12,8 +12,8 @@ import (
func newLogsCmd(app *App) *cobra.Command {
var (
codespace string
follow bool
selector *CodespaceSelector
follow bool
)
logsCmd := &cobra.Command{
@ -21,22 +21,23 @@ func newLogsCmd(app *App) *cobra.Command {
Short: "Access codespace logs",
Args: noArgsConstraint,
RunE: func(cmd *cobra.Command, args []string) error {
return app.Logs(cmd.Context(), codespace, follow)
return app.Logs(cmd.Context(), selector, follow)
},
}
logsCmd.Flags().StringVarP(&codespace, "codespace", "c", "", "Name of the codespace")
selector = AddCodespaceSelector(logsCmd, app.apiClient)
logsCmd.Flags().BoolVarP(&follow, "follow", "f", false, "Tail and follow the logs")
return logsCmd
}
func (a *App) Logs(ctx context.Context, codespaceName string, follow bool) (err error) {
func (a *App) Logs(ctx context.Context, selector *CodespaceSelector, follow bool) (err error) {
// Ensure all child tasks (port forwarding, remote exec) terminate before return.
ctx, cancel := context.WithCancel(ctx)
defer cancel()
codespace, err := getOrChooseCodespace(ctx, a.apiClient, codespaceName)
codespace, err := selector.Select(ctx)
if err != nil {
return err
}

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

@ -10,8 +10,9 @@ import (
func TestPendingOperationDisallowsLogs(t *testing.T) {
app := testingLogsApp()
selector := &CodespaceSelector{api: app.apiClient, codespaceName: "disabledCodespace"}
if err := app.Logs(context.Background(), "disabledCodespace", false); err != nil {
if err := app.Logs(context.Background(), selector, false); err != nil {
if err.Error() != "codespace is disabled while it has a pending operation: Some pending operation" {
t.Errorf("expected pending operation error, but got: %v", err)
}

50
pkg/cmd/codespace/ports.go
View file

@ -29,30 +29,33 @@ const (
// newPortsCmd returns a Cobra "ports" command that displays a table of available ports,
// according to the specified flags.
func newPortsCmd(app *App) *cobra.Command {
var codespace string
var exporter cmdutil.Exporter
var (
selector *CodespaceSelector
exporter cmdutil.Exporter
)
portsCmd := &cobra.Command{
Use: "ports",
Short: "List ports in a codespace",
Args: noArgsConstraint,
RunE: func(cmd *cobra.Command, args []string) error {
return app.ListPorts(cmd.Context(), codespace, exporter)
return app.ListPorts(cmd.Context(), selector, exporter)
},
}
portsCmd.PersistentFlags().StringVarP(&codespace, "codespace", "c", "", "Name of the codespace")
selector = AddCodespaceSelector(portsCmd, app.apiClient)
cmdutil.AddJSONFlags(portsCmd, &exporter, portFields)
portsCmd.AddCommand(newPortsForwardCmd(app))
portsCmd.AddCommand(newPortsVisibilityCmd(app))
portsCmd.AddCommand(newPortsForwardCmd(app, selector))
portsCmd.AddCommand(newPortsVisibilityCmd(app, selector))
return portsCmd
}
// ListPorts lists known ports in a codespace.
func (a *App) ListPorts(ctx context.Context, codespaceName string, exporter cmdutil.Exporter) (err error) {
codespace, err := getOrChooseCodespace(ctx, a.apiClient, codespaceName)
func (a *App) ListPorts(ctx context.Context, selector *CodespaceSelector, exporter cmdutil.Exporter) (err error) {
codespace, err := selector.Select(ctx)
if err != nil {
return err
}
@ -220,21 +223,14 @@ func getDevContainer(ctx context.Context, apiClient apiClient, codespace *api.Co
return ch
}
func newPortsVisibilityCmd(app *App) *cobra.Command {
func newPortsVisibilityCmd(app *App, selector *CodespaceSelector) *cobra.Command {
return &cobra.Command{
Use: "visibility <port>:{public|private|org}...",
Short: "Change the visibility of the forwarded port",
Example: "gh codespace ports visibility 80:org 3000:private 8000:public",
Args: cobra.MinimumNArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
codespace, err := cmd.Flags().GetString("codespace")
if err != nil {
// should only happen if flag is not defined
// or if the flag is not of string type
// since it's a persistent flag that we control it should never happen
return fmt.Errorf("get codespace flag: %w", err)
}
return app.UpdatePortVisibility(cmd.Context(), codespace, args)
return app.UpdatePortVisibility(cmd.Context(), selector, args)
},
}
}
@ -263,13 +259,13 @@ func (e *ErrUpdatingPortVisibility) Unwrap() error {
var errUpdatePortVisibilityForbidden = errors.New("organization admin has forbidden this privacy setting")
func (a *App) UpdatePortVisibility(ctx context.Context, codespaceName string, args []string) (err error) {
func (a *App) UpdatePortVisibility(ctx context.Context, selector *CodespaceSelector, args []string) (err error) {
ports, err := a.parsePortVisibilities(args)
if err != nil {
return fmt.Errorf("error parsing port arguments: %w", err)
}
codespace, err := getOrChooseCodespace(ctx, a.apiClient, codespaceName)
codespace, err := selector.Select(ctx)
if err != nil {
return err
}
@ -349,32 +345,24 @@ func (a *App) parsePortVisibilities(args []string) ([]portVisibility, error) {
// NewPortsForwardCmd returns a Cobra "ports forward" subcommand, which forwards a set of
// port pairs from the codespace to localhost.
func newPortsForwardCmd(app *App) *cobra.Command {
func newPortsForwardCmd(app *App, selector *CodespaceSelector) *cobra.Command {
return &cobra.Command{
Use: "forward <remote-port>:<local-port>...",
Short: "Forward ports",
Args: cobra.MinimumNArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
codespace, err := cmd.Flags().GetString("codespace")
if err != nil {
// should only happen if flag is not defined
// or if the flag is not of string type
// since it's a persistent flag that we control it should never happen
return fmt.Errorf("get codespace flag: %w", err)
}
return app.ForwardPorts(cmd.Context(), codespace, args)
return app.ForwardPorts(cmd.Context(), selector, args)
},
}
}
func (a *App) ForwardPorts(ctx context.Context, codespaceName string, ports []string) (err error) {
func (a *App) ForwardPorts(ctx context.Context, selector *CodespaceSelector, ports []string) (err error) {
portPairs, err := getPortPairs(ports)
if err != nil {
return fmt.Errorf("get port pairs: %w", err)
}
codespace, err := getOrChooseCodespace(ctx, a.apiClient, codespaceName)
codespace, err := selector.Select(ctx)
if err != nil {
return err
}

13
pkg/cmd/codespace/ports_test.go
View file

@ -207,13 +207,16 @@ func runUpdateVisibilityTest(t *testing.T, portVisibilities []portVisibility, ev
portArgs = append(portArgs, fmt.Sprintf("%d:%s", pv.number, pv.visibility))
}
return a.UpdatePortVisibility(ctx, "codespace-name", portArgs)
selector := &CodespaceSelector{api: a.apiClient, codespaceName: "codespace-name"}
return a.UpdatePortVisibility(ctx, selector, portArgs)
}
func TestPendingOperationDisallowsListPorts(t *testing.T) {
app := testingPortsApp()
selector := &CodespaceSelector{api: app.apiClient, codespaceName: "disabledCodespace"}
if err := app.ListPorts(context.Background(), "disabledCodespace", nil); err != nil {
if err := app.ListPorts(context.Background(), selector, nil); err != nil {
if err.Error() != "codespace is disabled while it has a pending operation: Some pending operation" {
t.Errorf("expected pending operation error, but got: %v", err)
}
@ -224,8 +227,9 @@ func TestPendingOperationDisallowsListPorts(t *testing.T) {
func TestPendingOperationDisallowsUpdatePortVisability(t *testing.T) {
app := testingPortsApp()
selector := &CodespaceSelector{api: app.apiClient, codespaceName: "disabledCodespace"}
if err := app.UpdatePortVisibility(context.Background(), "disabledCodespace", nil); err != nil {
if err := app.UpdatePortVisibility(context.Background(), selector, nil); err != nil {
if err.Error() != "codespace is disabled while it has a pending operation: Some pending operation" {
t.Errorf("expected pending operation error, but got: %v", err)
}
@ -236,8 +240,9 @@ func TestPendingOperationDisallowsUpdatePortVisability(t *testing.T) {
func TestPendingOperationDisallowsForwardPorts(t *testing.T) {
app := testingPortsApp()
selector := &CodespaceSelector{api: app.apiClient, codespaceName: "disabledCodespace"}
if err := app.ForwardPorts(context.Background(), "disabledCodespace", nil); err != nil {
if err := app.ForwardPorts(context.Background(), selector, nil); err != nil {
if err.Error() != "codespace is disabled while it has a pending operation: Some pending operation" {
t.Errorf("expected pending operation error, but got: %v", err)
}

15
pkg/cmd/codespace/rebuild.go
View file

@ -10,8 +10,10 @@ import (
)
func newRebuildCmd(app *App) *cobra.Command {
var codespace string
var fullRebuild bool
var (
selector *CodespaceSelector
fullRebuild bool
)
rebuildCmd := &cobra.Command{
Use: "rebuild",
@ -21,21 +23,22 @@ preserved. Your codespace will be rebuilt using your working directory's
dev container. A full rebuild also removes cached Docker images.`,
Args: cobra.NoArgs,
RunE: func(cmd *cobra.Command, args []string) error {
return app.Rebuild(cmd.Context(), codespace, fullRebuild)
return app.Rebuild(cmd.Context(), selector, fullRebuild)
},
}
rebuildCmd.Flags().StringVarP(&codespace, "codespace", "c", "", "name of the codespace")
selector = AddCodespaceSelector(rebuildCmd, app.apiClient)
rebuildCmd.Flags().BoolVar(&fullRebuild, "full", false, "perform a full rebuild")
return rebuildCmd
}
func (a *App) Rebuild(ctx context.Context, codespaceName string, full bool) (err error) {
func (a *App) Rebuild(ctx context.Context, selector *CodespaceSelector, full bool) (err error) {
ctx, cancel := context.WithCancel(ctx)
defer cancel()
codespace, err := getOrChooseCodespace(ctx, a.apiClient, codespaceName)
codespace, err := selector.Select(ctx)
if err != nil {
return err
}

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

@ -14,8 +14,9 @@ func TestAlreadyRebuildingCodespace(t *testing.T) {
State: api.CodespaceStateRebuilding,
}
app := testingRebuildApp(*rebuildingCodespace)
selector := &CodespaceSelector{api: app.apiClient, codespaceName: "rebuildingCodespace"}
err := app.Rebuild(context.Background(), "rebuildingCodespace", false)
err := app.Rebuild(context.Background(), selector, false)
if err != nil {
t.Errorf("rebuilding a codespace that was already rebuilding: %v", err)
}

37
pkg/cmd/codespace/select.go
View file

@ -10,10 +10,13 @@ import (
type selectOptions struct {
filePath string
selector *CodespaceSelector
}
func newSelectCmd(app *App) *cobra.Command {
opts := selectOptions{}
var (
opts selectOptions
)
selectCmd := &cobra.Command{
Use: "select",
@ -21,19 +24,39 @@ func newSelectCmd(app *App) *cobra.Command {
Hidden: true,
Args: noArgsConstraint,
RunE: func(cmd *cobra.Command, args []string) error {
return app.Select(cmd.Context(), "", opts)
return app.Select(cmd.Context(), opts)
},
}
opts.selector = AddCodespaceSelector(selectCmd, app.apiClient)
selectCmd.Flags().StringVarP(&opts.filePath, "file", "f", "", "Output file path")
return selectCmd
}
// Hidden codespace select command allows to reuse existing codespace selection
// dialog by external GH CLI extensions. By default, print selected codespace name
// to stdout. Pass file argument to save result into a file instead.
func (a *App) Select(ctx context.Context, name string, opts selectOptions) (err error) {
codespace, err := getOrChooseCodespace(ctx, a.apiClient, name)
// Hidden codespace `select` command allows to reuse existing codespace selection
// dialog by external GH CLI extensions. By default output selected codespace name
// into `stdout`. Pass `--file`(`-f`) flag along with a file path to output selected
// codespace name into a file instead.
//
// ## Examples
//
// With `stdout` output:
//
// ```shell
//
// gh codespace select
//
// ```
//
// With `into-a-file` output:
//
// ```shell
//
// gh codespace select --file /tmp/selected_codespace.txt
//
// ```
func (a *App) Select(ctx context.Context, opts selectOptions) (err error) {
codespace, err := opts.selector.Select(ctx)
if err != nil {
return err
}

5
pkg/cmd/codespace/select_test.go
View file

@ -50,6 +50,7 @@ func TestApp_Select(t *testing.T) {
a := NewApp(ios, nil, testSelectApiMock(), nil, nil)
opts := selectOptions{}
if tt.outputToFile {
file, err := os.CreateTemp("", "codespace-selection-test")
if err != nil {
@ -61,7 +62,9 @@ func TestApp_Select(t *testing.T) {
opts = selectOptions{filePath: file.Name()}
}
if err := a.Select(context.Background(), tt.arg, opts); (err != nil) != tt.wantErr {
opts.selector = &CodespaceSelector{api: a.apiClient, codespaceName: tt.arg}
if err := a.Select(context.Background(), opts); (err != nil) != tt.wantErr {
t.Errorf("App.Select() error = %v, wantErr %v", err, tt.wantErr)
}

14
pkg/cmd/codespace/ssh.go
View file

@ -36,7 +36,7 @@ const automaticPrivateKeyName = "codespaces.auto"
var errKeyFileNotFound = errors.New("SSH key file does not exist")
type sshOptions struct {
codespace string
selector *CodespaceSelector
profile string
serverPort int
debug bool
@ -87,7 +87,7 @@ func newSSHCmd(app *App) *cobra.Command {
`),
PreRunE: func(c *cobra.Command, args []string) error {
if opts.stdio {
if opts.codespace == "" {
if opts.selector.codespaceName == "" {
return errors.New("`--stdio` requires explicit `--codespace`")
}
if opts.config {
@ -122,7 +122,7 @@ func newSSHCmd(app *App) *cobra.Command {
sshCmd.Flags().StringVarP(&opts.profile, "profile", "", "", "Name of the SSH profile to use")
sshCmd.Flags().IntVarP(&opts.serverPort, "server-port", "", 0, "SSH server port number (0 => pick unused)")
sshCmd.Flags().StringVarP(&opts.codespace, "codespace", "c", "", "Name of the codespace")
opts.selector = AddCodespaceSelector(sshCmd, app.apiClient)
sshCmd.Flags().BoolVarP(&opts.debug, "debug", "d", false, "Log debug data to a file")
sshCmd.Flags().StringVarP(&opts.debugFile, "debug-file", "", "", "Path of the file log to")
sshCmd.Flags().BoolVarP(&opts.config, "config", "", false, "Write OpenSSH configuration to stdout")
@ -160,7 +160,7 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e
args = append([]string{"-i", keyPair.PrivateKeyPath}, args...)
}
codespace, err := getOrChooseCodespace(ctx, a.apiClient, opts.codespace)
codespace, err := opts.selector.Select(ctx)
if err != nil {
return err
}
@ -480,7 +480,7 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts sshOptions) (err erro
})
} else {
var codespace *api.Codespace
codespace, err = getOrChooseCodespace(ctx, a.apiClient, opts.codespace)
codespace, err = opts.selector.Select(ctx)
csList = []*api.Codespace{codespace}
}
if err != nil {
@ -497,7 +497,7 @@ func (a *App) printOpenSSHConfig(ctx context.Context, opts sshOptions) (err erro
var wg sync.WaitGroup
var status error
for _, cs := range csList {
if cs.State != "Available" && opts.codespace == "" {
if cs.State != "Available" && opts.selector.codespaceName == "" {
fmt.Fprintf(os.Stderr, "skipping unavailable codespace %s: %s\n", cs.Name, cs.State)
status = cmdutil.SilentError
continue
@ -659,7 +659,7 @@ func newCpCmd(app *App) *cobra.Command {
// We don't expose all sshOptions.
cpCmd.Flags().BoolVarP(&opts.recursive, "recursive", "r", false, "Recursively copy directories")
cpCmd.Flags().BoolVarP(&opts.expand, "expand", "e", false, "Expand remote file names on remote shell")
cpCmd.Flags().StringVarP(&opts.codespace, "codespace", "c", "", "Name of the codespace")
opts.selector = AddCodespaceSelector(cpCmd, app.apiClient)
cpCmd.Flags().StringVarP(&opts.profile, "profile", "p", "", "Name of the SSH profile to use")
return cpCmd
}

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

@ -15,8 +15,9 @@ import (
func TestPendingOperationDisallowsSSH(t *testing.T) {
app := testingSSHApp()
selector := &CodespaceSelector{api: app.apiClient, codespaceName: "disabledCodespace"}
if err := app.SSH(context.Background(), []string{}, sshOptions{codespace: "disabledCodespace"}); err != nil {
if err := app.SSH(context.Background(), []string{}, sshOptions{selector: selector}); err != nil {
if err.Error() != "codespace is disabled while it has a pending operation: Some pending operation" {
t.Errorf("expected pending operation error, but got: %v", err)
}

26
pkg/cmd/codespace/stop.go
View file

@ -11,9 +11,9 @@ import (
)
type stopOptions struct {
codespaceName string
orgName string
userName string
selector *CodespaceSelector
orgName string
userName string
}
func newStopCmd(app *App) *cobra.Command {
@ -24,13 +24,13 @@ func newStopCmd(app *App) *cobra.Command {
Short: "Stop a running codespace",
Args: noArgsConstraint,
RunE: func(cmd *cobra.Command, args []string) error {
if opts.orgName != "" && opts.codespaceName != "" && opts.userName == "" {
if opts.orgName != "" && opts.selector.codespaceName != "" && opts.userName == "" {
return cmdutil.FlagErrorf("using `--org` with `--codespace` requires `--user`")
}
return app.StopCodespace(cmd.Context(), opts)
},
}
stopCmd.Flags().StringVarP(&opts.codespaceName, "codespace", "c", "", "Name of the codespace")
opts.selector = AddCodespaceSelector(stopCmd, app.apiClient)
stopCmd.Flags().StringVarP(&opts.orgName, "org", "o", "", "The `login` handle of the organization (admin-only)")
stopCmd.Flags().StringVarP(&opts.userName, "user", "u", "", "The `username` to stop codespace for (used with --org)")
@ -38,13 +38,20 @@ func newStopCmd(app *App) *cobra.Command {
}
func (a *App) StopCodespace(ctx context.Context, opts *stopOptions) error {
codespaceName := opts.codespaceName
ownerName := opts.userName
var (
codespaceName = opts.selector.codespaceName
repoName = opts.selector.repoName
ownerName = opts.userName
)
if codespaceName == "" {
var codespaces []*api.Codespace
err := a.RunWithProgress("Fetching codespaces", func() (err error) {
codespaces, err = a.apiClient.ListCodespaces(ctx, api.ListCodespacesOptions{OrgName: opts.orgName, UserName: ownerName})
codespaces, err = a.apiClient.ListCodespaces(ctx, api.ListCodespacesOptions{
RepoName: repoName,
OrgName: opts.orgName,
UserName: ownerName,
})
return
})
if err != nil {
@ -63,7 +70,8 @@ func (a *App) StopCodespace(ctx context.Context, opts *stopOptions) error {
}
includeOwner := opts.orgName != ""
codespace, err := chooseCodespaceFromList(ctx, runningCodespaces, includeOwner)
skipPromptForSingleOption := repoName != ""
codespace, err := chooseCodespaceFromList(ctx, runningCodespaces, includeOwner, skipPromptForSingleOption)
if err != nil {
return fmt.Errorf("failed to choose codespace: %w", err)
}

8
pkg/cmd/codespace/stop_test.go
View file

@ -22,7 +22,7 @@ func TestApp_StopCodespace(t *testing.T) {
{
name: "Stop a codespace I own",
opts: &stopOptions{
codespaceName: "test-codespace",
selector: &CodespaceSelector{codespaceName: "test-codespace"},
},
fields: fields{
apiClient: &apiClientMock{
@ -52,9 +52,9 @@ func TestApp_StopCodespace(t *testing.T) {
{
name: "Stop a codespace as an org admin",
opts: &stopOptions{
codespaceName: "test-codespace",
orgName: "test-org",
userName: "test-user",
selector: &CodespaceSelector{codespaceName: "test-codespace"},
orgName: "test-org",
userName: "test-user",
},
fields: fields{
apiClient: &apiClientMock{

2
pkg/cmd/config/list/list.go
View file

@ -51,7 +51,7 @@ func listRun(opts *ListOptions) error {
if opts.Hostname != "" {
host = opts.Hostname
} else {
host, _ = cfg.DefaultHost()
host, _ = cfg.Authentication().DefaultHost()
}
configOptions := config.ConfigOptions()

2
pkg/cmd/extension/browse/browse.go
View file

@ -343,7 +343,7 @@ func getExtensions(opts ExtBrowseOpts) ([]extEntry, error) {
return extEntries, fmt.Errorf("failed to search for extensions: %w", err)
}
host, _ := opts.Cfg.DefaultHost()
host, _ := opts.Cfg.Authentication().DefaultHost()
for _, repo := range result.Items {
if !strings.HasPrefix(repo.Name, "gh-") {

6
pkg/cmd/extension/browse/browse_test.go
View file

@ -76,7 +76,11 @@ func Test_getExtensionRepos(t *testing.T) {
}
cfg := config.NewBlankConfig()
cfg.DefaultHostFunc = func() (string, string) { return "github.com", "" }
cfg.AuthenticationFunc = func() *config.AuthConfig {
authCfg := &config.AuthConfig{}
authCfg.SetDefaultHost("github.com", "")
return authCfg
}
reg.Register(
httpmock.QueryMatcher("GET", "search/repositories", values),

4
pkg/cmd/extension/command.go
View file

@ -137,7 +137,7 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command {
query.Keywords = args
query.Qualifiers = qualifiers
host, _ := cfg.DefaultHost()
host, _ := cfg.Authentication().DefaultHost()
searcher := search.NewSearcher(client, host)
if webMode {
@ -445,7 +445,7 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command {
if err != nil {
return err
}
host, _ := cfg.DefaultHost()
host, _ := cfg.Authentication().DefaultHost()
client, err := f.HttpClient()
if err != nil {
return err

2
pkg/cmd/extension/manager.go
View file

@ -708,7 +708,7 @@ func (m *Manager) goBinScaffolding(name string) error {
return err
}
host, _ := m.config.DefaultHost()
host, _ := m.config.Authentication().DefaultHost()
currentUser, err := api.CurrentLoginName(api.NewClientFromHTTP(m.client), host)
if err != nil {

2
pkg/cmd/factory/default.go
View file

@ -96,7 +96,7 @@ func httpClientFunc(f *cmdutil.Factory, appVersion string) func() (*http.Client,
return nil, err
}
opts := api.HTTPClientOptions{
Config: cfg,
Config: cfg.Authentication(),
Log: io.ErrOut,
LogColorize: io.ColorEnabled(),
AppVersion: appVersion,

32
pkg/cmd/factory/default_test.go
View file

@ -71,21 +71,19 @@ func Test_BaseRepo(t *testing.T) {
},
getConfig: func() (config.Config, error) {
cfg := &config.ConfigMock{}
cfg.HostsFunc = func() []string {
cfg.AuthenticationFunc = func() *config.AuthConfig {
authCfg := &config.AuthConfig{}
hosts := []string{"nonsense.com"}
if tt.override != "" {
hosts = append([]string{tt.override}, hosts...)
}
return hosts
}
cfg.DefaultHostFunc = func() (string, string) {
authCfg.SetHosts(hosts)
authCfg.SetToken("", "")
authCfg.SetDefaultHost("nonsense.com", "hosts")
if tt.override != "" {
return tt.override, "GH_HOST"
authCfg.SetDefaultHost(tt.override, "GH_HOST")
}
return "nonsense.com", "hosts"
}
cfg.AuthTokenFunc = func(string) (string, string) {
return "", ""
return authCfg
}
return cfg, nil
},
@ -211,21 +209,19 @@ func Test_SmartBaseRepo(t *testing.T) {
},
getConfig: func() (config.Config, error) {
cfg := &config.ConfigMock{}
cfg.AuthTokenFunc = func(_ string) (string, string) {
return "", ""
}
cfg.HostsFunc = func() []string {
cfg.AuthenticationFunc = func() *config.AuthConfig {
authCfg := &config.AuthConfig{}
hosts := []string{"nonsense.com"}
if tt.override != "" {
hosts = append([]string{tt.override}, hosts...)
}
return hosts
}
cfg.DefaultHostFunc = func() (string, string) {
authCfg.SetHosts(hosts)
authCfg.SetToken("", "")
authCfg.SetDefaultHost("nonsense.com", "hosts")
if tt.override != "" {
return tt.override, "GH_HOST"
authCfg.SetDefaultHost(tt.override, "GH_HOST")
}
return "nonsense.com", "hosts"
return authCfg
}
return cfg, nil
},

6
pkg/cmd/factory/remote_resolver.go
View file

@ -53,11 +53,11 @@ func (rr *remoteResolver) Resolver() func() (context.Remotes, error) {
return nil, err
}
authedHosts := cfg.Hosts()
authedHosts := cfg.Authentication().Hosts()
if len(authedHosts) == 0 {
return nil, errors.New("could not find any host configurations")
}
defaultHost, src := cfg.DefaultHost()
defaultHost, src := cfg.Authentication().DefaultHost()
// Use set to dedupe list of hosts
hostsSet := set.NewStringSet()
@ -86,7 +86,7 @@ func (rr *remoteResolver) Resolver() func() (context.Remotes, error) {
dummyHostname := "example.com"
if isHostEnv(src) {
return nil, fmt.Errorf("none of the git remotes configured for this repository correspond to the %s environment variable. Try adding a matching remote or unsetting the variable.", src)
} else if v, _ := cfg.AuthToken(dummyHostname); v != "" {
} else if v, _ := cfg.Authentication().Token(dummyHostname); v != "" {
return nil, errors.New("set the GH_HOST environment variable to specify which GitHub host to use")
}
return nil, errors.New("none of the git remotes configured for this repository point to a known GitHub host. To tell gh about a new GitHub host, please use `gh auth login`")

128
pkg/cmd/factory/remote_resolver_test.go
View file

@ -32,11 +32,11 @@ func Test_remoteResolver(t *testing.T) {
},
config: func() config.Config {
cfg := &config.ConfigMock{}
cfg.HostsFunc = func() []string {
return []string{}
}
cfg.DefaultHostFunc = func() (string, string) {
return "github.com", "default"
cfg.AuthenticationFunc = func() *config.AuthConfig {
authCfg := &config.AuthConfig{}
authCfg.SetHosts([]string{})
authCfg.SetDefaultHost("github.com", "default")
return authCfg
}
return cfg
}(),
@ -49,11 +49,11 @@ func Test_remoteResolver(t *testing.T) {
},
config: func() config.Config {
cfg := &config.ConfigMock{}
cfg.HostsFunc = func() []string {
return []string{"example.com"}
}
cfg.DefaultHostFunc = func() (string, string) {
return "example.com", "hosts"
cfg.AuthenticationFunc = func() *config.AuthConfig {
authCfg := &config.AuthConfig{}
authCfg.SetHosts([]string{"example.com"})
authCfg.SetDefaultHost("example.com", "hosts")
return authCfg
}
return cfg
}(),
@ -68,14 +68,12 @@ func Test_remoteResolver(t *testing.T) {
},
config: func() config.Config {
cfg := &config.ConfigMock{}
cfg.HostsFunc = func() []string {
return []string{"example.com"}
}
cfg.DefaultHostFunc = func() (string, string) {
return "example.com", "hosts"
}
cfg.AuthTokenFunc = func(string) (string, string) {
return "", ""
cfg.AuthenticationFunc = func() *config.AuthConfig {
authCfg := &config.AuthConfig{}
authCfg.SetHosts([]string{"example.com"})
authCfg.SetToken("", "")
authCfg.SetDefaultHost("example.com", "hosts")
return authCfg
}
return cfg
}(),
@ -90,11 +88,11 @@ func Test_remoteResolver(t *testing.T) {
},
config: func() config.Config {
cfg := &config.ConfigMock{}
cfg.HostsFunc = func() []string {
return []string{"example.com"}
}
cfg.DefaultHostFunc = func() (string, string) {
return "example.com", "hosts"
cfg.AuthenticationFunc = func() *config.AuthConfig {
authCfg := &config.AuthConfig{}
authCfg.SetHosts([]string{"example.com"})
authCfg.SetDefaultHost("example.com", "hosts")
return authCfg
}
return cfg
}(),
@ -109,11 +107,11 @@ func Test_remoteResolver(t *testing.T) {
},
config: func() config.Config {
cfg := &config.ConfigMock{}
cfg.HostsFunc = func() []string {
return []string{"example.com"}
}
cfg.DefaultHostFunc = func() (string, string) {
return "example.com", "default"
cfg.AuthenticationFunc = func() *config.AuthConfig {
authCfg := &config.AuthConfig{}
authCfg.SetHosts([]string{"example.com"})
authCfg.SetDefaultHost("example.com", "default")
return authCfg
}
return cfg
}(),
@ -131,11 +129,11 @@ func Test_remoteResolver(t *testing.T) {
},
config: func() config.Config {
cfg := &config.ConfigMock{}
cfg.HostsFunc = func() []string {
return []string{"example.com"}
}
cfg.DefaultHostFunc = func() (string, string) {
return "example.com", "default"
cfg.AuthenticationFunc = func() *config.AuthConfig {
authCfg := &config.AuthConfig{}
authCfg.SetHosts([]string{"example.com"})
authCfg.SetDefaultHost("example.com", "default")
return authCfg
}
return cfg
}(),
@ -150,14 +148,12 @@ func Test_remoteResolver(t *testing.T) {
},
config: func() config.Config {
cfg := &config.ConfigMock{}
cfg.HostsFunc = func() []string {
return []string{"example.com", "github.com"}
}
cfg.DefaultHostFunc = func() (string, string) {
return "github.com", "default"
}
cfg.AuthTokenFunc = func(string) (string, string) {
return "", ""
cfg.AuthenticationFunc = func() *config.AuthConfig {
authCfg := &config.AuthConfig{}
authCfg.SetHosts([]string{"example.com", "github.com"})
authCfg.SetToken("", "")
authCfg.SetDefaultHost("example.com", "default")
return authCfg
}
return cfg
}(),
@ -173,11 +169,11 @@ func Test_remoteResolver(t *testing.T) {
},
config: func() config.Config {
cfg := &config.ConfigMock{}
cfg.HostsFunc = func() []string {
return []string{"example.com", "github.com"}
}
cfg.DefaultHostFunc = func() (string, string) {
return "github.com", "default"
cfg.AuthenticationFunc = func() *config.AuthConfig {
authCfg := &config.AuthConfig{}
authCfg.SetHosts([]string{"example.com", "github.com"})
authCfg.SetDefaultHost("github.com", "default")
return authCfg
}
return cfg
}(),
@ -196,11 +192,11 @@ func Test_remoteResolver(t *testing.T) {
},
config: func() config.Config {
cfg := &config.ConfigMock{}
cfg.HostsFunc = func() []string {
return []string{"example.com", "github.com"}
}
cfg.DefaultHostFunc = func() (string, string) {
return "github.com", "default"
cfg.AuthenticationFunc = func() *config.AuthConfig {
authCfg := &config.AuthConfig{}
authCfg.SetHosts([]string{"example.com", "github.com"})
authCfg.SetDefaultHost("github.com", "default")
return authCfg
}
return cfg
}(),
@ -215,11 +211,11 @@ func Test_remoteResolver(t *testing.T) {
},
config: func() config.Config {
cfg := &config.ConfigMock{}
cfg.HostsFunc = func() []string {
return []string{"example.com"}
}
cfg.DefaultHostFunc = func() (string, string) {
return "test.com", "GH_HOST"
cfg.AuthenticationFunc = func() *config.AuthConfig {
authCfg := &config.AuthConfig{}
authCfg.SetHosts([]string{"example.com"})
authCfg.SetDefaultHost("test.com", "GH_HOST")
return authCfg
}
return cfg
}(),
@ -235,11 +231,11 @@ func Test_remoteResolver(t *testing.T) {
},
config: func() config.Config {
cfg := &config.ConfigMock{}
cfg.HostsFunc = func() []string {
return []string{"example.com"}
}
cfg.DefaultHostFunc = func() (string, string) {
return "test.com", "GH_HOST"
cfg.AuthenticationFunc = func() *config.AuthConfig {
authCfg := &config.AuthConfig{}
authCfg.SetHosts([]string{"example.com"})
authCfg.SetDefaultHost("test.com", "GH_HOST")
return authCfg
}
return cfg
}(),
@ -256,11 +252,11 @@ func Test_remoteResolver(t *testing.T) {
},
config: func() config.Config {
cfg := &config.ConfigMock{}
cfg.HostsFunc = func() []string {
return []string{"example.com", "test.com"}
}
cfg.DefaultHostFunc = func() (string, string) {
return "test.com", "GH_HOST"
cfg.AuthenticationFunc = func() *config.AuthConfig {
authCfg := &config.AuthConfig{}
authCfg.SetHosts([]string{"example.com", "test.com"})
authCfg.SetDefaultHost("test.com", "GH_HOST")
return authCfg
}
return cfg
}(),

2
pkg/cmd/gist/clone/clone.go
View file

@ -79,7 +79,7 @@ func cloneRun(opts *CloneOptions) error {
if err != nil {
return err
}
hostname, _ := cfg.DefaultHost()
hostname, _ := cfg.Authentication().DefaultHost()
protocol, err := cfg.GetOrDefault(hostname, "git_protocol")
if err != nil {
return err

2
pkg/cmd/gist/create/create.go
View file

@ -143,7 +143,7 @@ func createRun(opts *CreateOptions) error {
return err
}
host, _ := cfg.DefaultHost()
host, _ := cfg.Authentication().DefaultHost()
opts.IO.StartProgressIndicator()
gist, err := createGist(httpClient, host, opts.Description, opts.Public, files)

2
pkg/cmd/gist/delete/delete.go
View file

@ -64,7 +64,7 @@ func deleteRun(opts *DeleteOptions) error {
return err
}
host, _ := cfg.DefaultHost()
host, _ := cfg.Authentication().DefaultHost()
apiClient := api.NewClientFromHTTP(client)
if err := deleteGist(apiClient, host, gistID); err != nil {

2
pkg/cmd/gist/edit/edit.go
View file

@ -107,7 +107,7 @@ func editRun(opts *EditOptions) error {
return err
}
host, _ := cfg.DefaultHost()
host, _ := cfg.Authentication().DefaultHost()
gist, err := shared.GetGist(client, host, gistID)
if err != nil {

2
pkg/cmd/gist/list/list.go
View file

@ -76,7 +76,7 @@ func listRun(opts *ListOptions) error {
return err
}
host, _ := cfg.DefaultHost()
host, _ := cfg.Authentication().DefaultHost()
gists, err := shared.ListGists(client, host, opts.Limit, opts.Visibility)
if err != nil {

2
pkg/cmd/gist/view/view.go
View file

@ -85,7 +85,7 @@ func viewRun(opts *ViewOptions) error {
return err
}
hostname, _ := cfg.DefaultHost()
hostname, _ := cfg.Authentication().DefaultHost()
cs := opts.IO.ColorScheme()
if gistID == "" {

6
pkg/cmd/gpg-key/add/add.go
View file

@ -20,6 +20,7 @@ type AddOptions struct {
HTTPClient func() (*http.Client, error)
KeyFile string
Title string
}
func NewCmdAdd(f *cmdutil.Factory, runF func(*AddOptions) error) *cobra.Command {
@ -50,6 +51,7 @@ func NewCmdAdd(f *cmdutil.Factory, runF func(*AddOptions) error) *cobra.Command
},
}
cmd.Flags().StringVarP(&opts.Title, "title", "t", "", "Title for the new key")
return cmd
}
@ -77,9 +79,9 @@ func runAdd(opts *AddOptions) error {
return err
}
hostname, _ := cfg.DefaultHost()
hostname, _ := cfg.Authentication().DefaultHost()
err = gpgKeyUpload(httpClient, hostname, keyReader)
err = gpgKeyUpload(httpClient, hostname, keyReader, opts.Title)
if err != nil {
cs := opts.IO.ColorScheme()
if errors.Is(err, errScopesMissing) {

21
pkg/cmd/gpg-key/add/add_test.go
View file

@ -27,13 +27,32 @@ func Test_runAdd(t *testing.T) {
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("POST", "user/gpg_keys"),
httpmock.StatusStringResponse(200, ``))
httpmock.RESTPayload(200, ``, func(payload map[string]interface{}) {
assert.Contains(t, payload, "armored_public_key")
assert.NotContains(t, payload, "title")
}))
},
wantStdout: "✓ GPG key added to your account\n",
wantStderr: "",
wantErrMsg: "",
opts: AddOptions{KeyFile: "-"},
},
{
name: "valid key with title",
stdin: "-----BEGIN PGP PUBLIC KEY BLOCK-----",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("POST", "user/gpg_keys"),
httpmock.RESTPayload(200, ``, func(payload map[string]interface{}) {
assert.Contains(t, payload, "armored_public_key")
assert.Contains(t, payload, "name")
}))
},
wantStdout: "✓ GPG key added to your account\n",
wantStderr: "",
wantErrMsg: "",
opts: AddOptions{KeyFile: "-", Title: "some title"},
},
{
name: "binary format fails",
stdin: "gCAAAAA7H7MHTZWFLJKD3vP4F7v",

5
pkg/cmd/gpg-key/add/http.go
View file

@ -15,7 +15,7 @@ var errScopesMissing = errors.New("insufficient OAuth scopes")
var errDuplicateKey = errors.New("key already exists")
var errWrongFormat = errors.New("key in wrong format")
func gpgKeyUpload(httpClient *http.Client, hostname string, keyFile io.Reader) error {
func gpgKeyUpload(httpClient *http.Client, hostname string, keyFile io.Reader, title string) error {
url := ghinstance.RESTPrefix(hostname) + "user/gpg_keys"
keyBytes, err := io.ReadAll(keyFile)
@ -26,6 +26,9 @@ func gpgKeyUpload(httpClient *http.Client, hostname string, keyFile io.Reader) e
payload := map[string]string{
"armored_public_key": string(keyBytes),
}
if title != "" {
payload["name"] = title
}
payloadBytes, err := json.Marshal(payload)
if err != nil {

2
pkg/cmd/gpg-key/delete/delete.go
View file

@ -65,7 +65,7 @@ func deleteRun(opts *DeleteOptions) error {
return err
}
host, _ := cfg.DefaultHost()
host, _ := cfg.Authentication().DefaultHost()
gpgKeys, err := getGPGKeys(httpClient, host)
if err != nil {
return err

2
pkg/cmd/gpg-key/list/list.go
View file

@ -54,7 +54,7 @@ func listRun(opts *ListOptions) error {
return err
}
host, _ := cfg.DefaultHost()
host, _ := cfg.Authentication().DefaultHost()
gpgKeys, err := userKeys(apiClient, host, "")
if err != nil {

29
pkg/cmd/issue/create/create_test.go
View file

@ -235,6 +235,15 @@ func Test_createRun(t *testing.T) {
],
"pageInfo": { "hasNextPage": false }
} } } }`))
r.Register(
httpmock.GraphQL(`query UserProjectV2List\b`),
httpmock.StringResponse(`
{ "data": { "viewer": { "projectsV2": {
"nodes": [
{ "title": "Monalisa", "id": "MONALISAID", "resourcePath": "/users/MONALISA/projects/1" }
],
"pageInfo": { "hasNextPage": false }
} } } }`))
},
wantsBrowse: "https://github.com/OWNER/REPO/issues/new?body=&projects=OWNER%2FREPO%2F1",
wantsStderr: "Opening github.com/OWNER/REPO/issues/new in your browser.\n",
@ -646,6 +655,14 @@ func TestIssueCreate_metadata(t *testing.T) {
"pageInfo": { "hasNextPage": false }
} } } }
`))
http.Register(
httpmock.GraphQL(`query UserProjectV2List\b`),
httpmock.StringResponse(`
{ "data": { "viewer": { "projectsV2": {
"nodes": [],
"pageInfo": { "hasNextPage": false }
} } } }
`))
http.Register(
httpmock.GraphQL(`mutation IssueCreate\b`),
httpmock.GraphQLMutation(`
@ -781,7 +798,17 @@ func TestIssueCreate_projectsV2(t *testing.T) {
httpmock.StringResponse(`
{ "data": { "organization": { "projectsV2": {
"nodes": [
{ "title": "TriageV2", "id": "TriageV2ID" }
{ "title": "TriageV2", "id": "TRIAGEV2ID" }
],
"pageInfo": { "hasNextPage": false }
} } } }
`))
http.Register(
httpmock.GraphQL(`query UserProjectV2List\b`),
httpmock.StringResponse(`
{ "data": { "viewer": { "projectsV2": {
"nodes": [
{ "title": "MonalisaV2", "id": "MONALISAV2ID" }
],
"pageInfo": { "hasNextPage": false }
} } } }

10
pkg/cmd/issue/edit/edit_test.go
View file

@ -489,6 +489,16 @@ func mockRepoMetadata(_ *testing.T, reg *httpmock.Registry) {
"pageInfo": { "hasNextPage": false }
} } } }
`))
reg.Register(
httpmock.GraphQL(`query UserProjectV2List\b`),
httpmock.StringResponse(`
{ "data": { "viewer": { "projectsV2": {
"nodes": [
{ "title": "MonalisaV2", "id": "MONALISAV2ID" }
],
"pageInfo": { "hasNextPage": false }
} } } }
`))
}
func mockIssueUpdate(t *testing.T, reg *httpmock.Registry) {

1
pkg/cmd/issue/shared/lookup.go
View file

@ -102,6 +102,7 @@ func findIssueOrPR(httpClient *http.Client, repo ghrepo.Interface, number int, f
if fieldSet.Contains("projectItems") {
getProjectItems = true
fieldSet.Remove("projectItems")
fieldSet.Add("number")
}
fields = fieldSet.ToSlice()

35
pkg/cmd/pr/create/create.go
View file

@ -11,6 +11,7 @@ import (
"time"
"github.com/MakeNowJust/heredoc"
"github.com/cenkalti/backoff/v4"
"github.com/cli/cli/v2/api"
ghContext "github.com/cli/cli/v2/context"
"github.com/cli/cli/v2/git"
@ -741,27 +742,23 @@ func handlePush(opts CreateOptions, ctx CreateContext) error {
// automatically push the branch if it hasn't been pushed anywhere yet
if ctx.IsPushEnabled {
pushBranch := func() error {
pushTries := 0
maxPushTries := 3
for {
w := NewRegexpWriter(opts.IO.ErrOut, gitPushRegexp, "")
defer w.Flush()
gitClient := ctx.GitClient
ref := fmt.Sprintf("HEAD:%s", ctx.HeadBranch)
if err := gitClient.Push(context.Background(), headRemote.Name, ref, git.WithStderr(w)); err != nil {
if didForkRepo && pushTries < maxPushTries {
pushTries++
// first wait 2 seconds after forking, then 4s, then 6s
waitSeconds := 2 * pushTries
fmt.Fprintf(opts.IO.ErrOut, "waiting %s before retrying...\n", text.Pluralize(waitSeconds, "second"))
time.Sleep(time.Duration(waitSeconds) * time.Second)
continue
w := NewRegexpWriter(opts.IO.ErrOut, gitPushRegexp, "")
defer w.Flush()
gitClient := ctx.GitClient
ref := fmt.Sprintf("HEAD:%s", ctx.HeadBranch)
bo := backoff.NewConstantBackOff(2 * time.Second)
ctx := context.Background()
return backoff.Retry(func() error {
if err := gitClient.Push(ctx, headRemote.Name, ref, git.WithStderr(w)); err != nil {
// Only retry if we have forked the repo else the push should succeed the first time.
if didForkRepo {
fmt.Fprintf(opts.IO.ErrOut, "waiting 2 seconds before retrying...\n")
return err
}
return err
return backoff.Permanent(err)
}
break
}
return nil
return nil
}, backoff.WithContext(backoff.WithMaxRetries(bo, 3), ctx))
}
err := pushBranch()

161
pkg/cmd/pr/create/create_test.go
View file

@ -296,31 +296,7 @@ func Test_createRun(t *testing.T) {
reg.Register(
httpmock.GraphQL(`query UserCurrent\b`),
httpmock.StringResponse(`{"data": {"viewer": {"login": "OWNER"} } }`))
reg.Register(
httpmock.GraphQL(`query RepositoryProjectList\b`),
httpmock.StringResponse(`{ "data": { "repository": { "projects": { "nodes": [], "pageInfo": { "hasNextPage": false } } } } }`))
reg.Register(
httpmock.GraphQL(`query OrganizationProjectList\b`),
httpmock.StringResponse(`{ "data": { "organization": { "projects": { "nodes": [], "pageInfo": { "hasNextPage": false } } } } }`))
reg.Register(
httpmock.GraphQL(`query RepositoryProjectV2List\b`),
httpmock.StringResponse(`
{ "data": { "repository": { "projectsV2": {
"nodes": [
{ "title": "CleanupV2", "id": "CLEANUPV2ID" },
{ "title": "RoadmapV2", "id": "ROADMAPV2ID" }
],
"pageInfo": { "hasNextPage": false }
} } } }
`))
reg.Register(
httpmock.GraphQL(`query OrganizationProjectV2List\b`),
httpmock.StringResponse(`
{ "data": { "organization": { "projectsV2": {
"nodes": [],
"pageInfo": { "hasNextPage": false }
} } } }
`))
mockRetrieveProjects(t, reg)
reg.Register(
httpmock.GraphQL(`mutation PullRequestCreate\b`),
httpmock.GraphQLMutation(`
@ -652,44 +628,7 @@ func Test_createRun(t *testing.T) {
"pageInfo": { "hasNextPage": false }
} } } }
`))
reg.Register(
httpmock.GraphQL(`query RepositoryProjectList\b`),
httpmock.StringResponse(`
{ "data": { "repository": { "projects": {
"nodes": [
{ "name": "Cleanup", "id": "CLEANUPID" },
{ "name": "Roadmap", "id": "ROADMAPID" }
],
"pageInfo": { "hasNextPage": false }
} } } }
`))
reg.Register(
httpmock.GraphQL(`query RepositoryProjectV2List\b`),
httpmock.StringResponse(`
{ "data": { "repository": { "projectsV2": {
"nodes": [
{ "title": "CleanupV2", "id": "CLEANUPV2ID" },
{ "title": "RoadmapV2", "id": "ROADMAPV2ID" }
],
"pageInfo": { "hasNextPage": false }
} } } }
`))
reg.Register(
httpmock.GraphQL(`query OrganizationProjectList\b`),
httpmock.StringResponse(`
{ "data": { "organization": { "projects": {
"nodes": [],
"pageInfo": { "hasNextPage": false }
} } } }
`))
reg.Register(
httpmock.GraphQL(`query OrganizationProjectV2List\b`),
httpmock.StringResponse(`
{ "data": { "organization": { "projectsV2": {
"nodes": [],
"pageInfo": { "hasNextPage": false }
} } } }
`))
mockRetrieveProjects(t, reg)
reg.Register(
httpmock.GraphQL(`mutation PullRequestCreate\b`),
httpmock.GraphQLMutation(`
@ -794,46 +733,7 @@ func Test_createRun(t *testing.T) {
reg.Register(
httpmock.GraphQL(`query UserCurrent\b`),
httpmock.StringResponse(`{"data": {"viewer": {"login": "OWNER"} } }`))
reg.Register(
httpmock.GraphQL(`query RepositoryProjectList\b`),
httpmock.StringResponse(`
{ "data": { "repository": { "projects": {
"nodes": [
{ "name": "Cleanup", "id": "CLEANUPID", "resourcePath": "/OWNER/REPO/projects/1" }
],
"pageInfo": { "hasNextPage": false }
} } } }
`))
reg.Register(
httpmock.GraphQL(`query RepositoryProjectV2List\b`),
httpmock.StringResponse(`
{ "data": { "repository": { "projectsV2": {
"nodes": [
{ "title": "CleanupV2", "id": "CLEANUPV2ID", "resourcePath": "/OWNER/REPO/projects/2" }
],
"pageInfo": { "hasNextPage": false }
} } } }
`))
reg.Register(
httpmock.GraphQL(`query OrganizationProjectList\b`),
httpmock.StringResponse(`
{ "data": { "organization": { "projects": {
"nodes": [
{ "name": "Triage", "id": "TRIAGEID", "resourcePath": "/orgs/ORG/projects/1" }
],
"pageInfo": { "hasNextPage": false }
} } } }
`))
reg.Register(
httpmock.GraphQL(`query OrganizationProjectV2List\b`),
httpmock.StringResponse(`
{ "data": { "organization": { "projectsV2": {
"nodes": [
{ "title": "TriageV2", "id": "TRIAGEV2ID", "resourcePath": "/orgs/ORG/projects/2" }
],
"pageInfo": { "hasNextPage": false }
} } } }
`))
mockRetrieveProjects(t, reg)
},
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config --get-regexp.+branch\\\.feature\\\.`, 0, "")
@ -1289,4 +1189,59 @@ func Test_generateCompareURL(t *testing.T) {
}
}
func mockRetrieveProjects(_ *testing.T, reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(`query RepositoryProjectList\b`),
httpmock.StringResponse(`
{ "data": { "repository": { "projects": {
"nodes": [
{ "name": "Cleanup", "id": "CLEANUPID", "resourcePath": "/OWNER/REPO/projects/1" },
{ "name": "Roadmap", "id": "ROADMAPID", "resourcePath": "/OWNER/REPO/projects/2" }
],
"pageInfo": { "hasNextPage": false }
} } } }
`))
reg.Register(
httpmock.GraphQL(`query RepositoryProjectV2List\b`),
httpmock.StringResponse(`
{ "data": { "repository": { "projectsV2": {
"nodes": [
{ "title": "CleanupV2", "id": "CLEANUPV2ID", "resourcePath": "/OWNER/REPO/projects/3" },
{ "title": "RoadmapV2", "id": "ROADMAPV2ID", "resourcePath": "/OWNER/REPO/projects/4" }
],
"pageInfo": { "hasNextPage": false }
} } } }
`))
reg.Register(
httpmock.GraphQL(`query OrganizationProjectList\b`),
httpmock.StringResponse(`
{ "data": { "organization": { "projects": {
"nodes": [
{ "name": "Triage", "id": "TRIAGEID", "resourcePath": "/orgs/ORG/projects/1" }
],
"pageInfo": { "hasNextPage": false }
} } } }
`))
reg.Register(
httpmock.GraphQL(`query OrganizationProjectV2List\b`),
httpmock.StringResponse(`
{ "data": { "organization": { "projectsV2": {
"nodes": [
{ "title": "TriageV2", "id": "TRIAGEV2ID", "resourcePath": "/orgs/ORG/projects/2" }
],
"pageInfo": { "hasNextPage": false }
} } } }
`))
reg.Register(
httpmock.GraphQL(`query UserProjectV2List\b`),
httpmock.StringResponse(`
{ "data": { "viewer": { "projectsV2": {
"nodes": [
{ "title": "MonalisaV2", "id": "MONALISAV2ID", "resourcePath": "/user/MONALISA/projects/2" }
],
"pageInfo": { "hasNextPage": false }
} } } }
`))
}
// TODO interactive metadata tests once: 1) we have test utils for Prompter and 2) metadata questions use Prompter

39
pkg/cmd/pr/edit/edit_test.go
View file

@ -464,22 +464,21 @@ func Test_editRun(t *testing.T) {
},
}
for _, tt := range tests {
ios, _, stdout, stderr := iostreams.Test()
ios.SetStdoutTTY(true)
ios.SetStdinTTY(true)
ios.SetStderrTTY(true)
reg := &httpmock.Registry{}
defer reg.Verify(t)
tt.httpStubs(t, reg)
httpClient := func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }
tt.input.IO = ios
tt.input.HttpClient = httpClient
t.Run(tt.name, func(t *testing.T) {
fmt.Println(tt.name)
ios, _, stdout, stderr := iostreams.Test()
ios.SetStdoutTTY(true)
ios.SetStdinTTY(true)
ios.SetStderrTTY(true)
reg := &httpmock.Registry{}
defer reg.Verify(t)
tt.httpStubs(t, reg)
httpClient := func() (*http.Client, error) { return &http.Client{Transport: reg}, nil }
tt.input.IO = ios
tt.input.HttpClient = httpClient
err := editRun(tt.input)
assert.NoError(t, err)
assert.Equal(t, tt.stdout, stdout.String())
@ -566,6 +565,16 @@ func mockRepoMetadata(_ *testing.T, reg *httpmock.Registry, skipReviewers bool)
"pageInfo": { "hasNextPage": false }
} } } }
`))
reg.Register(
httpmock.GraphQL(`query UserProjectV2List\b`),
httpmock.StringResponse(`
{ "data": { "viewer": { "projectsV2": {
"nodes": [
{ "title": "MonalisaV2", "id": "MONALISAV2ID" }
],
"pageInfo": { "hasNextPage": false }
} } } }
`))
if !skipReviewers {
reg.Register(
httpmock.GraphQL(`query OrganizationTeamList\b`),

24
pkg/cmd/release/shared/upload.go
View file

@ -13,6 +13,7 @@ import (
"strings"
"time"
"github.com/cenkalti/backoff/v4"
"github.com/cli/cli/v2/api"
"golang.org/x/sync/errgroup"
)
@ -117,10 +118,6 @@ func ConcurrentUpload(httpClient httpDoer, uploadURL string, numWorkers int, ass
return g.Wait()
}
var retryInterval = time.Millisecond * 200
const maxRetries = 3
func shouldRetry(err error) bool {
var networkError errNetwork
if errors.As(err, &networkError) {
@ -130,26 +127,23 @@ func shouldRetry(err error) bool {
return errors.As(err, &httpError) && httpError.StatusCode >= 500
}
// Allow injecting backoff interval in tests.
var retryInterval = time.Millisecond * 200
func uploadWithDelete(ctx context.Context, httpClient httpDoer, uploadURL string, a AssetForUpload) error {
if a.ExistingURL != "" {
if err := deleteAsset(ctx, httpClient, a.ExistingURL); err != nil {
return err
}
}
retries := 0
for {
bo := backoff.NewConstantBackOff(retryInterval)
return backoff.Retry(func() error {
_, err := uploadAsset(ctx, httpClient, uploadURL, a)
if err == nil || retries == maxRetries || !shouldRetry(err) {
if err == nil || shouldRetry(err) {
return err
}
retries++
select {
case <-ctx.Done():
return ctx.Err()
case <-time.After(time.Duration(retries) * retryInterval):
}
}
return backoff.Permanent(err)
}, backoff.WithContext(backoff.WithMaxRetries(bo, 3), ctx))
}
func uploadAsset(ctx context.Context, httpClient httpDoer, uploadURL string, asset AssetForUpload) (*ReleaseAsset, error) {

3
pkg/cmd/release/shared/upload_test.go
View file

@ -7,7 +7,6 @@ import (
"io"
"net/http"
"testing"
"time"
)
func Test_typeForFilename(t *testing.T) {
@ -77,7 +76,7 @@ func Test_typeForFilename(t *testing.T) {
}
func Test_uploadWithDelete_retry(t *testing.T) {
retryInterval = time.Millisecond
retryInterval = 0
ctx := context.Background()
tries := 0

2
pkg/cmd/repo/archive/archive.go
View file

@ -87,7 +87,7 @@ func archiveRun(opts *ArchiveOptions) error {
return err
}
hostname, _ := cfg.DefaultHost()
hostname, _ := cfg.Authentication().DefaultHost()
currentUser, err := api.CurrentLoginName(apiClient, hostname)
if err != nil {

2
pkg/cmd/repo/clone/clone.go
View file

@ -114,7 +114,7 @@ func cloneRun(opts *CloneOptions) error {
if repositoryIsFullName {
fullName = opts.Repository
} else {
host, _ := cfg.DefaultHost()
host, _ := cfg.Authentication().DefaultHost()
currentUser, err := api.CurrentLoginName(apiClient, host)
if err != nil {
return err

8
pkg/cmd/repo/create/create.go
View file

@ -206,7 +206,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co
if err != nil {
return nil, cobra.ShellCompDirectiveError
}
hostname, _ := cfg.DefaultHost()
hostname, _ := cfg.Authentication().DefaultHost()
results, err := listGitIgnoreTemplates(httpClient, hostname)
if err != nil {
return nil, cobra.ShellCompDirectiveError
@ -223,7 +223,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co
if err != nil {
return nil, cobra.ShellCompDirectiveError
}
hostname, _ := cfg.DefaultHost()
hostname, _ := cfg.Authentication().DefaultHost()
licenses, err := listLicenseTemplates(httpClient, hostname)
if err != nil {
return nil, cobra.ShellCompDirectiveError
@ -271,7 +271,7 @@ func createFromScratch(opts *CreateOptions) error {
return err
}
host, _ := cfg.DefaultHost()
host, _ := cfg.Authentication().DefaultHost()
if opts.Interactive {
opts.Name, opts.Description, opts.Visibility, err = interactiveRepoInfo(httpClient, host, opts.Prompter, "")
@ -420,7 +420,7 @@ func createFromLocal(opts *CreateOptions) error {
if err != nil {
return err
}
host, _ := cfg.DefaultHost()
host, _ := cfg.Authentication().DefaultHost()
if opts.Interactive {
var err error

30
pkg/cmd/repo/fork/fork.go
View file

@ -2,6 +2,7 @@ package fork
import (
"context"
"errors"
"fmt"
"net/http"
"net/url"
@ -9,6 +10,7 @@ import (
"time"
"github.com/MakeNowJust/heredoc"
"github.com/cenkalti/backoff/v4"
"github.com/cli/cli/v2/api"
ghContext "github.com/cli/cli/v2/context"
"github.com/cli/cli/v2/git"
@ -32,6 +34,7 @@ type ForkOptions struct {
BaseRepo func() (ghrepo.Interface, error)
Remotes func() (ghContext.Remotes, error)
Since func(time.Time) time.Duration
BackOff backoff.BackOff
GitArgs []string
Repository string
@ -46,6 +49,10 @@ type ForkOptions struct {
DefaultBranchOnly bool
}
type errWithExitCode interface {
ExitCode() int
}
// TODO warn about useless flags (--remote, --remote-name) when running from outside a repository
// TODO output over STDOUT not STDERR
// TODO remote-name has no effect on its own; error that or change behavior
@ -202,7 +209,7 @@ func forkRun(opts *ForkOptions) error {
cs.Bold(ghrepo.FullName(forkedRepo)),
"already exists")
} else {
fmt.Fprintf(stderr, "%s already exists", ghrepo.FullName(forkedRepo))
fmt.Fprintf(stderr, "%s already exists\n", ghrepo.FullName(forkedRepo))
}
} else {
if connectedToTerminal {
@ -317,8 +324,25 @@ func forkRun(opts *ForkOptions) error {
}
}
if cloneDesired {
forkedRepoURL := ghrepo.FormatRemoteURL(forkedRepo, protocol)
cloneDir, err := gitClient.Clone(ctx, forkedRepoURL, opts.GitArgs)
// Allow injecting alternative BackOff in tests.
if opts.BackOff == nil {
bo := backoff.NewConstantBackOff(2 * time.Second)
opts.BackOff = bo
}
cloneDir, err := backoff.RetryWithData(func() (string, error) {
forkedRepoURL := ghrepo.FormatRemoteURL(forkedRepo, protocol)
dir, err := gitClient.Clone(ctx, forkedRepoURL, opts.GitArgs)
if err == nil {
return dir, err
}
var execError errWithExitCode
if errors.As(err, &execError) && execError.ExitCode() == 128 {
return "", err
}
return "", backoff.Permanent(err)
}, backoff.WithContext(backoff.WithMaxRetries(opts.BackOff, 3), ctx))
if err != nil {
return fmt.Errorf("failed to clone fork: %w", err)
}

160
pkg/cmd/repo/fork/fork_test.go
View file

@ -9,6 +9,7 @@ import (
"testing"
"time"
"github.com/cenkalti/backoff/v4"
"github.com/cli/cli/v2/context"
"github.com/cli/cli/v2/git"
"github.com/cli/cli/v2/internal/config"
@ -406,7 +407,7 @@ func TestRepoFork(t *testing.T) {
},
},
httpStubs: forkPost,
wantErrOut: "someone/REPO already exists",
wantErrOut: "someone/REPO already exists\n",
},
{
name: "implicit nontty --remote",
@ -559,7 +560,7 @@ func TestRepoFork(t *testing.T) {
},
},
httpStubs: forkPost,
wantErrOut: "someone/REPO already exists",
wantErrOut: "someone/REPO already exists\n",
},
{
name: "repo arg nontty clone arg already exists",
@ -576,7 +577,7 @@ func TestRepoFork(t *testing.T) {
cs.Register(`git -C REPO remote add upstream https://github\.com/OWNER/REPO\.git`, 0, "")
cs.Register(`git -C REPO fetch upstream`, 0, "")
},
wantErrOut: "someone/REPO already exists",
wantErrOut: "someone/REPO already exists\n",
},
{
name: "repo arg nontty clone arg",
@ -663,78 +664,111 @@ func TestRepoFork(t *testing.T) {
},
wantErrOut: "✓ Created fork OWNER/REPO\n✓ Renamed fork to OWNER/NEW_REPO\n",
},
{
name: "retries clone up to four times if necessary",
opts: &ForkOptions{
Repository: "OWNER/REPO",
Clone: true,
BackOff: &backoff.ZeroBackOff{},
},
httpStubs: forkPost,
execStubs: func(cs *run.CommandStubber) {
cs.Register(`git clone https://github.com/someone/REPO\.git`, 128, "")
cs.Register(`git clone https://github.com/someone/REPO\.git`, 128, "")
cs.Register(`git clone https://github.com/someone/REPO\.git`, 128, "")
cs.Register(`git clone https://github.com/someone/REPO\.git`, 0, "")
cs.Register(`git -C REPO remote add upstream https://github\.com/OWNER/REPO\.git`, 0, "")
cs.Register(`git -C REPO fetch upstream`, 0, "")
},
},
{
name: "does not retry clone if error occurs and exit code is not 128",
opts: &ForkOptions{
Repository: "OWNER/REPO",
Clone: true,
BackOff: &backoff.ZeroBackOff{},
},
httpStubs: forkPost,
execStubs: func(cs *run.CommandStubber) {
cs.Register(`git clone https://github.com/someone/REPO\.git`, 128, "")
cs.Register(`git clone https://github.com/someone/REPO\.git`, 65, "")
},
wantErr: true,
errMsg: `failed to clone fork: failed to run git: git -c credential.helper= -c credential.helper=!"[^"]+" auth git-credential clone https://github.com/someone/REPO\.git exited with status 65`,
},
}
for _, tt := range tests {
ios, _, stdout, stderr := iostreams.Test()
ios.SetStdinTTY(tt.tty)
ios.SetStdoutTTY(tt.tty)
ios.SetStderrTTY(tt.tty)
tt.opts.IO = ios
tt.opts.BaseRepo = func() (ghrepo.Interface, error) {
return ghrepo.New("OWNER", "REPO"), nil
}
reg := &httpmock.Registry{}
if tt.httpStubs != nil {
tt.httpStubs(reg)
}
tt.opts.HttpClient = func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
}
cfg := config.NewBlankConfig()
if tt.cfgStubs != nil {
tt.cfgStubs(cfg)
}
tt.opts.Config = func() (config.Config, error) {
return cfg, nil
}
tt.opts.Remotes = func() (context.Remotes, error) {
if tt.remotes == nil {
return []*context.Remote{
{
Remote: &git.Remote{
Name: "origin",
FetchURL: &url.URL{},
},
Repo: ghrepo.New("OWNER", "REPO"),
},
}, nil
}
return tt.remotes, nil
}
tt.opts.GitClient = &git.Client{
GhPath: "some/path/gh",
GitPath: "some/path/git",
}
//nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber
as, teardown := prompt.InitAskStubber()
defer teardown()
if tt.askStubs != nil {
tt.askStubs(as)
}
cs, restoreRun := run.Stub()
defer restoreRun(t)
if tt.execStubs != nil {
tt.execStubs(cs)
}
t.Run(tt.name, func(t *testing.T) {
ios, _, stdout, stderr := iostreams.Test()
ios.SetStdinTTY(tt.tty)
ios.SetStdoutTTY(tt.tty)
ios.SetStderrTTY(tt.tty)
tt.opts.IO = ios
tt.opts.BaseRepo = func() (ghrepo.Interface, error) {
return ghrepo.New("OWNER", "REPO"), nil
}
reg := &httpmock.Registry{}
if tt.httpStubs != nil {
tt.httpStubs(reg)
}
tt.opts.HttpClient = func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
}
cfg := config.NewBlankConfig()
if tt.cfgStubs != nil {
tt.cfgStubs(cfg)
}
tt.opts.Config = func() (config.Config, error) {
return cfg, nil
}
tt.opts.Remotes = func() (context.Remotes, error) {
if tt.remotes == nil {
return []*context.Remote{
{
Remote: &git.Remote{
Name: "origin",
FetchURL: &url.URL{},
},
Repo: ghrepo.New("OWNER", "REPO"),
},
}, nil
}
return tt.remotes, nil
}
tt.opts.GitClient = &git.Client{
GhPath: "some/path/gh",
GitPath: "some/path/git",
}
//nolint:staticcheck // SA1019: prompt.InitAskStubber is deprecated: use NewAskStubber
as, teardown := prompt.InitAskStubber()
defer teardown()
if tt.askStubs != nil {
tt.askStubs(as)
}
cs, restoreRun := run.Stub()
defer restoreRun(t)
if tt.execStubs != nil {
tt.execStubs(cs)
}
if tt.opts.Since == nil {
tt.opts.Since = func(t time.Time) time.Duration {
return 2 * time.Second
}
}
defer reg.Verify(t)
err := forkRun(tt.opts)
if tt.wantErr {
assert.Error(t, err)
assert.Equal(t, tt.errMsg, err.Error())
assert.Error(t, err, tt.errMsg)
return
}

2
pkg/cmd/repo/garden/garden.go
View file

@ -155,7 +155,7 @@ func gardenRun(opts *GardenOptions) error {
if err != nil {
return err
}
hostname, _ := cfg.DefaultHost()
hostname, _ := cfg.Authentication().DefaultHost()
currentUser, err := api.CurrentLoginName(apiClient, hostname)
if err != nil {
return err

2
pkg/cmd/repo/list/list.go
View file

@ -119,7 +119,7 @@ func listRun(opts *ListOptions) error {
return err
}
host, _ := cfg.DefaultHost()
host, _ := cfg.Authentication().DefaultHost()
if opts.Detector == nil {
cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24)

2
pkg/cmd/repo/repo.go
View file

@ -15,6 +15,7 @@ import (
repoRenameCmd "github.com/cli/cli/v2/pkg/cmd/repo/rename"
repoDefaultCmd "github.com/cli/cli/v2/pkg/cmd/repo/setdefault"
repoSyncCmd "github.com/cli/cli/v2/pkg/cmd/repo/sync"
repoUnarchiveCmd "github.com/cli/cli/v2/pkg/cmd/repo/unarchive"
repoViewCmd "github.com/cli/cli/v2/pkg/cmd/repo/view"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/spf13/cobra"
@ -55,6 +56,7 @@ func NewCmdRepo(f *cmdutil.Factory) *cobra.Command {
deployKeyCmd.NewCmdDeployKey(f),
repoRenameCmd.NewCmdRename(f, nil),
repoArchiveCmd.NewCmdArchive(f, nil),
repoUnarchiveCmd.NewCmdUnarchive(f, nil),
repoDeleteCmd.NewCmdDelete(f, nil),
creditsCmd.NewCmdRepoCredits(f, nil),
gardenCmd.NewCmdGarden(f, nil),

28
pkg/cmd/repo/unarchive/http.go Normal file
View file

@ -0,0 +1,28 @@
package unarchive
import (
"net/http"
"github.com/cli/cli/v2/api"
"github.com/shurcooL/githubv4"
)
func unarchiveRepo(client *http.Client, repo *api.Repository) error {
var mutation struct {
UnarchiveRepository struct {
Repository struct {
ID string
}
} `graphql:"unarchiveRepository(input: $input)"`
}
variables := map[string]interface{}{
"input": githubv4.UnarchiveRepositoryInput{
RepositoryID: repo.ID,
},
}
gql := api.NewClientFromHTTP(client)
err := gql.Mutate(repo.RepoHost(), "UnarchiveRepository", &mutation, variables)
return err
}

Some files were not shown because too many files have changed in this diff Show more