From 854025ec03206a4cc18935147547afaaee8f77d0 Mon Sep 17 00:00:00 2001 From: Anmol Singh Bhatia <121005188+anmolsinghbhatia@users.noreply.github.com> Date: Tue, 14 May 2024 15:27:31 +0530 Subject: [PATCH] fix: auth fixes and improvement (#4452) * chore: change password api updated and missing password error code added * chore: auth helper updated * chore: disable send code input suggestion * chore: change password function updated * fix: application error on sign in page * chore: change password validation added and enhancement --- .../plane/authentication/adapter/error.py | 1 + .../plane/authentication/views/common.py | 91 +++---- packages/constants/src/auth.ts | 11 + .../accounts/auth-forms/unique-code.tsx | 1 + space/helpers/authentication.helper.tsx | 14 +- .../account/auth-forms/auth-root.tsx | 2 +- web/helpers/authentication.helper.tsx | 12 + web/pages/profile/change-password.tsx | 234 +++++++++++++----- web/services/user.service.ts | 8 +- 9 files changed, 254 insertions(+), 120 deletions(-) diff --git a/apiserver/plane/authentication/adapter/error.py b/apiserver/plane/authentication/adapter/error.py index 2a3e7f6cc..8ea33fe0f 100644 --- a/apiserver/plane/authentication/adapter/error.py +++ b/apiserver/plane/authentication/adapter/error.py @@ -38,6 +38,7 @@ AUTHENTICATION_ERROR_CODES = { "EXPIRED_PASSWORD_TOKEN": 5130, # Change password "INCORRECT_OLD_PASSWORD": 5135, + "MISSING_PASSWORD": 5138, "INVALID_NEW_PASSWORD": 5140, # set passowrd "PASSWORD_ALREADY_SET": 5145, diff --git a/apiserver/plane/authentication/views/common.py b/apiserver/plane/authentication/views/common.py index 16ac058b0..3d17e93f5 100644 --- a/apiserver/plane/authentication/views/common.py +++ b/apiserver/plane/authentication/views/common.py @@ -36,55 +36,60 @@ class CSRFTokenEndpoint(APIView): class ChangePasswordEndpoint(APIView): def post(self, request): - serializer = ChangePasswordSerializer(data=request.data) user = User.objects.get(pk=request.user.id) - if serializer.is_valid(): - if not user.check_password(serializer.data.get("old_password")): - exc = AuthenticationException( - error_code=AUTHENTICATION_ERROR_CODES[ - "INCORRECT_OLD_PASSWORD" - ], - error_message="INCORRECT_OLD_PASSWORD", - payload={"error": "Old password is not correct"}, - ) - return Response( - exc.get_error_dict(), - status=status.HTTP_400_BAD_REQUEST, - ) - # check the password score - results = zxcvbn(serializer.data.get("new_password")) - if results["score"] < 3: - exc = AuthenticationException( - error_code=AUTHENTICATION_ERROR_CODES[ - "INVALID_NEW_PASSWORD" - ], - error_message="INVALID_NEW_PASSWORD", - ) - return Response( - exc.get_error_dict(), - status=status.HTTP_400_BAD_REQUEST, - ) + old_password = request.data.get("old_password", False) + new_password = request.data.get("new_password", False) - # set_password also hashes the password that the user will get - user.set_password(serializer.data.get("new_password")) - user.is_password_autoset = False - user.save() - user_login(user=user, request=request, is_app=True) - return Response( - {"message": "Password updated successfully"}, - status=status.HTTP_200_OK, + if not old_password or not new_password: + exc = AuthenticationException( + error_code=AUTHENTICATION_ERROR_CODES["MISSING_PASSWORD"], + error_message="MISSING_PASSWORD", + payload={"error": "Old or new password is missing"}, + ) + return Response( + exc.get_error_dict(), + status=status.HTTP_400_BAD_REQUEST, ) - exc = AuthenticationException( - error_code=AUTHENTICATION_ERROR_CODES["INVALID_PASSWORD"], - error_message="INVALID_PASSWORD", - ) - return Response( - exc.get_error_dict(), - status=status.HTTP_400_BAD_REQUEST, - ) + if not user.check_password(old_password): + exc = AuthenticationException( + error_code=AUTHENTICATION_ERROR_CODES[ + "INCORRECT_OLD_PASSWORD" + ], + error_message="INCORRECT_OLD_PASSWORD", + payload={"error": "Old password is not correct"}, + ) + return Response( + exc.get_error_dict(), + status=status.HTTP_400_BAD_REQUEST, + ) + + # check the password score + results = zxcvbn(new_password) + if results["score"] < 3: + exc = AuthenticationException( + error_code=AUTHENTICATION_ERROR_CODES[ + "INVALID_NEW_PASSWORD" + ], + error_message="INVALID_NEW_PASSWORD", + ) + return Response( + exc.get_error_dict(), + status=status.HTTP_400_BAD_REQUEST, + ) + + # set_password also hashes the password that the user will get + user.set_password(new_password) + user.is_password_autoset = False + user.save() + user_login(user=user, request=request, is_app=True) + return Response( + {"message": "Password updated successfully"}, + status=status.HTTP_200_OK, + ) + class SetUserPasswordEndpoint(APIView): def post(self, request): user = User.objects.get(pk=request.user.id) diff --git a/packages/constants/src/auth.ts b/packages/constants/src/auth.ts index a7e467153..86e75f4ff 100644 --- a/packages/constants/src/auth.ts +++ b/packages/constants/src/auth.ts @@ -41,6 +41,7 @@ export enum EAuthErrorCodes { // Password strength INVALID_PASSWORD = "5020", // Sign Up + USER_ACCOUNT_DEACTIVATED = "5019", USER_ALREADY_EXIST = "5030", AUTHENTICATION_FAILED_SIGN_UP = "5035", REQUIRED_EMAIL_PASSWORD_SIGN_UP = "5040", @@ -68,6 +69,7 @@ export enum EAuthErrorCodes { EXPIRED_PASSWORD_TOKEN = "5130", // Change password INCORRECT_OLD_PASSWORD = "5135", + MISSING_PASSWORD= "5138", INVALID_NEW_PASSWORD = "5140", // set passowrd PASSWORD_ALREADY_SET = "5145", @@ -158,6 +160,10 @@ const errorCodeMessages: { }, // sign in + [EAuthenticationErrorCodes.USER_ACCOUNT_DEACTIVATED]: { + title: `User account deactivated`, + message: () =>
Your account is deactivated. Contact support@plane.so.
, + }, [EAuthenticationErrorCodes.USER_DOES_NOT_EXIST]: { title: `User does not exist`, message: (email = undefined) => ( @@ -237,6 +243,11 @@ const errorCodeMessages: { }, // Change password + + [EAuthenticationErrorCodes.MISSING_PASSWORD]: { + title: `Password required`, + message: () => `Password required. Please try again.`, + }, [EAuthenticationErrorCodes.INCORRECT_OLD_PASSWORD]: { title: `Incorrect old password`, message: () => `Incorrect old password. Please try again.`, diff --git a/space/components/accounts/auth-forms/unique-code.tsx b/space/components/accounts/auth-forms/unique-code.tsx index fc04938bc..785d921a6 100644 --- a/space/components/accounts/auth-forms/unique-code.tsx +++ b/space/components/accounts/auth-forms/unique-code.tsx @@ -151,6 +151,7 @@ export const UniqueCodeForm: React.FC = (props) => { placeholder="gets-sets-flys" className="h-[46px] w-full border border-onboarding-border-100 !bg-onboarding-background-200 pr-12 placeholder:text-onboarding-text-400" autoFocus + autoComplete="off" />

diff --git a/space/helpers/authentication.helper.tsx b/space/helpers/authentication.helper.tsx index 579fe0496..2626b35c2 100644 --- a/space/helpers/authentication.helper.tsx +++ b/space/helpers/authentication.helper.tsx @@ -39,12 +39,12 @@ export enum EAuthenticationErrorCodes { INVALID_EMAIL = "5012", EMAIL_REQUIRED = "5013", // Sign Up + USER_ACCOUNT_DEACTIVATED = "5019", USER_ALREADY_EXIST = "5003", REQUIRED_EMAIL_PASSWORD_SIGN_UP = "5015", AUTHENTICATION_FAILED_SIGN_UP = "5006", INVALID_EMAIL_SIGN_UP = "5017", MAGIC_SIGN_UP_EMAIL_CODE_REQUIRED = "5023", - INVALID_EMAIL_MAGIC_SIGN_UP = "5019", // Sign In USER_DOES_NOT_EXIST = "5004", REQUIRED_EMAIL_PASSWORD_SIGN_IN = "5014", @@ -140,12 +140,14 @@ const errorCodeMessages: { title: `Email and code required`, message: () => `Email and code required. Please try again.`, }, - [EAuthenticationErrorCodes.INVALID_EMAIL_MAGIC_SIGN_UP]: { - title: `Invalid email`, - message: () => `Invalid email. Please try again.`, - }, // sign in + + [EAuthenticationErrorCodes.USER_ACCOUNT_DEACTIVATED]: { + title: `User account deactivated`, + message: () =>

Your account is deactivated. Please reach out to support@plane.so
, + }, + [EAuthenticationErrorCodes.USER_DOES_NOT_EXIST]: { title: `User does not exist`, message: (email = undefined) => ( @@ -250,7 +252,6 @@ export const authErrorHandler = ( EAuthenticationErrorCodes.AUTHENTICATION_FAILED_SIGN_UP, EAuthenticationErrorCodes.INVALID_EMAIL_SIGN_UP, EAuthenticationErrorCodes.MAGIC_SIGN_UP_EMAIL_CODE_REQUIRED, - EAuthenticationErrorCodes.INVALID_EMAIL_MAGIC_SIGN_UP, EAuthenticationErrorCodes.AUTHENTICATION_FAILED_SIGN_IN, EAuthenticationErrorCodes.INVALID_EMAIL_SIGN_IN, EAuthenticationErrorCodes.INVALID_EMAIL_MAGIC_SIGN_IN, @@ -273,6 +274,7 @@ export const authErrorHandler = ( EAuthenticationErrorCodes.REQUIRED_EMAIL_PASSWORD_SIGN_UP, EAuthenticationErrorCodes.REQUIRED_EMAIL_PASSWORD_SIGN_IN, EAuthenticationErrorCodes.MAGIC_SIGN_IN_EMAIL_CODE_REQUIRED, + EAuthenticationErrorCodes.USER_ACCOUNT_DEACTIVATED, ]; if (toastAlertErrorCodes.includes(errorCode)) diff --git a/web/components/account/auth-forms/auth-root.tsx b/web/components/account/auth-forms/auth-root.tsx index 4c479af28..2b31feb8b 100644 --- a/web/components/account/auth-forms/auth-root.tsx +++ b/web/components/account/auth-forms/auth-root.tsx @@ -92,7 +92,7 @@ export const AuthRoot: FC = observer((props) => { } }) .catch((error) => { - const errorhandler = authErrorHandler(error?.error_code.toString(), data?.email || undefined); + const errorhandler = authErrorHandler(error?.error_code?.toString(), data?.email || undefined); if (errorhandler?.type) setErrorInfo(errorhandler); }); }; diff --git a/web/helpers/authentication.helper.tsx b/web/helpers/authentication.helper.tsx index 88a40dd57..0f331394b 100644 --- a/web/helpers/authentication.helper.tsx +++ b/web/helpers/authentication.helper.tsx @@ -45,6 +45,7 @@ export enum EAuthenticationErrorCodes { INVALID_EMAIL_MAGIC_SIGN_UP = "5050", MAGIC_SIGN_UP_EMAIL_CODE_REQUIRED = "5055", // Sign In + USER_ACCOUNT_DEACTIVATED = "5019", USER_DOES_NOT_EXIST = "5060", AUTHENTICATION_FAILED_SIGN_IN = "5065", REQUIRED_EMAIL_PASSWORD_SIGN_IN = "5070", @@ -65,6 +66,7 @@ export enum EAuthenticationErrorCodes { EXPIRED_PASSWORD_TOKEN = "5130", // Change password INCORRECT_OLD_PASSWORD = "5135", + MISSING_PASSWORD = "5138", INVALID_NEW_PASSWORD = "5140", // set passowrd PASSWORD_ALREADY_SET = "5145", @@ -155,6 +157,11 @@ const errorCodeMessages: { }, // sign in + [EAuthenticationErrorCodes.USER_ACCOUNT_DEACTIVATED]: { + title: `User account deactivated`, + message: () =>
Your account is deactivated. Contact support@plane.so.
, + }, + [EAuthenticationErrorCodes.USER_DOES_NOT_EXIST]: { title: `User does not exist`, message: (email = undefined) => ( @@ -234,6 +241,10 @@ const errorCodeMessages: { }, // Change password + [EAuthenticationErrorCodes.MISSING_PASSWORD]: { + title: `Password required`, + message: () => `Password required. Please try again.`, + }, [EAuthenticationErrorCodes.INCORRECT_OLD_PASSWORD]: { title: `Incorrect old password`, message: () => `Incorrect old password. Please try again.`, @@ -343,6 +354,7 @@ export const authErrorHandler = ( EAuthenticationErrorCodes.ADMIN_AUTHENTICATION_FAILED, EAuthenticationErrorCodes.ADMIN_USER_ALREADY_EXIST, EAuthenticationErrorCodes.ADMIN_USER_DOES_NOT_EXIST, + EAuthenticationErrorCodes.USER_ACCOUNT_DEACTIVATED, ]; if (bannerAlertErrorCodes.includes(errorCode)) diff --git a/web/pages/profile/change-password.tsx b/web/pages/profile/change-password.tsx index ae4e7be70..b8979ea14 100644 --- a/web/pages/profile/change-password.tsx +++ b/web/pages/profile/change-password.tsx @@ -1,12 +1,15 @@ -import { ReactElement, useEffect, useState } from "react"; +import { ReactElement, useEffect, useMemo, useState } from "react"; import { observer } from "mobx-react"; import { useRouter } from "next/router"; import { Controller, useForm } from "react-hook-form"; +import { Eye, EyeOff } from "lucide-react"; // ui import { Button, Input, Spinner, TOAST_TYPE, setPromiseToast, setToast } from "@plane/ui"; // components +import { PasswordStrengthMeter } from "@/components/account"; import { PageHead } from "@/components/core"; import { SidebarHamburgerToggle } from "@/components/core/sidebar"; +import { getPasswordStrength } from "@/helpers/password.helper"; // hooks import { useAppTheme, useUser } from "@/hooks/store"; // layout @@ -14,6 +17,7 @@ import { ProfileSettingsLayout } from "@/layouts/settings-layout"; // types import { NextPageWithLayout } from "@/lib/types"; // services +import { AuthService } from "@/services/auth.service"; import { UserService } from "@/services/user.service"; export interface FormValues { @@ -29,9 +33,18 @@ const defaultValues: FormValues = { }; export const userService = new UserService(); +export const authService = new AuthService(); const ChangePasswordPage: NextPageWithLayout = observer(() => { + const [csrfToken, setCsrfToken] = useState(undefined); const [isPageLoading, setIsPageLoading] = useState(true); + const [showPassword, setShowPassword] = useState({ + oldPassword: false, + password: false, + retypePassword: false, + }); + const [isPasswordInputFocused, setIsPasswordInputFocused] = useState(false); + // router const router = useRouter(); // store hooks @@ -42,32 +55,54 @@ const ChangePasswordPage: NextPageWithLayout = observer(() => { const { control, handleSubmit, + watch, formState: { errors, isSubmitting }, + reset, } = useForm({ defaultValues }); + const oldPassword = watch("old_password"); + const password = watch("new_password"); + const retypePassword = watch("confirm_password"); + + const handleShowPassword = (key: keyof typeof showPassword) => + setShowPassword((prev) => ({ ...prev, [key]: !prev[key] })); + const handleChangePassword = async (formData: FormValues) => { - if (formData.new_password !== formData.confirm_password) { + try { + if (!csrfToken) throw new Error("csrf token not found"); + const changePasswordPromise = userService + .changePassword(csrfToken, { + old_password: formData.old_password, + new_password: formData.new_password, + }) + .then(() => { + reset(defaultValues); + }); + setPromiseToast(changePasswordPromise, { + loading: "Changing password...", + success: { + title: "Success!", + message: () => "Password changed successfully.", + }, + error: { + title: "Error!", + message: () => "Something went wrong. Please try again 1.", + }, + }); + } catch (err: any) { setToast({ type: TOAST_TYPE.ERROR, title: "Error!", - message: "The new password and the confirm password don't match.", + message: err?.error ?? "Something went wrong. Please try again 2.", }); - return; } - const changePasswordPromise = userService.changePassword(formData); - setPromiseToast(changePasswordPromise, { - loading: "Changing password...", - success: { - title: "Success!", - message: () => "Password changed successfully.", - }, - error: { - title: "Error!", - message: () => "Something went wrong. Please try again.", - }, - }); }; + useEffect(() => { + if (csrfToken === undefined) + authService.requestCSRFToken().then((data) => data?.csrf_token && setCsrfToken(data.csrf_token)); + }, [csrfToken]); + useEffect(() => { if (!currentUser) return; @@ -75,6 +110,25 @@ const ChangePasswordPage: NextPageWithLayout = observer(() => { else setIsPageLoading(false); }, [currentUser, router]); + const isButtonDisabled = useMemo( + () => + !isSubmitting && + !!oldPassword && + !!password && + !!retypePassword && + getPasswordStrength(password) >= 3 && + password === retypePassword && + password !== oldPassword + ? false + : true, + + [isSubmitting, oldPassword, password, retypePassword] + ); + + const passwordSupport = password.length > 0 && (getPasswordStrength(password) < 3 || isPasswordInputFocused) && ( + + ); + if (isPageLoading) return (
@@ -93,82 +147,126 @@ const ChangePasswordPage: NextPageWithLayout = observer(() => { onSubmit={handleSubmit(handleChangePassword)} className="mx-auto md:mt-16 mt-10 flex h-full w-full flex-col gap-8 px-4 md:px-8 pb-8 lg:w-3/5" > + +

Change password

Current password

- ( - + ( + + )} + /> + {showPassword?.oldPassword ? ( + handleShowPassword("oldPassword")} + /> + ) : ( + handleShowPassword("oldPassword")} /> )} - /> +
+ {errors.old_password && {errors.old_password.message}}

New password

- ( - + ( + setIsPasswordInputFocused(true)} + onBlur={() => setIsPasswordInputFocused(false)} + /> + )} + /> + {showPassword?.password ? ( + handleShowPassword("password")} + /> + ) : ( + handleShowPassword("password")} /> )} - /> - {errors.new_password && {errors.new_password.message}} +
+ {passwordSupport}

Confirm password

- ( - + ( + + )} + /> + {showPassword?.retypePassword ? ( + handleShowPassword("retypePassword")} + /> + ) : ( + handleShowPassword("retypePassword")} /> )} - /> - {errors.confirm_password && ( - {errors.confirm_password.message} +
+ {!!retypePassword && password !== retypePassword && ( + Passwords don{"'"}t match )}
-
diff --git a/web/services/user.service.ts b/web/services/user.service.ts index b9f9ce0fa..fa8a06542 100644 --- a/web/services/user.service.ts +++ b/web/services/user.service.ts @@ -143,8 +143,12 @@ export class UserService extends APIService { }); } - async changePassword(data: { old_password: string; new_password: string; confirm_password: string }): Promise { - return this.post(`/api/users/me/change-password/`, data) + async changePassword(token: string, data: { old_password: string; new_password: string }): Promise { + return this.post(`/auth/change-password/`, data, { + headers: { + "X-CSRFTOKEN": token, + }, + }) .then((response) => response?.data) .catch((error) => { throw error?.response?.data;