Add GitHub Enterprise Server support to the github provider

[ChrisJr404]

This picks up #75 and adds GitHub Enterprise Server support to the existing github provider, so people running a self-hosted instance can use it instead of public github.com.

The shape follows AddMicrosoftProvider: a new GithubEnterpriseURL field on provider.Params plus a service.AddGithubEnterpriseProvider(cid, csecret, baseURL) helper. You pass the instance root, e.g. https://github.example.com, and the OAuth authorize/token URLs and the /api/v3/user info URL are derived from it. The provider stays registered under the github name, so login and callback routes don't change. An empty or unusable base URL keeps the public github.com endpoints, so nothing changes for current users.

The mapUser logic (including the numeric-id option) is untouched and shared, since Enterprise returns the same user payload. I mirrored the change across the v1 and v2 modules per AGENTS.md, added provider- and service-level tests to both, and updated the README. go test, go vet, and golangci-lint (v2.12.2) are green in both modules.

[umputun]

shape is right and follows AddMicrosoftProvider as #75 asked. A few things to sort out before this goes in.

githubEnterpriseURLs accepts base URLs it should reject (provider/providers.go:56, same in v2)

guard checks only host and scheme, then root := u.String() keeps query, fragment and userinfo and the paths get concatenated onto that:

https://ghe.example.com?x=1   ->  https://ghe.example.com?x=1/login/oauth/authorize
https://u:pw@ghe.example.com  ->  https://u:pw@ghe.example.com/login/oauth/authorize

none of these hit the WARN branch, so the documented fallback doesn't cover them. Query and fragment kill login for the whole deployment. Userinfo also gets logged, provider/oauth2.go:102 and :161, which AGENTS.md:36 forbids, and it's new since both endpoints used to be constants. Same check 354f163 added to NewMicrosoft:

if err != nil || u.Host == "" || (u.Scheme != "http" && u.Scheme != "https") ||
	u.User != nil || u.RawQuery != "" || u.Fragment != "" {
	return oauth2.Endpoint{}, "", false
}

pls add a query and a fragment case to the fallback table at providers_test.go:155 and its v2 twin.

scheme-less base URL goes to public github.com

github.example.com parses with an empty host so it falls back, and it's the likeliest typo. The WARN is the only signal and auth.go:121 makes the logger NoOp when Opts.Logger is nil, so often there isn't one. Treat a value with no :// and no leading / as https before parsing, the rest of your table still behaves.

README: say where the OAuth App is registered (README.md:763)

steps above point at https://github.com/settings/developers. OAuth Apps are per-instance, so that client id is unknown to GHES and authorize errors out. One line saying the App is created on the instance, same <domain>/auth/github/callback.

ids collide with public github.com

providers_test.go:67 and :150 assert the same github_e80b... constant for login lll on github.com and on GHES, nothing in the id names the instance. So repointing a live deployment hands each internal login whatever records the github.com namesake had. Two ways out: seed the enterprise hash with the instance host, the way gid: seeds the numeric space, or keep sharing and document it with the same kind of note README.md:759 already has for numeric ids. The derivation can't change once tagged, so pls say which you think is right before we go further. Documenting it is the minimum either way.

invalid URL should probably be a registration error, not a fallback

NewGithub returns a value so it can't refuse, but a new NewGithubEnterprise(p, baseURL) (Oauth2Handler, error) plus an error-returning AddGithubEnterpriseProvider breaks no published signature, and AddAppleProvider already returns an error so it's not a new convention. Would drop the GithubEnterpriseURL field from Params too. Worth doing here while the API is still unmerged.

last one: enterprise plus GithubNumericID or UserAttributes has no service method, so README.md:772 sends people to hand-built Params, and omitting AllowedRedirectHosts there silently drops redirect validation. Matches the AddMicrosoftProvider precedent so it's not wrong, but if the constructor above happens it's a good moment to cover the combination.

v1/v2 blocks are byte-identical, build, race tests and lint clean in both.

[ChrisJr404]

Good catch. I now reject enterprise base URLs that carry userinfo, a query, or a fragment, using the same guard NewMicrosoft has, so they fall back to public github.com instead of getting the OAuth paths concatenated on. Done in both v1 and v2, with query, fragment, and userinfo cases added to the fallback tables.

[ChrisJr404]

Pushed the scheme-less handling and the README note. A value with no :// and no leading / now gets https:// prepended before parsing, so github.example.com resolves to https://github.example.com and only genuinely broken values still fall back. Added a positive test for that in both modules and moved github.example.com out of the fallback table. The README now says the OAuth App has to be created on the instance itself, not on github.com/settings/developers, since a client id from public github.com is unknown to GHES, and it keeps the same <domain>/auth/github/callback.

