Add hooks to createAppRegsiterHandler (#183)

* Add hooks to createAppRegsiterHandler

* Add error handling and flatten structure

* Add tests

* Add docs

* CR suggestions applied

* Update docs
This commit is contained in:
Lukasz Ostrowski 2023-02-27 14:36:22 +01:00 committed by GitHub
parent ff1d92a48a
commit 195f2d9127
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 386 additions and 13 deletions

View file

@ -0,0 +1,5 @@
---
"@saleor/app-sdk": minor
---
Add hooks to createRegisterHandler, allowing to hook into token exchange process or to interrupt it

View file

@ -65,6 +65,11 @@ export default createAppRegisterHandler({
restToken: "...",
}),
allowedSaleorUrls: ["https://your-saleor.saleor.cloud/graphql/"], // optional, see options below
async onRequestVerified(req, { authData, respondWithError }) {
await doSomethingAndBlockInstallation(authData.token).catch((err) => {
throw respondWithError({ body: "Error, installation will fail" });
});
},
});
```
@ -78,6 +83,53 @@ export type CreateAppRegisterHandlerOptions = {
* to allow app registration only in allowed Saleor instances.
*/
allowedSaleorUrls?: Array<string | ((saleorApiUrl: string) => boolean)>;
/**
* Optional
* Run right after Saleor calls this endpoint
*/
onRequestStart?(
request: Request,
context: {
authToken?: string;
saleorDomain?: string;
saleorApiUrl?: string;
respondWithError: ({ status, message, body }) => never; // should throw
}
): Promise<void>;
/**
* Optional
* Run after all security checks
*/
onRequestVerified?(
request: Request,
context: {
authData: AuthData;
respondWithError: ({ status, message, body }) => never; // should throw
}
): Promise<void>;
/**
* Optional
* Run after APL successfully AuthData, assuming that APL.set will reject a Promise in case of error
*/
onAuthAplSaved?(
request: Request,
context: {
authData: AuthData;
respondWithError: ({ status, message, body }) => never; // should throw
}
): Promise<void>;
/**
* Optional
* Run after APL fails to set AuthData
*/
onAplSetFailed?(
request: Request,
context: {
authData: AuthData;
error: unknown;
respondWithError: ({ status, message, body }) => never; // should throw
}
): Promise<void>;
};
```

View file

