Skip to content

Commit eb99adf

Browse files
sdangolsdangol
committed
Addressed the remaining feedbacks
1 parent c06500b commit eb99adf

File tree

8 files changed

+225
-707
lines changed

8 files changed

+225
-707
lines changed

‎packages/event-handler/src/appsync-events/Router.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
import type { GenericLogger } from '@aws-lambda-powertools/commons/types';
22
import { isRecord } from '@aws-lambda-powertools/commons/typeutils';
3-
import {
4-
getStringFromEnv,
5-
isDevMode,
6-
} from '@aws-lambda-powertools/commons/utils/env';
3+
import { getStringFromEnv, isDevMode } from '@aws-lambda-powertools/commons/utils/env';
74
import type {
85
OnPublishHandler,
96
OnSubscribeHandler,
@@ -34,7 +31,7 @@ class Router {
3431
* Whether the router is running in development mode.
3532
*/
3633
protected readonly isDev: boolean = false;
37-
34+
3835
public constructor(options?: RouterOptions) {
3936
const alcLogLevel = getStringFromEnv({
4037
key: 'AWS_LAMBDA_LOG_LEVEL',

‎packages/event-handler/src/appsync-graphql/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
export { AppSyncGraphQLResolver } from './AppSyncGraphQLResolver.js';
22
export {
3-
InvalidBatchResponseException,
43
ResolverNotFoundException,
4+
InvalidBatchResponseException,
55
} from './errors.js';
66
export {
77
awsDate,

‎packages/event-handler/src/rest/index.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ export {
1919
ServiceUnavailableError,
2020
UnauthorizedError,
2121
} from './errors.js';
22-
export { cors } from './middleware/index.js';
2322
export { Router } from './Router.js';
2423
export {
2524
composeMiddleware,

‎packages/event-handler/src/rest/middleware/cors.ts‎

Lines changed: 51 additions & 146 deletions
Original file line numberDiff line numberDiff line change
@@ -1,145 +1,24 @@
11
import type {
22
CorsOptions,
3-
HandlerResponse,
43
Middleware,
5-
RequestContext,
64
} from '../../types/rest.js';
75
import {
86
DEFAULT_CORS_OPTIONS,
97
HttpErrorCodes,
108
HttpVerbs,
119
} from '../constants.js';
1210

13-
/**
14-
* Resolved CORS configuration with all defaults applied
15-
*/
16-
type ResolvedCorsConfig = {
17-
origin: CorsOptions['origin'];
18-
allowMethods: string[];
19-
allowHeaders: string[];
20-
exposeHeaders: string[];
21-
credentials: boolean;
22-
maxAge?: number;
23-
};
24-
25-
/**
26-
* Resolves and validates the CORS configuration
27-
*/
28-
const resolveConfiguration = (userOptions: CorsOptions): ResolvedCorsConfig => {
29-
return {
30-
origin: userOptions.origin ?? DEFAULT_CORS_OPTIONS.origin,
31-
allowMethods: userOptions.allowMethods ?? [
32-
...DEFAULT_CORS_OPTIONS.allowMethods,
33-
],
34-
allowHeaders: userOptions.allowHeaders ?? [
35-
...DEFAULT_CORS_OPTIONS.allowHeaders,
36-
],
37-
exposeHeaders: userOptions.exposeHeaders ?? [
38-
...DEFAULT_CORS_OPTIONS.exposeHeaders,
39-
],
40-
credentials: userOptions.credentials ?? DEFAULT_CORS_OPTIONS.credentials,
41-
maxAge: userOptions.maxAge,
42-
};
43-
};
44-
4511
/**
4612
* Resolves the origin value based on the configuration
4713
*/
4814
const resolveOrigin = (
49-
originConfig: CorsOptions['origin'],
50-
requestOrigin: string | null | undefined,
51-
reqCtx: RequestContext
15+
originConfig: NonNullable<CorsOptions['origin']>,
16+
requestOrigin: string | null,
5217
): string => {
53-
const origin = requestOrigin || undefined;
54-
55-
if (typeof originConfig === 'function') {
56-
const result = originConfig(origin, reqCtx);
57-
if (typeof result === 'boolean') {
58-
return result ? origin || '*' : '';
59-
}
60-
return result;
61-
}
62-
6318
if (Array.isArray(originConfig)) {
64-
return origin && originConfig.includes(origin) ? origin : '';
65-
}
66-
67-
if (typeof originConfig === 'string') {
68-
return originConfig;
69-
}
70-
71-
return DEFAULT_CORS_OPTIONS.origin;
72-
};
73-
74-
/**
75-
* Handles preflight OPTIONS requests
76-
*/
77-
const handlePreflight = (
78-
config: ResolvedCorsConfig,
79-
reqCtx: RequestContext
80-
): Response => {
81-
const { request, res } = reqCtx;
82-
const requestOrigin = request.headers.get('Origin');
83-
const resolvedOrigin = resolveOrigin(config.origin, requestOrigin, reqCtx);
84-
85-
// Mutate existing response headers
86-
if (resolvedOrigin) {
87-
res.headers.set('Access-Control-Allow-Origin', resolvedOrigin);
88-
}
89-
90-
if (config.allowMethods.length > 0) {
91-
res.headers.set(
92-
'Access-Control-Allow-Methods',
93-
config.allowMethods.join(', ')
94-
);
95-
}
96-
97-
if (config.allowHeaders.length > 0) {
98-
res.headers.set(
99-
'Access-Control-Allow-Headers',
100-
config.allowHeaders.join(', ')
101-
);
102-
}
103-
104-
if (config.credentials) {
105-
res.headers.set('Access-Control-Allow-Credentials', 'true');
106-
}
107-
108-
if (config.maxAge !== undefined) {
109-
res.headers.set('Access-Control-Max-Age', config.maxAge.toString());
110-
}
111-
112-
return new Response(null, {
113-
status: HttpErrorCodes.NO_CONTENT,
114-
headers: res.headers,
115-
});
116-
};
117-
118-
/**
119-
* Adds CORS headers to regular requests
120-
*/
121-
const addCorsHeaders = (
122-
config: ResolvedCorsConfig,
123-
reqCtx: RequestContext
124-
): void => {
125-
const { request, res } = reqCtx;
126-
const requestOrigin = request.headers.get('Origin');
127-
const resolvedOrigin = resolveOrigin(config.origin, requestOrigin, reqCtx);
128-
129-
if (resolvedOrigin) {
130-
res.headers.set('Access-Control-Allow-Origin', resolvedOrigin);
131-
}
132-
133-
if (config.exposeHeaders.length > 0) {
134-
res.headers.set(
135-
'Access-Control-Expose-Headers',
136-
config.exposeHeaders.join(', ')
137-
);
138-
}
139-
140-
if (config.credentials) {
141-
res.headers.set('Access-Control-Allow-Credentials', 'true');
19+
return requestOrigin && originConfig.includes(requestOrigin) ? requestOrigin : '';
14220
}
21+
return originConfig;
14322
};
14423

14524
/**
@@ -148,8 +27,11 @@ const addCorsHeaders = (
14827
*
14928
* @example
15029
* ```typescript
151-
* import { cors } from '@aws-lambda-powertools/event-handler/rest';
152-
*
30+
* import { Router } from '@aws-lambda-powertools/event-handler/experimental-rest';
31+
* import { cors } from '@aws-lambda-powertools/event-handler/experimental-rest/middleware';
32+
*
33+
* const app = new Router();
34+
*
15335
* // Use default configuration
15436
* app.use(cors());
15537
*
@@ -168,28 +50,51 @@ const addCorsHeaders = (
16850
* }
16951
* }));
17052
* ```
171-
* @param options - CORS configuration options
53+
*
54+
* @param options.origin - The origin to allow requests from
55+
* @param options.allowMethods - The HTTP methods to allow
56+
* @param options.allowHeaders - The headers to allow
57+
* @param options.exposeHeaders - The headers to expose
58+
* @param options.credentials - Whether to allow credentials
59+
* @param options.maxAge - The maximum age for the preflight response
17260
*/
173-
export const cors = (options: CorsOptions = {}): Middleware => {
174-
const config = resolveConfiguration(options);
175-
176-
return async (
177-
_params: Record<string, string>,
178-
reqCtx: RequestContext,
179-
next: () => Promise<HandlerResponse | void>
180-
) => {
181-
const { request } = reqCtx;
182-
const method = request.method.toUpperCase();
183-
61+
export const cors = (options?: CorsOptions): Middleware => {
62+
const config = {
63+
...DEFAULT_CORS_OPTIONS,
64+
...options
65+
};
66+
67+
return async (_params, reqCtx, next) => {
68+
const requestOrigin = reqCtx.request.headers.get('Origin');
69+
const resolvedOrigin = resolveOrigin(config.origin, requestOrigin);
70+
71+
reqCtx.res.headers.set('access-control-allow-origin', resolvedOrigin);
72+
if (resolvedOrigin !== '*') {
73+
reqCtx.res.headers.set('Vary', 'Origin');
74+
}
75+
config.allowMethods.forEach(method => {
76+
reqCtx.res.headers.append('access-control-allow-methods', method);
77+
});
78+
config.allowHeaders.forEach(header => {
79+
reqCtx.res.headers.append('access-control-allow-headers', header);
80+
});
81+
config.exposeHeaders.forEach(header => {
82+
reqCtx.res.headers.append('access-control-expose-headers', header);
83+
});
84+
reqCtx.res.headers.set('access-control-allow-credentials', config.credentials.toString());
85+
if (config.maxAge !== undefined) {
86+
reqCtx.res.headers.set('access-control-max-age', config.maxAge.toString());
87+
}
88+
18489
// Handle preflight OPTIONS request
185-
if (method === HttpVerbs.OPTIONS) {
186-
return handlePreflight(config, reqCtx);
90+
if (reqCtx.request.method === HttpVerbs.OPTIONS && reqCtx.request.headers.has('Access-Control-Request-Method')) {
91+
return new Response(null, {
92+
status: HttpErrorCodes.NO_CONTENT,
93+
headers: reqCtx.res.headers,
94+
});
18795
}
188-
189-
// Continue to next middleware/handler first
96+
97+
// Continue to next middleware/handler
19098
await next();
191-
192-
// Add CORS headers to the response after handler
193-
addCorsHeaders(config, reqCtx);
19499
};
195100
};

‎packages/event-handler/src/types/rest.ts‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -117,16 +117,10 @@ type ValidationResult = {
117117
type CorsOptions = {
118118
/**
119119
* The Access-Control-Allow-Origin header value.
120-
* Can be a string, array of strings, or a function that returns a string or boolean.
120+
* Can be a string, array of strings.
121121
* @default '*'
122122
*/
123-
origin?:
124-
| string
125-
| string[]
126-
| ((
127-
origin: string | undefined,
128-
reqCtx: RequestContext
129-
) => string | boolean);
123+
origin?: string | string[];
130124

131125
/**
132126
* The Access-Control-Allow-Methods header value.

‎packages/event-handler/tests/unit/rest/Router/cors-integration.test.ts‎

Lines changed: 0 additions & 93 deletions
This file was deleted.

‎packages/event-handler/tests/unit/rest/helpers.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,12 @@ import type { Middleware } from '../../../src/types/rest.js';
33

44
export const createTestEvent = (
55
path: string,
6-
httpMethod: string
6+
httpMethod: string,
7+
headers: Record<string, string> = {}
78
): APIGatewayProxyEvent => ({
89
path,
910
httpMethod,
10-
headers: {},
11+
headers,
1112
body: null,
1213
multiValueHeaders: {},
1314
isBase64Encoded: false,

0 commit comments

Comments
 (0)