diff --git a/handlers/github.go b/handlers/github.go index 9b33594d1..1c4fb5767 100644 --- a/handlers/github.go +++ b/handlers/github.go @@ -66,14 +66,21 @@ func GetOpenGithubIssues(w http.ResponseWriter, r *http.Request) { } func githubClient() *github.Client { - gh_token := os.Getenv("GITHUB_TOKEN") + return githubClientWithToken(os.Getenv("GITHUB_TOKEN")) +} + +func githubClientWithToken(gh_token string) *github.Client { + gh_token = strings.TrimSpace(gh_token) + if gh_token == "" { + return github.NewClient(nil) + } + ctx := context.Background() ts := oauth2.StaticTokenSource( &oauth2.Token{AccessToken: gh_token}, ) tc := oauth2.NewClient(ctx, ts) - gc := github.NewClient(tc) - return gc + return github.NewClient(tc) } func GetRepoIssues(owner string, repo string) ([]db.GithubIssue, error) { @@ -82,15 +89,7 @@ func GetRepoIssues(owner string, repo string) ([]db.GithubIssue, error) { ret := []db.GithubIssue{} if err == nil { for _, iss := range issues { - assignee := "" - if iss.Assignee != nil { - assignee = *iss.Assignee.Login - } - ret = append(ret, db.GithubIssue{ - Title: *iss.Title, - Status: *iss.State, - Assignee: assignee, - }) + ret = append(ret, githubIssueToDBIssue(iss)) } } return ret, err @@ -101,20 +100,36 @@ func GetIssue(owner string, repo string, id int) (db.GithubIssue, error) { iss, _, err := client.Issues.Get(context.Background(), owner, repo, id) issue := db.GithubIssue{} if err == nil && iss != nil { - assignee := "" - if iss.Assignee != nil { - assignee = *iss.Assignee.Login - } - issue = db.GithubIssue{ - Title: *iss.Title, - Status: *iss.State, - Assignee: assignee, - Description: *iss.Body, - } + issue = githubIssueToDBIssue(iss) } return issue, err } +func githubIssueToDBIssue(issue *github.Issue) db.GithubIssue { + if issue == nil { + return db.GithubIssue{} + } + + assignee := "" + if issue.Assignee != nil { + assignee = githubString(issue.Assignee.Login) + } + + return db.GithubIssue{ + Title: githubString(issue.Title), + Status: githubString(issue.State), + Assignee: assignee, + Description: githubString(issue.Body), + } +} + +func githubString(value *string) string { + if value == nil { + return "" + } + return *value +} + func PubkeyForGithubUser(owner string) (string, error) { client := githubClient() gs, _, err := client.Gists.List(context.Background(), owner, nil) diff --git a/handlers/github_test.go b/handlers/github_test.go new file mode 100644 index 000000000..1d4d41921 --- /dev/null +++ b/handlers/github_test.go @@ -0,0 +1,92 @@ +package handlers + +import ( + "context" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/google/go-github/v39/github" + "github.com/stretchr/testify/assert" +) + +func TestGithubClientWithTokenDoesNotSendEmptyAuthorization(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if got := r.Header.Get("Authorization"); got != "" { + t.Fatalf("expected no Authorization header for empty token, got %q", got) + } + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"title":"Public issue","state":"open","body":"body"}`)) + })) + defer server.Close() + + client := githubClientWithToken("") + baseURL, err := url.Parse(server.URL + "/") + if err != nil { + t.Fatalf("failed to parse test server url: %v", err) + } + client.BaseURL = baseURL + + if _, _, err := client.Issues.Get(context.Background(), "owner", "repo", 1); err != nil { + t.Fatalf("expected public issue request without a token to succeed: %v", err) + } +} + +func TestGithubClientWithTokenSendsAuthorizationWhenConfigured(t *testing.T) { + const token = "test-token" + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if got := r.Header.Get("Authorization"); got != "Bearer "+token { + t.Fatalf("expected bearer Authorization header, got %q", got) + } + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"title":"Private issue","state":"open","body":"body"}`)) + })) + defer server.Close() + + client := githubClientWithToken(token) + baseURL, err := url.Parse(server.URL + "/") + if err != nil { + t.Fatalf("failed to parse test server url: %v", err) + } + client.BaseURL = baseURL + + if _, _, err := client.Issues.Get(context.Background(), "owner", "repo", 1); err != nil { + t.Fatalf("expected issue request with a token to succeed: %v", err) + } +} + +func TestGithubIssueToDBIssueHandlesNilFields(t *testing.T) { + issue := &github.Issue{ + Title: github.String("Stakwork LN-auth"), + State: github.String("open"), + } + + got := githubIssueToDBIssue(issue) + + assert.Equal(t, "Stakwork LN-auth", got.Title) + assert.Equal(t, "open", got.Status) + assert.Equal(t, "", got.Assignee) + assert.Equal(t, "", got.Description) +} + +func TestGithubIssueToDBIssueIncludesAssigneeAndBody(t *testing.T) { + issue := &github.Issue{ + Title: github.String("Add LN-AUTH to Stakwork"), + State: github.String("closed"), + Body: github.String("Issue body"), + Assignee: &github.User{Login: github.String("octocat")}, + } + + got := githubIssueToDBIssue(issue) + + assert.Equal(t, "Add LN-AUTH to Stakwork", got.Title) + assert.Equal(t, "closed", got.Status) + assert.Equal(t, "octocat", got.Assignee) + assert.Equal(t, "Issue body", got.Description) +}