feat: Allow hiding password auth, changing OpenID Connect text and OpenID Connect icon by normana10 · Pull Request #5101 · coder/coder

normana10 · 2022-11-16T02:48:37Z

Just some ideas to help decrease friction in onboarding developers to my coder deployment

For context, my users will never use password auth, only Oauth

Examples:

Default coder login (nothing new here, just for reference)
With the following env vars set (also nothing new here, just for reference):

CODER_OIDC_ISSUER_URL
CODER_OIDC_EMAIL_DOMAIN
CODER_OIDC_CLIENT_ID
CODER_OIDC_CLIENT_SECRET

With the following env vars set:

CODER_OIDC_ISSUER_URL
CODER_OIDC_EMAIL_DOMAIN
CODER_OIDC_CLIENT_ID
CODER_OIDC_CLIENT_SECRET
CODER_PASSWORD_AUTH_HIDDEN=true
CODER_OIDC_SIGN_IN_TEXT="Sign in with Gitea"
CODER_OIDC_ICON_URL=https://gitea.io/images/gitea.png

I only reference Gitea because that's my OpenID Connect provider at home, feel free to replace that with anything you like 👍

I also acknowledge that I'm changing the shape of the return body of api/v2/users/authmethods. Not sure if that's a deal breaker?

Also let me know if Coder intentionally didn't want to do any of these. Don't want my random PRs to steamroll any actual conversations that have taken place

…Connect icon

kylecarbs

Hey 👋 I appreciate the contribution!

I think we'll want to end up disabling password authentication instead of hiding it for security reasons. I'll review this later today!

bpmct · 2022-11-22T15:47:32Z

@kylecarbs and I just had a chat and decided we shouldn't completely disable the password login so the default Owner account can log in.

However, we can put it behind a backdoor (e.g. ?auth=password) so that users don't see an option to log in with a user/pass. Also, OIDC users wouldn't be able to log in with a password anyways

normana10 · 2022-11-24T17:00:49Z

Implemented

With the following env vars set:

CODER_OIDC_ISSUER_URL
CODER_OIDC_EMAIL_DOMAIN
CODER_OIDC_CLIENT_ID
CODER_OIDC_CLIENT_SECRET
CODER_PASSWORD_AUTH_HIDDEN=true
CODER_OIDC_SIGN_IN_TEXT="Sign in with Gitea"
CODER_OIDC_ICON_URL=https://gitea.io/images/gitea.png

github-actions · 2022-12-06T00:10:35Z

This Pull Request is becoming stale. In order to minimize WIP, prevent merge conflicts and keep the tracker readable, I'm going close to this PR in 3 days if there isn't more activity.

# Conflicts: # cli/deployment/config.go # cli/server.go # cli/testdata/coder_server_--help.golden # coderd/userauth.go # codersdk/deploymentconfig.go # docs/admin/auth.md # site/src/api/typesGenerated.ts # site/src/components/SignInForm/SignInForm.tsx

github-actions · 2022-12-14T00:11:33Z

This Pull Request is becoming stale. In order to minimize WIP, prevent merge conflicts and keep the tracker readable, I'm going close to this PR in 3 days if there isn't more activity.

mafredri · 2022-12-14T14:26:30Z

I think we'll want to end up disabling password authentication instead of hiding it for security reasons. I'll review this later today!

@kylecarbs how would an owner/admin sign in if we disable password auth? As @bpmct proposed, we could put it behind ?password=password, or maybe better /login/admin. But ultimately that seems like an inconvenience. Maybe it's negligible though.

mafredri

Backend changes look good to me with one minor request if we keep the flag.

kylecarbs · 2022-12-14T15:23:58Z

I think I'd prefer if we did a small "Login with Password" link at the bottom that would permit authenticating with a password. It's awkward to not allow it, since in emergency situations it would be required.

An alternative is to take the stance that owners/admins should be able to modify the deployment, so they could always disable this feature.

bpmct · 2022-12-14T15:29:31Z

I think I'd prefer if we did a small "Login with Password" link at the bottom that would permit authenticating with a password. It's awkward to not allow it, since in emergency situations it would be required.

