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.

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

Jump to bottom
Merged
merged 4 commits into from
Feb 25, 2025
Merged

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

merged 4 commits into from
Feb 25, 2025

Conversation

hugodutka
Copy link
Contributor

@hugodutka hugodutka commented Feb 19, 2025 •
edited
Loading

Second PR to address #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.

Screenshot 2025-02-21 at 17 51 22

There's a separate, follow up issue to visually align the /setup page with the new design system: #16653

@hugodutka hugodutka force-pushed the hugodutka/github-oauth2-setup branch 3 times, most recently from 5fb6932 to a4258c7 Compare February 20, 2025 16:16
@hugodutka hugodutka changed the title feat: first user sign up with github feat: implement sign up with GitHub for the first user Feb 20, 2025
@hugodutka hugodutka force-pushed the hugodutka/github-oauth2-setup branch 13 times, most recently from b16682c to 2473a5e Compare February 21, 2025 16:19
@hugodutka hugodutka marked this pull request as ready for review February 21, 2025 16:57
@hugodutka hugodutka force-pushed the hugodutka/github-oauth2-device-flow branch from dab3105 to be8cca1 Compare February 21, 2025 17:18
@hugodutka hugodutka force-pushed the hugodutka/github-oauth2-setup branch from 2473a5e to 1055883 Compare February 21, 2025 17:19
@hugodutka hugodutka force-pushed the hugodutka/github-oauth2-device-flow branch from be8cca1 to cfc3645 Compare February 21, 2025 17:23
@hugodutka hugodutka force-pushed the hugodutka/github-oauth2-setup branch from 1055883 to 484afd4 Compare February 21, 2025 17:23
Base automatically changed from hugodutka/github-oauth2-device-flow to main February 21, 2025 17:42
@hugodutka hugodutka force-pushed the hugodutka/github-oauth2-setup branch from 484afd4 to 9549df5 Compare February 21, 2025 17:44
@hugodutka
Copy link
Contributor Author

By the way, you can test the changes locally by starting the coder server like this:

export CODER_OAUTH2_GITHUB_DEVICE_FLOW=true
export CODER_OAUTH2_GITHUB_CLIENT_ID=Iv23liSBHklRMBNx5lk9
export CODER_OAUTH2_GITHUB_ALLOW_EVERYONE=true
go run cmd/coder/main.go server

This was referenced Feb 22, 2025
Merged
Merged
@dannykopping
Copy link
Contributor

By the way, you can test the changes locally by starting the coder server like this:

export CODER_OAUTH2_GITHUB_DEVICE_FLOW=true
export CODER_OAUTH2_GITHUB_CLIENT_ID=Iv23liSBHklRMBNx5lk9
export CODER_OAUTH2_GITHUB_ALLOW_EVERYONE=true
go run cmd/coder/main.go server

PS go run cmd/coder/main.go server only produces a slim build, so the web UI doesn't work.

Once I get a full build running and auth with your client ID I get:

image

@dannykopping
Copy link
Contributor

I tested using scripts/develop.sh which creates the first user; maybe that caused an issue?

@hugodutka
Copy link
Contributor Author

@dannykopping yes - only the first user can be created by default. This matches how Coder works currently.

Tu use go run as I mentioned above you’d have to also start the frontend separately:

cd site && pnpm run dev

@dannykopping
Copy link
Contributor

Sweet 👌 works nicely

Emyrk
Emyrk approved these changes Feb 24, 2025
View reviewed changes
Copy link
Member

@Emyrk Emyrk left a comment

Choose a reason for hiding this comment

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

LGTM, I did not test it though

coderd/users.go
@@ -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.

❤️

BrunoQuaresma
BrunoQuaresma reviewed Feb 24, 2025
View reviewed changes
site/src/pages/SetupPage/SetupPageView.tsx Outdated
onChangeTrimmed(form)(event);
},
[form],
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO, there is no need to use useCallback since the function is not used as a dependency in an effect and the function doesn't look heavy for optimization.

https://www.joshwcomeau.com/react/usememo-and-usecallback/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved it out of useCallback

site/src/pages/SetupPage/SetupPageView.tsx Outdated
<div className="shrink-0 text-xs uppercase text-content-secondary tracking-wider">
or
</div>
<div className="h-[1px] w-full bg-divider" />
Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't use the token divider so you could use bg-border-default. Let me know in case it does not look good.

Copy link
Contributor Author

@hugodutka hugodutka Feb 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I added support for the divider token to mirror the color palette of the sign in screen exactly. Would you like me to remove it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes please, since we don't use it in our design system.

Copy link
Contributor Author

@hugodutka hugodutka Feb 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

done! bg-border-default isn't valid, but bg-border is and it looks good.
Screenshot 2025-02-24 at 17 54 59

@hugodutka hugodutka force-pushed the hugodutka/github-oauth2-setup branch 2 times, most recently from 1a67ae7 to dfd49d0 Compare February 24, 2025 16:59
@hugodutka hugodutka force-pushed the hugodutka/github-oauth2-setup branch from dfd49d0 to 3b36025 Compare February 24, 2025 20:37
@hugodutka hugodutka merged commit 67d89bb into main Feb 25, 2025
31 of 33 checks passed
@hugodutka hugodutka deleted the hugodutka/github-oauth2-setup branch February 25, 2025 14:54
@github-actions github-actions bot locked and limited conversation to collaborators Feb 25, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@BrunoQuaresma BrunoQuaresma BrunoQuaresma approved these changes

@Emyrk Emyrk Emyrk approved these changes

Assignees

@hugodutka hugodutka

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hugodutka @dannykopping @BrunoQuaresma @Emyrk