Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Setup AsyncLocalStoreage context for Express.User throughout the callstack #2199

Merged
merged 4 commits into from
Sep 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api.planx.uk/admin/feedback/downloadFeedbackCSV.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ type ParsedFeedback = Feedback & {
* tags:
* - admin
* security:
* - userJWT: []
* - bearerAuth: []
*/
export const downloadFeedbackCSV = async (
req: Request,
Expand Down
7 changes: 4 additions & 3 deletions api.planx.uk/admin/session/bops.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Request, Response, NextFunction } from "express";
import { $admin } from "../../client";
import { getClient } from "../../client";

/**
* @swagger
Expand All @@ -12,15 +12,16 @@ import { $admin } from "../../client";
* parameters:
* - $ref: '#/components/parameters/sessionId'
* security:
* - userJWT: []
* - bearerAuth: []
*/
export const getBOPSPayload = async (
req: Request,
res: Response,
next: NextFunction,
) => {
try {
const { exportData } = await $admin.export.bopsPayload(
const $client = getClient();
const { exportData } = await $client.export.bopsPayload(
req.params.sessionId,
);
res.set("content-type", "application/json");
Expand Down
12 changes: 7 additions & 5 deletions api.planx.uk/admin/session/csv.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { stringify } from "csv-stringify";
import { NextFunction, Request, Response } from "express";
import { $admin } from "../../client";
import { getClient } from "../../client";

/**
* @swagger
Expand All @@ -18,15 +18,16 @@ import { $admin } from "../../client";
* required: false
* description: If a CSV file should be downloaded, or its raw data returned
* security:
* - userJWT: []
* - bearerAuth: []
*/
export async function getCSVData(
req: Request,
res: Response,
next: NextFunction,
) {
try {
const { responses } = await $admin.export.csvData(req.params.sessionId);
const $client = getClient();
const { responses } = await $client.export.csvData(req.params.sessionId);

if (req.query?.download) {
stringify(responses, {
Expand Down Expand Up @@ -61,15 +62,16 @@ export async function getCSVData(
* required: false
* description: If a CSV file should be downloaded, or its raw data returned
* security:
* - userJWT: []
* - bearerAuth: []
*/
export async function getRedactedCSVData(
req: Request,
res: Response,
next: NextFunction,
) {
try {
const { redactedResponses } = await $admin.export.csvData(
const $client = getClient();
const { redactedResponses } = await $client.export.csvData(
req.params.sessionId,
);

Expand Down
16 changes: 9 additions & 7 deletions api.planx.uk/admin/session/html.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { generateApplicationHTML } from "@opensystemslab/planx-core";
import { $admin } from "../../client";
import { getClient } from "../../client";
import type { RequestHandler } from "express";
import type { PlanXExportData } from "@opensystemslab/planx-core/types";

Expand All @@ -16,14 +16,15 @@ type HTMLExportHandler = RequestHandler<{ sessionId: string }, string>;
* parameters:
* - $ref: '#/components/parameters/sessionId'
* security:
* - userJWT: []
* - bearerAuth: []
*/
export const getHTMLExport: HTMLExportHandler = async (req, res, next) => {
try {
const session = await $admin.session.find(req.params.sessionId);
const $client = getClient();
const session = await $client.session.find(req.params.sessionId);
if (!session) throw Error(`Unable to find session ${req.params.sessionId}`);

const { responses } = await $admin.export.csvData(req.params.sessionId);
const { responses } = await $client.export.csvData(req.params.sessionId);
const boundingBox =
session.data.passport.data["property.boundary.site.buffered"];

Expand Down Expand Up @@ -52,18 +53,19 @@ export const getHTMLExport: HTMLExportHandler = async (req, res, next) => {
* parameters:
* - $ref: '#/components/parameters/sessionId'
* security:
* - userJWT: []
* - bearerAuth: []
*/
export const getRedactedHTMLExport: HTMLExportHandler = async (
req,
res,
next,
) => {
try {
const session = await $admin.session.find(req.params.sessionId);
const $client = getClient();
const session = await $client.session.find(req.params.sessionId);
if (!session) throw Error(`Unable to find session ${req.params.sessionId}`);

const { redactedResponses } = await $admin.export.csvData(
const { redactedResponses } = await $client.export.csvData(
req.params.sessionId,
);
const boundingBox =
Expand Down
2 changes: 1 addition & 1 deletion api.planx.uk/admin/session/oneAppXML.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { adminGraphQLClient as client } from "../../hasura";
* parameters:
* - $ref: '#/components/parameters/sessionId'
* security:
* - userJWT: []
* - bearerAuth: []
*/
export const getOneAppXML = async (
req: Request,
Expand Down
2 changes: 1 addition & 1 deletion api.planx.uk/admin/session/summary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { Breadcrumb, Flow, LowCalSession, Passport, Team } from "../../types";
* parameters:
* - $ref: '#/components/parameters/sessionId'
* security:
* - userJWT: []
* - bearerAuth: []
*/
export async function getSessionSummary(
req: Request,
Expand Down
2 changes: 1 addition & 1 deletion api.planx.uk/admin/session/zip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { buildSubmissionExportZip } from "../../send/exportZip";
* required: false
* description: If the OneApp XML file should be included in the zip
* security:
* - userJWT: []
* - bearerAuth: []
*/
export async function generateZip(
req: Request,
Expand Down
33 changes: 31 additions & 2 deletions api.planx.uk/client/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
import { CoreDomainClient } from "@opensystemslab/planx-core";
import { userContext } from "../modules/auth/middleware";
import { ServerError } from "../errors";

/**
* core doesn't expose a graphql interface like the graphql/hasura clients do
* instead, they encapsulates query and business logic to only expose declarative interfaces
* @deprecated This client's permissions set are higher than required.
* Should only be used by trusted service-to-service calls (e.g Hasura -> API).
* Calls made by users should be directed through $public or the role-scoped getClient().
*
* Consider removing this and replacing with an "api" role using "backend-only" operations in Hasura
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*/
export const $admin = new CoreDomainClient({
auth: { adminSecret: process.env.HASURA_GRAPHQL_ADMIN_SECRET! },
Expand All @@ -11,3 +17,26 @@ export const $admin = new CoreDomainClient({
export const $public = new CoreDomainClient({
targetURL: process.env.HASURA_GRAPHQL_URL!,
});

/**
* Get a planx-core client with permissions scoped to the current user.
* This client instance ensures that all operations are restricted
* to the permissions of the user who initiated the request.
*/
export const getClient = () => {
Copy link
Contributor Author

@DafyddLlyr DafyddLlyr Sep 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not thrilled on the naming here - feels a little generic and meaningless.

Something like getUserRoleClient() might be more accurate but it's pretty wordy...

No strong opinions here on naming, but if we move to a future where the API only calls a $public client (pretty clear what this is) and a permissions-scoped $client then this might actually be fine?

const store = userContext.getStore();
if (!store)
throw new ServerError({
status: 500,
message: "Missing user context",
});

const client = new CoreDomainClient({
targetURL: process.env.HASURA_GRAPHQL_URL!,
auth: {
jwt: store.user.jwt,
},
});

return client;
};
7 changes: 5 additions & 2 deletions api.planx.uk/editor/copyFlow.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Request, Response, NextFunction } from "express";
import { makeUniqueFlow, getFlowData, insertFlow } from "../helpers";
import { Flow } from "../types";
import { userContext } from "../modules/auth/middleware";

const copyFlow = async (
req: Request,
Expand All @@ -25,13 +26,15 @@ const copyFlow = async (
const shouldInsert = (req.body?.insert as boolean) || false;
if (shouldInsert) {
const newSlug = flow.slug + "-copy";
const creatorId = parseInt(req.user!.sub!, 10);
const creatorId = userContext.getStore()?.user?.sub;
if (!creatorId) throw Error("User details missing from request");

// Insert the flow and an associated operation
await insertFlow(
flow.team_id,
newSlug,
uniqueFlowData,
creatorId,
parseInt(creatorId),
req.params.flowId,
);
}
Expand Down
6 changes: 5 additions & 1 deletion api.planx.uk/editor/publish.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import { gql } from "graphql-request";
import intersection from "lodash/intersection";
import { ComponentType } from "@opensystemslab/planx-core/types";
import { userContext } from "../modules/auth/middleware";

const validateAndDiffFlow = async (
req: Request,
Expand Down Expand Up @@ -73,6 +74,9 @@
const mostRecent = await getMostRecentPublishedFlow(req.params.flowId);
const delta = jsondiffpatch.diff(mostRecent, flattenedFlow);

const userId = userContext.getStore()?.user?.sub;
if (!userId) throw Error("User details missing from request");

if (delta) {
const response = await adminClient.request(
gql`
Expand Down Expand Up @@ -101,7 +105,7 @@
{
data: flattenedFlow,
flow_id: req.params.flowId,
publisher_id: parseInt(req.user!.sub!, 10),
publisher_id: parseInt(userId),
summary: req.query?.summary || null,
},
);
Expand Down Expand Up @@ -135,7 +139,7 @@
description?: string;
};

const validateSections = (flow: Record<string, any>): ValidationResponse => {

Check warning on line 142 in api.planx.uk/editor/publish.ts

View workflow job for this annotation

GitHub Actions / Run API Tests

Unexpected any. Specify a different type
if (getSectionNodeIds(flow)?.length > 0) {
if (!sectionIsInFirstPosition(flow)) {
return {
Expand All @@ -161,18 +165,18 @@
};
};

const getSectionNodeIds = (flow: Record<string, any>): string[] => {

Check warning on line 168 in api.planx.uk/editor/publish.ts

View workflow job for this annotation

GitHub Actions / Run API Tests

Unexpected any. Specify a different type
return Object.entries(flow)
.filter(([_nodeId, nodeData]) => nodeData?.type === ComponentType.Section)
?.map(([nodeId, _nodeData]) => nodeId);
};

const sectionIsInFirstPosition = (flow: Record<string, any>): boolean => {

Check warning on line 174 in api.planx.uk/editor/publish.ts

View workflow job for this annotation

GitHub Actions / Run API Tests

Unexpected any. Specify a different type
const firstNodeId = flow["_root"].edges[0];
return flow[firstNodeId].type === ComponentType.Section;
};

const allSectionsOnRoot = (flow: Record<string, any>): boolean => {

Check warning on line 179 in api.planx.uk/editor/publish.ts

View workflow job for this annotation

GitHub Actions / Run API Tests

Unexpected any. Specify a different type
const sectionTypeNodeIds = getSectionNodeIds(flow);
const intersectingNodeIds = intersection(
flow["_root"].edges,
Expand Down
4 changes: 2 additions & 2 deletions api.planx.uk/inviteToPay/sendConfirmationEmail.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { $admin } from "../client";
import { $public, $admin } from "../client";
import { sendEmail } from "../notify";
import { gql } from "graphql-request";
import { convertSlugToName } from "../saveAndReturn/utils";
Expand All @@ -8,7 +8,7 @@ export async function sendAgentAndPayeeConfirmationEmail(sessionId: string) {
const { personalisation, applicantEmail, payeeEmail, projectTypes } =
await getDataForPayeeAndAgentEmails(sessionId);
const projectType = projectTypes.length
? await $admin.formatRawProjectTypes(projectTypes)
? await $public.formatRawProjectTypes(projectTypes)
: "Project type not submitted";
const config: AgentAndPayeeSubmissionNotifyConfig = {
personalisation: {
Expand Down
4 changes: 2 additions & 2 deletions api.planx.uk/inviteToPay/sendPaymentEmail.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { Template, getClientForTemplate, sendEmail } from "../notify";
import { InviteToPayNotifyConfig } from "../types";
import { Team } from "../types";
import type { PaymentRequest } from "@opensystemslab/planx-core/types";
import { $admin } from "../client";
import { $public } from "../client";

interface SessionDetails {
email: string;
Expand Down Expand Up @@ -118,7 +118,7 @@ const getInviteToPayNotifyConfig = async (
).title,
fee: getFee(paymentRequest),
projectType:
(await $admin.formatRawProjectTypes(
(await $public.formatRawProjectTypes(
paymentRequest.sessionPreviewData?.["proposal.projectType"] as string[],
)) || "Project type not submitted",
serviceName: convertSlugToName(session.flow.slug),
Expand Down
41 changes: 39 additions & 2 deletions api.planx.uk/modules/auth/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ import passport from "passport";

import { RequestHandler } from "http-proxy-middleware";
import { Role } from "@opensystemslab/planx-core/types";
import { AsyncLocalStorage } from "async_hooks";

export const userContext = new AsyncLocalStorage<{ user: Express.User }>();

/**
* Validate that a provided string (e.g. API key) matches the expected value
Expand Down Expand Up @@ -153,15 +156,49 @@ export const useRoleAuth: UseRoleAuth =
});
}

next();
// Establish a context for the current request/response call stack using AsyncLocalStorage
// The validated user will be accessible to all subsequent functions
// Store the raw JWT to pass on to plan-core client
userContext.run(
{
user: {
...req.user,
jwt: req.cookies.jwt,
},
},
() => next(),
);
});
};

// Convenience methods
// Convenience methods for role-based access
export const useTeamViewerAuth = useRoleAuth([
"teamViewer",
"teamEditor",
"platformAdmin",
]);
export const useTeamEditorAuth = useRoleAuth(["teamEditor", "platformAdmin"]);
export const usePlatformAdminAuth = useRoleAuth(["platformAdmin"]);

/**
* Allow any logged in user to access route, without checking roles
*/
export const useLoginAuth: RequestHandler = (req, res, next) =>
useJWT(req, res, () => {
if (req?.user?.sub) {
userContext.run(
{
user: {
...req.user,
jwt: req.cookies.jwt,
},
},
() => next(),
);
} else {
return next({
status: 401,
message: "No authorization token was found",
});
}
});
1 change: 1 addition & 0 deletions api.planx.uk/modules/auth/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export const buildJWT = async (email: string): Promise<string | undefined> => {

const data = {
sub: user.id.toString(),
email,
"https://hasura.io/jwt/claims": generateHasuraClaimsForUser(user),
};

Expand Down
35 changes: 35 additions & 0 deletions api.planx.uk/modules/misc/controller.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { RequestHandler } from "express";
import { getClient } from "../../client";
import { userContext } from "../auth/middleware";
import { ServerError } from "../../errors";

export const getLoggedInUserDetails: RequestHandler = async (
_req,
res,
next,
) => {
try {
const $client = getClient();

const email = userContext.getStore()?.user.email;
if (!email)
throw new ServerError({
message: "User email missing from request",
status: 400,
});

const user = await $client.user.getByEmail(email);
if (!user)
throw new ServerError({
message: `Unable to locate user with email ${email}`,
status: 400,
});

res.json(user);
} catch (error) {
next(error);
}
};

export const healthCheck: RequestHandler = (_req, res) =>
res.json({ hello: "world" });
Loading
Loading