Skip to content

Commit 7b95565

Browse files
committed
refactor: improve typing of generic route execution context
1 parent 844f02a commit 7b95565

File tree

7 files changed

+66
-61
lines changed

7 files changed

+66
-61
lines changed

apps/meteor/app/api/server/ApiClass.ts

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import type {
2828
NotFoundResult,
2929
Operations,
3030
Options,
31-
PartialThis,
3231
SuccessResult,
3332
TypedThis,
3433
TypedAction,
@@ -37,6 +36,7 @@ import type {
3736
RedirectStatusCodes,
3837
RedirectResult,
3938
UnavailableResult,
39+
GenericRouteExecutionContext,
4040
} from './definition';
4141
import { getUserInfo } from './helpers/getUserInfo';
4242
import { parseJsonQuery } from './helpers/parseJsonQuery';
@@ -156,12 +156,7 @@ const generateConnection = (
156156
clientAddress: ipAddress,
157157
});
158158

159-
export class APIClass<
160-
TBasePath extends string = '',
161-
TOperations extends {
162-
[x: string]: unknown;
163-
} = {},
164-
> {
159+
export class APIClass<TBasePath extends string = '', TOperations extends Record<string, unknown> = Record<string, never>> {
165160
public typedRoutes: Record<string, Record<string, Route>> = {};
166161

167162
protected apiPath?: string;
@@ -170,7 +165,7 @@ export class APIClass<
170165

171166
private _routes: { path: string; options: Options; endpoints: Record<string, string> }[] = [];
172167

173-
public authMethods: ((req: Request) => Promise<IUser | undefined>)[];
168+
public authMethods: ((routeContext: GenericRouteExecutionContext) => Promise<IUser | undefined>)[];
174169

175170
protected helperMethods: Map<string, () => any> = new Map();
176171

@@ -248,11 +243,11 @@ export class APIClass<
248243
};
249244
}
250245

251-
async parseJsonQuery(this: PartialThis) {
252-
return parseJsonQuery(this);
246+
async parseJsonQuery(routeContext: GenericRouteExecutionContext) {
247+
return parseJsonQuery(routeContext);
253248
}
254249

255-
public addAuthMethod(func: (req: Request) => Promise<IUser | undefined>): void {
250+
public addAuthMethod(func: (routeContext: GenericRouteExecutionContext) => Promise<IUser | undefined>): void {
256251
this.authMethods.push(func);
257252
}
258253

@@ -822,8 +817,12 @@ export class APIClass<
822817
const api = this;
823818
(operations[method as keyof Operations<TPathPattern, TOptions>] as Record<string, any>).action =
824819
async function _internalRouteActionHandler() {
820+
this.queryOperations = options.queryOperations;
821+
this.queryFields = options.queryFields;
822+
this.logger = logger;
823+
825824
if (options.authRequired || options.authOrAnonRequired) {
826-
const user = await api.authenticatedRoute.call(api, this.request);
825+
const user = await api.authenticatedRoute(this);
827826
this.user = user!;
828827
this.userId = this.user?._id;
829828
const authToken = this.request.headers.get('x-auth-token');
@@ -918,9 +917,7 @@ export class APIClass<
918917
connection: connection as unknown as IMethodConnection,
919918
}));
920919

921-
this.queryOperations = options.queryOperations;
922-
(this as any).queryFields = options.queryFields;
923-
this.parseJsonQuery = api.parseJsonQuery.bind(this as unknown as PartialThis);
920+
this.parseJsonQuery = () => api.parseJsonQuery(this);
924921

925922
result = (await DDP._CurrentInvocation.withValue(invocation as any, async () => originalAction.apply(this))) || api.success();
926923
} catch (e: any) {
@@ -972,9 +969,9 @@ export class APIClass<
972969
});
973970
}
974971

