From e95511ab3ab08a4116c4b095cda1130e57146db4 Mon Sep 17 00:00:00 2001 From: Mihaly Lengyel Date: Sat, 28 Sep 2024 03:39:50 +0200 Subject: [PATCH] refactor: some cleanup and error handling --- .../oauth2provider/api/implementation.js | 9 ++++++- .../recipe/oauth2provider/api/utils.d.ts | 17 +++++++++--- lib/build/recipe/oauth2provider/api/utils.js | 18 ++++++++++--- .../oauth2provider/recipeImplementation.js | 22 ++++++++++++++++ lib/build/recipe/oauth2provider/types.d.ts | 12 ++++++++- .../oauth2provider/api/implementation.ts | 12 ++++++++- lib/ts/recipe/oauth2provider/api/utils.ts | 20 +++++++++++--- .../oauth2provider/recipeImplementation.ts | 26 ++++++++++++++++--- lib/ts/recipe/oauth2provider/types.ts | 9 +++++-- 9 files changed, 125 insertions(+), 20 deletions(-) diff --git a/lib/build/recipe/oauth2provider/api/implementation.js b/lib/build/recipe/oauth2provider/api/implementation.js index 5baf073ea..c234fe9c1 100644 --- a/lib/build/recipe/oauth2provider/api/implementation.js +++ b/lib/build/recipe/oauth2provider/api/implementation.js @@ -26,6 +26,9 @@ function getAPIImplementation() { isDirectCall: true, userContext, }); + if ("error" in response) { + return response; + } const respAfterInternalRedirects = await utils_1.handleLoginInternalRedirects({ response, cookie: options.req.getHeaderValue("cookie"), @@ -69,10 +72,14 @@ function getAPIImplementation() { }); }, loginInfoGET: async ({ loginChallenge, options, userContext }) => { - const { client } = await options.recipeImplementation.getLoginRequest({ + const loginRes = await options.recipeImplementation.getLoginRequest({ challenge: loginChallenge, userContext, }); + if (loginRes.status === "ERROR") { + return loginRes; + } + const { client } = loginRes; return { status: "OK", info: { diff --git a/lib/build/recipe/oauth2provider/api/utils.d.ts b/lib/build/recipe/oauth2provider/api/utils.d.ts index 63245a812..34aec7ed5 100644 --- a/lib/build/recipe/oauth2provider/api/utils.d.ts +++ b/lib/build/recipe/oauth2provider/api/utils.d.ts @@ -18,10 +18,19 @@ export declare function loginGET({ setCookie?: string; userContext: UserContext; isDirectCall: boolean; -}): Promise<{ - redirectTo: string; - setCookie: string | undefined; -}>; +}): Promise< + | ErrorOAuth2 + | { + status: string; + redirectTo: string; + setCookie: string | undefined; + } + | { + redirectTo: string; + setCookie: string | undefined; + status?: undefined; + } +>; export declare function handleLoginInternalRedirects({ response, recipeImplementation, diff --git a/lib/build/recipe/oauth2provider/api/utils.js b/lib/build/recipe/oauth2provider/api/utils.js index 58390408c..8c6a6bd32 100644 --- a/lib/build/recipe/oauth2provider/api/utils.js +++ b/lib/build/recipe/oauth2provider/api/utils.js @@ -27,6 +27,9 @@ async function loginGET({ challenge: loginChallenge, userContext, }); + if (loginRequest.status === "ERROR") { + return loginRequest; + } const sessionInfo = session !== undefined ? await session_1.getSessionInformation( @@ -49,23 +52,25 @@ async function loginGET({ const reject = await recipeImplementation.rejectLoginRequest({ challenge: loginChallenge, error: { + status: "ERROR", error: "invalid_request", errorDescription: "max_age cannot be negative", }, userContext, }); - return { redirectTo: reject.redirectTo, setCookie }; + return { status: "REDIRECT", redirectTo: reject.redirectTo, setCookie }; } } catch (_c) { const reject = await recipeImplementation.rejectLoginRequest({ challenge: loginChallenge, error: { + status: "ERROR", error: "invalid_request", errorDescription: "max_age must be an integer", }, userContext, }); - return { redirectTo: reject.redirectTo, setCookie }; + return { status: "REDIRECT", redirectTo: reject.redirectTo, setCookie }; } } const tenantIdParam = incomingAuthUrlQueryParams.get("tenant_id"); @@ -86,7 +91,7 @@ async function loginGET({ rememberFor: 3600, userContext, }); - return { redirectTo: accept.redirectTo, setCookie }; + return { status: "REDIRECT", redirectTo: accept.redirectTo, setCookie }; } if (shouldTryRefresh && promptParam !== "login") { return { @@ -102,15 +107,17 @@ async function loginGET({ const reject = await recipeImplementation.rejectLoginRequest({ challenge: loginChallenge, error: { + status: "ERROR", error: "login_required", errorDescription: "The Authorization Server requires End-User authentication. Prompt 'none' was requested, but no existing or expired login session was found.", }, userContext, }); - return { redirectTo: reject.redirectTo, setCookie }; + return { status: "REDIRECT", redirectTo: reject.redirectTo, setCookie }; } return { + status: "REDIRECT", redirectTo: await recipeImplementation.getFrontendRedirectionURL({ type: "login", loginChallenge, @@ -208,6 +215,9 @@ async function handleLoginInternalRedirects({ isDirectCall: false, userContext, }); + if ("error" in loginRes) { + return loginRes; + } response = { redirectTo: loginRes.redirectTo, setCookie: mergeSetCookieHeaders(loginRes.setCookie, response.setCookie), diff --git a/lib/build/recipe/oauth2provider/recipeImplementation.js b/lib/build/recipe/oauth2provider/recipeImplementation.js index f77e6ffc7..d194e1964 100644 --- a/lib/build/recipe/oauth2provider/recipeImplementation.js +++ b/lib/build/recipe/oauth2provider/recipeImplementation.js @@ -90,7 +90,16 @@ function getRecipeInterface( { loginChallenge: input.challenge }, input.userContext ); + if (resp.status !== "OK") { + return { + status: "ERROR", + statusCode: resp.statusCode, + error: resp.error, + errorDescription: resp.errorDescription, + }; + } return { + status: "OK", challenge: resp.challenge, client: OAuth2Client_1.OAuth2Client.fromAPIResponse(resp.client), oidcContext: resp.oidcContext, @@ -216,6 +225,7 @@ function getRecipeInterface( let payloads; if (input.params.client_id === undefined || typeof input.params.client_id !== "string") { return { + status: "ERROR", statusCode: 400, error: "invalid_request", errorDescription: "client_id is required and must be a string", @@ -240,6 +250,7 @@ function getRecipeInterface( }); if (clientInfo.status === "ERROR") { return { + status: "ERROR", statusCode: 400, error: clientInfo.error, errorDescription: clientInfo.errorDescription, @@ -249,6 +260,7 @@ function getRecipeInterface( const user = await __1.getUser(input.session.getUserId()); if (!user) { return { + status: "ERROR", statusCode: 400, error: "invalid_request", errorDescription: "User deleted", @@ -292,6 +304,7 @@ function getRecipeInterface( ); if (resp.status === "CLIENT_NOT_FOUND_ERROR") { return { + status: "ERROR", statusCode: 400, error: "invalid_request", errorDescription: "The provided client_id is not valid", @@ -299,6 +312,7 @@ function getRecipeInterface( } if (resp.status !== "OK") { return { + status: "ERROR", statusCode: resp.statusCode, error: resp.error, errorDescription: resp.errorDescription, @@ -342,6 +356,7 @@ function getRecipeInterface( body.iss = await this.getIssuer({ userContext: input.userContext }); if (input.body.grant_type === "password") { return { + status: "ERROR", statusCode: 400, error: "invalid_request", errorDescription: "Unsupported grant type: password", @@ -350,6 +365,7 @@ function getRecipeInterface( if (input.body.grant_type === "client_credentials") { if (input.body.client_id === undefined) { return { + status: "ERROR", statusCode: 400, error: "invalid_request", errorDescription: "client_id is required", @@ -366,6 +382,7 @@ function getRecipeInterface( }); if (clientInfo.status === "ERROR") { return { + status: "ERROR", statusCode: 400, error: clientInfo.error, errorDescription: clientInfo.errorDescription, @@ -406,6 +423,7 @@ function getRecipeInterface( }); if (clientInfo.status === "ERROR") { return { + status: "ERROR", statusCode: 400, error: clientInfo.error, errorDescription: clientInfo.errorDescription, @@ -415,6 +433,7 @@ function getRecipeInterface( const user = await __1.getUser(tokenInfo.sub); if (!user) { return { + status: "ERROR", statusCode: 400, error: "invalid_request", errorDescription: "User not found", @@ -446,6 +465,7 @@ function getRecipeInterface( ); if (res.status !== "OK") { return { + status: "ERROR", statusCode: res.statusCode, error: res.error, errorDescription: res.errorDescription, @@ -678,6 +698,7 @@ function getRecipeInterface( ); if (res.status !== "OK") { return { + status: "ERROR", statusCode: res.statusCode, error: res.error, errorDescription: res.errorDescription, @@ -756,6 +777,7 @@ function getRecipeInterface( ); if ("error" in resp) { return { + status: "ERROR", statusCode: resp.statusCode, error: resp.error, errorDescription: resp.errorDescription, diff --git a/lib/build/recipe/oauth2provider/types.d.ts b/lib/build/recipe/oauth2provider/types.d.ts index c7252c7f8..2b2760837 100644 --- a/lib/build/recipe/oauth2provider/types.d.ts +++ b/lib/build/recipe/oauth2provider/types.d.ts @@ -33,6 +33,7 @@ export declare type APIOptions = { res: BaseResponse; }; export declare type ErrorOAuth2 = { + status: "ERROR"; error: string; errorDescription: string; statusCode?: number; @@ -137,7 +138,15 @@ export declare type RecipeInterface = { }): Promise<{ redirectTo: string; }>; - getLoginRequest(input: { challenge: string; userContext: UserContext }): Promise; + getLoginRequest(input: { + challenge: string; + userContext: UserContext; + }): Promise< + | (LoginRequest & { + status: "OK"; + }) + | ErrorOAuth2 + >; acceptLoginRequest(input: { challenge: string; acr?: string; @@ -414,6 +423,7 @@ export declare type APIInterface = { status: "OK"; info: LoginInfo; } + | ErrorOAuth2 | GeneralErrorResponse >); userInfoGET: diff --git a/lib/ts/recipe/oauth2provider/api/implementation.ts b/lib/ts/recipe/oauth2provider/api/implementation.ts index dd5e3e1fe..de61323a8 100644 --- a/lib/ts/recipe/oauth2provider/api/implementation.ts +++ b/lib/ts/recipe/oauth2provider/api/implementation.ts @@ -27,6 +27,11 @@ export default function getAPIImplementation(): APIInterface { isDirectCall: true, userContext, }); + + if ("error" in response) { + return response; + } + const respAfterInternalRedirects = await handleLoginInternalRedirects({ response, cookie: options.req.getHeaderValue("cookie"), @@ -75,11 +80,16 @@ export default function getAPIImplementation(): APIInterface { }); }, loginInfoGET: async ({ loginChallenge, options, userContext }) => { - const { client } = await options.recipeImplementation.getLoginRequest({ + const loginRes = await options.recipeImplementation.getLoginRequest({ challenge: loginChallenge, userContext, }); + if (loginRes.status === "ERROR") { + return loginRes; + } + const { client } = loginRes; + return { status: "OK", info: { diff --git a/lib/ts/recipe/oauth2provider/api/utils.ts b/lib/ts/recipe/oauth2provider/api/utils.ts index 8d3481f24..486cf0d7e 100644 --- a/lib/ts/recipe/oauth2provider/api/utils.ts +++ b/lib/ts/recipe/oauth2provider/api/utils.ts @@ -31,6 +31,10 @@ export async function loginGET({ userContext, }); + if (loginRequest.status === "ERROR") { + return loginRequest; + } + const sessionInfo = session !== undefined ? await getSessionInformation(session?.getHandle()) : undefined; if (!sessionInfo) { session = undefined; @@ -47,23 +51,25 @@ export async function loginGET({ const reject = await recipeImplementation.rejectLoginRequest({ challenge: loginChallenge, error: { + status: "ERROR", error: "invalid_request", errorDescription: "max_age cannot be negative", }, userContext, }); - return { redirectTo: reject.redirectTo, setCookie }; + return { status: "REDIRECT", redirectTo: reject.redirectTo, setCookie }; } } catch { const reject = await recipeImplementation.rejectLoginRequest({ challenge: loginChallenge, error: { + status: "ERROR", error: "invalid_request", errorDescription: "max_age must be an integer", }, userContext, }); - return { redirectTo: reject.redirectTo, setCookie }; + return { status: "REDIRECT", redirectTo: reject.redirectTo, setCookie }; } } const tenantIdParam = incomingAuthUrlQueryParams.get("tenant_id"); @@ -84,7 +90,7 @@ export async function loginGET({ rememberFor: 3600, userContext, }); - return { redirectTo: accept.redirectTo, setCookie }; + return { status: "REDIRECT", redirectTo: accept.redirectTo, setCookie }; } if (shouldTryRefresh && promptParam !== "login") { @@ -101,16 +107,18 @@ export async function loginGET({ const reject = await recipeImplementation.rejectLoginRequest({ challenge: loginChallenge, error: { + status: "ERROR", error: "login_required", errorDescription: "The Authorization Server requires End-User authentication. Prompt 'none' was requested, but no existing or expired login session was found.", }, userContext, }); - return { redirectTo: reject.redirectTo, setCookie }; + return { status: "REDIRECT", redirectTo: reject.redirectTo, setCookie }; } return { + status: "REDIRECT", redirectTo: await recipeImplementation.getFrontendRedirectionURL({ type: "login", loginChallenge, @@ -226,6 +234,10 @@ export async function handleLoginInternalRedirects({ userContext, }); + if ("error" in loginRes) { + return loginRes; + } + response = { redirectTo: loginRes.redirectTo, setCookie: mergeSetCookieHeaders(loginRes.setCookie, response.setCookie), diff --git a/lib/ts/recipe/oauth2provider/recipeImplementation.ts b/lib/ts/recipe/oauth2provider/recipeImplementation.ts index 3a0991e33..fbc2421a7 100644 --- a/lib/ts/recipe/oauth2provider/recipeImplementation.ts +++ b/lib/ts/recipe/oauth2provider/recipeImplementation.ts @@ -21,7 +21,6 @@ import { RecipeInterface, TypeNormalisedInput, ConsentRequest, - LoginRequest, PayloadBuilderFunction, UserInfoBuilderFunction, } from "./types"; @@ -58,14 +57,22 @@ export default function getRecipeInterface( getDefaultUserInfoPayload: UserInfoBuilderFunction ): RecipeInterface { return { - getLoginRequest: async function (this: RecipeInterface, input): Promise { + getLoginRequest: async function (this: RecipeInterface, input) { const resp = await querier.sendGetRequest( new NormalisedURLPath("/recipe/oauth/auth/requests/login"), { loginChallenge: input.challenge }, input.userContext ); - + if (resp.status !== "OK") { + return { + status: "ERROR", + statusCode: resp.statusCode, + error: resp.error, + errorDescription: resp.errorDescription, + }; + } return { + status: "OK", challenge: resp.challenge, client: OAuth2Client.fromAPIResponse(resp.client), oidcContext: resp.oidcContext, @@ -198,6 +205,7 @@ export default function getRecipeInterface( if (input.params.client_id === undefined || typeof input.params.client_id !== "string") { return { + status: "ERROR", statusCode: 400, error: "invalid_request", errorDescription: "client_id is required and must be a string", @@ -222,6 +230,7 @@ export default function getRecipeInterface( if (clientInfo.status === "ERROR") { return { + status: "ERROR", statusCode: 400, error: clientInfo.error, errorDescription: clientInfo.errorDescription, @@ -232,6 +241,7 @@ export default function getRecipeInterface( const user = await getUser(input.session.getUserId()); if (!user) { return { + status: "ERROR", statusCode: 400, error: "invalid_request", errorDescription: "User deleted", @@ -281,6 +291,7 @@ export default function getRecipeInterface( if (resp.status === "CLIENT_NOT_FOUND_ERROR") { return { + status: "ERROR", statusCode: 400, error: "invalid_request", errorDescription: "The provided client_id is not valid", @@ -289,6 +300,7 @@ export default function getRecipeInterface( if (resp.status !== "OK") { return { + status: "ERROR", statusCode: resp.statusCode, error: resp.error, errorDescription: resp.errorDescription, @@ -337,6 +349,7 @@ export default function getRecipeInterface( if (input.body.grant_type === "password") { return { + status: "ERROR", statusCode: 400, error: "invalid_request", errorDescription: "Unsupported grant type: password", @@ -346,6 +359,7 @@ export default function getRecipeInterface( if (input.body.grant_type === "client_credentials") { if (input.body.client_id === undefined) { return { + status: "ERROR", statusCode: 400, error: "invalid_request", errorDescription: "client_id is required", @@ -360,6 +374,7 @@ export default function getRecipeInterface( if (clientInfo.status === "ERROR") { return { + status: "ERROR", statusCode: 400, error: clientInfo.error, errorDescription: clientInfo.errorDescription, @@ -399,6 +414,7 @@ export default function getRecipeInterface( }); if (clientInfo.status === "ERROR") { return { + status: "ERROR", statusCode: 400, error: clientInfo.error, errorDescription: clientInfo.errorDescription, @@ -408,6 +424,7 @@ export default function getRecipeInterface( const user = await getUser(tokenInfo.sub as string); if (!user) { return { + status: "ERROR", statusCode: 400, error: "invalid_request", errorDescription: "User not found", @@ -442,6 +459,7 @@ export default function getRecipeInterface( if (res.status !== "OK") { return { + status: "ERROR", statusCode: res.statusCode, error: res.error, errorDescription: res.errorDescription, @@ -676,6 +694,7 @@ export default function getRecipeInterface( if (res.status !== "OK") { return { + status: "ERROR", statusCode: res.statusCode, error: res.error, errorDescription: res.errorDescription, @@ -764,6 +783,7 @@ export default function getRecipeInterface( if ("error" in resp) { return { + status: "ERROR", statusCode: resp.statusCode, error: resp.error, errorDescription: resp.errorDescription, diff --git a/lib/ts/recipe/oauth2provider/types.ts b/lib/ts/recipe/oauth2provider/types.ts index 53b507ed7..da4842a89 100644 --- a/lib/ts/recipe/oauth2provider/types.ts +++ b/lib/ts/recipe/oauth2provider/types.ts @@ -51,6 +51,8 @@ export type APIOptions = { }; export type ErrorOAuth2 = { + status: "ERROR"; + // The error should follow the OAuth2 error format (e.g. invalid_request, login_required). // Defaults to request_denied. error: string; @@ -219,7 +221,10 @@ export type RecipeInterface = { userContext: UserContext; }): Promise<{ redirectTo: string }>; - getLoginRequest(input: { challenge: string; userContext: UserContext }): Promise; + getLoginRequest(input: { + challenge: string; + userContext: UserContext; + }): Promise<(LoginRequest & { status: "OK" }) | ErrorOAuth2>; acceptLoginRequest(input: { challenge: string; @@ -467,7 +472,7 @@ export type APIInterface = { loginChallenge: string; options: APIOptions; userContext: UserContext; - }) => Promise<{ status: "OK"; info: LoginInfo } | GeneralErrorResponse>); + }) => Promise<{ status: "OK"; info: LoginInfo } | ErrorOAuth2 | GeneralErrorResponse>); userInfoGET: | undefined | ((input: {