Merge pull request #51 from jferrl/feat/with-base-url

feat: add WithBaseURL option for custom API base URLs (#50)

Author: jferrl

README.md

@@ -278,6 +278,38 @@ func main() {
 }
 ```
 
+### GitHub Enterprise and Custom Base URLs
+
+The installation token source talks to `https://api.github.com/` by default. Two options point it elsewhere:
+
+- `WithEnterpriseURL` — for **GitHub Enterprise Server (GHES)**. The supplied URL is normalized the way GHES expects, appending `/api/v3/` when the host is not already an `api.` subdomain.
+- `WithBaseURL` — uses the URL **verbatim** (only adding a trailing slash when missing), closely mirroring how `go-github` lets you set a custom endpoint. No `/api/v3/` suffix is added. Use it for **GitHub Enterprise Cloud (GHEC) with data residency** (`https://api.SUBDOMAIN.ghe.com/`) or to point the client at an `httptest` server in your tests.
+
+```go
+// GitHub Enterprise Server (GHES)
+installationTokenSource := githubauth.NewInstallationTokenSource(
+    installationID,
+    appTokenSource,
+    githubauth.WithEnterpriseURL("https://github.example.com"),
+)
+
+// GitHub Enterprise Cloud (GHEC) with data residency
+installationTokenSource = githubauth.NewInstallationTokenSource(
+    installationID,
+    appTokenSource,
+    githubauth.WithBaseURL("https://api.octocorp.ghe.com"),
+)
+
+// Point at an httptest server in tests
+installationTokenSource = githubauth.NewInstallationTokenSource(
+    installationID,
+    appTokenSource,
+    githubauth.WithBaseURL(server.URL),
+)
+```
+
+Option order does not matter — `WithBaseURL`, `WithEnterpriseURL`, `WithHTTPClient`, and `WithRetryOnThrottle` can be combined in any order. If the provided URL cannot be parsed (or a `nil` HTTP client is passed), the misconfiguration is reported by the first call to `Token()` rather than silently falling back to the public GitHub API.
+
 ### Proactive Token Refresh
 
 `oauth2.ReuseTokenSource` only refreshes a cached token *after* its expiry has passed. A request that starts at `T-100ms` with a token expiring at `T` can arrive at GitHub with an already-expired credential and receive a 401 that the caller must manually retry. This is especially painful with short application-token windows (default 10 min, optionally lower).

auth.go

@@ -273,29 +273,69 @@ func WithInstallationTokenOptions(opts *InstallationTokenOptions) InstallationTo
 	}
 }
 
