diff --git a/pkg/api/api_host_test.go b/pkg/api/api_host_test.go new file mode 100644 index 0000000..3b481b3 --- /dev/null +++ b/pkg/api/api_host_test.go @@ -0,0 +1,424 @@ +package api + +import ( + "context" + "crypto/tls" + "fmt" + "io" + "net" + "net/http" + "net/http/httptest" + "net/http/httputil" + "net/url" + "strings" + "sync" + "testing" + + "github.com/cli/go-gh/v2/internal/testutils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// thirdPartyHost is a host that is neither the canonical API host nor the configured +// api_host, and which must therefore never be sent the auth token. +const thirdPartyHost = "unrelated.example" + +type recordedRequest struct { + method string + path string + rawQuery string + host string + authorization string +} + +type requestRecorder struct { + mu sync.Mutex + requests []recordedRequest +} + +func (recorder *requestRecorder) record(req *http.Request) { + recorder.mu.Lock() + defer recorder.mu.Unlock() + + recorder.requests = append(recorder.requests, recordedRequest{ + method: req.Method, + path: req.URL.Path, + rawQuery: req.URL.RawQuery, + host: req.Host, + authorization: req.Header.Get("Authorization"), + }) +} + +func (recorder *requestRecorder) recordedRequests() []recordedRequest { + recorder.mu.Lock() + defer recorder.mu.Unlock() + + return append([]recordedRequest(nil), recorder.requests...) +} + +func requireRequest(t *testing.T, recorder *requestRecorder, want recordedRequest) { + t.Helper() + + require.Contains(t, recorder.recordedRequests(), want) +} + +type apiHostTestHarness struct { + transport *http.Transport + githubAPIRequests requestRecorder + gatewayRequests requestRecorder + thirdPartyRequests requestRecorder +} + +func newAPIHostTestHarness(t *testing.T, apiHost string) *apiHostTestHarness { + t.Helper() + + harness := &apiHostTestHarness{} + + // First we stand up a TLS server that fakes the real GitHub API. It will record requests and respond with canned responses. + fakeGitHub := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + harness.githubAPIRequests.record(req) + w.Header().Set(contentType, jsonContentType) + switch req.URL.Path { + case "/http-client": + _, _ = io.WriteString(w, `{"message":"http client response"}`) + case "/direct-api-host": + _, _ = io.WriteString(w, `{"message":"direct api host response"}`) + case "/repos/cli/example-repository", "/api/v3/repos/cli/example-repository": + _, _ = io.WriteString(w, `{"name":"example-repository"}`) + case "/repositories", "/api/v3/repositories": + if req.URL.Query().Get("page") == "2" { + _, _ = io.WriteString(w, `[{"name":"example-repository-page-2"}]`) + return + } + w.Header().Set("Link", fmt.Sprintf(`; rel="next"`, apiHost, req.URL.Path)) + _, _ = io.WriteString(w, `[{"name":"example-repository-page-1"}]`) + case "/graphql", "/api/graphql": + _, _ = io.WriteString(w, `{"data":{"viewer":{"login":"hubot"}}}`) + default: + http.NotFound(w, req) + } + })) + t.Cleanup(fakeGitHub.Close) + + target, err := url.Parse(fakeGitHub.URL) + require.NoError(t, err) + + // Then we stand up a proxy server that will forward requests to the fake GitHub server, i.e. the api_host. + // It will also record requests that it receives, so we can assert that it was called when we expected it to be. + proxy := httputil.NewSingleHostReverseProxy(target) + proxy.Transport = fakeGitHub.Client().Transport + + gateway := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + harness.gatewayRequests.record(req) + if req.URL.Path == "/redirect-to-third-party" { + http.Redirect(w, req, "https://"+thirdPartyHost+"/redirected", http.StatusFound) + return + } + proxy.ServeHTTP(w, req) + })) + t.Cleanup(gateway.Close) + + // Finally we stand up a server representing an unrelated third party, which must never + // receive the auth token, whether it is requested directly or arrived at via a redirect. + thirdParty := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + harness.thirdPartyRequests.record(req) + w.Header().Set(contentType, jsonContentType) + _, _ = io.WriteString(w, `{"message":"third party response"}`) + })) + t.Cleanup(thirdParty.Close) + + // To allow us to use fake domain names that are representative, we use a custom transport + // that rewrites the hostnames to point to our test servers. + fakeAddress := fakeGitHub.Listener.Addr().String() + gatewayAddress := gateway.Listener.Addr().String() + dialMap := map[string]string{ + "api.github.com:443": fakeAddress, + "api.example.ghe.com:443": fakeAddress, + "ghes.example.com:443": fakeAddress, + "gw.example.net:443": gatewayAddress, + thirdPartyHost + ":443": thirdParty.Listener.Addr().String(), + } + harness.transport = &http.Transport{ + // We must turn off TLS verification because our test servers use self-signed certs. + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec + DialContext: func(ctx context.Context, network, address string) (net.Conn, error) { + if mapped, ok := dialMap[address]; ok { + address = mapped + } + return (&net.Dialer{}).DialContext(ctx, network, address) + }, + } + t.Cleanup(harness.transport.CloseIdleConnections) + + return harness +} + +func newConfiguredAPIHostTest(t *testing.T, host, apiHost string) (*apiHostTestHarness, ClientOptions) { + t.Helper() + + harness := newAPIHostTestHarness(t, apiHost) + testutils.StubConfig(t, fmt.Sprintf("hosts:\n %s:\n api_host: %q\n", host, apiHost)) + return harness, ClientOptions{ + Host: host, + AuthToken: "test-token", + Transport: harness.transport, + } +} + +func newCanonicalAPIHostTest(t *testing.T, apiHost string) (*apiHostTestHarness, ClientOptions) { + t.Helper() + + harness := newAPIHostTestHarness(t, apiHost) + testutils.StubConfig(t, "") + return harness, ClientOptions{ + Host: "github.com", + AuthToken: "test-token", + Transport: harness.transport, + } +} + +func TestAPIHostRouting(t *testing.T) { + const apiHost = "gw.example.net" + + tests := []struct { + name string + host string + restPath string + pagePath string + graphqlPath string + }{ + { + name: "github.com", + host: "github.com", + restPath: "/repos/cli/example-repository", + pagePath: "/repositories", + graphqlPath: "/graphql", + }, + { + name: "ghe.com tenancy", + host: "example.ghe.com", + restPath: "/repos/cli/example-repository", + pagePath: "/repositories", + graphqlPath: "/graphql", + }, + { + name: "GHES", + host: "ghes.example.com", + restPath: "/api/v3/repos/cli/example-repository", + pagePath: "/api/v3/repositories", + graphqlPath: "/api/graphql", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Run("HTTP client looks up api_host from config", func(t *testing.T) { + harness, opts := newConfiguredAPIHostTest(t, tt.host, apiHost) + httpClient, err := NewHTTPClient(opts) + require.NoError(t, err) + + response, err := httpClient.Get("https://" + apiHost + "/http-client") + require.NoError(t, err) + require.NoError(t, response.Body.Close()) + + requireRequest(t, &harness.gatewayRequests, recordedRequest{ + method: http.MethodGet, + path: "/http-client", + host: apiHost, + authorization: "token test-token", + }) + }) + + t.Run("REST client looks up api_host from config", func(t *testing.T) { + harness, opts := newConfiguredAPIHostTest(t, tt.host, apiHost) + restClient, err := NewRESTClient(opts) + require.NoError(t, err) + + var restResult struct { + Name string `json:"name"` + } + require.NoError(t, restClient.Get("repos/cli/example-repository", &restResult)) + assert.Equal(t, "example-repository", restResult.Name) + + requireRequest(t, &harness.gatewayRequests, recordedRequest{ + method: http.MethodGet, + path: tt.restPath, + host: apiHost, + authorization: "token test-token", + }) + }) + + t.Run("GraphQL client looks up api_host from config", func(t *testing.T) { + harness, opts := newConfiguredAPIHostTest(t, tt.host, apiHost) + graphQLClient, err := NewGraphQLClient(opts) + require.NoError(t, err) + + var graphQLResult struct { + Viewer struct { + Login string `json:"login"` + } `json:"viewer"` + } + require.NoError(t, graphQLClient.Do("query { viewer { login } }", nil, &graphQLResult)) + assert.Equal(t, "hubot", graphQLResult.Viewer.Login) + + requireRequest(t, &harness.gatewayRequests, recordedRequest{ + method: http.MethodPost, + path: tt.graphqlPath, + host: apiHost, + authorization: "token test-token", + }) + }) + + t.Run("correctly provides token for direct api_host request", func(t *testing.T) { + harness, opts := newConfiguredAPIHostTest(t, tt.host, apiHost) + restClient, err := NewRESTClient(opts) + require.NoError(t, err) + + var result struct { + Message string `json:"message"` + } + require.NoError(t, restClient.Get("https://"+apiHost+"/direct-api-host", &result)) + assert.Equal(t, "direct api host response", result.Message) + + requireRequest(t, &harness.gatewayRequests, recordedRequest{ + method: http.MethodGet, + path: "/direct-api-host", + host: apiHost, + authorization: "token test-token", + }) + }) + + t.Run("correctly provides tokens for pagination", func(t *testing.T) { + harness, opts := newConfiguredAPIHostTest(t, tt.host, apiHost) + restClient, err := NewRESTClient(opts) + require.NoError(t, err) + + response, err := restClient.Request(http.MethodGet, "repositories?per_page=1", nil) + require.NoError(t, err) + require.NoError(t, response.Body.Close()) + + nextPageURL := strings.TrimSuffix(strings.TrimPrefix(response.Header.Get("Link"), "<"), `>; rel="next"`) + require.Equal(t, fmt.Sprintf("https://%s%s?page=2", apiHost, tt.pagePath), nextPageURL) + + var nextPageResult []struct { + Name string `json:"name"` + } + require.NoError(t, restClient.Get(nextPageURL, &nextPageResult)) + require.Len(t, nextPageResult, 1) + assert.Equal(t, "example-repository-page-2", nextPageResult[0].Name) + + requireRequest(t, &harness.gatewayRequests, recordedRequest{ + method: http.MethodGet, + path: tt.pagePath, + rawQuery: "page=2", + host: apiHost, + authorization: "token test-token", + }) + }) + + t.Run("does not provide token to an unrelated host", func(t *testing.T) { + harness, opts := newConfiguredAPIHostTest(t, tt.host, apiHost) + restClient, err := NewRESTClient(opts) + require.NoError(t, err) + + var result struct { + Message string `json:"message"` + } + require.NoError(t, restClient.Get("https://"+thirdPartyHost+"/third-party", &result)) + assert.Equal(t, "third party response", result.Message) + + requireRequest(t, &harness.thirdPartyRequests, recordedRequest{ + method: http.MethodGet, + path: "/third-party", + host: thirdPartyHost, + authorization: "", + }) + }) + + t.Run("does not provide token when redirected off the api_host", func(t *testing.T) { + harness, opts := newConfiguredAPIHostTest(t, tt.host, apiHost) + httpClient, err := NewHTTPClient(opts) + require.NoError(t, err) + + response, err := httpClient.Get("https://" + apiHost + "/redirect-to-third-party") + require.NoError(t, err) + require.NoError(t, response.Body.Close()) + + requireRequest(t, &harness.gatewayRequests, recordedRequest{ + method: http.MethodGet, + path: "/redirect-to-third-party", + host: apiHost, + authorization: "token test-token", + }) + requireRequest(t, &harness.thirdPartyRequests, recordedRequest{ + method: http.MethodGet, + path: "/redirected", + host: thirdPartyHost, + authorization: "", + }) + }) + }) + } + + t.Run("no override goes directly to canonical host", func(t *testing.T) { + t.Run("HTTP client uses canonical host", func(t *testing.T) { + harness, opts := newCanonicalAPIHostTest(t, apiHost) + httpClient, err := NewHTTPClient(opts) + require.NoError(t, err) + + response, err := httpClient.Get("https://api.github.com/http-client") + require.NoError(t, err) + require.NoError(t, response.Body.Close()) + + requireRequest(t, &harness.githubAPIRequests, recordedRequest{ + method: http.MethodGet, + path: "/http-client", + host: "api.github.com", + authorization: "token test-token", + }) + assert.Empty(t, harness.gatewayRequests.recordedRequests()) + }) + + t.Run("REST client uses canonical host", func(t *testing.T) { + harness, opts := newCanonicalAPIHostTest(t, apiHost) + restClient, err := NewRESTClient(opts) + require.NoError(t, err) + + var restResult struct { + Name string `json:"name"` + } + require.NoError(t, restClient.Get("repos/cli/example-repository", &restResult)) + assert.Equal(t, "example-repository", restResult.Name) + + requireRequest(t, &harness.githubAPIRequests, recordedRequest{ + method: http.MethodGet, + path: "/repos/cli/example-repository", + host: "api.github.com", + authorization: "token test-token", + }) + assert.Empty(t, harness.gatewayRequests.recordedRequests()) + }) + + t.Run("GraphQL client uses canonical host", func(t *testing.T) { + harness, opts := newCanonicalAPIHostTest(t, apiHost) + graphQLClient, err := NewGraphQLClient(opts) + require.NoError(t, err) + + var graphQLResult struct { + Viewer struct { + Login string `json:"login"` + } `json:"viewer"` + } + require.NoError(t, graphQLClient.Do("query { viewer { login } }", nil, &graphQLResult)) + assert.Equal(t, "hubot", graphQLResult.Viewer.Login) + + requireRequest(t, &harness.githubAPIRequests, recordedRequest{ + method: http.MethodPost, + path: "/graphql", + host: "api.github.com", + authorization: "token test-token", + }) + assert.Empty(t, harness.gatewayRequests.recordedRequests()) + }) + }) +} diff --git a/pkg/api/cache_test.go b/pkg/api/cache_test.go index 5ae9196..5a1a89b 100644 --- a/pkg/api/cache_test.go +++ b/pkg/api/cache_test.go @@ -9,10 +9,13 @@ import ( "testing" "time" + "github.com/cli/go-gh/v2/internal/testutils" "github.com/stretchr/testify/assert" ) func TestCacheResponse(t *testing.T) { + testutils.StubConfig(t, "") + counter := 0 fakeHTTP := tripper{ roundTrip: func(req *http.Request) (*http.Response, error) { @@ -97,6 +100,8 @@ func TestCacheResponse(t *testing.T) { } func TestCacheResponseRequestCacheOptions(t *testing.T) { + testutils.StubConfig(t, "") + counter := 0 fakeHTTP := tripper{ roundTrip: func(req *http.Request) (*http.Response, error) { diff --git a/pkg/api/client_options.go b/pkg/api/client_options.go index 3464aac..9e0f93e 100644 --- a/pkg/api/client_options.go +++ b/pkg/api/client_options.go @@ -5,6 +5,8 @@ import ( "fmt" "io" "net/http" + "net/url" + "strings" "time" "github.com/cli/go-gh/v2/pkg/auth" @@ -13,6 +15,19 @@ import ( // ClientOptions holds available options to configure API clients. type ClientOptions struct { + // APIHost overrides the hostname that REST and GraphQL API requests are + // sent to while authentication continues to use Host. It must be a bare + // hostname, without a scheme or port, for example "api.example.com". + // + // When empty, the api_host value configured for Host in gh config is used, + // if there is one. Client construction fails when the resulting value, + // whether set here or read from gh config, is not a bare hostname. + // + // The auth token is sent to APIHost as well as to Host, so it must be a + // trusted endpoint. Absolute URLs passed to RESTClient methods are + // requested as given and are never rewritten to APIHost. + APIHost string + // AuthToken is the authorization token that will be used // to authenticate against API endpoints. AuthToken string @@ -25,6 +40,16 @@ type ClientOptions struct { // Default is 24 hours. CacheTTL time.Duration + // CheckRedirect specifies the policy for handling redirects, matching the + // field of the same name on http.Client. If nil, the default policy of + // following up to 10 redirects is used. + // + // This matters for requests where following a redirect silently changes + // the meaning of the request. Go's default policy converts a DELETE into a + // GET when it follows a 301, so a caller deleting a renamed resource can + // receive a success response having deleted nothing. + CheckRedirect func(*http.Request, []*http.Request) error + // EnableCache specifies if API requests will be cached or not. // Default is no caching. EnableCache bool @@ -104,3 +129,36 @@ func resolveOptions(opts ClientOptions) (ClientOptions, error) { } return opts, nil } + +func resolveAPIHost(opts ClientOptions) (ClientOptions, error) { + if opts.APIHost == "" { + configuredAPIHost, ok := apiHost(opts.Host) + if !ok { + return opts, nil + } + + opts.APIHost = configuredAPIHost + } + + if !validAPIHost(opts.APIHost) { + return ClientOptions{}, fmt.Errorf( + `invalid api_host for %s: %q must be a hostname without a scheme or port, for example "api.example.com"`, + opts.Host, + opts.APIHost, + ) + } + + return opts, nil +} + +func validAPIHost(apiHost string) bool { + // A bare hostname has no surrounding whitespace, and no port or IPv6 literal. + if apiHost == "" || strings.TrimSpace(apiHost) != apiHost || strings.Contains(apiHost, ":") { + return false + } + + // Parsing as a scheme relative URL rejects userinfo, paths, queries and fragments, + // since any of those make the parsed host differ from the input. + u, err := url.Parse("//" + apiHost) + return err == nil && u.Host == apiHost && u.Hostname() != "" +} diff --git a/pkg/api/client_options_test.go b/pkg/api/client_options_test.go index 35cfb34..ee3cac6 100644 --- a/pkg/api/client_options_test.go +++ b/pkg/api/client_options_test.go @@ -1,11 +1,14 @@ package api import ( + "errors" "net/http" "testing" "github.com/cli/go-gh/v2/internal/testutils" + "github.com/cli/go-gh/v2/pkg/config" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestResolveOptions(t *testing.T) { @@ -150,6 +153,220 @@ func TestOptionsNeedResolution(t *testing.T) { } } +func TestValidAPIHost(t *testing.T) { + tests := []struct { + name string + value string + valid bool + }{ + {name: "bare hostname", value: "gw.example.net", valid: true}, + {name: "mixed-case hostname", value: "GW.Example.NET", valid: true}, + {name: "empty"}, + {name: "scheme", value: "https://gw.example.net"}, + {name: "path", value: "gw.example.net/api"}, + {name: "query", value: "gw.example.net?trace=1"}, + {name: "fragment", value: "gw.example.net#fragment"}, + {name: "userinfo", value: "user@gw.example.net"}, + {name: "leading whitespace", value: " gw.example.net"}, + {name: "trailing whitespace", value: "gw.example.net "}, + {name: "empty host", value: ":8443"}, + {name: "missing port", value: "gw.example.net:"}, + {name: "non-numeric port", value: "gw.example.net:http"}, + {name: "minimum port", value: "gw.example.net:1"}, + {name: "typical port", value: "gw.example.net:8443"}, + {name: "maximum port", value: "gw.example.net:65535"}, + {name: "zero port", value: "gw.example.net:0"}, + {name: "out-of-range port", value: "gw.example.net:65536"}, + {name: "IPv6 literal", value: "[::1]"}, + {name: "bare IPv6 address", value: "::1"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.valid, validAPIHost(tt.value)) + }) + } +} + +func TestResolveAPIHost(t *testing.T) { + tests := []struct { + name string + host string + apiHost string + config string + want string + wantErr string + }{ + { + name: "explicit option takes precedence", + host: "example.ghe.com", + apiHost: "explicit.example.net", + config: "hosts:\n example.ghe.com:\n api_host: configured.example.net\n", + want: "explicit.example.net", + }, + { + name: "config fills an empty option", + host: "example.ghe.com", + config: "hosts:\n example.ghe.com:\n api_host: configured.example.net\n", + want: "configured.example.net", + }, + { + name: "no option or config stays empty", + host: "example.ghe.com", + }, + { + name: "invalid explicit option fails", + host: "example.ghe.com", + apiHost: "https://explicit.example.net", + wantErr: `invalid api_host for example.ghe.com: "https://explicit.example.net" must be a hostname without a scheme or port, for example "api.example.com"`, + }, + { + name: "explicit port fails", + host: "example.ghe.com", + apiHost: "explicit.example.net:8443", + wantErr: `invalid api_host for example.ghe.com: "explicit.example.net:8443" must be a hostname without a scheme or port, for example "api.example.com"`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + testutils.StubConfig(t, tt.config) + opts := ClientOptions{Host: tt.host, APIHost: tt.apiHost} + + got, err := resolveAPIHost(opts) + + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + } else { + require.NoError(t, err) + } + assert.Equal(t, tt.want, got.APIHost) + }) + } +} + +func TestAPIClientConstructorsRejectInvalidAPIHost(t *testing.T) { + constructors := []struct { + name string + construct func(ClientOptions) error + }{ + { + name: "HTTP", + construct: func(opts ClientOptions) error { + _, err := NewHTTPClient(opts) + return err + }, + }, + { + name: "REST", + construct: func(opts ClientOptions) error { + _, err := NewRESTClient(opts) + return err + }, + }, + { + name: "GraphQL", + construct: func(opts ClientOptions) error { + _, err := NewGraphQLClient(opts) + return err + }, + }, + } + + for _, constructor := range constructors { + t.Run(constructor.name+" configured value", func(t *testing.T) { + testutils.StubConfig(t, "hosts:\n example.ghe.com:\n api_host: gw.example.net:8443\n") + opts := ClientOptions{ + Host: "example.ghe.com", + AuthToken: "token", + Transport: http.DefaultTransport, + } + + err := constructor.construct(opts) + + require.EqualError( + t, + err, + `invalid api_host for example.ghe.com: "gw.example.net:8443" must be a hostname without a scheme or port, for example "api.example.com"`, + ) + }) + + t.Run(constructor.name+" explicit value", func(t *testing.T) { + testutils.StubConfig(t, "") + opts := ClientOptions{ + Host: "example.ghe.com", + APIHost: "gw.example.net:8443", + AuthToken: "token", + Transport: http.DefaultTransport, + } + + err := constructor.construct(opts) + + require.EqualError( + t, + err, + `invalid api_host for example.ghe.com: "gw.example.net:8443" must be a hostname without a scheme or port, for example "api.example.com"`, + ) + }) + } +} + +func TestAPIClientConstructorsIgnoreConfigReadErrors(t *testing.T) { + oldRead := config.Read + config.Read = func(*config.Config) (*config.Config, error) { + return nil, &config.InvalidConfigFileError{ + Path: "hosts.yml", + Err: errors.New("invalid YAML"), + } + } + t.Cleanup(func() { + config.Read = oldRead + }) + + opts := ClientOptions{ + Host: "example.ghe.com", + AuthToken: "token", + Transport: http.DefaultTransport, + } + + resolved, err := resolveAPIHost(opts) + require.NoError(t, err) + assert.Equal(t, opts, resolved) + + constructors := []struct { + name string + construct func(ClientOptions) error + }{ + { + name: "HTTP", + construct: func(opts ClientOptions) error { + _, err := NewHTTPClient(opts) + return err + }, + }, + { + name: "REST", + construct: func(opts ClientOptions) error { + _, err := NewRESTClient(opts) + return err + }, + }, + { + name: "GraphQL", + construct: func(opts ClientOptions) error { + _, err := NewGraphQLClient(opts) + return err + }, + }, + } + + for _, constructor := range constructors { + t.Run(constructor.name, func(t *testing.T) { + require.NoError(t, constructor.construct(opts)) + }) + } +} + func testConfig() string { return ` hosts: diff --git a/pkg/api/graphql_client.go b/pkg/api/graphql_client.go index a985f70..d3e38fa 100644 --- a/pkg/api/graphql_client.go +++ b/pkg/api/graphql_client.go @@ -32,20 +32,28 @@ func DefaultGraphQLClient() (*GraphQLClient, error) { // and unix domain socket are resolved from the gh environment configuration. // These behaviors can be overridden using the opts argument. func NewGraphQLClient(opts ClientOptions) (*GraphQLClient, error) { + var err error if optionsNeedResolution(opts) { - var err error opts, err = resolveOptions(opts) if err != nil { return nil, err } } + opts, err = resolveAPIHost(opts) + if err != nil { + return nil, err + } + httpClient, err := NewHTTPClient(opts) if err != nil { return nil, err } endpoint := graphQLEndpoint(opts.Host) + if opts.APIHost != "" { + endpoint = swapHost(endpoint, opts.APIHost) + } return &GraphQLClient{ client: graphql.NewClient(endpoint, httpClient), diff --git a/pkg/api/graphql_client_test.go b/pkg/api/graphql_client_test.go index 15cdc7a..6b7eadf 100644 --- a/pkg/api/graphql_client_test.go +++ b/pkg/api/graphql_client_test.go @@ -104,6 +104,8 @@ func TestGraphQLClientMutateError(t *testing.T) { } func TestGraphQLClientDo(t *testing.T) { + testutils.StubConfig(t, "") + tests := []struct { name string host string @@ -214,6 +216,8 @@ func TestGraphQLClientDo(t *testing.T) { } func TestGraphQLClientDoWithContext(t *testing.T) { + testutils.StubConfig(t, "") + tests := []struct { name string wantErrMsg string @@ -274,37 +278,46 @@ func TestGraphQLEndpoint(t *testing.T) { host string wantEndpoint string }{ - { - name: "github", - host: "github.com", - wantEndpoint: "https://api.github.com/graphql", - }, - { - name: "localhost", - host: "github.localhost", - wantEndpoint: "http://api.github.localhost/graphql", - }, - { - name: "garage", - host: "garage.github.com", - wantEndpoint: "https://garage.github.com/api/graphql", - }, - { - name: "enterprise", - host: "enterprise.com", - wantEndpoint: "https://enterprise.com/api/graphql", - }, - { - name: "tenant", - host: "tenant.ghe.com", - wantEndpoint: "https://api.tenant.ghe.com/graphql", - }, + {name: "github", host: "github.com", wantEndpoint: "https://api.github.com/graphql"}, + {name: "localhost", host: "github.localhost", wantEndpoint: "http://api.github.localhost/graphql"}, + {name: "garage", host: "garage.github.com", wantEndpoint: "https://garage.github.com/api/graphql"}, + {name: "enterprise", host: "enterprise.com", wantEndpoint: "https://enterprise.com/api/graphql"}, + {name: "tenant", host: "tenant.ghe.com", wantEndpoint: "https://api.tenant.ghe.com/graphql"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - endpoint := graphQLEndpoint(tt.host) - assert.Equal(t, tt.wantEndpoint, endpoint) + assert.Equal(t, tt.wantEndpoint, graphQLEndpoint(tt.host)) + }) + } +} + +func TestNewGraphQLClientAPIHostEndpoint(t *testing.T) { + tests := []struct { + name string + host string + wantEndpoint string + }{ + {name: "github", host: "github.com", wantEndpoint: "https://gw.example.net/graphql"}, + {name: "localhost preserves http", host: "github.localhost", wantEndpoint: "http://gw.example.net/graphql"}, + {name: "garage", host: "garage.github.com", wantEndpoint: "https://gw.example.net/api/graphql"}, + {name: "enterprise", host: "enterprise.com", wantEndpoint: "https://gw.example.net/api/graphql"}, + {name: "tenant", host: "tenant.ghe.com", wantEndpoint: "https://gw.example.net/graphql"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + testutils.StubConfig(t, "") + + client, err := NewGraphQLClient(ClientOptions{ + Host: tt.host, + APIHost: "gw.example.net", + AuthToken: "token", + Transport: http.DefaultTransport, + }) + + assert.NoError(t, err) + assert.Equal(t, tt.wantEndpoint, client.host) }) } } diff --git a/pkg/api/host.go b/pkg/api/host.go new file mode 100644 index 0000000..a8c9545 --- /dev/null +++ b/pkg/api/host.go @@ -0,0 +1,27 @@ +package api + +import ( + "github.com/cli/go-gh/v2/pkg/auth" + "github.com/cli/go-gh/v2/pkg/config" +) + +const ( + hostsKey = "hosts" + apiHostKey = "api_host" +) + +// apiHost returns the api_host value configured for host in hosts.yml. +// The boolean reports whether a non-empty value was found. The value is not validated. +func apiHost(host string) (string, bool) { + cfg, err := config.Read(nil) + if err != nil || cfg == nil { + return "", false + } + + normalizedHost := auth.NormalizeHostname(host) + configuredAPIHost, err := cfg.Get([]string{hostsKey, normalizedHost, apiHostKey}) + if err != nil || configuredAPIHost == "" { + return "", false + } + return configuredAPIHost, true +} diff --git a/pkg/api/host_test.go b/pkg/api/host_test.go new file mode 100644 index 0000000..69b1259 --- /dev/null +++ b/pkg/api/host_test.go @@ -0,0 +1,88 @@ +package api + +import ( + "testing" + + "github.com/cli/go-gh/v2/internal/testutils" + "github.com/stretchr/testify/assert" +) + +func TestAPIHost(t *testing.T) { + tests := []struct { + name string + host string + config string + want string + wantOK bool + }{ + { + name: "missing api_host", + host: "example.ghe.com", + config: ` +hosts: + example.ghe.com: + oauth_token: token +`, + }, + { + name: "null api_host", + host: "example.ghe.com", + config: ` +hosts: + example.ghe.com: + api_host: +`, + }, + { + name: "empty api_host", + host: "example.ghe.com", + config: ` +hosts: + example.ghe.com: + api_host: "" +`, + }, + { + name: "host absent from config", + host: "other.ghe.com", + config: ` +hosts: + example.ghe.com: + api_host: gw.example.net +`, + }, + { + name: "configured value", + host: "example.ghe.com", + config: ` +hosts: + example.ghe.com: + api_host: gw.example.net +`, + want: "gw.example.net", + wantOK: true, + }, + { + name: "normalizes canonical host for lookup", + host: "Example.ghe.com", + config: ` +hosts: + example.ghe.com: + api_host: GW.Example.NET +`, + want: "GW.Example.NET", + wantOK: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + testutils.StubConfig(t, tt.config) + + got, ok := apiHost(tt.host) + + assert.Equal(t, tt.want, got) + assert.Equal(t, tt.wantOK, ok) + }) + } +} diff --git a/pkg/api/http_client.go b/pkg/api/http_client.go index c2f0d79..97effea 100644 --- a/pkg/api/http_client.go +++ b/pkg/api/http_client.go @@ -5,6 +5,7 @@ import ( "io" "net" "net/http" + "net/url" "os" "regexp" "runtime/debug" @@ -50,14 +51,19 @@ func DefaultHTTPClient() (*http.Client, error) { // This is to protect against the case where tokens could be sent to an arbitrary // host. func NewHTTPClient(opts ClientOptions) (*http.Client, error) { + var err error if optionsNeedResolution(opts) { - var err error opts, err = resolveOptions(opts) if err != nil { return nil, err } } + opts, err = resolveAPIHost(opts) + if err != nil { + return nil, err + } + transport := http.DefaultTransport if opts.UnixDomainSocket != "" { @@ -116,9 +122,9 @@ func NewHTTPClient(opts ClientOptions) (*http.Client, error) { if !opts.SkipDefaultHeaders { setDefaultHeaders(opts.Headers) } - transport = newHeaderRoundTripper(opts.Host, opts.AuthToken, opts.Headers, transport) + transport = newHeaderRoundTripper(opts.Host, opts.APIHost, opts.AuthToken, opts.Headers, transport) - return &http.Client{Transport: transport, Timeout: opts.Timeout}, nil + return &http.Client{Transport: transport, Timeout: opts.Timeout, CheckRedirect: opts.CheckRedirect}, nil } func inspectableMIMEType(t string) bool { @@ -133,6 +139,25 @@ func isSameDomain(requestHost, domain string) bool { return (requestHost == domain) || strings.HasSuffix(requestHost, "."+domain) } +// isAPIHost reports whether requestHost is the configured API host override. +// An unset override matches nothing, including an empty request host. +func isAPIHost(requestHost, apiHost string) bool { + return apiHost != "" && strings.EqualFold(requestHost, apiHost) +} + +// swapHost returns rawURL with its host replaced by apiHost. +func swapHost(rawURL, apiHost string) string { + if apiHost == "" { + return rawURL + } + u, err := url.Parse(rawURL) + if err != nil { + return rawURL + } + u.Host = apiHost + return u.String() +} + func isGarage(host string) bool { return strings.EqualFold(host, "garage.github.com") } @@ -140,6 +165,7 @@ func isGarage(host string) bool { type headerRoundTripper struct { headers map[string]string host string + apiHost string rt http.RoundTripper } @@ -177,14 +203,19 @@ func setDefaultHeaders(headers map[string]string) { } } -func newHeaderRoundTripper(host string, authToken string, headers map[string]string, rt http.RoundTripper) http.RoundTripper { +func newHeaderRoundTripper(host string, apiHost string, authToken string, headers map[string]string, rt http.RoundTripper) http.RoundTripper { if _, ok := headers[authorization]; !ok && authToken != "" { headers[authorization] = fmt.Sprintf("token %s", authToken) } if len(headers) == 0 { return rt } - return headerRoundTripper{host: host, headers: headers, rt: rt} + return headerRoundTripper{ + host: host, + apiHost: apiHost, + headers: headers, + rt: rt, + } } func (hrt headerRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { @@ -192,7 +223,10 @@ func (hrt headerRoundTripper) RoundTrip(req *http.Request) (*http.Response, erro // If the authorization header has been set and the request // host is not in the same domain that was specified in the ClientOptions // then do not add the authorization header to the request. - if k == authorization && !isSameDomain(req.URL.Hostname(), hrt.host) { + requestHost := req.URL.Hostname() + if k == authorization && + !isSameDomain(requestHost, hrt.host) && + !isAPIHost(requestHost, hrt.apiHost) { continue } diff --git a/pkg/api/http_client_test.go b/pkg/api/http_client_test.go index e7cb4df..a4d0c8c 100644 --- a/pkg/api/http_client_test.go +++ b/pkg/api/http_client_test.go @@ -32,7 +32,49 @@ func TestHTTPClient(t *testing.T) { assert.Equal(t, 200, res.StatusCode) } +func TestIsAPIHost(t *testing.T) { + tests := []struct { + name string + requestHost string + apiHost string + want bool + }{ + { + name: "matches exact host", + requestHost: "gateway.example", + apiHost: "gateway.example", + want: true, + }, + { + name: "matches host ignoring case", + requestHost: "GATEWAY.example", + apiHost: "gateway.example", + want: true, + }, + { + name: "unset override matches nothing", + requestHost: "", + apiHost: "", + want: false, + }, + { + name: "empty request host does not match configured override", + requestHost: "", + apiHost: "gateway.example", + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, isAPIHost(tt.requestHost, tt.apiHost)) + }) + } +} + func TestNewHTTPClient(t *testing.T) { + testutils.StubConfig(t, "") + reflectHTTP := tripper{ roundTrip: func(req *http.Request) (*http.Response, error) { header := req.Header.Clone() @@ -50,6 +92,8 @@ func TestNewHTTPClient(t *testing.T) { enableLog bool log *bytes.Buffer host string + apiHost string + reqURL string headers map[string]string skipHeaders bool wantHeaders http.Header @@ -116,6 +160,79 @@ func TestNewHTTPClient(t *testing.T) { host: "TeSt.CoM", wantHeaders: defaultHeaders(), }, + { + name: "adds authorization for a canonical subdomain", + host: "test.com", + reqURL: "https://api.test.com", + wantHeaders: defaultHeaders(), + }, + { + name: "adds authorization for exact API host", + host: "test.com", + apiHost: "gateway.example", + reqURL: "https://gateway.example", + wantHeaders: defaultHeaders(), + }, + { + name: "adds authorization for case-differing API host", + host: "test.com", + apiHost: "gateway.example", + reqURL: "https://GATEWAY.example", + wantHeaders: defaultHeaders(), + }, + { + name: "withholds authorization from an API host subdomain", + host: "test.com", + apiHost: "gateway.example", + reqURL: "https://sub.gateway.example", + wantHeaders: func() http.Header { + h := defaultHeaders() + h.Del(authorization) + return h + }(), + }, + { + name: "withholds authorization from unrelated host", + host: "test.com", + apiHost: "gateway.example", + reqURL: "https://unrelated.example", + wantHeaders: func() http.Header { + h := defaultHeaders() + h.Del(authorization) + return h + }(), + }, + { + name: "withholds authorization from a port-only empty hostname without override", + host: "test.com", + reqURL: "http://:1234/x", + wantHeaders: func() http.Header { + h := defaultHeaders() + h.Del(authorization) + return h + }(), + }, + { + name: "withholds authorization from an empty hostname without override", + host: "test.com", + reqURL: "http:///x", + wantHeaders: func() http.Header { + h := defaultHeaders() + h.Del(authorization) + return h + }(), + }, + { + name: "withholds authorization from an empty hostname with override configured", + host: "test.com", + apiHost: "gateway.example", + reqURL: "http://:1234/x", + wantHeaders: func() http.Header { + h := defaultHeaders() + h.Del(authorization) + return h + }(), + }, { name: "skips default headers", skipHeaders: true, @@ -136,8 +253,12 @@ func TestNewHTTPClient(t *testing.T) { if tt.host == "" { tt.host = "test.com" } + if tt.reqURL == "" { + tt.reqURL = "https://test.com" + } opts := ClientOptions{ Host: tt.host, + APIHost: tt.apiHost, AuthToken: "oauth_token", Headers: tt.headers, SkipDefaultHeaders: tt.skipHeaders, @@ -148,7 +269,7 @@ func TestNewHTTPClient(t *testing.T) { opts.Log = tt.log } client, _ := NewHTTPClient(opts) - res, err := client.Get("https://test.com") + res, err := client.Get(tt.reqURL) assert.NoError(t, err) assert.Equal(t, tt.wantHeaders, res.Header) if tt.enableLog { @@ -158,6 +279,74 @@ func TestNewHTTPClient(t *testing.T) { } } +func TestNewHTTPClientCheckRedirect(t *testing.T) { + // Redirect handling belongs to http.Client rather than the transport, so a + // stub transport still exercises the real policy: the client asks it for the + // redirected request only if the policy allows the redirect. + newRecordingTransport := func(methods *[]string) tripper { + return tripper{ + roundTrip: func(req *http.Request) (*http.Response, error) { + *methods = append(*methods, req.Method) + if len(*methods) == 1 { + return &http.Response{ + StatusCode: http.StatusMovedPermanently, + Header: http.Header{"Location": []string{"https://api.github.com/repos/OWNER/NEW"}}, + Body: io.NopCloser(bytes.NewBufferString("")), + }, nil + } + return &http.Response{ + StatusCode: http.StatusNoContent, + Body: io.NopCloser(bytes.NewBufferString("")), + }, nil + }, + } + } + + t.Run("follows redirects by default, downgrading DELETE to GET", func(t *testing.T) { + var methods []string + client, err := NewHTTPClient(ClientOptions{ + Host: "github.com", + AuthToken: "oauth_token", + Transport: newRecordingTransport(&methods), + }) + assert.NoError(t, err) + + req, err := http.NewRequest(http.MethodDelete, "https://api.github.com/repos/OWNER/OLD", nil) + assert.NoError(t, err) + res, err := client.Do(req) + assert.NoError(t, err) + defer res.Body.Close() + + // This is the behaviour that makes the option necessary. Go turns the + // DELETE into a GET when it follows the redirect, so the caller is told + // the request succeeded while nothing was deleted. + assert.Equal(t, []string{http.MethodDelete, http.MethodGet}, methods) + assert.Equal(t, http.StatusNoContent, res.StatusCode) + }) + + t.Run("honours a CheckRedirect that stops at the redirect", func(t *testing.T) { + var methods []string + client, err := NewHTTPClient(ClientOptions{ + Host: "github.com", + AuthToken: "oauth_token", + Transport: newRecordingTransport(&methods), + CheckRedirect: func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + }, + }) + assert.NoError(t, err) + + req, err := http.NewRequest(http.MethodDelete, "https://api.github.com/repos/OWNER/OLD", nil) + assert.NoError(t, err) + res, err := client.Do(req) + assert.NoError(t, err) + defer res.Body.Close() + + assert.Equal(t, []string{http.MethodDelete}, methods) + assert.Equal(t, http.StatusMovedPermanently, res.StatusCode) + }) +} + type tripper struct { roundTrip func(*http.Request) (*http.Response, error) } diff --git a/pkg/api/rest_client.go b/pkg/api/rest_client.go index 097e637..b233502 100644 --- a/pkg/api/rest_client.go +++ b/pkg/api/rest_client.go @@ -14,8 +14,8 @@ import ( // RESTClient wraps methods for the different types of // API requests that are supported by the server. type RESTClient struct { - client *http.Client - host string + client *http.Client + endpoint string } func DefaultRESTClient() (*RESTClient, error) { @@ -28,22 +28,32 @@ func DefaultRESTClient() (*RESTClient, error) { // and unix domain socket are resolved from the gh environment configuration. // These behaviors can be overridden using the opts argument. func NewRESTClient(opts ClientOptions) (*RESTClient, error) { + var err error if optionsNeedResolution(opts) { - var err error opts, err = resolveOptions(opts) if err != nil { return nil, err } } + opts, err = resolveAPIHost(opts) + if err != nil { + return nil, err + } + client, err := NewHTTPClient(opts) if err != nil { return nil, err } + endpoint := restPrefix(opts.Host) + if opts.APIHost != "" { + endpoint = swapHost(endpoint, opts.APIHost) + } + return &RESTClient{ - client: client, - host: opts.Host, + client: client, + endpoint: endpoint, }, nil } @@ -52,7 +62,7 @@ func NewRESTClient(opts ClientOptions) (*RESTClient, error) { // The response is returned rather than being populated // into a response argument. func (c *RESTClient) RequestWithContext(ctx context.Context, method string, path string, body io.Reader) (*http.Response, error) { - url := restURL(c.host, path) + url := restURL(c.endpoint, path) req, err := http.NewRequestWithContext(ctx, method, url, body) if err != nil { return nil, err @@ -81,7 +91,7 @@ func (c *RESTClient) Request(method string, path string, body io.Reader) (*http. // specified path with the specified body. // The response is populated into the response argument. func (c *RESTClient) DoWithContext(ctx context.Context, method string, path string, body io.Reader, response interface{}) error { - url := restURL(c.host, path) + url := restURL(c.endpoint, path) req, err := http.NewRequestWithContext(ctx, method, url, body) if err != nil { return err @@ -156,11 +166,11 @@ func (c *RESTClient) Put(path string, body io.Reader, resp interface{}) error { return c.Do(http.MethodPut, path, body, resp) } -func restURL(hostname string, pathOrURL string) string { +func restURL(endpoint string, pathOrURL string) string { if strings.HasPrefix(pathOrURL, "https://") || strings.HasPrefix(pathOrURL, "http://") { return pathOrURL } - return restPrefix(hostname) + pathOrURL + return endpoint + pathOrURL } func restPrefix(hostname string) string { diff --git a/pkg/api/rest_client_test.go b/pkg/api/rest_client_test.go index 905888e..ebd5cf7 100644 --- a/pkg/api/rest_client_test.go +++ b/pkg/api/rest_client_test.go @@ -34,6 +34,8 @@ func TestRESTClient(t *testing.T) { } func TestRESTClientRequest(t *testing.T) { + testutils.StubConfig(t, "") + tests := []struct { name string host string @@ -150,6 +152,8 @@ func TestRESTClientRequest(t *testing.T) { } func TestRESTClientDo(t *testing.T) { + testutils.StubConfig(t, "") + tests := []struct { name string host string @@ -255,6 +259,7 @@ func TestRESTClientDo(t *testing.T) { } func TestRESTClientDelete(t *testing.T) { + testutils.StubConfig(t, "") t.Cleanup(gock.Off) gock.New("https://api.github.com"). Delete("/some/path/here"). @@ -271,6 +276,7 @@ func TestRESTClientDelete(t *testing.T) { } func TestRESTClientGet(t *testing.T) { + testutils.StubConfig(t, "") t.Cleanup(gock.Off) gock.New("https://api.github.com"). Get("/some/path/here"). @@ -287,6 +293,7 @@ func TestRESTClientGet(t *testing.T) { } func TestRESTClientPatch(t *testing.T) { + testutils.StubConfig(t, "") t.Cleanup(gock.Off) gock.New("https://api.github.com"). Patch("/some/path/here"). @@ -305,6 +312,7 @@ func TestRESTClientPatch(t *testing.T) { } func TestRESTClientPatchStatus205(t *testing.T) { + testutils.StubConfig(t, "") t.Cleanup(gock.Off) gock.New("https://api.github.com"). Patch("/some/path/here"). @@ -323,6 +331,7 @@ func TestRESTClientPatchStatus205(t *testing.T) { } func TestRESTClientPost(t *testing.T) { + testutils.StubConfig(t, "") t.Cleanup(gock.Off) gock.New("https://api.github.com"). Post("/some/path/here"). @@ -341,6 +350,7 @@ func TestRESTClientPost(t *testing.T) { } func TestRESTClientPut(t *testing.T) { + testutils.StubConfig(t, "") t.Cleanup(gock.Off) gock.New("https://api.github.com"). Put("/some/path/here"). @@ -359,6 +369,8 @@ func TestRESTClientPut(t *testing.T) { } func TestRESTClientDoWithContext(t *testing.T) { + testutils.StubConfig(t, "") + tests := []struct { name string wantErrMsg string @@ -414,6 +426,8 @@ func TestRESTClientDoWithContext(t *testing.T) { } func TestRESTClientRequestWithContext(t *testing.T) { + testutils.StubConfig(t, "") + tests := []struct { name string wantErrMsg string @@ -472,38 +486,87 @@ func TestRestPrefix(t *testing.T) { name string host string wantEndpoint string + }{ + {name: "github", host: "github.com", wantEndpoint: "https://api.github.com/"}, + {name: "localhost", host: "github.localhost", wantEndpoint: "http://api.github.localhost/"}, + {name: "garage", host: "garage.github.com", wantEndpoint: "https://garage.github.com/api/v3/"}, + {name: "enterprise", host: "enterprise.com", wantEndpoint: "https://enterprise.com/api/v3/"}, + {name: "tenant", host: "tenant.ghe.com", wantEndpoint: "https://api.tenant.ghe.com/"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.wantEndpoint, restPrefix(tt.host)) + }) + } +} + +func TestNewRESTClientAPIHostEndpoint(t *testing.T) { + tests := []struct { + name string + host string + wantEndpoint string + }{ + {name: "github", host: "github.com", wantEndpoint: "https://gw.example.net/"}, + {name: "localhost preserves http", host: "github.localhost", wantEndpoint: "http://gw.example.net/"}, + {name: "garage", host: "garage.github.com", wantEndpoint: "https://gw.example.net/api/v3/"}, + {name: "enterprise", host: "enterprise.com", wantEndpoint: "https://gw.example.net/api/v3/"}, + {name: "tenant", host: "tenant.ghe.com", wantEndpoint: "https://gw.example.net/"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + testutils.StubConfig(t, "") + + client, err := NewRESTClient(ClientOptions{ + Host: tt.host, + APIHost: "gw.example.net", + AuthToken: "token", + Transport: http.DefaultTransport, + }) + + assert.NoError(t, err) + assert.Equal(t, tt.wantEndpoint, client.endpoint) + }) + } +} + +func TestRestURL(t *testing.T) { + tests := []struct { + name string + endpoint string + path string + wantURL string }{ { - name: "github", - host: "github.com", - wantEndpoint: "https://api.github.com/", - }, - { - name: "localhost", - host: "github.localhost", - wantEndpoint: "http://api.github.localhost/", + name: "joins a relative path to an endpoint", + endpoint: "https://gw.example.net/api/v3/", + path: "repos/o/r", + wantURL: "https://gw.example.net/api/v3/repos/o/r", }, { - name: "garage", - host: "garage.github.com", - wantEndpoint: "https://garage.github.com/api/v3/", + name: "leaves a canonical absolute URL unchanged", + endpoint: "https://gw.example.net/", + path: "https://api.github.com/repositories/1/issues?page=2", + wantURL: "https://api.github.com/repositories/1/issues?page=2", }, { - name: "enterprise", - host: "enterprise.com", - wantEndpoint: "https://enterprise.com/api/v3/", + name: "leaves an asset absolute URL unchanged", + endpoint: "https://gw.example.net/", + path: "https://github.com/o/r/releases/download/v1/asset.zip", + wantURL: "https://github.com/o/r/releases/download/v1/asset.zip", }, { - name: "tenant", - host: "tenant.ghe.com", - wantEndpoint: "https://api.tenant.ghe.com/", + name: "leaves an http absolute URL unchanged", + endpoint: "https://gw.example.net/", + path: "http://downloads.example.net/asset.zip", + wantURL: "http://downloads.example.net/asset.zip", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - endpoint := restPrefix(tt.host) - assert.Equal(t, tt.wantEndpoint, endpoint) + assert.Equal(t, tt.wantURL, restURL(tt.endpoint, tt.path)) }) } }