githubauth: fix response parsing (#291)

This fixes a bug where the TokenSource would return the correct value,
but the direct invocations would fail.

This is affecting the latest Minty release.

Review the first commit for the nicer diff. The second commit is moving
things around so this mistake doesn't happen again:
[1dc001b](1dc001b).

Author: sethvargo

githubauth/app.go

@@ -27,7 +27,6 @@ import (
 	"fmt"
 	"io"
 	"net/http"
-	"strings"
 	"time"
 
 	"golang.org/x/oauth2"
@@ -199,10 +198,25 @@ func (g *App) AppToken() ([]byte, error) {
 	return token, nil
 }
 
+// OAuthAppTokenSource adheres to the oauth2 TokenSource interface and returns a oauth2 token
+// by creating a JWT token.
+func (g *App) OAuthAppTokenSource() oauth2.TokenSource {
+	return oauth2TokenSource(func() (*oauth2.Token, error) {
+		jwt, err := g.AppToken()
+		if err != nil {
+			return nil, fmt.Errorf("failed to generate app token: %w", err)
+		}
+
+		return &oauth2.Token{
+			AccessToken: string(jwt),
+		}, nil
+	})
+}
+
 // AccessToken calls the GitHub API to generate a new access token for this
 // application installation with the requested permissions and repositories.
 func (g *App) AccessToken(ctx context.Context, request *TokenRequest) (string, error) {
-	if request.Repositories == nil {
+	if request == nil || request.Repositories == nil {
 		return "", fmt.Errorf("requested repositories cannot be nil, did you mean to use AccessTokenAllRepos to request all repos?")
 	}
 
@@ -214,6 +228,21 @@ func (g *App) AccessToken(ctx context.Context, request *TokenRequest) (string, e
 	return g.githubAccessToken(ctx, requestJSON)
 }
 
+// SelectedReposTokenSource returns a [TokenSource] that mints a GitHub token
+// with permissions on the selected repos.
+func (g *App) SelectedReposTokenSource(permissions map[string]string, repos ...string) TokenSource {
+	return TokenSourceFunc(func(ctx context.Context) (string, error) {
+		token, err := g.AccessToken(ctx, &TokenRequest{
+			Permissions:  permissions,
+			Repositories: repos,
+		})
+		if err != nil {
+			return "", fmt.Errorf("failed to get github access token for repos %q: %w", repos, err)
+		}
+		return token, nil
+	})
+}
+
 // AccessTokenAllRepos calls the GitHub API to generate a new access token for
 // this application installation with the requested permissions and all granted
 // repositories.
@@ -226,92 +255,64 @@ func (g *App) AccessTokenAllRepos(ctx context.Context, request *TokenRequestAllR
 	return g.githubAccessToken(ctx, requestJSON)
 }
 
+// AllReposTokenSource returns a [TokenSource] that mints a GitHub token with
+// permissions on all repos.
+func (g *App) AllReposTokenSource(permissions map[string]string) TokenSource {
+	return TokenSourceFunc(func(ctx context.Context) (string, error) {
+		token, err := g.AccessTokenAllRepos(ctx, &TokenRequestAllRepos{
+			Permissions: permissions,
+		})
+		if err != nil {
+			return "", fmt.Errorf("failed to get github access token for all repos: %w", err)
+		}
+		return token, nil
+	})
+}
+
 // githubAccessToken calls the GitHub API to generate a new access token with
 // provided JSON payload bytes.
 func (g *App) githubAccessToken(ctx context.Context, requestJSON []byte) (string, error) {
 	appJWT, err := g.AppToken()
 	if err != nil {
-		return "", fmt.Errorf("error generating app jwt: %w", err)
+		return "", fmt.Errorf("failed to generate github app jws: %w", err)
 	}
 	requestURL := fmt.Sprintf(g.accessTokenURLPattern, g.InstallationID)
 
 	requestReader := bytes.NewReader(requestJSON)
 	req, err := http.NewRequestWithContext(ctx, http.MethodPost, requestURL, requestReader)
 	if err != nil {
-		return "", fmt.Errorf("error creating http request for GitHub installation information: %w", err)
+		return "", fmt.Errorf("failed to create http request: %w", err)
 	}
 	req.Header.Set("Accept", "application/vnd.github+json")
 	req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", appJWT))
 
 	res, err := g.httpClient.Do(req)
 	if err != nil {
-		return "", fmt.Errorf("error making http request for GitHub installation access token %w", err)
+		return "", fmt.Errorf("failed to make http request: %w", err)
 	}
 	defer res.Body.Close()
 
 	b, err := io.ReadAll(io.LimitReader(res.Body, 64_000))
 	if err != nil {
-		return "", fmt.Errorf("error reading http response for GitHub installation access token %w", err)
+		return "", fmt.Errorf("failed to read response body: %w", err)
 	}
 
-	if res.StatusCode != http.StatusCreated {
-		return "", fmt.Errorf("failed to retrieve token from GitHub - Status: %s - Body: %s", res.Status, string(b))
+	if got, want := res.StatusCode, http.StatusCreated; got != want {
+		return "", fmt.Errorf("invalid http response status (expected %d to be %d): %s", got, want, string(b))
 	}
 
-	// GitHub will respond with a 201 when you send a request for an invalid combination,
-	// e.g. 'issues':'write' for an empty repository list. This 201 comes with a response that is not actually JSON.
-	// Attempt to parse the JSON to see if this is a valid token, if it is not
-	// then respond with an error containing the
-	// actual response from GitHub.
-	tokenContent := map[string]any{}
-	if err := json.Unmarshal(b, &tokenContent); err != nil {
-		return "", fmt.Errorf("invalid access token from GitHub - Body: %s", string(b))
+	// GitHub will respond with a 201 when you send a request for an invalid
+	// combination, e.g. 'issues':'write' for an empty repository list. This 201
+	// comes with a response that is not actually JSON. Attempt to parse the JSON
+	// to see if this is a valid token, if it is not then respond with an error
+	// containing the actual response from GitHub.
+	var resp struct {
+		Token string `json:"token"`
 	}
-	return string(b), nil
-}
-
-// OAuthAppTokenSource adheres to the oauth2 TokenSource interface and returns a oauth2 token
-// by creating a JWT token.
-func (g *App) OAuthAppTokenSource() oauth2.TokenSource {
-	return oauth2TokenSource(func() (*oauth2.Token, error) {
-		jwt, err := g.AppToken()
-		if err != nil {
-			return nil, fmt.Errorf("failed to generate app token: %w", err)
-		}
-
-		return &oauth2.Token{
-			AccessToken: string(jwt),
-		}, nil
-	})
-}
-
-// AllReposTokenSource returns a [TokenSource] that mints a GitHub token with
-// permissions on all repos.
-func (g *App) AllReposTokenSource(permissions map[string]string) TokenSource {
-	return TokenSourceFunc(func(ctx context.Context) (string, error) {
-		resp, err := g.AccessTokenAllRepos(ctx, &TokenRequestAllRepos{
-			Permissions: permissions,
-		})
-		if err != nil {
-			return "", fmt.Errorf("failed to get github access token for all repos: %w", err)
-		}
-		return parseAppTokenResponse(resp)
-	})
-}
-
-// SelectedReposTokenSource returns a [TokenSource] that mints a GitHub token
-// with permissions on the selected repos.
-func (g *App) SelectedReposTokenSource(permissions map[string]string, repos ...string) TokenSource {
-	return TokenSourceFunc(func(ctx context.Context) (string, error) {
-		resp, err := g.AccessToken(ctx, &TokenRequest{
-			Permissions:  permissions,
-			Repositories: repos,
-		})
-		if err != nil {
-			return "", fmt.Errorf("failed to get github access token for repos %q: %w", repos, err)
-		}
-		return parseAppTokenResponse(resp)
-	})
+	if err := json.Unmarshal(b, &resp); err != nil {
+		return "", fmt.Errorf("failed to parse response as json: %w: %s", err, string(b))
+	}
+	return resp.Token, nil
 }
 
 // generateAppJWT builds a signed JWT that can be used to communicate with
@@ -363,18 +364,3 @@ func parseRSAPrivateKeyPEM(data []byte) (*rsa.PrivateKey, error) {
 	}
 	return key, nil
 }
-
-// parseAppTokenResponse parses the given JWT and returns the embedded token.
-func parseAppTokenResponse(data string) (string, error) {
-	var resp struct {
-		Token string `json:"token"`
-	}
-
-	if err := json.NewDecoder(strings.NewReader(data)).Decode(&resp); err != nil {
-		return "", fmt.Errorf("failed to parse json: %w", err)
-	}
-	if resp.Token == "" {
-		return "", fmt.Errorf("no token in json response")
-	}
-	return resp.Token, nil
-}