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:
parent
ff1d92a48a
commit
195f2d9127
4 changed files with 386 additions and 13 deletions
5
.changeset/slow-squids-hope.md
Normal file
5
.changeset/slow-squids-hope.md
Normal file
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
"@saleor/app-sdk": minor
|
||||
---
|
||||
|
||||
Add hooks to createRegisterHandler, allowing to hook into token exchange process or to interrupt it
|
|
@ -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>;
|
||||
};
|
||||
```
|
||||
|
||||
|
|
|
@ -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");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
@ -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 });
|
||||
};
|
||||
|
||||
|
|
Loading…
Reference in a new issue