From 76968e6cdbbbae4266447e5de5ff66420907e9b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mih=C3=A1ly=20Lengyel?= Date: Sun, 27 Oct 2024 02:17:35 +0100 Subject: [PATCH] fix: better error handling in oauth flows (#862) --- lib/build/index2.js | 3 +- lib/build/oauth2providerprebuiltui.js | 19 +++- .../components/feature/authPage/authPage.tsx | 3 +- .../features/oauth2LogoutScreen/index.tsx | 13 ++- test/end-to-end/oauth2provider.test.js | 93 +++++++++++++++++++ 5 files changed, 123 insertions(+), 8 deletions(-) diff --git a/lib/build/index2.js b/lib/build/index2.js index 5acf21394..81099efc5 100644 --- a/lib/build/index2.js +++ b/lib/build/index2.js @@ -1002,7 +1002,8 @@ var AuthPageInner = function (props) { }); }, function () { - return genericComponentOverrideContext.clearQueryParams(["loginChallenge"]); + genericComponentOverrideContext.clearQueryParams(["loginChallenge"]); + setError("SOMETHING_WENT_WRONG_ERROR"); } ); React.useEffect( diff --git a/lib/build/oauth2providerprebuiltui.js b/lib/build/oauth2providerprebuiltui.js index fb958740c..7f7a99de6 100644 --- a/lib/build/oauth2providerprebuiltui.js +++ b/lib/build/oauth2providerprebuiltui.js @@ -223,7 +223,7 @@ var OAuth2LogoutScreen = function (props) { setIsLoggingOut(true); _a.label = 1; case 1: - _a.trys.push([1, 4, , 5]); + _a.trys.push([1, 4, , 6]); return [ 4 /*yield*/, recipe.OAuth2Provider.getInstanceOrThrow().webJSRecipe.logOut({ @@ -250,12 +250,23 @@ var OAuth2LogoutScreen = function (props) { ]; case 3: _a.sent(); - return [3 /*break*/, 5]; + return [3 /*break*/, 6]; case 4: err_1 = _a.sent(); - rethrowInRender(err_1); - return [3 /*break*/, 5]; + return [4 /*yield*/, session.doesSessionExist(userContext)]; case 5: + if (!_a.sent()) { + void genericComponentOverrideContext.SuperTokens.getInstanceOrThrow() + .redirectToAuth({ + userContext: userContext, + redirectBack: false, + }) + .catch(rethrowInRender); + } else { + rethrowInRender(err_1); + } + return [3 /*break*/, 6]; + case 6: return [2 /*return*/]; } }); diff --git a/lib/ts/recipe/authRecipe/components/feature/authPage/authPage.tsx b/lib/ts/recipe/authRecipe/components/feature/authPage/authPage.tsx index 9027d10b8..6ce5705c2 100644 --- a/lib/ts/recipe/authRecipe/components/feature/authPage/authPage.tsx +++ b/lib/ts/recipe/authRecipe/components/feature/authPage/authPage.tsx @@ -197,7 +197,8 @@ const AuthPageInner: React.FC = (props) => { } }, () => { - return clearQueryParams(["loginChallenge"]); + clearQueryParams(["loginChallenge"]); + setError("SOMETHING_WENT_WRONG_ERROR"); } ); diff --git a/lib/ts/recipe/oauth2provider/components/features/oauth2LogoutScreen/index.tsx b/lib/ts/recipe/oauth2provider/components/features/oauth2LogoutScreen/index.tsx index 175a56e8e..56fb9f067 100644 --- a/lib/ts/recipe/oauth2provider/components/features/oauth2LogoutScreen/index.tsx +++ b/lib/ts/recipe/oauth2provider/components/features/oauth2LogoutScreen/index.tsx @@ -25,7 +25,7 @@ import SuperTokens from "../../../../../superTokens"; import UI from "../../../../../ui"; import { useUserContext } from "../../../../../usercontext"; import { getQueryParams, getTenantIdFromQueryParams, useRethrowInRender } from "../../../../../utils"; -import { SessionContext } from "../../../../session"; +import { doesSessionExist, SessionContext } from "../../../../session"; import OAuth2Provider from "../../../recipe"; import { OAuth2LogoutScreenTheme } from "../../themes/oauth2LogoutScreen"; import { defaultTranslationsOAuth2Provider } from "../../themes/translations"; @@ -75,7 +75,16 @@ export const OAuth2LogoutScreen: React.FC = (props) => { userContext ); } catch (err: any) { - rethrowInRender(err); + if (!(await doesSessionExist(userContext))) { + void SuperTokens.getInstanceOrThrow() + .redirectToAuth({ + userContext, + redirectBack: false, + }) + .catch(rethrowInRender); + } else { + rethrowInRender(err); + } } }, [logoutChallenge, navigate, props.recipe, userContext, rethrowInRender]); diff --git a/test/end-to-end/oauth2provider.test.js b/test/end-to-end/oauth2provider.test.js index 4e2a052f8..df23aab95 100644 --- a/test/end-to-end/oauth2provider.test.js +++ b/test/end-to-end/oauth2provider.test.js @@ -40,6 +40,7 @@ import { waitForSTElement, isOauth2Supported, setupBrowser, + getGeneralError, } from "../helpers"; import fetch from "isomorphic-fetch"; @@ -48,6 +49,7 @@ import { TEST_SERVER_BASE_URL, SIGN_OUT_API, TEST_APPLICATION_SERVER_BASE_URL, + SOMETHING_WENT_WRONG_ERROR, } from "../constants"; // We do no thave to use a separate domain for the oauth2 client, since the way we are testing @@ -120,6 +122,36 @@ describe("SuperTokens OAuth2Provider", function () { await removeOAuth2ClientInfo(page); }); + it("should clear invalid/expired loginChallenge from the url and show an error", async function () { + const { client } = await createOAuth2Client({ + scope: "offline_access profile openid email", + redirectUris: [`${TEST_CLIENT_BASE_URL}/oauth/callback`], + tokenEndpointAuthMethod: "none", + grantTypes: ["authorization_code", "refresh_token"], + responseTypes: ["code", "id_token"], + }); + + await setOAuth2ClientInfo(page, client.clientId); + + await Promise.all([ + page.waitForNavigation({ waitUntil: "networkidle0" }), + page.goto(`${TEST_CLIENT_BASE_URL}/oauth/login`), + ]); + + let loginButton = await getOAuth2LoginButton(page); + await loginButton.click(); + + await waitForUrl(page, "/auth"); + + await Promise.all([ + page.waitForNavigation({ waitUntil: "networkidle0" }), + page.goto(page.url().replace("loginChallenge=", "loginChallenge=nooope")), + ]); + + const error = await getGeneralError(page); + assert.strictEqual(error, SOMETHING_WENT_WRONG_ERROR); + }); + it("should successfully complete the OAuth2 flow", async function () { const { client } = await createOAuth2Client({ scope: "offline_access profile openid email", @@ -165,6 +197,67 @@ describe("SuperTokens OAuth2Provider", function () { await waitForUrl(page, "/oauth/callback"); }); + it("should handle invalid logoutChallenge in the OAuth2 Logout flow", async function () { + const postLogoutRedirectUri = `${TEST_CLIENT_BASE_URL}/oauth/login?logout=true`; + + const { client } = await createOAuth2Client({ + scope: "offline_access profile openid email", + redirectUris: [`${TEST_CLIENT_BASE_URL}/oauth/callback`], + postLogoutRedirectUris: [postLogoutRedirectUri], + accessTokenStrategy: "jwt", + tokenEndpointAuthMethod: "none", + grantTypes: ["authorization_code", "refresh_token"], + responseTypes: ["code", "id_token"], + skipConsent: true, + }); + + await setOAuth2ClientInfo( + page, + client.clientId, + undefined, + undefined, + undefined, + JSON.stringify({ + post_logout_redirect_uri: postLogoutRedirectUri, + }) + ); + + await Promise.all([ + page.waitForNavigation({ waitUntil: "networkidle0" }), + page.goto(`${TEST_CLIENT_BASE_URL}/oauth/login`), + ]); + + let loginButton = await getOAuth2LoginButton(page); + await loginButton.click(); + + await waitForUrl(page, "/auth"); + + await toggleSignInSignUp(page); + const { fieldValues, postValues } = getDefaultSignUpFieldValues({ email: getTestEmail() }); + await signUp(page, fieldValues, postValues, "emailpassword"); + + await waitForUrl(page, "/oauth/callback"); + + // Logout + const logoutButton = await getOAuth2LogoutButton(page, "redirect"); + await logoutButton.click(); + + await waitForUrl(page, "/auth/oauth/logout"); + + await Promise.all([ + page.waitForNavigation({ waitUntil: "networkidle0" }), + page.goto(page.url().replace("logoutChallenge=", "logoutChallenge=nooope")), + ]); + + // Click the Logout button on the provider website + const stLogoutButton = await waitForSTElement(page, "[data-supertokens~=button]"); + await stLogoutButton.click(); + + // Ensure the we get redirected to the auth page + await waitForUrl(page, "/auth/"); + await waitForSTElement(page); + }); + it("should successfully complete the OAuth2 Logout flow", async function () { const postLogoutRedirectUri = `${TEST_CLIENT_BASE_URL}/oauth/login?logout=true`;