-// WithHTTPClient sets the HTTP client for the GitHub App installation token source.
+// WithHTTPClient sets the HTTP client used to call the GitHub API. Its transport
+// is wrapped so installation-token requests are authenticated with the GitHub
+// App JWT; any base URL configured via WithBaseURL or WithEnterpriseURL is
+// preserved, so these options may be combined in any order.
+//
+// A nil client is a configuration error reported by the first call to Token().
 func WithHTTPClient(client *http.Client) InstallationTokenSourceOpt {
 	return func(i *installationTokenSource) {
-		client.Transport = &oauth2.Transport{
+		if client == nil {
+			i.setConfigErr(errors.New("WithHTTPClient: http client must not be nil"))
+			return
+		}
+
+		// Work on a shallow copy so the caller's *http.Client is not mutated;
+		// only the Transport is swapped to inject GitHub App authentication.
+		authClient := *client
+		authClient.Transport = &oauth2.Transport{
 			Source: i.src,
 			Base:   client.Transport,
 		}
 
-		i.client = newGitHubClient(client)
+		// Swap only the underlying *http.Client so the base URL and retry
+		// settings already configured on i.client survive, regardless of the
+		// order options are applied in.
+		i.client.httpClient = &authClient
 	}
 }
 
-// WithEnterpriseURL sets the base URL for GitHub Enterprise Server.
-// This option should be used after WithHTTPClient to ensure the HTTP client is properly configured.
-// If the provided base URL is invalid, the option is ignored and default GitHub base URL is used.
+// WithEnterpriseURL sets the base URL for GitHub Enterprise Server (GHES). The
+// URL is normalized the way GHES expects, appending the "/api/v3/" path when the
+// host is not already an "api." subdomain. For GitHub Enterprise Cloud or a
+// verbatim URL (such as an httptest server), use WithBaseURL instead.
+//
+// Option order does not matter; it may be combined with WithHTTPClient,
+// WithBaseURL, and WithRetryOnThrottle in any order. If the URL cannot be
+// parsed, the error is reported by the first call to Token() rather than
+// silently falling back to the public GitHub API.
 func WithEnterpriseURL(baseURL string) InstallationTokenSourceOpt {
 	return func(i *installationTokenSource) {
-		enterpriseClient, err := i.client.withEnterpriseURL(baseURL)
-		if err != nil {
-			return
+		if _, err := i.client.withEnterpriseURL(baseURL); err != nil {
+			i.setConfigErr(err)
 		}
+	}
+}
 
-		i.client = enterpriseClient
+// WithBaseURL sets the API base URL used to create installation tokens, closely
+// mirroring how go-github lets callers point the client at a custom endpoint.
+// Unlike WithEnterpriseURL, the URL is used verbatim — only a trailing slash is
+// appended when missing, and no "/api/v3/" suffix is added.
+//
+// Use this for:
+//   - GitHub Enterprise Cloud with data residency (https://api.SUBDOMAIN.ghe.com/)
+//   - pointing the client at an httptest server in tests
+//
+// Option order does not matter; it may be combined with WithHTTPClient,
+// WithEnterpriseURL, and WithRetryOnThrottle in any order. If the URL cannot be
+// parsed, the error is reported by the first call to Token() rather than
+// silently falling back to the public GitHub API.
+func WithBaseURL(baseURL string) InstallationTokenSourceOpt {
+	return func(i *installationTokenSource) {
+		if _, err := i.client.withBaseURL(baseURL); err != nil {
+			i.setConfigErr(err)
+		}
 	}
 }
 
@@ -347,6 +387,20 @@ type installationTokenSource struct {
 	client *githubClient
 	opts   *InstallationTokenOptions
 	skew   time.Duration
+
+	// configErr records the first invalid-configuration error encountered while
+	// applying options (e.g. an unparseable base URL or a nil HTTP client). It is
+	// surfaced by Token() so misconfiguration fails loudly instead of silently
+	// falling back to the public GitHub API.
+	configErr error
+}
+
+// setConfigErr records err as the source's configuration error, keeping the
+// first error so the reported failure is independent of option order.
+func (t *installationTokenSource) setConfigErr(err error) {
+	if t.configErr == nil {
+		t.configErr = err
+	}
 }
 
 // NewInstallationTokenSource creates a GitHub App installation token source.
@@ -384,6 +438,10 @@ func NewInstallationTokenSource(id int64, src oauth2.TokenSource, opts ...Instal
 
 // Token generates a new GitHub App installation token for authenticating as a GitHub App installation.
 func (t *installationTokenSource) Token() (*oauth2.Token, error) {
+	if t.configErr != nil {
+		return nil, t.configErr
+	}
+
 	token, err := t.client.createInstallationToken(t.ctx, t.id, t.opts)
 	if err != nil {
 		return nil, err

auth_test.go

@@ -15,6 +15,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"net/http/httptest"
 	"reflect"
 	"strings"
 	"sync"
@@ -225,8 +226,10 @@ func TestNewApplicationTokenSourceFromSigner(t *testing.T) {
 			wantErr: true,
 		},
 		{
-			name:    "non-rsa signer is rejected",
-			new:     func() (oauth2.TokenSource, error) { return NewApplicationTokenSourceFromSigner(int64(42), &stubSigner{pub: edPub}) },
+			name: "non-rsa signer is rejected",
+			new: func() (oauth2.TokenSource, error) {
+				return NewApplicationTokenSourceFromSigner(int64(42), &stubSigner{pub: edPub})
+			},
 			wantErr: true,
 		},
 		{
@@ -472,21 +475,179 @@ func TestWithEnterpriseURL_InvalidURL(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// Test with invalid URL - error is silently ignored in WithEnterpriseURL
+	// An invalid URL must not silently fall back to the public GitHub API; the
+	// error surfaces on the first Token() call instead.
 	installationTokenSource := NewInstallationTokenSource(
 		1,
 		appSrc,
 		WithEnterpriseURL("ht\ntp://invalid"),
 	)
 
-	// The error is silently ignored in WithEnterpriseURL, so this should still work
-	// but will use the default URL
 	if installationTokenSource == nil {
-		t.Error("Expected non-nil token source")
+		t.Fatal("Expected non-nil token source")
+	}
+
+	if _, err := installationTokenSource.Token(); err == nil {
+		t.Error("Token() error = nil, want error for invalid enterprise URL")
+	}
+}
+
+func TestWithBaseURL_InvalidURL(t *testing.T) {
+	privateKey, err := generatePrivateKey()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	appSrc, err := NewApplicationTokenSource(int64(12345), privateKey)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// An invalid URL must not silently fall back to the public GitHub API; the
+	// error surfaces on the first Token() call instead.
+	installationTokenSource := NewInstallationTokenSource(
+		1,
+		appSrc,
+		WithBaseURL("ht\ntp://invalid"),
+	)
+
+	if installationTokenSource == nil {
+		t.Fatal("Expected non-nil token source")
 	}
 
-	// Test that the token source is created successfully
-	// The error is silently ignored, so the source uses the default URL
+	if _, err := installationTokenSource.Token(); err == nil {
+		t.Error("Token() error = nil, want error for invalid base URL")
+	}
+}
+
+func TestWithBaseURL_DrivesTokenThroughServer(t *testing.T) {
+	now := time.Now().UTC()
+	expiration := now.Add(10 * time.Minute)
+
+	// A bare httptest server URL (e.g. http://127.0.0.1:PORT) is exactly the
+	// case WithEnterpriseURL mishandles: it would append "/api/v3/" and break
+	// the mock. WithBaseURL uses the URL verbatim, so the installation-token
+	// POST lands on the expected path.
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/app/installations/1/access_tokens" {
+			t.Errorf("unexpected request path = %q", r.URL.Path)
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		w.Header().Set("Content-Type", "application/json")
+		_ = json.NewEncoder(w).Encode(InstallationToken{
+			Token:     "mocked-installation-token",
+			ExpiresAt: expiration,
+		})
+	}))
+	defer server.Close()
+
+	privateKey, err := generatePrivateKey()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	appSrc, err := NewApplicationTokenSource(int64(34434), privateKey)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	ts := NewInstallationTokenSource(1, appSrc, WithBaseURL(server.URL))
+
+	got, err := ts.Token()
+	if err != nil {
+		t.Fatalf("Token() error = %v", err)
+	}
+
+	want := &oauth2.Token{
+		AccessToken: "mocked-installation-token",
+		TokenType:   "Bearer",
+		Expiry:      expiration,
+	}
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("Token() = %v, want %v", got, want)
+	}
+}
+
+// TestWithBaseURL_SurvivesHTTPClientOrder guards against the ordering footgun:
+// WithBaseURL is applied BEFORE WithHTTPClient, and the configured base URL must
+// survive the HTTP client swap. If it did not, the request would be sent to the
+// public GitHub API instead of the test server.
+func TestWithBaseURL_SurvivesHTTPClientOrder(t *testing.T) {
+	now := time.Now().UTC()
+	expiration := now.Add(10 * time.Minute)
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/app/installations/7/access_tokens" {
+			t.Errorf("request path = %q, want /app/installations/7/access_tokens", r.URL.Path)
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		w.Header().Set("Content-Type", "application/json")
+		_ = json.NewEncoder(w).Encode(InstallationToken{Token: "ordered-token", ExpiresAt: expiration})
+	}))
+	defer server.Close()
+
+	privateKey, err := generatePrivateKey()
+	if err != nil {
+		t.Fatal(err)
+	}
+	appSrc, err := NewApplicationTokenSource(int64(1), privateKey)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	ts := NewInstallationTokenSource(7, appSrc,
+		WithBaseURL(server.URL),
+		WithHTTPClient(&http.Client{}),
+	)
+
+	got, err := ts.Token()
+	if err != nil {
+		t.Fatalf("Token() error = %v", err)
+	}
+	if got.AccessToken != "ordered-token" {
+		t.Errorf("AccessToken = %q, want %q", got.AccessToken, "ordered-token")
+	}
+}
+
+func TestWithHTTPClient_NilClient(t *testing.T) {
+	privateKey, err := generatePrivateKey()
+	if err != nil {
+		t.Fatal(err)
+	}
+	appSrc, err := NewApplicationTokenSource(int64(1), privateKey)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// A nil HTTP client must not panic; the error surfaces on Token().
+	ts := NewInstallationTokenSource(1, appSrc, WithHTTPClient(nil))
+	if _, err := ts.Token(); err == nil {
+		t.Error("Token() error = nil, want error for nil http client")
+	}
+}
+
+func TestWithHTTPClient_DoesNotMutateCallerClient(t *testing.T) {
+	privateKey, err := generatePrivateKey()
+	if err != nil {
+		t.Fatal(err)
+	}
+	appSrc, err := NewApplicationTokenSource(int64(1), privateKey)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	originalTransport := http.DefaultTransport
+	client := &http.Client{Transport: originalTransport}
+
+	_ = NewInstallationTokenSource(1, appSrc, WithHTTPClient(client))
+
+	// The option must operate on a copy; the caller's client (which may be
+	// shared elsewhere) must keep its original transport.
+	if client.Transport != originalTransport {
+		t.Errorf("WithHTTPClient mutated the caller's client Transport = %T, want it unchanged", client.Transport)
+	}
 }
 
 func Test_installationTokenSource_Token(t *testing.T) {
@@ -1035,9 +1196,9 @@ func TestWithExpirySkew_Wiring(t *testing.T) {
 	}
 
 	tests := []struct {
-		name      string
-		skewOpts  []ApplicationTokenOpt
-		wantSkew  bool // true → *reuseTokenSourceWithSkew, false → delegated
+		name     string
+		skewOpts []ApplicationTokenOpt
+		wantSkew bool // true → *reuseTokenSourceWithSkew, false → delegated
 	}{
 		{
 			name:     "default skew uses wrapper",