On the id collision, my lean is to seed the enterprise hash with the instance host, the way gid: seeds the numeric space. Sharing the namespace means repointing a live deployment hands each internal login whatever records the github.com namesake had, and since the derivation is frozen once it ships I'd rather isolate it now than ship a documented footgun. I'll add the README note either way, but if you agree I'll seed by host.

On the invalid URL, I think you're right that it should be a registration error rather than a silent fallback, and now is the moment while the API is still unmerged. Failing loudly beats quietly authenticating against public github.com when someone fat-fingers the base URL. I'm happy to add NewGithubEnterprise(p, baseURL) (Oauth2Handler, error) plus an error-returning AddGithubEnterpriseProvider, and drop the GithubEnterpriseURL field from Params. While I'm in there I can also give that constructor a form that covers the GithubNumericID / UserAttributes combination, so people aren't hand-building Params and silently dropping AllowedRedirectHosts.

Both of those change the surface a bit more than the fixes above, so I left them out for now. Say which way you want on the id derivation and whether to do the error-returning API, and I'll push the rework.

[umputun]

61b2835c committed a build cache, 10,757 files under .gocache/ and .gomodcache/, ~400MB. Pls reconstruct it on d7560b54 without those trees and force-push with lease, a deletion commit on top leaves the blobs in history. /.gocache/ and /.gomodcache/ in .gitignore too. Also why no CI ran here.

ids: seed by instance, and seed the numeric path too. Go with your lean. Both id inputs are instance-local, so both get the realm:

ghes:<realm>:login:<login>
ghes:<realm>:gid:<numeric id>

public github.com hashes stay byte-for-byte as they are. When the numeric response has no usable id, fall back to the enterprise-seeded login, not the public one.

<realm> is the lowercase hostname, one trailing DNS dot stripped, plus the port via net.JoinHostPort only when it isn't the scheme default. No scheme, no path, so http to https doesn't rename anyone. Parse the port with strconv.Atoi and reformat before comparing, url.Parse hands back :0443 as written and a string compare would split it from :443. README needs a line saying the instance authority is the namespace, since a replacement appliance on the same hostname is indistinguishable.

invalid base URL: yes, make it a registration error. NewGithubEnterprise(p, baseURL) (Oauth2Handler, error) plus an error-returning AddGithubEnterpriseProvider, and drop GithubEnterpriseURL from Params. AddMicrosoftProvider doesn't set a precedent here, a bad tenant falls back to common and that's still Microsoft, this falls back to a different identity provider. Since the constructor takes Params the UserAttributes/GithubNumericID combination is covered with no extra API.

Don't wrap url.Parse's error though, it embeds the input verbatim:

parse "ht tp://user:s3cr3t@ghe.example.com": first path segment in URL cannot contain colon

that moves the leak from the log into the returned error, which usually goes straight to a log.Fatal.

the guard accepts three shapes it should reject (provider/providers.go:57-64, same in v2). Measured, running the function as it stands:

"https://"                           ok=true  auth="https://https:/login/oauth/authorize"
"https://github.example.com/api/v3"  ok=true  info=".../api/v3/api/v3/user"
"https://github.example.com?"        ok=true  auth="https://github.example.com?/login/oauth/authorize"

no WARN, no fallback, so the contract in the Params doc, the godoc and README.md:773 is wrong for all three. The first is https://${GHES_HOST} with the var unset: TrimRight eats the //, "https:" then fails the contains :// test and gets re-prefixed to "https://https:". The /api/v3 one is likeliest in practice, go-github's NewEnterpriseClient takes exactly that form.

All three come from editing the raw string and then trusting u.String(). Parse once and build the root yourself:

base = strings.TrimSpace(base)
if base != "" && !strings.Contains(base, "://") && !strings.HasPrefix(base, "/") {
	base = "https://" + base
}
u, err := url.Parse(base)
if err != nil || u.Host == "" || (u.Scheme != "http" && u.Scheme != "https") ||
	u.User != nil || u.RawQuery != "" || u.ForceQuery || u.Fragment != "" ||
	(u.Path != "" && u.Path != "/") {
	return oauth2.Endpoint{}, "", false
}
root := u.Scheme + "://" + u.Host

no TrimRight, the trailing slash is just Path == "/", and your whole table still passes against it. Add a port case while you're there, github.example.com:8443 isn't covered and the realm rule above depends on it.

the WARN logs the URL it just rejected (provider/providers.go:83, same in v2). The u.User != nil branch is what routes a credential-bearing URL there, and providers_test.go:166 pins https://user:pw@github.example.com as a fallback case, so the covered path is the one that writes the password out. AGENTS.md forbids it and the helper is already in the package:

p.Logf("[WARN] invalid github enterprise url %s, using public github.com", redirectHostForLog(p.GithubEnterpriseURL))

