feat: implement sign up with GitHub for the first user (#16629)

Second PR to address https://github.com/coder/coder/issues/16230. See the issue for more context and discussion. It adds a "Continue with GitHub" button to the `/setup` page, so the deployment's admin can sign up with it. It also removes the "Username" and "Full Name" fields to make signing up with email faster. In the email flow, the username is now auto-generated based on the email, and full name is left empty. <img width="1512" alt="Screenshot 2025-02-21 at 17 51 22" src="https://github.com/user-attachments/assets/e7c6986b-c05e-458b-bb01-c3aea3b74c0e" /> There's a separate, follow up issue to visually align the `/setup` page with the new design system: https://github.com/coder/coder/issues/16653
2026-09-22 05:05:20 +08:00 · 2025-02-25 15:54:38 +01:00
parent b419b36ada
commit 67d89bb102
7 changed files with 171 additions and 59 deletions
@@ -27,6 +27,7 @@ import (
 	"github.com/coder/coder/v2/coderd/cryptokeys"
 	"github.com/coder/coder/v2/coderd/idpsync"
 	"github.com/coder/coder/v2/coderd/jwtutils"
+	"github.com/coder/coder/v2/coderd/telemetry"
 	"github.com/coder/coder/v2/coderd/util/ptr"

 	"github.com/coder/coder/v2/coderd/apikey"
@@ -1054,6 +1055,10 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
 	defer params.CommitAuditLogs()
 	if err != nil {
 		if httpErr := idpsync.IsHTTPError(err); httpErr != nil {
+			// In the device flow, the error page is rendered client-side.
+			if api.GithubOAuth2Config.DeviceFlowEnabled && httpErr.RenderStaticPage {
+				httpErr.RenderStaticPage = false
+			}
 			httpErr.Write(rw, r)
 			return
 		}
@@ -1634,7 +1639,17 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C
 			isConvertLoginType = true
 		}

-		if user.ID == uuid.Nil && !params.AllowSignups {
+		// nolint:gocritic // Getting user count is a system function.
+		userCount, err := tx.GetUserCount(dbauthz.AsSystemRestricted(ctx))
+		if err != nil {
+			return xerrors.Errorf("unable to fetch user count: %w", err)
+		}
+
+		// Allow the first user to sign up with OIDC, regardless of
+		// whether signups are enabled or not.
+		allowSignup := userCount == 0 || params.AllowSignups
+
+		if user.ID == uuid.Nil && !allowSignup {
 			signupsDisabledText := "Please contact your Coder administrator to request access."
 			if api.OIDCConfig != nil && api.OIDCConfig.SignupsDisabledText != "" {
 				signupsDisabledText = render.HTMLFromMarkdown(api.OIDCConfig.SignupsDisabledText)
@@ -1695,6 +1710,12 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C
 				return xerrors.Errorf("unable to fetch default organization: %w", err)
 			}

+			rbacRoles := []string{}
+			// If this is the first user, add the owner role.
+			if userCount == 0 {
+				rbacRoles = append(rbacRoles, rbac.RoleOwner().String())
+			}
+
 			//nolint:gocritic
 			user, err = api.CreateUser(dbauthz.AsSystemRestricted(ctx), tx, CreateUserRequest{
 				CreateUserRequestWithOrgs: codersdk.CreateUserRequestWithOrgs{
@@ -1709,10 +1730,20 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C
 				},
 				LoginType:          params.LoginType,
 				accountCreatorName: "oauth",
+				RBACRoles:          rbacRoles,
 			})
 			if err != nil {
 				return xerrors.Errorf("create user: %w", err)
 			}
+
+			if userCount == 0 {
+				telemetryUser := telemetry.ConvertUser(user)
+				// The email is not anonymized for the first user.
+				telemetryUser.Email = &user.Email
+				api.Telemetry.Report(&telemetry.Snapshot{
+					Users: []telemetry.User{telemetryUser},
+				})
+			}
 		}

 		// Activate dormant user on sign-in
@@ -22,6 +22,7 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"go.uber.org/atomic"
 	"golang.org/x/oauth2"
 	"golang.org/x/xerrors"

@@ -254,11 +255,20 @@ func TestUserOAuth2Github(t *testing.T) {
 	})
 	t.Run("BlockSignups", func(t *testing.T) {
 		t.Parallel()
+
+		db, ps := dbtestutil.NewDB(t)
+
+		id := atomic.NewInt64(100)
+		login := atomic.NewString("testuser")
+		email := atomic.NewString("testuser@coder.com")
+
 		client := coderdtest.New(t, &coderdtest.Options{
+			Database: db,
+			Pubsub:   ps,
 			GithubOAuth2Config: &coderd.GithubOAuth2Config{
 				OAuth2Config:       &testutil.OAuth2Config{},
 				AllowOrganizations: []string{"coder"},
-				ListOrganizationMemberships: func(ctx context.Context, client *http.Client) ([]*github.Membership, error) {
+				ListOrganizationMemberships: func(_ context.Context, _ *http.Client) ([]*github.Membership, error) {
 					return []*github.Membership{{
 						State: &stateActive,
 						Organization: &github.Organization{
@@ -266,16 +276,19 @@ func TestUserOAuth2Github(t *testing.T) {
 						},
 					}}, nil
 				},
-				AuthenticatedUser: func(ctx context.Context, client *http.Client) (*github.User, error) {
+				AuthenticatedUser: func(_ context.Context, _ *http.Client) (*github.User, error) {
+					id := id.Load()
+					login := login.Load()
 					return &github.User{
-						ID:    github.Int64(100),
-						Login: github.String("testuser"),
+						ID:    &id,
+						Login: &login,
 						Name:  github.String("The Right Honorable Sir Test McUser"),
 					}, nil
 				},
-				ListEmails: func(ctx context.Context, client *http.Client) ([]*github.UserEmail, error) {
+				ListEmails: func(_ context.Context, _ *http.Client) ([]*github.UserEmail, error) {
+					email := email.Load()
 					return []*github.UserEmail{{
-						Email:    github.String("testuser@coder.com"),
+						Email:    &email,
 						Verified: github.Bool(true),
 						Primary:  github.Bool(true),
 					}}, nil
@@ -283,8 +296,23 @@ func TestUserOAuth2Github(t *testing.T) {
 			},
 		})

+		// The first user in a deployment with signups disabled will be allowed to sign up,
+		// but all the other users will not.
 		resp := oauth2Callback(t, client)
+		require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode)

+		ctx := testutil.Context(t, testutil.WaitLong)
+
+		// nolint:gocritic // Unit test
+		count, err := db.GetUserCount(dbauthz.AsSystemRestricted(ctx))
+		require.NoError(t, err)
+		require.Equal(t, int64(1), count)
+
+		id.Store(101)
+		email.Store("someotheruser@coder.com")
+		login.Store("someotheruser")
+
+		resp = oauth2Callback(t, client)
 		require.Equal(t, http.StatusForbidden, resp.StatusCode)
 	})
 	t.Run("MultiLoginNotAllowed", func(t *testing.T) {
@@ -988,6 +1016,7 @@ func TestUserOIDC(t *testing.T) {
 		IgnoreEmailVerified bool
 		IgnoreUserInfo      bool
 		UseAccessToken      bool
+		PrecreateFirstUser  bool
 	}{
 		{
 			Name: "NoSub",
@@ -1150,7 +1179,17 @@ func TestUserOIDC(t *testing.T) {
 				"email_verified": true,
 				"sub":            uuid.NewString(),
 			},
-			StatusCode: http.StatusForbidden,
+			StatusCode:         http.StatusForbidden,
+			PrecreateFirstUser: true,
+		},
+		{
+			Name: "FirstSignup",
+			IDTokenClaims: jwt.MapClaims{
+				"email":          "kyle@kwc.io",
+				"email_verified": true,
+				"sub":            uuid.NewString(),
+			},
+			StatusCode: http.StatusOK,
 		},
 		{
 			Name: "UsernameFromEmail",
@@ -1443,6 +1482,15 @@ func TestUserOIDC(t *testing.T) {
 			})
 			numLogs := len(auditor.AuditLogs())

+			ctx := testutil.Context(t, testutil.WaitShort)
+			if tc.PrecreateFirstUser {
+				owner.CreateFirstUser(ctx, codersdk.CreateFirstUserRequest{
+					Email:    "precreated@coder.com",
+					Username: "precreated",
+					Password: "SomeSecurePassword!",
+				})
+			}
+
 			client, resp := fake.AttemptLogin(t, owner, tc.IDTokenClaims)
 			numLogs++ // add an audit log for login
 			require.Equal(t, tc.StatusCode, resp.StatusCode)
@@ -1450,8 +1498,6 @@ func TestUserOIDC(t *testing.T) {
 				tc.AssertResponse(t, resp)
 			}

-			ctx := testutil.Context(t, testutil.WaitShort)
-
 			if tc.AssertUser != nil {
 				user, err := client.User(ctx, "me")
 				require.NoError(t, err)
@@ -118,6 +118,8 @@ func (api *API) firstUser(rw http.ResponseWriter, r *http.Request) {
 // @Success 201 {object} codersdk.CreateFirstUserResponse
 // @Router /users/first [post]
 func (api *API) postFirstUser(rw http.ResponseWriter, r *http.Request) {
+	// The first user can also be created via oidc, so if making changes to the flow,
+	// ensure that the oidc flow is also updated.
 	ctx := r.Context()
 	var createUser codersdk.CreateFirstUserRequest
 	if !httpapi.Read(ctx, rw, r, &createUser) {
@@ -198,6 +200,7 @@ func (api *API) postFirstUser(rw http.ResponseWriter, r *http.Request) {
 			OrganizationIDs: []uuid.UUID{defaultOrg.ID},
 		},
 		LoginType:          database.LoginTypePassword,
+		RBACRoles:          []string{rbac.RoleOwner().String()},
 		accountCreatorName: "coder",
 	})
 	if err != nil {
@@ -225,23 +228,6 @@ func (api *API) postFirstUser(rw http.ResponseWriter, r *http.Request) {
 		Users: []telemetry.User{telemetryUser},
 	})

-	// TODO: @emyrk this currently happens outside the database tx used to create
-	// 	the user. Maybe I add this ability to grant roles in the createUser api
-	//	and add some rbac bypass when calling api functions this way??
-	// Add the admin role to this first user.
-	//nolint:gocritic // needed to create first user
-	_, err = api.Database.UpdateUserRoles(dbauthz.AsSystemRestricted(ctx), database.UpdateUserRolesParams{
-		GrantedRoles: []string{rbac.RoleOwner().String()},
-		ID:           user.ID,
-	})
-	if err != nil {
-		httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
-			Message: "Internal error updating user's roles.",
-			Detail:  err.Error(),
-		})
-		return
-	}
-
 	httpapi.Write(ctx, rw, http.StatusCreated, codersdk.CreateFirstUserResponse{
 		UserID:         user.ID,
 		OrganizationID: defaultOrg.ID,
@@ -1351,6 +1337,7 @@ type CreateUserRequest struct {
 	LoginType          database.LoginType
 	SkipNotifications  bool
 	accountCreatorName string
+	RBACRoles          []string
 }

 func (api *API) CreateUser(ctx context.Context, store database.Store, req CreateUserRequest) (database.User, error) {
@@ -1360,6 +1347,13 @@ func (api *API) CreateUser(ctx context.Context, store database.Store, req Create
 		return database.User{}, xerrors.Errorf("invalid username %q: %w", req.Username, usernameValid)
 	}

+	// If the caller didn't specify rbac roles, default to
+	// a member of the site.
+	rbacRoles := []string{}
+	if req.RBACRoles != nil {
+		rbacRoles = req.RBACRoles
+	}
+
 	var user database.User
 	err := store.InTx(func(tx database.Store) error {
 		orgRoles := make([]string, 0)
@@ -1376,10 +1370,9 @@ func (api *API) CreateUser(ctx context.Context, store database.Store, req Create
 			CreatedAt:      dbtime.Now(),
 			UpdatedAt:      dbtime.Now(),
 			HashedPassword: []byte{},
-			// All new users are defaulted to members of the site.
-			RBACRoles: []string{},
-			LoginType: req.LoginType,
-			Status:    status,
+			RBACRoles:      rbacRoles,
+			LoginType:      req.LoginType,
+			Status:         status,
 		}
 		// If a user signs up with OAuth, they can have no password!
 		if req.Password != "" {
@@ -1437,6 +1430,10 @@ func (api *API) CreateUser(ctx context.Context, store database.Store, req Create
 	}

 	for _, u := range userAdmins {
+		if u.ID == user.ID {
+			// If the new user is an admin, don't notify them about themselves.
+			continue
+		}
 		if _, err := api.NotificationsEnqueuer.EnqueueWithData(
 			// nolint:gocritic // Need notifier actor to enqueue notifications
 			dbauthz.AsNotifier(ctx),