authz/github: validate provider against default github URL if not set (#24598)

enterprise/cmd/frontend/internal/auth/githuboauth/provider.go

@@ -23,10 +23,7 @@ import (
 const sessionKey = "githuboauth@0"
 
 func parseProvider(p *schema.GitHubAuthProvider, db dbutil.DB, sourceCfg schema.AuthProviders) (provider *oauth.Provider, messages []string) {
-	rawURL := p.Url
-	if rawURL == "" {
-		rawURL = "https://github.com/"
-	}
+	rawURL := p.GetURL()
 	parsedURL, err := url.Parse(rawURL)
 	if err != nil {
 		messages = append(messages, fmt.Sprintf("Could not parse GitHub URL %q. You will not be able to login via this GitHub instance.", rawURL))

enterprise/internal/authz/github/authz.go

@@ -26,10 +26,10 @@ func NewAuthzProviders(
 	for _, p := range authProviders {
 		if p.Github != nil {
 			var id string
-			ghURL, err := url.Parse(p.Github.Url)
+			ghURL, err := url.Parse(p.Github.GetURL())
 			if err != nil {
 				// error reporting for this should happen elsewhere, for now just use what is given
-				id = p.Github.Url
+				id = p.Github.GetURL()
 			} else {
 				// use codehost normalized URL as ID
 				ch := extsvc.NewCodeHost(ghURL, p.Github.Type)
@@ -52,7 +52,8 @@ func NewAuthzProviders(
 		// with restricted permissions will not be visible to non-admins.
 		if authProvider, exists := githubAuthProviders[p.ServiceID()]; !exists {
 			warnings = append(warnings,
-				fmt.Sprintf("Did not find authentication provider matching %[1]q. "+
+				fmt.Sprintf("GitHub config for %[1]s has `authorization` enabled, "+
+					"but no authentication provider matching %[1]q was found. "+
 					"Check the [**site configuration**](/site-admin/configuration) to "+
 					"verify an entry in [`auth.providers`](https://docs.sourcegraph.com/admin/auth) exists for %[1]s.",
 					p.ServiceID()))

enterprise/internal/authz/github/authz_test.go

@@ -35,7 +35,7 @@ func TestNewAuthzProviders(t *testing.T) {
 		providers, problems, warnings := NewAuthzProviders(
 			[]*types.GitHubConnection{{
 				GitHubConnection: &schema.GitHubConnection{
-					Url:           "https://github.com/my-org",
+					Url:           "https://github.com/my-org", // incorrect
 					Authorization: &schema.GitHubAuthorization{},
 				},
 			}},
@@ -50,7 +50,7 @@ func TestNewAuthzProviders(t *testing.T) {
 		if len(problems) != 0 {
 			t.Fatalf("unexpected problems: %+v", problems)
 		}
-		if len(warnings) != 1 || !strings.Contains(warnings[0], "not find authentication provider") {
+		if len(warnings) != 1 || !strings.Contains(warnings[0], "no authentication provider") {
 			t.Fatalf("unexpected warnings: %+v", warnings)
 		}
 	})
@@ -60,14 +60,13 @@ func TestNewAuthzProviders(t *testing.T) {
 			providers, problems, warnings := NewAuthzProviders(
 				[]*types.GitHubConnection{{
 					GitHubConnection: &schema.GitHubConnection{
-						Url:           "https://github.com/",
+						Url:           schema.DefaultGitHubURL,
 						Authorization: &schema.GitHubAuthorization{},
 					},
 				}},
 				[]schema.AuthProviders{{
-					Github: &schema.GitHubAuthProvider{
-						Url: "https://github.com",
-					},
+					// falls back to schema.DefaultGitHubURL
+					Github: &schema.GitHubAuthProvider{},
 				}})
 			if len(providers) != 1 || providers[0] == nil {
 				t.Fatal("expected a provider")

schema/github_util.go

@@ -0,0 +1,24 @@
+package schema
+
+// DefaultGitHubURL is the default GitHub instance that configuration points to.
+const DefaultGitHubURL = "https://github.com/"
+
+// GetURL retrieves the configured GitHub URL or a default if one is not set.
+func (p *GitHubAuthProvider) GetURL() string {
+	if p != nil && p.Url != "" {
+		return p.Url
+	}
+	return DefaultGitHubURL
+}
+
+func (c *GitHubConnection) SetRepos(all bool, repos []string) error {
+	if all {
+		c.RepositoryQuery = []string{"affiliated"}
+		c.Repos = nil
+		return nil
+	} else {
+		c.RepositoryQuery = []string{}
+	}
+	c.Repos = repos
+	return nil
+}

schema/gitlab_util.go

@@ -1,17 +1,5 @@
 package schema
 
-func (c *GitHubConnection) SetRepos(all bool, repos []string) error {
-	if all {
-		c.RepositoryQuery = []string{"affiliated"}
-		c.Repos = nil
-		return nil
-	} else {
-		c.RepositoryQuery = []string{}
-	}
-	c.Repos = repos
-	return nil
-}
-
 func (c *GitLabConnection) SetRepos(all bool, repos []string) error {
 	if all {
 		c.ProjectQuery = []string{"projects?membership=true&archived=no"}