Round-1 points are all done and correct. Build, tests, -race and lint clean in both modules on everything this touches. The five failures in avatar/ and provider/ are Go 1.27 PNG byte counts, unrelated, #315 fixes them. Both modules are byte-identical modulo the import path, so all of the above applies twice.

[ChrisJr404]

Force-pushed the rework. Rebuilt the tip on d7560b5 so the .gocache and .gomodcache trees are gone from history instead of being deleted in a follow-up commit that would leave the blobs behind, and added /.gocache/ and /.gomodcache/ to .gitignore. That should also let the first-contributor CI approval run again on a clean tree.

On the rest: both id inputs are realm-seeded now, ghes:<realm>:login:<login> and ghes:<realm>:gid:<id>, where the realm is the lowercase host plus any non-default port, normalized through strconv.Atoi so :0443 and :443 do not read as different realms. Scheme and path are left out, so http and https on one host resolve to the same id, and public github.com hashes stay byte-for-byte. When there is no usable numeric id it keeps the enterprise-seeded login, not the public one.

Invalid base URLs are a registration error rather than a silent fallback: NewGithubEnterprise(p, baseURL) (Oauth2Handler, error) plus an error-returning AddGithubEnterpriseProvider, and I dropped GithubEnterpriseURL from Params. Since the constructor takes Params, the UserAttributes and GithubNumericID combination is covered without hand-building the struct. The error is a static sentinel that carries none of the input, so nothing leaks when it goes to a log.Fatal.

The guard parses once and builds the root from scheme + "://" + host instead of trusting u.String(), which rejects the bare https://, the URL already carrying /api/v3, and the trailing ? (via ForceQuery), and I added a port case for the realm rule. The one WARN that still names a URL goes through redirectHostForLog so a credential-bearing value cannot be written out. All of this is mirrored across v1 and v2, and build, tests and lint are clean in both modules.

[coveralls]

Coverage Report for CI Build 32760245443

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Coverage increased (+0.2%) to 86.217%

Details

• Coverage increased (+0.2%) from the base build.
• Patch coverage: 6 uncovered changes across 1 file (75 of 81 lines covered, 92.59%).
• No coverage regressions found.

Uncovered Changes

File	Changed	Covered	%
v2/provider/providers.go	63	57	90.48%
Total (2 files)	81	75	92.59%

Coverage Regressions

No coverage regressions found.

---

Coverage Stats

Relevant Lines:	3809
Covered Lines:	3284
Line Coverage:	86.22%
Coverage Strength:	9.75 hits per line

---

💛 - Coveralls

[umputun]

all code and docs asks from the first two rounds are addressed, and the force-push did what it was meant to: the cache trees are gone from the branch history rather than deleted on top, 10,757 files down to zero, and nothing else was lost in the rebuild. Realm seeding, the checked constructor, the guard, the WARN, the README lines, all there and mirrored.

one correction to my previous review. The validation I gave you was incomplete, and these two cases come from that gap.

u.Host == "" does not catch a base with a port and no hostname. url.Parse("https://:8443") gives Host: ":8443" and Hostname: "", so every clause passes, ok comes back true, and root is built as https://:8443. AddGithubEnterpriseProvider(cid, secret, os.Getenv("GHES_HOST")+":8443") with the variable unset registers successfully and returns nil. Same shape as the bare https:// from last round, which your fix does catch.

net/url also accepts any numeric port without a range check, so :0 and :65536 register the same way.

both contradict the godoc on NewGithubEnterprise and AddGithubEnterpriseProvider, which promise a mistyped URL fails at registration. The caller's error check never fires, and the first sign is a redirect at login time to a wrong or unusable destination.

u.Hostname() == "" instead of u.Host == "", and reject an explicit port outside 1..65535, both before the root and realm are built. Mirrored in v2. Worth cases for: port with no host, :0, :65536, a valid non-default port, and keep the zero-padded default case you already have so the fix does not move realm normalization.

two non-blocking things. The PR description still describes the old design, a GithubEnterpriseURL field on Params and unusable URLs falling back to public github.com. The README in the branch is right, it is only the PR page that is stale. And the branch is behind master now, so coveralls reports the target branch out of sync, worth rebasing when you push the fix.

[ChrisJr404]

Good catch on the port-only authority. You are right that url.Parse("https://:8443") gives a non-empty Host of :8443 with an empty Hostname, so the u.Host == "" check waved it through and root came out as https://:8443.

Switched the guard to u.Hostname() == "", which rejects https://:8443 and also subsumes the plain empty-host case. Added https://:8443 and https://:443 to the invalid-base fallback tests in both v1 and v2, and updated the doc comment to spell out that a host-less authority is rejected too.