From 50d0dcbe7560059be3fbaede62dc76609fb60a40 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Mon, 13 Feb 2023 21:30:20 +0000 Subject: [PATCH 1/2] fix: allow disabling all password auth even if owner Removes any and all ability to auth with a password. --- coderd/userauth.go | 18 ++++------------- .../SignInForm/SignInForm.stories.tsx | 20 +++++++++++++++++++ site/src/components/SignInForm/SignInForm.tsx | 15 +++++++++++--- 3 files changed, 36 insertions(+), 17 deletions(-) diff --git a/coderd/userauth.go b/coderd/userauth.go index 3518481d508fb..f53eb3f2b6e8c 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -23,7 +23,6 @@ import ( "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/coderd/httpmw" - "github.com/coder/coder/coderd/rbac" "github.com/coder/coder/coderd/userpassword" "github.com/coder/coder/codersdk" ) @@ -89,19 +88,10 @@ func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) { // If password authentication is disabled and the user does not have the // owner role, block the request. if api.DeploymentConfig.DisablePasswordAuth.Value { - permitted := false - for _, role := range user.RBACRoles { - if role == rbac.RoleOwner() { - permitted = true - break - } - } - if !permitted { - httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{ - Message: "Password authentication is disabled. Only administrators can sign in with password authentication.", - }) - return - } + httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{ + Message: "Password authentication is disabled.", + }) + return } if user.LoginType != database.LoginTypePassword { diff --git a/site/src/components/SignInForm/SignInForm.stories.tsx b/site/src/components/SignInForm/SignInForm.stories.tsx index 02bcd1c53e3da..c7dee4106346b 100644 --- a/site/src/components/SignInForm/SignInForm.stories.tsx +++ b/site/src/components/SignInForm/SignInForm.stories.tsx @@ -116,6 +116,26 @@ WithOIDC.args = { }, } +export const WithOIDCWithoutPassword = Template.bind({}) +WithOIDCWithoutPassword.args = { + ...SignedOut.args, + authMethods: { + password: { enabled: false }, + github: { enabled: false }, + oidc: { enabled: true, signInText: "", iconUrl: "" }, + }, +} + +export const WithoutAny = Template.bind({}) +WithoutAny.args = { + ...SignedOut.args, + authMethods: { + password: { enabled: false }, + github: { enabled: false }, + oidc: { enabled: false, signInText: "", iconUrl: "" }, + }, +} + export const WithGithubAndOIDC = Template.bind({}) WithGithubAndOIDC.args = { ...SignedOut.args, diff --git a/site/src/components/SignInForm/SignInForm.tsx b/site/src/components/SignInForm/SignInForm.tsx index 870f67b81b97e..091e21a17cdc5 100644 --- a/site/src/components/SignInForm/SignInForm.tsx +++ b/site/src/components/SignInForm/SignInForm.tsx @@ -9,6 +9,7 @@ import { OAuthSignInForm } from "./OAuthSignInForm" import { BuiltInAuthFormValues } from "./SignInForm.types" import Button from "@material-ui/core/Button" import EmailIcon from "@material-ui/icons/EmailOutlined" +import { AlertBanner } from "components/AlertBanner/AlertBanner" export enum LoginErrors { AUTH_ERROR = "authError", @@ -94,6 +95,7 @@ export const SignInForm: FC> = ({ const oAuthEnabled = Boolean( authMethods?.github.enabled || authMethods?.oidc.enabled, ) + const passwordEnabled = authMethods?.password.enabled ?? true // Hide password auth by default if any OAuth method is enabled const [showPasswordAuth, setShowPasswordAuth] = useState(!oAuthEnabled) @@ -108,7 +110,7 @@ export const SignInForm: FC> = ({ {loginPageTranslation.t("signInTo")}{" "} {commonTranslation.t("coder")} - + > = ({ isLoading={isLoading} /> - +
Or
@@ -131,7 +133,14 @@ export const SignInForm: FC> = ({ /> - + + + + +
Or
From b4c2a42a667a2d85f5deb0f4a72c9e84a89a2027 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Mon, 13 Feb 2023 22:28:31 +0000 Subject: [PATCH 2/2] Hide create user if password auth is disabled --- coderd/users.go | 9 +++ coderd/users_test.go | 17 +----- .../components/UsersLayout/UsersLayout.tsx | 24 ++++---- .../src/xServices/auth/authMethodsXService.ts | 55 +++++++++++++++++++ 4 files changed, 80 insertions(+), 25 deletions(-) create mode 100644 site/src/xServices/auth/authMethodsXService.ts diff --git a/coderd/users.go b/coderd/users.go index 9f49a74aec64b..1f2b8fc5f8650 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -293,6 +293,15 @@ func (api *API) postUser(rw http.ResponseWriter, r *http.Request) { return } + // If password auth is disabled, don't allow new users to be + // created with a password! + if api.DeploymentConfig.DisablePasswordAuth.Value { + httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{ + Message: "You cannot manually provision new users with password authentication disabled!", + }) + return + } + // TODO: @emyrk Authorize the organization create if the createUser will do that. _, err := api.Database.GetUserByEmailOrUsername(ctx, database.GetUserByEmailOrUsernameParams{ diff --git a/coderd/users_test.go b/coderd/users_test.go index 283a50a25e9d8..0c3a6fb850225 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -187,7 +187,6 @@ func TestPostLogin(t *testing.T) { t.Parallel() dc := coderdtest.DeploymentConfig(t) - dc.DisablePasswordAuth.Value = true client := coderdtest.New(t, &coderdtest.Options{ DeploymentConfig: dc, }) @@ -207,6 +206,8 @@ func TestPostLogin(t *testing.T) { }) require.NoError(t, err) + dc.DisablePasswordAuth.Value = true + userClient := codersdk.New(client.URL) _, err = userClient.LoginWithPassword(ctx, codersdk.LoginWithPasswordRequest{ Email: user.Email, @@ -217,20 +218,6 @@ func TestPostLogin(t *testing.T) { require.ErrorAs(t, err, &apiErr) require.Equal(t, http.StatusForbidden, apiErr.StatusCode()) require.Contains(t, apiErr.Message, "Password authentication is disabled") - - // Promote the user account to an owner. - _, err = client.UpdateUserRoles(ctx, user.ID.String(), codersdk.UpdateRoles{ - Roles: []string{rbac.RoleOwner(), rbac.RoleMember()}, - }) - require.NoError(t, err) - - // Login with the user account. - res, err := userClient.LoginWithPassword(ctx, codersdk.LoginWithPasswordRequest{ - Email: user.Email, - Password: password, - }) - require.NoError(t, err) - require.NotEmpty(t, res.SessionToken) }) t.Run("Success", func(t *testing.T) { diff --git a/site/src/components/UsersLayout/UsersLayout.tsx b/site/src/components/UsersLayout/UsersLayout.tsx index e57f24f84159c..b7cd81aa5367e 100644 --- a/site/src/components/UsersLayout/UsersLayout.tsx +++ b/site/src/components/UsersLayout/UsersLayout.tsx @@ -3,6 +3,7 @@ import Link from "@material-ui/core/Link" import { makeStyles } from "@material-ui/core/styles" import GroupAdd from "@material-ui/icons/GroupAddOutlined" import PersonAdd from "@material-ui/icons/PersonAddOutlined" +import { useMachine } from "@xstate/react" import { USERS_LINK } from "components/NavbarView/NavbarView" import { PageHeader, PageHeaderTitle } from "components/PageHeader/PageHeader" import { useFeatureVisibility } from "hooks/useFeatureVisibility" @@ -15,6 +16,7 @@ import { useNavigate, } from "react-router-dom" import { combineClasses } from "util/combineClasses" +import { authMethodsXService } from "xServices/auth/authMethodsXService" import { Margins } from "../../components/Margins/Margins" import { Stack } from "../../components/Stack/Stack" @@ -22,6 +24,7 @@ export const UsersLayout: FC = () => { const styles = useStyles() const { createUser: canCreateUser, createGroup: canCreateGroup } = usePermissions() + const [authMethods] = useMachine(authMethodsXService) const navigate = useNavigate() const { template_rbac: isTemplateRBACEnabled } = useFeatureVisibility() @@ -31,16 +34,17 @@ export const UsersLayout: FC = () => { - {canCreateUser && ( - - )} + {canCreateUser && + authMethods.context.authMethods?.password.enabled && ( + + )} {canCreateGroup && isTemplateRBACEnabled && ( event.data, + }), + setError: assign({ + error: (_, event) => event.data, + }), + }, + services: { + getAuthMethods: () => API.getAuthMethods(), + }, + }, +)