feat: enable GitHub OAuth2 login by default on new deployments (#16662)

Third and final PR to address
#16230.

This PR enables GitHub OAuth2 login by default on new deployments.
Combined with #16629, this will allow
the first admin user to sign up with GitHub rather than email and
password.

We take care not to enable the default on deployments that would upgrade
to a Coder version with this change.

To disable the default provider an admin can set the
`CODER_OAUTH2_GITHUB_DEFAULT_PROVIDER` env variable to false.

Author: hugodutka

cli/server.go

@@ -688,24 +688,6 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
 				}
 			}
 
-			if vals.OAuth2.Github.ClientSecret != "" || vals.OAuth2.Github.DeviceFlow.Value() {
-				options.GithubOAuth2Config, err = configureGithubOAuth2(
-					oauthInstrument,
-					vals.AccessURL.Value(),
-					vals.OAuth2.Github.ClientID.String(),
-					vals.OAuth2.Github.ClientSecret.String(),
-					vals.OAuth2.Github.DeviceFlow.Value(),
-					vals.OAuth2.Github.AllowSignups.Value(),
-					vals.OAuth2.Github.AllowEveryone.Value(),
-					vals.OAuth2.Github.AllowedOrgs,
-					vals.OAuth2.Github.AllowedTeams,
-					vals.OAuth2.Github.EnterpriseBaseURL.String(),
-				)
-				if err != nil {
-					return xerrors.Errorf("configure github oauth2: %w", err)
-				}
-			}
-
 			// As OIDC clients can be confidential or public,
 			// we should only check for a client id being set.
 			// The underlying library handles the case of no
@@ -793,6 +775,20 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
 				return xerrors.Errorf("set deployment id: %w", err)
 			}
 
+			githubOAuth2ConfigParams, err := getGithubOAuth2ConfigParams(ctx, options.Database, vals)
+			if err != nil {
+				return xerrors.Errorf("get github oauth2 config params: %w", err)
+			}
+			if githubOAuth2ConfigParams != nil {
+				options.GithubOAuth2Config, err = configureGithubOAuth2(
+					oauthInstrument,
+					githubOAuth2ConfigParams,
+				)
+				if err != nil {
+					return xerrors.Errorf("configure github oauth2: %w", err)
+				}
+			}
+
 			options.RuntimeConfig = runtimeconfig.NewManager()
 
 			// This should be output before the logs start streaming.
@@ -1843,25 +1839,101 @@ func configureCAPool(tlsClientCAFile string, tlsConfig *tls.Config) error {
 	return nil
 }
 
-// TODO: convert the argument list to a struct, it's easy to mix up the order of the arguments
-//
+const (
+	// Client ID for https://github.com/apps/coder
+	GithubOAuth2DefaultProviderClientID      = "Iv1.6a2b4b4aec4f4fe7"
+	GithubOAuth2DefaultProviderAllowEveryone = true
+	GithubOAuth2DefaultProviderDeviceFlow    = true
+)
+
+type githubOAuth2ConfigParams struct {
+	accessURL         *url.URL
+	clientID          string
+	clientSecret      string
+	deviceFlow        bool
+	allowSignups      bool
+	allowEveryone     bool
+	allowOrgs         []string
+	rawTeams          []string
+	enterpriseBaseURL string
+}
+
+func getGithubOAuth2ConfigParams(ctx context.Context, db database.Store, vals *codersdk.DeploymentValues) (*githubOAuth2ConfigParams, error) {
+	params := githubOAuth2ConfigParams{
+		accessURL:         vals.AccessURL.Value(),
+		clientID:          vals.OAuth2.Github.ClientID.String(),
+		clientSecret:      vals.OAuth2.Github.ClientSecret.String(),
+		deviceFlow:        vals.OAuth2.Github.DeviceFlow.Value(),
+		allowSignups:      vals.OAuth2.Github.AllowSignups.Value(),
+		allowEveryone:     vals.OAuth2.Github.AllowEveryone.Value(),
+		allowOrgs:         vals.OAuth2.Github.AllowedOrgs.Value(),
+		rawTeams:          vals.OAuth2.Github.AllowedTeams.Value(),
+		enterpriseBaseURL: vals.OAuth2.Github.EnterpriseBaseURL.String(),
+	}
+
+	// If the user manually configured the GitHub OAuth2 provider,
+	// we won't add the default configuration.
+	if params.clientID != "" || params.clientSecret != "" || params.enterpriseBaseURL != "" {
+		return &params, nil
+	}
+
+	// Check if the user manually disabled the default GitHub OAuth2 provider.
+	if !vals.OAuth2.Github.DefaultProviderEnable.Value() {
+		return nil, nil //nolint:nilnil
+	}
+
+	// Check if the deployment is eligible for the default GitHub OAuth2 provider.
+	// We want to enable it only for new deployments, and avoid enabling it
+	// if a deployment was upgraded from an older version.
+	// nolint:gocritic // Requires system privileges
+	defaultEligible, err := db.GetOAuth2GithubDefaultEligible(dbauthz.AsSystemRestricted(ctx))
+	if err != nil && !errors.Is(err, sql.ErrNoRows) {
+		return nil, xerrors.Errorf("get github default eligible: %w", err)
+	}
+	defaultEligibleNotSet := errors.Is(err, sql.ErrNoRows)
+
+	if defaultEligibleNotSet {
+		// nolint:gocritic // User count requires system privileges
+		userCount, err := db.GetUserCount(dbauthz.AsSystemRestricted(ctx))
+		if err != nil {
+			return nil, xerrors.Errorf("get user count: %w", err)
+		}
+		// We check if a deployment is new by checking if it has any users.
+		defaultEligible = userCount == 0
+		// nolint:gocritic // Requires system privileges
+		if err := db.UpsertOAuth2GithubDefaultEligible(dbauthz.AsSystemRestricted(ctx), defaultEligible); err != nil {
+			return nil, xerrors.Errorf("upsert github default eligible: %w", err)
+		}
+	}
+
+	if !defaultEligible {
+		return nil, nil //nolint:nilnil
+	}
+
+	params.clientID = GithubOAuth2DefaultProviderClientID
+	params.allowEveryone = GithubOAuth2DefaultProviderAllowEveryone
+	params.deviceFlow = GithubOAuth2DefaultProviderDeviceFlow
+
+	return &params, nil
+}
+
 //nolint:revive // Ignore flag-parameter: parameter 'allowEveryone' seems to be a control flag, avoid control coupling (revive)
