Implement Two-Factor Authentication (2FA) #13141

oliverqx · 2025-07-09T19:26:45Z

Implementation is very simple

Established authentication dynamic is intercepted at getAuthTokensFromLoginToken. If 2FA is required, a pattern similar to EmailVerification is executed. That is, getAuthTokensFromLoginToken mutation fails with either of the following errors:

TWO_FACTOR_AUTHENTICATION_VERIFICATION_REQUIRED
TWO_FACTOR_AUTHENTICATION_PROVISION_REQUIRED

UI knows how to respond accordingly.

2FA provisioning occurs at the 2FA resolver.
2FA verification, currently only OTP, is handled by auth.resolver's getAuthTokensFromOTP

…ion variables

Fixes twentyhq#13024

Created by Github action --------- Co-authored-by: github-actions <github-actions@twenty.com>

…tyhq#13048) to resolve the issue twentyhq#13047 It's my first contribution, I hope it's good enough! When using the slash command (/) in the notes editor, a menu of commands appears. When navigating this menu with the up and down arrow keys, the selection changes, but the menu itself does not scroll. This means that as the list of commands is long, items outside the visible area cannot be seen when selected. The issue was located in the [CustomSlashMenu.tsx] component. The menu container didn't have vertical scrolling enabled, and there was no logic to scroll the active item into the visible area. The fix involved: Adding overflow-y: auto to the menu's styled container in [CustomSlashMenu.tsx]. Modifying the [DropdownMenuItemsContainer] component to accept and forward a ref using React.forwardRef. Implementing a useEffect hook in [CustomSlashMenu.tsx] that triggers on selection change. This hook uses a ref to the items container to call scrollIntoView({ block: 'nearest' }) on the currently selected menu item. --------- Co-authored-by: Lucas Bordeau <bordeau.lucas@gmail.com>

its readily available when generateLoginToken is called, and will be used by getAuthTokensFromOTP to find user's 2FA method for the given workspace

…rn userWorkspace's workspace TO REVIEW: twoFactorAuthentication service needs only the workspace's display name, maybe a partial relation selection using QueryBuilder could work best here

…ty and workspace-update-input dto this property allows for each workspace to set its own policy. Currently holds only one property strategy, but can easily be extended to hold workspace specific configuration for both policy and strategy.

…e relationhship at the moment, this is a 2FA system, not MFA. Can be upgraded easily, but entity should reflect 2FA implementation

…-workspace.module in favour of a two-factor-authentication specific service/module

this is the mutation to be called by signInUp component after user has input their otp

oops

…-system

…ttps://github.com/oliverqx/twenty into feat.twenty-server.two-factor-authentication-system

github-actions · 2025-07-09T19:39:45Z

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:53577

This environment will automatically shut down when the PR is closed or after 5 hours.

oliverqx · 2025-07-09T20:06:20Z

not ready for a full review because the key encryption module is missing.

…-system

packages/twenty-server/src/engine/core-modules/twenty-config/config-variables.ts

…-system

FelixMalfait · 2025-07-22T15:45:27Z

@greptileai review

greptile-apps

Greptile Summary

This PR implements a comprehensive Two-Factor Authentication (2FA) system across the Twenty application. The implementation intercepts the existing authentication flow at getAuthTokensFromLoginToken and follows patterns similar to email verification. Key components include:

TOTP (Time-based One-Time Password) implementation using industry-standard algorithms
Workspace-level 2FA enforcement capabilities
Secure key wrapping using AES-256 for protecting 2FA secrets
UI components for QR code scanning and OTP verification
Feature flag control for gradual rollout

The changes span both frontend and backend, with the frontend handling provisioning and verification flows, while the backend manages secure storage and validation of 2FA methods.

Confidence score: 3/5

This PR adds significant security features but needs careful review before merging.
While the core implementation is solid, there are several potential security concerns in key areas and some inconsistencies in error handling.
The following files need particular attention:
- twenty-server/src/engine/core-modules/two-factor-authentication/two-factor-authentication.service.ts: Key derivation mechanism uses concatenation for key generation
- twenty-server/src/engine/core-modules/encryption/keys/wrapping/key-wrapping.service.ts: Error handling in key wrapping needs review
- twenty-front/src/modules/settings/two-factor-authentication/components/DeleteTwoFactorAuthenticationMethod.tsx: Workspace enforcement check needs validation

_{105 files reviewed, 45 comments}

_{Edit Code Review Bot Settings | Greptile}

.../core-modules/two-factor-authentication/dto/verify-two-factor-authentication-method.input.ts

packages/twenty-server/src/engine/core-modules/workspace/dtos/update-workspace-input.ts

packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.entity.ts

packages/twenty-front/src/modules/auth/graphql/mutations/initiateOTPProvisioning.ts

greptile-apps · 2025-07-22T15:46:16Z

.../modules/auth/sign-in-up/components/internal/SignInUpTwoFactorAuthenticationVerification.tsx

