Skip to content

Commit

Permalink
Merge pull request #146 from jasonraimondi/fix/oauth-exception-in-ada…
Browse files Browse the repository at this point in the history
…pters

fix: swallowed exceptions from improper exports
  • Loading branch information
jasonraimondi authored Jul 26, 2024
2 parents 580922f + 9ff4e7c commit a9e6311
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 3 deletions.
3 changes: 2 additions & 1 deletion src/adapters/express.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { OAuthException } from "../exceptions/oauth.exception.js";

import { OAuthRequest } from "../requests/request.js";
import { OAuthResponse } from "../responses/response.js";
import { isOAuthError } from "../utils/errors.js";

export function responseFromExpress(res: Response): OAuthResponse {
return new OAuthResponse(res);
Expand All @@ -25,7 +26,7 @@ export function handleExpressResponse(expressResponse: Response, oauthResponse:

// @todo v4.0 flip these to always be Express as first arg, OAuth as second. Then update Docs
export function handleExpressError(e: unknown | OAuthException, res: Response): void {
if (e instanceof OAuthException) {
if (isOAuthError(e)) {
res.status(e.status);
res.send({
status: e.status,
Expand Down
3 changes: 2 additions & 1 deletion src/adapters/fastify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { OAuthException } from "../exceptions/oauth.exception.js";

import { OAuthRequest } from "../requests/request.js";
import { OAuthResponse } from "../responses/response.js";
import { isOAuthError } from "../utils/errors.js";

export function responseFromFastify(res: FastifyReply): OAuthResponse {
return new OAuthResponse({
Expand All @@ -20,7 +21,7 @@ export function requestFromFastify(req: FastifyRequest): OAuthRequest {

// @todo v4.0 flip these to always be Fastify as first arg, OAuth as second. Then update Docs
export function handleFastifyError(e: unknown | OAuthException, res: FastifyReply): void {
if (e instanceof OAuthException) {
if (isOAuthError(e)) {
res.status(e.status).send({
status: e.status,
message: e.message,
Expand Down
6 changes: 6 additions & 0 deletions src/exceptions/oauth.exception.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ export enum ErrorType {
}

export class OAuthException extends Error {
private readonly oauth = true;

constructor(
public readonly error: string,
public readonly errorType: ErrorType,
Expand Down Expand Up @@ -123,4 +125,8 @@ export class OAuthException extends Error {
HttpStatus.INTERNAL_SERVER_ERROR,
);
}

get isOAuth(): boolean {
return this.oauth;
}
}
7 changes: 7 additions & 0 deletions src/utils/errors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { OAuthException } from "../exceptions/oauth.exception.js";

export function isOAuthError(error: unknown): error is OAuthException {
if (!error) return false;
if (typeof error !== "object") return false;
return "oauth" in error;
}
59 changes: 59 additions & 0 deletions test/e2e/adapters/fastify.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { describe, it, expect, vi } from "vitest";
import { FastifyReply, FastifyRequest } from "fastify";
import { handleFastifyError, requestFromFastify, responseFromFastify } from "../../../src/adapters/fastify.js";
import { ErrorType, OAuthException, OAuthRequest, OAuthResponse } from "../../../src/index.js";

describe("Fastify OAuth Utilities", () => {
describe("responseFromFastify", () => {
it("should create an OAuthResponse from FastifyReply", () => {
const mockReply = {
headers: { "Content-Type": "application/json" },
} as unknown as FastifyReply;

const result = responseFromFastify(mockReply);
expect(result).toBeInstanceOf(OAuthResponse);
expect(result.headers).toEqual({ "Content-Type": "application/json" });
});
});

describe("requestFromFastify", () => {
it("should create an OAuthRequest from FastifyRequest", () => {
const mockRequest = {
query: { code: "abc123" },
body: { grant_type: "authorization_code" },
headers: { "Content-Type": "application/json" },
} as unknown as FastifyRequest;

const result = requestFromFastify(mockRequest);
expect(result).toBeInstanceOf(OAuthRequest);
expect(result.query).toEqual({ code: "abc123" });
expect(result.body).toEqual({ grant_type: "authorization_code" });
expect(result.headers).toEqual({ "Content-Type": "application/json" });
});
});

describe("handleFastifyError", () => {
it("should handle OAuthException", () => {
const mockReply = {
status: vi.fn().mockReturnThis(),
send: vi.fn(),
} as unknown as FastifyReply;

const oauthError = new OAuthException("Test error", ErrorType.InternalServerError);
handleFastifyError(oauthError, mockReply);

expect(mockReply.status).toHaveBeenCalledWith(400);
expect(mockReply.send).toHaveBeenCalledWith({
status: 400,
message: "Test error",
});
});

it("should throw non-OAuthException errors", () => {
const mockReply = {} as FastifyReply;
const error = new Error("Regular error");

expect(() => handleFastifyError(error, mockReply)).toThrow("Regular error");
});
});
});
2 changes: 1 addition & 1 deletion vitest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ export default defineConfig({
exclude: [".github/**", ".idea/**", "docs/**", "example/**"],
},
setupFiles: ["test/setup.ts"],
exclude: ["node_modules/**", "example/**", "version-check.ts"],
exclude: ["docs/**", "example/**", "node_modules/**", "version-check.ts"],
},
});

0 comments on commit a9e6311

Please sign in to comment.