Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Feb 25, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2ce9dbc
siteconfig and deployment config
hugodutka Feb 21, 2025
5a95330
make gen
hugodutka Feb 22, 2025
1e2fc29
make gen impl
hugodutka Feb 22, 2025
8c1e867
server
hugodutka Feb 22, 2025
47ffefd
golden files
hugodutka Feb 22, 2025
bd50031
frontend changes
hugodutka Feb 24, 2025
297e90b
server tests
hugodutka Feb 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 114 additions & 41 deletions cli/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand All @@ -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)
}
Expand All @@ -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{
Expand All @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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
}

Expand Down
141 changes: 141 additions & 0 deletions cli/server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Expand Down
Loading
Loading