fix: allow disabling all password auth even if owner (#6193)

kylecarbs · web-flow · commit 51f17b182017 · 2023-02-14T08:58:12.000-06:00
* fix: allow disabling all password auth even if owner

Removes any and all ability to auth with a password.

* Hide create user if password auth is disabled
diff --git a/coderd/userauth.go b/coderd/userauth.go
@@ -23,7 +23,6 @@ import (
 	"github.com/coder/coder/coderd/database/dbauthz"
 	"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"
 )
@@ -90,19 +89,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/coderd/users.go b/coderd/users.go
@@ -299,6 +299,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
@@ -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/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
@@ -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<React.PropsWithChildren<SignInFormProps>> = ({
   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,15 +110,15 @@ export const SignInForm: FC<React.PropsWithChildren<SignInFormProps>> = ({
         {loginPageTranslation.t("signInTo")}{" "}
         <strong>{commonTranslation.t("coder")}</strong>
       </h1>
-      <Maybe condition={showPasswordAuth}>
+      <Maybe condition={passwordEnabled && showPasswordAuth}>
         <PasswordSignInForm
           loginErrors={loginErrors}
           onSubmit={onSubmit}
           initialTouched={initialTouched}
           isLoading={isLoading}
         />
       </Maybe>
-      <Maybe condition={showPasswordAuth && oAuthEnabled}>
+      <Maybe condition={passwordEnabled && showPasswordAuth && oAuthEnabled}>
         <div className={styles.divider}>
           <div className={styles.dividerLine} />
           <div className={styles.dividerLabel}>Or</div>
@@ -131,7 +133,14 @@ export const SignInForm: FC<React.PropsWithChildren<SignInFormProps>> = ({
         />
       </Maybe>
 
-      <Maybe condition={!showPasswordAuth}>
+      <Maybe condition={!passwordEnabled && !oAuthEnabled}>
+        <AlertBanner
+          severity="error"
+          text="No authentication methods configured!"
+        />
+      </Maybe>
+
+      <Maybe condition={passwordEnabled && !showPasswordAuth}>
         <div className={styles.divider}>
           <div className={styles.dividerLine} />
           <div className={styles.dividerLabel}>Or</div>
diff --git 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,13 +16,15 @@ 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"
 
 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 = () => {
         <PageHeader
           actions={
             <>
-              {canCreateUser && (
-                <Button
-                  onClick={() => {
-                    navigate("/users/create")
-                  }}
-                  startIcon={<PersonAdd />}
-                >
-                  Create user
-                </Button>
-              )}
+              {canCreateUser &&
+                authMethods.context.authMethods?.password.enabled && (
+                  <Button
+                    onClick={() => {
+                      navigate("/users/create")
+                    }}
+                    startIcon={<PersonAdd />}
+                  >
+                    Create user
+                  </Button>
+                )}
               {canCreateGroup && isTemplateRBACEnabled && (
                 <Link
                   underline="none"
diff --git a/site/src/xServices/auth/authMethodsXService.ts b/site/src/xServices/auth/authMethodsXService.ts
@@ -0,0 +1,55 @@
+import { assign, createMachine } from "xstate"
+import * as TypeGen from "api/typesGenerated"
+import * as API from "api/api"
+
+export interface AuthMethodsContext {
+  authMethods?: TypeGen.AuthMethods
+  error?: Error | unknown
+}
+
+export const authMethodsXService = createMachine(
+  {
+    id: "authMethods",
+    predictableActionArguments: true,
+    tsTypes: {} as import("./authMethodsXService.typegen").Typegen0,
+    schema: {
+      context: {} as AuthMethodsContext,
+      services: {} as {
+        getAuthMethods: {
+          data: TypeGen.AuthMethods
+        }
+      },
+    },
+    context: {},
+    initial: "gettingAuthMethods",
+    states: {
+      gettingAuthMethods: {
+        invoke: {
+          src: "getAuthMethods",
+          onDone: {
+            target: "idle",
+            actions: ["assignAuthMethods"],
+          },
+          onError: {
+            target: "idle",
+            actions: ["setError"],
+          },
+        },
+      },
+      idle: {},
+    },
+  },
+  {
+    actions: {
+      assignAuthMethods: assign({
+        authMethods: (_, event) => event.data,
+      }),
+      setError: assign({
+        error: (_, event) => event.data,
+      }),
+    },
+    services: {
+      getAuthMethods: () => API.getAuthMethods(),
+    },
+  },
+)
