feat(condo): DOMA-12564 added 2FA (frontend) #6968
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds end-to-end two‑factor authentication (2FA): new GraphQL types/mutations and errors, server-side 2FA-aware credential validation, frontend 2FA hooks/components and modal flows, masking utilities/tests, schema/client updates, and localization/UI tweaks. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Client as Client (SignIn/Register)
participant Frontend2FA as SecondFactor UI
participant Backend as Backend API
participant AuthSvc as Auth Validation/DB
User->>Client: Submit primary credentials
Client->>Backend: authenticate* mutation (primary)
Backend->>AuthSvc: validateUserCredentials(primary)
alt Sufficient factors
AuthSvc-->>Backend: success (session token)
Backend-->>Client: auth token / success
Client-->>User: redirect
else NOT_ENOUGH_AUTH_FACTORS
AuthSvc-->>Backend: error NOT_ENOUGH_AUTH_FACTORS + authDetails
Backend-->>Client: NOT_ENOUGH_AUTH_FACTORS + availableSecondFactors, maskedData
Client->>Frontend2FA: show second-factor UI with maskedData
User->>Frontend2FA: select factor
Frontend2FA->>Backend: startConfirmEmailAction/startConfirmPhoneAction
Backend-->>Frontend2FA: code sent
User->>Frontend2FA: enter code
Frontend2FA->>Backend: authenticate* mutation + secondFactor
Backend->>AuthSvc: validate secondFactor
AuthSvc-->>Backend: success (session token)
Backend-->>Client: auth token / success
Client-->>User: redirect
end
sequenceDiagram
actor User
participant Settings as User Settings UI
participant SudoModal as SudoTokenProvider
participant Backend as Backend API
participant ConfirmSvc as ConfirmActionService
User->>Settings: Click "Enable/Disable 2FA"
Settings->>SudoModal: getSudoTokenWithModal()
SudoModal->>User: show password prompt
User->>SudoModal: submit password
SudoModal->>Backend: generateSudoToken(password)
alt NOT_ENOUGH_AUTH_FACTORS
Backend-->>SudoModal: NOT_ENOUGH_AUTH_FACTORS + authDetails
SudoModal->>Settings: open SecondFactor modal with authDetails
User->>SudoModal: choose factor / request code
SudoModal->>ConfirmSvc: startConfirm*Action(user/identifier)
ConfirmSvc-->>SudoModal: code sent
User->>SudoModal: enter code
SudoModal->>Backend: generateSudoToken(confirmToken)
Backend-->>SudoModal: sudo token
else Direct success
Backend-->>SudoModal: sudo token
end
SudoModal-->>Settings: return sudo token
Settings->>Backend: changeTwoFactorAuthentication(sudoToken)
Backend-->>Settings: success
Settings-->>User: updated 2FA state
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.40.5)apps/condo/domains/user/schema/AuthenticateOrRegisterUserWithTokenService.test.js[] apps/condo/domains/user/utils/serverSchema/auth.spec.jsThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/condo/domains/user/schema/ConfirmEmailActionService.js (1)
321-335: Rate limiting uses non-normalized email while storage uses normalized email.Line 335 calls
checkEmailSendingLimits(context, email, ip)with the rawConfirmEmailActionis created withnormalizedEmail(line 352). This inconsistency could allow rate limit bypass by using different email casing/formatting.Additionally, when
userFromInput?.idis provided but the user has no email set,null, which will fail atnormalizeEmail(email)with a potentially confusing error message.Consider normalizing email before rate limiting and adding explicit handling for missing user email:
if (userFromInput?.id) { const user = await User.getOne(context, { id: userFromInput.id, deletedAt: null }, 'id email') email = user?.email || null + if (!email) { + throw new GQLError({ + mutation: 'startConfirmEmailAction', + code: BAD_USER_INPUT, + type: 'USER_HAS_NO_EMAIL', + message: 'User does not have an email address', + }, context) + } } else { email = emailFromInput } const normalizedEmail = normalizeEmail(email) if (!normalizedEmail) { throw new GQLError(ERRORS.WRONG_EMAIL_FORMAT, context) } - await checkEmailSendingLimits(context, email, ip) + await checkEmailSendingLimits(context, normalizedEmail, ip)
🧹 Nitpick comments (37)
apps/condo/domains/common/utils/helpers.ts (1)
21-35: Masked phone formatting logic is sound; ensure input shape matches masked regex expectationsThe new masked regexes and
isMaskedPhoneflag preserve existing behavior for normal phones and correctly avoidnormalizePhoneso*characters aren’t stripped. One thing to watch:RUSSIAN_MASKED_PHONE_FORMAT_REGEXP/SPANISH_MASKED_PHONE_FORMAT_REGEXPassume the portion after+7/+34is a contiguous block of digits/*without spaces or other separators. IfmaskPhone(or any caller) ever passes already formatted masked values like+7 *** *** ** **, these patterns won’t match and you’ll quietly fall back to the unformatted string.If that’s a possible input, consider:
- Either normalizing masked input to
+7/+34+[0-9*]+before callingformatPhone(..., true), or- Relaxing the masked regexes to tolerate spaces/parentheses, and documenting the expected format via a short JSDoc on
formatPhone.Overall, behavior for existing callers (
isMaskedPhonedefaulting tofalse) remains unchanged.apps/condo/domains/user/constants/authFactors.js (1)
1-10: AUTH_FACTOR_TYPES enum is clear and matches intended 2FA factor typesThe constant names and string values (
confirmPhoneToken,confirmEmailToken,password) are self-explanatory and will keep multi-factor flows readable. If you want to guard against accidental mutation, you could optionally freeze it:const AUTH_FACTOR_TYPES = Object.freeze({ CONFIRM_PHONE_TOKEN: 'confirmPhoneToken', CONFIRM_EMAIL_TOKEN: 'confirmEmailToken', PASSWORD: 'password', })but that’s not strictly necessary.
apps/condo/lang/ru/ru.json (1)
2391-2392: Profile 2FA section copy is understandable; factor placeholder keeps it flexible
pages.condo.profile.2FA.title/.tooltipcorrectly describe the feature and guide the user to the “Информация” tab. Using{factors}as a placeholder allows you to localize or vary the required factor list at the call site (e.g. “почту и телефон”) without touching translations again, which is a good choice.Optionally, ensure the
{factors}phrase passed from code is also localized (rather than hard‑coding Russian in JSX), to keep copy consistent across languages.apps/condo/domains/common/utils/phone.spec.js (1)
78-95: LGTM! Comprehensive test coverage for the masking function.The parameterized tests cover edge cases well, including boundary conditions for different string lengths. The masking logic is thoroughly verified.
Minor nit: The test description "should correct work" could be "should work correctly" for grammatical accuracy.
- test.each(cases)('should correct work (%p)', (input, output) => { + test.each(cases)('should work correctly (%p)', (input, output) => {apps/condo/domains/common/utils/mail.spec.js (1)
1-1: maskEmail test coverage matches implementation and common casesThe new import and
maskEmail()tests look correct and align with the helper’s behavior (first 1–2 chars +***and full domain). You’re covering several username lengths and a non‑trivial domain (yandex.ru), which should prevent regressions.If you want to harden further, you could optionally add a case for edge inputs (e.g.
'@example.com'or already-lower/uppercased emails), but it’s not strictly necessary.Also applies to: 68-82
apps/condo/domains/common/utils/phone.js (1)
13-40: maskPhone logic is sound; consider a small defensive guardThe masking rules (preserve optional
+, reveal 1–2 leading and 1–2 trailing digits, mask the rest) are consistent and match typical phone masking expectations. Export wiring is correct.To reduce the chance of runtime errors if a non-string sneaks in (e.g.
undefinedbefore normalization), you could add a lightweight type guard:-function maskPhone (normalizedPhone) { - const hasPlus = normalizedPhone.startsWith('+') - const digits = normalizedPhone.replace(/\D/g, '') +function maskPhone (normalizedPhone) { + if (typeof normalizedPhone !== 'string') return normalizedPhone + + const hasPlus = normalizedPhone.startsWith('+') + const digits = normalizedPhone.replace(/\D/g, '')This keeps current behavior for valid inputs while failing gracefully for unexpected ones.
Also applies to: 45-45
apps/condo/domains/user/schema/AuthenticateUserWithEmailAndPasswordService.test.js (1)
14-15: 2FA tests are well‑designed; consider loosening factor list assertionThe new imports and 2FA tests correctly exercise:
- Failure path when 2FA is enabled but only password is provided (expecting
NOT_ENOUGH_AUTH_FACTORSwithauthDetailsand masked email/phone).- Success paths for both
confirmPhoneTokenandconfirmEmailToken, including verifying that related Confirm*Action records are marked as completed.That gives good coverage of the new behavior.
To make the tests slightly less brittle against future additions or ordering changes of second factors, you could switch:
availableSecondFactors: ['confirmEmailToken', 'confirmPhoneToken'],to something like:
availableSecondFactors: expect.arrayContaining(['confirmEmailToken', 'confirmPhoneToken']),but this is an optional hardening, not a blocker.
Also applies to: 31-35, 183-263
apps/condo/domains/common/utils/mail.js (1)
57-70: maskEmail implementation aligns with tests and intended maskingThe helper correctly:
- Leaves invalid shapes (
'no-at-sign') untouched by returning the original value when there is no domain.- For valid emails, reveals up to the first two user characters and appends
***@domain, matching the new test cases.If you want parity with
maskPhoneand a bit more robustness, you could optionally guard against non-string inputs in the same way, but current behavior is acceptable given the “normalizedEmail” contract.Also applies to: 73-76
apps/condo/domains/user/schema/GenerateSudoTokenService.test.js (1)
16-17: GenerateSudoToken 2FA tests correctly exercise new error pathsThe added imports and tests do a good job of:
- Ensuring missing identity (no phone/email/confirm tokens, only password) yields
CREDENTIAL_VALIDATION_FAILED.- Verifying that a 2FA‑enabled user with only a password gets
NOT_ENOUGH_AUTH_FACTORSplusauthDetailsthat includeis2FAEnabled,userId,availableSecondFactors, and masked email/phone using the shared helpers.That keeps the sudo‑token flow consistent with the other auth services.
As with the email/password tests, you might optionally relax the
availableSecondFactorsassertion to:availableSecondFactors: expect.arrayContaining(['confirmEmailToken', 'confirmPhoneToken']),to make future factor additions less disruptive, but that’s a nice‑to‑have.
Also applies to: 125-139, 178-204
apps/condo/domains/user/schema/AuthenticateOrRegisterUserWithTokenService.test.js (1)
1276-1305: NOT_ENOUGH_AUTH_FACTORS test is well-structured and verifies masking payload
- The test correctly models a user with 2FA enabled and only one factor (phone token) provided.
- Expected error:
type: 'NOT_ENOUGH_AUTH_FACTORS'authDetailscontainingis2FAEnabled,userId,availableSecondFactors, andmaskedData.email.- Using
maskEmail(userAttrs.email)in the expectation keeps the test resilient to masking implementation details while still asserting that sensitive data is properly obfuscated in the error payload.This is a solid, behavior-focused test for the new 2FA error path.
If you want to make this slightly more future‑proof, you could also assert that
availableSecondFactorscontains the expected values at least (e.g. viaexpect.arrayContaining) instead of an exact array, so adding new factors won’t break the test.apps/condo/lang/es/es.json (3)
718-731: Unify tone and 2FA terminology; align resend phrasing.Use tuteo (“Introduce …”), “autenticación” instead of “autorización”, “correo electrónico” over “email”, and “Reenviar código” for brevity and consistency.
- "component.SecondFactorForm.title.forAuthorization": "Segundo factor de autorización", + "component.SecondFactorForm.title.forAuthorization": "Segundo factor de autenticación", - "component.SecondFactorForm.type.confirmEmailToken.description": "Introduce el código enviado a {email}", + "component.SecondFactorForm.type.confirmEmailToken.description": "Introduce el código enviado a {email}", - "component.SecondFactorForm.type.confirmEmailToken.label": "Por email", + "component.SecondFactorForm.type.confirmEmailToken.label": "Por correo electrónico", - "component.SecondFactorForm.type.confirmEmailToken.sendCodeAgain": "Enviar el código de nuevo", + "component.SecondFactorForm.type.confirmEmailToken.sendCodeAgain": "Reenviar código", - "component.SecondFactorForm.type.confirmPhoneToken.description": "Ingrese el código enviado a {phone}", + "component.SecondFactorForm.type.confirmPhoneToken.description": "Introduce el código enviado al número {phone}", - "component.SecondFactorForm.type.confirmPhoneToken.sendCodeAgain": "Enviar el código de nuevo", + "component.SecondFactorForm.type.confirmPhoneToken.sendCodeAgain": "Reenviar código", - "component.SecondFactorForm.type.password.description": "Introduzca la contraseña", + "component.SecondFactorForm.type.password.description": "Introduce la contraseña"
734-738: Consistent tuteo and action verbs for enable/disable.Switch to “Introduce …” and use the “activar/desactivar” pair.
- "component.SudoTokenProvider.modal.password.label": "Introduzca la contraseña", + "component.SudoTokenProvider.modal.password.label": "Introduce la contraseña", - "component.SudoTokenProvider.modal.password.title.disable2FA": "Para desactivar la autenticación de dos factores, necesitas ingresar una contraseña.", + "component.SudoTokenProvider.modal.password.title.disable2FA": "Para desactivar la autenticación de dos factores, introduce la contraseña.", - "component.SudoTokenProvider.modal.password.title.enable2FA": "Para habilitar la autenticación de dos factores, necesitas ingresar una contraseña.", + "component.SudoTokenProvider.modal.password.title.enable2FA": "Para activar la autenticación de dos factores, introduce la contraseña."
2391-2393: Match register and phrasing in tooltip.Prefer tuteo and simpler phrasing; use Spanish quotes consistent with the file.
- "pages.condo.profile.2FA.tooltip": "Para permitirnos enviarle el código, por favor proporcione sus {factors}. Para hacerlo, haga clic en \"Editar\" en la pestaña \"Información\" y agregue cualquier información que falte.", + "pages.condo.profile.2FA.tooltip": "Para poder enviarte el código, añade tus {factors}. Para ello, haz clic en «Editar» en la pestaña «Información» y añade la información que falte."apps/condo/domains/user/components/UserSettingsContent.tsx (1)
249-252: Consider adding error handling for better UX.If
change2FAthrows an error (which it does on failure), there's no error handling here to provide user feedback. The switch UI may appear unresponsive without any indication of what went wrong.const handle2FAEnabledChange: SwitchChangeEventHandler = useCallback(async (checked) => { - await change2FA(checked) - setIs2FAEnabled(checked) + try { + await change2FA(checked) + setIs2FAEnabled(checked) + } catch (e) { + // Error already logged in change2FA + // Optionally show notification to user here + } }, [change2FA])apps/condo/lang/en/en.json (1)
718-730: Second factor copy is clear; only minor wording polish possibleThe new 2FA strings read well and are consistent with surrounding auth copy. If you want slightly more natural English, consider tweaks like:
"Second factor of authorization"→"Second authentication factor"or"Two-factor authentication"."By password"→"Using password".Purely stylistic; current wording is acceptable.
Also applies to: 734-737
apps/condo/domains/user/utils/serverSchema/auth.js (2)
192-222: Token-derived phone/email fallback is reasonable; consider tiny optimizationLetting
_getUserderivephone/confirmPhoneToken/confirmEmailTokenwhenever both identifiers areundefinedmakes the flow more flexible for token‑only auth/2FA scenarios and still relies on_matchUser*to enforce expiry and verification.If you ever see performance pressure on
Userqueries, you could early‑return when both tokens resolve to no actions (i.e.phoneanditemsQuery('User', …)with only{ type, deletedAt: null }. This is purely a micro‑optimization; behaviour today is correct.
139-139: Return-type JSDoc is accurate but hard to maintainThe updated
@returnJSDoc forvalidateUserCredentialsand_matchUserreflects the richer_errorpayload, but the union type is now quite long and duplicated.Consider introducing a named typedef (e.g.
@typedef AuthErrorPayload) and reusing it in both places to reduce drift and keep the docs readable.Also applies to: 264-264
apps/condo/pages/auth/change-password.tsx (2)
546-554: Use JSX children instead ofchildrenprop.As flagged by Biome, passing children via prop is not the canonical React pattern.
Apply this diff:
<Button key='submit' type='primary' loading={isLoading} htmlType='submit' - children={`${SaveMsg} ${AndSignInMessage}`} block - /> + > + {`${SaveMsg} ${AndSignInMessage}`} + </Button>
525-525: Consider using a ternary for clarity.The conditional style expression
style={(currentSecondFactor || step !== 'changePassword') && { display: 'none' }}could evaluate tofalse, which while handled by React, is less explicit than a ternary.- <Row justify='center' style={(currentSecondFactor || step !== 'changePassword') && { display: 'none' }}> + <Row justify='center' style={(currentSecondFactor || step !== 'changePassword') ? { display: 'none' } : undefined}>apps/condo/gql/index.ts (1)
7307-7339: 2FA mutation hook is consistent with generated types and surrounding auth mutationsThe
changeTwoFactorAuthenticationoperation, TS mutation function alias, hook, and exported helper types all line up with the definitions inoperation.types.tsand follow the existing codegen pattern used for other auth mutations in this file. No issues from the frontend/graphql side.If you find yourself manually refetching
authenticatedUserafter toggling 2FA, consider (in the source.graphqloperation, not this generated file) returning the updated user booleans (e.g.isTwoFactorAuthenticationEnabled) from this mutation to simplify optimistic updates and cache syncing. Based on learnings, this file is generated, so changes should come from the schema/operations, not here.apps/condo/domains/user/schema/ConfirmEmailActionService.js (1)
86-101: TODO comments indicate missing translations.The error definitions include
TODO(DOMA-12564): translatecomments formessageForUserfields. Ensure these translations are added before release to provide proper localized error messages to users.Would you like me to open an issue to track the translation work for these error messages?
apps/condo/domains/user/schema/AuthenticateUserWithEmailAndPasswordService.js (1)
135-151: Consider extracting 2FA error handling to a shared utility.This pattern of checking for
NOT_ENOUGH_AUTH_FACTORSand throwing a detailed error withauthDetailsis duplicated across multiple authentication services (this file andAuthenticateUserWithPhoneAndPasswordService.jsbased on the summary). Consider extracting this to a shared helper function.apps/condo/domains/user/components/auth/SignInForm.tsx (2)
228-247: SecondFactorHeader creates new component instance on each render.Using
useCallbackfor a component that returns JSX doesn't prevent re-renders of its children. The callback itself is stable, but when called, it returns fresh JSX. Consider extracting this to a separate component or usinguseMemofor the JSX if you want to prevent re-renders.
108-118: Consider using TypeScript interface for handleSubmit parameters.The
handleSubmitfunction accepts many parameters as an object. Defining a TypeScript interface would improve type safety and documentation.interface SecondFactorSubmitParams { isLoading: boolean setIsLoading: (loading: boolean) => void userIdRef: React.MutableRefObject<string | null> setUserMaskedData: (data: { email?: string; phone?: string } | null) => void setAvailableSecondFactors: (factors: string[]) => void setCurrentSecondFactor: (factor: string | null) => void currentSecondFactor: string | null confirmEmailToken: string | null confirmPhoneToken: string | null }apps/condo/domains/user/components/auth/RegisterForm.tsx (1)
211-231: Duplicate SecondFactorHeader component between SignInForm and RegisterForm.The
SecondFactorHeaderimplementation is nearly identical in both files. Consider extracting this to a shared component to reduce duplication.// In a shared file like SecondFactorHeader.tsx export const createSecondFactorHeader = (title: string, onBack: () => void) => ( <Col span={24}> <Row gutter={[0, 16]}> <Col span={24}> <Space direction='vertical' size={24}> <Button.Icon onClick={onBack} size='small'> <ArrowLeft /> </Button.Icon> <Typography.Title level={2}>{title}</Typography.Title> </Space> </Col> </Row> </Col> )apps/condo/domains/user/components/SudoTokenProvider.tsx (1)
567-567: Using string replace instead of intl.formatMessage for placeholders.Lines 567 and 591 use
.replace('{phone}', ...)and.replace('{email}', ...)for placeholder substitution. This bypasses the standard i18n pattern and could cause issues if the translation format changes.Consider using
intl.formatMessagewith values:const ConfirmPhoneTokenDescription = intl.formatMessage( { id: 'component.SecondFactorForm.type.confirmPhoneToken.description' }, { phone: formatPhone(userMaskedData?.phone || '', true) } )Then use the message directly without
.replace().Also applies to: 591-591
apps/condo/domains/user/components/auth/SecondFactorForm.tsx (4)
119-122: Redundant expiry date setting.The expiry date is set twice: once before the mutation (line 122) and again after (line 140) with identical values. The first assignment provides immediate rate-limiting UI feedback, but if that's the intent, the second assignment is unnecessary.
const shouldWait = confirmEmailTokenExpiredDate && dayjs(confirmEmailTokenExpiredDate) > dayjs() if (shouldWait) return setConfirmEmailTokenExpiredDate(dayjs().add(1, 'minute').toISOString()) const sender = getClientSideSenderInfo() const captcha = await executeCaptcha() const res = await startConfirmEmailActionMutation({ variables: { data: { dv: 1, sender, captcha, user: { id: userIdRef.current, }, }, }, }) - setConfirmEmailTokenExpiredDate(dayjs().add(1, 'minute').toISOString()) - const token = res?.data?.result?.tokenAlso applies to: 140-140
254-265: Missing dependencies in useEffect.The effect depends on
startConfirmPhoneActionandstartConfirmEmailActioncallbacks but they're omitted from the dependency array. This could lead to stale closure issues if the callbacks reference outdated state. If this is intentional to only trigger oncurrentSecondFactorchanges, consider adding an ESLint disable comment with an explanation.useEffect(() => { if (currentSecondFactor !== 'confirmEmailToken' && currentSecondFactor !== 'confirmPhoneToken') return if (!userIdRef.current) return if (currentSecondFactor === 'confirmPhoneToken') { // send sms-code by userId startConfirmPhoneAction() } else if (currentSecondFactor === 'confirmEmailToken') { // send email-code by userId startConfirmEmailAction() } + // eslint-disable-next-line react-hooks/exhaustive-deps -- Only trigger when currentSecondFactor changes }, [currentSecondFactor])
230-244: Fragile error message extraction with@ts-ignore.The error extraction uses
@ts-ignoreand accesses a deeply nested path that may not match Apollo's error structure. Consider using a type-safe utility or properly typing the response to avoid runtime issues.
31-37: Consider adding TypeScript types for hook parameters and return value.The
useSecondFactorhook lacks TypeScript type definitions for its parameters and return value. Adding proper types would improve IDE support and catch potential misuse at compile time.type UseSecondFactorParams = { form: FormInstance onSubmit: (params: OnSubmitParams) => Promise<void> Header?: React.ComponentType<HeaderProps> isLoading: boolean setIsLoading: React.Dispatch<React.SetStateAction<boolean>> } type UseSecondFactorReturn = { SecondFactorForm: React.ReactNode ProblemsModal: React.ReactNode onSubmitWithSecondFactor: () => Promise<void> currentSecondFactor: 'confirmPhoneToken' | 'confirmEmailToken' | 'password' | null }apps/condo/domains/user/schema/AuthenticateUserWithPhoneAndPasswordService.js (2)
55-62: TODO comment indicates missing translations.The TODO on line 60 indicates translations need to be added for
api.user.NOT_ENOUGH_AUTH_FACTORS. Ensure this is tracked and completed before release to avoid showing untranslated error messages to users.Would you like me to open an issue to track this translation task?
150-162: Consider error handling for ConfirmAction updates.If the
ConfirmEmailAction.updateorConfirmPhoneAction.updatecalls fail after successful validation, the user authentication succeeds but the action completion isn't recorded. This could leave orphaned action records. Consider wrapping in try-catch or handling failures gracefully.apps/condo/schema.ts (3)
2457-2478: Email+password second factor type/enum mirror token flow; consider enum reuse only if desiredThe
AuthenticateUserWithEmailAndPasswordSecondFactorInputand its enum (ConfirmEmailToken, ConfirmPhoneToken) are consistent with the token flow, and attaching them viasecondFactoron the input looks correct. If you find yourself duplicating these exact enum variants across multiple flows, you might consider a shared GraphQL enum for “email/phone token” factors to avoid drift over time, but it’s not strictly necessary.
96333-96340: StartConfirmEmailActionInput now allows user reference; ensure invariants are validatedMaking
user?: UserWhereUniqueInputlets callers initiate email confirmation either by raw email or by user id, which is flexible. However, the type alone now permits requests with neither field (or conflicting values), so please make sure the resolver enforces a clear contract (e.g. “require at least one ofuser, and if both are provided they must match”) and that any corresponding API docs describe this behavior.
96347-96353: StartConfirmPhoneActionInput mirrors email flow; same validation concern
phonebecoming optional anduser?: UserWhereUniqueInputbeing added gives analogous flexibility for phone confirmations. As with email, the schema now permits calls without eitherphoneoruser, so the resolver should mandate at least one identifier and define how to handle both present (e.g. mismatch cases). Worth double‑checking tests for these edge conditions.apps/condo/domains/user/schema/AuthenticateOrRegisterUserWithTokenService.js (2)
161-176: TODO comments for translations should be tracked.The error definitions follow existing patterns, but the TODO comments for translations at lines 166-167 and 174-175 should be addressed before the feature is fully released to ensure proper user-facing messages.
Also, minor inconsistency:
NOT_ENOUGH_AUTH_FACTORSuses an imported constant fortype(line 164), whileUNEXPECTED_SECOND_FACTORuses a string literal (line 172). Consider using an imported constant for consistency:UNEXPECTED_SECOND_FACTOR: { mutation: 'authenticateOrRegisterUserWithToken', code: BAD_USER_INPUT, - type: 'UNEXPECTED_SECOND_FACTOR', + type: UNEXPECTED_SECOND_FACTOR, // import from constants/errors message: 'Unexpected second factor',Do you want me to open an issue to track the missing translations for DOMA-12564?
430-448: Second factor validation logic is sound.The logic correctly prevents using the same factor type for both primary and secondary authentication. The validation against
supportedSecondFactorsproperly enforces the constraint.One observation: lines 440, 443, and 446 use
secondFactor?.value || ''as a fallback. Since the GraphQL schema definesvalue: String!(required), this fallback should never be triggered. However, if it were, passing an empty string would likely cause downstream validation to fail silently rather than explicitly. Consider logging or handling this edge case more explicitly if you want stricter guarantees:- if (secondFactor.type === AUTH_FACTOR_TYPES.CONFIRM_EMAIL_TOKEN) { - authFactors.confirmEmailToken = secondFactor?.value || '' - } + if (secondFactor.type === AUTH_FACTOR_TYPES.CONFIRM_EMAIL_TOKEN) { + authFactors.confirmEmailToken = secondFactor.value + }Since the schema already enforces the requirement, the defensive
|| ''may be unnecessary.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (40)
apps/condo/domains/common/utils/helpers.ts(1 hunks)apps/condo/domains/common/utils/mail.js(1 hunks)apps/condo/domains/common/utils/mail.spec.js(2 hunks)apps/condo/domains/common/utils/phone.js(1 hunks)apps/condo/domains/common/utils/phone.spec.js(2 hunks)apps/condo/domains/user/components/SudoTokenProvider.tsx(14 hunks)apps/condo/domains/user/components/UserSettingsContent.tsx(9 hunks)apps/condo/domains/user/components/auth/RegisterForm.tsx(11 hunks)apps/condo/domains/user/components/auth/SecondFactorForm.tsx(1 hunks)apps/condo/domains/user/components/auth/SignInForm.tsx(5 hunks)apps/condo/domains/user/components/auth/ValidateIdentifierForm.tsx(7 hunks)apps/condo/domains/user/constants/authFactors.js(1 hunks)apps/condo/domains/user/constants/errors.js(2 hunks)apps/condo/domains/user/gql.js(2 hunks)apps/condo/domains/user/queries/ChangeTwoFactorAuthentication.graphql(1 hunks)apps/condo/domains/user/queries/User.graphql(1 hunks)apps/condo/domains/user/schema/AuthenticateOrRegisterUserWithTokenService.js(5 hunks)apps/condo/domains/user/schema/AuthenticateOrRegisterUserWithTokenService.test.js(4 hunks)apps/condo/domains/user/schema/AuthenticateUserWithEmailAndPasswordService.js(5 hunks)apps/condo/domains/user/schema/AuthenticateUserWithEmailAndPasswordService.test.js(3 hunks)apps/condo/domains/user/schema/AuthenticateUserWithPhoneAndPasswordService.js(5 hunks)apps/condo/domains/user/schema/AuthenticateUserWithPhoneAndPasswordService.test.js(3 hunks)apps/condo/domains/user/schema/ConfirmEmailActionService.js(6 hunks)apps/condo/domains/user/schema/ConfirmEmailActionService.test.js(4 hunks)apps/condo/domains/user/schema/ConfirmPhoneActionService.js(5 hunks)apps/condo/domains/user/schema/ConfirmPhoneActionService.test.js(1 hunks)apps/condo/domains/user/schema/GenerateSudoTokenService.js(3 hunks)apps/condo/domains/user/schema/GenerateSudoTokenService.test.js(3 hunks)apps/condo/domains/user/utils/serverSchema/auth.js(8 hunks)apps/condo/domains/user/utils/serverSchema/auth.spec.js(30 hunks)apps/condo/domains/user/utils/testSchema/index.js(3 hunks)apps/condo/gql/index.ts(2 hunks)apps/condo/gql/operation.types.ts(2 hunks)apps/condo/lang/en/en.json(2 hunks)apps/condo/lang/es/es.json(2 hunks)apps/condo/lang/ru/ru.json(2 hunks)apps/condo/pages/auth/change-password.tsx(13 hunks)apps/condo/pages/auth/signin.tsx(2 hunks)apps/condo/schema.graphql(9 hunks)apps/condo/schema.ts(11 hunks)
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2025-09-17T13:39:10.781Z
Learnt from: SavelevMatthew
Repo: open-condo-software/condo PR: 6645
File: apps/dev-portal-web/domains/common/components/auth/LoginForm.tsx:1-3
Timestamp: 2025-09-17T13:39:10.781Z
Learning: In the condo project, authMethods fallback is handled at the Next.js configuration level in next.config.ts with AUTH_METHODS defaulting to ["condo"], making direct destructuring of publicRuntimeConfig.authMethods safe without additional guards in components.
Applied to files:
apps/condo/pages/auth/signin.tsxapps/condo/domains/user/components/SudoTokenProvider.tsxapps/condo/domains/user/schema/AuthenticateUserWithPhoneAndPasswordService.jsapps/condo/domains/user/components/auth/SignInForm.tsxapps/condo/domains/user/components/auth/SecondFactorForm.tsxapps/condo/domains/user/schema/AuthenticateUserWithEmailAndPasswordService.jsapps/condo/domains/user/utils/serverSchema/auth.jsapps/condo/domains/user/components/UserSettingsContent.tsxapps/condo/domains/user/components/auth/RegisterForm.tsxapps/condo/pages/auth/change-password.tsx
📚 Learning: 2025-09-17T13:39:10.781Z
Learnt from: SavelevMatthew
Repo: open-condo-software/condo PR: 6645
File: apps/dev-portal-web/domains/common/components/auth/LoginForm.tsx:1-3
Timestamp: 2025-09-17T13:39:10.781Z
Learning: In the condo project, authMethods fallback is handled at the Next.js configuration level in next.config.ts with AUTH_METHODS defaulting to '["condo"]' via JSON.parse(conf['AUTH_METHODS'] || '["condo"]'), making direct destructuring of publicRuntimeConfig.authMethods safe without additional guards in components.
Applied to files:
apps/condo/pages/auth/signin.tsxapps/condo/domains/user/constants/authFactors.jsapps/condo/domains/user/components/SudoTokenProvider.tsxapps/condo/domains/user/schema/AuthenticateUserWithPhoneAndPasswordService.jsapps/condo/domains/user/components/auth/SignInForm.tsxapps/condo/domains/user/schema/GenerateSudoTokenService.jsapps/condo/domains/user/schema/AuthenticateUserWithEmailAndPasswordService.jsapps/condo/domains/user/utils/serverSchema/auth.jsapps/condo/domains/user/components/UserSettingsContent.tsxapps/condo/domains/user/components/auth/RegisterForm.tsxapps/condo/pages/auth/change-password.tsx
📚 Learning: 2025-09-17T13:38:39.633Z
Learnt from: SavelevMatthew
Repo: open-condo-software/condo PR: 6645
File: apps/dev-portal-web/domains/common/components/auth/AuthForm.tsx:12-12
Timestamp: 2025-09-17T13:38:39.633Z
Learning: In the condo project, runtime configuration fallbacks for authMethods are handled at the Next.js config level in next.config.ts, where AUTH_METHODS defaults to ['condo'] if not provided via environment variables, ensuring safe destructuring in components that access publicRuntimeConfig.authMethods.
Applied to files:
apps/condo/pages/auth/signin.tsxapps/condo/domains/user/components/SudoTokenProvider.tsxapps/condo/domains/user/schema/AuthenticateUserWithPhoneAndPasswordService.jsapps/condo/domains/user/components/auth/SignInForm.tsxapps/condo/domains/user/utils/serverSchema/auth.jsapps/condo/domains/user/components/UserSettingsContent.tsxapps/condo/domains/user/components/auth/RegisterForm.tsxapps/condo/pages/auth/change-password.tsx
📚 Learning: 2025-09-17T13:38:39.633Z
Learnt from: SavelevMatthew
Repo: open-condo-software/condo PR: 6645
File: apps/dev-portal-web/domains/common/components/auth/AuthForm.tsx:12-12
Timestamp: 2025-09-17T13:38:39.633Z
Learning: In the condo project, runtime configuration fallbacks for authMethods are handled at the Next.js config level in next.config.ts, where AUTH_METHODS defaults to ["condo"] via JSON.parse(conf['AUTH_METHODS'] || '["condo"]') if not provided via environment variables, ensuring safe destructuring in components that access publicRuntimeConfig.authMethods.
Applied to files:
apps/condo/pages/auth/signin.tsxapps/condo/domains/user/components/SudoTokenProvider.tsxapps/condo/domains/user/schema/AuthenticateUserWithPhoneAndPasswordService.jsapps/condo/domains/user/components/auth/SignInForm.tsxapps/condo/domains/user/utils/serverSchema/auth.jsapps/condo/domains/user/components/UserSettingsContent.tsxapps/condo/pages/auth/change-password.tsx
📚 Learning: 2025-08-18T07:33:48.748Z
Learnt from: SavelevMatthew
Repo: open-condo-software/condo PR: 6533
File: apps/condo/domains/user/integration/passport/errors.js:72-76
Timestamp: 2025-08-18T07:33:48.748Z
Learning: The UNKNOWN_CONFIRM_TOKEN_TYPE error in apps/condo/domains/user/integration/passport/errors.js uses INTERNAL_ERROR classification because it can only be caused by wrong server utility use (e.g., passing invalid token types to internal functions), not by user input.
Applied to files:
apps/condo/domains/user/constants/errors.jsapps/condo/domains/user/schema/ConfirmPhoneActionService.test.jsapps/condo/domains/user/schema/AuthenticateUserWithPhoneAndPasswordService.jsapps/condo/domains/user/schema/GenerateSudoTokenService.jsapps/condo/domains/user/schema/AuthenticateOrRegisterUserWithTokenService.test.jsapps/condo/domains/user/schema/ConfirmEmailActionService.jsapps/condo/domains/user/schema/AuthenticateUserWithEmailAndPasswordService.jsapps/condo/domains/user/utils/serverSchema/auth.jsapps/condo/domains/user/schema/AuthenticateOrRegisterUserWithTokenService.js
📚 Learning: 2025-09-30T13:04:44.424Z
Learnt from: SavelevMatthew
Repo: open-condo-software/condo PR: 6701
File: apps/dev-portal-api/domains/miniapp/schema/GetB2CAppInfoService.js:36-48
Timestamp: 2025-09-30T13:04:44.424Z
Learning: In the open-condo-software/condo repository using Keystone framework, thrown errors in GraphQL resolvers are handled globally by Keystone, so explicit try-catch blocks for error handling in resolver functions are not required.
Applied to files:
apps/condo/domains/user/schema/AuthenticateUserWithPhoneAndPasswordService.test.jsapps/condo/domains/user/utils/serverSchema/auth.js
📚 Learning: 2025-08-18T07:35:07.758Z
Learnt from: SavelevMatthew
Repo: open-condo-software/condo PR: 6533
File: apps/condo/domains/user/integration/passport/utils/routes.js:3-11
Timestamp: 2025-08-18T07:35:07.758Z
Learning: In apps/condo/domains/user/integration/passport/utils/routes.js, the checkAuthRateLimits middleware runs after other middleware that ensures req.session exists, so defensive programming with optional chaining for req.session?.userType is not needed.
Applied to files:
apps/condo/domains/user/schema/AuthenticateUserWithPhoneAndPasswordService.jsapps/condo/domains/user/schema/ConfirmEmailActionService.jsapps/condo/domains/user/utils/serverSchema/auth.js
📚 Learning: 2025-08-04T08:04:46.817Z
Learnt from: SavelevMatthew
Repo: open-condo-software/condo PR: 6472
File: apps/condo/schema.graphql:390-394
Timestamp: 2025-08-04T08:04:46.817Z
Learning: In the condo codebase, the schema.graphql file is auto-generated (codegen) from Keystone 5 schema definitions. Security and access controls are implemented in the actual Keystone schema files (like User.js) using access control properties such as SERVER_CREATE_ONLY_ACCESS, not in the generated GraphQL schema file. When reviewing schema.graphql, consider that field presence doesn't indicate lack of access controls - check the corresponding Keystone schema files for actual security implementation.
Applied to files:
apps/condo/gql/index.tsapps/condo/domains/user/utils/serverSchema/auth.js
📚 Learning: 2025-09-17T13:40:42.895Z
Learnt from: SavelevMatthew
Repo: open-condo-software/condo PR: 6645
File: apps/dev-portal-web/pages/_app.tsx:40-41
Timestamp: 2025-09-17T13:40:42.895Z
Learning: In the condo project, runtime configuration fallbacks for runtimeTranslations are handled at the Next.js config level in next.config.ts, where RUNTIME_TRANSLATIONS defaults to '{}' via JSON.parse(conf['RUNTIME_TRANSLATIONS'] || '{}') if not provided via environment variables, ensuring safe destructuring in components that access publicRuntimeConfig.runtimeTranslations.
Applied to files:
apps/condo/domains/user/components/UserSettingsContent.tsx
🧬 Code graph analysis (18)
apps/condo/domains/common/utils/phone.js (2)
apps/condo/domains/user/schema/ConfirmPhoneActionService.js (1)
normalizedPhone(275-275)apps/condo/domains/user/schema/GenerateSudoTokenService.js (1)
normalizedPhone(117-117)
apps/condo/pages/auth/signin.tsx (1)
apps/condo/domains/user/hooks/useAuthMethods.ts (1)
useAuthMethods(32-85)
apps/condo/domains/common/utils/mail.js (2)
apps/condo/domains/user/schema/ConfirmEmailActionService.js (2)
normalizedEmail(330-330)user(324-324)apps/condo/domains/user/schema/GenerateSudoTokenService.js (1)
normalizedEmail(116-116)
apps/condo/domains/user/schema/ConfirmEmailActionService.test.js (3)
apps/condo/domains/user/utils/testSchema/index.js (16)
client(177-179)client(187-187)client(198-198)client(206-206)client(214-214)client(222-222)client(319-319)client(333-333)client(354-354)client(368-368)client(382-382)client(397-397)client(411-411)client(427-427)client(509-509)client(553-553)apps/dev-portal-api/domains/user/utils/testSchema/index.js (2)
startConfirmEmailActionByTestClient(217-217)userAttrs(235-235)apps/condo/domains/user/constants/confirmEmailAction.js (1)
CONFIRM_EMAIL_ACTION_MESSAGE_TYPES(7-10)
apps/condo/domains/user/schema/GenerateSudoTokenService.test.js (2)
packages/keystone/test.utils.js (1)
expectToThrowGQLErrorToResult(1026-1028)apps/condo/domains/user/schema/AuthenticateUserWithPhoneAndPasswordService.js (1)
STAFF(94-104)
apps/condo/domains/user/components/SudoTokenProvider.tsx (4)
apps/condo/domains/user/components/auth/SecondaryLink.tsx (1)
SecondaryLink(8-14)apps/condo/domains/user/constants/errors.js (1)
NOT_ENOUGH_AUTH_FACTORS(96-96)apps/condo/gql/index.ts (4)
useStartConfirmEmailActionMutation(7723-7726)useCompleteConfirmEmailActionMutation(7476-7479)useStartConfirmPhoneActionMutation(7756-7759)useCompleteConfirmPhoneActionMutation(7509-7512)apps/condo/domains/common/utils/helpers.ts (1)
formatPhone(28-37)
apps/condo/domains/user/utils/testSchema/index.js (1)
apps/condo/domains/user/gql.js (1)
START_CONFIRM_PHONE_ACTION_MUTATION(224-228)
apps/condo/domains/user/schema/AuthenticateUserWithPhoneAndPasswordService.js (3)
apps/condo/domains/user/constants/errors.js (1)
NOT_ENOUGH_AUTH_FACTORS(96-96)apps/condo/domains/user/constants/authFactors.js (1)
AUTH_FACTOR_TYPES(1-5)apps/condo/domains/user/utils/serverSchema/index.js (2)
ConfirmEmailAction(148-148)ConfirmPhoneAction(39-39)
apps/condo/domains/user/schema/GenerateSudoTokenService.js (3)
apps/condo/domains/user/constants/errors.js (2)
NOT_ENOUGH_AUTH_FACTORS(96-96)ERRORS(237-306)apps/condo/domains/user/schema/AuthenticateUserWithPhoneAndPasswordService.js (1)
ERRORS(22-63)apps/condo/domains/user/utils/serverSchema/auth.js (1)
ERROR_TYPES(12-14)
apps/condo/domains/user/utils/serverSchema/auth.spec.js (1)
apps/condo/domains/user/utils/serverSchema/auth.js (7)
require(2-2)require(4-4)require(5-5)require(6-6)require(7-7)require(8-8)require(9-9)
apps/condo/domains/user/schema/AuthenticateOrRegisterUserWithTokenService.test.js (2)
apps/condo/domains/user/constants/common.js (2)
RESIDENT(2-2)SERVICE(4-4)apps/condo/domains/user/utils/tokens.js (1)
TOKEN_TYPES(4-8)
apps/condo/domains/user/components/auth/SecondFactorForm.tsx (5)
apps/condo/domains/user/components/auth/SecondaryLink.tsx (1)
SecondaryLink(8-14)packages/miniapp-utils/src/helpers/urls.ts (1)
isSafeUrl(1-20)apps/condo/gql/index.ts (4)
useStartConfirmEmailActionMutation(7723-7726)useCompleteConfirmEmailActionMutation(7476-7479)useStartConfirmPhoneActionMutation(7756-7759)useCompleteConfirmPhoneActionMutation(7509-7512)apps/condo/domains/user/components/containers/ResponsiveCol.tsx (1)
ResponsiveCol(10-29)apps/condo/domains/common/utils/helpers.ts (1)
formatPhone(28-37)
apps/condo/gql/index.ts (1)
apps/condo/gql/operation.types.ts (2)
ChangeTwoFactorAuthenticationMutation(1134-1134)ChangeTwoFactorAuthenticationMutationVariables(1129-1131)
apps/condo/domains/user/schema/AuthenticateUserWithEmailAndPasswordService.js (5)
apps/condo/domains/user/schema/AuthenticateUserWithPhoneAndPasswordService.js (2)
validation(123-130)getSchemaCtx(164-164)apps/condo/domains/user/constants/errors.js (1)
NOT_ENOUGH_AUTH_FACTORS(96-96)apps/condo/domains/user/constants/authFactors.js (1)
AUTH_FACTOR_TYPES(1-5)apps/condo/domains/user/utils/serverSchema/auth.js (6)
ERROR_TYPES(12-14)getSchemaCtx(195-195)getSchemaCtx(369-369)getSchemaCtx(389-389)getSchemaCtx(423-423)getSchemaCtx(457-457)apps/condo/domains/user/utils/serverSchema/index.js (2)
ConfirmPhoneAction(39-39)ConfirmEmailAction(148-148)
apps/condo/domains/user/components/UserSettingsContent.tsx (6)
apps/condo/domains/user/schema/ConfirmEmailActionService.js (1)
user(324-324)apps/condo/domains/user/schema/ConfirmPhoneActionService.js (1)
user(269-269)apps/condo/domains/user/schema/ChangeTwoFactorAuthenticationService.js (2)
user(150-153)sudoToken(136-141)apps/condo/domains/user/components/SudoTokenProvider.tsx (1)
useSudoToken(51-51)apps/condo/gql/index.ts (1)
useChangeTwoFactorAuthenticationMutation(7333-7336)apps/condo/domains/user/schema/GenerateSudoTokenService.js (1)
sudoToken(162-168)
apps/condo/domains/user/components/auth/ValidateIdentifierForm.tsx (1)
apps/condo/domains/user/components/auth/SecondaryLink.tsx (1)
SecondaryLink(8-14)
apps/condo/domains/user/schema/ConfirmPhoneActionService.js (2)
packages/keystone/utils/errors/constants.js (1)
BAD_USER_INPUT(10-10)apps/condo/domains/user/schema/ConfirmEmailActionService.js (5)
data(238-238)data(428-428)data(510-510)ERRORS(57-141)user(324-324)
apps/condo/pages/auth/change-password.tsx (4)
apps/condo/domains/user/constants/errors.js (1)
NOT_ENOUGH_AUTH_FACTORS(96-96)apps/condo/domains/user/schema/AuthenticateOrRegisterUserWithTokenService.js (1)
authFactors(421-421)apps/condo/domains/user/utils/tokens.js (1)
TOKEN_TYPES(4-8)apps/condo/domains/user/components/auth/SecondFactorForm.tsx (1)
useSecondFactor(31-431)
🪛 Biome (2.1.2)
apps/condo/pages/auth/change-password.tsx
[error] 551-551: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
🪛 Gitleaks (8.29.0)
apps/condo/domains/user/utils/serverSchema/auth.js
[high] 340-340: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build open-source part (ubuntu-22.04, 22.x, 3.13)
- GitHub Check: Semgrep vulnerabilities check
- GitHub Check: Check dependencies integrity (22.x)
- GitHub Check: Lint source code (22.x)
|
|
|
||
| function maskPhone (normalizedPhone) { | ||
| const hasPlus = normalizedPhone.startsWith('+') | ||
| const digits = normalizedPhone.replace(/\D/g, '') |
| ['+79008007060', '+79*******60'], | ||
| ['79008007060', '79*******60'], | ||
| ['1', '1'], | ||
| ['12', '1*'], | ||
| ['123', '1*3'], | ||
| ['1234', '1**4'], | ||
| ['12345', '1***5'], | ||
| ['123456', '1****6'], | ||
| ['1234567', '12***67'], | ||
| ['12345678', '12****78'], | ||
| ['123456789', '12*****89'], |
There was a problem hiding this comment.
please also add negative cases, like '' or when not a phone nubmer
There was a problem hiding this comment.
You must hand over the already normalized phone
There was a problem hiding this comment.
renamed function to maskNormalizedPhone
There was a problem hiding this comment.
Added case with ''
| const cases = [ | ||
| ['[email protected]', 'a***@example.com'], | ||
| ['[email protected]', 'ab***@example.com'], | ||
| ['[email protected]', 'ab***@example.com'], | ||
| ['[email protected]', 'ab***@example.com'], | ||
| ['[email protected]', 'ab***@example.com'], | ||
| ['[email protected]', 'ab***@example.com'], | ||
| ['[email protected]', 'ab***@example.com'], | ||
| ['[email protected]', 'qw***@yandex.ru'], | ||
| ] |
There was a problem hiding this comment.
best to add negative cases too
There was a problem hiding this comment.
You must use only normalized emails
There was a problem hiding this comment.
yes, but the function is named maskEmail, and not maskNormalizedEmail.. Really easy to forget to normalize and use your function. Also.. what about undefined case?
There was a problem hiding this comment.
renamed function to maskNormalizedEmail
Alllex202
force-pushed
the
feat/condo/DOMA-12564/2fa-frontend
branch
from
77bb08b to
c2a862b
Compare
Alllex202
requested review from
nomerdvadcatpyat,
tolmachev21 and
toplenboren
Alllex202
added
✋🙂 Review please
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@apps/condo/domains/user/components/auth/RegisterForm.tsx`:
- Around line 294-315: The NOT_ENOUGH_AUTH_FACTORS check is using the wrong path
(res?.errors?.graphQLErrors) so it never detects the error; update the checks to
inspect res?.errors directly (use res?.errors?.some(isNotEnoughAuthFactors) and
res?.errors?.find(isNotEnoughAuthFactors)) and then proceed to read
extensions/authDetails from that found error, keeping the existing logic that
sets userIdRef.current, setUserMaskedData, setAvailableSecondFactors,
setCurrentSecondFactor (using currentSecondFactor or availableSecondFactors[0])
and setStep('authenticate2FA'); also remove or adjust the `@ts-ignore` stubs and
ensure types align with GraphQLError[] if needed.
In `@apps/condo/domains/user/components/auth/SignInForm.tsx`:
- Around line 179-199: The NOT_ENOUGH_AUTH_FACTORS check is using the wrong
error path (res?.errors?.graphQLErrors) which is undefined for Apollo mutation
results; update the logic in the SignInForm mutation handling (the
isNotEnoughAuthFactors predicate usage around res) to inspect res?.errors
directly (e.g., res?.errors?.some(isNotEnoughAuthFactors) and
res?.errors.find(isNotEnoughAuthFactors)), then use the found
graphQLError.extensions.authDetails to populate userIdRef.current,
setUserMaskedData, setAvailableSecondFactors and setCurrentSecondFactor as
before; ensure null/undefined guards remain for authDetails,
availableSecondFactors and currentSecondFactor.
In `@apps/condo/domains/user/components/SudoTokenProvider.tsx`:
- Around line 265-285: The NOT_ENOUGH_AUTH_FACTORS check is using the wrong
error path (res?.errors?.graphQLErrors) which is undefined for Apollo mutation
results; update the logic in the block using isNotEnoughAuthFactors to inspect
res?.errors (an array of GraphQLFormattedError) instead of
res?.errors?.graphQLErrors, i.e. find the matching error via
res?.errors?.find(isNotEnoughAuthFactors) and then read
graphQLError.extensions.authDetails, then proceed to set userIdRef.current,
setUserMaskedData, setAvailableSecondFactors, setConfirmActionType,
setIsModalVisible and setIsSecondFactorModalVisible exactly as before.
♻️ Duplicate comments (7)
apps/condo/domains/user/components/SudoTokenProvider.tsx (3)
146-154: Consider adding error logging to empty onError handlers.The
completeConfirmEmailMutationandcompleteConfirmPhoneMutationhave emptyonErrorhandlers, which silently ignore errors and make debugging difficult.Proposed improvement
const [completeConfirmEmailMutation] = useCompleteConfirmEmailActionMutation({ - onError: () => {}, + // NOTE: Errors are handled in handleChangeCodeInput response checking + onError: (error) => console.error('Complete confirm email action failed:', error), }) // ... const [completeConfirmPhoneMutation] = useCompleteConfirmPhoneActionMutation({ - onError: () => {}, + // NOTE: Errors are handled in handleChangeCodeInput response checking + onError: (error) => console.error('Complete confirm phone action failed:', error), })
169-203: Duplicate expiry date setting and potential retry prevention on failure.The
confirmEmailTokenExpiredDateis set twice (lines 177 and 195). If the mutation fails, the expiry date is still set at line 177, preventing retries for a minute even on failure.Proposed fix
const startConfirmEmailAction = useCallback(async () => { if (!userIdRef.current) { throw new Error('No userid for StartConfirmEmailAction!') } const shouldWait = confirmEmailTokenExpiredDate && dayjs(confirmEmailTokenExpiredDate) > dayjs() if (shouldWait) return - setConfirmEmailTokenExpiredDate(dayjs().add(1, 'minute').toISOString()) const sender = getClientSideSenderInfo() const captcha = await executeCaptcha() const res = await startConfirmEmailActionMutation({...}) - setConfirmEmailTokenExpiredDate(dayjs().add(1, 'minute').toISOString()) const token = res?.data?.result?.token if (token) { setConfirmEmailToken(token) + setConfirmEmailTokenExpiredDate(dayjs().add(1, 'minute').toISOString()) } return res }, [...])Apply the same pattern to
startConfirmPhoneAction(lines 205-239).
456-467: Missing dependencies in useEffect.The effect calls
startConfirmPhoneActionandstartConfirmEmailActionbut they're not in the dependency array, which could lead to stale closures.Proposed fix
useEffect(() => { if (currentSecondFactor !== 'confirmEmailToken' && currentSecondFactor !== 'confirmPhoneToken') return if (!userIdRef.current) return if (currentSecondFactor === 'confirmPhoneToken') { startConfirmPhoneAction() } else if (currentSecondFactor === 'confirmEmailToken') { startConfirmEmailAction() } - }, [currentSecondFactor]) + }, [currentSecondFactor, startConfirmPhoneAction, startConfirmEmailAction])apps/condo/domains/user/components/UserSettingsContent.tsx (1)
218-229: Incorrect error path for TOKEN_NOT_FOUND check.The
res.errorsfrom Apollo'suseMutationis a direct array of GraphQL errors, not an object with agraphQLErrorsproperty. The current pathres?.errors?.graphQLErrorswill always beundefined, causing the sudo token retry logic to never trigger.🐛 Proposed fix
if (res.errors || res?.data?.result?.status !== 'ok') { - // `@ts-ignore` - const isExpiredSudoToken = res?.errors?.graphQLErrors?.some((gqlError) => gqlError?.extensions?.type === 'TOKEN_NOT_FOUND') + const isExpiredSudoToken = res?.errors?.some((gqlError) => gqlError?.extensions?.type === 'TOKEN_NOT_FOUND') if (isExpiredSudoToken) {apps/condo/domains/user/components/auth/SignInForm.tsx (1)
262-268: Missingrouterin useEffect dependency array.The effect uses
routerviaupdateQuerybut doesn't include it in the dependency array, which could cause stale closures.Proposed fix
useEffect(() => { if (currentSecondFactor) { updateQuery(router, { newParameters: { 'authStep': 'secondFactor' } }, { resetOldParameters: false, routerAction: 'replace' }) } else { updateQuery(router, { newParameters: { 'authStep': 'firstFactor' } }, { resetOldParameters: false, routerAction: 'replace' }) } - }, [currentSecondFactor]) + }, [currentSecondFactor, router])apps/condo/domains/user/components/auth/SecondFactorForm.tsx (2)
254-265: Missing dependencies in useEffect.The effect calls
startConfirmPhoneActionandstartConfirmEmailActionbut they're not in the dependency array, which could lead to stale closures.Proposed fix
useEffect(() => { if (currentSecondFactor !== 'confirmEmailToken' && currentSecondFactor !== 'confirmPhoneToken') return if (!userIdRef.current) return if (currentSecondFactor === 'confirmPhoneToken') { // send sms-code by userId startConfirmPhoneAction() } else if (currentSecondFactor === 'confirmEmailToken') { // send email-code by userId startConfirmEmailAction() } - }, [currentSecondFactor]) + }, [currentSecondFactor, startConfirmPhoneAction, startConfirmEmailAction])
396-396: Missing dependencies in useMemo.The
SecondFactorFormmemoization is missinguserMaskedData(used on lines 312 and 335) andPasswordContinueMessage(used on line 385) from the dependency array, which could cause stale data to be displayed.Proposed fix
- }, [ConfirmEmailTokenDescription, ConfirmEmailTokenLabel, ConfirmPhoneTokenDescription, ConfirmPhoneTokenLabel, EmailProblemsMessage, Header, PasswordDescription, PasswordLabel, PhoneProblemsMessage, ResetPasswordMessage, SendEmailCodeAgainMessage, SendPhoneCodeAgainMessage, availableSecondFactors, currentSecondFactor, handleChangeSecondFactorInput, redirectUrl, startConfirmEmailAction, startConfirmPhoneAction]) + }, [ConfirmEmailTokenDescription, ConfirmEmailTokenLabel, ConfirmPhoneTokenDescription, ConfirmPhoneTokenLabel, EmailProblemsMessage, Header, PasswordContinueMessage, PasswordDescription, PasswordLabel, PhoneProblemsMessage, ResetPasswordMessage, SendEmailCodeAgainMessage, SendPhoneCodeAgainMessage, availableSecondFactors, currentSecondFactor, handleChangeSecondFactorInput, redirectUrl, startConfirmEmailAction, startConfirmPhoneAction, userMaskedData])
🧹 Nitpick comments (1)
apps/condo/domains/user/components/auth/SecondFactorForm.tsx (1)
87-95: Consider adding error logging to empty onError handlers.The
completeConfirmEmailMutationandcompleteConfirmPhoneMutationhave emptyonErrorhandlers. While errors appear to be handled in the response checking (lines 230-244), silent error handlers make debugging difficult.Proposed improvement
const [completeConfirmEmailMutation] = useCompleteConfirmEmailActionMutation({ - onError: () => {}, + // NOTE: Errors are handled in handleChangeSecondFactorInput response checking + onError: (error) => console.error('Complete confirm email action failed:', error), }) // ... const [completeConfirmPhoneMutation] = useCompleteConfirmPhoneActionMutation({ - onError: () => {}, + // NOTE: Errors are handled in handleChangeSecondFactorInput response checking + onError: (error) => console.error('Complete confirm phone action failed:', error), })
| const isNotEnoughAuthFactors = (error: GraphQLFormattedError) => error?.extensions?.type === NOT_ENOUGH_AUTH_FACTORS | ||
| // @ts-ignore | ||
| if (res?.errors?.graphQLErrors?.some(isNotEnoughAuthFactors)) { | ||
| // @ts-ignore | ||
| const graphQLError = res?.errors?.graphQLErrors.find(isNotEnoughAuthFactors) | ||
| const authDetails = graphQLError?.extensions?.authDetails | ||
|
|
||
| if (authDetails?.is2FAEnabled && authDetails?.userId && authDetails?.availableSecondFactors?.length > 0) { | ||
| userIdRef.current = authDetails.userId | ||
| const availableSecondFactors = authDetails?.availableSecondFactors?.filter(factor => ['confirmPhoneToken', 'confirmEmailToken', 'password'].includes(factor)) || [] | ||
| const prioritySecondFactor = currentSecondFactor || availableSecondFactors?.[0] || null | ||
|
|
||
| if (availableSecondFactors.length > 0) { | ||
| setUserMaskedData(authDetails?.maskedData || null) | ||
| setAvailableSecondFactors(availableSecondFactors) | ||
| setCurrentSecondFactor(prioritySecondFactor) | ||
| setStep('authenticate2FA') | ||
| } | ||
| } | ||
|
|
||
| return | ||
| } |
There was a problem hiding this comment.
Incorrect error path for NOT_ENOUGH_AUTH_FACTORS check in mutation response.
The res.errors from Apollo's mutation result is a direct array of GraphQL errors, not an object with a graphQLErrors property. The current path res?.errors?.graphQLErrors will always be undefined, causing the 2FA flow to never trigger from the response.
🐛 Proposed fix
const isNotEnoughAuthFactors = (error: GraphQLFormattedError) => error?.extensions?.type === NOT_ENOUGH_AUTH_FACTORS
- // `@ts-ignore`
- if (res?.errors?.graphQLErrors?.some(isNotEnoughAuthFactors)) {
- // `@ts-ignore`
- const graphQLError = res?.errors?.graphQLErrors.find(isNotEnoughAuthFactors)
+ if (res?.errors?.some(isNotEnoughAuthFactors)) {
+ const graphQLError = res?.errors?.find(isNotEnoughAuthFactors)
const authDetails = graphQLError?.extensions?.authDetails📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const isNotEnoughAuthFactors = (error: GraphQLFormattedError) => error?.extensions?.type === NOT_ENOUGH_AUTH_FACTORS | |
| // @ts-ignore | |
| if (res?.errors?.graphQLErrors?.some(isNotEnoughAuthFactors)) { | |
| // @ts-ignore | |
| const graphQLError = res?.errors?.graphQLErrors.find(isNotEnoughAuthFactors) | |
| const authDetails = graphQLError?.extensions?.authDetails | |
| if (authDetails?.is2FAEnabled && authDetails?.userId && authDetails?.availableSecondFactors?.length > 0) { | |
| userIdRef.current = authDetails.userId | |
| const availableSecondFactors = authDetails?.availableSecondFactors?.filter(factor => ['confirmPhoneToken', 'confirmEmailToken', 'password'].includes(factor)) || [] | |
| const prioritySecondFactor = currentSecondFactor || availableSecondFactors?.[0] || null | |
| if (availableSecondFactors.length > 0) { | |
| setUserMaskedData(authDetails?.maskedData || null) | |
| setAvailableSecondFactors(availableSecondFactors) | |
| setCurrentSecondFactor(prioritySecondFactor) | |
| setStep('authenticate2FA') | |
| } | |
| } | |
| return | |
| } | |
| const isNotEnoughAuthFactors = (error: GraphQLFormattedError) => error?.extensions?.type === NOT_ENOUGH_AUTH_FACTORS | |
| if (res?.errors?.some(isNotEnoughAuthFactors)) { | |
| const graphQLError = res?.errors?.find(isNotEnoughAuthFactors) | |
| const authDetails = graphQLError?.extensions?.authDetails | |
| if (authDetails?.is2FAEnabled && authDetails?.userId && authDetails?.availableSecondFactors?.length > 0) { | |
| userIdRef.current = authDetails.userId | |
| const availableSecondFactors = authDetails?.availableSecondFactors?.filter(factor => ['confirmPhoneToken', 'confirmEmailToken', 'password'].includes(factor)) || [] | |
| const prioritySecondFactor = currentSecondFactor || availableSecondFactors?.[0] || null | |
| if (availableSecondFactors.length > 0) { | |
| setUserMaskedData(authDetails?.maskedData || null) | |
| setAvailableSecondFactors(availableSecondFactors) | |
| setCurrentSecondFactor(prioritySecondFactor) | |
| setStep('authenticate2FA') | |
| } | |
| } | |
| return | |
| } |
🤖 Prompt for AI Agents
In `@apps/condo/domains/user/components/auth/RegisterForm.tsx` around lines 294 -
315, The NOT_ENOUGH_AUTH_FACTORS check is using the wrong path
(res?.errors?.graphQLErrors) so it never detects the error; update the checks to
inspect res?.errors directly (use res?.errors?.some(isNotEnoughAuthFactors) and
res?.errors?.find(isNotEnoughAuthFactors)) and then proceed to read
extensions/authDetails from that found error, keeping the existing logic that
sets userIdRef.current, setUserMaskedData, setAvailableSecondFactors,
setCurrentSecondFactor (using currentSecondFactor or availableSecondFactors[0])
and setStep('authenticate2FA'); also remove or adjust the `@ts-ignore` stubs and
ensure types align with GraphQLError[] if needed.
| const isNotEnoughAuthFactors = (error: GraphQLFormattedError) => error?.extensions?.type === NOT_ENOUGH_AUTH_FACTORS | ||
| // @ts-ignore | ||
| if (res?.errors?.graphQLErrors?.some(isNotEnoughAuthFactors)) { | ||
| // @ts-ignore | ||
| const graphQLError = res?.errors?.graphQLErrors.find(isNotEnoughAuthFactors) | ||
| const authDetails = graphQLError?.extensions?.authDetails | ||
|
|
||
| if (authDetails?.is2FAEnabled && authDetails?.userId && authDetails?.availableSecondFactors?.length > 0) { | ||
| userIdRef.current = authDetails.userId | ||
| const availableSecondFactors = authDetails?.availableSecondFactors?.filter(factor => ['confirmPhoneToken', 'confirmEmailToken'].includes(factor)) || [] | ||
| const prioritySecondFactor = currentSecondFactor || availableSecondFactors?.[0] || null | ||
|
|
||
| if (availableSecondFactors.length > 0) { | ||
| setUserMaskedData(authDetails?.maskedData || null) | ||
| setAvailableSecondFactors(availableSecondFactors) | ||
| setConfirmActionType(prioritySecondFactor) | ||
| setIsModalVisible(false) | ||
| setIsSecondFactorModalVisible(true) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Incorrect error path for NOT_ENOUGH_AUTH_FACTORS check.
Same issue as in other files - the res?.errors?.graphQLErrors path will always be undefined since Apollo mutation results return errors as a direct array.
🐛 Proposed fix
const isNotEnoughAuthFactors = (error: GraphQLFormattedError) => error?.extensions?.type === NOT_ENOUGH_AUTH_FACTORS
- // `@ts-ignore`
- if (res?.errors?.graphQLErrors?.some(isNotEnoughAuthFactors)) {
- // `@ts-ignore`
- const graphQLError = res?.errors?.graphQLErrors.find(isNotEnoughAuthFactors)
+ if (res?.errors?.some(isNotEnoughAuthFactors)) {
+ const graphQLError = res?.errors?.find(isNotEnoughAuthFactors)
const authDetails = graphQLError?.extensions?.authDetails🤖 Prompt for AI Agents
In `@apps/condo/domains/user/components/SudoTokenProvider.tsx` around lines 265 -
285, The NOT_ENOUGH_AUTH_FACTORS check is using the wrong error path
(res?.errors?.graphQLErrors) which is undefined for Apollo mutation results;
update the logic in the block using isNotEnoughAuthFactors to inspect
res?.errors (an array of GraphQLFormattedError) instead of
res?.errors?.graphQLErrors, i.e. find the matching error via
res?.errors?.find(isNotEnoughAuthFactors) and then read
graphQLError.extensions.authDetails, then proceed to set userIdRef.current,
setUserMaskedData, setAvailableSecondFactors, setConfirmActionType,
setIsModalVisible and setIsSecondFactorModalVisible exactly as before.
| const cases = [ | ||
| ['[email protected]', 'a***@example.com'], | ||
| ['[email protected]', 'ab***@example.com'], | ||
| ['[email protected]', 'ab***@example.com'], | ||
| ['[email protected]', 'ab***@example.com'], | ||
| ['[email protected]', 'ab***@example.com'], | ||
| ['[email protected]', 'ab***@example.com'], | ||
| ['[email protected]', 'ab***@example.com'], | ||
| ['[email protected]', 'qw***@yandex.ru'], | ||
| ] |
There was a problem hiding this comment.
yes, but the function is named maskEmail, and not maskNormalizedEmail.. Really easy to forget to normalize and use your function. Also.. what about undefined case?
|
|
||
| function maskPhone (normalizedPhone) { | ||
| const hasPlus = normalizedPhone.startsWith('+') | ||
| const digits = normalizedPhone.replace(/\D/g, '') |
| ['+79008007060', '+79*******60'], | ||
| ['79008007060', '79*******60'], | ||
| ['1', '1'], | ||
| ['12', '1*'], | ||
| ['123', '1*3'], | ||
| ['1234', '1**4'], | ||
| ['12345', '1***5'], | ||
| ['123456', '1****6'], | ||
| ['1234567', '12***67'], | ||
| ['12345678', '12****78'], | ||
| ['123456789', '12*****89'], |
| export const useSudoToken: UseSudoTokenContext = () => useContext(SudoTokenContext) | ||
|
|
||
|
|
||
| const NOT_NUMBER_REGEX = /\D/g |
There was a problem hiding this comment.
Mb move to common/constants/regex
| onError: authOrRegisterErrorHandler, | ||
| onError: (error) => { | ||
| if (error?.graphQLErrors?.some(gqlError => gqlError.extensions?.type === NOT_ENOUGH_AUTH_FACTORS)) { | ||
| // show flow with 2FA |
There was a problem hiding this comment.
mb it's better to set some state here and use it?
| import { AgreementText } from './AgreementText' | ||
| import { useSecondFactor } from './SecondFactorForm' | ||
|
|
||
| import { updateQuery } from '../../../common/utils/helpers' |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/condo/domains/user/components/auth/SecondFactorForm.tsx`:
- Around line 229-243: In SecondFactorForm, replace the incorrect access to
res.errors.graphQLErrors by reading the first GraphQLError from the errors array
on the mutation result (res.errors?.[0]); derive the message as
firstError?.extensions?.messageForUser || firstError?.message ||
ServerErrorMessage, remove the `@ts-ignore`, and use that errorMessage in the
existing form.setFields calls (when currentSecondFactor is 'confirmPhoneToken'
or 'confirmEmailToken') so the correct error text is shown to the user.
In `@apps/condo/domains/user/components/SudoTokenProvider.tsx`:
- Around line 77-81: The external Telegram link rendered in
InstructionStepSupportTelegramChat uses SecondaryLink with target='_blank' but
lacks rel attributes; update the SecondaryLink element where it constructs the
chatBotLink (using HelpRequisites.support_bot and ChatInTelegramMessage) to
include rel='noopener noreferrer' so the opened page cannot access window.opener
and to prevent reverse-tabnabbing.
🧹 Nitpick comments (1)
apps/condo/domains/common/utils/mail.js (1)
62-75: Consider adding null/undefined guard for robustness.If
normalizedEmailisnullorundefined, calling.split('@')will throw a TypeError. While the function name implies a normalized (non-null) input, adding a defensive check would prevent runtime errors if misused.🛡️ Optional defensive guard
function maskNormalizedEmail (normalizedEmail) { + if (!normalizedEmail) return normalizedEmail + const [user, domain] = normalizedEmail.split('@') if (!domain) return normalizedEmail
| if (res?.errors) { | ||
| // @ts-ignore | ||
| const errorMessage = res.errors?.graphQLErrors?.[0]?.extensions?.messageForUser || ServerErrorMessage | ||
|
|
||
| if (currentSecondFactor === 'confirmPhoneToken') { | ||
| form.setFields([{ | ||
| name: 'confirmPhoneCode', | ||
| errors: [errorMessage], | ||
| }]) | ||
| } else if (currentSecondFactor === 'confirmEmailToken') { | ||
| form.setFields([{ | ||
| name: 'confirmEmailCode', | ||
| errors: [errorMessage], | ||
| }]) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "SecondFactorForm.tsx" | head -20Repository: open-condo-software/condo
Length of output: 131
🏁 Script executed:
cat -n apps/condo/domains/user/components/auth/SecondFactorForm.tsx | sed -n '220,250p'Repository: open-condo-software/condo
Length of output: 1365
🌐 Web query:
Apollo Client FetchResult errors structure graphQLErrors array
💡 Result:
In Apollo Client, the FetchResult<T> you get back from a link (and that flows into mutation results, update functions, etc.) carries GraphQL execution errors in an errors array that matches the GraphQL response’s top-level errors field:
FetchResult.errors?: ReadonlyArray<GraphQLFormattedError>(v3+) — each entry is a formatted GraphQL error object. [1]
That same array is what Apollo Client exposes as “GraphQL errors” on the client-side error objects:
- Apollo Client v3:
ApolloError.graphQLErrors: ReadonlyArray<GraphQLFormattedError>(built from the responseerrorsarray). [2] - Apollo Client v4: GraphQL errors are represented as a
CombinedGraphQLErrorsinstance; the underlying list is available ascombined.errors. [3]
Structure of each entry (what’s inside the array)
Each element is a standard GraphQL error shape (commonly including):
message: stringlocations?: [{ line: number, column: number }]path?: (string | number)[]extensions?: { ... }(often includescode, and in dev may include stacktrace-like info) [4]
Example (server response shape): an errors: [...] array of objects with message, locations, and extensions (e.g., extensions.code). [4]
Sources
[1] Apollo Client type defs showing errors?: ReadonlyArray<GraphQLFormattedError> on operation results. (app.unpkg.com)
[2] Apollo Client ApolloError type showing graphQLErrors: ReadonlyArray<GraphQLFormattedError>. (app.unpkg.com)
[3] Apollo docs for CombinedGraphQLErrors and access via .errors (v4 style). (apollographql.com)
[4] Apollo Server docs showing the GraphQL response errors array structure and extensions. (apollographql.com)
Citations:
- 1: https://app.unpkg.com/%40apollo/client%403.13.3/files/core/types.d.ts?utm_source=openai
- 2: https://app.unpkg.com/%40apollo/client%403.13.3/files/errors/index.d.ts?utm_source=openai
- 3: https://www.apollographql.com/docs/react/api/errors/CombinedGraphQLErrors?utm_source=openai
- 4: https://www.apollographql.com/docs/deploy-preview/2dc45dbb5ad0b4965a6aa027/apollo-server/data/errors/?utm_source=openai
🏁 Script executed:
cat -n apps/condo/domains/user/components/auth/SecondFactorForm.tsx | sed -n '180,230p'Repository: open-condo-software/condo
Length of output: 2447
🏁 Script executed:
rg "confirmPhoneTokenMutation|confirmEmailTokenMutation" apps/condo/domains/user/components/auth/SecondFactorForm.tsx -B 5 -A 5Repository: open-condo-software/condo
Length of output: 51
🏁 Script executed:
rg "completeConfirmPhoneMutation|completeConfirmEmailMutation" apps/condo/domains/user/components/auth/ -A 3Repository: open-condo-software/condo
Length of output: 4553
🏁 Script executed:
rg "gql\`.*completeConfirmPhone\|gql\`.*completeConfirmEmail" apps/condo/domains/user/ -A 15Repository: open-condo-software/condo
Length of output: 51
🏁 Script executed:
find apps/condo/domains/user -type f -name "*.graphql" -o -name "*mutations*" | head -20Repository: open-condo-software/condo
Length of output: 1244
🏁 Script executed:
rg "useCompleteConfirmPhoneActionMutation|useCompleteConfirmEmailActionMutation" apps/condo/domains/user -lRepository: open-condo-software/condo
Length of output: 253
🏁 Script executed:
cat apps/condo/domains/user/queries/CompleteConfirmPhoneAction.graphqlRepository: open-condo-software/condo
Length of output: 212
🏁 Script executed:
cat apps/condo/domains/user/queries/CompleteConfirmEmailAction.graphqlRepository: open-condo-software/condo
Length of output: 212
🏁 Script executed:
rg "useCompleteConfirmPhoneActionMutation\|useCompleteConfirmEmailActionMutation" -A 2 apps/condo/domains/user/components/auth/SecondFactorForm.tsxRepository: open-condo-software/condo
Length of output: 51
🏁 Script executed:
cat -n apps/condo/domains/user/components/auth/SecondFactorForm.tsx | sed -n '1,50p' | grep -E "import|@ts-ignore"Repository: open-condo-software/condo
Length of output: 1166
🏁 Script executed:
rg "graphQLErrors" apps/condo/domains/user/components/auth/SecondFactorForm.tsxRepository: open-condo-software/condo
Length of output: 182
Fix error message extraction from mutation results.
FetchResult.errors is an array of GraphQL errors, not an object with a graphQLErrors property. The current code accessing errors.graphQLErrors always returns undefined and falls back to ServerErrorMessage. Extract the error directly from the array and remove the @ts-ignore.
🐛 Suggested fix
- // `@ts-ignore`
- const errorMessage = res.errors?.graphQLErrors?.[0]?.extensions?.messageForUser || ServerErrorMessage
+ const errorMessage = res.errors?.[0]?.extensions?.messageForUser || ServerErrorMessage📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (res?.errors) { | |
| // @ts-ignore | |
| const errorMessage = res.errors?.graphQLErrors?.[0]?.extensions?.messageForUser || ServerErrorMessage | |
| if (currentSecondFactor === 'confirmPhoneToken') { | |
| form.setFields([{ | |
| name: 'confirmPhoneCode', | |
| errors: [errorMessage], | |
| }]) | |
| } else if (currentSecondFactor === 'confirmEmailToken') { | |
| form.setFields([{ | |
| name: 'confirmEmailCode', | |
| errors: [errorMessage], | |
| }]) | |
| } | |
| if (res?.errors) { | |
| const errorMessage = res.errors?.[0]?.extensions?.messageForUser || ServerErrorMessage | |
| if (currentSecondFactor === 'confirmPhoneToken') { | |
| form.setFields([{ | |
| name: 'confirmPhoneCode', | |
| errors: [errorMessage], | |
| }]) | |
| } else if (currentSecondFactor === 'confirmEmailToken') { | |
| form.setFields([{ | |
| name: 'confirmEmailCode', | |
| errors: [errorMessage], | |
| }]) | |
| } |
🤖 Prompt for AI Agents
In `@apps/condo/domains/user/components/auth/SecondFactorForm.tsx` around lines
229 - 243, In SecondFactorForm, replace the incorrect access to
res.errors.graphQLErrors by reading the first GraphQLError from the errors array
on the mutation result (res.errors?.[0]); derive the message as
firstError?.extensions?.messageForUser || firstError?.message ||
ServerErrorMessage, remove the `@ts-ignore`, and use that errorMessage in the
existing form.setFields calls (when currentSecondFactor is 'confirmPhoneToken'
or 'confirmEmailToken') so the correct error text is shown to the user.
| const InstructionStepSupportTelegramChat = intl.formatMessage({ id: 'pages.auth.validateIdentifierForm.problemsModal.instruction.steps.supportTelegramChat' }, { | ||
| chatBotLink: ( | ||
| <SecondaryLink target='_blank' href={HelpRequisites?.support_bot ? `https://t.me/${HelpRequisites.support_bot}` : '#'}> | ||
| {ChatInTelegramMessage} | ||
| </SecondaryLink> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find SecondaryLink component definition
fd -a 'SecondaryLink' -E node_modules | head -20Repository: open-condo-software/condo
Length of output: 229
🏁 Script executed:
#!/bin/bash
# Search for SecondaryLink in the codebase more broadly
rg 'export.*SecondaryLink|const SecondaryLink|function SecondaryLink' -E --max-count=20Repository: open-condo-software/condo
Length of output: 475
🏁 Script executed:
#!/bin/bash
# Check if SecondaryLink is from `@open-condo/ui`
rg "from ['\"@open-condo/ui" --max-count=50 | grep -i linkRepository: open-condo-software/condo
Length of output: 201
🏁 Script executed:
cat -n /home/jailuser/git/apps/condo/domains/user/components/auth/SecondaryLink.tsxRepository: open-condo-software/condo
Length of output: 510
🏁 Script executed:
rg 'rel' /home/jailuser/git/apps/condo/domains/user/components/auth/SecondaryLink.tsx -A 2 -B 2Repository: open-condo-software/condo
Length of output: 51
🏁 Script executed:
#!/bin/bash
# Search for other uses of target="_blank" in the codebase to see the pattern
rg "target=['\"]_blank['\"]" -A 2 -B 2 --max-count=20Repository: open-condo-software/condo
Length of output: 38151
🏁 Script executed:
#!/bin/bash
# Check if rel is typically added alongside target="_blank"
rg "target=['\"]_blank['\"].*rel=" --max-count=20Repository: open-condo-software/condo
Length of output: 1672
Add rel='noopener noreferrer' to prevent reverse-tabnabbing.
The external Telegram link opens with target='_blank' but lacks the rel attribute. SecondaryLink does not automatically inject rel, so add it here.
Proposed fix
- <SecondaryLink target='_blank' href={HelpRequisites?.support_bot ? `https://t.me/${HelpRequisites.support_bot}` : '#'}>
+ <SecondaryLink target='_blank' rel='noopener noreferrer' href={HelpRequisites?.support_bot ? `https://t.me/${HelpRequisites.support_bot}` : '#'}>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const InstructionStepSupportTelegramChat = intl.formatMessage({ id: 'pages.auth.validateIdentifierForm.problemsModal.instruction.steps.supportTelegramChat' }, { | |
| chatBotLink: ( | |
| <SecondaryLink target='_blank' href={HelpRequisites?.support_bot ? `https://t.me/${HelpRequisites.support_bot}` : '#'}> | |
| {ChatInTelegramMessage} | |
| </SecondaryLink> | |
| const InstructionStepSupportTelegramChat = intl.formatMessage({ id: 'pages.auth.validateIdentifierForm.problemsModal.instruction.steps.supportTelegramChat' }, { | |
| chatBotLink: ( | |
| <SecondaryLink target='_blank' rel='noopener noreferrer' href={HelpRequisites?.support_bot ? `https://t.me/${HelpRequisites.support_bot}` : '#'}> | |
| {ChatInTelegramMessage} | |
| </SecondaryLink> |
🤖 Prompt for AI Agents
In `@apps/condo/domains/user/components/SudoTokenProvider.tsx` around lines 77 -
81, The external Telegram link rendered in InstructionStepSupportTelegramChat
uses SecondaryLink with target='_blank' but lacks rel attributes; update the
SecondaryLink element where it constructs the chatBotLink (using
HelpRequisites.support_bot and ChatInTelegramMessage) to include rel='noopener
noreferrer' so the opened page cannot access window.opener and to prevent
reverse-tabnabbing.
c7ead17 to
a2f5bad
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@apps/condo/domains/user/schema/ConfirmPhoneActionService.js`:
- Around line 266-276: The code currently assigns phone = user?.phone || null
which causes normalizePhone(null) to trigger ERRORS.WRONG_PHONE_FORMAT; instead,
after fetching the user in ConfirmPhoneActionService (check userFromInput and
User.getOne), add an explicit check for a missing phone (user.phone === null or
undefined) and throw a clearer GQLError (e.g. ERRORS.USER_HAS_NO_PHONE) before
calling normalizePhone; update the error enum/constants to include
USER_HAS_NO_PHONE if needed and ensure you still fall back to phoneFromInput
when no user ID is provided.
In `@apps/condo/lang/en/en.json`:
- Around line 743-755: Replace the inconsistent "Second factor" phrasing with
"Two-factor authentication" across the SecondFactorForm keys: update
component.SecondFactorForm.title,
component.SecondFactorForm.title.forAuthorization, and any user-facing
labels/descriptions under component.SecondFactorForm.type.* to use "Two-factor
authentication" (or "Two-factor authentication for authorization" where context
requires) while keeping placeholders like {email}/{phone} and button text (e.g.,
"Send code again", "Continue") unchanged; make the JSON value edits for these
keys so wording matches the profile section's "Two-factor authentication"
terminology.
- Around line 2453-2454: The JSON string for the key
"pages.condo.profile.2FA.tooltip" contains unescaped double quotes around Edit
and Information which breaks parsing; update that value so the internal quotes
are escaped (e.g., change "click "Edit" in the "Information" tab" to "click
\"Edit\" in the \"Information\" tab") or replace the inner quotes with single
quotes, ensuring the string remains valid JSON and referencing the
"pages.condo.profile.2FA.tooltip" key.
In `@apps/condo/lang/es/es.json`:
- Around line 556-557: Update the Spanish strings for the two 2FA keys to use
consistent terminology: replace "api.user.NOT_ENOUGH_AUTH_FACTORS" value with
wording matching the existing INSUFFICIENT_AUTHENTICATION_FACTORS phrasing
(e.g., "Factores de autenticación insuficientes") and change
"api.user.authenticateOrRegisterUserWithToken.UNEXPECTED_SECOND_FACTOR" to use
"segundo factor" instead of "segundo nivel" (e.g., "Factor inesperado: segundo
factor"). Ensure both keys use the same noun choice ("factor"/"factores") and
similar adjective ordering for consistency in the UI.
- Around line 759-762: The two i18n keys
"component.SudoTokenProvider.modal.password.title.disable2FA" and
"component.SudoTokenProvider.modal.password.title.enable2FA" use the verb
“ingresar” while the rest of the modal strings use “introducir”; update the
values of those two keys to use “introducir” (e.g., replace “necesitas ingresar
una contraseña.” with “necesitas introducir una contraseña.”) so the verb choice
is consistent with "component.SudoTokenProvider.modal.password.label" and
"component.SudoTokenProvider.modal.password.title".
- Around line 2453-2454: The tooltip string for key
pages.condo.profile.2FA.tooltip uses formal "usted" but the app uses informal
"tú"; update the value to use informal phrasing (e.g., "Para permitirnos
enviarte el código, por favor proporciona tus {factors}. Para hacerlo, haz clic
en \"Editar\" en la pestaña \"Información\" y añade cualquier información que
falte.") so it matches the UI tone while preserving the {factors} placeholder
and escaped quotes.
- Around line 743-755: Update the 2FA copy to use the informal tone
consistently: change
"component.SecondFactorForm.type.confirmPhoneToken.description" from "Ingrese el
código enviado a {phone}" to use the informal verb "Introduce..." and change
"component.SecondFactorForm.type.password.description" from "Introduzca la
contraseña" to "Introduce la contraseña"; keep placeholders ({phone}, etc.)
unchanged and leave the existing labels
(component.SecondFactorForm.type.confirmPhoneToken.label,
component.SecondFactorForm.type.password.label) as-is.
In `@apps/condo/lang/ru/ru.json`:
- Around line 759-762: The Russian strings for keys
component.SudoTokenProvider.modal.password.title.disable2FA and
component.SudoTokenProvider.modal.password.title.enable2FA are missing a comma
after “авторизацию”; update those two values to insert a comma after
“авторизацию” (i.e., "Чтобы выключить двухфакторную авторизацию, требуется
ввести пароль" and "Чтобы включить двухфакторную авторизацию, требуется ввести
пароль") so the subordinate clause is punctuated correctly.
In `@apps/condo/pages/auth/change-password.tsx`:
- Around line 472-476: The useEffect listening for step ===
'getSudoTokenBeforeChangePassword' uses onSubmitWithSecondFactor but doesn't
include it in the dependency array; either add onSubmitWithSecondFactor to the
dependency array of the effect or memoize onSubmitWithSecondFactor with
useCallback so the effect won't capture a stale closure—update the effect to use
[step, onSubmitWithSecondFactor] (or wrap the onSubmitWithSecondFactor
definition in useCallback and keep the dependency array as [step]) ensuring
references to onSubmitWithSecondFactor and step are consistent.
🧹 Nitpick comments (5)
apps/condo/domains/user/schema/ConfirmEmailActionService.js (2)
86-99: Document the new validation errors in the mutation docs.
These errors are now thrown but aren’t listed in thestartConfirmEmailActiondoc block. Consider adding them to improve GraphiQL docs and client expectations.Proposed doc update
errors: { ...pick(ERRORS, [ DV_VERSION_MISMATCH, 'WRONG_SENDER_FORMAT', CAPTCHA_CHECK_FAILED, UNABLE_TO_FIND_CONFIRM_EMAIL_ACTION, 'WRONG_EMAIL_FORMAT', 'GENERATE_TOKEN_ERROR', + 'EMAIL_AND_USER_ID_IS_MISSING', + 'SHOULD_BE_ONE_IDENTIFIER_ONLY', ]), },
287-327: Consider a clearer error whenuserIddoesn’t resolve to an email.
Right now a missing user/email falls through toWRONG_EMAIL_FORMAT, which is confusing for userId-based calls. A dedicated error (or mapping to a “user not found / missing email” case) would be clearer.apps/condo/pages/auth/change-password.tsx (2)
551-559: Use JSX children instead of thechildrenprop.The static analysis tool correctly flagged that passing children via a prop is not the canonical React pattern.
♻️ Proposed fix
<Button key='submit' type='primary' loading={isLoading} htmlType='submit' - children={`${SaveMsg} ${AndSignInMessage}`} block - /> + > + {`${SaveMsg} ${AndSignInMessage}`} + </Button>
159-164: Consider extracting duplicate second factor mapping logic.The same conditional mapping for
secondFactorbased oncurrentSecondFactortype is repeated for both email (lines 159-164) and phone (lines 178-183) authentication paths. This could be extracted into a helper function.♻️ Example helper extraction
// Helper to build second factor input const buildSecondFactor = <T extends { ConfirmEmailToken: string, ConfirmPhoneToken: string }>( secondFactorData: { currentSecondFactor?: 'confirmEmailToken' | 'confirmPhoneToken', confirmToken: string } | undefined, types: T ): { value: string, type: T['ConfirmEmailToken'] | T['ConfirmPhoneToken'] } | undefined => { if (!secondFactorData) return undefined if (secondFactorData.currentSecondFactor === 'confirmEmailToken') { return { value: secondFactorData.confirmToken, type: types.ConfirmEmailToken } } if (secondFactorData.currentSecondFactor === 'confirmPhoneToken') { return { value: secondFactorData.confirmToken, type: types.ConfirmPhoneToken } } return undefined }apps/condo/lang/ru/ru.json (1)
556-561: Unify 2FA terminology for consistency.Here it’s “факторов авторизации”, while existing 2FA strings nearby use “факторов аутентификации”. Consider aligning to avoid mixed terminology in user-facing errors.
♻️ Suggested wording alignment
-"api.user.NOT_ENOUGH_AUTH_FACTORS": "Недостаточно факторов авторизации", +"api.user.NOT_ENOUGH_AUTH_FACTORS": "Недостаточно факторов аутентификации",
| if (userFromInput?.id) { | ||
| const user = await User.getOne(context, { id: userFromInput.id, deletedAt: null }, 'id phone') | ||
| phone = user?.phone || null | ||
| } else { | ||
| phone = phoneFromInput | ||
| } | ||
|
|
||
| const normalizedPhone = normalizePhone(phone) | ||
| if (!normalizedPhone) { | ||
| throw new GQLError(ERRORS.WRONG_PHONE_FORMAT, context) | ||
| } |
There was a problem hiding this comment.
User without phone returns null, leading to unclear error.
If a user exists but has no phone number set (user.phone is null/undefined), the code will proceed with phone = null, and later normalizePhone(null) will return a falsy value, throwing WRONG_PHONE_FORMAT. This error message is misleading when the actual issue is that the user has no phone.
🔧 Proposed fix - add explicit check for missing phone
if (userFromInput?.id) {
const user = await User.getOne(context, { id: userFromInput.id, deletedAt: null }, 'id phone')
- phone = user?.phone || null
+ if (!user) {
+ throw new GQLError({ ...ERRORS.WRONG_PHONE_FORMAT, message: 'User not found' }, context)
+ }
+ if (!user.phone) {
+ throw new GQLError({ ...ERRORS.WRONG_PHONE_FORMAT, message: 'User has no phone number' }, context)
+ }
+ phone = user.phone
} else {
phone = phoneFromInput
}Alternatively, consider adding a dedicated error type like USER_HAS_NO_PHONE for clarity.
🤖 Prompt for AI Agents
In `@apps/condo/domains/user/schema/ConfirmPhoneActionService.js` around lines 266
- 276, The code currently assigns phone = user?.phone || null which causes
normalizePhone(null) to trigger ERRORS.WRONG_PHONE_FORMAT; instead, after
fetching the user in ConfirmPhoneActionService (check userFromInput and
User.getOne), add an explicit check for a missing phone (user.phone === null or
undefined) and throw a clearer GQLError (e.g. ERRORS.USER_HAS_NO_PHONE) before
calling normalizePhone; update the error enum/constants to include
USER_HAS_NO_PHONE if needed and ensure you still fall back to phoneFromInput
when no user ID is provided.
| "component.SecondFactorForm.title": "Second factor", | ||
| "component.SecondFactorForm.title.forAuthorization": "Second factor of authorization", | ||
| "component.SecondFactorForm.type.confirmEmailToken.description": "Enter the code sent to {email}", | ||
| "component.SecondFactorForm.type.confirmEmailToken.label": "By email", | ||
| "component.SecondFactorForm.type.confirmEmailToken.problems": "I haven't received the email with the code", | ||
| "component.SecondFactorForm.type.confirmEmailToken.sendCodeAgain": "Send code again", | ||
| "component.SecondFactorForm.type.confirmPhoneToken.description": "Enter the code sent to {phone}", | ||
| "component.SecondFactorForm.type.confirmPhoneToken.label": "By phone", | ||
| "component.SecondFactorForm.type.confirmPhoneToken.problems": "I haven't received an SMS with a code", | ||
| "component.SecondFactorForm.type.confirmPhoneToken.sendCodeAgain": "Send code again", | ||
| "component.SecondFactorForm.type.password.description": "Enter password", | ||
| "component.SecondFactorForm.type.password.label": "By password", | ||
| "component.SecondFactorForm.type.password.continue": "Continue", |
There was a problem hiding this comment.
Align “Second factor” wording with the “Two‑factor authentication” terminology.
This section currently uses “Second factor” while the profile section uses “Two-factor authentication.” Consider consistent phrasing across UI.
✏️ Suggested copy update
- "component.SecondFactorForm.title": "Second factor",
- "component.SecondFactorForm.title.forAuthorization": "Second factor of authorization",
+ "component.SecondFactorForm.title": "Two-factor authentication",
+ "component.SecondFactorForm.title.forAuthorization": "Two-factor authentication for sign-in",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "component.SecondFactorForm.title": "Second factor", | |
| "component.SecondFactorForm.title.forAuthorization": "Second factor of authorization", | |
| "component.SecondFactorForm.type.confirmEmailToken.description": "Enter the code sent to {email}", | |
| "component.SecondFactorForm.type.confirmEmailToken.label": "By email", | |
| "component.SecondFactorForm.type.confirmEmailToken.problems": "I haven't received the email with the code", | |
| "component.SecondFactorForm.type.confirmEmailToken.sendCodeAgain": "Send code again", | |
| "component.SecondFactorForm.type.confirmPhoneToken.description": "Enter the code sent to {phone}", | |
| "component.SecondFactorForm.type.confirmPhoneToken.label": "By phone", | |
| "component.SecondFactorForm.type.confirmPhoneToken.problems": "I haven't received an SMS with a code", | |
| "component.SecondFactorForm.type.confirmPhoneToken.sendCodeAgain": "Send code again", | |
| "component.SecondFactorForm.type.password.description": "Enter password", | |
| "component.SecondFactorForm.type.password.label": "By password", | |
| "component.SecondFactorForm.type.password.continue": "Continue", | |
| "component.SecondFactorForm.title": "Two-factor authentication", | |
| "component.SecondFactorForm.title.forAuthorization": "Two-factor authentication for sign-in", | |
| "component.SecondFactorForm.type.confirmEmailToken.description": "Enter the code sent to {email}", | |
| "component.SecondFactorForm.type.confirmEmailToken.label": "By email", | |
| "component.SecondFactorForm.type.confirmEmailToken.problems": "I haven't received the email with the code", | |
| "component.SecondFactorForm.type.confirmEmailToken.sendCodeAgain": "Send code again", | |
| "component.SecondFactorForm.type.confirmPhoneToken.description": "Enter the code sent to {phone}", | |
| "component.SecondFactorForm.type.confirmPhoneToken.label": "By phone", | |
| "component.SecondFactorForm.type.confirmPhoneToken.problems": "I haven't received an SMS with a code", | |
| "component.SecondFactorForm.type.confirmPhoneToken.sendCodeAgain": "Send code again", | |
| "component.SecondFactorForm.type.password.description": "Enter password", | |
| "component.SecondFactorForm.type.password.label": "By password", | |
| "component.SecondFactorForm.type.password.continue": "Continue", |
🤖 Prompt for AI Agents
In `@apps/condo/lang/en/en.json` around lines 743 - 755, Replace the inconsistent
"Second factor" phrasing with "Two-factor authentication" across the
SecondFactorForm keys: update component.SecondFactorForm.title,
component.SecondFactorForm.title.forAuthorization, and any user-facing
labels/descriptions under component.SecondFactorForm.type.* to use "Two-factor
authentication" (or "Two-factor authentication for authorization" where context
requires) while keeping placeholders like {email}/{phone} and button text (e.g.,
"Send code again", "Continue") unchanged; make the JSON value edits for these
keys so wording matches the profile section's "Two-factor authentication"
terminology.
| "api.user.NOT_ENOUGH_AUTH_FACTORS": "No hay suficientes factores de autenticación", | ||
| "api.user.authenticateOrRegisterUserWithToken.UNEXPECTED_SECOND_FACTOR": "Factor inesperado de segundo nivel", |
There was a problem hiding this comment.
Align 2FA error phrasing for consistency.
Line 556 uses different wording than the existing “INSUFFICIENT_AUTHENTICATION_FACTORS”, and Line 557 says “segundo nivel” while the UI uses “segundo factor”. Consider unifying terminology to avoid mixed messages.
✏️ Suggested update
- "api.user.NOT_ENOUGH_AUTH_FACTORS": "No hay suficientes factores de autenticación",
- "api.user.authenticateOrRegisterUserWithToken.UNEXPECTED_SECOND_FACTOR": "Factor inesperado de segundo nivel",
+ "api.user.NOT_ENOUGH_AUTH_FACTORS": "Factores de autenticación insuficientes",
+ "api.user.authenticateOrRegisterUserWithToken.UNEXPECTED_SECOND_FACTOR": "Segundo factor inesperado",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "api.user.NOT_ENOUGH_AUTH_FACTORS": "No hay suficientes factores de autenticación", | |
| "api.user.authenticateOrRegisterUserWithToken.UNEXPECTED_SECOND_FACTOR": "Factor inesperado de segundo nivel", | |
| "api.user.NOT_ENOUGH_AUTH_FACTORS": "Factores de autenticación insuficientes", | |
| "api.user.authenticateOrRegisterUserWithToken.UNEXPECTED_SECOND_FACTOR": "Segundo factor inesperado", |
🤖 Prompt for AI Agents
In `@apps/condo/lang/es/es.json` around lines 556 - 557, Update the Spanish
strings for the two 2FA keys to use consistent terminology: replace
"api.user.NOT_ENOUGH_AUTH_FACTORS" value with wording matching the existing
INSUFFICIENT_AUTHENTICATION_FACTORS phrasing (e.g., "Factores de autenticación
insuficientes") and change
"api.user.authenticateOrRegisterUserWithToken.UNEXPECTED_SECOND_FACTOR" to use
"segundo factor" instead of "segundo nivel" (e.g., "Factor inesperado: segundo
factor"). Ensure both keys use the same noun choice ("factor"/"factores") and
similar adjective ordering for consistency in the UI.
| "component.SecondFactorForm.title": "Segundo factor", | ||
| "component.SecondFactorForm.title.forAuthorization": "Segundo factor de autorización", | ||
| "component.SecondFactorForm.type.confirmEmailToken.description": "Introduce el código enviado a {email}", | ||
| "component.SecondFactorForm.type.confirmEmailToken.label": "Por email", | ||
| "component.SecondFactorForm.type.confirmEmailToken.problems": "No he recibido el correo electrónico con el código", | ||
| "component.SecondFactorForm.type.confirmEmailToken.sendCodeAgain": "Enviar el código de nuevo", | ||
| "component.SecondFactorForm.type.confirmPhoneToken.description": "Ingrese el código enviado a {phone}", | ||
| "component.SecondFactorForm.type.confirmPhoneToken.label": "Por teléfono", | ||
| "component.SecondFactorForm.type.confirmPhoneToken.problems": "No he recibido un SMS con un código", | ||
| "component.SecondFactorForm.type.confirmPhoneToken.sendCodeAgain": "Enviar el código de nuevo", | ||
| "component.SecondFactorForm.type.password.description": "Introduzca la contraseña", | ||
| "component.SecondFactorForm.type.password.label": "Por contraseña", | ||
| "component.SecondFactorForm.type.password.continue": "Continuar", |
There was a problem hiding this comment.
Normalize tone and terminology in 2FA form strings.
Lines 746, 749, and 753 mix “email/telefono” and formal verbs (“Ingrese/Introduzca”) with mostly informal copy elsewhere. Suggest aligning to the prevalent informal tone.
✏️ Suggested update
- "component.SecondFactorForm.type.confirmEmailToken.label": "Por email",
+ "component.SecondFactorForm.type.confirmEmailToken.label": "Por correo electrónico",
- "component.SecondFactorForm.type.confirmPhoneToken.description": "Ingrese el código enviado a {phone}",
+ "component.SecondFactorForm.type.confirmPhoneToken.description": "Introduce el código enviado a {phone}",
- "component.SecondFactorForm.type.confirmPhoneToken.problems": "No he recibido un SMS con un código",
+ "component.SecondFactorForm.type.confirmPhoneToken.problems": "No he recibido el SMS con el código",
- "component.SecondFactorForm.type.password.description": "Introduzca la contraseña",
+ "component.SecondFactorForm.type.password.description": "Introduce la contraseña",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "component.SecondFactorForm.title": "Segundo factor", | |
| "component.SecondFactorForm.title.forAuthorization": "Segundo factor de autorización", | |
| "component.SecondFactorForm.type.confirmEmailToken.description": "Introduce el código enviado a {email}", | |
| "component.SecondFactorForm.type.confirmEmailToken.label": "Por email", | |
| "component.SecondFactorForm.type.confirmEmailToken.problems": "No he recibido el correo electrónico con el código", | |
| "component.SecondFactorForm.type.confirmEmailToken.sendCodeAgain": "Enviar el código de nuevo", | |
| "component.SecondFactorForm.type.confirmPhoneToken.description": "Ingrese el código enviado a {phone}", | |
| "component.SecondFactorForm.type.confirmPhoneToken.label": "Por teléfono", | |
| "component.SecondFactorForm.type.confirmPhoneToken.problems": "No he recibido un SMS con un código", | |
| "component.SecondFactorForm.type.confirmPhoneToken.sendCodeAgain": "Enviar el código de nuevo", | |
| "component.SecondFactorForm.type.password.description": "Introduzca la contraseña", | |
| "component.SecondFactorForm.type.password.label": "Por contraseña", | |
| "component.SecondFactorForm.type.password.continue": "Continuar", | |
| "component.SecondFactorForm.title": "Segundo factor", | |
| "component.SecondFactorForm.title.forAuthorization": "Segundo factor de autorización", | |
| "component.SecondFactorForm.type.confirmEmailToken.description": "Introduce el código enviado a {email}", | |
| "component.SecondFactorForm.type.confirmEmailToken.label": "Por correo electrónico", | |
| "component.SecondFactorForm.type.confirmEmailToken.problems": "No he recibido el correo electrónico con el código", | |
| "component.SecondFactorForm.type.confirmEmailToken.sendCodeAgain": "Enviar el código de nuevo", | |
| "component.SecondFactorForm.type.confirmPhoneToken.description": "Introduce el código enviado a {phone}", | |
| "component.SecondFactorForm.type.confirmPhoneToken.label": "Por teléfono", | |
| "component.SecondFactorForm.type.confirmPhoneToken.problems": "No he recibido el SMS con el código", | |
| "component.SecondFactorForm.type.confirmPhoneToken.sendCodeAgain": "Enviar el código de nuevo", | |
| "component.SecondFactorForm.type.password.description": "Introduce la contraseña", | |
| "component.SecondFactorForm.type.password.label": "Por contraseña", | |
| "component.SecondFactorForm.type.password.continue": "Continuar", |
🤖 Prompt for AI Agents
In `@apps/condo/lang/es/es.json` around lines 743 - 755, Update the 2FA copy to
use the informal tone consistently: change
"component.SecondFactorForm.type.confirmPhoneToken.description" from "Ingrese el
código enviado a {phone}" to use the informal verb "Introduce..." and change
"component.SecondFactorForm.type.password.description" from "Introduzca la
contraseña" to "Introduce la contraseña"; keep placeholders ({phone}, etc.)
unchanged and leave the existing labels
(component.SecondFactorForm.type.confirmPhoneToken.label,
component.SecondFactorForm.type.password.label) as-is.
| "component.SudoTokenProvider.modal.password.label": "Introduzca la contraseña", | ||
| "component.SudoTokenProvider.modal.password.title": "Introduzca la contraseña", | ||
| "component.SudoTokenProvider.modal.password.title.disable2FA": "Para desactivar la autenticación de dos factores, necesitas ingresar una contraseña.", | ||
| "component.SudoTokenProvider.modal.password.title.enable2FA": "Para habilitar la autenticación de dos factores, necesitas ingresar una contraseña.", |
There was a problem hiding this comment.
Use consistent verb choice in 2FA enable/disable modal.
Lines 761-762 use “ingresar”, while nearby strings use “introducir”. Consider aligning to “introducir” for consistency.
✏️ Suggested update
- "component.SudoTokenProvider.modal.password.title.disable2FA": "Para desactivar la autenticación de dos factores, necesitas ingresar una contraseña.",
- "component.SudoTokenProvider.modal.password.title.enable2FA": "Para habilitar la autenticación de dos factores, necesitas ingresar una contraseña.",
+ "component.SudoTokenProvider.modal.password.title.disable2FA": "Para desactivar la autenticación de dos factores, necesitas introducir la contraseña.",
+ "component.SudoTokenProvider.modal.password.title.enable2FA": "Para habilitar la autenticación de dos factores, necesitas introducir la contraseña.",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "component.SudoTokenProvider.modal.password.label": "Introduzca la contraseña", | |
| "component.SudoTokenProvider.modal.password.title": "Introduzca la contraseña", | |
| "component.SudoTokenProvider.modal.password.title.disable2FA": "Para desactivar la autenticación de dos factores, necesitas ingresar una contraseña.", | |
| "component.SudoTokenProvider.modal.password.title.enable2FA": "Para habilitar la autenticación de dos factores, necesitas ingresar una contraseña.", | |
| "component.SudoTokenProvider.modal.password.label": "Introduzca la contraseña", | |
| "component.SudoTokenProvider.modal.password.title": "Introduzca la contraseña", | |
| "component.SudoTokenProvider.modal.password.title.disable2FA": "Para desactivar la autenticación de dos factores, necesitas introducir la contraseña.", | |
| "component.SudoTokenProvider.modal.password.title.enable2FA": "Para habilitar la autenticación de dos factores, necesitas introducir la contraseña.", |
🤖 Prompt for AI Agents
In `@apps/condo/lang/es/es.json` around lines 759 - 762, The two i18n keys
"component.SudoTokenProvider.modal.password.title.disable2FA" and
"component.SudoTokenProvider.modal.password.title.enable2FA" use the verb
“ingresar” while the rest of the modal strings use “introducir”; update the
values of those two keys to use “introducir” (e.g., replace “necesitas ingresar
una contraseña.” with “necesitas introducir una contraseña.”) so the verb choice
is consistent with "component.SudoTokenProvider.modal.password.label" and
"component.SudoTokenProvider.modal.password.title".
apps/condo/lang/es/es.json
Outdated
| "pages.condo.profile.2FA.title": "Autenticación de dos factores", | ||
| "pages.condo.profile.2FA.tooltip": "Para permitirnos enviarle el código, por favor proporcione sus {factors}. Para hacerlo, haga clic en \"Editar\" en la pestaña \"Información\" y agregue cualquier información que falte.", |
There was a problem hiding this comment.
Match informal tone in the profile 2FA tooltip.
Line 2454 is in formal “usted” while most UI strings use “tú”. Consider switching to informal for consistency.
✏️ Suggested update
- "pages.condo.profile.2FA.tooltip": "Para permitirnos enviarle el código, por favor proporcione sus {factors}. Para hacerlo, haga clic en \"Editar\" en la pestaña \"Información\" y agregue cualquier información que falte.",
+ "pages.condo.profile.2FA.tooltip": "Para poder enviarte el código, por favor proporciona tus {factors}. Para hacerlo, haz clic en \"Editar\" en la pestaña \"Información\" y añade la información que falte.",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "pages.condo.profile.2FA.title": "Autenticación de dos factores", | |
| "pages.condo.profile.2FA.tooltip": "Para permitirnos enviarle el código, por favor proporcione sus {factors}. Para hacerlo, haga clic en \"Editar\" en la pestaña \"Información\" y agregue cualquier información que falte.", | |
| "pages.condo.profile.2FA.title": "Autenticación de dos factores", | |
| "pages.condo.profile.2FA.tooltip": "Para poder enviarte el código, por favor proporciona tus {factors}. Para hacerlo, haz clic en \"Editar\" en la pestaña \"Información\" y añade la información que falte.", |
🤖 Prompt for AI Agents
In `@apps/condo/lang/es/es.json` around lines 2453 - 2454, The tooltip string for
key pages.condo.profile.2FA.tooltip uses formal "usted" but the app uses
informal "tú"; update the value to use informal phrasing (e.g., "Para
permitirnos enviarte el código, por favor proporciona tus {factors}. Para
hacerlo, haz clic en \"Editar\" en la pestaña \"Información\" y añade cualquier
información que falte.") so it matches the UI tone while preserving the
{factors} placeholder and escaped quotes.
| "component.SudoTokenProvider.modal.password.label": "Введите пароль", | ||
| "component.SudoTokenProvider.modal.password.title": "Введите пароль", | ||
| "component.SudoTokenProvider.modal.password.title.disable2FA": "Чтобы выключить двухфакторную авторизацию требуется ввести пароль", | ||
| "component.SudoTokenProvider.modal.password.title.enable2FA": "Чтобы включить двухфакторную авторизацию требуется ввести пароль", |
There was a problem hiding this comment.
Add missing comma in subordinate clause.
In Russian, a comma is needed after “авторизацию” in these sentences.
✏️ Proposed fix
-"component.SudoTokenProvider.modal.password.title.disable2FA": "Чтобы выключить двухфакторную авторизацию требуется ввести пароль",
-"component.SudoTokenProvider.modal.password.title.enable2FA": "Чтобы включить двухфакторную авторизацию требуется ввести пароль",
+"component.SudoTokenProvider.modal.password.title.disable2FA": "Чтобы выключить двухфакторную авторизацию, требуется ввести пароль",
+"component.SudoTokenProvider.modal.password.title.enable2FA": "Чтобы включить двухфакторную авторизацию, требуется ввести пароль",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "component.SudoTokenProvider.modal.password.label": "Введите пароль", | |
| "component.SudoTokenProvider.modal.password.title": "Введите пароль", | |
| "component.SudoTokenProvider.modal.password.title.disable2FA": "Чтобы выключить двухфакторную авторизацию требуется ввести пароль", | |
| "component.SudoTokenProvider.modal.password.title.enable2FA": "Чтобы включить двухфакторную авторизацию требуется ввести пароль", | |
| "component.SudoTokenProvider.modal.password.label": "Введите пароль", | |
| "component.SudoTokenProvider.modal.password.title": "Введите пароль", | |
| "component.SudoTokenProvider.modal.password.title.disable2FA": "Чтобы выключить двухфакторную авторизацию, требуется ввести пароль", | |
| "component.SudoTokenProvider.modal.password.title.enable2FA": "Чтобы включить двухфакторную авторизацию, требуется ввести пароль", |
🤖 Prompt for AI Agents
In `@apps/condo/lang/ru/ru.json` around lines 759 - 762, The Russian strings for
keys component.SudoTokenProvider.modal.password.title.disable2FA and
component.SudoTokenProvider.modal.password.title.enable2FA are missing a comma
after “авторизацию”; update those two values to insert a comma after
“авторизацию” (i.e., "Чтобы выключить двухфакторную авторизацию, требуется
ввести пароль" and "Чтобы включить двухфакторную авторизацию, требуется ввести
пароль") so the subordinate clause is punctuated correctly.
| useEffect(() => { | ||
| if (step !== 'getSudoTokenBeforeChangePassword') return | ||
|
|
||
| onSubmitWithSecondFactor() | ||
| }, [step]) |
There was a problem hiding this comment.
Missing dependency in useEffect may cause stale closure.
The onSubmitWithSecondFactor function is called inside the effect but is not listed in the dependency array. This could lead to stale closure issues if the callback changes.
🔧 Proposed fix
useEffect(() => {
if (step !== 'getSudoTokenBeforeChangePassword') return
onSubmitWithSecondFactor()
- }, [step])
+ }, [step, onSubmitWithSecondFactor])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| if (step !== 'getSudoTokenBeforeChangePassword') return | |
| onSubmitWithSecondFactor() | |
| }, [step]) | |
| useEffect(() => { | |
| if (step !== 'getSudoTokenBeforeChangePassword') return | |
| onSubmitWithSecondFactor() | |
| }, [step, onSubmitWithSecondFactor]) |
🤖 Prompt for AI Agents
In `@apps/condo/pages/auth/change-password.tsx` around lines 472 - 476, The
useEffect listening for step === 'getSudoTokenBeforeChangePassword' uses
onSubmitWithSecondFactor but doesn't include it in the dependency array; either
add onSubmitWithSecondFactor to the dependency array of the effect or memoize
onSubmitWithSecondFactor with useCallback so the effect won't capture a stale
closure—update the effect to use [step, onSubmitWithSecondFactor] (or wrap the
onSubmitWithSecondFactor definition in useCallback and keep the dependency array
as [step]) ensuring references to onSubmitWithSecondFactor and step are
consistent.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@apps/condo/domains/common/components/Form/FormItem.ts`:
- Line 11: Replace the invalid CSS declaration "white-space: wrap" in
FormItem.ts with a valid value; locate the styled rule or CSS block that sets
"white-space" (search for "white-space: wrap" in FormItem.ts) and change it to
"white-space: normal" (or "white-space: pre-wrap" if you need to preserve
existing line breaks while allowing wrapping) so the label text wraps correctly
in browsers.
In `@apps/condo/domains/user/components/auth/SecondFactorForm.tsx`:
- Around line 114-184: Both startConfirmEmailAction and startConfirmPhoneAction
currently set the token expiry before performing the mutation, which blocks
resends even when the mutation fails; remove the pre-mutation
setConfirmEmailTokenExpiredDate/setConfirmPhoneTokenExpiredDate calls and
instead set the expiry only on success (after you obtain token from
res?.data?.result?.token, e.g. inside the if (token) block) so failures do not
start the cooldown; keep the initial shouldWait check, captcha execution
(executeCaptcha), and mutations
(startConfirmEmailActionMutation/startConfirmPhoneActionMutation) unchanged
otherwise.
- Around line 57-61: The external Telegram link in the
InstructionStepSupportTelegramChat JSX uses target='_blank' without rel
attributes; update the SecondaryLink element (used in
InstructionStepSupportTelegramChat) to include rel='noopener noreferrer' when
rendering the href built from HelpRequisites?.support_bot and
ChatInTelegramMessage so the external link is safe from reverse-tabnabbing.
In `@apps/condo/domains/user/components/SudoTokenProvider.tsx`:
- Around line 354-367: The code assumes res.errors has a graphQLErrors property
which is wrong; update the extraction to read the first error from the errors
array and fallback to its extension or message before using ServerErrorMessage.
Change the errorMessage assignment to something like: const errorMessage =
res.errors?.[0]?.extensions?.messageForUser || res.errors?.[0]?.message ||
ServerErrorMessage (remove the `@ts-ignore`), leaving the existing handling for
currentSecondFactor and form.setFields intact so
confirmPhoneCode/confirmEmailCode get the proper message.
In `@apps/condo/lang/ru/ru.json`:
- Around line 2454-2457: Update the Russian translation value for the key
pages.condo.profile.2FA.tooltip.notVerifiedPhone to correct the typo: replace
"Чтобы мы могли присылать вам код, подтвердите ваш номер телефон" with "Чтобы мы
могли присылать вам код, подтвердите ваш номер телефона" so the phrase uses the
correct genitive form; leave the rest of the JSON key/value intact.
| label { | ||
| height: auto; | ||
| white-space: nowrap; | ||
| white-space: wrap; |
There was a problem hiding this comment.
Invalid CSS value: wrap is not a valid white-space property value.
white-space: wrap is not a recognized CSS value. The valid values are normal, nowrap, pre, pre-wrap, pre-line, and break-spaces. This declaration will be ignored by browsers, and the label wrapping behavior will not work as intended.
To allow text to wrap normally, use white-space: normal:
🐛 Proposed fix
label {
height: auto;
- white-space: wrap;
+ white-space: normal;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| white-space: wrap; | |
| label { | |
| height: auto; | |
| white-space: normal; | |
| } |
🤖 Prompt for AI Agents
In `@apps/condo/domains/common/components/Form/FormItem.ts` at line 11, Replace
the invalid CSS declaration "white-space: wrap" in FormItem.ts with a valid
value; locate the styled rule or CSS block that sets "white-space" (search for
"white-space: wrap" in FormItem.ts) and change it to "white-space: normal" (or
"white-space: pre-wrap" if you need to preserve existing line breaks while
allowing wrapping) so the label text wraps correctly in browsers.
| const InstructionStepSupportTelegramChat = intl.formatMessage({ id: 'pages.auth.validateIdentifierForm.problemsModal.instruction.steps.supportTelegramChat' }, { | ||
| chatBotLink: ( | ||
| <SecondaryLink target='_blank' href={HelpRequisites?.support_bot ? `https://t.me/${HelpRequisites.support_bot}` : '#'}> | ||
| {ChatInTelegramMessage} | ||
| </SecondaryLink> |
There was a problem hiding this comment.
Add rel='noopener noreferrer' to the external Telegram link.
Opening a new tab without rel allows reverse-tabnabbing.
🛡️ Proposed fix
- <SecondaryLink target='_blank' href={HelpRequisites?.support_bot ? `https://t.me/${HelpRequisites.support_bot}` : '#'}>
+ <SecondaryLink target='_blank' rel='noopener noreferrer' href={HelpRequisites?.support_bot ? `https://t.me/${HelpRequisites.support_bot}` : '#'}>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const InstructionStepSupportTelegramChat = intl.formatMessage({ id: 'pages.auth.validateIdentifierForm.problemsModal.instruction.steps.supportTelegramChat' }, { | |
| chatBotLink: ( | |
| <SecondaryLink target='_blank' href={HelpRequisites?.support_bot ? `https://t.me/${HelpRequisites.support_bot}` : '#'}> | |
| {ChatInTelegramMessage} | |
| </SecondaryLink> | |
| const InstructionStepSupportTelegramChat = intl.formatMessage({ id: 'pages.auth.validateIdentifierForm.problemsModal.instruction.steps.supportTelegramChat' }, { | |
| chatBotLink: ( | |
| <SecondaryLink target='_blank' rel='noopener noreferrer' href={HelpRequisites?.support_bot ? `https://t.me/${HelpRequisites.support_bot}` : '#'}> | |
| {ChatInTelegramMessage} | |
| </SecondaryLink> |
🤖 Prompt for AI Agents
In `@apps/condo/domains/user/components/auth/SecondFactorForm.tsx` around lines 57
- 61, The external Telegram link in the InstructionStepSupportTelegramChat JSX
uses target='_blank' without rel attributes; update the SecondaryLink element
(used in InstructionStepSupportTelegramChat) to include rel='noopener
noreferrer' when rendering the href built from HelpRequisites?.support_bot and
ChatInTelegramMessage so the external link is safe from reverse-tabnabbing.
| const startConfirmEmailAction = useCallback(async () => { | ||
| if (!userIdRef.current) { | ||
| throw new Error('No userid for StartConfirmEmailAction!') | ||
| } | ||
|
|
||
| const shouldWait = confirmEmailTokenExpiredDate && dayjs(confirmEmailTokenExpiredDate) > dayjs() | ||
| if (shouldWait) return | ||
|
|
||
| setConfirmEmailTokenExpiredDate(dayjs().add(1, 'minute').toISOString()) | ||
|
|
||
| const sender = getClientSideSenderInfo() | ||
| const captcha = await executeCaptcha() | ||
|
|
||
| const res = await startConfirmEmailActionMutation({ | ||
| variables: { | ||
| data: { | ||
| dv: 1, | ||
| sender, | ||
| captcha, | ||
| user: { | ||
| id: userIdRef.current, | ||
| }, | ||
| }, | ||
| }, | ||
| }) | ||
|
|
||
| setConfirmEmailTokenExpiredDate(dayjs().add(1, 'minute').toISOString()) | ||
|
|
||
| const token = res?.data?.result?.token | ||
| if (token) { | ||
| setConfirmEmailToken(token) | ||
| } | ||
|
|
||
| return res | ||
| }, [executeCaptcha, startConfirmEmailActionMutation, confirmEmailTokenExpiredDate]) | ||
|
|
||
| const startConfirmPhoneAction = useCallback(async () => { | ||
| if (!userIdRef.current) { | ||
| throw new Error('No userid for StartConfirmPhoneAction!') | ||
| } | ||
|
|
||
| const shouldWait = confirmPhoneTokenExpiredDate && dayjs(confirmPhoneTokenExpiredDate) > dayjs() | ||
| if (shouldWait) return | ||
|
|
||
| setConfirmPhoneTokenExpiredDate(dayjs().add(1, 'minute').toISOString()) | ||
|
|
||
| const sender = getClientSideSenderInfo() | ||
| const captcha = await executeCaptcha() | ||
|
|
||
| const res = await startConfirmPhoneActionMutation({ | ||
| variables: { | ||
| data: { | ||
| dv: 1, | ||
| sender, | ||
| captcha, | ||
| user: { | ||
| id: userIdRef.current, | ||
| }, | ||
| }, | ||
| }, | ||
| }) | ||
|
|
||
| setConfirmPhoneTokenExpiredDate(dayjs().add(1, 'minute').toISOString()) | ||
|
|
||
| const token = res?.data?.result?.token | ||
| if (token) { | ||
| setConfirmPhoneToken(token) | ||
| } | ||
|
|
||
| return res | ||
| }, [executeCaptcha, startConfirmPhoneActionMutation, confirmPhoneTokenExpiredDate]) |
There was a problem hiding this comment.
Avoid blocking resend when the startConfirm mutation fails.*
Setting the expiry before the mutation prevents retries for a minute even if the request fails. Move the expiry update to the success path (after token retrieval).
🛠️ Proposed fix
- setConfirmEmailTokenExpiredDate(dayjs().add(1, 'minute').toISOString())
-
const sender = getClientSideSenderInfo()
const captcha = await executeCaptcha()
@@
- setConfirmEmailTokenExpiredDate(dayjs().add(1, 'minute').toISOString())
-
const token = res?.data?.result?.token
if (token) {
setConfirmEmailToken(token)
+ setConfirmEmailTokenExpiredDate(dayjs().add(1, 'minute').toISOString())
}
@@
- setConfirmPhoneTokenExpiredDate(dayjs().add(1, 'minute').toISOString())
-
const sender = getClientSideSenderInfo()
const captcha = await executeCaptcha()
@@
- setConfirmPhoneTokenExpiredDate(dayjs().add(1, 'minute').toISOString())
-
const token = res?.data?.result?.token
if (token) {
setConfirmPhoneToken(token)
+ setConfirmPhoneTokenExpiredDate(dayjs().add(1, 'minute').toISOString())
}🤖 Prompt for AI Agents
In `@apps/condo/domains/user/components/auth/SecondFactorForm.tsx` around lines
114 - 184, Both startConfirmEmailAction and startConfirmPhoneAction currently
set the token expiry before performing the mutation, which blocks resends even
when the mutation fails; remove the pre-mutation
setConfirmEmailTokenExpiredDate/setConfirmPhoneTokenExpiredDate calls and
instead set the expiry only on success (after you obtain token from
res?.data?.result?.token, e.g. inside the if (token) block) so failures do not
start the cooldown; keep the initial shouldWait check, captcha execution
(executeCaptcha), and mutations
(startConfirmEmailActionMutation/startConfirmPhoneActionMutation) unchanged
otherwise.
| "pages.condo.profile.2FA.title": "Двухфакторная авторизация", | ||
| "pages.condo.profile.2FA.tooltip.emptyFactors": "Чтобы мы могли присылать вам код, укажите {factors}. Для этого во вкладке «Информация» нажмите «Редактировать» и добавьте недостающие данные.", | ||
| "pages.condo.profile.2FA.tooltip.notVerifiedEmail": "Чтобы мы могли присылать вам код, подтвердите ваш email. Для этого проверьте почту и прочитайте письмо от {companyName}", | ||
| "pages.condo.profile.2FA.tooltip.notVerifiedPhone": "Чтобы мы могли присылать вам код, подтвердите ваш номер телефон", |
There was a problem hiding this comment.
Опечатка: «номер телефон» → «номер телефона».
✏️ Proposed fix
-"pages.condo.profile.2FA.tooltip.notVerifiedPhone": "Чтобы мы могли присылать вам код, подтвердите ваш номер телефон",
+"pages.condo.profile.2FA.tooltip.notVerifiedPhone": "Чтобы мы могли присылать вам код, подтвердите ваш номер телефона",🤖 Prompt for AI Agents
In `@apps/condo/lang/ru/ru.json` around lines 2454 - 2457, Update the Russian
translation value for the key pages.condo.profile.2FA.tooltip.notVerifiedPhone
to correct the typo: replace "Чтобы мы могли присылать вам код, подтвердите ваш
номер телефон" with "Чтобы мы могли присылать вам код, подтвердите ваш номер
телефона" so the phrase uses the correct genitive form; leave the rest of the
JSON key/value intact.
…tartConfirmEmailAction. Some fixes
…d with enabled 2FA
Alllex202
force-pushed
the
feat/condo/DOMA-12564/2fa-frontend
branch
from
34ee8ba to
a0cb6ff
Compare
|
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/condo/domains/user/schema/AuthenticateUserWithPhoneAndPasswordService.test.js (1)
31-38:⚠️ Potential issue | 🟡 MinorPre-existing: mutation fragment doesn't select
token, but line 243 asserts on it.The
AUTHENTICATE_WITH_PHONE_AND_PASSWORD_MUTATION_WITH_TOKENGQL query only selectsitem { id }— it never requeststoken. Line 243 then assertsresult.data.result.tokenis not null, which would always beundefinedunless the server returns it implicitly. This isn't part of your change, but worth noting since the test name explicitly says "return token."apps/condo/domains/user/schema/ConfirmEmailActionService.js (2)
269-283:⚠️ Potential issue | 🟡 MinorNew error types missing from
doc.errorsforstartConfirmEmailAction.
EMAIL_AND_USER_ID_IS_MISSINGandSHOULD_BE_ONE_IDENTIFIER_ONLYare thrown by the resolver (lines 303-308) but not listed in thedoc.errorsblock. They won't appear in GraphiQL documentation.Proposed fix
doc: { summary: 'Send confirmation email and return confirmation token...', errors: { ...pick(ERRORS, [ DV_VERSION_MISMATCH, 'WRONG_SENDER_FORMAT', CAPTCHA_CHECK_FAILED, UNABLE_TO_FIND_CONFIRM_EMAIL_ACTION, 'WRONG_EMAIL_FORMAT', 'GENERATE_TOKEN_ERROR', + 'EMAIL_AND_USER_ID_IS_MISSING', + 'SHOULD_BE_ONE_IDENTIFIER_ONLY', ]), }, },
319-333:⚠️ Potential issue | 🟡 MinorWhen user has no email, the error message is misleading.
If
User.getOnefinds a user butuser.emailisnull,null(line 323), andnormalizeEmail(null)returns falsy, throwingWRONG_EMAIL_FORMAT. The error says "Wrong format of provided email" — confusing when the real issue is the user has no email on file. Consider adding an explicit check or a dedicated error type for this case.Additionally, on line 333,
checkEmailSendingLimitsreceives the raw (un-normalized)emailFromInputpath, different casings of the same address could bypass rate limits. Consider usingnormalizedEmailinstead:+ const normalizedEmail = normalizeEmail(email) + if (!normalizedEmail) { + throw new GQLError(ERRORS.WRONG_EMAIL_FORMAT, context) + } - const normalizedEmail = normalizeEmail(email) - if (!normalizedEmail) { - throw new GQLError(ERRORS.WRONG_EMAIL_FORMAT, context) - } - - await checkEmailSendingLimits(context, email, ip) + await checkEmailSendingLimits(context, normalizedEmail, ip)apps/condo/domains/user/schema/ConfirmPhoneActionService.js (1)
217-231:⚠️ Potential issue | 🟡 MinorNew error types missing from
doc.errorsforstartConfirmPhoneAction.Same issue as
ConfirmEmailActionService.js—PHONE_AND_USER_ID_IS_MISSINGandSHOULD_BE_ONE_IDENTIFIER_ONLYare thrown by the resolver but not listed indoc.errors.Proposed fix
doc: { summary: 'Send confirmation phone SMS message...', errors: { ...pick(ERRORS, [ CAPTCHA_CHECK_FAILED, WRONG_PHONE_FORMAT, + 'PHONE_AND_USER_ID_IS_MISSING', + 'SHOULD_BE_ONE_IDENTIFIER_ONLY', ]), TOO_MANY_REQUESTS: GQL_ERRORS.TOO_MANY_REQUESTS,
🤖 Fix all issues with AI agents
In `@apps/condo/domains/common/constants/regexps.js`:
- Line 20: The shared constant NOT_NUMBER_REGEX uses the global flag and is
stateful (lastIndex), so replace the constant with a factory function (e.g.,
getNotNumberRegex or createNotNumberRegex) that returns a fresh RegExp(/\D/g)
each call, and update all call sites to use that factory (or call the factory
only where .test()/.exec() might be used) so no consumer shares a mutated regex
instance; alternatively add a clear comment on NOT_NUMBER_REGEX’s intended use
and change callers that use .test()/.exec() to instantiate a new RegExp(/\D/g)
instead of reusing the constant.
In `@apps/condo/domains/common/utils/helpers.ts`:
- Around line 28-36: The formatPhone function's signature declares a string
return but can return undefined via the final "return phone" path when phone?:
string is undefined (especially when isMaskedPhone is true and normalizePhone is
bypassed); update the function return type from string to string | undefined on
the formatPhone declaration and ensure any downstream usages that assume a
non-null string handle the possible undefined accordingly; locate the
formatPhone function and adjust its signature and any related type expectations
(references: formatPhone, normalizePhone, isMaskedPhone).
In `@apps/condo/domains/user/components/auth/RegisterForm.tsx`:
- Around line 211-231: The SecondFactorHeader useCallback currently lists only
[onReset] but captures SecondFactorForAuthorizationTitle; update the dependency
array for SecondFactorHeader to include SecondFactorForAuthorizationTitle so the
callback re-creates when the title changes (i.e., use [onReset,
SecondFactorForAuthorizationTitle]); locate the useCallback declaration named
SecondFactorHeader in RegisterForm.tsx and add SecondFactorForAuthorizationTitle
to its dependency list.
In `@apps/condo/domains/user/components/auth/SecondFactorForm.tsx`:
- Around line 87-95: The empty onError handlers for
useCompleteConfirmEmailActionMutation and useCompleteConfirmPhoneActionMutation
silently swallow network errors; update these mutations
(completeConfirmEmailMutation and completeConfirmPhoneMutation) to handle the
error argument and surface it (e.g., call confirmActionErrorHandler(error) or
log and show a generic notification/toast) instead of using onError: () => {},
and ensure startConfirmPhoneActionMutation continues to use
confirmActionErrorHandler for consistent user feedback.
- Line 268: The Row component's style prop is receiving a boolean false when
currentSecondFactor is truthy because the expression "!currentSecondFactor && {
display: 'none' }" returns false; change this to an explicit ternary so style
always receives either an object or undefined (for example using
"currentSecondFactor ? undefined : { display: 'none' }" or equivalent) to avoid
passing boolean to the style prop on the Row component in SecondFactorForm.
In `@apps/condo/domains/user/components/UserSettingsContent.tsx`:
- Around line 264-267: handle2FAEnabledChange currently awaits change2FA without
error handling and only updates is2FAEnabled on success, causing unhandled
rejections and a stuck UI on failure; wrap the async body of
handle2FAEnabledChange in try/catch, call setIs2FAEnabled(checked)
optimistically before awaiting or keep as-is but on catch reset
setIs2FAEnabled(prevValue) (or set to !checked) to revert the Switch, and
surface the error via your app’s notification API (e.g.,
message.error/toast/error notification) so failures are handled and the UI is
reverted and user-informed; update references to change2FA,
handle2FAEnabledChange, and setIs2FAEnabled accordingly.
In
`@apps/condo/domains/user/schema/AuthenticateUserWithPhoneAndPasswordService.js`:
- Around line 149-161: The confirm-action updates (ConfirmEmailAction.update and
ConfirmPhoneAction.update) are applied before calling startAuthedSession, which
can consume one-time tokens if session creation fails; move the completion
updates to after startAuthedSession or wrap both the session creation and the
ConfirmEmailAction/ConfirmPhoneAction.update calls in a single atomic
operation/transaction so the actions are only marked completed when
startAuthedSession succeeds—locate the block around startAuthedSession and the
two Confirm*Action.update calls and either reorder to call startAuthedSession
first and then update the actions, or perform them inside a
transaction/try-commit so failures roll back the action completions.
- Around line 131-147: The branch currently treats NOT_ENOUGH_AUTH_FACTORS as
WRONG_CREDENTIALS when validation._error.is2FAEnabled is false; change the logic
in AuthenticateUserWithPhoneAndPasswordService so that whenever
validation._error.errorType === ERROR_TYPES.NOT_ENOUGH_AUTH_FACTORS you throw a
GQLError reflecting that specific failure instead of falling through to
ERRORS.WRONG_CREDENTIALS — if is2FAEnabled is true keep the existing authDetails
payload, and if false throw a GQLError using a clear error constant/payload
(e.g., a dedicated ERRORS.NOT_ENOUGH_AUTH_FACTORS_NO_2FA or the same
NOT_ENOUGH_AUTH_FACTORS with is2FAEnabled:false) so callers/tests can
distinguish “no auth factors provided” from “wrong credentials.”
In `@apps/condo/pages/auth/change-password.tsx`:
- Around line 554-558: Replace the usage of the children prop on the Button with
JSX children: remove the children prop from the Button element and place the
combined label (the template combining SaveMsg and AndSignInMessage) between the
Button's opening and closing tags; keep the other props (loading={isLoading},
htmlType='submit', block) unchanged so Button renders the same text via its
inner JSX rather than a children prop.
- Around line 393-423: The NOT_ENOUGH_AUTH_FACTORS branch is unreachable because
it checks error.originalErrors.graphQLErrors, but Apollo provides
error.graphQLErrors and originalErrors (when present) is a flat GraphQLError[];
update the detection to inspect both shapes: check
error?.graphQLErrors?.some(isNotEnoughAuthFactors) and, if not present, check
Array.isArray(error?.originalErrors) &&
error.originalErrors.some(isNotEnoughAuthFactors). Use the same
isNotEnoughAuthFactors predicate, then find the matching GraphQLError from
whichever array matched, extract extensions.authDetails and proceed with the
existing logic that sets userIdRef, setUserMaskedData,
setAvailableSecondFactors, setCurrentSecondFactor, and the Router.push/return
path; also keep the form.resetFields(['confirmPhoneCode','confirmEmailCode'])
when step === 'changePassword'. Ensure you remove the incorrect property access
and the `@ts-ignore` usages while handling both possible error shapes.
- Around line 296-330: The error handler must treat NOT_ENOUGH_AUTH_FACTORS
raised by authWithIdentifierAndPassword the same when the user is
re-authenticating after a password change (step may be 'changePassword' due to
stale closure); update the handler to detect NOT_ENOUGH_AUTH_FACTORS and either
check if step === 'getSudoTokenBeforeChangePassword' || step ===
'changePassword' or read a live ref (e.g., stepRef.current) instead of the stale
state, then populate authFactors/authFactorsRef (using currentSecondFactor,
confirmEmailToken, confirmPhoneToken), set available second factors
(setAvailableSecondFactors), and transition into the 2FA flow
(setCurrentSecondFactor / setStep to 'authentication' or the existing 2FA step)
so users are prompted for their second factor instead of being redirected to
sign-in.
In `@apps/condo/schema.graphql`:
- Around line 102203-102208: Update the documented error payloads in
schema.graphql so messageForUser shows the i18n keys returned at runtime (not
the hardcoded English text): replace "Not enough auth factors" with
"api.user.NOT_ENOUGH_AUTH_FACTORS" for the NOT_ENOUGH_AUTH_FACTORS entries used
by the authenticateUserWithEmailAndPassword,
authenticateUserWithPhoneAndPassword, generateSudoToken, and
authenticateOrRegisterUserWithToken mutation docs (match the same messageForUser
values defined in the service constants).
🧹 Nitpick comments (19)
apps/condo/domains/user/schema/AuthenticateUserWithEmailAndPasswordService.test.js (1)
183-209:availableSecondFactorsassertion is order-dependent — considerexpect.arrayContaining.Line 205: The exact array
['confirmEmailToken', 'confirmPhoneToken']will fail if the server returns factors in a different order. Usingexpect.arrayContainingmakes this resilient to ordering changes while still validating the content.Proposed fix
authDetails: expect.objectContaining({ is2FAEnabled: true, userId: user.id, - availableSecondFactors: ['confirmEmailToken', 'confirmPhoneToken'], + availableSecondFactors: expect.arrayContaining(['confirmEmailToken', 'confirmPhoneToken']), maskedData: { email: maskNormalizedEmail(userAttrs.email), phone: maskNormalizedPhone(userAttrs.phone) }, }),apps/condo/domains/user/schema/AuthenticateUserWithPhoneAndPasswordService.test.js (1)
148-174:availableSecondFactorsassertion relies on exact array order.Line 170 asserts
['confirmEmailToken', 'confirmPhoneToken']with strict equality (viaobjectContaining). If the server ever returns these in a different order, the test will break despite correct behavior. Consider usingexpect.arrayContainingcombined with a length check for a more resilient assertion.- availableSecondFactors: ['confirmEmailToken', 'confirmPhoneToken'], + availableSecondFactors: expect.arrayContaining(['confirmEmailToken', 'confirmPhoneToken']),(And optionally assert
.toHaveLength(2)on the array separately if you want to ensure no extra factors.)apps/condo/domains/common/utils/phone.spec.js (1)
78-96: Well-structured test cases for the new masking utility.The coverage is good across varying lengths. Consider adding a case for
undefined/nullinput to document the expected behavior when no phone is provided, similar to hownormalizePhonetests handle the "no data" scenario.apps/condo/domains/user/schema/AuthenticateUserWithPhoneAndPasswordService.js (2)
13-15: Mixing relative and absolute@condo/imports for modules in the same domain.Lines 6–11 import from
@condo/domains/user/…while lines 13–15 import from../constants/…and../utils/…. Both resolve to the same domain. Prefer one style consistently — the rest of this file (and the codebase) uses@condo/…aliases.Proposed fix
-const { AUTH_FACTOR_TYPES } = require('../constants/authFactors') -const { NOT_ENOUGH_AUTH_FACTORS } = require('../constants/errors') -const { ConfirmPhoneAction, ConfirmEmailAction } = require('../utils/serverSchema') +const { AUTH_FACTOR_TYPES } = require('@condo/domains/user/constants/authFactors') +const { NOT_ENOUGH_AUTH_FACTORS } = require('@condo/domains/user/constants/errors') +const { ConfirmPhoneAction, ConfirmEmailAction } = require('@condo/domains/user/utils/serverSchema')
122-129: Spreadingnullis valid but less obvious than spreading an empty object.
...(condition ? { key: val } : null)works because...nullis a no-op, but...{}communicates the "no properties" intent more clearly and avoids surprising readers.Proposed fix
const validation = await validateUserCredentials( { phone, userType }, { password, - ...(secondFactor?.type === AUTH_FACTOR_TYPES.CONFIRM_EMAIL_TOKEN ? { confirmEmailToken: secondFactor?.value || '' } : null), - ...(secondFactor?.type === AUTH_FACTOR_TYPES.CONFIRM_PHONE_TOKEN ? { confirmPhoneToken: secondFactor?.value || '' } : null), + ...(secondFactor?.type === AUTH_FACTOR_TYPES.CONFIRM_EMAIL_TOKEN ? { confirmEmailToken: secondFactor?.value || '' } : {}), + ...(secondFactor?.type === AUTH_FACTOR_TYPES.CONFIRM_PHONE_TOKEN ? { confirmPhoneToken: secondFactor?.value || '' } : {}), } )apps/condo/domains/user/components/auth/SecondFactorForm.tsx (2)
31-37: Hook parameters lack TypeScript typing.The hook accepts
form,onSubmit,Header,isLoading, andsetIsLoadingwithout any type annotations. This loses type safety on a public API that's consumed bySignInForm,RegisterForm, and the change-password page.Suggested type definition
+type UseSecondFactorParams = { + form: FormInstance + onSubmit: (args: { ... }) => Promise<void> + Header?: React.ComponentType<{ setCurrentSecondFactor: ..., setAvailableSecondFactors: ... }> + isLoading: boolean + setIsLoading: (v: boolean) => void +} + -export const useSecondFactor = ({ - form, - onSubmit, - Header, - isLoading, - setIsLoading, -}) => { +export const useSecondFactor = ({ + form, + onSubmit, + Header, + isLoading, + setIsLoading, +}: UseSecondFactorParams) => {
254-265: MissingstartConfirmPhoneActionandstartConfirmEmailActionin useEffect deps.These callbacks are omitted from the dependency array, likely to avoid re-triggering on every callback recreation. However, this means the effect can capture stale closures (e.g., an outdated
shouldWaitcheck). Consider using refs for the callbacks or adding them to the deps if the callbacks are properly memoized.apps/condo/domains/user/components/SudoTokenProvider.tsx (3)
567-571: Fragile manual string interpolation for i18n descriptions.
ConfirmPhoneTokenDescription.replace('{phone}', ...)andConfirmEmailTokenDescription.replace('{email}', ...)bypassintl.formatMessage's built-in interpolation. If the translation ever renames the placeholder (e.g.{phoneNumber}), this breaks silently.Instead, call
intl.formatMessageat the point of use with the value:Proposed fix (phone example)
- <FormItem - required={false} - name='confirmPhoneCode' - label={ConfirmPhoneTokenDescription.replace('{phone}', formatPhone(userMaskedData?.phone || '', true))} - > + <FormItem + required={false} + name='confirmPhoneCode' + label={intl.formatMessage( + { id: 'component.SecondFactorForm.type.confirmPhoneToken.description' }, + { phone: formatPhone(userMaskedData?.phone || '', true) }, + )} + >Apply the same pattern for
confirmEmailCode.Also applies to: 592-598
108-108: Misleading state setter namesetConfirmActionTypeforcurrentSecondFactor.The destructured setter
setConfirmActionTypedoesn't match the state variablecurrentSecondFactor, making the code harder to follow. Consider renaming for clarity:- const [currentSecondFactor, setConfirmActionType] = useState<'confirmPhoneToken' | 'confirmEmailToken'>(null) + const [currentSecondFactor, setCurrentSecondFactor] = useState<'confirmPhoneToken' | 'confirmEmailToken'>(null)This requires updating all usages of
setConfirmActionTypethroughout the file.
291-293: Early return onres?.errors?.length > 0masks the 2FA flow.After the NOT_ENOUGH_AUTH_FACTORS block (lines 265-285), execution falls through to line 291 which checks
res?.errors?.length > 0and returns early. This means even if the 2FA block correctly fires (once thegraphQLErrorsbug is fixed), the function also exits here. This is likely intentional (the 2FA block sets state and shouldn't resolve the promise), but the flow depends on the 2FA block not having set the token — which it doesn't. Worth a brief comment clarifying the intent.apps/condo/domains/user/components/auth/SignInForm.tsx (2)
107-117:handleSubmitparameter object is untyped.The destructured parameter object has no type annotation, defeating TypeScript's type-safety for this callback's contract with the
useSecondFactorhook. If the hook changes the shape of the object it passes, this will silently break.Consider defining a type for the parameter:
Suggested approach
+type HandleSubmitParams = { + isLoading: boolean + setIsLoading: (v: boolean) => void + userIdRef: React.MutableRefObject<string | null> + setUserMaskedData: (data: { email?: string, phone?: string } | null) => void + setAvailableSecondFactors: (factors: Array<'confirmPhoneToken' | 'confirmEmailToken'>) => void + setCurrentSecondFactor: (factor: 'confirmPhoneToken' | 'confirmEmailToken' | null) => void + currentSecondFactor: 'confirmPhoneToken' | 'confirmEmailToken' | null + confirmEmailToken: string | null + confirmPhoneToken: string | null +} - const handleSubmit = useCallback(async ({ - isLoading, - setIsLoading, - userIdRef, - setUserMaskedData, - setAvailableSecondFactors, - setCurrentSecondFactor, - currentSecondFactor, - confirmEmailToken, - confirmPhoneToken, - }): Promise<void> => { + const handleSubmit = useCallback(async ({ + isLoading, + setIsLoading, + userIdRef, + setUserMaskedData, + setAvailableSecondFactors, + setCurrentSecondFactor, + currentSecondFactor, + confirmEmailToken, + confirmPhoneToken, + }: HandleSubmitParams): Promise<void> => {Ideally this type should be exported from the
useSecondFactorhook or shared module so both sides stay in sync. Based on learnings, prefer using type aliases (type X = { ... }) over interface declarations for object shapes.
269-270: Unusual inline style pattern:style={!!currentSecondFactor && { display: 'none' }}When
currentSecondFactoris falsy, this evaluates tostyle={false}, which React ignores (it works). However,style={undefined}is the conventional way to express "no style". Consider:- <Row style={!!currentSecondFactor && { display: 'none' }}> + <Row style={currentSecondFactor ? { display: 'none' } : undefined}>apps/condo/gql/index.ts (1)
8692-8724: Align codegen output with repo JS/TS formatting rules.This generated block still uses semicolons and 8‑space indentation. Since this file is auto-generated, consider updating the GraphQL codegen config (or excluding this file from lint) so the output matches repo formatting and avoids future lint churn.
As per coding guidelines: "Use 4 spaces for indentation in JavaScript/TypeScript" and "Never use semicolons".
apps/condo/domains/user/components/auth/RegisterForm.tsx (1)
271-271: Assigningundefinedto a typed variable bypasses type safety.
AuthenticateOrRegisterUserWithTokenSecondFactorInputlikely expects an object withvalueandtypefields, notundefined. Usenullor declare the variable as... | undefinedexplicitly to make intent clear.🔧 Proposed fix
- let secondFactor: AuthenticateOrRegisterUserWithTokenSecondFactorInput = undefined + let secondFactor: AuthenticateOrRegisterUserWithTokenSecondFactorInput | undefined = undefinedapps/condo/domains/user/components/UserSettingsContent.tsx (1)
70-90:EmptyFactorsTooltipMessagelogic may show misleading messages.When
user.emailexists butisEmailVerifiedis false, the tooltip says the email is not verified. But it doesn't check whether the phone is also unverified — if both are unverified, only the email message is shown, potentially hiding the phone verification issue. Consider whether both conditions should be surfaced.apps/condo/pages/auth/change-password.tsx (1)
530-530:styleprop receivesfalsewhen condition is falsy.
(currentSecondFactor || step !== 'changePassword') && { display: 'none' }evaluates tofalsewhen the left side is falsy. While React ignoresfalsefor style, this is type-unsafe and may confuse TS/readers. Consider a ternary orundefined.🔧 Proposed fix
- <Row justify='center' style={(currentSecondFactor || step !== 'changePassword') && { display: 'none' }}> + <Row justify='center' style={(currentSecondFactor || step !== 'changePassword') ? { display: 'none' } : undefined}>apps/condo/domains/user/schema/AuthenticateOrRegisterUserWithTokenService.js (2)
454-466:authDetailsexposure in error extensions is intentional but worth noting.The error response leaks
userId,availableSecondFactors, andmaskedDatato the client. This is necessary for the frontend 2FA flow but means anyone with a valid first factor (confirmed phone/email token) can enumerate user IDs and available 2FA methods. Since the first factor already requires CAPTCHA + token verification, the risk is limited — just flagging for awareness.
273-280: GraphQL enum uses camelCase values — unconventional but functional.The enum
AuthenticateOrRegisterUserWithTokenSecondFactorTypegenerates values likeconfirmPhoneToken,confirmEmailToken,password(camelCase). GraphQL convention isUPPER_CASEfor enum values (e.g.,CONFIRM_PHONE_TOKEN). This works as long as the generated TypeScript types align, but it may surprise developers familiar with GraphQL conventions.apps/condo/schema.graphql (1)
90057-90060: Consider consolidating the three near-identical second-factor enum types.
AuthenticateUserWithPhoneAndPasswordSecondFactorType,AuthenticateOrRegisterUserWithTokenSecondFactorType, andAuthenticateUserWithEmailAndPasswordSecondFactorTypeshareconfirmEmailTokenandconfirmPhoneToken. Only the token-based variant addspassword. A single shared enum (e.g.,SecondFactorType) would reduce duplication, though separate types do allow independent evolution per auth flow.Also applies to: 90282-90286, 90361-90364
| const URL_REGEX = new RegExp('(https?:\\/\\/(?:www\\.|(?!www))[a-zA-Z0-9][a-zA-Z0-9-]+[a-zA-Z0-9]\\.[^\\s]{2,}|www\\.[a-zA-Z0-9][a-zA-Z0-9-]+[a-zA-Z0-9]\\.[^\\s]{2,}|https?:\\/\\/(?:www\\.|(?!www))[a-zA-Z0-9]+\\.[^\\s]{2,}|www\\.[a-zA-Z0-9]+\\.[^\\s]{2,})', 'gm') | ||
| const URL_WITH_CYRILLIC_REGEX = new RegExp('(https?:\\/\\/(?:www\\.|(?!www))[a-zа-я0-9][a-zа-я0-9-]+[a-zа-я0-9]\\.[^\\s]{2,}|www\\.[a-zа-я0-9][a-zа-я0-9-]+[a-zа-я0-9]\\.[^\\s]{2,}|https?:\\/\\/(?:www\\.|(?!www))[a-zа-я0-9]+\\.[^\\s]{2,}|www\\.[a-zа-я0-9]+\\.[^\\s]{2,})', 'gmi') | ||
| const IPv4_REGEX = /^((25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.){3}(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)$/ | ||
| const NOT_NUMBER_REGEX = /\D/g |
There was a problem hiding this comment.
Shared regex with global flag is stateful — safe for .replace() but risky for .test()/.exec().
/\D/g carries mutable lastIndex state. Current usage via String.prototype.replace is fine because replace resets lastIndex internally. However, if a future consumer calls .test() or .exec() on this shared constant, it will produce intermittent incorrect results due to lastIndex not resetting between calls.
Consider documenting the intended usage or dropping the g flag if it's only consumed by .replace() (which applies globally by default when the pattern is a regex with g). Since .replace() without g only replaces the first match, the g flag is actually required here — just be aware of the pitfall for other callers.
🤖 Prompt for AI Agents
In `@apps/condo/domains/common/constants/regexps.js` at line 20, The shared
constant NOT_NUMBER_REGEX uses the global flag and is stateful (lastIndex), so
replace the constant with a factory function (e.g., getNotNumberRegex or
createNotNumberRegex) that returns a fresh RegExp(/\D/g) each call, and update
all call sites to use that factory (or call the factory only where
.test()/.exec() might be used) so no consumer shares a mutated regex instance;
alternatively add a clear comment on NOT_NUMBER_REGEX’s intended use and change
callers that use .test()/.exec() to instantiate a new RegExp(/\D/g) instead of
reusing the constant.
| export const formatPhone = (phone?: string, isMaskedPhone: boolean = false): string => { | ||
| const normalizedPhone = isMaskedPhone ? phone : normalizePhone(phone, true) | ||
| if (normalizedPhone?.startsWith('+7')){ | ||
| return normalizedPhone.replace(RUSSIAN_PHONE_FORMAT_REGEXP, '$1 ($2) $3-$4-$5') | ||
| return normalizedPhone.replace(isMaskedPhone ? RUSSIAN_MASKED_PHONE_FORMAT_REGEXP : RUSSIAN_PHONE_FORMAT_REGEXP, '$1 ($2) $3-$4-$5') | ||
| } | ||
| if (normalizedPhone?.startsWith('+34')){ | ||
| return normalizedPhone.replace(SPANISH_PHONE_FORMAT_REGEXP, '$1-$2-$3-$4') | ||
| return normalizedPhone.replace(isMaskedPhone ? SPANISH_MASKED_PHONE_FORMAT_REGEXP : SPANISH_PHONE_FORMAT_REGEXP, '$1-$2-$3-$4') | ||
| } | ||
| return phone |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/condo/domains/common/utils/helpers.ts | head -60Repository: open-condo-software/condo
Length of output: 2601
🏁 Script executed:
rg -n 'formatPhone\s*\(' --type=ts --type=tsx -g '!*.spec.*' -g '!*.test.*' -A2 -B2Repository: open-condo-software/condo
Length of output: 96
🏁 Script executed:
rg -n 'formatPhone\s*\(' -A2 -B2 --type-not=test --type-not=spec -g '!*.spec.ts' -g '!*.test.ts'Repository: open-condo-software/condo
Length of output: 97
🏁 Script executed:
rg -n 'formatPhone\s*\(' -A2 -B2 -g '!*.spec.ts' -g '!*.test.ts' -g '!*.spec.js' -g '!*.test.js'Repository: open-condo-software/condo
Length of output: 7142
🏁 Script executed:
rg -n 'userMaskedData' -B5 -A5 'SecondFactorForm.tsx'Repository: open-condo-software/condo
Length of output: 129
🏁 Script executed:
fd -t f 'SecondFactorForm.tsx'Repository: open-condo-software/condo
Length of output: 129
🏁 Script executed:
rg -n 'userMaskedData' apps/condo/domains/user/components/auth/SecondFactorForm.tsx -B5 -A5Repository: open-condo-software/condo
Length of output: 2347
🏁 Script executed:
rg -n 'setUserMaskedData' apps/condo/domains/user/components/auth/SecondFactorForm.tsx -B10 -A10Repository: open-condo-software/condo
Length of output: 2213
🏁 Script executed:
rg -n 'setUserMaskedData\(' apps/condo/domains/user/components/auth/SecondFactorForm.tsx -A10 | head -40Repository: open-condo-software/condo
Length of output: 51
🏁 Script executed:
rg -n 'const onSubmit' apps/condo/domains/user/components/auth/SecondFactorForm.tsx -B5 -A30 | head -60Repository: open-condo-software/condo
Length of output: 1464
🏁 Script executed:
rg -n 'interface.*SecondFactorFormProps\|type.*SecondFactorFormProps' apps/condo/domains/user/components/auth/SecondFactorForm.tsx -A20Repository: open-condo-software/condo
Length of output: 51
🏁 Script executed:
head -50 apps/condo/domains/user/components/auth/SecondFactorForm.tsxRepository: open-condo-software/condo
Length of output: 2942
🏁 Script executed:
fd -t f 'SecondFactorForm.ts' apps/condo/domains/userRepository: open-condo-software/condo
Length of output: 129
🏁 Script executed:
rg -n 'setUserMaskedData.*phone' apps/condo/domains/user -B5 -A5Repository: open-condo-software/condo
Length of output: 2698
🏁 Script executed:
rg -n 'setUserMaskedData\(\{' apps/condo/domains/user -B10 -A5Repository: open-condo-software/condo
Length of output: 51
🏁 Script executed:
rg -n 'setUserMaskedData' apps/condo/domains/user -A2 -B2Repository: open-condo-software/condo
Length of output: 4491
Fix return type annotation to reflect that undefined can be returned.
The function signature declares string as the return type, but line 36 returns phone which can be undefined. When isMaskedPhone is true, the input bypasses normalizePhone, so any phone string that doesn't start with +7 or +34 falls through to return phone and may return undefined if the input was undefined. Update the return type to string | undefined to accurately reflect this behavior.
The concern about whether callers pass pre-normalized masked phones when isMaskedPhone=true is valid—callers found in SecondFactorForm.tsx and SudoTokenProvider.tsx do pass data from authDetails?.maskedData which is already masked/normalized, mitigating the risk in practice.
🤖 Prompt for AI Agents
In `@apps/condo/domains/common/utils/helpers.ts` around lines 28 - 36, The
formatPhone function's signature declares a string return but can return
undefined via the final "return phone" path when phone?: string is undefined
(especially when isMaskedPhone is true and normalizePhone is bypassed); update
the function return type from string to string | undefined on the formatPhone
declaration and ensure any downstream usages that assume a non-null string
handle the possible undefined accordingly; locate the formatPhone function and
adjust its signature and any related type expectations (references: formatPhone,
normalizePhone, isMaskedPhone).
| const SecondFactorHeader = useCallback(({ setCurrentSecondFactor, setAvailableSecondFactors }) => ( | ||
| <Col span={24}> | ||
| <Row gutter={[0, 16]}> | ||
| <Col span={24}> | ||
| <Space direction='vertical' size={24}> | ||
| <Button.Icon | ||
| onClick={() => { | ||
| setCurrentSecondFactor(null) | ||
| setAvailableSecondFactors([]) | ||
| onReset() | ||
| }} | ||
| size='small' | ||
| > | ||
| <ArrowLeft /> | ||
| </Button.Icon> | ||
| <Typography.Title level={2}>{SecondFactorForAuthorizationTitle}</Typography.Title> | ||
| </Space> | ||
| </Col> | ||
| </Row> | ||
| </Col> | ||
| ), [onReset]) |
There was a problem hiding this comment.
SecondFactorForAuthorizationTitle is missing from useCallback dependency array.
SecondFactorHeader captures SecondFactorForAuthorizationTitle (used at line 226) but the dependency array at line 231 only lists [onReset]. If the intl locale changes, the title won't update.
🔧 Proposed fix
- }, [onReset])
+ }, [onReset, SecondFactorForAuthorizationTitle])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const SecondFactorHeader = useCallback(({ setCurrentSecondFactor, setAvailableSecondFactors }) => ( | |
| <Col span={24}> | |
| <Row gutter={[0, 16]}> | |
| <Col span={24}> | |
| <Space direction='vertical' size={24}> | |
| <Button.Icon | |
| onClick={() => { | |
| setCurrentSecondFactor(null) | |
| setAvailableSecondFactors([]) | |
| onReset() | |
| }} | |
| size='small' | |
| > | |
| <ArrowLeft /> | |
| </Button.Icon> | |
| <Typography.Title level={2}>{SecondFactorForAuthorizationTitle}</Typography.Title> | |
| </Space> | |
| </Col> | |
| </Row> | |
| </Col> | |
| ), [onReset]) | |
| const SecondFactorHeader = useCallback(({ setCurrentSecondFactor, setAvailableSecondFactors }) => ( | |
| <Col span={24}> | |
| <Row gutter={[0, 16]}> | |
| <Col span={24}> | |
| <Space direction='vertical' size={24}> | |
| <Button.Icon | |
| onClick={() => { | |
| setCurrentSecondFactor(null) | |
| setAvailableSecondFactors([]) | |
| onReset() | |
| }} | |
| size='small' | |
| > | |
| <ArrowLeft /> | |
| </Button.Icon> | |
| <Typography.Title level={2}>{SecondFactorForAuthorizationTitle}</Typography.Title> | |
| </Space> | |
| </Col> | |
| </Row> | |
| </Col> | |
| ), [onReset, SecondFactorForAuthorizationTitle]) |
🤖 Prompt for AI Agents
In `@apps/condo/domains/user/components/auth/RegisterForm.tsx` around lines 211 -
231, The SecondFactorHeader useCallback currently lists only [onReset] but
captures SecondFactorForAuthorizationTitle; update the dependency array for
SecondFactorHeader to include SecondFactorForAuthorizationTitle so the callback
re-creates when the title changes (i.e., use [onReset,
SecondFactorForAuthorizationTitle]); locate the useCallback declaration named
SecondFactorHeader in RegisterForm.tsx and add SecondFactorForAuthorizationTitle
to its dependency list.
| const [completeConfirmEmailMutation] = useCompleteConfirmEmailActionMutation({ | ||
| onError: () => {}, | ||
| }) | ||
| const [startConfirmPhoneActionMutation] = useStartConfirmPhoneActionMutation({ | ||
| onError: confirmActionErrorHandler, | ||
| }) | ||
| const [completeConfirmPhoneMutation] = useCompleteConfirmPhoneActionMutation({ | ||
| onError: () => {}, | ||
| }) |
There was a problem hiding this comment.
Silent onError swallows network errors with no user feedback.
The onError: () => {} handlers prevent Apollo from throwing, but network-level errors (as opposed to GraphQL errors in the response) won't appear in res.errors either — they're silently discarded. The user sees no feedback on network failure. Consider at minimum logging or showing a generic error notification.
🤖 Prompt for AI Agents
In `@apps/condo/domains/user/components/auth/SecondFactorForm.tsx` around lines 87
- 95, The empty onError handlers for useCompleteConfirmEmailActionMutation and
useCompleteConfirmPhoneActionMutation silently swallow network errors; update
these mutations (completeConfirmEmailMutation and completeConfirmPhoneMutation)
to handle the error argument and surface it (e.g., call
confirmActionErrorHandler(error) or log and show a generic notification/toast)
instead of using onError: () => {}, and ensure startConfirmPhoneActionMutation
continues to use confirmActionErrorHandler for consistent user feedback.
| }, [currentSecondFactor]) | ||
|
|
||
| const SecondFactorForm = useMemo(() => ( | ||
| <Row justify='center' style={!currentSecondFactor && { display: 'none' }}> |
There was a problem hiding this comment.
style prop receives false when currentSecondFactor is set.
!currentSecondFactor && { display: 'none' } evaluates to false (not undefined) when the factor is set. Use a ternary to be explicit:
- <Row justify='center' style={!currentSecondFactor && { display: 'none' }}>
+ <Row justify='center' style={!currentSecondFactor ? { display: 'none' } : undefined}>🤖 Prompt for AI Agents
In `@apps/condo/domains/user/components/auth/SecondFactorForm.tsx` at line 268,
The Row component's style prop is receiving a boolean false when
currentSecondFactor is truthy because the expression "!currentSecondFactor && {
display: 'none' }" returns false; change this to an explicit ternary so style
always receives either an object or undefined (for example using
"currentSecondFactor ? undefined : { display: 'none' }" or equivalent) to avoid
passing boolean to the style prop on the Row component in SecondFactorForm.
| if (validation.confirmEmailAction) { | ||
| await ConfirmEmailAction.update(context, validation.confirmEmailAction.id, { | ||
| dv, sender, | ||
| completedAt: new Date().toISOString(), | ||
| }) | ||
| } | ||
|
|
||
| if (validation.confirmPhoneAction) { | ||
| await ConfirmPhoneAction.update(context, validation.confirmPhoneAction.id, { | ||
| dv, sender, | ||
| completedAt: new Date().toISOString(), | ||
| }) | ||
| } |
There was a problem hiding this comment.
Confirm-action completion runs before the session is established.
If startAuthedSession (line 164) throws, the ConfirmEmailAction/ConfirmPhoneAction records are already marked completedAt, consuming the one-time tokens. The user would need to re-initiate the 2FA flow. Consider wrapping the session start and the action updates together, or marking actions complete after the session is created, so a transient session failure doesn't burn the tokens.
🤖 Prompt for AI Agents
In
`@apps/condo/domains/user/schema/AuthenticateUserWithPhoneAndPasswordService.js`
around lines 149 - 161, The confirm-action updates (ConfirmEmailAction.update
and ConfirmPhoneAction.update) are applied before calling startAuthedSession,
which can consume one-time tokens if session creation fails; move the completion
updates to after startAuthedSession or wrap both the session creation and the
ConfirmEmailAction/ConfirmPhoneAction.update calls in a single atomic
operation/transaction so the actions are only marked completed when
startAuthedSession succeeds—locate the block around startAuthedSession and the
two Confirm*Action.update calls and either reorder to call startAuthedSession
first and then update the actions, or perform them inside a
transaction/try-commit so failures roll back the action completions.
| if (step === 'getSudoTokenBeforeChangePassword') { | ||
| const tokenTypeInfo = detectTokenTypeSafely(token) | ||
|
|
||
| const status = res?.data?.result?.status | ||
| if (!res.errors && status === 'ok') { | ||
| invalidateSudoToken() | ||
| if (tokenTypeInfo.error) { | ||
| throw tokenTypeInfo.error | ||
| } | ||
| if (!tokenTypeInfo.tokenType) { | ||
| throw new Error('Unknown token type') | ||
| } | ||
| if (!allowedTokenTypes.includes(tokenTypeInfo.tokenType)) { | ||
| throw new Error('Unsupported token type') | ||
| } | ||
|
|
||
| if (!identifier || !identifierType) { | ||
| await Router.push(`/auth/signin?next=${redirectUrl}`) | ||
| const authFactors: { confirmEmailToken?: string, confirmPhoneToken?: string } = {} | ||
| if (tokenTypeInfo.tokenType === TOKEN_TYPES.CONFIRM_EMAIL) { | ||
| authFactors.confirmEmailToken = token | ||
| } | ||
| if (tokenTypeInfo.tokenType === TOKEN_TYPES.CONFIRM_PHONE) { | ||
| authFactors.confirmPhoneToken = token | ||
| } | ||
| if (currentSecondFactor === 'confirmPhoneToken' && tokenTypeInfo.tokenType !== TOKEN_TYPES.CONFIRM_PHONE) { | ||
| authFactors.confirmPhoneToken = confirmPhoneToken | ||
| } | ||
| if (currentSecondFactor === 'confirmEmailToken' && tokenTypeInfo.tokenType !== TOKEN_TYPES.CONFIRM_EMAIL) { | ||
| authFactors.confirmEmailToken = confirmEmailToken | ||
| } | ||
|
|
||
| authFactorsRef.current = authFactors | ||
|
|
||
| await generateSudoToken() | ||
|
|
||
| setCurrentSecondFactor(null) | ||
| setAvailableSecondFactors([]) | ||
| setStep('changePassword') | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Get file size and relevant lines from the change-password.tsx file
wc -l apps/condo/pages/auth/change-password.tsxRepository: open-condo-software/condo
Length of output: 114
🏁 Script executed:
# View the handleSubmit function and surrounding context
sed -n '280,420p' apps/condo/pages/auth/change-password.tsxRepository: open-condo-software/condo
Length of output: 6368
🏁 Script executed:
# Search for where handleSubmit is called and how form submission works
rg -n 'handleSubmit|onFinish' apps/condo/pages/auth/change-password.tsx -B2 -A2Repository: open-condo-software/condo
Length of output: 613
🏁 Script executed:
# Look for error handling blocks and NOT_ENOUGH_AUTH_FACTORS
rg -n 'NOT_ENOUGH_AUTH_FACTORS|catch|error' apps/condo/pages/auth/change-password.tsx -B2 -A2Repository: open-condo-software/condo
Length of output: 4628
Missing 2FA handling in error flow during authentication after password change.
When authWithIdentifierAndPassword throws NOT_ENOUGH_AUTH_FACTORS after setStep('authentication') at line 344, the error handler checks if (step === 'getSudoTokenBeforeChangePassword') but step retains the value 'changePassword' from the closure (state updates are asynchronous). The code then falls through to the else branch and redirects to sign-in, bypassing the 2FA challenge. This means users changing password with 2FA enabled will be incorrectly redirected instead of prompted for their second factor during the re-authentication flow.
Add handling for when NOT_ENOUGH_AUTH_FACTORS occurs during the 'changePassword' step—either by checking step === 'changePassword' in the error handler's 2FA condition, or by restructuring to avoid relying on the stale step value in error closures.
🤖 Prompt for AI Agents
In `@apps/condo/pages/auth/change-password.tsx` around lines 296 - 330, The error
handler must treat NOT_ENOUGH_AUTH_FACTORS raised by
authWithIdentifierAndPassword the same when the user is re-authenticating after
a password change (step may be 'changePassword' due to stale closure); update
the handler to detect NOT_ENOUGH_AUTH_FACTORS and either check if step ===
'getSudoTokenBeforeChangePassword' || step === 'changePassword' or read a live
ref (e.g., stepRef.current) instead of the stale state, then populate
authFactors/authFactorsRef (using currentSecondFactor, confirmEmailToken,
confirmPhoneToken), set available second factors (setAvailableSecondFactors),
and transition into the 2FA flow (setCurrentSecondFactor / setStep to
'authentication' or the existing 2FA step) so users are prompted for their
second factor instead of being redirected to sign-in.
| if (error?.originalErrors) { | ||
| const isNotEnoughAuthFactors = (error: GraphQLFormattedError) => error?.extensions?.type === NOT_ENOUGH_AUTH_FACTORS | ||
| // @ts-ignore | ||
| if (error?.originalErrors?.graphQLErrors?.some(isNotEnoughAuthFactors)) { | ||
| // @ts-ignore | ||
| const graphQLError = error?.originalErrors?.graphQLErrors.find(isNotEnoughAuthFactors) | ||
| const authDetails = graphQLError?.extensions?.authDetails | ||
|
|
||
| if (authDetails?.is2FAEnabled && authDetails?.userId && authDetails?.availableSecondFactors?.length > 0) { | ||
| if (step === 'getSudoTokenBeforeChangePassword') { | ||
| userIdRef.current = authDetails.userId | ||
| const availableSecondFactors = authDetails?.availableSecondFactors?.filter(factor => ['confirmPhoneToken', 'confirmEmailToken'].includes(factor)) || [] | ||
| const prioritySecondFactor = currentSecondFactor || availableSecondFactors?.[0] || null | ||
|
|
||
| if (availableSecondFactors.length > 0) { | ||
| setUserMaskedData(authDetails?.maskedData || null) | ||
| setAvailableSecondFactors(availableSecondFactors) | ||
| setCurrentSecondFactor(prioritySecondFactor) | ||
| } | ||
| } else { | ||
| await Router.push(`/auth/signin?next=${redirectUrl}`) | ||
| return | ||
| } | ||
| } | ||
|
|
||
| if (step === 'changePassword') { | ||
| form.resetFields(['confirmPhoneCode', 'confirmEmailCode']) | ||
| } | ||
|
|
||
| return | ||
| } |
There was a problem hiding this comment.
NOT_ENOUGH_AUTH_FACTORS detection in the catch block is unreachable.
Two compounding issues make the 2FA routing dead code:
-
Wrong error shape: For authentication errors, Apollo throws an
ApolloError(which has.graphQLErrors), not a custom error with.originalErrors. Theerror?.originalErrorscheck at line 393 will beundefinedforApolloErrorinstances. -
Wrong property path: Even for the custom errors created at line 199 where
originalErrors = res.errors, the value is a flatGraphQLError[]array — not an object with a.graphQLErrorssub-property. Soerror?.originalErrors?.graphQLErrors?.some(...)at line 396 will always beundefined.
The fix should check error.graphQLErrors (the standard Apollo error property) directly:
🐛 Proposed fix
} catch (error) {
console.error('Change password failed')
console.error(error)
- if (error?.originalErrors) {
- const isNotEnoughAuthFactors = (error: GraphQLFormattedError) => error?.extensions?.type === NOT_ENOUGH_AUTH_FACTORS
- // `@ts-ignore`
- if (error?.originalErrors?.graphQLErrors?.some(isNotEnoughAuthFactors)) {
- // `@ts-ignore`
- const graphQLError = error?.originalErrors?.graphQLErrors.find(isNotEnoughAuthFactors)
+ const isNotEnoughAuthFactors = (gqlErr: GraphQLFormattedError) => gqlErr?.extensions?.type === NOT_ENOUGH_AUTH_FACTORS
+ if (error?.graphQLErrors?.some(isNotEnoughAuthFactors)) {
+ const graphQLError = error.graphQLErrors.find(isNotEnoughAuthFactors)
const authDetails = graphQLError?.extensions?.authDetails🤖 Prompt for AI Agents
In `@apps/condo/pages/auth/change-password.tsx` around lines 393 - 423, The
NOT_ENOUGH_AUTH_FACTORS branch is unreachable because it checks
error.originalErrors.graphQLErrors, but Apollo provides error.graphQLErrors and
originalErrors (when present) is a flat GraphQLError[]; update the detection to
inspect both shapes: check error?.graphQLErrors?.some(isNotEnoughAuthFactors)
and, if not present, check Array.isArray(error?.originalErrors) &&
error.originalErrors.some(isNotEnoughAuthFactors). Use the same
isNotEnoughAuthFactors predicate, then find the matching GraphQLError from
whichever array matched, extract extensions.authDetails and proceed with the
existing logic that sets userIdRef, setUserMaskedData,
setAvailableSecondFactors, setCurrentSecondFactor, and the Router.push/return
path; also keep the form.resetFields(['confirmPhoneCode','confirmEmailCode'])
when step === 'changePassword'. Ensure you remove the incorrect property access
and the `@ts-ignore` usages while handling both possible error shapes.
| loading={isLoading} | ||
| htmlType='submit' | ||
| children={`${SaveMsg} ${AndSignInMessage}`} | ||
| block | ||
| /> |
There was a problem hiding this comment.
Avoid passing children using a prop.
Biome (lint/correctness/noChildrenProp) flags this. The canonical way to pass children in React is via JSX nesting.
🔧 Proposed fix
<Button
key='submit'
type='primary'
loading={isLoading}
htmlType='submit'
- children={`${SaveMsg} ${AndSignInMessage}`}
block
- />
+ >
+ {`${SaveMsg} ${AndSignInMessage}`}
+ </Button>🧰 Tools
🪛 Biome (2.3.13)
[error] 556-556: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
🤖 Prompt for AI Agents
In `@apps/condo/pages/auth/change-password.tsx` around lines 554 - 558, Replace
the usage of the children prop on the Button with JSX children: remove the
children prop from the Button element and place the combined label (the template
combining SaveMsg and AndSignInMessage) between the Button's opening and closing
tags; keep the other props (loading={isLoading}, htmlType='submit', block)
unchanged so Button renders the same text via its inner JSX rather than a
children prop.
| `{ | ||
| "code": "BAD_USER_INPUT", | ||
| "type": "NOT_ENOUGH_AUTH_FACTORS", | ||
| "message": "Not enough auth factors", | ||
| "messageForUser": "Not enough auth factors" | ||
| }` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the schema.graphql file at the specified lines
wc -l apps/condo/schema.graphqlRepository: open-condo-software/condo
Length of output: 101
🏁 Script executed:
# Get the context around lines 102203-102208 and 102627-102633
sed -n '102200,102210p' apps/condo/schema.graphqlRepository: open-condo-software/condo
Length of output: 472
🏁 Script executed:
# Check the other location mentioned
sed -n '102624,102636p' apps/condo/schema.graphqlRepository: open-condo-software/condo
Length of output: 413
🏁 Script executed:
# Search for how NOT_ENOUGH_AUTH_FACTORS errors are actually constructed
rg 'NOT_ENOUGH_AUTH_FACTORS' --type ts --type js -C 5Repository: open-condo-software/condo
Length of output: 50381
Update schema.graphql documentation to reflect actual messageForUser values returned at runtime.
The schema.graphql documentation shows "messageForUser": "Not enough auth factors" (hardcoded English string), but the actual server responses use the i18n key "messageForUser": "api.user.NOT_ENOUGH_AUTH_FACTORS". Update the documented error payloads in the schema to show the i18n key instead of the misleading hardcoded English string. This pattern applies to all four mutations (lines 102203–102208, 102627–102633, and others).
Implementation context
The error definitions in the service files use:
NOT_ENOUGH_AUTH_FACTORS: {
mutation: 'authenticateUserWithEmailAndPassword',
code: BAD_USER_INPUT,
type: NOT_ENOUGH_AUTH_FACTORS,
message: 'Not enough auth factors',
messageForUser: 'api.user.NOT_ENOUGH_AUTH_FACTORS', // i18n key, not hardcoded string
},This pattern is consistent across AuthenticateUserWithEmailAndPasswordService.js, AuthenticateUserWithPhoneAndPasswordService.js, GenerateSudoTokenService.js, and AuthenticateOrRegisterUserWithTokenService.js.
🤖 Prompt for AI Agents
In `@apps/condo/schema.graphql` around lines 102203 - 102208, Update the
documented error payloads in schema.graphql so messageForUser shows the i18n
keys returned at runtime (not the hardcoded English text): replace "Not enough
auth factors" with "api.user.NOT_ENOUGH_AUTH_FACTORS" for the
NOT_ENOUGH_AUTH_FACTORS entries used by the
authenticateUserWithEmailAndPassword, authenticateUserWithPhoneAndPassword,
generateSudoToken, and authenticateOrRegisterUserWithToken mutation docs (match
the same messageForUser values defined in the service constants).
3 participants
Demo
2026-01-20.19.26.33.mov
2026-01-20.19.24.44.mov
Summary by CodeRabbit
New Features
Privacy
Localization
Style