From 026ea29847ea4e154fd17c40a931f1dc1c7efd39 Mon Sep 17 00:00:00 2001 From: Joel Biddle Date: Tue, 22 Aug 2023 20:42:07 +1000 Subject: [PATCH] further fixes to password check logic --- .../checks/checkIsPasswordBreached.ts | 13 +- .../utilities/checks/checkPassword.ts | 8 +- .../ChangePasswordSection.tsx | 264 +++++++++--------- 3 files changed, 138 insertions(+), 147 deletions(-) diff --git a/frontend/src/components/utilities/checks/checkIsPasswordBreached.ts b/frontend/src/components/utilities/checks/checkIsPasswordBreached.ts index 40a9330db..fba537977 100644 --- a/frontend/src/components/utilities/checks/checkIsPasswordBreached.ts +++ b/frontend/src/components/utilities/checks/checkIsPasswordBreached.ts @@ -15,21 +15,26 @@ export const checkIsPasswordBreached = async (password: string) => { const textEncoder = new TextEncoder(); const encodedPwd = textEncoder.encode(password); const hash = crypto.createHash("sha1").update(encodedPwd).digest(); - let hashedPwd = Array.from(new Uint8Array(hash)) + + const hashedPwd = Array.from(new Uint8Array(hash)) .map((byte) => byte.toString(16).padStart(2, "0")) .join("") .toUpperCase(); - hashedPwd = hashedPwd.slice(0, 5); // ONLY the first five SHA-1 hash chars are sent over HTTPS (the whole string can be sent but that's not very secure due to SHA-1 flaws) - const response = await axios.get(`${dataBreachCheckAPIBaseURL}${hashedPwd}`); + const hashedPwdToSend = hashedPwd.slice(0, 5); // ONLY the first five SHA-1 hash chars are sent over HTTPS (the whole string can be sent but that's not very secure due to SHA-1 flaws) + + const response = await axios.get(`${dataBreachCheckAPIBaseURL}${hashedPwdToSend}`); const responseData = response.data.toUpperCase(); + const isBreachedPassword = responseData.includes(hashedPwd.slice(5, 40)); // compare against the API's ranged db's hash table + console.log("isBreachedPassword:", isBreachedPassword); // remove log later // Clear the hashed password from memory + const zeroBuffer = new Uint8Array(encodedPwd.length); encodedPwd.set(zeroBuffer); - return isBreachedPassword; // boolean: true === "password has been involved in a data breach" + return isBreachedPassword; // boolean: true indicates the password has been involved in a data breach } catch (err: any) { if (axios.isAxiosError(err) && err.response && err.response.status === 429) { console.error("Received a 429 response from the Pwnd Passwords API"); diff --git a/frontend/src/components/utilities/checks/checkPassword.ts b/frontend/src/components/utilities/checks/checkPassword.ts index 4f0d0a523..210d3f040 100644 --- a/frontend/src/components/utilities/checks/checkPassword.ts +++ b/frontend/src/components/utilities/checks/checkPassword.ts @@ -70,15 +70,15 @@ const checkPassword = async ({ errors.repeatedChar = "No 3 repeat, consecutive characters"; } - if (commonPasswords.includes(password)) { - errors.commonPassword = "No common passwords"; - } - if (await checkIsPasswordBreached(password)) { errors.breachedPassword = "The password you provided is in a list of passwords commonly used on other websites. Please try again with a stronger password."; } + if (commonPasswords.includes(password)) { + errors.commonPassword = "No common passwords"; + } + setErrors(errors); return Object.keys(errors).length > 0; }; diff --git a/frontend/src/views/Settings/PersonalSettingsPage/ChangePasswordSection/ChangePasswordSection.tsx b/frontend/src/views/Settings/PersonalSettingsPage/ChangePasswordSection/ChangePasswordSection.tsx index e17864485..86cb58a01 100644 --- a/frontend/src/views/Settings/PersonalSettingsPage/ChangePasswordSection/ChangePasswordSection.tsx +++ b/frontend/src/views/Settings/PersonalSettingsPage/ChangePasswordSection/ChangePasswordSection.tsx @@ -1,5 +1,5 @@ import { useState } from "react"; -import { Controller, useForm } from "react-hook-form"; +import { Controller, useForm } from "react-hook-form"; import { useTranslation } from "react-i18next"; import { faXmark } from "@fortawesome/free-solid-svg-icons"; import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; @@ -9,160 +9,146 @@ import * as yup from "yup"; import { useNotificationContext } from "@app/components/context/Notifications/NotificationProvider"; import attemptChangePassword from "@app/components/utilities/attemptChangePassword"; import checkPassword from "@app/components/utilities/checks/checkPassword"; -import { - Button, - FormControl, - Input -} from "@app/components/v2"; +import { Button, FormControl, Input } from "@app/components/v2"; import { useUser } from "@app/context"; import { useGetCommonPasswords } from "@app/hooks/api"; type Errors = { - tooShort?: string, - tooLong?: string, - upperCase?: string, - lowerCase?: string, - number?: string, - specialChar?: string, - repeatedChar?: string, - breachedPassword?: string + tooShort?: string; + tooLong?: string; + upperCase?: string; + lowerCase?: string; + number?: string; + specialChar?: string; + repeatedChar?: string; + commonPassword?: string; + breachedPassword?: string; }; -const schema = yup.object({ +const schema = yup + .object({ oldPassword: yup.string().required("Old password is required"), newPassword: yup.string().required("New password is required") -}).required(); + }) + .required(); export type FormData = yup.InferType; export const ChangePasswordSection = () => { - const { t } = useTranslation(); - const { createNotification } = useNotificationContext(); - const { user } = useUser(); - const { data: commonPasswords } = useGetCommonPasswords(); - const { reset, control, handleSubmit } = useForm({ - defaultValues: { - oldPassword: "", - newPassword: "" - }, - resolver: yupResolver(schema) - }); - const [errors, setErrors] = useState({}); - const [isLoading, setIsLoading] = useState(false); + const { t } = useTranslation(); + const { createNotification } = useNotificationContext(); + const { user } = useUser(); + const { data: commonPasswords } = useGetCommonPasswords(); + const { reset, control, handleSubmit } = useForm({ + defaultValues: { + oldPassword: "", + newPassword: "" + }, + resolver: yupResolver(schema) + }); + const [errors, setErrors] = useState({}); + const [isLoading, setIsLoading] = useState(false); - const onFormSubmit = async ({ oldPassword, newPassword }: FormData) => { - try { - if (!user?.email) return; - if (!commonPasswords) return; + const onFormSubmit = async ({ oldPassword, newPassword }: FormData) => { + try { + if (!user?.email) return; + if (!commonPasswords) return; - const errorCheck = await checkPassword({ - password: newPassword, - commonPasswords, - setErrors - }); + const errorCheck = await checkPassword({ + password: newPassword, + commonPasswords, + setErrors + }); - if (errorCheck) return; - - setIsLoading(true); - await attemptChangePassword({ - email: user.email, - currentPassword: oldPassword, - newPassword - }); - - setIsLoading(false); - createNotification({ - text: "Successfully changed password", - type: "success" - }); + if (errorCheck) return; - reset(); - window.location.href = "/login"; - - } catch (err) { - console.error(err); - setIsLoading(false); - createNotification({ - text: "Failed to change password", - type: "error" - }); - } + setIsLoading(true); + await attemptChangePassword({ + email: user.email, + currentPassword: oldPassword, + newPassword + }); + + setIsLoading(false); + createNotification({ + text: "Successfully changed password", + type: "success" + }); + + reset(); + window.location.href = "/login"; + } catch (err) { + console.error(err); + setIsLoading(false); + createNotification({ + text: "Failed to change password", + type: "error" + }); } - - return ( -
-

- Change password -

-
- ( - - - - )} - control={control} - name="oldPassword" - /> -
-
- ( - - - - )} - control={control} - name="newPassword" - /> -
- {Object.keys(errors).length > 0 && ( -
-
{t("section.password.validate-base")}
- {Object.keys(errors).map((key) => { - if (errors[key as keyof Errors]) { - return ( -
-
- -
-

- {errors[key as keyof Errors]} -

-
- ); - } + }; - return null; - })} + return ( + +

Change password

+
+ ( + + + + )} + control={control} + name="oldPassword" + /> +
+
+ ( + + + + )} + control={control} + name="newPassword" + /> +
+ {Object.keys(errors).length > 0 && ( +
+
{t("section.password.validate-base")}
+ {Object.keys(errors).map((key) => { + if (errors[key as keyof Errors]) { + return ( +
+
+ +
+

{errors[key as keyof Errors]}

- )} - - - ); -} \ No newline at end of file + ); + } + + return null; + })} +
+ )} + + + ); +};