An alternative is to take the stance that owners/admins should be able to modify the deployment, so they could always disable this feature.

I can get behind a small link as long as it's clear that SSO is preferred. Many login forms tend to have both even if 99% of an organization's users use SSO

bpmct

This UX LGTM! Thanks for the contribution, let's ship it 🚢

Kira-Pilot · 2023-01-19T20:31:38Z

-          <div>
-            <LoadingButton
-              loading={isLoading}
+      {showPasswordAuth && (


We have some new conditionals in this file introduced by the new booleans: nonPasswordAuthEnabled and showPasswordAuth. Considering the file is getting fairly large, it might be nice to break the JSX inside these conditionals into separate components in separate files. This will make everything a bit more readable. Not a blocker though!

I agree

I took a hack at it (might have made a few stylistic choices, open to feedback)

Also turned these var && into <Maybes as I saw that used all over the codebase (and might have to steal that for myself)

Looks beautiful! Thanks for leaving things better than you found them :)

github-actions · 2023-01-20T04:29:38Z

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

normana10 · 2023-01-20T04:33:26Z

Oops sorry pushed the golden files update from a different computer

kylecarbs · 2023-01-24T14:12:43Z

@normana10 seems like make gen isn't quite producing the same for you... want me to post a patch you can apply?

normana10 · 2023-01-25T01:34:36Z

@kylecarbs

Feel free

I keep trying to run make gen and it does nothing

Then I do a make gen -B

And I get a:

$ make gen -B
panic: could not start resource: : dial unix /var/run/docker.sock: connect: connection refused

goroutine 1 [running]:
main.main()
	/Users/normana10/git/coder/coderd/database/gen/dump/main.go:23 +0xb25
exit status 2
make: *** [Makefile:461: coderd/database/dump.sql] Error 1

(On my Mac)

Which.... totally makes sense, I don't have Docker installed on my Mac

Buuuut, when I try the same on my Fedora computer it ends up erroring out with a:

...
../docs/api/workspaces.md 99ms
../docs/manifest.json 16ms
../coderd/apidoc/swagger.json 351ms
Done in 4.37s.
ERROR: The 'yq' dependency is required, but is not available.

ERROR: One or more dependencies are not available, check above log output for more details.
make: *** [Makefile:509: site/.prettierrc.yaml] Error 1

But... yq is definitely installed

mafredri · 2023-01-25T10:51:02Z

@normana10 sorry the error is so unhelpful, you'll need to have yq v4 installed, either as yq4 or yq --version >= v4.

normana10 · 2023-01-26T03:33:35Z

$ yq --version
yq version 4.2.0

🤷

normana10 · 2023-01-26T03:59:26Z

Wait I just noticed that yq outside of nix-shell is v4 but inside it's v3 o.O

Just symlinked my install of yq to yq4 and make gen -B ran just fine

Pushing up now

normana10 · 2023-01-26T04:22:51Z

That was a roller coaster but I think it's all good now? 👍

Thanks!

kylecarbs · 2023-01-26T14:36:36Z

Woot woot. Thanks for pushing through @normana10 🥳

kylecarbs · 2023-01-30T02:06:39Z

@normana10 can you merge in main then I'll merge this in ASAP! Thank you again for being patient, this was a controversial one for us.

normana10 · 2023-01-30T05:26:41Z

@kylecarbs

Done

Sorry if I caused any issues 😬

kylecarbs · 2023-01-31T19:09:09Z

@normana10 we're now using this on https://dev.coder.com 😎

normana10 added 3 commits November 15, 2022 21:13

Allow hiding password entry, changing OpenID Connect text and OpenID …

3addcaf

…Connect icon

Docs

40bc156

Cleaning

0f4a40e

normana10 requested a review from a team as a code owner November 16, 2022 02:48

normana10 requested review from presleyp and removed request for a team November 16, 2022 02:48

normana10 added 4 commits November 15, 2022 22:05

Fix Prettier and Go test and TS compile error

90d900c

Fix LoginPage test

97b5019

Prettier

8019ec7

Fix storybook

97d9d46

presleyp requested a review from kylecarbs November 16, 2022 14:46

kylecarbs reviewed Nov 16, 2022

View reviewed changes

Add query param to un-hide password auth

0858ad0

presleyp removed their request for review November 28, 2022 15:39

github-actions Bot added the stale This issue is like stale bread. label Dec 6, 2022

normana10 added 2 commits December 5, 2022 22:05

Cleaning

6afca64

normana10 requested a review from kylecarbs December 6, 2022 04:36

github-actions Bot removed the stale This issue is like stale bread. label Dec 7, 2022

github-actions Bot added the stale This issue is like stale bread. label Dec 14, 2022

Merge branch 'main' into configurable-openid-connect-text

410fc98

mafredri reviewed Dec 14, 2022

View reviewed changes

Comment thread cli/deployment/config.go Outdated

kylecarbs mentioned this pull request Dec 14, 2022

Option to disable login form when oidc is enabled #4433

Closed

bpmct approved these changes Jan 19, 2023

View reviewed changes

Kira-Pilot reviewed Jan 19, 2023

View reviewed changes

Comment thread site/src/pages/LoginPage/LoginPage.test.tsx

Kira-Pilot approved these changes Jan 19, 2023

View reviewed changes

PR Feedback and split up SignInForm.tsx

77f0fb2

Updated golden files

1ed7911

normana10 force-pushed the configurable-openid-connect-text branch from 6e20df2 to 1ed7911 Compare January 20, 2023 04:33

Fix auto-genned-files

e31a37e

normana10 added 2 commits January 25, 2023 23:01

make gen -B

9207175

Revert provisioner files?

201d424

Merge branch 'main' into configurable-openid-connect-text

8333256

normana10 changed the title ~~Allow hiding password auth, changing OpenID Connect text and OpenID Connect icon~~ feat: Allow hiding password auth, changing OpenID Connect text and OpenID Connect icon Jan 30, 2023

normana10 and others added 2 commits January 30, 2023 00:39

Fix lint error

7ce525e

Merge branch 'main' into configurable-openid-connect-text

3d5ec00

kylecarbs enabled auto-merge (squash) January 31, 2023 18:26

kylecarbs merged commit 69fce04 into coder:main Jan 31, 2023

github-actions Bot locked and limited conversation to collaborators Jan 31, 2023

github-actions Bot deleted the configurable-openid-connect-text branch May 17, 2024 00:30

Conversation

normana10 commented Nov 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kylecarbs left a comment

Choose a reason for hiding this comment

Uh oh!

bpmct commented Nov 22, 2022

Uh oh!

normana10 commented Nov 24, 2022

Uh oh!

github-actions Bot commented Dec 6, 2022

Uh oh!

github-actions Bot commented Dec 14, 2022

Uh oh!

mafredri commented Dec 14, 2022

Uh oh!

mafredri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kylecarbs commented Dec 14, 2022

Uh oh!

bpmct commented Dec 14, 2022

Uh oh!

bpmct left a comment

Choose a reason for hiding this comment

Uh oh!

Kira-Pilot Jan 19, 2023

Choose a reason for hiding this comment

Uh oh!

normana10 Jan 20, 2023

Choose a reason for hiding this comment

Uh oh!

Kira-Pilot Jan 20, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Jan 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

normana10 commented Jan 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kylecarbs commented Jan 24, 2023

Uh oh!

normana10 commented Jan 25, 2023

Uh oh!

mafredri commented Jan 25, 2023

Uh oh!

normana10 commented Jan 26, 2023

Uh oh!

normana10 commented Jan 26, 2023

Uh oh!

normana10 commented Jan 26, 2023

Uh oh!

kylecarbs commented Jan 26, 2023

Uh oh!

kylecarbs commented Jan 30, 2023

Uh oh!

normana10 commented Jan 30, 2023

Uh oh!

kylecarbs commented Jan 31, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

normana10 commented Nov 16, 2022 •

edited

Loading

github-actions Bot commented Jan 20, 2023 •

edited

Loading

normana10 commented Jan 20, 2023 •

edited

Loading