@ -1,15 +1,18 @@
import { createMocks } from "node-mocks-http";
import { describe, expect, it, vi } from "vitest";
import { describe, expect, it, Mock, vi } from "vitest";
import { APL } from "../../APL";
import { APL, AuthData } from "../../APL";
import { createAppRegisterHandler } from "./create-app-register-handler";
const mockJwksValue = "{}";
const mockAppId = "42";
vi.mock("../../get-app-id", () => ({
getAppId: vi.fn().mockResolvedValue("42"),
getAppId: vi.fn().mockResolvedValue("42"), // can't use var reference, due to hoisting
}));
vi.mock("../../fetch-remote-jwks", () => ({
fetchRemoteJwks: vi.fn().mockResolvedValue("{}"),
fetchRemoteJwks: vi.fn().mockResolvedValue("{}"), // can't use var reference, due to hoisting
}));
const mockApl: APL = {
@ -90,4 +93,167 @@ describe("create-app-register-handler", () => {
expect(res._getStatusCode()).toBe(403);
expect(res._getData().success).toBe(false);
});
describe("Callback hooks", () => {
it("Runs callback hooks - successful saving to APL scenario", async () => {
const mockOnRequestStart = vi.fn();
const mockOnRequestVerified = vi.fn();
const mockOnAuthAplFailed = vi.fn();
const mockOnAuthAplSaved = vi.fn();
const { res, req } = createMocks({
/**
* Use body, instead of params, otherwise - for some reason - param is not accessible in mock request
* Maybe this is a bug https://github.com/howardabrams/node-mocks-http/blob/master/lib/mockRequest.js
*/
body: {
auth_token: "mock-auth-token",
},
headers: {
host: "some-saleor-host.cloud",
"x-forwarded-proto": "https",
"saleor-api-url": "https://mock-saleor-domain.saleor.cloud/graphql/",
"saleor-domain": "https://mock-saleor-domain.saleor.cloud/",
},
method: "POST",
});
const handler = createAppRegisterHandler({
apl: mockApl,
onRequestStart: mockOnRequestStart,
onRequestVerified: mockOnRequestVerified,
onAplSetFailed: mockOnAuthAplFailed,
onAuthAplSaved: mockOnAuthAplSaved,
});
const expectedAuthData: AuthData = {
token: "mock-auth-token",
domain: "https://mock-saleor-domain.saleor.cloud/",
saleorApiUrl: "https://mock-saleor-domain.saleor.cloud/graphql/",
jwks: mockJwksValue,
appId: mockAppId,
};
await handler(req, res);
expect(mockOnRequestStart).toHaveBeenCalledWith(
expect.anything(/* Assume original request */),
expect.objectContaining({
authToken: "mock-auth-token",
saleorDomain: "https://mock-saleor-domain.saleor.cloud/",
saleorApiUrl: "https://mock-saleor-domain.saleor.cloud/graphql/",
})
);
expect(mockOnRequestVerified).toHaveBeenCalledWith(
expect.anything(/* Assume original request */),
expect.objectContaining({
authData: expectedAuthData,
})
);
expect(mockOnAuthAplSaved).toHaveBeenCalledWith(
expect.anything(/* Assume original request */),
expect.objectContaining({
authData: expectedAuthData,
})
);
expect(mockOnAuthAplFailed).not.toHaveBeenCalled();
});
it("Runs callback hooks - failed saving to APL scenario", async () => {
const mockOnAuthAplFailed = vi.fn();
const mockOnAuthAplSaved = vi.fn();
(mockApl.set as Mock).mockImplementationOnce(() => {
throw new Error("test error");
});
const { res, req } = createMocks({
/**
* Use body, instead of params, otherwise - for some reason - param is not accessible in mock request
* Maybe this is a bug https://github.com/howardabrams/node-mocks-http/blob/master/lib/mockRequest.js
*/
body: {
auth_token: "mock-auth-token",
},
headers: {
host: "some-saleor-host.cloud",
"x-forwarded-proto": "https",
"saleor-api-url": "https://mock-saleor-domain.saleor.cloud/graphql/",
"saleor-domain": "https://mock-saleor-domain.saleor.cloud/",
},
method: "POST",
});
const handler = createAppRegisterHandler({
apl: mockApl,
onAplSetFailed: mockOnAuthAplFailed,
onAuthAplSaved: mockOnAuthAplSaved,
});
const expectedAuthData: AuthData = {
token: "mock-auth-token",
domain: "https://mock-saleor-domain.saleor.cloud/",
saleorApiUrl: "https://mock-saleor-domain.saleor.cloud/graphql/",
jwks: mockJwksValue,
appId: mockAppId,
};
await handler(req, res);
expect(mockOnAuthAplSaved).not.toHaveBeenCalled();
expect(mockOnAuthAplFailed).toHaveBeenCalledWith(
expect.anything(/* Assume original request */),
expect.objectContaining({
authData: expectedAuthData,
error: expect.objectContaining({
message: "test error",
}),
})
);
});
it("Allows to send custom error response via callback hook", async () => {
const mockOnRequestStart = vi.fn().mockImplementation(
(
req,
context: {
respondWithError(params: { status: number; body: string; message: string }): Error;
}
) => {
throw context.respondWithError({
status: 401,
body: "test",
message: "test message",
});
}
);
const { res, req } = createMocks({
/**
* Use body, instead of params, otherwise - for some reason - param is not accessible in mock request
* Maybe this is a bug https://github.com/howardabrams/node-mocks-http/blob/master/lib/mockRequest.js
*/
body: {
auth_token: "mock-auth-token",
},
headers: {
host: "some-saleor-host.cloud",
"x-forwarded-proto": "https",
"saleor-api-url": "https://mock-saleor-domain.saleor.cloud/graphql/",
"saleor-domain": "https://mock-saleor-domain.saleor.cloud/",
},
method: "POST",
});
const handler = createAppRegisterHandler({
apl: mockApl,
onRequestStart: mockOnRequestStart,
});
await handler(req, res);
expect(res._getStatusCode()).toBe(401);
expect(res._getData()).toBe("test");
});
});
});

View file

@ -1,8 +1,9 @@
import type { Handler } from "retes";
import type { Handler, Request } from "retes";
import { toNextHandler } from "retes/adapter";
import { withMethod } from "retes/middleware";
import { Response } from "retes/response";
import { AuthData } from "../../APL";
import { SALEOR_API_URL_HEADER, SALEOR_DOMAIN_HEADER } from "../../const";
import { createDebug } from "../../debug";
import { fetchRemoteJwks } from "../../fetch-remote-jwks";
@ -13,6 +14,39 @@ import { validateAllowSaleorUrls } from "./validate-allow-saleor-urls";
const debug = createDebug("createAppRegisterHandler");
type HookCallbackErrorParams = {
status?: number;
body?: object;
message?: string;
};
class RegisterCallbackError extends Error {
public status = 500;
public body: object = {};
constructor(errorParams: HookCallbackErrorParams) {
super(errorParams.message);
if (errorParams.status) {
this.status = errorParams.status;
}
if (errorParams.body) {
this.body = errorParams.body;
}
}
}
const createCallbackError = (params: HookCallbackErrorParams) => new RegisterCallbackError(params);
const handleHookError = (e: RegisterCallbackError | unknown) => {
if (e instanceof RegisterCallbackError) {
return new Response(e.body, { status: e.status });
}
return Response.InternalServerError("Error during app installation");
};
export type CreateAppRegisterHandlerOptions = HasAPL & {
/**
* Protect app from being registered in Saleor other than specific.
@ -22,6 +56,49 @@ export type CreateAppRegisterHandlerOptions = HasAPL & {
* or a function that receives a full Saleor API URL ad returns true/false.
*/
allowedSaleorUrls?: Array<string | ((saleorApiUrl: string) => boolean)>;
/**
* Run right after Saleor calls this endpoint
*/
onRequestStart?(
request: Request,
context: {
authToken?: string;
saleorDomain?: string;
saleorApiUrl?: string;
respondWithError: typeof createCallbackError;
}
): Promise<void>;
/**
* Run after all security checks
*/
onRequestVerified?(
request: Request,
context: {
authData: AuthData;
respondWithError: typeof createCallbackError;
}
): Promise<void>;
/**
* Run after APL successfully AuthData, assuming that APL.set will reject a Promise in case of error
*/
onAuthAplSaved?(
request: Request,
context: {
authData: AuthData;
respondWithError: typeof createCallbackError;
}
): Promise<void>;
/**
* Run after APL fails to set AuthData
*/
onAplSetFailed?(
request: Request,
context: {
authData: AuthData;
error: unknown;
respondWithError: typeof createCallbackError;
}
): Promise<void>;
};
/**
@ -32,13 +109,35 @@ export type CreateAppRegisterHandlerOptions = HasAPL & {
export const createAppRegisterHandler = ({
apl,
allowedSaleorUrls,
onAplSetFailed,
onAuthAplSaved,
onRequestVerified,
onRequestStart,
}: CreateAppRegisterHandlerOptions) => {
const baseHandler: Handler = async (request) => {
debug("Request received");
const authToken = request.params.auth_token;
const saleorDomain = request.headers[SALEOR_DOMAIN_HEADER] as string;
const saleorApiUrl = request.headers[SALEOR_API_URL_HEADER] as string;
if (onRequestStart) {
debug("Calling \"onRequestStart\" hook");
try {
await onRequestStart(request, {
authToken,
saleorApiUrl,
saleorDomain,
respondWithError: createCallbackError,
});
} catch (e: RegisterCallbackError | unknown) {
debug("\"onRequestStart\" hook thrown error: %o", e);
return handleHookError(e);
}
}
if (!validateAllowSaleorUrls(saleorApiUrl, allowedSaleorUrls)) {
debug("Validation of URL %s against allowSaleorUrls param resolves to false, throwing");
@ -104,16 +203,65 @@ export const createAppRegisterHandler = ({
);
}
const authData = {
domain: saleorDomain,
token: authToken,
saleorApiUrl,
appId,
jwks,
};
if (onRequestVerified) {
debug("Calling \"onRequestVerified\" hook");
try {
await onRequestVerified(request, {
authData,
respondWithError: createCallbackError,
});
} catch (e: RegisterCallbackError | unknown) {
debug("\"onRequestVerified\" hook thrown error: %o", e);
return handleHookError(e);
}
}
try {
await apl.set({
domain: saleorDomain,
token: authToken,
saleorApiUrl,
appId,
jwks,
});
} catch {
await apl.set(authData);
if (onAuthAplSaved) {
debug("Calling \"onAuthAplSaved\" hook");
try {
await onAuthAplSaved(request, {
authData,
respondWithError: createCallbackError,
});
} catch (e: RegisterCallbackError | unknown) {
debug("\"onAuthAplSaved\" hook thrown error: %o", e);
return handleHookError(e);
}
}
} catch (aplError: unknown) {
debug("There was an error during saving the auth data");
if (onAplSetFailed) {
debug("Calling \"onAuthAplFailed\" hook");
try {
await onAplSetFailed(request, {
authData,
error: aplError,
respondWithError: createCallbackError,
});
} catch (hookError: RegisterCallbackError | unknown) {
debug("\"onAuthAplFailed\" hook thrown error: %o", hookError);
return handleHookError(hookError);
}
}
return Response.InternalServerError({
success: false,
error: {
@ -121,7 +269,9 @@ export const createAppRegisterHandler = ({
},
});
}
debug("Register complete");
return Response.OK({ success: true });
};