Skip to content

Commit

Permalink
feat: unconfirmed user login error fix (#3949)
Browse files Browse the repository at this point in the history
* feat: unconfirmed user login error fix

* fix: unconfirmed user attempting to login, public user logging into partner site, seeding es

* updates per cade

* fix: undefined check

* Merge remote-tracking branch 'origin/main' into security-patch-2

* fix: merge mistakes were made

---------

Co-authored-by: Cade Wolcott <cade.wolcott@exygy.com>
  • Loading branch information
YazeedLoonat and cade-exygy committed Mar 14, 2024
1 parent 18df523 commit 5801e31
Show file tree
Hide file tree
Showing 11 changed files with 233 additions and 177 deletions.
2 changes: 1 addition & 1 deletion api/Procfile
Original file line number Diff line number Diff line change
@@ -1 +1 @@
web: yarn start:prod
web: yarn db:migration:run && yarn start:prod
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ SET translations = jsonb_set(translations, '{singleUseCodeEmail}', '{"greeting":
WHERE jurisdiction_id IS NULL
and language = 'en';


UPDATE translations
SET translations = jsonb_set(translations, '{mfaCodeEmail, mfaCode}', '{"mfaCode": "Your access code is: %{singleUseCode}"}')
SET translations = jsonb_set(translations, '{mfaCodeEmail, mfaCode}', '"Your access code is: %{singleUseCode}"')
WHERE language = 'en';

348 changes: 193 additions & 155 deletions api/prisma/seed-helpers/translation-factory.ts

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions api/prisma/seed-staging.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {
ApplicationAddressTypeEnum,
ApplicationMethodsTypeEnum,
LanguagesEnum,
ListingsStatusEnum,
MultiselectQuestions,
MultiselectQuestionsApplicationSectionEnum,
Expand Down Expand Up @@ -92,6 +93,9 @@ export const stagingSeed = async (
await prismaClient.translations.create({
data: translationFactory(jurisdiction.id, jurisdiction.name),
});
await prismaClient.translations.create({
data: translationFactory(undefined, undefined, LanguagesEnum.es),
});
await prismaClient.translations.create({
data: translationFactory(),
});
Expand Down
24 changes: 12 additions & 12 deletions api/src/passports/mfa.strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,18 @@ export class MfaStrategy extends PassportStrategy(Strategy, 'mfa') {
Number(process.env.AUTH_LOCK_LOGIN_AFTER_FAILED_ATTEMPTS),
Number(process.env.AUTH_LOCK_LOGIN_COOLDOWN),
);
if (!rawUser.confirmedAt) {
if (!(await isPasswordValid(rawUser.passwordHash, dto.password))) {
// if incoming password does not match
await this.updateFailedLoginCount(
rawUser.failedLoginAttemptsCount + 1,
rawUser.id,
);
throw new UnauthorizedException({
failureCountRemaining:
Number(process.env.AUTH_LOCK_LOGIN_AFTER_FAILED_ATTEMPTS) -
rawUser.failedLoginAttemptsCount,
});
} else if (!rawUser.confirmedAt) {
// if user is not confirmed already
throw new UnauthorizedException(
`user ${rawUser.id} attempted to login, but is not confirmed`,
Expand All @@ -78,17 +89,6 @@ export class MfaStrategy extends PassportStrategy(Strategy, 'mfa') {
throw new UnauthorizedException(
`user ${rawUser.id} attempted to login, but password is no longer valid`,
);
} else if (!(await isPasswordValid(rawUser.passwordHash, dto.password))) {
// if incoming password does not match
await this.updateFailedLoginCount(
rawUser.failedLoginAttemptsCount + 1,
rawUser.id,
);
throw new UnauthorizedException({
failureCountRemaining:
Number(process.env.AUTH_LOCK_LOGIN_AFTER_FAILED_ATTEMPTS) -
rawUser.failedLoginAttemptsCount,
});
}

if (!rawUser.mfaEnabled) {
Expand Down
2 changes: 2 additions & 0 deletions api/test/unit/passports/mfa.strategy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ describe('Testing mfa strategy', () => {
lastLoginAt: new Date(),
failedLoginAttemptsCount: 0,
confirmedAt: null,
passwordHash: await passwordToHash('abcdef'),
});

const request = {
Expand Down Expand Up @@ -127,6 +128,7 @@ describe('Testing mfa strategy', () => {
passwordValidForDays: 0,
passwordUpdatedAt: new Date(0),
userRoles: { isAdmin: true },
passwordHash: await passwordToHash('abcdef'),
});

const request = {
Expand Down
16 changes: 13 additions & 3 deletions shared-helpers/src/auth/AuthContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ type ContextProps = {
email: string,
password: string,
mfaCode?: string,
mfaType?: MfaType
mfaType?: MfaType,
forPartners?: boolean
) => Promise<User | undefined>
resetPassword: (
token: string,
Expand Down Expand Up @@ -223,16 +224,25 @@ export const AuthProvider: FunctionComponent<React.PropsWithChildren> = ({ child
email,
password,
mfaCode: string | undefined = undefined,
mfaType: MfaType | undefined = undefined
mfaType: MfaType | undefined = undefined,
forPartners: boolean | undefined = undefined
) => {
dispatch(startLoading())
try {
const response = await authService?.login({ body: { email, password, mfaCode, mfaType } })
if (response) {
const profile = await userService?.profile()
if (profile) {
if (
profile &&
(!forPartners ||
profile.userRoles?.isAdmin ||
profile.userRoles?.isJurisdictionalAdmin ||
profile.userRoles?.isPartner)
) {
dispatch(saveProfile(profile))
return profile
} else {
throw Error("User cannot log in")
}
}
return undefined
Expand Down
2 changes: 1 addition & 1 deletion shared-helpers/src/auth/catchNetworkError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export const useCatchNetworkError = () => {
const [networkError, setNetworkError] = useState<NetworkStatusContent>(null)

const check401Error = (message: string, error: AxiosError) => {
if (message.includes(NetworkErrorMessage.PasswordOutdated)) {
if (message?.includes(NetworkErrorMessage.PasswordOutdated)) {
setNetworkError({
title: t("authentication.signIn.passwordOutdated"),
description: `${t(
Expand Down
4 changes: 2 additions & 2 deletions sites/partners/src/lib/users/signInHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export const onSubmitEmailAndPassword =
async (data: { email: string; password: string }) => {
const { email, password } = data
try {
await login(email, password)
await login(email, password, undefined, undefined, true)
await router.push("/")
} catch (error) {
if (error?.response?.data?.name === "mfaCodeIsMissing") {
Expand Down Expand Up @@ -86,7 +86,7 @@ export const onSubmitMfaCode =
async (data: { mfaCode: string }) => {
const { mfaCode } = data
try {
await login(email, password, mfaCode, mfaType)
await login(email, password, mfaCode, mfaType, true)
resetNetworkError()
await router.push("/")
} catch (error) {
Expand Down
2 changes: 1 addition & 1 deletion sites/partners/src/pages/sign-in.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ const SignIn = () => {
)

useEffect(() => {
if (networkError?.error.response.data?.message === "accountConfirmed") {
if (networkError?.error.response?.data?.message === "accountConfirmed") {
setConfirmationStatusModal(true)
}
}, [networkError])
Expand Down
2 changes: 1 addition & 1 deletion sites/public/src/pages/sign-in.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ const SignIn = () => {
})()

useEffect(() => {
if (networkError?.error?.response?.data?.message === "accountNotConfirmed") {
if (networkError?.error?.response?.data?.message?.includes("but is not confirmed")) {
setConfirmationStatusModal(true)
}
}, [networkError])
Expand Down

0 comments on commit 5801e31

Please sign in to comment.