-func configureGithubOAuth2(instrument *promoauth.Factory, accessURL *url.URL, clientID, clientSecret string, deviceFlow, allowSignups, allowEveryone bool, allowOrgs []string, rawTeams []string, enterpriseBaseURL string) (*coderd.GithubOAuth2Config, error) {
-	redirectURL, err := accessURL.Parse("/api/v2/users/oauth2/github/callback")
+func configureGithubOAuth2(instrument *promoauth.Factory, params *githubOAuth2ConfigParams) (*coderd.GithubOAuth2Config, error) {
+	redirectURL, err := params.accessURL.Parse("/api/v2/users/oauth2/github/callback")
 	if err != nil {
 		return nil, xerrors.Errorf("parse github oauth callback url: %w", err)
 	}
-	if allowEveryone && len(allowOrgs) > 0 {
+	if params.allowEveryone && len(params.allowOrgs) > 0 {
 		return nil, xerrors.New("allow everyone and allowed orgs cannot be used together")
 	}
-	if allowEveryone && len(rawTeams) > 0 {
+	if params.allowEveryone && len(params.rawTeams) > 0 {
 		return nil, xerrors.New("allow everyone and allowed teams cannot be used together")
 	}
-	if !allowEveryone && len(allowOrgs) == 0 {
+	if !params.allowEveryone && len(params.allowOrgs) == 0 {
 		return nil, xerrors.New("allowed orgs is empty: must specify at least one org or allow everyone")
 	}
-	allowTeams := make([]coderd.GithubOAuth2Team, 0, len(rawTeams))
-	for _, rawTeam := range rawTeams {
+	allowTeams := make([]coderd.GithubOAuth2Team, 0, len(params.rawTeams))
+	for _, rawTeam := range params.rawTeams {
 		parts := strings.SplitN(rawTeam, "/", 2)
 		if len(parts) != 2 {
 			return nil, xerrors.Errorf("github team allowlist is formatted incorrectly. got %s; wanted <organization>/<team>", rawTeam)
@@ -1873,8 +1945,8 @@ func configureGithubOAuth2(instrument *promoauth.Factory, accessURL *url.URL, cl
 	}
 
 	endpoint := xgithub.Endpoint
-	if enterpriseBaseURL != "" {
-		enterpriseURL, err := url.Parse(enterpriseBaseURL)
+	if params.enterpriseBaseURL != "" {
+		enterpriseURL, err := url.Parse(params.enterpriseBaseURL)
 		if err != nil {
 			return nil, xerrors.Errorf("parse enterprise base url: %w", err)
 		}
@@ -1893,8 +1965,8 @@ func configureGithubOAuth2(instrument *promoauth.Factory, accessURL *url.URL, cl
 	}
 
 	instrumentedOauth := instrument.NewGithub("github-login", &oauth2.Config{
-		ClientID:     clientID,
-		ClientSecret: clientSecret,
+		ClientID:     params.clientID,
+		ClientSecret: params.clientSecret,
 		Endpoint:     endpoint,
 		RedirectURL:  redirectURL.String(),
 		Scopes: []string{
@@ -1906,17 +1978,17 @@ func configureGithubOAuth2(instrument *promoauth.Factory, accessURL *url.URL, cl
 
 	createClient := func(client *http.Client, source promoauth.Oauth2Source) (*github.Client, error) {
 		client = instrumentedOauth.InstrumentHTTPClient(client, source)
-		if enterpriseBaseURL != "" {
-			return github.NewEnterpriseClient(enterpriseBaseURL, "", client)
+		if params.enterpriseBaseURL != "" {
+			return github.NewEnterpriseClient(params.enterpriseBaseURL, "", client)
 		}
 		return github.NewClient(client), nil
 	}
 
 	var deviceAuth *externalauth.DeviceAuth
-	if deviceFlow {
+	if params.deviceFlow {
 		deviceAuth = &externalauth.DeviceAuth{
 			Config:   instrumentedOauth,
-			ClientID: clientID,
+			ClientID: params.clientID,
 			TokenURL: endpoint.TokenURL,
 			Scopes:   []string{"read:user", "read:org", "user:email"},
 			CodeURL:  endpoint.DeviceAuthURL,
@@ -1925,9 +1997,9 @@ func configureGithubOAuth2(instrument *promoauth.Factory, accessURL *url.URL, cl
 
 	return &coderd.GithubOAuth2Config{
 		OAuth2Config:       instrumentedOauth,
-		AllowSignups:       allowSignups,
-		AllowEveryone:      allowEveryone,
-		AllowOrganizations: allowOrgs,
+		AllowSignups:       params.allowSignups,
+		AllowEveryone:      params.allowEveryone,
+		AllowOrganizations: params.allowOrgs,
 		AllowTeams:         allowTeams,
 		AuthenticatedUser: func(ctx context.Context, client *http.Client) (*github.User, error) {
 			api, err := createClient(client, promoauth.SourceGitAPIAuthUser)
@@ -1966,19 +2038,20 @@ func configureGithubOAuth2(instrument *promoauth.Factory, accessURL *url.URL, cl
 			team, _, err := api.Teams.GetTeamMembershipBySlug(ctx, org, teamSlug, username)
 			return team, err
 		},
-		DeviceFlowEnabled: deviceFlow,
+		DeviceFlowEnabled: params.deviceFlow,
 		ExchangeDeviceCode: func(ctx context.Context, deviceCode string) (*oauth2.Token, error) {
-			if !deviceFlow {
+			if !params.deviceFlow {
 				return nil, xerrors.New("device flow is not enabled")
 			}
 			return deviceAuth.ExchangeDeviceCode(ctx, deviceCode)
 		},
 		AuthorizeDevice: func(ctx context.Context) (*codersdk.ExternalAuthDevice, error) {
-			if !deviceFlow {
+			if !params.deviceFlow {
 				return nil, xerrors.New("device flow is not enabled")
 			}
 			return deviceAuth.AuthorizeDevice(ctx)
 		},
+		DefaultProviderConfigured: params.clientID == GithubOAuth2DefaultProviderClientID,
 	}, nil
 }
 

cli/server_test.go

@@ -45,6 +45,8 @@ import (
 	"github.com/coder/coder/v2/cli/clitest"
 	"github.com/coder/coder/v2/cli/config"
 	"github.com/coder/coder/v2/coderd/coderdtest"
+	"github.com/coder/coder/v2/coderd/database"
+	"github.com/coder/coder/v2/coderd/database/dbgen"
 	"github.com/coder/coder/v2/coderd/database/dbtestutil"
 	"github.com/coder/coder/v2/coderd/database/migrations"
 	"github.com/coder/coder/v2/coderd/httpapi"
@@ -306,6 +308,145 @@ func TestServer(t *testing.T) {
 		require.Less(t, numLines, 20)
 	})
 
+	t.Run("OAuth2GitHubDefaultProvider", func(t *testing.T) {
+		type testCase struct {
+			name                                  string
+			githubDefaultProviderEnabled          string
+			githubClientID                        string
+			githubClientSecret                    string
+			expectGithubEnabled                   bool
+			expectGithubDefaultProviderConfigured bool
+			createUserPreStart                    bool
+			createUserPostRestart                 bool
+		}
+
+		runGitHubProviderTest := func(t *testing.T, tc testCase) {
+			t.Parallel()
+			if !dbtestutil.WillUsePostgres() {
+				t.Skip("test requires postgres")
+			}
+
+			ctx, cancelFunc := context.WithCancel(testutil.Context(t, testutil.WaitLong))
+			defer cancelFunc()
+
+			dbURL, err := dbtestutil.Open(t)
+			require.NoError(t, err)
+			db, _ := dbtestutil.NewDB(t, dbtestutil.WithURL(dbURL))
+
+			if tc.createUserPreStart {
+				_ = dbgen.User(t, db, database.User{})
+			}
+
+			args := []string{
+				"server",
+				"--postgres-url", dbURL,
+				"--http-address", ":0",
+				"--access-url", "https://example.com",
+			}
+			if tc.githubClientID != "" {
+				args = append(args, fmt.Sprintf("--oauth2-github-client-id=%s", tc.githubClientID))
+			}
+			if tc.githubClientSecret != "" {
+				args = append(args, fmt.Sprintf("--oauth2-github-client-secret=%s", tc.githubClientSecret))
+			}
+			if tc.githubClientID != "" || tc.githubClientSecret != "" {
+				args = append(args, "--oauth2-github-allow-everyone")
+			}
+			if tc.githubDefaultProviderEnabled != "" {
+				args = append(args, fmt.Sprintf("--oauth2-github-default-provider-enable=%s", tc.githubDefaultProviderEnabled))
+			}
+
+			inv, cfg := clitest.New(t, args...)
+			errChan := make(chan error, 1)
+			go func() {
+				errChan <- inv.WithContext(ctx).Run()
+			}()
+			accessURLChan := make(chan *url.URL, 1)
+			go func() {
+				accessURLChan <- waitAccessURL(t, cfg)
+			}()
+
+			var accessURL *url.URL
+			select {
+			case err := <-errChan:
+				require.NoError(t, err)
+			case accessURL = <-accessURLChan:
+				require.NotNil(t, accessURL)
+			}
+
+			client := codersdk.New(accessURL)
+
+			authMethods, err := client.AuthMethods(ctx)
+			require.NoError(t, err)
+			require.Equal(t, tc.expectGithubEnabled, authMethods.Github.Enabled)
+			require.Equal(t, tc.expectGithubDefaultProviderConfigured, authMethods.Github.DefaultProviderConfigured)
+
+			cancelFunc()
+			select {
+			case err := <-errChan:
+				require.NoError(t, err)
+			case <-time.After(testutil.WaitLong):
+				t.Fatal("server did not exit")
+			}
+
+			if tc.createUserPostRestart {
+				_ = dbgen.User(t, db, database.User{})
+			}
+
+			// Ensure that it stays at that setting after the server restarts.
+			inv, cfg = clitest.New(t, args...)
+			clitest.Start(t, inv)
+			accessURL = waitAccessURL(t, cfg)
+			client = codersdk.New(accessURL)
+
+			ctx = testutil.Context(t, testutil.WaitLong)
+			authMethods, err = client.AuthMethods(ctx)
+			require.NoError(t, err)
+			require.Equal(t, tc.expectGithubEnabled, authMethods.Github.Enabled)
+			require.Equal(t, tc.expectGithubDefaultProviderConfigured, authMethods.Github.DefaultProviderConfigured)
+		}
+
+		for _, tc := range []testCase{
+			{
+				name:                                  "NewDeployment",
+				expectGithubEnabled:                   true,
+				expectGithubDefaultProviderConfigured: true,
+				createUserPreStart:                    false,
+				createUserPostRestart:                 true,
+			},
+			{
+				name:                                  "ExistingDeployment",
+				expectGithubEnabled:                   false,
+				expectGithubDefaultProviderConfigured: false,
+				createUserPreStart:                    true,
+				createUserPostRestart:                 false,
+			},
+			{
+				name:                                  "ManuallyDisabled",
+				githubDefaultProviderEnabled:          "false",
+				expectGithubEnabled:                   false,
+				expectGithubDefaultProviderConfigured: false,
+			},
+			{
+				name:                                  "ConfiguredClientID",
+				githubClientID:                        "123",
+				expectGithubEnabled:                   true,
+				expectGithubDefaultProviderConfigured: false,
+			},
+			{
+				name:                                  "ConfiguredClientSecret",
+				githubClientSecret:                    "456",
+				expectGithubEnabled:                   true,
+				expectGithubDefaultProviderConfigured: false,
+			},
+		} {
+			tc := tc
+			t.Run(tc.name, func(t *testing.T) {
+				runGitHubProviderTest(t, tc)
+			})
+		}
+	})
+
 	// Validate that a warning is printed that it may not be externally
 	// reachable.
 	t.Run("LocalAccessURL", func(t *testing.T) {