Skip to content

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

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.

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 4 commits into from
Feb 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 32 additions & 1 deletion coderd/userauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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{
Expand All @@ -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
Expand Down
64 changes: 55 additions & 9 deletions coderd/userauth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -254,37 +255,64 @@ 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{
Login: github.String("coder"),
},
}}, 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
},
},
})

// 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) {
Expand Down Expand Up @@ -988,6 +1016,7 @@ func TestUserOIDC(t *testing.T) {
IgnoreEmailVerified bool
IgnoreUserInfo bool
UseAccessToken bool
PrecreateFirstUser bool
}{
{
Name: "NoSub",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -1443,15 +1482,22 @@ 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)
if tc.AssertResponse != nil {
tc.AssertResponse(t, resp)
}

ctx := testutil.Context(t, testutil.WaitShort)

if tc.AssertUser != nil {
user, err := client.User(ctx, "me")
require.NoError(t, err)
Expand Down
39 changes: 18 additions & 21 deletions coderd/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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()},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

accountCreatorName: "coder",
})
if err != nil {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand All @@ -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)
Expand All @@ -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 != "" {
Expand Down Expand Up @@ -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),
Expand Down
1 change: 0 additions & 1 deletion site/e2e/setup/addUsersAndLicense.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ test("setup deployment", async ({ page }) => {
}

// Setup first user
await page.getByLabel(Language.usernameLabel).fill(users.admin.username);
await page.getByLabel(Language.emailLabel).fill(users.admin.email);
await page.getByLabel(Language.passwordLabel).fill(users.admin.password);
await page.getByTestId("create").click();
Expand Down
3 changes: 0 additions & 3 deletions site/src/pages/SetupPage/SetupPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,15 @@ import { SetupPage } from "./SetupPage";
import { Language as PageViewLanguage } from "./SetupPageView";

const fillForm = async ({
username = "someuser",
email = "someone@coder.com",
password = "password",
}: {
username?: string;
email?: string;
password?: string;
} = {}) => {
const usernameField = screen.getByLabelText(PageViewLanguage.usernameLabel);
const emailField = screen.getByLabelText(PageViewLanguage.emailLabel);
const passwordField = screen.getByLabelText(PageViewLanguage.passwordLabel);
await userEvent.type(usernameField, username);
await userEvent.type(emailField, email);
await userEvent.type(passwordField, password);
const submitButton = screen.getByRole("button", {
Expand Down
6 changes: 4 additions & 2 deletions site/src/pages/SetupPage/SetupPage.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { buildInfo } from "api/queries/buildInfo";
import { createFirstUser } from "api/queries/users";
import { authMethods, createFirstUser } from "api/queries/users";
import { Loader } from "components/Loader/Loader";
import { useAuthContext } from "contexts/auth/AuthProvider";
import { useEmbeddedMetadata } from "hooks/useEmbeddedMetadata";
Expand All @@ -19,6 +19,7 @@ export const SetupPage: FC = () => {
isSignedIn,
isSigningIn,
} = useAuthContext();
const authMethodsQuery = useQuery(authMethods());
const createFirstUserMutation = useMutation(createFirstUser());
const setupIsComplete = !isConfiguringTheFirstUser;
const { metadata } = useEmbeddedMetadata();
Expand All @@ -34,7 +35,7 @@ export const SetupPage: FC = () => {
});
}, [buildInfoQuery.data]);

if (isLoading) {
if (isLoading || authMethodsQuery.isLoading) {
return <Loader fullscreen />;
}

Expand All @@ -54,6 +55,7 @@ export const SetupPage: FC = () => {
<title>{pageTitle("Set up your account")}</title>
</Helmet>
<SetupPageView
authMethods={authMethodsQuery.data}
isLoading={isSigningIn || createFirstUserMutation.isLoading}
error={createFirstUserMutation.error}
onSubmit={async (firstUser) => {
Expand Down
Loading
Loading