fix: MFA implementation #743

sattvikc · 2023-11-16T11:33:25Z

Summary of change

(A few sentences about this PR)

Related issues

Link to issue1 here
Link to issue1 here

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Bonus points for screenshots and videos!)

Documentation changes

(If relevant, please create a PR in our docs repo, or create a checklist here highlighting the necessary changes)

Checklist for important updates

Remaining TODOs for this PR

Item1
Item2

…ForUser impl

lib/ts/recipe/multifactorauth/recipeImplementation.ts

…aram

lib/ts/index.ts

lib/ts/querier.ts

lib/ts/recipe/multifactorauth/recipe.ts

lib/ts/recipe/multifactorauth/recipeImplementation.ts

lib/ts/recipe/session/recipeImplementation.ts

lib/ts/supertokens.ts

porcellus · 2023-12-12T23:18:22Z

lib/ts/recipe/multifactorauth/recipeImplementation.ts

                    ) {
                        return {
                            status: "ACCOUNT_INFO_ALREADY_ASSOCIATED_WITH_ANOTHER_PRIMARY_USER_ID_ERROR",
-                            message: "Error setting up MFA for the user. Please contact support. (ERR_CODE_011)",
+                            message:
+                                "Cannot complete factor setup as the account info is already associated with another primary user. Please contact support. (ERR_CODE_012)",


Could you update the FDI with the new status codes? (Moving it here since I've resolved the other thread)

lib/ts/recipe/emailpassword/api/implementation.ts

porcellus · 2023-12-13T21:33:51Z

lib/ts/recipe/multifactorauth/index.ts

@@ -86,24 +86,28 @@ export default class Wrapper {
            factorsSetUpForUser: factorsSetup,
            defaultRequiredFactorIdsForUser: defaultMFARequirementsForUser,
            defaultRequiredFactorIdsForTenant: defaultMFARequirementsForTenant,
-            userContext: userContext ?? ({} as UserContext),
+            userContext: getUserContext(userContext),


this should re-use ctx from above. it can be an issue if this is done multiple times as they can be separate objects (and so separate cache instances)

porcellus · 2023-12-13T21:52:45Z

lib/ts/supertokens.ts

@@ -212,7 +213,7 @@ export default class SuperTokens {
                includeRecipeIds: includeRecipeIdsStr,
                includeAllTenants: tenantId === undefined,
            },
-            userContext ?? ({} as UserContext)
+            getUserContext(userContext)


This is already getting a UserContext typed param

porcellus · 2023-12-13T21:56:03Z

test/accountlinking/userstructure.test.js

@@ -258,6 +258,7 @@ describe(`accountlinkingTests: ${printPath("[test/accountlinking/userstructure.t
                    },
                }),
                Session.init({
+                    overwriteSessionDuringSignIn: true,


Is this required? Isn't this the default?

the default is false. In this test we need it to set to true as there is an override for createNewSession where we are trying to reuse the session.

lib/ts/recipe/session/recipeImplementation.ts

lib/ts/recipe/multifactorauth/recipe.ts

porcellus · 2023-12-13T22:22:45Z

lib/ts/recipe/session/sessionClass.ts

        await this.helpers.getRecipeImpl().revokeSession({
            sessionHandle: this.sessionHandle,
-            userContext: userContext === undefined ? {} : userContext,
+            userContext: userContext === undefined ? ({} as UserContext) : userContext,


Missed this?

porcellus · 2023-12-13T22:23:35Z

lib/ts/recipe/session/sessionClass.ts

        return this.recipeUserId;
    }

-    async revokeSession(userContext?: any) {
+    async revokeSession(userContext?: UserContext) {


These functions are sometimes called by devs directly, so I'd prefer if these were Record<string, any>

porcellus · 2023-12-13T22:37:40Z

lib/ts/recipe/session/utils.ts

@@ -73,7 +73,7 @@ export async function sendTokenTheftDetectedResponse(
    ___: BaseRequest,
    response: BaseResponse
 ) {
-    await recipeInstance.recipeInterfaceImpl.revokeSession({ sessionHandle, userContext: {} });
+    await recipeInstance.recipeInterfaceImpl.revokeSession({ sessionHandle, userContext: {} as UserContext }); // TODO should userContext be passed to error handlers?


* feat(mfa): initial types (#708) * feat: add (partial) initial types for MFA * feat: expand the MFA recipe interface * feat: add export point for mfa * feat: update based on review discussions * feat: add extra params to MFARequirements callbacks to help customizations * feat: implement review feedback * feat: implement review comments * feat: implement review comments * feat: stricter type for first factor/mfa requirement * feat: remove distinction between built-in and custom factors (#729) * fix: MFA type updates (#737) * fix: type fix and account linking functions * fix: cdi version update * fix: more type updates * fix: tests * fix: TOTP recipe (#739) * fix: type fix and account linking functions * fix: cdi version update * fix: more type updates * fix: tests * fix: totp recipe * fix: totp types * fix: update types * fix: totp apis * fix: user identifier info * fix: recipe tests * fix: test * fix: pr comments * fix: tests * fix: PR comment * fix: MFA implementation (#743) * fix: type fix and account linking functions * fix: cdi version update * fix: more type updates * fix: tests * fix: totp recipe * fix: totp types * fix: update types * fix: totp apis * fix: user identifier info * fix: recipe tests * fix: test * fix: basic mfa impl * fix: pr comments * fix: tests * fix: factors setup from other recipe * fix: getFactorsSetupForUser impl * fix: getMFARequirementsForAuth impl * fix: isAllowedToSetupFactor impl * fix: addToDefaultRequiredFactorsForUser and getDefaultRequiredFactorsForUser impl * fix: typo * fix: build next array * fix: remove error file * fix: factorSetupForUser refactor * fix: next array * fix: api impl * fix: typo * fix: isValidFirstFactorForTenant * fix: impl * fix: updated impl * feat: fix and update mfa imlp to make all e2e tests pass * fix: adds overwriteSessionDuringSignIn config in session * fix: error messages in claims * fix: cleanup * fix: new errors for sign in up APIs * fix: add error in totp * fix: marked MFA TODOs * fix: new param in createNewSession * fix: impl cleanup * fix: remove MFA_ERROR * fix: cdi version * fix: test fix * fix: update/fix mfa impl to match e2e tests * fix: pr comments * fix: session user deleted error * fix: adding cache to getUserById * fix: get user cache * caching in querier * fix: mfa impl * fix: email selection * fix: mfa claims * fix: remove unnecessary file * fix: pr comment * fix: PR comments * fix: session handling * fix: review comments * fix: defaultRequiredFactorsForUser is now appwide * fix: using accountlinking instead of mfa for primary user and link accounts * fix: overwrite session flag refactor * fix: race conditions in createOrUpdateSessionForMultifactorAuthAfterFactorCompletion * fix: race conditions in createOrUpdateSessionForMultifactorAuthAfterFactorCompletion * fix: recipe functions refactor * fix: contact support case * fix: unnecessary file * fix: test * refactor: added shouldRefetch + fetchValue building the next array into MFAclaim (#758) * fix: usercontext type * fix: test * fix: test * feat: add access token payload param to claim.build * feat: expose addToDefaultRequiredFactorsForUser and remove tenantId param * fix: remaining TODOs * fix: auto init tests related to mfa * fix: recipe function tests * fix: create new session refactor * fix: recipe interface refactor * fix: userContext type fix * fix: test * fix: test * fix: session * fix: user context and support codes * fix: type fixes after merge * fix: test * fix: pr comments * fix: pr comment * fix: test * fix: available factors * fix: updated user object * fix: shouldAttemptAccountLinkingIfAllowed * fix: missed types and test fixes * fix: mfa fixes and tests * fix: more tests --------- Co-authored-by: Mihaly Lengyel <mihaly@lengyel.tech> * fix: contact support case when existing user signs in * fix: shouldAttemptAccountLinkingIfAllowed * fix: tackling some corner cases with account linking and user sign up * fix: isSignUpAllowed condition * fix: branded type explanation * fix: revert verifyEmailForRecipeUserIfLinkedAccountsAreVerified * fix: fixing shouldAttemptAccountLinkingIfAllowed typing * fix: recipe id check in emailpassword * fix: comment on not checking for tenantId * fix: remove implicit tenantInfo * fix: remove implicit tenantInfo * fix: duplicate factorIds * fix: not required params in validateForMultifactorAuthBeforeFactorCompletion * fix: input type of validateForMultifactorAuthBeforeFactorCompletion * changes impl of getMFARequirementsForAuth to remove oldest factor id * fix: rename defaultRequiredFactors to requiredSecondaryFactors for user * fix: update multitenancy core api * fix: add remove required secondary factors for user * fix: exposing known factor idsstnbp * fix: move to recipe impl * fix: not automatically assuming otp as setup * fix: update mfa info response * fix: changed mfa/info to PUT * fix: user refetch in emailpassword * fix: updated comments * fix: constant name * fix: factor flow impl in emailpassword and passwordless * fix: factor flow for thirdparty * fix: totp verification * fix: remove createNewOrKeepExistingSession * fix: improve createOrUpdateSessionForMultifactorAuthAfterFactorCompletion * fix: phoneNumber check * fix: sign in/up status refactor * fix: sign in/up status * fix: tests * fix: tests * feat: return email addresses for pwless factors based on the discussed priority order * fix: firstFactor validation and loginmethods * fix: userContext types and test * fix: misc changes * fix: emails for factor * fix: param rename * fix: msg update * fix: shouldAttemptAccountLinkingIfAllowed in thirdparty * fix: updated race conditions recursions * fix: removed tenantId checks for mfa * fix: recursion for support cases * fix: revert recursion fix to return 011 * fix: support code flows * fix: recursion point * fix: rename allOf to allOfInAnyOrder * fix: support codes * fix: copyright update * fix: copyright update * fix: implicit check * fix: support codes * fix: mfa flow refactor (#771) mfa flow refactor and support codes * fix: typo * fix: comments * fix: next array (#770) mfa info endpoint changes * fix: support code messages * fix: rename isAllowedToSetup and return claim error * fix: call checkAllowedToSetupFactorElseThrowInvalidClaimError in createCode and createDevice * fix: fixed status * fix: createNewSession param type * fix: mfa validation only on sign up in createCode * fix: factor check in create code * fix: factor check in create code * fix: remove mfa check in createCode * fix: removed unused status * fix: thirdparty api refactor * fix: passwordless api refactor and thirdparty api refactor fixes * fix: emailpassword api refactor and fixes in pless and tparty apis * fix: fixes after refactor * fix: clean up is valid first factor * fix: pr comments * fix: pr comments * fix: refactor * fix: refactor * fix: internal function for get user metadata for MFA * fix: pr comment * fix: race conditions * fix: refactor all factors * fix: comments * fix: assertAllowedToSetupFactorElseThrowInvalidClaimError in verify device * fix: tests * fix: add comment * fix: refactor resync api stuff * fix: refactor missing claims * fix: dedup code in mfa claim * fix: pr comments for emailpassword * fix: pr comments for emailpassword * fix: pr comments * fix: pr comments for email password * fix: pr comments for passwordless * fix: pr comments for thirdparty * fix: pr comments * fix: move recurse outside * fix: move assert sign in is allowed * fixes and changes * thirdparty recipe change * fix: cyclic dependency * fix: test * fix: claim value type * fix: pr comments from ep to pless * fix: remove internal functions from usermetadata * fix: context in session class * fix: session required in signout * fix: remove implicit check * fix: doUnionOfAccountInfo false for consistency * fix: pr comments from ep in pless * fix: remove shouldAttemptAccountLinkingIfAllowed in passwordless * fix: make isValidFirstFactor more readble with comments * fix: remove tenantId from getFactorsSetupForuser * fix: PR comments * fix: refactor totp * fix: post init callbacks to constructor * fix: totp PR comments * fix: error messages, test and fetch failure check * fix: tests * fix: tests * fix: PR comments * fix: Pr comments * fix: missed await + test fix * fix: missed await * fix: pr comments * fix: not use splice * fix: user metadata refactor * fix: mfa refactor * fix: self review * fix: clean up and comments * fix: comment * fix: refactor factorIds * fix: refactor factorIds * fix: handle unknown user id error in totp * fix: updated signout * fix: removed extra code * fix: session and tenantId in createCode * fix: first factor computation * fix: comment * fix: createRecipeUser in pless * fix: factor completion in thirdparty signInUp * fix: updated fake email * fix: should attempt account linking in third party * fix: throw unauthorised for tenant not found * fix: signout api * fix: cyclic dependency * fix: shouldAttemptAccountLinkingIfAllowed * fix: check claims error and throw others * fix: remove unnecessary session undefined check * fix: session in create code and resend code POST * fix: tenant not found * fix: mfa claim updation in util function * fix: revert to original :( * fix: revert to original :( * fix: cleanup * fix: pless createRecipeUser type * fix: pless revert * fix: querier caching to include headers * feat: use dynamic signing key switching (#782) * feat: enable a smooth switch between useDynamicAccessTokenSigningKey true and false * Merging feat/mfa/base into feat/useDynamicSigningKey_switching * fix: update prop name * feat: move account linking related MFA things into account linking recipe (#788) * feat: move account linking related MFA things into account linking recipe * test: update test to match new linking logic * feat(mfa)!: simplifying account linking flows * feat(mfa): updating impl of other recipe sign in/up endpoints + add createRecipeUserIfNotExists to consume code * feat: implement updated linking flow + fix tests * chore: remove unnecessary code * fix: fix typo/missing update * refactor: implementing review comments * refactor: implementing review comments * fix: add retry logic to createPrimaryUserIdOrLinkAccounts index func * fix: use hasSameEmailAs instead of === * refactor: improve getAuthenticatingUserAndAddToCurrentTenantIfRequired input types & comments * fix: remove wrong emailverification check from tryLinkAccounts * refactor: removed unnecessary status + added explanation comments * refactor: implement review comments * refactor: implement review comments + move functions * fix: fix accidentally flipped condition * fix: tidy up typos * fix: update func impl after moving * fix: cleaning up and updating tests * feat: refactoring for latest review comments * test: add mock for verifyCode * fix: cleanup and test fixes * chore: update error code reason strings * feat: remove factorIds from createCode input * fix: properly compare recipeUserIds as strings * feat: add signInVerifiesLoginMethod and minor cleanup/fixes * test: add new tests + update cases for new account linking logic/interface * fix: login methods (#794) * fix: login methods fix * fix: cleanup * fix: cleanup * fix: pr comments * fix: pr comment * fix: better test names * fix: comparision * feat: self-review fixes * chore: update changelog * feat: self-review fixes * feat: remove shouldAttemptAccountLinkingIfAllowed * feat: add verifyCredentials to ep and tpep recipes * feat: self-review fixes and latest discussions * fix: return userType instead of the user class consistently in the index files * feat: properly expose verifyCredentials and verifyCode * fix: add some missing tests and a related fix * feat: add call count tests and improve call counts * refactor: not trying linking if shouldDoAutomaticAccountLinking wasn't defined by the app * feat: implement review feedback * chore: add version number and compatibility section into changelog * feat: update mfa interface to optimize for less core calls * feat: make the core call cache reset globally if a call was made without usercontext * feat: ensure the email verification api can update the session if necessary * feat: verify the user in consume code if possible before trying to make it primary * feat: update verifyCredentials types to re-use it in signIn * feat: add disableCoreCallCache * refactor: remove resolved comment * chore: update changelog * feat: move the skipSessionUserUpdateInCore check before we check EV status * feat: fix tests & update for latest core * docs: fix jsdocs for pwless * docs: add explanation comment * feat: cache the result of checkCode in pwless consume code api * feat: add new param to revokeCode * feat: update changelog + consistency --------- Co-authored-by: Mihály Lengyel <mihaly@lengyel.tech> Co-authored-by: Rishabh Poddar <rishabh.poddar@gmail.com>

sattvikc added 24 commits November 8, 2023 10:34

fix: type fix and account linking functions

8419883

fix: cdi version update

4506694

fix: more type updates

394b8fa

fix: tests

9d379f6

fix: totp recipe

9fd1020

fix: totp types

84f0d27

fix: update types

c72528f

Merge branch 'feat/mfa/base' into totp-impl

38852c0

fix: totp apis

2a34e84

fix: user identifier info

6ffe90e

fix: recipe tests

b037f82

fix: test

c4ad6ec

fix: basic mfa impl

87b597c

fix: pr comments

b426ad3

fix: tests

727dce8

Merge branch 'totp-impl' into mfa-impl

f51c5e7

fix: factors setup from other recipe

6c79c5f

fix: getFactorsSetupForUser impl

7b5a8d7

fix: getMFARequirementsForAuth impl

e554f12

fix: isAllowedToSetupFactor impl

8faa11f

fix: addToDefaultRequiredFactorsForUser and getDefaultRequiredFactors…

4b63d95

…ForUser impl

fix: typo

c856448

fix: build next array

40aca56

Merge branch 'feat/mfa/base' into mfa-impl

57bb831

sattvikc self-assigned this Nov 16, 2023

fix: remove error file

c187b34

porcellus requested changes Nov 16, 2023

View reviewed changes

lib/ts/recipe/multifactorauth/recipeImplementation.ts Outdated Show resolved Hide resolved

lib/ts/recipe/multifactorauth/recipeImplementation.ts Outdated Show resolved Hide resolved

lib/ts/recipe/multifactorauth/recipeImplementation.ts Outdated Show resolved Hide resolved

sattvikc added 3 commits November 16, 2023 18:23

fix: factorSetupForUser refactor

6031ab0

fix: next array

ce72f53

fix: api impl

2c149d7

porcellus and others added 7 commits December 11, 2023 22:51

feat: add access token payload param to claim.build

4cb28f8

feat: expose addToDefaultRequiredFactorsForUser and remove tenantId p…

cb3ae0c

…aram

fix: remaining TODOs

33e329a

fix: auto init tests related to mfa

c7e658a

fix: recipe function tests

6fa097e

fix: create new session refactor

3a8f19a

fix: recipe interface refactor

bc4ea29

porcellus requested changes Dec 12, 2023

View reviewed changes

sattvikc added 2 commits December 13, 2023 13:35

fix: userContext type fix

0432773

fix: test

2fc6dd4

porcellus reviewed Dec 13, 2023

View reviewed changes

lib/ts/recipe/emailpassword/api/implementation.ts Outdated Show resolved Hide resolved

sattvikc added 6 commits December 13, 2023 14:54

fix: test

2518cca

fix: session

22cc9dd

fix: user context and support codes

fae1242

Merge branch 'feat/mfa/base' into mfa-impl

13778d1

fix: type fixes after merge

a533964

fix: test

463cbfa

porcellus requested changes Dec 13, 2023

View reviewed changes

sattvikc added 9 commits December 14, 2023 11:45

fix: pr comments

74f0b46

fix: pr comment

d2b648b

fix: test

634fb9a

fix: available factors

257dab6

fix: updated user object

c15cea0

fix: shouldAttemptAccountLinkingIfAllowed

27cfdfc

fix: missed types and test fixes

4d9f1c2

fix: mfa fixes and tests

64f4f39

fix: more tests

4e30c36

sattvikc merged commit 520a320 into feat/mfa/base Dec 15, 2023
3 of 14 checks passed

sattvikc deleted the mfa-impl branch December 15, 2023 13:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: MFA implementation #743

fix: MFA implementation #743

sattvikc commented Nov 16, 2023

porcellus Dec 12, 2023

porcellus Dec 13, 2023

sattvikc Dec 14, 2023

porcellus Dec 13, 2023

sattvikc Dec 14, 2023

porcellus Dec 13, 2023

sattvikc Dec 14, 2023

porcellus Dec 13, 2023

sattvikc Dec 14, 2023

porcellus Dec 13, 2023

sattvikc Dec 14, 2023

porcellus Dec 13, 2023

sattvikc Dec 14, 2023

fix: MFA implementation #743

fix: MFA implementation #743

Conversation

sattvikc commented Nov 16, 2023

Summary of change

Related issues

Test Plan

Documentation changes

Checklist for important updates

Remaining TODOs for this PR

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment