From 45453686fe4d13e63376e3879a3d69d888078e72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mih=C3=A1ly=20Lengyel?= Date: Fri, 18 Oct 2024 01:03:14 +0200 Subject: [PATCH] feat!: removed overwriteSessionDuringSignInUp (#940) --- CHANGELOG.md | 2 +- lib/build/authUtils.js | 56 +--- lib/build/recipe/session/recipe.d.ts | 1 - lib/build/recipe/session/recipe.js | 8 - lib/build/recipe/session/types.d.ts | 2 - lib/build/recipe/session/utils.js | 2 - lib/ts/authUtils.ts | 37 +- lib/ts/recipe/session/recipe.ts | 9 +- lib/ts/recipe/session/types.ts | 2 - lib/ts/recipe/session/utils.ts | 1 - .../overwriteSessionDuringSignInUp.test.js | 317 ------------------ test/test-server/src/index.ts | 2 +- 12 files changed, 19 insertions(+), 420 deletions(-) delete mode 100644 test/session/overwriteSessionDuringSignInUp.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index a2c58402e..32eec5d61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Changes type of value in formField object to be `unknown` instead of `string` to add support for accepting any type of value in form fields. - Only supporting CDI 5.2, Compatible with Core version >= 10.0 -- Changed the default value of `overwriteSessionDuringSignInUp` to true. +- Removed the `overwriteSessionDuringSignInUp` option. - Added a new `shouldTryLinkingWithSessionUser` to sign in/up related APIs (and the related recipe functions) - This will default to false on the API - This will be set to true in function calls if you pass a session, otherwise it is set to false diff --git a/lib/build/authUtils.js b/lib/build/authUtils.js index b03e8cb94..252f94688 100644 --- a/lib/build/authUtils.js +++ b/lib/build/authUtils.js @@ -15,7 +15,6 @@ const utils_1 = require("./recipe/multifactorauth/utils"); const utils_2 = require("./recipe/multitenancy/utils"); const error_1 = __importDefault(require("./recipe/session/error")); const _1 = require("."); -const recipe_3 = __importDefault(require("./recipe/session/recipe")); const logger_1 = require("./logger"); const emailverification_1 = require("./recipe/emailverification"); const error_2 = __importDefault(require("./error")); @@ -231,36 +230,22 @@ exports.AuthUtils = { await multifactorauth_1.default.markFactorAsCompleteInSession(respSession, factorId, userContext); } } else { - logger_1.logDebugMessage(`postAuthChecks checking overwriteSessionDuringSignInUp`); - // If the new user wasn't linked to the current one, we check the config and overwrite the session if required + // If the new user wasn't linked to the current one, we overwrite the session // Note: we could also get here if MFA is enabled, but the app didn't want to link the user to the session user. - // This is intentional, since the MFA and overwriteSessionDuringSignInUp configs should work independently. - let overwriteSessionDuringSignInUp = recipe_3.default - .getInstanceOrThrowError() - .getNormalisedOverwriteSessionDuringSignInUp(req); - if (overwriteSessionDuringSignInUp) { - respSession = await session_1.default.createNewSession( - req, - res, - tenantId, - recipeUserId, - {}, - {}, - userContext - ); - if (mfaInstance !== undefined) { - await multifactorauth_1.default.markFactorAsCompleteInSession( - respSession, - factorId, - userContext - ); - } + respSession = await session_1.default.createNewSession( + req, + res, + tenantId, + recipeUserId, + {}, + {}, + userContext + ); + if (mfaInstance !== undefined) { + await multifactorauth_1.default.markFactorAsCompleteInSession(respSession, factorId, userContext); } } } else { - // We do not have to care about overwriting the session here, since we either: - // - have overwriteSessionDuringSignInUp true and we didn't even try to load the session because we ignore it anyway - // - have overwriteSessionDuringSignInUp false and we checked in the api imlp that there is no session logger_1.logDebugMessage(`postAuthChecks creating session for first factor sign in/up`); // If there is no input session, we do not need to do anything other checks and create a new session respSession = await session_1.default.createNewSession( @@ -901,9 +886,6 @@ exports.AuthUtils = { return validFactorIds; }, loadSessionInAuthAPIIfNeeded: async function (req, res, shouldTryLinkingWithSessionUser, userContext) { - const overwriteSessionDuringSignInUp = recipe_3.default - .getInstanceOrThrowError() - .getNormalisedOverwriteSessionDuringSignInUp(req); if (shouldTryLinkingWithSessionUser !== false) { logger_1.logDebugMessage( "loadSessionInAuthAPIIfNeeded: loading session because shouldTryLinkingWithSessionUser is not set to false so we may want to link later" @@ -920,20 +902,6 @@ exports.AuthUtils = { userContext ); } - if (overwriteSessionDuringSignInUp === false) { - logger_1.logDebugMessage( - "loadSessionInAuthAPIIfNeeded: loading session in optional mode because overwriteSessionDuringSignInUp is false so if it is not found we will skip session creation" - ); - return await session_1.default.getSession( - req, - res, - { - sessionRequired: false, - overrideGlobalClaimValidators: () => [], - }, - userContext - ); - } logger_1.logDebugMessage( "loadSessionInAuthAPIIfNeeded: skipping session loading because we are not linking and we would overwrite it anyway" ); diff --git a/lib/build/recipe/session/recipe.d.ts b/lib/build/recipe/session/recipe.d.ts index ecb163142..2880f6536 100644 --- a/lib/build/recipe/session/recipe.d.ts +++ b/lib/build/recipe/session/recipe.d.ts @@ -54,5 +54,4 @@ export default class SessionRecipe extends RecipeModule { response: BaseResponse, userContext: UserContext ) => Promise; - getNormalisedOverwriteSessionDuringSignInUp: (req: any) => boolean; } diff --git a/lib/build/recipe/session/recipe.js b/lib/build/recipe/session/recipe.js index fcec65ac9..39d82e0ed 100644 --- a/lib/build/recipe/session/recipe.js +++ b/lib/build/recipe/session/recipe.js @@ -178,14 +178,6 @@ class SessionRecipe extends recipeModule_1.default { userContext, }); }; - this.getNormalisedOverwriteSessionDuringSignInUp = (req) => { - var _a; - const supportsFDI31 = utils_2.hasGreaterThanEqualToFDI(req, "3.1"); - const res = - (_a = this.config.overwriteSessionDuringSignInUp) !== null && _a !== void 0 ? _a : supportsFDI31; - logger_1.logDebugMessage("getNormalisedOverwriteSessionDuringSignInUp returning: " + res); - return res; - }; this.config = utils_1.validateAndNormaliseUserInput(this, appInfo, config); const antiCsrfToLog = typeof this.config.antiCsrfFunctionOrString === "string" diff --git a/lib/build/recipe/session/types.d.ts b/lib/build/recipe/session/types.d.ts index b573925ed..14ef7e785 100644 --- a/lib/build/recipe/session/types.d.ts +++ b/lib/build/recipe/session/types.d.ts @@ -46,7 +46,6 @@ export declare type TypeInput = { cookieSameSite?: "strict" | "lax" | "none"; cookieDomain?: string; olderCookieDomain?: string; - overwriteSessionDuringSignInUp?: boolean; getTokenTransferMethod?: (input: { req: BaseRequest; forCreateNewSession: boolean; @@ -77,7 +76,6 @@ export declare type TypeNormalisedInput = { cookieSecure: boolean; sessionExpiredStatusCode: number; errorHandlers: NormalisedErrorHandlers; - overwriteSessionDuringSignInUp: boolean | undefined; antiCsrfFunctionOrString: | "VIA_TOKEN" | "VIA_CUSTOM_HEADER" diff --git a/lib/build/recipe/session/utils.js b/lib/build/recipe/session/utils.js index e4bfc3e2b..d39bbaf15 100644 --- a/lib/build/recipe/session/utils.js +++ b/lib/build/recipe/session/utils.js @@ -230,8 +230,6 @@ function validateAndNormaliseUserInput(recipeInstance, appInfo, config) { antiCsrfFunctionOrString: antiCsrf, override, invalidClaimStatusCode, - overwriteSessionDuringSignInUp: - config === null || config === void 0 ? void 0 : config.overwriteSessionDuringSignInUp, jwksRefreshIntervalSec: (_d = config === null || config === void 0 ? void 0 : config.jwksRefreshIntervalSec) !== null && _d !== void 0 diff --git a/lib/ts/authUtils.ts b/lib/ts/authUtils.ts index ae278847c..6ee636d19 100644 --- a/lib/ts/authUtils.ts +++ b/lib/ts/authUtils.ts @@ -13,7 +13,6 @@ import SessionError from "./recipe/session/error"; import { Error as STError, getUser } from "."; import { AccountInfoWithRecipeId } from "./recipe/accountlinking/types"; import { BaseRequest, BaseResponse } from "./framework"; -import SessionRecipe from "./recipe/session/recipe"; import { logDebugMessage } from "./logger"; import { EmailVerificationClaim } from "./recipe/emailverification"; import SuperTokensError from "./error"; @@ -276,24 +275,14 @@ export const AuthUtils = { await MultiFactorAuth.markFactorAsCompleteInSession(respSession!, factorId, userContext); } } else { - logDebugMessage(`postAuthChecks checking overwriteSessionDuringSignInUp`); - // If the new user wasn't linked to the current one, we check the config and overwrite the session if required + // If the new user wasn't linked to the current one, we overwrite the session // Note: we could also get here if MFA is enabled, but the app didn't want to link the user to the session user. - // This is intentional, since the MFA and overwriteSessionDuringSignInUp configs should work independently. - let overwriteSessionDuringSignInUp = SessionRecipe.getInstanceOrThrowError().getNormalisedOverwriteSessionDuringSignInUp( - req - ); - if (overwriteSessionDuringSignInUp) { - respSession = await Session.createNewSession(req, res, tenantId, recipeUserId, {}, {}, userContext); - if (mfaInstance !== undefined) { - await MultiFactorAuth.markFactorAsCompleteInSession(respSession!, factorId, userContext); - } + respSession = await Session.createNewSession(req, res, tenantId, recipeUserId, {}, {}, userContext); + if (mfaInstance !== undefined) { + await MultiFactorAuth.markFactorAsCompleteInSession(respSession!, factorId, userContext); } } } else { - // We do not have to care about overwriting the session here, since we either: - // - have overwriteSessionDuringSignInUp true and we didn't even try to load the session because we ignore it anyway - // - have overwriteSessionDuringSignInUp false and we checked in the api imlp that there is no session logDebugMessage(`postAuthChecks creating session for first factor sign in/up`); // If there is no input session, we do not need to do anything other checks and create a new session respSession = await Session.createNewSession(req, res, tenantId, recipeUserId, {}, {}, userContext); @@ -1024,10 +1013,6 @@ export const AuthUtils = { shouldTryLinkingWithSessionUser: boolean | undefined, userContext: UserContext ) { - const overwriteSessionDuringSignInUp = SessionRecipe.getInstanceOrThrowError().getNormalisedOverwriteSessionDuringSignInUp( - req - ); - if (shouldTryLinkingWithSessionUser !== false) { logDebugMessage( "loadSessionInAuthAPIIfNeeded: loading session because shouldTryLinkingWithSessionUser is not set to false so we may want to link later" @@ -1045,20 +1030,6 @@ export const AuthUtils = { ); } - if (overwriteSessionDuringSignInUp === false) { - logDebugMessage( - "loadSessionInAuthAPIIfNeeded: loading session in optional mode because overwriteSessionDuringSignInUp is false so if it is not found we will skip session creation" - ); - return await Session.getSession( - req, - res, - { - sessionRequired: false, - overrideGlobalClaimValidators: () => [], - }, - userContext - ); - } logDebugMessage( "loadSessionInAuthAPIIfNeeded: skipping session loading because we are not linking and we would overwrite it anyway" ); diff --git a/lib/ts/recipe/session/recipe.ts b/lib/ts/recipe/session/recipe.ts index 7dd003e3a..612eb7d73 100644 --- a/lib/ts/recipe/session/recipe.ts +++ b/lib/ts/recipe/session/recipe.ts @@ -42,7 +42,7 @@ import OverrideableBuilder from "supertokens-js-override"; import { APIOptions } from "."; import { logDebugMessage } from "../../logger"; import { resetCombinedJWKS } from "../../combinedRemoteJWKSet"; -import { hasGreaterThanEqualToFDI, isTestEnv } from "../../utils"; +import { isTestEnv } from "../../utils"; // For Express export default class SessionRecipe extends RecipeModule { @@ -272,11 +272,4 @@ export default class SessionRecipe extends RecipeModule { userContext, }); }; - - getNormalisedOverwriteSessionDuringSignInUp = (req: any) => { - const supportsFDI31 = hasGreaterThanEqualToFDI(req, "3.1"); - const res = this.config.overwriteSessionDuringSignInUp ?? supportsFDI31; - logDebugMessage("getNormalisedOverwriteSessionDuringSignInUp returning: " + res); - return res; - }; } diff --git a/lib/ts/recipe/session/types.ts b/lib/ts/recipe/session/types.ts index accd318bc..371cdc34e 100644 --- a/lib/ts/recipe/session/types.ts +++ b/lib/ts/recipe/session/types.ts @@ -68,7 +68,6 @@ export type TypeInput = { cookieSameSite?: "strict" | "lax" | "none"; cookieDomain?: string; olderCookieDomain?: string; - overwriteSessionDuringSignInUp?: boolean; getTokenTransferMethod?: (input: { req: BaseRequest; @@ -102,7 +101,6 @@ export type TypeNormalisedInput = { cookieSecure: boolean; sessionExpiredStatusCode: number; errorHandlers: NormalisedErrorHandlers; - overwriteSessionDuringSignInUp: boolean | undefined; antiCsrfFunctionOrString: | "VIA_TOKEN" diff --git a/lib/ts/recipe/session/utils.ts b/lib/ts/recipe/session/utils.ts index 8a2538c49..44f972dc0 100644 --- a/lib/ts/recipe/session/utils.ts +++ b/lib/ts/recipe/session/utils.ts @@ -303,7 +303,6 @@ export function validateAndNormaliseUserInput( antiCsrfFunctionOrString: antiCsrf, override, invalidClaimStatusCode, - overwriteSessionDuringSignInUp: config?.overwriteSessionDuringSignInUp, jwksRefreshIntervalSec: config?.jwksRefreshIntervalSec ?? 3600 * 4, }; } diff --git a/test/session/overwriteSessionDuringSignInUp.test.js b/test/session/overwriteSessionDuringSignInUp.test.js deleted file mode 100644 index dd397811a..000000000 --- a/test/session/overwriteSessionDuringSignInUp.test.js +++ /dev/null @@ -1,317 +0,0 @@ -/* Copyright (c) 2024, VRAI Labs and/or its affiliates. All rights reserved. - * - * This software is licensed under the Apache License, Version 2.0 (the - * "License") as published by the Apache Software Foundation. - * - * You may not use this file except in compliance with the License. You may - * obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the - * License for the specific language governing permissions and limitations - * under the License. - */ - -const { printPath, setupST, startST, killAllST, cleanST, extractInfoFromResponse, resetAll } = require("../utils"); -const assert = require("assert"); -const { Querier } = require("../../lib/build/querier"); -const express = require("express"); -const request = require("supertest"); -const { ProcessState, PROCESS_STATE } = require("../../lib/build/processState"); -const SuperTokens = require("../../"); -const Session = require("../../recipe/session"); -const EmailPassword = require("../../recipe/emailpassword"); -const { middleware, errorHandler } = require("../../framework/express"); -const { json } = require("body-parser"); - -describe(`overwriteSessionDuringSignInUp config: ${printPath( - "[test/session/overwriteSessionDuringSignInUp.test.js]" -)}`, function () { - beforeEach(async function () { - await killAllST(); - await setupST(); - ProcessState.getInstance().reset(); - }); - - after(async function () { - await killAllST(); - await cleanST(); - }); - - describe("createNewSession", () => { - it("test default", async function () { - const connectionURI = await startST(); - SuperTokens.init({ - supertokens: { - connectionURI, - }, - appInfo: { - apiDomain: "api.supertokens.io", - appName: "SuperTokens", - websiteDomain: "supertokens.io", - }, - recipeList: [EmailPassword.init(), Session.init({})], - }); - - const app = getTestExpressApp(); - - await EmailPassword.signUp("public", "test@example.com", "password"); - await EmailPassword.signUp("public", "test2@example.com", "password"); - - let res = await new Promise((resolve) => - request(app) - .post("/auth/signin") - .send({ - formFields: [ - { - id: "password", - value: "password", - }, - { - id: "email", - value: "test@example.com", - }, - ], - }) - .expect(200) - .end((err, res) => { - if (err) { - resolve(undefined); - } else { - resolve(res); - } - }) - ); - - let cookies = extractInfoFromResponse(res); - - assert(cookies.accessTokenFromAny !== undefined); - assert(cookies.refreshTokenFromAny !== undefined); - assert(cookies.frontToken !== undefined); - - let accessToken = cookies.accessTokenFromAny; - - res = await new Promise((resolve) => - request(app) - .post("/auth/signin") - .set("Authorization", "Bearer " + accessToken) - .send({ - formFields: [ - { - id: "password", - value: "password", - }, - { - id: "email", - value: "test2@example.com", - }, - ], - }) - .expect(200) - .end((err, res) => { - if (err) { - resolve(undefined); - } else { - resolve(res); - } - }) - ); - - cookies = extractInfoFromResponse(res); - assert.notStrictEqual(cookies.accessTokenFromAny, undefined); - }); - - it("test false", async function () { - const connectionURI = await startST(); - SuperTokens.init({ - supertokens: { - connectionURI, - }, - appInfo: { - apiDomain: "api.supertokens.io", - appName: "SuperTokens", - websiteDomain: "supertokens.io", - }, - recipeList: [EmailPassword.init(), Session.init({ overwriteSessionDuringSignInUp: false })], - }); - - const app = getTestExpressApp(); - - await EmailPassword.signUp("public", "test@example.com", "password"); - await EmailPassword.signUp("public", "test2@example.com", "password"); - - let res = await new Promise((resolve) => - request(app) - .post("/auth/signin") - .send({ - formFields: [ - { - id: "password", - value: "password", - }, - { - id: "email", - value: "test@example.com", - }, - ], - }) - .expect(200) - .end((err, res) => { - if (err) { - resolve(undefined); - } else { - resolve(res); - } - }) - ); - - let cookies = extractInfoFromResponse(res); - - assert(cookies.accessTokenFromAny !== undefined); - assert(cookies.refreshTokenFromAny !== undefined); - assert(cookies.frontToken !== undefined); - - let accessToken = cookies.accessTokenFromAny; - - res = await new Promise((resolve) => - request(app) - .post("/auth/signin") - .set("Authorization", "Bearer " + accessToken) - .send({ - formFields: [ - { - id: "password", - value: "password", - }, - { - id: "email", - value: "test2@example.com", - }, - ], - }) - .expect(200) - .end((err, res) => { - if (err) { - resolve(undefined); - } else { - resolve(res); - } - }) - ); - - cookies = extractInfoFromResponse(res); - assert(cookies.accessTokenFromAny === undefined); - }); - - it("test true", async function () { - const connectionURI = await startST(); - SuperTokens.init({ - supertokens: { - connectionURI, - }, - appInfo: { - apiDomain: "api.supertokens.io", - appName: "SuperTokens", - websiteDomain: "supertokens.io", - }, - recipeList: [EmailPassword.init(), Session.init({ overwriteSessionDuringSignInUp: true })], - }); - - const app = getTestExpressApp(); - - await EmailPassword.signUp("public", "test@example.com", "password"); - await EmailPassword.signUp("public", "test2@example.com", "password"); - - let res = await new Promise((resolve) => - request(app) - .post("/auth/signin") - .send({ - formFields: [ - { - id: "password", - value: "password", - }, - { - id: "email", - value: "test@example.com", - }, - ], - }) - .expect(200) - .end((err, res) => { - if (err) { - resolve(undefined); - } else { - resolve(res); - } - }) - ); - - let cookies = extractInfoFromResponse(res); - - assert(cookies.accessTokenFromAny !== undefined); - assert(cookies.refreshTokenFromAny !== undefined); - assert(cookies.frontToken !== undefined); - - let accessToken = cookies.accessTokenFromAny; - - res = await new Promise((resolve) => - request(app) - .post("/auth/signin") - .set("Authorization", "Bearer " + accessToken) - .send({ - formFields: [ - { - id: "password", - value: "password", - }, - { - id: "email", - value: "test2@example.com", - }, - ], - }) - .expect(200) - .end((err, res) => { - if (err) { - resolve(undefined); - } else { - resolve(res); - } - }) - ); - - cookies = extractInfoFromResponse(res); - assert.notStrictEqual(cookies.accessTokenFromAny, undefined); - }); - }); -}); - -function getTestExpressApp() { - const app = express(); - - app.use(middleware()); - app.use(json()); - - app.post("/create", async (req, res) => { - const userId = req.body.userId || ""; - try { - await Session.createNewSession( - req, - res, - "public", - SuperTokens.convertToRecipeUserId(userId), - req.body.payload, - {}, - false // alwaysOverwriteSessionInRequest - ); - res.status(200).send(""); - } catch (ex) { - res.status(400).json({ message: ex.message }); - } - }); - - app.use(errorHandler()); - return app; -} diff --git a/test/test-server/src/index.ts b/test/test-server/src/index.ts index e41f2f41e..3aa41da73 100644 --- a/test/test-server/src/index.ts +++ b/test/test-server/src/index.ts @@ -386,7 +386,7 @@ app.get("/test/overrideparams", async (req, res, next) => { }); app.get("/test/featureflag", async (req, res, next) => { - res.json([]); + res.json(["removedOverwriteSessionDuringSignInUp"]); }); app.post("/test/resetoverrideparams", async (req, res, next) => {