fix: thirdparty recursion and config reference issues (#846)

CHANGELOG.md

@@ -33,6 +33,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 -   If you were using `ThirdPartyPasswordless`, you should now init `ThirdParty` and `Passwordless` recipes separately. The config for the individual recipes are mostly the same, except the syntax may be different. Check our recipe guides for [ThirdParty](https://supertokens.com/docs/thirdparty/introduction) and [Passwordless](https://supertokens.com/docs/passwordless/introduction) for more information.
 
+### Fixes
+
+-   Fixes override recursion build-up in built-in providers due to the modification of the `input.override` object in the ThirdParty providers list.
+-   Fixes issue with reference to `config` object in `TypeProvider` in the provider override. The issue was the `originalImplementation.config` object did not have the updated `config` values that was being used in the provider implementation.
+
 ## [17.1.2] - 2024-05-21
 
 ### Fixes

lib/build/recipe/thirdparty/providers/configUtils.js

@@ -15,38 +15,53 @@ function getProviderConfigForClient(providerConfig, clientConfig) {
 exports.getProviderConfigForClient = getProviderConfigForClient;
 async function fetchAndSetConfig(provider, clientType, userContext) {
     let config = await provider.getConfigForClientType({ clientType, userContext });
-    config = await utils_1.discoverOIDCEndpoints(config);
-    provider.config = config;
+    await utils_1.discoverOIDCEndpoints(config);
+    // We are doing Object.assign here to retain the reference to the provider.config which originally came from
+    // the impl object in the `custom.ts`. We are doing this because if we replace the provider.config with the
+    // new config object, then when the user overrides a provider function, and access the `originalImplementaion.config`,
+    // it will not receive the updated config, since the user's override would have returned a new provider object and
+    // the config is being set on to it. By doing Object.assign, we retain the original reference and the config is
+    // consistently available in the override as well as the implementation.
+    Object.assign(provider.config, config);
 }
 function createProvider(input) {
-    if (input.config.thirdPartyId.startsWith("active-directory")) {
-        return _1.ActiveDirectory(input);
-    } else if (input.config.thirdPartyId.startsWith("apple")) {
-        return _1.Apple(input);
-    } else if (input.config.thirdPartyId.startsWith("bitbucket")) {
-        return _1.Bitbucket(input);
-    } else if (input.config.thirdPartyId.startsWith("discord")) {
-        return _1.Discord(input);
-    } else if (input.config.thirdPartyId.startsWith("facebook")) {
-        return _1.Facebook(input);
-    } else if (input.config.thirdPartyId.startsWith("github")) {
-        return _1.Github(input);
-    } else if (input.config.thirdPartyId.startsWith("gitlab")) {
-        return _1.Gitlab(input);
-    } else if (input.config.thirdPartyId.startsWith("google-workspaces")) {
-        return _1.GoogleWorkspaces(input);
-    } else if (input.config.thirdPartyId.startsWith("google")) {
-        return _1.Google(input);
-    } else if (input.config.thirdPartyId.startsWith("okta")) {
-        return _1.Okta(input);
-    } else if (input.config.thirdPartyId.startsWith("linkedin")) {
-        return _1.Linkedin(input);
-    } else if (input.config.thirdPartyId.startsWith("twitter")) {
-        return _1.Twitter(input);
-    } else if (input.config.thirdPartyId.startsWith("boxy-saml")) {
-        return _1.BoxySAML(input);
+    // We are cloning the input object because the built-in providers modify the `override` in the input
+    // object, and we don't want to modify the original input object. If we do not clone the input object,
+    // we add a new function for override and then call the override from the input object, and this way,
+    // the override call stack keeps increasing over time. After a certain number of calls, the call stack
+    // will overflow and the thirdparty provider will start failing.
+    // We are only doing a shallow clone because the `override` is in the top level of the input object,
+    // and other properties are just used for reading. The input object is also not exposed by any of the
+    // interface for modification.
+    const clonedInput = Object.assign({}, input);
+    if (clonedInput.config.thirdPartyId.startsWith("active-directory")) {
+        return _1.ActiveDirectory(clonedInput);
+    } else if (clonedInput.config.thirdPartyId.startsWith("apple")) {
+        return _1.Apple(clonedInput);
+    } else if (clonedInput.config.thirdPartyId.startsWith("bitbucket")) {
+        return _1.Bitbucket(clonedInput);
+    } else if (clonedInput.config.thirdPartyId.startsWith("discord")) {
+        return _1.Discord(clonedInput);
+    } else if (clonedInput.config.thirdPartyId.startsWith("facebook")) {
+        return _1.Facebook(clonedInput);
+    } else if (clonedInput.config.thirdPartyId.startsWith("github")) {
+        return _1.Github(clonedInput);
+    } else if (clonedInput.config.thirdPartyId.startsWith("gitlab")) {
+        return _1.Gitlab(clonedInput);
+    } else if (clonedInput.config.thirdPartyId.startsWith("google-workspaces")) {
+        return _1.GoogleWorkspaces(clonedInput);
+    } else if (clonedInput.config.thirdPartyId.startsWith("google")) {
+        return _1.Google(clonedInput);
+    } else if (clonedInput.config.thirdPartyId.startsWith("okta")) {
+        return _1.Okta(clonedInput);
+    } else if (clonedInput.config.thirdPartyId.startsWith("linkedin")) {
+        return _1.Linkedin(clonedInput);
+    } else if (clonedInput.config.thirdPartyId.startsWith("twitter")) {
+        return _1.Twitter(clonedInput);
+    } else if (clonedInput.config.thirdPartyId.startsWith("boxy-saml")) {
+        return _1.BoxySAML(clonedInput);
     }
-    return custom_1.default(input);
+    return custom_1.default(clonedInput);
 }
 async function findAndCreateProviderInstance(providers, thirdPartyId, clientType, userContext) {
     for (const providerInput of providers) {

lib/build/recipe/thirdparty/providers/utils.d.ts

@@ -32,6 +32,4 @@ export declare function verifyIdTokenFromJWKSEndpointAndGetPayload(
     jwks: jose.JWTVerifyGetKey,
     otherOptions: jose.JWTVerifyOptions
 ): Promise<any>;
-export declare function discoverOIDCEndpoints(
-    config: ProviderConfigForClientType
-): Promise<ProviderConfigForClientType>;
+export declare function discoverOIDCEndpoints(config: ProviderConfigForClientType): Promise<void>;

lib/build/recipe/thirdparty/providers/utils.js

@@ -143,6 +143,5 @@ async function discoverOIDCEndpoints(config) {
             config.jwksURI = oidcInfo.jwks_uri;
         }
     }
-    return config;
 }
 exports.discoverOIDCEndpoints = discoverOIDCEndpoints;

lib/ts/recipe/thirdparty/providers/configUtils.ts

@@ -37,41 +37,57 @@ export function getProviderConfigForClient(
 async function fetchAndSetConfig(provider: TypeProvider, clientType: string | undefined, userContext: UserContext) {
     let config = await provider.getConfigForClientType({ clientType, userContext });
 
-    config = await discoverOIDCEndpoints(config);
-
-    provider.config = config;
+    await discoverOIDCEndpoints(config);
+
+    // We are doing Object.assign here to retain the reference to the provider.config which originally came from
+    // the impl object in the `custom.ts`. We are doing this because if we replace the provider.config with the
+    // new config object, then when the user overrides a provider function, and access the `originalImplementaion.config`,
+    // it will not receive the updated config, since the user's override would have returned a new provider object and
+    // the config is being set on to it. By doing Object.assign, we retain the original reference and the config is
+    // consistently available in the override as well as the implementation.
+    Object.assign(provider.config, config);
 }
 
 function createProvider(input: ProviderInput): TypeProvider {
-    if (input.config.thirdPartyId.startsWith("active-directory")) {
-        return ActiveDirectory(input);
-    } else if (input.config.thirdPartyId.startsWith("apple")) {
-        return Apple(input);
-    } else if (input.config.thirdPartyId.startsWith("bitbucket")) {
-        return Bitbucket(input);
-    } else if (input.config.thirdPartyId.startsWith("discord")) {
-        return Discord(input);
-    } else if (input.config.thirdPartyId.startsWith("facebook")) {
-        return Facebook(input);
-    } else if (input.config.thirdPartyId.startsWith("github")) {
-        return Github(input);
-    } else if (input.config.thirdPartyId.startsWith("gitlab")) {
-        return Gitlab(input);
-    } else if (input.config.thirdPartyId.startsWith("google-workspaces")) {
-        return GoogleWorkspaces(input);
-    } else if (input.config.thirdPartyId.startsWith("google")) {
-        return Google(input);
-    } else if (input.config.thirdPartyId.startsWith("okta")) {
-        return Okta(input);
-    } else if (input.config.thirdPartyId.startsWith("linkedin")) {
-        return Linkedin(input);
-    } else if (input.config.thirdPartyId.startsWith("twitter")) {
-        return Twitter(input);
-    } else if (input.config.thirdPartyId.startsWith("boxy-saml")) {
-        return BoxySAML(input);
+    // We are cloning the input object because the built-in providers modify the `override` in the input
+    // object, and we don't want to modify the original input object. If we do not clone the input object,
+    // we add a new function for override and then call the override from the input object, and this way,
+    // the override call stack keeps increasing over time. After a certain number of calls, the call stack
+    // will overflow and the thirdparty provider will start failing.
+    // We are only doing a shallow clone because the `override` is in the top level of the input object,
+    // and other properties are just used for reading. The input object is also not exposed by any of the
+    // interface for modification.
+    const clonedInput = { ...input };
+
+    if (clonedInput.config.thirdPartyId.startsWith("active-directory")) {
+        return ActiveDirectory(clonedInput);
+    } else if (clonedInput.config.thirdPartyId.startsWith("apple")) {
+        return Apple(clonedInput);
+    } else if (clonedInput.config.thirdPartyId.startsWith("bitbucket")) {
+        return Bitbucket(clonedInput);
+    } else if (clonedInput.config.thirdPartyId.startsWith("discord")) {
+        return Discord(clonedInput);
+    } else if (clonedInput.config.thirdPartyId.startsWith("facebook")) {
+        return Facebook(clonedInput);
+    } else if (clonedInput.config.thirdPartyId.startsWith("github")) {
+        return Github(clonedInput);
+    } else if (clonedInput.config.thirdPartyId.startsWith("gitlab")) {
+        return Gitlab(clonedInput);
+    } else if (clonedInput.config.thirdPartyId.startsWith("google-workspaces")) {
+        return GoogleWorkspaces(clonedInput);
+    } else if (clonedInput.config.thirdPartyId.startsWith("google")) {
+        return Google(clonedInput);
+    } else if (clonedInput.config.thirdPartyId.startsWith("okta")) {
+        return Okta(clonedInput);
+    } else if (clonedInput.config.thirdPartyId.startsWith("linkedin")) {
+        return Linkedin(clonedInput);
+    } else if (clonedInput.config.thirdPartyId.startsWith("twitter")) {
+        return Twitter(clonedInput);
+    } else if (clonedInput.config.thirdPartyId.startsWith("boxy-saml")) {
+        return BoxySAML(clonedInput);
     }
 
-    return NewProvider(input);
+    return NewProvider(clonedInput);
 }
 
 export async function findAndCreateProviderInstance(

lib/ts/recipe/thirdparty/providers/utils.ts

@@ -123,7 +123,7 @@ async function getOIDCDiscoveryInfo(issuer: string): Promise<any> {
     return oidcInfo.jsonResponse!;
 }
 
-export async function discoverOIDCEndpoints(config: ProviderConfigForClientType): Promise<ProviderConfigForClientType> {
+export async function discoverOIDCEndpoints(config: ProviderConfigForClientType): Promise<void> {
     if (config.oidcDiscoveryEndpoint !== undefined) {
         const oidcInfo = await getOIDCDiscoveryInfo(config.oidcDiscoveryEndpoint);
 
@@ -143,6 +143,4 @@ export async function discoverOIDCEndpoints(config: ProviderConfigForClientType)
             config.jwksURI = oidcInfo.jwks_uri;
         }
     }
-
-    return config;
 }

test/thirdparty/provider.config.test.js

@@ -39,6 +39,159 @@ describe(`providerConfigTest: ${printPath("[test/thirdparty/provider.config.test
         await cleanST();
     });
 
+    it("test the provider override is not increasing call stack", async function () {
+        const connectionURI = await startSTWithMultitenancy();
+
+        let stackCount = [];
+
+        STExpress.init({
+            supertokens: {
+                connectionURI,
+            },
+            appInfo: {
+                apiDomain: "api.supertokens.io",
+                appName: "SuperTokens",
+                websiteDomain: "supertokens.io",
+            },
+            recipeList: [
+                ThirdPartyRecipe.init({
+                    signInAndUpFeature: {
+                        providers: [
+                            {
+                                config: {
+                                    thirdPartyId: "google",
+                                    clients: [{ clientId: "test", clientSecret: "secret" }],
+                                },
+                                override: (oI) => {
+                                    const err = new Error();
+                                    stackCount.push(err.stack.split("\n").length);
+                                    return {
+                                        ...oI,
+                                        getAuthorisationRedirectURL: async (input) => {
+                                            return await oI.getAuthorisationRedirectURL(input);
+                                        },
+                                    };
+                                },
+                            },
+                        ],
+                    },
+                }),
+            ],
+        });
+
+        for (let i = 0; i < 10; i++) {
+            await (await ThirdParty.getProvider("public", "google")).getAuthorisationRedirectURL({
+                redirectURIOnProviderDashboard: "http://localhost:8080",
+                userContext: {},
+            });
+        }
+
+        for (let i = 1; i < stackCount.length; i++) {
+            assert.equal(stackCount[i], stackCount[i - 1]); // stack should be same and not increasing
+        }
+    });
+
+    it("test the config is same within override and provider instance", async function () {
+        const connectionURI = await startSTWithMultitenancy();
+
+        let oIConfig;
+        STExpress.init({
+            supertokens: {
+                connectionURI,
+            },
+            appInfo: {
+                apiDomain: "api.supertokens.io",
+                appName: "SuperTokens",
+                websiteDomain: "supertokens.io",
+            },
+            recipeList: [
+                ThirdPartyRecipe.init({
+                    signInAndUpFeature: {
+                        providers: [
+                            {
+                                config: {
+                                    thirdPartyId: "google",
+                                    clients: [{ clientId: "test", clientSecret: "secret" }],
+                                },
+                                override: (oI) => {
+                                    return {
+                                        ...oI,
+                                        getAuthorisationRedirectURL: async (input) => {
+                                            oIConfig = oI.config;
+                                            return await oI.getAuthorisationRedirectURL(input);
+                                        },
+                                    };
+                                },
+                            },
+                        ],
+                    },
+                }),
+            ],
+        });
+
+        const provider = await ThirdParty.getProvider("public", "google");
+        await provider.getAuthorisationRedirectURL({
+            redirectURIOnProviderDashboard: "http://localhost:8080",
+            userContext: {},
+        });
+
+        assert.strictEqual(provider.config, oIConfig);
+    });
+
+    it("test the config is same within override and provider instance when overriding getConfigForClientType", async function () {
+        const connectionURI = await startSTWithMultitenancy();
+
+        let oIConfig;
+        STExpress.init({
+            supertokens: {
+                connectionURI,
+            },
+            appInfo: {
+                apiDomain: "api.supertokens.io",
+                appName: "SuperTokens",
+                websiteDomain: "supertokens.io",
+            },
+            recipeList: [
+                ThirdPartyRecipe.init({
+                    signInAndUpFeature: {
+                        providers: [
+                            {
+                                config: {
+                                    thirdPartyId: "google",
+                                    clients: [{ clientId: "test", clientSecret: "secret" }],
+                                },
+                                override: (oI) => {
+                                    return {
+                                        ...oI,
+                                        getConfigForClientType: async (input) => {
+                                            return {
+                                                id: "google",
+                                                clientId: "hello",
+                                                authorizationEndpoint: "http://localhost:8080/authorize",
+                                            };
+                                        },
+                                        getAuthorisationRedirectURL: async (input) => {
+                                            oIConfig = oI.config;
+                                            return await oI.getAuthorisationRedirectURL(input);
+                                        },
+                                    };
+                                },
+                            },
+                        ],
+                    },
+                }),
+            ],
+        });
+
+        const provider = await ThirdParty.getProvider("public", "google");
+        await provider.getAuthorisationRedirectURL({
+            redirectURIOnProviderDashboard: "http://localhost:8080",
+            userContext: {},
+        });
+
+        assert.strictEqual(provider.config, oIConfig);
+    });
+
     it("test built-in provider computed config from static config", async function () {
         const connectionURI = await startSTWithMultitenancy();