975-
protected async authenticatedRoute(req: Request): Promise<IUser | null> {
976-
const userId = req.headers.get('x-user-id');
977-
const userToken = req.headers.get('x-auth-token');
972+
protected async authenticatedRoute(routeContext: GenericRouteExecutionContext): Promise<IUser | null> {
973+
const userId = routeContext.request.headers.get('x-user-id');
974+
const userToken = routeContext.request.headers.get('x-auth-token');
978975

979976
if (userId && userToken) {
980977
return Users.findOne(
@@ -990,7 +987,7 @@ export class APIClass<
990987

991988
for (const method of this.authMethods) {
992989
// eslint-disable-next-line no-await-in-loop -- we want serial execution
993-
const user = await method(req);
990+
const user = await method(routeContext);
994991

995992
if (user) {
996993
return user;

apps/meteor/app/api/server/definition.ts

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -142,21 +142,16 @@ export type SharedOptions<TMethod extends string> = (
142142
};
143143
};
144144

145-
export type PartialThis = {
146-
user(bodyParams: Record<string, unknown>, user: any): Promise<any>;
147-
readonly request: Request & { query: Record<string, string> };
148-
readonly response: Response;
149-
readonly userId: string;
150-
readonly bodyParams: Record<string, unknown>;
151-
readonly path: string;
152-
readonly queryParams: Record<string, string>;
153-
readonly queryOperations?: string[];
154-
readonly queryFields?: string[];
155-
readonly logger: Logger;
156-
readonly route: string;
157-
};
145+
export type GenericRouteExecutionContext = ActionThis<any, any, any>;
146+
147+
export type RouteExecutionContext<TMethod extends Method, TPathPattern extends PathPattern, TOptions> = ActionThis<
148+
TMethod,
149+
TPathPattern,
150+
TOptions
151+
>;
158152

159153
export type ActionThis<TMethod extends Method, TPathPattern extends PathPattern, TOptions> = {
154+
readonly logger: Logger;
160155
route: string;
161156
readonly requestIp: string;
162157
urlParams: UrlParams<TPathPattern>;
@@ -183,6 +178,8 @@ export type ActionThis<TMethod extends Method, TPathPattern extends PathPattern,
183178
readonly request: Request;
184179

185180
readonly queryOperations: TOptions extends { queryOperations: infer T } ? T : never;
181+
readonly queryFields: TOptions extends { queryFields: infer T } ? T : never;
182+
186183
parseJsonQuery(): Promise<{
187184
sort: Record<string, 1 | -1>;
188185
/**

apps/meteor/app/api/server/helpers/parseJsonQuery.ts

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
import ejson from 'ejson';
22
import { Meteor } from 'meteor/meteor';
33

4+
import { isPlainObject } from '../../../../lib/utils/isPlainObject';
45
import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission';
56
import { apiDeprecationLogger } from '../../../lib/server/lib/deprecationWarningLogger';
67
import { API } from '../api';
7-
import type { PartialThis } from '../definition';
8+
import type { GenericRouteExecutionContext } from '../definition';
89
import { clean } from '../lib/cleanQuery';
910
import { isValidQuery } from '../lib/isValidQuery';
1011

@@ -13,7 +14,7 @@ const pathAllowConf = {
1314
'def': ['$or', '$and', '$regex'],
1415
};
1516

16-
export async function parseJsonQuery(api: PartialThis): Promise<{
17+
export async function parseJsonQuery(api: GenericRouteExecutionContext): Promise<{
1718
sort: Record<string, 1 | -1>;
1819
/**
1920
* @deprecated To access "fields" parameter, use ALLOW_UNSAFE_QUERY_AND_FIELDS_API_PARAMS environment variable.
@@ -24,10 +25,18 @@ export async function parseJsonQuery(api: PartialThis): Promise<{
2425
*/
2526
query: Record<string, unknown>;
2627
}> {
27-
const { userId, queryParams: params, logger, queryFields, queryOperations, response, route } = api;
28+
const { userId, response, route, logger } = api;
29+
30+
const params = isPlainObject(api.queryParams) ? api.queryParams : {};
31+
const queryFields = Array.isArray(api.queryFields) ? (api.queryFields as string[]) : [];
32+
const queryOperations = Array.isArray(api.queryOperations) ? (api.queryOperations as string[]) : [];
33+
34+
if (!userId) {
35+
throw new Meteor.Error('error-invalid-user', 'Invalid user');
36+
}
2837

2938
let sort;
30-
if (params.sort) {
39+
if (typeof params?.sort === 'string') {
3140
try {
3241
sort = JSON.parse(params.sort);
3342
Object.entries(sort).forEach(([key, value]) => {
@@ -50,7 +59,7 @@ export async function parseJsonQuery(api: PartialThis): Promise<{
5059
`The usage of the "${parameter}" parameter in endpoint "${endpoint}" breaks the security of the API and can lead to data exposure. It has been deprecated and will be removed in the version ${version}.`;
5160

5261
let fields: Record<string, 0 | 1> | undefined;
53-
if (params.fields && isUnsafeQueryParamsAllowed) {
62+
if (typeof params?.fields === 'string' && isUnsafeQueryParamsAllowed) {
5463
try {
5564
apiDeprecationLogger.parameter(route, 'fields', '8.0.0', response, messageGenerator);
5665
fields = JSON.parse(params.fields) as Record<string, 0 | 1>;
@@ -100,7 +109,7 @@ export async function parseJsonQuery(api: PartialThis): Promise<{
100109
}
101110

102111
let query: Record<string, any> = {};
103-
if (params.query && isUnsafeQueryParamsAllowed) {
112+
if (typeof params?.query === 'string' && isUnsafeQueryParamsAllowed) {
104113
apiDeprecationLogger.parameter(route, 'query', '8.0.0', response, messageGenerator);
105114
try {
106115
query = ejson.parse(params.query);

apps/meteor/app/integrations/server/api/api.ts

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,11 @@ import type { RateLimiterOptionsToCheck } from 'meteor/rate-limit';
99
import { WebApp } from 'meteor/webapp';
1010
import _ from 'underscore';
1111

12+
import { isPlainObject } from '../../../../lib/utils/isPlainObject';
1213
import { APIClass } from '../../../api/server/ApiClass';
1314
import type { RateLimiterOptions } from '../../../api/server/api';
1415
import { API, defaultRateLimiterOptions } from '../../../api/server/api';
15-
import type { FailureResult, PartialThis, SuccessResult, UnavailableResult } from '../../../api/server/definition';
16+
import type { FailureResult, GenericRouteExecutionContext, SuccessResult, UnavailableResult } from '../../../api/server/definition';
1617
import type { WebhookResponseItem } from '../../../lib/server/functions/processWebhookMessage';
1718
import { processWebhookMessage } from '../../../lib/server/functions/processWebhookMessage';
1819
import { settings } from '../../../settings/server';
@@ -39,11 +40,10 @@ type IntegrationOptions = {
3940
};
4041
};
4142

42-
type IntegrationThis = Omit<PartialThis, 'user'> & {
43+
type IntegrationThis = GenericRouteExecutionContext & {
4344
request: Request & {
4445
integration: IIncomingIntegration;
4546
};
46-
urlParams: Record<string, string>;
4747
user: IUser & { username: RequiredField<IUser, 'username'> };
4848
};
4949

@@ -138,8 +138,8 @@ async function executeIntegrationRest(
138138

139139
const scriptEngine = getEngine(this.request.integration);
140140

141-
let { bodyParams } = this;
142-
const separateResponse = this.bodyParams?.separateResponse === true;
141+
let bodyParams = isPlainObject(this.bodyParams) ? this.bodyParams : {};
142+
const separateResponse = bodyParams.separateResponse === true;
143143
let scriptResponse: Record<string, any> | undefined;
144144

145145
if (scriptEngine.integrationHasValidScript(this.request.integration) && this.request.body) {
@@ -152,21 +152,22 @@ async function executeIntegrationRest(
152152
const contentRaw = Buffer.concat(buffers).toString('utf8');
153153
const protocol = `${this.request.headers.get('x-forwarded-proto')}:` || 'http:';
154154
const url = new URL(this.request.url, `${protocol}//${this.request.headers.get('host')}`);
155+
const query = isPlainObject(this.queryParams) ? this.queryParams : {};
155156

156157
const request = {
157158
url: {
159+
query,
158160
hash: url.hash,
159161
search: url.search,
160-
query: this.queryParams,
161162
pathname: url.pathname,
162163
path: this.request.url,
163164
},
164165
url_raw: this.request.url,
165166
url_params: this.urlParams,
166-
content: this.bodyParams,
167+
content: bodyParams,
167168
content_raw: contentRaw,
168169
headers: Object.fromEntries(this.request.headers.entries()),
169-
body: this.bodyParams,
170+
body: bodyParams,
170171
user: {
171172
_id: this.user._id,
172173
name: this.user.name || '',
@@ -191,7 +192,7 @@ async function executeIntegrationRest(
191192
return API.v1.failure(result.error);
192193
}
193194

194-
bodyParams = result && result.content;
195+
bodyParams = result?.content;
195196

196197
if (!('separateResponse' in bodyParams)) {
197198
bodyParams.separateResponse = separateResponse;
@@ -312,8 +313,8 @@ function integrationInfoRest(): { statusCode: number; body: { success: boolean }
312313
}
313314

314315
class WebHookAPI extends APIClass<'/hooks'> {
315-
override async authenticatedRoute(this: IntegrationThis): Promise<IUser | null> {
316-
const { integrationId, token } = this.urlParams;
316+
override async authenticatedRoute(routeContext: IntegrationThis): Promise<IUser | null> {
317+
const { integrationId, token } = routeContext.urlParams;
317318
const integration = await Integrations.findOneByIdAndToken<IIncomingIntegration>(integrationId, decodeURIComponent(token));
318319

319320
if (!integration) {
@@ -322,9 +323,9 @@ class WebHookAPI extends APIClass<'/hooks'> {
322323
throw new Error('Invalid integration id or token provided.');
323324
}
324325

325-
this.request.integration = integration;
326+
routeContext.request.integration = integration;
326327

327-
return Users.findOneById(this.request.integration.userId);
328+
return Users.findOneById(routeContext.request.integration.userId);
328329
}
329330

330331
override shouldAddRateLimitToRoute(options: { rateLimiterOptions?: RateLimiterOptions | boolean }): boolean {

apps/meteor/app/oauth2-server-config/server/oauth/oauth2-server.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import type express from 'express';
55
import { Meteor } from 'meteor/meteor';
66
import { WebApp } from 'meteor/webapp';
77

8+
import { isPlainObject } from '../../../../lib/utils/isPlainObject';
89
import { OAuth2Server } from '../../../../server/oauth2-server/oauth';
910
import { API } from '../../../api/server';
1011

@@ -74,10 +75,9 @@ oauth2server.app.get('/oauth/userinfo', async (req: Request, res: Response) => {
7475
});
7576
});
7677

77-
API.v1.addAuthMethod((request: globalThis.Request) => {
78-
const url = new URL(request.url);
79-
const headers = Object.fromEntries(request.headers.entries());
80-
const query = Object.fromEntries(url.searchParams.entries());
78+
API.v1.addAuthMethod((routeContext) => {
79+
const headers = Object.fromEntries(routeContext.request.headers.entries());
80+
const query = (isPlainObject(routeContext.queryParams) ? routeContext.queryParams : {}) as Record<string, string | undefined>;
8181

8282
return oAuth2ServerAuth({ headers, query });
8383
});

apps/meteor/ee/server/apps/communication/endpoints/appLogsExportHandler.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { json2csv } from 'json-2-csv';
77
import type { AppsRestApi } from '../rest';
88
import { makeAppLogsQuery } from './lib/makeAppLogsQuery';
99
import { APIClass } from '../../../../../app/api/server/ApiClass';
10+
import type { GenericRouteExecutionContext } from '../../../../../app/api/server/definition';
1011

1112
const isErrorResponse = ajv.compile<{
1213
success: false;
@@ -25,18 +26,18 @@ const isErrorResponse = ajv.compile<{
2526
});
2627

2728
class ExportHandlerAPI extends APIClass {
28-
protected override async authenticatedRoute(req: Request): Promise<IUser | null> {
29-
const { rc_uid, rc_token } = parse(req.headers.get('cookie') || '');
29+
protected override async authenticatedRoute(routeContext: GenericRouteExecutionContext): Promise<IUser | null> {
30+
const { rc_uid, rc_token } = parse(routeContext.request.headers.get('cookie') || '');
3031

3132
if (rc_uid) {
32-
req.headers.set('x-user-id', rc_uid);
33+
routeContext.request.headers.set('x-user-id', rc_uid);
3334
}
3435

3536
if (rc_token) {
36-
req.headers.set('x-auth-token', rc_token);
37+
routeContext.request.headers.set('x-auth-token', rc_token);
3738
}
3839

39-
return super.authenticatedRoute(req);
40+
return super.authenticatedRoute(routeContext);
4041
}
4142
}
4243

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
export function isPlainObject(value: unknown) {
1+
export function isPlainObject(value: unknown): value is Record<string | symbol, unknown> {
22
return value !== null && typeof value === 'object' && !Array.isArray(value);
33
}

0 commit comments

Comments
 (0)