Skip to content

Commit

Permalink
fix: self review test fixes (#955)
Browse files Browse the repository at this point in the history
* feat: better error handling for invalid login/out challenges

* ci: make ci job only run if necessary
  • Loading branch information
porcellus authored Oct 27, 2024
1 parent f734bc4 commit fc6c0e9
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 16 deletions.
6 changes: 6 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,12 @@ jobs:
workflows:
version: 2
tagged-build:
when: # One must be true to trigger
or:
- equal: [true, <<pipeline.parameters.force>>]
- matches: { pattern: "dev-v[0-9]+(\\.[0-9]+)*", value: << pipeline.git.tag >> }
- matches: { pattern: "v[0-9]+(\\.[0-9]+)*", value: << pipeline.git.tag >> }
- equal: ["master", << pipeline.git.branch >>]
jobs:
- publish:
context:
Expand Down
3 changes: 3 additions & 0 deletions lib/build/recipe/oauth2provider/api/implementation.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,9 @@ function getAPIImplementation() {
challenge: logoutChallenge,
userContext,
});
if ("error" in response) {
return response;
}
const res = await utils_1.handleLogoutInternalRedirects({
response,
recipeImplementation: options.recipeImplementation,
Expand Down
22 changes: 16 additions & 6 deletions lib/build/recipe/oauth2provider/recipeImplementation.js
Original file line number Diff line number Diff line change
Expand Up @@ -803,12 +803,14 @@ function getRecipeInterface(
};
} else {
// Accept the logout challenge immediately as there is no supertokens session
redirectTo = (
await this.acceptLogoutRequest({
challenge: logoutChallenge,
userContext: input.userContext,
})
).redirectTo;
const acceptLogoutResponse = await this.acceptLogoutRequest({
challenge: logoutChallenge,
userContext: input.userContext,
});
if ("error" in acceptLogoutResponse) {
return acceptLogoutResponse;
}
return { redirectTo: acceptLogoutResponse.redirectTo };
}
}
// CASE 2 or 3 (See above notes)
Expand All @@ -831,6 +833,14 @@ function getRecipeInterface(
{},
input.userContext
);
if (resp.status !== "OK") {
return {
status: "ERROR",
statusCode: resp.statusCode,
error: resp.error,
errorDescription: resp.errorDescription,
};
}
const redirectTo = getUpdatedRedirectTo(appInfo, resp.redirectTo);
if (redirectTo.endsWith("/fallbacks/logout/callback")) {
return {
Expand Down
9 changes: 6 additions & 3 deletions lib/build/recipe/oauth2provider/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -354,9 +354,12 @@ export declare type RecipeInterface = {
acceptLogoutRequest(input: {
challenge: string;
userContext: UserContext;
}): Promise<{
redirectTo: string;
}>;
}): Promise<
| {
redirectTo: string;
}
| ErrorOAuth2
>;
rejectLogoutRequest(input: {
challenge: string;
userContext: UserContext;
Expand Down
4 changes: 4 additions & 0 deletions lib/ts/recipe/oauth2provider/api/implementation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,10 @@ export default function getAPIImplementation(): APIInterface {
userContext,
});

if ("error" in response) {
return response;
}

const res = await handleLogoutInternalRedirects({
response,
recipeImplementation: options.recipeImplementation,
Expand Down
23 changes: 17 additions & 6 deletions lib/ts/recipe/oauth2provider/recipeImplementation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -819,12 +819,14 @@ export default function getRecipeInterface(
};
} else {
// Accept the logout challenge immediately as there is no supertokens session
redirectTo = (
await this.acceptLogoutRequest({
challenge: logoutChallenge,
userContext: input.userContext,
})
).redirectTo;
const acceptLogoutResponse = await this.acceptLogoutRequest({
challenge: logoutChallenge,
userContext: input.userContext,
});
if ("error" in acceptLogoutResponse) {
return acceptLogoutResponse;
}
return { redirectTo: acceptLogoutResponse.redirectTo };
}
}

Expand All @@ -851,6 +853,15 @@ export default function getRecipeInterface(
input.userContext
);

if (resp.status !== "OK") {
return {
status: "ERROR",
statusCode: resp.statusCode,
error: resp.error,
errorDescription: resp.errorDescription,
};
}

const redirectTo = getUpdatedRedirectTo(appInfo, resp.redirectTo);

if (redirectTo.endsWith("/fallbacks/logout/callback")) {
Expand Down
5 changes: 4 additions & 1 deletion lib/ts/recipe/oauth2provider/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,10 @@ export type RecipeInterface = {
shouldTryRefresh: boolean;
userContext: UserContext;
}): Promise<{ redirectTo: string } | ErrorOAuth2>;
acceptLogoutRequest(input: { challenge: string; userContext: UserContext }): Promise<{ redirectTo: string }>;
acceptLogoutRequest(input: {
challenge: string;
userContext: UserContext;
}): Promise<{ redirectTo: string } | ErrorOAuth2>;
rejectLogoutRequest(input: { challenge: string; userContext: UserContext }): Promise<{ status: "OK" }>;
};

Expand Down

0 comments on commit fc6c0e9

Please sign in to comment.