Fix GHEC with Data Residency (*.ghe.com) base URL (#4777)
* use ghinstallation.NewFromAppsTransport() instead of ghinstallation.New() to ensure the enterprise BaseURL is properly inherited * update the endpoint for ghec with data residency * addressed bugbot comments
This commit is contained in:
@@ -18,6 +18,8 @@ import (
|
||||
const (
|
||||
cloudV3Endpoint = "https://api.github.com"
|
||||
cloudGraphqlEndpoint = "https://api.github.com/graphql" // https://docs.github.com/en/graphql/guides/forming-calls-with-graphql#the-graphql-endpoint
|
||||
|
||||
gheCloudSuffix = ".ghe.com"
|
||||
)
|
||||
|
||||
// Connector abstracts over the authenticated ways to interact with GitHub: cloning and API operations.
|
||||
@@ -34,6 +36,81 @@ type Connector interface {
|
||||
Clone(ctx context.Context, repoURL string, args ...string) (string, *gogit.Repository, error)
|
||||
}
|
||||
|
||||
// isGHECloud returns true if the endpoint is a GHE.com instance
|
||||
// (GHEC with data residency). GHE.com uses the same root-level API layout as
|
||||
// api.github.com, NOT the GHES /api/v3/ layout.
|
||||
//
|
||||
// Examples:
|
||||
//
|
||||
// https://api.company.ghe.com -> true
|
||||
// https://company.ghe.com -> true (web URL; user might pass this)
|
||||
// https://github.mycompany.com -> false (GHES)
|
||||
// https://api.github.com -> false (regular github.com)
|
||||
func isGHECloud(endpoint string) bool {
|
||||
u, err := url.Parse(strings.TrimRight(endpoint, "/"))
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
return strings.HasSuffix(strings.ToLower(u.Hostname()), gheCloudSuffix)
|
||||
}
|
||||
|
||||
// normalizeGHECloudAPIEndpoint ensures the endpoint points to the API subdomain
|
||||
// with a trailing slash (required by go-github's BaseURL).
|
||||
//
|
||||
// On GHE.com the web UI lives at SUBDOMAIN.ghe.com while the API lives at
|
||||
// api.SUBDOMAIN.ghe.com. Users may pass either form.
|
||||
//
|
||||
// https://company.ghe.com -> https://api.company.ghe.com/
|
||||
// https://company.ghe.com/ -> https://api.company.ghe.com/
|
||||
// https://api.company.ghe.com -> https://api.company.ghe.com/
|
||||
// https://api.company.ghe.com/ -> https://api.company.ghe.com/
|
||||
func normalizeGHECloudAPIEndpoint(endpoint string) (string, error) {
|
||||
u, err := url.Parse(strings.TrimRight(endpoint, "/"))
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("invalid GHE.com endpoint URL: %w", err)
|
||||
}
|
||||
|
||||
host := u.Hostname()
|
||||
port := u.Port()
|
||||
|
||||
// If user passed the web URL (company.ghe.com), prepend "api."
|
||||
if !strings.HasPrefix(strings.ToLower(host), "api.") {
|
||||
host = "api." + host
|
||||
}
|
||||
|
||||
if port != "" {
|
||||
u.Host = host + ":" + port
|
||||
} else {
|
||||
u.Host = host
|
||||
}
|
||||
|
||||
// go-github requires a trailing slash on BaseURL.
|
||||
u.Path = "/"
|
||||
|
||||
return u.String(), nil
|
||||
}
|
||||
|
||||
// createGHECloudClient creates a go-github client configured for GHE.com.
|
||||
// GHE.com serves its REST API at the root of api.SUBDOMAIN.ghe.com (same
|
||||
// layout as api.github.com), so we must NOT use WithEnterpriseURLs which
|
||||
// would append /api/v3/.
|
||||
func createGHECloudClient(httpClient *http.Client, apiEndpoint string) (*github.Client, error) {
|
||||
apiURL, err := normalizeGHECloudAPIEndpoint(apiEndpoint)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
parsedURL, err := url.Parse(apiURL)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to parse GHE.com API URL: %w", err)
|
||||
}
|
||||
|
||||
client := github.NewClient(httpClient)
|
||||
client.BaseURL = parsedURL
|
||||
client.UploadURL = parsedURL
|
||||
return client, nil
|
||||
}
|
||||
|
||||
func newConnector(ctx context.Context, source *Source) (Connector, error) {
|
||||
apiEndpoint := canonicalAPIEndpoint(source.conn.Endpoint)
|
||||
|
||||
@@ -60,27 +137,44 @@ func createAPIClient(ctx context.Context, httpClient *http.Client, apiEndpoint s
|
||||
ctx.Logger().V(2).Info("Creating API client", "url", apiEndpoint)
|
||||
|
||||
// If we're using public GitHub, make a regular client.
|
||||
// Otherwise, make an enterprise client.
|
||||
if strings.EqualFold(apiEndpoint, cloudV3Endpoint) {
|
||||
return github.NewClient(httpClient), nil
|
||||
}
|
||||
|
||||
// GHE.com (GHEC with data residency) serves its API at the root level
|
||||
// (like api.github.com), NOT under /api/v3/.
|
||||
if isGHECloud(apiEndpoint) {
|
||||
return createGHECloudClient(httpClient, apiEndpoint)
|
||||
}
|
||||
|
||||
// GHES (self-hosted) — WithEnterpriseURLs appends /api/v3/ and /api/uploads/.
|
||||
return github.NewClient(httpClient).WithEnterpriseURLs(apiEndpoint, apiEndpoint)
|
||||
}
|
||||
|
||||
func createGraphqlClient(ctx context.Context, client *http.Client, apiEndpoint string) (*githubv4.Client, error) {
|
||||
var graphqlEndpoint string
|
||||
if apiEndpoint == cloudV3Endpoint {
|
||||
switch {
|
||||
case apiEndpoint == cloudV3Endpoint:
|
||||
graphqlEndpoint = cloudGraphqlEndpoint
|
||||
} else {
|
||||
// Use the root endpoint for the host.
|
||||
|
||||
case isGHECloud(apiEndpoint):
|
||||
// GHE.com: GraphQL lives at the root level, same as api.github.com.
|
||||
// https://api.SUBDOMAIN.ghe.com/graphql
|
||||
apiURL, err := normalizeGHECloudAPIEndpoint(apiEndpoint)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("error normalizing GHE.com endpoint: %w", err)
|
||||
}
|
||||
graphqlEndpoint = strings.TrimRight(apiURL, "/") + "/graphql"
|
||||
|
||||
default:
|
||||
// GHES: GraphQL lives under /api/graphql.
|
||||
// https://docs.github.com/en/[email protected]/graphql/guides/introduction-to-graphql
|
||||
parsedURL, err := url.Parse(apiEndpoint)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("error parsing URL: %w", err)
|
||||
}
|
||||
|
||||
// GitHub Enterprise uses `/api/v3` for the base. (https://github.com/google/go-github/issues/958)
|
||||
// GitHub Enterprise Server uses `/api/v3` for the base. (https://github.com/google/go-github/issues/958)
|
||||
// Swap it, and anything before `/api`, with GraphQL.
|
||||
before, _ := strings.CutSuffix(parsedURL.Path, "/api/v3")
|
||||
parsedURL.Path = before + "/api/graphql"
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync"
|
||||
|
||||
"github.com/bradleyfalzon/ghinstallation/v2"
|
||||
@@ -273,8 +274,10 @@ func (c *appConnector) createAPIClientForInstallation(installationID int64) (*gi
|
||||
return nil, fmt.Errorf("could not create app transport for installation %d: %w", installationID, err)
|
||||
}
|
||||
|
||||
// NewFromAppsTransport inherits BaseURL from appsTransport, which
|
||||
// newAppsTransport already set correctly (incl. GHE.com). Don't override it
|
||||
// with the raw endpoint here or GHE.com token refresh would 401.
|
||||
transport := ghinstallation.NewFromAppsTransport(appsTransport, installationID)
|
||||
transport.BaseURL = c.apiEndpoint
|
||||
|
||||
client, err := newGitHubClientWithTransport(c.apiEndpoint, transport)
|
||||
if err != nil {
|
||||
@@ -289,12 +292,41 @@ func newAppsTransport(apiEndpoint string, appID int64, privateKey []byte) (*ghin
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
appsTransport.BaseURL = apiEndpoint
|
||||
|
||||
// ghinstallation uses BaseURL to build the token exchange URL
|
||||
// {BaseURL}/app/installations/{id}/access_tokens. For GHE.com this must be
|
||||
// the api.SUBDOMAIN.ghe.com base with no /api/v3 and no trailing slash
|
||||
// (which ghinstallation does not expect), otherwise token refresh 401s.
|
||||
baseURL, err := appsBaseURL(apiEndpoint)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
appsTransport.BaseURL = baseURL
|
||||
return appsTransport, nil
|
||||
}
|
||||
|
||||
// appsBaseURL returns the BaseURL that ghinstallation transports should use for
|
||||
// token exchange/refresh. For GHE.com it resolves to the api.* subdomain with
|
||||
// the trailing slash trimmed; for github.com and GHES it is the endpoint as-is.
|
||||
func appsBaseURL(apiEndpoint string) (string, error) {
|
||||
if isGHECloud(apiEndpoint) {
|
||||
normalizedURL, err := normalizeGHECloudAPIEndpoint(apiEndpoint)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("could not normalize GHE.com endpoint: %w", err)
|
||||
}
|
||||
return strings.TrimRight(normalizedURL, "/"), nil
|
||||
}
|
||||
return apiEndpoint, nil
|
||||
}
|
||||
|
||||
func newGitHubClientWithTransport(apiEndpoint string, transport http.RoundTripper) (*github.Client, error) {
|
||||
httpClient := common.RetryableHTTPClientTimeout(githubHTTPTimeoutSeconds)
|
||||
httpClient.Transport = transport
|
||||
|
||||
// GHE.com (GHEC with data residency) serves its REST API at the root of
|
||||
// api.SUBDOMAIN.ghe.com (like api.github.com), NOT under the GHES /api/v3 path.
|
||||
if isGHECloud(apiEndpoint) {
|
||||
return createGHECloudClient(httpClient, apiEndpoint)
|
||||
}
|
||||
return github.NewClient(httpClient).WithEnterpriseURLs(apiEndpoint, apiEndpoint)
|
||||
}
|
||||
|
||||
@@ -49,11 +49,16 @@ func NewTokenConnector(ctx context.Context, apiEndpoint, token, clonePath string
|
||||
return nil, fmt.Errorf("error creating GraphQL client: %w", err)
|
||||
}
|
||||
|
||||
// Treat both GHES and GHE.com as "enterprise" for enumeration purposes.
|
||||
// GHE.com (GHEC with data residency) is a dedicated enterprise environment
|
||||
// with EMU, so enterprise-level enumeration (e.g., addAllVisibleOrgs) applies.
|
||||
isEnterprise := !strings.EqualFold(apiEndpoint, cloudV3Endpoint)
|
||||
|
||||
return &tokenConnector{
|
||||
apiClient: apiClient,
|
||||
graphqlClient: graphqlClient,
|
||||
token: token,
|
||||
isGitHubEnterprise: !strings.EqualFold(apiEndpoint, cloudV3Endpoint),
|
||||
isGitHubEnterprise: isEnterprise,
|
||||
handleRateLimit: handleRateLimit,
|
||||
authInUrl: authInUrl,
|
||||
clonePath: clonePath,
|
||||
|
||||
@@ -366,6 +366,51 @@ func TestAddReposByApp(t *testing.T) {
|
||||
assert.True(t, gock.IsDone())
|
||||
}
|
||||
|
||||
// TestAppConnector_EnterpriseBaseURL verifies that GitHub App authentication
|
||||
// correctly sets the BaseURL on the internal AppsTransport for token refresh.
|
||||
// This is critical for GitHub Enterprise Server and GHEC with Data Residency.
|
||||
// Without this fix, token refresh requests would go to api.github.com instead
|
||||
// of the configured enterprise endpoint.
|
||||
func TestAppConnector_EnterpriseBaseURL(t *testing.T) {
|
||||
privateKey := createPrivateKey()
|
||||
enterpriseEndpoint := "https://api.example.ghe.com"
|
||||
|
||||
connector, err := NewAppConnector(
|
||||
context.Background(),
|
||||
enterpriseEndpoint,
|
||||
&credentialspb.GitHubApp{
|
||||
PrivateKey: privateKey,
|
||||
InstallationId: "1337",
|
||||
AppId: "4141",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
appConn, ok := connector.(*appConnector)
|
||||
require.True(t, ok, "connector should be an appConnector")
|
||||
|
||||
// Get the HTTP client's transport, which should be a ghinstallation.Transport
|
||||
transport := appConn.APIClient().Client().Transport
|
||||
|
||||
// Use reflection to access the Transport's BaseURL field
|
||||
transportVal := reflect.ValueOf(transport).Elem()
|
||||
baseURLField := transportVal.FieldByName("BaseURL")
|
||||
require.True(t, baseURLField.IsValid(), "Transport should have a BaseURL field")
|
||||
assert.Equal(t, enterpriseEndpoint, baseURLField.String(),
|
||||
"Transport.BaseURL should be set to enterprise endpoint")
|
||||
|
||||
// Use reflection to access the internal appsTransport and verify its BaseURL
|
||||
// This is the critical check - the internal AppsTransport is used for token refresh
|
||||
appsTransportField := transportVal.FieldByName("appsTransport")
|
||||
require.True(t, appsTransportField.IsValid(), "Transport should have an appsTransport field")
|
||||
|
||||
// Access the BaseURL of the internal AppsTransport
|
||||
appsTransportVal := appsTransportField.Elem()
|
||||
appsBaseURLField := appsTransportVal.FieldByName("BaseURL")
|
||||
require.True(t, appsBaseURLField.IsValid(), "AppsTransport should have a BaseURL field")
|
||||
assert.Equal(t, enterpriseEndpoint, appsBaseURLField.String(),
|
||||
"AppsTransport.BaseURL should be set to enterprise endpoint")
|
||||
}
|
||||
|
||||
func TestAddOrgsByUser(t *testing.T) {
|
||||
defer gock.Off()
|
||||
|
||||
@@ -2737,3 +2782,182 @@ func TestSource_ExcludeArchivedForkInteraction(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsGHECloud(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
endpoint string
|
||||
want bool
|
||||
}{
|
||||
// GHE.com endpoints — should return true
|
||||
{"api subdomain", "https://api.company.ghe.com", true},
|
||||
{"web subdomain", "https://company.ghe.com", true},
|
||||
{"api subdomain trailing slash", "https://api.company.ghe.com/", true},
|
||||
{"web subdomain trailing slash", "https://company.ghe.com/", true},
|
||||
{"uppercase", "https://API.company.GHE.COM", true},
|
||||
{"mixed case", "https://Api.company.Ghe.Com/", true},
|
||||
{"deep subdomain", "https://api.team.company.ghe.com", true},
|
||||
|
||||
// GHES endpoints — should return false
|
||||
{"ghes custom domain", "https://github.mycompany.com", false},
|
||||
{"ghes with path", "https://github.mycompany.com/api/v3", false},
|
||||
{"ghes internal", "https://github.internal.corp.com", false},
|
||||
|
||||
// github.com — should return false
|
||||
{"cloud api", "https://api.github.com", false},
|
||||
{"cloud web", "https://github.com", false},
|
||||
|
||||
// Edge cases — should return false
|
||||
{"empty string", "", false},
|
||||
{"localhost", "http://localhost:8080", false},
|
||||
{"malicious suffix", "https://notghe.com", false},
|
||||
{"partial match", "https://api.fakeghe.com.evil.com", false},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := isGHECloud(tt.endpoint)
|
||||
if got != tt.want {
|
||||
t.Errorf("isGHECloud(%q) = %v, want %v", tt.endpoint, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestNormalizeGHECloudAPIEndpoint(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
input string
|
||||
want string
|
||||
wantErr bool
|
||||
}{
|
||||
{
|
||||
name: "web URL to API URL",
|
||||
input: "https://company.ghe.com",
|
||||
want: "https://api.company.ghe.com/",
|
||||
},
|
||||
{
|
||||
name: "web URL with trailing slash",
|
||||
input: "https://company.ghe.com/",
|
||||
want: "https://api.company.ghe.com/",
|
||||
},
|
||||
{
|
||||
name: "api URL already correct",
|
||||
input: "https://api.company.ghe.com",
|
||||
want: "https://api.company.ghe.com/",
|
||||
},
|
||||
{
|
||||
name: "api URL with trailing slash",
|
||||
input: "https://api.company.ghe.com/",
|
||||
want: "https://api.company.ghe.com/",
|
||||
},
|
||||
{
|
||||
name: "preserves port",
|
||||
input: "http://company.ghe.com:8080",
|
||||
want: "http://api.company.ghe.com:8080/",
|
||||
},
|
||||
{
|
||||
name: "api with port",
|
||||
input: "http://api.company.ghe.com:8080",
|
||||
want: "http://api.company.ghe.com:8080/",
|
||||
},
|
||||
{
|
||||
name: "strips path",
|
||||
input: "https://company.ghe.com/some/path",
|
||||
want: "https://api.company.ghe.com/",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got, err := normalizeGHECloudAPIEndpoint(tt.input)
|
||||
if (err != nil) != tt.wantErr {
|
||||
t.Errorf("normalizeGHECloudAPIEndpoint(%q) error = %v, wantErr %v", tt.input, err, tt.wantErr)
|
||||
return
|
||||
}
|
||||
if got != tt.want {
|
||||
t.Errorf("normalizeGHECloudAPIEndpoint(%q) = %q, want %q", tt.input, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestCreateAPIClient_GHECloud_NoAPIV3(t *testing.T) {
|
||||
// The most critical test: GHE.com clients must NOT have /api/v3/ in their BaseURL.
|
||||
endpoints := []string{
|
||||
"https://api.company.ghe.com",
|
||||
"https://company.ghe.com",
|
||||
"https://api.company.ghe.com/",
|
||||
"https://company.ghe.com/",
|
||||
}
|
||||
|
||||
for _, endpoint := range endpoints {
|
||||
t.Run(endpoint, func(t *testing.T) {
|
||||
client, err := createGHECloudClient(http.DefaultClient, endpoint)
|
||||
if err != nil {
|
||||
t.Fatalf("createGHECloudClient(%q) unexpected error: %v", endpoint, err)
|
||||
}
|
||||
|
||||
baseURL := client.BaseURL.String()
|
||||
|
||||
// Must NOT contain /api/v3
|
||||
if strings.Contains(baseURL, "/api/v3") {
|
||||
t.Errorf("GHE.com client BaseURL must not contain /api/v3/, got: %s", baseURL)
|
||||
}
|
||||
|
||||
// Must point to api.company.ghe.com
|
||||
if client.BaseURL.Hostname() != "api.company.ghe.com" {
|
||||
t.Errorf("expected hostname api.company.ghe.com, got: %s", client.BaseURL.Hostname())
|
||||
}
|
||||
|
||||
// Must have trailing slash
|
||||
if !strings.HasSuffix(baseURL, "/") {
|
||||
t.Errorf("BaseURL must have trailing slash, got: %s", baseURL)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestCreateGraphqlClient_GHECloud(t *testing.T) {
|
||||
// GHE.com GraphQL should be at /graphql, NOT /api/graphql.
|
||||
// We can't easily inspect the URL from the githubv4.Client,
|
||||
// so we test the URL construction logic indirectly by checking
|
||||
// the normalized endpoint.
|
||||
apiURL, err := normalizeGHECloudAPIEndpoint("https://company.ghe.com")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
graphqlURL := strings.TrimRight(apiURL, "/") + "/graphql"
|
||||
expected := "https://api.company.ghe.com/graphql"
|
||||
|
||||
if graphqlURL != expected {
|
||||
t.Errorf("GHE.com GraphQL URL = %q, want %q", graphqlURL, expected)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCreateAPIClient_GHES_HasAPIV3(t *testing.T) {
|
||||
// GHES clients SHOULD have /api/v3/ — make sure we didn't break that.
|
||||
// Note: WithEnterpriseURLs returns an error for invalid URLs, but for
|
||||
// valid ones it appends /api/v3/.
|
||||
client, err := createAPIClient(context.Background(), http.DefaultClient, "https://github.mycompany.com")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
|
||||
if !strings.Contains(client.BaseURL.String(), "/api/v3") {
|
||||
t.Errorf("GHES client BaseURL should contain /api/v3/, got: %s", client.BaseURL.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestCreateAPIClient_CloudGitHub(t *testing.T) {
|
||||
// Regular github.com should use the default client (api.github.com).
|
||||
client, err := createAPIClient(context.Background(), http.DefaultClient, cloudV3Endpoint)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
|
||||
if client.BaseURL.String() != "https://api.github.com/" {
|
||||
t.Errorf("expected https://api.github.com/, got: %s", client.BaseURL.String())
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user