From e388f1cb7cff6d9f2064102c0f097c94e484ff88 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Wed, 21 Jun 2023 16:48:15 +0000 Subject: [PATCH 01/10] Minor form spacing and verbiage improvements --- .../SettingsAccountForm/SettingsAccountForm.tsx | 12 ++++++------ site/src/components/SettingsLayout/Section.tsx | 2 +- .../SettingsSecurityForm/SettingsSecurityForm.tsx | 10 +++++----- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx b/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx index 1525e1ecb37a9..0141146e8f84b 100644 --- a/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx +++ b/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx @@ -8,8 +8,8 @@ import { onChangeTrimmed, } from "../../utils/formUtils" import { LoadingButton } from "../LoadingButton/LoadingButton" -import { Stack } from "../Stack/Stack" import { ErrorAlert } from "components/Alert/ErrorAlert" +import { Form, FormFields } from "components/Form/Form" export interface AccountFormValues { username: string @@ -18,7 +18,7 @@ export interface AccountFormValues { export const Language = { usernameLabel: "Username", emailLabel: "Email", - updateSettings: "Update settings", + updateSettings: "Update account", } const validationSchema = Yup.object({ @@ -59,8 +59,8 @@ export const AccountForm: FC> = ({ return ( <> -
- + + {Boolean(updateProfileError) && ( )} @@ -91,8 +91,8 @@ export const AccountForm: FC> = ({ {isLoading ? "" : Language.updateSettings} - -
+ + ) } diff --git a/site/src/components/SettingsLayout/Section.tsx b/site/src/components/SettingsLayout/Section.tsx index 29e761fdec030..f4300ab433921 100644 --- a/site/src/components/SettingsLayout/Section.tsx +++ b/site/src/components/SettingsLayout/Section.tsx @@ -35,7 +35,7 @@ export const Section: SectionFC = ({ {(title || description) && (
- {title && {title}} + {title && {title}} {description && typeof description === "string" && ( {description} diff --git a/site/src/components/SettingsSecurityForm/SettingsSecurityForm.tsx b/site/src/components/SettingsSecurityForm/SettingsSecurityForm.tsx index 0335b4cfd77c6..55668bcdde2ef 100644 --- a/site/src/components/SettingsSecurityForm/SettingsSecurityForm.tsx +++ b/site/src/components/SettingsSecurityForm/SettingsSecurityForm.tsx @@ -4,8 +4,8 @@ import { FC } from "react" import * as Yup from "yup" import { getFormHelpers } from "../../utils/formUtils" import { LoadingButton } from "../LoadingButton/LoadingButton" -import { Stack } from "../Stack/Stack" import { ErrorAlert } from "components/Alert/ErrorAlert" +import { Form, FormFields } from "components/Form/Form" interface SecurityFormValues { old_password: string @@ -70,8 +70,8 @@ export const SecurityForm: FC = ({ return ( <> -
- + + {Boolean(updateSecurityError) && ( )} @@ -106,8 +106,8 @@ export const SecurityForm: FC = ({ {isLoading ? "" : Language.updatePassword}
- - + + ) } From 7db91b458b19af7b76139081fb3a4edd600e09d9 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Wed, 21 Jun 2023 18:24:03 +0000 Subject: [PATCH 02/10] Add basic flow using mocks --- .vscode/settings.json | 2 +- .../SettingsAccountForm.test.tsx | 6 + .../SettingsAccountForm.tsx | 10 +- .../AccountPage/AccountPage.tsx | 220 ++++++++++++++++-- 4 files changed, 218 insertions(+), 20 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 1ae5f3419f969..ac3c9e10adf17 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -212,5 +212,5 @@ // We often use a version of TypeScript that's ahead of the version shipped // with VS Code. "typescript.tsdk": "./site/node_modules/typescript/lib", - "prettier.prettierPath": "./node_modules/prettier" + "prettier.prettierPath": "./site/node_modules/prettier" } diff --git a/site/src/components/SettingsAccountForm/SettingsAccountForm.test.tsx b/site/src/components/SettingsAccountForm/SettingsAccountForm.test.tsx index df5268883ad2e..bebaf19e7fefd 100644 --- a/site/src/components/SettingsAccountForm/SettingsAccountForm.test.tsx +++ b/site/src/components/SettingsAccountForm/SettingsAccountForm.test.tsx @@ -24,6 +24,9 @@ describe("AccountForm", () => { onSubmit={() => { return }} + onChangeToOIDCAuth={() => { + return + }} />, ) @@ -54,6 +57,9 @@ describe("AccountForm", () => { onSubmit={() => { return }} + onChangeToOIDCAuth={() => { + return + }} />, ) diff --git a/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx b/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx index 0141146e8f84b..b2e1feb595134 100644 --- a/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx +++ b/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx @@ -10,6 +10,8 @@ import { import { LoadingButton } from "../LoadingButton/LoadingButton" import { ErrorAlert } from "components/Alert/ErrorAlert" import { Form, FormFields } from "components/Form/Form" +import { Stack } from "components/Stack/Stack" +import Button from "@mui/material/Button" export interface AccountFormValues { username: string @@ -34,6 +36,7 @@ export interface AccountFormProps { updateProfileError?: Error | unknown // initialTouched is only used for testing the error state of the form. initialTouched?: FormikTouched + onChangeToOIDCAuth: () => void } export const AccountForm: FC> = ({ @@ -44,6 +47,7 @@ export const AccountForm: FC> = ({ initialValues, updateProfileError, initialTouched, + onChangeToOIDCAuth }) => { const form: FormikContextType = useFormik({ @@ -80,7 +84,7 @@ export const AccountForm: FC> = ({ label={Language.usernameLabel} /> -
+ > = ({ > {isLoading ? "" : Language.updateSettings} -
+ + + diff --git a/site/src/pages/UserSettingsPage/AccountPage/AccountPage.tsx b/site/src/pages/UserSettingsPage/AccountPage/AccountPage.tsx index bc2cdc793fb2a..05e4b03d4ba67 100644 --- a/site/src/pages/UserSettingsPage/AccountPage/AccountPage.tsx +++ b/site/src/pages/UserSettingsPage/AccountPage/AccountPage.tsx @@ -1,9 +1,29 @@ -import { FC } from "react" +import { FC, useState } from "react" import { Section } from "../../../components/SettingsLayout/Section" import { AccountForm } from "../../../components/SettingsAccountForm/SettingsAccountForm" import { useAuth } from "components/AuthProvider/AuthProvider" import { useMe } from "hooks/useMe" import { usePermissions } from "hooks/usePermissions" +import { Dialog } from "components/Dialogs/Dialog" +import TextField from "@mui/material/TextField" +import { FormFields, VerticalForm } from "components/Form/Form" +import { LoadingButton } from "components/LoadingButton/LoadingButton" +import Box from "@mui/material/Box" +import GitHubIcon from "@mui/icons-material/GitHub" +import KeyIcon from "@mui/icons-material/VpnKey" +import Button from "@mui/material/Button" +import { MockAuthMethods } from "testHelpers/entities" +import CircularProgress from "@mui/material/CircularProgress" +import { useLocation } from "react-router-dom" +import { retrieveRedirect } from "utils/redirect" +import Typography from "@mui/material/Typography" + +type OIDCState = + | { status: "closed" } + | { status: "confirmPassword"; error?: unknown } + | { status: "confirmingPassword" } + | { status: "selectOIDCProvider" } + | { status: "updatingProvider" } export const AccountPage: FC = () => { const [authState, authSend] = useAuth() @@ -11,25 +31,191 @@ export const AccountPage: FC = () => { const permissions = usePermissions() const { updateProfileError } = authState.context const canEditUsers = permissions && permissions.updateUsers + const [OIDCState, setOIDCState] = useState({ + status: "closed", + }) + const location = useLocation() + const redirectTo = retrieveRedirect(location.search) return ( -
- { - authSend({ - type: "UPDATE_PROFILE", - data, - }) - }} + <> +
+ { + authSend({ + type: "UPDATE_PROFILE", + data, + }) + }} + onChangeToOIDCAuth={() => { + setOIDCState({ status: "confirmPassword" }) + }} + /> +
+ -
+ + ) +} + +const OIDCChangeModal = ({ + state, + onChangeState, + redirectTo, +}: { + redirectTo: string + state: OIDCState + onChangeState: (newState: OIDCState) => void +}) => { + const authMethods = MockAuthMethods + + const updateProvider = (provider: string) => { + onChangeState({ status: "updatingProvider" }) + setTimeout(() => { + window.location.href = `/api/v2/users/oidc/callback?oidc_merge_state=something&redirect=${encodeURIComponent( + redirectTo, + )}` + }, 1000) + } + + return ( + onChangeState({ status: "closed" })} + sx={{ + "& .MuiPaper-root": { + padding: (theme) => theme.spacing(5), + backgroundColor: (theme) => theme.palette.background.paper, + border: (theme) => `1px solid ${theme.palette.divider}`, + width: 440, + }, + }} + > + {(state.status === "confirmPassword" || + state.status === "confirmingPassword") && ( +
+ + Confirm password + + theme.palette.text.secondary, + mt: 1, + mb: 3, + }} + > + We need to confirm your identity in order to proceed with the + authentication changes + + { + e.preventDefault() + onChangeState({ status: "confirmingPassword" }) + await new Promise((resolve) => setTimeout(resolve, 1000)) + onChangeState({ status: "selectOIDCProvider" }) + }} + > + + + + Confirm password + + + +
+ )} + + {(state.status === "selectOIDCProvider" || + state.status === "updatingProvider") && ( +
+ + Select a provider + + theme.palette.text.secondary, + mt: 1, + mb: 3, + }} + > + After selecting the provider, you will be redirected to the + provider‘s authentication page. + + + + + + {state.status === "updatingProvider" && ( + theme.spacing(2), + gap: 1, + fontSize: 13, + color: (theme) => theme.palette.text.secondary, + }} + > + + Updating authentication method... + + )} +
+ )} + ) } From 118383df88c0dff11b92f1bb35c26773976acb28 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Thu, 22 Jun 2023 14:00:14 +0000 Subject: [PATCH 03/10] Change the UI flow --- site/src/api/api.ts | 33 +- .../SettingsAccountForm.test.tsx | 6 - .../SettingsAccountForm.tsx | 10 +- .../AccountPage/AccountPage.tsx | 375 ++++++++++-------- 4 files changed, 220 insertions(+), 204 deletions(-) diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 2ce560d22880c..0dff38cff4786 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -108,33 +108,12 @@ export const login = async ( return response.data } -export const convertToOauth = async ( - email: string, - password: string, - to_login_type: string, -): Promise => { - const payload = JSON.stringify({ - email, - password, - to_login_type, - }) - - try { - const response = await axios.post( - "/api/v2/users/convert-login", - payload, - { - headers: { ...CONTENT_TYPE_JSON }, - }, - ) - return response.data - } catch (error) { - if (axios.isAxiosError(error) && error.response?.status === 401) { - return undefined - } - - throw error - } +export const convertToOAUTH = async (request: TypesGen.ConvertLoginRequest) => { + const response = await axios.post( + "/api/v2/users/convert-login", + request, + ) + return response.data } export const logout = async (): Promise => { diff --git a/site/src/components/SettingsAccountForm/SettingsAccountForm.test.tsx b/site/src/components/SettingsAccountForm/SettingsAccountForm.test.tsx index bebaf19e7fefd..df5268883ad2e 100644 --- a/site/src/components/SettingsAccountForm/SettingsAccountForm.test.tsx +++ b/site/src/components/SettingsAccountForm/SettingsAccountForm.test.tsx @@ -24,9 +24,6 @@ describe("AccountForm", () => { onSubmit={() => { return }} - onChangeToOIDCAuth={() => { - return - }} />, ) @@ -57,9 +54,6 @@ describe("AccountForm", () => { onSubmit={() => { return }} - onChangeToOIDCAuth={() => { - return - }} />, ) diff --git a/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx b/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx index b2e1feb595134..0141146e8f84b 100644 --- a/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx +++ b/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx @@ -10,8 +10,6 @@ import { import { LoadingButton } from "../LoadingButton/LoadingButton" import { ErrorAlert } from "components/Alert/ErrorAlert" import { Form, FormFields } from "components/Form/Form" -import { Stack } from "components/Stack/Stack" -import Button from "@mui/material/Button" export interface AccountFormValues { username: string @@ -36,7 +34,6 @@ export interface AccountFormProps { updateProfileError?: Error | unknown // initialTouched is only used for testing the error state of the form. initialTouched?: FormikTouched - onChangeToOIDCAuth: () => void } export const AccountForm: FC> = ({ @@ -47,7 +44,6 @@ export const AccountForm: FC> = ({ initialValues, updateProfileError, initialTouched, - onChangeToOIDCAuth }) => { const form: FormikContextType = useFormik({ @@ -84,7 +80,7 @@ export const AccountForm: FC> = ({ label={Language.usernameLabel} /> - +
> = ({ > {isLoading ? "" : Language.updateSettings} - - - +
diff --git a/site/src/pages/UserSettingsPage/AccountPage/AccountPage.tsx b/site/src/pages/UserSettingsPage/AccountPage/AccountPage.tsx index 05e4b03d4ba67..3fd8e823f9134 100644 --- a/site/src/pages/UserSettingsPage/AccountPage/AccountPage.tsx +++ b/site/src/pages/UserSettingsPage/AccountPage/AccountPage.tsx @@ -1,29 +1,34 @@ -import { FC, useState } from "react" +import { ComponentProps, FC, useState } from "react" import { Section } from "../../../components/SettingsLayout/Section" import { AccountForm } from "../../../components/SettingsAccountForm/SettingsAccountForm" import { useAuth } from "components/AuthProvider/AuthProvider" import { useMe } from "hooks/useMe" import { usePermissions } from "hooks/usePermissions" -import { Dialog } from "components/Dialogs/Dialog" import TextField from "@mui/material/TextField" -import { FormFields, VerticalForm } from "components/Form/Form" -import { LoadingButton } from "components/LoadingButton/LoadingButton" import Box from "@mui/material/Box" import GitHubIcon from "@mui/icons-material/GitHub" import KeyIcon from "@mui/icons-material/VpnKey" import Button from "@mui/material/Button" -import { MockAuthMethods } from "testHelpers/entities" -import CircularProgress from "@mui/material/CircularProgress" import { useLocation } from "react-router-dom" import { retrieveRedirect } from "utils/redirect" import Typography from "@mui/material/Typography" +import { convertToOAUTH, getAuthMethods } from "api/api" +import { AuthMethods, LoginType } from "api/typesGenerated" +import Skeleton from "@mui/material/Skeleton" +import { Stack } from "components/Stack/Stack" +import { useMutation, useQuery } from "@tanstack/react-query" +import { ConfirmDialog } from "components/Dialogs/ConfirmDialog/ConfirmDialog" +import { getErrorMessage } from "api/errors" -type OIDCState = - | { status: "closed" } - | { status: "confirmPassword"; error?: unknown } - | { status: "confirmingPassword" } - | { status: "selectOIDCProvider" } - | { status: "updatingProvider" } +type LoginTypeConfirmation = + | { + open: false + selectedType: undefined + } + | { + open: true + selectedType: LoginType + } export const AccountPage: FC = () => { const [authState, authSend] = useAuth() @@ -31,14 +36,24 @@ export const AccountPage: FC = () => { const permissions = usePermissions() const { updateProfileError } = authState.context const canEditUsers = permissions && permissions.updateUsers - const [OIDCState, setOIDCState] = useState({ - status: "closed", - }) const location = useLocation() const redirectTo = retrieveRedirect(location.search) + const [loginTypeConfirmation, setLoginTypeConfirmation] = + useState({ open: false, selectedType: undefined }) + const { data: authMethods } = useQuery({ + queryKey: ["authMethods"], + queryFn: getAuthMethods, + }) + const loginTypeMutation = useMutation(convertToOAUTH, { + onSuccess: (data) => { + window.location.href = `/api/v2/users/oidc/callback?oidc_merge_state=${ + data.state_string + }&redirect=${encodeURIComponent(redirectTo)}` + }, + }) return ( - <> +
{ data, }) }} - onChangeToOIDCAuth={() => { - setOIDCState({ status: "confirmPassword" }) - }} />
- + + {authMethods ? ( + authMethods.me_login_type === "password" ? ( + <> + {authMethods.github.enabled && ( + + setLoginTypeConfirmation({ + open: true, + selectedType: "github", + }) + } + > + GitHub + + )} + {authMethods.oidc.enabled && ( + + setLoginTypeConfirmation({ + open: true, + selectedType: "oidc", + }) + } + > + {getOIDCLabel(authMethods)} + + )} + + ) : ( + <> + {authMethods.me_login_type === "github" && ( + + Authenticated with GitHub + + )} + + {authMethods.me_login_type === "oidc" && ( + + Authenticated with {getOIDCLabel(authMethods)} + + )} + + ) + ) : ( + <> + + + + )} + + + + { + setLoginTypeConfirmation({ open: false, selectedType: undefined }) + loginTypeMutation.reset() + }} + onConfirm={(password) => { + if (!loginTypeConfirmation.selectedType) { + throw new Error("No login type selected") + } + loginTypeMutation.mutate({ + to_login_type: loginTypeConfirmation.selectedType, + email: me.email, + password, + }) + }} /> - + + ) +} + +const GitHubButton = (props: ComponentProps) => { + return ( + - - - {state.status === "updatingProvider" && ( - theme.spacing(2), - gap: 1, - fontSize: 13, - color: (theme) => theme.palette.text.secondary, - }} - > - - Updating authentication method... - - )} -
- )} - + }} + error={Boolean(error)} + helperText={ + error + ? getErrorMessage(error, "Your password is incorrect") + : undefined + } + name="confirm-password" + id="confirm-password" + value={password} + onChange={(e) => setPassword(e.currentTarget.value)} + label="Confirm your password" + type="password" + /> + + } + /> ) } From 251b41bbdb8ada03e7ee09f71c0a47ab3a3ba3d1 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Thu, 22 Jun 2023 16:56:59 +0000 Subject: [PATCH 04/10] End --- .../SettingsSecurityForm.tsx | 11 + .../AccountPage/AccountPage.tsx | 263 +----------------- .../SecurityPage/SecurityPage.tsx | 55 ++-- .../SecurityPage/SingleSignOnSection.tsx | 259 +++++++++++++++++ 4 files changed, 322 insertions(+), 266 deletions(-) create mode 100644 site/src/pages/UserSettingsPage/SecurityPage/SingleSignOnSection.tsx diff --git a/site/src/components/SettingsSecurityForm/SettingsSecurityForm.tsx b/site/src/components/SettingsSecurityForm/SettingsSecurityForm.tsx index 55668bcdde2ef..1f5796d38a7b6 100644 --- a/site/src/components/SettingsSecurityForm/SettingsSecurityForm.tsx +++ b/site/src/components/SettingsSecurityForm/SettingsSecurityForm.tsx @@ -6,6 +6,7 @@ import { getFormHelpers } from "../../utils/formUtils" import { LoadingButton } from "../LoadingButton/LoadingButton" import { ErrorAlert } from "components/Alert/ErrorAlert" import { Form, FormFields } from "components/Form/Form" +import { Alert } from "components/Alert/Alert" interface SecurityFormValues { old_password: string @@ -41,6 +42,7 @@ const validationSchema = Yup.object({ }) export interface SecurityFormProps { + disabled?: boolean isLoading: boolean initialValues: SecurityFormValues onSubmit: (values: SecurityFormValues) => void @@ -50,6 +52,7 @@ export interface SecurityFormProps { } export const SecurityForm: FC = ({ + disabled, isLoading, onSubmit, initialValues, @@ -68,6 +71,14 @@ export const SecurityForm: FC = ({ updateSecurityError, ) + if (disabled) { + return ( + + Password changes are only allowed for password based accounts. + + ) + } + return ( <>
diff --git a/site/src/pages/UserSettingsPage/AccountPage/AccountPage.tsx b/site/src/pages/UserSettingsPage/AccountPage/AccountPage.tsx index 3fd8e823f9134..bc2cdc793fb2a 100644 --- a/site/src/pages/UserSettingsPage/AccountPage/AccountPage.tsx +++ b/site/src/pages/UserSettingsPage/AccountPage/AccountPage.tsx @@ -1,34 +1,9 @@ -import { ComponentProps, FC, useState } from "react" +import { FC } from "react" import { Section } from "../../../components/SettingsLayout/Section" import { AccountForm } from "../../../components/SettingsAccountForm/SettingsAccountForm" import { useAuth } from "components/AuthProvider/AuthProvider" import { useMe } from "hooks/useMe" import { usePermissions } from "hooks/usePermissions" -import TextField from "@mui/material/TextField" -import Box from "@mui/material/Box" -import GitHubIcon from "@mui/icons-material/GitHub" -import KeyIcon from "@mui/icons-material/VpnKey" -import Button from "@mui/material/Button" -import { useLocation } from "react-router-dom" -import { retrieveRedirect } from "utils/redirect" -import Typography from "@mui/material/Typography" -import { convertToOAUTH, getAuthMethods } from "api/api" -import { AuthMethods, LoginType } from "api/typesGenerated" -import Skeleton from "@mui/material/Skeleton" -import { Stack } from "components/Stack/Stack" -import { useMutation, useQuery } from "@tanstack/react-query" -import { ConfirmDialog } from "components/Dialogs/ConfirmDialog/ConfirmDialog" -import { getErrorMessage } from "api/errors" - -type LoginTypeConfirmation = - | { - open: false - selectedType: undefined - } - | { - open: true - selectedType: LoginType - } export const AccountPage: FC = () => { const [authState, authSend] = useAuth() @@ -36,235 +11,25 @@ export const AccountPage: FC = () => { const permissions = usePermissions() const { updateProfileError } = authState.context const canEditUsers = permissions && permissions.updateUsers - const location = useLocation() - const redirectTo = retrieveRedirect(location.search) - const [loginTypeConfirmation, setLoginTypeConfirmation] = - useState({ open: false, selectedType: undefined }) - const { data: authMethods } = useQuery({ - queryKey: ["authMethods"], - queryFn: getAuthMethods, - }) - const loginTypeMutation = useMutation(convertToOAUTH, { - onSuccess: (data) => { - window.location.href = `/api/v2/users/oidc/callback?oidc_merge_state=${ - data.state_string - }&redirect=${encodeURIComponent(redirectTo)}` - }, - }) return ( - -
- { - authSend({ - type: "UPDATE_PROFILE", - data, - }) - }} - /> -
- -
- - {authMethods ? ( - authMethods.me_login_type === "password" ? ( - <> - {authMethods.github.enabled && ( - - setLoginTypeConfirmation({ - open: true, - selectedType: "github", - }) - } - > - GitHub - - )} - {authMethods.oidc.enabled && ( - - setLoginTypeConfirmation({ - open: true, - selectedType: "oidc", - }) - } - > - {getOIDCLabel(authMethods)} - - )} - - ) : ( - <> - {authMethods.me_login_type === "github" && ( - - Authenticated with GitHub - - )} - - {authMethods.me_login_type === "oidc" && ( - - Authenticated with {getOIDCLabel(authMethods)} - - )} - - ) - ) : ( - <> - - - - )} - -
- - { - setLoginTypeConfirmation({ open: false, selectedType: undefined }) - loginTypeMutation.reset() +
+ { - if (!loginTypeConfirmation.selectedType) { - throw new Error("No login type selected") - } - loginTypeMutation.mutate({ - to_login_type: loginTypeConfirmation.selectedType, - email: me.email, - password, + onSubmit={(data) => { + authSend({ + type: "UPDATE_PROFILE", + data, }) }} /> - - ) -} - -const GitHubButton = (props: ComponentProps) => { - return ( -
) } diff --git a/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.tsx b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.tsx index 75fa1290636bd..d92e7f132f383 100644 --- a/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.tsx +++ b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.tsx @@ -4,10 +4,14 @@ import { FC } from "react" import { userSecuritySettingsMachine } from "xServices/userSecuritySettings/userSecuritySettingsXService" import { Section } from "../../../components/SettingsLayout/Section" import { SecurityForm } from "../../../components/SettingsSecurityForm/SettingsSecurityForm" - -export const Language = { - title: "Security", -} +import { useQuery } from "@tanstack/react-query" +import { getAuthMethods } from "api/api" +import { + SingleSignOnSection, + useSingleSignOnSection, +} from "./SingleSignOnSection" +import { Loader } from "components/Loader/Loader" +import { Stack } from "components/Stack/Stack" export const SecurityPage: FC = () => { const me = useMe() @@ -20,21 +24,38 @@ export const SecurityPage: FC = () => { }, ) const { error } = securityState.context + const { data: authMethods } = useQuery({ + queryKey: ["authMethods"], + queryFn: getAuthMethods, + }) + const singleSignOnSection = useSingleSignOnSection() + + if (!authMethods) { + return + } return ( -
- { - securitySend({ - type: "UPDATE_SECURITY", - data, - }) - }} - /> -
+ +
+ { + securitySend({ + type: "UPDATE_SECURITY", + data, + }) + }} + /> +
+ + ) } diff --git a/site/src/pages/UserSettingsPage/SecurityPage/SingleSignOnSection.tsx b/site/src/pages/UserSettingsPage/SecurityPage/SingleSignOnSection.tsx new file mode 100644 index 0000000000000..ee4056d003af4 --- /dev/null +++ b/site/src/pages/UserSettingsPage/SecurityPage/SingleSignOnSection.tsx @@ -0,0 +1,259 @@ +import { useState } from "react" +import { Section } from "../../../components/SettingsLayout/Section" +import { useMe } from "hooks/useMe" +import TextField from "@mui/material/TextField" +import Box from "@mui/material/Box" +import GitHubIcon from "@mui/icons-material/GitHub" +import KeyIcon from "@mui/icons-material/VpnKey" +import Button from "@mui/material/Button" +import { useLocation } from "react-router-dom" +import { retrieveRedirect } from "utils/redirect" +import Typography from "@mui/material/Typography" +import { convertToOAUTH } from "api/api" +import { AuthMethods, LoginType } from "api/typesGenerated" +import Skeleton from "@mui/material/Skeleton" +import { Stack } from "components/Stack/Stack" +import { useMutation } from "@tanstack/react-query" +import { ConfirmDialog } from "components/Dialogs/ConfirmDialog/ConfirmDialog" +import { getErrorMessage } from "api/errors" +import CheckCircleOutlined from "@mui/icons-material/CheckCircleOutlined" + +type LoginTypeConfirmation = + | { + open: false + selectedType: undefined + } + | { + open: true + selectedType: LoginType + } + +export const useSingleSignOnSection = () => { + const me = useMe() + const location = useLocation() + const redirectTo = retrieveRedirect(location.search) + const [loginTypeConfirmation, setLoginTypeConfirmation] = + useState({ open: false, selectedType: undefined }) + + const mutation = useMutation(convertToOAUTH, { + onSuccess: (data) => { + window.location.href = `/api/v2/users/oidc/callback?oidc_merge_state=${ + data.state_string + }&redirect=${encodeURIComponent(redirectTo)}` + }, + }) + + const openConfirmation = (selectedType: LoginType) => { + setLoginTypeConfirmation({ open: true, selectedType }) + } + + const closeConfirmation = () => { + setLoginTypeConfirmation({ open: false, selectedType: undefined }) + mutation.reset() + } + + const confirm = (password: string) => { + if (!loginTypeConfirmation.selectedType) { + throw new Error("No login type selected") + } + mutation.mutate({ + to_login_type: loginTypeConfirmation.selectedType, + email: me.email, + password, + }) + } + + return { + mutation, + openConfirmation, + closeConfirmation, + confirm, + // We still want to show it loading when it is success so the modal does not + // change until the redirect + isUpdating: mutation.isLoading || mutation.isSuccess, + isConfirming: loginTypeConfirmation.open, + error: mutation.error, + } +} + +type SingleSignOnSectionProps = ReturnType & { + authMethods: AuthMethods +} + +export const SingleSignOnSection = ({ + authMethods, + openConfirmation, + closeConfirmation, + confirm, + isUpdating, + isConfirming, + error, +}: SingleSignOnSectionProps) => { + return ( + <> +
+ + {authMethods ? ( + authMethods.me_login_type === "password" ? ( + <> + {authMethods.github.enabled && ( + + )} + {authMethods.oidc.enabled && ( + + )} + + ) : ( + theme.palette.background.paper, + borderRadius: 1, + border: (theme) => `1px solid ${theme.palette.divider}`, + padding: 2, + display: "flex", + gap: 2, + alignItems: "center", + fontSize: 14, + }} + > + theme.palette.success.light, + fontSize: 16, + }} + /> + + Authenticated with{" "} + + {authMethods.me_login_type === "github" + ? "GitHub" + : getOIDCLabel(authMethods)} + + + + {authMethods.me_login_type === "github" ? ( + + ) : ( + + )} + + + ) + ) : ( + + )} + +
+ + + + ) +} + +const OIDCIcon = ({ authMethods }: { authMethods: AuthMethods }) => { + return authMethods.oidc.iconUrl ? ( + + ) : ( + + ) +} + +const getOIDCLabel = (authMethods: AuthMethods) => { + return authMethods.oidc.signInText || "OpenID Connect" +} + +const ConfirmLoginTypeChangeModal = ({ + open, + loading, + error, + onClose, + onConfirm, +}: { + open: boolean + loading: boolean + error: unknown + onClose: () => void + onConfirm: (password: string) => void +}) => { + const [password, setPassword] = useState("") + + const handleConfirm = () => { + onConfirm(password) + } + + return ( + { + onClose() + }} + onConfirm={handleConfirm} + hideCancel={false} + cancelText="Cancel" + confirmText="Update" + title="Change login type" + confirmLoading={loading} + description={ + + + After changing your login type, you will not be able to change it + again. Are you sure you want to proceed and change your login type? + + { + if (event.key === "Enter") { + handleConfirm() + } + }} + error={Boolean(error)} + helperText={ + error + ? getErrorMessage(error, "Your password is incorrect") + : undefined + } + name="confirm-password" + id="confirm-password" + value={password} + onChange={(e) => setPassword(e.currentTarget.value)} + label="Confirm your password" + type="password" + /> + + } + /> + ) +} From 08036e8cc3e916738822a4e59192e18364b6186b Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 23 Jun 2023 14:03:07 +0000 Subject: [PATCH 05/10] Add storybooks --- .vscode/settings.json | 1 - .../SettingsSecurityForm.tsx | 26 ++++--- .../SecurityPage/SecurityPage.tsx | 57 ++++++++++----- .../SecurityPage/SecurityPageView.stories.tsx | 70 +++++++++++++++++++ .../SecurityPage/SingleSignOnSection.tsx | 1 - site/src/testHelpers/entities.ts | 1 + 6 files changed, 124 insertions(+), 32 deletions(-) create mode 100644 site/src/pages/UserSettingsPage/SecurityPage/SecurityPageView.stories.tsx diff --git a/.vscode/settings.json b/.vscode/settings.json index ac3c9e10adf17..d028ad30211f1 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -212,5 +212,4 @@ // We often use a version of TypeScript that's ahead of the version shipped // with VS Code. "typescript.tsdk": "./site/node_modules/typescript/lib", - "prettier.prettierPath": "./site/node_modules/prettier" } diff --git a/site/src/components/SettingsSecurityForm/SettingsSecurityForm.tsx b/site/src/components/SettingsSecurityForm/SettingsSecurityForm.tsx index 1f5796d38a7b6..87151469ac772 100644 --- a/site/src/components/SettingsSecurityForm/SettingsSecurityForm.tsx +++ b/site/src/components/SettingsSecurityForm/SettingsSecurityForm.tsx @@ -1,5 +1,5 @@ import TextField from "@mui/material/TextField" -import { FormikContextType, FormikTouched, useFormik } from "formik" +import { FormikContextType, useFormik } from "formik" import { FC } from "react" import * as Yup from "yup" import { getFormHelpers } from "../../utils/formUtils" @@ -42,33 +42,31 @@ const validationSchema = Yup.object({ }) export interface SecurityFormProps { - disabled?: boolean + disabled: boolean isLoading: boolean - initialValues: SecurityFormValues onSubmit: (values: SecurityFormValues) => void - updateSecurityError?: Error | unknown - // initialTouched is only used for testing the error state of the form. - initialTouched?: FormikTouched + error?: unknown } export const SecurityForm: FC = ({ disabled, isLoading, onSubmit, - initialValues, - updateSecurityError, - initialTouched, + error, }) => { const form: FormikContextType = useFormik({ - initialValues, + initialValues: { + old_password: "", + password: "", + confirm_password: "", + }, validationSchema, onSubmit, - initialTouched, }) const getFieldHelpers = getFormHelpers( form, - updateSecurityError, + error, ) if (disabled) { @@ -83,8 +81,8 @@ export const SecurityForm: FC = ({ <> - {Boolean(updateSecurityError) && ( - + {Boolean(error) && ( + )} { } return ( - -
- { + { securitySend({ type: "UPDATE_SECURITY", data, }) - }} - /> + }, + }, + }} + oidc={ + authMethods.convert_to_oidc_enabled + ? { + section: { + authMethods, + ...singleSignOnSection, + }, + } + : undefined + } + /> + ) +} + +export const SecurityPageView = ({ + security, + oidc, +}: { + security: { + form: ComponentProps + } + oidc?: { + section: ComponentProps + } +}) => { + return ( + +
+
- + {oidc && } ) } diff --git a/site/src/pages/UserSettingsPage/SecurityPage/SecurityPageView.stories.tsx b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPageView.stories.tsx new file mode 100644 index 0000000000000..b908dcff21797 --- /dev/null +++ b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPageView.stories.tsx @@ -0,0 +1,70 @@ +import type { Meta, StoryObj } from "@storybook/react" +import { SecurityPageView } from "./SecurityPage" +import { action } from "@storybook/addon-actions" +import { MockAuthMethods } from "testHelpers/entities" +import { ComponentProps } from "react" +import set from "lodash/fp/set" +import { AuthMethods } from "api/typesGenerated" + +const defaultArgs: ComponentProps = { + security: { + form: { + disabled: false, + error: undefined, + isLoading: false, + onSubmit: action("onSubmit"), + }, + }, + oidc: { + section: { + authMethods: MockAuthMethods, + closeConfirmation: action("closeConfirmation"), + confirm: action("confirm"), + error: undefined, + isConfirming: false, + isUpdating: false, + openConfirmation: action("openConfirmation"), + }, + }, +} + +const meta: Meta = { + title: "pages/SecurityPageView", + component: SecurityPageView, + args: defaultArgs, +} + +export default meta +type Story = StoryObj + +export const Default: Story = {} + +export const NoOIDCAvailable: Story = { + args: { + ...defaultArgs, + oidc: undefined, + }, +} + +const authMethodsWithPassword: AuthMethods = { + ...MockAuthMethods, + me_login_type: "password", + github: { enabled: true }, + oidc: { enabled: true, signInText: "", iconUrl: "" }, +} + +export const UserLoginTypeIsPassword: Story = { + args: set("oidc.section.authMethods", authMethodsWithPassword, defaultArgs), +} + +export const ConfirmingOIDCConversion: Story = { + args: set( + "oidc.section", + { + ...defaultArgs.oidc?.section, + authMethods: authMethodsWithPassword, + isConfirming: true, + }, + defaultArgs, + ), +} diff --git a/site/src/pages/UserSettingsPage/SecurityPage/SingleSignOnSection.tsx b/site/src/pages/UserSettingsPage/SecurityPage/SingleSignOnSection.tsx index ee4056d003af4..16c372b29bebe 100644 --- a/site/src/pages/UserSettingsPage/SecurityPage/SingleSignOnSection.tsx +++ b/site/src/pages/UserSettingsPage/SecurityPage/SingleSignOnSection.tsx @@ -64,7 +64,6 @@ export const useSingleSignOnSection = () => { } return { - mutation, openConfirmation, closeConfirmation, confirm, diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 19db62cfec90f..11527c05ea210 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -1021,6 +1021,7 @@ export const MockAuthMethods: TypesGen.AuthMethods = { password: { enabled: true }, github: { enabled: false }, oidc: { enabled: false, signInText: "", iconUrl: "" }, + convert_to_oidc_enabled: true, } export const MockGitSSHKey: TypesGen.GitSSHKey = { From cdea1d626157f501c2d690e1de747eae5cbfdd17 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 23 Jun 2023 14:08:18 +0000 Subject: [PATCH 06/10] Change label name --- .../UserSettingsPage/SecurityPage/SecurityPageView.stories.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/pages/UserSettingsPage/SecurityPage/SecurityPageView.stories.tsx b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPageView.stories.tsx index b908dcff21797..e115c8bfe9e48 100644 --- a/site/src/pages/UserSettingsPage/SecurityPage/SecurityPageView.stories.tsx +++ b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPageView.stories.tsx @@ -37,7 +37,7 @@ const meta: Meta = { export default meta type Story = StoryObj -export const Default: Story = {} +export const UsingOIDC: Story = {} export const NoOIDCAvailable: Story = { args: { From 338f410e334c3bd8218623fa7c8f02cd6f11e055 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 23 Jun 2023 14:27:52 +0000 Subject: [PATCH 07/10] Fic previous tests --- .../SecurityPage/SecurityPage.test.tsx | 171 +++++++++--------- .../SecurityPage/SecurityPageView.stories.tsx | 21 +-- site/src/testHelpers/entities.ts | 7 + 3 files changed, 103 insertions(+), 96 deletions(-) diff --git a/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.test.tsx b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.test.tsx index 9a809d4b57602..1de58898d7f43 100644 --- a/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.test.tsx +++ b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.test.tsx @@ -1,116 +1,117 @@ import { fireEvent, screen, waitFor } from "@testing-library/react" import * as API from "../../../api/api" import * as SecurityForm from "../../../components/SettingsSecurityForm/SettingsSecurityForm" -import { renderWithAuth } from "../../../testHelpers/renderHelpers" +import { + renderWithAuth, + waitForLoaderToBeRemoved, +} from "../../../testHelpers/renderHelpers" import { SecurityPage } from "./SecurityPage" import i18next from "i18next" -import { mockApiError } from "testHelpers/entities" +import { + MockAuthMethodsWithPasswordType, + mockApiError, +} from "testHelpers/entities" const { t } = i18next -const renderPage = () => { - return renderWithAuth() +const renderPage = async () => { + const utils = renderWithAuth() + await waitForLoaderToBeRemoved() + return utils } -const newData = { +const newSecurityFormValues = { old_password: "password1", password: "password2", confirm_password: "password2", } -const fillAndSubmitForm = async () => { - await waitFor(() => screen.findByLabelText("Old Password")) +const fillAndSubmitSecurityForm = () => { fireEvent.change(screen.getByLabelText("Old Password"), { - target: { value: newData.old_password }, + target: { value: newSecurityFormValues.old_password }, }) fireEvent.change(screen.getByLabelText("New Password"), { - target: { value: newData.password }, + target: { value: newSecurityFormValues.password }, }) fireEvent.change(screen.getByLabelText("Confirm Password"), { - target: { value: newData.confirm_password }, + target: { value: newSecurityFormValues.confirm_password }, }) fireEvent.click(screen.getByText(SecurityForm.Language.updatePassword)) } -describe("SecurityPage", () => { - describe("when it is a success", () => { - it("shows the success message", async () => { - jest - .spyOn(API, "updateUserPassword") - .mockImplementationOnce((_userId, _data) => Promise.resolve(undefined)) - const { user } = renderPage() - await fillAndSubmitForm() - - const expectedMessage = t("securityUpdateSuccessMessage", { - ns: "userSettingsPage", - }) - const successMessage = await screen.findByText(expectedMessage) - expect(successMessage).toBeDefined() - expect(API.updateUserPassword).toBeCalledTimes(1) - expect(API.updateUserPassword).toBeCalledWith(user.id, newData) - - await waitFor(() => expect(window.location).toBeAt("/")) - }) - }) +beforeEach(() => { + jest + .spyOn(API, "getAuthMethods") + .mockResolvedValue(MockAuthMethodsWithPasswordType) +}) + +test("update password successfully", async () => { + jest + .spyOn(API, "updateUserPassword") + .mockImplementationOnce((_userId, _data) => Promise.resolve(undefined)) + const { user } = await renderPage() + fillAndSubmitSecurityForm() - describe("when the old_password is incorrect", () => { - it("shows an error", async () => { - jest.spyOn(API, "updateUserPassword").mockRejectedValueOnce( - mockApiError({ - message: "Incorrect password.", - validations: [ - { detail: "Incorrect password.", field: "old_password" }, - ], - }), - ) - - const { user } = renderPage() - await fillAndSubmitForm() - - const errorMessage = await screen.findAllByText("Incorrect password.") - expect(errorMessage).toBeDefined() - expect(errorMessage).toHaveLength(2) - expect(API.updateUserPassword).toBeCalledTimes(1) - expect(API.updateUserPassword).toBeCalledWith(user.id, newData) - }) + const expectedMessage = t("securityUpdateSuccessMessage", { + ns: "userSettingsPage", }) + const successMessage = await screen.findByText(expectedMessage) + expect(successMessage).toBeDefined() + expect(API.updateUserPassword).toBeCalledTimes(1) + expect(API.updateUserPassword).toBeCalledWith(user.id, newSecurityFormValues) + + await waitFor(() => expect(window.location).toBeAt("/")) +}) - describe("when the password is invalid", () => { - it("shows an error", async () => { - jest.spyOn(API, "updateUserPassword").mockRejectedValueOnce( - mockApiError({ - message: "Invalid password.", - validations: [{ detail: "Invalid password.", field: "password" }], - }), - ) - - const { user } = renderPage() - await fillAndSubmitForm() - - const errorMessage = await screen.findAllByText("Invalid password.") - expect(errorMessage).toBeDefined() - expect(errorMessage).toHaveLength(2) - expect(API.updateUserPassword).toBeCalledTimes(1) - expect(API.updateUserPassword).toBeCalledWith(user.id, newData) - }) +test("update password with incorrect old password", async () => { + jest.spyOn(API, "updateUserPassword").mockRejectedValueOnce( + mockApiError({ + message: "Incorrect password.", + validations: [{ detail: "Incorrect password.", field: "old_password" }], + }), + ) + + const { user } = await renderPage() + fillAndSubmitSecurityForm() + + const errorMessage = await screen.findAllByText("Incorrect password.") + expect(errorMessage).toBeDefined() + expect(errorMessage).toHaveLength(2) + expect(API.updateUserPassword).toBeCalledTimes(1) + expect(API.updateUserPassword).toBeCalledWith(user.id, newSecurityFormValues) +}) + +test("update password with invalid password", async () => { + jest.spyOn(API, "updateUserPassword").mockRejectedValueOnce( + mockApiError({ + message: "Invalid password.", + validations: [{ detail: "Invalid password.", field: "password" }], + }), + ) + + const { user } = await renderPage() + fillAndSubmitSecurityForm() + + const errorMessage = await screen.findAllByText("Invalid password.") + expect(errorMessage).toBeDefined() + expect(errorMessage).toHaveLength(2) + expect(API.updateUserPassword).toBeCalledTimes(1) + expect(API.updateUserPassword).toBeCalledWith(user.id, newSecurityFormValues) +}) + +test("update password when submit returns an unknown error", async () => { + jest.spyOn(API, "updateUserPassword").mockRejectedValueOnce({ + data: "unknown error", }) - describe("when it is an unknown error", () => { - it("shows a generic error message", async () => { - jest.spyOn(API, "updateUserPassword").mockRejectedValueOnce({ - data: "unknown error", - }) - - const { user } = renderPage() - await fillAndSubmitForm() - - const errorText = t("warningsAndErrors.somethingWentWrong", { - ns: "common", - }) - const errorMessage = await screen.findByText(errorText) - expect(errorMessage).toBeDefined() - expect(API.updateUserPassword).toBeCalledTimes(1) - expect(API.updateUserPassword).toBeCalledWith(user.id, newData) - }) + const { user } = await renderPage() + fillAndSubmitSecurityForm() + + const errorText = t("warningsAndErrors.somethingWentWrong", { + ns: "common", }) + const errorMessage = await screen.findByText(errorText) + expect(errorMessage).toBeDefined() + expect(API.updateUserPassword).toBeCalledTimes(1) + expect(API.updateUserPassword).toBeCalledWith(user.id, newSecurityFormValues) }) diff --git a/site/src/pages/UserSettingsPage/SecurityPage/SecurityPageView.stories.tsx b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPageView.stories.tsx index e115c8bfe9e48..09a34d5ac6f84 100644 --- a/site/src/pages/UserSettingsPage/SecurityPage/SecurityPageView.stories.tsx +++ b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPageView.stories.tsx @@ -1,10 +1,12 @@ import type { Meta, StoryObj } from "@storybook/react" import { SecurityPageView } from "./SecurityPage" import { action } from "@storybook/addon-actions" -import { MockAuthMethods } from "testHelpers/entities" +import { + MockAuthMethods, + MockAuthMethodsWithPasswordType, +} from "testHelpers/entities" import { ComponentProps } from "react" import set from "lodash/fp/set" -import { AuthMethods } from "api/typesGenerated" const defaultArgs: ComponentProps = { security: { @@ -46,15 +48,12 @@ export const NoOIDCAvailable: Story = { }, } -const authMethodsWithPassword: AuthMethods = { - ...MockAuthMethods, - me_login_type: "password", - github: { enabled: true }, - oidc: { enabled: true, signInText: "", iconUrl: "" }, -} - export const UserLoginTypeIsPassword: Story = { - args: set("oidc.section.authMethods", authMethodsWithPassword, defaultArgs), + args: set( + "oidc.section.authMethods", + MockAuthMethodsWithPasswordType, + defaultArgs, + ), } export const ConfirmingOIDCConversion: Story = { @@ -62,7 +61,7 @@ export const ConfirmingOIDCConversion: Story = { "oidc.section", { ...defaultArgs.oidc?.section, - authMethods: authMethodsWithPassword, + authMethods: MockAuthMethodsWithPasswordType, isConfirming: true, }, defaultArgs, diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 11527c05ea210..870ddcf077b11 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -1024,6 +1024,13 @@ export const MockAuthMethods: TypesGen.AuthMethods = { convert_to_oidc_enabled: true, } +export const MockAuthMethodsWithPasswordType: TypesGen.AuthMethods = { + ...MockAuthMethods, + me_login_type: "password", + github: { enabled: true }, + oidc: { enabled: true, signInText: "", iconUrl: "" }, +} + export const MockGitSSHKey: TypesGen.GitSSHKey = { user_id: "1fa0200f-7331-4524-a364-35770666caa7", created_at: "2022-05-16T14:30:34.148205897Z", From 135209d86eef49254141e8f37daaa1a9b5b4d150 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 23 Jun 2023 14:41:21 +0000 Subject: [PATCH 08/10] Add integration test --- .../src/components/SettingsLayout/Section.tsx | 11 +++++-- .../SecurityPage/SecurityPage.test.tsx | 29 ++++++++++++++++++- .../SecurityPage/SingleSignOnSection.tsx | 1 + 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/site/src/components/SettingsLayout/Section.tsx b/site/src/components/SettingsLayout/Section.tsx index f4300ab433921..35419ff4ab89d 100644 --- a/site/src/components/SettingsLayout/Section.tsx +++ b/site/src/components/SettingsLayout/Section.tsx @@ -6,6 +6,8 @@ import { SectionAction } from "../SectionAction/SectionAction" type SectionLayout = "fixed" | "fluid" export interface SectionProps { + // Useful for testing + id?: string title?: ReactNode | string description?: ReactNode toolbar?: ReactNode @@ -20,6 +22,7 @@ type SectionFC = FC> & { } export const Section: SectionFC = ({ + id, title, description, toolbar, @@ -30,12 +33,16 @@ export const Section: SectionFC = ({ }) => { const styles = useStyles({ layout }) return ( -
+
{(title || description) && (
- {title && {title}} + {title && ( + + {title} + + )} {description && typeof description === "string" && ( {description} diff --git a/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.test.tsx b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.test.tsx index 1de58898d7f43..eeb9a5eb639b6 100644 --- a/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.test.tsx +++ b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.test.tsx @@ -1,4 +1,4 @@ -import { fireEvent, screen, waitFor } from "@testing-library/react" +import { fireEvent, screen, waitFor, within } from "@testing-library/react" import * as API from "../../../api/api" import * as SecurityForm from "../../../components/SettingsSecurityForm/SettingsSecurityForm" import { @@ -11,6 +11,7 @@ import { MockAuthMethodsWithPasswordType, mockApiError, } from "testHelpers/entities" +import userEvent from "@testing-library/user-event" const { t } = i18next @@ -115,3 +116,29 @@ test("update password when submit returns an unknown error", async () => { expect(API.updateUserPassword).toBeCalledTimes(1) expect(API.updateUserPassword).toBeCalledWith(user.id, newSecurityFormValues) }) + +test("change login type to OIDC", async () => { + const convertToOAUTHSpy = jest.spyOn(API, "convertToOAUTH") + const user = userEvent.setup() + const { user: userData } = await renderPage() + + const ssoSection = screen.getByTestId("sso-section") + const githubButton = within(ssoSection).getByText("GitHub", { exact: false }) + await user.click(githubButton) + + const confirmationDialog = await screen.findByTestId("dialog") + const confirmPasswordField = within(confirmationDialog).getByLabelText( + "Confirm your password", + ) + await user.type(confirmPasswordField, "password123") + const updateButton = within(confirmationDialog).getByText("Update") + await user.click(updateButton) + + await waitFor(() => { + expect(convertToOAUTHSpy).toHaveBeenCalledWith({ + password: "password123", + to_login_type: "github", + email: userData.email, + }) + }) +}) diff --git a/site/src/pages/UserSettingsPage/SecurityPage/SingleSignOnSection.tsx b/site/src/pages/UserSettingsPage/SecurityPage/SingleSignOnSection.tsx index 16c372b29bebe..9e10c328ba183 100644 --- a/site/src/pages/UserSettingsPage/SecurityPage/SingleSignOnSection.tsx +++ b/site/src/pages/UserSettingsPage/SecurityPage/SingleSignOnSection.tsx @@ -91,6 +91,7 @@ export const SingleSignOnSection = ({ return ( <>
From f81fff3579f107cf9337b1ea39cd0ca3daf1b356 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 23 Jun 2023 17:11:11 +0000 Subject: [PATCH 09/10] Fix issues --- .vscode/settings.json | 2 +- coderd/userauth.go | 4 ++-- .../SettingsAccountForm/SettingsAccountForm.test.tsx | 4 ++-- .../SettingsSecurityForm/SettingsSecurityForm.tsx | 9 ++------- 4 files changed, 7 insertions(+), 12 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index d028ad30211f1..1ff3ea0883482 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -211,5 +211,5 @@ "go.testFlags": ["-short", "-coverpkg=./..."], // We often use a version of TypeScript that's ahead of the version shipped // with VS Code. - "typescript.tsdk": "./site/node_modules/typescript/lib", + "typescript.tsdk": "./site/node_modules/typescript/lib" } diff --git a/coderd/userauth.go b/coderd/userauth.go index e09b38cf3fe99..92e93474a8b51 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -840,7 +840,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { // Convert the []interface{} we get to a []string. groupsInterface, ok := groupsRaw.([]interface{}) if ok { - logger.Debug(ctx, "groups returned in oidc claims", + api.Logger.Debug(ctx, "groups returned in oidc claims", slog.F("len", len(groupsInterface)), slog.F("groups", groupsInterface), ) @@ -861,7 +861,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { groups = append(groups, group) } } else { - logger.Debug(ctx, "groups field was an unknown type", + api.Logger.Debug(ctx, "groups field was an unknown type", slog.F("type", fmt.Sprintf("%T", groupsRaw)), ) } diff --git a/site/src/components/SettingsAccountForm/SettingsAccountForm.test.tsx b/site/src/components/SettingsAccountForm/SettingsAccountForm.test.tsx index df5268883ad2e..000e6a8adc3fc 100644 --- a/site/src/components/SettingsAccountForm/SettingsAccountForm.test.tsx +++ b/site/src/components/SettingsAccountForm/SettingsAccountForm.test.tsx @@ -31,7 +31,7 @@ describe("AccountForm", () => { const el = await screen.findByLabelText("Username") expect(el).toBeEnabled() const btn = await screen.findByRole("button", { - name: /Update settings/i, + name: /Update account/i, }) expect(btn).toBeEnabled() }) @@ -61,7 +61,7 @@ describe("AccountForm", () => { const el = await screen.findByLabelText("Username") expect(el).toBeDisabled() const btn = await screen.findByRole("button", { - name: /Update settings/i, + name: /Update account/i, }) expect(btn).toBeDisabled() }) diff --git a/site/src/components/SettingsSecurityForm/SettingsSecurityForm.tsx b/site/src/components/SettingsSecurityForm/SettingsSecurityForm.tsx index 87151469ac772..39230f3837b1b 100644 --- a/site/src/components/SettingsSecurityForm/SettingsSecurityForm.tsx +++ b/site/src/components/SettingsSecurityForm/SettingsSecurityForm.tsx @@ -64,10 +64,7 @@ export const SecurityForm: FC = ({ validationSchema, onSubmit, }) - const getFieldHelpers = getFormHelpers( - form, - error, - ) + const getFieldHelpers = getFormHelpers(form, error) if (disabled) { return ( @@ -81,9 +78,7 @@ export const SecurityForm: FC = ({ <> - {Boolean(error) && ( - - )} + {Boolean(error) && } Date: Mon, 26 Jun 2023 09:29:04 -0500 Subject: [PATCH 10/10] Fix storybook stories --- .../SettingsSecurityForm.stories.tsx | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/site/src/components/SettingsSecurityForm/SettingsSecurityForm.stories.tsx b/site/src/components/SettingsSecurityForm/SettingsSecurityForm.stories.tsx index d2ca25ff16bd0..506b81270eebc 100644 --- a/site/src/components/SettingsSecurityForm/SettingsSecurityForm.stories.tsx +++ b/site/src/components/SettingsSecurityForm/SettingsSecurityForm.stories.tsx @@ -17,12 +17,6 @@ const Template: Story = (args: SecurityFormProps) => ( export const Example = Template.bind({}) Example.args = { isLoading: false, - initialValues: { - old_password: "", - password: "", - confirm_password: "", - }, - updateSecurityError: undefined, onSubmit: () => { return Promise.resolve() }, @@ -37,7 +31,7 @@ Loading.args = { export const WithError = Template.bind({}) WithError.args = { ...Example.args, - updateSecurityError: mockApiError({ + error: mockApiError({ message: "Old password is incorrect", validations: [ { @@ -46,7 +40,4 @@ WithError.args = { }, ], }), - initialTouched: { - old_password: true, - }, }