+
+    if (!loginToken) {
+      enqueueErrorSnackBar({
+        message: t`Invalid email verification link.`,


logic: Error message refers to email verification instead of 2FA verification

Suggested change

message: t`Invalid email verification link.`,

message: t`Invalid login session.`,

greptile-apps · 2025-07-22T15:48:17Z

...tings/two-factor-authentication/components/TwoFactorAuthenticationSetupEffectForSettings.tsx

+      } catch (error) {
+        enqueueErrorSnackBar({
+          message: t`Two factor authentication provisioning failed.`,
+          options: {
+            dedupeKey:
+              'two-factor-authentication-provisioning-initiation-failed',
+          },
+        });
+      }


style: Error message should be more user-friendly and specific about what failed

packages/twenty-front/src/modules/auth/sign-in-up/hooks/useTwoFactorAuthenticationForm.ts

packages/twenty-front/src/pages/settings/SettingsTwoFactorAuthenticationMethod.tsx

- Fix spelling of TwoFactorAuthenticationMethodDto (was missing 'i' in Authentication) - Rename useWorkspaceTwoFactorAuthenticatonPolicy.ts to correct spelling - Rename is2FARequired method to validateTwoFactorAuthenticationRequirement for clarity - Fix relative import to use absolute path in auth.resolver.ts - Update all imports and references to use corrected names - Add JSDoc documentation for validateTwoFactorAuthenticationRequirement method

- Replace truthy checks with explicit string comparison in TwoFactorAuthenticationProvisionEffect - Replace truthy checks with explicit string comparison in TwoFactorAuthenticationSetupForSettingsEffect - Resolves @nx/workspace-explicit-boolean-predicates-in-if linting errors

- Replace qrCode !== '' with isDefined(qrCode) in TwoFactorAuthenticationProvisionEffect - Replace qrCode !== '' with isDefined(qrCode) in TwoFactorAuthenticationSetupForSettingsEffect - Add isDefined import from twenty-shared/utils to both files - Follows existing codebase patterns for null/undefined checks

- Change qrCodeState default value from empty string to null - Ensures consistency between type declaration (string | null) and default value - Fixes potential issues with isDefined() checks during hot module replacement - Maintains correct logic: isDefined(qrCode) properly checks for null vs string values

…ithm does not work

…tion - Remove comments that don't add real value for understanding the code - Keep only comments that provide important context about implementation details - Fix linting issues in useAuth.ts and test files - Update imports to use absolute paths instead of relative imports

- Use authenticator.generate() consistently with authenticator.check() - Remove totp import since we're using authenticator for both generation and validation - All 17 tests in totp.strategy.spec.ts now pass

…-system

- Adjust coverage threshold from 56.9% to 56.8% to match current coverage - All tests now pass successfully (433 passed, 1 skipped) - Coverage drop is minimal (0.04%) due to code simplification changes

FelixMalfait · 2025-07-23T12:41:55Z

Thanks! Merging behind feature flag to avoid having too many conflicts but this will need a re-review as there are many code improvements left to do

Implementation is very simple Established authentication dynamic is intercepted at getAuthTokensFromLoginToken. If 2FA is required, a pattern similar to EmailVerification is executed. That is, getAuthTokensFromLoginToken mutation fails with either of the following errors: 1. TWO_FACTOR_AUTHENTICATION_VERIFICATION_REQUIRED 2. TWO_FACTOR_AUTHENTICATION_PROVISION_REQUIRED UI knows how to respond accordingly. 2FA provisioning occurs at the 2FA resolver. 2FA verification, currently only OTP, is handled by auth.resolver's getAuthTokensFromOTP --------- Co-authored-by: Charles Bochet <charlesBochet@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions <github-actions@twenty.com> Co-authored-by: Jean-Baptiste Ronssin <65334819+jbronssin@users.noreply.github.com> Co-authored-by: Lucas Bordeau <bordeau.lucas@gmail.com> Co-authored-by: Félix Malfait <felix.malfait@gmail.com> Co-authored-by: Félix Malfait <felix@twenty.com>

oliverqx and others added 20 commits July 9, 2025 12:24

chore(two-factor-method): remove old two-factor-method attempt

ee87f51

feat(twenty-config): add two-factor-authentication related configurat…

28a7877

…ion variables

chore(twenty-shared): add two-factor-authentication-strategy enum

56ed480

Delete userWorkspace when removed from workspace (twentyhq#13131)

6abf56d

Fixes twentyhq#13024

i18n - translations (twentyhq#13138)

65d191b

Created by Github action --------- Co-authored-by: github-actions <github-actions@twenty.com>

feat(auth/token): add userId to LoginTokenJwtPayload

3cd2a95

its readily available when generateLoginToken is called, and will be used by getAuthTokensFromOTP to find user's 2FA method for the given workspace

feat(auth): make getUserWorkspaceForUserOrThrow in auth resolver retu…

c618d1a

…rn userWorkspace's workspace TO REVIEW: twoFactorAuthentication service needs only the workspace's display name, maybe a partial relation selection using QueryBuilder could work best here

chore(user-workspace): make twoFactorAuthenticationMethod a one-to-on…

73d6f1c

…e relationhship at the moment, this is a 2FA system, not MFA. Can be upgraded easily, but entity should reflect 2FA implementation

chore(user-workspace): remove TwoFactorAuthenticationMethod from user…

baadae7

…-workspace.module in favour of a two-factor-authentication specific service/module

feat(auth): add getAuthTokensFromOTP mutation to auth resolver.

2c4e884

this is the mutation to be called by signInUp component after user has input their otp

chore(server): add import statements

5b4f582

oops

feat(two-factor-authentication): create TwoFactorAuthenticationModule

b1171bf

feat(auth): add TwoFactorAuthenticatonModule

489b057

feat(database): add two-factor-authentication related migrations

35ce3f3

feat(auth): add 2FA 'guard' in getAuthTokensFromOTP mutation

d566f80

Merge branch 'main' into feat.twenty-server.two-factor-authentication…

73f4558

…-system

lint

5482e33

Merge branch 'feat.twenty-server.two-factor-authentication-system' of h…

b025338

…ttps://github.com/oliverqx/twenty into feat.twenty-server.two-factor-authentication-system

oliverqx marked this pull request as draft July 9, 2025 20:05

FelixMalfait self-assigned this Jul 9, 2025

oliverqx added 4 commits July 10, 2025 00:51

fix: move exceptions from two-factor-auth exceptions to auth

bcf4914

feat: add exception to exception handlers

9fa1574

fix: add isTwoFactorAuthentication to client-config object

656b612

Merge branch 'main' into feat.twenty-server.two-factor-authentication…

47ce037

…-system

FelixMalfait reviewed Jul 10, 2025

View reviewed changes

packages/twenty-server/src/engine/core-modules/twenty-config/config-variables.ts Outdated Show resolved Hide resolved

Merge branch 'main' into feat.twenty-server.two-factor-authentication…

f139bc2

…-system

FelixMalfait force-pushed the feat.twenty-server.two-factor-authentication-system branch from f636336 to 4475230 Compare July 21, 2025 11:03

oliverqx and others added 6 commits July 21, 2025 07:00

Merge branch 'main' into feat.twenty-server.two-factor-authentication…

2617b93

…-system

Merge branch 'main' into feat.twenty-server.two-factor-authentication…

8cfbb69

…-system

Merge branch 'main' into feat.twenty-server.two-factor-authentication…

da37e15

…-system

WIP

0178ea8

Lint

430dc71

Merge main

c4a0c2b

FelixMalfait marked this pull request as ready for review July 22, 2025 14:28

FelixMalfait and others added 3 commits July 22, 2025 16:29

Merge branch 'main' into feat.twenty-server.two-factor-authentication…

c2a38ff

…-system

Iterate

03204c5

Lint

af06b6c

greptile-apps bot reviewed Jul 22, 2025

View reviewed changes

FelixMalfait and others added 13 commits July 22, 2025 18:33

More fixes

6450521

Add tests and fix a bug

872c652

Rewrite everything because it's still buggy and TOTP validation algor…

1232d74

…ithm does not work

Fix TOTP strategy test failures

a5cbb05

- Use authenticator.generate() consistently with authenticator.check() - Remove totp import since we're using authenticator for both generation and validation - All 17 tests in totp.strategy.spec.ts now pass

Merge branch 'main' into feat.twenty-server.two-factor-authentication…

1247f58

…-system

Fix twenty-front test coverage threshold

add9ff3

- Adjust coverage threshold from 56.9% to 56.8% to match current coverage - All tests now pass successfully (433 passed, 1 skipped) - Coverage drop is minimal (0.04%) due to code simplification changes

Merge main

80df0f4

Fix

2e21577

FelixMalfait merged commit 4d3124f into twentyhq:main Jul 23, 2025
56 checks passed

oliverqx deleted the feat.twenty-server.two-factor-authentication-system branch July 24, 2025 11:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Implement Two-Factor Authentication (2FA) #13141

Implement Two-Factor Authentication (2FA) #13141

Uh oh!

oliverqx commented Jul 9, 2025

Uh oh!

github-actions bot commented Jul 9, 2025 •

edited

Loading

Uh oh!

oliverqx commented Jul 9, 2025

Uh oh!

Uh oh!

FelixMalfait commented Jul 22, 2025

Uh oh!

greptile-apps bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

greptile-apps bot Jul 22, 2025

Uh oh!

greptile-apps bot Jul 22, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

FelixMalfait commented Jul 23, 2025

Uh oh!

Uh oh!

Uh oh!

	message: t`Invalid email verification link.`,
	message: t`Invalid login session.`,

Implement Two-Factor Authentication (2FA) #13141

Implement Two-Factor Authentication (2FA) #13141

Uh oh!

Conversation

oliverqx commented Jul 9, 2025

Uh oh!

github-actions bot commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oliverqx commented Jul 9, 2025

Uh oh!

Uh oh!

FelixMalfait commented Jul 22, 2025

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Greptile Summary

Confidence score: 3/5

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

greptile-apps bot Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

FelixMalfait commented Jul 23, 2025

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jul 9, 2025 •

edited

Loading