diff --git a/CHANGELOG.md b/CHANGELOG.md index a25065a00..744bf30d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [unreleased] +## [17.0.4] - 2024-04-09 + +### Changes + +- Improved error message to help debugging if MFA was enabled without account linking. +- Now the `resyncSessionAndFetchMFAInfoPUT` will throw if the user is in a stuck state, because they are required to complete factors, but they are not allowed to because of some configuration issue. + ## [17.0.3] - 2024-04-08 ### Fixes diff --git a/lib/build/authUtils.js b/lib/build/authUtils.js index 8471d687f..a0afc304a 100644 --- a/lib/build/authUtils.js +++ b/lib/build/authUtils.js @@ -839,7 +839,8 @@ exports.AuthUtils = { } else { throw new error_2.default({ type: error_2.default.BAD_INPUT_ERROR, - message: "First factor sign in/up called for a non-first factor with an active session.", + message: + "First factor sign in/up called for a non-first factor with an active session. This might indicate that you are trying to use this as a secondary factor, but disabled account linking.", }); } } diff --git a/lib/build/recipe/multifactorauth/api/implementation.js b/lib/build/recipe/multifactorauth/api/implementation.js index cf3f9f93f..954ae11bd 100644 --- a/lib/build/recipe/multifactorauth/api/implementation.js +++ b/lib/build/recipe/multifactorauth/api/implementation.js @@ -93,18 +93,26 @@ function getAPIInterface() { message: "User no longer associated with the session", }); } + // next array is filtered to only include factors that are allowed to be setup or are already setup + // we do this because the factor chooser in the frontend will be based on the next array only + // we do not simply filter out the factors that are not allowed to be setup because in the case + // where user has already setup a factor and not completed it, none of the factors will be allowed to + // be setup, and that that will result in an empty next array. However, we want to show the factor + // that the user has already setup in that case. + const next = nextSetOfUnsatisfiedFactors.factorIds.filter( + (factorId) => factorsAllowedToSetup.includes(factorId) || factorsSetUpForUser.includes(factorId) + ); + if (next.length === 0 && nextSetOfUnsatisfiedFactors.factorIds.length !== 0) { + throw new Error( + `The user is required to complete secondary factors they are not allowed to (${nextSetOfUnsatisfiedFactors.factorIds.join( + ", " + )}), likely because of configuration issues.` + ); + } return { status: "OK", factors: { - // next array is filtered to only include factors that are allowed to be setup or are already setup - // we do this because the factor chooser in the frontend will be based on the next array only - // we do not simply filter out the factors that are not allowed to be setup because in the case - // where user has already setup a factor and not completed it, none of the factors will be allowed to - // be setup, and that that will result in an empty next array. However, we want to show the factor - // that the user has already setup in that case. - next: nextSetOfUnsatisfiedFactors.factorIds.filter( - (factorId) => factorsAllowedToSetup.includes(factorId) || factorsSetUpForUser.includes(factorId) - ), + next, alreadySetup: factorsSetUpForUser, allowedToSetup: factorsAllowedToSetup, }, diff --git a/lib/build/version.d.ts b/lib/build/version.d.ts index 7abac1cca..8a8a9ffd6 100644 --- a/lib/build/version.d.ts +++ b/lib/build/version.d.ts @@ -1,4 +1,4 @@ // @ts-nocheck -export declare const version = "17.0.3"; +export declare const version = "17.0.4"; export declare const cdiSupported: string[]; export declare const dashboardVersion = "0.11"; diff --git a/lib/build/version.js b/lib/build/version.js index 7a2005dff..9b2b1d1e5 100644 --- a/lib/build/version.js +++ b/lib/build/version.js @@ -15,7 +15,7 @@ exports.dashboardVersion = exports.cdiSupported = exports.version = void 0; * License for the specific language governing permissions and limitations * under the License. */ -exports.version = "17.0.3"; +exports.version = "17.0.4"; exports.cdiSupported = ["5.0"]; // Note: The actual script import for dashboard uses v{DASHBOARD_VERSION} exports.dashboardVersion = "0.11"; diff --git a/lib/ts/authUtils.ts b/lib/ts/authUtils.ts index 85586e960..0443941bf 100644 --- a/lib/ts/authUtils.ts +++ b/lib/ts/authUtils.ts @@ -951,7 +951,8 @@ export const AuthUtils = { } else { throw new SuperTokensError({ type: SuperTokensError.BAD_INPUT_ERROR, - message: "First factor sign in/up called for a non-first factor with an active session.", + message: + "First factor sign in/up called for a non-first factor with an active session. This might indicate that you are trying to use this as a secondary factor, but disabled account linking.", }); } } diff --git a/lib/ts/recipe/multifactorauth/api/implementation.ts b/lib/ts/recipe/multifactorauth/api/implementation.ts index b135e2c7e..c0c8ced7e 100644 --- a/lib/ts/recipe/multifactorauth/api/implementation.ts +++ b/lib/ts/recipe/multifactorauth/api/implementation.ts @@ -96,18 +96,28 @@ export default function getAPIInterface(): APIInterface { }); } + // next array is filtered to only include factors that are allowed to be setup or are already setup + // we do this because the factor chooser in the frontend will be based on the next array only + // we do not simply filter out the factors that are not allowed to be setup because in the case + // where user has already setup a factor and not completed it, none of the factors will be allowed to + // be setup, and that that will result in an empty next array. However, we want to show the factor + // that the user has already setup in that case. + const next = nextSetOfUnsatisfiedFactors.factorIds.filter( + (factorId) => factorsAllowedToSetup.includes(factorId) || factorsSetUpForUser.includes(factorId) + ); + + if (next.length === 0 && nextSetOfUnsatisfiedFactors.factorIds.length !== 0) { + throw new Error( + `The user is required to complete secondary factors they are not allowed to (${nextSetOfUnsatisfiedFactors.factorIds.join( + ", " + )}), likely because of configuration issues.` + ); + } + return { status: "OK", factors: { - // next array is filtered to only include factors that are allowed to be setup or are already setup - // we do this because the factor chooser in the frontend will be based on the next array only - // we do not simply filter out the factors that are not allowed to be setup because in the case - // where user has already setup a factor and not completed it, none of the factors will be allowed to - // be setup, and that that will result in an empty next array. However, we want to show the factor - // that the user has already setup in that case. - next: nextSetOfUnsatisfiedFactors.factorIds.filter( - (factorId) => factorsAllowedToSetup.includes(factorId) || factorsSetUpForUser.includes(factorId) - ), + next, alreadySetup: factorsSetUpForUser, allowedToSetup: factorsAllowedToSetup, }, diff --git a/lib/ts/version.ts b/lib/ts/version.ts index e405fb685..40f128f22 100644 --- a/lib/ts/version.ts +++ b/lib/ts/version.ts @@ -12,7 +12,7 @@ * License for the specific language governing permissions and limitations * under the License. */ -export const version = "17.0.3"; +export const version = "17.0.4"; export const cdiSupported = ["5.0"]; diff --git a/package-lock.json b/package-lock.json index c337625a4..451222073 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "supertokens-node", - "version": "17.0.3", + "version": "17.0.4", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "supertokens-node", - "version": "17.0.3", + "version": "17.0.4", "license": "Apache-2.0", "dependencies": { "content-type": "^1.0.5", diff --git a/package.json b/package.json index e5273c435..8a8c3eea0 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "supertokens-node", - "version": "17.0.3", + "version": "17.0.4", "description": "NodeJS driver for SuperTokens core", "main": "index.js", "scripts": { diff --git a/test/mfa/mfa.api.test.js b/test/mfa/mfa.api.test.js index cdd96985f..5e99b0307 100644 --- a/test/mfa/mfa.api.test.js +++ b/test/mfa/mfa.api.test.js @@ -226,6 +226,64 @@ describe(`mfa-api: ${printPath("[test/mfa/mfa.api.test.js]")}`, function () { assert.deepEqual(["emailpassword", "otp-email", "totp"], res.body.factors.allowedToSetup); }); + it("mfa info errors if the user is stuck", async function () { + const connectionURI = await startSTWithMultitenancy(); + let requireFactor = false; + + SuperTokens.init({ + supertokens: { + connectionURI, + }, + appInfo: { + apiDomain: "api.supertokens.io", + appName: "SuperTokens", + websiteDomain: "supertokens.io", + }, + recipeList: [ + EmailPassword.init(), + ThirdParty.init(), + AccountLinking.init({ + shouldDoAutomaticAccountLinking: async () => ({ + shouldAutomaticallyLink: true, + shouldRequireVerification: true, + }), + }), + MultiFactorAuth.init({ + override: { + functions: (oI) => ({ + ...oI, + getMFARequirementsForAuth: () => (requireFactor ? ["otp-phone"] : []), + }), + }, + }), + Session.init(), + ], + }); + + const app = getTestExpressApp(); + + await EmailPassword.signUp("public", "test@example.com", "password"); + + let res = await epSignIn(app, "test@example.com", "password"); + assert.equal("OK", res.body.status); + + let cookies = extractInfoFromResponse(res); + const accessToken = cookies.accessTokenFromAny; + + res = await getMfaInfo(app, accessToken); + assert.equal("OK", res.body.status); + assert.deepEqual([], res.body.factors.next); + + requireFactor = true; + + res = await getMfaInfo(app, accessToken, 500); + assert( + res.text.includes( + "The user is required to complete secondary factors they are not allowed to (otp-phone), likely because of configuration issues." + ) + ); + }); + it("test that only a valid first factor is allowed to login", async function () { const connectionURI = await startSTWithMultitenancy(); SuperTokens.init({ diff --git a/test/mfa/utils.js b/test/mfa/utils.js index 28aab1797..7dfc87f0a 100644 --- a/test/mfa/utils.js +++ b/test/mfa/utils.js @@ -222,6 +222,6 @@ module.exports.tpSignInUp = async function (app, thirdPartyId, email, accessToke } }; -module.exports.getMfaInfo = async function (app, accessToken) { - return request(app).put("/auth/mfa/info").set("Authorization", `Bearer ${accessToken}`).expect(200); +module.exports.getMfaInfo = async function (app, accessToken, statusCode = 200) { + return request(app).put("/auth/mfa/info").set("Authorization", `Bearer ${accessToken}`).expect(statusCode); };