mirror of https://github.com/coder/coder.git
fix: allow disabling all password auth even if owner (#6193)
* 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
This commit is contained in:
parent
41ae01d2e9
commit
51f17b1820
|
@ -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 {
|
||||
|
|
|
@ -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{
|
||||
|
|
|
@ -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) {
|
||||
|
|
|
@ -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,
|
||||
|
|
|
@ -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,7 +110,7 @@ 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}
|
||||
|
@ -116,7 +118,7 @@ export const SignInForm: FC<React.PropsWithChildren<SignInFormProps>> = ({
|
|||
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>
|
||||
|
|
|
@ -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 = () => {
|
|||
<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"
|
||||
|
|
|
@ -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(),
|
||||
},
|
||||
},
|
||||
)
|
Loading…
Reference in New Issue